-----Original Message-----
From: Stephen Hemminger <stephen@networkplumber.org>
Sent: Monday 11 October 2021 23:16
To: Akhil Goyal <redacted>
Cc: Thomas Monjalon <redacted>; dev@dpdk.org;
david.marchand@redhat.com; hemant.agrawal@nxp.com; Anoob Joseph
[off-list ref]; De Lara Guarch, Pablo
[off-list ref]; Trahe, Fiona [off-list ref];
Doherty, Declan [off-list ref]; matan@nvidia.com;
g.singh@nxp.com; Zhang, Roy Fan [off-list ref];
jianjay.zhou@huawei.com; asomalap@amd.com; ruifeng.wang@arm.com;
Ananyev, Konstantin [off-list ref]; Nicolau, Radu
[off-list ref]; ajit.khaparde@broadcom.com; Nagadheeraj
Rottela [off-list ref]; Ankur Dwivedi
[off-list ref]; Power, Ciara [off-list ref]; Kinsella,
Ray [off-list ref]; Richardson, Bruce
[off-list ref]
Subject: Re: [EXT] Re: [PATCH v2 3/3] security: add reserved bitfields

On Mon, 11 Oct 2021 16:58:24 +0000
Akhil Goyal [off-list ref] wrote:

quoted

quoted

08/10/2021 22:45, Akhil Goyal:

quoted

In struct rte_security_ipsec_sa_options, for every new option
added, there is an ABI breakage, to avoid, a reserved_opts
bitfield is added to for the remaining bits available in the
structure.
Now for every new sa option, these reserved_opts can be reduced
and new option can be added.

How do you make sure this field is initialized to 0?

Struct rte_security_ipsec_xform Is part of rte_security_capability as
well As a configuration structure in session create.
User, should ensure that if a device support that option(in
capability), then only these options will take into effect or else it

will be don't care for the PMD.

quoted

The initial values of capabilities are set by PMD statically based on
the features that it support.
So if someone sets a bit in reserved_opts, it will work only if PMD
support it And sets the corresponding field in capabilities.
But yes, if a new field is added in future, and user sets the
reserved_opts by mistake And the PMD supports that feature as well,

then that feature will be enabled.

quoted

This may or may not create issue depending on the feature which is

enabled.

quoted

Should I add a note in the comments to clarify that reserved_opts
should be set as 0 And future releases may change this without

notice(But reserved in itself suggest that)?

quoted

Adding an explicit check in session_create does not make sense to me.
What do you suggest?

Regards,
Akhil

The problem is if user creates an on stack variable and sets the
unreserved fields to good values but other parts are garbage.  This
passes API/ABI unless you strictly enforce that all reserved fields are
zero.

Right, but that is no better or worse than the current struct, in that respect, right?
User case be careless there also - declare it on the stack and forget to memset.

struct rte_security_ipsec_sa_options {
     uint32_t esn : 1;
 
     uint32_t udp_encap : 1;
 
     uint32_t copy_dscp : 1;
 
     uint32_t copy_flabel : 1;
 
     uint32_t copy_df : 1;
 
     uint32_t dec_ttl : 1;
 
     uint32_t ecn : 1;
 
     uint32_t stats : 1;
 
     uint32_t iv_gen_disable : 1;
 
     uint32_t tunnel_hdr_verify : 2;
 };