Re: [PATCH 1/3] security: introduce out of place support for inline ingress

[Jerin Jacob <jerinjacobk@gmail.com>]

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?




>
>