netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 0/5] mlxsw: spectrum_acl: Include delta bits into hashtable key
@ 2019-01-30  8:58 Ido Schimmel
  2019-01-30  8:58 ` [PATCH net-next 1/5] " Ido Schimmel
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Ido Schimmel @ 2019-01-30  8:58 UTC (permalink / raw)
  To: netdev; +Cc: davem, Jiri Pirko, mlxsw, Ido Schimmel

The Spectrum-2 ASIC allows multiple rules to use the same mask provided
that the difference between their masks is small enough (up to 8
consecutive delta bits). A more detailed explanation is provided in
merge commit 756cd36626f7 ("Merge branch
'mlxsw-Introduce-algorithmic-TCAM-support'").

These delta bits are part of the rule's key and therefore rules that
only differ in their delta bits can be inserted with the same A-TCAM
mask. In case two rules share the same key and only differ in their
priority, then the second will spill to the C-TCAM.

Current code does not take the delta bits into account when checking for
duplicate rules, which leads to unnecessary spillage to the C-TCAM.
This may result in reduced scale and performance.

Patch #1 includes the delta bits in the rule's key to avoid the above
mentioned problem.

Patch #2 adds a tracepoint when a rule is inserted into the C-TCAM.

Patches #3-#5 add test cases to make sure unnecessary spillage into the
C-TCAM does not occur.

Jiri Pirko (5):
  mlxsw: spectrum_acl: Include delta bits into hashtable key
  mlxsw: spectrum_acl: Add C-TCAM spill tracepoint
  selftests: spectrum-2: Extend and move trace helpers
  selftests: spectrum-2: Fix multiple_masks_test
  selftests: spectrum-2: Add delta two masks one key test

 .../mellanox/mlxsw/spectrum_acl_atcam.c       |  24 +--
 .../mlxsw/spectrum_acl_bloom_filter.c         |   2 +-
 .../mellanox/mlxsw/spectrum_acl_tcam.h        |  10 +-
 include/trace/events/mlxsw.h                  |  38 +++++
 .../drivers/net/mlxsw/spectrum-2/tc_flower.sh | 143 ++++++++++++++----
 5 files changed, 174 insertions(+), 43 deletions(-)
 create mode 100644 include/trace/events/mlxsw.h

-- 
2.20.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH net-next 1/5] mlxsw: spectrum_acl: Include delta bits into hashtable key
  2019-01-30  8:58 [PATCH net-next 0/5] mlxsw: spectrum_acl: Include delta bits into hashtable key Ido Schimmel
@ 2019-01-30  8:58 ` Ido Schimmel
  2019-01-30  8:58 ` [PATCH net-next 2/5] mlxsw: spectrum_acl: Add C-TCAM spill tracepoint Ido Schimmel
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Ido Schimmel @ 2019-01-30  8:58 UTC (permalink / raw)
  To: netdev; +Cc: davem, Jiri Pirko, mlxsw, Ido Schimmel

From: Jiri Pirko <jiri@mellanox.com>

Currently only ERP mask masked bits in key are considered for
the hashtable key. That leads to false negative collisions
and fallbacks to C-TCAM in case two keys differ only in delta bits.

Fix this by taking full encoded key as a hashtable key,
including delta bits.

Reported-by: Nir Dotan <nird@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
 .../mellanox/mlxsw/spectrum_acl_atcam.c       | 21 +++++++++----------
 .../mlxsw/spectrum_acl_bloom_filter.c         |  2 +-
 .../mellanox/mlxsw/spectrum_acl_tcam.h        | 10 +++++----
 3 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_atcam.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_atcam.c
index 40dc76a5c412..cda0a7170c34 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_atcam.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_atcam.c
@@ -390,8 +390,7 @@ mlxsw_sp_acl_atcam_region_entry_insert(struct mlxsw_sp *mlxsw_sp,
 	if (err)
 		return err;
 
-	lkey_id = aregion->ops->lkey_id_get(aregion, aentry->ht_key.enc_key,
-					    erp_id);
+	lkey_id = aregion->ops->lkey_id_get(aregion, aentry->enc_key, erp_id);
 	if (IS_ERR(lkey_id))
 		return PTR_ERR(lkey_id);
 	aentry->lkey_id = lkey_id;
@@ -399,7 +398,7 @@ mlxsw_sp_acl_atcam_region_entry_insert(struct mlxsw_sp *mlxsw_sp,
 	kvdl_index = mlxsw_afa_block_first_kvdl_index(rulei->act_block);
 	mlxsw_reg_ptce3_pack(ptce3_pl, true, MLXSW_REG_PTCE3_OP_WRITE_WRITE,
 			     priority, region->tcam_region_info,
-			     aentry->ht_key.enc_key, erp_id,
+			     aentry->enc_key, erp_id,
 			     aentry->delta_info.start,
 			     aentry->delta_info.mask,
 			     aentry->delta_info.value,
@@ -424,12 +423,11 @@ mlxsw_sp_acl_atcam_region_entry_remove(struct mlxsw_sp *mlxsw_sp,
 	struct mlxsw_sp_acl_atcam_lkey_id *lkey_id = aentry->lkey_id;
 	struct mlxsw_sp_acl_tcam_region *region = aregion->region;
 	u8 erp_id = mlxsw_sp_acl_erp_mask_erp_id(aentry->erp_mask);
-	char *enc_key = aentry->ht_key.enc_key;
 	char ptce3_pl[MLXSW_REG_PTCE3_LEN];
 
 	mlxsw_reg_ptce3_pack(ptce3_pl, false, MLXSW_REG_PTCE3_OP_WRITE_WRITE, 0,
 			     region->tcam_region_info,
-			     enc_key, erp_id,
+			     aentry->enc_key, erp_id,
 			     aentry->delta_info.start,
 			     aentry->delta_info.mask,
 			     aentry->delta_info.value,
@@ -458,7 +456,7 @@ mlxsw_sp_acl_atcam_region_entry_action_replace(struct mlxsw_sp *mlxsw_sp,
 	kvdl_index = mlxsw_afa_block_first_kvdl_index(rulei->act_block);
 	mlxsw_reg_ptce3_pack(ptce3_pl, true, MLXSW_REG_PTCE3_OP_WRITE_UPDATE,
 			     priority, region->tcam_region_info,
-			     aentry->ht_key.enc_key, erp_id,
+			     aentry->enc_key, erp_id,
 			     aentry->delta_info.start,
 			     aentry->delta_info.mask,
 			     aentry->delta_info.value,
@@ -481,15 +479,15 @@ __mlxsw_sp_acl_atcam_entry_add(struct mlxsw_sp *mlxsw_sp,
 	int err;
 
 	mlxsw_afk_encode(afk, region->key_info, &rulei->values,
-			 aentry->full_enc_key, mask);
+			 aentry->ht_key.full_enc_key, mask);
 
 	erp_mask = mlxsw_sp_acl_erp_mask_get(aregion, mask, false);
 	if (IS_ERR(erp_mask))
 		return PTR_ERR(erp_mask);
 	aentry->erp_mask = erp_mask;
 	aentry->ht_key.erp_id = mlxsw_sp_acl_erp_mask_erp_id(erp_mask);
-	memcpy(aentry->ht_key.enc_key, aentry->full_enc_key,
-	       sizeof(aentry->ht_key.enc_key));
+	memcpy(aentry->enc_key, aentry->ht_key.full_enc_key,
+	       sizeof(aentry->enc_key));
 
 	/* Compute all needed delta information and clear the delta bits
 	 * from the encrypted key.
@@ -498,8 +496,9 @@ __mlxsw_sp_acl_atcam_entry_add(struct mlxsw_sp *mlxsw_sp,
 	aentry->delta_info.start = mlxsw_sp_acl_erp_delta_start(delta);
 	aentry->delta_info.mask = mlxsw_sp_acl_erp_delta_mask(delta);
 	aentry->delta_info.value =
-		mlxsw_sp_acl_erp_delta_value(delta, aentry->full_enc_key);
-	mlxsw_sp_acl_erp_delta_clear(delta, aentry->ht_key.enc_key);
+		mlxsw_sp_acl_erp_delta_value(delta,
+					     aentry->ht_key.full_enc_key);
+	mlxsw_sp_acl_erp_delta_clear(delta, aentry->enc_key);
 
 	/* Add rule to the list of A-TCAM rules, assuming this
 	 * rule is intended to A-TCAM. In case this rule does
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
index f5c381dcb015..9545b572747e 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
@@ -133,7 +133,7 @@ mlxsw_sp_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
 		memcpy(chunk + MLXSW_BLOOM_CHUNK_PAD_BYTES, &erp_region_id,
 		       sizeof(erp_region_id));
 		memcpy(chunk + MLXSW_BLOOM_CHUNK_KEY_OFFSET,
-		       &aentry->ht_key.enc_key[chunk_key_offsets[chunk_index]],
+		       &aentry->enc_key[chunk_key_offsets[chunk_index]],
 		       MLXSW_BLOOM_CHUNK_KEY_BYTES);
 		chunk += MLXSW_BLOOM_KEY_CHUNK_BYTES;
 	}
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.h
index 10512b7c6d50..0858d5b06353 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.h
@@ -161,9 +161,9 @@ struct mlxsw_sp_acl_atcam_region {
 };
 
 struct mlxsw_sp_acl_atcam_entry_ht_key {
-	char enc_key[MLXSW_REG_PTCEX_FLEX_KEY_BLOCKS_LEN]; /* Encoded key,
-							    * minus delta bits.
-							    */
+	char full_enc_key[MLXSW_REG_PTCEX_FLEX_KEY_BLOCKS_LEN]; /* Encoded
+								 * key.
+								 */
 	u8 erp_id;
 };
 
@@ -175,7 +175,9 @@ struct mlxsw_sp_acl_atcam_entry {
 	struct rhash_head ht_node;
 	struct list_head list; /* Member in entries_list */
 	struct mlxsw_sp_acl_atcam_entry_ht_key ht_key;
-	char full_enc_key[MLXSW_REG_PTCEX_FLEX_KEY_BLOCKS_LEN]; /* Encoded key */
+	char enc_key[MLXSW_REG_PTCEX_FLEX_KEY_BLOCKS_LEN]; /* Encoded key,
+							    * minus delta bits.
+							    */
 	struct {
 		u16 start;
 		u8 mask;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH net-next 2/5] mlxsw: spectrum_acl: Add C-TCAM spill tracepoint
  2019-01-30  8:58 [PATCH net-next 0/5] mlxsw: spectrum_acl: Include delta bits into hashtable key Ido Schimmel
  2019-01-30  8:58 ` [PATCH net-next 1/5] " Ido Schimmel
