DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH v2 1/2] net/virtio: refactor devargs parsing
       [not found] <CGME20200120170529eucas1p2d319a567923ee26ba0bd34a122127fac@eucas1p2.samsung.com>
@ 2020-01-20 17:05 ` Ivan Dyukov
       [not found]   ` <CGME20200120170531eucas1p218252a80fbf9252baf21921f8c72fe82@eucas1p2.samsung.com>
  0 siblings, 1 reply; 6+ messages in thread
From: Ivan Dyukov @ 2020-01-20 17:05 UTC (permalink / raw)
  To: dev, maxime.coquelin, tiwei.bie; +Cc: 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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [dpdk-dev] [PATCH v2 2/2] net/virtio: add link speed devarg
       [not found]   ` <CGME20200120170531eucas1p218252a80fbf9252baf21921f8c72fe82@eucas1p2.samsung.com>
@ 2020-01-20 17:05     ` Ivan Dyukov
  2020-01-29 10:10       ` Adrian Moreno
  0 siblings, 1 reply; 6+ messages in thread
From: Ivan Dyukov @ 2020-01-20 17:05 UTC (permalink / raw)
  To: dev, maxime.coquelin, tiwei.bie; +Cc: 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>
---
 drivers/net/virtio/virtio_ethdev.c | 101 ++++++++++++++++++++++++-----
 drivers/net/virtio/virtio_pci.h    |   1 +
 2 files changed, 85 insertions(+), 17 deletions(-)

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH v2 2/2] net/virtio: add link speed devarg
  2020-01-20 17:05     ` [dpdk-dev] [PATCH v2 2/2] net/virtio: add link speed devarg Ivan Dyukov
@ 2020-01-29 10:10       ` Adrian Moreno
  2020-02-06 14:22         ` Maxime Coquelin
  0 siblings, 1 reply; 6+ messages in thread
From: Adrian Moreno @ 2020-01-29 10:10 UTC (permalink / raw)
  To: dev

On 1/20/20 6:05 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>
> ---
>  drivers/net/virtio/virtio_ethdev.c | 101 ++++++++++++++++++++++++-----
>  drivers/net/virtio/virtio_pci.h    |   1 +
>  2 files changed, 85 insertions(+), 17 deletions(-)
> 

Hi Ivan,

IMHO, this new option deserves being documented in doc/guides/nics/virtio.rst.

Otherwise it looks good to me.

Thank you.
Adrian
> 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;
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH v2 2/2] net/virtio: add link speed devarg
  2020-01-29 10:10       ` Adrian Moreno
@ 2020-02-06 14:22         ` Maxime Coquelin
  2020-02-06 14:26           ` Adrian Moreno
  0 siblings, 1 reply; 6+ messages in thread
From: Maxime Coquelin @ 2020-02-06 14:22 UTC (permalink / raw)
  To: Adrian Moreno, dev, Ivan Dyukov

Adding back Ivan as you removed it from the To: list.
So he may not have seen your comment.

On 1/29/20 11:10 AM, Adrian Moreno wrote:
> On 1/20/20 6:05 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>
>> ---
>>  drivers/net/virtio/virtio_ethdev.c | 101 ++++++++++++++++++++++++-----
>>  drivers/net/virtio/virtio_pci.h    |   1 +
>>  2 files changed, 85 insertions(+), 17 deletions(-)
>>
> 
> Hi Ivan,
> 
> IMHO, this new option deserves being documented in doc/guides/nics/virtio.rst.
> 
> Otherwise it looks good to me.

I agree with Adrian here, the new option need to be documented.

Thanks,
Maxime

> Thank you.
> Adrian
>> 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;
>>
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH v2 2/2] net/virtio: add link speed devarg
  2020-02-06 14:22         ` Maxime Coquelin
@ 2020-02-06 14:26           ` Adrian Moreno
  2020-02-07 11:34             ` Ivan Dyukov
  0 siblings, 1 reply; 6+ messages in thread
From: Adrian Moreno @ 2020-02-06 14:26 UTC (permalink / raw)
  To: Maxime Coquelin, dev, Ivan Dyukov

On 2/6/20 3:22 PM, Maxime Coquelin wrote:
> Adding back Ivan as you removed it from the To: list.
> So he may not have seen your comment.
Oops, sorry about that.
Adrian
> 
> On 1/29/20 11:10 AM, Adrian Moreno wrote:
>> On 1/20/20 6:05 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>
>>> ---
>>>  drivers/net/virtio/virtio_ethdev.c | 101 ++++++++++++++++++++++++-----
>>>  drivers/net/virtio/virtio_pci.h    |   1 +
>>>  2 files changed, 85 insertions(+), 17 deletions(-)
>>>
>>
>> Hi Ivan,
>>
>> IMHO, this new option deserves being documented in doc/guides/nics/virtio.rst.
>>
>> Otherwise it looks good to me.
> 
> I agree with Adrian here, the new option need to be documented.
> 
> Thanks,
> Maxime
> 
>> Thank you.
>> Adrian
>>> 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;
>>>
>>
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH v2 2/2] net/virtio: add link speed devarg
  2020-02-06 14:26           ` Adrian Moreno
@ 2020-02-07 11:34             ` Ivan Dyukov
  0 siblings, 0 replies; 6+ messages in thread
From: Ivan Dyukov @ 2020-02-07 11:34 UTC (permalink / raw)
  To: Adrian Moreno, Maxime Coquelin, dev

Hi Maxime, Adrian,

Thank you for comments!  I have sent new patch with updated documentation.
This is not final version of the link speed. I still have plan to rework 
it and use link speed which is stored in qemu.

Best regards,
Ivan
06.02.2020 17:26, Adrian Moreno пишет:
> On 2/6/20 3:22 PM, Maxime Coquelin wrote:
>> Adding back Ivan as you removed it from the To: list.
>> So he may not have seen your comment.
I really missed it. Thanks.

> Oops, sorry about that.
> Adrian
>> On 1/29/20 11:10 AM, Adrian Moreno wrote:
>>> On 1/20/20 6:05 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>
>>>> ---
>>>>   drivers/net/virtio/virtio_ethdev.c | 101 ++++++++++++++++++++++++-----
>>>>   drivers/net/virtio/virtio_pci.h    |   1 +
>>>>   2 files changed, 85 insertions(+), 17 deletions(-)
>>>>
>>> Hi Ivan,
>>>
>>> IMHO, this new option deserves being documented in doc/guides/nics/virtio.rst.
>>>
>>> Otherwise it looks good to me.
>> I agree with Adrian here, the new option need to be documented.
>>
>> Thanks,
>> Maxime
>>
>>> Thank you.
>>> Adrian
>>>> 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;
>>>>
>


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-02-07 11:34 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20200120170529eucas1p2d319a567923ee26ba0bd34a122127fac@eucas1p2.samsung.com>
2020-01-20 17:05 ` [dpdk-dev] [PATCH v2 1/2] net/virtio: refactor devargs parsing Ivan Dyukov
     [not found]   ` <CGME20200120170531eucas1p218252a80fbf9252baf21921f8c72fe82@eucas1p2.samsung.com>
2020-01-20 17:05     ` [dpdk-dev] [PATCH v2 2/2] net/virtio: add link speed devarg Ivan Dyukov
2020-01-29 10:10       ` Adrian Moreno
2020-02-06 14:22         ` Maxime Coquelin
2020-02-06 14:26           ` Adrian Moreno
2020-02-07 11:34             ` Ivan Dyukov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).