Skip to contentSkip to conversation

sfc: conntrack offload for tunnels

PATCH Applied [email protected] 3 participants 7 messages Oct 2, 2023 – Oct 6, 2023

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

cover sfc: conntrack offload for tunnels

From: Edward Cree <[email protected]>

This series adds support for offloading TC flower rules which require
both connection tracking and tunnel decapsulation. Depending on the match keys required, the left-hand-side rule may go in either the Outer Rule table or the Action Rule table.

Edward Cree (4):
sfc: support TC left-hand-side rules on foreign netdevs
sfc: offload foreign RHS rules without an encap match
sfc: ensure an extack msg from efx_tc_flower_replace_foreign
EOPNOTSUPPs
sfc: support TC rules which require OR-AR-CT-AR flow

 drivers/net/ethernet/sfc/mae.c |  59 +++++-
 drivers/net/ethernet/sfc/tc.c  | 329 ++++++++++++++++++++++++++++++++-
 drivers/net/ethernet/sfc/tc.h  |   6 +
 3 files changed, 388 insertions(+), 6 deletions(-)
#1 lore raw

PATCH 1/4 sfc: support TC left-hand-side rules on foreign netdevs

From: Edward Cree <[email protected]>

Allow a tunnel netdevice (such as a vxlan) to offload conntrack lookups,
in much the same way as efx netdevs.
To ensure this rule does not overlap with other tunnel rules on the same
sip,dip,dport tuple, register a pseudo encap match of a new type (EFX_TC_EM_PSEUDO_OR), which unlike PSEUDO_MASK may only be referenced once (because an actual Outer Rule in hardware exists, although its fw_id is not recorded in the encap match entry).

---
 drivers/net/ethernet/sfc/mae.c |   6 +-
 drivers/net/ethernet/sfc/tc.c  | 213 +++++++++++++++++++++++++++++++++
 drivers/net/ethernet/sfc/tc.h  |   5 +
 3 files changed, 222 insertions(+), 2 deletions(-)