@ 2019-01-30  8:58 ` Ido Schimmel
  2019-01-30  8:58 ` [PATCH net-next 3/5] selftests: spectrum-2: Extend and move trace helpers Ido Schimmel
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Ido Schimmel @ 2019-01-30  8:58 UTC (permalink / raw)
  To: netdev; +Cc: davem, Jiri Pirko, mlxsw, Ido Schimmel

From: Jiri Pirko <jiri@mellanox.com>

Add some visibility to the rule addition process and trace whenever rule
spilled into C-TCAM.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
 .../mellanox/mlxsw/spectrum_acl_atcam.c       |  3 ++
 include/trace/events/mlxsw.h                  | 38 +++++++++++++++++++
 2 files changed, 41 insertions(+)
 create mode 100644 include/trace/events/mlxsw.h

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_atcam.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_atcam.c
index cda0a7170c34..a74a390901ac 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_atcam.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_atcam.c
@@ -7,6 +7,8 @@
 #include <linux/gfp.h>
 #include <linux/refcount.h>
 #include <linux/rhashtable.h>
+#define CREATE_TRACE_POINTS
+#include <trace/events/mlxsw.h>
 
 #include "reg.h"
 #include "core.h"
@@ -578,6 +580,7 @@ int mlxsw_sp_acl_atcam_entry_add(struct mlxsw_sp *mlxsw_sp,
 	/* It is possible we failed to add the rule to the A-TCAM due to
 	 * exceeded number of masks. Try to spill into C-TCAM.
 	 */
+	trace_mlxsw_sp_acl_atcam_entry_add_ctcam_spill(mlxsw_sp, aregion);
 	err = mlxsw_sp_acl_ctcam_entry_add(mlxsw_sp, &aregion->cregion,
 					   &achunk->cchunk, &aentry->centry,
 					   rulei, true);
diff --git a/include/trace/events/mlxsw.h b/include/trace/events/mlxsw.h
new file mode 100644
index 000000000000..6c2bafcade18
--- /dev/null
+++ b/include/trace/events/mlxsw.h
@@ -0,0 +1,38 @@
+/* SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0 */
+/* Copyright (c) 2019 Mellanox Technologies. All rights reserved */
+
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM mlxsw
+
+#if !defined(_MLXSW_TRACEPOINT_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _MLXSW_TRACEPOINT_H
+
+#include <linux/tracepoint.h>
+
+struct mlxsw_sp;
+struct mlxsw_sp_acl_atcam_region;
+
+TRACE_EVENT(mlxsw_sp_acl_atcam_entry_add_ctcam_spill,
+	TP_PROTO(const struct mlxsw_sp *mlxsw_sp,
+		 const struct mlxsw_sp_acl_atcam_region *aregion),
+
+	TP_ARGS(mlxsw_sp, aregion),
+
+	TP_STRUCT__entry(
+		__field(const void *, mlxsw_sp)
+		__field(const void *, aregion)
+	),
+
+	TP_fast_assign(
+		__entry->mlxsw_sp = mlxsw_sp;
+		__entry->aregion = aregion;
+	),
+
+	TP_printk("mlxsw_sp %p, aregion %p",
+		  __entry->mlxsw_sp, __entry->aregion)
+);
+
+#endif /* _MLXSW_TRACEPOINT_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH net-next 3/5] selftests: spectrum-2: Extend and move trace helpers
  2019-01-30  8:58 [PATCH net-next 0/5] mlxsw: spectrum_acl: Include delta bits into hashtable key Ido Schimmel
  2019-01-30  8:58 ` [PATCH net-next 1/5] " Ido Schimmel
  2019-01-30  8:58 ` [PATCH net-next 2/5] mlxsw: spectrum_acl: Add C-TCAM spill tracepoint Ido Schimmel
@ 2019-01-30  8:58 ` Ido Schimmel
  2019-01-30  8:58 ` [PATCH net-next 4/5] selftests: spectrum-2: Fix multiple_masks_test Ido Schimmel
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Ido Schimmel @ 2019-01-30  8:58 UTC (permalink / raw)
  To: netdev; +Cc: davem, Jiri Pirko, mlxsw, Ido Schimmel

From: Jiri Pirko <jiri@mellanox.com>

Allow to specify number of trace hits and move helpers
to the beginning of the file.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
 .../drivers/net/mlxsw/spectrum-2/tc_flower.sh | 71 +++++++++++++------
 1 file changed, 49 insertions(+), 22 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/mlxsw/spectrum-2/tc_flower.sh b/tools/testing/selftests/drivers/net/mlxsw/spectrum-2/tc_flower.sh
index b41d6256b2d0..ed1ae902f2af 100755
--- a/tools/testing/selftests/drivers/net/mlxsw/spectrum-2/tc_flower.sh
+++ b/tools/testing/selftests/drivers/net/mlxsw/spectrum-2/tc_flower.sh
@@ -38,6 +38,55 @@ h2_destroy()
 	simple_if_fini $h2 192.0.2.2/24 198.51.100.2/24
 }
 
