On Tue 02 Aug 2022 at 14:01, Simon Horman <[email protected]> wrote: > On Tue, Aug 02, 2022 at 10:38:03AM +0300, Vlad Buslov wrote: >> On Wed 27 Jul 2022 at 11:49, Vlad Buslov <[email protected]> wrote: >> > On Wed 27 Jul 2022 at 10:26, Simon Horman <[email protected]> >> > wrote: >> >> On Tue, Jul 26, 2022 at 07:52:31AM -0500, Marcelo Ricardo Leitner wrote: >> >>> On Tue, Jul 26, 2022 at 09:24:12AM +0200, 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: > > ... > >> Hi Simon, >> >> Any updates on this? I'm asking because 5.19 has been released and we >> would like to restore port rate limiting offload support with upstream >> OvS. > > Hi Vlad, > > I apologise for the delay in responding, and in turn that our investigation > took a little longer than it might have. > > Firstly, regarding BPS (byte/bit per-second) and PPS (packet per-second) > rate limits. It turns out that the need for TC_ACT_PIPE is unrelated > to BPS/PPS. And it was only included in the patch that introduced PPS > rate limiting as an artifact of the development process. Sorry about that. > > Secondly, with the introduction of metering the TC_ACT_PIPE behaviour came to > be relied on for some use cases. And unfortunately, because these case can > use either BPS or PPS the proposed patch causes a regression. > > The example I have is when a mirred action follows a meter. It is not > strictly related to conntrack (CT) but it is convenient for us to > illustrate the problem with an example that includes CT. > > Suppose that part of the OVS flow table looks like this: > > table=0,in_port=eth6,ct_state=-trk,ip,actions=ct(table=1) > table=1,in_port=eth6,ct_state=+trk+new,ip,actions=ct(commit),output:eth5 > table=1,in_port=eth6,ct_state=+trk+est,ip,actions=meter:1,output:eth5 > > And one of the corresponding TC rules is as follows: > > filter protocol ip pref 2 flower chain 1 > filter protocol ip pref 2 flower chain 1 handle 0x2 > eth_type ipv4 > ip_flags nofrag > ct_state +trk+est > in_hw in_hw_count 1 > action order 1: police 0x10000000 rate 4Gbit burst 125000Kb mtu 64Kb > action drop/continue overhead 0b linklayer unspec > ref 41 bind 40 installed 18 sec firstused 15 sec > Action statistics: > Sent 6876408827 bytes 4658827 pkt (dropped 0, overlimits 0 requeues 0) > backlog 0b 0p requeues 0 > used_hw_stats delayed > > action order 2: mirred (Egress Redirect to device eth5) stolen > index 3 ref 1 bind 1 installed 15 sec used 15 sec > Action statistics: > Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0) > backlog 0b 0p requeues 0 > cookie 20bea269bf40fdf3893eccbc6a7a07d8 > no_percpu > used_hw_stats delayed > > In this case, if the under-limit action for the policer action is TC_ACT_PIPE > then things work as expected (the packet is output to eth5 if the rate is > not exceeded). > > However, if the under-limit action for the policer action is TC_ACT_UNSPEC, > as in the example above, the mirred action is never hit and packets are not > output to eth5, regardless if the reate is exceded or not.
Thanks for the review! I've just sent V3 that sets action type based on target classifier instead of policer BPS/PPS. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
