DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH v2 0/5] Virtio PMD RSS support & RSS fixes
@ 2021-09-22  9:57 Maxime Coquelin
  2021-09-22  9:57 ` [dpdk-dev] [PATCH v2 1/5] net/virtio: add initial RSS support Maxime Coquelin
                   ` (4 more replies)
  0 siblings, 5 replies; 16+ messages in thread
From: Maxime Coquelin @ 2021-09-22  9:57 UTC (permalink / raw)
  To: dev, chenbo.xia, amorenoz, david.marchand, andrew.rybchenko,
	ferruh.yigit, michaelba, viacheslavo, xiaoyun.li
  Cc: stable, nelio.laranjeiro, yvugenfi, ybendito, Maxime Coquelin

This series is mainly adding support for RSS to Virtio PMD
driver. The two last patches are fixing an issue in testpmd
that could cause out of bounds access, and fix
an issue spotted in the mlx5 driver while looking for
inspiration.

The first motivation for this series is to eventually
support RSS down to the Vhost-user library, so that OVS can
benefit from it. But it will be also useful with vDPA
devices in the future.

Regarding the testing, I have tested it with qemu v5.2 from
Fedora 34. Since libvirt does not support yet enabling RSS
feature in the Qemu virtio-net device, and this feature is
disabled by default, the tester can either rebuild the qemu
package to enable it by default or use the qemu cmdline to
do the same.

The tester can use testpmd in icmpecho mode in the guest
and scapy on the host to inject random traffic on the tap
interface, e.g.:
sendp(Ether(src=RandMAC()) / IP(src=RandIP(), dst='192.168.123.9') / UDP(sport=RandShort(), dport=RandShort()), loop=True, iface='vnet7')

Then it can play with RSS config in testpmd to change the
RETA, or hash type and see traffic being steered
accordingly by checking the Rx xstats.

Changes in v2:
==============
- Rework patch 2 to keep old behaviour, but fix possible out of bounds due to key length (Andrew/Nelio/Xiaoyun)
- s/reta/RETA/ (Andrew)
- Applied A-by on patch 3 (Slava)
- Fix display of configured hash types
- Add missing flow types definition to testpmd's port info command

Maxime Coquelin (3):
  net/virtio: add initial RSS support
  app/testpmd: fix RSS key length
  app/testpmd: fix RSS type display
  net/mlx5: fix RSS RETA update
  app/testpmd: add missing flow types in port info

 app/test-pmd/config.c                  |  10 +-
 doc/guides/nics/features/virtio.ini    |   3 +
 doc/guides/nics/virtio.rst             |   3 +
 doc/guides/rel_notes/release_21_11.rst |   6 +
 drivers/net/mlx5/mlx5_rss.c            |   2 +-
 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 ++
 9 files changed, 437 insertions(+), 9 deletions(-)

-- 
2.31.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [dpdk-dev] [PATCH v2 1/5] net/virtio: add initial RSS support
  2021-09-22  9:57 [dpdk-dev] [PATCH v2 0/5] Virtio PMD RSS support & RSS fixes Maxime Coquelin
