Implement new devargs framework

PATCH Superseded Gaetan Rivet 5 participants 47 messages Aug 30, 2018 – Feb 19, 2020 Arm/Amlogic Meson SoC support

Share & tools

Gaetan Rivet

author

2018-08-30 13:42 UTC #0 lore raw

cover Implement new devargs framework

Last release saw the introduction of the new devargs system. To this end, the "class" abstraction was described as well as a common API for querying and declaring devices.

This patchset implements the "eth" device class and the query/declaration part of the framework for PCI and vdev buses, enabling a minimal support for the new system.

A new testpmd command is added to test device querying.

Devargs parsing is extended in the relevant buses to test device declaration. This part uses the new "rte_eth_dev_create" API, introduced last release and used by only two PMDs, for now.

The new devargs format is also made available through the new --dev parameter.

Next work is to generalize use of new API for eth_dev creation, compatibility layer for -w, -b and --vdev with --dev, and devargs unit test.

Gaetan Rivet (13):
bus/pci: implement device iteration and comparison bus/pci: add device matching field id bus/vdev: implement device iteration bus/vdev: add device matching field driver
ethdev: add private generic device iterator
ethdev: register ether layer as a class
ethdev: add device matching field name app/testpmd: add show device command bus/pci: pre-process declarative PCI devargs bus/vdev: pre-process declarative vdev devargs bus/pci: process declarative PCI devargs
ethdev: process declarative eth devargs
eal: add generic dev parameter

 app/test-pmd/cmdline.c                      |  54 +++++++
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  24 ++++
 drivers/bus/pci/Makefile                    |   3 +-
 drivers/bus/pci/bsd/pci.c                   |   5 +
 drivers/bus/pci/linux/pci.c                 |   5 +
 drivers/bus/pci/meson.build                 |   6 +-
 drivers/bus/pci/pci_common.c                |   6 +-
 drivers/bus/pci/pci_params.c                | 139 ++++++++++++++++++
 drivers/bus/pci/private.h                   |  54 +++++++
 drivers/bus/vdev/Makefile                   |   3 +-
 drivers/bus/vdev/meson.build                |   5 +-
 drivers/bus/vdev/vdev.c                     |  20 +--
 drivers/bus/vdev/vdev_params.c              | 112 +++++++++++++++
 drivers/bus/vdev/vdev_private.h             |  32 +++++
 lib/librte_eal/common/eal_common_devargs.c  |   4 +
 lib/librte_eal/common/eal_common_options.c  |  36 ++++-
 lib/librte_eal/common/eal_options.h         |   2 +
 lib/librte_ethdev/Makefile                  |   4 +-
 lib/librte_ethdev/eth_private.c             |  31 ++++
 lib/librte_ethdev/eth_private.h             |  31 ++++
 lib/librte_ethdev/meson.build               |   4 +-
 lib/librte_ethdev/rte_class_eth.c           | 148 ++++++++++++++++++++
 lib/librte_ethdev/rte_ethdev.c              |   7 +
 23 files changed, 714 insertions(+), 21 deletions(-)
 create mode 100644 drivers/bus/pci/pci_params.c
 create mode 100644 drivers/bus/vdev/vdev_params.c
 create mode 100644 drivers/bus/vdev/vdev_private.h
 create mode 100644 lib/librte_ethdev/eth_private.c
 create mode 100644 lib/librte_ethdev/eth_private.h
 create mode 100644 lib/librte_ethdev/rte_class_eth.c

Signature · 1 line

2.18.0

Gaetan Rivet

author

2018-08-30 13:42 UTC #1 lore raw

PATCH 1/13 bus/pci: implement device iteration and comparison

Signed-off-by Gaetan Rivet <[email protected]>

---
 drivers/bus/pci/Makefile     |  3 +-
 drivers/bus/pci/meson.build  |  6 +++-
 drivers/bus/pci/pci_common.c |  3 +-
 drivers/bus/pci/pci_params.c | 53 ++++++++++++++++++++++++++++++++++++
 drivers/bus/pci/private.h    | 25 +++++++++++++++++
 5 files changed, 86 insertions(+), 4 deletions(-)
 create mode 100644 drivers/bus/pci/pci_params.c

drivers/bus/pci/Makefile +2 -1

index cf373068a..4de953f8f 100644
@@ -26,10 +26,11 @@ CFLAGS += -I$(RTE_SDK)/lib/librte_eal/$(SYSTEM)app/eal
 CFLAGS += -DALLOW_EXPERIMENTAL_API
 
 LDLIBS += -lrte_eal -lrte_mbuf -lrte_mempool -lrte_ring
-LDLIBS += -lrte_ethdev -lrte_pci
+LDLIBS += -lrte_ethdev -lrte_pci -lrte_kvargs
 
 include $(RTE_SDK)/drivers/bus/pci/$(SYSTEM)/Makefile
 SRCS-$(CONFIG_RTE_LIBRTE_PCI_BUS) := $(addprefix $(SYSTEM)/,$(SRCS))
+SRCS-$(CONFIG_RTE_LIBRTE_PCI_BUS) += pci_params.c
 SRCS-$(CONFIG_RTE_LIBRTE_PCI_BUS) += pci_common.c
 SRCS-$(CONFIG_RTE_LIBRTE_PCI_BUS) += pci_common_uio.c

drivers/bus/pci/meson.build +5 -1

index 72939e598..23d6a5fec 100644
@@ -3,7 +3,9 @@
 
 deps += ['pci']
 install_headers('rte_bus_pci.h')
