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