@ 2021-09-22  9:57 ` Maxime Coquelin
  2021-09-22 11:20   ` Andrew Rybchenko
  2021-09-23  7:35   ` Xia, Chenbo
  2021-09-22  9:57 ` [dpdk-dev] [PATCH v2 2/5] app/testpmd: fix RSS key length Maxime Coquelin
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 16+ messages in thread
From: Maxime Coquelin @ 2021-09-22  9:57 UTC (permalink / raw)
  To: dev, chenbo.xia, amorenoz, david.marchand, andrew.rybchenko,
	ferruh.yigit, michaelba, viacheslavo, xiaoyun.li
  Cc: stable, nelio.laranjeiro, yvugenfi, ybendito, Maxime Coquelin

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.coquelin@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)
+#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;
+
+	rss.hash_types = hw->rss_hash_types & VIRTIO_NET_HASH_TYPE_MASK;
+	rss.indirection_table_mask = VIRTIO_NET_RSS_RETA_SIZE - 1;
+	rss.unclassified_queue = 0;
+	memcpy(rss.indirection_table, hw->rss_reta, VIRTIO_NET_RSS_RETA_SIZE * sizeof(uint16_t));
+	rss.max_tx_vq = nb_queues;
+	rss.hash_key_length = VIRTIO_NET_RSS_KEY_SIZE;
+	memcpy(rss.hash_key_data, hw->rss_key, VIRTIO_NET_RSS_KEY_SIZE);
+
+	ctrl.hdr.class = VIRTIO_NET_CTRL_MQ;
+	ctrl.hdr.cmd = VIRTIO_NET_CTRL_MQ_RSS_CONFIG;
+	memcpy(ctrl.data, &rss, sizeof(rss));
+
+	dlen[0] = sizeof(rss);
+
+	ret = virtio_send_command(hw->cvq, &ctrl, dlen, 1);
+	if (ret) {
+		PMD_INIT_LOG(ERR, "RSS multiqueue configured but send command failed");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int
+virtio_set_multiple_queues_auto(struct rte_eth_dev *dev, uint16_t nb_queues)
 {
 	struct virtio_hw *hw = dev->data->dev_private;
 	struct virtio_pmd_ctrl ctrl;
@@ -370,6 +411,17 @@ virtio_set_multiple_queues(struct rte_eth_dev *dev, uint16_t nb_queues)
 	return 0;
 }
 
+static int
+virtio_set_multiple_queues(struct rte_eth_dev *dev, uint16_t nb_queues)
+{
+	struct virtio_hw *hw = dev->data->dev_private;
+
+	if (virtio_with_feature(hw, VIRTIO_NET_F_RSS))
+		return virtio_set_multiple_queues_rss(dev, nb_queues);
+	else
+		return virtio_set_multiple_queues_auto(dev, nb_queues);
+}
+
 static void
 virtio_dev_queue_release(void *queue __rte_unused)
 {
@@ -701,6 +753,16 @@ virtio_alloc_queues(struct rte_eth_dev *dev)
 
 static void virtio_queues_unbind_intr(struct rte_eth_dev *dev);
 
+static void
+virtio_free_rss(struct virtio_hw *hw)
+{
+	rte_free(hw->rss_key);
+	hw->rss_key = NULL;
+
+	rte_free(hw->rss_reta);
+	hw->rss_reta = NULL;
+}
+
 int
 virtio_dev_close(struct rte_eth_dev *dev)
 {
@@ -731,6 +793,7 @@ virtio_dev_close(struct rte_eth_dev *dev)
 	virtio_reset(hw);
 	virtio_dev_free_mbufs(dev);
 	virtio_free_queues(hw);
+	virtio_free_rss(hw);
 
 	return VIRTIO_OPS(hw)->dev_close(hw);
 }
@@ -971,6 +1034,10 @@ static const struct eth_dev_ops virtio_eth_dev_ops = {
 	.rx_queue_release        = virtio_dev_queue_release,
 	.tx_queue_setup          = virtio_dev_tx_queue_setup,
 	.tx_queue_release        = virtio_dev_queue_release,
+	.rss_hash_update         = virtio_dev_rss_hash_update,
+	.rss_hash_conf_get       = virtio_dev_rss_hash_conf_get,
+	.reta_update             = virtio_dev_rss_reta_update,
+	.reta_query              = virtio_dev_rss_reta_query,
 	/* collect stats per queue */
 	.queue_stats_mapping_set = virtio_dev_queue_stats_mapping_set,
 	.vlan_filter_set         = virtio_vlan_filter_set,
@@ -1714,6 +1781,270 @@ virtio_configure_intr(struct rte_eth_dev *dev)
 
 	return 0;
 }
+
+static uint64_t
+virtio_to_ethdev_rss_offloads(uint64_t virtio_hash_types)
+{
+	uint64_t rss_offloads = 0;
+
+	if (virtio_hash_types & VIRTIO_NET_HASH_TYPE_IPV4)
+		rss_offloads |= ETH_RSS_IPV4 | ETH_RSS_FRAG_IPV4 | ETH_RSS_NONFRAG_IPV4_OTHER;
+
+	if (virtio_hash_types & VIRTIO_NET_HASH_TYPE_TCPV4)
+		rss_offloads |= ETH_RSS_NONFRAG_IPV4_TCP;
+
+	if (virtio_hash_types & VIRTIO_NET_HASH_TYPE_UDPV4)
+		rss_offloads |= ETH_RSS_NONFRAG_IPV4_UDP;
+
+	if (virtio_hash_types & VIRTIO_NET_HASH_TYPE_IPV6)
+		rss_offloads |= ETH_RSS_IPV6 | ETH_RSS_FRAG_IPV6 | ETH_RSS_NONFRAG_IPV6_OTHER;
+
+	if (virtio_hash_types & VIRTIO_NET_HASH_TYPE_TCPV6)
+		rss_offloads |= ETH_RSS_NONFRAG_IPV6_TCP;
+
+	if (virtio_hash_types & VIRTIO_NET_HASH_TYPE_UDPV6)
+		rss_offloads |= ETH_RSS_NONFRAG_IPV6_UDP;
+
+	if (virtio_hash_types & VIRTIO_NET_HASH_TYPE_IP_EX)
+		rss_offloads |= ETH_RSS_IPV6_EX;
+
+	if (virtio_hash_types & VIRTIO_NET_HASH_TYPE_TCP_EX)
+		rss_offloads |= ETH_RSS_IPV6_TCP_EX;
+
+	if (virtio_hash_types & VIRTIO_NET_HASH_TYPE_UDP_EX)
+		rss_offloads |= ETH_RSS_IPV6_UDP_EX;
+
+	return rss_offloads;
+}
+
+static int
+virtio_dev_get_rss_config(struct virtio_hw *hw, uint32_t *rss_hash_types)
+{
+	struct virtio_net_config local_config;
+	struct virtio_net_config *config = &local_config;
+
+	virtio_read_dev_config(hw,
+			offsetof(struct virtio_net_config, rss_max_key_size),
+			&config->rss_max_key_size,
+			sizeof(config->rss_max_key_size));
+	if (config->rss_max_key_size < VIRTIO_NET_RSS_KEY_SIZE) {
+		PMD_INIT_LOG(ERR, "Invalid device RSS max key size (%u)",
+				config->rss_max_key_size);
+		return -1;
+	}
+
+	virtio_read_dev_config(hw,
+			offsetof(struct virtio_net_config,
+				rss_max_indirection_table_length),
+			&config->rss_max_indirection_table_length,
+			sizeof(config->rss_max_indirection_table_length));
+	if (config->rss_max_indirection_table_length < VIRTIO_NET_RSS_RETA_SIZE) {
+		PMD_INIT_LOG(ERR, "Invalid device RSS max reta size (%u)",
+				config->rss_max_indirection_table_length);
+		return -1;
+	}
+
+	virtio_read_dev_config(hw,
+			offsetof(struct virtio_net_config, supported_hash_types),
+			&config->supported_hash_types,
+			sizeof(config->supported_hash_types));
+	if ((config->supported_hash_types & VIRTIO_NET_HASH_TYPE_MASK) == 0) {
+		PMD_INIT_LOG(ERR, "Invalid device RSS hash types (%u)",
+				config->supported_hash_types);
+		return -1;
+	}
+
+	*rss_hash_types = config->supported_hash_types & VIRTIO_NET_HASH_TYPE_MASK;
+
+	PMD_INIT_LOG(DEBUG, "Device RSS config:");
+	PMD_INIT_LOG(DEBUG, "\t-Max key size: %u", config->rss_max_key_size);
+	PMD_INIT_LOG(DEBUG, "\t-Max reta size: %u", config->rss_max_indirection_table_length);
+	PMD_INIT_LOG(DEBUG, "\t-Supported hash types: 0x%x", *rss_hash_types);
+
+	return 0;
+}
+
+static int
+virtio_dev_rss_hash_update(struct rte_eth_dev *dev,
+		struct rte_eth_rss_conf *rss_conf)
+{
+	struct virtio_hw *hw = dev->data->dev_private;
+	uint16_t nb_queues;
+
+	if (!virtio_with_feature(hw, VIRTIO_NET_F_RSS))
+		return -ENOTSUP;
+
+	if (rss_conf->rss_hf & ~virtio_to_ethdev_rss_offloads(VIRTIO_NET_HASH_TYPE_MASK))
+		return -EINVAL;
+
+	hw->rss_hash_types = 0;
+
+	if (rss_conf->rss_hf & (ETH_RSS_IPV4 | ETH_RSS_FRAG_IPV4 | ETH_RSS_NONFRAG_IPV4_OTHER))
+		hw->rss_hash_types |= VIRTIO_NET_HASH_TYPE_IPV4;
+
+	if (rss_conf->rss_hf & ETH_RSS_NONFRAG_IPV4_TCP)
+		hw->rss_hash_types |= VIRTIO_NET_HASH_TYPE_TCPV4;
+
+	if (rss_conf->rss_hf & ETH_RSS_NONFRAG_IPV4_UDP)
+		hw->rss_hash_types |= VIRTIO_NET_HASH_TYPE_UDPV4;
+
+	if (rss_conf->rss_hf & (ETH_RSS_IPV6 | ETH_RSS_FRAG_IPV6 | ETH_RSS_NONFRAG_IPV6_OTHER))
+		hw->rss_hash_types |= VIRTIO_NET_HASH_TYPE_IPV6;
+
+	if (rss_conf->rss_hf & ETH_RSS_NONFRAG_IPV6_TCP)
+		hw->rss_hash_types |= VIRTIO_NET_HASH_TYPE_TCPV6;
+
+	if (rss_conf->rss_hf & ETH_RSS_NONFRAG_IPV6_UDP)
+		hw->rss_hash_types |= VIRTIO_NET_HASH_TYPE_UDPV6;
+
+	if (rss_conf->rss_hf & ETH_RSS_IPV6_EX)
+		hw->rss_hash_types |= VIRTIO_NET_HASH_TYPE_IP_EX;
+
+	if (rss_conf->rss_hf & ETH_RSS_IPV6_TCP_EX)
+		hw->rss_hash_types |= VIRTIO_NET_HASH_TYPE_TCP_EX;
+
+	if (rss_conf->rss_hf & ETH_RSS_IPV6_UDP_EX)
+		hw->rss_hash_types |= VIRTIO_NET_HASH_TYPE_UDP_EX;
+
+	if (rss_conf->rss_key && rss_conf->rss_key_len) {
+		if (rss_conf->rss_key_len != VIRTIO_NET_RSS_KEY_SIZE) {
+			PMD_INIT_LOG(ERR, "Driver only supports %u RSS key length",
+					VIRTIO_NET_RSS_KEY_SIZE);
+			return -EINVAL;
+		}
+		memcpy(hw->rss_key, rss_conf->rss_key, VIRTIO_NET_RSS_KEY_SIZE);
+	}
+
+	nb_queues = RTE_MAX(dev->data->nb_rx_queues, dev->data->nb_tx_queues);
+	return virtio_set_multiple_queues_rss(dev, nb_queues);
+}
+
+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;
+
+	if (rss_conf->rss_key && rss_conf->rss_key_len >= VIRTIO_NET_RSS_KEY_SIZE)
+		memcpy(rss_conf->rss_key, hw->rss_key, VIRTIO_NET_RSS_KEY_SIZE);
+	rss_conf->rss_key_len = VIRTIO_NET_RSS_KEY_SIZE;
+	rss_conf->rss_hf = virtio_to_ethdev_rss_offloads(hw->rss_hash_types);
+
+	return 0;
+}
+
+static int virtio_dev_rss_reta_update(struct rte_eth_dev *dev,
+			 struct rte_eth_rss_reta_entry64 *reta_conf,
+			 uint16_t reta_size)
+{
+	struct virtio_hw *hw = dev->data->dev_private;
+	uint16_t nb_queues;
+	int idx, pos, i;
+
+	if (!virtio_with_feature(hw, VIRTIO_NET_F_RSS))
+		return -ENOTSUP;
+
+	if (!reta_conf)
+		return -EINVAL;
+
+	if (reta_size != VIRTIO_NET_RSS_RETA_SIZE)
+		return -EINVAL;
+
+	for (idx = 0, i = 0; i < reta_size; i++) {
+		idx = i / RTE_RETA_GROUP_SIZE;
+		pos = i % RTE_RETA_GROUP_SIZE;
+
+		if (((reta_conf[idx].mask >> pos) & 0x1) == 0)
+			continue;
+
+		hw->rss_reta[i] = reta_conf[idx].reta[pos];
+	}
+
+	nb_queues = RTE_MAX(dev->data->nb_rx_queues, dev->data->nb_tx_queues);
+	return virtio_set_multiple_queues_rss(dev, nb_queues);
+}
+
+static int virtio_dev_rss_reta_query(struct rte_eth_dev *dev,
+			 struct rte_eth_rss_reta_entry64 *reta_conf,
+			 uint16_t reta_size)
+{
+	struct virtio_hw *hw = dev->data->dev_private;
+	int idx, i;
+
+	if (!virtio_with_feature(hw, VIRTIO_NET_F_RSS))
+		return -ENOTSUP;
+
+	if (!reta_conf)
+		return -EINVAL;
+
+	if (reta_size != VIRTIO_NET_RSS_RETA_SIZE)
+		return -EINVAL;
+
+	for (idx = 0, i = 0; i < reta_size; i++) {
+		idx = i / RTE_RETA_GROUP_SIZE;
+		reta_conf[idx].reta[i % RTE_RETA_GROUP_SIZE] = hw->rss_reta[i];
+	}
+
+	return 0;
+}
+
+/*
+ * As default RSS hash key, it uses the default key of the
+ * Intel IXGBE devices. It can be updated by the application
+ * with any 40B key value.
+ */
+static uint8_t rss_intel_key[VIRTIO_NET_RSS_KEY_SIZE] = {
+	0x6D, 0x5A, 0x56, 0xDA, 0x25, 0x5B, 0x0E, 0xC2,
+	0x41, 0x67, 0x25, 0x3D, 0x43, 0xA3, 0x8F, 0xB0,
+	0xD0, 0xCA, 0x2B, 0xCB, 0xAE, 0x7B, 0x30, 0xB4,
+	0x77, 0xCB, 0x2D, 0xA3, 0x80, 0x30, 0xF2, 0x0C,
+	0x6A, 0x42, 0xB7, 0x3B, 0xBE, 0xAC, 0x01, 0xFA,
+};
+
+static int
+virtio_dev_rss_init(struct rte_eth_dev *eth_dev)
+{
+	struct virtio_hw *hw = eth_dev->data->dev_private;
+	uint16_t nb_rx_queues = eth_dev->data->nb_rx_queues;
+	int i;
+
+	if (virtio_dev_get_rss_config(hw, &hw->rss_hash_types))
+		return -1;
+
+	if (!hw->rss_key) {
+		/* Setup default RSS key if not already setup by the user */
+		hw->rss_key = rte_malloc_socket("rss_key",
+				VIRTIO_NET_RSS_KEY_SIZE, 0,
+				eth_dev->device->numa_node);
+		if (!hw->rss_key) {
+			PMD_INIT_LOG(ERR, "Failed to allocate RSS key");
+			return -1;
+		}
+		rte_memcpy(hw->rss_key, rss_intel_key, VIRTIO_NET_RSS_KEY_SIZE);
+	}
+
+	if (!hw->rss_reta) {
+		/* Setup default RSS reta if not already setup by the user */
+		hw->rss_reta = rte_malloc_socket("rss_reta",
+				VIRTIO_NET_RSS_RETA_SIZE * sizeof(uint16_t), 0,
+				eth_dev->device->numa_node);
+		if (!hw->rss_reta) {
+			PMD_INIT_LOG(ERR, "Failed to allocate RSS reta");
+			return -1;
+		}
+		for (i = 0; i < VIRTIO_NET_RSS_RETA_SIZE; i++)
+			hw->rss_reta[i] = i % nb_rx_queues;
+	}
+
+	return 0;
+}
+
 #define DUPLEX_UNKNOWN   0xff
 /* reset device and renegotiate features if needed */
 static int
@@ -1801,14 +2132,15 @@ virtio_init_device(struct rte_eth_dev *eth_dev, uint64_t req_features)
 			config->status = 0;
 		}
 
-		if (virtio_with_feature(hw, VIRTIO_NET_F_MQ)) {
+		if (virtio_with_feature(hw, VIRTIO_NET_F_MQ) ||
+				virtio_with_feature(hw, VIRTIO_NET_F_RSS)) {
 			virtio_read_dev_config(hw,
 				offsetof(struct virtio_net_config, max_virtqueue_pairs),
 				&config->max_virtqueue_pairs,
 				sizeof(config->max_virtqueue_pairs));
 		} else {
 			PMD_INIT_LOG(DEBUG,
-				     "VIRTIO_NET_F_MQ is not supported");
+				     "Neither VIRTIO_NET_F_MQ nor VIRTIO_NET_F_RSS are supported");
 			config->max_virtqueue_pairs = 1;
 		}
 
