On Mon 25 Jul 2022 at 16:01, Ilya Maximets <[email protected]> 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 <[email protected]> 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 <[email protected]> >>> --- >>> 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 >> [email protected] >> 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 >> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
