[dpdk-dev] [PATCH v2] doc: add reserve fields to eventdev public structures

[Bruce Richardson]

On Tue, Aug 04, 2020 at 05:07:12PM +0530, Jerin Jacob wrote:
> On Tue, Aug 4, 2020 at 4:12 PM Bruce Richardson
> <bruce.richardson at intel.com> wrote:
> >
> > On Mon, Aug 03, 2020 at 12:59:03PM +0530, pbhagavatula at marvell.com wrote:
> > > From: Pavan Nikhilesh <pbhagavatula at marvell.com>
> > >
> > > Add 64 byte padding at the end of event device public structure to allow
> > > future extensions.
> > >
> > > Signed-off-by: Pavan Nikhilesh <pbhagavatula at marvell.com>
> > > Acked-by: Jerin Jacob <jerinj at marvell.com>
> > > ---
> > >  v2 Changes:
> > >  - Modify commit title.
> > >  - Add patch reference to doc.
> > >
> > >  doc/guides/rel_notes/deprecation.rst | 11 +++++++++++
> > >  1 file changed, 11 insertions(+)
> > >
> > > diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> > > index ea4cfa7a4..ec5db68e9 100644
> > > --- a/doc/guides/rel_notes/deprecation.rst
> > > +++ b/doc/guides/rel_notes/deprecation.rst
> > > @@ -151,3 +151,14 @@ Deprecation Notices
> > >    Python 2 support will be completely removed in 20.11.
> > >    In 20.08, explicit deprecation warnings will be displayed when running
> > >    scripts with Python 2.
> > > +
> > > +* eventdev: A 64 byte padding is added at the end of the following structures
> > > +  in event device library to support future extensions:
> > > +  ``rte_event_crypto_adapter_conf``, ``rte_event_eth_rx_adapter_conf``,
> > > +  ``rte_event_eth_rx_adapter_queue_conf``, ``rte_event_eth_tx_adapter_conf``,
> > > +  ``rte_event_timer_adapter_conf``, ``rte_event_timer_adapter_info``,
> > > +  ``rte_event_dev_info``, ``rte_event_dev_config``, ``rte_event_queue_conf``,
> > > +  ``rte_event_port_conf``, ``rte_event_timer_adapter``,
> > > +  ``rte_event_timer_adapter_data``.
> > > +  Reference:
> > > +  http://patches.dpdk.org/project/dpdk/list/?series=10728&archive=both&state=*
> > > --
> >
> > I don't like this idea of adding lots of padding to the ends of these
> > structures. For some structures, such as the public arrays for devices it
> > may be necessary, but for all the conf structures passed as parameters to
> > functions I think we can do better. Since these structures are passed by
> > the user to various functions, function versioning can be used to ensure
> > that the correct function in eventdev is always called. From there to the
> > individual PMDs, we can implement ABI compatibility by either:
> > 1. including the length of the struct as a parameter to the driver. (This is
> >   a bit similar to my proposal for rawdev [1])
> > 2. including the ABI version as a parameter to the driver.
> 
> But, Will the above solution work if the application is dependent on
> struct size?
> i.e change of s1 size will change offset of s3 i.e
> app_sepecific_struct_s3. Right?
> i.e DPDK version should not change the offset of s3. Right?
> 
> example,
> struct app_struct {
>           struct dpdk_public_struct_s1 s1;
>           struct dpdk_public_struct_s2 s2;
>           struct app_sepecific_struct_s3 s3;
> }
> 
Not sure what exactly you mean here. The actual offsets and sizes of the
structs will obviously change as you change the struct, but the end
compiled app has no idea of structs, all it knows of is offsets, which is
why you provide ABI compatible versions of the functions which use "legacy"
copies of the structs to ensure correct offsets. It's pretty much standard
practice for ABI versioning.

The real complication arises because the actual eventdev driver functions
are not called directly with the linker resolving symbol versioning.
Instead they are called using function pointers from the code. This is why
one needs to add in the additional parameter to the driver APIs so that the
ABI info - be it struct size or version - can be passed from the versioned
eventdev library function through to the driver.

Regards,
/Bruce