On Tue, Aug 02, 2022 at 10:38:03AM +0300, Vlad Buslov wrote:
> On Wed 27 Jul 2022 at 11:49, Vlad Buslov <vla...@nvidia.com> wrote:
> > On Wed 27 Jul 2022 at 10:26, Simon Horman <simon.hor...@corigine.com> 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.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to