[v5,0/5] eal: enable global device syntax by default

  The new Global Device Syntax [1] is used to identify a device with full
bus, class and driver description, example:
 -a bus=pci,addr=82:00.0/class=eth/driver=mlx5,...

This patchset fixes bugs and enable global device syntax with
backward compatibility by:
- unify devargs memory buffer cleanup
- parse name from bus driver callback api 
- try new global syntax parsing firstly and fallback to legacy parsing.


History:

V1:
 - Initial version

V2:
 - add devargs.src as complete source dev string
 - change devargs.data to scratch buffer
 - add rte_devargs_free() to release scratch memory
 - change name policy to align with rte_eth_iterator_init()
 - remove PCI bus fix as name already resolved in rte_devargs_parse().
V3:
 - remove devargs.src
 - rename rte_devargs_free() to rte_devargs_reset()
 - add bus callback api to resolve devargs.
V4:
 - add RTE_DEVARGS_KEY_BUS/CLASS/DIRVER macro
 - parsing "name" by default if no bus devargs parsing callback
 - Minor fixes suggested by Ray and Thomas
v5:
 - Update relrease notes
 - Remove NULL support of kvargs_get function
 - Rebased on latest code
 - Small updates according to review comments


[1] Global Device Syntax:
https://www.dpdk.org/wp-content/uploads/sites/35/2018/10/am-07-DPDK-hotplug-20180905.pdf

[2] RFC:
http://patchwork.dpdk.org/project/dpdk/list/?series=14378

[3] V1:
http://patchwork.dpdk.org/project/dpdk/list/?series=14610

[4] V2:
http://patchwork.dpdk.org/project/dpdk/list/?series=14816

[5] V3:
http://patchwork.dpdk.org/project/dpdk/list/?series=15979

[6] v4:
http://patchwork.dpdk.org/project/dpdk/list/?series=16267


Xueming Li (5):
  devargs: unify scratch buffer storage
  devargs: fix memory leak on parsing error
  kvargs: add get by key function
  bus: add device arguments name parsing API
  devargs: parse global device syntax

 app/test-pmd/config.c                        |  3 +-
 app/test-pmd/testpmd.c                       |  5 +-
 doc/guides/rel_notes/release_21_05.rst       |  7 ++
 drivers/bus/pci/pci_common.c                 |  1 +
 drivers/bus/pci/pci_params.c                 | 47 ++++++++++
 drivers/bus/pci/private.h                    | 14 +++
 drivers/bus/vdev/vdev.c                      |  9 +-
 drivers/net/failsafe/failsafe_args.c         |  3 +-
 drivers/net/failsafe/failsafe_eal.c          |  2 +-
 examples/multi_process/hotplug_mp/commands.c |  6 +-
 lib/librte_eal/common/eal_common_dev.c       |  9 +-
 lib/librte_eal/common/eal_common_devargs.c   | 91 +++++++++++++++-----
 lib/librte_eal/common/hotplug_mp.c           |  6 +-
 lib/librte_eal/include/rte_bus.h             | 17 ++++
 lib/librte_eal/include/rte_devargs.h         | 22 ++++-
 lib/librte_eal/version.map                   |  1 +
 lib/librte_ethdev/rte_ethdev.c               |  9 +-
 lib/librte_kvargs/rte_kvargs.c               | 15 ++++
 lib/librte_kvargs/rte_kvargs.h               | 20 +++++
 lib/librte_kvargs/version.map                |  3 +
 20 files changed, 238 insertions(+), 52 deletions(-)
  

13/04/2021 05:14, Xueming Li:
> Xueming Li (5):
>   devargs: unify scratch buffer storage
>   devargs: fix memory leak on parsing error
>   kvargs: add get by key function
>   bus: add device arguments name parsing API
>   devargs: parse global device syntax

The patch 4 adds a new callback in rte_bus.
I thought about it during the whole day and I don't see any good way
to merge it without breaking the ABI compatibility.

Only first 3 patches are applied for now, thanks.
  

On 14/04/2021 20:49, Thomas Monjalon wrote:
> 13/04/2021 05:14, Xueming Li:
>> Xueming Li (5):
>>   devargs: unify scratch buffer storage
>>   devargs: fix memory leak on parsing error
>>   kvargs: add get by key function
>>   bus: add device arguments name parsing API
>>   devargs: parse global device syntax
> 
> The patch 4 adds a new callback in rte_bus.
> I thought about it during the whole day and I don't see any good way
> to merge it without breaking the ABI compatibility.
> 
> Only first 3 patches are applied for now, thanks.
> 

