Message ID | 20200903160652.31654-3-akhil.goyal@nxp.com |
---|---|
State | Superseded, archived |
Delegated to: | akhil goyal |
Headers | show |
Series |
|
Related | show |
Context | Check | Description |
---|---|---|
ci/checkpatch | success | coding style OK |
Hi Akhil > -----Original Message----- > From: akhil.goyal@nxp.com <akhil.goyal@nxp.com> <snip> > diff --git a/doc/guides/prog_guide/rte_security.rst > b/doc/guides/prog_guide/rte_security.rst > index 127da2e4f..ab535d1cd 100644 > --- a/doc/guides/prog_guide/rte_security.rst > +++ b/doc/guides/prog_guide/rte_security.rst > @@ -1,5 +1,5 @@ <snip> > @@ -693,6 +693,23 @@ PDCP related configuration parameters are defined > in ``rte_security_pdcp_xform`` > uint32_t hfn; > /** HFN Threshold for key renegotiation */ > uint32_t hfn_threshold; > + /** 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. > + */ > + uint8_t hfn_ovrd; > + /** In case of 5G NR, a new protocol(SDAP) header may be set > + * inside PDCP payload which should be authenticated but not > + * encrypted. Hence, driver should be notified if SDAP is > + * enabled or not, so that SDAP header is not encrypted. > + */ > + uint8_t sdap_enabled; > + /** Reserved for future */ > + uint16_t reserved; > }; [DC] Should we consider removing the API code out of the security documentation? It's a direct copy of the API code itself, and just means 2 files need to be updated for every API change. And as with 'hfn_ovrd', sometimes it's forgotten. From maintainability point of view, it might be better just remove it. > > DOCSIS related configuration parameters are defined in > ``rte_security_docsis_xform`` diff --git a/lib/librte_security/rte_security.h > b/lib/librte_security/rte_security.h > index 16839e539..48b377b20 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,2019 NXP > + * Copyright 2017,2019-2020 NXP > * Copyright(c) 2017-2020 Intel Corporation. > */ > > @@ -290,7 +290,15 @@ struct rte_security_pdcp_xform { > * per packet HFN in place of IV. PMDs will extract the HFN > * and perform operations accordingly. > */ > - uint32_t hfn_ovrd; > + uint8_t hfn_ovrd; > + /** In case of 5G NR, a new protocol(SDAP) header may be set [DC] Very minor thing... add space between 'protocol' and '(SDAP)' in the comment block. And same comment for the documentation if you choose to keep the API code blocks there too. > + * inside PDCP payload which should be authenticated but not > + * encrypted. Hence, driver should be notified if SDAP is > + * enabled or not, so that SDAP header is not encrypted. > + */ > + uint8_t sdap_enabled; > + /** Reserved for future */ > + uint16_t reserved; > }; > > /** DOCSIS direction */ > -- > 2.17.1
Hi David, > Hi Akhil > > > > @@ -693,6 +693,23 @@ PDCP related configuration parameters are defined > > in ``rte_security_pdcp_xform`` > > uint32_t hfn; > > /** HFN Threshold for key renegotiation */ > > uint32_t hfn_threshold; > > + /** 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. > > + */ > > + uint8_t hfn_ovrd; > > + /** In case of 5G NR, a new protocol(SDAP) header may be set > > + * inside PDCP payload which should be authenticated but not > > + * encrypted. Hence, driver should be notified if SDAP is > > + * enabled or not, so that SDAP header is not encrypted. > > + */ > > + uint8_t sdap_enabled; > > + /** Reserved for future */ > > + uint16_t reserved; > > }; > > [DC] Should we consider removing the API code out of the security > documentation? > It's a direct copy of the API code itself, and just means 2 files need to be updated > for every API change. > And as with 'hfn_ovrd', sometimes it's forgotten. > From maintainability point of view, it might be better just remove it. Yes we can remove it. I will remove it in a separate patch. > > > > > DOCSIS related configuration parameters are defined in > > ``rte_security_docsis_xform`` diff --git a/lib/librte_security/rte_security.h > > b/lib/librte_security/rte_security.h > > index 16839e539..48b377b20 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,2019 NXP > > + * Copyright 2017,2019-2020 NXP > > * Copyright(c) 2017-2020 Intel Corporation. > > */ > > > > @@ -290,7 +290,15 @@ struct rte_security_pdcp_xform { > > * per packet HFN in place of IV. PMDs will extract the HFN > > * and perform operations accordingly. > > */ > > - uint32_t hfn_ovrd; > > + uint8_t hfn_ovrd; > > + /** In case of 5G NR, a new protocol(SDAP) header may be set > > [DC] Very minor thing... add space between 'protocol' and '(SDAP)' in the > comment block. Sure. Will add it. > Adding Techboard for request for approval of the change in the xform structure to Add sdap support. > And same comment for the documentation if you choose to keep the API code > blocks there too. > > > + * inside PDCP payload which should be authenticated but not > > + * encrypted. Hence, driver should be notified if SDAP is > > + * enabled or not, so that SDAP header is not encrypted. > > + */ > > + uint8_t sdap_enabled; > > + /** Reserved for future */ > > + uint16_t reserved; > > }; > > > > /** DOCSIS direction */ > > -- > > 2.17.1
diff --git a/doc/guides/prog_guide/rte_security.rst b/doc/guides/prog_guide/rte_security.rst index 127da2e4f..ab535d1cd 100644 --- a/doc/guides/prog_guide/rte_security.rst +++ b/doc/guides/prog_guide/rte_security.rst @@ -1,5 +1,5 @@ .. SPDX-License-Identifier: BSD-3-Clause - Copyright 2017 NXP + Copyright 2017,2020 NXP @@ -693,6 +693,23 @@ PDCP related configuration parameters are defined in ``rte_security_pdcp_xform`` uint32_t hfn; /** HFN Threshold for key renegotiation */ uint32_t hfn_threshold; + /** 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. + */ + uint8_t hfn_ovrd; + /** In case of 5G NR, a new protocol(SDAP) header may be set + * inside PDCP payload which should be authenticated but not + * encrypted. Hence, driver should be notified if SDAP is + * enabled or not, so that SDAP header is not encrypted. + */ + uint8_t sdap_enabled; + /** Reserved for future */ + uint16_t reserved; }; DOCSIS related configuration parameters are defined in ``rte_security_docsis_xform`` diff --git a/lib/librte_security/rte_security.h b/lib/librte_security/rte_security.h index 16839e539..48b377b20 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,2019 NXP + * Copyright 2017,2019-2020 NXP * Copyright(c) 2017-2020 Intel Corporation. */ @@ -290,7 +290,15 @@ struct rte_security_pdcp_xform { * per packet HFN in place of IV. PMDs will extract the HFN * and perform operations accordingly. */ - uint32_t hfn_ovrd; + uint8_t hfn_ovrd; + /** In case of 5G NR, a new protocol(SDAP) header may be set + * inside PDCP payload which should be authenticated but not + * encrypted. Hence, driver should be notified if SDAP is + * enabled or not, so that SDAP header is not encrypted. + */ + uint8_t sdap_enabled; + /** Reserved for future */ + uint16_t reserved; }; /** DOCSIS direction */