+tp_record()
+{
+	local tracepoint=$1
+	local cmd=$2
+
+	perf record -q -e $tracepoint $cmd
+	return $?
+}
+
+tp_record_all()
+{
+	local tracepoint=$1
+	local seconds=$2
+
+	perf record -a -q -e $tracepoint sleep $seconds
+	return $?
+}
+
+__tp_hit_count()
+{
+	local tracepoint=$1
+
+	local perf_output=`perf script -F trace:event,trace`
+	return `echo $perf_output | grep "$tracepoint:" | wc -l`
+}
+
+tp_check_hits()
+{
+	local tracepoint=$1
+	local count=$2
+
+	__tp_hit_count $tracepoint
+	if [[ "$?" -ne "$count" ]]; then
+		return 1
+	fi
+	return 0
+}
+
+tp_check_hits_any()
+{
+	local tracepoint=$1
+
+	__tp_hit_count $tracepoint
+	if [[ "$?" -eq "0" ]]; then
+		return 1
+	fi
+	return 0
+}
+
 single_mask_test()
 {
 	# When only a single mask is required, the device uses the master
@@ -325,28 +374,6 @@ ctcam_edge_cases_test()
 	ctcam_no_atcam_masks_test
 }
 
-tp_record()
-{
-	local tracepoint=$1
-	local cmd=$2
-
-	perf record -q -e $tracepoint $cmd
-	return $?
-}
-
-tp_check_hits()
-{
-	local tracepoint=$1
-	local count=$2
-
-	perf_output=`perf script -F trace:event,trace`
-	hits=`echo $perf_output | grep "$tracepoint:" | wc -l`
-	if [[ "$count" -ne "$hits" ]]; then
-		return 1
-	fi
-	return 0
-}
-
 delta_simple_test()
 {
 	# The first filter will create eRP, the second filter will fit into
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH net-next 4/5] selftests: spectrum-2: Fix multiple_masks_test
  2019-01-30  8:58 [PATCH net-next 0/5] mlxsw: spectrum_acl: Include delta bits into hashtable key Ido Schimmel
                   ` (2 preceding siblings ...)
  2019-01-30  8:58 ` [PATCH net-next 3/5] selftests: spectrum-2: Extend and move trace helpers Ido Schimmel
