Skip to contentSkip to conversation

I40E refactors

PATCH Superseded Anatoly Burakov 2 participants 9 messages Aug 19 – Aug 21 Network drivers

lore
Share & tools
lore
#0 lore raw

cover I40E refactors

A small amount of refactor in hash engine to prepare for future code. Specifically, disentangling the mutable filter list metadata from immutable RSS configuration supplied from rte_flow API, and getting rid of embedded rte_flow RSS conf struct and replacing it with direct values embedded in config struct.

Anatoly Burakov (2):
net/i40e: do not use flow RSS conf struct
net/i40e: move mutable fields out of RSS config

 drivers/net/intel/i40e/i40e_ethdev.h |  18 +++-
 drivers/net/intel/i40e/i40e_flow.c   |  11 ++-
 drivers/net/intel/i40e/i40e_hash.c   | 125 +++++++++++++--------------
 drivers/net/intel/i40e/i40e_hash.h   |   1 -
 4 files changed, 81 insertions(+), 74 deletions(-)
Signature · 1 line
2.52.0
#1 lore raw

PATCH 1/2 net/i40e: do not use flow RSS conf struct

Currently, the RSS filter structure includes an rte_flow RSS conf structure directly. This is suboptimal, because that structure has pointers in it, which makes copying data out of this structure a non-trivial operation that may introduce potentially dangling pointers to queue lists or RSS key.

Replace it with direct members of the RSS config struct, and adjust all users accordingly.

---
 drivers/net/intel/i40e/i40e_ethdev.h | 11 +++++---
 drivers/net/intel/i40e/i40e_flow.c   | 11 +++++---
 drivers/net/intel/i40e/i40e_hash.c   | 39 ++++++++++++----------------
 drivers/net/intel/i40e/i40e_hash.h   |  1 -
 4 files changed, 31 insertions(+), 31 deletions(-)
drivers/net/intel/i40e/i40e_ethdev.h +7 -4
index 9d9bde6aeb..b6f341169f 100644
@@ -1053,12 +1053,15 @@ struct i40e_customized_pctype {
 	bool valid;   /* Check if it's valid */
 };
 
+#define I40E_RSS_KEY_LEN ((I40E_PFQF_HKEY_MAX_INDEX + 1) * sizeof(uint32_t))
+
 struct i40e_rte_flow_rss_conf {
-	struct rte_flow_action_rss conf;	/**< RSS parameters. */
+	enum rte_eth_hash_function func;
+	uint64_t types; /**< Specific RSS hash types (see RTE_ETH_RSS_*). */
+	uint32_t key_len; /**< Hash key length in bytes. */
+	uint32_t queue_num; /**< Number of entries in @p queue. */
 
-	uint8_t key[(I40E_VFQF_HKEY_MAX_INDEX > I40E_PFQF_HKEY_MAX_INDEX ?
-		     I40E_VFQF_HKEY_MAX_INDEX : I40E_PFQF_HKEY_MAX_INDEX + 1) *
-		    sizeof(uint32_t)];		/**< Hash key. */
+	uint8_t key[I40E_RSS_KEY_LEN];		/**< Hash key. */
 	uint16_t queue[RTE_ETH_RSS_RETA_SIZE_512];	/**< Queues indices to use. */
 
 	bool symmetric_enable;		/**< true, if enable symmetric */
drivers/net/intel/i40e/i40e_flow.c +8 -3
index 6eb85a7d0d..f0eea61b1e 100644
@@ -4338,9 +4338,14 @@ i40e_flow_query(struct rte_eth_dev *dev __rte_unused,
 						   "action not supported");
 				return -rte_errno;
 			}
