Hello, David Marchand <david.march...@redhat.com> writes:
> Hello Dodji, o/ [...] > 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. Thank you for the context. [...] > 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: [...] > 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); [...] > I would argue this change do not impact ABI as the layout of the mbuf > object is not impacted. I agree that on the /particular platform/ that the checker runs on, there is no incompatible ABI change because no data member offset from the 'struct rte_mbuf' type got modified and the size of the type hasn't changed either. > > 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)... Right. To avoid having subsequent changes to that type from being "overly" suppressed, maybe do something like: [suppress_type] name = rte_mbuf has_size_change = no has_data_member = {cacheline0, rearm_data, rx_descriptor_fields1, cacheline1} That way, only size-impacting changes to struct rte_mbuf in its form that predates this patch would be suppressed, hopefully. [...] Cheers, -- Dodji
