[1/3] security: introduce out of place support for inline ingress
Checks
Commit Message
Similar to out of place(OOP) processing support that exists for
Lookaside crypto/security sessions, Inline ingress security
sessions may also need out of place processing in usecases
where original encrypted packet needs to be retained for post
processing. So for NIC's which have such a kind of HW support,
a new SA option is provided to indicate whether OOP needs to
be enabled on that Inline ingress security session or not.
Since for inline ingress sessions, packet is not received by
CPU until the processing is done, we can only have per-SA
option and not per-packet option like Lookaside sessions.
Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
---
devtools/libabigail.abignore | 4 +++
lib/security/rte_security.c | 17 +++++++++++++
lib/security/rte_security.h | 39 +++++++++++++++++++++++++++++-
lib/security/rte_security_driver.h | 8 ++++++
lib/security/version.map | 2 ++
5 files changed, 69 insertions(+), 1 deletion(-)
Comments
On Tue, 11 Apr 2023 15:34:07 +0530
Nithin Dabilpuram <ndabilpuram@marvell.com> wrote:
> diff --git a/lib/security/rte_security.h b/lib/security/rte_security.h
> index 4bacf9fcd9..866cd4e8ee 100644
> --- a/lib/security/rte_security.h
> +++ b/lib/security/rte_security.h
> @@ -275,6 +275,17 @@ struct rte_security_ipsec_sa_options {
> */
> uint32_t ip_reassembly_en : 1;
>
> + /** Enable out of place processing on inline inbound packets.
> + *
> + * * 1: Enable driver to perform Out-of-place(OOP) processing for this inline
> + * inbound SA if supported by driver. PMD need to register mbuf
> + * dynamic field using rte_security_oop_dynfield_register()
> + * and security session creation would fail if dynfield is not
> + * registered successfully.
> + * * 0: Disable OOP processing for this session (default).
> + */
> + uint32_t ingress_oop : 1;
> +
> /** Reserved bit fields for future extension
> *
> * User should ensure reserved_opts is cleared as it may change in
> @@ -282,7 +293,7 @@ struct rte_security_ipsec_sa_options {
> *
> * Note: Reduce number of bits in reserved_opts for every new option.
> */
> - uint32_t reserved_opts : 17;
> + uint32_t reserved_opts : 16;
> };
NAK
Let me repeat the reserved bit rant. YAGNI
Reserved space is not usable without ABI breakage unless the existing
code enforces that reserved space has to be zero.
Just saying "User should ensure reserved_opts is cleared" is not enough.
On Tue, Apr 11, 2023 at 11:36 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Tue, 11 Apr 2023 15:34:07 +0530
> Nithin Dabilpuram <ndabilpuram@marvell.com> wrote:
>
> > diff --git a/lib/security/rte_security.h b/lib/security/rte_security.h
> > index 4bacf9fcd9..866cd4e8ee 100644
> > --- a/lib/security/rte_security.h
> > +++ b/lib/security/rte_security.h
> > @@ -275,6 +275,17 @@ struct rte_security_ipsec_sa_options {
> > */
> > uint32_t ip_reassembly_en : 1;
> >
> > + /** Enable out of place processing on inline inbound packets.
> > + *
> > + * * 1: Enable driver to perform Out-of-place(OOP) processing for this inline
> > + * inbound SA if supported by driver. PMD need to register mbuf
> > + * dynamic field using rte_security_oop_dynfield_register()
> > + * and security session creation would fail if dynfield is not
> > + * registered successfully.
> > + * * 0: Disable OOP processing for this session (default).
> > + */
> > + uint32_t ingress_oop : 1;
> > +
> > /** Reserved bit fields for future extension
> > *
> > * User should ensure reserved_opts is cleared as it may change in
> > @@ -282,7 +293,7 @@ struct rte_security_ipsec_sa_options {
> > *
> > * Note: Reduce number of bits in reserved_opts for every new option.
> > */
> > - uint32_t reserved_opts : 17;
> > + uint32_t reserved_opts : 16;
> > };
>
> NAK
> Let me repeat the reserved bit rant. YAGNI
>
> Reserved space is not usable without ABI breakage unless the existing
> code enforces that reserved space has to be zero.
>
> Just saying "User should ensure reserved_opts is cleared" is not enough.
Yes. I think, we need to enforce to have _init functions for the
structures which is using reserved filed.
On the same note on YAGNI, I am wondering why NOT introduce
RTE_NEXT_ABI marco kind of scheme to compile out ABI breaking changes.
By keeping RTE_NEXT_ABI disable by default, enable explicitly if user
wants it to avoid waiting for one year any ABI breaking changes.
There are a lot of "fixed appliance" customers (not OS distribution
driven customer) they are willing to recompile DPDK for new feature.
What we are loosing with this scheme?
>
>
18/04/2023 10:33, Jerin Jacob:
> On Tue, Apr 11, 2023 at 11:36 PM Stephen Hemminger
> <stephen@networkplumber.org> wrote:
> >
> > On Tue, 11 Apr 2023 15:34:07 +0530
> > Nithin Dabilpuram <ndabilpuram@marvell.com> wrote:
> >
> > > diff --git a/lib/security/rte_security.h b/lib/security/rte_security.h
> > > index 4bacf9fcd9..866cd4e8ee 100644
> > > --- a/lib/security/rte_security.h
> > > +++ b/lib/security/rte_security.h
> > > @@ -275,6 +275,17 @@ struct rte_security_ipsec_sa_options {
> > > */
> > > uint32_t ip_reassembly_en : 1;
> > >
> > > + /** Enable out of place processing on inline inbound packets.
> > > + *
> > > + * * 1: Enable driver to perform Out-of-place(OOP) processing for this inline
> > > + * inbound SA if supported by driver. PMD need to register mbuf
> > > + * dynamic field using rte_security_oop_dynfield_register()
> > > + * and security session creation would fail if dynfield is not
> > > + * registered successfully.
> > > + * * 0: Disable OOP processing for this session (default).
> > > + */
> > > + uint32_t ingress_oop : 1;
> > > +
> > > /** Reserved bit fields for future extension
> > > *
> > > * User should ensure reserved_opts is cleared as it may change in
> > > @@ -282,7 +293,7 @@ struct rte_security_ipsec_sa_options {
> > > *
> > > * Note: Reduce number of bits in reserved_opts for every new option.
> > > */
> > > - uint32_t reserved_opts : 17;
> > > + uint32_t reserved_opts : 16;
> > > };
> >
> > NAK
> > Let me repeat the reserved bit rant. YAGNI
> >
> > Reserved space is not usable without ABI breakage unless the existing
> > code enforces that reserved space has to be zero.
> >
> > Just saying "User should ensure reserved_opts is cleared" is not enough.
>
> Yes. I think, we need to enforce to have _init functions for the
> structures which is using reserved filed.
>
> On the same note on YAGNI, I am wondering why NOT introduce
> RTE_NEXT_ABI marco kind of scheme to compile out ABI breaking changes.
> By keeping RTE_NEXT_ABI disable by default, enable explicitly if user
> wants it to avoid waiting for one year any ABI breaking changes.
> There are a lot of "fixed appliance" customers (not OS distribution
> driven customer) they are willing to recompile DPDK for new feature.
> What we are loosing with this scheme?
RTE_NEXT_ABI is described in the ABI policy.
We are not doing it currently, but I think we could
when it is not too much complicate in the code.
The only problems I see are:
- more #ifdef clutter
- 2 binary versions to test
- CI and checks must handle RTE_NEXT_ABI version
On Tue, Apr 25, 2023 at 4:11 AM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 18/04/2023 10:33, Jerin Jacob:
> > On Tue, Apr 11, 2023 at 11:36 PM Stephen Hemminger
> > <stephen@networkplumber.org> wrote:
> > >
> > > On Tue, 11 Apr 2023 15:34:07 +0530
> > > Nithin Dabilpuram <ndabilpuram@marvell.com> wrote:
> > >
> > > > diff --git a/lib/security/rte_security.h b/lib/security/rte_security.h
> > > > index 4bacf9fcd9..866cd4e8ee 100644
> > > > --- a/lib/security/rte_security.h
> > > > +++ b/lib/security/rte_security.h
> > > > @@ -275,6 +275,17 @@ struct rte_security_ipsec_sa_options {
> > > > */
> > > > uint32_t ip_reassembly_en : 1;
> > > >
> > > > + /** Enable out of place processing on inline inbound packets.
> > > > + *
> > > > + * * 1: Enable driver to perform Out-of-place(OOP) processing for this inline
> > > > + * inbound SA if supported by driver. PMD need to register mbuf
> > > > + * dynamic field using rte_security_oop_dynfield_register()
> > > > + * and security session creation would fail if dynfield is not
> > > > + * registered successfully.
> > > > + * * 0: Disable OOP processing for this session (default).
> > > > + */
> > > > + uint32_t ingress_oop : 1;
> > > > +
> > > > /** Reserved bit fields for future extension
> > > > *
> > > > * User should ensure reserved_opts is cleared as it may change in
> > > > @@ -282,7 +293,7 @@ struct rte_security_ipsec_sa_options {
> > > > *
> > > > * Note: Reduce number of bits in reserved_opts for every new option.
> > > > */
> > > > - uint32_t reserved_opts : 17;
> > > > + uint32_t reserved_opts : 16;
> > > > };
> > >
> > > NAK
> > > Let me repeat the reserved bit rant. YAGNI
> > >
> > > Reserved space is not usable without ABI breakage unless the existing
> > > code enforces that reserved space has to be zero.
> > >
> > > Just saying "User should ensure reserved_opts is cleared" is not enough.
> >
> > Yes. I think, we need to enforce to have _init functions for the
> > structures which is using reserved filed.
> >
> > On the same note on YAGNI, I am wondering why NOT introduce
> > RTE_NEXT_ABI marco kind of scheme to compile out ABI breaking changes.
> > By keeping RTE_NEXT_ABI disable by default, enable explicitly if user
> > wants it to avoid waiting for one year any ABI breaking changes.
> > There are a lot of "fixed appliance" customers (not OS distribution
> > driven customer) they are willing to recompile DPDK for new feature.
> > What we are loosing with this scheme?
>
> RTE_NEXT_ABI is described in the ABI policy.
> We are not doing it currently, but I think we could
> when it is not too much complicate in the code.
>
> The only problems I see are:
> - more #ifdef clutter
> - 2 binary versions to test
> - CI and checks must handle RTE_NEXT_ABI version
I think, we have two buckets of ABI breakages via RTE_NEXT_ABI
1) Changes that introduces compilation failures like adding new
argument to API or change API name etc
2) Structure size change which won't affect the compilation but breaks
the ABI for shared library usage.
I think, (1) is very distributive, and I don't see recently such
changes. I think, we should avoid (1) for non XX.11 releases.(or two
or three-year cycles if we decide that path)
The (2) comes are very common due to the fact HW features are
evolving. I think, to address the (2), we have two options
a) Have reserved fields and have _init() function to initialize the structures
b) Follow YAGNI style and introduce RTE_NEXT_ABI for structure size change.
The above concerns[1] can greatly reduce with option b OR option a.
[1]
1) more #ifdef clutter
For option (a) this is not needed or option (b) the clutter will be
limited, it will be around structure which add the new filed and
around the FULL block where new functions are added (not inside the
functions)
2) 2 binary versions to test
For option (a) this is not needed, for option (b) it is limited as for
new features only one needs to test another binary (rather than NOT
adding a new feature).
3) CI and checks must handle RTE_NEXT_ABI version
I think, it is cheap to add this, at least for compilation test.
IMO, We need to change the API break release to 3 year kind of time
frame to have very good end user experience
and allow ABI related change to get in every release and force
_rebuild_ shared objects in major LTS release.
I think, in this major LTS version(23.11) if we can decide (a) vs (b)
then we can align the code accordingly . e.s.p for (a) we need to add
_init() functions.
Thoughts?
> > > > > + */
> > > > > + uint32_t ingress_oop : 1;
> > > > > +
> > > > > /** Reserved bit fields for future extension
> > > > > *
> > > > > * User should ensure reserved_opts is cleared as it may change in
> > > > > @@ -282,7 +293,7 @@ struct rte_security_ipsec_sa_options {
> > > > > *
> > > > > * Note: Reduce number of bits in reserved_opts for every new option.
> > > > > */
> > > > > - uint32_t reserved_opts : 17;
> > > > > + uint32_t reserved_opts : 16;
> > > > > };
> > > >
> > > > NAK
> > > > Let me repeat the reserved bit rant. YAGNI
> > > >
> > > > Reserved space is not usable without ABI breakage unless the existing
> > > > code enforces that reserved space has to be zero.
> > > >
> > > > Just saying "User should ensure reserved_opts is cleared" is not enough.
> > >
> > > Yes. I think, we need to enforce to have _init functions for the
> > > structures which is using reserved filed.
> > >
> > > On the same note on YAGNI, I am wondering why NOT introduce
> > > RTE_NEXT_ABI marco kind of scheme to compile out ABI breaking changes.
> > > By keeping RTE_NEXT_ABI disable by default, enable explicitly if user
> > > wants it to avoid waiting for one year any ABI breaking changes.
> > > There are a lot of "fixed appliance" customers (not OS distribution
> > > driven customer) they are willing to recompile DPDK for new feature.
> > > What we are loosing with this scheme?
> >
> > RTE_NEXT_ABI is described in the ABI policy.
> > We are not doing it currently, but I think we could
> > when it is not too much complicate in the code.
> >
> > The only problems I see are:
> > - more #ifdef clutter
> > - 2 binary versions to test
> > - CI and checks must handle RTE_NEXT_ABI version
>
> I think, we have two buckets of ABI breakages via RTE_NEXT_ABI
>
> 1) Changes that introduces compilation failures like adding new
> argument to API or change API name etc
> 2) Structure size change which won't affect the compilation but breaks
> the ABI for shared library usage.
>
> I think, (1) is very distributive, and I don't see recently such
> changes. I think, we should avoid (1) for non XX.11 releases.(or two
> or three-year cycles if we decide that path)
>
> The (2) comes are very common due to the fact HW features are
> evolving. I think, to address the (2), we have two options
> a) Have reserved fields and have _init() function to initialize the structures
> b) Follow YAGNI style and introduce RTE_NEXT_ABI for structure size change.
>
> The above concerns[1] can greatly reduce with option b OR option a.
>
> [1]
> 1) more #ifdef clutter
> For option (a) this is not needed or option (b) the clutter will be
> limited, it will be around structure which add the new filed and
> around the FULL block where new functions are added (not inside the
> functions)
>
> 2) 2 binary versions to test
> For option (a) this is not needed, for option (b) it is limited as for
> new features only one needs to test another binary (rather than NOT
> adding a new feature).
>
> 3) CI and checks must handle RTE_NEXT_ABI version
>
> I think, it is cheap to add this, at least for compilation test.
>
> IMO, We need to change the API break release to 3 year kind of time
> frame to have very good end user experience
> and allow ABI related change to get in every release and force
> _rebuild_ shared objects in major LTS release.
>
> I think, in this major LTS version(23.11) if we can decide (a) vs (b)
> then we can align the code accordingly . e.s.p for (a) we need to add
> _init() functions.
>
> Thoughts?
Not much input from mailing list. Can we discuss this next TB meeting?
Especially how to align with next LTS release on
-YAGNI vs reserved fileds with init()
-What it takes to Extend the API breaking release more than a year as
first step.
30/05/2023 11:23, Jerin Jacob:
> > > > > > + */
> > > > > > + uint32_t ingress_oop : 1;
> > > > > > +
> > > > > > /** Reserved bit fields for future extension
> > > > > > *
> > > > > > * User should ensure reserved_opts is cleared as it may change in
> > > > > > @@ -282,7 +293,7 @@ struct rte_security_ipsec_sa_options {
> > > > > > *
> > > > > > * Note: Reduce number of bits in reserved_opts for every new option.
> > > > > > */
> > > > > > - uint32_t reserved_opts : 17;
> > > > > > + uint32_t reserved_opts : 16;
> > > > > > };
> > > > >
> > > > > NAK
> > > > > Let me repeat the reserved bit rant. YAGNI
> > > > >
> > > > > Reserved space is not usable without ABI breakage unless the existing
> > > > > code enforces that reserved space has to be zero.
> > > > >
> > > > > Just saying "User should ensure reserved_opts is cleared" is not enough.
> > > >
> > > > Yes. I think, we need to enforce to have _init functions for the
> > > > structures which is using reserved filed.
> > > >
> > > > On the same note on YAGNI, I am wondering why NOT introduce
> > > > RTE_NEXT_ABI marco kind of scheme to compile out ABI breaking changes.
> > > > By keeping RTE_NEXT_ABI disable by default, enable explicitly if user
> > > > wants it to avoid waiting for one year any ABI breaking changes.
> > > > There are a lot of "fixed appliance" customers (not OS distribution
> > > > driven customer) they are willing to recompile DPDK for new feature.
> > > > What we are loosing with this scheme?
> > >
> > > RTE_NEXT_ABI is described in the ABI policy.
> > > We are not doing it currently, but I think we could
> > > when it is not too much complicate in the code.
> > >
> > > The only problems I see are:
> > > - more #ifdef clutter
> > > - 2 binary versions to test
> > > - CI and checks must handle RTE_NEXT_ABI version
> >
> > I think, we have two buckets of ABI breakages via RTE_NEXT_ABI
> >
> > 1) Changes that introduces compilation failures like adding new
> > argument to API or change API name etc
> > 2) Structure size change which won't affect the compilation but breaks
> > the ABI for shared library usage.
> >
> > I think, (1) is very distributive, and I don't see recently such
> > changes. I think, we should avoid (1) for non XX.11 releases.(or two
> > or three-year cycles if we decide that path)
> >
> > The (2) comes are very common due to the fact HW features are
> > evolving. I think, to address the (2), we have two options
> > a) Have reserved fields and have _init() function to initialize the structures
> > b) Follow YAGNI style and introduce RTE_NEXT_ABI for structure size change.
> >
> > The above concerns[1] can greatly reduce with option b OR option a.
> >
> > [1]
> > 1) more #ifdef clutter
> > For option (a) this is not needed or option (b) the clutter will be
> > limited, it will be around structure which add the new filed and
> > around the FULL block where new functions are added (not inside the
> > functions)
> >
> > 2) 2 binary versions to test
> > For option (a) this is not needed, for option (b) it is limited as for
> > new features only one needs to test another binary (rather than NOT
> > adding a new feature).
> >
> > 3) CI and checks must handle RTE_NEXT_ABI version
> >
> > I think, it is cheap to add this, at least for compilation test.
> >
> > IMO, We need to change the API break release to 3 year kind of time
> > frame to have very good end user experience
> > and allow ABI related change to get in every release and force
> > _rebuild_ shared objects in major LTS release.
> >
> > I think, in this major LTS version(23.11) if we can decide (a) vs (b)
> > then we can align the code accordingly . e.s.p for (a) we need to add
> > _init() functions.
> >
> > Thoughts?
>
> Not much input from mailing list. Can we discuss this next TB meeting?
> Especially how to align with next LTS release on
> -YAGNI vs reserved fileds with init()
> -What it takes to Extend the API breaking release more than a year as
> first step.
Yes I agree it should be discussed interactively in techboard meeting.
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Tuesday, 30 May 2023 15.52
>
> 30/05/2023 11:23, Jerin Jacob:
> > > > > > > + */
> > > > > > > + uint32_t ingress_oop : 1;
> > > > > > > +
> > > > > > > /** Reserved bit fields for future extension
> > > > > > > *
> > > > > > > * User should ensure reserved_opts is cleared as it may
> change in
> > > > > > > @@ -282,7 +293,7 @@ struct rte_security_ipsec_sa_options {
> > > > > > > *
> > > > > > > * Note: Reduce number of bits in reserved_opts for every
> new option.
> > > > > > > */
> > > > > > > - uint32_t reserved_opts : 17;
> > > > > > > + uint32_t reserved_opts : 16;
> > > > > > > };
> > > > > >
> > > > > > NAK
> > > > > > Let me repeat the reserved bit rant. YAGNI
> > > > > >
> > > > > > Reserved space is not usable without ABI breakage unless the
> existing
> > > > > > code enforces that reserved space has to be zero.
> > > > > >
> > > > > > Just saying "User should ensure reserved_opts is cleared" is not
> enough.
> > > > >
> > > > > Yes. I think, we need to enforce to have _init functions for the
> > > > > structures which is using reserved filed.
> > > > >
> > > > > On the same note on YAGNI, I am wondering why NOT introduce
> > > > > RTE_NEXT_ABI marco kind of scheme to compile out ABI breaking changes.
> > > > > By keeping RTE_NEXT_ABI disable by default, enable explicitly if user
> > > > > wants it to avoid waiting for one year any ABI breaking changes.
> > > > > There are a lot of "fixed appliance" customers (not OS distribution
> > > > > driven customer) they are willing to recompile DPDK for new feature.
> > > > > What we are loosing with this scheme?
> > > >
> > > > RTE_NEXT_ABI is described in the ABI policy.
> > > > We are not doing it currently, but I think we could
> > > > when it is not too much complicate in the code.
> > > >
> > > > The only problems I see are:
> > > > - more #ifdef clutter
> > > > - 2 binary versions to test
> > > > - CI and checks must handle RTE_NEXT_ABI version
> > >
> > > I think, we have two buckets of ABI breakages via RTE_NEXT_ABI
> > >
> > > 1) Changes that introduces compilation failures like adding new
> > > argument to API or change API name etc
> > > 2) Structure size change which won't affect the compilation but breaks
> > > the ABI for shared library usage.
> > >
> > > I think, (1) is very distributive, and I don't see recently such
> > > changes. I think, we should avoid (1) for non XX.11 releases.(or two
> > > or three-year cycles if we decide that path)
> > >
> > > The (2) comes are very common due to the fact HW features are
> > > evolving. I think, to address the (2), we have two options
> > > a) Have reserved fields and have _init() function to initialize the
> structures
High probability that (a) is not going to work: There will not be enough reserved fields, and/or they will be in the wrong places in the structures.
Also, (a) is really intrusive on existing applications: They MUST be rewritten to call the _init() function instead of using pre-initialized structures, or the library will behave unexpectedly. Extreme example, to prove my point: A new field "allow_ingress" (don't drop all packets on ingress) is introduced, and _init() sets it to true. If the application doesn't call _init(), it will not receive any packets.
Are _init() functions required on all structures, or only some? And how about structures containing other structures?
How does the application developer know which structures have _init() functions, and which do not?
<irony>
We could also switch to C++, where the _init() function comes native in the form of an object constructor.
</irony>
> > > b) Follow YAGNI style and introduce RTE_NEXT_ABI for structure size
> change.
+1 for (b), because (a) is too problematic.
> > >
> > > The above concerns[1] can greatly reduce with option b OR option a.
> > >
> > > [1]
> > > 1) more #ifdef clutter
> > > For option (a) this is not needed or option (b) the clutter will be
> > > limited, it will be around structure which add the new filed and
> > > around the FULL block where new functions are added (not inside the
> > > functions)
> > >
> > > 2) 2 binary versions to test
> > > For option (a) this is not needed, for option (b) it is limited as for
> > > new features only one needs to test another binary (rather than NOT
> > > adding a new feature).
> > >
> > > 3) CI and checks must handle RTE_NEXT_ABI version
> > >
> > > I think, it is cheap to add this, at least for compilation test.
> > >
> > > IMO, We need to change the API break release to 3 year kind of time
> > > frame to have very good end user experience
> > > and allow ABI related change to get in every release and force
> > > _rebuild_ shared objects in major LTS release.
> > >
> > > I think, in this major LTS version(23.11) if we can decide (a) vs (b)
> > > then we can align the code accordingly . e.s.p for (a) we need to add
> > > _init() functions.
> > >
> > > Thoughts?
> >
> > Not much input from mailing list. Can we discuss this next TB meeting?
> > Especially how to align with next LTS release on
> > -YAGNI vs reserved fileds with init()
Whichever decision is made on this, remember to also consider if it has any consequences regarding older LTS versions and possibly backporting.
> > -What it takes to Extend the API breaking release more than a year as
> > first step.
Others might disagree, but in my personal opinion, DPDK is still evolving much too rapidly to lock down its ABI/API for more than one year. For reference, consider what has been changed within the last three years, i.e. since DPDK 20.05, and if those changes could have been done within the DPDK 20.05 ABI/API without requiring a substantial additional effort, and while still providing clean and understandable APIs (and not a bunch of weird hacks to shoehorn the new features into the existing APIs).
If you want continuity, use an LTS release. If we lock down the ABI/API for multiple years at a time, what is the point of the LTS releases?
PS: If we start using the RTE_NEXT_ABI concept more, we should remember to promote the additions with each ABI/API breaking release. And we should probably have a rule of thumb to choose between using RTE_NEXT_ABI and using "experimental" marking.
>
> Yes I agree it should be discussed interactively in techboard meeting.
I'm unable to participate in today's techboard meeting, so I have provided my opinions in this email.
-Morten
@@ -40,3 +40,7 @@
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
; Temporary exceptions till next major ABI version ;
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
+
+; Ignore change to reserved opts for new SA option
+[suppress_type]
+ name = rte_security_ipsec_sa_options
@@ -27,7 +27,10 @@
} while (0)
#define RTE_SECURITY_DYNFIELD_NAME "rte_security_dynfield_metadata"
+#define RTE_SECURITY_OOP_DYNFIELD_NAME "rte_security_oop_dynfield_metadata"
+
int rte_security_dynfield_offset = -1;
+int rte_security_oop_dynfield_offset = -1;
int
rte_security_dynfield_register(void)
@@ -42,6 +45,20 @@ rte_security_dynfield_register(void)
return rte_security_dynfield_offset;
}
+int
+rte_security_oop_dynfield_register(void)
+{
+ static const struct rte_mbuf_dynfield dynfield_desc = {
+ .name = RTE_SECURITY_OOP_DYNFIELD_NAME,
+ .size = sizeof(rte_security_oop_dynfield_t),
+ .align = __alignof__(rte_security_oop_dynfield_t),
+ };
+
+ rte_security_oop_dynfield_offset =
+ rte_mbuf_dynfield_register(&dynfield_desc);
+ return rte_security_oop_dynfield_offset;
+}
+
void *
rte_security_session_create(struct rte_security_ctx *instance,
struct rte_security_session_conf *conf,
@@ -275,6 +275,17 @@ struct rte_security_ipsec_sa_options {
*/
uint32_t ip_reassembly_en : 1;
+ /** Enable out of place processing on inline inbound packets.
+ *
+ * * 1: Enable driver to perform Out-of-place(OOP) processing for this inline
+ * inbound SA if supported by driver. PMD need to register mbuf
+ * dynamic field using rte_security_oop_dynfield_register()
+ * and security session creation would fail if dynfield is not
+ * registered successfully.
+ * * 0: Disable OOP processing for this session (default).
+ */
+ uint32_t ingress_oop : 1;
+
/** Reserved bit fields for future extension
*
* User should ensure reserved_opts is cleared as it may change in
@@ -282,7 +293,7 @@ struct rte_security_ipsec_sa_options {
*
* Note: Reduce number of bits in reserved_opts for every new option.
*/
- uint32_t reserved_opts : 17;
+ uint32_t reserved_opts : 16;
};
/** IPSec security association direction */
@@ -812,6 +823,13 @@ typedef uint64_t rte_security_dynfield_t;
/** Dynamic mbuf field for device-specific metadata */
extern int rte_security_dynfield_offset;
+/** Out-of-Place(OOP) processing field type */
+typedef struct rte_mbuf *rte_security_oop_dynfield_t;
+/** Dynamic mbuf field for pointer to original mbuf for
+ * OOP processing session.
+ */
+extern int rte_security_oop_dynfield_offset;
+
/**
* @warning
* @b EXPERIMENTAL: this API may change without prior notice
@@ -834,6 +852,25 @@ rte_security_dynfield(struct rte_mbuf *mbuf)
rte_security_dynfield_t *);
}
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Get pointer to mbuf field for original mbuf pointer when
+ * Out-Of-Place(OOP) processing is enabled in security session.
+ *
+ * @param mbuf packet to access
+ * @return pointer to mbuf field
+ */
+__rte_experimental
+static inline rte_security_oop_dynfield_t *
+rte_security_oop_dynfield(struct rte_mbuf *mbuf)
+{
+ return RTE_MBUF_DYNFIELD(mbuf,
+ rte_security_oop_dynfield_offset,
+ rte_security_oop_dynfield_t *);
+}
+
/**
* @warning
* @b EXPERIMENTAL: this API may change without prior notice
@@ -190,6 +190,14 @@ typedef int (*security_macsec_sa_stats_get_t)(void *device, uint16_t sa_id,
__rte_internal
int rte_security_dynfield_register(void);
+/**
+ * @internal
+ * Register mbuf dynamic field for Security inline ingress Out-of-Place(OOP)
+ * processing.
+ */
+__rte_internal
+int rte_security_oop_dynfield_register(void);
+
/**
* Update the mbuf with provided metadata.
*
@@ -23,10 +23,12 @@ EXPERIMENTAL {
rte_security_macsec_sc_stats_get;
rte_security_session_stats_get;
rte_security_session_update;
+ rte_security_oop_dynfield_offset;
};
INTERNAL {
global:
rte_security_dynfield_register;
+ rte_security_oop_dynfield_register;
};