DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 0/3] add support for RSS level
@ 2019-12-07  0:59 Ajit Khaparde
  2019-12-07  0:59 ` [dpdk-dev] [PATCH 1/3] ethdev: add RSS hash level Ajit Khaparde
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Ajit Khaparde @ 2019-12-07  0:59 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit

Some of NICs can allow a DPDK application to select the RSS level
to perform RSS hash on an incoming encapsulated packets. But it is
not possible to set this currently because rte_eth_rss_conf does
not have any field to indicate the RSS level while specifying RSS
configuration parameters.

This patchset extends rte_eth_rss_conf by adding rss_level.
An application can specify the RSS level based on the packet
encapsulation level and apply the setting to the specified device.

- rss_level of 0 requests the default behavior.
- rss_level of 1 requests RSS to be performed on the outermost packet
encapsulation level.
- rss_level of 2 and subsequent values request RSS to be performed
on the specified inner packet encapsulation level, from outermost to
innermost.

Patch 1/3 extends rte_ethdev.h to add this support.
Patch 2/3 has modifications to testpmd to use this feature.
Patch 3/3 has bnxt modifications to configure rss_level based on the
settings passed by the application.


Please review.


Ajit Khaparde (3):
  ethdev: add RSS hash level
  app/testpmd: support RSS hash level setting
  net/bnxt: add support to set RSS hash level

 app/test-pmd/cmdline.c         | 187 +++++++++++++++++++++++++++++++++
 app/test-pmd/config.c          |   7 ++
 app/test-pmd/parameters.c      |   6 +-
 app/test-pmd/testpmd.c         |   8 ++
 app/test-pmd/testpmd.h         |   1 +
 drivers/net/bnxt/bnxt.h        |   1 +
 drivers/net/bnxt/bnxt_ethdev.c |   7 ++
 drivers/net/bnxt/bnxt_hwrm.c   |   9 +-
 drivers/net/bnxt/bnxt_rxq.c    |   3 +
 drivers/net/bnxt/bnxt_vnic.c   |  75 +++++++++++++
 drivers/net/bnxt/bnxt_vnic.h   |   4 +
 lib/librte_ethdev/rte_ethdev.h |  27 +++++
 12 files changed, 332 insertions(+), 3 deletions(-)

-- 
2.21.0 (Apple Git-122.2)


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

* [dpdk-dev] [PATCH 1/3] ethdev: add RSS hash level
  2019-12-07  0:59 [dpdk-dev] [PATCH 0/3] add support for RSS level Ajit Khaparde
@ 2019-12-07  0:59 ` Ajit Khaparde
  2019-12-07  9:13   ` Andrew Rybchenko
                     ` (2 more replies)
  2019-12-07  0:59 ` [dpdk-dev] [PATCH 2/3] app/testpmd: support RSS hash level setting Ajit Khaparde
  2019-12-07  0:59 ` [dpdk-dev] [PATCH 3/3] net/bnxt: add support to set RSS hash level Ajit Khaparde
  2 siblings, 3 replies; 10+ messages in thread
From: Ajit Khaparde @ 2019-12-07  0:59 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit

