Message ID | 1618283653-16510-1-git-send-email-xuemingl@nvidia.com (mailing list archive) |
---|---|
Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id A1A34A0524; Tue, 13 Apr 2021 05:14:32 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 5EBA61609B0; Tue, 13 Apr 2021 05:14:32 +0200 (CEST) Received: from mellanox.co.il (mail-il-dmz.mellanox.com [193.47.165.129]) by mails.dpdk.org (Postfix) with ESMTP id C3FFE1609AF for <dev@dpdk.org>; Tue, 13 Apr 2021 05:14:30 +0200 (CEST) Received: from Internal Mail-Server by MTLPINE1 (envelope-from xuemingl@nvidia.com) with SMTP; 13 Apr 2021 06:14:26 +0300 Received: from nvidia.com (pegasus05.mtr.labs.mlnx [10.210.16.100]) by labmailer.mlnx (8.13.8/8.13.8) with ESMTP id 13D3EQxo015492; Tue, 13 Apr 2021 06:14:26 +0300 From: Xueming Li <xuemingl@nvidia.com> To: Thomas Monjalon <thomas@monjalon.net>, Gaetan Rivet <gaetanr@nvidia.com> Cc: dev@dpdk.org, xuemingl@nvidia.com, Asaf Penso <asafp@nvidia.com> Date: Tue, 13 Apr 2021 03:14:07 +0000 Message-Id: <1618283653-16510-1-git-send-email-xuemingl@nvidia.com> X-Mailer: git-send-email 1.8.3.1 In-Reply-To: <1608304614-13908-2-git-send-email-xuemingl@nvidia.com> References: <1608304614-13908-2-git-send-email-xuemingl@nvidia.com> Subject: [dpdk-dev] [PATCH v5 0/5] eal: enable global device syntax by default X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions <dev.dpdk.org> List-Unsubscribe: <https://mails.dpdk.org/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://mails.dpdk.org/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <https://mails.dpdk.org/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
Series | eal: enable global device syntax by default | |
Message
Xueming Li
April 13, 2021, 3:14 a.m. UTC
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(-)
Comments
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.