drivers/net/ethernet/sfc/mae.c +4 -2
index c3e2b4a21d10..d1c8872efac9 100644
@@ -1705,8 +1705,10 @@ static int efx_mae_insert_lhs_outer_rule(struct efx_nic *efx,
 
 	/* action */
 	act = &rule->lhs_act;
-	MCDI_SET_DWORD(inbuf, MAE_OUTER_RULE_INSERT_IN_ENCAP_TYPE,
-		       MAE_MCDI_ENCAP_TYPE_NONE);
+	rc = efx_mae_encap_type_to_mae_type(act->tun_type);
+	if (rc < 0)
+		return rc;
+	MCDI_SET_DWORD(inbuf, MAE_OUTER_RULE_INSERT_IN_ENCAP_TYPE, rc);
 	/* We always inhibit CT lookup on TCP_INTERESTING_FLAGS, since the
 	 * SW path needs to process the packet to update the conntrack tables
 	 * on connection establishment (SYN) or termination (FIN, RST).
drivers/net/ethernet/sfc/tc.c +213 -0

237 lines not on this page · raw message

drivers/net/ethernet/sfc/tc.h +5 -0
index 4dd2c378fd9f..c4cb52dda057 100644
@@ -140,10 +140,14 @@ static inline bool efx_tc_match_is_encap(const struct efx_tc_match_fields *mask)
  *	The pseudo encap match may be referenced again by an encap match
  *	with different values for these fields, but all masks must match the
  *	first (stored in our child_* fields).
+ * @EFX_TC_EM_PSEUDO_OR: registered by an fLHS rule that fits in the OR
+ *	table.  The &struct efx_tc_lhs_rule already holds the HW OR entry.
+ *	Only one reference to this encap match may exist.
  */
 enum efx_tc_em_pseudo_type {
 	EFX_TC_EM_DIRECT,
 	EFX_TC_EM_PSEUDO_MASK,
+	EFX_TC_EM_PSEUDO_OR,
 };
 
 struct efx_tc_encap_match {
@@ -183,6 +187,7 @@ struct efx_tc_action_set_list {
 };
 
 struct efx_tc_lhs_action {
+	enum efx_encap_type tun_type;
 	struct efx_tc_recirc_id *rid;
 	struct efx_tc_ct_zone *zone;
 	struct efx_tc_counter_index *count;
#4 lore raw

PATCH 3/4 sfc: ensure an extack msg from efx_tc_flower_replace_foreign EOPNOTSUPPs

In reply to [email protected]

From: Edward Cree <[email protected]>

There were a few places where no extack error message was set, or the
extack was not forwarded to callees, potentially resulting in a return of -EOPNOTSUPP with no additional information.
Make sure to populate the error message in these cases. In practice
this does us no good as TC indirect block callbacks don't come with an extack to fill in; but maybe they will someday and when debugging it's possible to provide a fake extack and emit its message to the console.

---
 drivers/net/ethernet/sfc/tc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/net/ethernet/sfc/tc.c +4 -2
index f4172fc6bd4f..6acd30f2db1e 100644
@@ -1580,7 +1580,7 @@ static int efx_tc_flower_replace_foreign(struct efx_nic *efx,
 
 	/* Parse match */
 	memset(&match, 0, sizeof(match));
-	rc = efx_tc_flower_parse_match(efx, fr, &match, NULL);
+	rc = efx_tc_flower_parse_match(efx, fr, &match, extack);
 	if (rc)
 		return rc;
 	/* The rule as given to us doesn't specify a source netdevice.
@@ -1629,6 +1629,7 @@ static int efx_tc_flower_replace_foreign(struct efx_nic *efx,
 	if (match.mask.ct_state_est && !match.value.ct_state_est) {
 		if (match.value.tcp_syn_fin_rst) {
 			/* Can't offload this combination */
+			NL_SET_ERR_MSG_MOD(extack, "TCP flags and -est conflict for offload");
 			rc = -EOPNOTSUPP;
 			goto release;
 		}
@@ -1655,7 +1656,7 @@ static int efx_tc_flower_replace_foreign(struct efx_nic *efx,
 		goto release;
 	}
 
-	rc = efx_mae_match_check_caps(efx, &match.mask, NULL);
+	rc = efx_mae_match_check_caps(efx, &match.mask, extack);
 	if (rc)
 		goto release;
 
@@ -1743,6 +1744,7 @@ static int efx_tc_flower_replace_foreign(struct efx_nic *efx,
 					goto release;
 				}
 				if (!efx_tc_flower_action_order_ok(act, EFX_TC_AO_COUNT)) {
+					NL_SET_ERR_MSG_MOD(extack, "Count action violates action order (can't happen)");
 					rc = -EOPNOTSUPP;
 					goto release;
 				}
#3 lore raw

PATCH 2/4 sfc: offload foreign RHS rules without an encap match

In reply to [email protected]

From: Edward Cree <[email protected]>

Normally, if a TC filter on a tunnel netdev does not match on any
encap fields, we decline to offload it, as it cannot meet our requirement for a <sip,dip,dport> tuple for the encap match.
However, if the rule has a nonzero chain_index, then for a packet to
reach the rule, it must already have matched a LHS rule which will have included an encap match and determined the tunnel type, so in that case we can offload the right-hand-side rule.

---
 drivers/net/ethernet/sfc/tc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/ethernet/sfc/tc.c +1 -1
index 257bec75e952..f4172fc6bd4f 100644
@@ -1683,7 +1683,7 @@ static int efx_tc_flower_replace_foreign(struct efx_nic *efx,
 						      extack);
 		if (rc)
 			goto release;
-	} else {
+	} else if (!tc->common.chain_index) {
 		/* This is not a tunnel decap rule, ignore it */
 		netif_dbg(efx, drv, efx->net_dev,
 			  "Ignoring foreign filter without encap match\n");
#5 lore raw

PATCH 4/4 sfc: support TC rules which require OR-AR-CT-AR flow

In reply to [email protected]

From: Edward Cree <[email protected]>

When a foreign LHS rule (TC rule from a tunnel netdev which requests
conntrack lookup) matches on inner headers or enc_key_id, these matches cannot be performed by the Outer Rule table, as the keys are only available after the tunnel type has been identified (by the OR lookup) and the rest of the headers parsed accordingly.
Offload such rules with an Action Rule, using the LOOKUP_CONTROL section
of the AR response to specify the conntrack and/or recirculation actions, combined with an Outer Rule which performs only the usual Encap Match duties.
This processing flow, as it requires two AR lookups per packet, is less
performant than OR-CT-AR, so only use it where necessary.

---
 drivers/net/ethernet/sfc/mae.c |  53 +++++++++++++++
 drivers/net/ethernet/sfc/tc.c  | 116 +++++++++++++++++++++++++++++++--
 drivers/net/ethernet/sfc/tc.h  |   1 +
 3 files changed, 165 insertions(+), 5 deletions(-)
drivers/net/ethernet/sfc/mae.c +53 -0
index d1c8872efac9..021980a958b7 100644
@@ -1736,9 +1736,60 @@ static int efx_mae_insert_lhs_outer_rule(struct efx_nic *efx,
 	return 0;
 }
 
+static int efx_mae_populate_match_criteria(MCDI_DECLARE_STRUCT_PTR(match_crit),
+					   const struct efx_tc_match *match);
+
+static int efx_mae_insert_lhs_action_rule(struct efx_nic *efx,
+					  struct efx_tc_lhs_rule *rule,
+					  u32 prio)
+{
+	MCDI_DECLARE_BUF(inbuf, MC_CMD_MAE_ACTION_RULE_INSERT_IN_LEN(MAE_FIELD_MASK_VALUE_PAIRS_V2_LEN));
+	MCDI_DECLARE_BUF(outbuf, MC_CMD_MAE_ACTION_RULE_INSERT_OUT_LEN);
+	struct efx_tc_lhs_action *act = &rule->lhs_act;
+	MCDI_DECLARE_STRUCT_PTR(match_crit);
+	MCDI_DECLARE_STRUCT_PTR(response);
+	size_t outlen;
+	int rc;
+
+	match_crit = _MCDI_DWORD(inbuf, MAE_ACTION_RULE_INSERT_IN_MATCH_CRITERIA);
+	response = _MCDI_DWORD(inbuf, MAE_ACTION_RULE_INSERT_IN_RESPONSE);
+	MCDI_STRUCT_SET_DWORD(response, MAE_ACTION_RULE_RESPONSE_ASL_ID,
+			      MC_CMD_MAE_ACTION_SET_LIST_ALLOC_OUT_ACTION_SET_LIST_ID_NULL);
+	MCDI_STRUCT_SET_DWORD(response, MAE_ACTION_RULE_RESPONSE_AS_ID,
+			      MC_CMD_MAE_ACTION_SET_ALLOC_OUT_ACTION_SET_ID_NULL);
+	EFX_POPULATE_DWORD_5(*_MCDI_STRUCT_DWORD(response, MAE_ACTION_RULE_RESPONSE_LOOKUP_CONTROL),
+			     MAE_ACTION_RULE_RESPONSE_DO_CT, !!act->zone,
+			     MAE_ACTION_RULE_RESPONSE_DO_RECIRC,
+			     act->rid && !act->zone,
+			     MAE_ACTION_RULE_RESPONSE_CT_VNI_MODE,
+			     MAE_CT_VNI_MODE_ZERO,
+			     MAE_ACTION_RULE_RESPONSE_RECIRC_ID,
+			     act->rid ? act->rid->fw_id : 0,
+			     MAE_ACTION_RULE_RESPONSE_CT_DOMAIN,
+			     act->zone ? act->zone->zone : 0);
+	MCDI_STRUCT_SET_DWORD(response, MAE_ACTION_RULE_RESPONSE_COUNTER_ID,
+			      act->count ? act->count->cnt->fw_id :
+			      MC_CMD_MAE_COUNTER_ALLOC_OUT_COUNTER_ID_NULL);
+	MCDI_SET_DWORD(inbuf, MAE_ACTION_RULE_INSERT_IN_PRIO, prio);
+	rc = efx_mae_populate_match_criteria(match_crit, &rule->match);
+	if (rc)
+		return rc;
+
+	rc = efx_mcdi_rpc(efx, MC_CMD_MAE_ACTION_RULE_INSERT, inbuf, sizeof(inbuf),
+			  outbuf, sizeof(outbuf), &outlen);
+	if (rc)
+		return rc;
+	if (outlen < sizeof(outbuf))
+		return -EIO;
+	rule->fw_id = MCDI_DWORD(outbuf, MAE_ACTION_RULE_INSERT_OUT_AR_ID);
+	return 0;
+}
+
 int efx_mae_insert_lhs_rule(struct efx_nic *efx, struct efx_tc_lhs_rule *rule,
 			    u32 prio)
 {
+	if (rule->is_ar)
+		return efx_mae_insert_lhs_action_rule(efx, rule, prio);
 	return efx_mae_insert_lhs_outer_rule(efx, rule, prio);
 }
 
@@ -1772,6 +1823,8 @@ static int efx_mae_remove_lhs_outer_rule(struct efx_nic *efx,
 
 int efx_mae_remove_lhs_rule(struct efx_nic *efx, struct efx_tc_lhs_rule *rule)
 {
+	if (rule->is_ar)
+		return efx_mae_delete_rule(efx, rule->fw_id);
 	return efx_mae_remove_lhs_outer_rule(efx, rule);
 }
 
drivers/net/ethernet/sfc/tc.c +111 -5

143 lines not on this page · raw message

drivers/net/ethernet/sfc/tc.h +1 -0
index c4cb52dda057..86e38ea7988c 100644
@@ -208,6 +208,7 @@ struct efx_tc_lhs_rule {
 	struct efx_tc_lhs_action lhs_act;
 	struct rhash_head linkage;
 	u32 fw_id;
+	bool is_ar; /* Action Rule (for OR-AR-CT-AR sequence) */
 };
 
 enum efx_tc_rule_prios {
reviewer
#2 lore raw

Re: [PATCH net-next 1/4] sfc: support TC left-hand-side rules on foreign netdevs

In reply to [email protected]
Quoted 12 lines · On Mon, Oct 02, 2023 at 04:44:41PM +0100, [email protected] wrote:

From: Edward Cree <[email protected]>

Allow a tunnel netdevice (such as a vxlan) to offload conntrack lookups,
in much the same way as efx netdevs.
To ensure this rule does not overlap with other tunnel rules on the same
sip,dip,dport tuple, register a pseudo encap match of a new type (EFX_TC_EM_PSEUDO_OR), which unlike PSEUDO_MASK may only be referenced once (because an actual Outer Rule in hardware exists, although its fw_id is not recorded in the encap match entry).

...

Quoted 13 lines
drivers/net/ethernet/sfc/tc.c +9 -0
@@ -1354,6 +1450,119 @@ static int efx_tc_incomplete_mangle(struct efx_tc_mangler_state *mung,
 	return 0;
 }
 
+static int efx_tc_flower_replace_foreign_lhs(struct efx_nic *efx,
+					     struct flow_cls_offload *tc,
+					     struct flow_rule *fr,
+					     struct efx_tc_match *match,
+					     struct net_device *net_dev)
+{
+	struct netlink_ext_ack *extack = tc->common.extack;
+	struct efx_tc_lhs_rule *rule, *old;
+	enum efx_encap_type type;

drivers/net/ethernet/sfc/tc.c static int efx_tc_incomplete_mangle(struct efx_tc_mangler_state *mung, · line 1471

Quoted 10 lines
drivers/net/ethernet/sfc/tc.c +10 -0
+	int rc;
+
+	if (tc->common.chain_index) {
+		NL_SET_ERR_MSG_MOD(extack, "LHS rule only allowed in chain 0");
+		return -EOPNOTSUPP;
+	}
+
+	if (!efx_tc_match_is_encap(&match->mask)) {
+		/* This is not a tunnel decap rule, ignore it */
+		netif_dbg(efx, drv, efx->net_dev, "Ignoring foreign LHS filter without encap match\n");

Hi Edward,

is NL_SET_ERR_MSG_MOD() appropriate here?

Quoted 93 lines
drivers/net/ethernet/sfc/tc.c +93 -0
+		return -EOPNOTSUPP;
+	}
+
+	if (efx_tc_flower_flhs_needs_ar(match)) {
+		NL_SET_ERR_MSG_MOD(extack, "Match keys not available in Outer Rule");
+		return -EOPNOTSUPP;
+	}
+
+	type = efx_tc_indr_netdev_type(net_dev);
+	if (type == EFX_ENCAP_TYPE_NONE) {
+		NL_SET_ERR_MSG_MOD(extack, "Egress encap match on unsupported tunnel device\n");
+		return -EOPNOTSUPP;
+	}
+
+	rc = efx_mae_check_encap_type_supported(efx, type);
+	if (rc) {
+		NL_SET_ERR_MSG_FMT_MOD(extack,
+				       "Firmware reports no support for %s encap match",
+				       efx_tc_encap_type_name(type));
+		return rc;
+	}
+	/* Reserve the outer tuple with a pseudo Encap Match */
+	rc = efx_tc_flower_record_encap_match(efx, match, type,
+					      EFX_TC_EM_PSEUDO_OR, 0, 0,
+					      extack);
+	if (rc)
+		return rc;
+
+	if (match->mask.ct_state_trk && match->value.ct_state_trk) {
+		NL_SET_ERR_MSG_MOD(extack, "LHS rule can never match +trk");
+		rc = -EOPNOTSUPP;
+		goto release_encap_match;
+	}
+	/* LHS rules are always -trk, so we don't need to match on that */
+	match->mask.ct_state_trk = 0;
+	match->value.ct_state_trk = 0;
+
+	rc = efx_tc_flower_translate_flhs_match(match);
+	if (rc) {
+		NL_SET_ERR_MSG_MOD(extack, "LHS rule cannot match on inner fields");
+		goto release_encap_match;
+	}
+
+	rc = efx_mae_match_check_caps_lhs(efx, &match->mask, extack);
+	if (rc)
+		goto release_encap_match;
+
+	rule = kzalloc(sizeof(*rule), GFP_USER);
+	if (!rule) {
+		rc = -ENOMEM;
+		goto release_encap_match;
+	}
+	rule->cookie = tc->cookie;
+	old = rhashtable_lookup_get_insert_fast(&efx->tc->lhs_rule_ht,
+						&rule->linkage,
+						efx_tc_lhs_rule_ht_params);
+	if (old) {
+		netif_dbg(efx, drv, efx->net_dev,
+			  "Already offloaded rule (cookie %lx)\n", tc->cookie);
+		rc = -EEXIST;
+		NL_SET_ERR_MSG_MOD(extack, "Rule already offloaded");
+		goto release;
+	}
+
+	/* Parse actions */
+	rc = efx_tc_flower_handle_lhs_actions(efx, tc, fr, net_dev, rule);
+	if (rc)
+		goto release;
+
+	rule->match = *match;
+	rule->lhs_act.tun_type = type;
+
+	rc = efx_mae_insert_lhs_rule(efx, rule, EFX_TC_PRIO_TC);
+	if (rc) {
+		NL_SET_ERR_MSG_MOD(extack, "Failed to insert rule in hw");
+		goto release;
+	}
+	netif_dbg(efx, drv, efx->net_dev,
+		  "Successfully parsed lhs rule (cookie %lx)\n",
+		  tc->cookie);
+	return 0;
+
+release:
+	efx_tc_flower_release_lhs_actions(efx, &rule->lhs_act);
+	if (!old)
+		rhashtable_remove_fast(&efx->tc->lhs_rule_ht, &rule->linkage,
+				       efx_tc_lhs_rule_ht_params);
+	kfree(rule);
+release_encap_match:
+	if (match->encap)
+		efx_tc_flower_release_encap_match(efx, match->encap);
+	return rc;
+}

...

#6 lore raw
In reply to [email protected]
This series was applied to netdev/net-next.git (main) by David S. Miller <[email protected]>:

Hello:

This series was applied to netdev/net-next.git (main) by David S. Miller <[email protected]>:

Quoted 8 lines · On Mon, 2 Oct 2023 16:44:40 +0100 you wrote:

From: Edward Cree <[email protected]>

This series adds support for offloading TC flower rules which require
both connection tracking and tunnel decapsulation. Depending on the match keys required, the left-hand-side rule may go in either the Outer Rule table or the Action Rule table.

[...]

Here is the summary with links:
- [net-next,1/4] sfc: support TC left-hand-side rules on foreign netdevs

    https://git.kernel.org/netdev/net-next/c/ec1dc6c88ce4
  - [net-next,2/4] sfc: offload foreign RHS rules without an encap match
    https://git.kernel.org/netdev/net-next/c/937a0feab42e
  - [net-next,3/4] sfc: ensure an extack msg from efx_tc_flower_replace_foreign EOPNOTSUPPs
    https://git.kernel.org/netdev/net-next/c/f96622fd3a74
  - [net-next,4/4] sfc: support TC rules which require OR-AR-CT-AR flow
    https://git.kernel.org/netdev/net-next/c/e447056147ef

You are awesome, thank you!

Signature · 2 lines
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html