[dpdk-dev] [PATCH v2 0/2] enable virtual PMD promiscuous and multicast

[dpdk-dev] [PATCH v2 0/2] enable virtual PMD promiscuous and multicast

[Ciara Power]

When virtual PMDs are used to run sample applications, the application
fails due to the enable promiscuous function being called. The virtual
PMDs do not have dev_ops for enabling promiscuous mode, and return
-ENOTSUP as a result. 

Promiscuous and multicast modes are now initialised as enabled for the
virtual PMDs included in this patchset. When the enable promiscuous
function is called, a check now exists to allow the function return
successfully if the state is already enabled for the device. This allows
virtual PMDs run existing sample applications without failing.

---
v2:
* Removed dummy dev_ops functions.
* Added promiscuous state check in ethdev API functions.
* Initialised promiscuous and multicast states to enabled.

Ciara Power (2):
  ethdev: add check for device promiscuous state
  drivers/net: set enabled promiscuous and multicast

 drivers/net/kni/rte_eth_kni.c   |  2 ++
 drivers/net/null/rte_eth_null.c |  2 ++
 drivers/net/pcap/rte_eth_pcap.c |  2 ++
 drivers/net/ring/rte_eth_ring.c |  2 ++
 lib/librte_ethdev/rte_ethdev.c  | 22 ++++++++++++----------
 5 files changed, 20 insertions(+), 10 deletions(-)

-- 
2.17.1

[dpdk-dev] [PATCH v2 1/2] ethdev: add check for device promiscuous state

[Ciara Power]

The promiscuous enable and disable functions now check the
promiscuous state of the device before checking if the dev_ops
function exists for the device.

This change is necessary to allow sample applications run on
virtual PMDs, as previously -ENOTSUP returned when the promiscuous
enable function was called. This caused the sample application to
fail unnecessarily.

Signed-off-by: Ciara Power <ciara.power@intel.com>
---
 lib/librte_ethdev/rte_ethdev.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index af823607c..67db0d1dd 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -1930,12 +1930,13 @@ rte_eth_promiscuous_enable(uint16_t port_id)
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 	dev = &rte_eth_devices[port_id];
 
+	if (dev->data->promiscuous == 1)
+		return 0;
+
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->promiscuous_enable, -ENOTSUP);
 
-	if (dev->data->promiscuous == 0) {
-		diag = (*dev->dev_ops->promiscuous_enable)(dev);
-		dev->data->promiscuous = (diag == 0) ? 1 : 0;
-	}
+	diag = (*dev->dev_ops->promiscuous_enable)(dev);
+	dev->data->promiscuous = (diag == 0) ? 1 : 0;
 
 	return eth_err(port_id, diag);
 }
@@ -1949,14 +1950,15 @@ rte_eth_promiscuous_disable(uint16_t port_id)
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 	dev = &rte_eth_devices[port_id];
 
+	if (dev->data->promiscuous == 0)
+		return 0;
+
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->promiscuous_disable, -ENOTSUP);
 
-	if (dev->data->promiscuous == 1) {
-		dev->data->promiscuous = 0;
-		diag = (*dev->dev_ops->promiscuous_disable)(dev);
-		if (diag != 0)
-			dev->data->promiscuous = 1;
-	}
+	dev->data->promiscuous = 0;
+	diag = (*dev->dev_ops->promiscuous_disable)(dev);
+	if (diag != 0)
+		dev->data->promiscuous = 1;
 
 	return eth_err(port_id, diag);
 }
-- 
2.17.1

[dpdk-dev] [PATCH v2 2/2] drivers/net: set enabled promiscuous and multicast

[Ciara Power]

The promiscuous and multicast fields are now initialised as enabled for
some virtual PMDs. This allows the devices to be used when running
applications that attempt to enable promiscuous or multicast mode.

Signed-off-by: Ciara Power <ciara.power@intel.com>
---
 drivers/net/kni/rte_eth_kni.c   | 2 ++
 drivers/net/null/rte_eth_null.c | 2 ++
 drivers/net/pcap/rte_eth_pcap.c | 2 ++
 drivers/net/ring/rte_eth_ring.c | 2 ++
 4 files changed, 8 insertions(+)