-sources = files('pci_common.c', 'pci_common_uio.c')
+sources = files('pci_common.c',
+	'pci_common_uio.c',
+	'pci_params.c')
 if host_machine.system() == 'linux'
 	sources += files('linux/pci.c',
 			'linux/pci_uio.c',

@@ -17,3 +19,5 @@ endif
 
 # memseg walk is not part of stable API yet
 allow_experimental_apis = true
+
+deps += ['kvargs']

drivers/bus/pci/pci_common.c +1 -2

index 7736b3f9c..c7695d108 100644
@@ -27,8 +27,6 @@
 #include "private.h"
 
 
-extern struct rte_pci_bus rte_pci_bus;
-
 #define SYSFS_PCI_DEVICES "/sys/bus/pci/devices"
 
 const char *rte_pci_get_sysfs_path(void)

@@ -435,6 +433,7 @@ struct rte_pci_bus rte_pci_bus = {
 		.unplug = pci_unplug,
 		.parse = pci_parse,
 		.get_iommu_class = rte_pci_get_iommu_class,
+		.dev_iterate = rte_pci_dev_iterate,
 	},
 	.device_list = TAILQ_HEAD_INITIALIZER(rte_pci_bus.device_list),
 	.driver_list = TAILQ_HEAD_INITIALIZER(rte_pci_bus.driver_list),

drivers/bus/pci/pci_params.c new file +53 -0

53 lines not on this page · raw message

drivers/bus/pci/private.h +25 -0

index 8ddd03e16..0e689fa74 100644
@@ -10,6 +10,8 @@
 #include <rte_pci.h>
 #include <rte_bus_pci.h>
 
+extern struct rte_pci_bus rte_pci_bus;
+
 struct rte_pci_driver;
 struct rte_pci_device;

@@ -166,4 +168,27 @@ rte_pci_match(const struct rte_pci_driver *pci_drv,
 enum rte_iova_mode
 rte_pci_get_iommu_class(void);
 
+/*
+ * Iterate over internal devices,
+ * matching any device against the provided
+ * string.
+ *
+ * @param start
+ *   Iteration starting point.
+ *
+ * @param str
+ *   Device string to match against.
+ *
+ * @param it
+ *   (unused) iterator structure.
+ *
+ * @return
+ *   A pointer to the next matching device if any.
+ *   NULL otherwise.
+ */
+void *
+rte_pci_dev_iterate(const void *start,
+		    const char *str,
+		    const struct rte_dev_iterator *it);
+
 #endif /* _PCI_PRIVATE_H_ */

Signature · 1 line

2.18.0

Gaetan Rivet

author

2018-08-30 13:42 UTC #2 lore raw

PATCH 2/13 bus/pci: add device matching field id

In reply to Gaetan Rivet

The PCI bus can now parse a matching field "id" as follows:

"bus=pci,id=0000:00:00.0"

"bus=pci,id=00:00.0"

Signed-off-by Gaetan Rivet <[email protected]>

---
 drivers/bus/pci/pci_params.c | 29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

drivers/bus/pci/pci_params.c +27 -2

index 0fde75803..7630d4845 100644
@@ -3,6 +3,7 @@
  */
 
 #include <rte_bus.h>
+#include <rte_bus_pci.h>
 #include <rte_dev.h>
 #include <rte_errno.h>
 #include <rte_kvargs.h>

@@ -11,21 +12,45 @@
 #include "private.h"
 
 enum pci_params {
+	RTE_PCI_PARAMS_ID,
 	RTE_PCI_PARAMS_MAX,
 };
 
 static const char * const pci_params_keys[] = {
+	[RTE_PCI_PARAMS_ID] = "id",
 	[RTE_PCI_PARAMS_MAX] = NULL,
 };
 
+static int
+pci_addr_kv_cmp(const char *key __rte_unused,
+		const char *value,
+		void *_addr2)
+{
+	struct rte_pci_addr _addr1;
+	struct rte_pci_addr *addr1 = &_addr1;
+	struct rte_pci_addr *addr2 = _addr2;
+
+	if (rte_pci_addr_parse(value, addr1))
+		return -1;
+	return -abs(rte_pci_addr_cmp(addr1, addr2));
+}
+
 static int
 pci_dev_match(const struct rte_device *dev,
 	      const void *_kvlist)
 {
 	const struct rte_kvargs *kvlist = _kvlist;
+	const struct rte_pci_device *pdev;
 
-	(void) dev;
-	(void) kvlist;
+	if (kvlist == NULL)
+		/* Empty string matches everything. */
+		return 0;
+	pdev = RTE_DEV_TO_PCI_CONST(dev);
+	/* if any field does not match. */
+	if (rte_kvargs_process(kvlist, "id",
+			       &pci_addr_kv_cmp,
+			       (void *)(intptr_t)&pdev->addr))
+		return 1;
 	return 0;
 }

Signature · 1 line

2.18.0

Gaetan Rivet

author

2018-08-30 13:42 UTC #3 lore raw

PATCH 3/13 bus/vdev: implement device iteration

In reply to Gaetan Rivet

Signed-off-by Gaetan Rivet <[email protected]>

---
 drivers/bus/vdev/Makefile       |  3 +-
 drivers/bus/vdev/meson.build    |  5 +++-
 drivers/bus/vdev/vdev.c         | 10 ++++---
 drivers/bus/vdev/vdev_params.c  | 51 +++++++++++++++++++++++++++++++++
 drivers/bus/vdev/vdev_private.h | 26 +++++++++++++++++
 5 files changed, 89 insertions(+), 6 deletions(-)
 create mode 100644 drivers/bus/vdev/vdev_params.c
 create mode 100644 drivers/bus/vdev/vdev_private.h

drivers/bus/vdev/Makefile +2 -1

index bd0bb8955..1f9cd7ebe 100644
@@ -19,8 +19,9 @@ EXPORT_MAP := rte_bus_vdev_version.map
 LIBABIVER := 1
 
 SRCS-y += vdev.c
+SRCS-y += vdev_params.c
 
-LDLIBS += -lrte_eal
+LDLIBS += -lrte_eal -lrte_kvargs
 
 #
 # Export include files

drivers/bus/vdev/meson.build +4 -1

index 2ee648b49..12605e5c7 100644
@@ -1,7 +1,10 @@
 # SPDX-License-Identifier: BSD-3-Clause
 # Copyright(c) 2017 Intel Corporation
 
-sources = files('vdev.c')
+sources = files('vdev.c',
+	'vdev_params.c')
 install_headers('rte_bus_vdev.h')
 
 allow_experimental_apis = true
+
+deps += ['kvargs']

drivers/bus/vdev/vdev.c +6 -4

index 6139dd551..e8518833d 100644
@@ -23,6 +23,7 @@
 
 #include "rte_bus_vdev.h"
 #include "vdev_logs.h"
+#include "vdev_private.h"
 
 #define VDEV_MP_KEY	"bus_vdev_mp"

@@ -493,9 +494,9 @@ vdev_probe(void)
 	return ret;
 }
 
-static struct rte_device *
-vdev_find_device(const struct rte_device *start, rte_dev_cmp_t cmp,
-		 const void *data)
+struct rte_device *
+rte_vdev_find_device(const struct rte_device *start, rte_dev_cmp_t cmp,
+		     const void *data)
 {
 	const struct rte_vdev_device *vstart;
 	struct rte_vdev_device *dev;

@@ -532,10 +533,11 @@ vdev_unplug(struct rte_device *dev)
 static struct rte_bus rte_vdev_bus = {
 	.scan = vdev_scan,
 	.probe = vdev_probe,
-	.find_device = vdev_find_device,
+	.find_device = rte_vdev_find_device,
 	.plug = vdev_plug,
 	.unplug = vdev_unplug,
 	.parse = vdev_parse,
+	.dev_iterate = rte_vdev_dev_iterate,
 };
 
 RTE_REGISTER_BUS(vdev, rte_vdev_bus);

drivers/bus/vdev/vdev_params.c new file +51 -0

51 lines not on this page · raw message

drivers/bus/vdev/vdev_private.h new file +26 -0

new file mode 100644
index 000000000..ba6dc48ff
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright 2018 Gaëtan Rivet
+ */
+
+#ifndef _VDEV_PRIVATE_H_
+#define _VDEV_PRIVATE_H_
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+struct rte_device *
+rte_vdev_find_device(const struct rte_device *start,
+		     rte_dev_cmp_t cmp,
+		     const void *data);
+
+void *
+rte_vdev_dev_iterate(const void *start,
+		     const char *str,
+		     const struct rte_dev_iterator *it);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* _VDEV_PRIVATE_H_ */

Signature · 1 line

2.18.0

Gaetan Rivet

author

2018-08-30 13:42 UTC #4 lore raw

PATCH 4/13 bus/vdev: add device matching field driver

In reply to Gaetan Rivet

The vdev bus parses a field "driver", matching
a vdev driver name with one passed as follows:

"bus=vdev,driver=xxxx"

Signed-off-by Gaetan Rivet <[email protected]>

---
 drivers/bus/vdev/vdev_params.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

drivers/bus/vdev/vdev_params.c +13 -2

index 842a4684e..2f55f451f 100644
@@ -4,6 +4,7 @@
 
 #include <rte_dev.h>
 #include <rte_bus.h>
+#include <rte_bus_vdev.h>
 #include <rte_kvargs.h>
 #include <rte_errno.h>

@@ -11,10 +12,12 @@
 #include "vdev_private.h"
 
 enum vdev_params {
+	RTE_VDEV_PARAMS_DRIVER,
 	RTE_VDEV_PARAMS_MAX,
 };
 
 static const char * const vdev_params_keys[] = {
+	[RTE_VDEV_PARAMS_DRIVER] = "driver",
 	[RTE_VDEV_PARAMS_MAX] = NULL,
 };

@@ -23,9 +26,17 @@ vdev_dev_match(const struct rte_device *dev,
 	       const void *_kvlist)
 {
 	const struct rte_kvargs *kvlist = _kvlist;
+	const struct rte_vdev_device *vdev;
 
-	(void) kvlist;
-	(void) dev;
+	if (kvlist == NULL)
+		/* Empty string matches everything. */
+		return 0;
+	vdev = RTE_DEV_TO_VDEV_CONST(dev);
+	/* if any field does not match. */
+	if (rte_kvargs_process(kvlist, "driver",
+		rte_kvargs_strcmp,
+		(void *)(intptr_t)vdev->device.driver->name))
+		return -1;
 	return 0;
 }

Signature · 1 line

2.18.0

Gaetan Rivet

author

2018-08-30 13:42 UTC #5 lore raw

PATCH 5/13 ethdev: add private generic device iterator

In reply to Gaetan Rivet

This iterator can be customized with a comparison function that will trigger a stopping condition.

It can be leveraged to write several different iterators that have similar but non-identical purposes.

It is private to librte_ethdev.

Signed-off-by Gaetan Rivet <[email protected]>

---
 lib/librte_ethdev/Makefile      |  1 +
 lib/librte_ethdev/eth_private.c | 31 +++++++++++++++++++++++++++++++
 lib/librte_ethdev/eth_private.h | 26 ++++++++++++++++++++++++++
 lib/librte_ethdev/meson.build   |  3 ++-
 4 files changed, 60 insertions(+), 1 deletion(-)
 create mode 100644 lib/librte_ethdev/eth_private.c
 create mode 100644 lib/librte_ethdev/eth_private.h

lib/librte_ethdev/Makefile +1 -0

index 0935a275e..3c1c92cb9 100644
@@ -18,6 +18,7 @@ EXPORT_MAP := rte_ethdev_version.map
 
 LIBABIVER := 10
 
+SRCS-y += eth_private.c
 SRCS-y += rte_ethdev.c
 SRCS-y += rte_flow.c
 SRCS-y += rte_tm.c

lib/librte_ethdev/eth_private.c new file +31 -0

new file mode 100644
index 000000000..d565568a0
@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2018 Gaëtan Rivet
+ */
+
+#include "rte_ethdev.h"
+#include "eth_private.h"
+
+struct rte_eth_dev *
+eth_find_device(const struct rte_eth_dev *start, rte_eth_cmp_t cmp,
+		const void *data)
+{
+	struct rte_eth_dev *edev;
+	ptrdiff_t idx;
+
+	/* Avoid Undefined Behaviour */
+	if (start != NULL &&
+	    (start < &rte_eth_devices[0] ||
+	     start > &rte_eth_devices[RTE_MAX_ETHPORTS]))
+		return NULL;
+	if (start != NULL)
+		idx = start - &rte_eth_devices[0] + 1;
+	else
+		idx = 0;
+	for (; idx < RTE_MAX_ETHPORTS; idx++) {
+		edev = &rte_eth_devices[idx];
+		if (cmp(edev, data) == 0)
+			return edev;
+	}
+	return NULL;
+}
+

lib/librte_ethdev/eth_private.h new file +26 -0

new file mode 100644
index 000000000..0f5c6d5c4
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2018 Gaëtan Rivet
+ */
+
+#ifndef _RTE_ETH_PRIVATE_H_
+#define _RTE_ETH_PRIVATE_H_
+
+#include "rte_ethdev.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/* Generic rte_eth_dev comparison function. */
+typedef int (*rte_eth_cmp_t)(const struct rte_eth_dev *, const void *);
+
+/* Generic rte_eth_dev iterator. */
+struct rte_eth_dev *
+eth_find_device(const struct rte_eth_dev *_start, rte_eth_cmp_t cmp,
+		const void *data);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* _RTE_ETH_PRIVATE_H_ */

lib/librte_ethdev/meson.build +2 -1

index 596cd0f39..372d3ca06 100644
@@ -4,7 +4,8 @@
 name = 'ethdev'
 version = 10
 allow_experimental_apis = true
-sources = files('ethdev_profile.c',
+sources = files('eth_private.c',
+	'ethdev_profile.c',
 	'rte_ethdev.c',
 	'rte_flow.c',
 	'rte_mtr.c',

Signature · 1 line

2.18.0

Andrew Rybchenko

2018-08-31 10:09 UTC #6 lore raw

Re: [PATCH v1 05/13] ethdev: add private generic device iterator

lib/librte_ethdev/eth_private.c

Quoted 35 lines · Gaetan Rivet

This iterator can be customized with a comparison function that will trigger a stopping condition.
It can be leveraged to write several different iterators that have similar but non-identical purposes.
It is private to librte_ethdev.
Signed-off-by Gaetan Rivet <[email protected]>
---
  lib/librte_ethdev/Makefile      |  1 +
  lib/librte_ethdev/eth_private.c | 31 +++++++++++++++++++++++++++++++
  lib/librte_ethdev/eth_private.h | 26 ++++++++++++++++++++++++++
  lib/librte_ethdev/meson.build   |  3 ++-
  4 files changed, 60 insertions(+), 1 deletion(-)
create mode 100644 lib/librte_ethdev/eth_private.c create mode 100644 lib/librte_ethdev/eth_private.h
lib/librte_ethdev/Makefile +1 -0
index 0935a275e..3c1c92cb9 100644
@@ -18,6 +18,7 @@ EXPORT_MAP := rte_ethdev_version.map
 
  LIBABIVER := 10
 
+SRCS-y += eth_private.c
  SRCS-y += rte_ethdev.c
  SRCS-y += rte_flow.c
  SRCS-y += rte_tm.c
lib/librte_ethdev/eth_private.c new file +0 -0
new file mode 100644
index 000000000..d565568a0

Just a nit
I think it is better to name it ethdev_private.c since we already have ethdev_profile.[ch] and it is about ethdev, not Ethernet.

Quoted 16 lines

lib/librte_ethdev/eth_private.c new file +15 -0

@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2018 Gaëtan Rivet
+ */
+
+#include "rte_ethdev.h"
+#include "eth_private.h"
+
+struct rte_eth_dev *
+eth_find_device(const struct rte_eth_dev *start, rte_eth_cmp_t cmp,
+		const void *data)
+{
+	struct rte_eth_dev *edev;
+	ptrdiff_t idx;
+
+	/* Avoid Undefined Behaviour */

lib/librte_ethdev/eth_private.c line 25

Quoted 10 lines

lib/librte_ethdev/eth_private.c new file +10 -0

+	if (start != NULL &&
+	    (start < &rte_eth_devices[0] ||
+	     start > &rte_eth_devices[RTE_MAX_ETHPORTS]))
+		return NULL;
+	if (start != NULL)
+		idx = start - &rte_eth_devices[0] + 1;
+	else
+		idx = 0;
+	for (; idx < RTE_MAX_ETHPORTS; idx++) {
+		edev = &rte_eth_devices[idx];

Shouldn't we limit it to valid ports only?
If no, I think it would be useful to highlight it in the function description that it iterates over all devices including unused.

lib/librte_ethdev/eth_private.h

Quoted 11 lines

lib/librte_ethdev/eth_private.c new file +6 -0
+		if (cmp(edev, data) == 0)
+			return edev;
+	}
+	return NULL;
+}
+
lib/librte_ethdev/eth_private.h new file +0 -0
new file mode 100644
index 000000000..0f5c6d5c4

ethdev_private.h

<...>

Gaëtan Rivet

author

2018-08-31 10:22 UTC #7 lore raw

Re: [PATCH v1 05/13] ethdev: add private generic device iterator

Quoted 30 lines · Andrew Rybchenko

Quoted 29 lines · Gaetan Rivet
This iterator can be customized with a comparison function that will trigger a stopping condition.
It can be leveraged to write several different iterators that have similar but non-identical purposes.
It is private to librte_ethdev.
Signed-off-by Gaetan Rivet <[email protected]>
---
  lib/librte_ethdev/Makefile      |  1 +
  lib/librte_ethdev/eth_private.c | 31 +++++++++++++++++++++++++++++++
  lib/librte_ethdev/eth_private.h | 26 ++++++++++++++++++++++++++
  lib/librte_ethdev/meson.build   |  3 ++-
  4 files changed, 60 insertions(+), 1 deletion(-)
create mode 100644 lib/librte_ethdev/eth_private.c create mode 100644 lib/librte_ethdev/eth_private.h
lib/librte_ethdev/Makefile +1 -0
index 0935a275e..3c1c92cb9 100644
@@ -18,6 +18,7 @@ EXPORT_MAP := rte_ethdev_version.map
  LIBABIVER := 10
+SRCS-y += eth_private.c
  SRCS-y += rte_ethdev.c
  SRCS-y += rte_flow.c
  SRCS-y += rte_tm.c
lib/librte_ethdev/eth_private.c +0 -0

Quoted 9 lines

lib/librte_ethdev/eth_private.c
Quoted 4 lines
new file mode 100644
index 000000000..d565568a0
lib/librte_ethdev/eth_private.c new file +0 -0
Just a nit
I think it is better to name it ethdev_private.c since we already have ethdev_profile.[ch] and it is about ethdev, not Ethernet.

Agreed.

Quoted 22 lines

lib/librte_ethdev/eth_private.c new file +21 -0

@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2018 Gaëtan Rivet
+ */
+
+#include "rte_ethdev.h"
+#include "eth_private.h"
+
+struct rte_eth_dev *
+eth_find_device(const struct rte_eth_dev *start, rte_eth_cmp_t cmp,
+		const void *data)
+{
+	struct rte_eth_dev *edev;
+	ptrdiff_t idx;
+
+	/* Avoid Undefined Behaviour */
+	if (start != NULL &&
+	    (start < &rte_eth_devices[0] ||
+	     start > &rte_eth_devices[RTE_MAX_ETHPORTS]))
+		return NULL;
+	if (start != NULL)
+		idx = start - &rte_eth_devices[0] + 1;

Quoted 9 lines

lib/librte_ethdev/eth_private.c line 25
Quoted 4 lines
lib/librte_ethdev/eth_private.c new file +4 -0
+	else
+		idx = 0;
+	for (; idx < RTE_MAX_ETHPORTS; idx++) {
+		edev = &rte_eth_devices[idx];
Shouldn't we limit it to valid ports only?
If no, I think it would be useful to highlight it in the function description that it iterates over all devices including unused.

I'm undecided about it, I wanted eth_find_device to be and internal operator in the most generic way, allowing ethdev layer to move from manually iterating to using this, to introduce the probable future evolution of not using an array of rte_eth_devices.

So I thought of this operator as something stateless, only looking at currently available memory and letting the comparison select what is useful. Maybe for example this operator would be used to find the next unused device, etc.

My doubts is that this is kind of future-proofing the design, something that I don't think is good practice.

So, if you prefer something that takes care of state, as far as devargs parsing is concerned, I'm ok with that. Otherwise it can stay that way, UNUSED devices are taken care of afterward.

Quoted 5 lines

Quoted 5 lines
lib/librte_ethdev/eth_private.c new file +5 -0
+		if (cmp(edev, data) == 0)
+			return edev;
+	}
+	return NULL;
+}

Quoted 8 lines

lib/librte_ethdev/eth_private.h
Quoted 6 lines
lib/librte_ethdev/eth_private.c new file +1 -0
+
lib/librte_ethdev/eth_private.h new file +0 -0
new file mode 100644
index 000000000..0f5c6d5c4
ethdev_private.h

sure.

Quoted 2 lines

<...>

Signature · 2 lines

Gaëtan Rivet
6WIND

Gaetan Rivet

author

2018-08-30 13:42 UTC #8 lore raw

PATCH 6/13 ethdev: register ether layer as a class

In reply to Gaetan Rivet

Signed-off-by Gaetan Rivet <[email protected]>

---
 lib/librte_ethdev/Makefile        |  3 +-
 lib/librte_ethdev/meson.build     |  1 +
 lib/librte_ethdev/rte_class_eth.c | 79 +++++++++++++++++++++++++++++++
 3 files changed, 82 insertions(+), 1 deletion(-)
 create mode 100644 lib/librte_ethdev/rte_class_eth.c

lib/librte_ethdev/Makefile +2 -1

index 3c1c92cb9..91899b083 100644
@@ -12,7 +12,7 @@ CFLAGS += -DALLOW_EXPERIMENTAL_API
 CFLAGS += -O3
 CFLAGS += $(WERROR_FLAGS)
 LDLIBS += -lrte_net -lrte_eal -lrte_mempool -lrte_ring
-LDLIBS += -lrte_mbuf
+LDLIBS += -lrte_mbuf -lrte_kvargs
 
 EXPORT_MAP := rte_ethdev_version.map

@@ -20,6 +20,7 @@ LIBABIVER := 10
 
 SRCS-y += eth_private.c
 SRCS-y += rte_ethdev.c
+SRCS-y += rte_class_eth.c
 SRCS-y += rte_flow.c
 SRCS-y += rte_tm.c
 SRCS-y += rte_mtr.c

lib/librte_ethdev/meson.build +1 -0

index 372d3ca06..8d2e2bf4e 100644
@@ -6,6 +6,7 @@ version = 10
 allow_experimental_apis = true
 sources = files('eth_private.c',
 	'ethdev_profile.c',
+	'rte_class_eth.c',
 	'rte_ethdev.c',
 	'rte_flow.c',
 	'rte_mtr.c',

lib/librte_ethdev/rte_class_eth.c new file +79 -0

79 lines not on this page · raw message

Signature · 1 line

2.18.0

Andrew Rybchenko

Acked-by on v1 6/13

2018-08-31 10:09 UTC #9 lore raw

Re: [PATCH v1 06/13] ethdev: register ether layer as a class

Quoted 1 line · Gaetan Rivet

Signed-off-by Gaetan Rivet <[email protected]>

Acked-by Andrew Rybchenko <[email protected]>

Gaetan Rivet

author

2018-08-30 13:42 UTC #10 lore raw

PATCH 7/13 ethdev: add device matching field name

In reply to Gaetan Rivet

The eth device class can now parse a field name,
matching the eth_dev name with one passed as

"class=eth,name=xxxxxx"

Signed-off-by Gaetan Rivet <[email protected]>

---
 lib/librte_ethdev/rte_class_eth.c | 7 +++++++
 1 file changed, 7 insertions(+)

lib/librte_ethdev/rte_class_eth.c +7 -0

index 32c736d32..d8d8e8845 100644
@@ -15,10 +15,12 @@
 #include "eth_private.h"
 
 enum eth_params {
+	RTE_ETH_PARAMS_NAME,
 	RTE_ETH_PARAMS_MAX,
 };
 
 static const char * const eth_params_keys[] = {
+	[RTE_ETH_PARAMS_NAME] = "name",
 	[RTE_ETH_PARAMS_MAX] = NULL,
 };

@@ -39,6 +41,7 @@ eth_dev_match(const struct rte_eth_dev *edev,
 {
 	const struct eth_dev_match_arg *arg = _arg;
 	const struct rte_kvargs *kvlist = arg->kvlist;
+	struct rte_eth_dev_data *data;
 
 	if (edev->state == RTE_ETH_DEV_UNUSED)
 		return -1;

@@ -47,6 +50,10 @@ eth_dev_match(const struct rte_eth_dev *edev,
 	if (kvlist == NULL)
 		/* Empty string matches everything. */
 		return 0;
+	data = edev->data;
+	if (rte_kvargs_process(kvlist, "name",
+			rte_kvargs_strcmp, data->name))
+		return -1;
 	return 0;
 }

Signature · 1 line

2.18.0

Andrew Rybchenko

Acked-by on v1 7/13

2018-08-31 10:10 UTC #11 lore raw

Re: [PATCH v1 07/13] ethdev: add device matching field name

Quoted 6 lines · Gaetan Rivet

The eth device class can now parse a field name,
matching the eth_dev name with one passed as
"class=eth,name=xxxxxx"
Signed-off-by Gaetan Rivet <[email protected]>

Acked-by Andrew Rybchenko <[email protected]>

Gaetan Rivet

author

2018-08-30 13:42 UTC #12 lore raw

PATCH 8/13 app/testpmd: add show device command

In reply to Gaetan Rivet

A new interactive command is offered:

show device <device description>

This commands lists all rte_device element matching the device description. e.g.:

show device bus=pci
show device bus=vdev
show device bus=vdev/class=eth
show device bus=vdev,driver=net_ring/class=eth
show device bus=vdev/class=eth,name=net_ring0

These devices may not be otherwise useful, some buses will spawn devices to keep track of their assets without having a driver to use them.

Signed-off-by Gaetan Rivet <[email protected]>

Acked-by Bernard Iremonger <[email protected]>

---
 app/test-pmd/cmdline.c                      | 54 +++++++++++++++++++++
 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 24 +++++++++
 2 files changed, 78 insertions(+)

app/test-pmd/cmdline.c +54 -0

index 589121d69..a7c0e622a 100644
@@ -228,6 +228,9 @@ static void cmd_help_long_parsed(void *parsed_result,
                         "show port tm node stats (port_id) (node_id) (clear)\n"
                         "       Display the port TM node stats.\n\n"
 
+			"show device (device_string)\n"
+			"       Display devices matching the device string.\n\n"
+
 		);
 	}

@@ -7117,6 +7120,56 @@ cmdline_parse_inst_t cmd_showportall = {
 	},
 };
 
+/* *** SHOW DEVICE INFO *** */
+struct cmd_showdevice_result {
+	cmdline_fixed_string_t show;
+	cmdline_fixed_string_t device;
+	cmdline_fixed_string_t filter;
+};
+
+static void
+cmd_showdevice_dump_device(const struct rte_device *dev)
+{
+	const struct rte_driver *drv = dev->driver;
+
+	printf("0x%p: %s:%s\n", (const void *)dev, dev->name,
+		drv ? drv->name : "<nil>");
+}
+
+static void cmd_showdevice_parsed(void *parsed_result,
+				__attribute__((unused)) struct cmdline *cl,
+				__attribute__((unused)) void *data)
+{
+	struct cmd_showdevice_result *res = parsed_result;
+	struct rte_dev_iterator it;
+	const struct rte_device *dev;
+
+	RTE_DEV_FOREACH(dev, res->filter, &it)
+		cmd_showdevice_dump_device(dev);
+}
+
+cmdline_parse_token_string_t cmd_showdevice_show =
+	TOKEN_STRING_INITIALIZER(struct cmd_showdevice_result,
+				show, "show");
+cmdline_parse_token_string_t cmd_showdevice_device =
+	TOKEN_STRING_INITIALIZER(struct cmd_showdevice_result,
+				device, "device");
+cmdline_parse_token_string_t cmd_showdevice_filter =
+	TOKEN_STRING_INITIALIZER(struct cmd_showdevice_result,
+			filter, NULL);
+
+cmdline_parse_inst_t cmd_showdevice = {
+	.f = cmd_showdevice_parsed,
+	.data = NULL,
+	.help_str = "show device <device string>",
+	.tokens = {
+		(void *)&cmd_showdevice_show,
+		(void *)&cmd_showdevice_device,
+		(void *)&cmd_showdevice_filter,
+		NULL,
+	},
+};
+
 /* *** SHOW PORT INFO *** */
 struct cmd_showport_result {
 	cmdline_fixed_string_t show;

@@ -17610,6 +17663,7 @@ cmdline_parse_ctx_t main_ctx[] = {
 	(cmdline_parse_inst_t *)&cmd_help_long,
 	(cmdline_parse_inst_t *)&cmd_quit,
 	(cmdline_parse_inst_t *)&cmd_load_from_file,
+	(cmdline_parse_inst_t *)&cmd_showdevice,
 	(cmdline_parse_inst_t *)&cmd_showport,
 	(cmdline_parse_inst_t *)&cmd_showqueue,
 	(cmdline_parse_inst_t *)&cmd_showportall,

doc/guides/testpmd_app_ug/testpmd_funcs.rst +24 -0

index dde205a2b..417072cce 100644
@@ -2663,6 +2663,30 @@ set the traffic management default hierarchy on the port::
 
    testpmd> set port tm hierarchy default (port_id)
 
+Device functions
+----------------
+
+Show devices
+~~~~~~~~~~~~
+
+Display any registered devices::
+
+   testpmd> show device <device_string>
+
+where:
+
+* ``device_string``: Device description string, of the format
+
+  layer[/layer[/layer]]
+
+  where one layer is in the form
+
+  layer_key=layer_name[,key1=value1[,...]]
+
+  Valid layer keys are ``bus`` and ``class``.
+  Their respective values is defined by registered ``bus`` and ``class``
+  drivers.
+
 Filter Functions
 ----------------

Signature · 1 line

2.18.0

Gaetan Rivet

author

2018-08-30 13:42 UTC #13 lore raw

PATCH 9/13 bus/pci: pre-process declarative PCI devargs

In reply to Gaetan Rivet

The new devargs format does not recognize a particular device name. Each bus uses its specific format.

Instead of introducing a new bus API, process those devargs privately for the moment. Prepare them for matching during scan against the bus devices.

Signed-off-by Gaetan Rivet <[email protected]>

---
 drivers/bus/pci/bsd/pci.c    |  5 ++++
 drivers/bus/pci/linux/pci.c  |  5 ++++
 drivers/bus/pci/pci_params.c | 51 ++++++++++++++++++++++++++++++++++++
 drivers/bus/pci/private.h    | 16 +++++++++++
 4 files changed, 77 insertions(+)

drivers/bus/pci/bsd/pci.c +5 -0

index 655b34b7e..046cd11d5 100644
@@ -327,6 +327,7 @@ pci_scan_one(int dev_pci_fd, struct pci_conf *conf)
 int
 rte_pci_scan(void)
 {
+	struct rte_devargs *devargs;
 	int fd;
 	unsigned dev_count = 0;
 	struct pci_conf matches[16];

@@ -342,6 +343,10 @@ rte_pci_scan(void)
 	if (!rte_eal_has_pci())
 		return 0;
 
+	RTE_EAL_DEVARGS_FOREACH("pci", devargs)
+		if (rte_pci_devargs_prepare(devargs))
+			continue;
+
 	fd = open("/dev/pci", O_RDONLY);
 	if (fd < 0) {
 		RTE_LOG(ERR, EAL, "%s(): error opening /dev/pci\n", __func__);

drivers/bus/pci/linux/pci.c +5 -0

index 04648ac93..12f246089 100644
@@ -429,6 +429,7 @@ parse_pci_addr_format(const char *buf, int bufsize, struct rte_pci_addr *addr)
 int
 rte_pci_scan(void)
 {
+	struct rte_devargs *devargs;
 	struct dirent *e;
 	DIR *dir;
 	char dirname[PATH_MAX];

@@ -438,6 +439,10 @@ rte_pci_scan(void)
 	if (!rte_eal_has_pci())
 		return 0;
 
+	RTE_EAL_DEVARGS_FOREACH("pci", devargs)
+		if (rte_pci_devargs_prepare(devargs))
+			continue;
+
 #ifdef VFIO_PRESENT
 	if (!pci_vfio_is_enabled())
 		RTE_LOG(DEBUG, EAL, "VFIO PCI modules not loaded\n");

drivers/bus/pci/pci_params.c +51 -0

63 lines not on this page · raw message

drivers/bus/pci/private.h +16 -0

index 0e689fa74..9beb24c6a 100644
@@ -191,4 +191,20 @@ rte_pci_dev_iterate(const void *start,
 		    const char *str,
 		    const struct rte_dev_iterator *it);
 
+/*
+ * Prepare a devargs meant for this bus.
+ * This function is only used for a transitory period,
+ * to translate the new devargs format in one
+ * compatible with the old form.
+ *
+ * @param da
+ *   Devargs to process.
+ *
+ * @return
+ *   0 on success.
+ *   <0 on error.
+ */
+int
+rte_pci_devargs_prepare(struct rte_devargs *da);
+
 #endif /* _PCI_PRIVATE_H_ */

Signature · 1 line

2.18.0

Gaetan Rivet

author

2018-08-30 13:42 UTC #14 lore raw

PATCH 10/13 bus/vdev: pre-process declarative vdev devargs

In reply to Gaetan Rivet

The new devargs format does not recognize a particular device name. Each bus uses its specific format.

Process each devargs privately prior to attempting a bus scan. Prepare them if they are using the new devargs format.

Signed-off-by Gaetan Rivet <[email protected]>

---
 drivers/bus/vdev/vdev.c         | 10 ++++---
 drivers/bus/vdev/vdev_params.c  | 50 +++++++++++++++++++++++++++++++++
 drivers/bus/vdev/vdev_private.h |  6 ++++
 3 files changed, 62 insertions(+), 4 deletions(-)

drivers/bus/vdev/vdev.c +6 -4

index e8518833d..f2dace245 100644
@@ -115,8 +115,8 @@ rte_vdev_remove_custom_scan(rte_vdev_scan_callback callback, void *user_arg)
 	return 0;
 }
 
-static int
-vdev_parse(const char *name, void *addr)
+int
+rte_vdev_parse(const char *name, void *addr)
 {
 	struct rte_vdev_driver **out = addr;
 	struct rte_vdev_driver *driver = NULL;

@@ -148,7 +148,7 @@ vdev_probe_all_drivers(struct rte_vdev_device *dev)
 	VDEV_LOG(DEBUG, "Search driver %s to probe device %s", name,
 		rte_vdev_device_name(dev));
 
-	if (vdev_parse(name, &driver))
+	if (rte_vdev_parse(name, &driver))
 		return -1;
 	dev->device.driver = &driver->driver;
 	ret = driver->probe(dev);

@@ -443,6 +443,8 @@ vdev_scan(void)
 
 	/* for virtual devices we scan the devargs_list populated via cmdline */
 	RTE_EAL_DEVARGS_FOREACH("vdev", devargs) {
+		if (rte_vdev_devargs_prepare(devargs))
+			continue;
 
 		dev = calloc(1, sizeof(*dev));
 		if (!dev)

@@ -536,7 +538,7 @@ static struct rte_bus rte_vdev_bus = {
 	.find_device = rte_vdev_find_device,
 	.plug = vdev_plug,
 	.unplug = vdev_unplug,
-	.parse = vdev_parse,
+	.parse = rte_vdev_parse,
 	.dev_iterate = rte_vdev_dev_iterate,
 };

drivers/bus/vdev/vdev_params.c +50 -0

64 lines not on this page · raw message

drivers/bus/vdev/vdev_private.h +6 -0

index ba6dc48ff..da57b84dd 100644
@@ -19,6 +19,12 @@ rte_vdev_dev_iterate(const void *start,
 		     const char *str,
 		     const struct rte_dev_iterator *it);
 
+int
+rte_vdev_parse(const char *name, void *addr);
+
+int
+rte_vdev_devargs_prepare(struct rte_devargs *da);
+
 #ifdef __cplusplus
 }
 #endif

Signature · 1 line

2.18.0

Gaetan Rivet

author

2018-08-30 13:42 UTC #15 lore raw

PATCH 11/13 bus/pci: process declarative PCI devargs

In reply to Gaetan Rivet

Introduce the facility to process future PCI parameters.

Once the matching between PCI devices and devargs has been done, it is possible to process each devargs. New parameters would have the PCI device handle to work with when parsing the device (bus specific) parameters.

Signed-off-by Gaetan Rivet <[email protected]>

---
 drivers/bus/pci/pci_common.c |  3 +++
 drivers/bus/pci/pci_params.c | 10 ++++++++++
 drivers/bus/pci/private.h    | 13 +++++++++++++
 3 files changed, 26 insertions(+)

drivers/bus/pci/pci_common.c +3 -0

index c7695d108..900cd9090 100644
@@ -247,6 +247,9 @@ pci_probe_all_drivers(struct rte_pci_device *dev)
 	if (dev->driver != NULL)
 		return 0;
 
+	if (rte_pci_devargs_process(dev) < 0)
+		return -1;
+
 	FOREACH_DRIVER_ON_PCIBUS(dr) {
 		rc = rte_pci_probe_one_driver(dr, dev);
 		if (rc < 0)

drivers/bus/pci/pci_params.c +10 -0

index a09af3b1c..f34bf3da9 100644
@@ -127,3 +127,13 @@ rte_pci_devargs_prepare(struct rte_devargs *devargs)
 	rte_kvargs_free(kvargs);
 	return ret;
 }
+
+int
+rte_pci_devargs_process(struct rte_pci_device *pdev)
+{
+	/* For the moment, no PCI param
+	 * needs to be processed.
+	 */
+	(void) pdev;
+	return 0;
+}

drivers/bus/pci/private.h +13 -0

index 9beb24c6a..06dc85e85 100644
@@ -207,4 +207,17 @@ rte_pci_dev_iterate(const void *start,
 int
 rte_pci_devargs_prepare(struct rte_devargs *da);
 
+/*
+ * Process the device devargs, if any.
+ *
+ * @param pdev
+ *   PCI device
+ *
+ * @return
+ *   0 on success.
+ *   <0 on error.
+ */
+int
+rte_pci_devargs_process(struct rte_pci_device *pdev);
+
 #endif /* _PCI_PRIVATE_H_ */

Signature · 1 line

2.18.0

Stephen Hemminger

2018-08-30 16:15 UTC #16 lore raw

Re: [PATCH v1 11/13] bus/pci: process declarative PCI devargs

drivers/bus/pci/pci_params.c rte_pci_devargs_prepare(struct rte_devargs *devargs) · line 139

Quoted 9 lines · On Thu, 30 Aug 2018 15:42:02 +0200 Gaetan Rivet <[email protected]> wrote:

drivers/bus/pci/pci_params.c +9 -0

+int
+rte_pci_devargs_process(struct rte_pci_device *pdev)
+{
+	/* For the moment, no PCI param
+	 * needs to be processed.
+	 */
+	(void) pdev;
+	return 0;
+}

Use __rte_unused rather than the (void) cast?

Gaëtan Rivet

author

2018-08-30 16:37 UTC #17 lore raw

Re: [PATCH v1 11/13] bus/pci: process declarative PCI devargs

Quoted 14 lines · Stephen Hemminger

drivers/bus/pci/pci_params.c rte_pci_devargs_prepare(struct rte_devargs *devargs) · line 139
Quoted 9 lines · On Thu, 30 Aug 2018 15:42:02 +0200 Gaetan Rivet <[email protected]> wrote:
drivers/bus/pci/pci_params.c +9 -0
+int
+rte_pci_devargs_process(struct rte_pci_device *pdev)
+{
+	/* For the moment, no PCI param
+	 * needs to be processed.
+	 */
+	(void) pdev;
+	return 0;
+}
Use __rte_unused rather than the (void) cast?

Ok.

I was wondering whether we should keep this patch. Its only use is to mark the proper place where this function, once it has become needed, would need to be called.

PCI bus does not have any parameters configuring devices, this function will only be needed once some have been added.

Signature · 2 lines

Gaëtan Rivet
6WIND

Gaetan Rivet

author

2018-08-30 13:42 UTC #18 lore raw

PATCH 12/13 ethdev: process declarative eth devargs

In reply to Gaetan Rivet

Process the class-specific arguments in a devargs.
This processing takes the form of setting the proper eth_dev fields when relevant.

Signed-off-by Gaetan Rivet <[email protected]>

---
 lib/librte_ethdev/eth_private.h   |  5 +++
 lib/librte_ethdev/rte_class_eth.c | 62 +++++++++++++++++++++++++++++++
 lib/librte_ethdev/rte_ethdev.c    |  7 ++++
 3 files changed, 74 insertions(+)

lib/librte_ethdev/eth_private.h +5 -0

index 0f5c6d5c4..c0c065165 100644
@@ -19,6 +19,11 @@ struct rte_eth_dev *
 eth_find_device(const struct rte_eth_dev *_start, rte_eth_cmp_t cmp,
 		const void *data);
 
+/* Generic rte_eth_dev parameters processor. */
+int
+rte_eth_dev_args_parse(struct rte_eth_dev *eth_dev,
+		       struct rte_devargs *da);
+
 #ifdef __cplusplus
 }
 #endif

lib/librte_ethdev/rte_class_eth.c +62 -0

80 lines not on this page · raw message

lib/librte_ethdev/rte_ethdev.c +7 -0

index 4c3202505..633c7ee82 100644
@@ -41,6 +41,7 @@
 #include "rte_ethdev.h"
 #include "rte_ethdev_driver.h"
 #include "ethdev_profile.h"
+#include "eth_private.h"
 
 int rte_eth_dev_logtype;

@@ -3496,6 +3497,12 @@ rte_eth_dev_create(struct rte_device *device, const char *name,
 		}
 	}
 
+	retval = rte_eth_dev_args_parse(ethdev, device->devargs);
+	if (retval) {
+		RTE_LOG(ERR, EAL, "ethdev parsing failed");
+		goto probe_failed;
+	}
+
 	retval = ethdev_init(ethdev, init_params);
 	if (retval) {
 		RTE_LOG(ERR, EAL, "ethdev initialisation failed");

Signature · 1 line

2.18.0

Andrew Rybchenko

2018-08-31 10:10 UTC #19 lore raw

Re: [PATCH v1 12/13] ethdev: process declarative eth devargs

Quoted 31 lines · Gaetan Rivet

Process the class-specific arguments in a devargs.
This processing takes the form of setting the proper eth_dev fields when relevant.

Signed-off-by Gaetan Rivet <[email protected]>

---
  lib/librte_ethdev/eth_private.h   |  5 +++
  lib/librte_ethdev/rte_class_eth.c | 62 +++++++++++++++++++++++++++++++
  lib/librte_ethdev/rte_ethdev.c    |  7 ++++
  3 files changed, 74 insertions(+)

lib/librte_ethdev/eth_private.h +5 -0

index 0f5c6d5c4..c0c065165 100644
@@ -19,6 +19,11 @@ struct rte_eth_dev *
  eth_find_device(const struct rte_eth_dev *_start, rte_eth_cmp_t cmp,
  		const void *data);
 
+/* Generic rte_eth_dev parameters processor. */
+int
+rte_eth_dev_args_parse(struct rte_eth_dev *eth_dev,
+		       struct rte_devargs *da);
+
  #ifdef __cplusplus
  }
  #endif

lib/librte_ethdev/rte_class_eth.c +0 -0

index d8d8e8845..18fdef605 100644

<...>

lib/librte_ethdev/rte_class_eth.c eth_dev_iterate(const void *start, · line 102

Quoted 11 lines

lib/librte_ethdev/rte_class_eth.c +7 -0

@@ -79,6 +93,54 @@ eth_dev_iterate(const void *start,
  	return edev;
  }
 
+static int
+eth_dev_set_name(struct rte_eth_dev *edev,
+		 const char *value)
+{
+	snprintf(edev->data->name,
+		 sizeof(edev->data->name),
+		 "%s", value);

strlcpy()?
Shouldn't we return error if name does fit in name buffer?

Quoted 26 lines

lib/librte_ethdev/rte_class_eth.c +26 -0

+	return 0;
+}
+
+static int
+ethdev_args_process(const char *key,
+		    const char *value,
+		    void *_edev)
+{
+	static eth_dev_set_t eth_dev_set[] = {
+		[RTE_ETH_PARAMS_NAME] = eth_dev_set_name,
+		[RTE_ETH_PARAMS_MAX] = NULL,
+	};
+	struct rte_eth_dev *edev = _edev;
+	int param;
+
+	param = ethdev_param_id(key);
+	if (eth_dev_set[param])
+		return eth_dev_set[param](edev, value);
+	return 0;
+}
+
+int
+rte_eth_dev_args_parse(struct rte_eth_dev *edev,
+		       struct rte_devargs *devargs)
+{
+	struct rte_kvargs *kvargs = NULL;

lib/librte_ethdev/rte_class_eth.c eth_dev_iterate(const void *start, · line 138

Quoted 10 lines

lib/librte_ethdev/rte_class_eth.c +10 -0

+
+	if (devargs == NULL || devargs->cls_str == NULL)
+		return 0;
+
+	kvargs = rte_kvargs_parse_delim(devargs->cls_str, eth_params_keys, "/");
+	if (kvargs == NULL) {
+		RTE_LOG(ERR, EAL, "cannot parse argument list\n");
+		return -EINVAL;
+	}
+	if (rte_kvargs_process(kvargs, NULL, ethdev_args_process, edev))

Shouldn't we free kvargs here as well?

Quoted 8 lines

lib/librte_ethdev/rte_class_eth.c +5 -0

+		return -1;
+	rte_kvargs_free(kvargs);
+	return 0;
+}
+
  struct rte_class rte_class_eth = {
  	.dev_iterate = eth_dev_iterate,
  };

<...>

Gaëtan Rivet

author

2018-08-31 12:17 UTC #20 lore raw

Re: [PATCH v1 12/13] ethdev: process declarative eth devargs

Quoted 34 lines · Andrew Rybchenko

Quoted 30 lines · Gaetan Rivet

Process the class-specific arguments in a devargs.
This processing takes the form of setting the proper eth_dev fields when relevant.

Signed-off-by Gaetan Rivet <[email protected]>

---
  lib/librte_ethdev/eth_private.h   |  5 +++
  lib/librte_ethdev/rte_class_eth.c | 62 +++++++++++++++++++++++++++++++
  lib/librte_ethdev/rte_ethdev.c    |  7 ++++
  3 files changed, 74 insertions(+)

lib/librte_ethdev/eth_private.h +5 -0

index 0f5c6d5c4..c0c065165 100644
@@ -19,6 +19,11 @@ struct rte_eth_dev *
  eth_find_device(const struct rte_eth_dev *_start, rte_eth_cmp_t cmp,
  		const void *data);
+/* Generic rte_eth_dev parameters processor. */
+int
+rte_eth_dev_args_parse(struct rte_eth_dev *eth_dev,
+		       struct rte_devargs *da);
+
  #ifdef __cplusplus
  }
  #endif

lib/librte_ethdev/rte_class_eth.c +0 -0

index d8d8e8845..18fdef605 100644

<...>

Quoted 12 lines

lib/librte_ethdev/rte_class_eth.c eth_dev_iterate(const void *start, · line 101
Quoted 10 lines
lib/librte_ethdev/rte_class_eth.c +7 -0
@@ -79,6 +93,54 @@ eth_dev_iterate(const void *start,
  	return edev;
  }
+static int
+eth_dev_set_name(struct rte_eth_dev *edev,
+		 const char *value)
+{
+	snprintf(edev->data->name,
+		 sizeof(edev->data->name),
+		 "%s", value);
strlcpy()?

Ok.

Quoted 1 line

Shouldn't we return error if name does fit in name buffer?

Good idea.

Quoted 31 lines

lib/librte_ethdev/rte_class_eth.c +30 -0

+	return 0;
+}
+
+static int
+ethdev_args_process(const char *key,
+		    const char *value,
+		    void *_edev)
+{
+	static eth_dev_set_t eth_dev_set[] = {
+		[RTE_ETH_PARAMS_NAME] = eth_dev_set_name,
+		[RTE_ETH_PARAMS_MAX] = NULL,
+	};
+	struct rte_eth_dev *edev = _edev;
+	int param;
+
+	param = ethdev_param_id(key);
+	if (eth_dev_set[param])
+		return eth_dev_set[param](edev, value);
+	return 0;
+}
+
+int
+rte_eth_dev_args_parse(struct rte_eth_dev *edev,
+		       struct rte_devargs *devargs)
+{
+	struct rte_kvargs *kvargs = NULL;
+
+	if (devargs == NULL || devargs->cls_str == NULL)
+		return 0;
+

Quoted 9 lines

lib/librte_ethdev/rte_class_eth.c eth_dev_iterate(const void *start, · line 138
Quoted 6 lines
lib/librte_ethdev/rte_class_eth.c +6 -0
+	kvargs = rte_kvargs_parse_delim(devargs->cls_str, eth_params_keys, "/");
+	if (kvargs == NULL) {
+		RTE_LOG(ERR, EAL, "cannot parse argument list\n");
+		return -EINVAL;
+	}
+	if (rte_kvargs_process(kvargs, NULL, ethdev_args_process, edev))
Shouldn't we free kvargs here as well?

Indeed, silly mistake.
Thanks

Quoted 10 lines

Quoted 8 lines

lib/librte_ethdev/rte_class_eth.c +5 -0

+		return -1;
+	rte_kvargs_free(kvargs);
+	return 0;
+}
+
  struct rte_class rte_class_eth = {
  	.dev_iterate = eth_dev_iterate,
  };

<...>

Signature · 2 lines

Gaëtan Rivet
6WIND

Gaetan Rivet

author

2018-08-30 13:42 UTC #21 lore raw

PATCH 13/13 eal: add generic dev parameter

In reply to Gaetan Rivet

Add the --dev parameter to the EAL.
This new parameter takes a generic device declaration as argument.

It uses the new devargs parsing API.

Signed-off-by Gaetan Rivet <[email protected]>

---
 lib/librte_eal/common/eal_common_devargs.c |  4 +++
 lib/librte_eal/common/eal_common_options.c | 36 +++++++++++++++++++---
 lib/librte_eal/common/eal_options.h        |  2 ++
 3 files changed, 37 insertions(+), 5 deletions(-)

lib/librte_eal/common/eal_common_devargs.c +4 -0

index dac2402a4..f1f4628db 100644
@@ -219,6 +219,10 @@ rte_devargs_parse(struct rte_devargs *da, const char *dev)
 	if (da == NULL)
 		return -EINVAL;
 
+	if (strncmp(dev, "bus=", 4) == 0 ||
+	    strncmp(dev, "class=", 6) == 0)
+		return rte_devargs_layers_parse(da, dev);
+
 	/* Retrieve eventual bus info */
 	do {
 		devname = dev;

lib/librte_eal/common/eal_common_options.c +31 -5

index dd5f97402..4a8b87676 100644
@@ -54,6 +54,7 @@ const struct option
 eal_long_options[] = {
 	{OPT_BASE_VIRTADDR,     1, NULL, OPT_BASE_VIRTADDR_NUM    },
 	{OPT_CREATE_UIO_DEV,    0, NULL, OPT_CREATE_UIO_DEV_NUM   },
+	{OPT_DEV,               1, NULL, OPT_DEV_NUM              },
 	{OPT_FILE_PREFIX,       1, NULL, OPT_FILE_PREFIX_NUM      },
 	{OPT_HELP,              0, NULL, OPT_HELP_NUM             },
 	{OPT_HUGE_DIR,          1, NULL, OPT_HUGE_DIR_NUM         },

@@ -111,6 +112,7 @@ TAILQ_HEAD(device_option_list, device_option);
 struct device_option {
 	TAILQ_ENTRY(device_option) next;
 
+	int new;
 	enum rte_devtype type;
 	char arg[];
 };

@@ -123,7 +125,8 @@ static int mem_parsed;
 static int core_parsed;
 
 static int
-eal_option_device_add(enum rte_devtype type, const char *optarg)
+eal_option_device_add(enum rte_devtype type, const char *optarg,
+		      int new)
 {
 	struct device_option *devopt;
 	size_t optlen;

@@ -137,6 +140,7 @@ eal_option_device_add(enum rte_devtype type, const char *optarg)
 	}
 
 	devopt->type = type;
+	devopt->new = new;
 	ret = snprintf(devopt->arg, optlen, "%s", optarg);
 	if (ret < 0) {
 		RTE_LOG(ERR, EAL, "Unable to copy device option\n");

@@ -156,7 +160,22 @@ eal_option_device_parse(void)
 
 	TAILQ_FOREACH_SAFE(devopt, &devopt_list, next, tmp) {
 		if (ret == 0) {
-			ret = rte_devargs_add(devopt->type, devopt->arg);
+			if (devopt->new) {
+				struct rte_devargs *da;
+
+				da = calloc(1, sizeof(*da));
+				ret = rte_devargs_parse(da, devopt->arg);
+				if (ret) {
+					free(da);
+				} else {
+					ret = rte_devargs_insert(da);
+					if (ret)
+						free(da);
+				}
+			} else {
+				ret = rte_devargs_add(devopt->type,
+						      devopt->arg);
+			}
 			if (ret)
 				RTE_LOG(ERR, EAL, "Unable to parse device '%s'\n",
 					devopt->arg);

@@ -1088,7 +1107,7 @@ eal_parse_common_option(int opt, const char *optarg,
 		if (w_used)
 			goto bw_used;
 		if (eal_option_device_add(RTE_DEVTYPE_BLACKLISTED_PCI,
-				optarg) < 0) {
+				optarg, 0) < 0) {
 			return -1;
 		}
 		b_used = 1;

@@ -1098,7 +1117,7 @@ eal_parse_common_option(int opt, const char *optarg,
 		if (b_used)
 			goto bw_used;
 		if (eal_option_device_add(RTE_DEVTYPE_WHITELISTED_PCI,
-				optarg) < 0) {
+				optarg, 0) < 0) {
 			return -1;
 		}
 		w_used = 1;

@@ -1234,9 +1253,16 @@ eal_parse_common_option(int opt, const char *optarg,
 		}
 		break;
 
+	case OPT_DEV_NUM:
+		if (eal_option_device_add(RTE_DEVTYPE_VIRTUAL,
+				optarg, 1) < 0) {
+			return -1;
+		}
+		break;
+
 	case OPT_VDEV_NUM:
 		if (eal_option_device_add(RTE_DEVTYPE_VIRTUAL,
-				optarg) < 0) {
+				optarg, 0) < 0) {
 			return -1;
 		}
 		break;

lib/librte_eal/common/eal_options.h +2 -0

index 96e166787..8a17eb22c 100644
@@ -21,6 +21,8 @@ enum {
 	OPT_BASE_VIRTADDR_NUM,
 #define OPT_CREATE_UIO_DEV    "create-uio-dev"
 	OPT_CREATE_UIO_DEV_NUM,
+#define OPT_DEV               "dev"
+	OPT_DEV_NUM,
 #define OPT_FILE_PREFIX       "file-prefix"
 	OPT_FILE_PREFIX_NUM,
 #define OPT_HUGE_DIR          "huge-dir"

Signature · 1 line

2.18.0

Stephen Hemminger

2018-08-30 15:42 UTC #22 lore raw

In reply to Gaetan Rivet

Quoted 56 lines · On Thu, 30 Aug 2018 15:41:51 +0200 Gaetan Rivet <[email protected]> wrote:

Last release saw the introduction of the new devargs system. To this end, the "class" abstraction was described as well as a common API for querying and declaring devices.
This patchset implements the "eth" device class and the query/declaration part of the framework for PCI and vdev buses, enabling a minimal support for the new system.
A new testpmd command is added to test device querying.
Devargs parsing is extended in the relevant buses to test device declaration. This part uses the new "rte_eth_dev_create" API, introduced last release and used by only two PMDs, for now.
The new devargs format is also made available through the new --dev parameter.
Next work is to generalize use of new API for eth_dev creation, compatibility layer for -w, -b and --vdev with --dev, and devargs unit test.
Gaetan Rivet (13):
bus/pci: implement device iteration and comparison bus/pci: add device matching field id bus/vdev: implement device iteration bus/vdev: add device matching field driver
ethdev: add private generic device iterator
ethdev: register ether layer as a class
ethdev: add device matching field name app/testpmd: add show device command bus/pci: pre-process declarative PCI devargs bus/vdev: pre-process declarative vdev devargs bus/pci: process declarative PCI devargs
ethdev: process declarative eth devargs
eal: add generic dev parameter
 app/test-pmd/cmdline.c                      |  54 +++++++
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  24 ++++
 drivers/bus/pci/Makefile                    |   3 +-
 drivers/bus/pci/bsd/pci.c                   |   5 +
 drivers/bus/pci/linux/pci.c                 |   5 +
 drivers/bus/pci/meson.build                 |   6 +-
 drivers/bus/pci/pci_common.c                |   6 +-
 drivers/bus/pci/pci_params.c                | 139 ++++++++++++++++++
 drivers/bus/pci/private.h                   |  54 +++++++
 drivers/bus/vdev/Makefile                   |   3 +-
 drivers/bus/vdev/meson.build                |   5 +-
 drivers/bus/vdev/vdev.c                     |  20 +--
 drivers/bus/vdev/vdev_params.c              | 112 +++++++++++++++
 drivers/bus/vdev/vdev_private.h             |  32 +++++
 lib/librte_eal/common/eal_common_devargs.c  |   4 +
 lib/librte_eal/common/eal_common_options.c  |  36 ++++-
 lib/librte_eal/common/eal_options.h         |   2 +
 lib/librte_ethdev/Makefile                  |   4 +-
 lib/librte_ethdev/eth_private.c             |  31 ++++
 lib/librte_ethdev/eth_private.h             |  31 ++++
 lib/librte_ethdev/meson.build               |   4 +-
 lib/librte_ethdev/rte_class_eth.c           | 148 ++++++++++++++++++++

Quoted 12 lines

 lib/librte_ethdev/rte_ethdev.c              |   7 +
 23 files changed, 714 insertions(+), 21 deletions(-)
 create mode 100644 drivers/bus/pci/pci_params.c
 create mode 100644 drivers/bus/vdev/vdev_params.c
 create mode 100644 drivers/bus/vdev/vdev_private.h
 create mode 100644 lib/librte_ethdev/eth_private.c
 create mode 100644 lib/librte_ethdev/eth_private.h
 create mode 100644 lib/librte_ethdev/rte_class_eth.c

Signature · 1 line

2.18.0

I have devargs for vmbus queued.

Gaetan Rivet

author

2018-09-19 16:04 UTC #23 lore raw

v2 cover Implement new devargs framework

In reply to Gaetan Rivet

Last release saw the introduction of the new devargs system. To this end, the "class" abstraction was described as well as a common API for querying and declaring devices.

This patchset implements the "eth" device class and the query/declaration part of the framework for PCI and vdev buses, enabling a minimal support for the new system.

A new testpmd command is added to test device querying.

Devargs parsing is extended in the relevant buses to test device declaration. This part uses the new "rte_eth_dev_create" API, introduced last release and used by only two PMDs, for now.

The new devargs format is also made available through the new --dev parameter.

Next work is to generalize use of new API for eth_dev creation, compatibility layer for -w, -b and --vdev with --dev, and devargs unit test.

v2: fixed Andrew's and Stephen's remarks,
added acks.

 app/test-pmd/cmdline.c                      |  54 +++++++
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  24 +++
 drivers/bus/pci/Makefile                    |   3 +-
 drivers/bus/pci/bsd/pci.c                   |   5 +
 drivers/bus/pci/linux/pci.c                 |   5 +
 drivers/bus/pci/meson.build                 |   6 +-
 drivers/bus/pci/pci_common.c                |   6 +-
 drivers/bus/pci/pci_params.c                | 136 +++++++++++++++++
 drivers/bus/pci/private.h                   |  54 +++++++
 drivers/bus/vdev/Makefile                   |   3 +-
 drivers/bus/vdev/meson.build                |   5 +-
 drivers/bus/vdev/vdev.c                     |  20 ++-
 drivers/bus/vdev/vdev_params.c              | 112 ++++++++++++++
 drivers/bus/vdev/vdev_private.h             |  32 ++++
 lib/librte_eal/common/eal_common_devargs.c  |   4 +
 lib/librte_eal/common/eal_common_options.c  |  36 ++++-
 lib/librte_eal/common/eal_options.h         |   2 +
 lib/librte_ethdev/Makefile                  |   4 +-
 lib/librte_ethdev/ethdev_private.c          |  31 ++++
 lib/librte_ethdev/ethdev_private.h          |  31 ++++
 lib/librte_ethdev/meson.build               |   4 +-
 lib/librte_ethdev/rte_class_eth.c           | 158 ++++++++++++++++++++
 lib/librte_ethdev/rte_ethdev.c              |   7 +
 23 files changed, 721 insertions(+), 21 deletions(-)
 create mode 100644 drivers/bus/pci/pci_params.c
 create mode 100644 drivers/bus/vdev/vdev_params.c
 create mode 100644 drivers/bus/vdev/vdev_private.h
 create mode 100644 lib/librte_ethdev/ethdev_private.c
 create mode 100644 lib/librte_ethdev/ethdev_private.h
 create mode 100644 lib/librte_ethdev/rte_class_eth.c

Signature · 1 line

2.18.0

Gaetan Rivet

author

2018-09-19 16:04 UTC #24 lore raw

PATCH v2 1/13 bus/pci: implement device iteration and comparison

Signed-off-by Gaetan Rivet <[email protected]>

---
 drivers/bus/pci/Makefile     |  3 +-
 drivers/bus/pci/meson.build  |  6 +++-
 drivers/bus/pci/pci_common.c |  3 +-
 drivers/bus/pci/pci_params.c | 53 ++++++++++++++++++++++++++++++++++++
 drivers/bus/pci/private.h    | 25 +++++++++++++++++
 5 files changed, 86 insertions(+), 4 deletions(-)
 create mode 100644 drivers/bus/pci/pci_params.c

drivers/bus/pci/Makefile +2 -1

index cf373068a..4de953f8f 100644
@@ -26,10 +26,11 @@ CFLAGS += -I$(RTE_SDK)/lib/librte_eal/$(SYSTEM)app/eal
 CFLAGS += -DALLOW_EXPERIMENTAL_API
 
 LDLIBS += -lrte_eal -lrte_mbuf -lrte_mempool -lrte_ring
-LDLIBS += -lrte_ethdev -lrte_pci
+LDLIBS += -lrte_ethdev -lrte_pci -lrte_kvargs
 
 include $(RTE_SDK)/drivers/bus/pci/$(SYSTEM)/Makefile
 SRCS-$(CONFIG_RTE_LIBRTE_PCI_BUS) := $(addprefix $(SYSTEM)/,$(SRCS))
+SRCS-$(CONFIG_RTE_LIBRTE_PCI_BUS) += pci_params.c
 SRCS-$(CONFIG_RTE_LIBRTE_PCI_BUS) += pci_common.c
 SRCS-$(CONFIG_RTE_LIBRTE_PCI_BUS) += pci_common_uio.c

drivers/bus/pci/meson.build +5 -1

index 72939e598..23d6a5fec 100644
@@ -3,7 +3,9 @@
 
 deps += ['pci']
 install_headers('rte_bus_pci.h')
-sources = files('pci_common.c', 'pci_common_uio.c')
+sources = files('pci_common.c',
+	'pci_common_uio.c',
+	'pci_params.c')
 if host_machine.system() == 'linux'
 	sources += files('linux/pci.c',
 			'linux/pci_uio.c',

@@ -17,3 +19,5 @@ endif
 
 # memseg walk is not part of stable API yet
 allow_experimental_apis = true
+
+deps += ['kvargs']

drivers/bus/pci/pci_common.c +1 -2

index 7736b3f9c..c7695d108 100644
@@ -27,8 +27,6 @@
 #include "private.h"
 
 
-extern struct rte_pci_bus rte_pci_bus;
-
 #define SYSFS_PCI_DEVICES "/sys/bus/pci/devices"
 
 const char *rte_pci_get_sysfs_path(void)

@@ -435,6 +433,7 @@ struct rte_pci_bus rte_pci_bus = {
 		.unplug = pci_unplug,
 		.parse = pci_parse,
 		.get_iommu_class = rte_pci_get_iommu_class,
+		.dev_iterate = rte_pci_dev_iterate,
 	},
 	.device_list = TAILQ_HEAD_INITIALIZER(rte_pci_bus.device_list),
 	.driver_list = TAILQ_HEAD_INITIALIZER(rte_pci_bus.driver_list),

drivers/bus/pci/pci_params.c new file +53 -0

53 lines not on this page · raw message

drivers/bus/pci/private.h +25 -0

index 8ddd03e16..0e689fa74 100644
@@ -10,6 +10,8 @@
 #include <rte_pci.h>
 #include <rte_bus_pci.h>
 
+extern struct rte_pci_bus rte_pci_bus;
+
 struct rte_pci_driver;
 struct rte_pci_device;

@@ -166,4 +168,27 @@ rte_pci_match(const struct rte_pci_driver *pci_drv,
 enum rte_iova_mode
 rte_pci_get_iommu_class(void);
 
+/*
+ * Iterate over internal devices,
+ * matching any device against the provided
+ * string.
+ *
+ * @param start
+ *   Iteration starting point.
+ *
+ * @param str
+ *   Device string to match against.
+ *
+ * @param it
+ *   (unused) iterator structure.
+ *
+ * @return
+ *   A pointer to the next matching device if any.
+ *   NULL otherwise.
+ */
+void *
+rte_pci_dev_iterate(const void *start,
+		    const char *str,
+		    const struct rte_dev_iterator *it);
+
 #endif /* _PCI_PRIVATE_H_ */

Signature · 1 line

2.18.0

Gaetan Rivet

author

2018-09-19 16:04 UTC #25 lore raw

PATCH v2 2/13 bus/pci: add device matching field id

In reply to Gaetan Rivet

The PCI bus can now parse a matching field "id" as follows:

"bus=pci,id=0000:00:00.0"

"bus=pci,id=00:00.0"

Signed-off-by Gaetan Rivet <[email protected]>

---
 drivers/bus/pci/pci_params.c | 29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

drivers/bus/pci/pci_params.c +27 -2

index 0fde75803..7630d4845 100644
@@ -3,6 +3,7 @@
  */
 
 #include <rte_bus.h>
+#include <rte_bus_pci.h>
 #include <rte_dev.h>
 #include <rte_errno.h>
 #include <rte_kvargs.h>

@@ -11,21 +12,45 @@
 #include "private.h"
 
 enum pci_params {
+	RTE_PCI_PARAMS_ID,
 	RTE_PCI_PARAMS_MAX,
 };
 
 static const char * const pci_params_keys[] = {
+	[RTE_PCI_PARAMS_ID] = "id",
 	[RTE_PCI_PARAMS_MAX] = NULL,
 };
 
+static int
+pci_addr_kv_cmp(const char *key __rte_unused,
+		const char *value,
+		void *_addr2)
+{
+	struct rte_pci_addr _addr1;
+	struct rte_pci_addr *addr1 = &_addr1;
+	struct rte_pci_addr *addr2 = _addr2;
+
+	if (rte_pci_addr_parse(value, addr1))
+		return -1;
+	return -abs(rte_pci_addr_cmp(addr1, addr2));
+}
+
 static int
 pci_dev_match(const struct rte_device *dev,
 	      const void *_kvlist)
 {
 	const struct rte_kvargs *kvlist = _kvlist;
+	const struct rte_pci_device *pdev;
 
-	(void) dev;
-	(void) kvlist;
+	if (kvlist == NULL)
+		/* Empty string matches everything. */
+		return 0;
+	pdev = RTE_DEV_TO_PCI_CONST(dev);
+	/* if any field does not match. */
+	if (rte_kvargs_process(kvlist, "id",
+			       &pci_addr_kv_cmp,
+			       (void *)(intptr_t)&pdev->addr))
+		return 1;
 	return 0;
 }

Signature · 1 line

2.18.0

Gaetan Rivet

author

2018-09-19 16:04 UTC #26 lore raw

PATCH v2 3/13 bus/vdev: implement device iteration

In reply to Gaetan Rivet

Signed-off-by Gaetan Rivet <[email protected]>

---
 drivers/bus/vdev/Makefile       |  3 +-
 drivers/bus/vdev/meson.build    |  5 +++-
 drivers/bus/vdev/vdev.c         | 10 ++++---
 drivers/bus/vdev/vdev_params.c  | 51 +++++++++++++++++++++++++++++++++
 drivers/bus/vdev/vdev_private.h | 26 +++++++++++++++++
 5 files changed, 89 insertions(+), 6 deletions(-)
 create mode 100644 drivers/bus/vdev/vdev_params.c
 create mode 100644 drivers/bus/vdev/vdev_private.h

drivers/bus/vdev/Makefile +2 -1

index bd0bb8955..1f9cd7ebe 100644
@@ -19,8 +19,9 @@ EXPORT_MAP := rte_bus_vdev_version.map
 LIBABIVER := 1
 
 SRCS-y += vdev.c
+SRCS-y += vdev_params.c
 
-LDLIBS += -lrte_eal
+LDLIBS += -lrte_eal -lrte_kvargs
 
 #
 # Export include files

drivers/bus/vdev/meson.build +4 -1

index 2ee648b49..12605e5c7 100644
@@ -1,7 +1,10 @@
 # SPDX-License-Identifier: BSD-3-Clause
 # Copyright(c) 2017 Intel Corporation
 
-sources = files('vdev.c')
+sources = files('vdev.c',
+	'vdev_params.c')
 install_headers('rte_bus_vdev.h')
 
 allow_experimental_apis = true
+
+deps += ['kvargs']

drivers/bus/vdev/vdev.c +6 -4

index 69dee89a8..ef3ad6d99 100644
@@ -23,6 +23,7 @@
 
 #include "rte_bus_vdev.h"
 #include "vdev_logs.h"
+#include "vdev_private.h"
 
 #define VDEV_MP_KEY	"bus_vdev_mp"

@@ -497,9 +498,9 @@ vdev_probe(void)
 	return ret;
 }
 
-static struct rte_device *
-vdev_find_device(const struct rte_device *start, rte_dev_cmp_t cmp,
-		 const void *data)
+struct rte_device *
+rte_vdev_find_device(const struct rte_device *start, rte_dev_cmp_t cmp,
+		     const void *data)
 {
 	const struct rte_vdev_device *vstart;
 	struct rte_vdev_device *dev;

@@ -536,10 +537,11 @@ vdev_unplug(struct rte_device *dev)
 static struct rte_bus rte_vdev_bus = {
 	.scan = vdev_scan,
 	.probe = vdev_probe,
-	.find_device = vdev_find_device,
+	.find_device = rte_vdev_find_device,
 	.plug = vdev_plug,
 	.unplug = vdev_unplug,
 	.parse = vdev_parse,
+	.dev_iterate = rte_vdev_dev_iterate,
 };
 
 RTE_REGISTER_BUS(vdev, rte_vdev_bus);

drivers/bus/vdev/vdev_params.c new file +51 -0

51 lines not on this page · raw message

drivers/bus/vdev/vdev_private.h new file +26 -0

new file mode 100644
index 000000000..ba6dc48ff
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright 2018 Gaëtan Rivet
+ */
+
+#ifndef _VDEV_PRIVATE_H_
+#define _VDEV_PRIVATE_H_
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+struct rte_device *
+rte_vdev_find_device(const struct rte_device *start,
+		     rte_dev_cmp_t cmp,
+		     const void *data);
+
+void *
+rte_vdev_dev_iterate(const void *start,
+		     const char *str,
+		     const struct rte_dev_iterator *it);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* _VDEV_PRIVATE_H_ */

Signature · 1 line

2.18.0

Gaetan Rivet

author

2018-09-19 16:04 UTC #27 lore raw

PATCH v2 4/13 bus/vdev: add device matching field driver

In reply to Gaetan Rivet

The vdev bus parses a field "driver", matching
a vdev driver name with one passed as follows:

"bus=vdev,driver=xxxx"

Signed-off-by Gaetan Rivet <[email protected]>

---
 drivers/bus/vdev/vdev_params.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

drivers/bus/vdev/vdev_params.c +13 -2

index 842a4684e..2f55f451f 100644
@@ -4,6 +4,7 @@
 
 #include <rte_dev.h>
 #include <rte_bus.h>
+#include <rte_bus_vdev.h>
 #include <rte_kvargs.h>
 #include <rte_errno.h>

@@ -11,10 +12,12 @@
 #include "vdev_private.h"
 
 enum vdev_params {
+	RTE_VDEV_PARAMS_DRIVER,
 	RTE_VDEV_PARAMS_MAX,
 };
 
 static const char * const vdev_params_keys[] = {
+	[RTE_VDEV_PARAMS_DRIVER] = "driver",
 	[RTE_VDEV_PARAMS_MAX] = NULL,
 };

@@ -23,9 +26,17 @@ vdev_dev_match(const struct rte_device *dev,
 	       const void *_kvlist)
 {
 	const struct rte_kvargs *kvlist = _kvlist;
+	const struct rte_vdev_device *vdev;
 
-	(void) kvlist;
-	(void) dev;
+	if (kvlist == NULL)
+		/* Empty string matches everything. */
+		return 0;
+	vdev = RTE_DEV_TO_VDEV_CONST(dev);
+	/* if any field does not match. */
+	if (rte_kvargs_process(kvlist, "driver",
+		rte_kvargs_strcmp,
+		(void *)(intptr_t)vdev->device.driver->name))
+		return -1;
 	return 0;
 }

Signature · 1 line

2.18.0

Thomas Monjalon

2018-09-20 16:11 UTC #28 lore raw

Re: [PATCH v2 04/13] bus/vdev: add device matching field driver

19/09/2018 18:03, Gaetan Rivet:

Quoted 4 lines

The vdev bus parses a field "driver", matching
a vdev driver name with one passed as follows:
"bus=vdev,driver=xxxx"

I think the property should be "name".
We already have a "driver" category.
So it may be "bus=vdev,name=mytap/driver=tap"

Until now we were using the name of the driver as a prefix for the device name because it was the only way of knowing the driver to use. With a richer syntax like above, this restriction can be removed.

Gaëtan Rivet

author

2018-09-21 11:53 UTC #29 lore raw

Re: [PATCH v2 04/13] bus/vdev: add device matching field driver

Quoted 6 lines · Thomas Monjalon

19/09/2018 18:03, Gaetan Rivet:
Quoted 4 lines
The vdev bus parses a field "driver", matching
a vdev driver name with one passed as follows:
"bus=vdev,driver=xxxx"

Quoted 7 lines

I think the property should be "name".
We already have a "driver" category.
So it may be "bus=vdev,name=mytap/driver=tap"
Until now we were using the name of the driver as a prefix for the device name because it was the only way of knowing the driver to use. With a richer syntax like above, this restriction can be removed.

Do you want all buses to read the driver= key and use it to know which driver to probe, or is it an exception of the vdev bus?

Signature · 2 lines

Gaëtan Rivet
6WIND

Thomas Monjalon

2018-09-21 12:55 UTC #30 lore raw

Re: [PATCH v2 04/13] bus/vdev: add device matching field driver

21/09/2018 13:53, Gaëtan Rivet:

Quoted 11 lines · Thomas Monjalon

Quoted 10 lines
19/09/2018 18:03, Gaetan Rivet:
Quoted 4 lines
The vdev bus parses a field "driver", matching
a vdev driver name with one passed as follows:
"bus=vdev,driver=xxxx"
I think the property should be "name".
We already have a "driver" category.
So it may be "bus=vdev,name=mytap/driver=tap"

Quoted 6 lines

Quoted 3 lines
Until now we were using the name of the driver as a prefix for the device name because it was the only way of knowing the driver to use. With a richer syntax like above, this restriction can be removed.
Do you want all buses to read the driver= key and use it to know which driver to probe, or is it an exception of the vdev bus?

Good point. Yes, if the driver= key is provided, the bus should respect it. It may allow to have 2 different drivers supporting the same PCI ids, and let the user override the default driver to use.

Gaetan Rivet

author

2018-09-19 16:04 UTC #31 lore raw

PATCH v2 5/13 ethdev: add private generic device iterator

In reply to Gaetan Rivet

This iterator can be customized with a comparison function that will trigger a stopping condition.

It can be leveraged to write several different iterators that have similar but non-identical purposes.

It is private to librte_ethdev.

Signed-off-by Gaetan Rivet <[email protected]>

---
 lib/librte_ethdev/Makefile         |  1 +
 lib/librte_ethdev/ethdev_private.c | 31 ++++++++++++++++++++++++++++++
 lib/librte_ethdev/ethdev_private.h | 26 +++++++++++++++++++++++++
 lib/librte_ethdev/meson.build      |  3 ++-
 4 files changed, 60 insertions(+), 1 deletion(-)
 create mode 100644 lib/librte_ethdev/ethdev_private.c
 create mode 100644 lib/librte_ethdev/ethdev_private.h

lib/librte_ethdev/Makefile +1 -0

index 0935a275e..4b0eeaac7 100644
@@ -18,6 +18,7 @@ EXPORT_MAP := rte_ethdev_version.map
 
 LIBABIVER := 10
 
+SRCS-y += ethdev_private.c
 SRCS-y += rte_ethdev.c
 SRCS-y += rte_flow.c
 SRCS-y += rte_tm.c

lib/librte_ethdev/ethdev_private.c new file +31 -0

new file mode 100644
index 000000000..768c8b2ed
@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2018 Gaëtan Rivet
+ */
+
+#include "rte_ethdev.h"
+#include "ethdev_private.h"
+
+struct rte_eth_dev *
+eth_find_device(const struct rte_eth_dev *start, rte_eth_cmp_t cmp,
+		const void *data)
+{
+	struct rte_eth_dev *edev;
+	ptrdiff_t idx;
+
+	/* Avoid Undefined Behaviour */
+	if (start != NULL &&
+	    (start < &rte_eth_devices[0] ||
+	     start > &rte_eth_devices[RTE_MAX_ETHPORTS]))
+		return NULL;
+	if (start != NULL)
+		idx = start - &rte_eth_devices[0] + 1;
+	else
+		idx = 0;
+	for (; idx < RTE_MAX_ETHPORTS; idx++) {
+		edev = &rte_eth_devices[idx];
+		if (cmp(edev, data) == 0)
+			return edev;
+	}
+	return NULL;
+}
+

lib/librte_ethdev/ethdev_private.h new file +26 -0

new file mode 100644
index 000000000..0f5c6d5c4
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2018 Gaëtan Rivet
+ */
+
+#ifndef _RTE_ETH_PRIVATE_H_
+#define _RTE_ETH_PRIVATE_H_
+
+#include "rte_ethdev.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/* Generic rte_eth_dev comparison function. */
+typedef int (*rte_eth_cmp_t)(const struct rte_eth_dev *, const void *);
+
+/* Generic rte_eth_dev iterator. */
+struct rte_eth_dev *
+eth_find_device(const struct rte_eth_dev *_start, rte_eth_cmp_t cmp,
+		const void *data);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* _RTE_ETH_PRIVATE_H_ */

lib/librte_ethdev/meson.build +2 -1

index 596cd0f39..7798217d0 100644
@@ -4,7 +4,8 @@
 name = 'ethdev'
 version = 10
 allow_experimental_apis = true
-sources = files('ethdev_profile.c',
+sources = files('ethdev_private.c',
+	'ethdev_profile.c',
 	'rte_ethdev.c',
 	'rte_flow.c',
 	'rte_mtr.c',

Signature · 1 line

2.18.0

Andrew Rybchenko

Acked-by on v2 5/13

2018-09-20 10:02 UTC #32 lore raw

Re: [PATCH v2 05/13] ethdev: add private generic device iterator

Quoted 9 lines · Gaetan Rivet

This iterator can be customized with a comparison function that will trigger a stopping condition.
It can be leveraged to write several different iterators that have similar but non-identical purposes.
It is private to librte_ethdev.
Signed-off-by Gaetan Rivet <[email protected]>

Acked-by Andrew Rybchenko <[email protected]>

Gaetan Rivet

author

2018-09-19 16:04 UTC #33 lore raw

PATCH v2 6/13 ethdev: register ether layer as a class

In reply to Gaetan Rivet

Implement the operators of an rte_class for the
ethdev abstraction layer.

Signed-off-by Gaetan Rivet <[email protected]>

Acked-by Andrew Rybchenko <[email protected]>

---
 lib/librte_ethdev/Makefile        |  3 +-
 lib/librte_ethdev/meson.build     |  1 +
 lib/librte_ethdev/rte_class_eth.c | 79 +++++++++++++++++++++++++++++++
 3 files changed, 82 insertions(+), 1 deletion(-)
 create mode 100644 lib/librte_ethdev/rte_class_eth.c

lib/librte_ethdev/Makefile +2 -1

index 4b0eeaac7..d720dd207 100644
@@ -12,7 +12,7 @@ CFLAGS += -DALLOW_EXPERIMENTAL_API
 CFLAGS += -O3
 CFLAGS += $(WERROR_FLAGS)
 LDLIBS += -lrte_net -lrte_eal -lrte_mempool -lrte_ring
-LDLIBS += -lrte_mbuf
+LDLIBS += -lrte_mbuf -lrte_kvargs
 
 EXPORT_MAP := rte_ethdev_version.map

@@ -20,6 +20,7 @@ LIBABIVER := 10
 
 SRCS-y += ethdev_private.c
 SRCS-y += rte_ethdev.c
+SRCS-y += rte_class_eth.c
 SRCS-y += rte_flow.c
 SRCS-y += rte_tm.c
 SRCS-y += rte_mtr.c

lib/librte_ethdev/meson.build +1 -0

index 7798217d0..172e302f0 100644
@@ -6,6 +6,7 @@ version = 10
 allow_experimental_apis = true
 sources = files('ethdev_private.c',
 	'ethdev_profile.c',
+	'rte_class_eth.c',
 	'rte_ethdev.c',
 	'rte_flow.c',
 	'rte_mtr.c',

lib/librte_ethdev/rte_class_eth.c new file +79 -0

79 lines not on this page · raw message

Signature · 1 line

2.18.0

Gaetan Rivet

author

2018-09-19 16:04 UTC #34 lore raw

PATCH v2 7/13 ethdev: add device matching field name

In reply to Gaetan Rivet

The eth device class can now parse a field name,
matching the eth_dev name with one passed as

"class=eth,name=xxxxxx"

Signed-off-by Gaetan Rivet <[email protected]>

Acked-by Andrew Rybchenko <[email protected]>

---
 lib/librte_ethdev/rte_class_eth.c | 7 +++++++
 1 file changed, 7 insertions(+)

lib/librte_ethdev/rte_class_eth.c +7 -0

index b6557db97..66fd48dc2 100644
@@ -15,10 +15,12 @@
 #include "ethdev_private.h"
 
 enum eth_params {
+	RTE_ETH_PARAMS_NAME,
 	RTE_ETH_PARAMS_MAX,
 };
 
 static const char * const eth_params_keys[] = {
+	[RTE_ETH_PARAMS_NAME] = "name",
 	[RTE_ETH_PARAMS_MAX] = NULL,
 };

@@ -39,6 +41,7 @@ eth_dev_match(const struct rte_eth_dev *edev,
 {
 	const struct eth_dev_match_arg *arg = _arg;
 	const struct rte_kvargs *kvlist = arg->kvlist;
+	struct rte_eth_dev_data *data;
 
 	if (edev->state == RTE_ETH_DEV_UNUSED)
 		return -1;

@@ -47,6 +50,10 @@ eth_dev_match(const struct rte_eth_dev *edev,
 	if (kvlist == NULL)
 		/* Empty string matches everything. */
 		return 0;
+	data = edev->data;
+	if (rte_kvargs_process(kvlist, "name",
+			rte_kvargs_strcmp, data->name))
+		return -1;
 	return 0;
 }

Signature · 1 line

2.18.0

Thomas Monjalon

2018-09-20 16:17 UTC #35 lore raw

Re: [PATCH v2 07/13] ethdev: add device matching field name

19/09/2018 18:03, Gaetan Rivet:

Quoted 4 lines

The eth device class can now parse a field name,
matching the eth_dev name with one passed as
"class=eth,name=xxxxxx"

I am not sure what is the purpose of the "name" property.
I think we should not need it to choose a port by its ethdev name. If you are thinking about a vdev, we can use the rte_device name (at bus level).

Gaëtan Rivet

author

2018-09-21 12:16 UTC #36 lore raw

Re: [PATCH v2 07/13] ethdev: add device matching field name

Quoted 8 lines · Thomas Monjalon

19/09/2018 18:03, Gaetan Rivet:
Quoted 4 lines
The eth device class can now parse a field name,
matching the eth_dev name with one passed as
"class=eth,name=xxxxxx"
I am not sure what is the purpose of the "name" property. I think we should not need it to choose a port by its ethdev name.

rte_eth_dev_get_port_by_name seems pretty close.
Which fields do you think should be proposed first as a getter on the eth class? Or do you have a list of fields the eth class should be restricted to?

Quoted 1 line

If you are thinking about a vdev, we can use the rte_device name (at bus level).

This patch is only about eth class, it has no impact on buses.

Signature · 2 lines

Gaëtan Rivet
6WIND

Thomas Monjalon

2018-09-21 13:06 UTC #37 lore raw

Re: [PATCH v2 07/13] ethdev: add device matching field name

21/09/2018 14:16, Gaëtan Rivet:

Quoted 11 lines · Thomas Monjalon

Quoted 8 lines
19/09/2018 18:03, Gaetan Rivet:
Quoted 4 lines
The eth device class can now parse a field name,
matching the eth_dev name with one passed as
"class=eth,name=xxxxxx"
I am not sure what is the purpose of the "name" property. I think we should not need it to choose a port by its ethdev name.
rte_eth_dev_get_port_by_name seems pretty close.

Exact, this function already exists to get a port id by name.
I think we should discourage the use of ethdev "internal" name. The point of the devargs is to match device with explicit known properties.

Quoted 2 lines

Which fields do you think should be proposed first as a getter on the eth class? Or do you have a list of fields the eth class should be restricted to?

In other words, which ethdev properties can help to match a port? I think about "mac=" (which is already implemented in OVS devargs), and "representor=" (which requires a new field in ethdev). About representors, we could also match all representor ports of a switch (see rte_eth_switch_info).
We could also have a property for the kernel netdev we are (or were) bound.

Does it make sense?

Quoted 3 lines

Quoted 1 line
If you are thinking about a vdev, we can use the rte_device name (at bus level).
This patch is only about eth class, it has no impact on buses.

So we agree :)

Gaetan Rivet

author

2018-09-19 16:04 UTC #38 lore raw

PATCH v2 8/13 app/testpmd: add show device command

In reply to Gaetan Rivet

A new interactive command is offered:

show device <device description>

This commands lists all rte_device element matching the device description. e.g.:

show device bus=pci
show device bus=vdev
show device bus=vdev/class=eth
show device bus=vdev,driver=net_ring/class=eth
show device bus=vdev/class=eth,name=net_ring0

These devices may not be otherwise useful, some buses will spawn devices to keep track of their assets without having a driver to use them.

Signed-off-by Gaetan Rivet <[email protected]>

Acked-by Bernard Iremonger <[email protected]>

---
 app/test-pmd/cmdline.c                      | 54 +++++++++++++++++++++
 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 24 +++++++++
 2 files changed, 78 insertions(+)

app/test-pmd/cmdline.c +54 -0

index 0cbd340c1..a4846d9a8 100644
@@ -228,6 +228,9 @@ static void cmd_help_long_parsed(void *parsed_result,
                         "show port tm node stats (port_id) (node_id) (clear)\n"
                         "       Display the port TM node stats.\n\n"
 
+			"show device (device_string)\n"
+			"       Display devices matching the device string.\n\n"
+
 		);
 	}

@@ -7115,6 +7118,56 @@ cmdline_parse_inst_t cmd_showportall = {
 	},
 };
 
+/* *** SHOW DEVICE INFO *** */
+struct cmd_showdevice_result {
+	cmdline_fixed_string_t show;
+	cmdline_fixed_string_t device;
+	cmdline_fixed_string_t filter;
+};
+
+static void
+cmd_showdevice_dump_device(const struct rte_device *dev)
+{
+	const struct rte_driver *drv = dev->driver;
+
+	printf("0x%p: %s:%s\n", (const void *)dev, dev->name,
+		drv ? drv->name : "<nil>");
+}
+
+static void cmd_showdevice_parsed(void *parsed_result,
+				__attribute__((unused)) struct cmdline *cl,
+				__attribute__((unused)) void *data)
+{
+	struct cmd_showdevice_result *res = parsed_result;
+	struct rte_dev_iterator it;
+	const struct rte_device *dev;
+
+	RTE_DEV_FOREACH(dev, res->filter, &it)
+		cmd_showdevice_dump_device(dev);
+}
+
+cmdline_parse_token_string_t cmd_showdevice_show =
+	TOKEN_STRING_INITIALIZER(struct cmd_showdevice_result,
+				show, "show");
+cmdline_parse_token_string_t cmd_showdevice_device =
+	TOKEN_STRING_INITIALIZER(struct cmd_showdevice_result,
+				device, "device");
+cmdline_parse_token_string_t cmd_showdevice_filter =
+	TOKEN_STRING_INITIALIZER(struct cmd_showdevice_result,
+			filter, NULL);
+
+cmdline_parse_inst_t cmd_showdevice = {
+	.f = cmd_showdevice_parsed,
+	.data = NULL,
+	.help_str = "show device <device string>",
+	.tokens = {
+		(void *)&cmd_showdevice_show,
+		(void *)&cmd_showdevice_device,
+		(void *)&cmd_showdevice_filter,
+		NULL,
+	},
+};
+
 /* *** SHOW PORT INFO *** */
 struct cmd_showport_result {
 	cmdline_fixed_string_t show;

@@ -17608,6 +17661,7 @@ cmdline_parse_ctx_t main_ctx[] = {
 	(cmdline_parse_inst_t *)&cmd_help_long,
 	(cmdline_parse_inst_t *)&cmd_quit,
 	(cmdline_parse_inst_t *)&cmd_load_from_file,
+	(cmdline_parse_inst_t *)&cmd_showdevice,
 	(cmdline_parse_inst_t *)&cmd_showport,
 	(cmdline_parse_inst_t *)&cmd_showqueue,
 	(cmdline_parse_inst_t *)&cmd_showportall,

doc/guides/testpmd_app_ug/testpmd_funcs.rst +24 -0

index dde205a2b..417072cce 100644
@@ -2663,6 +2663,30 @@ set the traffic management default hierarchy on the port::
 
    testpmd> set port tm hierarchy default (port_id)
 
+Device functions
+----------------
+
+Show devices
+~~~~~~~~~~~~
+
+Display any registered devices::
+
+   testpmd> show device <device_string>
+
+where:
+
+* ``device_string``: Device description string, of the format
+
+  layer[/layer[/layer]]
+
+  where one layer is in the form
+
+  layer_key=layer_name[,key1=value1[,...]]
+
+  Valid layer keys are ``bus`` and ``class``.
+  Their respective values is defined by registered ``bus`` and ``class``
+  drivers.
+
 Filter Functions
 ----------------

Signature · 1 line

2.18.0

Gaetan Rivet

author

2018-09-19 16:04 UTC #39 lore raw

PATCH v2 9/13 bus/pci: pre-process declarative PCI devargs

In reply to Gaetan Rivet

The new devargs format does not recognize a particular device name. Each bus uses its specific format.

Instead of introducing a new bus API, process those devargs privately for the moment. Prepare them for matching during scan against the bus devices.

Signed-off-by Gaetan Rivet <[email protected]>

---
 drivers/bus/pci/bsd/pci.c    |  5 ++++
 drivers/bus/pci/linux/pci.c  |  5 ++++
 drivers/bus/pci/pci_params.c | 51 ++++++++++++++++++++++++++++++++++++
 drivers/bus/pci/private.h    | 16 +++++++++++
 4 files changed, 77 insertions(+)

drivers/bus/pci/bsd/pci.c +5 -0

index 655b34b7e..046cd11d5 100644
@@ -327,6 +327,7 @@ pci_scan_one(int dev_pci_fd, struct pci_conf *conf)
 int
 rte_pci_scan(void)
 {
+	struct rte_devargs *devargs;
 	int fd;
 	unsigned dev_count = 0;
 	struct pci_conf matches[16];

@@ -342,6 +343,10 @@ rte_pci_scan(void)
 	if (!rte_eal_has_pci())
 		return 0;
 
+	RTE_EAL_DEVARGS_FOREACH("pci", devargs)
+		if (rte_pci_devargs_prepare(devargs))
+			continue;
+
 	fd = open("/dev/pci", O_RDONLY);
 	if (fd < 0) {
 		RTE_LOG(ERR, EAL, "%s(): error opening /dev/pci\n", __func__);

drivers/bus/pci/linux/pci.c +5 -0

index 04648ac93..12f246089 100644
@@ -429,6 +429,7 @@ parse_pci_addr_format(const char *buf, int bufsize, struct rte_pci_addr *addr)
 int
 rte_pci_scan(void)
 {
+	struct rte_devargs *devargs;
 	struct dirent *e;
 	DIR *dir;
 	char dirname[PATH_MAX];

@@ -438,6 +439,10 @@ rte_pci_scan(void)
 	if (!rte_eal_has_pci())
 		return 0;
 
+	RTE_EAL_DEVARGS_FOREACH("pci", devargs)
+		if (rte_pci_devargs_prepare(devargs))
+			continue;
+
 #ifdef VFIO_PRESENT
 	if (!pci_vfio_is_enabled())
 		RTE_LOG(DEBUG, EAL, "VFIO PCI modules not loaded\n");

drivers/bus/pci/pci_params.c +51 -0

63 lines not on this page · raw message

drivers/bus/pci/private.h +16 -0

index 0e689fa74..9beb24c6a 100644
@@ -191,4 +191,20 @@ rte_pci_dev_iterate(const void *start,
 		    const char *str,
 		    const struct rte_dev_iterator *it);
 
+/*
+ * Prepare a devargs meant for this bus.
+ * This function is only used for a transitory period,
+ * to translate the new devargs format in one
+ * compatible with the old form.
+ *
+ * @param da
+ *   Devargs to process.
+ *
+ * @return
+ *   0 on success.
+ *   <0 on error.
+ */
+int
+rte_pci_devargs_prepare(struct rte_devargs *da);
+
 #endif /* _PCI_PRIVATE_H_ */

Signature · 1 line

2.18.0

Gaetan Rivet

author

2018-09-19 16:04 UTC #40 lore raw

PATCH v2 10/13 bus/vdev: pre-process declarative vdev devargs

In reply to Gaetan Rivet

The new devargs format does not recognize a particular device name. Each bus uses its specific format.

Process each devargs privately prior to attempting a bus scan. Prepare them if they are using the new devargs format.

Signed-off-by Gaetan Rivet <[email protected]>

---
 drivers/bus/vdev/vdev.c         | 10 ++++---
 drivers/bus/vdev/vdev_params.c  | 50 +++++++++++++++++++++++++++++++++
 drivers/bus/vdev/vdev_private.h |  6 ++++
 3 files changed, 62 insertions(+), 4 deletions(-)

drivers/bus/vdev/vdev.c +6 -4

index ef3ad6d99..5699e0744 100644
@@ -115,8 +115,8 @@ rte_vdev_remove_custom_scan(rte_vdev_scan_callback callback, void *user_arg)
 	return 0;
 }
 
-static int
-vdev_parse(const char *name, void *addr)
+int
+rte_vdev_parse(const char *name, void *addr)
 {
 	struct rte_vdev_driver **out = addr;
 	struct rte_vdev_driver *driver = NULL;

@@ -148,7 +148,7 @@ vdev_probe_all_drivers(struct rte_vdev_device *dev)
 	VDEV_LOG(DEBUG, "Search driver %s to probe device %s", name,
 		rte_vdev_device_name(dev));
 
-	if (vdev_parse(name, &driver))
+	if (rte_vdev_parse(name, &driver))
 		return -1;
 	dev->device.driver = &driver->driver;
 	ret = driver->probe(dev);

@@ -447,6 +447,8 @@ vdev_scan(void)
 
 	/* for virtual devices we scan the devargs_list populated via cmdline */
 	RTE_EAL_DEVARGS_FOREACH("vdev", devargs) {
+		if (rte_vdev_devargs_prepare(devargs))
+			continue;
 
 		dev = calloc(1, sizeof(*dev));
 		if (!dev)

@@ -540,7 +542,7 @@ static struct rte_bus rte_vdev_bus = {
 	.find_device = rte_vdev_find_device,
 	.plug = vdev_plug,
 	.unplug = vdev_unplug,
-	.parse = vdev_parse,
+	.parse = rte_vdev_parse,
 	.dev_iterate = rte_vdev_dev_iterate,
 };

drivers/bus/vdev/vdev_params.c +50 -0

64 lines not on this page · raw message

drivers/bus/vdev/vdev_private.h +6 -0

index ba6dc48ff..da57b84dd 100644
@@ -19,6 +19,12 @@ rte_vdev_dev_iterate(const void *start,
 		     const char *str,
 		     const struct rte_dev_iterator *it);
 
+int
+rte_vdev_parse(const char *name, void *addr);
+
+int
+rte_vdev_devargs_prepare(struct rte_devargs *da);
+
 #ifdef __cplusplus
 }
 #endif

Signature · 1 line

2.18.0

Gaetan Rivet

author

2018-09-19 16:04 UTC #41 lore raw

PATCH v2 11/13 bus/pci: process declarative PCI devargs

In reply to Gaetan Rivet

Introduce the facility to process future PCI parameters.

Signed-off-by Gaetan Rivet <[email protected]>

---
 drivers/bus/pci/pci_common.c |  3 +++
 drivers/bus/pci/pci_params.c |  7 +++++++
 drivers/bus/pci/private.h    | 13 +++++++++++++
 3 files changed, 23 insertions(+)

drivers/bus/pci/pci_common.c +3 -0

index c7695d108..900cd9090 100644
@@ -247,6 +247,9 @@ pci_probe_all_drivers(struct rte_pci_device *dev)
 	if (dev->driver != NULL)
 		return 0;
 
+	if (rte_pci_devargs_process(dev) < 0)
+		return -1;
+
 	FOREACH_DRIVER_ON_PCIBUS(dr) {
 		rc = rte_pci_probe_one_driver(dr, dev);
 		if (rc < 0)

drivers/bus/pci/pci_params.c +7 -0

index a09af3b1c..31143c845 100644
@@ -127,3 +127,10 @@ rte_pci_devargs_prepare(struct rte_devargs *devargs)
 	rte_kvargs_free(kvargs);
 	return ret;
 }
+
+int
+rte_pci_devargs_process(struct rte_pci_device *pdev __rte_unused)
+{
+	/* For the moment, no PCI param needs to be processed. */
+	return 0;
+}

drivers/bus/pci/private.h +13 -0

index 9beb24c6a..06dc85e85 100644
@@ -207,4 +207,17 @@ rte_pci_dev_iterate(const void *start,
 int
 rte_pci_devargs_prepare(struct rte_devargs *da);
 
+/*
+ * Process the device devargs, if any.
+ *
+ * @param pdev
+ *   PCI device
+ *
+ * @return
+ *   0 on success.
+ *   <0 on error.
+ */
+int
+rte_pci_devargs_process(struct rte_pci_device *pdev);
+
 #endif /* _PCI_PRIVATE_H_ */

Signature · 1 line

2.18.0

Gaetan Rivet

author

2018-09-19 16:04 UTC #42 lore raw

PATCH v2 12/13 ethdev: process declarative eth devargs

In reply to Gaetan Rivet

Process the eth parameters of a devargs. For each parameters that have a setter implemented, the relevant field in rte_eth_dev field is written.

Currently only "name" is implemented.

Signed-off-by Gaetan Rivet <[email protected]>

---
 lib/librte_ethdev/ethdev_private.h |  5 +++
 lib/librte_ethdev/rte_class_eth.c  | 72 ++++++++++++++++++++++++++++++
 lib/librte_ethdev/rte_ethdev.c     |  7 +++
 3 files changed, 84 insertions(+)

lib/librte_ethdev/ethdev_private.h +5 -0

index 0f5c6d5c4..c0c065165 100644
@@ -19,6 +19,11 @@ struct rte_eth_dev *
 eth_find_device(const struct rte_eth_dev *_start, rte_eth_cmp_t cmp,
 		const void *data);
 
+/* Generic rte_eth_dev parameters processor. */
+int
+rte_eth_dev_args_parse(struct rte_eth_dev *eth_dev,
+		       struct rte_devargs *da);
+
 #ifdef __cplusplus
 }
 #endif

lib/librte_ethdev/rte_class_eth.c +72 -0

93 lines not on this page · raw message

lib/librte_ethdev/rte_ethdev.c +7 -0

index aa7730ce2..b03fe7f4b 100644
@@ -41,6 +41,7 @@
 #include "rte_ethdev.h"
 #include "rte_ethdev_driver.h"
 #include "ethdev_profile.h"
+#include "ethdev_private.h"
 
 int rte_eth_dev_logtype;

@@ -3504,6 +3505,12 @@ rte_eth_dev_create(struct rte_device *device, const char *name,
 		}
 	}
 
+	retval = rte_eth_dev_args_parse(ethdev, device->devargs);
+	if (retval) {
+		RTE_LOG(ERR, EAL, "ethdev parsing failed");
+		goto probe_failed;
+	}
+
 	retval = ethdev_init(ethdev, init_params);
 	if (retval) {
 		RTE_LOG(ERR, EAL, "ethdev initialisation failed");

Signature · 1 line

2.18.0

Andrew Rybchenko

Acked-by on v2 12/13

2018-09-20 10:12 UTC #43 lore raw

Re: [PATCH v2 12/13] ethdev: process declarative eth devargs

Quoted 7 lines · Gaetan Rivet

Process the eth parameters of a devargs. For each parameters that have a setter implemented, the relevant field in rte_eth_dev field is written.
Currently only "name" is implemented.
Signed-off-by Gaetan Rivet <[email protected]>

Acked-by Andrew Rybchenko <[email protected]>

Gaetan Rivet

author

2018-09-19 16:04 UTC #44 lore raw

PATCH v2 13/13 eal: add generic dev parameter

In reply to Gaetan Rivet

Add the --dev parameter to the EAL.
This new parameter takes a generic device declaration as argument.

It uses the new devargs parsing API.

Signed-off-by Gaetan Rivet <[email protected]>

---
 lib/librte_eal/common/eal_common_devargs.c |  4 +++
 lib/librte_eal/common/eal_common_options.c | 36 +++++++++++++++++++---
 lib/librte_eal/common/eal_options.h        |  2 ++
 3 files changed, 37 insertions(+), 5 deletions(-)

lib/librte_eal/common/eal_common_devargs.c +4 -0

index dac2402a4..f1f4628db 100644
@@ -219,6 +219,10 @@ rte_devargs_parse(struct rte_devargs *da, const char *dev)
 	if (da == NULL)
 		return -EINVAL;
 
+	if (strncmp(dev, "bus=", 4) == 0 ||
+	    strncmp(dev, "class=", 6) == 0)
+		return rte_devargs_layers_parse(da, dev);
+
 	/* Retrieve eventual bus info */
 	do {
 		devname = dev;

lib/librte_eal/common/eal_common_options.c +31 -5

index ddd624110..703932a30 100644
@@ -54,6 +54,7 @@ const struct option
 eal_long_options[] = {
 	{OPT_BASE_VIRTADDR,     1, NULL, OPT_BASE_VIRTADDR_NUM    },
 	{OPT_CREATE_UIO_DEV,    0, NULL, OPT_CREATE_UIO_DEV_NUM   },
+	{OPT_DEV,               1, NULL, OPT_DEV_NUM              },
 	{OPT_FILE_PREFIX,       1, NULL, OPT_FILE_PREFIX_NUM      },
 	{OPT_HELP,              0, NULL, OPT_HELP_NUM             },
 	{OPT_HUGE_DIR,          1, NULL, OPT_HUGE_DIR_NUM         },

@@ -111,6 +112,7 @@ TAILQ_HEAD(device_option_list, device_option);
 struct device_option {
 	TAILQ_ENTRY(device_option) next;
 
+	int new;
 	enum rte_devtype type;
 	char arg[];
 };

@@ -123,7 +125,8 @@ static int mem_parsed;
 static int core_parsed;
 
 static int
-eal_option_device_add(enum rte_devtype type, const char *optarg)
+eal_option_device_add(enum rte_devtype type, const char *optarg,
+		      int new)
 {
 	struct device_option *devopt;
 	size_t optlen;

@@ -137,6 +140,7 @@ eal_option_device_add(enum rte_devtype type, const char *optarg)
 	}
 
 	devopt->type = type;
+	devopt->new = new;
 	ret = snprintf(devopt->arg, optlen, "%s", optarg);
 	if (ret < 0) {
 		RTE_LOG(ERR, EAL, "Unable to copy device option\n");

@@ -156,7 +160,22 @@ eal_option_device_parse(void)
 
 	TAILQ_FOREACH_SAFE(devopt, &devopt_list, next, tmp) {
 		if (ret == 0) {
-			ret = rte_devargs_add(devopt->type, devopt->arg);
+			if (devopt->new) {
+				struct rte_devargs *da;
+
+				da = calloc(1, sizeof(*da));
+				ret = rte_devargs_parse(da, devopt->arg);
+				if (ret) {
+					free(da);
+				} else {
+					ret = rte_devargs_insert(da);
+					if (ret)
+						free(da);
+				}
+			} else {
+				ret = rte_devargs_add(devopt->type,
+						      devopt->arg);
+			}
 			if (ret)
 				RTE_LOG(ERR, EAL, "Unable to parse device '%s'\n",
 					devopt->arg);

@@ -1088,7 +1107,7 @@ eal_parse_common_option(int opt, const char *optarg,
 		if (w_used)
 			goto bw_used;
 		if (eal_option_device_add(RTE_DEVTYPE_BLACKLISTED_PCI,
-				optarg) < 0) {
+				optarg, 0) < 0) {
 			return -1;
 		}
 		b_used = 1;

@@ -1098,7 +1117,7 @@ eal_parse_common_option(int opt, const char *optarg,
 		if (b_used)
 			goto bw_used;
 		if (eal_option_device_add(RTE_DEVTYPE_WHITELISTED_PCI,
-				optarg) < 0) {
+				optarg, 0) < 0) {
 			return -1;
 		}
 		w_used = 1;

@@ -1234,9 +1253,16 @@ eal_parse_common_option(int opt, const char *optarg,
 		}
 		break;
 
+	case OPT_DEV_NUM:
+		/* devtype is meaningless in the new format. */
+		if (eal_option_device_add(0, optarg, 1) < 0) {
+			return -1;
+		}
+		break;
+
 	case OPT_VDEV_NUM:
 		if (eal_option_device_add(RTE_DEVTYPE_VIRTUAL,
-				optarg) < 0) {
+				optarg, 0) < 0) {
 			return -1;
 		}
 		break;

lib/librte_eal/common/eal_options.h +2 -0

index 96e166787..8a17eb22c 100644
@@ -21,6 +21,8 @@ enum {
 	OPT_BASE_VIRTADDR_NUM,
 #define OPT_CREATE_UIO_DEV    "create-uio-dev"
 	OPT_CREATE_UIO_DEV_NUM,
+#define OPT_DEV               "dev"
+	OPT_DEV_NUM,
 #define OPT_FILE_PREFIX       "file-prefix"
 	OPT_FILE_PREFIX_NUM,
 #define OPT_HUGE_DIR          "huge-dir"

Signature · 1 line

2.18.0

Thomas Monjalon

2018-10-03 12:32 UTC #45 lore raw

In reply to Gaetan Rivet

19/09/2018 18:03, Gaetan Rivet:

Quoted 7 lines

Last release saw the introduction of the new devargs system. To this end, the "class" abstraction was described as well as a common API for querying and declaring devices.
This patchset implements the "eth" device class and the query/declaration part of the framework for PCI and vdev buses, enabling a minimal support for the new system.

This part of the patchset is applied
(except patches 4 and 7 that were discussed).

Quoted 7 lines

A new testpmd command is added to test device querying.
Devargs parsing is extended in the relevant buses to test device declaration. This part uses the new "rte_eth_dev_create" API, introduced last release and used by only two PMDs, for now.
The new devargs format is also made available through the new --dev parameter.

This second part of the patchset is left for next release.

Quoted 2 lines

Next work is to generalize use of new API for eth_dev creation, compatibility layer for -w, -b and --vdev with --dev, and devargs unit test.

I think it's better to wait having the new devargs usable in these options, so we will have more tests and feedbacks.

Quoted 6 lines

Gaetan Rivet (13):
bus/pci: implement device iteration and comparison bus/pci: add device matching field id bus/vdev: implement device iteration bus/vdev: add device matching field driver
ethdev: add private generic device iterator

Quoted 8 lines

ethdev: register ether layer as a class
ethdev: add device matching field name
app/testpmd: add show device command
bus/pci: pre-process declarative PCI devargs
bus/vdev: pre-process declarative vdev devargs
bus/pci: process declarative PCI devargs
ethdev: process declarative eth devargs
eal: add generic dev parameter

Summary: for 18.11, patches 1, 2, 3, 5, 6 are applied, thanks!

Pavan Nikhilesh Bhagavatula

2020-02-19 05:43 UTC #46 lore raw

@Thomas Monjalon, Ping?. Some parts of this patch set was scheduled for 19.02.

Quoted 59 lines

-----Original Message-----
From: dev <[email protected]> On Behalf Of Thomas Monjalon
Sent: Wednesday, October 3, 2018 6:02 PM
To: Gaetan Rivet <[email protected]>
Cc: [email protected]; Shreyansh Jain <[email protected]>; Andrew Rybchenko <[email protected]>
Subject: Re: [dpdk-dev] [PATCH v2 00/13] Implement new devargs framework
External Email
19/09/2018 18:03, Gaetan Rivet:
Quoted 7 lines
Last release saw the introduction of the new devargs system. To this end, the "class" abstraction was described as well as a common API for querying and declaring devices.
This patchset implements the "eth" device class and the query/declaration part of the framework for PCI and vdev buses, enabling a minimal support for the new system.
This part of the patchset is applied
(except patches 4 and 7 that were discussed).
Quoted 7 lines
A new testpmd command is added to test device querying.
Devargs parsing is extended in the relevant buses to test device declaration. This part uses the new "rte_eth_dev_create" API, introduced last release and used by only two PMDs, for now.
The new devargs format is also made available through the new --dev
parameter.
This second part of the patchset is left for next release.
Quoted 2 lines
Next work is to generalize use of new API for eth_dev creation, compatibility layer for -w, -b and --vdev with --dev, and devargs unit
test.
I think it's better to wait having the new devargs usable in these options,
so we will have more tests and feedbacks.
Quoted 6 lines
Gaetan Rivet (13):
bus/pci: implement device iteration and comparison bus/pci: add device matching field id bus/vdev: implement device iteration bus/vdev: add device matching field driver
ethdev: add private generic device iterator
Quoted 8 lines
ethdev: register ether layer as a class
ethdev: add device matching field name
app/testpmd: add show device command
bus/pci: pre-process declarative PCI devargs
bus/vdev: pre-process declarative vdev devargs
bus/pci: process declarative PCI devargs
ethdev: process declarative eth devargs
eal: add generic dev parameter
Summary: for 18.11, patches 1, 2, 3, 5, 6 are applied, thanks!