On Tue, Feb 8, 2022 at 10:19 AM Akhil Goyal <gak...@marvell.com> wrote:
>
> > Hello Akhil,
> >
> >
> > On Fri, Feb 4, 2022 at 11:14 PM Akhil Goyal <gak...@marvell.com> wrote:
> > >
> > > A new option is added in IPsec to enable and attempt reassembly
> > > of inbound packets.
> >
> > First, about extending this structure.
> >
> > Copying the header:
> >
> > /** Reserved bit fields for future extension
> > *
> > * User should ensure reserved_opts is cleared as it may change in
> > * subsequent releases to support new options.
> > *
> > * Note: Reduce number of bits in reserved_opts for every new
> > option.
> > */
> > uint32_t reserved_opts : 18;
> >
> > I did not follow the introduction of the reserved_opts field, but
> > writing this comment in the API only is weak.
> > Why can't the rte_security API enforce reserved_opts == 0 (like in
> > rte_security_session_create)?
> >
> This was discussed here.
> http://patches.dpdk.org/project/dpdk/patch/20211008204516.3497060-3-gak...@marvell.com/
> rte_security_ipsec_sa_options is being used at multiple places as listed
> below in abiignore.
> Checking a particular field in each of the API does not make sense to me.
It's strange to me that a user may pass this structure as input in
multiple functions.
But if it's how the security lib works, ok.
>
> >
> >
> > >
> > > Signed-off-by: Akhil Goyal <gak...@marvell.com>
> > > Change-Id: I6f66f0b5a659550976a32629130594070cb16cb1
> > ^^^
> > Internal tag, please remove.
> >
> Yes, missed that will remove.
> >
> > > ---
> > > devtools/libabigail.abignore | 14 ++++++++++++++
> > > lib/security/rte_security.h | 12 +++++++++++-
> > > 2 files changed, 25 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/devtools/libabigail.abignore b/devtools/libabigail.abignore
> > > index 4b676f317d..3bd39042e8 100644
> > > --- a/devtools/libabigail.abignore
> > > +++ b/devtools/libabigail.abignore
> > > @@ -11,3 +11,17 @@
> > > ; Ignore generated PMD information strings
> > > [suppress_variable]
> > > name_regexp = _pmd_info$
> > > +
> > > +; Ignore fields inserted in place of reserved_opts of
> > rte_security_ipsec_sa_options
> > > +[suppress_type]
> > > + name = rte_ipsec_sa_prm
> > > + name = rte_security_ipsec_sa_options
> > > + has_data_member_inserted_between = {offset_of(reserved_opts), end}
> > > +
> > > +[suppress_type]
> > > + name = rte_security_capability
> > > + has_data_member_inserted_between = {offset_of(reserved_opts),
> > (offset_of(reserved_opts) + 18)}
> > > +
> > > +[suppress_type]
> > > + name = rte_security_session_conf
> > > + has_data_member_inserted_between = {offset_of(reserved_opts),
> > (offset_of(reserved_opts) + 18)}
> >
> > Now, about the suppression rule, I don't understand the intention of
> > those 3 rules.
> >
> > I would simply suppress modifications (after reserved_opts) to the
> > rte_security_ipsec_sa_options struct.
> > Like:
> >
> > ; Ignore fields inserted in place of reserved_opts of
> > rte_security_ipsec_sa_options
> > [suppress_type]
> > name = rte_security_ipsec_sa_options
> > has_data_member_inserted_between = {offset_of(reserved_opts), end}
> >
> I tried this in the first place but abi check was complaining in other
> structures which included
> rte_security_ipsec_sa_options. So I had to add suppression for those as well.
> Can you try at your end?
I tried before suggesting, and it works with a single rule on this structure.
I'm using libabigail current master, which version are you using so I
can try with the same?
--
David Marchand
