Merge branch 'mlxsw-sampling-fixes'

Ido Schimmel says:

====================
mlxsw: Two sampling fixes

This patchset fixes two bugs in recent sampling submissions.

The first fix, in patch #3, prevents matchall rules with sample action
to be added in front of flower rules on egress. Patches #1-#2 are
preparations meant at avoiding similar bugs in the future. Patch #4 is a
selftest.

The second fix, in patch #5, prevents sampling from being enabled on a
port if already enabled. Patch #6 is a selftest.
====================
Signed-off-by: David S. Miller <davem@davemloft.net>

drivers/net/ethernet/mellanox/mlxsw/spectrum.c

@@ -2668,6 +2668,11 @@ mlxsw_sp_sample_trigger_params_set(struct mlxsw_sp *mlxsw_sp,
 		return mlxsw_sp_sample_trigger_node_init(mlxsw_sp, &key,
 							 params);
 
+	if (trigger_node->trigger.local_port) {
+		NL_SET_ERR_MSG_MOD(extack, "Sampling already enabled on port");
+		return -EINVAL;
+	}
+
 	if (trigger_node->params.psample_group != params->psample_group ||
 	    trigger_node->params.truncate != params->truncate ||
 	    trigger_node->params.rate != params->rate ||

drivers/net/ethernet/mellanox/mlxsw/spectrum_matchall.c

@@ -238,6 +238,11 @@ int mlxsw_sp_mall_replace(struct mlxsw_sp *mlxsw_sp,
 		flower_prio_valid = true;
 	}
 
+	if (protocol != htons(ETH_P_ALL)) {
+		NL_SET_ERR_MSG(f->common.extack, "matchall rules only supported with 'all' protocol");
+		return -EOPNOTSUPP;
+	}
+
 	mall_entry = kzalloc(sizeof(*mall_entry), GFP_KERNEL);
 	if (!mall_entry)
 		return -ENOMEM;
@@ -245,9 +250,6 @@ int mlxsw_sp_mall_replace(struct mlxsw_sp *mlxsw_sp,
 	mall_entry->priority = f->common.prio;
 	mall_entry->ingress = mlxsw_sp_flow_block_is_ingress_bound(block);
 
-	act = &f->rule->action.entries[0];
-
-	if (act->id == FLOW_ACTION_MIRRED && protocol == htons(ETH_P_ALL)) {
 	if (flower_prio_valid && mall_entry->ingress &&
 	    mall_entry->priority >= flower_min_prio) {
 		NL_SET_ERR_MSG(f->common.extack, "Failed to add behind existing flower rules");
@@ -260,22 +262,22 @@ int mlxsw_sp_mall_replace(struct mlxsw_sp *mlxsw_sp,
 		err = -EOPNOTSUPP;
 		goto errout;
 	}
+
+	act = &f->rule->action.entries[0];
+
+	switch (act->id) {
+	case FLOW_ACTION_MIRRED:
 		mall_entry->type = MLXSW_SP_MALL_ACTION_TYPE_MIRROR;
 		mall_entry->mirror.to_dev = act->dev;
-	} else if (act->id == FLOW_ACTION_SAMPLE &&
-		   protocol == htons(ETH_P_ALL)) {
-		if (flower_prio_valid &&
-		    mall_entry->priority >= flower_min_prio) {
-			NL_SET_ERR_MSG(f->common.extack, "Failed to add behind existing flower rules");
-			err = -EOPNOTSUPP;
-			goto errout;
-		}
+		break;
+	case FLOW_ACTION_SAMPLE:
 		mall_entry->type = MLXSW_SP_MALL_ACTION_TYPE_SAMPLE;
 		mall_entry->sample.params.psample_group = act->sample.psample_group;
 		mall_entry->sample.params.truncate = act->sample.truncate;
 		mall_entry->sample.params.trunc_size = act->sample.trunc_size;
 		mall_entry->sample.params.rate = act->sample.rate;
-	} else {
+		break;
+	default:
 		err = -EOPNOTSUPP;
 		goto errout;
 	}

tools/testing/selftests/drivers/net/mlxsw/tc_restrictions.sh

@@ -11,6 +11,7 @@ ALL_TESTS="
 	matchall_mirror_behind_flower_ingress_test
 	matchall_sample_behind_flower_ingress_test
 	matchall_mirror_behind_flower_egress_test
+	matchall_proto_match_test
 	police_limits_test
 	multi_police_test
 "
@@ -291,6 +292,22 @@ matchall_mirror_behind_flower_egress_test()
 	matchall_behind_flower_egress_test "mirror" "mirred egress mirror dev $swp2"
 }
 
+matchall_proto_match_test()
+{
+	RET=0
+
+	tc qdisc add dev $swp1 clsact
+
+	tc filter add dev $swp1 ingress pref 1 proto ip handle 101 \
+		matchall skip_sw \
+		action sample group 1 rate 100
+	check_fail $? "Incorrect success to add matchall rule with protocol match"
+
+	tc qdisc del dev $swp1 clsact
+
+	log_test "matchall protocol match"
+}
+
 police_limits_test()
 {
 	RET=0

tools/testing/selftests/drivers/net/mlxsw/tc_sample.sh

@@ -34,6 +34,7 @@ lib_dir=$(dirname $0)/../../../net/forwarding
 ALL_TESTS="
 	tc_sample_rate_test
 	tc_sample_max_rate_test
+	tc_sample_conflict_test
 	tc_sample_group_conflict_test
 	tc_sample_md_iif_test
 	tc_sample_md_lag_iif_test
@@ -272,6 +273,35 @@ tc_sample_max_rate_test()
 	log_test "tc sample maximum rate"
 }
 
+tc_sample_conflict_test()
+{
+	RET=0
+
+	# Test that two sampling rules cannot be configured on the same port,
+	# even when they share the same parameters.
+
+	tc filter add dev $rp1 ingress protocol all pref 1 handle 101 matchall \
+		skip_sw action sample rate 1024 group 1
+	check_err $? "Failed to configure sampling rule"
+
+	tc filter add dev $rp1 ingress protocol all pref 2 handle 102 matchall \
+		skip_sw action sample rate 1024 group 1 &> /dev/null
+	check_fail $? "Managed to configure second sampling rule"
+
+	# Delete the first rule and make sure the second rule can now be
+	# configured.
+
+	tc filter del dev $rp1 ingress protocol all pref 1 handle 101 matchall
+
+	tc filter add dev $rp1 ingress protocol all pref 2 handle 102 matchall \
+		skip_sw action sample rate 1024 group 1
+	check_err $? "Failed to configure sampling rule after deletion"
+
+	log_test "tc sample conflict test"
+
+	tc filter del dev $rp1 ingress protocol all pref 2 handle 102 matchall
+}
+
 tc_sample_group_conflict_test()
 {
 	RET=0