diff mbox series

[net-next,v2,2/6] sfc: add notion of match on enc keys to MAE machinery

Message ID fd5021315abf37e392e432021c6668c52da90dd1.1679603051.git.ecree.xilinx@gmail.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series sfc: support TC decap rules | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 19 this patch: 19
netdev/cc_maintainers success CCed 7 of 7 maintainers
netdev/build_clang success Errors and warnings before: 18 this patch: 18
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 19 this patch: 19
netdev/checkpatch warning CHECK: Lines should not end with a '(' CHECK: Please use a blank line after function/struct/union/enum declarations WARNING: line length of 103 exceeds 80 columns WARNING: line length of 91 exceeds 80 columns WARNING: line length of 93 exceeds 80 columns WARNING: line length of 94 exceeds 80 columns WARNING: line length of 98 exceeds 80 columns WARNING: line length of 99 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

edward.cree@amd.com March 23, 2023, 8:45 p.m. UTC
From: Edward Cree <ecree.xilinx@gmail.com>

Extend the MAE caps check to validate that the hardware supports used
 outer-header matches.
Extend efx_mae_populate_match_criteria() to fill in the outer rule ID
 and VNI match fields.
Nothing yet populates these match fields, nor creates outer rules.

Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
---
Changed in v2: efx_mae_check_encap_match_caps now takes a `bool ipv6` rather
 than an `unsigned char ipv`, simplifying the code.
---
 drivers/net/ethernet/sfc/mae.c | 97 +++++++++++++++++++++++++++++++++-
 drivers/net/ethernet/sfc/mae.h |  3 ++
 drivers/net/ethernet/sfc/tc.h  | 24 +++++++++
 3 files changed, 122 insertions(+), 2 deletions(-)

Comments

Simon Horman March 25, 2023, 12:03 p.m. UTC | #1
Hi Edward,

Looks good to me.
A few minor comments inline.

On Thu, Mar 23, 2023 at 08:45:10PM +0000, edward.cree@amd.com wrote:
> From: Edward Cree <ecree.xilinx@gmail.com>
> 
> Extend the MAE caps check to validate that the hardware supports used
>  outer-header matches.

s/used// ?

> Extend efx_mae_populate_match_criteria() to fill in the outer rule ID
>  and VNI match fields.
> Nothing yet populates these match fields, nor creates outer rules.
> 
> Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>

...

>  int efx_mae_allocate_counter(struct efx_nic *efx, struct efx_tc_counter *cnt)
>  {
>  	MCDI_DECLARE_BUF(outbuf, MC_CMD_MAE_COUNTER_ALLOC_OUT_LEN(1));
> @@ -941,6 +1011,29 @@ static int efx_mae_populate_match_criteria(MCDI_DECLARE_STRUCT_PTR(match_crit),
>  				match->value.tcp_flags);
>  	MCDI_STRUCT_SET_WORD_BE(match_crit, MAE_FIELD_MASK_VALUE_PAIRS_V2_TCP_FLAGS_BE_MASK,
>  				match->mask.tcp_flags);
> +	/* enc-keys are handled indirectly, through encap_match ID */
> +	if (match->encap) {
> +		MCDI_STRUCT_SET_DWORD(match_crit, MAE_FIELD_MASK_VALUE_PAIRS_V2_OUTER_RULE_ID,
> +				      match->encap->fw_id);
> +		MCDI_STRUCT_SET_DWORD(match_crit, MAE_FIELD_MASK_VALUE_PAIRS_V2_OUTER_RULE_ID_MASK,
> +				      U32_MAX);
> +		/* enc_keyid (VNI/VSID) is not part of the encap_match */
> +		MCDI_STRUCT_SET_DWORD_BE(match_crit, MAE_FIELD_MASK_VALUE_PAIRS_V2_ENC_VNET_ID_BE,
> +					 match->value.enc_keyid);
> +		MCDI_STRUCT_SET_DWORD_BE(match_crit, MAE_FIELD_MASK_VALUE_PAIRS_V2_ENC_VNET_ID_BE_MASK,
> +					 match->mask.enc_keyid);

Is it intentional that value.enc_keyid is used as the mask.
Perhaps naively I would have expected something more like U32_MAX.

> +	} else if (WARN_ON_ONCE(match->mask.enc_src_ip) ||
> +		   WARN_ON_ONCE(match->mask.enc_dst_ip) ||
> +		   WARN_ON_ONCE(!ipv6_addr_any(&match->mask.enc_src_ip6)) ||
> +		   WARN_ON_ONCE(!ipv6_addr_any(&match->mask.enc_dst_ip6)) ||
> +		   WARN_ON_ONCE(match->mask.enc_ip_tos) ||
> +		   WARN_ON_ONCE(match->mask.enc_ip_ttl) ||
> +		   WARN_ON_ONCE(match->mask.enc_sport) ||
> +		   WARN_ON_ONCE(match->mask.enc_dport) ||
> +		   WARN_ON_ONCE(match->mask.enc_keyid)) {
> +		/* No enc-keys should appear in a rule without an encap_match */
> +		return -EOPNOTSUPP;
> +	}
>  	return 0;
>  }
Edward Cree March 27, 2023, 8:20 a.m. UTC | #2
On 25/03/2023 12:03, Simon Horman wrote:
> Hi Edward,
> 
> Looks good to me.
> A few minor comments inline.
> 
> On Thu, Mar 23, 2023 at 08:45:10PM +0000, edward.cree@amd.com wrote:
>> From: Edward Cree <ecree.xilinx@gmail.com>
>>
>> Extend the MAE caps check to validate that the hardware supports used
>>  outer-header matches.
> 
> s/used// ?

