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

[Kinsella, Ray]

On 25/01/2021 09:25, Kinsella, Ray wrote:
> 
> 
> On 23/01/2021 11:38, Thomas Monjalon wrote:
>> 22/01/2021 23:24, Dmitry Kozlyuk:
>>> On Fri, 22 Jan 2021 21:57:15 +0100, Thomas Monjalon wrote:
>>>> 22/01/2021 21:31, Dmitry Kozlyuk:
>>>>> On Wed, 20 Jan 2021 11:24:21 +0100, Thomas Monjalon wrote:  
>>>>>> 20/01/2021 08:23, Dmitry Kozlyuk:  
>>>>>>> On Wed, 20 Jan 2021 01:05:59 +0100, Thomas Monjalon wrote:    
>>>>>>>> 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.
>>>>
>>>>> "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?