This patch adds ability to configure RSS hash level in hardware.
This feature will allow an application to select RSS hash calculation
on outer or inner headers for tunneled packets.

Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 lib/librte_ethdev/rte_ethdev.h | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index 18a9defc2..5189bdbab 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -444,11 +444,35 @@ struct rte_vlan_filter_conf {
  * The *rss_hf* field of the *rss_conf* structure indicates the different
  * types of IPv4/IPv6 packets to which the RSS hashing must be applied.
  * Supplying an *rss_hf* equal to zero disables the RSS feature.
+ *
+ * The *rss_level* field of the *rss_conf* structure indicates the
+ * Packet encapsulation level RSS hash @p types apply to.
+ *
+ * - @p 0 requests the default behavior. Depending on the packet
+ *   type, it can mean outermost, innermost, anything in between or
+ *   even no RSS.
+ *
+ *   It basically stands for the innermost encapsulation level RSS
+ *   can be performed on according to PMD and device capabilities.
+ *
+ * - @p 1 requests RSS to be performed on the outermost packet
+ *   encapsulation level.
+ *
+ * - @p 2 and subsequent values request RSS to be performed on the
+ *   specified inner packet encapsulation level, from outermost to
+ *   innermost (lower to higher values).
+ *
+ * Support for values other than @p 0 is dependent on the underlying
+ * hardware in use.
+ *
+ * Requesting a specific RSS level on unrecognized traffic results
+ * in undefined behavior.
  */
 struct rte_eth_rss_conf {
 	uint8_t *rss_key;    /**< If not NULL, 40-byte hash key. */
 	uint8_t rss_key_len; /**< hash key length in bytes. */
 	uint64_t rss_hf;     /**< Hash functions to apply - see below. */
+	uint32_t rss_level;  /**< RSS hash level */
 };
 
 /*
@@ -599,6 +623,8 @@ rte_eth_rss_hf_refine(uint64_t rss_hf)
 	ETH_RSS_GENEVE | \
 	ETH_RSS_NVGRE)
 
+#define ETH_RSS_LEVEL_DEFAULT	0
+
 /*
  * Definitions used for redirection table entry size.
  * Some RSS RETA sizes may not be supported by some drivers, check the
@@ -1103,6 +1129,7 @@ struct rte_eth_conf {
 #define DEV_RX_OFFLOAD_SCTP_CKSUM	0x00020000
 #define DEV_RX_OFFLOAD_OUTER_UDP_CKSUM  0x00040000
 #define DEV_RX_OFFLOAD_RSS_HASH		0x00080000
+#define DEV_RX_OFFLOAD_RSS_LEVEL	0x00100000
 
 #define DEV_RX_OFFLOAD_CHECKSUM (DEV_RX_OFFLOAD_IPV4_CKSUM | \
 				 DEV_RX_OFFLOAD_UDP_CKSUM | \
-- 
2.21.0 (Apple Git-122.2)


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

* [dpdk-dev] [PATCH 2/3] app/testpmd: support RSS hash level setting
  2019-12-07  0:59 [dpdk-dev] [PATCH 0/3] add support for RSS level Ajit Khaparde
  2019-12-07  0:59 ` [dpdk-dev] [PATCH 1/3] ethdev: add RSS hash level Ajit Khaparde
@ 2019-12-07  0:59 ` Ajit Khaparde
  2019-12-07  0:59 ` [dpdk-dev] [PATCH 3/3] net/bnxt: add support to set RSS hash level Ajit Khaparde
  2 siblings, 0 replies; 10+ messages in thread
From: Ajit Khaparde @ 2019-12-07  0:59 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit

This patch adds support to configure RSS hash level for the device.

Example testpmd commands to set the RSS hash level using testpmd cli:

port config <port id> rss_level <value>
port config all rss_level <value>

Example command to set the RSS hash level using an argument to testpmd:

./build/app/testpmd -l1-4 -n2 -- -i --rxq=<N> --txq=<N> --rss-level=<N>

Example testpmd commands to get the RSS hash level:

show port <port id> rss-hash

Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 app/test-pmd/cmdline.c    | 187 ++++++++++++++++++++++++++++++++++++++
 app/test-pmd/config.c     |   7 ++
 app/test-pmd/parameters.c |   6 +-
 app/test-pmd/testpmd.c    |   8 ++
 app/test-pmd/testpmd.h    |   1 +
 5 files changed, 208 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 2d74df896..225ec3034 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -795,6 +795,9 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"port config port-id rss reta (hash,queue)[,(hash,queue)]\n"
 			"    Set the RSS redirection table.\n\n"
 
+			"port config (port_id|all) rss_level (value)\n"
+			"    Set the RSS hash level for the port.\n\n"
+
 			"port config (port_id) dcb vt (on|off) (traffic_class)"
 			" pfc (on|off)\n"
 			"    Set the DCB mode.\n\n"
@@ -2303,6 +2306,7 @@ cmd_config_rss_parsed(void *parsed_result,
 		return;
 	}
 	rss_conf.rss_key = NULL;
+	rss_conf.rss_level = rss_level;
 	/* Update global configuration for RSS types. */
 	RTE_ETH_FOREACH_DEV(i) {
 		struct rte_eth_rss_conf local_rss_conf;
@@ -2360,6 +2364,187 @@ cmdline_parse_inst_t cmd_config_rss = {
 	},
 };
 
+/* *** configure rss level all ports *** */
+struct cmd_config_rss_level {
+	cmdline_fixed_string_t port;
+	cmdline_fixed_string_t keyword;
+	cmdline_fixed_string_t all;
+	cmdline_fixed_string_t name;
+	uint32_t value;
+};
+
+static void
+cmd_config_rss_level_parsed(void *parsed_result,
+			__attribute__((unused)) struct cmdline *cl,
+			__attribute__((unused)) void *data)
+{
+	struct cmd_config_rss_level *res = parsed_result;
+	int all_updated = 1;
+	int diag;
+	uint16_t i;
+
+	/* Update global configuration for RSS hash level. */
+	RTE_ETH_FOREACH_DEV(i) {
+		struct rte_eth_rss_conf rss_conf;
+		struct rte_eth_dev_info dev_info;
+
+		diag = eth_dev_info_get_print_err(i, &dev_info);
+		if (diag != 0)
+			return;
+
+		if (!(dev_info.rx_offload_capa & DEV_RX_OFFLOAD_RSS_LEVEL)) {
+			printf("Port %d does not support RSS hash level "
+			       "selection\n", i);
+			return;
+		}
+
+		rss_conf.rss_key = NULL;
+		diag = rte_eth_dev_rss_hash_conf_get(i, &rss_conf);
+		if (diag) {
+			printf("Port %d failed to get RSS hash config with "
+			       "error (%d): %s.\n",
+			       i, -diag, strerror(-diag));
+			return;
+		}
+
+		if (rss_conf.rss_level == res->value)
+			return;
+
+		printf("Port %d modifying RSS hash level based on "
+		       "hardware support. Requested:%d Current:%d\n",
+			i, res->value, rss_conf.rss_level);
+
+		rss_conf.rss_level = res->value;
+		diag = rte_eth_dev_rss_hash_update(i, &rss_conf);
+		if (diag < 0) {
+			all_updated = 0;
+			printf("Configuration of RSS hash at ethernet port %d "
+				"failed with error (%d): %s.\n",
+				i, -diag, strerror(-diag));
+		}
+	}
+	if (all_updated)
+		rss_level = res->value;
+}
+
+cmdline_parse_token_string_t cmd_config_rss_level_port =
+	TOKEN_STRING_INITIALIZER(struct cmd_config_rss_level, port, "port");
+cmdline_parse_token_string_t cmd_config_rss_level_keyword =
+	TOKEN_STRING_INITIALIZER(struct cmd_config_rss_level, keyword,
+				 "config");
+cmdline_parse_token_string_t cmd_config_rss_level_all =
+	TOKEN_STRING_INITIALIZER(struct cmd_config_rss_level, all, "all");
+cmdline_parse_token_string_t cmd_config_rss_level_item =
+	TOKEN_STRING_INITIALIZER(struct cmd_config_rss_level, name,
+				 "rss_level");
+cmdline_parse_token_num_t cmd_config_rss_level_value =
+	TOKEN_NUM_INITIALIZER(struct cmd_config_rss_level, value, UINT16);
+
+cmdline_parse_inst_t cmd_config_rss_level = {
+	.f = cmd_config_rss_level_parsed,
+	.data = NULL,
+	.help_str = "port config all rss_level "
+		"<level>",
+	.tokens = {
+		(void *)&cmd_config_rss_level_port,
+		(void *)&cmd_config_rss_level_keyword,
+		(void *)&cmd_config_rss_level_all,
+		(void *)&cmd_config_rss_level_item,
+		(void *)&cmd_config_rss_level_value,
+		NULL,
+	},
+};
+
+/* *** configure rss_level for specific port *** */
+struct cmd_config_rss_level_specific_port {
+	cmdline_fixed_string_t port;
+	cmdline_fixed_string_t keyword;
+	portid_t id;
+	cmdline_fixed_string_t item;
+	uint32_t value;
+};
+
+static void
+cmd_config_rss_level_specific_port_parsed(void *parsed_result,
+				__attribute__((unused)) struct cmdline *cl,
+				__attribute__((unused)) void *data)
+{
+	struct cmd_config_rss_level_specific_port *res = parsed_result;
+	struct rte_eth_dev_info dev_info;
+	struct rte_eth_rss_conf rss_conf;
+	int diag;
+
+	if (port_id_is_invalid(res->id, ENABLED_WARN))
+		return;
+
+	diag = eth_dev_info_get_print_err(res->id, &dev_info);
+	if (diag != 0)
+		return;
+
+	if (!(dev_info.rx_offload_capa & DEV_RX_OFFLOAD_RSS_LEVEL)) {
+		printf("Port %d does not support RSS hash level "
+		       "selection\n", res->id);
+		return;
+	}
+
+	rss_conf.rss_key = NULL;
+	diag = rte_eth_dev_rss_hash_conf_get(res->id, &rss_conf);
+
+	if (rss_conf.rss_level == res->value)
+		return;
+
+	if (diag == 0) {
+		printf("Current RSS hash level %d is being updated to %d\n",
+		       rss_conf.rss_level, res->value);
+		rss_conf.rss_level = res->value;
+		diag = rte_eth_dev_rss_hash_update(res->id, &rss_conf);
+	}
+	if (diag == 0) {
+		rss_level = res->value;
+		return;
+	}
+
+	switch (diag) {
+	case -ENOTSUP:
+		printf("operation not supported by device\n");
+		break;
+	default:
+		printf("operation failed - diag=%d\n", diag);
+		break;
+	}
+}
+
+cmdline_parse_token_string_t cmd_config_rss_level_specific_port_port =
+	TOKEN_STRING_INITIALIZER(struct cmd_config_rss_level_specific_port,
+				 port, "port");
+cmdline_parse_token_string_t cmd_config_rss_level_specific_port_keyword =
+	TOKEN_STRING_INITIALIZER(struct cmd_config_rss_level_specific_port,
+				 keyword, "config");
+cmdline_parse_token_num_t cmd_config_rss_level_specific_port_id =
+	TOKEN_NUM_INITIALIZER(struct cmd_config_rss_level_specific_port, id,
+			      UINT16);
+cmdline_parse_token_string_t cmd_config_rss_level_specific_port_item =
+	TOKEN_STRING_INITIALIZER(struct cmd_config_rss_level_specific_port,
+				 item, "rss_level");
+cmdline_parse_token_num_t cmd_config_rss_level_specific_port_value =
+	TOKEN_NUM_INITIALIZER(struct cmd_config_rss_level_specific_port, value,
+			      UINT16);
+
+cmdline_parse_inst_t cmd_config_rss_level_specific_port = {
+	.f = cmd_config_rss_level_specific_port_parsed,
+	.data = NULL,
+	.help_str = "port config port_id rss_level "
+		"<level>",
+	.tokens = {
+		(void *)&cmd_config_rss_level_specific_port_port,
+		(void *)&cmd_config_rss_level_specific_port_keyword,
+		(void *)&cmd_config_rss_level_specific_port_id,
+		(void *)&cmd_config_rss_level_specific_port_item,
+		(void *)&cmd_config_rss_level_specific_port_value,
+		NULL,
+	},
+};
+
 /* *** configure rss hash key *** */
 struct cmd_config_rss_hash_key {
 	cmdline_fixed_string_t port;
@@ -19336,6 +19521,8 @@ cmdline_parse_ctx_t main_ctx[] = {
 	(cmdline_parse_inst_t *)&cmd_config_max_lro_pkt_size,
 	(cmdline_parse_inst_t *)&cmd_config_rx_mode_flag,
 	(cmdline_parse_inst_t *)&cmd_config_rss,
+	(cmdline_parse_inst_t *)&cmd_config_rss_level,
+	(cmdline_parse_inst_t *)&cmd_config_rss_level_specific_port,
 	(cmdline_parse_inst_t *)&cmd_config_rxtx_ring_size,
 	(cmdline_parse_inst_t *)&cmd_config_rxtx_queue,
 	(cmdline_parse_inst_t *)&cmd_config_deferred_start_rxtx_queue,
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 4e1c3cab5..87335d7a5 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1965,6 +1965,13 @@ port_rss_hash_conf_show(portid_t port_id, int show_rss_key)
 			printf("%s ", rss_type_table[i].str);
 	}
 	printf("\n");
+	if (dev_info.rx_offload_capa & DEV_RX_OFFLOAD_RSS_LEVEL) {
+		printf("RSS hash level:\n ");
+		if (rss_conf.rss_level == ETH_RSS_LEVEL_DEFAULT)
+			printf("%s ", "Default");
+		printf("%d", rss_conf.rss_level);
+		printf("\n");
+	}
 	if (!show_rss_key)
 		return;
 	printf("RSS key:\n");
diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index 2e7a50441..bac3b5a55 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -65,7 +65,7 @@ usage(char* progname)
 	       "--tx-ip=SRC,DST | --tx-udp=PORT | "
 #endif
 	       "--pkt-filter-mode= |"
-	       "--rss-ip | --rss-udp | "
+	       "--rss-ip | --rss-udp | --rss-level= | "
 	       "--rxpt= | --rxht= | --rxwt= | --rxfreet= | "
 	       "--txpt= | --txht= | --txwt= | --txfreet= | "
 	       "--txrst= | --tx-offloads= | | --rx-offloads= | "
@@ -147,6 +147,7 @@ usage(char* progname)
 	       list_pkt_forwarding_modes());
 	printf("  --rss-ip: set RSS functions to IPv4/IPv6 only .\n");
 	printf("  --rss-udp: set RSS functions to IPv4/IPv6 + UDP.\n");
+	printf("  --rss-level=N: set RSS hash level.\n");
 	printf("  --rxq=N: set the number of RX queues per port to N.\n");
 	printf("  --rxd=N: set the number of descriptors in RX rings to N.\n");
 	printf("  --txq=N: set the number of TX queues per port to N.\n");
@@ -623,6 +624,7 @@ launch_args_parse(int argc, char** argv)
 		{ "forward-mode",               1, 0, 0 },
 		{ "rss-ip",			0, 0, 0 },
 		{ "rss-udp",			0, 0, 0 },
+		{ "rss-level",			1, 0, 0 },
 		{ "rxq",			1, 0, 0 },
 		{ "txq",			1, 0, 0 },
 		{ "rxd",			1, 0, 0 },
@@ -1037,6 +1039,8 @@ launch_args_parse(int argc, char** argv)
 				rss_hf = ETH_RSS_IP;
 			if (!strcmp(lgopts[opt_idx].name, "rss-udp"))
 				rss_hf = ETH_RSS_UDP;
+			if (!strcmp(lgopts[opt_idx].name, "rss-level"))
+				rss_level = atoi(optarg);
 			if (!strcmp(lgopts[opt_idx].name, "rxq")) {
 				n = atoi(optarg);
 				if (n >= 0 && check_nb_rxq((queueid_t)n) == 0)
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index b37468223..37d6472c7 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -319,6 +319,11 @@ uint64_t noisy_lkup_num_reads_writes;
  */
 uint64_t rss_hf = ETH_RSS_IP; /* RSS IP by default. */
 
+/*
+ * Receive Side Scaling (RSS) configuration.
+ */
+uint32_t rss_level = ETH_RSS_LEVEL_DEFAULT;/* Use default hardware RSS level. */
+
 /*
  * Port topology configuration
  */
@@ -3068,9 +3073,12 @@ init_port_config(void)
 			port->dev_conf.rx_adv_conf.rss_conf.rss_key = NULL;
 			port->dev_conf.rx_adv_conf.rss_conf.rss_hf =
 				rss_hf & port->dev_info.flow_type_rss_offloads;
+			port->dev_conf.rx_adv_conf.rss_conf.rss_level =
+				rss_level;
 		} else {
 			port->dev_conf.rx_adv_conf.rss_conf.rss_key = NULL;
 			port->dev_conf.rx_adv_conf.rss_conf.rss_hf = 0;
+			port->dev_conf.rx_adv_conf.rss_conf.rss_level = 0;
 		}
 
 		if (port->dcb_flag == 0) {
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 857a11f8d..2578f6c1c 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -383,6 +383,7 @@ extern struct rte_eth_rxmode rx_mode;
 extern struct rte_eth_txmode tx_mode;
 
 extern uint64_t rss_hf;
+extern uint32_t rss_level;
 
 extern queueid_t nb_hairpinq;
 extern queueid_t nb_rxq;
-- 
2.21.0 (Apple Git-122.2)


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

* [dpdk-dev] [PATCH 3/3] net/bnxt: add support to set RSS hash level
  2019-12-07  0:59 [dpdk-dev] [PATCH 0/3] add support for RSS level Ajit Khaparde
  2019-12-07  0:59 ` [dpdk-dev] [PATCH 1/3] ethdev: add RSS hash level Ajit Khaparde
  2019-12-07  0:59 ` [dpdk-dev] [PATCH 2/3] app/testpmd: support RSS hash level setting Ajit Khaparde
@ 2019-12-07  0:59 ` Ajit Khaparde
  2 siblings, 0 replies; 10+ messages in thread
From: Ajit Khaparde @ 2019-12-07  0:59 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit

This patch adds support to configure RSS hash level in the hardware,
if the firmware advertises such a capability.

Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/bnxt.h        |  1 +
 drivers/net/bnxt/bnxt_ethdev.c |  7 ++++
 drivers/net/bnxt/bnxt_hwrm.c   |  9 +++-
 drivers/net/bnxt/bnxt_rxq.c    |  3 ++
 drivers/net/bnxt/bnxt_vnic.c   | 75 ++++++++++++++++++++++++++++++++++
 drivers/net/bnxt/bnxt_vnic.h   |  4 ++
 6 files changed, 97 insertions(+), 2 deletions(-)

diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h
index e259c8239..d796ac539 100644
--- a/drivers/net/bnxt/bnxt.h
+++ b/drivers/net/bnxt/bnxt.h
@@ -536,6 +536,7 @@ struct bnxt {
 
 	uint32_t		vnic_cap_flags;
 #define BNXT_VNIC_CAP_COS_CLASSIFY	BIT(0)
+#define BNXT_VNIC_CAP_OUTER_RSS		BIT(1)
 	unsigned int		rx_nr_rings;
 	unsigned int		rx_cp_nr_rings;
 	unsigned int		rx_num_qs_per_vnic;
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index c70b072bf..a12aecc26 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -524,6 +524,8 @@ static int bnxt_dev_info_get_op(struct rte_eth_dev *eth_dev,
 	dev_info->rx_offload_capa = BNXT_DEV_RX_OFFLOAD_SUPPORT;
 	if (bp->flags & BNXT_FLAG_PTP_SUPPORTED)
 		dev_info->rx_offload_capa |= DEV_RX_OFFLOAD_TIMESTAMP;
+	if (bp->vnic_cap_flags & BNXT_VNIC_CAP_OUTER_RSS)
+		dev_info->rx_offload_capa |= DEV_RX_OFFLOAD_RSS_LEVEL;
 	dev_info->tx_offload_capa = BNXT_DEV_TX_OFFLOAD_SUPPORT;
 	dev_info->flow_type_rss_offloads = BNXT_ETH_RSS_SUPPORT;
 
@@ -1404,6 +1406,8 @@ static int bnxt_rss_hash_update_op(struct rte_eth_dev *eth_dev,
 	/* Update the default RSS VNIC(s) */
 	vnic = BNXT_GET_DEFAULT_VNIC(bp);
 	vnic->hash_type = bnxt_rte_to_hwrm_hash_types(rss_conf->rss_hf);
+	vnic->hash_mode = bnxt_rte_to_hwrm_hash_level(bp, rss_conf->rss_level,
+						      rss_conf->rss_hf);
 
 	/*
 	 * If hashkey is not specified, use the previously configured
@@ -1438,6 +1442,9 @@ static int bnxt_rss_hash_conf_get_op(struct rte_eth_dev *eth_dev,
 
 	/* RSS configuration is the same for all VNICs */
 	if (vnic && vnic->rss_hash_key) {
+		rss_conf->rss_level =
+			bnxt_hwrm_to_rte_rss_level(bp, vnic->hash_mode);
+
 		if (rss_conf->rss_key) {
 			len = rss_conf->rss_key_len <= HW_HASH_KEY_SIZE ?
 			      rss_conf->rss_key_len : HW_HASH_KEY_SIZE;
diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index b1f908ee4..aba1ef34b 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -704,6 +704,7 @@ int bnxt_hwrm_func_qcaps(struct bnxt *bp)
 int bnxt_hwrm_vnic_qcaps(struct bnxt *bp)
 {
 	int rc = 0;
+	uint32_t flags;
 	struct hwrm_vnic_qcaps_input req = {.req_type = 0 };
 	struct hwrm_vnic_qcaps_output *resp = bp->hwrm_cmd_resp_addr;
 
@@ -715,12 +716,16 @@ int bnxt_hwrm_vnic_qcaps(struct bnxt *bp)
 
 	HWRM_CHECK_RESULT();
 
-	if (rte_le_to_cpu_32(resp->flags) &
-	    HWRM_VNIC_QCAPS_OUTPUT_FLAGS_COS_ASSIGNMENT_CAP) {
+	flags = rte_le_to_cpu_32(resp->flags);
+
+	if (flags & HWRM_VNIC_QCAPS_OUTPUT_FLAGS_COS_ASSIGNMENT_CAP) {
 		bp->vnic_cap_flags |= BNXT_VNIC_CAP_COS_CLASSIFY;
 		PMD_DRV_LOG(INFO, "CoS assignment capability enabled\n");
 	}
 
+	if (flags & HWRM_VNIC_QCAPS_OUTPUT_FLAGS_OUTERMOST_RSS_CAP)
+		bp->vnic_cap_flags |= BNXT_VNIC_CAP_OUTER_RSS;
+
 	bp->max_tpa_v2 = rte_le_to_cpu_16(resp->max_aggs_supported);
 
 	HWRM_UNLOCK();
diff --git a/drivers/net/bnxt/bnxt_rxq.c b/drivers/net/bnxt/bnxt_rxq.c
index 457ebede0..fabc4663c 100644
--- a/drivers/net/bnxt/bnxt_rxq.c
+++ b/drivers/net/bnxt/bnxt_rxq.c
@@ -177,6 +177,9 @@ int bnxt_mq_rx_configure(struct bnxt *bp)
 			vnic = &bp->vnic_info[i];
 			vnic->hash_type =
 				bnxt_rte_to_hwrm_hash_types(rss->rss_hf);
+			vnic->hash_mode =
+				bnxt_rte_to_hwrm_hash_level(bp, rss->rss_level,
+							    rss->rss_hf);
 
 			/*
 			 * Use the supplied key if the key length is
diff --git a/drivers/net/bnxt/bnxt_vnic.c b/drivers/net/bnxt/bnxt_vnic.c
index 104342e13..8aebb6069 100644
--- a/drivers/net/bnxt/bnxt_vnic.c
+++ b/drivers/net/bnxt/bnxt_vnic.c
@@ -261,3 +261,78 @@ uint16_t bnxt_rte_to_hwrm_hash_types(uint64_t rte_type)
 
 	return hwrm_type;
 }
+
+int bnxt_rte_to_hwrm_hash_level(struct bnxt *bp,
+				uint32_t rss_level,
+				uint64_t hash_f)
+{
+	int mode = HWRM_VNIC_RSS_CFG_INPUT_HASH_MODE_FLAGS_DEFAULT;
+	bool l3 = !!(hash_f & (ETH_RSS_IPV4 | ETH_RSS_IPV6));
+	bool l4 = !!(hash_f & (ETH_RSS_NONFRAG_IPV4_UDP |
+			       ETH_RSS_NONFRAG_IPV6_UDP |
+			       ETH_RSS_NONFRAG_IPV4_TCP |
+			       ETH_RSS_NONFRAG_IPV6_TCP));
+	bool l3_only = l3 && !l4;
+	bool l3_and_l4 = l3 && l4;
+
+	/* If FW has not advertised capability to configure outer/inner
+	 * RSS hashing , just log a message. HW will work in default RSS mode.
+	 */
+	if (!(bp->vnic_cap_flags & BNXT_VNIC_CAP_OUTER_RSS)) {
+		PMD_DRV_LOG(ERR, "RSS hash level cannot be configured\n");
+		return mode;
+	}
+
+	switch (rss_level) {
+	case 0:
+		mode = HWRM_VNIC_RSS_CFG_INPUT_HASH_MODE_FLAGS_DEFAULT;
+		break;
+	case 1:
+		if (l3_only)
+			mode =
+			HWRM_VNIC_RSS_CFG_INPUT_HASH_MODE_FLAGS_OUTERMOST_2;
+		else if (l3_and_l4)
+			mode =
+			HWRM_VNIC_RSS_CFG_INPUT_HASH_MODE_FLAGS_OUTERMOST_4;
+		break;
+	case 2:
+		if (l3_only)
+			mode =
+			HWRM_VNIC_RSS_CFG_INPUT_HASH_MODE_FLAGS_INNERMOST_2;
+		else if (l3_and_l4)
+			mode =
+			HWRM_VNIC_RSS_CFG_INPUT_HASH_MODE_FLAGS_INNERMOST_4;
+		break;
+	default:
+		PMD_DRV_LOG(ERR,
+			    "No support for RSS hash level %d. Using default\n",
+			    rss_level);
+		break;
+	}
+
+	return mode;
+}
+
+int bnxt_hwrm_to_rte_rss_level(struct bnxt *bp, uint32_t hash_mode)
+{
+	int rss_level;
+
+	if (hash_mode == HWRM_VNIC_RSS_CFG_INPUT_HASH_MODE_FLAGS_OUTERMOST_2 ||
+	    hash_mode == HWRM_VNIC_RSS_CFG_INPUT_HASH_MODE_FLAGS_OUTERMOST_4)
+		rss_level = 1;
+	else if (hash_mode ==
+		 HWRM_VNIC_RSS_CFG_INPUT_HASH_MODE_FLAGS_INNERMOST_2 ||
+		 hash_mode ==
+		 HWRM_VNIC_RSS_CFG_INPUT_HASH_MODE_FLAGS_INNERMOST_4)
+		rss_level = 2;
+	else
+		rss_level = 0;
+
+	/* If FW has not advertised capability to configure inner/outer RSS
+	 * return default hash mode.
+	 */
+	if (!(bp->vnic_cap_flags & BNXT_VNIC_CAP_OUTER_RSS))
+		rss_level = 0;
+
+	return rss_level;
+}
diff --git a/drivers/net/bnxt/bnxt_vnic.h b/drivers/net/bnxt/bnxt_vnic.h
index a372b899b..48445b026 100644
--- a/drivers/net/bnxt/bnxt_vnic.h
+++ b/drivers/net/bnxt/bnxt_vnic.h
@@ -69,4 +69,8 @@ int bnxt_alloc_vnic_mem(struct bnxt *bp);
 int bnxt_vnic_grp_alloc(struct bnxt *bp, struct bnxt_vnic_info *vnic);
 void prandom_bytes(void *dest_ptr, size_t len);
 uint16_t bnxt_rte_to_hwrm_hash_types(uint64_t rte_type);
+int bnxt_hwrm_to_rte_rss_level(struct bnxt *bp, uint32_t hash_mode);
+int bnxt_rte_to_hwrm_hash_level(struct bnxt *bp,
+				uint32_t rss_level,
+				uint64_t hash_f);
 #endif
-- 
2.21.0 (Apple Git-122.2)


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

* Re: [dpdk-dev] [PATCH 1/3] ethdev: add RSS hash level
  2019-12-07  0:59 ` [dpdk-dev] [PATCH 1/3] ethdev: add RSS hash level Ajit Khaparde
@ 2019-12-07  9:13   ` Andrew Rybchenko
  2019-12-07 19:56     ` Ajit Khaparde
  2019-12-07 22:27   ` Stephen Hemminger
  2019-12-08 18:02   ` Ananyev, Konstantin
  2 siblings, 1 reply; 10+ messages in thread
From: Andrew Rybchenko @ 2019-12-07  9:13 UTC (permalink / raw)
  To: Ajit Khaparde, dev; +Cc: ferruh.yigit, Thomas Monjalon

On 12/7/19 3:59 AM, Ajit Khaparde wrote:
> This patch adds ability to configure RSS hash level in hardware.
> This feature will allow an application to select RSS hash calculation
> on outer or inner headers for tunneled packets.
> 
> Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> ---
>   lib/librte_ethdev/rte_ethdev.h | 27 +++++++++++++++++++++++++++
>   1 file changed, 27 insertions(+)
> 
> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
> index 18a9defc2..5189bdbab 100644
> --- a/lib/librte_ethdev/rte_ethdev.h
> +++ b/lib/librte_ethdev/rte_ethdev.h
> @@ -444,11 +444,35 @@ struct rte_vlan_filter_conf {
>    * The *rss_hf* field of the *rss_conf* structure indicates the different
>    * types of IPv4/IPv6 packets to which the RSS hashing must be applied.
>    * Supplying an *rss_hf* equal to zero disables the RSS feature.
> + *
> + * The *rss_level* field of the *rss_conf* structure indicates the
> + * Packet encapsulation level RSS hash @p types apply to.
> + *
> + * - @p 0 requests the default behavior. Depending on the packet
> + *   type, it can mean outermost, innermost, anything in between or
> + *   even no RSS.
> + *
> + *   It basically stands for the innermost encapsulation level RSS
> + *   can be performed on according to PMD and device capabilities.
> + *
> + * - @p 1 requests RSS to be performed on the outermost packet
> + *   encapsulation level.
> + *
> + * - @p 2 and subsequent values request RSS to be performed on the
> + *   specified inner packet encapsulation level, from outermost to
> + *   innermost (lower to higher values).
> + *
> + * Support for values other than @p 0 is dependent on the underlying
> + * hardware in use.
> + *
> + * Requesting a specific RSS level on unrecognized traffic results
> + * in undefined behavior.
>    */
>   struct rte_eth_rss_conf {
>   	uint8_t *rss_key;    /**< If not NULL, 40-byte hash key. */
>   	uint8_t rss_key_len; /**< hash key length in bytes. */
>   	uint64_t rss_hf;     /**< Hash functions to apply - see below. */
> +	uint32_t rss_level;  /**< RSS hash level */
>   };

I'm not sure that offload flag is required in this case.
I think maximum supported rss_level in dev_info will provide
more information and per-queue level does not make sense
in this case. Even if per-queue group control is required,
it should be doable via rte_flow API RSS action.

Anyway, it looks like it is ABI breakage with all consequences.
In 64-bit case it is possible to put it before rss_hf to avoid
ABI breakage, but it will break ABI on 32-bit anyway.

>   /*
> @@ -599,6 +623,8 @@ rte_eth_rss_hf_refine(uint64_t rss_hf)
>   	ETH_RSS_GENEVE | \
>   	ETH_RSS_NVGRE)
>   
> +#define ETH_RSS_LEVEL_DEFAULT	0
> +
>   /*
>    * Definitions used for redirection table entry size.
>    * Some RSS RETA sizes may not be supported by some drivers, check the
> @@ -1103,6 +1129,7 @@ struct rte_eth_conf {
>   #define DEV_RX_OFFLOAD_SCTP_CKSUM	0x00020000
>   #define DEV_RX_OFFLOAD_OUTER_UDP_CKSUM  0x00040000
>   #define DEV_RX_OFFLOAD_RSS_HASH		0x00080000
> +#define DEV_RX_OFFLOAD_RSS_LEVEL	0x00100000
>   
>   #define DEV_RX_OFFLOAD_CHECKSUM (DEV_RX_OFFLOAD_IPV4_CKSUM | \
>   				 DEV_RX_OFFLOAD_UDP_CKSUM | \
> 


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

* Re: [dpdk-dev] [PATCH 1/3] ethdev: add RSS hash level
  2019-12-07  9:13   ` Andrew Rybchenko
@ 2019-12-07 19:56     ` Ajit Khaparde
  2019-12-09  7:35       ` Andrew Rybchenko
  0 siblings, 1 reply; 10+ messages in thread
From: Ajit Khaparde @ 2019-12-07 19:56 UTC (permalink / raw)
  To: Andrew Rybchenko; +Cc: dpdk-dev, Ferruh Yigit, Thomas Monjalon

On Sat, Dec 7, 2019 at 1:14 AM Andrew Rybchenko <arybchenko@solarflare.com>
wrote:

> On 12/7/19 3:59 AM, Ajit Khaparde wrote:
> > This patch adds ability to configure RSS hash level in hardware.
> > This feature will allow an application to select RSS hash calculation
> > on outer or inner headers for tunneled packets.
> >
> > Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> > ---
> >   lib/librte_ethdev/rte_ethdev.h | 27 +++++++++++++++++++++++++++
> >   1 file changed, 27 insertions(+)
> >
> > diff --git a/lib/librte_ethdev/rte_ethdev.h
> b/lib/librte_ethdev/rte_ethdev.h
> > index 18a9defc2..5189bdbab 100644
> > --- a/lib/librte_ethdev/rte_ethdev.h
> > +++ b/lib/librte_ethdev/rte_ethdev.h
> > @@ -444,11 +444,35 @@ struct rte_vlan_filter_conf {
> >    * The *rss_hf* field of the *rss_conf* structure indicates the
> different
> >    * types of IPv4/IPv6 packets to which the RSS hashing must be applied.
> >    * Supplying an *rss_hf* equal to zero disables the RSS feature.
> > + *
> > + * The *rss_level* field of the *rss_conf* structure indicates the
> > + * Packet encapsulation level RSS hash @p types apply to.
> > + *
> > + * - @p 0 requests the default behavior. Depending on the packet
> > + *   type, it can mean outermost, innermost, anything in between or
> > + *   even no RSS.
> > + *
> > + *   It basically stands for the innermost encapsulation level RSS
> > + *   can be performed on according to PMD and device capabilities.
> > + *
> > + * - @p 1 requests RSS to be performed on the outermost packet
> > + *   encapsulation level.
> > + *
> > + * - @p 2 and subsequent values request RSS to be performed on the
> > + *   specified inner packet encapsulation level, from outermost to
> > + *   innermost (lower to higher values).
> > + *
> > + * Support for values other than @p 0 is dependent on the underlying
> > + * hardware in use.
> > + *
> > + * Requesting a specific RSS level on unrecognized traffic results
> > + * in undefined behavior.
> >    */
> >   struct rte_eth_rss_conf {
> >       uint8_t *rss_key;    /**< If not NULL, 40-byte hash key. */
> >       uint8_t rss_key_len; /**< hash key length in bytes. */
> >       uint64_t rss_hf;     /**< Hash functions to apply - see below. */
> > +     uint32_t rss_level;  /**< RSS hash level */
> >   };
>
> I'm not sure that offload flag is required in this case.
> I think maximum supported rss_level in dev_info will provide

more information and per-queue level does not make sense
> in this case. Even if per-queue group control is required,
> it should be doable via rte_flow API RSS action.
>
This is dev config and not flow specific configuration. Ofcourse while
passing
the rss_config, not all the queues may be specified, but that is not a new
behavior and it is upto the application anyway.

Are we transitioning the device level configuration to rte_flow/flow
based scheme?


>
> Anyway, it looks like it is ABI breakage with all consequences.
> In 64-bit case it is possible to put it before rss_hf to avoid
> ABI breakage, but it will break ABI on 32-bit anyway.
>
Right.
I sent the proposal for review early to get it cleaned up and ready
when the window opens.


>
> >   /*
> > @@ -599,6 +623,8 @@ rte_eth_rss_hf_refine(uint64_t rss_hf)
> >       ETH_RSS_GENEVE | \
> >       ETH_RSS_NVGRE)
> >
> > +#define ETH_RSS_LEVEL_DEFAULT        0
> > +
> >   /*
> >    * Definitions used for redirection table entry size.
> >    * Some RSS RETA sizes may not be supported by some drivers, check the
> > @@ -1103,6 +1129,7 @@ struct rte_eth_conf {
> >   #define DEV_RX_OFFLOAD_SCTP_CKSUM   0x00020000
> >   #define DEV_RX_OFFLOAD_OUTER_UDP_CKSUM  0x00040000
> >   #define DEV_RX_OFFLOAD_RSS_HASH             0x00080000
> > +#define DEV_RX_OFFLOAD_RSS_LEVEL     0x00100000
> >
> >   #define DEV_RX_OFFLOAD_CHECKSUM (DEV_RX_OFFLOAD_IPV4_CKSUM | \
> >                                DEV_RX_OFFLOAD_UDP_CKSUM | \
> >
>
>

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

* Re: [dpdk-dev] [PATCH 1/3] ethdev: add RSS hash level
  2019-12-07  0:59 ` [dpdk-dev] [PATCH 1/3] ethdev: add RSS hash level Ajit Khaparde
  2019-12-07  9:13   ` Andrew Rybchenko
@ 2019-12-07 22:27   ` Stephen Hemminger
  2019-12-08 18:02   ` Ananyev, Konstantin
  2 siblings, 0 replies; 10+ messages in thread
From: Stephen Hemminger @ 2019-12-07 22:27 UTC (permalink / raw)
  To: Ajit Khaparde; +Cc: dev, ferruh.yigit

On Fri,  6 Dec 2019 16:59:17 -0800
Ajit Khaparde <ajit.khaparde@broadcom.com> wrote:

>   */
>  struct rte_eth_rss_conf {
>  	uint8_t *rss_key;    /**< If not NULL, 40-byte hash key. */
>  	uint8_t rss_key_len; /**< hash key length in bytes. */
>  	uint64_t rss_hf;     /**< Hash functions to apply - see below. */
> +	uint32_t rss_level;  /**< RSS hash level */
>  };
>  


This is an API/ABI change which is not allowed per current policy.
API/ABI is frozen since 19.11 until the DPDK 20.11 release

You need to figure out another way to do this.

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

* Re: [dpdk-dev] [PATCH 1/3] ethdev: add RSS hash level
  2019-12-07  0:59 ` [dpdk-dev] [PATCH 1/3] ethdev: add RSS hash level Ajit Khaparde
  2019-12-07  9:13   ` Andrew Rybchenko
  2019-12-07 22:27   ` Stephen Hemminger
@ 2019-12-08 18:02   ` Ananyev, Konstantin
  2 siblings, 0 replies; 10+ messages in thread
From: Ananyev, Konstantin @ 2019-12-08 18:02 UTC (permalink / raw)
  To: Ajit Khaparde, dev; +Cc: Yigit, Ferruh

Hi Ajit,

> This patch adds ability to configure RSS hash level in hardware.
> This feature will allow an application to select RSS hash calculation
> on outer or inner headers for tunneled packets.
> 
> Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> ---
>  lib/librte_ethdev/rte_ethdev.h | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
> index 18a9defc2..5189bdbab 100644
> --- a/lib/librte_ethdev/rte_ethdev.h
> +++ b/lib/librte_ethdev/rte_ethdev.h
> @@ -444,11 +444,35 @@ struct rte_vlan_filter_conf {
>   * The *rss_hf* field of the *rss_conf* structure indicates the different
>   * types of IPv4/IPv6 packets to which the RSS hashing must be applied.
>   * Supplying an *rss_hf* equal to zero disables the RSS feature.
> + *
> + * The *rss_level* field of the *rss_conf* structure indicates the
> + * Packet encapsulation level RSS hash @p types apply to.
> + *
> + * - @p 0 requests the default behavior. Depending on the packet
> + *   type, it can mean outermost, innermost, anything in between or
> + *   even no RSS.
> + *
> + *   It basically stands for the innermost encapsulation level RSS
> + *   can be performed on according to PMD and device capabilities.
> + *
> + * - @p 1 requests RSS to be performed on the outermost packet
> + *   encapsulation level.
> + *
> + * - @p 2 and subsequent values request RSS to be performed on the
> + *   specified inner packet encapsulation level, from outermost to
> + *   innermost (lower to higher values).

If it is just to add ability to do rss over inner header, then do we really need
all this concept of 'levels'?
Might be just:

#define DEV_RX_OFFLOAD_RSS_HASH		0x00080000
+#define DEV_RX_OFFLOAD_RSS_HASH_INNER	0x00100000 

and uint8_t in rte_eth_rss_conf (between key_len and rss_hf) to indicate
inner/outer?

> + *
> + * Support for values other than @p 0 is dependent on the underlying
> + * hardware in use.
> + *
> + * Requesting a specific RSS level on unrecognized traffic results
> + * in undefined behavior.
>   */
>  struct rte_eth_rss_conf {
>  	uint8_t *rss_key;    /**< If not NULL, 40-byte hash key. */
>  	uint8_t rss_key_len; /**< hash key length in bytes. */
>  	uint64_t rss_hf;     /**< Hash functions to apply - see below. */
> +	uint32_t rss_level;  /**< RSS hash level */
>  };
> 
>  /*
> @@ -599,6 +623,8 @@ rte_eth_rss_hf_refine(uint64_t rss_hf)
>  	ETH_RSS_GENEVE | \
>  	ETH_RSS_NVGRE)
> 
> +#define ETH_RSS_LEVEL_DEFAULT	0
> +
>  /*
>   * Definitions used for redirection table entry size.
>   * Some RSS RETA sizes may not be supported by some drivers, check the
> @@ -1103,6 +1129,7 @@ struct rte_eth_conf {
>  #define DEV_RX_OFFLOAD_SCTP_CKSUM	0x00020000
>  #define DEV_RX_OFFLOAD_OUTER_UDP_CKSUM  0x00040000
>  #define DEV_RX_OFFLOAD_RSS_HASH		0x00080000
> +#define DEV_RX_OFFLOAD_RSS_LEVEL	0x00100000
> 
>  #define DEV_RX_OFFLOAD_CHECKSUM (DEV_RX_OFFLOAD_IPV4_CKSUM | \
>  				 DEV_RX_OFFLOAD_UDP_CKSUM | \
> --
> 2.21.0 (Apple Git-122.2)


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

* Re: [dpdk-dev] [PATCH 1/3] ethdev: add RSS hash level
  2019-12-07 19:56     ` Ajit Khaparde
@ 2019-12-09  7:35       ` Andrew Rybchenko
  2019-12-09 14:41         ` Ferruh Yigit
  0 siblings, 1 reply; 10+ messages in thread
From: Andrew Rybchenko @ 2019-12-09  7:35 UTC (permalink / raw)
  To: Ajit Khaparde; +Cc: dpdk-dev, Ferruh Yigit, Thomas Monjalon

On 12/7/19 10:56 PM, Ajit Khaparde wrote:
> On Sat, Dec 7, 2019 at 1:14 AM Andrew Rybchenko <arybchenko@solarflare.com>
> wrote:
> 
>> On 12/7/19 3:59 AM, Ajit Khaparde wrote:
>>> This patch adds ability to configure RSS hash level in hardware.
>>> This feature will allow an application to select RSS hash calculation
>>> on outer or inner headers for tunneled packets.
>>>
>>> Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
>>> ---
>>>   lib/librte_ethdev/rte_ethdev.h | 27 +++++++++++++++++++++++++++
>>>   1 file changed, 27 insertions(+)
>>>
>>> diff --git a/lib/librte_ethdev/rte_ethdev.h
>> b/lib/librte_ethdev/rte_ethdev.h
>>> index 18a9defc2..5189bdbab 100644
>>> --- a/lib/librte_ethdev/rte_ethdev.h
>>> +++ b/lib/librte_ethdev/rte_ethdev.h
>>> @@ -444,11 +444,35 @@ struct rte_vlan_filter_conf {
>>>    * The *rss_hf* field of the *rss_conf* structure indicates the
>> different
>>>    * types of IPv4/IPv6 packets to which the RSS hashing must be applied.
>>>    * Supplying an *rss_hf* equal to zero disables the RSS feature.
>>> + *
>>> + * The *rss_level* field of the *rss_conf* structure indicates the
>>> + * Packet encapsulation level RSS hash @p types apply to.
>>> + *
>>> + * - @p 0 requests the default behavior. Depending on the packet
>>> + *   type, it can mean outermost, innermost, anything in between or
>>> + *   even no RSS.
>>> + *
>>> + *   It basically stands for the innermost encapsulation level RSS
>>> + *   can be performed on according to PMD and device capabilities.
>>> + *
>>> + * - @p 1 requests RSS to be performed on the outermost packet
>>> + *   encapsulation level.
>>> + *
>>> + * - @p 2 and subsequent values request RSS to be performed on the
>>> + *   specified inner packet encapsulation level, from outermost to
>>> + *   innermost (lower to higher values).
>>> + *
>>> + * Support for values other than @p 0 is dependent on the underlying
>>> + * hardware in use.
>>> + *
>>> + * Requesting a specific RSS level on unrecognized traffic results
>>> + * in undefined behavior.
>>>    */
>>>   struct rte_eth_rss_conf {
>>>       uint8_t *rss_key;    /**< If not NULL, 40-byte hash key. */
>>>       uint8_t rss_key_len; /**< hash key length in bytes. */
>>>       uint64_t rss_hf;     /**< Hash functions to apply - see below. */
>>> +     uint32_t rss_level;  /**< RSS hash level */
>>>   };
>>
>> I'm not sure that offload flag is required in this case.
>> I think maximum supported rss_level in dev_info will provide
> 
> more information and per-queue level does not make sense

I think information about maximum RSS hash level could be
useful. At least it provides clear information about
limitations instead of attempt to configure with RSS level
equal to 3 and getting error without clear understanding why
if the level is not supported.

>> in this case. Even if per-queue group control is required,
>> it should be doable via rte_flow API RSS action.
>>
> This is dev config and not flow specific configuration. Ofcourse while
> passing
> the rss_config, not all the queues may be specified, but that is not a new
> behavior and it is upto the application anyway.

Yes, of course. I was just trying to explain why Rx offload is
not required and it should be just dev_info field with maximum
RSS level supported.

> Are we transitioning the device level configuration to rte_flow/flow
> based scheme?

No-no, see above.

>> Anyway, it looks like it is ABI breakage with all consequences.
>> In 64-bit case it is possible to put it before rss_hf to avoid
>> ABI breakage, but it will break ABI on 32-bit anyway.
>>
> Right.
> I sent the proposal for review early to get it cleaned up and ready
> when the window opens.

OK, good.

>>
>>>   /*
>>> @@ -599,6 +623,8 @@ rte_eth_rss_hf_refine(uint64_t rss_hf)
>>>       ETH_RSS_GENEVE | \
>>>       ETH_RSS_NVGRE)
>>>
>>> +#define ETH_RSS_LEVEL_DEFAULT        0
>>> +
>>>   /*
>>>    * Definitions used for redirection table entry size.
>>>    * Some RSS RETA sizes may not be supported by some drivers, check the
>>> @@ -1103,6 +1129,7 @@ struct rte_eth_conf {
>>>   #define DEV_RX_OFFLOAD_SCTP_CKSUM   0x00020000
>>>   #define DEV_RX_OFFLOAD_OUTER_UDP_CKSUM  0x00040000
>>>   #define DEV_RX_OFFLOAD_RSS_HASH             0x00080000
>>> +#define DEV_RX_OFFLOAD_RSS_LEVEL     0x00100000
>>>
>>>   #define DEV_RX_OFFLOAD_CHECKSUM (DEV_RX_OFFLOAD_IPV4_CKSUM | \
>>>                                DEV_RX_OFFLOAD_UDP_CKSUM | \
>>>
>>
>>


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

* Re: [dpdk-dev] [PATCH 1/3] ethdev: add RSS hash level
  2019-12-09  7:35       ` Andrew Rybchenko
@ 2019-12-09 14:41         ` Ferruh Yigit
  0 siblings, 0 replies; 10+ messages in thread
From: Ferruh Yigit @ 2019-12-09 14:41 UTC (permalink / raw)
  To: Andrew Rybchenko, Ajit Khaparde
  Cc: dpdk-dev, Thomas Monjalon, Konstantin Ananyev

On 12/9/2019 7:35 AM, Andrew Rybchenko wrote:
> On 12/7/19 10:56 PM, Ajit Khaparde wrote:
>> On Sat, Dec 7, 2019 at 1:14 AM Andrew Rybchenko <arybchenko@solarflare.com>
>> wrote:
>>
>>> On 12/7/19 3:59 AM, Ajit Khaparde wrote:
>>>> This patch adds ability to configure RSS hash level in hardware.
>>>> This feature will allow an application to select RSS hash calculation
>>>> on outer or inner headers for tunneled packets.
>>>>
>>>> Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
>>>> ---
>>>>   lib/librte_ethdev/rte_ethdev.h | 27 +++++++++++++++++++++++++++
>>>>   1 file changed, 27 insertions(+)
>>>>
>>>> diff --git a/lib/librte_ethdev/rte_ethdev.h
>>> b/lib/librte_ethdev/rte_ethdev.h
>>>> index 18a9defc2..5189bdbab 100644
>>>> --- a/lib/librte_ethdev/rte_ethdev.h
>>>> +++ b/lib/librte_ethdev/rte_ethdev.h
>>>> @@ -444,11 +444,35 @@ struct rte_vlan_filter_conf {
>>>>    * The *rss_hf* field of the *rss_conf* structure indicates the
>>> different
>>>>    * types of IPv4/IPv6 packets to which the RSS hashing must be applied.
>>>>    * Supplying an *rss_hf* equal to zero disables the RSS feature.
>>>> + *
>>>> + * The *rss_level* field of the *rss_conf* structure indicates the
>>>> + * Packet encapsulation level RSS hash @p types apply to.
>>>> + *
>>>> + * - @p 0 requests the default behavior. Depending on the packet
>>>> + *   type, it can mean outermost, innermost, anything in between or
>>>> + *   even no RSS.
>>>> + *
>>>> + *   It basically stands for the innermost encapsulation level RSS
>>>> + *   can be performed on according to PMD and device capabilities.
>>>> + *
>>>> + * - @p 1 requests RSS to be performed on the outermost packet
>>>> + *   encapsulation level.
>>>> + *
>>>> + * - @p 2 and subsequent values request RSS to be performed on the
>>>> + *   specified inner packet encapsulation level, from outermost to
>>>> + *   innermost (lower to higher values).
>>>> + *
>>>> + * Support for values other than @p 0 is dependent on the underlying
>>>> + * hardware in use.
>>>> + *
>>>> + * Requesting a specific RSS level on unrecognized traffic results
>>>> + * in undefined behavior.
>>>>    */
>>>>   struct rte_eth_rss_conf {
>>>>       uint8_t *rss_key;    /**< If not NULL, 40-byte hash key. */
>>>>       uint8_t rss_key_len; /**< hash key length in bytes. */
>>>>       uint64_t rss_hf;     /**< Hash functions to apply - see below. */
>>>> +     uint32_t rss_level;  /**< RSS hash level */
>>>>   };
>>>
>>> I'm not sure that offload flag is required in this case.
>>> I think maximum supported rss_level in dev_info will provide
>>
>> more information and per-queue level does not make sense
> 
> I think information about maximum RSS hash level could be
> useful. At least it provides clear information about
> limitations instead of attempt to configure with RSS level
> equal to 3 and getting error without clear understanding why
> if the level is not supported.

+1 to not have 'DEV_RX_OFFLOAD_RSS_LEVEL' offload flag.

The existing 'DEV_RX_OFFLOAD_RSS_HASH' offload flag is just to enabling copying
calculated hash value to mbuf field, RSS configuration still done via the
configure API and related config structs.
Since there is single field in mbuf for rss hash, no need additional offload flags.

But I think we can have new field in rss config, and Andrew's suggestion to have
it in dev_info looks reasonable.

As Konstantin's point, do we really have multiple level, or just inner/outer?
If it is just inner/outer perhaps we can simplify the config/logic to enums
instead of 32bit rss levels?


> 
>>> in this case. Even if per-queue group control is required,
>>> it should be doable via rte_flow API RSS action.
>>>
>> This is dev config and not flow specific configuration. Ofcourse while
>> passing
>> the rss_config, not all the queues may be specified, but that is not a new
>> behavior and it is upto the application anyway.
> 
> Yes, of course. I was just trying to explain why Rx offload is
> not required and it should be just dev_info field with maximum
> RSS level supported.
> 
>> Are we transitioning the device level configuration to rte_flow/flow
>> based scheme?
> 
> No-no, see above.
> 
>>> Anyway, it looks like it is ABI breakage with all consequences.
>>> In 64-bit case it is possible to put it before rss_hf to avoid
>>> ABI breakage, but it will break ABI on 32-bit anyway.
>>>
>> Right.
>> I sent the proposal for review early to get it cleaned up and ready
>> when the window opens.
> 
> OK, good.

Just to reminder that unless something changes that window is 20.11, so there is
quite a lot of time for it.

We can clarify with Thomas, but as far as I understand we still need to update
(and ack) the deprecation notices for possible ABI breakages for next breakage
window.

> 
>>>
>>>>   /*
>>>> @@ -599,6 +623,8 @@ rte_eth_rss_hf_refine(uint64_t rss_hf)
>>>>       ETH_RSS_GENEVE | \
>>>>       ETH_RSS_NVGRE)
>>>>
>>>> +#define ETH_RSS_LEVEL_DEFAULT        0
>>>> +
>>>>   /*
>>>>    * Definitions used for redirection table entry size.
>>>>    * Some RSS RETA sizes may not be supported by some drivers, check the
>>>> @@ -1103,6 +1129,7 @@ struct rte_eth_conf {
>>>>   #define DEV_RX_OFFLOAD_SCTP_CKSUM   0x00020000
>>>>   #define DEV_RX_OFFLOAD_OUTER_UDP_CKSUM  0x00040000
>>>>   #define DEV_RX_OFFLOAD_RSS_HASH             0x00080000
>>>> +#define DEV_RX_OFFLOAD_RSS_LEVEL     0x00100000
>>>>
>>>>   #define DEV_RX_OFFLOAD_CHECKSUM (DEV_RX_OFFLOAD_IPV4_CKSUM | \
>>>>                                DEV_RX_OFFLOAD_UDP_CKSUM | \
>>>>
>>>
>>>
> 


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

end of thread, other threads:[~2019-12-09 14:41 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-07  0:59 [dpdk-dev] [PATCH 0/3] add support for RSS level Ajit Khaparde
2019-12-07  0:59 ` [dpdk-dev] [PATCH 1/3] ethdev: add RSS hash level Ajit Khaparde
2019-12-07  9:13   ` Andrew Rybchenko
2019-12-07 19:56     ` Ajit Khaparde
2019-12-09  7:35       ` Andrew Rybchenko
2019-12-09 14:41         ` Ferruh Yigit
2019-12-07 22:27   ` Stephen Hemminger
2019-12-08 18:02   ` Ananyev, Konstantin
2019-12-07  0:59 ` [dpdk-dev] [PATCH 2/3] app/testpmd: support RSS hash level setting Ajit Khaparde
2019-12-07  0:59 ` [dpdk-dev] [PATCH 3/3] net/bnxt: add support to set RSS hash level Ajit Khaparde

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