Skip to contentSkip to conversation

IXGBE fixes and cleanups

PATCH In review Anatoly Burakov 3 participants 20 messages Apr 30 – May 7 Network drivers

lore
Share & tools
lore

Anatoly Burakov

author
#0 lore raw

cover IXGBE fixes and cleanups

This patchset fixes a bunch of very old issues in flow management in IXGBE driver, such as:

- storing process-local pointers in shared memory
- incorrect L4 protocol matching for FDIR
- wrong handling of SCTP flow items
- reading stale FDIR state after flow destroy/flush
- storing all flows in global lists

In addition, some cleanup is also performed - refactors, moving things around to avoid accessing process-local state, and writing read-only values at init instead of deep in FDIR.

Finally, FDIR was also rejecting protocol-only matches for TCP and UDP, these are now supported.

Depends on flow dump patchset: https://patches.dpdk.org/project/dpdk/list/?series=38016

Anatoly Burakov (15):
net/ixgbe: fix flows not being scoped to port net/ixgbe: fix shared PF pointer in representor net/ixgbe: fix non-shared data in IPsec session net/ixgbe: fix SCTP protocol-only flow parsing net/ixgbe: fix L4 protocol mask handling net/ixgbe: reset flow state on clear paths net/ixgbe: store max VFs in adapter net/ixgbe: do not use flow list to count flows net/ixgbe: remove redundant flow tracking lists net/ixgbe: reduce FDIR conf macro usage net/ixgbe: use adapter in flow-related calls net/ixgbe: support protocol-only TCP and UDP rules net/ixgbe: write drop queue at init net/ixgbe: rely less on global flow state net/ixgbe: refactor flow creation

 drivers/net/intel/ixgbe/ixgbe_ethdev.c        | 112 +--
 drivers/net/intel/ixgbe/ixgbe_ethdev.h        |  45 +-
 drivers/net/intel/ixgbe/ixgbe_fdir.c          | 240 +++---
 drivers/net/intel/ixgbe/ixgbe_flow.c          | 716 +++++++++---------
 drivers/net/intel/ixgbe/ixgbe_ipsec.c         |  10 +-
 drivers/net/intel/ixgbe/ixgbe_ipsec.h         |   3 +-
 drivers/net/intel/ixgbe/ixgbe_rxtx.c          |  10 +-
 .../net/intel/ixgbe/ixgbe_vf_representor.c    |  63 +-
 8 files changed, 590 insertions(+), 609 deletions(-)
Signature · 1 line
2.47.3
author
#1 lore raw

PATCH 1/15 net/ixgbe: fix flows not being scoped to port

