DPDK patches and discussions
 help / color / mirror / Atom feed
From: Maxime Coquelin <maxime.coquelin@redhat.com>
To: Tiwei Bie <tiwei.bie@intel.com>, Ivan Dyukov <i.dyukov@samsung.com>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v2] net/virtio: add link speed tuning
Date: Mon, 16 Dec 2019 10:44:07 +0100	[thread overview]
Message-ID: <8ab7ee73-73ed-4c2f-a226-3f9be587729b@redhat.com> (raw)
In-Reply-To: <20191216060656.GA189589@___>



On 12/16/19 7:06 AM, Tiwei Bie wrote:
> On Fri, Dec 13, 2019 at 08:39:11PM +0300, Ivan Dyukov wrote:
>> Hi Maxime,
>> Thank you for comments.
>>
>>
>> 13.12.2019 17:59, Maxime Coquelin пишет:
>>> Hi Ivan,
>>>
>>> On 12/13/19 3:44 PM, Ivan Dyukov wrote:
>>>> Some applications like pktgen use link_speed to calculate transmit
>>>> rate. It limits outcome traffic to hardcoded 10G.
>>>>
>>>> This patch makes link_speed configurable at compile time.
>>>>
>>>> Signed-off-by: Ivan Dyukov <i.dyukov@samsung.com>
>>>> ---
>>>>   config/common_base                 |  1 +
>>>>   config/meson.build                 |  1 +
>>>>   drivers/net/vhost/rte_eth_vhost.c  |  2 +-
>>>>   drivers/net/virtio/virtio_ethdev.c | 20 ++++++++++++++++----
>>>>   4 files changed, 19 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/config/common_base b/config/common_base
>>>> index 7dec7ed45..8589ca4ec 100644
>>>> --- a/config/common_base
>>>> +++ b/config/common_base
>>>> @@ -433,6 +433,7 @@ CONFIG_RTE_LIBRTE_AVP_DEBUG_BUFFERS=n
>>>>   # Compile burst-oriented VIRTIO PMD driver
>>>>   #
>>>>   CONFIG_RTE_LIBRTE_VIRTIO_PMD=y
>>>> +CONFIG_RTE_LIBRTE_VIRTIO_LINK_SPEED=10000
>>>>   CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_RX=n
>>>>   CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_TX=n
>>>>   CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_DUMP=n
>>>> diff --git a/config/meson.build b/config/meson.build
>>>> index 364a8d739..78c30f334 100644
>>>> --- a/config/meson.build
>>>> +++ b/config/meson.build
>>>> @@ -202,6 +202,7 @@ dpdk_conf.set('RTE_LIBEAL_USE_HPET', get_option('use_hpet'))
>>>>   dpdk_conf.set('RTE_MAX_VFIO_GROUPS', 64)
>>>>   dpdk_conf.set('RTE_DRIVER_MEMPOOL_BUCKET_SIZE_KB', 64)
>>>>   dpdk_conf.set('RTE_LIBRTE_DPAA2_USE_PHYS_IOVA', true)
>>>> +dpdk_conf.set('RTE_LIBRTE_VIRTIO_LINK_SPEED', 10000)
>>>>   
>>>>   
>>>>   compile_time_cpuflags = []
>>>> diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
>>>> index 46f01a7f4..38eaa5955 100644
>>>> --- a/drivers/net/vhost/rte_eth_vhost.c
>>>> +++ b/drivers/net/vhost/rte_eth_vhost.c
>>>> @@ -115,7 +115,7 @@ static struct internal_list_head internal_list =
>>>>   static pthread_mutex_t internal_list_lock = PTHREAD_MUTEX_INITIALIZER;
>>>>   
>>>>   static struct rte_eth_link pmd_link = {
>>>> -		.link_speed = 10000,
>>>> +		.link_speed = RTE_LIBRTE_VIRTIO_LINK_SPEED,
>>>>   		.link_duplex = ETH_LINK_FULL_DUPLEX,
>>>>   		.link_status = ETH_LINK_DOWN
>>>>   };
>>>> diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
>>>> index 044eb10a7..948091cc2 100644
>>>> --- a/drivers/net/virtio/virtio_ethdev.c
>>>> +++ b/drivers/net/virtio/virtio_ethdev.c
>>>> @@ -2371,7 +2371,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  = RTE_LIBRTE_VIRTIO_LINK_SPEED;
>>>>   	link.link_autoneg = ETH_LINK_FIXED;
>>>>   
>>>>   	if (!hw->started) {
>>>> @@ -2426,9 +2426,21 @@ 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 */
>>>> -
>>>> +#if RTE_LIBRTE_VIRTIO_LINK_SPEED == ETH_LINK_SPEED_20G
>>>> +	dev_info->speed_capa = ETH_LINK_SPEED_20G;
>>>> +#elif RTE_LIBRTE_VIRTIO_LINK_SPEED == ETH_SPEED_NUM_25G
>>>> +	dev_info->speed_capa = ETH_LINK_SPEED_25G;
>>>> +#elif RTE_LIBRTE_VIRTIO_LINK_SPEED == ETH_SPEED_NUM_40G
>>>> +	dev_info->speed_capa = ETH_LINK_SPEED_40G;
>>>> +#elif RTE_LIBRTE_VIRTIO_LINK_SPEED == ETH_SPEED_NUM_50G
>>>> +	dev_info->speed_capa = ETH_LINK_SPEED_50G;
>>>> +#elif RTE_LIBRTE_VIRTIO_LINK_SPEED == ETH_SPEED_NUM_56G
>>>> +	dev_info->speed_capa = ETH_LINK_SPEED_56G;
>>>> +#elif RTE_LIBRTE_VIRTIO_LINK_SPEED == ETH_SPEED_NUM_100G
>>>> +	dev_info->speed_capa = ETH_LINK_SPEED_100G;
>>>> +#else
>>>> +	dev_info->speed_capa = ETH_LINK_SPEED_10G;
>>>> +#endif
>>>>   	dev_info->max_rx_queues =
>>>>   		RTE_MIN(hw->max_queue_pairs, VIRTIO_MAX_RX_QUEUES);
>>>>   	dev_info->max_tx_queues =
>>>>
>>> I think we may need toi extend the Virtio specification so that the
>>> device can advertise the link speed.
>> I agree. It will be more flexible solution, but it will require another 
>> effort. I'll evalutate virtio spec and check if it's suitable for such 
>> changes.
> 
> FYI: https://lists.oasis-open.org/archives/virtio-comment/201911/msg00058.html

Great, thanks Tiwei for the pointer!

> 
>>> Problem with your proposal is that it is build time only, so:
>>> 1. It won't be configurable when using distros DPDK package.
>>> 2. All the Virtio devices on a system will have the same value
>> Current implementation is same. Nothing is broken here. :)
>>> While any Virtio spec update introduce link speed support, wouldn't it
>>> be preferable to have this as a devarg?
>> For my case, compile time configuration is ok. Let me prepare solution 
>> with devarg then we can choose the better one.
>>> Thanks,
>>> Maxime
>>
>> Best regards,
>>
>> Ivan.
>>
> 


  reply	other threads:[~2019-12-16  9:44 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20191213144450eucas1p189850559438d6f9d5ead48b645e27a79@eucas1p1.samsung.com>
2019-12-13 14:44 ` Ivan Dyukov
2019-12-13 14:59   ` Maxime Coquelin
2019-12-13 16:26     ` Stephen Hemminger
2019-12-13 16:31       ` Maxime Coquelin
2019-12-13 17:39     ` Ivan Dyukov
2019-12-16  6:06       ` Tiwei Bie
2019-12-16  9:44         ` Maxime Coquelin [this message]
2020-01-14  9:47           ` Maxime Coquelin
2020-01-30  7:13             ` Ivan Dyukov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8ab7ee73-73ed-4c2f-a226-3f9be587729b@redhat.com \
    --to=maxime.coquelin@redhat.com \
    --cc=dev@dpdk.org \
    --cc=i.dyukov@samsung.com \
    --cc=tiwei.bie@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).