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.dyu...@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;
>>>
>>
>
