All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.