@@ -1840,6 +2172,11 @@ virtio_init_device(struct rte_eth_dev *eth_dev, uint64_t req_features)
 				VLAN_TAG_LEN - hw->vtnet_hdr_size;
 		}
 
+		hw->rss_hash_types = 0;
+		if (virtio_with_feature(hw, VIRTIO_NET_F_RSS))
+			if (virtio_dev_rss_init(eth_dev))
+				return -1;
+
 		PMD_INIT_LOG(DEBUG, "config->max_virtqueue_pairs=%d",
 				config->max_virtqueue_pairs);
 		PMD_INIT_LOG(DEBUG, "config->status=%d", config->status);
@@ -2083,7 +2420,7 @@ virtio_dev_configure(struct rte_eth_dev *dev)
 	PMD_INIT_LOG(DEBUG, "configure");
 	req_features = VIRTIO_PMD_DEFAULT_GUEST_FEATURES;
 
-	if (rxmode->mq_mode != ETH_MQ_RX_NONE) {
+	if (rxmode->mq_mode != ETH_MQ_RX_NONE && rxmode->mq_mode != ETH_MQ_RX_RSS) {
 		PMD_DRV_LOG(ERR,
 			"Unsupported Rx multi queue mode %d",
 			rxmode->mq_mode);
@@ -2103,6 +2440,9 @@ virtio_dev_configure(struct rte_eth_dev *dev)
 			return ret;
 	}
 
+	if (rxmode->mq_mode == ETH_MQ_RX_RSS)
+		req_features |= (1ULL << VIRTIO_NET_F_RSS);
+
 	if ((rx_offloads & DEV_RX_OFFLOAD_JUMBO_FRAME) &&
 	    (rxmode->max_rx_pkt_len > hw->max_mtu + ether_hdr_len))
 		req_features &= ~(1ULL << VIRTIO_NET_F_MTU);
@@ -2137,6 +2477,12 @@ virtio_dev_configure(struct rte_eth_dev *dev)
 			return ret;
 	}
 
