On Tue, Feb 27, 2024 at 04:18:10PM +0100, David Marchand wrote: > Hello Dodji, > > On Tue, Feb 27, 2024 at 6:44 AM Tyler Retzlaff > <roret...@linux.microsoft.com> wrote: > > > > RTE_MARKER typedefs are a GCC extension unsupported by MSVC. Remove > > RTE_MARKER fields from rte_mbuf struct. > > > > Maintain alignment of fields after removed cacheline1 marker by placing > > C11 alignas(RTE_CACHE_LINE_MIN_SIZE). > > > > Update implementation of rte_mbuf_prefetch_part1() and > > rte_mbuf_prefetch_part2() inline functions calculate pointer for > > prefetch of cachline0 and cachline1 without using removed markers. > > > > Update static_assert of rte_mbuf struct fields to reference data_off and > > packet_type fields that occupy the original offsets of the marker > > fields. > > > > Signed-off-by: Tyler Retzlaff <roret...@linux.microsoft.com> > > This change is reported as a potential ABI change. > > For the context, this patch > https://patchwork.dpdk.org/project/dpdk/patch/1709012499-12813-21-git-send-email-roret...@linux.microsoft.com/ > removes null-sized markers (those fields were using RTE_MARKER, see > https://git.dpdk.org/dpdk/tree/lib/eal/include/rte_common.h#n583) from > the rte_mbuf struct. > I would argue this change do not impact ABI as the layout of the mbuf > object is not impacted.
It isn't a surprise that the change got flagged because the 0 sized fields being removed probably not something the checker understands. So no ABI change just API break (as was requested). > As reported by the CI: > > [C] 'function const rte_eth_rxtx_callback* > rte_eth_add_first_rx_callback(uint16_t, uint16_t, rte_rx_callback_fn, > void*)' at rte_ethdev.c:5768:1 has some indirect sub-type changes: > parameter 3 of type 'typedef rte_rx_callback_fn' has sub-type changes: > underlying type 'typedef uint16_t (typedef uint16_t, typedef > uint16_t, rte_mbuf**, typedef uint16_t, typedef uint16_t, void*)*' > changed: > in pointed to type 'function type typedef uint16_t (typedef > uint16_t, typedef uint16_t, rte_mbuf**, typedef uint16_t, typedef > uint16_t, void*)': > parameter 3 of type 'rte_mbuf**' has sub-type changes: > in pointed to type 'rte_mbuf*': > in pointed to type 'struct rte_mbuf' at rte_mbuf_core.h:470:1: > type size hasn't changed > 4 data member deletions: > 'RTE_MARKER cacheline0', at offset 0 (in bits) at > rte_mbuf_core.h:467:1 > 'RTE_MARKER64 rearm_data', at offset 128 (in bits) > at rte_mbuf_core.h:490:1 > 'RTE_MARKER rx_descriptor_fields1', at offset 256 > (in bits) at rte_mbuf_core.h:517:1 > 'RTE_MARKER cacheline1', at offset 512 (in bits) at > rte_mbuf_core.h:598:1 > no data member change (1 filtered); > > Error: ABI issue reported for abidiff --suppr > /home/runner/work/dpdk/dpdk/devtools/libabigail.abignore > --no-added-syms --headers-dir1 reference/usr/local/include > --headers-dir2 install/usr/local/include > reference/usr/local/lib/librte_ethdev.so.24.0 > install/usr/local/lib/librte_ethdev.so.24.1 > ABIDIFF_ABI_CHANGE, this change requires a review (abidiff flagged > this as a potential issue). > > Opinions? > > Btw, I see no way to suppress this (except a global [suppress_type] > name = rte_mbuf)... I am unfamiliar with the ABI checker I'm afraid i have no suggestion to offer. Maybe we can just ignore the failure for this one series when we decide it is ready to be merged and don't suppress the checker? > > > -- > David Marchand
