Hi Ivan,
On 4/6/20 10:58 AM, Ivan Dyukov wrote:
> This patch adds a support of VIRTIO_NET_F_SPEED_DUPLEX feature
> for virtio driver. Set default speed to 0xffffffff and default
> duplex to 0xff
>
> There are few ways to specify speed of the link:
s/few/two/
> 'speed' devarg
> negotiate speed from qemu via VIRTIO_NET_F_SPEED_DUPLEX
Thanks for taking care of that, I appreciate it.
It should be used soon with vDPA.
> The highest priority is devarg. If devarg is not specified,
> drivers tries to negotiate it from qemu.
>
> Signed-off-by: Ivan Dyukov <i.dyu...@samsung.com>
> ---
> doc/guides/nics/virtio.rst | 4 ++--
> drivers/net/virtio/virtio_ethdev.c | 16 +++++++++++++++-
> drivers/net/virtio/virtio_ethdev.h | 3 ++-
> drivers/net/virtio/virtio_pci.h | 15 +++++++++++++++
> lib/librte_ethdev/rte_ethdev.h | 27 ++++++++++++++-------------
> 5 files changed, 48 insertions(+), 17 deletions(-)
>
> diff --git a/doc/guides/nics/virtio.rst b/doc/guides/nics/virtio.rst
> index e70b6653b..06c7a19aa 100644
> --- a/doc/guides/nics/virtio.rst
> +++ b/doc/guides/nics/virtio.rst
> @@ -361,7 +361,7 @@ Below devargs are supported by the PCI virtio driver:
> It is used to specify link speed of virtio device, in units of 1Mb.
> Link speed is a part of link status structure. It could be requested
> by application using rte_eth_link_get_nowait function.
> - (Default: 10000 (10G))
> + (Default: 0xffffffff (unknown))
I think the Default should be changed in the initial patch, not in the
last one.
>
> Below devargs are supported by the virtio-user vdev:
>
> @@ -415,7 +415,7 @@ Below devargs are supported by the virtio-user vdev:
> It is used to specify link speed of virtio device, in units of 1Mb.
> Link speed is a part of link status structure. It could be requested
> by application using rte_eth_link_get_nowait function.
> - (Default: 10000 (10G))
> + (Default: 0xffffffff (unknown))
>
>
> Virtio paths Selection and Usage
> diff --git a/drivers/net/virtio/virtio_ethdev.c
> b/drivers/net/virtio/virtio_ethdev.c
> index eb46a5a11..0137efcc5 100644
> --- a/drivers/net/virtio/virtio_ethdev.c
> +++ b/drivers/net/virtio/virtio_ethdev.c
> @@ -1718,6 +1718,20 @@ virtio_init_device(struct rte_eth_dev *eth_dev,
> uint64_t req_features)
> hw->mac_addr[0], hw->mac_addr[1], hw->mac_addr[2],
> hw->mac_addr[3], hw->mac_addr[4], hw->mac_addr[5]);
>
> + if (vtpci_with_feature(hw, VIRTIO_NET_F_SPEED_DUPLEX)) {
> + config = &local_config;
> + /* if speed is not specified in devargs */
> + if (hw->speed == ETH_SPEED_NUM_UNKNOWN) {
> + vtpci_read_dev_config(hw,
> + offsetof(struct virtio_net_config, speed),
> + &config->speed, sizeof(config->speed));
> + hw->speed = config->speed;
> + }
> + }
> +
> + PMD_INIT_LOG(DEBUG, "link speed = %u%s",
> + hw->speed, hw->speed == ETH_SPEED_NUM_UNKNOWN ?
> + "(UNKNOWN)" : "");
> if (vtpci_with_feature(hw, VIRTIO_NET_F_CTRL_VQ)) {
> config = &local_config;
>
> @@ -1865,7 +1879,7 @@ int
> eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
> {
> struct virtio_hw *hw = eth_dev->data->dev_private;
> - uint32_t speed = ETH_SPEED_NUM_10G;
> + uint32_t speed = ETH_SPEED_NUM_UNKNOWN;
> int ret;
>
> if (sizeof(struct virtio_net_hdr_mrg_rxbuf) > RTE_PKTMBUF_HEADROOM) {
> diff --git a/drivers/net/virtio/virtio_ethdev.h
> b/drivers/net/virtio/virtio_ethdev.h
> index cd8947656..febaf17a8 100644
> --- a/drivers/net/virtio/virtio_ethdev.h
> +++ b/drivers/net/virtio/virtio_ethdev.h
> @@ -37,7 +37,8 @@
> 1ULL << VIRTIO_F_RING_PACKED | \
> 1ULL << VIRTIO_F_IOMMU_PLATFORM | \
> 1ULL << VIRTIO_F_ORDER_PLATFORM | \
> - 1ULL << VIRTIO_F_NOTIFICATION_DATA)
> + 1ULL << VIRTIO_F_NOTIFICATION_DATA | \
> + 1ULL << VIRTIO_NET_F_SPEED_DUPLEX)
>
> #define VIRTIO_PMD_SUPPORTED_GUEST_FEATURES \
> (VIRTIO_PMD_DEFAULT_GUEST_FEATURES | \
> diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
> index ed98e11c3..2948760ab 100644
> --- a/drivers/net/virtio/virtio_pci.h
> +++ b/drivers/net/virtio/virtio_pci.h
> @@ -141,6 +141,9 @@ struct virtnet_ctl;
> */
> #define VIRTIO_F_NOTIFICATION_DATA 38
>
> +/* Device set linkspeed and duplex */
> +#define VIRTIO_NET_F_SPEED_DUPLEX 63
> +
> /* The Guest publishes the used index for which it expects an interrupt
> * at the end of the avail ring. Host should ignore the avail->flags field.
> */
> /* The Host publishes the avail index for which it expects a kick
> @@ -306,6 +309,18 @@ struct virtio_net_config {
> uint16_t status;
> uint16_t max_virtqueue_pairs;
> uint16_t mtu;
> + /*
> + * speed, in units of 1Mb. All values 0 to INT_MAX are legal.
> + * Any other value stands for unknown.
> + */
> + uint32_t speed;
> + /*
> + * 0x00 - half duplex
> + * 0x01 - full duplex
> + * Any other value stands for unknown.
> + */
> + uint8_t duplex;
> +
> } __attribute__((packed));
>
> /*
Below change should be in a dedicated patch, and I would prefer it to be
the first patch in the series.
> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
> index d1a593ad1..a15ea572e 100644
> --- a/lib/librte_ethdev/rte_ethdev.h
> +++ b/lib/librte_ethdev/rte_ethdev.h
> @@ -287,19 +287,20 @@ struct rte_eth_stats {
> /**
> * Ethernet numeric link speeds in Mbps
> */
> -#define ETH_SPEED_NUM_NONE 0 /**< Not defined */
> -#define ETH_SPEED_NUM_10M 10 /**< 10 Mbps */
> -#define ETH_SPEED_NUM_100M 100 /**< 100 Mbps */
> -#define ETH_SPEED_NUM_1G 1000 /**< 1 Gbps */
> -#define ETH_SPEED_NUM_2_5G 2500 /**< 2.5 Gbps */
> -#define ETH_SPEED_NUM_5G 5000 /**< 5 Gbps */
> -#define ETH_SPEED_NUM_10G 10000 /**< 10 Gbps */
> -#define ETH_SPEED_NUM_20G 20000 /**< 20 Gbps */
> -#define ETH_SPEED_NUM_25G 25000 /**< 25 Gbps */
> -#define ETH_SPEED_NUM_40G 40000 /**< 40 Gbps */
> -#define ETH_SPEED_NUM_50G 50000 /**< 50 Gbps */
> -#define ETH_SPEED_NUM_56G 56000 /**< 56 Gbps */
> -#define ETH_SPEED_NUM_100G 100000 /**< 100 Gbps */
> +#define ETH_SPEED_NUM_NONE 0 /**< Not defined */
> +#define ETH_SPEED_NUM_10M 10 /**< 10 Mbps */
> +#define ETH_SPEED_NUM_100M 100 /**< 100 Mbps */
> +#define ETH_SPEED_NUM_1G 1000 /**< 1 Gbps */
> +#define ETH_SPEED_NUM_2_5G 2500 /**< 2.5 Gbps */
> +#define ETH_SPEED_NUM_5G 5000 /**< 5 Gbps */
> +#define ETH_SPEED_NUM_10G 10000 /**< 10 Gbps */
> +#define ETH_SPEED_NUM_20G 20000 /**< 20 Gbps */
> +#define ETH_SPEED_NUM_25G 25000 /**< 25 Gbps */
> +#define ETH_SPEED_NUM_40G 40000 /**< 40 Gbps */
> +#define ETH_SPEED_NUM_50G 50000 /**< 50 Gbps */
> +#define ETH_SPEED_NUM_56G 56000 /**< 56 Gbps */
> +#define ETH_SPEED_NUM_100G 100000 /**< 100 Gbps */
> +#define ETH_SPEED_NUM_UNKNOWN 0xffffffff /**< Unknown */
>
> /**
> * A structure used to retrieve link-level information of an Ethernet port.
>
Thanks,
Maxime
