On 9/23/21 09:35, Xia, Chenbo wrote:
Hi Maxime,
-----Original Message-----
From: Maxime Coquelin <maxime.coque...@redhat.com>
Sent: Wednesday, September 22, 2021 5:58 PM
To: dev@dpdk.org; Xia, Chenbo <chenbo....@intel.com>; amore...@redhat.com;
david.march...@redhat.com; andrew.rybche...@oktetlabs.ru; Yigit, Ferruh
<ferruh.yi...@intel.com>; michae...@nvidia.com; viachesl...@nvidia.com; Li,
Xiaoyun <xiaoyun...@intel.com>
Cc: sta...@dpdk.org; nelio.laranje...@6wind.com; yvuge...@redhat.com;
ybend...@redhat.com; Maxime Coquelin <maxime.coque...@redhat.com>
Subject: [PATCH v2 1/5] net/virtio: add initial RSS support
Provide the capability to update the hash key, hash types
and RETA table on the fly (without needing to stop/start
the device). However, the key length and the number of RETA
entries are fixed to 40B and 128 entries respectively. This
is done in order to simplify the design, but may be
revisited later as the Virtio spec provides this
flexibility.
Note that only VIRTIO_NET_F_RSS support is implemented,
VIRTIO_NET_F_HASH_REPORT, which would enable reporting the
packet RSS hash calculated by the device into mbuf.rss, is
not yet supported.
Regarding the default RSS configuration, it has been
chosen to use the default Intel ixgbe key as default key,
and default RETA is a simple modulo between the hash and
the number of Rx queues.
Signed-off-by: Maxime Coquelin <maxime.coque...@redhat.com>
---
doc/guides/nics/features/virtio.ini | 3 +
doc/guides/nics/virtio.rst | 3 +
doc/guides/rel_notes/release_21_11.rst | 6 +
drivers/net/virtio/virtio.h | 31 ++-
drivers/net/virtio/virtio_ethdev.c | 367 ++++++++++++++++++++++++-
drivers/net/virtio/virtio_ethdev.h | 3 +-
drivers/net/virtio/virtqueue.h | 21 ++
7 files changed, 428 insertions(+), 6 deletions(-)
diff --git a/doc/guides/nics/features/virtio.ini
b/doc/guides/nics/features/virtio.ini
index 48f6f393b1..a5eab4932f 100644
--- a/doc/guides/nics/features/virtio.ini
+++ b/doc/guides/nics/features/virtio.ini
@@ -14,6 +14,9 @@ Promiscuous mode = Y
Allmulticast mode = Y
Unicast MAC filter = Y
Multicast MAC filter = Y
+RSS hash = P
+RSS key update = Y
+RSS reta update = Y
VLAN filter = Y
Basic stats = Y
Stats per queue = Y
diff --git a/doc/guides/nics/virtio.rst b/doc/guides/nics/virtio.rst
index 82ce7399ce..98e0d012b7 100644
--- a/doc/guides/nics/virtio.rst
+++ b/doc/guides/nics/virtio.rst
@@ -73,6 +73,9 @@ In this release, the virtio PMD driver provides the basic
functionality of packe
* Virtio supports using port IO to get PCI resource when UIO module is not
available.
+* Virtio supports RSS Rx mode with 40B configurable hash key length, 128
+ configurable RETA entries and configurable hash types.
+
Prerequisites
-------------
diff --git a/doc/guides/rel_notes/release_21_11.rst
b/doc/guides/rel_notes/release_21_11.rst
index f5d16993db..2f9d81926b 100644
--- a/doc/guides/rel_notes/release_21_11.rst
+++ b/doc/guides/rel_notes/release_21_11.rst
@@ -96,6 +96,12 @@ New Features
Added command-line options to specify total number of processes and
current process ID. Each process owns subset of Rx and Tx queues.
+* **Added initial RSS support to Virtio PMD.**
+
+ Initial support for RSS receive mode has been added to the Virtio PMD,
+ with the capability for the application to configure the hash key, the
+ RETA and the hash types. Virtio hash reporting is yet to be added.
+
Removed Items
-------------
diff --git a/drivers/net/virtio/virtio.h b/drivers/net/virtio/virtio.h
index 525e2dad4c..b4f21dc0c7 100644
--- a/drivers/net/virtio/virtio.h
+++ b/drivers/net/virtio/virtio.h
@@ -30,6 +30,7 @@
#define VIRTIO_NET_F_GUEST_ANNOUNCE 21 /* Guest can announce device on
the
network */
#define VIRTIO_NET_F_MQ 22 /* Device supports Receive Flow
Steering */
#define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */
+#define VIRTIO_NET_F_RSS 60 /* RSS supported */
/*
* Do we get callbacks when the ring is completely used,
@@ -100,6 +101,29 @@
*/
#define VIRTIO_MAX_INDIRECT ((int)(rte_mem_page_size() / 16))
+/* Virtio RSS hash types */
+#define VIRTIO_NET_HASH_TYPE_IPV4 (1 << 0)
+#define VIRTIO_NET_HASH_TYPE_TCPV4 (1 << 1)
+#define VIRTIO_NET_HASH_TYPE_UDPV4 (1 << 2)
+#define VIRTIO_NET_HASH_TYPE_IPV6 (1 << 3)
+#define VIRTIO_NET_HASH_TYPE_TCPV6 (1 << 4)
+#define VIRTIO_NET_HASH_TYPE_UDPV6 (1 << 5)
Spec uses 'v' instead of 'V' for macro definition.
Better to align it?
I actually put it in uper case on purpose. I don't have a strong opinion
on this, but prefer to keep defines in upper case.
The spec lacks consistency on this, as VIRTIO_NET_HDR_GSO_TCPV6 is all
upper case for example.
+#define VIRTIO_NET_HASH_TYPE_IP_EX (1 << 6)
+#define VIRTIO_NET_HASH_TYPE_TCP_EX (1 << 7)
+#define VIRTIO_NET_HASH_TYPE_UDP_EX (1 << 8)
+
+#define VIRTIO_NET_HASH_TYPE_MASK ( \
+ VIRTIO_NET_HASH_TYPE_IPV4 | \
+ VIRTIO_NET_HASH_TYPE_TCPV4 | \
+ VIRTIO_NET_HASH_TYPE_UDPV4 | \
+ VIRTIO_NET_HASH_TYPE_IPV6 | \
+ VIRTIO_NET_HASH_TYPE_TCPV6 | \
+ VIRTIO_NET_HASH_TYPE_UDPV6 | \
+ VIRTIO_NET_HASH_TYPE_IP_EX | \
+ VIRTIO_NET_HASH_TYPE_TCP_EX | \
+ VIRTIO_NET_HASH_TYPE_UDP_EX)
+
+
/*
* Maximum number of virtqueues per device.
*/
@@ -157,7 +181,9 @@ struct virtio_net_config {
* Any other value stands for unknown.
*/
uint8_t duplex;
-
+ uint8_t rss_max_key_size;
+ uint16_t rss_max_indirection_table_length;
+ uint32_t supported_hash_types;
} __rte_packed;
struct virtio_hw {
@@ -190,6 +216,9 @@ struct virtio_hw {
rte_spinlock_t state_lock;
struct rte_mbuf **inject_pkts;
uint16_t max_queue_pairs;
+ uint32_t rss_hash_types;
+ uint16_t *rss_reta;
+ uint8_t *rss_key;
uint64_t req_guest_features;
struct virtnet_ctl *cvq;
};
diff --git a/drivers/net/virtio/virtio_ethdev.c
b/drivers/net/virtio/virtio_ethdev.c
index da1633d77e..10a9f708eb 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -51,6 +51,16 @@ static int virtio_dev_info_get(struct rte_eth_dev *dev,
static int virtio_dev_link_update(struct rte_eth_dev *dev,
int wait_to_complete);
static int virtio_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask);
+static int virtio_dev_rss_hash_update(struct rte_eth_dev *dev,
+ struct rte_eth_rss_conf *rss_conf);
+static int virtio_dev_rss_hash_conf_get(struct rte_eth_dev *dev,
+ struct rte_eth_rss_conf *rss_conf);
+static int virtio_dev_rss_reta_update(struct rte_eth_dev *dev,
+ struct rte_eth_rss_reta_entry64 *reta_conf,
+ uint16_t reta_size);
+static int virtio_dev_rss_reta_query(struct rte_eth_dev *dev,
+ struct rte_eth_rss_reta_entry64 *reta_conf,
+ uint16_t reta_size);
static void virtio_set_hwaddr(struct virtio_hw *hw);
static void virtio_get_hwaddr(struct virtio_hw *hw);
@@ -347,7 +357,38 @@ virtio_send_command(struct virtnet_ctl *cvq, struct
virtio_pmd_ctrl *ctrl,
}
static int
-virtio_set_multiple_queues(struct rte_eth_dev *dev, uint16_t nb_queues)
+virtio_set_multiple_queues_rss(struct rte_eth_dev *dev, uint16_t nb_queues)
+{
+ struct virtio_hw *hw = dev->data->dev_private;
+ struct virtio_pmd_ctrl ctrl;
+ struct virtio_net_ctrl_rss rss;
+ int dlen[1], ret;
Do we really need 'int dlen[1]' rather than 'int dlen'?
No we don't, did that for consistency with
virtio_set_multiple_queues_auto, but I'm fixing both occurences in new
revision.
+static int
+virtio_dev_rss_hash_conf_get(struct rte_eth_dev *dev,
+ struct rte_eth_rss_conf *rss_conf)
+{
+ struct virtio_hw *hw = dev->data->dev_private;
+
+ if (!virtio_with_feature(hw, VIRTIO_NET_F_RSS))
+ return -ENOTSUP;
+
+ if (!rss_conf)
+ return -EINVAL;
No need to check, it's done in ethdev.
OK, removing it here and elsewhere.
...
+/**
+ * RSS control
+ *
+ * The RSS feature <todo>
+ */
+#define VIRTIO_NET_RSS_RETA_SIZE 128
+#define VIRTIO_NET_RSS_KEY_SIZE 40
Better align the numbers?
Will do.
Thanks!
Maxime
Thanks,
Chenbo