I think I meant it in the sense of "the outer-header matches which
 are used by the driver"; I can definitely reword it to spell that
 out better.

>>  int efx_mae_allocate_counter(struct efx_nic *efx, struct efx_tc_counter *cnt)
>>  {
>>  	MCDI_DECLARE_BUF(outbuf, MC_CMD_MAE_COUNTER_ALLOC_OUT_LEN(1));
>> @@ -941,6 +1011,29 @@ static int efx_mae_populate_match_criteria(MCDI_DECLARE_STRUCT_PTR(match_crit),
>>  				match->value.tcp_flags);
>>  	MCDI_STRUCT_SET_WORD_BE(match_crit, MAE_FIELD_MASK_VALUE_PAIRS_V2_TCP_FLAGS_BE_MASK,
>>  				match->mask.tcp_flags);
>> +	/* enc-keys are handled indirectly, through encap_match ID */
>> +	if (match->encap) {
>> +		MCDI_STRUCT_SET_DWORD(match_crit, MAE_FIELD_MASK_VALUE_PAIRS_V2_OUTER_RULE_ID,
>> +				      match->encap->fw_id);
>> +		MCDI_STRUCT_SET_DWORD(match_crit, MAE_FIELD_MASK_VALUE_PAIRS_V2_OUTER_RULE_ID_MASK,
>> +				      U32_MAX);
>> +		/* enc_keyid (VNI/VSID) is not part of the encap_match */
>> +		MCDI_STRUCT_SET_DWORD_BE(match_crit, MAE_FIELD_MASK_VALUE_PAIRS_V2_ENC_VNET_ID_BE,
>> +					 match->value.enc_keyid);
>> +		MCDI_STRUCT_SET_DWORD_BE(match_crit, MAE_FIELD_MASK_VALUE_PAIRS_V2_ENC_VNET_ID_BE_MASK,
>> +					 match->mask.enc_keyid);
> 
> Is it intentional that value.enc_keyid is used as the mask.

But it isn't.  mask.enc_keyid is.
Simon Horman March 27, 2023, 8:47 a.m. UTC | #3
On Mon, Mar 27, 2023 at 09:20:06AM +0100, Edward Cree wrote:
> On 25/03/2023 12:03, Simon Horman wrote:
> > Hi Edward,
> > 
> > Looks good to me.
> > A few minor comments inline.
> > 
> > On Thu, Mar 23, 2023 at 08:45:10PM +0000, edward.cree@amd.com wrote:
> >> From: Edward Cree <ecree.xilinx@gmail.com>
> >>
> >> Extend the MAE caps check to validate that the hardware supports used
> >>  outer-header matches.
> > 
> > s/used// ?
> 
> I think I meant it in the sense of "the outer-header matches which
>  are used by the driver"; I can definitely reword it to spell that
>  out better.

Thanks, I did have a bit of trouble parsing the text.

> >>  int efx_mae_allocate_counter(struct efx_nic *efx, struct efx_tc_counter *cnt)
> >>  {
> >>  	MCDI_DECLARE_BUF(outbuf, MC_CMD_MAE_COUNTER_ALLOC_OUT_LEN(1));
> >> @@ -941,6 +1011,29 @@ static int efx_mae_populate_match_criteria(MCDI_DECLARE_STRUCT_PTR(match_crit),
> >>  				match->value.tcp_flags);
> >>  	MCDI_STRUCT_SET_WORD_BE(match_crit, MAE_FIELD_MASK_VALUE_PAIRS_V2_TCP_FLAGS_BE_MASK,
> >>  				match->mask.tcp_flags);
> >> +	/* enc-keys are handled indirectly, through encap_match ID */
> >> +	if (match->encap) {
> >> +		MCDI_STRUCT_SET_DWORD(match_crit, MAE_FIELD_MASK_VALUE_PAIRS_V2_OUTER_RULE_ID,
> >> +				      match->encap->fw_id);
> >> +		MCDI_STRUCT_SET_DWORD(match_crit, MAE_FIELD_MASK_VALUE_PAIRS_V2_OUTER_RULE_ID_MASK,
> >> +				      U32_MAX);
> >> +		/* enc_keyid (VNI/VSID) is not part of the encap_match */
> >> +		MCDI_STRUCT_SET_DWORD_BE(match_crit, MAE_FIELD_MASK_VALUE_PAIRS_V2_ENC_VNET_ID_BE,
> >> +					 match->value.enc_keyid);
> >> +		MCDI_STRUCT_SET_DWORD_BE(match_crit, MAE_FIELD_MASK_VALUE_PAIRS_V2_ENC_VNET_ID_BE_MASK,
> >> +					 match->mask.enc_keyid);
> > 
> > Is it intentional that value.enc_keyid is used as the mask.
> 
> But it isn't.  mask.enc_keyid is.

Indeed it is :)
diff mbox series

Patch

diff --git a/drivers/net/ethernet/sfc/mae.c b/drivers/net/ethernet/sfc/mae.c
index c53d354c1fb2..2290a63908c5 100644
--- a/drivers/net/ethernet/sfc/mae.c
+++ b/drivers/net/ethernet/sfc/mae.c
@@ -254,13 +254,23 @@  static int efx_mae_get_rule_fields(struct efx_nic *efx, u32 cmd,
 	size_t outlen;
 	int rc, i;
 
+	/* AR and OR caps MCDIs have identical layout, so we are using the
+	 * same code for both.
+	 */
+	BUILD_BUG_ON(MC_CMD_MAE_GET_AR_CAPS_OUT_LEN(MAE_NUM_FIELDS) <
+		     MC_CMD_MAE_GET_OR_CAPS_OUT_LEN(MAE_NUM_FIELDS));
 	BUILD_BUG_ON(MC_CMD_MAE_GET_AR_CAPS_IN_LEN);
+	BUILD_BUG_ON(MC_CMD_MAE_GET_OR_CAPS_IN_LEN);
 
 	rc = efx_mcdi_rpc(efx, cmd, NULL, 0, outbuf, sizeof(outbuf), &outlen);
 	if (rc)
 		return rc;
+	BUILD_BUG_ON(MC_CMD_MAE_GET_AR_CAPS_OUT_COUNT_OFST !=
+		     MC_CMD_MAE_GET_OR_CAPS_OUT_COUNT_OFST);
 	count = MCDI_DWORD(outbuf, MAE_GET_AR_CAPS_OUT_COUNT);
 	memset(field_support, MAE_FIELD_UNSUPPORTED, MAE_NUM_FIELDS);
+	BUILD_BUG_ON(MC_CMD_MAE_GET_AR_CAPS_OUT_FIELD_FLAGS_OFST !=
+		     MC_CMD_MAE_GET_OR_CAPS_OUT_FIELD_FLAGS_OFST);
 	caps = _MCDI_DWORD(outbuf, MAE_GET_AR_CAPS_OUT_FIELD_FLAGS);
 	/* We're only interested in the support status enum, not any other
 	 * flags, so just extract that from each entry.
@@ -278,8 +288,12 @@  int efx_mae_get_caps(struct efx_nic *efx, struct mae_caps *caps)
 	rc = efx_mae_get_basic_caps(efx, caps);
 	if (rc)
 		return rc;
-	return efx_mae_get_rule_fields(efx, MC_CMD_MAE_GET_AR_CAPS,
-				       caps->action_rule_fields);
+	rc = efx_mae_get_rule_fields(efx, MC_CMD_MAE_GET_AR_CAPS,
+				     caps->action_rule_fields);
+	if (rc)
+		return rc;
+	return efx_mae_get_rule_fields(efx, MC_CMD_MAE_GET_OR_CAPS,
+				       caps->outer_rule_fields);
 }
 
 /* Bit twiddling:
@@ -432,11 +446,67 @@  int efx_mae_match_check_caps(struct efx_nic *efx,
 	    CHECK_BIT(IP_FIRST_FRAG, ip_firstfrag) ||
 	    CHECK(RECIRC_ID, recirc_id))
 		return rc;
+	/* Matches on outer fields are done in a separate hardware table,
+	 * the Outer Rule table.  Thus the Action Rule merely does an
+	 * exact match on Outer Rule ID if any outer field matches are
+	 * present.  The exception is the VNI/VSID (enc_keyid), which is
+	 * available to the Action Rule match iff the Outer Rule matched
+	 * (and thus identified the encap protocol to use to extract it).
+	 */
+	if (efx_tc_match_is_encap(mask)) {
+		rc = efx_mae_match_check_cap_typ(
+				supported_fields[MAE_FIELD_OUTER_RULE_ID],
+				MASK_ONES);
+		if (rc) {
+			NL_SET_ERR_MSG_MOD(extack, "No support for encap rule ID matches");
+			return rc;
+		}
+		if (CHECK(ENC_VNET_ID, enc_keyid))
+			return rc;
+	} else if (mask->enc_keyid) {
+		NL_SET_ERR_MSG_MOD(extack, "Match on enc_keyid requires other encap fields");
+		return -EINVAL;
+	}
 	return 0;
 }
 #undef CHECK_BIT
 #undef CHECK
 
+#define CHECK(_mcdi)	({						       \
+	rc = efx_mae_match_check_cap_typ(supported_fields[MAE_FIELD_ ## _mcdi],\
+					 MASK_ONES);			       \
+	if (rc)								       \
+		NL_SET_ERR_MSG_FMT_MOD(extack,				       \
+				       "No support for field %s", #_mcdi);     \
+	rc;								       \
+})
+/* Checks that the fields needed for encap-rule matches are supported by the
+ * MAE.  All the fields are exact-match.
+ */
+int efx_mae_check_encap_match_caps(struct efx_nic *efx, bool ipv6,
+				   struct netlink_ext_ack *extack)
+{
+	u8 *supported_fields = efx->tc->caps->outer_rule_fields;
+	int rc;
+
+	if (CHECK(ENC_ETHER_TYPE))
+		return rc;
+	if (ipv6) {
+		if (CHECK(ENC_SRC_IP6) ||
+		    CHECK(ENC_DST_IP6))
+			return rc;
+	} else {
+		if (CHECK(ENC_SRC_IP4) ||
+		    CHECK(ENC_DST_IP4))
+			return rc;
+	}
+	if (CHECK(ENC_L4_DPORT) ||
+	    CHECK(ENC_IP_PROTO))
+		return rc;
+	return 0;
+}
+#undef CHECK
+
 int efx_mae_allocate_counter(struct efx_nic *efx, struct efx_tc_counter *cnt)
 {
 	MCDI_DECLARE_BUF(outbuf, MC_CMD_MAE_COUNTER_ALLOC_OUT_LEN(1));
@@ -941,6 +1011,29 @@  static int efx_mae_populate_match_criteria(MCDI_DECLARE_STRUCT_PTR(match_crit),
 				match->value.tcp_flags);
 	MCDI_STRUCT_SET_WORD_BE(match_crit, MAE_FIELD_MASK_VALUE_PAIRS_V2_TCP_FLAGS_BE_MASK,
 				match->mask.tcp_flags);
+	/* enc-keys are handled indirectly, through encap_match ID */
+	if (match->encap) {
+		MCDI_STRUCT_SET_DWORD(match_crit, MAE_FIELD_MASK_VALUE_PAIRS_V2_OUTER_RULE_ID,
+				      match->encap->fw_id);
+		MCDI_STRUCT_SET_DWORD(match_crit, MAE_FIELD_MASK_VALUE_PAIRS_V2_OUTER_RULE_ID_MASK,
+				      U32_MAX);
+		/* enc_keyid (VNI/VSID) is not part of the encap_match */
+		MCDI_STRUCT_SET_DWORD_BE(match_crit, MAE_FIELD_MASK_VALUE_PAIRS_V2_ENC_VNET_ID_BE,
+					 match->value.enc_keyid);
+		MCDI_STRUCT_SET_DWORD_BE(match_crit, MAE_FIELD_MASK_VALUE_PAIRS_V2_ENC_VNET_ID_BE_MASK,
+					 match->mask.enc_keyid);
+	} else if (WARN_ON_ONCE(match->mask.enc_src_ip) ||
+		   WARN_ON_ONCE(match->mask.enc_dst_ip) ||
+		   WARN_ON_ONCE(!ipv6_addr_any(&match->mask.enc_src_ip6)) ||
+		   WARN_ON_ONCE(!ipv6_addr_any(&match->mask.enc_dst_ip6)) ||
+		   WARN_ON_ONCE(match->mask.enc_ip_tos) ||
+		   WARN_ON_ONCE(match->mask.enc_ip_ttl) ||
+		   WARN_ON_ONCE(match->mask.enc_sport) ||
+		   WARN_ON_ONCE(match->mask.enc_dport) ||
+		   WARN_ON_ONCE(match->mask.enc_keyid)) {
+		/* No enc-keys should appear in a rule without an encap_match */
+		return -EOPNOTSUPP;
+	}
 	return 0;
 }
 
diff --git a/drivers/net/ethernet/sfc/mae.h b/drivers/net/ethernet/sfc/mae.h
index bec293a06733..2ccbc62d79b9 100644
--- a/drivers/net/ethernet/sfc/mae.h
+++ b/drivers/net/ethernet/sfc/mae.h
@@ -72,6 +72,7 @@  struct mae_caps {
 	u32 match_field_count;
 	u32 action_prios;
 	u8 action_rule_fields[MAE_NUM_FIELDS];
+	u8 outer_rule_fields[MAE_NUM_FIELDS];
 };
 
 int efx_mae_get_caps(struct efx_nic *efx, struct mae_caps *caps);
@@ -79,6 +80,8 @@  int efx_mae_get_caps(struct efx_nic *efx, struct mae_caps *caps);
 int efx_mae_match_check_caps(struct efx_nic *efx,
 			     const struct efx_tc_match_fields *mask,
 			     struct netlink_ext_ack *extack);
+int efx_mae_check_encap_match_caps(struct efx_nic *efx, bool ipv6,
+				   struct netlink_ext_ack *extack);
 
 int efx_mae_allocate_counter(struct efx_nic *efx, struct efx_tc_counter *cnt);
 int efx_mae_free_counter(struct efx_nic *efx, struct efx_tc_counter *cnt);
diff --git a/drivers/net/ethernet/sfc/tc.h b/drivers/net/ethernet/sfc/tc.h
index 542853f60c2a..c1485679507c 100644
--- a/drivers/net/ethernet/sfc/tc.h
+++ b/drivers/net/ethernet/sfc/tc.h
@@ -48,11 +48,35 @@  struct efx_tc_match_fields {
 	/* L4 */
 	__be16 l4_sport, l4_dport; /* Ports (UDP, TCP) */
 	__be16 tcp_flags;
+	/* Encap.  The following are *outer* fields.  Note that there are no
+	 * outer eth (L2) fields; this is because TC doesn't have them.
+	 */
+	__be32 enc_src_ip, enc_dst_ip;
+	struct in6_addr enc_src_ip6, enc_dst_ip6;
+	u8 enc_ip_tos, enc_ip_ttl;
+	__be16 enc_sport, enc_dport;
+	__be32 enc_keyid; /* e.g. VNI, VSID */
+};
+
+static inline bool efx_tc_match_is_encap(const struct efx_tc_match_fields *mask)
+{
+	return mask->enc_src_ip || mask->enc_dst_ip ||
+	       !ipv6_addr_any(&mask->enc_src_ip6) ||
+	       !ipv6_addr_any(&mask->enc_dst_ip6) || mask->enc_ip_tos ||
+	       mask->enc_ip_ttl || mask->enc_sport || mask->enc_dport;
+}
+
+struct efx_tc_encap_match {
+	__be32 src_ip, dst_ip;
+	struct in6_addr src_ip6, dst_ip6;
+	__be16 udp_dport;
+	u32 fw_id; /* index of this entry in firmware encap match table */
 };
 
 struct efx_tc_match {
 	struct efx_tc_match_fields value;
 	struct efx_tc_match_fields mask;
+	struct efx_tc_encap_match *encap;
 };
 
 struct efx_tc_action_set_list {