diff --git a/drivers/net/kni/rte_eth_kni.c b/drivers/net/kni/rte_eth_kni.c
index 547e08c3d..7cba92e2e 100644
--- a/drivers/net/kni/rte_eth_kni.c
+++ b/drivers/net/kni/rte_eth_kni.c
@@ -377,6 +377,8 @@ eth_kni_create(struct rte_vdev_device *vdev,
 	data->nb_tx_queues = 1;
 	data->dev_link = pmd_link;
 	data->mac_addrs = &internals->eth_addr;
+	data->promiscuous = 1;
+	data->all_multicast = 1;
 
 	data->dev_flags |= RTE_ETH_DEV_CLOSE_REMOVE;
 
diff --git a/drivers/net/null/rte_eth_null.c b/drivers/net/null/rte_eth_null.c
index e2ff41a22..35920cd1e 100644
--- a/drivers/net/null/rte_eth_null.c
+++ b/drivers/net/null/rte_eth_null.c
@@ -514,6 +514,8 @@ eth_dev_null_create(struct rte_vdev_device *dev,
 	data->nb_tx_queues = (uint16_t)nb_tx_queues;
 	data->dev_link = pmd_link;
 	data->mac_addrs = &internals->eth_addr;
+	data->promiscuous = 1;
+	data->all_multicast = 1;
 
 	eth_dev->dev_ops = &ops;
 
diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index 5801915a8..5186d8fe5 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -1129,6 +1129,8 @@ pmd_init_internals(struct rte_vdev_device *vdev,
 	data->nb_tx_queues = (uint16_t)nb_tx_queues;
 	data->dev_link = pmd_link;
 	data->mac_addrs = &(*internals)->eth_addr;
+	data->promiscuous = 1;
+	data->all_multicast = 1;
 
 	/*
 	 * NOTE: we'll replace the data element, of originally allocated
diff --git a/drivers/net/ring/rte_eth_ring.c b/drivers/net/ring/rte_eth_ring.c
index c6733ee2b..41acbc513 100644
--- a/drivers/net/ring/rte_eth_ring.c
+++ b/drivers/net/ring/rte_eth_ring.c
@@ -318,6 +318,8 @@ do_eth_dev_ring_create(const char *name,
 	data->nb_tx_queues = (uint16_t)nb_tx_queues;
 	data->dev_link = pmd_link;
 	data->mac_addrs = &internals->address;
+	data->promiscuous = 1;
+	data->all_multicast = 1;
 
 	eth_dev->dev_ops = &ops;
 	data->kdrv = RTE_KDRV_NONE;
-- 
2.17.1

Re: [dpdk-dev] [PATCH v2 1/2] ethdev: add check for device promiscuous state

[Ferruh Yigit]

On 10/21/2019 1:22 PM, Ciara Power wrote:
> The promiscuous enable and disable functions now check the
> promiscuous state of the device before checking if the dev_ops
> function exists for the device.
> 
> This change is necessary to allow sample applications run on
> virtual PMDs, as previously -ENOTSUP returned when the promiscuous
> enable function was called. This caused the sample application to
> fail unnecessarily.
> 
> Signed-off-by: Ciara Power <ciara.power@intel.com>

Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

@Andrew, any concern/objection on the patch?

Re: [dpdk-dev] [PATCH v2 2/2] drivers/net: set enabled promiscuous and multicast

[Ferruh Yigit]

On 10/21/2019 1:22 PM, Ciara Power wrote:
> The promiscuous and multicast fields are now initialised as enabled for
> some virtual PMDs. This allows the devices to be used when running
> applications that attempt to enable promiscuous or multicast mode.
> 
> Signed-off-by: Ciara Power <ciara.power@intel.com>

Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

Re: [dpdk-dev] [PATCH v2 1/2] ethdev: add check for device promiscuous state

[Andrew Rybchenko]

On 10/21/19 3:22 PM, Ciara Power wrote:
> The promiscuous enable and disable functions now check the
> promiscuous state of the device before checking if the dev_ops
> function exists for the device.
>
> This change is necessary to allow sample applications run on
> virtual PMDs, as previously -ENOTSUP returned when the promiscuous
> enable function was called. This caused the sample application to
> fail unnecessarily.
>
> Signed-off-by: Ciara Power <ciara.power@intel.com>

Many thanks,
Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>

Re: [dpdk-dev] [PATCH v2 2/2] drivers/net: set enabled promiscuous and multicast

[Andrew Rybchenko]

On 10/21/19 3:22 PM, Ciara Power wrote:
> The promiscuous and multicast fields are now initialised as enabled for
> some virtual PMDs. This allows the devices to be used when running
> applications that attempt to enable promiscuous or multicast mode.
>
> Signed-off-by: Ciara Power <ciara.power@intel.com>

Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>

Re: [dpdk-dev] [PATCH v2 1/2] ethdev: add check for device promiscuous state

[Ferruh Yigit]

On 10/22/2019 8:03 AM, Andrew Rybchenko wrote:
> On 10/21/19 3:22 PM, Ciara Power wrote:
>> The promiscuous enable and disable functions now check the
>> promiscuous state of the device before checking if the dev_ops
>> function exists for the device.
>>
>> This change is necessary to allow sample applications run on
>> virtual PMDs, as previously -ENOTSUP returned when the promiscuous
>> enable function was called. This caused the sample application to
>> fail unnecessarily.
>>
>> Signed-off-by: Ciara Power <ciara.power@intel.com>
> 
> Many thanks,
> Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
> 

Series applied to dpdk-next-net/master, thanks.