@ 2019-01-30  8:58 ` Ido Schimmel
  2019-01-30  8:58 ` [PATCH net-next 5/5] selftests: spectrum-2: Add delta two masks one key test Ido Schimmel
  2019-01-30 18:07 ` [PATCH net-next 0/5] mlxsw: spectrum_acl: Include delta bits into hashtable key David Miller
  5 siblings, 0 replies; 7+ messages in thread
From: Ido Schimmel @ 2019-01-30  8:58 UTC (permalink / raw)
  To: netdev; +Cc: davem, Jiri Pirko, mlxsw, Ido Schimmel

From: Jiri Pirko <jiri@mellanox.com>

With recent fix in C-TCAM spillage for delta masks, the test stops to be
falsely positive. So fix it not to use delta by adding src_ip bits to the
masks. Alongside with that, use C-TCAM spill trace to see when the
spillage actually happens.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
 .../drivers/net/mlxsw/spectrum-2/tc_flower.sh | 26 ++++++++++++++++---
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/mlxsw/spectrum-2/tc_flower.sh b/tools/testing/selftests/drivers/net/mlxsw/spectrum-2/tc_flower.sh
index ed1ae902f2af..73a35ca827ac 100755
--- a/tools/testing/selftests/drivers/net/mlxsw/spectrum-2/tc_flower.sh
+++ b/tools/testing/selftests/drivers/net/mlxsw/spectrum-2/tc_flower.sh
@@ -231,20 +231,38 @@ multiple_masks_test()
 	# spillage is performed correctly and that the right filter is
 	# matched
 
+	if [[ "$tcflags" != "skip_sw" ]]; then
+		return 0;
+	fi
+
 	local index
 
 	RET=0
 
 	NUM_MASKS=32
+	NUM_ERPS=16
 	BASE_INDEX=100
 
 	for i in $(eval echo {1..$NUM_MASKS}); do
 		index=$((BASE_INDEX - i))
 
-		tc filter add dev $h2 ingress protocol ip pref $index \
-			handle $index \
-			flower $tcflags dst_ip 192.0.2.2/${i} src_ip 192.0.2.1 \
-			action drop
+		if ((i > NUM_ERPS)); then
+			exp_hits=1
+			err_msg="$i filters - C-TCAM spill did not happen when it was expected"
+		else
+			exp_hits=0
+			err_msg="$i filters - C-TCAM spill happened when it should not"
+		fi
+
+		tp_record "mlxsw:mlxsw_sp_acl_atcam_entry_add_ctcam_spill" \
+			"tc filter add dev $h2 ingress protocol ip pref $index \
+				handle $index \
+				flower $tcflags \
+				dst_ip 192.0.2.2/${i} src_ip 192.0.2.1/${i} \
+				action drop"
+		tp_check_hits "mlxsw:mlxsw_sp_acl_atcam_entry_add_ctcam_spill" \
+				$exp_hits
+		check_err $? "$err_msg"
 
 		$MZ $h1 -c 1 -p 64 -a $h1mac -b $h2mac -A 192.0.2.1 \
 			-B 192.0.2.2 -t ip -q
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH net-next 5/5] selftests: spectrum-2: Add delta two masks one key test
  2019-01-30  8:58 [PATCH net-next 0/5] mlxsw: spectrum_acl: Include delta bits into hashtable key Ido Schimmel
                   ` (3 preceding siblings ...)
  2019-01-30  8:58 ` [PATCH net-next 4/5] selftests: spectrum-2: Fix multiple_masks_test Ido Schimmel
@ 2019-01-30  8:58 ` Ido Schimmel
  2019-01-30 18:07 ` [PATCH net-next 0/5] mlxsw: spectrum_acl: Include delta bits into hashtable key David Miller
  5 siblings, 0 replies; 7+ messages in thread
From: Ido Schimmel @ 2019-01-30  8:58 UTC (permalink / raw)
  To: netdev; +Cc: davem, Jiri Pirko, mlxsw, Ido Schimmel

From: Jiri Pirko <jiri@mellanox.com>

Ensure that the bug is fixed and we no longer have C-TCAM spill for two
keys that differ only in delta.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
 .../drivers/net/mlxsw/spectrum-2/tc_flower.sh | 46 ++++++++++++++++++-
 1 file changed, 45 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/drivers/net/mlxsw/spectrum-2/tc_flower.sh b/tools/testing/selftests/drivers/net/mlxsw/spectrum-2/tc_flower.sh
index 73a35ca827ac..f1922bf597b0 100755
--- a/tools/testing/selftests/drivers/net/mlxsw/spectrum-2/tc_flower.sh
+++ b/tools/testing/selftests/drivers/net/mlxsw/spectrum-2/tc_flower.sh
@@ -9,7 +9,8 @@ lib_dir=$(dirname $0)/../../../../net/forwarding
 
 ALL_TESTS="single_mask_test identical_filters_test two_masks_test \
 	multiple_masks_test ctcam_edge_cases_test delta_simple_test \
-	bloom_simple_test bloom_complex_test bloom_delta_test"
+	delta_two_masks_one_key_test bloom_simple_test \
+	bloom_complex_test bloom_delta_test"
 NUM_NETIFS=2
 source $lib_dir/tc_common.sh
 source $lib_dir/lib.sh
@@ -450,6 +451,49 @@ delta_simple_test()
 	log_test "delta simple test ($tcflags)"
 }
 
+delta_two_masks_one_key_test()
+{
+	# If 2 keys are the same and only differ in mask in a way that
+	# they belong under the same ERP (second is delta of the first),
+	# there should be no C-TCAM spill.
+
+	RET=0
+
+	if [[ "$tcflags" != "skip_sw" ]]; then
+		return 0;
+	fi
+
+	tp_record "mlxsw:*" "tc filter add dev $h2 ingress protocol ip \
+		   pref 1 handle 101 flower $tcflags dst_ip 192.0.2.0/24 \
+		   action drop"
+	tp_check_hits "mlxsw:mlxsw_sp_acl_atcam_entry_add_ctcam_spill" 0
+	check_err $? "incorrect C-TCAM spill while inserting the first rule"
+
+	tp_record "mlxsw:*" "tc filter add dev $h2 ingress protocol ip \
+		   pref 2 handle 102 flower $tcflags dst_ip 192.0.2.2 \
+		   action drop"
+	tp_check_hits "mlxsw:mlxsw_sp_acl_atcam_entry_add_ctcam_spill" 0
+	check_err $? "incorrect C-TCAM spill while inserting the second rule"
+
+	$MZ $h1 -c 1 -p 64 -a $h1mac -b $h2mac -A 192.0.2.1 -B 192.0.2.2 \
+		-t ip -q
+
+	tc_check_packets "dev $h2 ingress" 101 1
+	check_err $? "Did not match on correct filter"
+
+	tc filter del dev $h2 ingress protocol ip pref 1 handle 101 flower
+
+	$MZ $h1 -c 1 -p 64 -a $h1mac -b $h2mac -A 192.0.2.1 -B 192.0.2.2 \
+		-t ip -q
+
+	tc_check_packets "dev $h2 ingress" 102 1
+	check_err $? "Did not match on correct filter"
+
+	tc filter del dev $h2 ingress protocol ip pref 2 handle 102 flower
+
+	log_test "delta two masks one key test ($tcflags)"
+}
+
 bloom_simple_test()
 {
 	# Bloom filter requires that the eRP table is used. This test
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH net-next 0/5] mlxsw: spectrum_acl: Include delta bits into hashtable key
  2019-01-30  8:58 [PATCH net-next 0/5] mlxsw: spectrum_acl: Include delta bits into hashtable key Ido Schimmel
                   ` (4 preceding siblings ...)
  2019-01-30  8:58 ` [PATCH net-next 5/5] selftests: spectrum-2: Add delta two masks one key test Ido Schimmel
@ 2019-01-30 18:07 ` David Miller
  5 siblings, 0 replies; 7+ messages in thread