+	if ((rxmode->mq_mode & ETH_MQ_RX_RSS_FLAG) &&
+			!virtio_with_feature(hw, VIRTIO_NET_F_RSS)) {
+		PMD_DRV_LOG(ERR, "RSS support requested but not supported by the device");
+		return -ENOTSUP;
+	}
+
 	if ((rx_offloads & (DEV_RX_OFFLOAD_UDP_CKSUM |
 			    DEV_RX_OFFLOAD_TCP_CKSUM)) &&
 		!virtio_with_feature(hw, VIRTIO_NET_F_GUEST_CSUM)) {
@@ -2503,6 +2849,7 @@ static int
 virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 {
 	uint64_t tso_mask, host_features;
+	uint32_t rss_hash_types = 0;
 	struct virtio_hw *hw = dev->data->dev_private;
 	dev_info->speed_capa = virtio_dev_speed_capa_get(hw->speed);
 
@@ -2543,6 +2890,18 @@ virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 		(1ULL << VIRTIO_NET_F_HOST_TSO6);
 	if ((host_features & tso_mask) == tso_mask)
 		dev_info->tx_offload_capa |= DEV_TX_OFFLOAD_TCP_TSO;
+	if (host_features & (1ULL << VIRTIO_NET_F_RSS)) {
+		virtio_dev_get_rss_config(hw, &rss_hash_types);
+		dev_info->hash_key_size = VIRTIO_NET_RSS_KEY_SIZE;
+		dev_info->reta_size = VIRTIO_NET_RSS_RETA_SIZE;
+		dev_info->flow_type_rss_offloads =
+			virtio_to_ethdev_rss_offloads(rss_hash_types);
+	} else {
+		dev_info->hash_key_size = 0;
+		dev_info->reta_size = 0;
+		dev_info->flow_type_rss_offloads = 0;
+	}
+
 
 	if (host_features & (1ULL << VIRTIO_F_RING_PACKED)) {
 		/*
diff --git a/drivers/net/virtio/virtio_ethdev.h b/drivers/net/virtio/virtio_ethdev.h
index 2f63ef2b2d..3fe30d8784 100644
--- a/drivers/net/virtio/virtio_ethdev.h
+++ b/drivers/net/virtio/virtio_ethdev.h
@@ -45,7 +45,8 @@
 	 1u << VIRTIO_NET_F_GUEST_TSO6     |	\
 	 1u << VIRTIO_NET_F_CSUM           |	\
 	 1u << VIRTIO_NET_F_HOST_TSO4      |	\
-	 1u << VIRTIO_NET_F_HOST_TSO6)
+	 1u << VIRTIO_NET_F_HOST_TSO6      |	\
+	 1ULL << VIRTIO_NET_F_RSS)
 
 extern const struct eth_dev_ops virtio_user_secondary_eth_dev_ops;
 
diff --git a/drivers/net/virtio/virtqueue.h b/drivers/net/virtio/virtqueue.h
index 03957b2bd0..99a0e68d38 100644
--- a/drivers/net/virtio/virtqueue.h
+++ b/drivers/net/virtio/virtqueue.h
@@ -182,6 +182,24 @@ struct virtio_net_ctrl_mac {
 #define VIRTIO_NET_CTRL_VLAN_ADD 0
 #define VIRTIO_NET_CTRL_VLAN_DEL 1
 
+/**
+ * RSS control
+ *
+ * The RSS feature <todo>
+ */
+#define VIRTIO_NET_RSS_RETA_SIZE 128
+#define VIRTIO_NET_RSS_KEY_SIZE       40
+
+struct virtio_net_ctrl_rss {
+	uint32_t hash_types;
+	uint16_t indirection_table_mask;
+	uint16_t unclassified_queue;
+	uint16_t indirection_table[VIRTIO_NET_RSS_RETA_SIZE];
+	uint16_t max_tx_vq;
+	uint8_t hash_key_length;
+	uint8_t hash_key_data[VIRTIO_NET_RSS_KEY_SIZE];
+};
+
 /*
  * Control link announce acknowledgement
  *
@@ -272,7 +290,10 @@ struct virtqueue {
 
 /* If multiqueue is provided by host, then we suppport it. */
 #define VIRTIO_NET_CTRL_MQ   4
+
 #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET        0
+#define VIRTIO_NET_CTRL_MQ_RSS_CONFIG          1
+
 #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN        1
 #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX        0x8000
 
-- 
2.31.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [dpdk-dev] [PATCH v2 2/5] app/testpmd: fix RSS key length
  2021-09-22  9:57 [dpdk-dev] [PATCH v2 0/5] Virtio PMD RSS support & RSS fixes Maxime Coquelin
  2021-09-22  9:57 ` [dpdk-dev] [PATCH v2 1/5] net/virtio: add initial RSS support Maxime Coquelin
@ 2021-09-22  9:57 ` Maxime Coquelin
  2021-09-22 11:25   ` Li, Xiaoyun
  2021-09-23  7:52   ` Xia, Chenbo
  2021-09-22  9:57 ` [dpdk-dev] [PATCH v2 3/5] app/testpmd: fix RSS type display Maxime Coquelin
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 16+ messages in thread
From: Maxime Coquelin @ 2021-09-22  9:57 UTC (permalink / raw)
  To: dev, chenbo.xia, amorenoz, david.marchand, andrew.rybchenko,
	ferruh.yigit, michaelba, viacheslavo, xiaoyun.li
  Cc: stable, nelio.laranjeiro, yvugenfi, ybendito, Maxime Coquelin

port_rss_hash_key_update() initializes rss_conf with the
RSS key configuration provided  by the user, but it calls
rte_eth_dev_rss_hash_conf_get() before calling
rte_eth_dev_rss_hash_update(), which overrides the parsed
RSS config.

While the RSS key value is set again after, this is not
the case of the key length. It could cause out of bounds
access if the key length parsed is smaller than the one
read from rte_eth_dev_rss_hash_conf_get().

This patch restores the key length before the
rte_eth_dev_rss_hash_update() call to ensure the RSS key
value/length pair is consistent.

Fixes: 8205e241b2b0 ("app/testpmd: add missing type to RSS hash commands")
Cc: stable@dpdk.org

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 app/test-pmd/config.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 9c66329e96..611965769c 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -2854,7 +2854,7 @@ port_rss_hash_key_update(portid_t port_id, char rss_type[], uint8_t *hash_key,
 	unsigned int i;
 
 	rss_conf.rss_key = NULL;
-	rss_conf.rss_key_len = hash_key_len;
+	rss_conf.rss_key_len = 0;
 	rss_conf.rss_hf = 0;
 	for (i = 0; rss_type_table[i].str; i++) {
 		if (!strcmp(rss_type_table[i].str, rss_type))
@@ -2863,6 +2863,7 @@ port_rss_hash_key_update(portid_t port_id, char rss_type[], uint8_t *hash_key,
 	diag = rte_eth_dev_rss_hash_conf_get(port_id, &rss_conf);
 	if (diag == 0) {
 		rss_conf.rss_key = hash_key;
+		rss_conf.rss_key_len = hash_key_len;
 		diag = rte_eth_dev_rss_hash_update(port_id, &rss_conf);
 	}
 	if (diag == 0)
-- 
2.31.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [dpdk-dev] [PATCH v2 3/5] app/testpmd: fix RSS type display
  2021-09-22  9:57 [dpdk-dev] [PATCH v2 0/5] Virtio PMD RSS support & RSS fixes Maxime Coquelin
  2021-09-22  9:57 ` [dpdk-dev] [PATCH v2 1/5] net/virtio: add initial RSS support Maxime Coquelin
  2021-09-22  9:57 ` [dpdk-dev] [PATCH v2 2/5] app/testpmd: fix RSS key length Maxime Coquelin
@ 2021-09-22  9:57 ` Maxime Coquelin
  2021-09-22 12:01   ` Li, Xiaoyun
  2021-09-23  7:59   ` Xia, Chenbo
  2021-09-22  9:57 ` [dpdk-dev] [PATCH v2 4/5] net/mlx5: fix RSS RETA update Maxime Coquelin
  2021-09-22  9:57 ` [dpdk-dev] [PATCH v2 5/5] app/testpmd: add missing flow types in port info Maxime Coquelin
  4 siblings, 2 replies; 16+ messages in thread
From: Maxime Coquelin @ 2021-09-22  9:57 UTC (permalink / raw)
  To: dev, chenbo.xia, amorenoz, david.marchand, andrew.rybchenko,
	ferruh.yigit, michaelba, viacheslavo, xiaoyun.li
  Cc: stable, nelio.laranjeiro, yvugenfi, ybendito, Maxime Coquelin

This patch fixes the display of the RSS hash types
configured in the port, which displayed "all" even
if only a single type was configured

Fixes: 3c90743dd3b9 ("app/testpmd: support more types for flow RSS")
Cc: stable@dpdk.org

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 app/test-pmd/config.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 611965769c..9a4a0c232b 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -2833,7 +2833,9 @@ port_rss_hash_conf_show(portid_t port_id, int show_rss_key)
 	}
 	printf("RSS functions:\n ");
 	for (i = 0; rss_type_table[i].str; i++) {
-		if (rss_hf & rss_type_table[i].rss_type)
+		if (rss_type_table[i].rss_type == 0)
+			continue;
+		if ((rss_hf & rss_type_table[i].rss_type) == rss_type_table[i].rss_type)
 			printf("%s ", rss_type_table[i].str);
 	}
 	printf("\n");
-- 
2.31.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [dpdk-dev] [PATCH v2 4/5] net/mlx5: fix RSS RETA update
  2021-09-22  9:57 [dpdk-dev] [PATCH v2 0/5] Virtio PMD RSS support & RSS fixes Maxime Coquelin
                   ` (2 preceding siblings ...)
  2021-09-22  9:57 ` [dpdk-dev] [PATCH v2 3/5] app/testpmd: fix RSS type display Maxime Coquelin
@ 2021-09-22  9:57 ` Maxime Coquelin
  2021-09-22  9:57 ` [dpdk-dev] [PATCH v2 5/5] app/testpmd: add missing flow types in port info Maxime Coquelin
  4 siblings, 0 replies; 16+ messages in thread
From: Maxime Coquelin @ 2021-09-22  9:57 UTC (permalink / raw)
  To: dev, chenbo.xia, amorenoz, david.marchand, andrew.rybchenko,
	ferruh.yigit, michaelba, viacheslavo, xiaoyun.li
  Cc: stable, nelio.laranjeiro, yvugenfi, ybendito, Maxime Coquelin

This patch fixes RETA updating for entries above 64.
Without ithat, these entries are never updated as
calculated mask value will always be 0.

Fixes: 634efbc2c8c0 ("mlx5: support RETA query and update")
Cc: stable@dpdk.org
Cc: nelio.laranjeiro@6wind.com

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
---
 drivers/net/mlx5/mlx5_rss.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/mlx5/mlx5_rss.c b/drivers/net/mlx5/mlx5_rss.c
index c32129cdc2..6dc52acee0 100644
--- a/drivers/net/mlx5/mlx5_rss.c
+++ b/drivers/net/mlx5/mlx5_rss.c
@@ -211,7 +211,7 @@ mlx5_dev_rss_reta_update(struct rte_eth_dev *dev,
 	for (idx = 0, i = 0; (i != reta_size); ++i) {
 		idx = i / RTE_RETA_GROUP_SIZE;
 		pos = i % RTE_RETA_GROUP_SIZE;
-		if (((reta_conf[idx].mask >> i) & 0x1) == 0)
+		if (((reta_conf[idx].mask >> pos) & 0x1) == 0)
 			continue;
 		MLX5_ASSERT(reta_conf[idx].reta[pos] < priv->rxqs_n);
 		(*priv->reta_idx)[i] = reta_conf[idx].reta[pos];
-- 
2.31.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [dpdk-dev] [PATCH v2 5/5] app/testpmd: add missing flow types in port info
  2021-09-22  9:57 [dpdk-dev] [PATCH v2 0/5] Virtio PMD RSS support & RSS fixes Maxime Coquelin
                   ` (3 preceding siblings ...)
  2021-09-22  9:57 ` [dpdk-dev] [PATCH v2 4/5] net/mlx5: fix RSS RETA update Maxime Coquelin
@ 2021-09-22  9:57 ` Maxime Coquelin
  2021-09-22 11:31   ` Li, Xiaoyun
  4 siblings, 1 reply; 16+ messages in thread
From: Maxime Coquelin @ 2021-09-22  9:57 UTC (permalink / raw)
  To: dev, chenbo.xia, amorenoz, david.marchand, andrew.rybchenko,
	ferruh.yigit, michaelba, viacheslavo, xiaoyun.li
  Cc: stable, nelio.laranjeiro, yvugenfi, ybendito, Maxime Coquelin

This patch adds missing IPv6-Ex flow types to port
info command.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 app/test-pmd/config.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 9a4a0c232b..3550e0a18f 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -4528,6 +4528,9 @@ flowtype_to_str(uint16_t flow_type)
 		{"ipv6-sctp", RTE_ETH_FLOW_NONFRAG_IPV6_SCTP},
 		{"ipv6-other", RTE_ETH_FLOW_NONFRAG_IPV6_OTHER},
 		{"l2_payload", RTE_ETH_FLOW_L2_PAYLOAD},
+		{"ipv6-ex", RTE_ETH_FLOW_IPV6_EX},
+		{"ipv6-tcp-ex", RTE_ETH_FLOW_IPV6_TCP_EX},
+		{"ipv6-udp-ex", RTE_ETH_FLOW_IPV6_UDP_EX},
 		{"port", RTE_ETH_FLOW_PORT},
 		{"vxlan", RTE_ETH_FLOW_VXLAN},
 		{"geneve", RTE_ETH_FLOW_GENEVE},
-- 
2.31.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [PATCH v2 1/5] net/virtio: add initial RSS support
  2021-09-22  9:57 ` [dpdk-dev] [PATCH v2 1/5] net/virtio: add initial RSS support Maxime Coquelin
@ 2021-09-22 11:20   ` Andrew Rybchenko
  2021-09-22 12:28     ` Maxime Coquelin
  2021-09-23  7:35   ` Xia, Chenbo
  1 sibling, 1 reply; 16+ messages in thread
From: Andrew Rybchenko @ 2021-09-22 11:20 UTC (permalink / raw)
  To: Maxime Coquelin, dev, chenbo.xia, amorenoz, david.marchand,
	ferruh.yigit, michaelba, viacheslavo, xiaoyun.li
  Cc: stable, nelio.laranjeiro, yvugenfi, ybendito

On 9/22/21 12:57 PM, Maxime Coquelin wrote:
> 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.coquelin@redhat.com>

RSS may be configured on rte_eth_dev_configure() stage
using eth_conf->rx_adv_conf.rss_conf. It looks like the
patch does not handle it.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [PATCH v2 2/5] app/testpmd: fix RSS key length
  2021-09-22  9:57 ` [dpdk-dev] [PATCH v2 2/5] app/testpmd: fix RSS key length Maxime Coquelin
@ 2021-09-22 11:25   ` Li, Xiaoyun
  2021-09-23  7:52   ` Xia, Chenbo
  1 sibling, 0 replies; 16+ messages in thread
From: Li, Xiaoyun @ 2021-09-22 11:25 UTC (permalink / raw)
  To: Maxime Coquelin, dev, Xia, Chenbo, amorenoz, david.marchand,
	andrew.rybchenko, Yigit, Ferruh, michaelba, viacheslavo
  Cc: stable, nelio.laranjeiro, yvugenfi, ybendito

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Wednesday, September 22, 2021 17:58
> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>;
> amorenoz@redhat.com; david.marchand@redhat.com;
> andrew.rybchenko@oktetlabs.ru; Yigit, Ferruh <ferruh.yigit@intel.com>;
> michaelba@nvidia.com; viacheslavo@nvidia.com; Li, Xiaoyun
> <xiaoyun.li@intel.com>
> Cc: stable@dpdk.org; nelio.laranjeiro@6wind.com; yvugenfi@redhat.com;
> ybendito@redhat.com; Maxime Coquelin <maxime.coquelin@redhat.com>
> Subject: [PATCH v2 2/5] app/testpmd: fix RSS key length
> 
> port_rss_hash_key_update() initializes rss_conf with the RSS key configuration
> provided  by the user, but it calls
> rte_eth_dev_rss_hash_conf_get() before calling rte_eth_dev_rss_hash_update(),
> which overrides the parsed RSS config.
> 
> While the RSS key value is set again after, this is not the case of the key length. It
> could cause out of bounds access if the key length parsed is smaller than the one
> read from rte_eth_dev_rss_hash_conf_get().
> 
> This patch restores the key length before the
> rte_eth_dev_rss_hash_update() call to ensure the RSS key value/length pair is
> consistent.
> 
> Fixes: 8205e241b2b0 ("app/testpmd: add missing type to RSS hash commands")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  app/test-pmd/config.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index
> 9c66329e96..611965769c 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -2854,7 +2854,7 @@ port_rss_hash_key_update(portid_t port_id, char
> rss_type[], uint8_t *hash_key,
>  	unsigned int i;
> 
>  	rss_conf.rss_key = NULL;
> -	rss_conf.rss_key_len = hash_key_len;
> +	rss_conf.rss_key_len = 0;
>  	rss_conf.rss_hf = 0;
>  	for (i = 0; rss_type_table[i].str; i++) {
>  		if (!strcmp(rss_type_table[i].str, rss_type)) @@ -2863,6 +2863,7
> @@ port_rss_hash_key_update(portid_t port_id, char rss_type[], uint8_t
> *hash_key,
>  	diag = rte_eth_dev_rss_hash_conf_get(port_id, &rss_conf);
>  	if (diag == 0) {
>  		rss_conf.rss_key = hash_key;
> +		rss_conf.rss_key_len = hash_key_len;
>  		diag = rte_eth_dev_rss_hash_update(port_id, &rss_conf);
>  	}
>  	if (diag == 0)
> --
> 2.31.1

Acked-by: Xiaoyun Li <xiaoyun.li@intel.com>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [PATCH v2 5/5] app/testpmd: add missing flow types in port info
  2021-09-22  9:57 ` [dpdk-dev] [PATCH v2 5/5] app/testpmd: add missing flow types in port info Maxime Coquelin
@ 2021-09-22 11:31   ` Li, Xiaoyun
  2021-09-29  7:27     ` Maxime Coquelin
  0 siblings, 1 reply; 16+ messages in thread
From: Li, Xiaoyun @ 2021-09-22 11:31 UTC (permalink / raw)
  To: Maxime Coquelin, dev, Xia, Chenbo, amorenoz, david.marchand,
	andrew.rybchenko, Yigit, Ferruh, michaelba, viacheslavo
  Cc: stable, nelio.laranjeiro, yvugenfi, ybendito

Hi

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Wednesday, September 22, 2021 17:58
> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>;
> amorenoz@redhat.com; david.marchand@redhat.com;
> andrew.rybchenko@oktetlabs.ru; Yigit, Ferruh <ferruh.yigit@intel.com>;
> michaelba@nvidia.com; viacheslavo@nvidia.com; Li, Xiaoyun
> <xiaoyun.li@intel.com>
> Cc: stable@dpdk.org; nelio.laranjeiro@6wind.com; yvugenfi@redhat.com;
> ybendito@redhat.com; Maxime Coquelin <maxime.coquelin@redhat.com>
> Subject: [PATCH v2 5/5] app/testpmd: add missing flow types in port info
> 
> This patch adds missing IPv6-Ex flow types to port info command.
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  app/test-pmd/config.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index
> 9a4a0c232b..3550e0a18f 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -4528,6 +4528,9 @@ flowtype_to_str(uint16_t flow_type)
>  		{"ipv6-sctp", RTE_ETH_FLOW_NONFRAG_IPV6_SCTP},
>  		{"ipv6-other", RTE_ETH_FLOW_NONFRAG_IPV6_OTHER},
>  		{"l2_payload", RTE_ETH_FLOW_L2_PAYLOAD},
> +		{"ipv6-ex", RTE_ETH_FLOW_IPV6_EX},
> +		{"ipv6-tcp-ex", RTE_ETH_FLOW_IPV6_TCP_EX},
> +		{"ipv6-udp-ex", RTE_ETH_FLOW_IPV6_UDP_EX},
>  		{"port", RTE_ETH_FLOW_PORT},
>  		{"vxlan", RTE_ETH_FLOW_VXLAN},
>  		{"geneve", RTE_ETH_FLOW_GENEVE},

You add missing ipv6 ex why not adding RTE_ETH_FLOW_GTPU too. It's also missing.

Also, flowtype_to_str is added but what about str2flowtype() in cmdline.c?

> --
> 2.31.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [PATCH v2 3/5] app/testpmd: fix RSS type display
  2021-09-22  9:57 ` [dpdk-dev] [PATCH v2 3/5] app/testpmd: fix RSS type display Maxime Coquelin
@ 2021-09-22 12:01   ` Li, Xiaoyun
  2021-09-23  7:59   ` Xia, Chenbo
  1 sibling, 0 replies; 16+ messages in thread
From: Li, Xiaoyun @ 2021-09-22 12:01 UTC (permalink / raw)
  To: Maxime Coquelin, dev, Xia, Chenbo, amorenoz, david.marchand,
	andrew.rybchenko, Yigit, Ferruh, michaelba, viacheslavo
  Cc: stable, nelio.laranjeiro, yvugenfi, ybendito


> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Wednesday, September 22, 2021 17:58
> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>;
> amorenoz@redhat.com; david.marchand@redhat.com;
> andrew.rybchenko@oktetlabs.ru; Yigit, Ferruh <ferruh.yigit@intel.com>;
> michaelba@nvidia.com; viacheslavo@nvidia.com; Li, Xiaoyun
> <xiaoyun.li@intel.com>
> Cc: stable@dpdk.org; nelio.laranjeiro@6wind.com; yvugenfi@redhat.com;
> ybendito@redhat.com; Maxime Coquelin <maxime.coquelin@redhat.com>
> Subject: [PATCH v2 3/5] app/testpmd: fix RSS type display
> 
> This patch fixes the display of the RSS hash types configured in the port, which
> displayed "all" even if only a single type was configured
> 
> Fixes: 3c90743dd3b9 ("app/testpmd: support more types for flow RSS")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  app/test-pmd/config.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index
> 611965769c..9a4a0c232b 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -2833,7 +2833,9 @@ port_rss_hash_conf_show(portid_t port_id, int
> show_rss_key)
>  	}
>  	printf("RSS functions:\n ");
>  	for (i = 0; rss_type_table[i].str; i++) {
> -		if (rss_hf & rss_type_table[i].rss_type)
> +		if (rss_type_table[i].rss_type == 0)
> +			continue;
> +		if ((rss_hf & rss_type_table[i].rss_type) ==
> +rss_type_table[i].rss_type)
>  			printf("%s ", rss_type_table[i].str);
>  	}
>  	printf("\n");
> --
> 2.31.1

Acked-by: Xiaoyun Li <xiaoyun.li@intel.com>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [PATCH v2 1/5] net/virtio: add initial RSS support
  2021-09-22 11:20   ` Andrew Rybchenko
@ 2021-09-22 12:28     ` Maxime Coquelin
  0 siblings, 0 replies; 16+ messages in thread
From: Maxime Coquelin @ 2021-09-22 12:28 UTC (permalink / raw)
  To: Andrew Rybchenko, dev, chenbo.xia, amorenoz, david.marchand,
	ferruh.yigit, michaelba, viacheslavo, xiaoyun.li
  Cc: stable, nelio.laranjeiro, yvugenfi, ybendito



On 9/22/21 13:20, Andrew Rybchenko wrote:
> On 9/22/21 12:57 PM, Maxime Coquelin wrote:
>> 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.coquelin@redhat.com>
> 
> RSS may be configured on rte_eth_dev_configure() stage
> using eth_conf->rx_adv_conf.rss_conf. It looks like the
> patch does not handle it.
> 

Good catch, I indeed missed that part.
I'll post a v3 including its support.

Thanks,
Maxime


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [PATCH v2 1/5] net/virtio: add initial RSS support
  2021-09-22  9:57 ` [dpdk-dev] [PATCH v2 1/5] net/virtio: add initial RSS support Maxime Coquelin
  2021-09-22 11:20   ` Andrew Rybchenko
@ 2021-09-23  7:35   ` Xia, Chenbo
  2021-09-29  9:02     ` Maxime Coquelin
  1 sibling, 1 reply; 16+ messages in thread
From: Xia, Chenbo @ 2021-09-23  7:35 UTC (permalink / raw)
  To: Maxime Coquelin, dev, amorenoz, david.marchand, andrew.rybchenko,
	Yigit, Ferruh, michaelba, viacheslavo, Li, Xiaoyun
  Cc: stable, nelio.laranjeiro, yvugenfi, ybendito

Hi Maxime,

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Wednesday, September 22, 2021 5:58 PM
> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; amorenoz@redhat.com;
> david.marchand@redhat.com; andrew.rybchenko@oktetlabs.ru; Yigit, Ferruh
> <ferruh.yigit@intel.com>; michaelba@nvidia.com; viacheslavo@nvidia.com; Li,
> Xiaoyun <xiaoyun.li@intel.com>
> Cc: stable@dpdk.org; nelio.laranjeiro@6wind.com; yvugenfi@redhat.com;
> ybendito@redhat.com; Maxime Coquelin <maxime.coquelin@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.coquelin@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?

> +#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'?

> +
> +	rss.hash_types = hw->rss_hash_types & VIRTIO_NET_HASH_TYPE_MASK;
> +	rss.indirection_table_mask = VIRTIO_NET_RSS_RETA_SIZE - 1;
> +	rss.unclassified_queue = 0;
> +	memcpy(rss.indirection_table, hw->rss_reta, VIRTIO_NET_RSS_RETA_SIZE *
> sizeof(uint16_t));
> +	rss.max_tx_vq = nb_queues;
> +	rss.hash_key_length = VIRTIO_NET_RSS_KEY_SIZE;
> +	memcpy(rss.hash_key_data, hw->rss_key, VIRTIO_NET_RSS_KEY_SIZE);
> +
> +	ctrl.hdr.class = VIRTIO_NET_CTRL_MQ;
> +	ctrl.hdr.cmd = VIRTIO_NET_CTRL_MQ_RSS_CONFIG;
> +	memcpy(ctrl.data, &rss, sizeof(rss));
> +
> +	dlen[0] = sizeof(rss);
> +
> +	ret = virtio_send_command(hw->cvq, &ctrl, dlen, 1);
> +	if (ret) {
> +		PMD_INIT_LOG(ERR, "RSS multiqueue configured but send command
> failed");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int
> +virtio_set_multiple_queues_auto(struct rte_eth_dev *dev, uint16_t nb_queues)
>  {
>  	struct virtio_hw *hw = dev->data->dev_private;
>  	struct virtio_pmd_ctrl ctrl;
> @@ -370,6 +411,17 @@ virtio_set_multiple_queues(struct rte_eth_dev *dev,
> uint16_t nb_queues)
>  	return 0;
>  }
> 
> +static int
> +virtio_set_multiple_queues(struct rte_eth_dev *dev, uint16_t nb_queues)
> +{
> +	struct virtio_hw *hw = dev->data->dev_private;
> +
> +	if (virtio_with_feature(hw, VIRTIO_NET_F_RSS))
> +		return virtio_set_multiple_queues_rss(dev, nb_queues);
> +	else
> +		return virtio_set_multiple_queues_auto(dev, nb_queues);
> +}
> +
>  static void
>  virtio_dev_queue_release(void *queue __rte_unused)
>  {
> @@ -701,6 +753,16 @@ virtio_alloc_queues(struct rte_eth_dev *dev)
> 
>  static void virtio_queues_unbind_intr(struct rte_eth_dev *dev);
> 
> +static void
> +virtio_free_rss(struct virtio_hw *hw)
> +{
> +	rte_free(hw->rss_key);
> +	hw->rss_key = NULL;
> +
> +	rte_free(hw->rss_reta);
> +	hw->rss_reta = NULL;
> +}
> +
>  int
>  virtio_dev_close(struct rte_eth_dev *dev)
>  {
> @@ -731,6 +793,7 @@ virtio_dev_close(struct rte_eth_dev *dev)
>  	virtio_reset(hw);
>  	virtio_dev_free_mbufs(dev);
>  	virtio_free_queues(hw);
> +	virtio_free_rss(hw);
> 
>  	return VIRTIO_OPS(hw)->dev_close(hw);
>  }
> @@ -971,6 +1034,10 @@ static const struct eth_dev_ops virtio_eth_dev_ops = {
>  	.rx_queue_release        = virtio_dev_queue_release,
>  	.tx_queue_setup          = virtio_dev_tx_queue_setup,
>  	.tx_queue_release        = virtio_dev_queue_release,
> +	.rss_hash_update         = virtio_dev_rss_hash_update,
> +	.rss_hash_conf_get       = virtio_dev_rss_hash_conf_get,
> +	.reta_update             = virtio_dev_rss_reta_update,
> +	.reta_query              = virtio_dev_rss_reta_query,
>  	/* collect stats per queue */
>  	.queue_stats_mapping_set = virtio_dev_queue_stats_mapping_set,
>  	.vlan_filter_set         = virtio_vlan_filter_set,
> @@ -1714,6 +1781,270 @@ virtio_configure_intr(struct rte_eth_dev *dev)
> 
>  	return 0;
>  }
> +
> +static uint64_t
> +virtio_to_ethdev_rss_offloads(uint64_t virtio_hash_types)
> +{
> +	uint64_t rss_offloads = 0;
> +
> +	if (virtio_hash_types & VIRTIO_NET_HASH_TYPE_IPV4)
> +		rss_offloads |= ETH_RSS_IPV4 | ETH_RSS_FRAG_IPV4 |
> ETH_RSS_NONFRAG_IPV4_OTHER;
> +
> +	if (virtio_hash_types & VIRTIO_NET_HASH_TYPE_TCPV4)
> +		rss_offloads |= ETH_RSS_NONFRAG_IPV4_TCP;
> +
> +	if (virtio_hash_types & VIRTIO_NET_HASH_TYPE_UDPV4)
> +		rss_offloads |= ETH_RSS_NONFRAG_IPV4_UDP;
> +
> +	if (virtio_hash_types & VIRTIO_NET_HASH_TYPE_IPV6)
> +		rss_offloads |= ETH_RSS_IPV6 | ETH_RSS_FRAG_IPV6 |
> ETH_RSS_NONFRAG_IPV6_OTHER;
> +
> +	if (virtio_hash_types & VIRTIO_NET_HASH_TYPE_TCPV6)
> +		rss_offloads |= ETH_RSS_NONFRAG_IPV6_TCP;
> +
> +	if (virtio_hash_types & VIRTIO_NET_HASH_TYPE_UDPV6)
> +		rss_offloads |= ETH_RSS_NONFRAG_IPV6_UDP;
> +
> +	if (virtio_hash_types & VIRTIO_NET_HASH_TYPE_IP_EX)
> +		rss_offloads |= ETH_RSS_IPV6_EX;
> +
> +	if (virtio_hash_types & VIRTIO_NET_HASH_TYPE_TCP_EX)
> +		rss_offloads |= ETH_RSS_IPV6_TCP_EX;
> +
> +	if (virtio_hash_types & VIRTIO_NET_HASH_TYPE_UDP_EX)
> +		rss_offloads |= ETH_RSS_IPV6_UDP_EX;
> +
> +	return rss_offloads;
> +}
> +
> +static int
> +virtio_dev_get_rss_config(struct virtio_hw *hw, uint32_t *rss_hash_types)
> +{
> +	struct virtio_net_config local_config;
> +	struct virtio_net_config *config = &local_config;
> +
> +	virtio_read_dev_config(hw,
> +			offsetof(struct virtio_net_config, rss_max_key_size),
> +			&config->rss_max_key_size,
> +			sizeof(config->rss_max_key_size));
> +	if (config->rss_max_key_size < VIRTIO_NET_RSS_KEY_SIZE) {
> +		PMD_INIT_LOG(ERR, "Invalid device RSS max key size (%u)",
> +				config->rss_max_key_size);
> +		return -1;
> +	}
> +
> +	virtio_read_dev_config(hw,
> +			offsetof(struct virtio_net_config,
> +				rss_max_indirection_table_length),
> +			&config->rss_max_indirection_table_length,
> +			sizeof(config->rss_max_indirection_table_length));
> +	if (config->rss_max_indirection_table_length < VIRTIO_NET_RSS_RETA_SIZE)
> {
> +		PMD_INIT_LOG(ERR, "Invalid device RSS max reta size (%u)",
> +				config->rss_max_indirection_table_length);
> +		return -1;
> +	}
> +
> +	virtio_read_dev_config(hw,
> +			offsetof(struct virtio_net_config, supported_hash_types),
> +			&config->supported_hash_types,
> +			sizeof(config->supported_hash_types));
> +	if ((config->supported_hash_types & VIRTIO_NET_HASH_TYPE_MASK) == 0) {
> +		PMD_INIT_LOG(ERR, "Invalid device RSS hash types (%u)",
> +				config->supported_hash_types);
> +		return -1;
> +	}
> +
> +	*rss_hash_types = config->supported_hash_types &
> VIRTIO_NET_HASH_TYPE_MASK;
> +
> +	PMD_INIT_LOG(DEBUG, "Device RSS config:");
> +	PMD_INIT_LOG(DEBUG, "\t-Max key size: %u", config->rss_max_key_size);
> +	PMD_INIT_LOG(DEBUG, "\t-Max reta size: %u", config-
> >rss_max_indirection_table_length);
> +	PMD_INIT_LOG(DEBUG, "\t-Supported hash types: 0x%x", *rss_hash_types);
> +
> +	return 0;
> +}
> +
> +static int
> +virtio_dev_rss_hash_update(struct rte_eth_dev *dev,
> +		struct rte_eth_rss_conf *rss_conf)
> +{
> +	struct virtio_hw *hw = dev->data->dev_private;
> +	uint16_t nb_queues;
> +
> +	if (!virtio_with_feature(hw, VIRTIO_NET_F_RSS))
> +		return -ENOTSUP;
> +
> +	if (rss_conf->rss_hf &
> ~virtio_to_ethdev_rss_offloads(VIRTIO_NET_HASH_TYPE_MASK))
> +		return -EINVAL;
> +
> +	hw->rss_hash_types = 0;
> +
> +	if (rss_conf->rss_hf & (ETH_RSS_IPV4 | ETH_RSS_FRAG_IPV4 |
> ETH_RSS_NONFRAG_IPV4_OTHER))
> +		hw->rss_hash_types |= VIRTIO_NET_HASH_TYPE_IPV4;
> +
> +	if (rss_conf->rss_hf & ETH_RSS_NONFRAG_IPV4_TCP)
> +		hw->rss_hash_types |= VIRTIO_NET_HASH_TYPE_TCPV4;
> +
> +	if (rss_conf->rss_hf & ETH_RSS_NONFRAG_IPV4_UDP)
> +		hw->rss_hash_types |= VIRTIO_NET_HASH_TYPE_UDPV4;
> +
> +	if (rss_conf->rss_hf & (ETH_RSS_IPV6 | ETH_RSS_FRAG_IPV6 |
> ETH_RSS_NONFRAG_IPV6_OTHER))
> +		hw->rss_hash_types |= VIRTIO_NET_HASH_TYPE_IPV6;
> +
> +	if (rss_conf->rss_hf & ETH_RSS_NONFRAG_IPV6_TCP)
> +		hw->rss_hash_types |= VIRTIO_NET_HASH_TYPE_TCPV6;
> +
> +	if (rss_conf->rss_hf & ETH_RSS_NONFRAG_IPV6_UDP)
> +		hw->rss_hash_types |= VIRTIO_NET_HASH_TYPE_UDPV6;
> +
> +	if (rss_conf->rss_hf & ETH_RSS_IPV6_EX)
> +		hw->rss_hash_types |= VIRTIO_NET_HASH_TYPE_IP_EX;
> +
> +	if (rss_conf->rss_hf & ETH_RSS_IPV6_TCP_EX)
> +		hw->rss_hash_types |= VIRTIO_NET_HASH_TYPE_TCP_EX;
> +
> +	if (rss_conf->rss_hf & ETH_RSS_IPV6_UDP_EX)
> +		hw->rss_hash_types |= VIRTIO_NET_HASH_TYPE_UDP_EX;
> +
> +	if (rss_conf->rss_key && rss_conf->rss_key_len) {
> +		if (rss_conf->rss_key_len != VIRTIO_NET_RSS_KEY_SIZE) {
> +			PMD_INIT_LOG(ERR, "Driver only supports %u RSS key length",
> +					VIRTIO_NET_RSS_KEY_SIZE);
> +			return -EINVAL;
> +		}
> +		memcpy(hw->rss_key, rss_conf->rss_key, VIRTIO_NET_RSS_KEY_SIZE);
> +	}
> +
> +	nb_queues = RTE_MAX(dev->data->nb_rx_queues, dev->data->nb_tx_queues);
> +	return virtio_set_multiple_queues_rss(dev, nb_queues);
> +}
> +
> +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.

> +
> +	if (rss_conf->rss_key && rss_conf->rss_key_len >=
> VIRTIO_NET_RSS_KEY_SIZE)
> +		memcpy(rss_conf->rss_key, hw->rss_key, VIRTIO_NET_RSS_KEY_SIZE);
> +	rss_conf->rss_key_len = VIRTIO_NET_RSS_KEY_SIZE;
> +	rss_conf->rss_hf = virtio_to_ethdev_rss_offloads(hw->rss_hash_types);
> +
> +	return 0;
> +}
> +
> +static int virtio_dev_rss_reta_update(struct rte_eth_dev *dev,
> +			 struct rte_eth_rss_reta_entry64 *reta_conf,
> +			 uint16_t reta_size)
> +{
> +	struct virtio_hw *hw = dev->data->dev_private;
> +	uint16_t nb_queues;
> +	int idx, pos, i;
> +
> +	if (!virtio_with_feature(hw, VIRTIO_NET_F_RSS))
> +		return -ENOTSUP;
> +
> +	if (!reta_conf)
> +		return -EINVAL;

Ditto.

> +
> +	if (reta_size != VIRTIO_NET_RSS_RETA_SIZE)
> +		return -EINVAL;
> +
> +	for (idx = 0, i = 0; i < reta_size; i++) {
> +		idx = i / RTE_RETA_GROUP_SIZE;
> +		pos = i % RTE_RETA_GROUP_SIZE;
> +
> +		if (((reta_conf[idx].mask >> pos) & 0x1) == 0)
> +			continue;
> +
> +		hw->rss_reta[i] = reta_conf[idx].reta[pos];
> +	}
> +
> +	nb_queues = RTE_MAX(dev->data->nb_rx_queues, dev->data->nb_tx_queues);
> +	return virtio_set_multiple_queues_rss(dev, nb_queues);
> +}
> +
> +static int virtio_dev_rss_reta_query(struct rte_eth_dev *dev,
> +			 struct rte_eth_rss_reta_entry64 *reta_conf,
> +			 uint16_t reta_size)
> +{
> +	struct virtio_hw *hw = dev->data->dev_private;
> +	int idx, i;
> +
> +	if (!virtio_with_feature(hw, VIRTIO_NET_F_RSS))
> +		return -ENOTSUP;
> +
> +	if (!reta_conf)
> +		return -EINVAL;

Ditto

> +
> +	if (reta_size != VIRTIO_NET_RSS_RETA_SIZE)
> +		return -EINVAL;
> +
> +	for (idx = 0, i = 0; i < reta_size; i++) {
> +		idx = i / RTE_RETA_GROUP_SIZE;
> +		reta_conf[idx].reta[i % RTE_RETA_GROUP_SIZE] = hw->rss_reta[i];
> +	}
> +
> +	return 0;
> +}
> +
> +/*
> + * As default RSS hash key, it uses the default key of the
> + * Intel IXGBE devices. It can be updated by the application
> + * with any 40B key value.
> + */
> +static uint8_t rss_intel_key[VIRTIO_NET_RSS_KEY_SIZE] = {
> +	0x6D, 0x5A, 0x56, 0xDA, 0x25, 0x5B, 0x0E, 0xC2,
> +	0x41, 0x67, 0x25, 0x3D, 0x43, 0xA3, 0x8F, 0xB0,
> +	0xD0, 0xCA, 0x2B, 0xCB, 0xAE, 0x7B, 0x30, 0xB4,
> +	0x77, 0xCB, 0x2D, 0xA3, 0x80, 0x30, 0xF2, 0x0C,
> +	0x6A, 0x42, 0xB7, 0x3B, 0xBE, 0xAC, 0x01, 0xFA,
> +};
> +
> +static int
> +virtio_dev_rss_init(struct rte_eth_dev *eth_dev)
> +{
> +	struct virtio_hw *hw = eth_dev->data->dev_private;
> +	uint16_t nb_rx_queues = eth_dev->data->nb_rx_queues;
> +	int i;
> +
> +	if (virtio_dev_get_rss_config(hw, &hw->rss_hash_types))
> +		return -1;
> +
> +	if (!hw->rss_key) {
> +		/* Setup default RSS key if not already setup by the user */
> +		hw->rss_key = rte_malloc_socket("rss_key",
> +				VIRTIO_NET_RSS_KEY_SIZE, 0,
> +				eth_dev->device->numa_node);
> +		if (!hw->rss_key) {
> +			PMD_INIT_LOG(ERR, "Failed to allocate RSS key");
> +			return -1;
> +		}
> +		rte_memcpy(hw->rss_key, rss_intel_key, VIRTIO_NET_RSS_KEY_SIZE);
> +	}
> +
> +	if (!hw->rss_reta) {
> +		/* Setup default RSS reta if not already setup by the user */
> +		hw->rss_reta = rte_malloc_socket("rss_reta",
> +				VIRTIO_NET_RSS_RETA_SIZE * sizeof(uint16_t), 0,
> +				eth_dev->device->numa_node);
> +		if (!hw->rss_reta) {
> +			PMD_INIT_LOG(ERR, "Failed to allocate RSS reta");
> +			return -1;
> +		}
> +		for (i = 0; i < VIRTIO_NET_RSS_RETA_SIZE; i++)
> +			hw->rss_reta[i] = i % nb_rx_queues;
> +	}
> +
> +	return 0;
> +}
> +
>  #define DUPLEX_UNKNOWN   0xff
>  /* reset device and renegotiate features if needed */
>  static int
> @@ -1801,14 +2132,15 @@ virtio_init_device(struct rte_eth_dev *eth_dev,
> uint64_t req_features)
>  			config->status = 0;
>  		}
> 
> -		if (virtio_with_feature(hw, VIRTIO_NET_F_MQ)) {
> +		if (virtio_with_feature(hw, VIRTIO_NET_F_MQ) ||
> +				virtio_with_feature(hw, VIRTIO_NET_F_RSS)) {
>  			virtio_read_dev_config(hw,
>  				offsetof(struct virtio_net_config,
> max_virtqueue_pairs),
>  				&config->max_virtqueue_pairs,
>  				sizeof(config->max_virtqueue_pairs));
>  		} else {
>  			PMD_INIT_LOG(DEBUG,
> -				     "VIRTIO_NET_F_MQ is not supported");
> +				     "Neither VIRTIO_NET_F_MQ nor VIRTIO_NET_F_RSS are
> supported");
>  			config->max_virtqueue_pairs = 1;
>  		}
> 
> @@ -1840,6 +2172,11 @@ virtio_init_device(struct rte_eth_dev *eth_dev,
> uint64_t req_features)
>  				VLAN_TAG_LEN - hw->vtnet_hdr_size;
>  		}
> 
> +		hw->rss_hash_types = 0;
> +		if (virtio_with_feature(hw, VIRTIO_NET_F_RSS))
> +			if (virtio_dev_rss_init(eth_dev))
> +				return -1;
> +
>  		PMD_INIT_LOG(DEBUG, "config->max_virtqueue_pairs=%d",
>  				config->max_virtqueue_pairs);
>  		PMD_INIT_LOG(DEBUG, "config->status=%d", config->status);
> @@ -2083,7 +2420,7 @@ virtio_dev_configure(struct rte_eth_dev *dev)
>  	PMD_INIT_LOG(DEBUG, "configure");
>  	req_features = VIRTIO_PMD_DEFAULT_GUEST_FEATURES;
> 
> -	if (rxmode->mq_mode != ETH_MQ_RX_NONE) {
> +	if (rxmode->mq_mode != ETH_MQ_RX_NONE && rxmode->mq_mode !=
> ETH_MQ_RX_RSS) {
>  		PMD_DRV_LOG(ERR,
>  			"Unsupported Rx multi queue mode %d",
>  			rxmode->mq_mode);
> @@ -2103,6 +2440,9 @@ virtio_dev_configure(struct rte_eth_dev *dev)
>  			return ret;
>  	}
> 
> +	if (rxmode->mq_mode == ETH_MQ_RX_RSS)
> +		req_features |= (1ULL << VIRTIO_NET_F_RSS);
> +
>  	if ((rx_offloads & DEV_RX_OFFLOAD_JUMBO_FRAME) &&
>  	    (rxmode->max_rx_pkt_len > hw->max_mtu + ether_hdr_len))
>  		req_features &= ~(1ULL << VIRTIO_NET_F_MTU);
> @@ -2137,6 +2477,12 @@ virtio_dev_configure(struct rte_eth_dev *dev)
>  			return ret;
>  	}
> 
> +	if ((rxmode->mq_mode & ETH_MQ_RX_RSS_FLAG) &&
> +			!virtio_with_feature(hw, VIRTIO_NET_F_RSS)) {
> +		PMD_DRV_LOG(ERR, "RSS support requested but not supported by the
> device");
> +		return -ENOTSUP;
> +	}
> +
>  	if ((rx_offloads & (DEV_RX_OFFLOAD_UDP_CKSUM |
>  			    DEV_RX_OFFLOAD_TCP_CKSUM)) &&
>  		!virtio_with_feature(hw, VIRTIO_NET_F_GUEST_CSUM)) {
> @@ -2503,6 +2849,7 @@ static int
>  virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info
> *dev_info)
>  {
>  	uint64_t tso_mask, host_features;
> +	uint32_t rss_hash_types = 0;
>  	struct virtio_hw *hw = dev->data->dev_private;
>  	dev_info->speed_capa = virtio_dev_speed_capa_get(hw->speed);
> 
> @@ -2543,6 +2890,18 @@ virtio_dev_info_get(struct rte_eth_dev *dev, struct
> rte_eth_dev_info *dev_info)
>  		(1ULL << VIRTIO_NET_F_HOST_TSO6);
>  	if ((host_features & tso_mask) == tso_mask)
>  		dev_info->tx_offload_capa |= DEV_TX_OFFLOAD_TCP_TSO;
> +	if (host_features & (1ULL << VIRTIO_NET_F_RSS)) {
> +		virtio_dev_get_rss_config(hw, &rss_hash_types);
> +		dev_info->hash_key_size = VIRTIO_NET_RSS_KEY_SIZE;
> +		dev_info->reta_size = VIRTIO_NET_RSS_RETA_SIZE;
> +		dev_info->flow_type_rss_offloads =
> +			virtio_to_ethdev_rss_offloads(rss_hash_types);
> +	} else {
> +		dev_info->hash_key_size = 0;
> +		dev_info->reta_size = 0;
> +		dev_info->flow_type_rss_offloads = 0;
> +	}
> +
> 
>  	if (host_features & (1ULL << VIRTIO_F_RING_PACKED)) {
>  		/*
> diff --git a/drivers/net/virtio/virtio_ethdev.h
> b/drivers/net/virtio/virtio_ethdev.h
> index 2f63ef2b2d..3fe30d8784 100644
> --- a/drivers/net/virtio/virtio_ethdev.h
> +++ b/drivers/net/virtio/virtio_ethdev.h
> @@ -45,7 +45,8 @@
>  	 1u << VIRTIO_NET_F_GUEST_TSO6     |	\
>  	 1u << VIRTIO_NET_F_CSUM           |	\
>  	 1u << VIRTIO_NET_F_HOST_TSO4      |	\
> -	 1u << VIRTIO_NET_F_HOST_TSO6)
> +	 1u << VIRTIO_NET_F_HOST_TSO6      |	\
> +	 1ULL << VIRTIO_NET_F_RSS)
> 
>  extern const struct eth_dev_ops virtio_user_secondary_eth_dev_ops;
> 
> diff --git a/drivers/net/virtio/virtqueue.h b/drivers/net/virtio/virtqueue.h
> index 03957b2bd0..99a0e68d38 100644
> --- a/drivers/net/virtio/virtqueue.h
> +++ b/drivers/net/virtio/virtqueue.h
> @@ -182,6 +182,24 @@ struct virtio_net_ctrl_mac {
>  #define VIRTIO_NET_CTRL_VLAN_ADD 0
>  #define VIRTIO_NET_CTRL_VLAN_DEL 1
> 
> +/**
> + * RSS control
> + *
> + * The RSS feature <todo>
> + */
> +#define VIRTIO_NET_RSS_RETA_SIZE 128
> +#define VIRTIO_NET_RSS_KEY_SIZE       40

Better align the numbers?

Thanks,
Chenbo

> +
> +struct virtio_net_ctrl_rss {
> +	uint32_t hash_types;
> +	uint16_t indirection_table_mask;
> +	uint16_t unclassified_queue;
> +	uint16_t indirection_table[VIRTIO_NET_RSS_RETA_SIZE];
> +	uint16_t max_tx_vq;
> +	uint8_t hash_key_length;
> +	uint8_t hash_key_data[VIRTIO_NET_RSS_KEY_SIZE];
> +};
> +
>  /*
>   * Control link announce acknowledgement
>   *
> @@ -272,7 +290,10 @@ struct virtqueue {
> 
>  /* If multiqueue is provided by host, then we suppport it. */
>  #define VIRTIO_NET_CTRL_MQ   4
> +
>  #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET        0
> +#define VIRTIO_NET_CTRL_MQ_RSS_CONFIG          1
> +
>  #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN        1
>  #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX        0x8000
> 
> --
> 2.31.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [PATCH v2 2/5] app/testpmd: fix RSS key length
  2021-09-22  9:57 ` [dpdk-dev] [PATCH v2 2/5] app/testpmd: fix RSS key length Maxime Coquelin
  2021-09-22 11:25   ` Li, Xiaoyun
@ 2021-09-23  7:52   ` Xia, Chenbo
  1 sibling, 0 replies; 16+ messages in thread
From: Xia, Chenbo @ 2021-09-23  7:52 UTC (permalink / raw)
  To: Maxime Coquelin, dev, amorenoz, david.marchand, andrew.rybchenko,
	Yigit, Ferruh, michaelba, viacheslavo, Li, Xiaoyun
  Cc: stable, nelio.laranjeiro, yvugenfi, ybendito

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Wednesday, September 22, 2021 5:58 PM
> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; amorenoz@redhat.com;
> david.marchand@redhat.com; andrew.rybchenko@oktetlabs.ru; Yigit, Ferruh
> <ferruh.yigit@intel.com>; michaelba@nvidia.com; viacheslavo@nvidia.com; Li,
> Xiaoyun <xiaoyun.li@intel.com>
> Cc: stable@dpdk.org; nelio.laranjeiro@6wind.com; yvugenfi@redhat.com;
> ybendito@redhat.com; Maxime Coquelin <maxime.coquelin@redhat.com>
> Subject: [PATCH v2 2/5] app/testpmd: fix RSS key length
> 
> port_rss_hash_key_update() initializes rss_conf with the
> RSS key configuration provided  by the user, but it calls

Double space between 'provided' and 'by'

With this fixed:

Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>

> rte_eth_dev_rss_hash_conf_get() before calling
> rte_eth_dev_rss_hash_update(), which overrides the parsed
> RSS config.
> 
> While the RSS key value is set again after, this is not
> the case of the key length. It could cause out of bounds
> access if the key length parsed is smaller than the one
> read from rte_eth_dev_rss_hash_conf_get().
> 
> This patch restores the key length before the
> rte_eth_dev_rss_hash_update() call to ensure the RSS key
> value/length pair is consistent.
> 
> Fixes: 8205e241b2b0 ("app/testpmd: add missing type to RSS hash commands")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [PATCH v2 3/5] app/testpmd: fix RSS type display
  2021-09-22  9:57 ` [dpdk-dev] [PATCH v2 3/5] app/testpmd: fix RSS type display Maxime Coquelin
  2021-09-22 12:01   ` Li, Xiaoyun
@ 2021-09-23  7:59   ` Xia, Chenbo
  1 sibling, 0 replies; 16+ messages in thread
From: Xia, Chenbo @ 2021-09-23  7:59 UTC (permalink / raw)
  To: Maxime Coquelin, dev, amorenoz, david.marchand, andrew.rybchenko,
	Yigit, Ferruh, michaelba, viacheslavo, Li, Xiaoyun
  Cc: stable, nelio.laranjeiro, yvugenfi, ybendito

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Wednesday, September 22, 2021 5:58 PM
> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; amorenoz@redhat.com;
> david.marchand@redhat.com; andrew.rybchenko@oktetlabs.ru; Yigit, Ferruh
> <ferruh.yigit@intel.com>; michaelba@nvidia.com; viacheslavo@nvidia.com; Li,
> Xiaoyun <xiaoyun.li@intel.com>
> Cc: stable@dpdk.org; nelio.laranjeiro@6wind.com; yvugenfi@redhat.com;
> ybendito@redhat.com; Maxime Coquelin <maxime.coquelin@redhat.com>
> Subject: [PATCH v2 3/5] app/testpmd: fix RSS type display
> 
> This patch fixes the display of the RSS hash types
> configured in the port, which displayed "all" even
> if only a single type was configured
> 
> Fixes: 3c90743dd3b9 ("app/testpmd: support more types for flow RSS")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  app/test-pmd/config.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 611965769c..9a4a0c232b 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -2833,7 +2833,9 @@ port_rss_hash_conf_show(portid_t port_id, int
> show_rss_key)
>  	}
>  	printf("RSS functions:\n ");
>  	for (i = 0; rss_type_table[i].str; i++) {
> -		if (rss_hf & rss_type_table[i].rss_type)
> +		if (rss_type_table[i].rss_type == 0)
> +			continue;
> +		if ((rss_hf & rss_type_table[i].rss_type) ==
> rss_type_table[i].rss_type)
>  			printf("%s ", rss_type_table[i].str);
>  	}
>  	printf("\n");
> --
> 2.31.1

Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [PATCH v2 5/5] app/testpmd: add missing flow types in port info
  2021-09-22 11:31   ` Li, Xiaoyun
@ 2021-09-29  7:27     ` Maxime Coquelin
  0 siblings, 0 replies; 16+ messages in thread
From: Maxime Coquelin @ 2021-09-29  7:27 UTC (permalink / raw)
  To: Li, Xiaoyun, dev, Xia, Chenbo, amorenoz, david.marchand,
	andrew.rybchenko, Yigit, Ferruh, michaelba, viacheslavo
  Cc: stable, nelio.laranjeiro, yvugenfi, ybendito



On 9/22/21 13:31, Li, Xiaoyun wrote:
> Hi
> 
>> -----Original Message-----
>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Sent: Wednesday, September 22, 2021 17:58
>> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>;
>> amorenoz@redhat.com; david.marchand@redhat.com;
>> andrew.rybchenko@oktetlabs.ru; Yigit, Ferruh <ferruh.yigit@intel.com>;
>> michaelba@nvidia.com; viacheslavo@nvidia.com; Li, Xiaoyun
>> <xiaoyun.li@intel.com>
>> Cc: stable@dpdk.org; nelio.laranjeiro@6wind.com; yvugenfi@redhat.com;
>> ybendito@redhat.com; Maxime Coquelin <maxime.coquelin@redhat.com>
>> Subject: [PATCH v2 5/5] app/testpmd: add missing flow types in port info
>>
>> This patch adds missing IPv6-Ex flow types to port info command.
>>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>> ---
>>   app/test-pmd/config.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index
>> 9a4a0c232b..3550e0a18f 100644
>> --- a/app/test-pmd/config.c
>> +++ b/app/test-pmd/config.c
>> @@ -4528,6 +4528,9 @@ flowtype_to_str(uint16_t flow_type)
>>   		{"ipv6-sctp", RTE_ETH_FLOW_NONFRAG_IPV6_SCTP},
>>   		{"ipv6-other", RTE_ETH_FLOW_NONFRAG_IPV6_OTHER},
>>   		{"l2_payload", RTE_ETH_FLOW_L2_PAYLOAD},
>> +		{"ipv6-ex", RTE_ETH_FLOW_IPV6_EX},
>> +		{"ipv6-tcp-ex", RTE_ETH_FLOW_IPV6_TCP_EX},
>> +		{"ipv6-udp-ex", RTE_ETH_FLOW_IPV6_UDP_EX},
>>   		{"port", RTE_ETH_FLOW_PORT},
>>   		{"vxlan", RTE_ETH_FLOW_VXLAN},
>>   		{"geneve", RTE_ETH_FLOW_GENEVE},
> 
> You add missing ipv6 ex why not adding RTE_ETH_FLOW_GTPU too. It's also missing.
> 
> Also, flowtype_to_str is added but what about str2flowtype() in cmdline.c?

Both suggestions are added to the next revision.

Thanks,
Maxime

>> --
>> 2.31.1
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [PATCH v2 1/5] net/virtio: add initial RSS support
  2021-09-23  7:35   ` Xia, Chenbo
@ 2021-09-29  9:02     ` Maxime Coquelin
  0 siblings, 0 replies; 16+ messages in thread
From: Maxime Coquelin @ 2021-09-29  9:02 UTC (permalink / raw)
  To: Xia, Chenbo, dev, amorenoz, david.marchand, andrew.rybchenko,
	Yigit, Ferruh, michaelba, viacheslavo, Li, Xiaoyun
  Cc: stable, nelio.laranjeiro, yvugenfi, ybendito



On 9/23/21 09:35, Xia, Chenbo wrote:
> Hi Maxime,
> 
>> -----Original Message-----
>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Sent: Wednesday, September 22, 2021 5:58 PM
>> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; amorenoz@redhat.com;
>> david.marchand@redhat.com; andrew.rybchenko@oktetlabs.ru; Yigit, Ferruh
>> <ferruh.yigit@intel.com>; michaelba@nvidia.com; viacheslavo@nvidia.com; Li,
>> Xiaoyun <xiaoyun.li@intel.com>
>> Cc: stable@dpdk.org; nelio.laranjeiro@6wind.com; yvugenfi@redhat.com;
>> ybendito@redhat.com; Maxime Coquelin <maxime.coquelin@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.coquelin@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


^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2021-09-29  9:02 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-22  9:57 [dpdk-dev] [PATCH v2 0/5] Virtio PMD RSS support & RSS fixes Maxime Coquelin
2021-09-22  9:57 ` [dpdk-dev] [PATCH v2 1/5] net/virtio: add initial RSS support Maxime Coquelin
2021-09-22 11:20   ` Andrew Rybchenko
2021-09-22 12:28     ` Maxime Coquelin
2021-09-23  7:35   ` Xia, Chenbo
2021-09-29  9:02     ` Maxime Coquelin
2021-09-22  9:57 ` [dpdk-dev] [PATCH v2 2/5] app/testpmd: fix RSS key length Maxime Coquelin
2021-09-22 11:25   ` Li, Xiaoyun
2021-09-23  7:52   ` Xia, Chenbo
2021-09-22  9:57 ` [dpdk-dev] [PATCH v2 3/5] app/testpmd: fix RSS type display Maxime Coquelin
2021-09-22 12:01   ` Li, Xiaoyun
2021-09-23  7:59   ` Xia, Chenbo
2021-09-22  9:57 ` [dpdk-dev] [PATCH v2 4/5] net/mlx5: fix RSS RETA update Maxime Coquelin
2021-09-22  9:57 ` [dpdk-dev] [PATCH v2 5/5] app/testpmd: add missing flow types in port info Maxime Coquelin
2021-09-22 11:31   ` Li, Xiaoyun
2021-09-29  7:27     ` Maxime Coquelin

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).