From: Ido Schimmel <idosch@nvidia.com>
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.
Ido Schimmel (6):
mlxsw: spectrum_matchall: Perform protocol check earlier
mlxsw: spectrum_matchall: Convert if statements to a switch statement
mlxsw: spectrum_matchall: Perform priority checks earlier
selftests: mlxsw: Test matchall failure with protocol match
mlxsw: spectrum: Veto sampling if already enabled on port
selftests: mlxsw: Test vetoing of double sampling
.../net/ethernet/mellanox/mlxsw/spectrum.c | 5 ++
.../mellanox/mlxsw/spectrum_matchall.c | 46 ++++++++++---------
.../drivers/net/mlxsw/tc_restrictions.sh | 17 +++++++
.../selftests/drivers/net/mlxsw/tc_sample.sh | 30 ++++++++++++
4 files changed, 76 insertions(+), 22 deletions(-)
--
2.30.2
From: Ido Schimmel <idosch@nvidia.com>
Perform the protocol check earlier in the function instead of repeating
it for every action. Example:
# tc filter add dev swp1 ingress proto ip matchall skip_sw action sample group 1 rate 100
Error: matchall rules only supported with 'all' protocol.
We have an error talking to the kernel
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
Reviewed-by: Jiri Pirko <redacted>
---
.../net/ethernet/mellanox/mlxsw/spectrum_matchall.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
@@ -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;
@@ -247,7 +252,7 @@ int mlxsw_sp_mall_replace(struct mlxsw_sp *mlxsw_sp,act=&f->rule->action.entries[0];-if(act->id==FLOW_ACTION_MIRRED&&protocol==htons(ETH_P_ALL)){+if(act->id==FLOW_ACTION_MIRRED){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");
@@ -262,8 +267,7 @@ int mlxsw_sp_mall_replace(struct mlxsw_sp *mlxsw_sp,}mall_entry->type=MLXSW_SP_MALL_ACTION_TYPE_MIRROR;mall_entry->mirror.to_dev=act->dev;-}elseif(act->id==FLOW_ACTION_SAMPLE&&-protocol==htons(ETH_P_ALL)){+}elseif(act->id==FLOW_ACTION_SAMPLE){if(flower_prio_valid&&mall_entry->priority>=flower_min_prio){NL_SET_ERR_MSG(f->common.extack,"Failed to add behind existing flower rules");
From: Ido Schimmel <idosch@nvidia.com>
Previous patch moved the protocol check out of the action check, so
these if statements can now be converted to a switch statement. Perform
the conversion.
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
Reviewed-by: Jiri Pirko <redacted>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum_matchall.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
From: Ido Schimmel <idosch@nvidia.com>
Perform the priority check earlier in the function instead of repeating
it for every action. This fixes a bug that allowed matchall rules with
sample action to be added in front of flower rules on egress.
Fixes: 54d0e963f683 ("mlxsw: spectrum_matchall: Add support for egress sampling")
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
Reviewed-by: Jiri Pirko <redacted>
---
.../mellanox/mlxsw/spectrum_matchall.c | 31 ++++++++-----------
1 file changed, 13 insertions(+), 18 deletions(-)
@@ -250,32 +250,27 @@ 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);+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");+err=-EOPNOTSUPP;+gotoerrout;+}+if(flower_prio_valid&&!mall_entry->ingress&&+mall_entry->priority<=flower_max_prio){+NL_SET_ERR_MSG(f->common.extack,"Failed to add in front of existing flower rules");+err=-EOPNOTSUPP;+gotoerrout;+}+act=&f->rule->action.entries[0];switch(act->id){caseFLOW_ACTION_MIRRED:-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");-err=-EOPNOTSUPP;-gotoerrout;-}-if(flower_prio_valid&&!mall_entry->ingress&&-mall_entry->priority<=flower_max_prio){-NL_SET_ERR_MSG(f->common.extack,"Failed to add in front of existing flower rules");-err=-EOPNOTSUPP;-gotoerrout;-}mall_entry->type=MLXSW_SP_MALL_ACTION_TYPE_MIRROR;mall_entry->mirror.to_dev=act->dev;break;caseFLOW_ACTION_SAMPLE:-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;-gotoerrout;-}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;
From: Ido Schimmel <idosch@nvidia.com>
Test that two sampling rules cannot be configured on the same port with
the same trigger.
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
Reviewed-by: Jiri Pirko <redacted>
---
.../selftests/drivers/net/mlxsw/tc_sample.sh | 30 +++++++++++++++++++
1 file changed, 30 insertions(+)
@@ -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.++tcfilteradddev$rp1ingressprotocolallpref1handle101matchall\+skip_swactionsamplerate1024group1+check_err$?"Failed to configure sampling rule"++tcfilteradddev$rp1ingressprotocolallpref2handle102matchall\+skip_swactionsamplerate1024group1&>/dev/null+check_fail$?"Managed to configure second sampling rule"++# Delete the first rule and make sure the second rule can now be+# configured.++tcfilterdeldev$rp1ingressprotocolallpref1handle101matchall++tcfilteradddev$rp1ingressprotocolallpref2handle102matchall\+skip_swactionsamplerate1024group1+check_err$?"Failed to configure sampling rule after deletion"++log_test"tc sample conflict test"++tcfilterdeldev$rp1ingressprotocolallpref2handle102matchall+}+ tc_sample_group_conflict_test(){RET=0
From: Ido Schimmel <idosch@nvidia.com>
The driver can only offload matchall rules that do not match on a
protocol. Test that matchall rules that match on a protocol are vetoed.
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
Reviewed-by: Jiri Pirko <redacted>
---
.../drivers/net/mlxsw/tc_restrictions.sh | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
Hello:
This series was applied to netdev/net-next.git (refs/heads/master):
On Mon, 29 Mar 2021 13:09:42 +0300 you wrote:
From: Ido Schimmel <idosch@nvidia.com>
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.
[...]