From: David Miller @ 2019-01-30 18:07 UTC (permalink / raw)
  To: idosch; +Cc: netdev, jiri, mlxsw

From: Ido Schimmel <idosch@mellanox.com>
Date: Wed, 30 Jan 2019 08:58:29 +0000

> The Spectrum-2 ASIC allows multiple rules to use the same mask provided
> that the difference between their masks is small enough (up to 8
> consecutive delta bits). A more detailed explanation is provided in
> merge commit 756cd36626f7 ("Merge branch
> 'mlxsw-Introduce-algorithmic-TCAM-support'").
> 
> These delta bits are part of the rule's key and therefore rules that
> only differ in their delta bits can be inserted with the same A-TCAM
> mask. In case two rules share the same key and only differ in their
> priority, then the second will spill to the C-TCAM.
> 
> Current code does not take the delta bits into account when checking for
> duplicate rules, which leads to unnecessary spillage to the C-TCAM.
> This may result in reduced scale and performance.
> 
> Patch #1 includes the delta bits in the rule's key to avoid the above
> mentioned problem.
> 
> Patch #2 adds a tracepoint when a rule is inserted into the C-TCAM.
> 
> Patches #3-#5 add test cases to make sure unnecessary spillage into the
> C-TCAM does not occur.

Series applied, thanks.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-01-30 18:07 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-30  8:58 [PATCH net-next 0/5] mlxsw: spectrum_acl: Include delta bits into hashtable key Ido Schimmel
2019-01-30  8:58 ` [PATCH net-next 1/5] " Ido Schimmel
2019-01-30  8:58 ` [PATCH net-next 2/5] mlxsw: spectrum_acl: Add C-TCAM spill tracepoint Ido Schimmel
2019-01-30  8:58 ` [PATCH net-next 3/5] selftests: spectrum-2: Extend and move trace helpers Ido Schimmel
2019-01-30  8:58 ` [PATCH net-next 4/5] selftests: spectrum-2: Fix multiple_masks_test Ido Schimmel
2019-01-30  8:58 ` [PATCH net-next 5/5] selftests: spectrum-2: Add delta two masks one key test Ido Schimmel
2019-01-30 18:07 ` [PATCH net-next 0/5] mlxsw: spectrum_acl: Include delta bits into hashtable key David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).