switching device representation

From: Adrien Mazarguil <[email protected]>

Add document to describe a model for representing switching capable devices in DPDK, using a general ethdev port model and through port representors.This document also details the port model and the rte_flow semantics required for flow programming, as well as listing some example use cases.

Signed-off-by Declan Doherty <[email protected]>

28/03/2018 15:54, Declan Doherty:

Quoted 8 lines

From: Adrien Mazarguil <[email protected]>

Add document to describe a model for representing switching capable devices in DPDK, using a general ethdev port model and through port representors.This document also details the port model and the rte_flow semantics required for flow programming, as well as listing some example use cases.

Quoted 1 line

Signed-off-by Declan Doherty <[email protected]>

It is strange to have different From: and SoB:
If Adrien participated in this writing, he should have his SoB too I think.

From: Adrien Mazarguil <[email protected]>

Add document to describe a model for representing switching capable devices in DPDK, using a general ethdev port model and through port representors.This document also details the port model and the rte_flow semantics required for flow programming, as well as listing some example use cases.

Signed-off-by Declan Doherty <[email protected]>

doc/guides/prog_guide/switch_representation.rst new file +52 -0

+Port Representors
+-----------------
+
+In many cases, traffic steering rules cannot be determined in advance;
+applications usually have to process a bit of traffic in software before
+thinking about offloading specific flows to hardware.
+
+Applications therefore need the ability to receive and inject traffic to
+various device endpoints (other VFs, PFs or physical ports) before
+connecting them together. Device drivers must provide means to hook the
+"other end" of these endpoints and to refer them when configuring flow
+rules.
+
+This role is left to so-called "port representors" (also known as "VF
+representors" in the specific context of VFs), which are to DPDK what the
+Ethernet switch device driver model (**switchdev**) [1]_ is to Linux, and
+which can be thought as a software "patch panel" front-end for applications.
+
+- DPDK port representors are implemented as additional virtual Ethernet
+  device (**ethdev**) instances, spawned on an as needed basis through
+  configuration parameters passed to the driver of the underlying
+  device using devargs.
+
+::
+
+   -w pci:dbdf,representor=0
+   -w pci:dbdf,representor=[0-3]
+   -w pci:dbdf,representor=[0,5-11]
+
+- As virtual devices, they may be more limited than their physical
+  counterparts, for instance by exposing only a subset of device
+  configuration callbacks and/or by not necessarily having Rx/Tx capability.
+
+- Among other things, they can be used to assign MAC addresses to the
+  resource they represent.
+
+- Applications can tell port representors apart from other physcial of virtual
+  port by checking the dev_flags field within their device information
+  structure for the RTE_ETH_DEV_REPRESENTOR bit-field.
+
+.. code-block:: c
+
+  struct rte_eth_dev_info {
+	..
+	uint32_t dev_flags; /**< Device flags */
+	..
+  };
+
+- The device or group relationship of ports can be discovered using the
+  switch_id field within the device information structure. By default the
+  switch_id of a port will be it's port_id but ports within the same switch
+  domain will share the same *switch_id* which in the case of SR-IOV devices

+VF representors
+~~~~~~~~~~~~~~~

doc/guides/prog_guide/switch_representation.rst new file +28 -0

+Switching Examples
+------------------
+
+This section provides practical examples based on the established Testpmd
+flow command syntax [2]_, in the context described in `traffic steering`_
+
+::
+
+      .-------------.                 .-------------. .-------------.
+      | hypervisor  |                 |    VM 1     | |    VM 2     |
+      | application |                 | application | | application |
+      `--+---+---+--'                 `----------+--' `--+----------'
+         |   |   |                               |       |
+         |   |   `-------------------.           |       |
+         |   `---------.             |           |       |
+         |             |             |           |       |
+   .----(A)----. .----(B)----. .----(C)----.     |       |
+   | port_id 3 | | port_id 4 | | port_id 5 |     |       |
+   `-----+-----' `-----+-----' `-----+-----'     |       |
+        |             |             |           |       |
+      .-+--.    .-----+-----. .-----+-----. .---+--. .--+---.
+      | PF |    | VF 1 rep. | | VF 2 rep. | | VF 1 | | VF 2 |
+      `-+--'    `-----+-----' `-----+-----' `--(D)-' `-(E)--'
+        |             |             |           |       |
+        |             |   .---------'           |       |
+        `-----.       |   |   .-----------------'       |
+              |       |   |   |   .---------------------'
+              |       |   |   |   |

Subject: [dpdk-dev][PATCH v6 2/8] ethdev: add switch identifier parameter to port

Introduces a new port attribute to ethdev port's which denotes the switch domain a port belongs to. By default all port's switch identifiers are the their port_id. Ports which share a common switch domain are configured with the same switch id.

Signed-off-by Declan Doherty <[email protected]>

---
 app/test-pmd/config.c              | 1 +
 lib/librte_ether/rte_ethdev.c      | 3 +++
 lib/librte_ether/rte_ethdev.h      | 1 +
 lib/librte_ether/rte_ethdev_core.h | 1 +
 4 files changed, 6 insertions(+)

app/test-pmd/config.c index renamed from app/test-pmd/config.c +0 -0

4bb255c62..e12f8c515 100644

app/test-pmd/config.c +1 -0

@@ -517,6 +517,7 @@ port_infos_display(portid_t port_id)
 	printf("Min possible number of TXDs per queue: %hu\n",
 		dev_info.tx_desc_lim.nb_min);
 	printf("TXDs number alignment: %hu\n",
 dev_info.tx_desc_lim.nb_align);
+	printf("Switch Id: %u\n", dev_info.switch_id);
 }
 

void

lib/librte_ether/rte_ethdev.c +0 -0

index 23857c91f..f32d18cad 100644

lib/librte_ether/rte_ethdev.c +0 -0

 }
 

int

lib/librte_ether/rte_ethdev.h +1 -0

index 036153306..dced4fc41 100644
@@ -1029,6 +1029,7 @@ struct rte_eth_dev_info {
 	/** Configured number of rx/tx queues */
 	uint16_t nb_rx_queues; /**< Number of RX queues. */
 	uint16_t nb_tx_queues; /**< Number of TX queues. */
+	uint16_t switch_id; /**< Switch Domain Id */
 };
 
 /**

lib/librte_ether/rte_ethdev_core.h +1 -0

index e5681e466..caed7a4e6 100644
@@ -585,6 +585,7 @@ struct rte_eth_dev_data {
 	struct ether_addr* hash_mac_addrs;
 	/** Device Ethernet MAC addresses of hash filtering. */
 	uint16_t port_id;           /**< Device [external] port identifier. */
+	uint16_t switch_id;	    /**< Switch which port is associated with
 */
 	__extension__
 	uint8_t promiscuous   : 1, /**< RX promiscuous mode ON(1) / OFF(0).

*/
scattered_rx : 1, /**< RX of scattered packets is ON(1) /
OFF(0) */
--
2.14.3

Hi Declan,

Thanks for the series! See some comments below

Wednesday, March 28, 2018 4:54 PM, Declan Doherty:

Quoted 28 lines

Subject: [dpdk-dev][PATCH v6 2/8] ethdev: add switch identifier parameter to port

Introduces a new port attribute to ethdev port's which denotes the switch domain a port belongs to. By default all port's switch identifiers are the their port_id. Ports which share a common switch domain are configured with the same switch id.

Signed-off-by Declan Doherty <[email protected]>

---
  app/test-pmd/config.c              | 1 +
  lib/librte_ether/rte_ethdev.c      | 3 +++
  lib/librte_ether/rte_ethdev.h      | 1 +
  lib/librte_ether/rte_ethdev_core.h | 1 +
  4 files changed, 6 insertions(+)

app/test-pmd/config.c index renamed from app/test-pmd/config.c +0 -0

4bb255c62..e12f8c515 100644

app/test-pmd/config.c +1 -0

@@ -517,6 +517,7 @@ port_infos_display(portid_t port_id)
  	printf("Min possible number of TXDs per queue: %hu\n",
  		dev_info.tx_desc_lim.nb_min);
  	printf("TXDs number alignment: %hu\n",
 dev_info.tx_desc_lim.nb_align);
+	printf("Switch Id: %u\n", dev_info.switch_id);
  }
 

lib/librte_ether/rte_ethdev.c rte_eth_dev_allocate(const char *name) · line 295

Quoted 12 lines

void

lib/librte_ether/rte_ethdev.c +2 -0

index 23857c91f..f32d18cad 100644
@@ -290,6 +290,8 @@ rte_eth_dev_allocate(const char *name)
  	eth_dev = eth_dev_get(port_id);
  	snprintf(eth_dev->data->name, sizeof(eth_dev->data->name),
 "%s", name);
  	eth_dev->data->port_id = port_id;
+	eth_dev->data->switch_id = port_id;
+	/**< Default switch_id is the port_id of the device */

Why such default is needed? Why not let the PMD to set it always?

lib/librte_ether/rte_ethdev.c rte_eth_dev_info_get(uint16_t port_id, struct · line 2401

Quoted 10 lines

lib/librte_ether/rte_ethdev.c +0 -0

  	eth_dev->data->mtu = ETHER_MTU;
 

unlock:

lib/librte_ether/rte_ethdev.c +1 -0

@@ -2395,6 +2397,7 @@ rte_eth_dev_info_get(uint16_t port_id, struct
 rte_eth_dev_info *dev_info)
  	dev_info->driver_name = dev->device->driver->name;
  	dev_info->nb_rx_queues = dev->data->nb_rx_queues;
  	dev_info->nb_tx_queues = dev->data->nb_tx_queues;
+	dev_info->switch_id = dev->data->switch_id;

Why there is a need to keep the switch_id on device data?
I think PMD to store it on its private structure and report it in dev_info is enough.

Quoted 33 lines

lib/librte_ether/rte_ethdev.c +0 -0

  }
 

int

lib/librte_ether/rte_ethdev.h +1 -0

index 036153306..dced4fc41 100644
@@ -1029,6 +1029,7 @@ struct rte_eth_dev_info {
  	/** Configured number of rx/tx queues */
  	uint16_t nb_rx_queues; /**< Number of RX queues. */
  	uint16_t nb_tx_queues; /**< Number of TX queues. */
+	uint16_t switch_id; /**< Switch Domain Id */
  };
 
  /**

lib/librte_ether/rte_ethdev_core.h +1 -0

index e5681e466..caed7a4e6 100644
@@ -585,6 +585,7 @@ struct rte_eth_dev_data {
  	struct ether_addr* hash_mac_addrs;
  	/** Device Ethernet MAC addresses of hash filtering. */
  	uint16_t port_id;           /**< Device [external] port identifier. */
+	uint16_t switch_id;	    /**< Switch which port is associated with
 */
  	__extension__
  	uint8_t promiscuous   : 1, /**< RX promiscuous mode ON(1) / OFF(0).

*/
scattered_rx : 1, /**< RX of scattered packets is ON(1) /
OFF(0) */
--
2.14.3

Quoted 33 lines

Hi Declan,

Thanks for the series! See some comments below

Wednesday, March 28, 2018 4:54 PM, Declan Doherty:

Quoted 28 lines

Subject: [dpdk-dev][PATCH v6 2/8] ethdev: add switch identifier parameter to port

Introduces a new port attribute to ethdev port's which denotes the switch domain a port belongs to. By default all port's switch identifiers are the their port_id. Ports which share a common switch domain are configured with the same switch id.

Signed-off-by Declan Doherty <[email protected]>

---
  app/test-pmd/config.c              | 1 +
  lib/librte_ether/rte_ethdev.c      | 3 +++
  lib/librte_ether/rte_ethdev.h      | 1 +
  lib/librte_ether/rte_ethdev_core.h | 1 +
  4 files changed, 6 insertions(+)

app/test-pmd/config.c index renamed from app/test-pmd/config.c +0 -0

4bb255c62..e12f8c515 100644

app/test-pmd/config.c +1 -0

@@ -517,6 +517,7 @@ port_infos_display(portid_t port_id)
  	printf("Min possible number of TXDs per queue: %hu\n",
  		dev_info.tx_desc_lim.nb_min);
  	printf("TXDs number alignment: %hu\n",
 dev_info.tx_desc_lim.nb_align);
+	printf("Switch Id: %u\n", dev_info.switch_id);
  }
 

Quoted 8 lines

lib/librte_ether/rte_ethdev.c rte_eth_dev_allocate(const char *name) · line 291

Quoted 8 lines

void

(quoted diff) +0 -0

b/lib/librte_ether/rte_ethdev.c index 23857c91f..f32d18cad 100644

lib/librte_ether/rte_ethdev.c +0 -0

@@ -290,6 +290,8 @@ rte_eth_dev_allocate(const char *name)
  	eth_dev = eth_dev_get(port_id);
  	snprintf(eth_dev->data->name, sizeof(eth_dev->data->name),

"%s",

Quoted 6 lines

lib/librte_ether/rte_ethdev.c rte_eth_dev_allocate(const char *name) · line 294

Quoted 4 lines

lib/librte_ether/rte_ethdev.c +2 -0

 name);
  	eth_dev->data->port_id = port_id;
+	eth_dev->data->switch_id = port_id;
+	/**< Default switch_id is the port_id of the device */

Why such default is needed? Why not let the PMD to set it always?

I saw this a simple way to have a consistent default value (the port_id) for all PMDs, without the need to modify existing PMD which don't currently have any concept of a switch domain.

Also taking the approach of just leaving it up to the PMD to decide the value would mean that some form of synchronisation would be required so that two device don't select the switch domain identifier.

Quoted 4 lines

Quoted 4 lines

lib/librte_ether/rte_ethdev.c +0 -0

  	eth_dev->data->mtu = ETHER_MTU;
 

Quoted 10 lines

lib/librte_ether/rte_ethdev.c rte_eth_dev_info_get(uint16_t port_id, struct · line 2401

Quoted 7 lines

unlock:

lib/librte_ether/rte_ethdev.c +1 -0

@@ -2395,6 +2397,7 @@ rte_eth_dev_info_get(uint16_t port_id, struct
 rte_eth_dev_info *dev_info)
  	dev_info->driver_name = dev->device->driver->name;
  	dev_info->nb_rx_queues = dev->data->nb_rx_queues;
  	dev_info->nb_tx_queues = dev->data->nb_tx_queues;
+	dev_info->switch_id = dev->data->switch_id;

Why there is a need to keep the switch_id on device data?
I think PMD to store it on its private structure and report it in dev_info is

enough.

Quoted 1 line

That way would require every PMD to be modified to maintain a switch_id structure, which I know isn't a big deal, as only the device which need to support it would need it, but we would need to change the dev_info-

Quoted 1 line

switch_id from being a uint16_t to being a signed value so we could have a

default of -1 for device which don't support switch domain.

I thought the former approach was cleaner.

Quoted 34 lines

Quoted 33 lines

lib/librte_ether/rte_ethdev.c +0 -0

  }
 

int

(quoted diff) +0 -0

b/lib/librte_ether/rte_ethdev.h index 036153306..dced4fc41 100644

lib/librte_ether/rte_ethdev.h +1 -0

@@ -1029,6 +1029,7 @@ struct rte_eth_dev_info {
  	/** Configured number of rx/tx queues */
  	uint16_t nb_rx_queues; /**< Number of RX queues. */
  	uint16_t nb_tx_queues; /**< Number of TX queues. */
+	uint16_t switch_id; /**< Switch Domain Id */
  };
 
  /**

lib/librte_ether/rte_ethdev_core.h +1 -0

index e5681e466..caed7a4e6 100644
@@ -585,6 +585,7 @@ struct rte_eth_dev_data {
  	struct ether_addr* hash_mac_addrs;
  	/** Device Ethernet MAC addresses of hash filtering. */
  	uint16_t port_id;           /**< Device [external] port identifier. */
+	uint16_t switch_id;	    /**< Switch which port is associated with
 */
  	__extension__
  	uint8_t promiscuous   : 1, /**< RX promiscuous mode ON(1) / OFF(0).

*/
scattered_rx : 1, /**< RX of scattered packets is ON(1) /
OFF(0) */
--
2.14.3

Thursday, March 29, 2018 12:14 PM, Doherty, Declan:

Quoted 50 lines · Shahaf Shuler

Quoted 33 lines

Hi Declan,

Thanks for the series! See some comments below

Wednesday, March 28, 2018 4:54 PM, Declan Doherty:

Quoted 28 lines

Subject: [dpdk-dev][PATCH v6 2/8] ethdev: add switch identifier parameter to port

Introduces a new port attribute to ethdev port's which denotes the switch domain a port belongs to. By default all port's switch identifiers are the their port_id. Ports which share a common switch domain are configured with the same switch id.

Signed-off-by Declan Doherty <[email protected]>

---
   app/test-pmd/config.c              | 1 +
   lib/librte_ether/rte_ethdev.c      | 3 +++
   lib/librte_ether/rte_ethdev.h      | 1 +
   lib/librte_ether/rte_ethdev_core.h | 1 +
   4 files changed, 6 insertions(+)

app/test-pmd/config.c index renamed from app/test-pmd/config.c +0 -0

4bb255c62..e12f8c515 100644

app/test-pmd/config.c +1 -0

@@ -517,6 +517,7 @@ port_infos_display(portid_t port_id)
   	printf("Min possible number of TXDs per queue: %hu\n",
   		dev_info.tx_desc_lim.nb_min);
   	printf("TXDs number alignment: %hu\n",
 dev_info.tx_desc_lim.nb_align);
+	printf("Switch Id: %u\n", dev_info.switch_id);
   }
 

Quoted 8 lines

lib/librte_ether/rte_ethdev.c rte_eth_dev_allocate(const char *name) · line 291

Quoted 8 lines

void

(quoted diff) +0 -0

b/lib/librte_ether/rte_ethdev.c index 23857c91f..f32d18cad 100644

lib/librte_ether/rte_ethdev.c +0 -0

@@ -290,6 +290,8 @@ rte_eth_dev_allocate(const char *name)
   	eth_dev = eth_dev_get(port_id);
   	snprintf(eth_dev->data->name, sizeof(eth_dev->data->name),

"%s",

Quoted 6 lines

lib/librte_ether/rte_ethdev.c rte_eth_dev_allocate(const char *name) · line 294

Quoted 4 lines

lib/librte_ether/rte_ethdev.c +2 -0

 name);
   	eth_dev->data->port_id = port_id;
+	eth_dev->data->switch_id = port_id;
+	/**< Default switch_id is the port_id of the device */

Why such default is needed? Why not let the PMD to set it always?

Quoted 3 lines

I saw this a simple way to have a consistent default value (the port_id) for all PMDs, without the need to modify existing PMD which don't currently have any concept of a switch domain.

The default value don't makes much sense though. By default it would mean the application each port is on different switch domain. This is obviously not true in case of multiple VFs

Maybe we can define ETH_SWITCH_ID_INVALID (0) to emphasis 0 is not the switch_id for the PMDs which didn't implemented it.

Quoted 4 lines

Also taking the approach of just leaving it up to the PMD to decide the value would mean that some form of synchronisation would be required so that two device don't select the switch domain identifier.

What kind of knowledge ethdev layer has to set the switche_id? It should be based on the underlying capabilities of the device. Some devices will use the kernel sysfs to check that.

Maybe there are devices which are able to expose the same switch for multiple devices, passing the packet between them using peer2peer trough the PCI.

The point is we need to have APIs which will enable all the future flexibility.

Maybe we can have array of switch_id which are allready taken by the underlying devices, and PMD will register their switch_id to it atomically. To ease the synchronization.

Quoted 14 lines

Quoted 4 lines

Quoted 4 lines

lib/librte_ether/rte_ethdev.c +0 -0

   	eth_dev->data->mtu = ETHER_MTU;
 

Quoted 10 lines

lib/librte_ether/rte_ethdev.c rte_eth_dev_info_get(uint16_t port_id, struct · line 2401

Quoted 7 lines

unlock:

lib/librte_ether/rte_ethdev.c +1 -0

@@ -2395,6 +2397,7 @@ rte_eth_dev_info_get(uint16_t port_id, struct
 rte_eth_dev_info *dev_info)
   	dev_info->driver_name = dev->device->driver->name;
   	dev_info->nb_rx_queues = dev->data->nb_rx_queues;
   	dev_info->nb_tx_queues = dev->data->nb_tx_queues;
+	dev_info->switch_id = dev->data->switch_id;

Why there is a need to keep the switch_id on device data?
I think PMD to store it on its private structure and report it in dev_info is

Quoted 8 lines

enough.

Quoted 1 line

That way would require every PMD to be modified to maintain a switch_id structure, which I know isn't a big deal, as only the device which need to support it would need it, but we would need to change the dev_info-

Quoted 1 line

switch_id from being a uint16_t to being a signed value so we could have a

default of -1 for device which don't support switch domain.

We can say also 0 is the invalid switch id.

Quoted 36 lines

I thought the former approach was cleaner.

Quoted 34 lines

Quoted 33 lines

lib/librte_ether/rte_ethdev.c +0 -0

   }
 

int

(quoted diff) +0 -0

b/lib/librte_ether/rte_ethdev.h index 036153306..dced4fc41 100644

lib/librte_ether/rte_ethdev.h +1 -0

@@ -1029,6 +1029,7 @@ struct rte_eth_dev_info {
   	/** Configured number of rx/tx queues */
   	uint16_t nb_rx_queues; /**< Number of RX queues. */
   	uint16_t nb_tx_queues; /**< Number of TX queues. */
+	uint16_t switch_id; /**< Switch Domain Id */
   };
 
   /**

lib/librte_ether/rte_ethdev_core.h +1 -0

index e5681e466..caed7a4e6 100644
@@ -585,6 +585,7 @@ struct rte_eth_dev_data {
   	struct ether_addr* hash_mac_addrs;
   	/** Device Ethernet MAC addresses of hash filtering. */
   	uint16_t port_id;           /**< Device [external] port identifier. */
+	uint16_t switch_id;	    /**< Switch which port is associated with
 */
   	__extension__
   	uint8_t promiscuous   : 1, /**< RX promiscuous mode ON(1) / OFF(0).

*/
scattered_rx : 1, /**< RX of scattered packets is ON(1) /
OFF(0) */
--
2.14.3

-----Original Message-----
From: Doherty, Declan [mailto:[email protected]]
Sent: Thursday, March 29, 2018 6:13 PM
To: Shahaf Shuler <[email protected]>; [email protected]
Cc: Alex Rosenbaum <[email protected]>; Ferruh Yigit <[email protected]>; Thomas Monjalon <[email protected]>; Qi Zhang <[email protected]>; Alejandro Lucero <[email protected]>; Andrew Rybchenko <[email protected]>; Mohammad Abdul Awal <[email protected]>; Remy Horton <[email protected]>; John McNamara <[email protected]>; Rony Efraim <[email protected]>; Jingjing Wu <[email protected]>; Wenzhuo Lu <[email protected]>; Vincent Jardin <[email protected]>; Yuanhan Liu <[email protected]>; Bruce Richardson <[email protected]>; Konstantin Ananyev <[email protected]>; Zhihong Wang <[email protected]>
Subject: Re: [dpdk-dev][PATCH v6 2/8] ethdev: add switch identifier parameter to port

Quoted 51 lines · Shahaf Shuler

Thursday, March 29, 2018 12:14 PM, Doherty, Declan:

Quoted 50 lines · Shahaf Shuler

Quoted 33 lines

Hi Declan,

Thanks for the series! See some comments below

Wednesday, March 28, 2018 4:54 PM, Declan Doherty:

Quoted 28 lines

Subject: [dpdk-dev][PATCH v6 2/8] ethdev: add switch identifier parameter to port

Introduces a new port attribute to ethdev port's which denotes the switch domain a port belongs to. By default all port's switch identifiers are the their port_id. Ports which share a common switch domain are configured with the same switch id.

Signed-off-by Declan Doherty <[email protected]>

---
   app/test-pmd/config.c              | 1 +
   lib/librte_ether/rte_ethdev.c      | 3 +++
   lib/librte_ether/rte_ethdev.h      | 1 +
   lib/librte_ether/rte_ethdev_core.h | 1 +
   4 files changed, 6 insertions(+)

app/test-pmd/config.c index renamed from app/test-pmd/config.c +0 -0

4bb255c62..e12f8c515 100644

app/test-pmd/config.c +1 -0

@@ -517,6 +517,7 @@ port_infos_display(portid_t port_id)
   	printf("Min possible number of TXDs per queue: %hu\n",
   		dev_info.tx_desc_lim.nb_min);
   	printf("TXDs number alignment: %hu\n",
 dev_info.tx_desc_lim.nb_align);
+	printf("Switch Id: %u\n", dev_info.switch_id);
   }
 

Quoted 8 lines

lib/librte_ether/rte_ethdev.c rte_eth_dev_allocate(const char *name) · line 291

Quoted 8 lines

void

(quoted diff) +0 -0

b/lib/librte_ether/rte_ethdev.c index 23857c91f..f32d18cad 100644

lib/librte_ether/rte_ethdev.c +0 -0

@@ -290,6 +290,8 @@ rte_eth_dev_allocate(const char *name)
   	eth_dev = eth_dev_get(port_id);
   	snprintf(eth_dev->data->name, sizeof(eth_dev->data->name),

"%s",

Quoted 6 lines

lib/librte_ether/rte_ethdev.c rte_eth_dev_allocate(const char *name) · line 294

Quoted 4 lines

lib/librte_ether/rte_ethdev.c +2 -0

 name);
   	eth_dev->data->port_id = port_id;
+	eth_dev->data->switch_id = port_id;
+	/**< Default switch_id is the port_id of the device */

Why such default is needed? Why not let the PMD to set it always?

Quoted 7 lines

Quoted 3 lines

I saw this a simple way to have a consistent default value (the port_id) for all PMDs, without the need to modify existing PMD which don't currently have any concept of a switch domain.

The default value don't makes much sense though. By default it would mean the application each port is on different switch domain. This is obviously not true in case of multiple VFs

But it is the case today for all ports, that they are assumed to be in separate switch domains or at least in unknowable switch domains by applications, with the exception of LAG groups, were it is implicit that they are in the same domain. Even if two ports were attached to the same switch, or if you where using multiple VFs from the same switch there is currently no way of telling the application that this is the case.

multiple VFs from the same port/switch domain in a single data path application. It doesn't seem likely to me that you would use hardware switching to move traffic between 2 VFs in the same data path application.

If you are using port representors, I assumed that you would set a valid switch domain id, possibly picking up the port id of the first port created on that switch domain, but it would be completely up to the PMD how this was done, I have just shown the simplest model in which the port_id of the PF is used for all subsequent representor ports in that switch domain, but if you wanted to create multiple domains within the same device, you could just use subsequent port representors port_ids to setup further switch domains. A switch_id is only meaningful within the contex of DPDK process it is running in.

Quoted 2 lines

Maybe we can define ETH_SWITCH_ID_INVALID (0) to emphasis 0 is not

the switch_id for the PMDs which didn't implemented it.

Quoted 7 lines

Quoted 5 lines

Also taking the approach of just leaving it up to the PMD to decide the value would mean that some form of synchronisation would be required so that two device don't select the switch domain identifier.

What kind of knowledge ethdev layer has to set the switche_id? It should

be based on the underlying capabilities of the device.

Quoted 3 lines

Some devices will use the kernel sysfs to check that.

Maybe there are devices which are able to expose the same switch for

multiple devices, passing the packet between them using peer2peer trough the PCI.

Quoted 4 lines

The point is we need to have APIs which will enable all the future flexibility.

Maybe we can have array of switch_id which are allready taken by the

underlying devices, and PMD will register their switch_id to it atomically. To ease the synchronization.

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

lib/librte_ether/rte_ethdev.c rte_eth_dev_info_get(uint16_t port_id,

Quoted 7 lines

lib/librte_ether/rte_ethdev.c +0 -0

   	eth_dev->data->mtu = ETHER_MTU;
 

unlock:

lib/librte_ether/rte_ethdev.c +0 -0

@@ -2395,6 +2397,7 @@ rte_eth_dev_info_get(uint16_t port_id,

struct

Quoted 59 lines

Quoted 12 lines

Quoted 9 lines

lib/librte_ether/rte_ethdev.c rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info) · line 2400

Quoted 5 lines

lib/librte_ether/rte_ethdev.c +1 -0

 rte_eth_dev_info *dev_info)
   	dev_info->driver_name = dev->device->driver->name;
   	dev_info->nb_rx_queues = dev->data->nb_rx_queues;
   	dev_info->nb_tx_queues = dev->data->nb_tx_queues;
+	dev_info->switch_id = dev->data->switch_id;

Why there is a need to keep the switch_id on device data? I think PMD to store it on its private structure and report it in dev_info is

enough.

Quoted 1 line

Quoted 7 lines

That way would require every PMD to be modified to maintain a switch_id structure, which I know isn't a big deal, as only the device which need to support it would need it, but we would need to change the dev_info-

Quoted 2 lines

switch_id from being a uint16_t to being a signed value so we could have a

default of -1 for device which don't support switch domain.

We can say also 0 is the invalid switch id.

Quoted 36 lines

I thought the former approach was cleaner.

Quoted 34 lines

Quoted 33 lines

lib/librte_ether/rte_ethdev.c +0 -0

   }
 

int

(quoted diff) +0 -0

b/lib/librte_ether/rte_ethdev.h index 036153306..dced4fc41 100644

lib/librte_ether/rte_ethdev.h +1 -0

@@ -1029,6 +1029,7 @@ struct rte_eth_dev_info {
   	/** Configured number of rx/tx queues */
   	uint16_t nb_rx_queues; /**< Number of RX queues. */
   	uint16_t nb_tx_queues; /**< Number of TX queues. */
+	uint16_t switch_id; /**< Switch Domain Id */
   };
 
   /**

lib/librte_ether/rte_ethdev_core.h +1 -0

index e5681e466..caed7a4e6 100644
@@ -585,6 +585,7 @@ struct rte_eth_dev_data {
   	struct ether_addr* hash_mac_addrs;
   	/** Device Ethernet MAC addresses of hash filtering. */
   	uint16_t port_id;           /**< Device [external] port identifier. */
+	uint16_t switch_id;	    /**< Switch which port is associated with
 */
   	__extension__
   	uint8_t promiscuous   : 1, /**< RX promiscuous mode ON(1) / OFF(0).

*/
scattered_rx : 1, /**< RX of scattered packets is ON(1) /
OFF(0) */
--
2.14.3

Subject: [dpdk-dev][PATCH v6 3/8] ethdev: add generic create/destroy ethdev APIs

Add new bus generic ethdev create/destroy APIs which are bus independent and provide hooks for bus specific initialisation.

Signed-off-by Declan Doherty <[email protected]>

---
 lib/librte_ether/Makefile                 |  1 +
 lib/librte_ether/meson.build              |  1 +
 lib/librte_ether/rte_ethdev.c             | 96
++++++++++++++++++++++++++++++-
 lib/librte_ether/rte_ethdev_driver.h      | 57 ++++++++++++++++++
 lib/librte_ether/rte_ethdev_pci.h         | 12 ++++
 lib/librte_ether/rte_ethdev_representor.h | 28 +++++++++
 lib/librte_ether/rte_ethdev_version.map   |  8 +++
 7 files changed, 202 insertions(+), 1 deletion(-)  create mode 100644

lib/librte_ether/rte_ethdev_representor.h

lib/librte_ether/Makefile index renamed from lib/librte_ether/Makefile +0 -0

3ca5782bb..5698cd47b 100644

lib/librte_ether/Makefile +0 -0

@@ -32,6 +32,7 @@ SYMLINK-y-include += rte_ethdev_driver.h  SYMLINK-y-

include += rte_ethdev_core.h SYMLINK-y-include += rte_ethdev_pci.h SYMLINK-y-include += rte_ethdev_vdev.h

lib/librte_ether/Makefile +1 -0

+SYMLINK-y-include += rte_ethdev_representor.h
 SYMLINK-y-include += rte_eth_ctrl.h
 SYMLINK-y-include += rte_dev_info.h
 SYMLINK-y-include += rte_flow.h

lib/librte_ether/meson.build +1 -0

index 7fed86056..163891556 100644
@@ -15,6 +15,7 @@ headers = files('rte_ethdev.h',
 	'rte_ethdev_core.h',
 	'rte_ethdev_pci.h',
 	'rte_ethdev_vdev.h',
+	'rte_ethdev_representor.h',
 	'rte_eth_ctrl.h',
 	'rte_dev_info.h',
 	'rte_flow.h',

lib/librte_ether/rte_ethdev.c +2 -1

index f32d18cad..c719f84a3 100644
@@ -345,7 +345,8 @@ rte_eth_dev_release_port(struct rte_eth_dev
 *eth_dev)
 	rte_eth_dev_shared_data_prepare();
 
 	rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
-
+	eth_dev->device = NULL;
+	eth_dev->intr_handle = NULL;
 	eth_dev->state = RTE_ETH_DEV_UNUSED;
 

memset(eth_dev->data, 0, sizeof(struct rte_eth_dev_data)); @@ -
3403,6 +3404,99 @@ rte_eth_dma_zone_reserve(const struct rte_eth_dev *dev, const char *ring_name,
return rte_memzone_reserve_aligned(z_name, size, socket_id, 0,
align); }

lib/librte_ether/rte_ethdev.c +19 -0

+int __rte_experimental
+rte_eth_dev_create(struct rte_device *device, const char *name,
+	size_t priv_data_size,
+	ethdev_bus_specific_init ethdev_bus_specific_init,
+	void *bus_init_params,
+	ethdev_init_t ethdev_init, void *init_params) {
+	struct rte_eth_dev *ethdev;
+	int retval;
+
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
+		ethdev = rte_eth_dev_allocate(name);
+		if (!ethdev) {
+			retval = -ENODEV;
+			goto probe_failed;
+		}
+
+		if (priv_data_size) {
+			ethdev->data->dev_private = rte_zmalloc_socket(
+				name, priv_data_size,

RTE_CACHE_LINE_SIZE,

lib/librte_ether/rte_ethdev.c +4 -0

+				device->numa_node);
+
+			if (!ethdev->data->dev_private) {
+				RTE_LOG(ERR, EAL, "failed to allocate private

data");

lib/librte_ether/rte_ethdev.c +8 -0

+				retval = -ENOMEM;
+				goto probe_failed;
+			}
+		}
+	} else {
+		ethdev = rte_eth_dev_attach_secondary(name);
+		if (!ethdev) {
+			RTE_LOG(ERR, EAL, "secondary process attach failed,

"

lib/librte_ether/rte_ethdev.c +60 -0

+				"ethdev doesn't exist");
+			retval = -ENODEV;
+			goto probe_failed;
+		}
+	}
+
+	ethdev->device = device;
+
+	if (ethdev_bus_specific_init) {
+		retval = ethdev_bus_specific_init(ethdev, bus_init_params);
+		if (retval) {
+			RTE_LOG(ERR, EAL,
+				"ethdev bus specific initialisation failed");
+			goto probe_failed;
+		}
+	}
+
+	RTE_FUNC_PTR_OR_ERR_RET(*ethdev_init, -EINVAL);
+	retval = ethdev_init(ethdev, init_params);
+	if (retval) {
+		RTE_LOG(ERR, EAL, "ethdev initialisation failed");
+		goto probe_failed;
+	}
+
+	return retval;
+probe_failed:
+	/* free ports private data if primary process */
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
+		rte_free(ethdev->data->dev_private);
+
+	rte_eth_dev_release_port(ethdev);
+
+	return retval;
+}
+
+int  __rte_experimental
+rte_eth_dev_destroy(struct rte_eth_dev *ethdev,
+	ethdev_uninit_t ethdev_uninit)
+{
+	int ret;
+
+	ethdev = rte_eth_dev_allocated(ethdev->data->name);
+	if (!ethdev)
+		return -ENODEV;
+
+	RTE_FUNC_PTR_OR_ERR_RET(*ethdev_uninit, -EINVAL);
+	if (ethdev_uninit) {
+		ret = ethdev_uninit(ethdev);
+		if (ret)
+			return ret;
+	}
+
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
+		rte_free(ethdev->data->dev_private);
+
+	ethdev->data->dev_private = NULL;
+
+	return rte_eth_dev_release_port(ethdev); }
+
+
 int
 rte_eth_dev_rx_intr_ctl_q(uint16_t port_id, uint16_t queue_id,
 			  int epfd, int op, void *data)

lib/librte_ether/rte_ethdev_driver.h +58 -0

index 45f08c65e..4896cea93 100644
@@ -125,6 +125,63 @@ rte_eth_dma_zone_reserve(const struct
 rte_eth_dev *eth_dev, const char *name,
 			 uint16_t queue_id, size_t size,
 			 unsigned align, int socket_id);
 
+
+typedef int (*ethdev_init_t)(struct rte_eth_dev *ethdev, void
+*init_params); typedef int (*ethdev_bus_specific_init)(struct rte_eth_dev
 *ethdev,
+	void *bus_specific_init_params);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * PMD helper function for the creation of a new ethdev ports.
+ *
+ * @param device
+ *  rte_device handle.
+ * @param	name
+ *  port name.
+ * @param priv_data_size
+ *  size of private data required for port.
+ * @param bus_specific_init
+ *  port bus specific initialisation callback function
+ * @param bus_init_params
+ *  port bus specific initialisation parameters
+ * @param ethdev_init
+ *  device specific port initialization callback function
+ * @param init_params
+ *  port initialisation parameters
+ *
+ * @return
+ *   Negative errno value on error, 0 on success.
+ */
+int __rte_experimental
+rte_eth_dev_create(struct rte_device *device, const char *name,
+	size_t priv_data_size,
+	ethdev_bus_specific_init bus_specific_init, void *bus_init_params,
+	ethdev_init_t ethdev_init, void *init_params);
+
+
+typedef int (*ethdev_uninit_t)(struct rte_eth_dev *ethdev);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * PMD helper function for cleaing up the resources of a ethdev port on
+it's
+ * destruction.
+ *
+ * @param ethdev
+ *   ethdev handle of port.
+ * @param ethdev
+ *   device specific port un-initialise callback function
+ *
+ * @return
+ *   Negative errno value on error, 0 on success.
+ */
+int __rte_experimental
+rte_eth_dev_destroy(struct rte_eth_dev *ethdev,
+	ethdev_uninit_t ethdev_uninit);
+
 #ifdef __cplusplus
 }
 #endif

lib/librte_ether/rte_ethdev_pci.h +12 -0

index 897ce5b41..8604a0474 100644
@@ -70,6 +70,18 @@ rte_eth_copy_pci_info(struct rte_eth_dev *eth_dev,
 	eth_dev->data->numa_node = pci_dev->device.numa_node;  }
 
+static inline int
+eth_dev_pci_specific_init(struct rte_eth_dev *eth_dev, void *bus_device)
 {
+	struct rte_pci_device *pci_dev = bus_device;
+
+	if (!pci_dev)
+		return -ENODEV;
+
+	rte_eth_copy_pci_info(eth_dev, pci_dev);
+
+	return 0;
+}
+
 /**
  * @internal
  * Allocates a new ethdev slot for an ethernet device and returns the pointer

lib/librte_ether/rte_ethdev_representor.h new file +13 -0

new file mode 100644
index 000000000..cbc1f2855
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2018 Intel Corporation.
+ */
+
+
+#ifndef _RTE_ETHDEV_REPRESENTOR_H_
+#define _RTE_ETHDEV_REPRESENTOR_H_
+
+#include <rte_ethdev_driver.h>
+
+static int
+eth_dev_representor_port_init(struct rte_eth_dev *ethdev, void
+*init_params) {

lib/librte_ether/rte_ethdev_representor.h new file +5 -0

+
+	return 0;
+}
+
+#endif /* _RTE_ETHDEV_REPRESENTOR_H_ */

lib/librte_ether/rte_ethdev_version.map +8 -1

index 87f02fb74..48b08bc36 100644
@@ -230,3 +230,11 @@ EXPERIMENTAL {
 	rte_mtr_stats_update;
 
 } DPDK_17.11;
+
+EXPERIMENTAL {
+	global:
+
+	rte_eth_dev_create;
+	rte_eth_dev_destroy;
+
+} DPDK_18.05;
--

2.14.3

Wednesday, March 28, 2018 4:54 PM, Declan Doherty:

Quoted 275 lines

Subject: [dpdk-dev][PATCH v6 3/8] ethdev: add generic create/destroy ethdev APIs

Add new bus generic ethdev create/destroy APIs which are bus independent and provide hooks for bus specific initialisation.

Signed-off-by Declan Doherty <[email protected]>

---
  lib/librte_ether/Makefile                 |  1 +
  lib/librte_ether/meson.build              |  1 +
  lib/librte_ether/rte_ethdev.c             | 96
++++++++++++++++++++++++++++++-
  lib/librte_ether/rte_ethdev_driver.h      | 57 ++++++++++++++++++
  lib/librte_ether/rte_ethdev_pci.h         | 12 ++++
  lib/librte_ether/rte_ethdev_representor.h | 28 +++++++++
  lib/librte_ether/rte_ethdev_version.map   |  8 +++
  7 files changed, 202 insertions(+), 1 deletion(-)  create mode 100644

lib/librte_ether/rte_ethdev_representor.h

lib/librte_ether/Makefile index renamed from lib/librte_ether/Makefile +0 -0

3ca5782bb..5698cd47b 100644

lib/librte_ether/Makefile +0 -0

@@ -32,6 +32,7 @@ SYMLINK-y-include += rte_ethdev_driver.h  SYMLINK-y-

include += rte_ethdev_core.h SYMLINK-y-include += rte_ethdev_pci.h SYMLINK-y-include += rte_ethdev_vdev.h

lib/librte_ether/Makefile +1 -0

+SYMLINK-y-include += rte_ethdev_representor.h
  SYMLINK-y-include += rte_eth_ctrl.h
  SYMLINK-y-include += rte_dev_info.h
  SYMLINK-y-include += rte_flow.h

lib/librte_ether/meson.build +1 -0

index 7fed86056..163891556 100644
@@ -15,6 +15,7 @@ headers = files('rte_ethdev.h',
  	'rte_ethdev_core.h',
  	'rte_ethdev_pci.h',
  	'rte_ethdev_vdev.h',
+	'rte_ethdev_representor.h',
  	'rte_eth_ctrl.h',
  	'rte_dev_info.h',
  	'rte_flow.h',

lib/librte_ether/rte_ethdev.c +2 -1

index f32d18cad..c719f84a3 100644
@@ -345,7 +345,8 @@ rte_eth_dev_release_port(struct rte_eth_dev
 *eth_dev)
  	rte_eth_dev_shared_data_prepare();
 
  	rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
-
+	eth_dev->device = NULL;
+	eth_dev->intr_handle = NULL;
  	eth_dev->state = RTE_ETH_DEV_UNUSED;
 

memset(eth_dev->data, 0, sizeof(struct rte_eth_dev_data)); @@ -
3403,6 +3404,99 @@ rte_eth_dma_zone_reserve(const struct rte_eth_dev *dev, const char *ring_name,
return rte_memzone_reserve_aligned(z_name, size, socket_id, 0,
align); }

lib/librte_ether/rte_ethdev.c +19 -0

+int __rte_experimental
+rte_eth_dev_create(struct rte_device *device, const char *name,
+	size_t priv_data_size,
+	ethdev_bus_specific_init ethdev_bus_specific_init,
+	void *bus_init_params,
+	ethdev_init_t ethdev_init, void *init_params) {
+	struct rte_eth_dev *ethdev;
+	int retval;
+
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
+		ethdev = rte_eth_dev_allocate(name);
+		if (!ethdev) {
+			retval = -ENODEV;
+			goto probe_failed;
+		}
+
+		if (priv_data_size) {
+			ethdev->data->dev_private = rte_zmalloc_socket(
+				name, priv_data_size,

RTE_CACHE_LINE_SIZE,

lib/librte_ether/rte_ethdev.c +4 -0

+				device->numa_node);
+
+			if (!ethdev->data->dev_private) {
+				RTE_LOG(ERR, EAL, "failed to allocate private

data");

lib/librte_ether/rte_ethdev.c +8 -0

+				retval = -ENOMEM;
+				goto probe_failed;
+			}
+		}
+	} else {
+		ethdev = rte_eth_dev_attach_secondary(name);
+		if (!ethdev) {
+			RTE_LOG(ERR, EAL, "secondary process attach failed,

"

lib/librte_ether/rte_ethdev.c +60 -0

+				"ethdev doesn't exist");
+			retval = -ENODEV;
+			goto probe_failed;
+		}
+	}
+
+	ethdev->device = device;
+
+	if (ethdev_bus_specific_init) {
+		retval = ethdev_bus_specific_init(ethdev, bus_init_params);
+		if (retval) {
+			RTE_LOG(ERR, EAL,
+				"ethdev bus specific initialisation failed");
+			goto probe_failed;
+		}
+	}
+
+	RTE_FUNC_PTR_OR_ERR_RET(*ethdev_init, -EINVAL);
+	retval = ethdev_init(ethdev, init_params);
+	if (retval) {
+		RTE_LOG(ERR, EAL, "ethdev initialisation failed");
+		goto probe_failed;
+	}
+
+	return retval;
+probe_failed:
+	/* free ports private data if primary process */
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
+		rte_free(ethdev->data->dev_private);
+
+	rte_eth_dev_release_port(ethdev);
+
+	return retval;
+}
+
+int  __rte_experimental
+rte_eth_dev_destroy(struct rte_eth_dev *ethdev,
+	ethdev_uninit_t ethdev_uninit)
+{
+	int ret;
+
+	ethdev = rte_eth_dev_allocated(ethdev->data->name);
+	if (!ethdev)
+		return -ENODEV;
+
+	RTE_FUNC_PTR_OR_ERR_RET(*ethdev_uninit, -EINVAL);
+	if (ethdev_uninit) {
+		ret = ethdev_uninit(ethdev);
+		if (ret)
+			return ret;
+	}
+
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
+		rte_free(ethdev->data->dev_private);
+
+	ethdev->data->dev_private = NULL;
+
+	return rte_eth_dev_release_port(ethdev); }
+
+
  int
  rte_eth_dev_rx_intr_ctl_q(uint16_t port_id, uint16_t queue_id,
  			  int epfd, int op, void *data)

lib/librte_ether/rte_ethdev_driver.h +58 -0

index 45f08c65e..4896cea93 100644
@@ -125,6 +125,63 @@ rte_eth_dma_zone_reserve(const struct
 rte_eth_dev *eth_dev, const char *name,
  			 uint16_t queue_id, size_t size,
  			 unsigned align, int socket_id);
 
+
+typedef int (*ethdev_init_t)(struct rte_eth_dev *ethdev, void
+*init_params); typedef int (*ethdev_bus_specific_init)(struct rte_eth_dev
 *ethdev,
+	void *bus_specific_init_params);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * PMD helper function for the creation of a new ethdev ports.
+ *
+ * @param device
+ *  rte_device handle.
+ * @param	name
+ *  port name.
+ * @param priv_data_size
+ *  size of private data required for port.
+ * @param bus_specific_init
+ *  port bus specific initialisation callback function
+ * @param bus_init_params
+ *  port bus specific initialisation parameters
+ * @param ethdev_init
+ *  device specific port initialization callback function
+ * @param init_params
+ *  port initialisation parameters
+ *
+ * @return
+ *   Negative errno value on error, 0 on success.
+ */
+int __rte_experimental
+rte_eth_dev_create(struct rte_device *device, const char *name,
+	size_t priv_data_size,
+	ethdev_bus_specific_init bus_specific_init, void *bus_init_params,
+	ethdev_init_t ethdev_init, void *init_params);
+
+
+typedef int (*ethdev_uninit_t)(struct rte_eth_dev *ethdev);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * PMD helper function for cleaing up the resources of a ethdev port on
+it's
+ * destruction.
+ *
+ * @param ethdev
+ *   ethdev handle of port.
+ * @param ethdev
+ *   device specific port un-initialise callback function
+ *
+ * @return
+ *   Negative errno value on error, 0 on success.
+ */
+int __rte_experimental
+rte_eth_dev_destroy(struct rte_eth_dev *ethdev,
+	ethdev_uninit_t ethdev_uninit);
+
  #ifdef __cplusplus
  }
  #endif

lib/librte_ether/rte_ethdev_pci.h +12 -0

index 897ce5b41..8604a0474 100644
@@ -70,6 +70,18 @@ rte_eth_copy_pci_info(struct rte_eth_dev *eth_dev,
  	eth_dev->data->numa_node = pci_dev->device.numa_node;  }
 
+static inline int
+eth_dev_pci_specific_init(struct rte_eth_dev *eth_dev, void *bus_device)
 {
+	struct rte_pci_device *pci_dev = bus_device;
+
+	if (!pci_dev)
+		return -ENODEV;
+
+	rte_eth_copy_pci_info(eth_dev, pci_dev);
+
+	return 0;
+}
+
  /**
   * @internal
   * Allocates a new ethdev slot for an ethernet device and returns the pointer

lib/librte_ether/rte_ethdev_representor.h new file +13 -0

new file mode 100644
index 000000000..cbc1f2855
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2018 Intel Corporation.
+ */
+
+
+#ifndef _RTE_ETHDEV_REPRESENTOR_H_
+#define _RTE_ETHDEV_REPRESENTOR_H_
+
+#include <rte_ethdev_driver.h>
+
+static int
+eth_dev_representor_port_init(struct rte_eth_dev *ethdev, void
+*init_params) {

lib/librte_ether/rte_ethdev_representor.h line 23

Quoted 10 lines

lib/librte_ether/rte_ethdev_representor.h new file +10 -0

+	struct rte_eth_dev *base_ethdev = init_params;
+
+	if (!ethdev || !base_ethdev)
+		return -ENODEV;
+
+	/** representor shares same driver as it's base device */
+	ethdev->device->driver = base_ethdev->device->driver;
+
+	/** representor inherits the switch id of it's base device */
+	ethdev->data->switch_id = base_ethdev->data->switch_id;

Why not let the PMD to set it?

The PMD knows the specific port is a represntor port and to which switch it belongs.
Doing it on ethdev layer will block us in the future from having more complex model were, for example, there are multiple switch domain for a set of PF + representors.

Quoted 24 lines

lib/librte_ether/rte_ethdev_representor.h new file +5 -0

+
+	return 0;
+}
+
+#endif /* _RTE_ETHDEV_REPRESENTOR_H_ */

lib/librte_ether/rte_ethdev_version.map +8 -1

index 87f02fb74..48b08bc36 100644
@@ -230,3 +230,11 @@ EXPERIMENTAL {
  	rte_mtr_stats_update;
 
  } DPDK_17.11;
+
+EXPERIMENTAL {
+	global:
+
+	rte_eth_dev_create;
+	rte_eth_dev_destroy;
+
+} DPDK_18.05;
--

2.14.3

Subject: [dpdk-dev][PATCH v6 4/8] ethdev: Add port representor device flag

Add new device flag to specify that ethdev port is a port representor. Extend rte_eth_dev_info structure to expose device flags to user which enable applications to discover if a port is a representor port.

Signed-off-by Declan Doherty <[email protected]>

---
 lib/librte_ether/rte_ethdev.c             | 1 +
 lib/librte_ether/rte_ethdev.h             | 9 ++++++---
 lib/librte_ether/rte_ethdev_representor.h | 3 +++
 3 files changed, 10 insertions(+), 3 deletions(-)

lib/librte_ether/rte_ethdev.c +1 -0

index c719f84a3..163246433 100644
@@ -2399,6 +2399,7 @@ rte_eth_dev_info_get(uint16_t port_id, struct
 rte_eth_dev_info *dev_info)
 	dev_info->nb_rx_queues = dev->data->nb_rx_queues;
 	dev_info->nb_tx_queues = dev->data->nb_tx_queues;
 	dev_info->switch_id = dev->data->switch_id;
+	dev_info->dev_flags = dev->data->dev_flags;
 }
 

int

lib/librte_ether/rte_ethdev.h +0 -0

index dced4fc41..226acc8b1 100644
@@ -996,6 +996,7 @@ struct rte_eth_dev_info {
 	const char *driver_name; /**< Device Driver name. */
 	unsigned int if_index; /**< Index to bound host interface, or 0 if

none.

lib/librte_ether/rte_ethdev.h +1 -0

 		Use if_indextoname() to translate into an interface name. */
+	uint32_t dev_flags; /**< Device flags */
 	uint32_t min_rx_bufsize; /**< Minimum size of RX buffer. */
 	uint32_t max_rx_pktlen; /**< Maximum configurable length of RX

pkt. */

lib/librte_ether/rte_ethdev.h +0 -0

 	uint16_t max_rx_queues; /**< Maximum number of RX queues. */

@@ -1229,11 +1230,13 @@ struct rte_eth_dev_owner {  };
 
 /** Device supports link state interrupt */

lib/librte_ether/rte_ethdev.h +0 -0

 /**
  * @warning

lib/librte_ether/rte_ethdev_representor.h +0 -0

index cbc1f2855..f3726d0ba 100644

lib/librte_ether/rte_ethdev_representor.h +1 -1

+
 	return 0;
 }
 
--

2.14.3

And device attributes like : RTE_ETH_DEV_BONDED_SLAVE and RTE_ETH_DEV_REPRESENTOR. I don't think they belong together under the genetic name of dev_flags.

Moreover, I am not sure the fact device is bonded slave should be exposed to the application. It should be internal to ethdev and its port iterators.

Wednesday, March 28, 2018 4:54 PM, Declan Doherty:

Quoted 43 lines

Subject: [dpdk-dev][PATCH v6 4/8] ethdev: Add port representor device flag

Add new device flag to specify that ethdev port is a port representor. Extend rte_eth_dev_info structure to expose device flags to user which enable applications to discover if a port is a representor port.

Signed-off-by Declan Doherty <[email protected]>

---
  lib/librte_ether/rte_ethdev.c             | 1 +
  lib/librte_ether/rte_ethdev.h             | 9 ++++++---
  lib/librte_ether/rte_ethdev_representor.h | 3 +++
  3 files changed, 10 insertions(+), 3 deletions(-)

lib/librte_ether/rte_ethdev.c +1 -0

index c719f84a3..163246433 100644
@@ -2399,6 +2399,7 @@ rte_eth_dev_info_get(uint16_t port_id, struct
 rte_eth_dev_info *dev_info)
  	dev_info->nb_rx_queues = dev->data->nb_rx_queues;
  	dev_info->nb_tx_queues = dev->data->nb_tx_queues;
  	dev_info->switch_id = dev->data->switch_id;
+	dev_info->dev_flags = dev->data->dev_flags;
  }
 

int

lib/librte_ether/rte_ethdev.h +0 -0

index dced4fc41..226acc8b1 100644
@@ -996,6 +996,7 @@ struct rte_eth_dev_info {
  	const char *driver_name; /**< Device Driver name. */
  	unsigned int if_index; /**< Index to bound host interface, or 0 if

none.
Use if_indextoname() to translate into an interface name. */

lib/librte_ether/rte_ethdev.h +1 -0

+	uint32_t dev_flags; /**< Device flags */
  	uint32_t min_rx_bufsize; /**< Minimum size of RX buffer. */
  	uint32_t max_rx_pktlen; /**< Maximum configurable length of RX

pkt. */
uint16_t max_rx_queues; /**< Maximum number of RX queues. */

lib/librte_ether/rte_ethdev.h +0 -0

@@ -1229,11 +1230,13 @@ struct rte_eth_dev_owner {  };
 
  /** Device supports link state interrupt */

lib/librte_ether/rte_ethdev.h struct rte_eth_dev_owner { }; · line 1238

Quoted 10 lines

lib/librte_ether/rte_ethdev.h +5 -3

-#define RTE_ETH_DEV_INTR_LSC     0x0002
+#define RTE_ETH_DEV_INTR_LSC		0x0002
  /** Device is a bonded slave */
-#define RTE_ETH_DEV_BONDED_SLAVE 0x0004
+#define RTE_ETH_DEV_BONDED_SLAVE	0x0004
  /** Device supports device removal interrupt */
-#define RTE_ETH_DEV_INTR_RMV     0x0008
+#define RTE_ETH_DEV_INTR_RMV		0x0008
+/** Device is port representor */
+#define RTE_ETH_DEV_REPRESENTOR		0x0010

Maybe it is a good time to make some order here.
I understand the decision to use flags instead of bit-field. It is better.

However there is a mix here of device capabilities like : RTE_ETH_DEV_INTR_LSC and RTE_ETH_DEV_INTR_RMV
And device attributes like : RTE_ETH_DEV_BONDED_SLAVE and RTE_ETH_DEV_REPRESENTOR. I don't think they belong together under the genetic name of dev_flags.

Moreover, I am not sure the fact device is bonded slave should be exposed to the application. It should be internal to ethdev and its port iterators.

Finally I think representor port may need more info now (and in the future), for example the associated vf id. For that, I think it is better it to be exposed as a dedicated struct on device info.

Quoted 14 lines

lib/librte_ether/rte_ethdev.h +0 -0

  /**
   * @warning

lib/librte_ether/rte_ethdev_representor.h +0 -0

index cbc1f2855..f3726d0ba 100644
@@ -22,6 +22,9 @@ eth_dev_representor_port_init(struct rte_eth_dev
 *ethdev, void *init_params)
  	/** representor inherits the switch id of it's base device */
  	ethdev->data->switch_id = base_ethdev->data->switch_id;
 

lib/librte_ether/rte_ethdev_representor.h eth_dev_representor_port_init(struct rte_eth_dev *ethdev, void *init_params) · line 26

Quoted 2 lines

lib/librte_ether/rte_ethdev_representor.h +2 -0

+	/** Set device flags to specify that device is a representor port */
+	ethdev->data->dev_flags |= RTE_ETH_DEV_REPRESENTOR;

Should be set in the PMD, not in ethdev layer

Quoted 7 lines

lib/librte_ether/rte_ethdev_representor.h +1 -1

+
  	return 0;
  }
 
--

2.14.3

Quoted 27 lines

Wednesday, March 28, 2018 4:54 PM, Declan Doherty:

Quoted 26 lines

Subject: [dpdk-dev][PATCH v6 4/8] ethdev: Add port representor device flag

Add new device flag to specify that ethdev port is a port representor. Extend rte_eth_dev_info structure to expose device flags to user which enable applications to discover if a port is a representor port.

Signed-off-by Declan Doherty <[email protected]>

---
  lib/librte_ether/rte_ethdev.c             | 1 +
  lib/librte_ether/rte_ethdev.h             | 9 ++++++---
  lib/librte_ether/rte_ethdev_representor.h | 3 +++
  3 files changed, 10 insertions(+), 3 deletions(-)

(quoted diff) +0 -0

b/lib/librte_ether/rte_ethdev.c index c719f84a3..163246433 100644

lib/librte_ether/rte_ethdev.c +1 -0

@@ -2399,6 +2399,7 @@ rte_eth_dev_info_get(uint16_t port_id, struct
 rte_eth_dev_info *dev_info)
  	dev_info->nb_rx_queues = dev->data->nb_rx_queues;
  	dev_info->nb_tx_queues = dev->data->nb_tx_queues;
  	dev_info->switch_id = dev->data->switch_id;
+	dev_info->dev_flags = dev->data->dev_flags;
  }
 

Quoted 15 lines

Quoted 13 lines

int

(quoted diff) +0 -0

b/lib/librte_ether/rte_ethdev.h index dced4fc41..226acc8b1 100644

lib/librte_ether/rte_ethdev.h +0 -0

@@ -996,6 +996,7 @@ struct rte_eth_dev_info {
  	const char *driver_name; /**< Device Driver name. */
  	unsigned int if_index; /**< Index to bound host interface, or 0 if

none.
Use if_indextoname() to translate into an interface name. */

lib/librte_ether/rte_ethdev.h +1 -0

+	uint32_t dev_flags; /**< Device flags */
  	uint32_t min_rx_bufsize; /**< Minimum size of RX buffer. */
  	uint32_t max_rx_pktlen; /**< Maximum configurable length of RX

Quoted 2 lines

pkt. */
uint16_t max_rx_queues; /**< Maximum number of RX queues. */

@@

Quoted 17 lines

lib/librte_ether/rte_ethdev.h struct rte_eth_dev_owner { · line 1239

Quoted 13 lines

-1229,11 +1230,13 @@ struct rte_eth_dev_owner { };

/** Device supports link state interrupt */

lib/librte_ether/rte_ethdev.h +5 -3

-#define RTE_ETH_DEV_INTR_LSC     0x0002
+#define RTE_ETH_DEV_INTR_LSC		0x0002
  /** Device is a bonded slave */
-#define RTE_ETH_DEV_BONDED_SLAVE 0x0004
+#define RTE_ETH_DEV_BONDED_SLAVE	0x0004
  /** Device supports device removal interrupt */
-#define RTE_ETH_DEV_INTR_RMV     0x0008
+#define RTE_ETH_DEV_INTR_RMV		0x0008
+/** Device is port representor */
+#define RTE_ETH_DEV_REPRESENTOR		0x0010

Maybe it is a good time to make some order here.
I understand the decision to use flags instead of bit-field. It is better.

Quoted 1 line

However there is a mix here of device capabilities like :

RTE_ETH_DEV_INTR_LSC and RTE_ETH_DEV_INTR_RMV

Quoted 1 line

And device attributes like : RTE_ETH_DEV_BONDED_SLAVE and

RTE_ETH_DEV_REPRESENTOR.

Quoted 3 lines

I don't think they belong together under the genetic name of dev_flags.

Moreover, I am not sure the fact device is bonded slave should be exposed

to the application. It should be internal to ethdev and its port iterators.

That's a good point on the bonded slave flag, I'll look at fixing that for the next release. I don't think changing it should effect ABI but I'll need to have a closer look.

Do you think that we should have a separate device attributes field, which the representor flag is contained in.

Quoted 1 line

Quoted 1 line

Finally I think representor port may need more info now (and in the

future), for example the associated vf id.

Quoted 1 line

For that, I think it is better it to be exposed as a dedicated struct on device

info.

I think a switch port id should suffice for that, for SR-IOV devices it would map to the vf_id.

Quoted 15 lines

Quoted 15 lines

/**
* @warning

lib/librte_ether/rte_ethdev_representor.h +0 -0

index cbc1f2855..f3726d0ba 100644
@@ -22,6 +22,9 @@ eth_dev_representor_port_init(struct rte_eth_dev
 *ethdev, void *init_params)
  	/** representor inherits the switch id of it's base device */
  	ethdev->data->switch_id = base_ethdev->data->switch_id;
 

Quoted 4 lines

lib/librte_ether/rte_ethdev_representor.h eth_dev_representor_port_init(struct rte_eth_dev *ethdev, void *init_params) · line 26

Quoted 2 lines

lib/librte_ether/rte_ethdev_representor.h +2 -0

+	/** Set device flags to specify that device is a representor port */
+	ethdev->data->dev_flags |= RTE_ETH_DEV_REPRESENTOR;

Should be set in the PMD, not in ethdev layer

As in the previous patch this is just a generic port bus init function which meets the simplest use case of representor port with a single switch domain, a PMD doesn't need to use it but having it here saves duplicating the same code across multiple PMD which are only supporting the basic mode.

Quoted 8 lines

Quoted 7 lines

lib/librte_ether/rte_ethdev_representor.h +1 -1

+
  	return 0;
  }
 
--

2.14.3

From: Remy Horton <[email protected]>

Introduces a new structure, rte_eth_devargs, to support generic ethdev arguments common across NET PMDs, with a new API rte_eth_devargs_parse API to support PMD parsing these arguments.

Signed-off-by Remy Horton <[email protected]>

Signed-off-by Declan Doherty <[email protected]>

---

Add document to describe the model for representing switching capable devices in DPDK, using a general ethdev port model and through port representors. This document also details the port model and the rte_flow semantics required for flow programming, as well as listing some example use cases.

Signed-off-by Adrien Mazarguil <[email protected]>

Signed-off-by Declan Doherty <[email protected]>

---
 doc/guides/prog_guide/index.rst                 |   1 +
 doc/guides/prog_guide/switch_representation.rst | 837
++++++++++++++++++++++++
 2 files changed, 838 insertions(+)

doc/guides/prog_guide/switch_representation.rst new file +14 -0

+Extensions
+~~~~~~~~~~
+
+Compared to creating a brand new dedicated interface, **rte_flow** was
+deemed flexible enough to manage representor traffic only with minor
+extensions:
+
+- Using physical ports, PF, VF or port representors as targets.
+
+- Affecting traffic that is not necessarily addressed to the DPDK port
+ID a
+  flow rule is associated with (e.g. forcing VF traffic redirection to PF).
+
+For advanced uses:

doc/guides/prog_guide/switch_representation.rst new file +51 -0

+Switching Examples
+------------------
+
+This section provides practical examples based on the established
+Testpmd flow command syntax [2]_, in the context described in `traffic
+steering`_
+
+::
+
+      .-------------.                 .-------------. .-------------.
+      | hypervisor  |                 |    VM 1     | |    VM 2     |
+      | application |                 | application | | application |
+      `--+---+---+--'                 `----------+--' `--+----------'
+         |   |   |                               |       |
+         |   |   `-------------------.           |       |
+         |   `---------.             |           |       |
+         |             |             |           |       |
+   .----(A)----. .----(B)----. .----(C)----.     |       |
+   | port_id 3 | | port_id 4 | | port_id 5 |     |       |
+   `-----+-----' `-----+-----' `-----+-----'     |       |
+         |             |             |           |       |
+       .-+--.    .-----+-----. .-----+-----. .---+--. .--+---.
+       | PF |    | VF 1 rep. | | VF 2 rep. | | VF 1 | | VF 2 |
+       `-+--'    `-----+-----' `-----+-----' `--(D)-' `-(E)--'
+         |             |             |           |       |
+         |             |   .---------'           |       |
+         `-----.       |   |   .-----------------'       |
+               |       |   |   |   .---------------------'
+               |       |   |   |   |
+            .--|-------|---|---|---|--.
+            |  |       |   `---|---'  |
+            |  |       `-------'      |
+            |  `---------.            |
+            `------------|------------'
+                         |
+                    .---(F)----.
+                    | physical |
+                    |  port 0  |
+                    `----------'
+
+By default, PF (**A**) can communicate with the physical port it is
+associated with (**F**), while VF 1 (**D**) and VF 2 (**E**) are
+isolated and restricted to communicate with the hypervisor application
+through their respective representors (**B** and **C**) if supported.
+
+Examples in subsequent sections apply to hypervisor applications only
+and are based on port representors **A**, **B** and **C**.
+
+.. [2] `Flow syntax
+
+<http://dpdk.org/doc/guides/testpmd_app_ug/testpmd_funcs.html#flow-

synt

lib/librte_ether/rte_ethdev.h +1 -0

+	struct rte_eth_switch_info switch_info;
 };

Add new bus generic ethdev create/destroy APIs which are bus independent and provide hooks for bus specific initialisation.

Signed-off-by Declan Doherty <[email protected]>

---
 lib/librte_ether/rte_ethdev.c           | 95 ++++++++++++++++++++++++++++++++-
 lib/librte_ether/rte_ethdev_driver.h    | 57 ++++++++++++++++++++
 lib/librte_ether/rte_ethdev_pci.h       | 12 +++++
 lib/librte_ether/rte_ethdev_version.map |  8 +++
 4 files changed, 171 insertions(+), 1 deletion(-)

lib/librte_ether/rte_ethdev.c +42 -1

index 3c049ef43..b16d23b9a 100644
@@ -348,7 +348,8 @@ rte_eth_dev_release_port(struct rte_eth_dev *eth_dev)
 	rte_eth_dev_shared_data_prepare();
 
 	rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
-
+	eth_dev->device = NULL;
+	eth_dev->intr_handle = NULL;
 	eth_dev->state = RTE_ETH_DEV_UNUSED;
 
 	memset(eth_dev->data, 0, sizeof(struct rte_eth_dev_data));

@@ -3439,6 +3440,98 @@ rte_eth_dma_zone_reserve(const struct rte_eth_dev *dev, const char *ring_name,
 			RTE_MEMZONE_IOVA_CONTIG, align);
 }
 
+int __rte_experimental
+rte_eth_dev_create(struct rte_device *device, const char *name,
+	size_t priv_data_size,
+	ethdev_bus_specific_init ethdev_bus_specific_init,
+	void *bus_init_params,
+	ethdev_init_t ethdev_init, void *init_params)
+{
+	struct rte_eth_dev *ethdev;
+	int retval;
+
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
+		ethdev = rte_eth_dev_allocate(name);
+		if (!ethdev) {
+			retval = -ENODEV;
+			goto probe_failed;
+		}
+
+		if (priv_data_size) {
+			ethdev->data->dev_private = rte_zmalloc_socket(
+				name, priv_data_size, RTE_CACHE_LINE_SIZE,
+				device->numa_node);
+
+			if (!ethdev->data->dev_private) {
+				RTE_LOG(ERR, EAL, "failed to allocate private data");
+				retval = -ENOMEM;
+				goto probe_failed;
+			}
+		}
+	} else {
+		ethdev = rte_eth_dev_attach_secondary(name);
+		if (!ethdev) {
+			RTE_LOG(ERR, EAL, "secondary process attach failed, "
+				"ethdev doesn't exist");
+			retval = -ENODEV;
+			goto probe_failed;
+		}
+	}
+
+	ethdev->device = device;
+

lib/librte_ether/rte_ethdev.c +42 -0

+	retval = ethdev_init(ethdev, init_params);
+	if (retval) {
+		RTE_LOG(ERR, EAL, "ethdev initialisation failed");
+		goto probe_failed;
+	}
+
+	return retval;
+probe_failed:
+	/* free ports private data if primary process */
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
+		rte_free(ethdev->data->dev_private);
+
+	rte_eth_dev_release_port(ethdev);
+
+	return retval;
+}
+
+int  __rte_experimental
+rte_eth_dev_destroy(struct rte_eth_dev *ethdev,
+	ethdev_uninit_t ethdev_uninit)
+{
+	int ret;
+
+	ethdev = rte_eth_dev_allocated(ethdev->data->name);
+	if (!ethdev)
+		return -ENODEV;
+
+	RTE_FUNC_PTR_OR_ERR_RET(*ethdev_uninit, -EINVAL);
+	if (ethdev_uninit) {
+		ret = ethdev_uninit(ethdev);
+		if (ret)
+			return ret;
+	}
+
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
+		rte_free(ethdev->data->dev_private);
+
+	ethdev->data->dev_private = NULL;
+
+	return rte_eth_dev_release_port(ethdev);
+}
+
 int
 rte_eth_dev_rx_intr_ctl_q(uint16_t port_id, uint16_t queue_id,
 			  int epfd, int op, void *data)

lib/librte_ether/rte_ethdev_driver.h +57 -0

index a406ef123..e52add0ad 100644
@@ -188,6 +188,63 @@ rte_eth_linkstatus_get(const struct rte_eth_dev *dev,
 #endif
 }
 
+
+typedef int (*ethdev_init_t)(struct rte_eth_dev *ethdev, void *init_params);
+typedef int (*ethdev_bus_specific_init)(struct rte_eth_dev *ethdev,
+	void *bus_specific_init_params);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * PMD helper function for the creation of a new ethdev ports.
+ *
+ * @param device
+ *  rte_device handle.
+ * @param	name
+ *  port name.
+ * @param priv_data_size
+ *  size of private data required for port.
+ * @param bus_specific_init
+ *  port bus specific initialisation callback function
+ * @param bus_init_params
+ *  port bus specific initialisation parameters
+ * @param ethdev_init
+ *  device specific port initialization callback function
+ * @param init_params
+ *  port initialisation parameters
+ *
+ * @return
+ *   Negative errno value on error, 0 on success.
+ */
+int __rte_experimental
+rte_eth_dev_create(struct rte_device *device, const char *name,
+	size_t priv_data_size,
+	ethdev_bus_specific_init bus_specific_init, void *bus_init_params,
+	ethdev_init_t ethdev_init, void *init_params);
+
+
+typedef int (*ethdev_uninit_t)(struct rte_eth_dev *ethdev);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * PMD helper function for cleaing up the resources of a ethdev port on it's
+ * destruction.
+ *
+ * @param ethdev
+ *   ethdev handle of port.
+ * @param ethdev
+ *   device specific port un-initialise callback function
+ *
+ * @return
+ *   Negative errno value on error, 0 on success.
+ */
+int __rte_experimental
+rte_eth_dev_destroy(struct rte_eth_dev *ethdev,
+	ethdev_uninit_t ethdev_uninit);
+
 #ifdef __cplusplus
 }
 #endif

lib/librte_ether/rte_ethdev_pci.h +12 -0

index 6565ae7d3..603287c28 100644
@@ -70,6 +70,18 @@ rte_eth_copy_pci_info(struct rte_eth_dev *eth_dev,
 	eth_dev->data->numa_node = pci_dev->device.numa_node;
 }
 
+static inline int
+eth_dev_pci_specific_init(struct rte_eth_dev *eth_dev, void *bus_device) {
+	struct rte_pci_device *pci_dev = bus_device;
+
+	if (!pci_dev)
+		return -ENODEV;
+
+	rte_eth_copy_pci_info(eth_dev, pci_dev);
+
+	return 0;
+}
+
 /**
  * @internal
  * Allocates a new ethdev slot for an ethernet device and returns the pointer

lib/librte_ether/rte_ethdev_version.map +8 -1

index 34df6c8b5..bd7232923 100644
@@ -229,3 +229,11 @@ EXPERIMENTAL {
 	rte_mtr_stats_update;
 
 } DPDK_17.11;
+
+EXPERIMENTAL {
+	global:
+
+	rte_eth_dev_create;
+	rte_eth_dev_destroy;
+
+} DPDK_18.05;
--

2.14.3

lib/librte_ether/rte_ethdev_version.map +0 -0

@@ -229,3 +229,11 @@ EXPERIMENTAL {
 	rte_mtr_stats_update;

Add new device flag to specify that an ethdev port is a port representor. Extend rte_eth_dev_info structure to expose device flags to the user which enables applications to discover if a port is a representor port.

lib/librte_ether/rte_ethdev.c +2 -0

@@ -2431,6 +2431,8 @@ rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info)
 	dev_info->driver_name = dev->device->driver->name;
 	dev_info->nb_rx_queues = dev->data->nb_rx_queues;
 	dev_info->nb_tx_queues = dev->data->nb_tx_queues;
+
+	dev_info->dev_flags = dev->data->dev_flags;
 }

16/04/2018 15:06, Declan Doherty:

Quoted 3 lines

Add new device flag to specify that an ethdev port is a port representor. Extend rte_eth_dev_info structure to expose device flags to the user which enables applications to discover if a port is a representor port.

[...]

Quoted 9 lines

lib/librte_ether/rte_ethdev.c +2 -0

@@ -2431,6 +2431,8 @@ rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info)
  	dev_info->driver_name = dev->device->driver->name;
  	dev_info->nb_rx_queues = dev->data->nb_rx_queues;
  	dev_info->nb_tx_queues = dev->data->nb_tx_queues;
+
+	dev_info->dev_flags = dev->data->dev_flags;
  }

[...]

lib/librte_ether/rte_ethdev.h struct rte_eth_dev_info { · line 1035

Quoted 7 lines

lib/librte_ether/rte_ethdev.h +1 -0

@@ -1032,6 +1032,7 @@ struct rte_eth_dev_info {
  	const char *driver_name; /**< Device Driver name. */
  	unsigned int if_index; /**< Index to bound host interface, or 0 if none.
  		Use if_indextoname() to translate into an interface name. */
+	uint32_t dev_flags; /**< Device flags */

A similar field has been added recently:

	http://dpdk.org/browse/next/dpdk-next-net/tree/lib/librte_ether/rte_ethdev.h#n1074

	/** Generic device capabilities */
	uint64_t dev_capa;

It is for flags DEV_CAPA_*
Note that the prefix should be fixed to RTE_ETH_DEV, and the doxygen comment should mention the flags prefix. Qi, please fix.

I think dev_capa and dev_flags are the same thing. They could be merged.

lib/librte_ether/rte_ethdev.h struct rte_eth_dev_owner { · line 1278

Quoted 12 lines

lib/librte_ether/rte_ethdev.h +5 -3

  /** Device supports link state interrupt */
-#define RTE_ETH_DEV_INTR_LSC     0x0002
+#define RTE_ETH_DEV_INTR_LSC		0x0002
  /** Device is a bonded slave */
-#define RTE_ETH_DEV_BONDED_SLAVE 0x0004
+#define RTE_ETH_DEV_BONDED_SLAVE	0x0004
  /** Device supports device removal interrupt */
-#define RTE_ETH_DEV_INTR_RMV     0x0008
+#define RTE_ETH_DEV_INTR_RMV		0x0008
+/** Device is port representor */
+#define RTE_ETH_DEV_REPRESENTOR		0x0010

It seems you tried to re-align but it fails. Better to use spaces for alignment.

Quoted 2 lines

I think dev_capa and dev_flags are the same thing. They could be merged.

Do you have a preference for which one to keep, as dev_flags within rte_eth_dev_data is widely used by PMDs and passing this same information out through rte_eth_dev_info makes sense to me?

Introduces a new structure, rte_eth_devargs, to support generic ethdev arguments common across NET PMDs, with a new API rte_eth_devargs_parse API to support PMD parsing these arguments.

Signed-off-by Remy Horton <[email protected]>

Signed-off-by Declan Doherty <[email protected]>

---
 lib/Makefile                            |   1 +
 lib/librte_ether/rte_ethdev.c           | 195 ++++++++++++++++++++++++++++++++
 lib/librte_ether/rte_ethdev_driver.h    |  30 +++++
 lib/librte_ether/rte_ethdev_version.map |   1 +
 4 files changed, 227 insertions(+)

lib/Makefile +1 -0

index ec965a606..4144d99f9 100644
@@ -21,6 +21,7 @@ DEPDIRS-librte_cmdline := librte_eal
 DIRS-$(CONFIG_RTE_LIBRTE_ETHER) += librte_ether
 DEPDIRS-librte_ether := librte_net librte_eal librte_mempool librte_ring
 DEPDIRS-librte_ether += librte_mbuf
+DEPDIRS-librte_ether += librte_kvargs
 DIRS-$(CONFIG_RTE_LIBRTE_BBDEV) += librte_bbdev
 DEPDIRS-librte_bbdev := librte_eal librte_mempool librte_mbuf
 DIRS-$(CONFIG_RTE_LIBRTE_CRYPTODEV) += librte_cryptodev

lib/librte_ether/rte_ethdev.c +1 -0

index 1d38d8e75..a082b211c 100644
@@ -34,6 +34,7 @@
 #include <rte_errno.h>
 #include <rte_spinlock.h>
 #include <rte_string_fns.h>
++#include <rte_kvargs.h>
 
 #include "rte_ether.h"
 #include "rte_ethdev.h"

@@ -4149,6 +4150,200 @@ rte_eth_dev_pool_ops_supported(uint16_t port_id, const char *pool)
 	return (*dev->dev_ops->pool_ops_supported)(dev, pool);

lib/librte_ether/rte_ethdev.c +104 -0

+	if (arglist->str == NULL)
+		return -ENOMEM;
+
+	letter = arglist->str;
+	state = 0;
+	arglist->count = 0;
+	pair = &arglist->pairs[0];
+	while (1) {
+		switch (state) {
+		case 0: /* Initial */
+			if (*letter == '=')
+				return -EINVAL;
+			else if (*letter == '\0')
+				return 0;
+
+			state = 1;
+			pair->key = letter;
+			/* fall-thru */
+
+		case 1: /* Parsing key */
+			if (*letter == '=') {
+				*letter = '\0';
+				pair->value = letter + 1;
+				state = 2;
+			} else if (*letter == ',' || *letter == '\0')
+				return -EINVAL;
+			break;
+
+
+		case 2: /* Parsing value */
+			if (*letter == '[')
+				state = 3;
+			else if (*letter == ',') {
+				*letter = '\0';
+				arglist->count++;
+				pair = &arglist->pairs[arglist->count];
+				state = 0;
+			} else if (*letter == '\0') {
+				letter--;
+				arglist->count++;
+				pair = &arglist->pairs[arglist->count];
+				state = 0;
+			}
+			break;
+
+		case 3: /* Parsing list */
+			if (*letter == ']')
+				state = 2;
+			else if (*letter == '\0')
+				return -EINVAL;
+			break;
+		}
+		letter++;
+	}
+}
+
+static int
+rte_eth_devargs_parse_list(char *str, rte_eth_devargs_callback_t callback,
+	void *data)
+{
+	char *str_start;
+	int state;
+	int result;
+
+	if (*str != '[')
+		/* Single element, not a list */
+		return callback(str, data);
+
+	/* Sanity check, then strip the brackets */
+	str_start = &str[strlen(str) - 1];
+	if (*str_start != ']') {
+		RTE_LOG(ERR, EAL, "(%s): List does not end with ']'", str);
+		return -EINVAL;
+	}
+	str++;
+	*str_start = '\0';
+
+	/* Process list elements */
+	state = 0;
+	while (1) {
+		if (state == 0) {
+			if (*str == '\0')
+				break;
+			if (*str != ',') {
+				str_start = str;
+				state = 1;
+			}
+		} else if (state == 1) {
+			if (*str == ',' || *str == '\0') {
+				if (str > str_start) {
+					/* Non-empty string fragment */
+					*str = '\0';
+					result = callback(str_start, data);
+					if (result < 0)
+						return result;
+				}
+				state = 0;
+			}
+		}
+		str++;
+	}
+	return 0;
+}
+

lib/librte_ether/rte_ethdev.c +70 -0

+	if (result == 1) {
+		if (*len_list >= max_list)
+			return -ENOMEM;
+		list[(*len_list)++] = lo;
+	} else if (result == 2) {
+		if (lo >= hi)
+			return -EINVAL;
+		for (value = lo; value <= hi; value++) {
+			if (*len_list >= max_list)
+				return -ENOMEM;
+			list[(*len_list)++] = value;
+		}
+	} else
+		return -EINVAL;
+	return 0;
+}
+
+static int
+rte_eth_devargs_parse_ports(char *str, void *data)
+{
+	struct rte_eth_devargs *eth_da = data;
+
+	return rte_eth_devargs_process_range(str, eth_da->ports,
+		&eth_da->nb_ports, RTE_MAX_ETHPORTS);
+}
+
+
+static int
+rte_eth_devargs_parse_representor_ports(char *str, void *data)
+{
+	struct rte_eth_devargs *eth_da = data;
+
+	return rte_eth_devargs_process_range(str, eth_da->representor_ports,
+		&eth_da->nb_representor_ports, RTE_MAX_ETHPORTS);
+}
+
+int __rte_experimental
+rte_eth_devargs_parse(const char *dargs, struct rte_eth_devargs *eth_da)
+{
+	struct rte_kvargs args;
+	struct rte_kvargs_pair *pair;
+	unsigned int i;
+	int result;
+
+	memset(eth_da, 0, sizeof(*eth_da));
+
+	result = rte_eth_devargs_tokenise(&args, dargs);
+	if (result < 0)
+		return result;
+
+	for (i = 0; i < args.count; i++) {
+		pair = &args.pairs[i];
+
+		if (strcmp("port", pair->key) == 0) {
+			result = rte_eth_devargs_parse_list(pair->value,
+				rte_eth_devargs_parse_ports, eth_da);
+			if (result < 0)
+				return result;
+		} else if (strcmp("representor", pair->key) == 0) {
+			result = rte_eth_devargs_parse_list(pair->value,
+				rte_eth_devargs_parse_representor_ports,
+				eth_da);
+			if (result < 0)
+				return result;
+		}
+	}
+
+	return 0;
+}
+
 RTE_INIT(ethdev_init_log);
 static void
 ethdev_init_log(void)

lib/librte_ether/rte_ethdev_driver.h +30 -0

index e52add0ad..3bce5747d 100644
@@ -189,6 +189,36 @@ rte_eth_linkstatus_get(const struct rte_eth_dev *dev,
 }
 
 
+/** Generic Ethernet device arguments  */
+struct rte_eth_devargs {
+	uint16_t ports[RTE_MAX_ETHPORTS];
+	/** port/s number to enable on a multi-port single function */
+	uint16_t nb_ports;
+	/** number of ports in ports field */
+	uint16_t representor_ports[RTE_MAX_ETHPORTS];
+	/** representor port/s identifier to enable on device */
+	uint16_t nb_representor_ports;
+	/** number of ports in representor port field */
+};
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * PMD helper function to parse ethdev arguments
+ *
+ * @param devargs
+ *  device arguments
+ * @param eth_devargs
+ *  parsed ethdev specific arguments.
+ *
+ * @return
+ *   Negative errno value on error, 0 on success.
+ */
+int __rte_experimental
+rte_eth_devargs_parse(const char *dargs, struct rte_eth_devargs *eth_devargs);
+
+
 typedef int (*ethdev_init_t)(struct rte_eth_dev *ethdev, void *init_params);
 typedef int (*ethdev_bus_specific_init)(struct rte_eth_dev *ethdev,
 	void *bus_specific_init_params);

lib/librte_ether/rte_ethdev_version.map +1 -1

index bd7232923..62ecbdb8a 100644
@@ -233,6 +233,7 @@ EXPERIMENTAL {
 EXPERIMENTAL {
 	global:
 
+	rt_eth_devargs_parse;
 	rte_eth_dev_create;
 	rte_eth_dev_destroy;
 
--

2.14.3