Hi Akhil,
Please see inline.
Thanks,
Anoob
> -----Original Message-----
> From: Akhil Goyal <akhil.go...@nxp.com>
> Sent: Thursday, September 19, 2019 9:01 PM
> To: konstantin.anan...@intel.com; Anoob Joseph <ano...@marvell.com>;
> Radu Nicolau <radu.nico...@intel.com>
> Cc: Hemant Agrawal <hemant.agra...@nxp.com>; Vakul Garg
> <vakul.g...@nxp.com>; dev@dpdk.org; Akhil Goyal <akhil.go...@nxp.com>
> Subject: [EXT] RE: [PATCH 03/20] security: add hfn override option in PDCP
>
> External Email
>
> ----------------------------------------------------------------------
> Hi Konstantin/Anoob/Radu,
>
> Any comments on this patch.
>
> Regards,
> Akhil
> >
> > HFN can be given as a per packet value also.
> > As we do not have IV in case of PDCP, and HFN is used to generate IV.
> > IV field can be used to get the per packet HFN while enq/deq If
> > hfn_ovrd field in pdcp_xform is set, application is expected to set
> > the per packet HFN in place of IV. Driver will extract the HFN and
> > perform operations accordingly.
> >
> > Signed-off-by: Akhil Goyal <akhil.go...@nxp.com>
> > ---
> > lib/librte_security/rte_security.h | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/librte_security/rte_security.h
> > b/lib/librte_security/rte_security.h
> > index 96806e3a2..4452545fe 100644
> > --- a/lib/librte_security/rte_security.h
> > +++ b/lib/librte_security/rte_security.h
> > @@ -1,5 +1,5 @@
> > /* SPDX-License-Identifier: BSD-3-Clause
> > - * Copyright 2017 NXP.
> > + * Copyright 2017,2019 NXP
> > * Copyright(c) 2017 Intel Corporation.
> > */
> >
> > @@ -270,6 +270,8 @@ struct rte_security_pdcp_xform {
> > uint32_t hfn;
> > /** HFN Threshold for key renegotiation */
> > uint32_t hfn_threshold;
> > + /** Enable per packet HFN override */
> > + uint32_t hfn_ovrd;
[Anoob] I think you should document the fact that IV field will be used for
HFN. Your patch description accurately describes the procedure but the above
comment fails to capture it. Also I would suggest renaming "hfn_ovrd" to
something else to make it obvious that IV field is being used. Something like,
use_iv_for_hfn or something.
Otherwise, I don't see any issues with the approach.
> > };
> >
> > /**
> > --
> > 2.17.1
