| Message ID | 20220706065320.3037932-1-vladbu@nvidia.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev] netdev-linux: set correct action for packets that passed policer | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/intel-ovs-compilation | success | test: success |
On 2022-07-06 9:53 AM, Vlad Buslov wrote: > Referenced commit changed policer action type from TC_ACT_UNSPEC (continue) > to TC_ACT_PIPE. However, since neither TC hardware offload layer nor mlx5 > driver at the time validated action type and always assumed 'continue', the > breakage wasn't caught until later validation code was added. The change > also broke valid configuration when sending from offload-capable device to > non-offload capable. For example, when sending from mlx5 VF to OvS bridge > netdevice the traffic that passed matchall classifier with policer could no > longer match the following flower rule in software: > > filter protocol all pref 1 matchall chain 0 > filter protocol all pref 1 matchall chain 0 handle 0x1 > in_hw (rule hit 7863) > action order 1: police 0x1 rate 32Mbit burst 1000Kb mtu 64Kb action drop/pipe overhead 0b > ref 1 bind 1 installed 17 sec firstused 17 sec > Action statistics: > Sent 152199634 bytes 102550 pkt (dropped 1315, overlimits 1315 requeues 0) > Sent software 74612172 bytes 51275 pkt > Sent hardware 77587462 bytes 51275 pkt > backlog 0b 0p requeues 0 > used_hw_stats delayed > > filter protocol ip pref 3 flower chain 0 > filter protocol ip pref 3 flower chain 0 handle 0x1 > dst_mac aa:94:1f:f2:f8:44 > src_mac e4:00:01:08:00:02 > eth_type ipv4 > ip_flags nofrag > not_in_hw > action order 1: skbedit ptype host pipe > index 1 ref 1 bind 1 installed 6 sec used 6 sec > Action statistics: > Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0) > backlog 0b 0p requeues 0 > > action order 2: mirred (Ingress Redirect to device br-ovs) stolen > index 1 ref 1 bind 1 installed 6 sec used 6 sec > Action statistics: > Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0) > backlog 0b 0p requeues 0 > cookie 401a9c8b3d403c62240d3eb5e21c1604 > no_percpu > > Fix the issue by restoring pps policer action type to 'continue'. > > Fixes: c2567e533f8a ("add port-based ingress policing based packet-per-second rate-limiting") > Signed-off-by: Vlad Buslov <vladbu@nvidia.com> > --- > lib/netdev-linux.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c > index 2766b3f2bf67..d73391624555 100644 > --- a/lib/netdev-linux.c > +++ b/lib/netdev-linux.c > @@ -2621,9 +2621,9 @@ nl_msg_act_police_start_nest(struct ofpbuf *request, uint32_t prio, > > static void > nl_msg_act_police_end_nest(struct ofpbuf *request, size_t offset, > - size_t act_offset) > + size_t act_offset, uint32_t notexceed_act) > { > - nl_msg_put_u32(request, TCA_POLICE_RESULT, TC_ACT_PIPE); > + nl_msg_put_u32(request, TCA_POLICE_RESULT, notexceed_act); > nl_msg_end_nested(request, offset); > nl_msg_end_nested(request, act_offset); > } > @@ -2643,7 +2643,7 @@ nl_msg_put_act_police(struct ofpbuf *request, struct tc_police police, > nl_msg_act_police_start_nest(request, ++prio, &offset, &act_offset); > tc_put_rtab(request, TCA_POLICE_RATE, &police.rate); > nl_msg_put_unspec(request, TCA_POLICE_TBF, &police, sizeof police); > - nl_msg_act_police_end_nest(request, offset, act_offset); > + nl_msg_act_police_end_nest(request, offset, act_offset, TC_ACT_UNSPEC); > } > if (kpkts_rate) { > unsigned int pkt_burst_ticks, pps_rate, size; > @@ -2658,7 +2658,7 @@ nl_msg_put_act_police(struct ofpbuf *request, struct tc_police police, > (uint64_t) pkt_burst_ticks); > nl_msg_put_unspec(request, TCA_POLICE_TBF, &null_police, > sizeof null_police); > - nl_msg_act_police_end_nest(request, offset, act_offset); > + nl_msg_act_police_end_nest(request, offset, act_offset, TC_ACT_PIPE); > } > } > Acked-by: Roi Dayan <roid@nvidia.com>
Dear maintainers, Could you please check out this patch? When kernel 5.19 is released with all the added validation code, it will make OvS matchall police offload completely broken on mlx5 without the fix. Thanks, Vlad On Wed 06 Jul 2022 at 08:53, Vlad Buslov <vladbu@nvidia.com> wrote: > Referenced commit changed policer action type from TC_ACT_UNSPEC (continue) > to TC_ACT_PIPE. However, since neither TC hardware offload layer nor mlx5 > driver at the time validated action type and always assumed 'continue', the > breakage wasn't caught until later validation code was added. The change > also broke valid configuration when sending from offload-capable device to > non-offload capable. For example, when sending from mlx5 VF to OvS bridge > netdevice the traffic that passed matchall classifier with policer could no > longer match the following flower rule in software: > > filter protocol all pref 1 matchall chain 0 > filter protocol all pref 1 matchall chain 0 handle 0x1 > in_hw (rule hit 7863) > action order 1: police 0x1 rate 32Mbit burst 1000Kb mtu 64Kb action drop/pipe overhead 0b > ref 1 bind 1 installed 17 sec firstused 17 sec > Action statistics: > Sent 152199634 bytes 102550 pkt (dropped 1315, overlimits 1315 requeues 0) > Sent software 74612172 bytes 51275 pkt > Sent hardware 77587462 bytes 51275 pkt > backlog 0b 0p requeues 0 > used_hw_stats delayed > > filter protocol ip pref 3 flower chain 0 > filter protocol ip pref 3 flower chain 0 handle 0x1 > dst_mac aa:94:1f:f2:f8:44 > src_mac e4:00:01:08:00:02 > eth_type ipv4 > ip_flags nofrag > not_in_hw > action order 1: skbedit ptype host pipe > index 1 ref 1 bind 1 installed 6 sec used 6 sec > Action statistics: > Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0) > backlog 0b 0p requeues 0 > > action order 2: mirred (Ingress Redirect to device br-ovs) stolen > index 1 ref 1 bind 1 installed 6 sec used 6 sec > Action statistics: > Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0) > backlog 0b 0p requeues 0 > cookie 401a9c8b3d403c62240d3eb5e21c1604 > no_percpu > > Fix the issue by restoring pps policer action type to 'continue'. > > Fixes: c2567e533f8a ("add port-based ingress policing based packet-per-second rate-limiting") > Signed-off-by: Vlad Buslov <vladbu@nvidia.com> > --- > lib/netdev-linux.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c > index 2766b3f2bf67..d73391624555 100644 > --- a/lib/netdev-linux.c > +++ b/lib/netdev-linux.c > @@ -2621,9 +2621,9 @@ nl_msg_act_police_start_nest(struct ofpbuf *request, uint32_t prio, > > static void > nl_msg_act_police_end_nest(struct ofpbuf *request, size_t offset, > - size_t act_offset) > + size_t act_offset, uint32_t notexceed_act) > { > - nl_msg_put_u32(request, TCA_POLICE_RESULT, TC_ACT_PIPE); > + nl_msg_put_u32(request, TCA_POLICE_RESULT, notexceed_act); > nl_msg_end_nested(request, offset); > nl_msg_end_nested(request, act_offset); > } > @@ -2643,7 +2643,7 @@ nl_msg_put_act_police(struct ofpbuf *request, struct tc_police police, > nl_msg_act_police_start_nest(request, ++prio, &offset, &act_offset); > tc_put_rtab(request, TCA_POLICE_RATE, &police.rate); > nl_msg_put_unspec(request, TCA_POLICE_TBF, &police, sizeof police); > - nl_msg_act_police_end_nest(request, offset, act_offset); > + nl_msg_act_police_end_nest(request, offset, act_offset, TC_ACT_UNSPEC); > } > if (kpkts_rate) { > unsigned int pkt_burst_ticks, pps_rate, size; > @@ -2658,7 +2658,7 @@ nl_msg_put_act_police(struct ofpbuf *request, struct tc_police police, > (uint64_t) pkt_burst_ticks); > nl_msg_put_unspec(request, TCA_POLICE_TBF, &null_police, > sizeof null_police); > - nl_msg_act_police_end_nest(request, offset, act_offset); > + nl_msg_act_police_end_nest(request, offset, act_offset, TC_ACT_PIPE); > } > }
On 7/25/22 14:45, Vlad Buslov via dev wrote: > Dear maintainers, > > Could you please check out this patch? > > When kernel 5.19 is released with all the added validation code, it will > make OvS matchall police offload completely broken on mlx5 without the > fix. Simon, Eelco, could you, please, take a look at this patch? Vlad, some re-work of this code went in since the patch submission, could you, please, rebase? I guess, the current version of the patch should still be reviewed in context of the backport to 2.17 and earlier branches. Best regards, Ilya Maximets. > > Thanks, > Vlad > > On Wed 06 Jul 2022 at 08:53, Vlad Buslov <vladbu@nvidia.com> wrote: >> Referenced commit changed policer action type from TC_ACT_UNSPEC (continue) >> to TC_ACT_PIPE. However, since neither TC hardware offload layer nor mlx5 >> driver at the time validated action type and always assumed 'continue', the >> breakage wasn't caught until later validation code was added. The change >> also broke valid configuration when sending from offload-capable device to >> non-offload capable. For example, when sending from mlx5 VF to OvS bridge >> netdevice the traffic that passed matchall classifier with policer could no >> longer match the following flower rule in software: >> >> filter protocol all pref 1 matchall chain 0 >> filter protocol all pref 1 matchall chain 0 handle 0x1 >> in_hw (rule hit 7863) >> action order 1: police 0x1 rate 32Mbit burst 1000Kb mtu 64Kb action drop/pipe overhead 0b >> ref 1 bind 1 installed 17 sec firstused 17 sec >> Action statistics: >> Sent 152199634 bytes 102550 pkt (dropped 1315, overlimits 1315 requeues 0) >> Sent software 74612172 bytes 51275 pkt >> Sent hardware 77587462 bytes 51275 pkt >> backlog 0b 0p requeues 0 >> used_hw_stats delayed >> >> filter protocol ip pref 3 flower chain 0 >> filter protocol ip pref 3 flower chain 0 handle 0x1 >> dst_mac aa:94:1f:f2:f8:44 >> src_mac e4:00:01:08:00:02 >> eth_type ipv4 >> ip_flags nofrag >> not_in_hw >> action order 1: skbedit ptype host pipe >> index 1 ref 1 bind 1 installed 6 sec used 6 sec >> Action statistics: >> Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0) >> backlog 0b 0p requeues 0 >> >> action order 2: mirred (Ingress Redirect to device br-ovs) stolen >> index 1 ref 1 bind 1 installed 6 sec used 6 sec >> Action statistics: >> Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0) >> backlog 0b 0p requeues 0 >> cookie 401a9c8b3d403c62240d3eb5e21c1604 >> no_percpu >> >> Fix the issue by restoring pps policer action type to 'continue'. >> >> Fixes: c2567e533f8a ("add port-based ingress policing based packet-per-second rate-limiting") >> Signed-off-by: Vlad Buslov <vladbu@nvidia.com> >> --- >> lib/netdev-linux.c | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c >> index 2766b3f2bf67..d73391624555 100644 >> --- a/lib/netdev-linux.c >> +++ b/lib/netdev-linux.c >> @@ -2621,9 +2621,9 @@ nl_msg_act_police_start_nest(struct ofpbuf *request, uint32_t prio, >> >> static void >> nl_msg_act_police_end_nest(struct ofpbuf *request, size_t offset, >> - size_t act_offset) >> + size_t act_offset, uint32_t notexceed_act) >> { >> - nl_msg_put_u32(request, TCA_POLICE_RESULT, TC_ACT_PIPE); >> + nl_msg_put_u32(request, TCA_POLICE_RESULT, notexceed_act); >> nl_msg_end_nested(request, offset); >> nl_msg_end_nested(request, act_offset); >> } >> @@ -2643,7 +2643,7 @@ nl_msg_put_act_police(struct ofpbuf *request, struct tc_police police, >> nl_msg_act_police_start_nest(request, ++prio, &offset, &act_offset); >> tc_put_rtab(request, TCA_POLICE_RATE, &police.rate); >> nl_msg_put_unspec(request, TCA_POLICE_TBF, &police, sizeof police); >> - nl_msg_act_police_end_nest(request, offset, act_offset); >> + nl_msg_act_police_end_nest(request, offset, act_offset, TC_ACT_UNSPEC); >> } >> if (kpkts_rate) { >> unsigned int pkt_burst_ticks, pps_rate, size; >> @@ -2658,7 +2658,7 @@ nl_msg_put_act_police(struct ofpbuf *request, struct tc_police police, >> (uint64_t) pkt_burst_ticks); >> nl_msg_put_unspec(request, TCA_POLICE_TBF, &null_police, >> sizeof null_police); >> - nl_msg_act_police_end_nest(request, offset, act_offset); >> + nl_msg_act_police_end_nest(request, offset, act_offset, TC_ACT_PIPE); >> } >> } > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On Mon, Jul 25, 2022 at 04:01:07PM +0200, Ilya Maximets wrote: > On 7/25/22 14:45, Vlad Buslov via dev wrote: > > Dear maintainers, > > > > Could you please check out this patch? > > > > When kernel 5.19 is released with all the added validation code, it will > > make OvS matchall police offload completely broken on mlx5 without the > > fix. > > Simon, Eelco, could you, please, take a look at this patch? > > Vlad, some re-work of this code went in since the patch submission, > could you, please, rebase? > > I guess, the current version of the patch should still be reviewed > in context of the backport to 2.17 and earlier branches. > > Best regards, Ilya Maximets. Thanks Ilya, this looks good to me. However, I'd like to get the team that wrote c2567e533f8a ("add port-based ingress policing based packet-per-second rate-limiting") to look over it. I'll contact them and either they or I will get back to you. Thanks in advance for your patience, Simon > > > > > Thanks, > > Vlad > > > > On Wed 06 Jul 2022 at 08:53, Vlad Buslov <vladbu@nvidia.com> wrote: > >> Referenced commit changed policer action type from TC_ACT_UNSPEC (continue) > >> to TC_ACT_PIPE. However, since neither TC hardware offload layer nor mlx5 > >> driver at the time validated action type and always assumed 'continue', the > >> breakage wasn't caught until later validation code was added. The change > >> also broke valid configuration when sending from offload-capable device to > >> non-offload capable. For example, when sending from mlx5 VF to OvS bridge > >> netdevice the traffic that passed matchall classifier with policer could no > >> longer match the following flower rule in software: > >> > >> filter protocol all pref 1 matchall chain 0 > >> filter protocol all pref 1 matchall chain 0 handle 0x1 > >> in_hw (rule hit 7863) > >> action order 1: police 0x1 rate 32Mbit burst 1000Kb mtu 64Kb action drop/pipe overhead 0b > >> ref 1 bind 1 installed 17 sec firstused 17 sec > >> Action statistics: > >> Sent 152199634 bytes 102550 pkt (dropped 1315, overlimits 1315 requeues 0) > >> Sent software 74612172 bytes 51275 pkt > >> Sent hardware 77587462 bytes 51275 pkt > >> backlog 0b 0p requeues 0 > >> used_hw_stats delayed > >> > >> filter protocol ip pref 3 flower chain 0 > >> filter protocol ip pref 3 flower chain 0 handle 0x1 > >> dst_mac aa:94:1f:f2:f8:44 > >> src_mac e4:00:01:08:00:02 > >> eth_type ipv4 > >> ip_flags nofrag > >> not_in_hw > >> action order 1: skbedit ptype host pipe > >> index 1 ref 1 bind 1 installed 6 sec used 6 sec > >> Action statistics: > >> Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0) > >> backlog 0b 0p requeues 0 > >> > >> action order 2: mirred (Ingress Redirect to device br-ovs) stolen > >> index 1 ref 1 bind 1 installed 6 sec used 6 sec > >> Action statistics: > >> Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0) > >> backlog 0b 0p requeues 0 > >> cookie 401a9c8b3d403c62240d3eb5e21c1604 > >> no_percpu > >> > >> Fix the issue by restoring pps policer action type to 'continue'. > >> > >> Fixes: c2567e533f8a ("add port-based ingress policing based packet-per-second rate-limiting") > >> Signed-off-by: Vlad Buslov <vladbu@nvidia.com> > >> --- > >> lib/netdev-linux.c | 8 ++++---- > >> 1 file changed, 4 insertions(+), 4 deletions(-) > >> > >> diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c > >> index 2766b3f2bf67..d73391624555 100644 > >> --- a/lib/netdev-linux.c > >> +++ b/lib/netdev-linux.c > >> @@ -2621,9 +2621,9 @@ nl_msg_act_police_start_nest(struct ofpbuf *request, uint32_t prio, > >> > >> static void > >> nl_msg_act_police_end_nest(struct ofpbuf *request, size_t offset, > >> - size_t act_offset) > >> + size_t act_offset, uint32_t notexceed_act) > >> { > >> - nl_msg_put_u32(request, TCA_POLICE_RESULT, TC_ACT_PIPE); > >> + nl_msg_put_u32(request, TCA_POLICE_RESULT, notexceed_act); > >> nl_msg_end_nested(request, offset); > >> nl_msg_end_nested(request, act_offset); > >> } > >> @@ -2643,7 +2643,7 @@ nl_msg_put_act_police(struct ofpbuf *request, struct tc_police police, > >> nl_msg_act_police_start_nest(request, ++prio, &offset, &act_offset); > >> tc_put_rtab(request, TCA_POLICE_RATE, &police.rate); > >> nl_msg_put_unspec(request, TCA_POLICE_TBF, &police, sizeof police); > >> - nl_msg_act_police_end_nest(request, offset, act_offset); > >> + nl_msg_act_police_end_nest(request, offset, act_offset, TC_ACT_UNSPEC); > >> } > >> if (kpkts_rate) { > >> unsigned int pkt_burst_ticks, pps_rate, size; > >> @@ -2658,7 +2658,7 @@ nl_msg_put_act_police(struct ofpbuf *request, struct tc_police police, > >> (uint64_t) pkt_burst_ticks); > >> nl_msg_put_unspec(request, TCA_POLICE_TBF, &null_police, > >> sizeof null_police); > >> - nl_msg_act_police_end_nest(request, offset, act_offset); > >> + nl_msg_act_police_end_nest(request, offset, act_offset, TC_ACT_PIPE); > >> } > >> } > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > >
On Mon 25 Jul 2022 at 16:01, Ilya Maximets <i.maximets@ovn.org> wrote: > On 7/25/22 14:45, Vlad Buslov via dev wrote: >> Dear maintainers, >> >> Could you please check out this patch? >> >> When kernel 5.19 is released with all the added validation code, it will >> make OvS matchall police offload completely broken on mlx5 without the >> fix. > > Simon, Eelco, could you, please, take a look at this patch? > > Vlad, some re-work of this code went in since the patch submission, > could you, please, rebase? Thanks, done! > > I guess, the current version of the patch should still be reviewed > in context of the backport to 2.17 and earlier branches. > > Best regards, Ilya Maximets. > >> >> Thanks, >> Vlad >> >> On Wed 06 Jul 2022 at 08:53, Vlad Buslov <vladbu@nvidia.com> wrote: >>> Referenced commit changed policer action type from TC_ACT_UNSPEC (continue) >>> to TC_ACT_PIPE. However, since neither TC hardware offload layer nor mlx5 >>> driver at the time validated action type and always assumed 'continue', the >>> breakage wasn't caught until later validation code was added. The change >>> also broke valid configuration when sending from offload-capable device to >>> non-offload capable. For example, when sending from mlx5 VF to OvS bridge >>> netdevice the traffic that passed matchall classifier with policer could no >>> longer match the following flower rule in software: >>> >>> filter protocol all pref 1 matchall chain 0 >>> filter protocol all pref 1 matchall chain 0 handle 0x1 >>> in_hw (rule hit 7863) >>> action order 1: police 0x1 rate 32Mbit burst 1000Kb mtu 64Kb action drop/pipe overhead 0b >>> ref 1 bind 1 installed 17 sec firstused 17 sec >>> Action statistics: >>> Sent 152199634 bytes 102550 pkt (dropped 1315, overlimits 1315 requeues 0) >>> Sent software 74612172 bytes 51275 pkt >>> Sent hardware 77587462 bytes 51275 pkt >>> backlog 0b 0p requeues 0 >>> used_hw_stats delayed >>> >>> filter protocol ip pref 3 flower chain 0 >>> filter protocol ip pref 3 flower chain 0 handle 0x1 >>> dst_mac aa:94:1f:f2:f8:44 >>> src_mac e4:00:01:08:00:02 >>> eth_type ipv4 >>> ip_flags nofrag >>> not_in_hw >>> action order 1: skbedit ptype host pipe >>> index 1 ref 1 bind 1 installed 6 sec used 6 sec >>> Action statistics: >>> Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0) >>> backlog 0b 0p requeues 0 >>> >>> action order 2: mirred (Ingress Redirect to device br-ovs) stolen >>> index 1 ref 1 bind 1 installed 6 sec used 6 sec >>> Action statistics: >>> Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0) >>> backlog 0b 0p requeues 0 >>> cookie 401a9c8b3d403c62240d3eb5e21c1604 >>> no_percpu >>> >>> Fix the issue by restoring pps policer action type to 'continue'. >>> >>> Fixes: c2567e533f8a ("add port-based ingress policing based packet-per-second rate-limiting") >>> Signed-off-by: Vlad Buslov <vladbu@nvidia.com> >>> --- >>> lib/netdev-linux.c | 8 ++++---- >>> 1 file changed, 4 insertions(+), 4 deletions(-) >>> >>> diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c >>> index 2766b3f2bf67..d73391624555 100644 >>> --- a/lib/netdev-linux.c >>> +++ b/lib/netdev-linux.c >>> @@ -2621,9 +2621,9 @@ nl_msg_act_police_start_nest(struct ofpbuf *request, uint32_t prio, >>> >>> static void >>> nl_msg_act_police_end_nest(struct ofpbuf *request, size_t offset, >>> - size_t act_offset) >>> + size_t act_offset, uint32_t notexceed_act) >>> { >>> - nl_msg_put_u32(request, TCA_POLICE_RESULT, TC_ACT_PIPE); >>> + nl_msg_put_u32(request, TCA_POLICE_RESULT, notexceed_act); >>> nl_msg_end_nested(request, offset); >>> nl_msg_end_nested(request, act_offset); >>> } >>> @@ -2643,7 +2643,7 @@ nl_msg_put_act_police(struct ofpbuf *request, struct tc_police police, >>> nl_msg_act_police_start_nest(request, ++prio, &offset, &act_offset); >>> tc_put_rtab(request, TCA_POLICE_RATE, &police.rate); >>> nl_msg_put_unspec(request, TCA_POLICE_TBF, &police, sizeof police); >>> - nl_msg_act_police_end_nest(request, offset, act_offset); >>> + nl_msg_act_police_end_nest(request, offset, act_offset, TC_ACT_UNSPEC); >>> } >>> if (kpkts_rate) { >>> unsigned int pkt_burst_ticks, pps_rate, size; >>> @@ -2658,7 +2658,7 @@ nl_msg_put_act_police(struct ofpbuf *request, struct tc_police police, >>> (uint64_t) pkt_burst_ticks); >>> nl_msg_put_unspec(request, TCA_POLICE_TBF, &null_police, >>> sizeof null_police); >>> - nl_msg_act_police_end_nest(request, offset, act_offset); >>> + nl_msg_act_police_end_nest(request, offset, act_offset, TC_ACT_PIPE); >>> } >>> } >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail.openvswitch.org%2Fmailman%2Flistinfo%2Fovs-dev&data=05%7C01%7Cvladbu%40nvidia.com%7Ce60d2462320e4bccc5ad08da6e462244%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637943544798758527%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=OQyDXq%2FOflU7pz2BC2tdqPACwnGraQ4avrtz7gpLEfI%3D&reserved=0 >>
diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c index 2766b3f2bf67..d73391624555 100644 --- a/lib/netdev-linux.c +++ b/lib/netdev-linux.c @@ -2621,9 +2621,9 @@ nl_msg_act_police_start_nest(struct ofpbuf *request, uint32_t prio, static void nl_msg_act_police_end_nest(struct ofpbuf *request, size_t offset, - size_t act_offset) + size_t act_offset, uint32_t notexceed_act) { - nl_msg_put_u32(request, TCA_POLICE_RESULT, TC_ACT_PIPE); + nl_msg_put_u32(request, TCA_POLICE_RESULT, notexceed_act); nl_msg_end_nested(request, offset); nl_msg_end_nested(request, act_offset); } @@ -2643,7 +2643,7 @@ nl_msg_put_act_police(struct ofpbuf *request, struct tc_police police, nl_msg_act_police_start_nest(request, ++prio, &offset, &act_offset); tc_put_rtab(request, TCA_POLICE_RATE, &police.rate); nl_msg_put_unspec(request, TCA_POLICE_TBF, &police, sizeof police); - nl_msg_act_police_end_nest(request, offset, act_offset); + nl_msg_act_police_end_nest(request, offset, act_offset, TC_ACT_UNSPEC); } if (kpkts_rate) { unsigned int pkt_burst_ticks, pps_rate, size; @@ -2658,7 +2658,7 @@ nl_msg_put_act_police(struct ofpbuf *request, struct tc_police police, (uint64_t) pkt_burst_ticks); nl_msg_put_unspec(request, TCA_POLICE_TBF, &null_police, sizeof null_police); - nl_msg_act_police_end_nest(request, offset, act_offset); + nl_msg_act_police_end_nest(request, offset, act_offset, TC_ACT_PIPE); } }
Referenced commit changed policer action type from TC_ACT_UNSPEC (continue) to TC_ACT_PIPE. However, since neither TC hardware offload layer nor mlx5 driver at the time validated action type and always assumed 'continue', the breakage wasn't caught until later validation code was added. The change also broke valid configuration when sending from offload-capable device to non-offload capable. For example, when sending from mlx5 VF to OvS bridge netdevice the traffic that passed matchall classifier with policer could no longer match the following flower rule in software: filter protocol all pref 1 matchall chain 0 filter protocol all pref 1 matchall chain 0 handle 0x1 in_hw (rule hit 7863) action order 1: police 0x1 rate 32Mbit burst 1000Kb mtu 64Kb action drop/pipe overhead 0b ref 1 bind 1 installed 17 sec firstused 17 sec Action statistics: Sent 152199634 bytes 102550 pkt (dropped 1315, overlimits 1315 requeues 0) Sent software 74612172 bytes 51275 pkt Sent hardware 77587462 bytes 51275 pkt backlog 0b 0p requeues 0 used_hw_stats delayed filter protocol ip pref 3 flower chain 0 filter protocol ip pref 3 flower chain 0 handle 0x1 dst_mac aa:94:1f:f2:f8:44 src_mac e4:00:01:08:00:02 eth_type ipv4 ip_flags nofrag not_in_hw action order 1: skbedit ptype host pipe index 1 ref 1 bind 1 installed 6 sec used 6 sec Action statistics: Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0) backlog 0b 0p requeues 0 action order 2: mirred (Ingress Redirect to device br-ovs) stolen index 1 ref 1 bind 1 installed 6 sec used 6 sec Action statistics: Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0) backlog 0b 0p requeues 0 cookie 401a9c8b3d403c62240d3eb5e21c1604 no_percpu Fix the issue by restoring pps policer action type to 'continue'. Fixes: c2567e533f8a ("add port-based ingress policing based packet-per-second rate-limiting") Signed-off-by: Vlad Buslov <vladbu@nvidia.com> --- lib/netdev-linux.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)