| Message ID | 1d33ad9e757a7cc301f249a81c9cf61321c3daca.1636994047.git.lorenzo.bianconi@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] northd: fix FIP traffic with distributed gw router port on the same hv | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
On Mon, Nov 15, 2021 at 11:36 AM Lorenzo Bianconi <lorenzo.bianconi@redhat.com> wrote: > > If the hv has FIP assigned, traffic has to be sent out using the FIP > even if a distributed gw router port is scheduled on the local hv. > In this particular use-case without the proposed patch, the traffic > is sent out with FIP mac but using distributed gw router port IP. > > Related bz: https://bugzilla.redhat.com/show_bug.cgi?id=1960096 > > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> Thanks. I applied this patch to the main branch and branch-21.09. Numan > --- > northd/northd.c | 4 ++++ > tests/system-ovn.at | 16 ++++++++-------- > 2 files changed, 12 insertions(+), 8 deletions(-) > > diff --git a/northd/northd.c b/northd/northd.c > index 1e8a3457c..d10470a4e 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -12427,6 +12427,10 @@ build_lrouter_out_snat_flow(struct hmap *lflows, struct ovn_datapath *od, > priority += 128; > ds_put_format(match, " && is_chassis_resident(%s)", > od->l3dgw_ports[0]->cr_port->json_key); > + } else if (distributed) { > + priority += 128; > + ds_put_format(match, " && is_chassis_resident(\"%s\")", > + nat->logical_port); > } > ds_clear(actions); > > diff --git a/tests/system-ovn.at b/tests/system-ovn.at > index 77c811946..c9f5771c9 100644 > --- a/tests/system-ovn.at > +++ b/tests/system-ovn.at > @@ -3547,9 +3547,9 @@ NS_CHECK_EXEC([foo2], [ping -q -c 3 -i 0.3 -w 2 172.16.1.2 | FORMAT_PING], \ > ]) > > # We verify that SNAT indeed happened via 'dump-conntrack' command. > -AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(172.16.1.1) | \ > +AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(172.16.1.4) | \ > sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl > -icmp,orig=(src=192.168.1.3,dst=172.16.1.2,id=<cleared>,type=8,code=0),reply=(src=172.16.1.2,dst=172.16.1.1,id=<cleared>,type=0,code=0),zone=<cleared> > +icmp,orig=(src=192.168.1.3,dst=172.16.1.2,id=<cleared>,type=8,code=0),reply=(src=172.16.1.2,dst=172.16.1.4,id=<cleared>,type=0,code=0),zone=<cleared> > ]) > > AT_CHECK([ovs-appctl dpctl/flush-conntrack]) > @@ -3719,9 +3719,9 @@ NS_CHECK_EXEC([foo2], [ping6 -q -c 3 -i 0.3 -w 2 fd20::2 | FORMAT_PING], \ > ]) > > # We verify that SNAT indeed happened via 'dump-conntrack' command. > -AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(fd20::1) | \ > +AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(fd11::3) | \ > sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl > -icmpv6,orig=(src=fd11::3,dst=fd20::2,id=<cleared>,type=128,code=0),reply=(src=fd20::2,dst=fd20::1,id=<cleared>,type=129,code=0),zone=<cleared> > +icmpv6,orig=(src=fd11::3,dst=fd20::2,id=<cleared>,type=128,code=0),reply=(src=fd20::2,dst=fd11::3,id=<cleared>,type=129,code=0),zone=<cleared> > ]) > > AT_CHECK([ovs-appctl dpctl/flush-conntrack]) > @@ -3907,8 +3907,8 @@ NS_CHECK_EXEC([foo1], [ping -q -c 3 -i 0.3 -w 2 172.16.1.4 | FORMAT_PING], \ > # Then DNAT of 'bar1' address happens (listed first below). > AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(172.16.1.4) | \ > sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl > -icmp,orig=(src=172.16.1.1,dst=172.16.1.4,id=<cleared>,type=8,code=0),reply=(src=192.168.2.2,dst=172.16.1.1,id=<cleared>,type=0,code=0),zone=<cleared> > -icmp,orig=(src=192.168.1.2,dst=172.16.1.4,id=<cleared>,type=8,code=0),reply=(src=172.16.1.4,dst=172.16.1.1,id=<cleared>,type=0,code=0),zone=<cleared> > +icmp,orig=(src=172.16.1.3,dst=172.16.1.4,id=<cleared>,type=8,code=0),reply=(src=192.168.2.2,dst=172.16.1.3,id=<cleared>,type=0,code=0),zone=<cleared> > +icmp,orig=(src=192.168.1.2,dst=172.16.1.4,id=<cleared>,type=8,code=0),reply=(src=172.16.1.4,dst=172.16.1.3,id=<cleared>,type=0,code=0),zone=<cleared> > icmp,orig=(src=192.168.1.2,dst=172.16.1.4,id=<cleared>,type=8,code=0),reply=(src=172.16.1.4,dst=192.168.1.2,id=<cleared>,type=0,code=0),zone=<cleared> > ]) > > @@ -4102,8 +4102,8 @@ NS_CHECK_EXEC([foo1], [ping -q -c 3 -i 0.3 -w 2 fd20::4 | FORMAT_PING], \ > AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(fd20::4) | \ > sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl > icmpv6,orig=(src=fd11::2,dst=fd20::4,id=<cleared>,type=128,code=0),reply=(src=fd20::4,dst=fd11::2,id=<cleared>,type=129,code=0),zone=<cleared> > -icmpv6,orig=(src=fd11::2,dst=fd20::4,id=<cleared>,type=128,code=0),reply=(src=fd20::4,dst=fd20::1,id=<cleared>,type=129,code=0),zone=<cleared> > -icmpv6,orig=(src=fd20::1,dst=fd20::4,id=<cleared>,type=128,code=0),reply=(src=fd12::2,dst=fd20::1,id=<cleared>,type=129,code=0),zone=<cleared> > +icmpv6,orig=(src=fd11::2,dst=fd20::4,id=<cleared>,type=128,code=0),reply=(src=fd20::4,dst=fd20::3,id=<cleared>,type=129,code=0),zone=<cleared> > +icmpv6,orig=(src=fd20::3,dst=fd20::4,id=<cleared>,type=128,code=0),reply=(src=fd12::2,dst=fd20::3,id=<cleared>,type=129,code=0),zone=<cleared> > ]) > > AT_CHECK([ovs-appctl dpctl/flush-conntrack]) > -- > 2.31.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
diff --git a/northd/northd.c b/northd/northd.c index 1e8a3457c..d10470a4e 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -12427,6 +12427,10 @@ build_lrouter_out_snat_flow(struct hmap *lflows, struct ovn_datapath *od, priority += 128; ds_put_format(match, " && is_chassis_resident(%s)", od->l3dgw_ports[0]->cr_port->json_key); + } else if (distributed) { + priority += 128; + ds_put_format(match, " && is_chassis_resident(\"%s\")", + nat->logical_port); } ds_clear(actions); diff --git a/tests/system-ovn.at b/tests/system-ovn.at index 77c811946..c9f5771c9 100644 --- a/tests/system-ovn.at +++ b/tests/system-ovn.at @@ -3547,9 +3547,9 @@ NS_CHECK_EXEC([foo2], [ping -q -c 3 -i 0.3 -w 2 172.16.1.2 | FORMAT_PING], \ ]) # We verify that SNAT indeed happened via 'dump-conntrack' command. -AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(172.16.1.1) | \ +AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(172.16.1.4) | \ sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl -icmp,orig=(src=192.168.1.3,dst=172.16.1.2,id=<cleared>,type=8,code=0),reply=(src=172.16.1.2,dst=172.16.1.1,id=<cleared>,type=0,code=0),zone=<cleared> +icmp,orig=(src=192.168.1.3,dst=172.16.1.2,id=<cleared>,type=8,code=0),reply=(src=172.16.1.2,dst=172.16.1.4,id=<cleared>,type=0,code=0),zone=<cleared> ]) AT_CHECK([ovs-appctl dpctl/flush-conntrack]) @@ -3719,9 +3719,9 @@ NS_CHECK_EXEC([foo2], [ping6 -q -c 3 -i 0.3 -w 2 fd20::2 | FORMAT_PING], \ ]) # We verify that SNAT indeed happened via 'dump-conntrack' command. -AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(fd20::1) | \ +AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(fd11::3) | \ sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl -icmpv6,orig=(src=fd11::3,dst=fd20::2,id=<cleared>,type=128,code=0),reply=(src=fd20::2,dst=fd20::1,id=<cleared>,type=129,code=0),zone=<cleared> +icmpv6,orig=(src=fd11::3,dst=fd20::2,id=<cleared>,type=128,code=0),reply=(src=fd20::2,dst=fd11::3,id=<cleared>,type=129,code=0),zone=<cleared> ]) AT_CHECK([ovs-appctl dpctl/flush-conntrack]) @@ -3907,8 +3907,8 @@ NS_CHECK_EXEC([foo1], [ping -q -c 3 -i 0.3 -w 2 172.16.1.4 | FORMAT_PING], \ # Then DNAT of 'bar1' address happens (listed first below). AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(172.16.1.4) | \ sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl -icmp,orig=(src=172.16.1.1,dst=172.16.1.4,id=<cleared>,type=8,code=0),reply=(src=192.168.2.2,dst=172.16.1.1,id=<cleared>,type=0,code=0),zone=<cleared> -icmp,orig=(src=192.168.1.2,dst=172.16.1.4,id=<cleared>,type=8,code=0),reply=(src=172.16.1.4,dst=172.16.1.1,id=<cleared>,type=0,code=0),zone=<cleared> +icmp,orig=(src=172.16.1.3,dst=172.16.1.4,id=<cleared>,type=8,code=0),reply=(src=192.168.2.2,dst=172.16.1.3,id=<cleared>,type=0,code=0),zone=<cleared> +icmp,orig=(src=192.168.1.2,dst=172.16.1.4,id=<cleared>,type=8,code=0),reply=(src=172.16.1.4,dst=172.16.1.3,id=<cleared>,type=0,code=0),zone=<cleared> icmp,orig=(src=192.168.1.2,dst=172.16.1.4,id=<cleared>,type=8,code=0),reply=(src=172.16.1.4,dst=192.168.1.2,id=<cleared>,type=0,code=0),zone=<cleared> ]) @@ -4102,8 +4102,8 @@ NS_CHECK_EXEC([foo1], [ping -q -c 3 -i 0.3 -w 2 fd20::4 | FORMAT_PING], \ AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(fd20::4) | \ sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl icmpv6,orig=(src=fd11::2,dst=fd20::4,id=<cleared>,type=128,code=0),reply=(src=fd20::4,dst=fd11::2,id=<cleared>,type=129,code=0),zone=<cleared> -icmpv6,orig=(src=fd11::2,dst=fd20::4,id=<cleared>,type=128,code=0),reply=(src=fd20::4,dst=fd20::1,id=<cleared>,type=129,code=0),zone=<cleared> -icmpv6,orig=(src=fd20::1,dst=fd20::4,id=<cleared>,type=128,code=0),reply=(src=fd12::2,dst=fd20::1,id=<cleared>,type=129,code=0),zone=<cleared> +icmpv6,orig=(src=fd11::2,dst=fd20::4,id=<cleared>,type=128,code=0),reply=(src=fd20::4,dst=fd20::3,id=<cleared>,type=129,code=0),zone=<cleared> +icmpv6,orig=(src=fd20::3,dst=fd20::4,id=<cleared>,type=128,code=0),reply=(src=fd12::2,dst=fd20::3,id=<cleared>,type=129,code=0),zone=<cleared> ]) AT_CHECK([ovs-appctl dpctl/flush-conntrack])
If the hv has FIP assigned, traffic has to be sent out using the FIP even if a distributed gw router port is scheduled on the local hv. In this particular use-case without the proposed patch, the traffic is sent out with FIP mac but using distributed gw router port IP. Related bz: https://bugzilla.redhat.com/show_bug.cgi?id=1960096 Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> --- northd/northd.c | 4 ++++ tests/system-ovn.at | 16 ++++++++-------- 2 files changed, 12 insertions(+), 8 deletions(-)