[ovs-dev] [PATCH 3/3] tunnel: Set ECN mask bits only when it is matched in the IP header

Sriharsha Basavapatna sriharsha.basavapatna at broadcom.com
Fri Oct 16 11:46:30 UTC 2020


On Wed, Oct 14, 2020 at 8:29 PM Mark Gray <mark.d.gray at redhat.com> wrote:
>
> This seems to break tests for me? Did you run a "make check"? I didn't
> apply the whole series because patch 2 doesn't apply so maybe that is
> needed for the tests to pass.

I rebased patch-2 and it now applies fine. But I'm seeing errors like
you mentioned with a "make check".  And the errors are seen only with
this patch (Set ECN...) These are the tests:
0758 - tunnel - Geneve option present
0759 - tunnel - Delete Geneve option
0763 - tunnel - Mix Geneve/GRE options
2270 - ptap - triangle bridge setup with L2 and L3 GRE tunnels
2273 - ptap - L3 over patch port

I looked at the testsuite.log for these tests, but it is not clear to
me why they are failing.

Ilya,

Could this be related to your comment on another patch ("Support vxlan
encap offload with load actions"), where you mentioned: "It seems like
tunnel metadata and a couple of other fields are special cases that
allowed to have masks set without having keys."

I'm wondering if this fix should also be moved into the offload layer.
With vxlan encap action we noticed that the mask for the IP tos field
was set though the field itself was not really being matched. Datapath
rule looks like this (tos=0/0x3 below).

ufid:255d198a-72ab-47a8-9a2b-a7e55feb66f5,
skb_priority(0/0),skb_mark(0/0),ct_state(0/0),ct_zone(0/0),ct_mark(0/0),ct_label(0/0),recirc_id(0),dp_hash(0/0),in_port(port1),packet_type(ns=0,id=0),eth(src=8e:40:64:9c:02:72,dst=36:b2:fc:ba:37:cc),eth_type(0x0800),ipv4(src=192.168.11.3/0.0.0.0,dst=192.168.11.1/0.0.0.0,proto=1/0,tos=0/0x3,ttl=64/0,frag=no),icmp(type=0/0,code=0/0),
packets:255, bytes:24990, used:0.000s, dp:ovs,
actions:clone(tnl_push(tnl_port(.....).....

 Any thoughts ?

Thanks,
-Harsha


>
> Also, could you add a description in the commit message to make it clear
> this should be done?
>
> This comment now also looks wrong?
>
> >      /* ECN fields are always inherited. */
>


More information about the dev mailing list