-			memcpy(rss_conf,
-				   &rss_rule->rss_filter_info.conf,
-				   sizeof(struct rte_flow_action_rss));
+			*rss_conf = (struct rte_flow_action_rss){
+				.func = rss_rule->rss_filter_info.func,
+				.types = rss_rule->rss_filter_info.types,
+				.key_len = rss_rule->rss_filter_info.key_len,
+				.queue_num = rss_rule->rss_filter_info.queue_num,
+				.key = rss_rule->rss_filter_info.key,
+				.queue = rss_rule->rss_filter_info.queue,
+			};
 			break;
 		default:
 			return rte_flow_error_set(error, ENOTSUP,
drivers/net/intel/i40e/i40e_hash.c +16 -23
index 6a23e2bf7f..2887460125 100644
@@ -747,7 +747,7 @@ i40e_hash_config_pctype(struct i40e_hw *hw,
 			struct i40e_rte_flow_rss_conf *rss_conf,
 			uint32_t pctype)
 {
-	uint64_t rss_types = rss_conf->conf.types;
+	uint64_t rss_types = rss_conf->types;
 	int ret;
 
 	if (rss_types == 0) {
@@ -829,17 +829,16 @@ static int
 i40e_hash_config(struct i40e_pf *pf,
 		 struct i40e_rte_flow_rss_conf *rss_conf)
 {
-	struct rte_flow_action_rss *rss_info = &rss_conf->conf;
 	struct i40e_hw *hw = &pf->adapter->hw;
 	uint64_t pctypes;
 	int ret;
 
-	if (rss_info->func != RTE_ETH_HASH_FUNCTION_DEFAULT) {
-		ret = i40e_hash_config_func(hw, rss_info->func);
+	if (rss_conf->func != RTE_ETH_HASH_FUNCTION_DEFAULT) {
+		ret = i40e_hash_config_func(hw, rss_conf->func);
 		if (ret)
 			return ret;
 
-		if (rss_info->func != RTE_ETH_HASH_FUNCTION_TOEPLITZ)
+		if (rss_conf->func != RTE_ETH_HASH_FUNCTION_TOEPLITZ)
 			rss_conf->misc_reset_flags |=
 					I40E_HASH_FLOW_RESET_FLAG_FUNC;
 	}
@@ -852,9 +851,9 @@ i40e_hash_config(struct i40e_pf *pf,
 		rss_conf->misc_reset_flags |= I40E_HASH_FLOW_RESET_FLAG_REGION;
 	}
 
-	if (rss_info->key_len > 0) {
+	if (rss_conf->key_len > 0) {
 		ret = i40e_set_rss_key(pf->main_vsi, rss_conf->key,
-				       rss_info->key_len);
+				       rss_conf->key_len);
 		if (ret)
 			return ret;
 
@@ -862,13 +861,13 @@ i40e_hash_config(struct i40e_pf *pf,
 	}
 
 	/* Update lookup table */
-	if (rss_info->queue_num > 0) {
+	if (rss_conf->queue_num > 0) {
 		uint8_t lut[RTE_ETH_RSS_RETA_SIZE_512];
 		uint32_t i, j = 0;
 
 		for (i = 0; i < hw->func_caps.rss_table_size; i++) {
-			lut[i] = (uint8_t)rss_info->queue[j];
-			j = (j == rss_info->queue_num - 1) ? 0 : (j + 1);
+			lut[i] = (uint8_t)rss_conf->queue[j];
+			j = (j == rss_conf->queue_num - 1) ? 0 : (j + 1);
 		}
 
 		ret = i40e_set_rss_lut(pf->main_vsi, lut, (uint16_t)i);
@@ -876,7 +875,7 @@ i40e_hash_config(struct i40e_pf *pf,
 			return ret;
 
 		pf->hash_enabled_queues = 0;
-		for (i = 0; i < rss_info->queue_num; i++)
+		for (i = 0; i < rss_conf->queue_num; i++)
 			pf->hash_enabled_queues |= BIT_ULL(lut[i]);
 
 		pf->adapter->rss_reta_updated = 0;
@@ -933,8 +932,7 @@ i40e_hash_parse_key(const struct rte_flow_action_rss *rss_act,
 		memcpy(rss_conf->key, key, sizeof(rss_conf->key));
 	}
 
-	rss_conf->conf.key = rss_conf->key;
-	rss_conf->conf.key_len = sizeof(rss_conf->key);
+	rss_conf->key_len = sizeof(rss_conf->key);
 }
 
 static int
@@ -949,8 +947,8 @@ i40e_hash_parse_pattern_act(const struct rte_eth_dev *dev,
 	if (rss_act->key_len)
 		i40e_hash_parse_key(rss_act, rss_conf);
 
-	rss_conf->conf.func = rss_act->func;
-	rss_conf->conf.types = rss_act->types;
+	rss_conf->func = rss_act->func;
+	rss_conf->types = rss_act->types;
 	rss_conf->inset = i40e_hash_get_inset(rss_act->types, rss_conf->symmetric_enable);
 
 	return i40e_hash_get_pattern_pctypes(dev, pattern, rss_act,
@@ -963,8 +961,7 @@ i40e_hash_parse_queues(const struct rte_flow_action_rss *rss_act,
 {
 	memcpy(rss_conf->queue, rss_act->queue,
 	       rss_act->queue_num * sizeof(rss_conf->queue[0]));
-	rss_conf->conf.queue = rss_conf->queue;
-	rss_conf->conf.queue_num = rss_act->queue_num;
+	rss_conf->queue_num = rss_act->queue_num;
 	return 0;
 }
 
@@ -1277,7 +1274,7 @@ i40e_hash_parse(struct rte_eth_dev *dev,
 			return ret;
 		rss_act = parsed_actions.actions[0]->conf;
 		/* set up RSS functions */
-		rss_conf->conf.func = rss_act->func;
+		rss_conf->func = rss_act->func;
 		return i40e_hash_parse_queue_region(pattern, rss_act, rss_conf, error);
 	}
 	/* Empty pattern path */
@@ -1287,7 +1284,7 @@ i40e_hash_parse(struct rte_eth_dev *dev,
 		if (ret)
 			return ret;
 		rss_act = parsed_actions.actions[0]->conf;
-		rss_conf->conf.func = rss_act->func;
+		rss_conf->func = rss_act->func;
 		/* if there is a queue list, take that path */
 		if (rss_act->queue != NULL)
 			return i40e_hash_parse_queues(rss_act, rss_conf);
@@ -1379,10 +1376,6 @@ i40e_hash_filter_create(struct i40e_pf *pf,
 	new_conf = &filter->rss_filter_info;
 
 	memcpy(new_conf, rss_conf, sizeof(*new_conf));
-	if (new_conf->conf.queue_num)
-		new_conf->conf.queue = new_conf->queue;
-	if (new_conf->conf.key_len)
-		new_conf->conf.key = new_conf->key;
 
 	ret = i40e_hash_config(pf, new_conf);
 	if (ret) {
drivers/net/intel/i40e/i40e_hash.h +0 -1
index 99df4bccd0..3bf30cdee5 100644
@@ -27,7 +27,6 @@ int i40e_hash_filter_destroy(struct i40e_pf *pf,
 			     const struct i40e_rss_filter *rss_filter);
 int i40e_hash_filter_flush(struct i40e_pf *pf);
 
-#define I40E_RSS_KEY_LEN ((I40E_PFQF_HKEY_MAX_INDEX + 1) * sizeof(uint32_t))
 extern const uint8_t i40e_rss_key_default[I40E_RSS_KEY_LEN];
 
 #ifdef __cplusplus
Signature · 1 line
2.52.0
#2 lore raw

PATCH 2/2 net/i40e: move mutable fields out of RSS config

In reply to Anatoly Burakov

Currently, the RSS filter configuration structure contains both data that is received from rte_flow API call, as well as some metadata for internal use (to decide on whether to reset the RSS configuration when removing the flow).

To separate these, move the metadata out of RSS configuration into a separate structure stored in RSS filter node.

---
 drivers/net/intel/i40e/i40e_ethdev.h |  7 +++
 drivers/net/intel/i40e/i40e_hash.c   | 86 ++++++++++++++--------------
 2 files changed, 50 insertions(+), 43 deletions(-)
drivers/net/intel/i40e/i40e_ethdev.h +7 -0
index b6f341169f..1e64a2d280 100644
@@ -1071,7 +1071,13 @@ struct i40e_rte_flow_rss_conf {
 	uint8_t region_priority;	/**< queue region priority */
 	uint8_t region_queue_num;	/**< region queue number */
 	uint16_t region_queue_start;	/**< region queue start */
+};
 
+/*
+ * Metadata for an installed RSS filter. Records which parts of the RSS
+ * configuration this filter owns, so they can be reset when filter is removed.
+ */
+struct i40e_rss_filter_data {
 	uint32_t misc_reset_flags;
 #define I40E_HASH_FLOW_RESET_FLAG_FUNC		0x01UL
 #define I40E_HASH_FLOW_RESET_FLAG_KEY		0x02UL
@@ -1088,6 +1094,7 @@ struct i40e_rte_flow_rss_conf {
 struct i40e_rss_filter {
 	TAILQ_ENTRY(i40e_rss_filter) next;
 	struct i40e_rte_flow_rss_conf rss_filter_info;
+	struct i40e_rss_filter_data filter_data;
 };
 
 TAILQ_HEAD(i40e_rss_conf_list, i40e_rss_filter);
drivers/net/intel/i40e/i40e_hash.c +43 -43

233 lines not on this page · raw message

Signature · 1 line
2.52.0
#3 lore raw

v2 cover I40E refactors

In reply to Anatoly Burakov

A small amount of refactor in hash engine to prepare for future code. Specifically, disentangling the mutable filter list metadata from immutable RSS configuration supplied from rte_flow API, and getting rid of embedded rte_flow RSS conf struct and replacing it with direct values embedded in config struct.

v1 -> v2:
- Fix patch 1 returning non-NULL pointers to queue/key when lengths of either are 0

Anatoly Burakov (2):
net/i40e: do not use flow RSS conf struct
net/i40e: move mutable fields out of RSS config

 drivers/net/intel/i40e/i40e_ethdev.h |  18 +++-
 drivers/net/intel/i40e/i40e_flow.c   |  13 ++-
 drivers/net/intel/i40e/i40e_hash.c   | 125 +++++++++++++--------------
 drivers/net/intel/i40e/i40e_hash.h   |   1 -
 4 files changed, 83 insertions(+), 74 deletions(-)
Signature · 1 line
2.52.0
#4 lore raw

PATCH v2 1/2 net/i40e: do not use flow RSS conf struct

Currently, the RSS filter structure includes an rte_flow RSS conf structure directly. This is suboptimal, because that structure has pointers in it, which makes copying data out of this structure a non-trivial operation that may introduce potentially dangling pointers to queue lists or RSS key.

Replace it with direct members of the RSS config struct, and adjust all users accordingly.

---
 drivers/net/intel/i40e/i40e_ethdev.h | 11 +++++---
 drivers/net/intel/i40e/i40e_flow.c   | 13 +++++++---
 drivers/net/intel/i40e/i40e_hash.c   | 39 ++++++++++++----------------
 drivers/net/intel/i40e/i40e_hash.h   |  1 -
 4 files changed, 33 insertions(+), 31 deletions(-)
drivers/net/intel/i40e/i40e_ethdev.h +7 -4
index 9d9bde6aeb..b6f341169f 100644
@@ -1053,12 +1053,15 @@ struct i40e_customized_pctype {
 	bool valid;   /* Check if it's valid */
 };
 
+#define I40E_RSS_KEY_LEN ((I40E_PFQF_HKEY_MAX_INDEX + 1) * sizeof(uint32_t))
+
 struct i40e_rte_flow_rss_conf {
-	struct rte_flow_action_rss conf;	/**< RSS parameters. */
+	enum rte_eth_hash_function func;
+	uint64_t types; /**< Specific RSS hash types (see RTE_ETH_RSS_*). */
+	uint32_t key_len; /**< Hash key length in bytes. */
+	uint32_t queue_num; /**< Number of entries in @p queue. */
 
-	uint8_t key[(I40E_VFQF_HKEY_MAX_INDEX > I40E_PFQF_HKEY_MAX_INDEX ?
-		     I40E_VFQF_HKEY_MAX_INDEX : I40E_PFQF_HKEY_MAX_INDEX + 1) *
-		    sizeof(uint32_t)];		/**< Hash key. */
+	uint8_t key[I40E_RSS_KEY_LEN];		/**< Hash key. */
 	uint16_t queue[RTE_ETH_RSS_RETA_SIZE_512];	/**< Queues indices to use. */
 
 	bool symmetric_enable;		/**< true, if enable symmetric */
drivers/net/intel/i40e/i40e_flow.c +10 -3
index 6eb85a7d0d..a017d3cd44 100644
@@ -4338,9 +4338,16 @@ i40e_flow_query(struct rte_eth_dev *dev __rte_unused,
 						   "action not supported");
 				return -rte_errno;
 			}
-			memcpy(rss_conf,
-				   &rss_rule->rss_filter_info.conf,
-				   sizeof(struct rte_flow_action_rss));
+			*rss_conf = (struct rte_flow_action_rss){
+				.func = rss_rule->rss_filter_info.func,
+				.types = rss_rule->rss_filter_info.types,
+				.key_len = rss_rule->rss_filter_info.key_len,
+				.queue_num = rss_rule->rss_filter_info.queue_num,
+				.key = rss_rule->rss_filter_info.key_len ?
+					rss_rule->rss_filter_info.key : NULL,
+				.queue = rss_rule->rss_filter_info.queue_num ?
+					rss_rule->rss_filter_info.queue : NULL,
+			};
 			break;
 		default:
 			return rte_flow_error_set(error, ENOTSUP,
drivers/net/intel/i40e/i40e_hash.c +16 -23
index 6a23e2bf7f..2887460125 100644
@@ -747,7 +747,7 @@ i40e_hash_config_pctype(struct i40e_hw *hw,
 			struct i40e_rte_flow_rss_conf *rss_conf,
 			uint32_t pctype)
 {
-	uint64_t rss_types = rss_conf->conf.types;
+	uint64_t rss_types = rss_conf->types;
 	int ret;
 
 	if (rss_types == 0) {
@@ -829,17 +829,16 @@ static int
 i40e_hash_config(struct i40e_pf *pf,
 		 struct i40e_rte_flow_rss_conf *rss_conf)
 {
-	struct rte_flow_action_rss *rss_info = &rss_conf->conf;
 	struct i40e_hw *hw = &pf->adapter->hw;
 	uint64_t pctypes;
 	int ret;
 
-	if (rss_info->func != RTE_ETH_HASH_FUNCTION_DEFAULT) {
-		ret = i40e_hash_config_func(hw, rss_info->func);
+	if (rss_conf->func != RTE_ETH_HASH_FUNCTION_DEFAULT) {
+		ret = i40e_hash_config_func(hw, rss_conf->func);
 		if (ret)
 			return ret;
 
-		if (rss_info->func != RTE_ETH_HASH_FUNCTION_TOEPLITZ)
+		if (rss_conf->func != RTE_ETH_HASH_FUNCTION_TOEPLITZ)
 			rss_conf->misc_reset_flags |=
 					I40E_HASH_FLOW_RESET_FLAG_FUNC;
 	}
@@ -852,9 +851,9 @@ i40e_hash_config(struct i40e_pf *pf,
 		rss_conf->misc_reset_flags |= I40E_HASH_FLOW_RESET_FLAG_REGION;
 	}
 
-	if (rss_info->key_len > 0) {
+	if (rss_conf->key_len > 0) {
 		ret = i40e_set_rss_key(pf->main_vsi, rss_conf->key,
-				       rss_info->key_len);
+				       rss_conf->key_len);
 		if (ret)
 			return ret;
 
@@ -862,13 +861,13 @@ i40e_hash_config(struct i40e_pf *pf,
 	}
 
 	/* Update lookup table */
-	if (rss_info->queue_num > 0) {
+	if (rss_conf->queue_num > 0) {
 		uint8_t lut[RTE_ETH_RSS_RETA_SIZE_512];
 		uint32_t i, j = 0;
 
 		for (i = 0; i < hw->func_caps.rss_table_size; i++) {
-			lut[i] = (uint8_t)rss_info->queue[j];
-			j = (j == rss_info->queue_num - 1) ? 0 : (j + 1);
+			lut[i] = (uint8_t)rss_conf->queue[j];
+			j = (j == rss_conf->queue_num - 1) ? 0 : (j + 1);
 		}
 
 		ret = i40e_set_rss_lut(pf->main_vsi, lut, (uint16_t)i);
@@ -876,7 +875,7 @@ i40e_hash_config(struct i40e_pf *pf,
 			return ret;
 
 		pf->hash_enabled_queues = 0;
-		for (i = 0; i < rss_info->queue_num; i++)
+		for (i = 0; i < rss_conf->queue_num; i++)
 			pf->hash_enabled_queues |= BIT_ULL(lut[i]);
 
 		pf->adapter->rss_reta_updated = 0;
@@ -933,8 +932,7 @@ i40e_hash_parse_key(const struct rte_flow_action_rss *rss_act,
 		memcpy(rss_conf->key, key, sizeof(rss_conf->key));
 	}
 
-	rss_conf->conf.key = rss_conf->key;
-	rss_conf->conf.key_len = sizeof(rss_conf->key);
+	rss_conf->key_len = sizeof(rss_conf->key);
 }
 
 static int
@@ -949,8 +947,8 @@ i40e_hash_parse_pattern_act(const struct rte_eth_dev *dev,
 	if (rss_act->key_len)
 		i40e_hash_parse_key(rss_act, rss_conf);
 
-	rss_conf->conf.func = rss_act->func;
-	rss_conf->conf.types = rss_act->types;
+	rss_conf->func = rss_act->func;
+	rss_conf->types = rss_act->types;
 	rss_conf->inset = i40e_hash_get_inset(rss_act->types, rss_conf->symmetric_enable);
 
 	return i40e_hash_get_pattern_pctypes(dev, pattern, rss_act,
@@ -963,8 +961,7 @@ i40e_hash_parse_queues(const struct rte_flow_action_rss *rss_act,
 {
 	memcpy(rss_conf->queue, rss_act->queue,
 	       rss_act->queue_num * sizeof(rss_conf->queue[0]));
-	rss_conf->conf.queue = rss_conf->queue;
-	rss_conf->conf.queue_num = rss_act->queue_num;
+	rss_conf->queue_num = rss_act->queue_num;
 	return 0;
 }
 
@@ -1277,7 +1274,7 @@ i40e_hash_parse(struct rte_eth_dev *dev,
 			return ret;
 		rss_act = parsed_actions.actions[0]->conf;
 		/* set up RSS functions */
-		rss_conf->conf.func = rss_act->func;
+		rss_conf->func = rss_act->func;
 		return i40e_hash_parse_queue_region(pattern, rss_act, rss_conf, error);
 	}
 	/* Empty pattern path */
@@ -1287,7 +1284,7 @@ i40e_hash_parse(struct rte_eth_dev *dev,
 		if (ret)
 			return ret;
 		rss_act = parsed_actions.actions[0]->conf;
-		rss_conf->conf.func = rss_act->func;
+		rss_conf->func = rss_act->func;
 		/* if there is a queue list, take that path */
 		if (rss_act->queue != NULL)
 			return i40e_hash_parse_queues(rss_act, rss_conf);
@@ -1379,10 +1376,6 @@ i40e_hash_filter_create(struct i40e_pf *pf,
 	new_conf = &filter->rss_filter_info;
 
 	memcpy(new_conf, rss_conf, sizeof(*new_conf));
-	if (new_conf->conf.queue_num)
-		new_conf->conf.queue = new_conf->queue;
-	if (new_conf->conf.key_len)
-		new_conf->conf.key = new_conf->key;
 
 	ret = i40e_hash_config(pf, new_conf);
 	if (ret) {
drivers/net/intel/i40e/i40e_hash.h +0 -1
index 99df4bccd0..3bf30cdee5 100644
@@ -27,7 +27,6 @@ int i40e_hash_filter_destroy(struct i40e_pf *pf,
 			     const struct i40e_rss_filter *rss_filter);
 int i40e_hash_filter_flush(struct i40e_pf *pf);
 
-#define I40E_RSS_KEY_LEN ((I40E_PFQF_HKEY_MAX_INDEX + 1) * sizeof(uint32_t))
 extern const uint8_t i40e_rss_key_default[I40E_RSS_KEY_LEN];
 
 #ifdef __cplusplus
Signature · 1 line
2.52.0
Acked-by on v2 1/2
#5 lore raw

Re: [PATCH v2 1/2] net/i40e: do not use flow RSS conf struct

Quoted 8 lines · Anatoly Burakov

Currently, the RSS filter structure includes an rte_flow RSS conf structure directly. This is suboptimal, because that structure has pointers in it, which makes copying data out of this structure a non-trivial operation that may introduce potentially dangling pointers to queue lists or RSS key.

Replace it with direct members of the RSS config struct, and adjust all users accordingly.

Quoted 8 lines
---
 drivers/net/intel/i40e/i40e_ethdev.h | 11 +++++---
 drivers/net/intel/i40e/i40e_flow.c   | 13 +++++++---
 drivers/net/intel/i40e/i40e_hash.c   | 39 ++++++++++++----------------
 drivers/net/intel/i40e/i40e_hash.h   |  1 -
 4 files changed, 33 insertions(+), 31 deletions(-)
#6 lore raw

PATCH v2 2/2 net/i40e: move mutable fields out of RSS config

In reply to Anatoly Burakov

Currently, the RSS filter configuration structure contains both data that is received from rte_flow API call, as well as some metadata for internal use (to decide on whether to reset the RSS configuration when removing the flow).

To separate these, move the metadata out of RSS configuration into a separate structure stored in RSS filter node.

---
 drivers/net/intel/i40e/i40e_ethdev.h |  7 +++
 drivers/net/intel/i40e/i40e_hash.c   | 86 ++++++++++++++--------------
 2 files changed, 50 insertions(+), 43 deletions(-)
drivers/net/intel/i40e/i40e_ethdev.h +7 -0
index b6f341169f..1e64a2d280 100644
@@ -1071,7 +1071,13 @@ struct i40e_rte_flow_rss_conf {
 	uint8_t region_priority;	/**< queue region priority */
 	uint8_t region_queue_num;	/**< region queue number */
 	uint16_t region_queue_start;	/**< region queue start */
+};
 
+/*
+ * Metadata for an installed RSS filter. Records which parts of the RSS
+ * configuration this filter owns, so they can be reset when filter is removed.
+ */
+struct i40e_rss_filter_data {
 	uint32_t misc_reset_flags;
 #define I40E_HASH_FLOW_RESET_FLAG_FUNC		0x01UL
 #define I40E_HASH_FLOW_RESET_FLAG_KEY		0x02UL
@@ -1088,6 +1094,7 @@ struct i40e_rte_flow_rss_conf {
 struct i40e_rss_filter {
 	TAILQ_ENTRY(i40e_rss_filter) next;
 	struct i40e_rte_flow_rss_conf rss_filter_info;
+	struct i40e_rss_filter_data filter_data;
 };
 
 TAILQ_HEAD(i40e_rss_conf_list, i40e_rss_filter);
drivers/net/intel/i40e/i40e_hash.c +43 -43

233 lines not on this page · raw message

Signature · 1 line
2.52.0
Acked-by on v2 2/2
#7 lore raw

Re: [PATCH v2 2/2] net/i40e: move mutable fields out of RSS config

Quoted 5 lines · Anatoly Burakov

Currently, the RSS filter configuration structure contains both data that is received from rte_flow API call, as well as some metadata for internal use (to decide on whether to reset the RSS configuration when removing the flow).

Quoted 9 lines

To separate these, move the metadata out of RSS configuration into a separate structure stored in RSS filter node.

---
 drivers/net/intel/i40e/i40e_ethdev.h |  7 +++
 drivers/net/intel/i40e/i40e_hash.c   | 86 ++++++++++++++--------------
 2 files changed, 50 insertions(+), 43 deletions(-)
#8 lore raw
In reply to Anatoly Burakov
Quoted 14 lines · Anatoly Burakov

A small amount of refactor in hash engine to prepare for future code. Specifically, disentangling the mutable filter list metadata from immutable RSS configuration supplied from rte_flow API, and getting rid of embedded rte_flow RSS conf struct and replacing it with direct values embedded in config struct.

v1 -> v2:
- Fix patch 1 returning non-NULL pointers to queue/key when lengths of either are 0

Anatoly Burakov (2):
net/i40e: do not use flow RSS conf struct
net/i40e: move mutable fields out of RSS config

Quoted 6 lines
 drivers/net/intel/i40e/i40e_ethdev.h |  18 +++-
 drivers/net/intel/i40e/i40e_flow.c   |  13 ++-
 drivers/net/intel/i40e/i40e_hash.c   | 125 +++++++++++++--------------
 drivers/net/intel/i40e/i40e_hash.h   |   1 -
 4 files changed, 83 insertions(+), 74 deletions(-)

Series applied to dpdk-next-net-intel

Thanks,
/Bruce