[dpdk-dev] [PATCH v1] ethdev: add rte_device to port_id function

[dpdk-dev] [PATCH v1] ethdev: add rte_device to port_id function

[Gaetan Rivet]

Some EAL facilities are using rte_devices. Once the rte_device of a
port is found, some need to transform it into a port id to be able to
use the ethdev API.

Add a function to transform an rte_device pointer into an ethdev port id,
as well as an ownership aware variant.

Use this new API in a few PMDs that could be simplified with it.

Cc: Chas Williams <chas3@att.com>
Cc: Liron Himi <lironh@marvell.com>
Cc: Andrew Rybchenko <arybchenko@solarflare.com>
Cc: Thomas Monjalon <thomas@monjalon.net>

Signed-off-by: Gaetan Rivet <grive@u256.net>
---

Simplifies a recurring usage pattern.
This patch was integrated on top of a small bonding fixes patchset:

 https://mails.dpdk.org/archives/dev/2020-April/164209.html

 doc/guides/rel_notes/release_20_05.rst   |  8 +++++++
 drivers/net/bonding/rte_eth_bond_args.c  |  6 +----
 drivers/net/mvneta/mvneta_ethdev.c       | 10 +--------
 drivers/net/mvpp2/mrvl_ethdev.c          | 10 +--------
 lib/librte_ethdev/rte_ethdev.c           | 18 +++++++++++++++
 lib/librte_ethdev/rte_ethdev.h           | 28 ++++++++++++++++++++++++
 lib/librte_ethdev/rte_ethdev_version.map |  4 ++++
 7 files changed, 61 insertions(+), 23 deletions(-)

diff --git a/doc/guides/rel_notes/release_20_05.rst b/doc/guides/rel_notes/release_20_05.rst
index 184967844..75b50e3bb 100644
--- a/doc/guides/rel_notes/release_20_05.rst
+++ b/doc/guides/rel_notes/release_20_05.rst
@@ -56,6 +56,14 @@ New Features
      Also, make sure to start the actual text at the margin.
      =========================================================
 
+* **New port id getter in ethdev.**
+
+  Two new functions are available in ``librte_ethdev`` to find the port id
+  of an ``rte_device``:
+
+  * ``rte_eth_port_from_dev()`` for default usage.
+  * ``rte_eth_port_from_dev_owned_by()`` to find an owned port id.
+
 * **Updated Mellanox mlx5 driver.**
 
   Updated Mellanox mlx5 driver with new features and improvements, including:
diff --git a/drivers/net/bonding/rte_eth_bond_args.c b/drivers/net/bonding/rte_eth_bond_args.c
index 8c5f90dc6..c7caa45a2 100644
--- a/drivers/net/bonding/rte_eth_bond_args.c
+++ b/drivers/net/bonding/rte_eth_bond_args.c
@@ -36,7 +36,6 @@ find_port_id_by_pci_addr(const struct rte_pci_addr *pci_addr)
 {
 	struct rte_bus *pci_bus;
 	struct rte_device *dev;
-	unsigned i;
 
 	pci_bus = rte_bus_find_by_name("pci");
 	if (pci_bus == NULL) {
@@ -50,10 +49,7 @@ find_port_id_by_pci_addr(const struct rte_pci_addr *pci_addr)
 		return -1;
 	}
 
-	RTE_ETH_FOREACH_DEV(i)
-		if (rte_eth_devices[i].device == dev)
-			return i;
-	return -1;
+	return rte_eth_port_from_dev(dev);
 }
 
 static inline int
