[PATCH net-next 0/4] sfc: conntrack offload for tunnels

[PATCH net-next 0/4] sfc: conntrack offload for tunnels

[edward.cree]

From: Edward Cree <ecree.xilinx@gmail.com>

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

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

[edward.cree]

From: Edward Cree <ecree.xilinx@gmail.com>

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

Reviewed-by: Pieter Jansen van Vuuren <pieter.jansen-van-vuuren@amd.com>
Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
---
 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(-)

diff --git a/drivers/net/ethernet/sfc/mae.c b/drivers/net/ethernet/sfc/mae.c
index c3e2b4a21d10..d1c8872efac9 100644
--- a/drivers/net/ethernet/sfc/mae.c
+++ b/drivers/net/ethernet/sfc/mae.c
@@ -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).
diff --git a/drivers/net/ethernet/sfc/tc.c b/drivers/net/ethernet/sfc/tc.c
index 834f000ba1c4..257bec75e952 100644
--- a/drivers/net/ethernet/sfc/tc.c
+++ b/drivers/net/ethernet/sfc/tc.c
@@ -642,6 +642,15 @@ static int efx_tc_flower_record_encap_match(struct efx_nic *efx,
 				return -EEXIST;
 			}
 			break;
+		case EFX_TC_EM_PSEUDO_OR:
+			/* old EM corresponds to an OR that has to be unique
+			 * (it must not overlap with any other OR, whether
+			 * direct-EM or pseudo).
+			 */
+			NL_SET_ERR_MSG_FMT_MOD(extack,
+					       "%s encap match conflicts with existing pseudo(OR) entry",
+					       em_type ? "Pseudo" : "Direct");
+			return -EEXIST;
 		default: /* Unrecognised pseudo-type.  Just say no */
 			NL_SET_ERR_MSG_FMT_MOD(extack,
 					       "%s encap match conflicts with existing pseudo(%d) entry",
@@ -872,6 +881,93 @@ static bool efx_tc_rule_is_lhs_rule(struct flow_rule *fr,
 	return false;
 }
 
+/* A foreign LHS rule has matches on enc_ keys at the TC layer (including an
+ * implied match on enc_ip_proto UDP).  Translate these into non-enc_ keys,
+ * so that we can use the same MAE machinery as local LHS rules (and so that
+ * the lhs_rules entries have uniform semantics).  It may seem odd to do it
+ * this way round, given that the corresponding fields in the MAE MCDIs are
+ * all ENC_, but (a) we don't have enc_L2 or enc_ip_proto in struct
+ * efx_tc_match_fields and (b) semantically an LHS rule doesn't have inner
+ * fields so it's just matching on *the* header rather than the outer header.
+ * Make sure that the non-enc_ keys were not already being matched on, as that
+ * would imply a rule that needed a triple lookup.  (Hardware can do that,
+ * with OR-AR-CT-AR, but it halves packet rate so we avoid it where possible;
+ * see efx_tc_flower_flhs_needs_ar().)
+ */
+static int efx_tc_flower_translate_flhs_match(struct efx_tc_match *match)
+{
+	int rc = 0;
+
+#define COPY_MASK_AND_VALUE(_key, _ekey)	({	\
+	if (match->mask._key) {				\
+		rc = -EOPNOTSUPP;			\
+	} else {					\
+		match->mask._key = match->mask._ekey;	\
+		match->mask._ekey = 0;			\
+		match->value._key = match->value._ekey;	\
+		match->value._ekey = 0;			\
+	}						\
+	rc;						\
+})
+#define COPY_FROM_ENC(_key)	COPY_MASK_AND_VALUE(_key, enc_##_key)
+	if (match->mask.ip_proto)
+		return -EOPNOTSUPP;
+	match->mask.ip_proto = ~0;
+	match->value.ip_proto = IPPROTO_UDP;
+	if (COPY_FROM_ENC(src_ip) || COPY_FROM_ENC(dst_ip))
+		return rc;
+#ifdef CONFIG_IPV6
+	if (!ipv6_addr_any(&match->mask.src_ip6))
+		return -EOPNOTSUPP;
+	match->mask.src_ip6 = match->mask.enc_src_ip6;
+	memset(&match->mask.enc_src_ip6, 0, sizeof(struct in6_addr));
+	if (!ipv6_addr_any(&match->mask.dst_ip6))
+		return -EOPNOTSUPP;
+	match->mask.dst_ip6 = match->mask.enc_dst_ip6;
+	memset(&match->mask.enc_dst_ip6, 0, sizeof(struct in6_addr));
+#endif
+	if (COPY_FROM_ENC(ip_tos) || COPY_FROM_ENC(ip_ttl))
+		return rc;
+	/* should really copy enc_ip_frag but we don't have that in
+	 * parse_match yet
+	 */
+	if (COPY_MASK_AND_VALUE(l4_sport, enc_sport) ||
+	    COPY_MASK_AND_VALUE(l4_dport, enc_dport))
+		return rc;
+	return 0;
+#undef COPY_FROM_ENC
+#undef COPY_MASK_AND_VALUE
+}
+
+/* If a foreign LHS rule wants to match on keys that are only available after
+ * encap header identification and parsing, then it can't be done in the Outer
+ * Rule lookup, because that lookup determines the encap type used to parse
+ * beyond the outer headers.  Thus, such rules must use the OR-AR-CT-AR lookup
+ * sequence, with an EM (struct efx_tc_encap_match) in the OR step.
+ * Return true iff the passed match requires this.
+ */
+static bool efx_tc_flower_flhs_needs_ar(struct efx_tc_match *match)
+{
+	/* matches on inner-header keys can't be done in OR */
+	return match->mask.eth_proto ||
+	       match->mask.vlan_tci[0] || match->mask.vlan_tci[1] ||
+	       match->mask.vlan_proto[0] || match->mask.vlan_proto[1] ||
+	       memchr_inv(match->mask.eth_saddr, 0, ETH_ALEN) ||
+	       memchr_inv(match->mask.eth_daddr, 0, ETH_ALEN) ||
+	       match->mask.ip_proto ||
+	       match->mask.ip_tos || match->mask.ip_ttl ||
+	       match->mask.src_ip || match->mask.dst_ip ||
+#ifdef CONFIG_IPV6
+	       !ipv6_addr_any(&match->mask.src_ip6) ||
+	       !ipv6_addr_any(&match->mask.dst_ip6) ||
+#endif
+	       match->mask.ip_frag || match->mask.ip_firstfrag ||
+	       match->mask.l4_sport || match->mask.l4_dport ||
+	       match->mask.tcp_flags ||
+	/* nor can VNI */
+	       match->mask.enc_keyid;
+}
+
 static int efx_tc_flower_handle_lhs_actions(struct efx_nic *efx,
 					    struct flow_cls_offload *tc,
 					    struct flow_rule *fr,
@@ -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;
+	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");
+		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;
+}
+
 static int efx_tc_flower_replace_foreign(struct efx_nic *efx,
 					 struct net_device *net_dev,
 					 struct flow_cls_offload *tc)
