diff mbox series

[net-next,4/5] sfc: add code to register and unregister encap matches

Message ID 27d54534188ab35e875d8c79daf1f59ecf66f2c5.1678815095.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 WARNING: line length of 86 exceeds 80 columns WARNING: line length of 89 exceeds 80 columns WARNING: line length of 94 exceeds 80 columns WARNING: line length of 95 exceeds 80 columns WARNING: line length of 96 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 14, 2023, 5:35 p.m. UTC
From: Edward Cree <ecree.xilinx@gmail.com>

Add a hashtable to detect duplicate and conflicting matches.  If match
 is not a duplicate, call MAE functions to add/remove it from OR table.
Calling code not added yet, so mark the new functions as unused.

Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
---
 drivers/net/ethernet/sfc/tc.c | 176 ++++++++++++++++++++++++++++++++++
 drivers/net/ethernet/sfc/tc.h |  11 +++
 2 files changed, 187 insertions(+)

Comments

Michal Swiatkowski March 15, 2023, 9:43 a.m. UTC | #1
On Tue, Mar 14, 2023 at 05:35:24PM +0000, edward.cree@amd.com wrote:
> From: Edward Cree <ecree.xilinx@gmail.com>
> 
> Add a hashtable to detect duplicate and conflicting matches.  If match
>  is not a duplicate, call MAE functions to add/remove it from OR table.
> Calling code not added yet, so mark the new functions as unused.
> 
> Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
> ---
>  drivers/net/ethernet/sfc/tc.c | 176 ++++++++++++++++++++++++++++++++++
>  drivers/net/ethernet/sfc/tc.h |  11 +++
>  2 files changed, 187 insertions(+)
> 
> diff --git a/drivers/net/ethernet/sfc/tc.c b/drivers/net/ethernet/sfc/tc.c
> index d683665a8d87..dc092403af12 100644
> --- a/drivers/net/ethernet/sfc/tc.c
> +++ b/drivers/net/ethernet/sfc/tc.c
> @@ -57,6 +57,12 @@ static s64 efx_tc_flower_external_mport(struct efx_nic *efx, struct efx_rep *efv
>  	return mport;
>  }
>  
> +static const struct rhashtable_params efx_tc_encap_match_ht_params = {
> +	.key_len	= offsetof(struct efx_tc_encap_match, linkage),
> +	.key_offset	= 0,
> +	.head_offset	= offsetof(struct efx_tc_encap_match, linkage),
> +};
> +
>  static const struct rhashtable_params efx_tc_match_action_ht_params = {
>  	.key_len	= sizeof(unsigned long),
>  	.key_offset	= offsetof(struct efx_tc_flow_rule, cookie),
> @@ -344,6 +350,157 @@ static int efx_tc_flower_parse_match(struct efx_nic *efx,
>  	return 0;
>  }
>  
> +__always_unused
> +static int efx_tc_flower_record_encap_match(struct efx_nic *efx,
> +					    struct efx_tc_match *match,
> +					    enum efx_encap_type type,
> +					    struct netlink_ext_ack *extack)
> +{
> +	struct efx_tc_encap_match *encap, *old;
> +	unsigned char ipv;
int? or even boolean is_ipv4

> +	int rc;
> +
> +	/* We require that the socket-defining fields (IP addrs and UDP dest
> +	 * port) are present and exact-match.  Other fields are currently not
> +	 * allowed.  This meets what OVS will ask for, and means that we don't
> +	 * need to handle difficult checks for overlapping matches as could
> +	 * come up if we allowed masks or varying sets of match fields.
> +	 */
> +	if (match->mask.enc_dst_ip | match->mask.enc_src_ip) {
> +		ipv = 4;
> +		if (!IS_ALL_ONES(match->mask.enc_dst_ip)) {
> +			NL_SET_ERR_MSG_MOD(extack,
> +					   "Egress encap match is not exact on dst IP address");
> +			return -EOPNOTSUPP;
> +		}
> +		if (!IS_ALL_ONES(match->mask.enc_src_ip)) {
> +			NL_SET_ERR_MSG_MOD(extack,
> +					   "Egress encap match is not exact on src IP address");
Do You mean that only exact match is supported?

> +			return -EOPNOTSUPP;
> +		}
> +#ifdef CONFIG_IPV6
> +		if (!ipv6_addr_any(&match->mask.enc_dst_ip6) ||
> +		    !ipv6_addr_any(&match->mask.enc_src_ip6)) {
> +			NL_SET_ERR_MSG_MOD(extack,
> +					   "Egress encap match on both IPv4 and IPv6, don't understand");
> +			return -EOPNOTSUPP;
> +		}
> +	} else {
> +		ipv = 6;
> +		if (!efx_ipv6_addr_all_ones(&match->mask.enc_dst_ip6)) {
> +			NL_SET_ERR_MSG_MOD(extack,
> +					   "Egress encap match is not exact on dst IP address");
> +			return -EOPNOTSUPP;
> +		}
> +		if (!efx_ipv6_addr_all_ones(&match->mask.enc_src_ip6)) {
> +			NL_SET_ERR_MSG_MOD(extack,
> +					   "Egress encap match is not exact on src IP address");
> +			return -EOPNOTSUPP;
> +		}
> +#endif
> +	}
> +	if (!IS_ALL_ONES(match->mask.enc_dport)) {
> +		NL_SET_ERR_MSG_MOD(extack, "Egress encap match is not exact on dst UDP port");
> +		return -EOPNOTSUPP;
> +	}
> +	if (match->mask.enc_sport) {
> +		NL_SET_ERR_MSG_MOD(extack, "Egress encap match on src UDP port not supported");
> +		return -EOPNOTSUPP;
> +	}
> +	if (match->mask.enc_ip_tos) {
> +		NL_SET_ERR_MSG_MOD(extack, "Egress encap match on IP ToS not supported");
> +		return -EOPNOTSUPP;
> +	}
> +	if (match->mask.enc_ip_ttl) {
> +		NL_SET_ERR_MSG_MOD(extack, "Egress encap match on IP TTL not supported");
> +		return -EOPNOTSUPP;
> +	}
> +
> +	rc = efx_mae_check_encap_match_caps(efx, ipv, extack);
> +	if (rc) {
> +		NL_SET_ERR_MSG_FMT_MOD(extack, "MAE hw reports no support for IPv%d encap matches",
> +				       ipv);
> +		return -EOPNOTSUPP;
> +	}
> +
> +	encap = kzalloc(sizeof(*encap), GFP_USER);
> +	if (!encap)
> +		return -ENOMEM;
> +	switch (ipv) {
> +	case 4:
> +		encap->src_ip = match->value.enc_src_ip;
> +		encap->dst_ip = match->value.enc_dst_ip;
> +		break;
> +#ifdef CONFIG_IPV6
> +	case 6:
> +		encap->src_ip6 = match->value.enc_src_ip6;
> +		encap->dst_ip6 = match->value.enc_dst_ip6;
> +		break;
> +#endif
> +	default: /* can't happen */
> +		NL_SET_ERR_MSG_FMT_MOD(extack, "Egress encap match on bad IP version %d",
> +				       ipv);
> +		rc = -EOPNOTSUPP;
> +		goto fail_allocated;
I will rewrite it to if. You will get rid of this unreachable code.

> +	}
> +	encap->udp_dport = match->value.enc_dport;
> +	encap->tun_type = type;
> +	old = rhashtable_lookup_get_insert_fast(&efx->tc->encap_match_ht,
> +						&encap->linkage,
> +						efx_tc_encap_match_ht_params);
> +	if (old) {
> +		/* don't need our new entry */
> +		kfree(encap);
> +		if (old->tun_type != type) {
> +			NL_SET_ERR_MSG_FMT_MOD(extack,
> +					       "Egress encap match with conflicting tun_type %u != %u",
> +					       old->tun_type, type);
> +			return -EEXIST;
> +		}
> +		if (!refcount_inc_not_zero(&old->ref))
> +			return -EAGAIN;
> +		/* existing entry found */
> +		encap = old;
> +	} else {
> +		rc = efx_mae_register_encap_match(efx, encap);
> +		if (rc) {
> +			NL_SET_ERR_MSG_MOD(extack, "Failed to record egress encap match in HW");
> +			goto fail_inserted;
> +		}
> +		refcount_set(&encap->ref, 1);
> +	}
> +	match->encap = encap;
> +	return 0;
> +fail_inserted:
> +	rhashtable_remove_fast(&efx->tc->encap_match_ht, &encap->linkage,
> +			       efx_tc_encap_match_ht_params);
> +fail_allocated:
> +	kfree(encap);
> +	return rc;
> +}
> +
[...]
Edward Cree March 15, 2023, 2:01 p.m. UTC | #2
On 15/03/2023 09:43, Michal Swiatkowski wrote:
> On Tue, Mar 14, 2023 at 05:35:24PM +0000, edward.cree@amd.com wrote:
>> +static int efx_tc_flower_record_encap_match(struct efx_nic *efx,
>> +					    struct efx_tc_match *match,
>> +					    enum efx_encap_type type,
>> +					    struct netlink_ext_ack *extack)
>> +{
>> +	struct efx_tc_encap_match *encap, *old;
>> +	unsigned char ipv;
> int? or even boolean is_ipv4
...
>> +	default: /* can't happen */
>> +		NL_SET_ERR_MSG_FMT_MOD(extack, "Egress encap match on bad IP version %d",
>> +				       ipv);
>> +		rc = -EOPNOTSUPP;
>> +		goto fail_allocated;
> I will rewrite it to if. You will get rid of this unreachable code.

Yeah, that's probably better.  Will do.
diff mbox series

Patch

diff --git a/drivers/net/ethernet/sfc/tc.c b/drivers/net/ethernet/sfc/tc.c
index d683665a8d87..dc092403af12 100644
--- a/drivers/net/ethernet/sfc/tc.c
+++ b/drivers/net/ethernet/sfc/tc.c
@@ -57,6 +57,12 @@  static s64 efx_tc_flower_external_mport(struct efx_nic *efx, struct efx_rep *efv
 	return mport;
 }
 
+static const struct rhashtable_params efx_tc_encap_match_ht_params = {
+	.key_len	= offsetof(struct efx_tc_encap_match, linkage),
+	.key_offset	= 0,
+	.head_offset	= offsetof(struct efx_tc_encap_match, linkage),
+};
+
 static const struct rhashtable_params efx_tc_match_action_ht_params = {
 	.key_len	= sizeof(unsigned long),
 	.key_offset	= offsetof(struct efx_tc_flow_rule, cookie),
@@ -344,6 +350,157 @@  static int efx_tc_flower_parse_match(struct efx_nic *efx,
 	return 0;
 }
 
+__always_unused
+static int efx_tc_flower_record_encap_match(struct efx_nic *efx,
+					    struct efx_tc_match *match,
+					    enum efx_encap_type type,
+					    struct netlink_ext_ack *extack)
+{
+	struct efx_tc_encap_match *encap, *old;
+	unsigned char ipv;
+	int rc;
+
+	/* We require that the socket-defining fields (IP addrs and UDP dest
+	 * port) are present and exact-match.  Other fields are currently not
+	 * allowed.  This meets what OVS will ask for, and means that we don't
+	 * need to handle difficult checks for overlapping matches as could
+	 * come up if we allowed masks or varying sets of match fields.
+	 */
+	if (match->mask.enc_dst_ip | match->mask.enc_src_ip) {
+		ipv = 4;
+		if (!IS_ALL_ONES(match->mask.enc_dst_ip)) {
+			NL_SET_ERR_MSG_MOD(extack,
+					   "Egress encap match is not exact on dst IP address");
+			return -EOPNOTSUPP;
+		}
+		if (!IS_ALL_ONES(match->mask.enc_src_ip)) {
+			NL_SET_ERR_MSG_MOD(extack,
+					   "Egress encap match is not exact on src IP address");
+			return -EOPNOTSUPP;
+		}
+#ifdef CONFIG_IPV6
+		if (!ipv6_addr_any(&match->mask.enc_dst_ip6) ||
+		    !ipv6_addr_any(&match->mask.enc_src_ip6)) {
+			NL_SET_ERR_MSG_MOD(extack,
+					   "Egress encap match on both IPv4 and IPv6, don't understand");
+			return -EOPNOTSUPP;
+		}
+	} else {
+		ipv = 6;
+		if (!efx_ipv6_addr_all_ones(&match->mask.enc_dst_ip6)) {
+			NL_SET_ERR_MSG_MOD(extack,
+					   "Egress encap match is not exact on dst IP address");
+			return -EOPNOTSUPP;
+		}
+		if (!efx_ipv6_addr_all_ones(&match->mask.enc_src_ip6)) {
+			NL_SET_ERR_MSG_MOD(extack,
+					   "Egress encap match is not exact on src IP address");
+			return -EOPNOTSUPP;
+		}
+#endif
+	}
+	if (!IS_ALL_ONES(match->mask.enc_dport)) {
+		NL_SET_ERR_MSG_MOD(extack, "Egress encap match is not exact on dst UDP port");
+		return -EOPNOTSUPP;
+	}
+	if (match->mask.enc_sport) {
+		NL_SET_ERR_MSG_MOD(extack, "Egress encap match on src UDP port not supported");
+		return -EOPNOTSUPP;
+	}
+	if (match->mask.enc_ip_tos) {
+		NL_SET_ERR_MSG_MOD(extack, "Egress encap match on IP ToS not supported");
+		return -EOPNOTSUPP;
+	}
+	if (match->mask.enc_ip_ttl) {
+		NL_SET_ERR_MSG_MOD(extack, "Egress encap match on IP TTL not supported");
+		return -EOPNOTSUPP;
+	}
+
+	rc = efx_mae_check_encap_match_caps(efx, ipv, extack);
+	if (rc) {
+		NL_SET_ERR_MSG_FMT_MOD(extack, "MAE hw reports no support for IPv%d encap matches",
+				       ipv);
+		return -EOPNOTSUPP;
+	}
+
+	encap = kzalloc(sizeof(*encap), GFP_USER);
+	if (!encap)
+		return -ENOMEM;
+	switch (ipv) {
+	case 4:
+		encap->src_ip = match->value.enc_src_ip;
+		encap->dst_ip = match->value.enc_dst_ip;
+		break;
+#ifdef CONFIG_IPV6
+	case 6:
+		encap->src_ip6 = match->value.enc_src_ip6;
+		encap->dst_ip6 = match->value.enc_dst_ip6;
+		break;
+#endif
+	default: /* can't happen */
+		NL_SET_ERR_MSG_FMT_MOD(extack, "Egress encap match on bad IP version %d",
+				       ipv);
+		rc = -EOPNOTSUPP;
+		goto fail_allocated;
+	}
+	encap->udp_dport = match->value.enc_dport;
+	encap->tun_type = type;
+	old = rhashtable_lookup_get_insert_fast(&efx->tc->encap_match_ht,
+						&encap->linkage,
+						efx_tc_encap_match_ht_params);
+	if (old) {
+		/* don't need our new entry */
+		kfree(encap);
+		if (old->tun_type != type) {
+			NL_SET_ERR_MSG_FMT_MOD(extack,
+					       "Egress encap match with conflicting tun_type %u != %u",
+					       old->tun_type, type);
+			return -EEXIST;
+		}
+		if (!refcount_inc_not_zero(&old->ref))
+			return -EAGAIN;
+		/* existing entry found */
+		encap = old;
+	} else {
+		rc = efx_mae_register_encap_match(efx, encap);
+		if (rc) {
+			NL_SET_ERR_MSG_MOD(extack, "Failed to record egress encap match in HW");
+			goto fail_inserted;
+		}
+		refcount_set(&encap->ref, 1);
+	}
+	match->encap = encap;
+	return 0;
+fail_inserted:
+	rhashtable_remove_fast(&efx->tc->encap_match_ht, &encap->linkage,
+			       efx_tc_encap_match_ht_params);
+fail_allocated:
+	kfree(encap);
+	return rc;
+}
+
+__always_unused
+static void efx_tc_flower_release_encap_match(struct efx_nic *efx,
+					      struct efx_tc_encap_match *encap)
+{
+	int rc;
+
+	if (!refcount_dec_and_test(&encap->ref))
+		return; /* still in use */
+
+	rc = efx_mae_unregister_encap_match(efx, encap);
+	if (rc)
+		/* Display message but carry on and remove entry from our
+		 * SW tables, because there's not much we can do about it.
+		 */
+		netif_err(efx, drv, efx->net_dev,
+			  "Failed to release encap match %#x, rc %d\n",
+			  encap->fw_id, rc);
+	rhashtable_remove_fast(&efx->tc->encap_match_ht, &encap->linkage,
+			       efx_tc_encap_match_ht_params);
+	kfree(encap);
+}
+
 /* For details of action order constraints refer to SF-123102-TC-1§12.6.1 */
 enum efx_tc_action_order {
 	EFX_TC_AO_VLAN_POP,
@@ -954,6 +1111,18 @@  void efx_fini_tc(struct efx_nic *efx)
 	efx->tc->up = false;
 }
 
+/* At teardown time, all TC filter rules (and thus all resources they created)
+ * should already have been removed.  If we find any in our hashtables, make a
+ * cursory attempt to clean up the software side.
+ */
+static void efx_tc_encap_match_free(void *ptr, void *__unused)
+{
+	struct efx_tc_encap_match *encap = ptr;
+
+	WARN_ON(refcount_read(&encap->ref));
+	kfree(encap);
+}
+
 int efx_init_struct_tc(struct efx_nic *efx)
 {
 	int rc;
@@ -976,6 +1145,9 @@  int efx_init_struct_tc(struct efx_nic *efx)
 	rc = efx_tc_init_counters(efx);
 	if (rc < 0)
 		goto fail_counters;
+	rc = rhashtable_init(&efx->tc->encap_match_ht, &efx_tc_encap_match_ht_params);
+	if (rc < 0)
+		goto fail_encap_match_ht;
 	rc = rhashtable_init(&efx->tc->match_action_ht, &efx_tc_match_action_ht_params);
 	if (rc < 0)
 		goto fail_match_action_ht;
@@ -988,6 +1160,8 @@  int efx_init_struct_tc(struct efx_nic *efx)
 	efx->extra_channel_type[EFX_EXTRA_CHANNEL_TC] = &efx_tc_channel_type;
 	return 0;
 fail_match_action_ht:
+	rhashtable_destroy(&efx->tc->encap_match_ht);
+fail_encap_match_ht:
 	efx_tc_destroy_counters(efx);
 fail_counters:
 	mutex_destroy(&efx->tc->mutex);
@@ -1010,6 +1184,8 @@  void efx_fini_struct_tc(struct efx_nic *efx)
 			     MC_CMD_MAE_ACTION_RULE_INSERT_OUT_ACTION_RULE_ID_NULL);
 	rhashtable_free_and_destroy(&efx->tc->match_action_ht, efx_tc_flow_free,
 				    efx);
+	rhashtable_free_and_destroy(&efx->tc->encap_match_ht,
+				    efx_tc_encap_match_free, NULL);
 	efx_tc_fini_counters(efx);
 	mutex_unlock(&efx->tc->mutex);
 	mutex_destroy(&efx->tc->mutex);
diff --git a/drivers/net/ethernet/sfc/tc.h b/drivers/net/ethernet/sfc/tc.h
index 19782c9a4354..d70c0ba86669 100644
--- a/drivers/net/ethernet/sfc/tc.h
+++ b/drivers/net/ethernet/sfc/tc.h
@@ -18,6 +18,13 @@ 
 
 #define IS_ALL_ONES(v)	(!(typeof (v))~(v))
 
+#ifdef CONFIG_IPV6
+static inline bool efx_ipv6_addr_all_ones(struct in6_addr *addr)
+{
+	return !memchr_inv(addr, 0xff, sizeof(*addr));
+}
+#endif
+
 struct efx_tc_action_set {
 	u16 vlan_push:2;
 	u16 vlan_pop:2;
@@ -70,7 +77,9 @@  struct efx_tc_encap_match {
 	__be32 src_ip, dst_ip;
 	struct in6_addr src_ip6, dst_ip6;
 	__be16 udp_dport;
+	struct rhash_head linkage;
 	u16 tun_type; /* enum efx_encap_type */
+	refcount_t ref;
 	u32 fw_id; /* index of this entry in firmware encap match table */
 };
 
@@ -107,6 +116,7 @@  enum efx_tc_rule_prios {
  * @mutex: Used to serialise operations on TC hashtables
  * @counter_ht: Hashtable of TC counters (FW IDs and counter values)
  * @counter_id_ht: Hashtable mapping TC counter cookies to counters
+ * @encap_match_ht: Hashtable of TC encap matches
  * @match_action_ht: Hashtable of TC match-action rules
  * @reps_mport_id: MAE port allocated for representor RX
  * @reps_filter_uc: VNIC filter for representor unicast RX (promisc)
@@ -130,6 +140,7 @@  struct efx_tc_state {
 	struct mutex mutex;
 	struct rhashtable counter_ht;
 	struct rhashtable counter_id_ht;
+	struct rhashtable encap_match_ht;
 	struct rhashtable match_action_ht;
 	u32 reps_mport_id, reps_mport_vport_id;
 	s32 reps_filter_uc, reps_filter_mc;