[dpdk-dev] [PATCH v3 1/2] net/virtio: refactor devargs parsing

[dpdk-dev] [PATCH v3 1/2] net/virtio: refactor devargs parsing

[Ivan Dyukov]

refactor vdpa specific devargs parsing to more generic way

Signed-off-by: Ivan Dyukov <i.dyukov@samsung.com>
---
 drivers/net/virtio/virtio_ethdev.c | 35 +++++++++++++++++++++---------
 1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 044eb10a7..22323d9a5 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1955,16 +1955,18 @@ eth_virtio_dev_uninit(struct rte_eth_dev *eth_dev)
 }
 
 static int vdpa_check_handler(__rte_unused const char *key,
-		const char *value, __rte_unused void *opaque)
+		const char *value, void *ret_val)
 {
-	if (strcmp(value, "1"))
-		return -1;
+	if (strcmp(value, "1") == 0)
+		*(int *)ret_val = 1;
+	else
+		*(int *)ret_val = 0;
 
 	return 0;
 }
 
 static int
-vdpa_mode_selected(struct rte_devargs *devargs)
+virtio_dev_devargs_parse(struct rte_devargs *devargs, int *vdpa)
 {
 	struct rte_kvargs *kvlist;
 	const char *key = "vdpa";
@@ -1980,12 +1982,16 @@ vdpa_mode_selected(struct rte_devargs *devargs)
 	if (!rte_kvargs_count(kvlist, key))
 		goto exit;
 
-	/* vdpa mode selected when there's a key-value pair: vdpa=1 */
-	if (rte_kvargs_process(kvlist, key,
-				vdpa_check_handler, NULL) < 0) {
-		goto exit;
+	if (vdpa) {
+		/* vdpa mode selected when there's a key-value pair:
+		 * vdpa=1
+		 */
+		ret = rte_kvargs_process(kvlist, key,
+				vdpa_check_handler, vdpa);
+		if (ret  < 0)
+			goto exit;
 	}
-	ret = 1;
+
 
 exit:
 	rte_kvargs_free(kvlist);
@@ -1995,8 +2001,17 @@ vdpa_mode_selected(struct rte_devargs *devargs)
 static int eth_virtio_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 	struct rte_pci_device *pci_dev)
 {
+	int vdpa = 0;
+	int ret = 0;
+
+	ret = virtio_dev_devargs_parse(pci_dev->device.devargs, &vdpa);
+	if (ret < 0) {
+		PMD_DRV_LOG(ERR,
+			"devargs parsing is failed");
+		return ret;
+	}
 	/* virtio pmd skips probe if device needs to work in vdpa mode */
-	if (vdpa_mode_selected(pci_dev->device.devargs))
+	if (vdpa == 1)
 		return 1;
 
 	return rte_eth_dev_pci_generic_probe(pci_dev, sizeof(struct virtio_hw),
-- 
2.17.1

[dpdk-dev] [PATCH v3 2/2] net/virtio: add link speed devarg

[Ivan Dyukov]

Some applications like pktgen use link_speed to calculate
transmit rate. It limits outcome traffic to hardcoded 10G.

This patch adds link_speed devarg which allows to configure
link_speed of virtio device.

Signed-off-by: Ivan Dyukov <i.dyukov@samsung.com>
---
 doc/guides/nics/virtio.rst         |   7 ++
 drivers/net/virtio/virtio_ethdev.c | 101 ++++++++++++++++++++++++-----
 drivers/net/virtio/virtio_pci.h    |   1 +
 3 files changed, 92 insertions(+), 17 deletions(-)

diff --git a/doc/guides/nics/virtio.rst b/doc/guides/nics/virtio.rst
index d1f5fb898..f190f2e4f 100644
--- a/doc/guides/nics/virtio.rst
+++ b/doc/guides/nics/virtio.rst
@@ -356,6 +356,13 @@ Below devargs are supported by the PCI virtio driver:
     a virtio device needs to work in vDPA mode.
     (Default: 0 (disabled))
 
+#.  ``link_speed``:
+
+    It is used to specify link speed of virtio device. Link speed is a part of
+    link status structure. It could be requested by application using
+    rte_eth_link_get_nowait function.
+    (Default: 10000 (10G))
+
 Below devargs are supported by the virtio-user vdev:
 
 #.  ``path``:
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 22323d9a5..5ef3c11a7 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -45,6 +45,10 @@ static int virtio_dev_promiscuous_enable(struct rte_eth_dev *dev);
 static int virtio_dev_promiscuous_disable(struct rte_eth_dev *dev);
 static int virtio_dev_allmulticast_enable(struct rte_eth_dev *dev);
 static int virtio_dev_allmulticast_disable(struct rte_eth_dev *dev);
+static uint32_t virtio_dev_speed_capa_get(uint32_t link_speed);
+static int virtio_dev_devargs_parse(struct rte_devargs *devargs,
+	int *vdpa,
+	uint32_t *link_speed);
 static int virtio_dev_info_get(struct rte_eth_dev *dev,
 				struct rte_eth_dev_info *dev_info);
 static int virtio_dev_link_update(struct rte_eth_dev *dev,
@@ -1861,6 +1865,7 @@ int
 eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
 {
 	struct virtio_hw *hw = eth_dev->data->dev_private;
+	uint32_t link_speed = ETH_SPEED_NUM_10G;
 	int ret;
 
 	if (sizeof(struct virtio_net_hdr_mrg_rxbuf) > RTE_PKTMBUF_HEADROOM) {
@@ -1886,7 +1891,11 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
 
 		return 0;
 	}
-
+	ret = virtio_dev_devargs_parse(eth_dev->device->devargs,
+		 NULL, &link_speed);
+	if (ret < 0)
+		return ret;
+	hw->link_speed = link_speed;
 	/*
 	 * Pass the information to the rte_eth_dev_close() that it should also
 	 * release the private port resources.
@@ -1953,6 +1962,14 @@ eth_virtio_dev_uninit(struct rte_eth_dev *eth_dev)
 
 	return 0;
 }
+#define VIRTIO_ARG_LINK_SPEED "link_speed"
+#define VIRTIO_ARG_VDPA       "vdpa"
+
+static const char * const valid_args[] = {
+	VIRTIO_ARG_LINK_SPEED,
+	VIRTIO_ARG_VDPA,
+	NULL
+};
 
 static int vdpa_check_handler(__rte_unused const char *key,
 		const char *value, void *ret_val)
@@ -1965,33 +1982,84 @@ static int vdpa_check_handler(__rte_unused const char *key,
 	return 0;
 }
 
+
+static uint32_t
+virtio_dev_speed_capa_get(uint32_t link_speed)
+{
+	switch (link_speed) {
+	case ETH_SPEED_NUM_10G:
+		return ETH_LINK_SPEED_10G;
+	case ETH_SPEED_NUM_20G:
+		return ETH_LINK_SPEED_20G;
+	case ETH_SPEED_NUM_25G:
+		return ETH_LINK_SPEED_25G;
+	case ETH_SPEED_NUM_40G:
+		return ETH_LINK_SPEED_40G;
+	case ETH_SPEED_NUM_50G:
+		return ETH_LINK_SPEED_50G;
+	case ETH_SPEED_NUM_56G:
+		return ETH_LINK_SPEED_56G;
+	case ETH_SPEED_NUM_100G:
+		return ETH_LINK_SPEED_100G;
+	default:
+		return 0;
+	}
+}
+
+static int link_speed_handler(const char *key __rte_unused,
+		const char *value, void *ret_val)
+{
+	uint32_t val;
+	if (!value || !ret_val)
+		return -EINVAL;
+	val = strtoul(value, NULL, 0);
+	/* validate input */
+	if (virtio_dev_speed_capa_get(val) == 0)
+		return -EINVAL;
+	*(uint32_t *)ret_val = val;
+
+	return 0;
+}
+
+
 static int
-virtio_dev_devargs_parse(struct rte_devargs *devargs, int *vdpa)
+virtio_dev_devargs_parse(struct rte_devargs *devargs, int *vdpa,
+	uint32_t *link_speed)
 {
 	struct rte_kvargs *kvlist;
-	const char *key = "vdpa";
 	int ret = 0;
 
 	if (devargs == NULL)
 		return 0;
 
-	kvlist = rte_kvargs_parse(devargs->args, NULL);
-	if (kvlist == NULL)
+	kvlist = rte_kvargs_parse(devargs->args, valid_args);
+	if (kvlist == NULL) {
+		PMD_INIT_LOG(ERR, "error when parsing param");
 		return 0;
-
-	if (!rte_kvargs_count(kvlist, key))
-		goto exit;
-
-	if (vdpa) {
+	}
+	if (vdpa && rte_kvargs_count(kvlist, VIRTIO_ARG_VDPA) == 1) {
 		/* vdpa mode selected when there's a key-value pair:
 		 * vdpa=1
 		 */
-		ret = rte_kvargs_process(kvlist, key,
+		ret = rte_kvargs_process(kvlist, VIRTIO_ARG_VDPA,
 				vdpa_check_handler, vdpa);
-		if (ret  < 0)
+		if (ret  < 0) {
+			PMD_INIT_LOG(ERR, "error to parse %s",
+				VIRTIO_ARG_VDPA);
 			goto exit;
+		}
+	}
+	if (link_speed &&
+		 rte_kvargs_count(kvlist, VIRTIO_ARG_LINK_SPEED) == 1) {
+		ret = rte_kvargs_process(kvlist,
+					VIRTIO_ARG_LINK_SPEED,
+					link_speed_handler, link_speed);
+		if (ret < 0) {
+			PMD_INIT_LOG(ERR, "error to parse %s",
+					VIRTIO_ARG_LINK_SPEED);
+			goto exit;
+		}
 	}
-
 
 exit:
 	rte_kvargs_free(kvlist);
@@ -2004,7 +2072,7 @@ static int eth_virtio_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 	int vdpa = 0;
 	int ret = 0;
 
-	ret = virtio_dev_devargs_parse(pci_dev->device.devargs, &vdpa);
+	ret = virtio_dev_devargs_parse(pci_dev->device.devargs, &vdpa, NULL);
 	if (ret < 0) {
 		PMD_DRV_LOG(ERR,
 			"devargs parsing is failed");
@@ -2386,7 +2454,7 @@ virtio_dev_link_update(struct rte_eth_dev *dev, __rte_unused int wait_to_complet
 
 	memset(&link, 0, sizeof(link));
 	link.link_duplex = ETH_LINK_FULL_DUPLEX;
-	link.link_speed  = ETH_SPEED_NUM_10G;
+	link.link_speed  = hw->link_speed;
 	link.link_autoneg = ETH_LINK_FIXED;
 
 	if (!hw->started) {
@@ -2441,8 +2509,7 @@ virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 {
 	uint64_t tso_mask, host_features;
 	struct virtio_hw *hw = dev->data->dev_private;
-
-	dev_info->speed_capa = ETH_LINK_SPEED_10G; /* fake value */
+	dev_info->speed_capa = virtio_dev_speed_capa_get(hw->link_speed);
 
 	dev_info->max_rx_queues =
 		RTE_MIN(hw->max_queue_pairs, VIRTIO_MAX_RX_QUEUES);
diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
index a38cb45ad..688eda914 100644
--- a/drivers/net/virtio/virtio_pci.h
+++ b/drivers/net/virtio/virtio_pci.h
@@ -253,6 +253,7 @@ struct virtio_hw {
 	uint16_t    port_id;
 	uint8_t     mac_addr[RTE_ETHER_ADDR_LEN];
 	uint32_t    notify_off_multiplier;
+	uint32_t    link_speed;  /* link speed in MB */
 	uint8_t     *isr;
 	uint16_t    *notify_base;
 	struct virtio_pci_common_cfg *common_cfg;
-- 
2.17.1

Re: [dpdk-dev] [PATCH v3 2/2] net/virtio: add link speed devarg

[Maxime Coquelin]

On 2/7/20 12:25 PM, Ivan Dyukov wrote:
> Some applications like pktgen use link_speed to calculate
> transmit rate. It limits outcome traffic to hardcoded 10G.
> 
> This patch adds link_speed devarg which allows to configure
> link_speed of virtio device.
> 
> Signed-off-by: Ivan Dyukov <i.dyukov@samsung.com>
> ---
>  doc/guides/nics/virtio.rst         |   7 ++
>  drivers/net/virtio/virtio_ethdev.c | 101 ++++++++++++++++++++++++-----
>  drivers/net/virtio/virtio_pci.h    |   1 +
>  3 files changed, 92 insertions(+), 17 deletions(-)
> 

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime

Re: [dpdk-dev] [PATCH v3 2/2] net/virtio: add link speed devarg

[Tiwei Bie]

On Fri, Feb 07, 2020 at 02:25:26PM +0300, Ivan Dyukov wrote:
> Some applications like pktgen use link_speed to calculate
> transmit rate. It limits outcome traffic to hardcoded 10G.
> 
> This patch adds link_speed devarg which allows to configure
> link_speed of virtio device.
> 
> Signed-off-by: Ivan Dyukov <i.dyukov@samsung.com>
> ---
>  doc/guides/nics/virtio.rst         |   7 ++
>  drivers/net/virtio/virtio_ethdev.c | 101 ++++++++++++++++++++++++-----
>  drivers/net/virtio/virtio_pci.h    |   1 +
>  3 files changed, 92 insertions(+), 17 deletions(-)

Maybe I missed something.. Why not enable the VIRTIO_NET_F_SPEED_DUPLEX
support directly? When that feature is supported and negotiated,
we will ignore this devarg?

If we want this devarg, it looks better to support it in virtio-user
as well (most code is shared between virtio-PMD and virtio-user).

Thanks!
Tiwei

Re: [dpdk-dev] [PATCH v3 2/2] net/virtio: add link speed devarg

[Maxime Coquelin]

On 2/12/20 11:35 AM, Tiwei Bie wrote:
> On Fri, Feb 07, 2020 at 02:25:26PM +0300, Ivan Dyukov wrote:
>> Some applications like pktgen use link_speed to calculate
>> transmit rate. It limits outcome traffic to hardcoded 10G.
>>
>> This patch adds link_speed devarg which allows to configure
>> link_speed of virtio device.
>>
>> Signed-off-by: Ivan Dyukov <i.dyukov@samsung.com>
>> ---
>>  doc/guides/nics/virtio.rst         |   7 ++
>>  drivers/net/virtio/virtio_ethdev.c | 101 ++++++++++++++++++++++++-----
>>  drivers/net/virtio/virtio_pci.h    |   1 +
>>  3 files changed, 92 insertions(+), 17 deletions(-)
> 
> Maybe I missed something.. Why not enable the VIRTIO_NET_F_SPEED_DUPLEX
> support directly? When that feature is supported and negotiated,
> we will ignore this devarg?

IMHO, yes. When VIRTIO_NET_F_SPEED_DUPLEX will be implemented and
negotiated, this devarg will be ignored.

> If we want this devarg, it looks better to support it in virtio-user
> as well (most code is shared between virtio-PMD and virtio-user).

That's a valid point, Virtio-user should also support it.
Ivan, can you send a new revision with Virtio-user support as well?

Thanks,
Maxime

> Thanks!
> Tiwei
> 

Re: [dpdk-dev] [PATCH v3 2/2] net/virtio: add link speed devarg

[Tiwei Bie]

On Wed, Feb 12, 2020 at 11:40:40AM +0100, Maxime Coquelin wrote:
> On 2/12/20 11:35 AM, Tiwei Bie wrote:
> > On Fri, Feb 07, 2020 at 02:25:26PM +0300, Ivan Dyukov wrote:
> >> Some applications like pktgen use link_speed to calculate
> >> transmit rate. It limits outcome traffic to hardcoded 10G.
> >>
> >> This patch adds link_speed devarg which allows to configure
> >> link_speed of virtio device.
> >>
> >> Signed-off-by: Ivan Dyukov <i.dyukov@samsung.com>
> >> ---
> >>  doc/guides/nics/virtio.rst         |   7 ++
> >>  drivers/net/virtio/virtio_ethdev.c | 101 ++++++++++++++++++++++++-----
> >>  drivers/net/virtio/virtio_pci.h    |   1 +
> >>  3 files changed, 92 insertions(+), 17 deletions(-)
> > 
> > Maybe I missed something.. Why not enable the VIRTIO_NET_F_SPEED_DUPLEX
> > support directly? When that feature is supported and negotiated,
> > we will ignore this devarg?
> 
> IMHO, yes. When VIRTIO_NET_F_SPEED_DUPLEX will be implemented and
> negotiated, this devarg will be ignored.

Thanks for the clarification.

> 
> > If we want this devarg, it looks better to support it in virtio-user
> > as well (most code is shared between virtio-PMD and virtio-user).
> 
> That's a valid point, Virtio-user should also support it.
> Ivan, can you send a new revision with Virtio-user support as well?

Thanks!

Regards,
Tiwei

> 
> Thanks,
> Maxime
> 
> > Thanks!
> > Tiwei
> > 
> 

Re: [dpdk-dev] [PATCH v3 2/2] net/virtio: add link speed devarg

[Maxime Coquelin]

Hi Ivan,

On 2/12/20 11:40 AM, Maxime Coquelin wrote:
> 
> 
> On 2/12/20 11:35 AM, Tiwei Bie wrote:
>> On Fri, Feb 07, 2020 at 02:25:26PM +0300, Ivan Dyukov wrote:
>>> Some applications like pktgen use link_speed to calculate
>>> transmit rate. It limits outcome traffic to hardcoded 10G.
>>>
>>> This patch adds link_speed devarg which allows to configure
>>> link_speed of virtio device.
>>>
>>> Signed-off-by: Ivan Dyukov <i.dyukov@samsung.com>
>>> ---
>>>  doc/guides/nics/virtio.rst         |   7 ++
>>>  drivers/net/virtio/virtio_ethdev.c | 101 ++++++++++++++++++++++++-----
>>>  drivers/net/virtio/virtio_pci.h    |   1 +
>>>  3 files changed, 92 insertions(+), 17 deletions(-)
>>
>> Maybe I missed something.. Why not enable the VIRTIO_NET_F_SPEED_DUPLEX
>> support directly? When that feature is supported and negotiated,
>> we will ignore this devarg?
> 
> IMHO, yes. When VIRTIO_NET_F_SPEED_DUPLEX will be implemented and
> negotiated, this devarg will be ignored.
> 
>> If we want this devarg, it looks better to support it in virtio-user
>> as well (most code is shared between virtio-PMD and virtio-user).
> 
> That's a valid point, Virtio-user should also support it.
> Ivan, can you send a new revision with Virtio-user support as well?

Do you plan to submit the new revision today?

Thanks,
Maxime

> Thanks,
> Maxime
> 
>> Thanks!
>> Tiwei
>>

Re: [dpdk-dev] [PATCH v3 2/2] net/virtio: add link speed devarg

[Ivan Dyukov]

Hi Maxime,

13.02.2020 16:54, Maxime Coquelin пишет:
> Hi Ivan,
>
> On 2/12/20 11:40 AM, Maxime Coquelin wrote:
>>
>> On 2/12/20 11:35 AM, Tiwei Bie wrote:
>>> On Fri, Feb 07, 2020 at 02:25:26PM +0300, Ivan Dyukov wrote:
>>>> Some applications like pktgen use link_speed to calculate
>>>> transmit rate. It limits outcome traffic to hardcoded 10G.
>>>>
>>>> This patch adds link_speed devarg which allows to configure
>>>> link_speed of virtio device.
>>>>
>>>> Signed-off-by: Ivan Dyukov <i.dyukov@samsung.com>
>>>> ---
>>>>   doc/guides/nics/virtio.rst         |   7 ++
>>>>   drivers/net/virtio/virtio_ethdev.c | 101 ++++++++++++++++++++++++-----
>>>>   drivers/net/virtio/virtio_pci.h    |   1 +
>>>>   3 files changed, 92 insertions(+), 17 deletions(-)
>>> Maybe I missed something.. Why not enable the VIRTIO_NET_F_SPEED_DUPLEX
>>> support directly? When that feature is supported and negotiated,
>>> we will ignore this devarg?
>> IMHO, yes. When VIRTIO_NET_F_SPEED_DUPLEX will be implemented and
>> negotiated, this devarg will be ignored.
>>
>>> If we want this devarg, it looks better to support it in virtio-user
>>> as well (most code is shared between virtio-PMD and virtio-user).
>> That's a valid point, Virtio-user should also support it.
>> Ivan, can you send a new revision with Virtio-user support as well?
> Do you plan to submit the new revision today?

I have a lot of work which is not related to opensource contribution. I need few days to prepare the patch.

Best regards,
Ivan

>
> Thanks,
> Maxime
>
>> Thanks,
>> Maxime
>>
>>> Thanks!
>>> Tiwei
>>>
>

Re: [dpdk-dev] [PATCH v3 2/2] net/virtio: add link speed devarg

[Maxime Coquelin]

Hi Ivan,

On 2/14/20 8:59 AM, Ivan Dyukov wrote:
> Hi Maxime,
> 
> 13.02.2020 16:54, Maxime Coquelin пишет:
>> Hi Ivan,
>>
>> On 2/12/20 11:40 AM, Maxime Coquelin wrote:
>>> On 2/12/20 11:35 AM, Tiwei Bie wrote:
>>>> On Fri, Feb 07, 2020 at 02:25:26PM +0300, Ivan Dyukov wrote:
>>>>> Some applications like pktgen use link_speed to calculate
>>>>> transmit rate. It limits outcome traffic to hardcoded 10G.
>>>>>
>>>>> This patch adds link_speed devarg which allows to configure
>>>>> link_speed of virtio device.
>>>>>
>>>>> Signed-off-by: Ivan Dyukov <i.dyukov@samsung.com>
>>>>> ---
>>>>>  doc/guides/nics/virtio.rst         |   7 ++
>>>>>  drivers/net/virtio/virtio_ethdev.c | 101 ++++++++++++++++++++++++-----
>>>>>  drivers/net/virtio/virtio_pci.h    |   1 +
>>>>>  3 files changed, 92 insertions(+), 17 deletions(-)
>>>> Maybe I missed something.. Why not enable the VIRTIO_NET_F_SPEED_DUPLEX
>>>> support directly? When that feature is supported and negotiated,
>>>> we will ignore this devarg?
>>> IMHO, yes. When VIRTIO_NET_F_SPEED_DUPLEX will be implemented and
>>> negotiated, this devarg will be ignored.
>>>
>>>> If we want this devarg, it looks better to support it in virtio-user
>>>> as well (most code is shared between virtio-PMD and virtio-user).
>>> That's a valid point, Virtio-user should also support it.
>>> Ivan, can you send a new revision with Virtio-user support as well?
>> Do you plan to submit the new revision today?
> 
> I have a lot of work which is not related to opensource contribution. I need few days to prepare the patch.

OK, that's a problem for v20.02 as we are already at -rc3, which should
not contain new features but only fixes.

I was fine to waive for -rc3, but it will be too risky for -rc4.

Regards,
Maxime

> Best regards,
> Ivan
> 
>> Thanks,
>> Maxime
>>
>>> Thanks,
>>> Maxime
>>>
>>>> Thanks!
>>>> Tiwei
>>>>
> 
>  
> 
>   
> 

[dpdk-dev] speed devarg for virtio driver

[Ivan Dyukov]

[PATCH v4 1/4] net/virtio: refactor devargs parsing
[PATCH v4 2/4] net/virtio: add link speed devarg
[PATCH v4 3/4] net/virtio-user: fix devargs parsing
[PATCH v4 4/4] net/virtio-user: adding link speed devarg
---
v4 changes:
* link_speed renamed to speed devarg
* speed devarg is added to virtio-user driver

v3 changes:
* link_speed devarg is added to virtio documentation

[dpdk-dev] [PATCH v4 1/4] net/virtio: refactor devargs parsing

[Ivan Dyukov]

refactor vdpa specific devargs parsing to more generic way

Signed-off-by: Ivan Dyukov <i.dyukov@samsung.com>
---
 drivers/net/virtio/virtio_ethdev.c | 35 +++++++++++++++++++++---------
 1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 044eb10a7..22323d9a5 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1955,16 +1955,18 @@ eth_virtio_dev_uninit(struct rte_eth_dev *eth_dev)
 }
 
 static int vdpa_check_handler(__rte_unused const char *key,
-		const char *value, __rte_unused void *opaque)
+		const char *value, void *ret_val)
 {
-	if (strcmp(value, "1"))
-		return -1;
+	if (strcmp(value, "1") == 0)
+		*(int *)ret_val = 1;
+	else
+		*(int *)ret_val = 0;
 
 	return 0;
 }
 
 static int
-vdpa_mode_selected(struct rte_devargs *devargs)
+virtio_dev_devargs_parse(struct rte_devargs *devargs, int *vdpa)
 {
 	struct rte_kvargs *kvlist;
 	const char *key = "vdpa";
@@ -1980,12 +1982,16 @@ vdpa_mode_selected(struct rte_devargs *devargs)
 	if (!rte_kvargs_count(kvlist, key))
 		goto exit;
 
-	/* vdpa mode selected when there's a key-value pair: vdpa=1 */
-	if (rte_kvargs_process(kvlist, key,
-				vdpa_check_handler, NULL) < 0) {
-		goto exit;
+	if (vdpa) {
+		/* vdpa mode selected when there's a key-value pair:
+		 * vdpa=1
+		 */
+		ret = rte_kvargs_process(kvlist, key,
+				vdpa_check_handler, vdpa);
+		if (ret  < 0)
+			goto exit;
 	}
-	ret = 1;
+
 
 exit:
 	rte_kvargs_free(kvlist);
@@ -1995,8 +2001,17 @@ vdpa_mode_selected(struct rte_devargs *devargs)
 static int eth_virtio_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 	struct rte_pci_device *pci_dev)
 {
+	int vdpa = 0;
+	int ret = 0;
+
+	ret = virtio_dev_devargs_parse(pci_dev->device.devargs, &vdpa);
+	if (ret < 0) {
+		PMD_DRV_LOG(ERR,
+			"devargs parsing is failed");
+		return ret;
+	}
 	/* virtio pmd skips probe if device needs to work in vdpa mode */
-	if (vdpa_mode_selected(pci_dev->device.devargs))
+	if (vdpa == 1)
 		return 1;
 
 	return rte_eth_dev_pci_generic_probe(pci_dev, sizeof(struct virtio_hw),
-- 
2.17.1

Re: [dpdk-dev] [PATCH v4 1/4] net/virtio: refactor devargs parsing

[Ye Xiaolong]

On 02/25, Ivan Dyukov wrote:
>refactor vdpa specific devargs parsing to more generic way
>
>Signed-off-by: Ivan Dyukov <i.dyukov@samsung.com>
>---
> drivers/net/virtio/virtio_ethdev.c | 35 +++++++++++++++++++++---------
> 1 file changed, 25 insertions(+), 10 deletions(-)
>
>diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
>index 044eb10a7..22323d9a5 100644
>--- a/drivers/net/virtio/virtio_ethdev.c
>+++ b/drivers/net/virtio/virtio_ethdev.c
>@@ -1955,16 +1955,18 @@ eth_virtio_dev_uninit(struct rte_eth_dev *eth_dev)
> }
> 
> static int vdpa_check_handler(__rte_unused const char *key,
>-		const char *value, __rte_unused void *opaque)
>+		const char *value, void *ret_val)
> {
>-	if (strcmp(value, "1"))
>-		return -1;
>+	if (strcmp(value, "1") == 0)
>+		*(int *)ret_val = 1;
>+	else
>+		*(int *)ret_val = 0;
> 
> 	return 0;
> }
> 
> static int
>-vdpa_mode_selected(struct rte_devargs *devargs)
>+virtio_dev_devargs_parse(struct rte_devargs *devargs, int *vdpa)
> {
> 	struct rte_kvargs *kvlist;
> 	const char *key = "vdpa";
>@@ -1980,12 +1982,16 @@ vdpa_mode_selected(struct rte_devargs *devargs)
> 	if (!rte_kvargs_count(kvlist, key))
> 		goto exit;
> 
>-	/* vdpa mode selected when there's a key-value pair: vdpa=1 */
>-	if (rte_kvargs_process(kvlist, key,
>-				vdpa_check_handler, NULL) < 0) {
>-		goto exit;
>+	if (vdpa) {
>+		/* vdpa mode selected when there's a key-value pair:
>+		 * vdpa=1
>+		 */
>+		ret = rte_kvargs_process(kvlist, key,
>+				vdpa_check_handler, vdpa);
>+		if (ret  < 0)

Double space between 'ret' and '<'.

>+			goto exit;
> 	}
>-	ret = 1;
>+
> 
> exit:
> 	rte_kvargs_free(kvlist);
>@@ -1995,8 +2001,17 @@ vdpa_mode_selected(struct rte_devargs *devargs)
> static int eth_virtio_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
> 	struct rte_pci_device *pci_dev)
> {
>+	int vdpa = 0;
>+	int ret = 0;
>+
>+	ret = virtio_dev_devargs_parse(pci_dev->device.devargs, &vdpa);
>+	if (ret < 0) {
>+		PMD_DRV_LOG(ERR,
>+			"devargs parsing is failed");

I think PMD_INIG_LOG should be used, and there is no need to split into 2 lines
as it won't exceed 80 chars.

Thanks,
Xiaolong

>+		return ret;
>+	}
> 	/* virtio pmd skips probe if device needs to work in vdpa mode */
>-	if (vdpa_mode_selected(pci_dev->device.devargs))
>+	if (vdpa == 1)
> 		return 1;
> 
> 	return rte_eth_dev_pci_generic_probe(pci_dev, sizeof(struct virtio_hw),
>-- 
>2.17.1
>

[dpdk-dev] [PATCH v4 2/4] net/virtio: add link speed devarg

[Ivan Dyukov]

Some applications like pktgen use link_speed to calculate
transmit rate. It limits outcome traffic to hardcoded 10G.

This patch adds link_speed devarg which allows to configure
link_speed of virtio device.

Signed-off-by: Ivan Dyukov <i.dyukov@samsung.com>
---
 doc/guides/nics/virtio.rst         |  7 +++
 drivers/net/virtio/virtio_ethdev.c | 97 +++++++++++++++++++++++++-----
 drivers/net/virtio/virtio_pci.h    |  1 +
 3 files changed, 89 insertions(+), 16 deletions(-)

diff --git a/doc/guides/nics/virtio.rst b/doc/guides/nics/virtio.rst
index d1f5fb898..0341907ef 100644
--- a/doc/guides/nics/virtio.rst
+++ b/doc/guides/nics/virtio.rst
@@ -356,6 +356,13 @@ Below devargs are supported by the PCI virtio driver:
     a virtio device needs to work in vDPA mode.
     (Default: 0 (disabled))
 
+#.  ``speed``:
+
+    It is used to specify link speed of virtio device. Link speed is a part of
+    link status structure. It could be requested by application using
+    rte_eth_link_get_nowait function.
+    (Default: 10000 (10G))
+
 Below devargs are supported by the virtio-user vdev:
 
 #.  ``path``:
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 22323d9a5..9707c8cbc 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -45,6 +45,10 @@ static int virtio_dev_promiscuous_enable(struct rte_eth_dev *dev);
 static int virtio_dev_promiscuous_disable(struct rte_eth_dev *dev);
 static int virtio_dev_allmulticast_enable(struct rte_eth_dev *dev);
 static int virtio_dev_allmulticast_disable(struct rte_eth_dev *dev);
+static uint32_t virtio_dev_speed_capa_get(uint32_t speed);
+static int virtio_dev_devargs_parse(struct rte_devargs *devargs,
+	int *vdpa,
+	uint32_t *speed);
 static int virtio_dev_info_get(struct rte_eth_dev *dev,
 				struct rte_eth_dev_info *dev_info);
 static int virtio_dev_link_update(struct rte_eth_dev *dev,
@@ -1861,6 +1865,7 @@ int
 eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
 {
 	struct virtio_hw *hw = eth_dev->data->dev_private;
+	uint32_t speed = ETH_SPEED_NUM_10G;
 	int ret;
 
 	if (sizeof(struct virtio_net_hdr_mrg_rxbuf) > RTE_PKTMBUF_HEADROOM) {
@@ -1886,7 +1891,11 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
 
 		return 0;
 	}
-
+	ret = virtio_dev_devargs_parse(eth_dev->device->devargs,
+		 NULL, &speed);
+	if (ret < 0)
+		return ret;
+	hw->speed = speed;
 	/*
 	 * Pass the information to the rte_eth_dev_close() that it should also
 	 * release the private port resources.
@@ -1954,6 +1963,7 @@ eth_virtio_dev_uninit(struct rte_eth_dev *eth_dev)
 	return 0;
 }
 
+
 static int vdpa_check_handler(__rte_unused const char *key,
 		const char *value, void *ret_val)
 {
@@ -1965,33 +1975,89 @@ static int vdpa_check_handler(__rte_unused const char *key,
 	return 0;
 }
 
+
+static uint32_t
+virtio_dev_speed_capa_get(uint32_t speed)
+{
+	switch (speed) {
+	case ETH_SPEED_NUM_10G:
+		return ETH_LINK_SPEED_10G;
+	case ETH_SPEED_NUM_20G:
+		return ETH_LINK_SPEED_20G;
+	case ETH_SPEED_NUM_25G:
+		return ETH_LINK_SPEED_25G;
+	case ETH_SPEED_NUM_40G:
+		return ETH_LINK_SPEED_40G;
+	case ETH_SPEED_NUM_50G:
+		return ETH_LINK_SPEED_50G;
+	case ETH_SPEED_NUM_56G:
+		return ETH_LINK_SPEED_56G;
+	case ETH_SPEED_NUM_100G:
+		return ETH_LINK_SPEED_100G;
+	default:
+		return 0;
+	}
+}
+
+
+#define VIRTIO_ARG_SPEED      "speed"
+#define VIRTIO_ARG_VDPA       "vdpa"
+
+
+static int link_speed_handler(const char *key __rte_unused,
+		const char *value, void *ret_val)
+{
+	uint32_t val;
+	if (!value || !ret_val)
+		return -EINVAL;
+	val = strtoul(value, NULL, 0);
+	/* validate input */
+	if (virtio_dev_speed_capa_get(val) == 0)
+		return -EINVAL;
+	*(uint32_t *)ret_val = val;
+
+	return 0;
+}
+
+
 static int
-virtio_dev_devargs_parse(struct rte_devargs *devargs, int *vdpa)
+virtio_dev_devargs_parse(struct rte_devargs *devargs, int *vdpa,
+	uint32_t *speed)
 {
 	struct rte_kvargs *kvlist;
-	const char *key = "vdpa";
 	int ret = 0;
 
 	if (devargs == NULL)
 		return 0;
 
 	kvlist = rte_kvargs_parse(devargs->args, NULL);
-	if (kvlist == NULL)
+	if (kvlist == NULL) {
+		PMD_INIT_LOG(ERR, "error when parsing param");
 		return 0;
-
-	if (!rte_kvargs_count(kvlist, key))
-		goto exit;
-
-	if (vdpa) {
+	}
+	if (vdpa && rte_kvargs_count(kvlist, VIRTIO_ARG_VDPA) == 1) {
 		/* vdpa mode selected when there's a key-value pair:
 		 * vdpa=1
 		 */
-		ret = rte_kvargs_process(kvlist, key,
+		ret = rte_kvargs_process(kvlist, VIRTIO_ARG_VDPA,
 				vdpa_check_handler, vdpa);
-		if (ret  < 0)
+		if (ret  < 0) {
+			PMD_INIT_LOG(ERR, "error to parse %s",
+				VIRTIO_ARG_VDPA);
 			goto exit;
+		}
+	}
+	if (speed &&
+		 rte_kvargs_count(kvlist, VIRTIO_ARG_SPEED) == 1) {
+		ret = rte_kvargs_process(kvlist,
+					VIRTIO_ARG_SPEED,
+					link_speed_handler, speed);
+		if (ret < 0) {
+			PMD_INIT_LOG(ERR, "error to parse %s",
+					VIRTIO_ARG_SPEED);
+			goto exit;
+		}
 	}
-
 
 exit:
 	rte_kvargs_free(kvlist);
@@ -2004,7 +2070,7 @@ static int eth_virtio_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 	int vdpa = 0;
 	int ret = 0;
 
-	ret = virtio_dev_devargs_parse(pci_dev->device.devargs, &vdpa);
+	ret = virtio_dev_devargs_parse(pci_dev->device.devargs, &vdpa, NULL);
 	if (ret < 0) {
 		PMD_DRV_LOG(ERR,
 			"devargs parsing is failed");
@@ -2386,7 +2452,7 @@ virtio_dev_link_update(struct rte_eth_dev *dev, __rte_unused int wait_to_complet
 
 	memset(&link, 0, sizeof(link));
 	link.link_duplex = ETH_LINK_FULL_DUPLEX;
-	link.link_speed  = ETH_SPEED_NUM_10G;
+	link.link_speed  = hw->speed;
 	link.link_autoneg = ETH_LINK_FIXED;
 
 	if (!hw->started) {
@@ -2441,8 +2507,7 @@ virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 {
 	uint64_t tso_mask, host_features;
 	struct virtio_hw *hw = dev->data->dev_private;
-
-	dev_info->speed_capa = ETH_LINK_SPEED_10G; /* fake value */
+	dev_info->speed_capa = virtio_dev_speed_capa_get(hw->speed);
 
 	dev_info->max_rx_queues =
 		RTE_MIN(hw->max_queue_pairs, VIRTIO_MAX_RX_QUEUES);
diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
index a38cb45ad..59012d55a 100644
--- a/drivers/net/virtio/virtio_pci.h
+++ b/drivers/net/virtio/virtio_pci.h
@@ -253,6 +253,7 @@ struct virtio_hw {
 	uint16_t    port_id;
 	uint8_t     mac_addr[RTE_ETHER_ADDR_LEN];
 	uint32_t    notify_off_multiplier;
+	uint32_t    speed;  /* link speed in MB */
 	uint8_t     *isr;
 	uint16_t    *notify_base;
 	struct virtio_pci_common_cfg *common_cfg;
-- 
2.17.1

Re: [dpdk-dev] [PATCH v4 2/4] net/virtio: add link speed devarg

[Ye Xiaolong]

On 02/25, Ivan Dyukov wrote:
>Some applications like pktgen use link_speed to calculate
>transmit rate. It limits outcome traffic to hardcoded 10G.

s/transmit/transmission

>
>This patch adds link_speed devarg which allows to configure

s/link_speed, given the fact that you've renamed link_speed devarg to speed
in this version.

>link_speed of virtio device.
>
>Signed-off-by: Ivan Dyukov <i.dyukov@samsung.com>
>---
> doc/guides/nics/virtio.rst         |  7 +++
> drivers/net/virtio/virtio_ethdev.c | 97 +++++++++++++++++++++++++-----
> drivers/net/virtio/virtio_pci.h    |  1 +
> 3 files changed, 89 insertions(+), 16 deletions(-)
>
>diff --git a/doc/guides/nics/virtio.rst b/doc/guides/nics/virtio.rst
>index d1f5fb898..0341907ef 100644
>--- a/doc/guides/nics/virtio.rst
>+++ b/doc/guides/nics/virtio.rst
>@@ -356,6 +356,13 @@ Below devargs are supported by the PCI virtio driver:
>     a virtio device needs to work in vDPA mode.
>     (Default: 0 (disabled))
> 
>+#.  ``speed``:
>+
>+    It is used to specify link speed of virtio device. Link speed is a part of
>+    link status structure. It could be requested by application using
>+    rte_eth_link_get_nowait function.
>+    (Default: 10000 (10G))
>+
> Below devargs are supported by the virtio-user vdev:
> 
> #.  ``path``:
>diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
>index 22323d9a5..9707c8cbc 100644
>--- a/drivers/net/virtio/virtio_ethdev.c
>+++ b/drivers/net/virtio/virtio_ethdev.c
>@@ -45,6 +45,10 @@ static int virtio_dev_promiscuous_enable(struct rte_eth_dev *dev);
> static int virtio_dev_promiscuous_disable(struct rte_eth_dev *dev);
> static int virtio_dev_allmulticast_enable(struct rte_eth_dev *dev);
> static int virtio_dev_allmulticast_disable(struct rte_eth_dev *dev);
>+static uint32_t virtio_dev_speed_capa_get(uint32_t speed);
>+static int virtio_dev_devargs_parse(struct rte_devargs *devargs,
>+	int *vdpa,
>+	uint32_t *speed);
> static int virtio_dev_info_get(struct rte_eth_dev *dev,
> 				struct rte_eth_dev_info *dev_info);
> static int virtio_dev_link_update(struct rte_eth_dev *dev,
>@@ -1861,6 +1865,7 @@ int
> eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
> {
> 	struct virtio_hw *hw = eth_dev->data->dev_private;
>+	uint32_t speed = ETH_SPEED_NUM_10G;
> 	int ret;
> 
> 	if (sizeof(struct virtio_net_hdr_mrg_rxbuf) > RTE_PKTMBUF_HEADROOM) {
>@@ -1886,7 +1891,11 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
> 
> 		return 0;
> 	}
>-
>+	ret = virtio_dev_devargs_parse(eth_dev->device->devargs,
>+		 NULL, &speed);
>+	if (ret < 0)
>+		return ret;
>+	hw->speed = speed;
> 	/*
> 	 * Pass the information to the rte_eth_dev_close() that it should also
> 	 * release the private port resources.
>@@ -1954,6 +1963,7 @@ eth_virtio_dev_uninit(struct rte_eth_dev *eth_dev)
> 	return 0;
> }
> 
>+
> static int vdpa_check_handler(__rte_unused const char *key,
> 		const char *value, void *ret_val)
> {
>@@ -1965,33 +1975,89 @@ static int vdpa_check_handler(__rte_unused const char *key,
> 	return 0;
> }
> 
>+
>+static uint32_t
>+virtio_dev_speed_capa_get(uint32_t speed)
>+{
>+	switch (speed) {
>+	case ETH_SPEED_NUM_10G:
>+		return ETH_LINK_SPEED_10G;
>+	case ETH_SPEED_NUM_20G:
>+		return ETH_LINK_SPEED_20G;
>+	case ETH_SPEED_NUM_25G:
>+		return ETH_LINK_SPEED_25G;
>+	case ETH_SPEED_NUM_40G:
>+		return ETH_LINK_SPEED_40G;
>+	case ETH_SPEED_NUM_50G:
>+		return ETH_LINK_SPEED_50G;
>+	case ETH_SPEED_NUM_56G:
>+		return ETH_LINK_SPEED_56G;
>+	case ETH_SPEED_NUM_100G:
>+		return ETH_LINK_SPEED_100G;
>+	default:
>+		return 0;
>+	}
>+}
>+
>+
>+#define VIRTIO_ARG_SPEED      "speed"
>+#define VIRTIO_ARG_VDPA       "vdpa"
>+
>+
>+static int link_speed_handler(const char *key __rte_unused,
>+		const char *value, void *ret_val)

Put the `static int` in a separate line.

>+{
>+	uint32_t val;
>+	if (!value || !ret_val)
>+		return -EINVAL;
>+	val = strtoul(value, NULL, 0);
>+	/* validate input */
>+	if (virtio_dev_speed_capa_get(val) == 0)
>+		return -EINVAL;
>+	*(uint32_t *)ret_val = val;
>+
>+	return 0;
>+}
>+
>+
> static int
>-virtio_dev_devargs_parse(struct rte_devargs *devargs, int *vdpa)
>+virtio_dev_devargs_parse(struct rte_devargs *devargs, int *vdpa,
>+	uint32_t *speed)
> {
> 	struct rte_kvargs *kvlist;
>-	const char *key = "vdpa";
> 	int ret = 0;
> 
> 	if (devargs == NULL)
> 		return 0;
> 
> 	kvlist = rte_kvargs_parse(devargs->args, NULL);
>-	if (kvlist == NULL)
>+	if (kvlist == NULL) {
>+		PMD_INIT_LOG(ERR, "error when parsing param");
> 		return 0;
>-
>-	if (!rte_kvargs_count(kvlist, key))
>-		goto exit;
>-
>-	if (vdpa) {
>+	}
>+	if (vdpa && rte_kvargs_count(kvlist, VIRTIO_ARG_VDPA) == 1) {
> 		/* vdpa mode selected when there's a key-value pair:
> 		 * vdpa=1
> 		 */
>-		ret = rte_kvargs_process(kvlist, key,
>+		ret = rte_kvargs_process(kvlist, VIRTIO_ARG_VDPA,
> 				vdpa_check_handler, vdpa);
>-		if (ret  < 0)
>+		if (ret  < 0) {
>+			PMD_INIT_LOG(ERR, "error to parse %s",

s/error to/Failed to

>+				VIRTIO_ARG_VDPA);
> 			goto exit;
>+		}
>+	}
>+	if (speed &&
>+		 rte_kvargs_count(kvlist, VIRTIO_ARG_SPEED) == 1) {

No need to split the condition statement line.

>+		ret = rte_kvargs_process(kvlist,
>+					VIRTIO_ARG_SPEED,
>+					link_speed_handler, speed);
>+		if (ret < 0) {
>+			PMD_INIT_LOG(ERR, "error to parse %s",

s/error to/Failed to

>+					VIRTIO_ARG_SPEED);
>+			goto exit;
>+		}
> 	}
>-
> 
> exit:
> 	rte_kvargs_free(kvlist);
>@@ -2004,7 +2070,7 @@ static int eth_virtio_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
> 	int vdpa = 0;
> 	int ret = 0;
> 
>-	ret = virtio_dev_devargs_parse(pci_dev->device.devargs, &vdpa);
>+	ret = virtio_dev_devargs_parse(pci_dev->device.devargs, &vdpa, NULL);
> 	if (ret < 0) {
> 		PMD_DRV_LOG(ERR,
> 			"devargs parsing is failed");
>@@ -2386,7 +2452,7 @@ virtio_dev_link_update(struct rte_eth_dev *dev, __rte_unused int wait_to_complet
> 
> 	memset(&link, 0, sizeof(link));
> 	link.link_duplex = ETH_LINK_FULL_DUPLEX;
>-	link.link_speed  = ETH_SPEED_NUM_10G;
>+	link.link_speed  = hw->speed;
> 	link.link_autoneg = ETH_LINK_FIXED;
> 
> 	if (!hw->started) {
>@@ -2441,8 +2507,7 @@ virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
> {
> 	uint64_t tso_mask, host_features;
> 	struct virtio_hw *hw = dev->data->dev_private;
>-
>-	dev_info->speed_capa = ETH_LINK_SPEED_10G; /* fake value */
>+	dev_info->speed_capa = virtio_dev_speed_capa_get(hw->speed);
> 
> 	dev_info->max_rx_queues =
> 		RTE_MIN(hw->max_queue_pairs, VIRTIO_MAX_RX_QUEUES);
>diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
>index a38cb45ad..59012d55a 100644
>--- a/drivers/net/virtio/virtio_pci.h
>+++ b/drivers/net/virtio/virtio_pci.h
>@@ -253,6 +253,7 @@ struct virtio_hw {
> 	uint16_t    port_id;
> 	uint8_t     mac_addr[RTE_ETHER_ADDR_LEN];
> 	uint32_t    notify_off_multiplier;
>+	uint32_t    speed;  /* link speed in MB */
> 	uint8_t     *isr;
> 	uint16_t    *notify_base;
> 	struct virtio_pci_common_cfg *common_cfg;
>-- 
>2.17.1
>

[dpdk-dev] [PATCH v5 0/4] net/virtio: add link speed devarg

[Ivan Dyukov]

[PATCH v5 1/4] net/virtio: refactor devargs parsing
[PATCH v5 2/4] net/virtio: add link speed devarg
[PATCH v5 3/4] net/virtio-user: fix devargs parsing
[PATCH v5 4/4] net/virtio-user: adding link speed devarg
---
v5 changes:
* fixed code style
* fixed commit message and logging text

v4 changes:
* link_speed renamed to speed devarg
* speed devarg is added to virtio-user driver

v3 changes:
* link_speed devarg is added to virtio documentation

[dpdk-dev] [PATCH v5 1/4] net/virtio: refactor devargs parsing

[Ivan Dyukov]

refactor vdpa specific devargs parsing to more generic way

Signed-off-by: Ivan Dyukov <i.dyukov@samsung.com>
---
 drivers/net/virtio/virtio_ethdev.c | 34 +++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 10 deletions(-)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 044eb10a7..d2c7f8c61 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1955,16 +1955,18 @@ eth_virtio_dev_uninit(struct rte_eth_dev *eth_dev)
 }
 
 static int vdpa_check_handler(__rte_unused const char *key,
-		const char *value, __rte_unused void *opaque)
+		const char *value, void *ret_val)
 {
-	if (strcmp(value, "1"))
-		return -1;
+	if (strcmp(value, "1") == 0)
+		*(int *)ret_val = 1;
+	else
+		*(int *)ret_val = 0;
 
 	return 0;
 }
 
 static int
-vdpa_mode_selected(struct rte_devargs *devargs)
+virtio_dev_devargs_parse(struct rte_devargs *devargs, int *vdpa)
 {
 	struct rte_kvargs *kvlist;
 	const char *key = "vdpa";
@@ -1980,12 +1982,16 @@ vdpa_mode_selected(struct rte_devargs *devargs)
 	if (!rte_kvargs_count(kvlist, key))
 		goto exit;
 
-	/* vdpa mode selected when there's a key-value pair: vdpa=1 */
-	if (rte_kvargs_process(kvlist, key,
-				vdpa_check_handler, NULL) < 0) {
-		goto exit;
+	if (vdpa) {
+		/* vdpa mode selected when there's a key-value pair:
+		 * vdpa=1
+		 */
+		ret = rte_kvargs_process(kvlist, key,
+				vdpa_check_handler, vdpa);
+		if (ret < 0)
+			goto exit;
 	}
-	ret = 1;
+
 
 exit:
 	rte_kvargs_free(kvlist);
@@ -1995,8 +2001,16 @@ vdpa_mode_selected(struct rte_devargs *devargs)
 static int eth_virtio_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 	struct rte_pci_device *pci_dev)
 {
+	int vdpa = 0;
+	int ret = 0;
+
+	ret = virtio_dev_devargs_parse(pci_dev->device.devargs, &vdpa);
+	if (ret < 0) {
+		PMD_INIG_LOG(ERR, "devargs parsing is failed");
+		return ret;
+	}
 	/* virtio pmd skips probe if device needs to work in vdpa mode */
-	if (vdpa_mode_selected(pci_dev->device.devargs))
+	if (vdpa == 1)
 		return 1;
 
 	return rte_eth_dev_pci_generic_probe(pci_dev, sizeof(struct virtio_hw),
-- 
2.17.1

Re: [dpdk-dev] [PATCH v5 1/4] net/virtio: refactor devargs parsing

[David Marchand]

On Thu, Feb 27, 2020 at 3:20 PM Ivan Dyukov <i.dyukov@samsung.com> wrote:
>
> refactor vdpa specific devargs parsing to more generic way
>
> Signed-off-by: Ivan Dyukov <i.dyukov@samsung.com>
> ---
>  drivers/net/virtio/virtio_ethdev.c | 34 +++++++++++++++++++++---------
>  1 file changed, 24 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
> index 044eb10a7..d2c7f8c61 100644
> --- a/drivers/net/virtio/virtio_ethdev.c
> +++ b/drivers/net/virtio/virtio_ethdev.c
[snip]
> @@ -1995,8 +2001,16 @@ vdpa_mode_selected(struct rte_devargs *devargs)
>  static int eth_virtio_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
>         struct rte_pci_device *pci_dev)
>  {
> +       int vdpa = 0;
> +       int ret = 0;
> +
> +       ret = virtio_dev_devargs_parse(pci_dev->device.devargs, &vdpa);
> +       if (ret < 0) {
> +               PMD_INIG_LOG(ERR, "devargs parsing is failed");

PMD_INIT_LOG

https://lab.dpdk.org/results/dashboard/patchsets/9772/
https://travis-ci.com/ovsrobot/dpdk/builds/150851427



-- 
David Marchand

[dpdk-dev] [PATCH v6 0/4] net/virtio: add link speed devarg

[Ivan Dyukov]

v5 changes:
* fixed code style
* fixed commit message and logging text

v4 changes:
* link_speed renamed to speed devarg
* speed devarg is added to virtio-user driver

v3 changes:
* link_speed devarg is added to virtio documentation

[dpdk-dev] [PATCH v6 1/4] net/virtio: refactor devargs parsing

[Ivan Dyukov]

refactor vdpa specific devargs parsing to more generic way

Signed-off-by: Ivan Dyukov <i.dyukov@samsung.com>
---
 drivers/net/virtio/virtio_ethdev.c | 34 +++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 10 deletions(-)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 044eb10a7..f814aa926 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1955,16 +1955,18 @@ eth_virtio_dev_uninit(struct rte_eth_dev *eth_dev)
 }
 
 static int vdpa_check_handler(__rte_unused const char *key,
-		const char *value, __rte_unused void *opaque)
+		const char *value, void *ret_val)
 {
-	if (strcmp(value, "1"))
-		return -1;
+	if (strcmp(value, "1") == 0)
+		*(int *)ret_val = 1;
+	else
+		*(int *)ret_val = 0;
 
 	return 0;
 }
 
 static int
-vdpa_mode_selected(struct rte_devargs *devargs)
+virtio_dev_devargs_parse(struct rte_devargs *devargs, int *vdpa)
 {
 	struct rte_kvargs *kvlist;
 	const char *key = "vdpa";
@@ -1980,12 +1982,16 @@ vdpa_mode_selected(struct rte_devargs *devargs)
 	if (!rte_kvargs_count(kvlist, key))
 		goto exit;
 
-	/* vdpa mode selected when there's a key-value pair: vdpa=1 */
-	if (rte_kvargs_process(kvlist, key,
-				vdpa_check_handler, NULL) < 0) {
-		goto exit;
+	if (vdpa) {
+		/* vdpa mode selected when there's a key-value pair:
+		 * vdpa=1
+		 */
+		ret = rte_kvargs_process(kvlist, key,
+				vdpa_check_handler, vdpa);
+		if (ret < 0)
+			goto exit;
 	}
-	ret = 1;
+
 
 exit:
 	rte_kvargs_free(kvlist);
@@ -1995,8 +2001,16 @@ vdpa_mode_selected(struct rte_devargs *devargs)
 static int eth_virtio_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 	struct rte_pci_device *pci_dev)
 {
+	int vdpa = 0;
+	int ret = 0;
+
+	ret = virtio_dev_devargs_parse(pci_dev->device.devargs, &vdpa);
+	if (ret < 0) {
+		PMD_INIT_LOG(ERR, "devargs parsing is failed");
+		return ret;
+	}
 	/* virtio pmd skips probe if device needs to work in vdpa mode */
-	if (vdpa_mode_selected(pci_dev->device.devargs))
+	if (vdpa == 1)
 		return 1;
 
 	return rte_eth_dev_pci_generic_probe(pci_dev, sizeof(struct virtio_hw),
-- 
2.17.1

[dpdk-dev] [PATCH v6 2/4] net/virtio: add link speed devarg

[Ivan Dyukov]

Some applications like pktgen use link speed to calculate
transmission rate. It limits outcome traffic to hardcoded 10G.

This patch adds speed devarg which allows to configure
link speed of virtio device.

Signed-off-by: Ivan Dyukov <i.dyukov@samsung.com>
---
 doc/guides/nics/virtio.rst         |  7 +++
 drivers/net/virtio/virtio_ethdev.c | 97 +++++++++++++++++++++++++-----
 drivers/net/virtio/virtio_pci.h    |  1 +
 3 files changed, 89 insertions(+), 16 deletions(-)

diff --git a/doc/guides/nics/virtio.rst b/doc/guides/nics/virtio.rst
index d1f5fb898..0341907ef 100644
--- a/doc/guides/nics/virtio.rst
+++ b/doc/guides/nics/virtio.rst
@@ -356,6 +356,13 @@ Below devargs are supported by the PCI virtio driver:
     a virtio device needs to work in vDPA mode.
     (Default: 0 (disabled))
 
+#.  ``speed``:
+
+    It is used to specify link speed of virtio device. Link speed is a part of
+    link status structure. It could be requested by application using
+    rte_eth_link_get_nowait function.
+    (Default: 10000 (10G))
+
 Below devargs are supported by the virtio-user vdev:
 
 #.  ``path``:
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index f814aa926..24fed9966 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -45,6 +45,10 @@ static int virtio_dev_promiscuous_enable(struct rte_eth_dev *dev);
 static int virtio_dev_promiscuous_disable(struct rte_eth_dev *dev);
 static int virtio_dev_allmulticast_enable(struct rte_eth_dev *dev);
 static int virtio_dev_allmulticast_disable(struct rte_eth_dev *dev);
+static uint32_t virtio_dev_speed_capa_get(uint32_t speed);
+static int virtio_dev_devargs_parse(struct rte_devargs *devargs,
+	int *vdpa,
+	uint32_t *speed);
 static int virtio_dev_info_get(struct rte_eth_dev *dev,
 				struct rte_eth_dev_info *dev_info);
 static int virtio_dev_link_update(struct rte_eth_dev *dev,
@@ -1861,6 +1865,7 @@ int
 eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
 {
 	struct virtio_hw *hw = eth_dev->data->dev_private;
+	uint32_t speed = ETH_SPEED_NUM_10G;
 	int ret;
 
 	if (sizeof(struct virtio_net_hdr_mrg_rxbuf) > RTE_PKTMBUF_HEADROOM) {
@@ -1886,7 +1891,11 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
 
 		return 0;
 	}
-
+	ret = virtio_dev_devargs_parse(eth_dev->device->devargs,
+		 NULL, &speed);
+	if (ret < 0)
+		return ret;
+	hw->speed = speed;
 	/*
 	 * Pass the information to the rte_eth_dev_close() that it should also
 	 * release the private port resources.
@@ -1954,6 +1963,7 @@ eth_virtio_dev_uninit(struct rte_eth_dev *eth_dev)
 	return 0;
 }
 
+
 static int vdpa_check_handler(__rte_unused const char *key,
 		const char *value, void *ret_val)
 {
@@ -1965,33 +1975,89 @@ static int vdpa_check_handler(__rte_unused const char *key,
 	return 0;
 }
 
+
+static uint32_t
+virtio_dev_speed_capa_get(uint32_t speed)
+{
+	switch (speed) {
+	case ETH_SPEED_NUM_10G:
+		return ETH_LINK_SPEED_10G;
+	case ETH_SPEED_NUM_20G:
+		return ETH_LINK_SPEED_20G;
+	case ETH_SPEED_NUM_25G:
+		return ETH_LINK_SPEED_25G;
+	case ETH_SPEED_NUM_40G:
+		return ETH_LINK_SPEED_40G;
+	case ETH_SPEED_NUM_50G:
+		return ETH_LINK_SPEED_50G;
+	case ETH_SPEED_NUM_56G:
+		return ETH_LINK_SPEED_56G;
+	case ETH_SPEED_NUM_100G:
+		return ETH_LINK_SPEED_100G;
+	default:
+		return 0;
+	}
+}
+
+
+#define VIRTIO_ARG_SPEED      "speed"
+#define VIRTIO_ARG_VDPA       "vdpa"
+
+
+static int
+link_speed_handler(const char *key __rte_unused,
+		const char *value, void *ret_val)
+{
+	uint32_t val;
+	if (!value || !ret_val)
+		return -EINVAL;
+	val = strtoul(value, NULL, 0);
+	/* validate input */
+	if (virtio_dev_speed_capa_get(val) == 0)
+		return -EINVAL;
+	*(uint32_t *)ret_val = val;
+
+	return 0;
+}
+
+
 static int
-virtio_dev_devargs_parse(struct rte_devargs *devargs, int *vdpa)
+virtio_dev_devargs_parse(struct rte_devargs *devargs, int *vdpa,
+	uint32_t *speed)
 {
 	struct rte_kvargs *kvlist;
-	const char *key = "vdpa";
 	int ret = 0;
 
 	if (devargs == NULL)
 		return 0;
 
 	kvlist = rte_kvargs_parse(devargs->args, NULL);
-	if (kvlist == NULL)
+	if (kvlist == NULL) {
+		PMD_INIT_LOG(ERR, "error when parsing param");
 		return 0;
-
-	if (!rte_kvargs_count(kvlist, key))
-		goto exit;
-
-	if (vdpa) {
+	}
+	if (vdpa && rte_kvargs_count(kvlist, VIRTIO_ARG_VDPA) == 1) {
 		/* vdpa mode selected when there's a key-value pair:
 		 * vdpa=1
 		 */
-		ret = rte_kvargs_process(kvlist, key,
+		ret = rte_kvargs_process(kvlist, VIRTIO_ARG_VDPA,
 				vdpa_check_handler, vdpa);
-		if (ret < 0)
+		if (ret < 0) {
+			PMD_INIT_LOG(ERR, "Failed to parse %s",
+				VIRTIO_ARG_VDPA);
 			goto exit;
+		}
+	}
+	if (speed && rte_kvargs_count(kvlist, VIRTIO_ARG_SPEED) == 1) {
+		ret = rte_kvargs_process(kvlist,
+					VIRTIO_ARG_SPEED,
+					link_speed_handler, speed);
+		if (ret < 0) {
+			PMD_INIT_LOG(ERR, "Failed to parse %s",
+					VIRTIO_ARG_SPEED);
+			goto exit;
+		}
 	}
-
 
 exit:
 	rte_kvargs_free(kvlist);
@@ -2004,7 +2070,7 @@ static int eth_virtio_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 	int vdpa = 0;
 	int ret = 0;
 
-	ret = virtio_dev_devargs_parse(pci_dev->device.devargs, &vdpa);
+	ret = virtio_dev_devargs_parse(pci_dev->device.devargs, &vdpa, NULL);
 	if (ret < 0) {
 		PMD_INIT_LOG(ERR, "devargs parsing is failed");
 		return ret;
@@ -2385,7 +2451,7 @@ virtio_dev_link_update(struct rte_eth_dev *dev, __rte_unused int wait_to_complet
 
 	memset(&link, 0, sizeof(link));
 	link.link_duplex = ETH_LINK_FULL_DUPLEX;
-	link.link_speed  = ETH_SPEED_NUM_10G;
+	link.link_speed  = hw->speed;
 	link.link_autoneg = ETH_LINK_FIXED;
 
 	if (!hw->started) {
@@ -2440,8 +2506,7 @@ virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 {
 	uint64_t tso_mask, host_features;
 	struct virtio_hw *hw = dev->data->dev_private;
-
-	dev_info->speed_capa = ETH_LINK_SPEED_10G; /* fake value */
+	dev_info->speed_capa = virtio_dev_speed_capa_get(hw->speed);
 
 	dev_info->max_rx_queues =
 		RTE_MIN(hw->max_queue_pairs, VIRTIO_MAX_RX_QUEUES);
diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
index a38cb45ad..59012d55a 100644
--- a/drivers/net/virtio/virtio_pci.h
+++ b/drivers/net/virtio/virtio_pci.h
@@ -253,6 +253,7 @@ struct virtio_hw {
 	uint16_t    port_id;
 	uint8_t     mac_addr[RTE_ETHER_ADDR_LEN];
 	uint32_t    notify_off_multiplier;
+	uint32_t    speed;  /* link speed in MB */
 	uint8_t     *isr;
 	uint16_t    *notify_base;
 	struct virtio_pci_common_cfg *common_cfg;
-- 
2.17.1

[dpdk-dev] [PATCH v6 3/4] net/virtio-user: fix devargs parsing

[Ivan Dyukov]

strtoull returns 0 if it fails to parse input string. It's ignored
in get_integer_arg.

This patch handles error cases for strtoull function.

Signed-off-by: Ivan Dyukov <i.dyukov@samsung.com>
---
 drivers/net/virtio/virtio_user_ethdev.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index 3fc172573..074527714 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -433,12 +433,17 @@ static int
 get_integer_arg(const char *key __rte_unused,
 		const char *value, void *extra_args)
 {
+	uint64_t integer = 0;
 	if (!value || !extra_args)
 		return -EINVAL;
-
-	*(uint64_t *)extra_args = strtoull(value, NULL, 0);
-
-	return 0;
+	errno = 0;
+	integer = strtoull(value, NULL, 0);
+	/* extra_args keeps default value, it should be replaced
+	 * only in case of successful parsing of the 'value' arg
+	 */
+	if (errno == 0)
+		*(uint64_t *)extra_args = integer;
+	return -errno;
 }
 
 static struct rte_eth_dev *
-- 
2.17.1

[dpdk-dev] [PATCH v6 4/4] net/virtio-user: adding link speed devarg

[Ivan Dyukov]

virtio driver already parses speed devarg. virtio-user should add
it to list of valid devargs and call eth_virtio_dev_init function
which init speed value.

eth_virtio_dev_init already is called from virtio_user_pmd_probe
function. The only change is required to enable speed devargs:
adding speed to list of valid devargs.

Signed-off-by: Ivan Dyukov <i.dyukov@samsung.com>
---
 doc/guides/nics/virtio.rst              | 8 ++++++++
 drivers/net/virtio/virtio_user_ethdev.c | 5 ++++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/doc/guides/nics/virtio.rst b/doc/guides/nics/virtio.rst
index 0341907ef..6286286db 100644
--- a/doc/guides/nics/virtio.rst
+++ b/doc/guides/nics/virtio.rst
@@ -410,6 +410,14 @@ Below devargs are supported by the virtio-user vdev:
     It is used to enable virtio device packed virtqueue feature.
     (Default: 0 (disabled))
 
+#.  ``speed``:
+
+    It is used to specify link speed of virtio device. Link speed is a part of
+    link status structure. It could be requested by application using
+    rte_eth_link_get_nowait function.
+    (Default: 10000 (10G))
+
+
 Virtio paths Selection and Usage
 --------------------------------
 
diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index 074527714..45c1541c5 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -406,6 +406,8 @@ static const char *valid_args[] = {
 	VIRTIO_USER_ARG_IN_ORDER,
 #define VIRTIO_USER_ARG_PACKED_VQ      "packed_vq"
 	VIRTIO_USER_ARG_PACKED_VQ,
+#define VIRTIO_USER_ARG_SPEED          "speed"
+	VIRTIO_USER_ARG_SPEED,
 	NULL
 };
 
@@ -738,4 +740,5 @@ RTE_PMD_REGISTER_PARAM_STRING(net_virtio_user,
 	"server=<0|1> "
 	"mrg_rxbuf=<0|1> "
 	"in_order=<0|1> "
-	"packed_vq=<0|1>");
+	"packed_vq=<0|1> "
+	"speed=<int>");
-- 
2.17.1

Re: [dpdk-dev] [PATCH v6 0/4] net/virtio: add link speed devarg

[Ye Xiaolong]

Minor nit, better to reply the new series to the first mail (cover letter or
first patch) of V1.

On 03/03, Ivan Dyukov wrote:
>v5 changes:
>* fixed code style
>* fixed commit message and logging text
>
>v4 changes:
>* link_speed renamed to speed devarg
>* speed devarg is added to virtio-user driver
>
>v3 changes:
>* link_speed devarg is added to virtio documentation
>

For Series:

Reviewed-by: Xiaolong Ye <xiaolong.ye@intel.com>

[dpdk-dev] [PATCH v5 2/4] net/virtio: add link speed devarg

[Ivan Dyukov]

Some applications like pktgen use link speed to calculate
transmission rate. It limits outcome traffic to hardcoded 10G.

This patch adds speed devarg which allows to configure
link speed of virtio device.

Signed-off-by: Ivan Dyukov <i.dyukov@samsung.com>
---
 doc/guides/nics/virtio.rst         |  7 +++
 drivers/net/virtio/virtio_ethdev.c | 97 +++++++++++++++++++++++++-----
 drivers/net/virtio/virtio_pci.h    |  1 +
 3 files changed, 89 insertions(+), 16 deletions(-)

diff --git a/doc/guides/nics/virtio.rst b/doc/guides/nics/virtio.rst
index d1f5fb898..0341907ef 100644
--- a/doc/guides/nics/virtio.rst
+++ b/doc/guides/nics/virtio.rst
@@ -356,6 +356,13 @@ Below devargs are supported by the PCI virtio driver:
     a virtio device needs to work in vDPA mode.
     (Default: 0 (disabled))
 
+#.  ``speed``:
+
+    It is used to specify link speed of virtio device. Link speed is a part of
+    link status structure. It could be requested by application using
+    rte_eth_link_get_nowait function.
+    (Default: 10000 (10G))
+
 Below devargs are supported by the virtio-user vdev:
 
 #.  ``path``:
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index d2c7f8c61..1fc1f1139 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -45,6 +45,10 @@ static int virtio_dev_promiscuous_enable(struct rte_eth_dev *dev);
 static int virtio_dev_promiscuous_disable(struct rte_eth_dev *dev);
 static int virtio_dev_allmulticast_enable(struct rte_eth_dev *dev);
 static int virtio_dev_allmulticast_disable(struct rte_eth_dev *dev);
+static uint32_t virtio_dev_speed_capa_get(uint32_t speed);
+static int virtio_dev_devargs_parse(struct rte_devargs *devargs,
+	int *vdpa,
+	uint32_t *speed);
 static int virtio_dev_info_get(struct rte_eth_dev *dev,
 				struct rte_eth_dev_info *dev_info);
 static int virtio_dev_link_update(struct rte_eth_dev *dev,
@@ -1861,6 +1865,7 @@ int
 eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
 {
 	struct virtio_hw *hw = eth_dev->data->dev_private;
+	uint32_t speed = ETH_SPEED_NUM_10G;
 	int ret;
 
 	if (sizeof(struct virtio_net_hdr_mrg_rxbuf) > RTE_PKTMBUF_HEADROOM) {
@@ -1886,7 +1891,11 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
 
 		return 0;
 	}
-
+	ret = virtio_dev_devargs_parse(eth_dev->device->devargs,
+		 NULL, &speed);
+	if (ret < 0)
+		return ret;
+	hw->speed = speed;
 	/*
 	 * Pass the information to the rte_eth_dev_close() that it should also
 	 * release the private port resources.
@@ -1954,6 +1963,7 @@ eth_virtio_dev_uninit(struct rte_eth_dev *eth_dev)
 	return 0;
 }
 
+
 static int vdpa_check_handler(__rte_unused const char *key,
 		const char *value, void *ret_val)
 {
@@ -1965,33 +1975,89 @@ static int vdpa_check_handler(__rte_unused const char *key,
 	return 0;
 }
 
+
+static uint32_t
+virtio_dev_speed_capa_get(uint32_t speed)
+{
+	switch (speed) {
+	case ETH_SPEED_NUM_10G:
+		return ETH_LINK_SPEED_10G;
+	case ETH_SPEED_NUM_20G:
+		return ETH_LINK_SPEED_20G;
+	case ETH_SPEED_NUM_25G:
+		return ETH_LINK_SPEED_25G;
+	case ETH_SPEED_NUM_40G:
+		return ETH_LINK_SPEED_40G;
+	case ETH_SPEED_NUM_50G:
+		return ETH_LINK_SPEED_50G;
+	case ETH_SPEED_NUM_56G:
+		return ETH_LINK_SPEED_56G;
+	case ETH_SPEED_NUM_100G:
+		return ETH_LINK_SPEED_100G;
+	default:
+		return 0;
+	}
+}
+
+
+#define VIRTIO_ARG_SPEED      "speed"
+#define VIRTIO_ARG_VDPA       "vdpa"
+
+
+static int
+link_speed_handler(const char *key __rte_unused,
+		const char *value, void *ret_val)
+{
+	uint32_t val;
+	if (!value || !ret_val)
+		return -EINVAL;
+	val = strtoul(value, NULL, 0);
+	/* validate input */
+	if (virtio_dev_speed_capa_get(val) == 0)
+		return -EINVAL;
+	*(uint32_t *)ret_val = val;
+
+	return 0;
+}
+
+
 static int
-virtio_dev_devargs_parse(struct rte_devargs *devargs, int *vdpa)
+virtio_dev_devargs_parse(struct rte_devargs *devargs, int *vdpa,
+	uint32_t *speed)
 {
 	struct rte_kvargs *kvlist;
-	const char *key = "vdpa";
 	int ret = 0;
 
 	if (devargs == NULL)
 		return 0;
 
 	kvlist = rte_kvargs_parse(devargs->args, NULL);
-	if (kvlist == NULL)
+	if (kvlist == NULL) {
+		PMD_INIT_LOG(ERR, "error when parsing param");
 		return 0;
-
-	if (!rte_kvargs_count(kvlist, key))
-		goto exit;
-
-	if (vdpa) {
+	}
+	if (vdpa && rte_kvargs_count(kvlist, VIRTIO_ARG_VDPA) == 1) {
 		/* vdpa mode selected when there's a key-value pair:
 		 * vdpa=1
 		 */
-		ret = rte_kvargs_process(kvlist, key,
+		ret = rte_kvargs_process(kvlist, VIRTIO_ARG_VDPA,
 				vdpa_check_handler, vdpa);
-		if (ret < 0)
+		if (ret < 0) {
+			PMD_INIT_LOG(ERR, "Failed to parse %s",
+				VIRTIO_ARG_VDPA);
 			goto exit;
+		}
+	}
+	if (speed && rte_kvargs_count(kvlist, VIRTIO_ARG_SPEED) == 1) {
+		ret = rte_kvargs_process(kvlist,
+					VIRTIO_ARG_SPEED,
+					link_speed_handler, speed);
+		if (ret < 0) {
+			PMD_INIT_LOG(ERR, "Failed to parse %s",
+					VIRTIO_ARG_SPEED);
+			goto exit;
+		}
 	}
-
 
 exit:
 	rte_kvargs_free(kvlist);
@@ -2004,7 +2070,7 @@ static int eth_virtio_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 	int vdpa = 0;
 	int ret = 0;
 
-	ret = virtio_dev_devargs_parse(pci_dev->device.devargs, &vdpa);
+	ret = virtio_dev_devargs_parse(pci_dev->device.devargs, &vdpa, NULL);
 	if (ret < 0) {
 		PMD_INIG_LOG(ERR, "devargs parsing is failed");
 		return ret;
@@ -2385,7 +2451,7 @@ virtio_dev_link_update(struct rte_eth_dev *dev, __rte_unused int wait_to_complet
 
 	memset(&link, 0, sizeof(link));
 	link.link_duplex = ETH_LINK_FULL_DUPLEX;
-	link.link_speed  = ETH_SPEED_NUM_10G;
+	link.link_speed  = hw->speed;
 	link.link_autoneg = ETH_LINK_FIXED;
 
 	if (!hw->started) {
@@ -2440,8 +2506,7 @@ virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 {
 	uint64_t tso_mask, host_features;
 	struct virtio_hw *hw = dev->data->dev_private;
-
-	dev_info->speed_capa = ETH_LINK_SPEED_10G; /* fake value */
+	dev_info->speed_capa = virtio_dev_speed_capa_get(hw->speed);
 
 	dev_info->max_rx_queues =
 		RTE_MIN(hw->max_queue_pairs, VIRTIO_MAX_RX_QUEUES);
diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
index a38cb45ad..59012d55a 100644
--- a/drivers/net/virtio/virtio_pci.h
+++ b/drivers/net/virtio/virtio_pci.h
@@ -253,6 +253,7 @@ struct virtio_hw {
 	uint16_t    port_id;
 	uint8_t     mac_addr[RTE_ETHER_ADDR_LEN];
 	uint32_t    notify_off_multiplier;
+	uint32_t    speed;  /* link speed in MB */
 	uint8_t     *isr;
 	uint16_t    *notify_base;
 	struct virtio_pci_common_cfg *common_cfg;
-- 
2.17.1

[dpdk-dev] [PATCH v5 3/4] net/virtio-user: fix devargs parsing

[Ivan Dyukov]

strtoull returns 0 if it fails to parse input string. It's ignored
in get_integer_arg.

This patch handles error cases for strtoull function.

Signed-off-by: Ivan Dyukov <i.dyukov@samsung.com>
---
 drivers/net/virtio/virtio_user_ethdev.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index 3fc172573..074527714 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -433,12 +433,17 @@ static int
 get_integer_arg(const char *key __rte_unused,
 		const char *value, void *extra_args)
 {
+	uint64_t integer = 0;
 	if (!value || !extra_args)
 		return -EINVAL;
-
-	*(uint64_t *)extra_args = strtoull(value, NULL, 0);
-
-	return 0;
+	errno = 0;
+	integer = strtoull(value, NULL, 0);
+	/* extra_args keeps default value, it should be replaced
+	 * only in case of successful parsing of the 'value' arg
+	 */
+	if (errno == 0)
+		*(uint64_t *)extra_args = integer;
+	return -errno;
 }
 
 static struct rte_eth_dev *
-- 
2.17.1

[dpdk-dev] [PATCH v5 4/4] net/virtio-user: adding link speed devarg

[Ivan Dyukov]

virtio driver already parses speed devarg. virtio-user should add
it to list of valid devargs and call eth_virtio_dev_init function
which init speed value.

eth_virtio_dev_init already is called from virtio_user_pmd_probe
function. The only change is required to enable speed devargs:
adding speed to list of valid devargs.

Signed-off-by: Ivan Dyukov <i.dyukov@samsung.com>
---
 doc/guides/nics/virtio.rst              | 8 ++++++++
 drivers/net/virtio/virtio_user_ethdev.c | 5 ++++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/doc/guides/nics/virtio.rst b/doc/guides/nics/virtio.rst
index 0341907ef..6286286db 100644
--- a/doc/guides/nics/virtio.rst
+++ b/doc/guides/nics/virtio.rst
@@ -410,6 +410,14 @@ Below devargs are supported by the virtio-user vdev:
     It is used to enable virtio device packed virtqueue feature.
     (Default: 0 (disabled))
 
+#.  ``speed``:
+
+    It is used to specify link speed of virtio device. Link speed is a part of
+    link status structure. It could be requested by application using
+    rte_eth_link_get_nowait function.
+    (Default: 10000 (10G))
+
+
 Virtio paths Selection and Usage
 --------------------------------
 
diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index 074527714..45c1541c5 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -406,6 +406,8 @@ static const char *valid_args[] = {
 	VIRTIO_USER_ARG_IN_ORDER,
 #define VIRTIO_USER_ARG_PACKED_VQ      "packed_vq"
 	VIRTIO_USER_ARG_PACKED_VQ,
+#define VIRTIO_USER_ARG_SPEED          "speed"
+	VIRTIO_USER_ARG_SPEED,
 	NULL
 };
 
@@ -738,4 +740,5 @@ RTE_PMD_REGISTER_PARAM_STRING(net_virtio_user,
 	"server=<0|1> "
 	"mrg_rxbuf=<0|1> "
 	"in_order=<0|1> "
-	"packed_vq=<0|1>");
+	"packed_vq=<0|1> "
+	"speed=<int>");
-- 
2.17.1

[dpdk-dev] [PATCH v4 3/4] net/virtio-user: fix devargs parsing

[Ivan Dyukov]

strtoull returns 0 if it fails to parse input string. It's ignored
in get_integer_arg.

This patch handles error cases for strtoull function.

Signed-off-by: Ivan Dyukov <i.dyukov@samsung.com>
---
 drivers/net/virtio/virtio_user_ethdev.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index 3fc172573..074527714 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -433,12 +433,17 @@ static int
 get_integer_arg(const char *key __rte_unused,
 		const char *value, void *extra_args)
 {
+	uint64_t integer = 0;
 	if (!value || !extra_args)
 		return -EINVAL;
-
-	*(uint64_t *)extra_args = strtoull(value, NULL, 0);
-
-	return 0;
+	errno = 0;
+	integer = strtoull(value, NULL, 0);
+	/* extra_args keeps default value, it should be replaced
+	 * only in case of successful parsing of the 'value' arg
+	 */
+	if (errno == 0)
+		*(uint64_t *)extra_args = integer;
+	return -errno;
 }
 
 static struct rte_eth_dev *
-- 
2.17.1

[dpdk-dev] [PATCH v4 4/4] net/virtio-user: adding link speed devarg

[Ivan Dyukov]

virtio driver already parses speed devarg. virtio-user should add
it to list of valid devargs and call eth_virtio_dev_init function
which init speed value.

eth_virtio_dev_init already is called from virtio_user_pmd_probe
function. The only change is required to enable speed devargs:
adding speed to list of valid devargs.

Signed-off-by: Ivan Dyukov <i.dyukov@samsung.com>
---
 doc/guides/nics/virtio.rst              | 8 ++++++++
 drivers/net/virtio/virtio_user_ethdev.c | 5 ++++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/doc/guides/nics/virtio.rst b/doc/guides/nics/virtio.rst
index 0341907ef..6286286db 100644
--- a/doc/guides/nics/virtio.rst
+++ b/doc/guides/nics/virtio.rst
@@ -410,6 +410,14 @@ Below devargs are supported by the virtio-user vdev:
     It is used to enable virtio device packed virtqueue feature.
     (Default: 0 (disabled))
 
+#.  ``speed``:
+
+    It is used to specify link speed of virtio device. Link speed is a part of
+    link status structure. It could be requested by application using
+    rte_eth_link_get_nowait function.
+    (Default: 10000 (10G))
+
+
 Virtio paths Selection and Usage
 --------------------------------
 
diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index 074527714..45c1541c5 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -406,6 +406,8 @@ static const char *valid_args[] = {
 	VIRTIO_USER_ARG_IN_ORDER,
 #define VIRTIO_USER_ARG_PACKED_VQ      "packed_vq"
 	VIRTIO_USER_ARG_PACKED_VQ,
+#define VIRTIO_USER_ARG_SPEED          "speed"
+	VIRTIO_USER_ARG_SPEED,
 	NULL
 };
 
@@ -738,4 +740,5 @@ RTE_PMD_REGISTER_PARAM_STRING(net_virtio_user,
 	"server=<0|1> "
 	"mrg_rxbuf=<0|1> "
 	"in_order=<0|1> "
-	"packed_vq=<0|1>");
+	"packed_vq=<0|1> "
+	"speed=<int>");
-- 
2.17.1

Re: [dpdk-dev] [PATCH v3 2/2] net/virtio: add link speed devarg

[Ivan Dyukov]

Hi Tiwei,


12.02.2020 13:35, Tiwei Bie пишет:
> On Fri, Feb 07, 2020 at 02:25:26PM +0300, Ivan Dyukov wrote:
>> Some applications like pktgen use link_speed to calculate
>> transmit rate. It limits outcome traffic to hardcoded 10G.
>>
>> This patch adds link_speed devarg which allows to configure
>> link_speed of virtio device.
>>
>> Signed-off-by: Ivan Dyukov <i.dyukov@samsung.com>
>> ---
>>   doc/guides/nics/virtio.rst         |   7 ++
>>   drivers/net/virtio/virtio_ethdev.c | 101 ++++++++++++++++++++++++-----
>>   drivers/net/virtio/virtio_pci.h    |   1 +
>>   3 files changed, 92 insertions(+), 17 deletions(-)
> Maybe I missed something.. Why not enable the VIRTIO_NET_F_SPEED_DUPLEX
> support directly? When that feature is supported and negotiated,
> we will ignore this devarg?

VIRTIO_NET_F_SPEED_DUPLEX keeps tunable values. In case of kernel driver, they could be changed with ethtool. If a device is mapped to poll mode driver, I think, we can tune it with
dpdk devarg.  If devarg is not specified then negotiate VIRTIO_NET_F_SPEED_DUPLEX. This is my current vision of the feature, but I have to investigate it.

> If we want this devarg, it looks better to support it in virtio-user
> as well (most code is shared between virtio-PMD and virtio-user).

> Thanks!
> Tiwei
>

Re: [dpdk-dev] [PATCH v3 1/2] net/virtio: refactor devargs parsing

[Maxime Coquelin]

On 2/7/20 12:25 PM, Ivan Dyukov wrote:
> refactor vdpa specific devargs parsing to more generic way
> 
> Signed-off-by: Ivan Dyukov <i.dyukov@samsung.com>
> ---
>  drivers/net/virtio/virtio_ethdev.c | 35 +++++++++++++++++++++---------
>  1 file changed, 25 insertions(+), 10 deletions(-)
> 

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime