* [dpdk-dev] [PATCH v2] net/virtio: add link speed tuning [not found] <CGME20191213144450eucas1p189850559438d6f9d5ead48b645e27a79@eucas1p1.samsung.com> @ 2019-12-13 14:44 ` Ivan Dyukov 2019-12-13 14:59 ` Maxime Coquelin 0 siblings, 1 reply; 9+ messages in thread From: Ivan Dyukov @ 2019-12-13 14:44 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 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 = -- 2.17.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH v2] net/virtio: add link speed tuning 2019-12-13 14:44 ` [dpdk-dev] [PATCH v2] net/virtio: add link speed tuning Ivan Dyukov @ 2019-12-13 14:59 ` Maxime Coquelin 2019-12-13 16:26 ` Stephen Hemminger 2019-12-13 17:39 ` Ivan Dyukov 0 siblings, 2 replies; 9+ messages in thread From: Maxime Coquelin @ 2019-12-13 14:59 UTC (permalink / raw) To: Ivan Dyukov, dev, tiwei.bie 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. 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 While any Virtio spec update introduce link speed support, wouldn't it be preferable to have this as a devarg? Thanks, Maxime ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH v2] net/virtio: add link speed tuning 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 1 sibling, 1 reply; 9+ messages in thread From: Stephen Hemminger @ 2019-12-13 16:26 UTC (permalink / raw) To: Maxime Coquelin; +Cc: Ivan Dyukov, dev, tiwei.bie On Fri, 13 Dec 2019 15:59:23 +0100 Maxime Coquelin <maxime.coquelin@redhat.com> wrote: > 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. > > 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 > > While any Virtio spec update introduce link speed support, wouldn't it > be preferable to have this as a devarg? > > Thanks, > Maxime > Why does link speed matter at all? If some code is looking at link speed to compute values like QoS then it will be broken based on how virtualized NIC's work (bytes are free). It all seems just like a workaround for broke legacy code or advertising. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH v2] net/virtio: add link speed tuning 2019-12-13 16:26 ` Stephen Hemminger @ 2019-12-13 16:31 ` Maxime Coquelin 0 siblings, 0 replies; 9+ messages in thread From: Maxime Coquelin @ 2019-12-13 16:31 UTC (permalink / raw) To: Stephen Hemminger; +Cc: Ivan Dyukov, dev, tiwei.bie On 12/13/19 5:26 PM, Stephen Hemminger wrote: > On Fri, 13 Dec 2019 15:59:23 +0100 > Maxime Coquelin <maxime.coquelin@redhat.com> wrote: > >> 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. >> >> 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 >> >> While any Virtio spec update introduce link speed support, wouldn't it >> be preferable to have this as a devarg? >> >> Thanks, >> Maxime >> > > Why does link speed matter at all? > If some code is looking at link speed to compute values like QoS then > it will be broken based on how virtualized NIC's work (bytes are free). > > It all seems just like a workaround for broke legacy code or > advertising. > In Ivan's case, I agree it should not matter, we could just default to the maximum. I was thinking about vDPA devices, where I think it could make sense to report the HW link speed. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH v2] net/virtio: add link speed tuning 2019-12-13 14:59 ` Maxime Coquelin 2019-12-13 16:26 ` Stephen Hemminger @ 2019-12-13 17:39 ` Ivan Dyukov 2019-12-16 6:06 ` Tiwei Bie 1 sibling, 1 reply; 9+ messages in thread From: Ivan Dyukov @ 2019-12-13 17:39 UTC (permalink / raw) To: Maxime Coquelin, dev, tiwei.bie 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. > 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. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH v2] net/virtio: add link speed tuning 2019-12-13 17:39 ` Ivan Dyukov @ 2019-12-16 6:06 ` Tiwei Bie 2019-12-16 9:44 ` Maxime Coquelin 0 siblings, 1 reply; 9+ messages in thread From: Tiwei Bie @ 2019-12-16 6:06 UTC (permalink / raw) To: Ivan Dyukov; +Cc: Maxime Coquelin, 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 <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 > > 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. > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH v2] net/virtio: add link speed tuning 2019-12-16 6:06 ` Tiwei Bie @ 2019-12-16 9:44 ` Maxime Coquelin 2020-01-14 9:47 ` Maxime Coquelin 0 siblings, 1 reply; 9+ messages in thread From: Maxime Coquelin @ 2019-12-16 9:44 UTC (permalink / raw) To: Tiwei Bie, Ivan Dyukov; +Cc: dev 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. >> > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH v2] net/virtio: add link speed tuning 2019-12-16 9:44 ` Maxime Coquelin @ 2020-01-14 9:47 ` Maxime Coquelin 2020-01-30 7:13 ` Ivan Dyukov 0 siblings, 1 reply; 9+ messages in thread From: Maxime Coquelin @ 2020-01-14 9:47 UTC (permalink / raw) To: Tiwei Bie, Ivan Dyukov; +Cc: dev Hi Ivan, On 12/16/19 10:44 AM, Maxime Coquelin wrote: > > > 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! The above spec patch is implemented in Qemu since v2.12: commit 9473939ed7addcaaeb8fde5c093918fb7fa0919c Author: Jason Baron <jbaron@akamai.com> Date: Wed Mar 7 22:25:41 2018 -0500 virtio-net: add linkspeed and duplex settings to virtio-net Although linkspeed and duplex can be set in a linux guest via 'ethtool -s', this requires custom ethtool commands for virtio-net by default. Introduce a new feature flag, VIRTIO_NET_F_SPEED_DUPLEX, which allows the hypervisor to export a linkspeed and duplex setting. The user can subsequently overwrite it later if desired via: 'ethtool -s'. Linkspeed and duplex settings can be set as: '-device virtio-net,speed=10000,duplex=full' where speed is [0...INT_MAX], and duplex is ["half"|"full"]. Signed-off-by: Jason Baron <jbaron@akamai.com> Cc: "Michael S. Tsirkin" <mst@redhat.com> Cc: Jason Wang <jasowang@redhat.com> Cc: virtio-dev@lists.oasis-open.org Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> I think the right way would be to implement it directly in Virtio-net PMD. Are you willing to do it? Thanks, Maxime > >> >>>> 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. >>> >> > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH v2] net/virtio: add link speed tuning 2020-01-14 9:47 ` Maxime Coquelin @ 2020-01-30 7:13 ` Ivan Dyukov 0 siblings, 0 replies; 9+ messages in thread From: Ivan Dyukov @ 2020-01-30 7:13 UTC (permalink / raw) To: Maxime Coquelin, Tiwei Bie; +Cc: dev Hi Maxime, 14.01.2020 12:47, Maxime Coquelin пишет: > Hi Ivan, > > On 12/16/19 10:44 AM, Maxime Coquelin wrote: >> >> 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://protect2.fireeye.com/url?k=6b9e2079-365021cd-6b9fab36-000babdfecba-2c7f1ed6043989c5&u=https://lists.oasis-open.org/archives/virtio-comment/201911/msg00058.html >> Great, thanks Tiwei for the pointer! > The above spec patch is implemented in Qemu since v2.12: > > commit 9473939ed7addcaaeb8fde5c093918fb7fa0919c > Author: Jason Baron <jbaron@akamai.com> > Date: Wed Mar 7 22:25:41 2018 -0500 > > virtio-net: add linkspeed and duplex settings to virtio-net > > Although linkspeed and duplex can be set in a linux guest via > 'ethtool -s', > this requires custom ethtool commands for virtio-net by default. > > Introduce a new feature flag, VIRTIO_NET_F_SPEED_DUPLEX, which allows > the hypervisor to export a linkspeed and duplex setting. The user can > subsequently overwrite it later if desired via: 'ethtool -s'. > > Linkspeed and duplex settings can be set as: > '-device virtio-net,speed=10000,duplex=full' > > where speed is [0...INT_MAX], and duplex is ["half"|"full"]. > > Signed-off-by: Jason Baron <jbaron@akamai.com> > Cc: "Michael S. Tsirkin" <mst@redhat.com> > Cc: Jason Wang <jasowang@redhat.com> > Cc: virtio-dev@lists.oasis-open.org > Reviewed-by: Michael S. Tsirkin <mst@redhat.com> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > I think the right way would be to implement it directly in Virtio-net > PMD. > > Are you willing to do it? Sorry for delay, I missed this email. I need to reconfigure my emails filters. Ok I'll take care about it, but I can't promise any dates for it. May be this winter or spring. Best regards, Ivan. > > Thanks, > Maxime > >>>>> 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. >>>> > ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-01-30 7:13 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CGME20191213144450eucas1p189850559438d6f9d5ead48b645e27a79@eucas1p1.samsung.com>
2019-12-13 14:44 ` [dpdk-dev] [PATCH v2] net/virtio: add link speed tuning 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
2020-01-14 9:47 ` Maxime Coquelin
2020-01-30 7:13 ` 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).