diff --git a/drivers/net/mvneta/mvneta_ethdev.c b/drivers/net/mvneta/mvneta_ethdev.c
index 865ad61ae..3e1f692d3 100644
--- a/drivers/net/mvneta/mvneta_ethdev.c
+++ b/drivers/net/mvneta/mvneta_ethdev.c
@@ -964,15 +964,7 @@ rte_pmd_mvneta_probe(struct rte_vdev_device *vdev)
 static int
 rte_pmd_mvneta_remove(struct rte_vdev_device *vdev)
 {
-	uint16_t port_id;
-
-	RTE_ETH_FOREACH_DEV(port_id) {
-		if (rte_eth_devices[port_id].device != &vdev->device)
-			continue;
-		rte_eth_dev_close(port_id);
-	}
-
-	return 0;
+	rte_eth_dev_close(rte_eth_port_from_dev(vdev->device));
 }
 
 static struct rte_vdev_driver pmd_mvneta_drv = {
diff --git a/drivers/net/mvpp2/mrvl_ethdev.c b/drivers/net/mvpp2/mrvl_ethdev.c
index b98b1fd66..fdac86854 100644
--- a/drivers/net/mvpp2/mrvl_ethdev.c
+++ b/drivers/net/mvpp2/mrvl_ethdev.c
@@ -3022,15 +3022,7 @@ rte_pmd_mrvl_probe(struct rte_vdev_device *vdev)
 static int
 rte_pmd_mrvl_remove(struct rte_vdev_device *vdev)
 {
-	uint16_t port_id;
-
-	RTE_ETH_FOREACH_DEV(port_id) {
-		if (rte_eth_devices[port_id].device != &vdev->device)
-			continue;
-		rte_eth_dev_close(port_id);
-	}
-
-	return 0;
+	rte_eth_dev_close(rte_eth_port_from_dev(vdev->device));
 }
 
 static struct rte_vdev_driver pmd_mrvl_drv = {
diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 0854ef883..69197159b 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -579,6 +579,24 @@ rte_eth_dev_is_valid_port(uint16_t port_id)
 		return 1;
 }
 
+uint16_t
+rte_eth_port_from_dev_owned_by(const struct rte_device *dev,
+			       const uint64_t owner)
+{
+	uint16_t pid;
+
+	RTE_ETH_FOREACH_DEV_OWNED_BY(pid, owner)
+		if (rte_eth_devices[pid].device == dev)
+			return pid;
+	return RTE_MAX_ETHPORTS;
+}
+
+uint16_t
+rte_eth_port_from_dev(const struct rte_device *dev)
+{
+	return rte_eth_port_from_dev_owned_by(dev, RTE_ETH_DEV_NO_OWNER);
+}
+
 static int
 rte_eth_is_valid_owner_id(uint64_t owner_id)
 {
diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index d1a593ad1..bbe8641a9 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -1530,6 +1530,34 @@ uint64_t rte_eth_find_next_owned_by(uint16_t port_id,
 	     (unsigned int)p < (unsigned int)RTE_MAX_ETHPORTS; \
 	     p = rte_eth_find_next_owned_by(p + 1, o))
 
+/**
+ * Find the owned ethdev port id of an `rte_device`.
+ *
+ * @param dev
+ *   An `rte_device`.
+ * @param owner
+ *   An owner id. Use `RTE_ETH_DEV_NO_OWNER` for ownerless ports.
+ *
+ * @return
+ *   The port id of an `rte_device` if it is owned by `owner`.
+ *   `RTE_MAX_ETHPORTS` otherwise.
+ */
+__rte_experimental
+uint16_t rte_eth_port_from_dev_owned_by(const struct rte_device *dev,
+					const uint64_t owner);
+
+/**
+ * Find the ethdev port id of an `rte_device`.
+ *
+ * @param dev
+ *   An `rte_device`.
+ *
+ * @return
+ *   The port id of an ownerless `rte_device`, `RTE_MAX_ETHPORTS` otherwise.
+ */
+__rte_experimental
+uint16_t rte_eth_port_from_dev(const struct rte_device *dev);
+
 /**
  * Iterates over valid ethdev ports.
  *
diff --git a/lib/librte_ethdev/rte_ethdev_version.map b/lib/librte_ethdev/rte_ethdev_version.map
index 3f32fdecf..650cc06d7 100644
--- a/lib/librte_ethdev/rte_ethdev_version.map
+++ b/lib/librte_ethdev/rte_ethdev_version.map
@@ -230,4 +230,8 @@ EXPERIMENTAL {
 
 	# added in 20.02
 	rte_flow_dev_dump;
+
+	# added in 20.05
+	rte_eth_port_from_dev;
+	rte_eth_port_from_dev_owned_by;
 };
-- 
2.26.0

Re: [dpdk-dev] [PATCH v1] ethdev: add rte_device to port_id function

[Stephen Hemminger]

On Fri, 17 Apr 2020 18:48:37 +0200
Gaetan Rivet <grive@u256.net> wrote:

> +/**
> + * Find the owned ethdev port id of an `rte_device`.
> + *
> + * @param dev
> + *   An `rte_device`.
> + * @param owner
> + *   An owner id. Use `RTE_ETH_DEV_NO_OWNER` for ownerless ports.
> + *
> + * @return
> + *   The port id of an `rte_device` if it is owned by `owner`.
> + *   `RTE_MAX_ETHPORTS` otherwise.
> + */
> +__rte_experimental
> +uint16_t rte_eth_port_from_dev_owned_by(const struct rte_device *dev,
> +					const uint64_t owner);
> +

Ok, but why introduce API with no users?
Also a device could in theory be owned multiple times by the same owner.
For example if two NIC's from same vendor were used in bonding.

Re: [dpdk-dev] [PATCH v1] ethdev: add rte_device to port_id function

[Gaëtan Rivet]

On 17/04/20 13:44 -0700, Stephen Hemminger wrote:
> On Fri, 17 Apr 2020 18:48:37 +0200
> Gaetan Rivet <grive@u256.net> wrote:
> 
> > +/**
> > + * Find the owned ethdev port id of an `rte_device`.
> > + *
> > + * @param dev
> > + *   An `rte_device`.
> > + * @param owner
> > + *   An owner id. Use `RTE_ETH_DEV_NO_OWNER` for ownerless ports.
> > + *
> > + * @return
> > + *   The port id of an `rte_device` if it is owned by `owner`.
> > + *   `RTE_MAX_ETHPORTS` otherwise.
> > + */
> > +__rte_experimental
> > +uint16_t rte_eth_port_from_dev_owned_by(const struct rte_device *dev,
> > +					const uint64_t owner);
> > +
> 
> Ok, but why introduce API with no users?
> Also a device could in theory be owned multiple times by the same owner.
> For example if two NIC's from same vendor were used in bonding.

I'm not sure what you mean by a device being owned multiple times by the
same owner. However if that's what you are referencing, the issue here
is of course the multiple ports spawning from a single device.

I forgot about this edge-case, so this API is incorrect. This also means
that my "fix" for bonding is incorrect.

The current API, RTE_ETH_FOREACH_DEV_OF(), does not offer an owner-aware
version. If the ownership model is bound to continue as it is, it should
probably offer a variant. The problem is that port iterators in ethdev
are already too many, I don't know how a user could make sense of it.

Thanks for the comment Stephen, this patch should be dropped.
-- 
Gaëtan