Currently, ixgbe keeps `rte_flow`` software state in global static lists without tracking which port created each rule. As a result, a rule removal request can affect rules that belong to another port (for example, a flow flush command will drop *all* flows from *all* ixgbe ports).

Fix this by moving the flow lists to adapter structure.

Fixes 72c135a89f80 ("net/ixgbe: create consistent filter")
Cc [email protected]
Cc [email protected]
Signed-off-by Anatoly Burakov <[email protected]>

---
Depends-on: series-38016 ("Implement flow dump")

 drivers/net/intel/ixgbe/ixgbe_ethdev.c |   4 +-
 drivers/net/intel/ixgbe/ixgbe_ethdev.h |  21 ++-
 drivers/net/intel/ixgbe/ixgbe_flow.c   | 247 +++++++++++--------------
 3 files changed, 126 insertions(+), 146 deletions(-)
drivers/net/intel/ixgbe/ixgbe_ethdev.c +2 -2
index 57d929cf2c..9454cbee0a 100644
@@ -1334,7 +1334,7 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev, void *init_params __rte_unused)
 		goto err_l2_tn_filter_init;
 
 	/* initialize flow filter lists */
-	ixgbe_filterlist_init();
+	ixgbe_filterlist_init(eth_dev);
 
 	/* initialize bandwidth configuration info */
 	memset(bw_conf, 0, sizeof(struct ixgbe_bw_conf));

@@ -3137,7 +3137,7 @@ ixgbe_dev_close(struct rte_eth_dev *dev)
 	ixgbe_ntuple_filter_uninit(dev);
 
 	/* clear all the filters list */
-	ixgbe_filterlist_flush();
+	ixgbe_filterlist_flush(dev);
 
 	/* Remove all Traffic Manager configuration */
 	ixgbe_tm_conf_uninit(dev);
drivers/net/intel/ixgbe/ixgbe_ethdev.h +19 -2
index 3f00896943..38d476d309 100644
@@ -455,6 +455,19 @@ struct ixgbe_tm_conf {
 	bool committed;
 };
 
+struct ixgbe_filter_ele_base;
+TAILQ_HEAD(ixgbe_filter_ele_list, ixgbe_filter_ele_base);
+
+struct ixgbe_flow_lists {
+	struct ixgbe_filter_ele_list ntuple_list;
+	struct ixgbe_filter_ele_list ethertype_list;
+	struct ixgbe_filter_ele_list syn_list;
+	struct ixgbe_filter_ele_list fdir_list;
+	struct ixgbe_filter_ele_list l2_tunnel_list;
+	struct ixgbe_filter_ele_list rss_list;
+	struct ixgbe_filter_ele_list flow_list;
+};
+
 /*
  * Structure to store private data for each driver instance (for each port).
  */

@@ -476,6 +489,7 @@ struct ixgbe_adapter {
 	struct ixgbe_bypass_info    bps;
 #endif /* RTE_LIBRTE_IXGBE_BYPASS */
 	struct ixgbe_filter_info    filter;
+	struct ixgbe_flow_lists     flow_lists;
 	struct ixgbe_l2_tn_info     l2_tn;
 	struct ixgbe_bw_conf        bw_conf;
 	struct ixgbe_ipsec          ipsec;

@@ -560,6 +574,9 @@ uint16_t ixgbe_vf_representor_tx_burst(void *tx_queue, struct rte_mbuf **tx_pkts
 #define IXGBE_DEV_PRIVATE_TO_FILTER_INFO(adapter) \
 	(&((struct ixgbe_adapter *)adapter)->filter)
 
+#define IXGBE_DEV_PRIVATE_TO_FLOW_LISTS(adapter) \
+	(&((struct ixgbe_adapter *)adapter)->flow_lists)
+
 #define IXGBE_DEV_PRIVATE_TO_L2_TN_INFO(adapter) \
 	(&((struct ixgbe_adapter *)adapter)->l2_tn)
 

@@ -690,8 +707,8 @@ ixgbe_dev_l2_tunnel_filter_add(struct rte_eth_dev *dev,
 int
 ixgbe_dev_l2_tunnel_filter_del(struct rte_eth_dev *dev,
 			       struct ixgbe_l2_tunnel_conf *l2_tunnel);
-void ixgbe_filterlist_init(void);
-void ixgbe_filterlist_flush(void);
+void ixgbe_filterlist_init(struct rte_eth_dev *dev);
+void ixgbe_filterlist_flush(struct rte_eth_dev *dev);
 /*
  * Flow director function prototypes
  */
drivers/net/intel/ixgbe/ixgbe_flow.c +105 -142

434 lines not on this page · raw message

Signature · 1 line
2.47.3
author
#2 lore raw

PATCH 2/15 net/ixgbe: fix shared PF pointer in representor

In reply to Anatoly Burakov

Currently, ixgbe representor private data stores a PF ethdev pointer. That pointer is process local, but it is stored in shared memory, so a secondary process can read an invalid pointer value.

Fix this by storing PF port id in representor private data and resolving PF ethdev from rte_eth_devices[] in each process. Return -ENODEV when the PF port is not valid.

This is not technically a bug in practice as using rte_flow from secondary processes isn't supported, but we still shouldn't do that.

Fixes cf80ba6e2038 ("net/ixgbe: add support for representor ports")
Cc [email protected]
Cc [email protected]
Signed-off-by Anatoly Burakov <[email protected]>
---
 drivers/net/intel/ixgbe/ixgbe_ethdev.c        |  2 +-
 drivers/net/intel/ixgbe/ixgbe_ethdev.h        |  2 +-
 .../net/intel/ixgbe/ixgbe_vf_representor.c    | 63 ++++++++++++++-----
 3 files changed, 50 insertions(+), 17 deletions(-)
drivers/net/intel/ixgbe/ixgbe_ethdev.c +1 -1
index 9454cbee0a..5c95507d60 100644
@@ -1818,7 +1818,7 @@ eth_ixgbe_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 
 		representor.vf_id = eth_da.representor_ports[i];
 		representor.switch_domain_id = vfinfo->switch_domain_id;
-		representor.pf_ethdev = pf_ethdev;
+		representor.pf_port_id = pf_ethdev->data->port_id;
 
 		/* representor port net_bdf_port */
 		snprintf(name, sizeof(name), "net_%s_representor_%d",
drivers/net/intel/ixgbe/ixgbe_ethdev.h +1 -1
index 38d476d309..1293ea49cb 100644
@@ -518,7 +518,7 @@ struct ixgbe_adapter {
 struct ixgbe_vf_representor {
 	uint16_t vf_id;
 	uint16_t switch_domain_id;
-	struct rte_eth_dev *pf_ethdev;
+	uint16_t pf_port_id;
 };
 
 int ixgbe_vf_representor_init(struct rte_eth_dev *ethdev, void *init_params);
drivers/net/intel/ixgbe/ixgbe_vf_representor.c +48 -15

147 lines not on this page · raw message

Signature · 1 line
2.47.3
author
#3 lore raw

PATCH 3/15 net/ixgbe: fix non-shared data in IPsec session

In reply to Anatoly Burakov

Currently, ixgbe IPsec session private data stores an ethdev pointer. That pointer is process local, but the session private data is shared, so a secondary process can read an invalid pointer value.

Fix this by storing ethdev data pointer in session private data instead, and using it for session/device binding checks and dev_private lookups when adding SAs.

Fixes 9a0752f498d2 ("net/ixgbe: enable inline IPsec")
Cc [email protected]
Cc [email protected]
Signed-off-by Anatoly Burakov <[email protected]>
---
 drivers/net/intel/ixgbe/ixgbe_ipsec.c | 10 +++++-----
 drivers/net/intel/ixgbe/ixgbe_ipsec.h |  3 ++-
 2 files changed, 7 insertions(+), 6 deletions(-)
drivers/net/intel/ixgbe/ixgbe_ipsec.c +5 -5
index fe9a96c54d..88225bccc0 100644
@@ -88,10 +88,10 @@ ixgbe_crypto_clear_ipsec_tables(struct rte_eth_dev *dev)
 static int
 ixgbe_crypto_add_sa(struct ixgbe_crypto_session *ic_session)
 {
-	struct rte_eth_dev *dev = ic_session->dev;
-	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct rte_eth_dev_data *dev_data = ic_session->dev_data;
+	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev_data->dev_private);
 	struct ixgbe_ipsec *priv = IXGBE_DEV_PRIVATE_TO_IPSEC(
-			dev->data->dev_private);
+			dev_data->dev_private);
 	uint32_t reg_val;
 	int sa_index = -1;
 

@@ -405,7 +405,7 @@ ixgbe_crypto_create_session(void *device,
 	memcpy(&ic_session->salt,
 	       &aead_xform->key.data[aead_xform->key.length], 4);
 	ic_session->spi = conf->ipsec.spi;
-	ic_session->dev = eth_dev;
+	ic_session->dev_data = eth_dev->data;
 
 	if (ic_session->op == IXGBE_OP_AUTHENTICATED_ENCRYPTION) {
 		if (ixgbe_crypto_add_sa(ic_session)) {

@@ -430,7 +430,7 @@ ixgbe_crypto_remove_session(void *device,
 	struct rte_eth_dev *eth_dev = device;
 	struct ixgbe_crypto_session *ic_session = SECURITY_GET_SESS_PRIV(session);
 
-	if (eth_dev != ic_session->dev) {
+	if (eth_dev->data != ic_session->dev_data) {
 		PMD_DRV_LOG(ERR, "Session not bound to this device");
 		return -ENODEV;
 	}
drivers/net/intel/ixgbe/ixgbe_ipsec.h +2 -1
index e7c7186264..356817c61b 100644
@@ -5,6 +5,7 @@
 #ifndef IXGBE_IPSEC_H_
 #define IXGBE_IPSEC_H_
 
+#include <ethdev_driver.h>
 #include <rte_security.h>
 #include <rte_security_driver.h>
 

@@ -72,7 +73,7 @@ struct __rte_cache_aligned ixgbe_crypto_session {
 	uint32_t spi;
 	struct ipaddr src_ip;
 	struct ipaddr dst_ip;
-	struct rte_eth_dev *dev;
+	struct rte_eth_dev_data *dev_data;
 };
 
 struct ixgbe_crypto_rx_ip_table {
Signature · 1 line
2.47.3
Acked-by on v1 3/15
#4 lore raw

Re: [PATCH v1 03/15] net/ixgbe: fix non-shared data in IPsec session

Quoted 8 lines · Anatoly Burakov

Currently, ixgbe IPsec session private data stores an ethdev pointer. That pointer is process local, but the session private data is shared, so a secondary process can read an invalid pointer value.

Fix this by storing ethdev data pointer in session private data instead, and using it for session/device binding checks and dev_private lookups when adding SAs.

Quoted 6 lines
Fixes 9a0752f498d2 ("net/ixgbe: enable inline IPsec")
Cc [email protected]
Cc [email protected]
Signed-off-by Anatoly Burakov <[email protected]>

---

Acked-by Radu Nicolau <[email protected]>
author
#5 lore raw

PATCH 4/15 net/ixgbe: fix SCTP protocol-only flow parsing

In reply to Anatoly Burakov

Currently, ixgbe_parse_fdir_filter_normal() checks the MAC type before checking whether the RTE_FLOW_ITEM_TYPE_SCTP item carries a mask. On ixgbe_mac_X550, ixgbe_mac_X550EM_x, ixgbe_mac_X550EM_a and ixgbe_mac_E610, this makes item->mask mandatory and rejects protocol-only SCTP patterns.

These devices can still match SCTP by protocol without L4 port masks. Only SCTP port masking is MAC type dependent, but the current check order makes the protocol-only path available only on older MAC types.

Fix this by checking whether a mask is present first. Keep the MAC type check only for SCTP port masks, and accept protocol-only SCTP matching on X550 and E610 as well.

Fixes 86e19565f5e2 ("net/ixgbe: fix SCTP port support")
Cc [email protected]
Cc [email protected]
Signed-off-by Anatoly Burakov <[email protected]>
---
 drivers/net/intel/ixgbe/ixgbe_flow.c | 86 ++++++++++++++--------------
 1 file changed, 42 insertions(+), 44 deletions(-)
drivers/net/intel/ixgbe/ixgbe_flow.c +42 -44
index c3abba4a90..71da5ac72e 100644
@@ -2180,57 +2180,55 @@ ixgbe_parse_fdir_filter_normal(struct rte_eth_dev *dev,
 			return -rte_errno;
 		}
 
-		/* only some mac types support sctp port */
-		if (hw->mac.type == ixgbe_mac_X550 ||
-		    hw->mac.type == ixgbe_mac_X550EM_x ||
-		    hw->mac.type == ixgbe_mac_X550EM_a ||
-		    hw->mac.type == ixgbe_mac_E610) {
-			/**
-			 * Only care about src & dst ports,
-			 * others should be masked.
-			 */
-			if (!item->mask) {
-				memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
-				rte_flow_error_set(error, EINVAL,
-					RTE_FLOW_ERROR_TYPE_ITEM,
-					item, "Not supported by fdir filter");
-				return -rte_errno;
-			}
-			rule->b_mask = TRUE;
-			sctp_mask = item->mask;
-			if (sctp_mask->hdr.tag ||
-				sctp_mask->hdr.cksum) {
-				memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
-				rte_flow_error_set(error, EINVAL,
-					RTE_FLOW_ERROR_TYPE_ITEM,
-					item, "Not supported by fdir filter");
-				return -rte_errno;
-			}
-			rule->mask.src_port_mask = sctp_mask->hdr.src_port;
-			rule->mask.dst_port_mask = sctp_mask->hdr.dst_port;
+		sctp_mask = item->mask;
+		if (sctp_mask != NULL) {
+			/* only some mac types support sctp port masking */
+			if (hw->mac.type == ixgbe_mac_X550 ||
+			    hw->mac.type == ixgbe_mac_X550EM_x ||
+			    hw->mac.type == ixgbe_mac_X550EM_a ||
+			    hw->mac.type == ixgbe_mac_E610) {
+				/**
+				 * Only care about src & dst ports,
+				 * others should be masked.
+				 */
+				rule->b_mask = TRUE;
+				if (sctp_mask->hdr.tag ||
+				    sctp_mask->hdr.cksum) {
+					memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
+					rte_flow_error_set(error, EINVAL,
+						RTE_FLOW_ERROR_TYPE_ITEM,
+						item, "Not supported by fdir filter");
+					return -rte_errno;
+				}
+				rule->mask.src_port_mask = sctp_mask->hdr.src_port;
+				rule->mask.dst_port_mask = sctp_mask->hdr.dst_port;
 
-			if (item->spec) {
-				rule->b_spec = TRUE;
-				sctp_spec = item->spec;
-				rule->ixgbe_fdir.formatted.src_port =
-					sctp_spec->hdr.src_port;
-				rule->ixgbe_fdir.formatted.dst_port =
-					sctp_spec->hdr.dst_port;
-			}
-		/* others even sctp port is not supported */
-		} else {
-			sctp_mask = item->mask;
-			if (sctp_mask &&
-				(sctp_mask->hdr.src_port ||
-				 sctp_mask->hdr.dst_port ||
-				 sctp_mask->hdr.tag ||
-				 sctp_mask->hdr.cksum)) {
+				if (item->spec) {
+					rule->b_spec = TRUE;
+					sctp_spec = item->spec;
+					rule->ixgbe_fdir.formatted.src_port =
+						sctp_spec->hdr.src_port;
+					rule->ixgbe_fdir.formatted.dst_port =
+						sctp_spec->hdr.dst_port;
+				}
+			/* others even sctp port masking is not supported */
+			} else if (sctp_mask->hdr.src_port ||
+				   sctp_mask->hdr.dst_port ||
+				   sctp_mask->hdr.tag ||
+				   sctp_mask->hdr.cksum) {
 				memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
 				rte_flow_error_set(error, EINVAL,
 					RTE_FLOW_ERROR_TYPE_ITEM,
 					item, "Not supported by fdir filter");
 				return -rte_errno;
 			}
+		} else if (item->spec != NULL) {
+			/* No port mask means protocol-only match; spec is invalid. */
+			memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
+			rte_flow_error_set(error, EINVAL,
+				RTE_FLOW_ERROR_TYPE_ITEM,
+				item, "Not supported by fdir filter");
+			return -rte_errno;
 		}
 
 		item = next_no_fuzzy_pattern(pattern, item);
Signature · 1 line
2.47.3
author
#6 lore raw

PATCH 5/15 net/ixgbe: fix L4 protocol mask handling

In reply to Anatoly Burakov

Currently, ixgbe uses zero src_port_mask and dst_port_mask to decide whether IXGBE_FDIRM_L4P should ignore the L4 protocol. This conflates two different cases: broad L4 matches that care about the L4 protocol but ignore L4 ports, and plain IP matches that should ignore L4 protocol entirely.

The current rte_flow path also copies parsed masks through struct rte_eth_fdir_masks, which cannot preserve that distinction. As a result, protocol-only rte_flow rules lose their L4 protocol match information before the hardware FDIR mask is programmed.

Fix this by storing explicit L4 protocol match state in struct ixgbe_hw_fdir_mask and by programming IXGBE_FDIRM_L4P from that state instead of inferring it from port masks. Remove the lossy struct rte_eth_fdir_masks mediation layer and program the hardware mask directly from the ixgbe FDIR mask structure.

Fixes 11777435c727 ("net/ixgbe: parse flow director filter")
Cc [email protected]
Cc [email protected]
Signed-off-by Anatoly Burakov <[email protected]>
---
 drivers/net/intel/ixgbe/ixgbe_ethdev.h |  1 +
 drivers/net/intel/ixgbe/ixgbe_fdir.c   | 90 +++-----------------------
 drivers/net/intel/ixgbe/ixgbe_flow.c   | 11 ++--
 3 files changed, 15 insertions(+), 87 deletions(-)
drivers/net/intel/ixgbe/ixgbe_ethdev.h +1 -0
index 1293ea49cb..a0a2ea23b2 100644
@@ -151,6 +151,7 @@ struct ixgbe_hw_fdir_mask {
 	uint32_t dst_ipv4_mask;
 	uint16_t src_ipv6_mask;
 	uint16_t dst_ipv6_mask;
+	uint8_t  l4_proto_match;
 	uint16_t src_port_mask;
 	uint16_t dst_port_mask;
 	uint16_t flex_bytes_mask;
drivers/net/intel/ixgbe/ixgbe_fdir.c +9 -81

126 lines not on this page · raw message

drivers/net/intel/ixgbe/ixgbe_flow.c +5 -6
index 71da5ac72e..6e87d373ad 100644
@@ -1713,6 +1713,7 @@ ixgbe_parse_fdir_filter_normal(struct rte_eth_dev *dev,
 	memset(&rule->mask, 0xFF, sizeof(struct ixgbe_hw_fdir_mask));
 	rule->mask.vlan_tci_mask = 0;
 	rule->mask.flex_bytes_mask = 0;
+	rule->mask.l4_proto_match = 0;
 	rule->mask.dst_port_mask = 0;
 	rule->mask.src_port_mask = 0;
 

@@ -2325,6 +2326,10 @@ ixgbe_parse_fdir_filter_normal(struct rte_eth_dev *dev,
 		}
 	}
 
+	/* L4 protocol matching is enabled when parser selected an L4 type. */
+	rule->mask.l4_proto_match =
+		(rule->ixgbe_fdir.formatted.flow_type & IXGBE_ATR_L4TYPE_MASK) != 0;
+
 	return ixgbe_parse_fdir_act_attr(attr, actions, rule, error);
 }
 

@@ -2852,12 +2857,6 @@ ixgbe_parse_fdir_filter(struct rte_eth_dev *dev,
 
 step_next:
 
-	if (hw->mac.type == ixgbe_mac_82599EB &&
-		rule->fdirflags == IXGBE_FDIRCMD_DROP &&
-		(rule->ixgbe_fdir.formatted.src_port != 0 ||
-		rule->ixgbe_fdir.formatted.dst_port != 0))
-		return -ENOTSUP;
-
 	if (fdir_conf->mode == RTE_FDIR_MODE_NONE) {
 		fdir_conf->mode = rule->mode;
 		ret = ixgbe_fdir_configure(dev);
Signature · 1 line
2.47.3
author
#7 lore raw

PATCH 6/15 net/ixgbe: reset flow state on clear paths

In reply to Anatoly Burakov

When all FDIR rules are removed (either through destroy or flush), the driver does not fully reset its state. This can result in picking up stale state in some circumstances.

Fix by clearing internal state when FDIR flow list is empty.

Fixes 11777435c727 ("net/ixgbe: parse flow director filter")
Cc [email protected]
Signed-off-by Anatoly Burakov <[email protected]>
---
 drivers/net/intel/ixgbe/ixgbe_fdir.c | 7 +++++++
 drivers/net/intel/ixgbe/ixgbe_flow.c | 7 ++++++-
 2 files changed, 13 insertions(+), 1 deletion(-)
drivers/net/intel/ixgbe/ixgbe_fdir.c +7 -0
index 38f589623e..f51582a4bf 100644
@@ -1358,6 +1358,7 @@ ixgbe_fdir_filter_restore(struct rte_eth_dev *dev)
 int
 ixgbe_clear_all_fdir_filter(struct rte_eth_dev *dev)
 {
+	struct rte_eth_fdir_conf *fdir_conf = IXGBE_DEV_FDIR_CONF(dev);
 	struct ixgbe_hw_fdir_info *fdir_info =
 		IXGBE_DEV_PRIVATE_TO_FDIR_INFO(dev->data->dev_private);
 	struct ixgbe_fdir_filter *fdir_filter;

@@ -1376,6 +1377,12 @@ ixgbe_clear_all_fdir_filter(struct rte_eth_dev *dev)
 		rte_free(fdir_filter);
 	}
 
+	/* reset internal FDIR state */
+	fdir_info->mask = (struct ixgbe_hw_fdir_mask){0};
+	fdir_info->flex_bytes_offset = 0;
+	fdir_info->mask_added = FALSE;
+	fdir_conf->mode = RTE_FDIR_MODE_NONE;
+
 	if (filter_flag != NULL)
 		ret = ixgbe_fdir_flush(dev);
drivers/net/intel/ixgbe/ixgbe_flow.c +6 -1
index 6e87d373ad..b73037d4c3 100644
@@ -3473,11 +3473,16 @@ ixgbe_flow_destroy(struct rte_eth_dev *dev,
 			sizeof(struct ixgbe_fdir_rule));
 		ret = ixgbe_fdir_filter_program(dev, &fdir_rule, TRUE, FALSE);
 		if (!ret) {
+			struct rte_eth_fdir_conf *fdir_conf = IXGBE_DEV_FDIR_CONF(dev);
 			TAILQ_REMOVE(&flow_lists->fdir_list,
 				&fdir_rule_ptr->base, entries);
 			rte_free(fdir_rule_ptr);
-			if (TAILQ_EMPTY(&flow_lists->fdir_list))
+			if (TAILQ_EMPTY(&flow_lists->fdir_list)) {
 				fdir_info->mask_added = false;
+				fdir_info->mask = (struct ixgbe_hw_fdir_mask){0};
+				fdir_info->flex_bytes_offset = 0;
+				fdir_conf->mode = RTE_FDIR_MODE_NONE;
+			}
 		}
 		break;
 	case RTE_ETH_FILTER_L2_TUNNEL:
Signature · 1 line
2.47.3
author
#8 lore raw

PATCH 7/15 net/ixgbe: store max VFs in adapter

In reply to Anatoly Burakov

Currently, rte_flow-related checks use rte_eth_dev to check for max VFs. With coming rework of flows, the aim is to make the code as multiprocess agnostic as possible, and rte_eth_dev pointer is process-local.

To support this in VF-related checks, cache max_vfs in ixgbe adapter during device init and read it in the rte_flow check paths to avoid direct dependency on rte_eth_dev.

Signed-off-by Anatoly Burakov <[email protected]>
---
 drivers/net/intel/ixgbe/ixgbe_ethdev.c | 3 ++-
 drivers/net/intel/ixgbe/ixgbe_ethdev.h | 1 +
 drivers/net/intel/ixgbe/ixgbe_flow.c   | 8 ++++----
 3 files changed, 7 insertions(+), 5 deletions(-)
drivers/net/intel/ixgbe/ixgbe_ethdev.c +2 -1
index 5c95507d60..1c4a2e1177 100644
@@ -1084,7 +1084,7 @@ ixgbe_parse_devargs(struct ixgbe_adapter *adapter,
 static int
 eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev, void *init_params __rte_unused)
 {
-	struct ixgbe_adapter *ad = eth_dev->data->dev_private;
+	struct ixgbe_adapter *ad = IXGBE_DEV_PRIVATE_TO_ADAPTER(eth_dev->data->dev_private);
 	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(eth_dev);
 	struct rte_intr_handle *intr_handle = pci_dev->intr_handle;
 	struct ixgbe_hw *hw =

@@ -1151,6 +1151,7 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev, void *init_params __rte_unused)
 	hw->vendor_id = pci_dev->id.vendor_id;
 	hw->hw_addr = (void *)pci_dev->mem_resource[0].addr;
 	hw->allow_unsupported_sfp = 1;
+	ad->max_vfs = pci_dev->max_vfs;
 
 	/* Initialize the shared code (base driver) */
 #ifdef RTE_LIBRTE_IXGBE_BYPASS
drivers/net/intel/ixgbe/ixgbe_ethdev.h +1 -0
index a0a2ea23b2..2fb6d55387 100644
@@ -506,6 +506,7 @@ struct ixgbe_adapter {
 
 	/* Used for limiting SDP3 TX_DISABLE checks */
 	uint8_t sdp3_no_tx_disable;
+	uint16_t max_vfs;
 
 	/* Used for VF link sync with PF's physical and logical (by checking
 	 * mailbox status) link status.
drivers/net/intel/ixgbe/ixgbe_flow.c +4 -4
index b73037d4c3..e641a9d405 100644
@@ -1263,7 +1263,7 @@ cons_parse_l2_tn_filter(struct rte_eth_dev *dev,
 	const struct rte_flow_item_e_tag *e_tag_mask;
 	const struct rte_flow_action *act;
 	const struct rte_flow_action_vf *act_vf;
-	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
+	struct ixgbe_adapter *ad = IXGBE_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
 
 	if (!pattern) {
 		rte_flow_error_set(error, EINVAL,

@@ -1395,7 +1395,7 @@ cons_parse_l2_tn_filter(struct rte_eth_dev *dev,
 		act_vf = (const struct rte_flow_action_vf *)act->conf;
 		filter->pool = act_vf->id;
 	} else {
-		filter->pool = pci_dev->max_vfs;
+		filter->pool = ad->max_vfs;
 	}
 
 	/* check if the next not void item is END */

@@ -1421,7 +1421,7 @@ ixgbe_parse_l2_tn_filter(struct rte_eth_dev *dev,
 {
 	int ret = 0;
 	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
+	struct ixgbe_adapter *ad = IXGBE_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
 	uint16_t vf_num;
 
 	ret = cons_parse_l2_tn_filter(dev, attr, pattern,

@@ -1438,7 +1438,7 @@ ixgbe_parse_l2_tn_filter(struct rte_eth_dev *dev,
 		return -rte_errno;
 	}
 
-	vf_num = pci_dev->max_vfs;
+	vf_num = ad->max_vfs;
 
 	if (l2_tn_filter->pool > vf_num)
 		return -rte_errno;
Signature · 1 line
2.47.3
author
#9 lore raw

PATCH 8/15 net/ixgbe: do not use flow list to count flows

In reply to Anatoly Burakov

Currently, FDIR code will use emptiness of its flow list as an indicator that there are no flows (and that we can install a mask). That usage is the only thing preventing us from getting rid of the FDIR flow list altogether, so introduce a new mechanism for flow count tracking.

Signed-off-by Anatoly Burakov <[email protected]>
---
 drivers/net/intel/ixgbe/ixgbe_ethdev.c | 1 +
 drivers/net/intel/ixgbe/ixgbe_ethdev.h | 1 +
 drivers/net/intel/ixgbe/ixgbe_fdir.c   | 8 +++++---
 drivers/net/intel/ixgbe/ixgbe_flow.c   | 3 ++-
 4 files changed, 9 insertions(+), 4 deletions(-)
drivers/net/intel/ixgbe/ixgbe_ethdev.c +1 -0
index 1c4a2e1177..ee1b499b49 100644
@@ -1465,6 +1465,7 @@ static int ixgbe_fdir_filter_init(struct rte_eth_dev *eth_dev)
 		rte_hash_free(fdir_info->hash_handle);
 		return -ENOMEM;
 	}
+	fdir_info->n_flows = 0;
 	fdir_info->mask_added = FALSE;
 
 	return 0;
drivers/net/intel/ixgbe/ixgbe_ethdev.h +1 -0
index 2fb6d55387..6147cd6bdf 100644
@@ -199,6 +199,7 @@ struct ixgbe_hw_fdir_info {
 	struct ixgbe_fdir_filter **hash_map;
 	struct rte_hash *hash_handle; /* cuckoo hash handler */
 	bool mask_added; /* If already got mask from consistent filter */
+	uint32_t    n_flows;
 };
 
 struct ixgbe_rte_flow_rss_conf {
drivers/net/intel/ixgbe/ixgbe_fdir.c +5 -3
index f51582a4bf..5f7159abf2 100644
@@ -1362,20 +1362,22 @@ ixgbe_clear_all_fdir_filter(struct rte_eth_dev *dev)
 	struct ixgbe_hw_fdir_info *fdir_info =
 		IXGBE_DEV_PRIVATE_TO_FDIR_INFO(dev->data->dev_private);
 	struct ixgbe_fdir_filter *fdir_filter;
-	struct ixgbe_fdir_filter *filter_flag;
+	bool had_flows;
 	int ret = 0;
 
+	had_flows = (fdir_info->n_flows != 0);
+
 	/* flush flow director */
 	rte_hash_reset(fdir_info->hash_handle);
 	memset(fdir_info->hash_map, 0,
 	       sizeof(struct ixgbe_fdir_filter *) * IXGBE_MAX_FDIR_FILTER_NUM);
-	filter_flag = TAILQ_FIRST(&fdir_info->fdir_list);
 	while ((fdir_filter = TAILQ_FIRST(&fdir_info->fdir_list))) {
 		TAILQ_REMOVE(&fdir_info->fdir_list,
 			     fdir_filter,
 			     entries);
 		rte_free(fdir_filter);
 	}
+	fdir_info->n_flows = 0;
 
 	/* reset internal FDIR state */
 	fdir_info->mask = (struct ixgbe_hw_fdir_mask){0};

@@ -1383,7 +1385,7 @@ ixgbe_clear_all_fdir_filter(struct rte_eth_dev *dev)
 	fdir_info->mask_added = FALSE;
 	fdir_conf->mode = RTE_FDIR_MODE_NONE;
 
-	if (filter_flag != NULL)
+	if (had_flows)
 		ret = ixgbe_fdir_flush(dev);
 
 	return ret;
drivers/net/intel/ixgbe/ixgbe_flow.c +2 -1
index e641a9d405..b68934e911 100644
@@ -3241,6 +3241,7 @@ ixgbe_flow_create(struct rte_eth_dev *dev,
 					&fdir_rule_ptr->base, entries);
 				flow->rule = fdir_rule_ptr;
 				flow->filter_type = RTE_ETH_FILTER_FDIR;
+				fdir_info->n_flows++;
 
 				return flow;
 			}

@@ -3477,7 +3478,7 @@ ixgbe_flow_destroy(struct rte_eth_dev *dev,
 			TAILQ_REMOVE(&flow_lists->fdir_list,
 				&fdir_rule_ptr->base, entries);
 			rte_free(fdir_rule_ptr);
-			if (TAILQ_EMPTY(&flow_lists->fdir_list)) {
+			if (fdir_info->n_flows > 0 && --(fdir_info->n_flows) == 0) {
 				fdir_info->mask_added = false;
 				fdir_info->mask = (struct ixgbe_hw_fdir_mask){0};
 				fdir_info->flex_bytes_offset = 0;
Signature · 1 line
2.47.3
#10 lore raw

Re: [PATCH v1 08/15] net/ixgbe: do not use flow list to count flows

Quoted 30 lines · Anatoly Burakov

Currently, FDIR code will use emptiness of its flow list as an indicator that there are no flows (and that we can install a mask). That usage is the only thing preventing us from getting rid of the FDIR flow list altogether, so introduce a new mechanism for flow count tracking.

Signed-off-by Anatoly Burakov <[email protected]>
---
 drivers/net/intel/ixgbe/ixgbe_ethdev.c | 1 +
 drivers/net/intel/ixgbe/ixgbe_ethdev.h | 1 +
 drivers/net/intel/ixgbe/ixgbe_fdir.c   | 8 +++++---
 drivers/net/intel/ixgbe/ixgbe_flow.c   | 3 ++-
 4 files changed, 9 insertions(+), 4 deletions(-)
drivers/net/intel/ixgbe/ixgbe_ethdev.c +1 -0
index 1c4a2e1177..ee1b499b49 100644
@@ -1465,6 +1465,7 @@ static int ixgbe_fdir_filter_init(struct rte_eth_dev *eth_dev)
 		rte_hash_free(fdir_info->hash_handle);
 		return -ENOMEM;
 	}
+	fdir_info->n_flows = 0;
 	fdir_info->mask_added = FALSE;
 
 	return 0;
drivers/net/intel/ixgbe/ixgbe_ethdev.h +0 -0
index 2fb6d55387..6147cd6bdf 100644

drivers/net/intel/ixgbe/ixgbe_ethdev.h struct ixgbe_hw_fdir_info { · line 204

Quoted 6 lines
drivers/net/intel/ixgbe/ixgbe_ethdev.h +1 -0
@@ -199,6 +199,7 @@ struct ixgbe_hw_fdir_info {
 	struct ixgbe_fdir_filter **hash_map;
 	struct rte_hash *hash_handle; /* cuckoo hash handler */
 	bool mask_added; /* If already got mask from consistent filter */
+	uint32_t    n_flows;
 };

Minor nit, for future reference: best to put this before the mask_added field, so that we have the fields sorted for size and avoid gaps. However, this struct already has gaps, and is not perf sensitive, so not a big deal here. If you don't mind me doing so, I can reorder the new field on apply.

/Bruce

author
#11 lore raw

PATCH 9/15 net/ixgbe: remove redundant flow tracking lists

In reply to Anatoly Burakov

Keep flow rule ownership in ixgbe_flow_list and drop duplicate per-type tracking lists in the flow path.

This removes redundant TAILQ bookkeeping for ntuple, ethertype, syn, FDIR, l2 tunnel, and RSS rules while preserving behavior.

Signed-off-by Anatoly Burakov <[email protected]>
---
 drivers/net/intel/ixgbe/ixgbe_ethdev.h | 15 +---
 drivers/net/intel/ixgbe/ixgbe_flow.c   | 96 ++++++--------------------
 2 files changed, 21 insertions(+), 90 deletions(-)
drivers/net/intel/ixgbe/ixgbe_ethdev.h +1 -14
index 6147cd6bdf..f49a179082 100644
@@ -460,16 +460,6 @@ struct ixgbe_tm_conf {
 struct ixgbe_filter_ele_base;
 TAILQ_HEAD(ixgbe_filter_ele_list, ixgbe_filter_ele_base);
 
-struct ixgbe_flow_lists {
-	struct ixgbe_filter_ele_list ntuple_list;
-	struct ixgbe_filter_ele_list ethertype_list;
-	struct ixgbe_filter_ele_list syn_list;
-	struct ixgbe_filter_ele_list fdir_list;
-	struct ixgbe_filter_ele_list l2_tunnel_list;
-	struct ixgbe_filter_ele_list rss_list;
-	struct ixgbe_filter_ele_list flow_list;
-};
-
 /*
  * Structure to store private data for each driver instance (for each port).
  */

@@ -491,7 +481,7 @@ struct ixgbe_adapter {
 	struct ixgbe_bypass_info    bps;
 #endif /* RTE_LIBRTE_IXGBE_BYPASS */
 	struct ixgbe_filter_info    filter;
-	struct ixgbe_flow_lists     flow_lists;
+	struct ixgbe_filter_ele_list flow_list;
 	struct ixgbe_l2_tn_info     l2_tn;
 	struct ixgbe_bw_conf        bw_conf;
 	struct ixgbe_ipsec          ipsec;

@@ -577,9 +567,6 @@ uint16_t ixgbe_vf_representor_tx_burst(void *tx_queue, struct rte_mbuf **tx_pkts
 #define IXGBE_DEV_PRIVATE_TO_FILTER_INFO(adapter) \
 	(&((struct ixgbe_adapter *)adapter)->filter)
 
-#define IXGBE_DEV_PRIVATE_TO_FLOW_LISTS(adapter) \
-	(&((struct ixgbe_adapter *)adapter)->flow_lists)
-
 #define IXGBE_DEV_PRIVATE_TO_L2_TN_INFO(adapter) \
 	(&((struct ixgbe_adapter *)adapter)->l2_tn)
drivers/net/intel/ixgbe/ixgbe_flow.c +20 -76

231 lines not on this page · raw message

Signature · 1 line
2.47.3
author
#12 lore raw

PATCH 10/15 net/ixgbe: reduce FDIR conf macro usage

In reply to Anatoly Burakov

Currently, there are quite a few places where FDIR_CONF macro is used repeatedly within the same function. Change these instances to only get the fdir conf pointer once, and use the pointer instead.

Signed-off-by Anatoly Burakov <[email protected]>
---
 drivers/net/intel/ixgbe/ixgbe_ethdev.c |  3 ++-
 drivers/net/intel/ixgbe/ixgbe_fdir.c   | 34 ++++++++++++++++----------
 2 files changed, 23 insertions(+), 14 deletions(-)
drivers/net/intel/ixgbe/ixgbe_ethdev.c +2 -1
index ee1b499b49..dc3aa49ec4 100644
@@ -2614,6 +2614,7 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
 {
 	struct ixgbe_hw *hw =
 		IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct rte_eth_fdir_conf *fdir_conf = IXGBE_DEV_FDIR_CONF(dev);
 	struct ixgbe_vf_info *vfinfo =
 		*IXGBE_DEV_PRIVATE_TO_P_VFDATA(dev->data->dev_private);
 	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);

@@ -2718,7 +2719,7 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
 	/* Configure DCB hw */
 	ixgbe_configure_dcb(dev);
 
-	if (IXGBE_DEV_FDIR_CONF(dev)->mode != RTE_FDIR_MODE_NONE) {
+	if (fdir_conf->mode != RTE_FDIR_MODE_NONE) {
 		err = ixgbe_fdir_configure(dev);
 		if (err)
 			goto error;
drivers/net/intel/ixgbe/ixgbe_fdir.c +21 -13
index 5f7159abf2..51557cf68d 100644
@@ -251,6 +251,7 @@ static int
 fdir_set_input_mask_82599(struct rte_eth_dev *dev)
 {
 	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct rte_eth_fdir_conf *fdir_conf = IXGBE_DEV_FDIR_CONF(dev);
 	struct ixgbe_hw_fdir_info *info =
 			IXGBE_DEV_PRIVATE_TO_FDIR_INFO(dev->data->dev_private);
 	/*

@@ -313,7 +314,7 @@ fdir_set_input_mask_82599(struct rte_eth_dev *dev)
 	reg = IXGBE_PCI_REG_ADDR(hw, IXGBE_FDIRDIP4M);
 	*reg = ~(info->mask.dst_ipv4_mask);
 
-	if (IXGBE_DEV_FDIR_CONF(dev)->mode == RTE_FDIR_MODE_SIGNATURE) {
+	if (fdir_conf->mode == RTE_FDIR_MODE_SIGNATURE) {
 		/*
 		 * Store source and destination IPv6 masks (bit reversed)
 		 */

@@ -334,6 +335,7 @@ static int
 fdir_set_input_mask_x550(struct rte_eth_dev *dev)
 {
 	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct rte_eth_fdir_conf *fdir_conf = IXGBE_DEV_FDIR_CONF(dev);
 	struct ixgbe_hw_fdir_info *info =
 			IXGBE_DEV_PRIVATE_TO_FDIR_INFO(dev->data->dev_private);
 	/* mask VM pool and DIPv6 since there are currently not supported

@@ -342,7 +344,7 @@ fdir_set_input_mask_x550(struct rte_eth_dev *dev)
 	uint32_t fdirm = IXGBE_FDIRM_POOL | IXGBE_FDIRM_DIPv6 |
 			 IXGBE_FDIRM_FLEX;
 	uint32_t fdiripv6m;
-	enum rte_fdir_mode mode = IXGBE_DEV_FDIR_CONF(dev)->mode;
+	enum rte_fdir_mode mode = fdir_conf->mode;
 	uint16_t mac_mask;
 
 	PMD_INIT_FUNC_TRACE();

@@ -425,7 +427,8 @@ fdir_set_input_mask_x550(struct rte_eth_dev *dev)
 int
 ixgbe_fdir_set_input_mask(struct rte_eth_dev *dev)
 {
-	enum rte_fdir_mode mode = IXGBE_DEV_FDIR_CONF(dev)->mode;
+	struct rte_eth_fdir_conf *fdir_conf = IXGBE_DEV_FDIR_CONF(dev);
+	enum rte_fdir_mode mode = fdir_conf->mode;
 
 	if (mode >= RTE_FDIR_MODE_SIGNATURE &&
 	    mode <= RTE_FDIR_MODE_PERFECT)

@@ -563,10 +566,11 @@ int
 ixgbe_fdir_configure(struct rte_eth_dev *dev)
 {
 	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct rte_eth_fdir_conf *fdir_conf = IXGBE_DEV_FDIR_CONF(dev);
 	int err;
 	uint32_t fdirctrl, pbsize;
 	int i;
-	enum rte_fdir_mode mode = IXGBE_DEV_FDIR_CONF(dev)->mode;
+	enum rte_fdir_mode mode = fdir_conf->mode;
 
 	PMD_INIT_FUNC_TRACE();
 

@@ -587,7 +591,7 @@ ixgbe_fdir_configure(struct rte_eth_dev *dev)
 	    mode != RTE_FDIR_MODE_PERFECT)
 		return -ENOSYS;
 
-	err = configure_fdir_flags(IXGBE_DEV_FDIR_CONF(dev), &fdirctrl);
+	err = configure_fdir_flags(fdir_conf, &fdirctrl);
 	if (err)
 		return err;
 

@@ -614,7 +618,7 @@ ixgbe_fdir_configure(struct rte_eth_dev *dev)
 		PMD_INIT_LOG(ERR, " Error on setting FD mask");
 		return err;
 	}
-	err = ixgbe_set_fdir_flex_conf(dev, &IXGBE_DEV_FDIR_CONF(dev)->flex_conf,
+	err = ixgbe_set_fdir_flex_conf(dev, &fdir_conf->flex_conf,
 				       &fdirctrl);
 	if (err < 0) {
 		PMD_INIT_LOG(ERR, " Error on setting FD flexible arguments.");

@@ -1043,6 +1047,7 @@ ixgbe_fdir_filter_program(struct rte_eth_dev *dev,
 			  bool update)
 {
 	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct rte_eth_fdir_conf *fdir_conf = IXGBE_DEV_FDIR_CONF(dev);
 	uint32_t fdircmd_flags;
 	uint32_t fdirhash;
 	uint8_t queue;

@@ -1050,7 +1055,7 @@ ixgbe_fdir_filter_program(struct rte_eth_dev *dev,
 	int err;
 	struct ixgbe_hw_fdir_info *info =
 		IXGBE_DEV_PRIVATE_TO_FDIR_INFO(dev->data->dev_private);
-	enum rte_fdir_mode fdir_mode = IXGBE_DEV_FDIR_CONF(dev)->mode;
+	enum rte_fdir_mode fdir_mode = fdir_conf->mode;
 	struct ixgbe_fdir_filter *node;
 	bool add_node = FALSE;
 

@@ -1095,12 +1100,12 @@ ixgbe_fdir_filter_program(struct rte_eth_dev *dev,
 			return -ENOTSUP;
 		}
 		fdirhash = atr_compute_perfect_hash_82599(&rule->ixgbe_fdir,
-							  IXGBE_DEV_FDIR_CONF(dev)->pballoc);
+				fdir_conf->pballoc);
 		fdirhash |= rule->soft_id <<
 			IXGBE_FDIRHASH_SIG_SW_INDEX_SHIFT;
 	} else
 		fdirhash = atr_compute_sig_hash_82599(&rule->ixgbe_fdir,
-						      IXGBE_DEV_FDIR_CONF(dev)->pballoc);
+				fdir_conf->pballoc);
 
 	if (del) {
 		err = ixgbe_remove_fdir_filter(info, &rule->ixgbe_fdir);

@@ -1118,7 +1123,7 @@ ixgbe_fdir_filter_program(struct rte_eth_dev *dev,
 	fdircmd_flags = (update) ? IXGBE_FDIRCMD_FILTER_UPDATE : 0;
 	if (rule->fdirflags & IXGBE_FDIRCMD_DROP) {
 		if (is_perfect) {
-			queue = IXGBE_DEV_FDIR_CONF(dev)->drop_queue;
+			queue = fdir_conf->drop_queue;
 			fdircmd_flags |= IXGBE_FDIRCMD_DROP;
 		} else {
 			PMD_DRV_LOG(ERR, "Drop option is not supported in"

@@ -1209,6 +1214,7 @@ void
 ixgbe_fdir_info_get(struct rte_eth_dev *dev, struct rte_eth_fdir_info *fdir_info)
 {
 	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct rte_eth_fdir_conf *fdir_conf = IXGBE_DEV_FDIR_CONF(dev);
 	struct ixgbe_hw_fdir_info *info =
 			IXGBE_DEV_PRIVATE_TO_FDIR_INFO(dev->data->dev_private);
 	uint32_t fdirctrl, max_num;

@@ -1218,7 +1224,7 @@ ixgbe_fdir_info_get(struct rte_eth_dev *dev, struct rte_eth_fdir_info *fdir_info
 	offset = ((fdirctrl & IXGBE_FDIRCTRL_FLEX_MASK) >>
 			IXGBE_FDIRCTRL_FLEX_SHIFT) * sizeof(uint16_t);
 
-	fdir_info->mode = IXGBE_DEV_FDIR_CONF(dev)->mode;
+	fdir_info->mode = fdir_conf->mode;
 	max_num = (1 << (FDIRENTRIES_NUM_SHIFT +
 			(fdirctrl & FDIRCTRL_PBALLOC_MASK)));
 	if (fdir_info->mode >= RTE_FDIR_MODE_PERFECT &&

@@ -1268,10 +1274,11 @@ void
 ixgbe_fdir_stats_get(struct rte_eth_dev *dev, struct rte_eth_fdir_stats *fdir_stats)
 {
 	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct rte_eth_fdir_conf *fdir_conf = IXGBE_DEV_FDIR_CONF(dev);
 	struct ixgbe_hw_fdir_info *info =
 		IXGBE_DEV_PRIVATE_TO_FDIR_INFO(dev->data->dev_private);
 	uint32_t reg, max_num;
-	enum rte_fdir_mode fdir_mode = IXGBE_DEV_FDIR_CONF(dev)->mode;
+	enum rte_fdir_mode fdir_mode = fdir_conf->mode;
 
 	/* Get the information from registers */
 	reg = IXGBE_READ_REG(hw, IXGBE_FDIRFREE);

@@ -1324,11 +1331,12 @@ void
 ixgbe_fdir_filter_restore(struct rte_eth_dev *dev)
 {
 	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct rte_eth_fdir_conf *fdir_conf = IXGBE_DEV_FDIR_CONF(dev);
 	struct ixgbe_hw_fdir_info *fdir_info =
 		IXGBE_DEV_PRIVATE_TO_FDIR_INFO(dev->data->dev_private);
 	struct ixgbe_fdir_filter *node;
 	bool is_perfect = FALSE;
-	enum rte_fdir_mode fdir_mode = IXGBE_DEV_FDIR_CONF(dev)->mode;
+	enum rte_fdir_mode fdir_mode = fdir_conf->mode;
 
 	if (fdir_mode >= RTE_FDIR_MODE_PERFECT &&
 	    fdir_mode <= RTE_FDIR_MODE_PERFECT_TUNNEL)
Signature · 1 line
2.47.3
author
#13 lore raw

PATCH 11/15 net/ixgbe: use adapter in flow-related calls

In reply to Anatoly Burakov

Currently, a lot of rte_flow-related code paths depend on using the dev pointer. This has been okay up until now, because all the infrastructure surrounding rte_flow has been ad-hoc and did not have any persistent driver identification mechanism that works across multiple drivers, so any API call was tied to immediate rte_eth_dev API invocation.

However, with coming shared infrastructure, we can no longer rely on things that are process-local (such as dev pointer), and because most calls can be implemented using adapter anyway, we'll just switch the flow-related internal calls to use adapter instead of dev.

Signed-off-by Anatoly Burakov <[email protected]>
---
 drivers/net/intel/ixgbe/ixgbe_ethdev.c | 94 +++++++++++++++-----------
 drivers/net/intel/ixgbe/ixgbe_ethdev.h | 23 ++++---
 drivers/net/intel/ixgbe/ixgbe_fdir.c   | 65 ++++++++++--------
 drivers/net/intel/ixgbe/ixgbe_flow.c   | 45 ++++++------
 drivers/net/intel/ixgbe/ixgbe_rxtx.c   | 10 +--
 5 files changed, 131 insertions(+), 106 deletions(-)
drivers/net/intel/ixgbe/ixgbe_ethdev.c +53 -41
index dc3aa49ec4..80d70fe083 100644
@@ -302,9 +302,9 @@ static int ixgbevf_add_mac_addr(struct rte_eth_dev *dev,
 static void ixgbevf_remove_mac_addr(struct rte_eth_dev *dev, uint32_t index);
 static int ixgbevf_set_default_mac_addr(struct rte_eth_dev *dev,
 					     struct rte_ether_addr *mac_addr);
-static int ixgbe_add_5tuple_filter(struct rte_eth_dev *dev,
+static int ixgbe_add_5tuple_filter(struct ixgbe_adapter *adapter,
 			struct ixgbe_5tuple_filter *filter);
-static void ixgbe_remove_5tuple_filter(struct rte_eth_dev *dev,
+static void ixgbe_remove_5tuple_filter(struct ixgbe_adapter *adapter,
 			struct ixgbe_5tuple_filter *filter);
 static int ixgbe_dev_flow_ops_get(struct rte_eth_dev *dev,
 				  const struct rte_flow_ops **ops);

@@ -2612,8 +2612,9 @@ ixgbe_flow_ctrl_enable(struct rte_eth_dev *dev, struct ixgbe_hw *hw)
 static int
 ixgbe_dev_start(struct rte_eth_dev *dev)
 {
-	struct ixgbe_hw *hw =
-		IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct ixgbe_adapter *adapter =
+		IXGBE_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
+	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(adapter);
 	struct rte_eth_fdir_conf *fdir_conf = IXGBE_DEV_FDIR_CONF(dev);
 	struct ixgbe_vf_info *vfinfo =
 		*IXGBE_DEV_PRIVATE_TO_P_VFDATA(dev->data->dev_private);

@@ -2720,7 +2721,7 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
 	ixgbe_configure_dcb(dev);
 
 	if (fdir_conf->mode != RTE_FDIR_MODE_NONE) {
-		err = ixgbe_fdir_configure(dev);
+		err = ixgbe_fdir_configure(adapter);
 		if (err)
 			goto error;
 	}

@@ -6446,13 +6447,13 @@ ixgbevf_set_default_mac_addr(struct rte_eth_dev *dev,
 }
 
 int
-ixgbe_syn_filter_set(struct rte_eth_dev *dev,
+ixgbe_syn_filter_set(struct ixgbe_adapter *adapter,
 			struct rte_eth_syn_filter *filter,
 			bool add)
 {
-	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(adapter);
 	struct ixgbe_filter_info *filter_info =
-		IXGBE_DEV_PRIVATE_TO_FILTER_INFO(dev->data->dev_private);
+		IXGBE_DEV_PRIVATE_TO_FILTER_INFO(adapter);
 	uint32_t syn_info;
 	uint32_t synqf;
 

@@ -6500,10 +6501,10 @@ convert_protocol_type(uint8_t protocol_value)
 
 /* inject a 5-tuple filter to HW */
 static inline void
-ixgbe_inject_5tuple_filter(struct rte_eth_dev *dev,
+ixgbe_inject_5tuple_filter(struct ixgbe_adapter *adapter,
 			   struct ixgbe_5tuple_filter *filter)
 {
-	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(adapter);
 	int i;
 	uint32_t ftqf, sdpqf;
 	uint32_t l34timir = 0;

@@ -6558,11 +6559,11 @@ ixgbe_inject_5tuple_filter(struct rte_eth_dev *dev,
  *    - On failure, a negative value.
  */
 static int
-ixgbe_add_5tuple_filter(struct rte_eth_dev *dev,
+ixgbe_add_5tuple_filter(struct ixgbe_adapter *adapter,
 			struct ixgbe_5tuple_filter *filter)
 {
 	struct ixgbe_filter_info *filter_info =
-		IXGBE_DEV_PRIVATE_TO_FILTER_INFO(dev->data->dev_private);
+		IXGBE_DEV_PRIVATE_TO_FILTER_INFO(adapter);
 	int i, idx, shift;
 
 	/*

@@ -6586,7 +6587,7 @@ ixgbe_add_5tuple_filter(struct rte_eth_dev *dev,
 		return -ENOSYS;
 	}
 
-	ixgbe_inject_5tuple_filter(dev, filter);
+	ixgbe_inject_5tuple_filter(adapter, filter);
 
 	return 0;
 }

@@ -6599,12 +6600,12 @@ ixgbe_add_5tuple_filter(struct rte_eth_dev *dev,
  * filter: the pointer of the filter will be removed.
  */
 static void
-ixgbe_remove_5tuple_filter(struct rte_eth_dev *dev,
+ixgbe_remove_5tuple_filter(struct ixgbe_adapter *adapter,
 			struct ixgbe_5tuple_filter *filter)
 {
-	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(adapter);
 	struct ixgbe_filter_info *filter_info =
-		IXGBE_DEV_PRIVATE_TO_FILTER_INFO(dev->data->dev_private);
+		IXGBE_DEV_PRIVATE_TO_FILTER_INFO(adapter);
 	uint16_t index = filter->index;
 
 	filter_info->fivetuple_mask[index / (sizeof(uint32_t) * NBBY)] &=

@@ -6772,12 +6773,12 @@ ntuple_filter_to_5tuple(struct rte_eth_ntuple_filter *filter,
  *    - On failure, a negative value.
  */
 int
-ixgbe_add_del_ntuple_filter(struct rte_eth_dev *dev,
+ixgbe_add_del_ntuple_filter(struct ixgbe_adapter *adapter,
 			struct rte_eth_ntuple_filter *ntuple_filter,
 			bool add)
 {
 	struct ixgbe_filter_info *filter_info =
-		IXGBE_DEV_PRIVATE_TO_FILTER_INFO(dev->data->dev_private);
+		IXGBE_DEV_PRIVATE_TO_FILTER_INFO(adapter);
 	struct ixgbe_5tuple_filter_info filter_5tuple;
 	struct ixgbe_5tuple_filter *filter;
 	int ret;

@@ -6812,25 +6813,25 @@ ixgbe_add_del_ntuple_filter(struct rte_eth_dev *dev,
 				 &filter_5tuple,
 				 sizeof(struct ixgbe_5tuple_filter_info));
 		filter->queue = ntuple_filter->queue;
-		ret = ixgbe_add_5tuple_filter(dev, filter);
+		ret = ixgbe_add_5tuple_filter(adapter, filter);
 		if (ret < 0) {
 			rte_free(filter);
 			return ret;
 		}
 	} else
-		ixgbe_remove_5tuple_filter(dev, filter);
+		ixgbe_remove_5tuple_filter(adapter, filter);
 
 	return 0;
 }
 
 int
-ixgbe_add_del_ethertype_filter(struct rte_eth_dev *dev,
+ixgbe_add_del_ethertype_filter(struct ixgbe_adapter *adapter,
 			struct rte_eth_ethertype_filter *filter,
 			bool add)
 {
-	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(adapter);
 	struct ixgbe_filter_info *filter_info =
-		IXGBE_DEV_PRIVATE_TO_FILTER_INFO(dev->data->dev_private);
+		IXGBE_DEV_PRIVATE_TO_FILTER_INFO(adapter);
 	uint32_t etqf = 0;
 	uint32_t etqs = 0;
 	int ret;

@@ -7700,11 +7701,11 @@ ixgbe_e_tag_enable(struct ixgbe_hw *hw)
 }
 
 static int
-ixgbe_e_tag_filter_del(struct rte_eth_dev *dev,
+ixgbe_e_tag_filter_del(struct ixgbe_adapter *adapter,
 		       struct ixgbe_l2_tunnel_conf *l2_tunnel)
 {
 	int ret = 0;
-	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(adapter);
 	uint32_t i, rar_entries;
 	uint32_t rar_low, rar_high;
 

@@ -7737,11 +7738,11 @@ ixgbe_e_tag_filter_del(struct rte_eth_dev *dev,
 }
 
 static int
-ixgbe_e_tag_filter_add(struct rte_eth_dev *dev,
+ixgbe_e_tag_filter_add(struct ixgbe_adapter *adapter,
 		       struct ixgbe_l2_tunnel_conf *l2_tunnel)
 {
 	int ret = 0;
-	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(adapter);
 	uint32_t i, rar_entries;
 	uint32_t rar_low, rar_high;
 

@@ -7753,7 +7754,7 @@ ixgbe_e_tag_filter_add(struct rte_eth_dev *dev,
 	}
 
 	/* One entry for one tunnel. Try to remove potential existing entry. */
-	ixgbe_e_tag_filter_del(dev, l2_tunnel);
+	ixgbe_e_tag_filter_del(adapter, l2_tunnel);
 
 	rar_entries = ixgbe_get_num_rx_addrs(hw);
 

@@ -7842,13 +7843,13 @@ ixgbe_remove_l2_tn_filter(struct ixgbe_l2_tn_info *l2_tn_info,
 
 /* Add l2 tunnel filter */
 int
-ixgbe_dev_l2_tunnel_filter_add(struct rte_eth_dev *dev,
+ixgbe_dev_l2_tunnel_filter_add(struct ixgbe_adapter *adapter,
 			       struct ixgbe_l2_tunnel_conf *l2_tunnel,
 			       bool restore)
 {
 	int ret;
 	struct ixgbe_l2_tn_info *l2_tn_info =
-		IXGBE_DEV_PRIVATE_TO_L2_TN_INFO(dev->data->dev_private);
+		IXGBE_DEV_PRIVATE_TO_L2_TN_INFO(adapter);
 	struct ixgbe_l2_tn_key key;
 	struct ixgbe_l2_tn_filter *node;
 

@@ -7883,7 +7884,7 @@ ixgbe_dev_l2_tunnel_filter_add(struct rte_eth_dev *dev,
 
 	switch (l2_tunnel->l2_tunnel_type) {
 	case RTE_ETH_L2_TUNNEL_TYPE_E_TAG:
-		ret = ixgbe_e_tag_filter_add(dev, l2_tunnel);
+		ret = ixgbe_e_tag_filter_add(adapter, l2_tunnel);
 		break;
 	default:
 		PMD_DRV_LOG(ERR, "Invalid tunnel type");

@@ -7899,12 +7900,12 @@ ixgbe_dev_l2_tunnel_filter_add(struct rte_eth_dev *dev,
 
 /* Delete l2 tunnel filter */
 int
-ixgbe_dev_l2_tunnel_filter_del(struct rte_eth_dev *dev,
+ixgbe_dev_l2_tunnel_filter_del(struct ixgbe_adapter *adapter,
 			       struct ixgbe_l2_tunnel_conf *l2_tunnel)
 {
 	int ret;
 	struct ixgbe_l2_tn_info *l2_tn_info =
-		IXGBE_DEV_PRIVATE_TO_L2_TN_INFO(dev->data->dev_private);
+		IXGBE_DEV_PRIVATE_TO_L2_TN_INFO(adapter);
 	struct ixgbe_l2_tn_key key;
 
 	key.l2_tn_type = l2_tunnel->l2_tunnel_type;

@@ -7915,7 +7916,7 @@ ixgbe_dev_l2_tunnel_filter_del(struct rte_eth_dev *dev,
 
 	switch (l2_tunnel->l2_tunnel_type) {
 	case RTE_ETH_L2_TUNNEL_TYPE_E_TAG:
-		ret = ixgbe_e_tag_filter_del(dev, l2_tunnel);
+		ret = ixgbe_e_tag_filter_del(adapter, l2_tunnel);
 		break;
 	default:
 		PMD_DRV_LOG(ERR, "Invalid tunnel type");

@@ -8312,12 +8313,14 @@ int ixgbe_enable_sec_tx_path_generic(struct ixgbe_hw *hw)
 static inline void
 ixgbe_ntuple_filter_restore(struct rte_eth_dev *dev)
 {
+	struct ixgbe_adapter *adapter =
+		IXGBE_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
 	struct ixgbe_filter_info *filter_info =
 		IXGBE_DEV_PRIVATE_TO_FILTER_INFO(dev->data->dev_private);
 	struct ixgbe_5tuple_filter *node;
 
 	TAILQ_FOREACH(node, &filter_info->fivetuple_list, entries) {
-		ixgbe_inject_5tuple_filter(dev, node);
+		ixgbe_inject_5tuple_filter(adapter, node);
 	}
 }
 

@@ -8362,8 +8365,10 @@ ixgbe_syn_filter_restore(struct rte_eth_dev *dev)
 static inline void
 ixgbe_l2_tn_filter_restore(struct rte_eth_dev *dev)
 {
+	struct ixgbe_adapter *adapter =
+		IXGBE_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
 	struct ixgbe_l2_tn_info *l2_tn_info =
-		IXGBE_DEV_PRIVATE_TO_L2_TN_INFO(dev->data->dev_private);
+		IXGBE_DEV_PRIVATE_TO_L2_TN_INFO(adapter);
 	struct ixgbe_l2_tn_filter *node;
 	struct ixgbe_l2_tunnel_conf l2_tn_conf;
 

@@ -8371,7 +8376,8 @@ ixgbe_l2_tn_filter_restore(struct rte_eth_dev *dev)
 		l2_tn_conf.l2_tunnel_type = node->key.l2_tn_type;
 		l2_tn_conf.tunnel_id      = node->key.tn_id;
 		l2_tn_conf.pool           = node->pool;
-		(void)ixgbe_dev_l2_tunnel_filter_add(dev, &l2_tn_conf, TRUE);
+		(void)ixgbe_dev_l2_tunnel_filter_add(adapter,
+						     &l2_tn_conf, TRUE);
 	}
 }
 

@@ -8379,11 +8385,13 @@ ixgbe_l2_tn_filter_restore(struct rte_eth_dev *dev)
 static inline void
 ixgbe_rss_filter_restore(struct rte_eth_dev *dev)
 {
+	struct ixgbe_adapter *adapter =
+		IXGBE_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
 	struct ixgbe_filter_info *filter_info =
-		IXGBE_DEV_PRIVATE_TO_FILTER_INFO(dev->data->dev_private);
+		IXGBE_DEV_PRIVATE_TO_FILTER_INFO(adapter);
 
 	if (filter_info->rss_info.conf.queue_num)
-		ixgbe_config_rss_filter(dev,
+		ixgbe_config_rss_filter(adapter,
 			&filter_info->rss_info, TRUE);
 }
 

@@ -8420,12 +8428,14 @@ ixgbe_l2_tunnel_conf(struct rte_eth_dev *dev)
 void
 ixgbe_clear_all_ntuple_filter(struct rte_eth_dev *dev)
 {
+	struct ixgbe_adapter *adapter =
+		IXGBE_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
 	struct ixgbe_filter_info *filter_info =
 		IXGBE_DEV_PRIVATE_TO_FILTER_INFO(dev->data->dev_private);
 	struct ixgbe_5tuple_filter *p_5tuple;
 
 	while ((p_5tuple = TAILQ_FIRST(&filter_info->fivetuple_list)))
-		ixgbe_remove_5tuple_filter(dev, p_5tuple);
+		ixgbe_remove_5tuple_filter(adapter, p_5tuple);
 }
 
 /* remove all the ether type filters */

@@ -8469,6 +8479,8 @@ ixgbe_clear_syn_filter(struct rte_eth_dev *dev)
 int
 ixgbe_clear_all_l2_tn_filter(struct rte_eth_dev *dev)
 {
+	struct ixgbe_adapter *adapter =
+			IXGBE_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
 	struct ixgbe_l2_tn_info *l2_tn_info =
 		IXGBE_DEV_PRIVATE_TO_L2_TN_INFO(dev->data->dev_private);
 	struct ixgbe_l2_tn_filter *l2_tn_filter;

@@ -8479,7 +8491,7 @@ ixgbe_clear_all_l2_tn_filter(struct rte_eth_dev *dev)
 		l2_tn_conf.l2_tunnel_type = l2_tn_filter->key.l2_tn_type;
 		l2_tn_conf.tunnel_id      = l2_tn_filter->key.tn_id;
 		l2_tn_conf.pool           = l2_tn_filter->pool;
-		ret = ixgbe_dev_l2_tunnel_filter_del(dev, &l2_tn_conf);
+		ret = ixgbe_dev_l2_tunnel_filter_del(adapter, &l2_tn_conf);
 		if (ret < 0)
 			return ret;
 	}
drivers/net/intel/ixgbe/ixgbe_ethdev.h +13 -10
index f49a179082..65e71e7689 100644
@@ -525,6 +525,9 @@ uint16_t ixgbe_vf_representor_tx_burst(void *tx_queue, struct rte_mbuf **tx_pkts
 #define IXGBE_DEV_PRIVATE_TO_ADAPTER(adapter) \
 	((struct ixgbe_adapter *)adapter)
 
+#define IXGBE_DEV_PRIVATE_TO_FDIR_CONF(adapter) \
+	(&(adapter)->fdir_conf)
+
 #define IXGBE_DEV_PRIVATE_TO_HW(adapter)\
 	(&((struct ixgbe_adapter *)adapter)->hw)
 

@@ -669,13 +672,13 @@ uint32_t ixgbe_rssrk_reg_get(enum ixgbe_mac_type mac_type, uint8_t i);
 
 bool ixgbe_rss_update_sp(enum ixgbe_mac_type mac_type);
 
-int ixgbe_add_del_ntuple_filter(struct rte_eth_dev *dev,
+int ixgbe_add_del_ntuple_filter(struct ixgbe_adapter *adapter,
 			struct rte_eth_ntuple_filter *filter,
 			bool add);
-int ixgbe_add_del_ethertype_filter(struct rte_eth_dev *dev,
+int ixgbe_add_del_ethertype_filter(struct ixgbe_adapter *adapter,
 			struct rte_eth_ethertype_filter *filter,
 			bool add);
-int ixgbe_syn_filter_set(struct rte_eth_dev *dev,
+int ixgbe_syn_filter_set(struct ixgbe_adapter *adapter,
 			struct rte_eth_syn_filter *filter,
 			bool add);
 

@@ -691,22 +694,22 @@ struct ixgbe_l2_tunnel_conf {
 };
 
 int
-ixgbe_dev_l2_tunnel_filter_add(struct rte_eth_dev *dev,
+ixgbe_dev_l2_tunnel_filter_add(struct ixgbe_adapter *adapter,
 			       struct ixgbe_l2_tunnel_conf *l2_tunnel,
 			       bool restore);
 int
-ixgbe_dev_l2_tunnel_filter_del(struct rte_eth_dev *dev,
+ixgbe_dev_l2_tunnel_filter_del(struct ixgbe_adapter *adapter,
 			       struct ixgbe_l2_tunnel_conf *l2_tunnel);
 void ixgbe_filterlist_init(struct rte_eth_dev *dev);
 void ixgbe_filterlist_flush(struct rte_eth_dev *dev);
 /*
  * Flow director function prototypes
  */
-int ixgbe_fdir_configure(struct rte_eth_dev *dev);
-int ixgbe_fdir_set_input_mask(struct rte_eth_dev *dev);
-int ixgbe_fdir_set_flexbytes_offset(struct rte_eth_dev *dev,
+int ixgbe_fdir_configure(struct ixgbe_adapter *adapter);
+int ixgbe_fdir_set_input_mask(struct ixgbe_adapter *adapter);
+int ixgbe_fdir_set_flexbytes_offset(struct ixgbe_adapter *adapter,
 				    uint16_t offset);
-int ixgbe_fdir_filter_program(struct rte_eth_dev *dev,
+int ixgbe_fdir_filter_program(struct ixgbe_adapter *adapter,
 			      struct ixgbe_fdir_rule *rule,
 			      bool del, bool update);
 void ixgbe_fdir_info_get(struct rte_eth_dev *dev,

@@ -766,7 +769,7 @@ int ixgbe_rss_conf_init(struct ixgbe_rte_flow_rss_conf *out,
 			const struct rte_flow_action_rss *in);
 int ixgbe_action_rss_same(const struct rte_flow_action_rss *comp,
 			  const struct rte_flow_action_rss *with);
-int ixgbe_config_rss_filter(struct rte_eth_dev *dev,
+int ixgbe_config_rss_filter(struct ixgbe_adapter *adapter,
 		struct ixgbe_rte_flow_rss_conf *conf, bool add);
 
 void ixgbe_dev_macsec_register_enable(struct rte_eth_dev *dev,
drivers/net/intel/ixgbe/ixgbe_fdir.c +35 -30

151 lines not on this page · raw message

drivers/net/intel/ixgbe/ixgbe_flow.c +25 -20

180 lines not on this page · raw message

drivers/net/intel/ixgbe/ixgbe_rxtx.c +5 -5
index 3be0f0492a..60222693fe 100644
@@ -6132,7 +6132,7 @@ ixgbe_action_rss_same(const struct rte_flow_action_rss *comp,
 }
 
 int
-ixgbe_config_rss_filter(struct rte_eth_dev *dev,
+ixgbe_config_rss_filter(struct ixgbe_adapter *adapter,
 		struct ixgbe_rte_flow_rss_conf *conf, bool add)
 {
 	struct ixgbe_hw *hw;

@@ -6148,17 +6148,17 @@ ixgbe_config_rss_filter(struct rte_eth_dev *dev,
 		.rss_hf = conf->conf.types,
 	};
 	struct ixgbe_filter_info *filter_info =
-		IXGBE_DEV_PRIVATE_TO_FILTER_INFO(dev->data->dev_private);
+		IXGBE_DEV_PRIVATE_TO_FILTER_INFO(adapter);
 
 	PMD_INIT_FUNC_TRACE();
-	hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	hw = IXGBE_DEV_PRIVATE_TO_HW(adapter);
 
 	sp_reta_size = ixgbe_reta_size_get(hw->mac.type);
 
 	if (!add) {
 		if (ixgbe_action_rss_same(&filter_info->rss_info.conf,
 					  &conf->conf)) {
-			ixgbe_rss_disable(dev);
+			ixgbe_mrqc_rss_remove(hw);
 			memset(&filter_info->rss_info, 0,
 				sizeof(struct ixgbe_rte_flow_rss_conf));
 			return 0;

@@ -6188,7 +6188,7 @@ ixgbe_config_rss_filter(struct rte_eth_dev *dev,
 	 * the RSS hash of input packets.
 	 */
 	if ((rss_conf.rss_hf & IXGBE_RSS_OFFLOAD_ALL) == 0) {
-		ixgbe_rss_disable(dev);
+		ixgbe_mrqc_rss_remove(hw);
 		return 0;
 	}
 	if (rss_conf.rss_key == NULL)
Signature · 1 line
2.47.3
author
#14 lore raw

PATCH 12/15 net/ixgbe: support protocol-only TCP and UDP rules

In reply to Anatoly Burakov

Currently, ixgbe rte_flow parsing requires a mask for TCP and UDP items. This means TCP and UDP FDIR rules can only be programmed as port-based matches, while protocol-only matches are rejected even though they are supported in hardware.

Allow TCP and UDP items without a mask so rte_flow can express broad L4 matches that care about the protocol only. This makes TCP and UDP handling consistent with SCTP.

Signed-off-by Anatoly Burakov <[email protected]>
---
 drivers/net/intel/ixgbe/ixgbe_flow.c | 130 ++++++++++++++-------------
 1 file changed, 67 insertions(+), 63 deletions(-)
drivers/net/intel/ixgbe/ixgbe_flow.c +67 -63
index a6fcfe7574..eae81462f6 100644
@@ -2055,42 +2055,44 @@ ixgbe_parse_fdir_filter_normal(struct rte_eth_dev *dev,
 				item, "Not supported last point for range");
 			return -rte_errno;
 		}
-		/**
-		 * Only care about src & dst ports,
-		 * others should be masked.
-		 */
-		if (!item->mask) {
-			memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
-			rte_flow_error_set(error, EINVAL,
-				RTE_FLOW_ERROR_TYPE_ITEM,
-				item, "Not supported by fdir filter");
-			return -rte_errno;
-		}
-		rule->b_mask = TRUE;
 		tcp_mask = item->mask;
-		if (tcp_mask->hdr.sent_seq ||
-		    tcp_mask->hdr.recv_ack ||
-		    tcp_mask->hdr.data_off ||
-		    tcp_mask->hdr.tcp_flags ||
-		    tcp_mask->hdr.rx_win ||
-		    tcp_mask->hdr.cksum ||
-		    tcp_mask->hdr.tcp_urp) {
-			memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
-			rte_flow_error_set(error, EINVAL,
-				RTE_FLOW_ERROR_TYPE_ITEM,
-				item, "Not supported by fdir filter");
-			return -rte_errno;
-		}
-		rule->mask.src_port_mask = tcp_mask->hdr.src_port;
-		rule->mask.dst_port_mask = tcp_mask->hdr.dst_port;
+		if (tcp_mask != NULL) {
+			/**
+			 * Only care about src & dst ports,
+			 * others should be masked.
+			 */
+			rule->b_mask = TRUE;
+			if (tcp_mask->hdr.sent_seq ||
+			    tcp_mask->hdr.recv_ack ||
+			    tcp_mask->hdr.data_off ||
+			    tcp_mask->hdr.tcp_flags ||
+			    tcp_mask->hdr.rx_win ||
+			    tcp_mask->hdr.cksum ||
+			    tcp_mask->hdr.tcp_urp) {
+				memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
+				rte_flow_error_set(error, EINVAL,
+					RTE_FLOW_ERROR_TYPE_ITEM,
+					item, "Not supported by fdir filter");
+				return -rte_errno;
+			}
+			rule->mask.src_port_mask = tcp_mask->hdr.src_port;
+			rule->mask.dst_port_mask = tcp_mask->hdr.dst_port;
 
-		if (item->spec) {
-			rule->b_spec = TRUE;
-			tcp_spec = item->spec;
-			rule->ixgbe_fdir.formatted.src_port =
-				tcp_spec->hdr.src_port;
-			rule->ixgbe_fdir.formatted.dst_port =
-				tcp_spec->hdr.dst_port;
+			if (item->spec) {
+				rule->b_spec = TRUE;
+				tcp_spec = item->spec;
+				rule->ixgbe_fdir.formatted.src_port =
+					tcp_spec->hdr.src_port;
+				rule->ixgbe_fdir.formatted.dst_port =
+					tcp_spec->hdr.dst_port;
+			}
+		} else if (item->spec != NULL) {
+			/* No port mask means protocol-only match; spec is invalid. */
+			memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
+			rte_flow_error_set(error, EINVAL,
+				RTE_FLOW_ERROR_TYPE_ITEM,
+				item, "Not supported by fdir filter");
+			return -rte_errno;
 		}
 
 		item = next_no_fuzzy_pattern(pattern, item);

@@ -2120,37 +2122,39 @@ ixgbe_parse_fdir_filter_normal(struct rte_eth_dev *dev,
 				item, "Not supported last point for range");
 			return -rte_errno;
 		}
-		/**
-		 * Only care about src & dst ports,
-		 * others should be masked.
-		 */
-		if (!item->mask) {
-			memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
-			rte_flow_error_set(error, EINVAL,
-				RTE_FLOW_ERROR_TYPE_ITEM,
-				item, "Not supported by fdir filter");
-			return -rte_errno;
-		}
-		rule->b_mask = TRUE;
 		udp_mask = item->mask;
-		if (udp_mask->hdr.dgram_len ||
-		    udp_mask->hdr.dgram_cksum) {
-			memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
-			rte_flow_error_set(error, EINVAL,
-				RTE_FLOW_ERROR_TYPE_ITEM,
-				item, "Not supported by fdir filter");
-			return -rte_errno;
-		}
-		rule->mask.src_port_mask = udp_mask->hdr.src_port;
-		rule->mask.dst_port_mask = udp_mask->hdr.dst_port;
+		if (udp_mask != NULL) {
+			/**
+			 * Only care about src & dst ports,
+			 * others should be masked.
+			 */
+			rule->b_mask = TRUE;
+			if (udp_mask->hdr.dgram_len ||
+			    udp_mask->hdr.dgram_cksum) {
+				memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
+				rte_flow_error_set(error, EINVAL,
+					RTE_FLOW_ERROR_TYPE_ITEM,
+					item, "Not supported by fdir filter");
+				return -rte_errno;
+			}
+			rule->mask.src_port_mask = udp_mask->hdr.src_port;
+			rule->mask.dst_port_mask = udp_mask->hdr.dst_port;
 
-		if (item->spec) {
-			rule->b_spec = TRUE;
-			udp_spec = item->spec;
-			rule->ixgbe_fdir.formatted.src_port =
-				udp_spec->hdr.src_port;
-			rule->ixgbe_fdir.formatted.dst_port =
-				udp_spec->hdr.dst_port;
+			if (item->spec) {
+				rule->b_spec = TRUE;
+				udp_spec = item->spec;
+				rule->ixgbe_fdir.formatted.src_port =
+					udp_spec->hdr.src_port;
+				rule->ixgbe_fdir.formatted.dst_port =
+					udp_spec->hdr.dst_port;
+			}
+		} else if (item->spec != NULL) {
+			/* No port mask means protocol-only match; spec is invalid. */
+			memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
+			rte_flow_error_set(error, EINVAL,
+				RTE_FLOW_ERROR_TYPE_ITEM,
+				item, "Not supported by fdir filter");
+			return -rte_errno;
 		}
 
 		item = next_no_fuzzy_pattern(pattern, item);
Signature · 1 line
2.47.3
author
#15 lore raw

PATCH 13/15 net/ixgbe: write drop queue at init

In reply to Anatoly Burakov

For FDIR, drop queue index never changes, so write it at init.

Signed-off-by Anatoly Burakov <[email protected]>
---
 drivers/net/intel/ixgbe/ixgbe_ethdev.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/net/intel/ixgbe/ixgbe_ethdev.c +3 -0
index 80d70fe083..62edada379 100644
@@ -1468,6 +1468,9 @@ static int ixgbe_fdir_filter_init(struct rte_eth_dev *eth_dev)
 	fdir_info->n_flows = 0;
 	fdir_info->mask_added = FALSE;
 
+	/* drop queue is always fixed */
+	IXGBE_DEV_FDIR_CONF(eth_dev)->drop_queue = IXGBE_FDIR_DROP_QUEUE;
+
 	return 0;
 }
Signature · 1 line
2.47.3
author
#16 lore raw

PATCH 14/15 net/ixgbe: rely less on global flow state

In reply to Anatoly Burakov

FDIR setup helpers were implicitly reading and updating global state. This made behavior depend on cached values and tightly coupled call ordering to hidden side effects.

Update ixgbe_fdir_configure(), ixgbe_fdir_set_input_mask(), and ixgbe_fdir_filter_program() to take explicit inputs (fdir_conf, mask, mode) and make the internal mask helpers use those arguments instead of reading globals.

Also remove implicit cached-state behavior from ixgbe_fdir_set_flexbytes_offset(): it no longer short-circuits on cached offset and no longer updates cached offset internally.

Adjust all current callers to match the new semantics:
- pass local config/mask/mode values into setup APIs,
- write global mode/mask/flex cached state in caller code only after successful programming.

Signed-off-by Anatoly Burakov <[email protected]>
---
 drivers/net/intel/ixgbe/ixgbe_ethdev.c |   4 +-
 drivers/net/intel/ixgbe/ixgbe_ethdev.h |  13 +++-
 drivers/net/intel/ixgbe/ixgbe_fdir.c   | 104 +++++++++++--------------
 drivers/net/intel/ixgbe/ixgbe_flow.c   |  32 +++++---
 4 files changed, 79 insertions(+), 74 deletions(-)
drivers/net/intel/ixgbe/ixgbe_ethdev.c +3 -1
index 62edada379..d4122107ac 100644
@@ -2724,7 +2724,9 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
 	ixgbe_configure_dcb(dev);
 
 	if (fdir_conf->mode != RTE_FDIR_MODE_NONE) {
-		err = ixgbe_fdir_configure(adapter);
+		struct ixgbe_hw_fdir_info *info =
+			IXGBE_DEV_PRIVATE_TO_FDIR_INFO(adapter);
+		err = ixgbe_fdir_configure(adapter, fdir_conf, &info->mask);
 		if (err)
 			goto error;
 	}
drivers/net/intel/ixgbe/ixgbe_ethdev.h +9 -4
index 65e71e7689..165bd93379 100644
@@ -705,13 +705,18 @@ void ixgbe_filterlist_flush(struct rte_eth_dev *dev);
 /*
  * Flow director function prototypes
  */
-int ixgbe_fdir_configure(struct ixgbe_adapter *adapter);
-int ixgbe_fdir_set_input_mask(struct ixgbe_adapter *adapter);
+int ixgbe_fdir_configure(struct ixgbe_adapter *adapter,
+			 const struct rte_eth_fdir_conf *fdir_conf,
+			 const struct ixgbe_hw_fdir_mask *fdir_mask);
+int ixgbe_fdir_set_input_mask(struct ixgbe_adapter *adapter,
+			      const struct ixgbe_hw_fdir_mask *mask,
+			      enum rte_fdir_mode mode);
 int ixgbe_fdir_set_flexbytes_offset(struct ixgbe_adapter *adapter,
 				    uint16_t offset);
 int ixgbe_fdir_filter_program(struct ixgbe_adapter *adapter,
-			      struct ixgbe_fdir_rule *rule,
-			      bool del, bool update);
+		struct rte_eth_fdir_conf *fdir_conf,
+		struct ixgbe_fdir_rule *rule,
+		bool del, bool update);
 void ixgbe_fdir_info_get(struct rte_eth_dev *dev,
 			 struct rte_eth_fdir_info *fdir_info);
 void ixgbe_fdir_stats_get(struct rte_eth_dev *dev,
drivers/net/intel/ixgbe/ixgbe_fdir.c +46 -58

248 lines not on this page · raw message

drivers/net/intel/ixgbe/ixgbe_flow.c +21 -11
index eae81462f6..cda2e95d22 100644
@@ -2838,7 +2838,6 @@ ixgbe_parse_fdir_filter(struct rte_eth_dev *dev,
 	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct ixgbe_adapter *adapter = IXGBE_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
 	struct rte_eth_fdir_conf *fdir_conf = IXGBE_DEV_FDIR_CONF(dev);
-	fdir_conf->drop_queue = IXGBE_FDIR_DROP_QUEUE;
 
 	if (hw->mac.type != ixgbe_mac_82599EB &&
 		hw->mac.type != ixgbe_mac_X540 &&

@@ -2863,12 +2862,15 @@ ixgbe_parse_fdir_filter(struct rte_eth_dev *dev,
 step_next:
 
 	if (fdir_conf->mode == RTE_FDIR_MODE_NONE) {
-		fdir_conf->mode = rule->mode;
-		ret = ixgbe_fdir_configure(adapter);
+		struct rte_eth_fdir_conf fc = *fdir_conf;
+
+		fc.mode = rule->mode;
+		ret = ixgbe_fdir_configure(adapter, &fc, &rule->mask);
 		if (ret) {
 			fdir_conf->mode = RTE_FDIR_MODE_NONE;
 			return ret;
 		}
+		fdir_conf->mode = rule->mode;
 	} else if (fdir_conf->mode != rule->mode) {
 		return -ENOTSUP;
 	}

@@ -3167,19 +3169,25 @@ ixgbe_flow_create(struct rte_eth_dev *dev,
 		/* A mask cannot be deleted. */
 		if (fdir_rule.b_mask) {
 			if (!fdir_info->mask_added) {
-				/* It's the first time the mask is set. */
-				*&fdir_info->mask = *&fdir_rule.mask;
+				bool flex_byte_offset_changed =
+					fdir_info->flex_bytes_offset !=
+					fdir_rule.flex_bytes_offset;
 
-				if (fdir_rule.mask.flex_bytes_mask) {
+				if (fdir_rule.mask.flex_bytes_mask &&
+				    flex_byte_offset_changed) {
 					ret = ixgbe_fdir_set_flexbytes_offset(adapter,
 						fdir_rule.flex_bytes_offset);
 					if (ret)
 						goto out;
 				}
-				ret = ixgbe_fdir_set_input_mask(adapter);
+				ret = ixgbe_fdir_set_input_mask(adapter,
+						&fdir_rule.mask, fdir_rule.mode);
 				if (ret)
 					goto out;
 
+				fdir_info->mask = fdir_rule.mask;
+				fdir_info->flex_bytes_offset =
+					fdir_rule.flex_bytes_offset;
 				fdir_info->mask_added = TRUE;
 				first_mask = TRUE;
 			} else {

@@ -3201,8 +3209,10 @@ ixgbe_flow_create(struct rte_eth_dev *dev,
 		}
 
 		if (fdir_rule.b_spec) {
-			ret = ixgbe_fdir_filter_program(adapter, &fdir_rule,
-					FALSE, FALSE);
+			struct rte_eth_fdir_conf *fdir_conf = IXGBE_DEV_FDIR_CONF(dev);
+
+			ret = ixgbe_fdir_filter_program(adapter, fdir_conf,
+					&fdir_rule, FALSE, FALSE);
 			if (!ret) {
 				fdir_rule_ptr = rte_zmalloc("ixgbe_fdir_filter",
 					sizeof(struct ixgbe_fdir_rule_ele), 0);

@@ -3374,6 +3384,7 @@ ixgbe_flow_destroy(struct rte_eth_dev *dev,
 	struct ixgbe_filter_ele_base *flow_mem_base;
 	struct ixgbe_hw_fdir_info *fdir_info =
 		IXGBE_DEV_PRIVATE_TO_FDIR_INFO(adapter);
+	struct rte_eth_fdir_conf *fdir_conf = IXGBE_DEV_FDIR_CONF(dev);
 	struct ixgbe_rss_conf_ele *rss_filter_ptr;
 
 	/* Validate ownership before touching HW/SW state. */

@@ -3433,9 +3444,8 @@ ixgbe_flow_destroy(struct rte_eth_dev *dev,
 		rte_memcpy(&fdir_rule,
 			&fdir_rule_ptr->filter_info,
 			sizeof(struct ixgbe_fdir_rule));
-		ret = ixgbe_fdir_filter_program(adapter, &fdir_rule, TRUE, FALSE);
+		ret = ixgbe_fdir_filter_program(adapter, fdir_conf, &fdir_rule, TRUE, FALSE);
 		if (!ret) {
-			struct rte_eth_fdir_conf *fdir_conf = IXGBE_DEV_FDIR_CONF(dev);
 			rte_free(fdir_rule_ptr);
 			if (fdir_info->n_flows > 0 && --(fdir_info->n_flows) == 0) {
 				fdir_info->mask_added = false;
Signature · 1 line
2.47.3
author
#17 lore raw

PATCH 15/15 net/ixgbe: refactor flow creation

In reply to Anatoly Burakov

For FDIR, the flow creation process is very complex and tangled in global state updates. To simplify the code, refactor global state handling and move it out of the main flow create function.

Signed-off-by Anatoly Burakov <[email protected]>
---
 drivers/net/intel/ixgbe/ixgbe_flow.c | 243 ++++++++++++++++-----------
 1 file changed, 144 insertions(+), 99 deletions(-)
drivers/net/intel/ixgbe/ixgbe_flow.c +144 -99
index cda2e95d22..7c0cdf8b74 100644
@@ -2828,16 +2828,14 @@ ixgbe_parse_fdir_filter_tunnel(const struct rte_flow_attr *attr,
 
 static int
 ixgbe_parse_fdir_filter(struct rte_eth_dev *dev,
-			const struct rte_flow_attr *attr,
-			const struct rte_flow_item pattern[],
-			const struct rte_flow_action actions[],
-			struct ixgbe_fdir_rule *rule,
-			struct rte_flow_error *error)
+		const struct rte_flow_attr *attr,
+		const struct rte_flow_item pattern[],
+		const struct rte_flow_action actions[],
+		struct ixgbe_fdir_rule *rule,
+		struct rte_flow_error *error)
 {
 	int ret;
 	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-	struct ixgbe_adapter *adapter = IXGBE_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
-	struct rte_eth_fdir_conf *fdir_conf = IXGBE_DEV_FDIR_CONF(dev);
 
 	if (hw->mac.type != ixgbe_mac_82599EB &&
 		hw->mac.type != ixgbe_mac_X540 &&

@@ -2849,36 +2847,131 @@ ixgbe_parse_fdir_filter(struct rte_eth_dev *dev,
 
 	ret = ixgbe_parse_fdir_filter_normal(dev, attr, pattern,
 					actions, rule, error);
-
 	if (!ret)
-		goto step_next;
+		return 0;
 
-	ret = ixgbe_parse_fdir_filter_tunnel(attr, pattern,
+	return ixgbe_parse_fdir_filter_tunnel(attr, pattern,
 					actions, rule, error);
+}
 
+static int
+ixgbe_fdir_process_rule(struct ixgbe_adapter *adapter,
+		struct ixgbe_hw_fdir_info *fdir_info,
+		struct ixgbe_fdir_rule *fdir_rule,
+		bool *first_mask,
+		struct rte_flow_error *error)
+{
+	bool flex_byte_offset_changed;
+	int ret;
+
+	/* rule must have a spec to be valid */
+	if (!fdir_rule->b_spec)
+		return rte_flow_error_set(error, EINVAL,
+			RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+			NULL, "No filter spec");
+
+	/* if rule doesn't have a mask, nothing to be done */
+	if (!fdir_rule->b_mask)
+		return 0;
+
+	/* if we already have a mask, check if it's compatible */
+	if (fdir_info->mask_added) {
+		ret = memcmp(&fdir_info->mask, &fdir_rule->mask,
+				sizeof(struct ixgbe_hw_fdir_mask));
+		if (ret)
+			return rte_flow_error_set(error, EINVAL,
+				RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+				NULL, "Mask mismatch");
+
+		if (fdir_rule->mask.flex_bytes_mask &&
+		    fdir_info->flex_bytes_offset !=
+		    fdir_rule->flex_bytes_offset)
+			return rte_flow_error_set(error, EINVAL,
+				RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+				NULL, "Flex bytes offset mismatch");
+
+		/* success */
+		return 0;
+	}
+
+	/* we don't have a mask yet, so set it up based on this rule */
+	flex_byte_offset_changed =
+			fdir_info->flex_bytes_offset !=
+			fdir_rule->flex_bytes_offset;
+
+	if (fdir_rule->mask.flex_bytes_mask != 0 && flex_byte_offset_changed) {
+		ret = ixgbe_fdir_set_flexbytes_offset(adapter,
+				fdir_rule->flex_bytes_offset);
+		if (ret)
+			return rte_flow_error_set(error, EINVAL,
+				RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+				NULL,
+				"Failed to set flex bytes offset");
+	}
+
+	ret = ixgbe_fdir_set_input_mask(adapter, &fdir_rule->mask,
+			fdir_rule->mode);
 	if (ret)
-		return ret;
+		return rte_flow_error_set(error, EINVAL,
+			RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+			NULL, "Failed to set fdir mask");
 
-step_next:
+	/* let the caller know that we've installed a mask */
+	*first_mask = true;
 
+	return 0;
+}
+
+static int
+ixgbe_fdir_flow_program(struct rte_eth_dev *dev,
+		struct ixgbe_adapter *adapter,
+		struct ixgbe_fdir_rule *fdir_rule,
+		bool *first_mask,
+		struct rte_flow_error *error)
+{
+	struct rte_eth_fdir_conf *fdir_conf = IXGBE_DEV_FDIR_CONF(dev);
+	struct rte_eth_fdir_conf local_fdir_conf = *fdir_conf;
+	struct ixgbe_hw_fdir_info *fdir_info =
+			IXGBE_DEV_PRIVATE_TO_FDIR_INFO(adapter);
+	int ret;
+
+	if (fdir_rule->queue >= dev->data->nb_rx_queues) {
+		return rte_flow_error_set(error, EINVAL,
+			RTE_FLOW_ERROR_TYPE_ACTION,
+			NULL, "queue id > max number of queues");
+	}
+
+	local_fdir_conf.mode = fdir_rule->mode;
+
+	/* Configure FDIR mode if this is the first filter */
 	if (fdir_conf->mode == RTE_FDIR_MODE_NONE) {
-		struct rte_eth_fdir_conf fc = *fdir_conf;
-
-		fc.mode = rule->mode;
-		ret = ixgbe_fdir_configure(adapter, &fc, &rule->mask);
+		ret = ixgbe_fdir_configure(adapter, &local_fdir_conf, &fdir_rule->mask);
 		if (ret) {
-			fdir_conf->mode = RTE_FDIR_MODE_NONE;
-			return ret;
+			return rte_flow_error_set(error, EINVAL,
+				RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+				NULL, "Failed to configure fdir mode");
 		}
-		fdir_conf->mode = rule->mode;
-	} else if (fdir_conf->mode != rule->mode) {
-		return -ENOTSUP;
+	} else if (fdir_conf->mode != fdir_rule->mode) {
+		return rte_flow_error_set(error, EINVAL,
+			RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+			NULL, "Conflict with existing fdir mode");
 	}
 
-	if (rule->queue >= dev->data->nb_rx_queues)
-		return -ENOTSUP;
+	/* Process and validate rule spec and mask */
+	ret = ixgbe_fdir_process_rule(adapter, fdir_info, fdir_rule,
+		first_mask, error);
+	if (ret)
+		return ret;
 
-	return ret;
+	/* Program the filter */
+	ret = ixgbe_fdir_filter_program(adapter, &local_fdir_conf,
+			fdir_rule, FALSE, FALSE);
+	if (ret)
+		return rte_flow_error_set(error, EINVAL,
+			RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+			NULL, "Failed to add fdir filter");
+
+	return 0;
 }
 
 static int

@@ -3066,7 +3159,6 @@ ixgbe_flow_create(struct rte_eth_dev *dev,
 	struct ixgbe_fdir_rule_ele *fdir_rule_ptr;
 	struct ixgbe_rss_conf_ele *rss_filter_ptr;
 	struct ixgbe_flow_mem *ixgbe_flow_mem_ptr;
-	uint8_t first_mask = FALSE;
 
 	flow = rte_zmalloc("ixgbe_rte_flow", sizeof(struct rte_flow), 0);
 	if (!flow) {

@@ -3166,82 +3258,35 @@ ixgbe_flow_create(struct rte_eth_dev *dev,
 	ret = ixgbe_parse_fdir_filter(dev, attr, pattern,
 				actions, &fdir_rule, error);
 	if (!ret) {
-		/* A mask cannot be deleted. */
-		if (fdir_rule.b_mask) {
-			if (!fdir_info->mask_added) {
-				bool flex_byte_offset_changed =
-					fdir_info->flex_bytes_offset !=
-					fdir_rule.flex_bytes_offset;
-
-				if (fdir_rule.mask.flex_bytes_mask &&
-				    flex_byte_offset_changed) {
-					ret = ixgbe_fdir_set_flexbytes_offset(adapter,
-						fdir_rule.flex_bytes_offset);
-					if (ret)
-						goto out;
-				}
-				ret = ixgbe_fdir_set_input_mask(adapter,
-						&fdir_rule.mask, fdir_rule.mode);
-				if (ret)
-					goto out;
-
-				fdir_info->mask = fdir_rule.mask;
-				fdir_info->flex_bytes_offset =
-					fdir_rule.flex_bytes_offset;
-				fdir_info->mask_added = TRUE;
-				first_mask = TRUE;
-			} else {
-				/**
-				 * Only support one global mask,
-				 * all the masks should be the same.
-				 */
-				ret = memcmp(&fdir_info->mask,
-					&fdir_rule.mask,
-					sizeof(struct ixgbe_hw_fdir_mask));
-				if (ret)
-					goto out;
-
-				if (fdir_rule.mask.flex_bytes_mask &&
-				    fdir_info->flex_bytes_offset !=
-				    fdir_rule.flex_bytes_offset)
-					goto out;
-			}
+		struct rte_eth_fdir_conf *fdir_conf = IXGBE_DEV_FDIR_CONF(dev);
+		bool first_mask = false;
+
+		ret = ixgbe_fdir_flow_program(dev, adapter, &fdir_rule,
+			&first_mask, error);
+		if (ret)
+			goto out;
+
+		fdir_rule_ptr = rte_zmalloc("ixgbe_fdir_filter",
+				sizeof(struct ixgbe_fdir_rule_ele), 0);
+		if (!fdir_rule_ptr) {
+			PMD_DRV_LOG(ERR, "failed to allocate memory");
+			goto out;
 		}
-
-		if (fdir_rule.b_spec) {
-			struct rte_eth_fdir_conf *fdir_conf = IXGBE_DEV_FDIR_CONF(dev);
-
-			ret = ixgbe_fdir_filter_program(adapter, fdir_conf,
-					&fdir_rule, FALSE, FALSE);
-			if (!ret) {
-				fdir_rule_ptr = rte_zmalloc("ixgbe_fdir_filter",
-					sizeof(struct ixgbe_fdir_rule_ele), 0);
-				if (!fdir_rule_ptr) {
-					PMD_DRV_LOG(ERR, "failed to allocate memory");
-					goto out;
-				}
-				rte_memcpy(&fdir_rule_ptr->filter_info,
-					&fdir_rule,
-					sizeof(struct ixgbe_fdir_rule));
-				flow->rule = fdir_rule_ptr;
-				flow->filter_type = RTE_ETH_FILTER_FDIR;
-				fdir_info->n_flows++;
-
-				return flow;
-			}
-
-			if (ret) {
-				/**
-				 * clean the mask_added flag if fail to
-				 * program
-				 **/
-				if (first_mask)
-					fdir_info->mask_added = FALSE;
-				goto out;
-			}
+		/* update global state */
+		if (first_mask) {
+			fdir_info->mask_added = TRUE;
+			fdir_info->mask = fdir_rule.mask;
+			fdir_info->flex_bytes_offset = fdir_rule.flex_bytes_offset;
 		}
-
-		goto out;
+		fdir_info->n_flows++;
+		fdir_conf->mode = fdir_rule.mode;
+
+		rte_memcpy(&fdir_rule_ptr->filter_info,
+			&fdir_rule,
+			sizeof(struct ixgbe_fdir_rule));
+		flow->rule = fdir_rule_ptr;
+		flow->filter_type = RTE_ETH_FILTER_FDIR;
+		return flow;
 	}
 
 	memset(&l2_tn_filter, 0, sizeof(struct ixgbe_l2_tunnel_conf));
Signature · 1 line
2.47.3
#18 lore raw
In reply to Anatoly Burakov
Quoted 18 lines · Anatoly Burakov

This patchset fixes a bunch of very old issues in flow management in IXGBE driver, such as:

- storing process-local pointers in shared memory
- incorrect L4 protocol matching for FDIR
- wrong handling of SCTP flow items
- reading stale FDIR state after flow destroy/flush
- storing all flows in global lists

In addition, some cleanup is also performed - refactors, moving things around to avoid accessing process-local state, and writing read-only values at init instead of deep in FDIR.

Finally, FDIR was also rejecting protocol-only matches for TCP and UDP, these are now supported.

Depends on flow dump patchset: https://patches.dpdk.org/project/dpdk/list/?series=38016

Quoted 17 lines

Anatoly Burakov (15):
net/ixgbe: fix flows not being scoped to port net/ixgbe: fix shared PF pointer in representor net/ixgbe: fix non-shared data in IPsec session net/ixgbe: fix SCTP protocol-only flow parsing net/ixgbe: fix L4 protocol mask handling net/ixgbe: reset flow state on clear paths net/ixgbe: store max VFs in adapter net/ixgbe: do not use flow list to count flows net/ixgbe: remove redundant flow tracking lists net/ixgbe: reduce FDIR conf macro usage net/ixgbe: use adapter in flow-related calls net/ixgbe: support protocol-only TCP and UDP rules net/ixgbe: write drop queue at init net/ixgbe: rely less on global flow state net/ixgbe: refactor flow creation

Some good cleanups, thanks.

Series-Acked-by Bruce Richardson <[email protected]>
#19 lore raw
Quoted 36 lines · Bruce Richardson
Quoted 18 lines · Anatoly Burakov

This patchset fixes a bunch of very old issues in flow management in IXGBE driver, such as:

- storing process-local pointers in shared memory
- incorrect L4 protocol matching for FDIR
- wrong handling of SCTP flow items
- reading stale FDIR state after flow destroy/flush
- storing all flows in global lists

In addition, some cleanup is also performed - refactors, moving things around to avoid accessing process-local state, and writing read-only values at init instead of deep in FDIR.

Finally, FDIR was also rejecting protocol-only matches for TCP and UDP, these are now supported.

Depends on flow dump patchset: https://patches.dpdk.org/project/dpdk/list/?series=38016

Quoted 17 lines

Anatoly Burakov (15):
net/ixgbe: fix flows not being scoped to port net/ixgbe: fix shared PF pointer in representor net/ixgbe: fix non-shared data in IPsec session net/ixgbe: fix SCTP protocol-only flow parsing net/ixgbe: fix L4 protocol mask handling net/ixgbe: reset flow state on clear paths net/ixgbe: store max VFs in adapter net/ixgbe: do not use flow list to count flows net/ixgbe: remove redundant flow tracking lists net/ixgbe: reduce FDIR conf macro usage net/ixgbe: use adapter in flow-related calls net/ixgbe: support protocol-only TCP and UDP rules net/ixgbe: write drop queue at init net/ixgbe: rely less on global flow state net/ixgbe: refactor flow creation

Quoted 3 lines

Some good cleanups, thanks.

Series-Acked-by Bruce Richardson <[email protected]>

Series applied to dpdk-next-net-intel.

Thanks,
/Bruce