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 <[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://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
