[v2,2/2] net/virtio: add link speed devarg
Checks
Commit Message
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(-)
Comments
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;
>
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;
>>
>
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;
>>>
>>
>
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;
>>>>
>
@@ -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);
@@ -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;