Re: [dpdk-dev] [PATCH v8 2/3] build: use Python pmdinfogen

On Mon, Jan 25, 2021 at 11:01 AM Kinsella, Ray [off-list ref] wrote:

quoted

On 25/01/2021 09:25, Kinsella, Ray wrote:

quoted

On 23/01/2021 11:38, Thomas Monjalon wrote:

quoted

22/01/2021 23:24, Dmitry Kozlyuk:

quoted

On Fri, 22 Jan 2021 21:57:15 +0100, Thomas Monjalon wrote:

quoted

22/01/2021 21:31, Dmitry Kozlyuk:

quoted

On Wed, 20 Jan 2021 11:24:21 +0100, Thomas Monjalon wrote:

quoted

20/01/2021 08:23, Dmitry Kozlyuk:

quoted

On Wed, 20 Jan 2021 01:05:59 +0100, Thomas Monjalon wrote:

quoted

This is now the right timeframe to introduce this change
with the new Python module dependency.
Unfortunately, the ABI check is returning an issue:

'const char mlx5_common_pci_pmd_info[62]' was changed
to 'const char mlx5_common_pci_pmd_info[60]' at rte_common_mlx5.pmd.c

Will investigate and fix ASAP.

Now that I think of it: strings like this change every time new PCI IDs are
added to a PMD, but AFAIK adding PCI IDs is not considered an ABI breakage,
is it? One example is 28c9a7d7b48e ("net/mlx5: add ConnectX-6 Lx device ID")
added 2020-07-08, i.e. clearly outside of ABI change window.

You're right.

quoted

"xxx_pmd_info" changes are due to JSON formatting (new is more canonical),
which can be worked around easily, if the above is wrong.

If the new format is better, please keep it.
What we need is an exception for the pmdinfo symbols
in the file devtools/libabigail.abignore.
You can probably use a regex for these symbols.

This would allow real breakages to pass ABI check, abidiff doesn't analyze
variable content and it's not easy to compare. Maybe later a script can be
added that checks lines with RTE_DEVICE_IN in patches. There are at most 32 of
5494 relevant commits between 19.11 and 20.11, though.

To verify there are no meaningful changes I ensured empty diff between
results of the following command for "main" and the branch:

    find build/drivers -name '*.so' -exec usertools/dpdk-pmdinfo.py

For now we cannot do such check as part of the ABI checker.
And we cannot merge this patch if the ABI check fails.
I think the only solution is to allow any change in the pmdinfo variables.

So my 2c on this is that this is an acceptable work-around for the v21 (DPDK v20.11) ABI.
However we are going to end up carrying this rule in libabigail.ignore indefinitely.

Would it make sense to just fix the size of _pmd_info to some reasonably large value -
say 128 bytes, to allow us to drop the rule in the DPDK 21.11 v22 release?

Ray K

Another point is - shouldn't _pmd_info probably live in "INTERNAL" is anycase?

The symbol itself can be hidden from the ABeyes.
It is only a placeholder for the PMD_INFO_STRING= string used by
usertools/dpdk-pmdinfo.py and maybe some other parsing tool.

I guess a static symbol would be enough:

diff --git a/buildtools/pmdinfogen/pmdinfogen.c

b/buildtools/pmdinfogen/pmdinfogen.c
index a68d1ea999..14bf7d9f42 100644

--- a/buildtools/pmdinfogen/pmdinfogen.c
+++ b/buildtools/pmdinfogen/pmdinfogen.c

@@ -393,7 +393,7 @@ static void output_pmd_info_string(struct elf_info

*info, char *outfile)
        drv = info->drivers;

        while (drv) {
-               fprintf(ofd, "const char %s_pmd_info[] __attribute__((used)) = "
+               fprintf(ofd, "static const char %s_pmd_info[]
__attribute__((used)) = "
                        "\"PMD_INFO_STRING= {",
                        drv->name);
                fprintf(ofd, "\\\"name\\\" : \\\"%s\\\", ", drv->name);


We will need an exception for the v21 ABI though.