diff mbox series

[03/20] security: add hfn override option in PDCP

Message ID 20190902121734.926-4-akhil.goyal@nxp.com (mailing list archive)
State Superseded, archived
Delegated to: akhil goyal
Headers
Series crypto/dpaaX_sec: Support Wireless algos |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail Compilation issues

Commit Message

Akhil Goyal Sept. 2, 2019, 12:17 p.m. UTC 
  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.goyal@nxp.com>
---
 lib/librte_security/rte_security.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Akhil Goyal Sept. 19, 2019, 3:31 p.m. UTC | #1 
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.goyal@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;
>  };
> 
>  /**
> --
> 2.17.1
Ananyev, Konstantin Sept. 24, 2019, 11:36 a.m. UTC | #2 
> > 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.goyal@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;
> >  };
> >
> >  /**
> > --

Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

> > 2.17.1
Anoob Joseph Sept. 25, 2019, 7:18 a.m. UTC | #3 
Hi Akhil,

Please see inline.

Thanks,
Anoob

> -----Original Message-----
> From: Akhil Goyal <akhil.goyal@nxp.com>
> Sent: Thursday, September 19, 2019 9:01 PM
> To: konstantin.ananyev@intel.com; Anoob Joseph <anoobj@marvell.com>;
> Radu Nicolau <radu.nicolau@intel.com>
> Cc: Hemant Agrawal <hemant.agrawal@nxp.com>; Vakul Garg
> <vakul.garg@nxp.com>; dev@dpdk.org; Akhil Goyal <akhil.goyal@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.goyal@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
Akhil Goyal Sept. 27, 2019, 3:06 p.m. UTC | #4 
Hi Anoob,

Thanks for review.
> > > @@ -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.

Will add comments here.
        /** 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 is set, user is expected to set the
         * per packet HFN in place of IV. PMDs will extract the HFN
         * and perform operations accordingly.
         */
But using a different name may not be useful. Here we want to specify that
HFN can be overridden from the per packet value.
Now the usage is explained in the comments. I believe hfn_ovrd is enough to
explain the intent. Though not a very strong opinion on this.

Will send the v2 shortly.

> 
> Otherwise, I don't see any issues with the approach.
> 
> > >  };
> > >
> > >  /**
> > > --
> > > 2.17.1
diff mbox series

Patch

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;
 };
 
 /**