@@ -1387,6 +1596,10 @@ static int efx_tc_flower_replace_foreign(struct efx_nic *efx,
 	match.value.ingress_port = rc;
 	match.mask.ingress_port = ~0;
 
+	if (efx_tc_rule_is_lhs_rule(fr, &match))
+		return efx_tc_flower_replace_foreign_lhs(efx, tc, fr, &match,
+							 net_dev);
+
 	if (tc->common.chain_index) {
 		struct efx_tc_recirc_id *rid;
 
diff --git a/drivers/net/ethernet/sfc/tc.h b/drivers/net/ethernet/sfc/tc.h
index 4dd2c378fd9f..c4cb52dda057 100644
--- a/drivers/net/ethernet/sfc/tc.h
+++ b/drivers/net/ethernet/sfc/tc.h
@@ -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;

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

[Simon Horman]

On Mon, Oct 02, 2023 at 04:44:41PM +0100, edward.cree@amd.com wrote:
> From: Edward Cree <ecree.xilinx@gmail.com>
> 
> 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).
> 
> Reviewed-by: Pieter Jansen van Vuuren <pieter.jansen-van-vuuren@amd.com>
> Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>

...

> @@ -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;
> +	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?

> +		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;
> +}

...

[PATCH net-next 2/4] sfc: offload foreign RHS rules without an encap match

[edward.cree]

From: Edward Cree <ecree.xilinx@gmail.com>

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.

Reviewed-by: Pieter Jansen van Vuuren <pieter.jansen-van-vuuren@amd.com>
Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
---
 drivers/net/ethernet/sfc/tc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/sfc/tc.c b/drivers/net/ethernet/sfc/tc.c
index 257bec75e952..f4172fc6bd4f 100644
--- a/drivers/net/ethernet/sfc/tc.c
+++ b/drivers/net/ethernet/sfc/tc.c
@@ -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");

[PATCH net-next 3/4] sfc: ensure an extack msg from efx_tc_flower_replace_foreign EOPNOTSUPPs

[edward.cree]

From: Edward Cree <ecree.xilinx@gmail.com>

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.

Reviewed-by: Pieter Jansen van Vuuren <pieter.jansen-van-vuuren@amd.com>
Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
---
 drivers/net/ethernet/sfc/tc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/sfc/tc.c b/drivers/net/ethernet/sfc/tc.c
index f4172fc6bd4f..6acd30f2db1e 100644
--- a/drivers/net/ethernet/sfc/tc.c
+++ b/drivers/net/ethernet/sfc/tc.c
@@ -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;
 				}