I took a look, I don't immediately see the concern.

The new entry is at the end of the memory structure.
The call back is internal and hidden behind the symbol rte_devargs_layers_parse.

So will only be trigger by a rte_devargs_layers_parse of the same version of DPDK that introduce the new callback.

Should be fine?
  

On Fri, Apr 23, 2021, at 13:06, Kinsella, Ray wrote:
> 
> 
> On 14/04/2021 20:49, Thomas Monjalon wrote:
> > 13/04/2021 05:14, Xueming Li:
> >> Xueming Li (5):
> >>   devargs: unify scratch buffer storage
> >>   devargs: fix memory leak on parsing error
> >>   kvargs: add get by key function
> >>   bus: add device arguments name parsing API
> >>   devargs: parse global device syntax
> > 
> > The patch 4 adds a new callback in rte_bus.
> > I thought about it during the whole day and I don't see any good way
> > to merge it without breaking the ABI compatibility.
> > 
> > Only first 3 patches are applied for now, thanks.
> > 
> 
> I took a look, I don't immediately see the concern.
> 
> The new entry is at the end of the memory structure.
> The call back is internal and hidden behind the symbol rte_devargs_layers_parse.
> 
> So will only be trigger by a rte_devargs_layers_parse of the same 
> version of DPDK that introduce the new callback.
> 
> Should be fine?
> 

It might have been an issue IMO with a structure exposed as an array, i.e. rte_eth_devices[].
But I thought this kind of ABI break was the kind that would be accepted between two LTS.

The only potential risk is in using a new version librte_eal.so with an older librte_bus_xxx.so
But I think it is fair to expect installations to be internally consistent.

Maybe we could have a runtime warning when loading mismatched versions
(if there isn't one already) -- each librte_*.so could have an internal version stamp and alignment could
be checked through a constructor in each lib?
  

On 23/04/2021 12:39, Gaëtan Rivet wrote:
> On Fri, Apr 23, 2021, at 13:06, Kinsella, Ray wrote:
>>
>>
>> On 14/04/2021 20:49, Thomas Monjalon wrote:
>>> 13/04/2021 05:14, Xueming Li:
>>>> Xueming Li (5):
>>>>   devargs: unify scratch buffer storage
>>>>   devargs: fix memory leak on parsing error
>>>>   kvargs: add get by key function
>>>>   bus: add device arguments name parsing API
>>>>   devargs: parse global device syntax
>>>
>>> The patch 4 adds a new callback in rte_bus.
>>> I thought about it during the whole day and I don't see any good way
>>> to merge it without breaking the ABI compatibility.
>>>
>>> Only first 3 patches are applied for now, thanks.
>>>
>>
>> I took a look, I don't immediately see the concern.
>>
>> The new entry is at the end of the memory structure.
>> The call back is internal and hidden behind the symbol rte_devargs_layers_parse.
>>
>> So will only be trigger by a rte_devargs_layers_parse of the same 
>> version of DPDK that introduce the new callback.
>>
>> Should be fine?
>>
> 
> It might have been an issue IMO with a structure exposed as an array, i.e. rte_eth_devices[].
> But I thought this kind of ABI break was the kind that would be accepted between two LTS.

Very much depends on how it is done. 
New fields are ok in some circumstances, at first glance I thought one is ok. 
 
> The only potential risk is in using a new version librte_eal.so with an older librte_bus_xxx.so

We don't account for or consider that, that would be an irrational environmnet. 

> But I think it is fair to expect installations to be internally consistent.
> 
> Maybe we could have a runtime warning when loading mismatched versions

Nope - that would be insanely complex. 

> (if there isn't one already) -- each librte_*.so could have an internal version stamp and alignment could
> be checked through a constructor in each lib?
>
  

14/04/2021 21:49, Thomas Monjalon:
> 13/04/2021 05:14, Xueming Li:
> > Xueming Li (5):
> >   devargs: unify scratch buffer storage
> >   devargs: fix memory leak on parsing error
> >   kvargs: add get by key function
> >   bus: add device arguments name parsing API
> >   devargs: parse global device syntax
> 
> The patch 4 adds a new callback in rte_bus.
> I thought about it during the whole day and I don't see any good way
> to merge it without breaking the ABI compatibility.
> 
> Only first 3 patches are applied for now, thanks.

Last 2 patches are applied for 21.11, thanks.