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 <vla...@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 <vla...@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
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
