Re: [dpdk-dev] [EXT] Re: [PATCH v8 1/3] eventdev: introduce crypto adapter enqueue API

[Thomas Monjalon <thomas@monjalon.net>]

14/04/2021 10:39, Akhil Goyal:
> Hi Thomas,
> 
> > 14/04/2021 09:58, Akhil Goyal:
> > > Hi,
> > > > > +
> > > > >  #define RTE_EVENTDEV_NAME_MAX_LEN	(64)
> > > > >  /**< @internal Max length of name of event PMD */
> > > > >
> > > > > @@ -1423,6 +1427,8 @@ struct rte_eventdev {
> > > > >  	 */
> > > > >  	event_tx_adapter_enqueue txa_enqueue;
> > > > >  	/**< Pointer to PMD eth Tx adapter enqueue function. */
> > > > > +	event_crypto_adapter_enqueue ca_enqueue;
> > > > > +	/**< Pointer to PMD crypto adapter enqueue function. */
> > > > >  	struct rte_eventdev_data *data;
> > > > >  	/**< Pointer to device data */
> > > > >  	struct rte_eventdev_ops *dev_ops;
> > > > > @@ -1435,7 +1441,7 @@ struct rte_eventdev {
> > > > >  	/**< Flag indicating the device is attached */
> > > > >
> > > > >  	uint64_t reserved_64s[4]; /**< Reserved for future fields */
> > > > > -	void *reserved_ptrs[4];   /**< Reserved for future fields */
> > > > > +	void *reserved_ptrs[3];   /**< Reserved for future fields */
> > > > >  } __rte_cache_aligned;
> > > >
> > > >
> > > > This change has following ABI breakage[1].
> > > >
> > > > Could you move ca_enqueue at end of struct to avoid the ABI breakage.
> > Also,
> > > > please update depreciation notice to move ca_enqueue to above(to align
> > > > with function pointers) in 21.11 release.
> > > >
> > > > [1]
> > > >   [C]'function rte_eventdev* rte_event_pmd_allocate(const char*, int)' at
> > > > rte_eventdev.c:1467:1 has some indirect sub-type changes:
> > > >     return type changed:
> > > >       in pointed to type 'struct rte_eventdev' at rte_eventdev.h:1411:1:
> > > >         type size hasn't changed
> > > >         1 data member insertion:
> > > >           'event_crypto_adapter_enqueue rte_eventdev::ca_enqueue', at
> > offset
> > > > 512 (in bits) at rte_eventdev.h:1430:1
> > > >         5 data member changes:
> > > >          'rte_eventdev_data* rte_eventdev::data' offset changed from 512 to
> > > > 576 (in bits) (by +64 bits)
> > > >          'rte_eventdev_ops* rte_eventdev::dev_ops' offset changed from
> > 576 to
> > > > 640 (in bits) (by +64 bits)
> > > >          'rte_device* rte_eventdev::dev' offset changed from 640 to 704 (in
> > bits)
> > > > (by +64 bits)
> > > >          'uint64_t rte_eventdev::reserved_64s[4]' offset changed from 768 to
> > > > 832 (in bits) (by +64 bits)
> > > >          type of 'void* rte_eventdev::reserved_ptrs[4]' changed:
> > > >            type name changed from 'void*[4]' to 'void*[3]'
> > > >            array type size changed from 256 to 192
> > > >            array type subrange 1 changed length from 4 to 3
> > > >          and offset changed from 1024 to 1088 (in bits) (by +64 bits)
> > > >
> > > >
> > > Yes my bad, it should be added in the end.
> > > But abi script will still shout for 'void*[4]' to 'void*[3]' conversion.
> > > We may need to add something in the devtools/libabigail.abignore
> > > So that, CI is not broken when reserved fields are changed.
> > > Otherwise, it does not make sense to introduce reserve fields.
> > > Can we have something generic for reserved fields?
> > > Any suggestions?
> > 
> > The ABI check is not aware about the reserved fields.
> > It needs to be added in libabigail.ignore.
> > 
> Can I add a generic ignore for all reserved fields.
> 
> +; Ignore changes in reserved fields
> +[suppress_variable]
> +       name_regexp = reserved

You can propose in a separate patch in your series.