[PATCH net-next 4/4] sfc: support TC rules which require OR-AR-CT-AR flow

[edward.cree]

From: Edward Cree <ecree.xilinx@gmail.com>

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.

Reviewed-by: Pieter Jansen van Vuuren <pieter.jansen-van-vuuren@amd.com>
Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
---
 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(-)

diff --git a/drivers/net/ethernet/sfc/mae.c b/drivers/net/ethernet/sfc/mae.c
index d1c8872efac9..021980a958b7 100644
--- a/drivers/net/ethernet/sfc/mae.c
+++ b/drivers/net/ethernet/sfc/mae.c
@@ -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);
 }
 
diff --git a/drivers/net/ethernet/sfc/tc.c b/drivers/net/ethernet/sfc/tc.c
index 6acd30f2db1e..3d76b7598631 100644
--- a/drivers/net/ethernet/sfc/tc.c
+++ b/drivers/net/ethernet/sfc/tc.c
@@ -978,9 +978,12 @@ static int efx_tc_flower_handle_lhs_actions(struct efx_nic *efx,
 	struct netlink_ext_ack *extack = tc->common.extack;
 	struct efx_tc_lhs_action *act = &rule->lhs_act;
 	const struct flow_action_entry *fa;
+	enum efx_tc_counter_type ctype;
 	bool pipe = true;
 	int i;
 
+	ctype = rule->is_ar ? EFX_TC_COUNTER_TYPE_AR : EFX_TC_COUNTER_TYPE_OR;
+
 	flow_action_for_each(i, fa, &fr->action) {
 		struct efx_tc_ct_zone *ct_zone;
 		struct efx_tc_recirc_id *rid;
@@ -1013,7 +1016,7 @@ static int efx_tc_flower_handle_lhs_actions(struct efx_nic *efx,
 					return -EOPNOTSUPP;
 				}
 				cnt = efx_tc_flower_get_counter_index(efx, tc->cookie,
-								      EFX_TC_COUNTER_TYPE_OR);
+								      ctype);
 				if (IS_ERR(cnt)) {
 					NL_SET_ERR_MSG_MOD(extack, "Failed to obtain a counter");
 					return PTR_ERR(cnt);
@@ -1450,6 +1453,110 @@ static int efx_tc_incomplete_mangle(struct efx_tc_mangler_state *mung,
 	return 0;
 }
 
+static int efx_tc_flower_replace_foreign_lhs_ar(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;
+	int rc;
+
+	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");
+		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;
+	}
+	/* This is an Action Rule, so it needs a separate Encap Match in the
+	 * Outer Rule table.  Insert that now.
+	 */
+	rc = efx_tc_flower_record_encap_match(efx, match, type,
+					      EFX_TC_EM_DIRECT, 0, 0, extack);
+	if (rc)
+		return rc;
+
+	match->mask.recirc_id = 0xff;
+	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;
+	/* We must inhibit match on TCP SYN/FIN/RST, so that SW can see
+	 * the packet and update the conntrack table.
+	 * Outer Rules will do that with CT_TCP_FLAGS_INHIBIT, but Action
+	 * Rules don't have that; instead they support matching on
+	 * TCP_SYN_FIN_RST (aka TCP_INTERESTING_FLAGS), so use that.
+	 * This is only strictly needed if there will be a DO_CT action,
+	 * which we don't know yet, but typically there will be and it's
+	 * simpler not to bother checking here.
+	 */
+	match->mask.tcp_syn_fin_rst = true;
+
+	rc = efx_mae_match_check_caps(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;
+	rule->is_ar = true;
+	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;
+}
+
 static int efx_tc_flower_replace_foreign_lhs(struct efx_nic *efx,
 					     struct flow_cls_offload *tc,
 					     struct flow_rule *fr,
@@ -1472,10 +1579,9 @@ static int efx_tc_flower_replace_foreign_lhs(struct efx_nic *efx,
 		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;
-	}
+	if (efx_tc_flower_flhs_needs_ar(match))
+		return efx_tc_flower_replace_foreign_lhs_ar(efx, tc, fr, match,
+							    net_dev);
 
 	type = efx_tc_indr_netdev_type(net_dev);
 	if (type == EFX_ENCAP_TYPE_NONE) {
diff --git a/drivers/net/ethernet/sfc/tc.h b/drivers/net/ethernet/sfc/tc.h
index c4cb52dda057..86e38ea7988c 100644
--- a/drivers/net/ethernet/sfc/tc.h
+++ b/drivers/net/ethernet/sfc/tc.h
@@ -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 {

Re: [PATCH net-next 0/4] sfc: conntrack offload for tunnels

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by David S. Miller <davem@davemloft.net>:

On Mon, 2 Oct 2023 16:44:40 +0100 you wrote:
> From: Edward Cree <ecree.xilinx@gmail.com>
> 
> 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!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html