From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 67796A04B3; Mon, 16 Dec 2019 07:06:42 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 6661E1C112; Mon, 16 Dec 2019 07:06:41 +0100 (CET) Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by dpdk.org (Postfix) with ESMTP id DEE5F1C0CE for ; Mon, 16 Dec 2019 07:06:39 +0100 (CET) X-Amp-Result: UNSCANNABLE X-Amp-File-Uploaded: False Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by fmsmga106.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 15 Dec 2019 22:06:38 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.69,320,1571727600"; d="scan'208";a="416332483" Received: from dpdk-virtio-tbie-2.sh.intel.com (HELO ___) ([10.67.104.74]) by fmsmga006.fm.intel.com with ESMTP; 15 Dec 2019 22:06:37 -0800 Date: Mon, 16 Dec 2019 14:06:56 +0800 From: Tiwei Bie To: Ivan Dyukov Cc: Maxime Coquelin , dev@dpdk.org Message-ID: <20191216060656.GA189589@___> References: <20191213144442.32048-1-i.dyukov@samsung.com> <46436ac3-5e45-cba2-45eb-82f5f7e24f71@redhat.com> <6afbfe48-52fd-c66f-9ac2-dfb89fb55832@samsung.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <6afbfe48-52fd-c66f-9ac2-dfb89fb55832@samsung.com> User-Agent: Mutt/1.9.4 (2018-02-28) Subject: Re: [dpdk-dev] [PATCH v2] net/virtio: add link speed tuning X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 > >> --- > >> 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 > > 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. >