| Message ID | 034361495642a40f342187be63c7f54115296d16.1562402015.git.lorenzo.bianconi@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,v3] OVN: run local logical flows first in S_ROUTER_OUT_SNAT table | expand |
Acked-by: Mark Michelson <mmichels@redhat.com> On 7/6/19 6:45 AM, Lorenzo Bianconi wrote: > Run local logical flows first if the gw router port is scheduled > on the local chassis in order to properly manage snat traffic > > Tested-by: Eran Kuris <ekuris@redhat.com> > Acked-by: Numan Siddique <nusiddiq@redhat.com> > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> > --- > Changes since v2: > - fix compilation error > Changes since v1: > - add priority change in ovn-northd.8.xml > --- > ovn/northd/ovn-northd.8.xml | 3 ++- > ovn/northd/ovn-northd.c | 7 +++++-- > 2 files changed, 7 insertions(+), 3 deletions(-) > > diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml > index 193aa210f..d2267de0e 100644 > --- a/ovn/northd/ovn-northd.8.xml > +++ b/ovn/northd/ovn-northd.8.xml > @@ -2428,7 +2428,8 @@ nd_ns { > <p> > If the NAT rule cannot be handled in a distributed manner, then > the flow above is only programmed on the > - <code>redirect-chassis</code>. > + <code>redirect-chassis</code> increasing flow priority by 128 in > + order to be run first > </p> > > <p> > diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c > index ba2719321..ce382ac89 100644 > --- a/ovn/northd/ovn-northd.c > +++ b/ovn/northd/ovn-northd.c > @@ -6634,6 +6634,8 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > count_1bits(ntohl(mask)) + 1, > ds_cstr(&match), ds_cstr(&actions)); > } else { > + uint16_t priority = count_1bits(ntohl(mask)) + 1; > + > /* Distributed router. */ > ds_clear(&match); > ds_put_format(&match, "ip && ip4.src == %s" > @@ -6643,6 +6645,7 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > if (!distributed && od->l3redirect_port) { > /* Flows for NAT rules that are centralized are only > * programmed on the "redirect-chassis". */ > + priority += 128; > ds_put_format(&match, " && is_chassis_resident(%s)", > od->l3redirect_port->json_key); > } > @@ -6657,8 +6660,8 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > * nat->logical_ip with the longest mask gets a higher > * priority. */ > ovn_lflow_add(lflows, od, S_ROUTER_OUT_SNAT, > - count_1bits(ntohl(mask)) + 1, > - ds_cstr(&match), ds_cstr(&actions)); > + priority, ds_cstr(&match), > + ds_cstr(&actions)); > } > } > >
On Sat, Jul 06, 2019 at 12:45:00PM +0200, Lorenzo Bianconi wrote: > Run local logical flows first if the gw router port is scheduled > on the local chassis in order to properly manage snat traffic > > Tested-by: Eran Kuris <ekuris@redhat.com> > Acked-by: Numan Siddique <nusiddiq@redhat.com> > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> > --- > Changes since v2: > - fix compilation error > Changes since v1: > - add priority change in ovn-northd.8.xml Thanks, applied to master.
The following tests consistently fail for kernel and userspace datapaths
136: ovn -- DNAT and SNAT on distributed router - N/S FAILED (
system-ovn.at:1337)
137: ovn -- DNAT and SNAT on distributed router - E/W FAILED (
system-ovn.at:1510)
after this commit
commit a6ee09882283426553e1a475e8b396af9bb378d0
Author: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
Date: Sat Jul 6 12:45:00 2019 +0200
OVN: run local logical flows first in S_ROUTER_OUT_SNAT table
Run local logical flows first if the gw router port is scheduled
on the local chassis in order to properly manage snat traffic
Tested-by: Eran Kuris <ekuris@redhat.com>
Acked-by: Numan Siddique <nusiddiq@redhat.com>
Acked-by: Mark Michelson <mmichels@redhat.com>
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
Signed-off-by: Ben Pfaff blp@ovn.org
*136: ovn -- DNAT and SNAT on distributed router - N/S FAILED
(system-ovn.at:1337 <http://system-ovn.at:1337>)*
Missing conntrack entry:
@@ -1,2 +1 @@
-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>
*137: ovn -- DNAT and SNAT on distributed router - E/W FAILED
(system-ovn.at:1510 <http://system-ovn.at:1510>)*
Missing conntrack entries:
+++
/home/dball/openvswitch/ovs/_gcc/tests/system-kmod-testsuite.dir/at-groups/137/stdout
2019-07-19
10:14:39.821883399 -0700
@@ -1,3 +1 @@
-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>
On Sat, Jul 6, 2019 at 3:48 AM Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
wrote:
> Run local logical flows first if the gw router port is scheduled
> on the local chassis in order to properly manage snat traffic
>
> Tested-by: Eran Kuris <ekuris@redhat.com>
> Acked-by: Numan Siddique <nusiddiq@redhat.com>
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
> ---
> Changes since v2:
> - fix compilation error
> Changes since v1:
> - add priority change in ovn-northd.8.xml
> ---
> ovn/northd/ovn-northd.8.xml | 3 ++-
> ovn/northd/ovn-northd.c | 7 +++++--
> 2 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml
> index 193aa210f..d2267de0e 100644
> --- a/ovn/northd/ovn-northd.8.xml
> +++ b/ovn/northd/ovn-northd.8.xml
> @@ -2428,7 +2428,8 @@ nd_ns {
> <p>
> If the NAT rule cannot be handled in a distributed manner, then
> the flow above is only programmed on the
> - <code>redirect-chassis</code>.
> + <code>redirect-chassis</code> increasing flow priority by 128 in
> + order to be run first
> </p>
>
> <p>
> diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> index ba2719321..ce382ac89 100644
> --- a/ovn/northd/ovn-northd.c
> +++ b/ovn/northd/ovn-northd.c
> @@ -6634,6 +6634,8 @@ build_lrouter_flows(struct hmap *datapaths, struct
> hmap *ports,
> count_1bits(ntohl(mask)) + 1,
> ds_cstr(&match), ds_cstr(&actions));
> } else {
> + uint16_t priority = count_1bits(ntohl(mask)) + 1;
> +
> /* Distributed router. */
> ds_clear(&match);
> ds_put_format(&match, "ip && ip4.src == %s"
> @@ -6643,6 +6645,7 @@ build_lrouter_flows(struct hmap *datapaths, struct
> hmap *ports,
> if (!distributed && od->l3redirect_port) {
> /* Flows for NAT rules that are centralized are
> only
> * programmed on the "redirect-chassis". */
> + priority += 128;
> ds_put_format(&match, " &&
> is_chassis_resident(%s)",
> od->l3redirect_port->json_key);
> }
> @@ -6657,8 +6660,8 @@ build_lrouter_flows(struct hmap *datapaths, struct
> hmap *ports,
> * nat->logical_ip with the longest mask gets a higher
> * priority. */
> ovn_lflow_add(lflows, od, S_ROUTER_OUT_SNAT,
> - count_1bits(ntohl(mask)) + 1,
> - ds_cstr(&match), ds_cstr(&actions));
> + priority, ds_cstr(&match),
> + ds_cstr(&actions));
> }
> }
>
> --
> 2.21.0
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
> The following tests consistently fail for kernel and userspace datapaths > > 136: ovn -- DNAT and SNAT on distributed router - N/S FAILED ( > system-ovn.at:1337) > 137: ovn -- DNAT and SNAT on distributed router - E/W FAILED ( > system-ovn.at:1510) > > after this commit > > commit a6ee09882283426553e1a475e8b396af9bb378d0 > > Author: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> > > Date: Sat Jul 6 12:45:00 2019 +0200 > > > > OVN: run local logical flows first in S_ROUTER_OUT_SNAT table > > > > Run local logical flows first if the gw router port is scheduled > > on the local chassis in order to properly manage snat traffic > > > > Tested-by: Eran Kuris <ekuris@redhat.com> > > Acked-by: Numan Siddique <nusiddiq@redhat.com> > > Acked-by: Mark Michelson <mmichels@redhat.com> > > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> > > Signed-off-by: Ben Pfaff blp@ovn.org > > > > > *136: ovn -- DNAT and SNAT on distributed router - N/S FAILED > (system-ovn.at:1337 <http://system-ovn.at:1337>)* Hi Darrell, thx a lot for the report. I guess this is a particular case since both the gw router port and the FIPs are on the same hypervisor. Could you please try the following patch: diff --git a/tests/system-ovn.at b/tests/system-ovn.at index 10fbd2649..a42cdf826 100644 --- a/tests/system-ovn.at +++ b/tests/system-ovn.at @@ -1334,11 +1334,13 @@ 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.4) | \ +AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(172.16.1.1) | \ 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.4,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.1,id=<cleared>,type=0,code=0),zone=<cleared> ]) +AT_CHECK([ovs-appctl dpctl/flush-conntrack]) + # South-North SNAT: 'bar1' pings 'alice1'. But 'alice1' receives traffic # from 172.16.1.1 NS_CHECK_EXEC([bar1], [ping -q -c 3 -i 0.3 -w 2 172.16.1.2 | FORMAT_PING], \ @@ -1347,7 +1349,7 @@ NS_CHECK_EXEC([bar1], [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.2) | \ sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl icmp,orig=(src=192.168.2.2,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> ]) @@ -1507,12 +1509,14 @@ NS_CHECK_EXEC([foo1], [ping -q -c 3 -i 0.3 -w 2 172.16.1.4 | FORMAT_PING], \ # Check conntrack entries. First SNAT of 'foo1' address happens. # Then DNAT of 'bar1' address happens (listed first below). -AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(172.16.1.3) | \ +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.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=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> ]) +AT_CHECK([ovs-appctl dpctl/flush-conntrack]) + # East-West NAT: 'foo2' pings 'bar1' using 172.16.1.4. NS_CHECK_EXEC([foo2], [ping -q -c 3 -i 0.3 -w 2 172.16.1.4 | FORMAT_PING], \ [0], [dnl Moreover I got some issues since I am running fedora 30 and I have not installed python2. Should we do something like? diff --git a/tests/system-userspace-macros.at b/tests/system-userspace-macros.at index 9d5f3bf41..a411e3d89 100644 --- a/tests/system-userspace-macros.at +++ b/tests/system-userspace-macros.at @@ -65,7 +65,7 @@ m4_define([CONFIGURE_VETH_OFFLOADS], # Perform requirements checks for running conntrack tests. # m4_define([CHECK_CONNTRACK], - [AT_SKIP_IF([test $HAVE_PYTHON2 = no])] + [AT_SKIP_IF([test $HAVE_PYTHON = no])] ) # CHECK_CONNTRACK_ALG() Regards, Lorenzo > > > > Missing conntrack entry: > > > > @@ -1,2 +1 @@ > > -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> > > > > *137: ovn -- DNAT and SNAT on distributed router - E/W FAILED > (system-ovn.at:1510 <http://system-ovn.at:1510>)* > > > > Missing conntrack entries: > > > > +++ > /home/dball/openvswitch/ovs/_gcc/tests/system-kmod-testsuite.dir/at-groups/137/stdout > 2019-07-19 > 10:14:39.821883399 -0700 > > @@ -1,3 +1 @@ > > -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> > > > > > On Sat, Jul 6, 2019 at 3:48 AM Lorenzo Bianconi <lorenzo.bianconi@redhat.com> > wrote: > > > Run local logical flows first if the gw router port is scheduled > > on the local chassis in order to properly manage snat traffic > > > > Tested-by: Eran Kuris <ekuris@redhat.com> > > Acked-by: Numan Siddique <nusiddiq@redhat.com> > > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> > > --- > > Changes since v2: > > - fix compilation error > > Changes since v1: > > - add priority change in ovn-northd.8.xml > > --- > > ovn/northd/ovn-northd.8.xml | 3 ++- > > ovn/northd/ovn-northd.c | 7 +++++-- > > 2 files changed, 7 insertions(+), 3 deletions(-) > > > > diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml > > index 193aa210f..d2267de0e 100644 > > --- a/ovn/northd/ovn-northd.8.xml > > +++ b/ovn/northd/ovn-northd.8.xml > > @@ -2428,7 +2428,8 @@ nd_ns { > > <p> > > If the NAT rule cannot be handled in a distributed manner, then > > the flow above is only programmed on the > > - <code>redirect-chassis</code>. > > + <code>redirect-chassis</code> increasing flow priority by 128 in > > + order to be run first > > </p> > > > > <p> > > diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c > > index ba2719321..ce382ac89 100644 > > --- a/ovn/northd/ovn-northd.c > > +++ b/ovn/northd/ovn-northd.c > > @@ -6634,6 +6634,8 @@ build_lrouter_flows(struct hmap *datapaths, struct > > hmap *ports, > > count_1bits(ntohl(mask)) + 1, > > ds_cstr(&match), ds_cstr(&actions)); > > } else { > > + uint16_t priority = count_1bits(ntohl(mask)) + 1; > > + > > /* Distributed router. */ > > ds_clear(&match); > > ds_put_format(&match, "ip && ip4.src == %s" > > @@ -6643,6 +6645,7 @@ build_lrouter_flows(struct hmap *datapaths, struct > > hmap *ports, > > if (!distributed && od->l3redirect_port) { > > /* Flows for NAT rules that are centralized are > > only > > * programmed on the "redirect-chassis". */ > > + priority += 128; > > ds_put_format(&match, " && > > is_chassis_resident(%s)", > > od->l3redirect_port->json_key); > > } > > @@ -6657,8 +6660,8 @@ build_lrouter_flows(struct hmap *datapaths, struct > > hmap *ports, > > * nat->logical_ip with the longest mask gets a higher > > * priority. */ > > ovn_lflow_add(lflows, od, S_ROUTER_OUT_SNAT, > > - count_1bits(ntohl(mask)) + 1, > > - ds_cstr(&match), ds_cstr(&actions)); > > + priority, ds_cstr(&match), > > + ds_cstr(&actions)); > > } > > } > > > > -- > > 2.21.0 > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >
Thanks for the quick turnaround couple comments inline On Sat, Jul 20, 2019 at 4:07 AM Lorenzo Bianconi < lorenzo.bianconi@redhat.com> wrote: > > The following tests consistently fail for kernel and userspace datapaths > > > > 136: ovn -- DNAT and SNAT on distributed router - N/S FAILED ( > > system-ovn.at:1337) > > 137: ovn -- DNAT and SNAT on distributed router - E/W FAILED ( > > system-ovn.at:1510) > > > > after this commit > > > > commit a6ee09882283426553e1a475e8b396af9bb378d0 > > > > Author: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> > > > > Date: Sat Jul 6 12:45:00 2019 +0200 > > > > > > > > OVN: run local logical flows first in S_ROUTER_OUT_SNAT table > > > > > > > > Run local logical flows first if the gw router port is scheduled > > > > on the local chassis in order to properly manage snat traffic > > > > > > > > Tested-by: Eran Kuris <ekuris@redhat.com> > > > > Acked-by: Numan Siddique <nusiddiq@redhat.com> > > > > Acked-by: Mark Michelson <mmichels@redhat.com> > > > > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> > > > > Signed-off-by: Ben Pfaff blp@ovn.org > > > > > > > > > > *136: ovn -- DNAT and SNAT on distributed router - N/S FAILED > > (system-ovn.at:1337 <http://system-ovn.at:1337>)* > > Hi Darrell, > > thx a lot for the report. I guess this is a particular case since both the > gw > router port and the FIPs are on the same hypervisor. Could you please try > the > following patch: > > diff --git a/tests/system-ovn.at b/tests/system-ovn.at > index 10fbd2649..a42cdf826 100644 > --- a/tests/system-ovn.at > +++ b/tests/system-ovn.at > @@ -1334,11 +1334,13 @@ 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.4) | \ > +AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(172.16.1.1) | \ > 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.4,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.1,id=<cleared>,type=0,code=0),zone=<cleared> > ]) > > +AT_CHECK([ovs-appctl dpctl/flush-conntrack]) > + > # South-North SNAT: 'bar1' pings 'alice1'. But 'alice1' receives traffic > # from 172.16.1.1 > NS_CHECK_EXEC([bar1], [ping -q -c 3 -i 0.3 -w 2 172.16.1.2 | > FORMAT_PING], \ > @@ -1347,7 +1349,7 @@ NS_CHECK_EXEC([bar1], [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.2) | \ > Above change to 'FORMAT_CT(172.16.1.2)' will not be needed. > sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl > > icmp,orig=(src=192.168.2.2,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> > ]) > @@ -1507,12 +1509,14 @@ NS_CHECK_EXEC([foo1], [ping -q -c 3 -i 0.3 -w 2 > 172.16.1.4 | FORMAT_PING], \ > > # Check conntrack entries. First SNAT of 'foo1' address happens. > # Then DNAT of 'bar1' address happens (listed first below). > -AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(172.16.1.3) | \ > +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.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=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> > ]) > > +AT_CHECK([ovs-appctl dpctl/flush-conntrack]) > + > # East-West NAT: 'foo2' pings 'bar1' using 172.16.1.4. > NS_CHECK_EXEC([foo2], [ping -q -c 3 -i 0.3 -w 2 172.16.1.4 | > FORMAT_PING], \ > [0], [dnl > > Moreover I got some issues since I am running fedora 30 and I have not > installed python2. Should we do something like? > > diff --git a/tests/system-userspace-macros.at b/tests/ > system-userspace-macros.at > index 9d5f3bf41..a411e3d89 100644 > --- a/tests/system-userspace-macros.at > +++ b/tests/system-userspace-macros.at > @@ -65,7 +65,7 @@ m4_define([CONFIGURE_VETH_OFFLOADS], > # Perform requirements checks for running conntrack tests. > # > m4_define([CHECK_CONNTRACK], > - [AT_SKIP_IF([test $HAVE_PYTHON2 = no])] > + [AT_SKIP_IF([test $HAVE_PYTHON = no])] > It is fine, but I think you also want to make the same change to system-kmod-macros.at <http://system-userspace-macros.at/> > ) > > # CHECK_CONNTRACK_ALG() > > Regards, > Lorenzo > > > > > > > > > Missing conntrack entry: > > > > > > > > @@ -1,2 +1 @@ > > > > > -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> > > > > > > > > *137: ovn -- DNAT and SNAT on distributed router - E/W FAILED > > (system-ovn.at:1510 <http://system-ovn.at:1510>)* > > > > > > > > Missing conntrack entries: > > > > > > > > +++ > > > /home/dball/openvswitch/ovs/_gcc/tests/system-kmod-testsuite.dir/at-groups/137/stdout > > 2019-07-19 > > 10:14:39.821883399 -0700 > > > > @@ -1,3 +1 @@ > > > > > -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> > > > > > > > > > > On Sat, Jul 6, 2019 at 3:48 AM Lorenzo Bianconi < > lorenzo.bianconi@redhat.com> > > wrote: > > > > > Run local logical flows first if the gw router port is scheduled > > > on the local chassis in order to properly manage snat traffic > > > > > > Tested-by: Eran Kuris <ekuris@redhat.com> > > > Acked-by: Numan Siddique <nusiddiq@redhat.com> > > > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> > > > --- > > > Changes since v2: > > > - fix compilation error > > > Changes since v1: > > > - add priority change in ovn-northd.8.xml > > > --- > > > ovn/northd/ovn-northd.8.xml | 3 ++- > > > ovn/northd/ovn-northd.c | 7 +++++-- > > > 2 files changed, 7 insertions(+), 3 deletions(-) > > > > > > diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml > > > index 193aa210f..d2267de0e 100644 > > > --- a/ovn/northd/ovn-northd.8.xml > > > +++ b/ovn/northd/ovn-northd.8.xml > > > @@ -2428,7 +2428,8 @@ nd_ns { > > > <p> > > > If the NAT rule cannot be handled in a distributed manner, > then > > > the flow above is only programmed on the > > > - <code>redirect-chassis</code>. > > > + <code>redirect-chassis</code> increasing flow priority by > 128 in > > > + order to be run first > > > </p> > > > > > > <p> > > > diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c > > > index ba2719321..ce382ac89 100644 > > > --- a/ovn/northd/ovn-northd.c > > > +++ b/ovn/northd/ovn-northd.c > > > @@ -6634,6 +6634,8 @@ build_lrouter_flows(struct hmap *datapaths, > struct > > > hmap *ports, > > > count_1bits(ntohl(mask)) + 1, > > > ds_cstr(&match), ds_cstr(&actions)); > > > } else { > > > + uint16_t priority = count_1bits(ntohl(mask)) + 1; > > > + > > > /* Distributed router. */ > > > ds_clear(&match); > > > ds_put_format(&match, "ip && ip4.src == %s" > > > @@ -6643,6 +6645,7 @@ build_lrouter_flows(struct hmap *datapaths, > struct > > > hmap *ports, > > > if (!distributed && od->l3redirect_port) { > > > /* Flows for NAT rules that are centralized > are > > > only > > > * programmed on the "redirect-chassis". */ > > > + priority += 128; > > > ds_put_format(&match, " && > > > is_chassis_resident(%s)", > > > od->l3redirect_port->json_key); > > > } > > > @@ -6657,8 +6660,8 @@ build_lrouter_flows(struct hmap *datapaths, > struct > > > hmap *ports, > > > * nat->logical_ip with the longest mask gets a > higher > > > * priority. */ > > > ovn_lflow_add(lflows, od, S_ROUTER_OUT_SNAT, > > > - count_1bits(ntohl(mask)) + 1, > > > - ds_cstr(&match), ds_cstr(&actions)); > > > + priority, ds_cstr(&match), > > > + ds_cstr(&actions)); > > > } > > > } > > > > > > -- > > > 2.21.0 > > > > > > _______________________________________________ > > > dev mailing list > > > dev@openvswitch.org > > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > >
> > Thanks for the quick turnaround > > couple comments inline > > On Sat, Jul 20, 2019 at 4:07 AM Lorenzo Bianconi <lorenzo.bianconi@redhat.com> wrote: >> >> > The following tests consistently fail for kernel and userspace datapaths >> > >> > 136: ovn -- DNAT and SNAT on distributed router - N/S FAILED ( >> > system-ovn.at:1337) >> > 137: ovn -- DNAT and SNAT on distributed router - E/W FAILED ( >> > system-ovn.at:1510) >> > >> > after this commit >> > >> > commit a6ee09882283426553e1a475e8b396af9bb378d0 >> > >> > Author: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> >> > >> > Date: Sat Jul 6 12:45:00 2019 +0200 >> > >> > >> > >> > OVN: run local logical flows first in S_ROUTER_OUT_SNAT table >> > >> > >> > >> > Run local logical flows first if the gw router port is scheduled >> > >> > on the local chassis in order to properly manage snat traffic >> > >> > >> > >> > Tested-by: Eran Kuris <ekuris@redhat.com> >> > >> > Acked-by: Numan Siddique <nusiddiq@redhat.com> >> > >> > Acked-by: Mark Michelson <mmichels@redhat.com> >> > >> > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> >> > >> > Signed-off-by: Ben Pfaff blp@ovn.org >> > >> > >> > >> > >> > *136: ovn -- DNAT and SNAT on distributed router - N/S FAILED >> > (system-ovn.at:1337 <http://system-ovn.at:1337>)* >> >> Hi Darrell, >> >> thx a lot for the report. I guess this is a particular case since both the gw >> router port and the FIPs are on the same hypervisor. Could you please try the >> following patch: >> >> diff --git a/tests/system-ovn.at b/tests/system-ovn.at >> index 10fbd2649..a42cdf826 100644 >> --- a/tests/system-ovn.at >> +++ b/tests/system-ovn.at >> @@ -1334,11 +1334,13 @@ 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.4) | \ >> +AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(172.16.1.1) | \ >> 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.4,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.1,id=<cleared>,type=0,code=0),zone=<cleared> >> ]) >> >> +AT_CHECK([ovs-appctl dpctl/flush-conntrack]) >> + >> # South-North SNAT: 'bar1' pings 'alice1'. But 'alice1' receives traffic >> # from 172.16.1.1 >> NS_CHECK_EXEC([bar1], [ping -q -c 3 -i 0.3 -w 2 172.16.1.2 | FORMAT_PING], \ >> @@ -1347,7 +1349,7 @@ NS_CHECK_EXEC([bar1], [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.2) | \ > > > Above change to 'FORMAT_CT(172.16.1.2)' will not be needed. > ack >> >> sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl >> icmp,orig=(src=192.168.2.2,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> >> ]) >> @@ -1507,12 +1509,14 @@ NS_CHECK_EXEC([foo1], [ping -q -c 3 -i 0.3 -w 2 172.16.1.4 | FORMAT_PING], \ >> >> # Check conntrack entries. First SNAT of 'foo1' address happens. >> # Then DNAT of 'bar1' address happens (listed first below). >> -AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(172.16.1.3) | \ >> +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.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=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> >> ]) >> >> +AT_CHECK([ovs-appctl dpctl/flush-conntrack]) >> + >> # East-West NAT: 'foo2' pings 'bar1' using 172.16.1.4. >> NS_CHECK_EXEC([foo2], [ping -q -c 3 -i 0.3 -w 2 172.16.1.4 | FORMAT_PING], \ >> [0], [dnl >> >> Moreover I got some issues since I am running fedora 30 and I have not >> installed python2. Should we do something like? >> >> diff --git a/tests/system-userspace-macros.at b/tests/system-userspace-macros.at >> index 9d5f3bf41..a411e3d89 100644 >> --- a/tests/system-userspace-macros.at >> +++ b/tests/system-userspace-macros.at >> @@ -65,7 +65,7 @@ m4_define([CONFIGURE_VETH_OFFLOADS], >> # Perform requirements checks for running conntrack tests. >> # >> m4_define([CHECK_CONNTRACK], >> - [AT_SKIP_IF([test $HAVE_PYTHON2 = no])] >> + [AT_SKIP_IF([test $HAVE_PYTHON = no])] > > > It is fine, but I think you also want to make the same change to system-kmod-macros.at > ack. I will post two separated patches. Next week I am in PTO so I will post them the week after. Regards, Lorenzo >> >> ) >> >> # CHECK_CONNTRACK_ALG() >> >> Regards, >> Lorenzo >> >> > >> > >> > >> > Missing conntrack entry: >> > >> > >> > >> > @@ -1,2 +1 @@ >> > >> > -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> >> > >> > >> > >> > *137: ovn -- DNAT and SNAT on distributed router - E/W FAILED >> > (system-ovn.at:1510 <http://system-ovn.at:1510>)* >> > >> > >> > >> > Missing conntrack entries: >> > >> > >> > >> > +++ >> > /home/dball/openvswitch/ovs/_gcc/tests/system-kmod-testsuite.dir/at-groups/137/stdout >> > 2019-07-19 >> > 10:14:39.821883399 -0700 >> > >> > @@ -1,3 +1 @@ >> > >> > -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> >> > >> > >> > >> > >> > On Sat, Jul 6, 2019 at 3:48 AM Lorenzo Bianconi <lorenzo.bianconi@redhat.com> >> > wrote: >> > >> > > Run local logical flows first if the gw router port is scheduled >> > > on the local chassis in order to properly manage snat traffic >> > > >> > > Tested-by: Eran Kuris <ekuris@redhat.com> >> > > Acked-by: Numan Siddique <nusiddiq@redhat.com> >> > > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> >> > > --- >> > > Changes since v2: >> > > - fix compilation error >> > > Changes since v1: >> > > - add priority change in ovn-northd.8.xml >> > > --- >> > > ovn/northd/ovn-northd.8.xml | 3 ++- >> > > ovn/northd/ovn-northd.c | 7 +++++-- >> > > 2 files changed, 7 insertions(+), 3 deletions(-) >> > > >> > > diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml >> > > index 193aa210f..d2267de0e 100644 >> > > --- a/ovn/northd/ovn-northd.8.xml >> > > +++ b/ovn/northd/ovn-northd.8.xml >> > > @@ -2428,7 +2428,8 @@ nd_ns { >> > > <p> >> > > If the NAT rule cannot be handled in a distributed manner, then >> > > the flow above is only programmed on the >> > > - <code>redirect-chassis</code>. >> > > + <code>redirect-chassis</code> increasing flow priority by 128 in >> > > + order to be run first >> > > </p> >> > > >> > > <p> >> > > diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c >> > > index ba2719321..ce382ac89 100644 >> > > --- a/ovn/northd/ovn-northd.c >> > > +++ b/ovn/northd/ovn-northd.c >> > > @@ -6634,6 +6634,8 @@ build_lrouter_flows(struct hmap *datapaths, struct >> > > hmap *ports, >> > > count_1bits(ntohl(mask)) + 1, >> > > ds_cstr(&match), ds_cstr(&actions)); >> > > } else { >> > > + uint16_t priority = count_1bits(ntohl(mask)) + 1; >> > > + >> > > /* Distributed router. */ >> > > ds_clear(&match); >> > > ds_put_format(&match, "ip && ip4.src == %s" >> > > @@ -6643,6 +6645,7 @@ build_lrouter_flows(struct hmap *datapaths, struct >> > > hmap *ports, >> > > if (!distributed && od->l3redirect_port) { >> > > /* Flows for NAT rules that are centralized are >> > > only >> > > * programmed on the "redirect-chassis". */ >> > > + priority += 128; >> > > ds_put_format(&match, " && >> > > is_chassis_resident(%s)", >> > > od->l3redirect_port->json_key); >> > > } >> > > @@ -6657,8 +6660,8 @@ build_lrouter_flows(struct hmap *datapaths, struct >> > > hmap *ports, >> > > * nat->logical_ip with the longest mask gets a higher >> > > * priority. */ >> > > ovn_lflow_add(lflows, od, S_ROUTER_OUT_SNAT, >> > > - count_1bits(ntohl(mask)) + 1, >> > > - ds_cstr(&match), ds_cstr(&actions)); >> > > + priority, ds_cstr(&match), >> > > + ds_cstr(&actions)); >> > > } >> > > } >> > > >> > > -- >> > > 2.21.0 >> > > >> > > _______________________________________________ >> > > dev mailing list >> > > dev@openvswitch.org >> > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> > >
diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml index 193aa210f..d2267de0e 100644 --- a/ovn/northd/ovn-northd.8.xml +++ b/ovn/northd/ovn-northd.8.xml @@ -2428,7 +2428,8 @@ nd_ns { <p> If the NAT rule cannot be handled in a distributed manner, then the flow above is only programmed on the - <code>redirect-chassis</code>. + <code>redirect-chassis</code> increasing flow priority by 128 in + order to be run first </p> <p> diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c index ba2719321..ce382ac89 100644 --- a/ovn/northd/ovn-northd.c +++ b/ovn/northd/ovn-northd.c @@ -6634,6 +6634,8 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, count_1bits(ntohl(mask)) + 1, ds_cstr(&match), ds_cstr(&actions)); } else { + uint16_t priority = count_1bits(ntohl(mask)) + 1; + /* Distributed router. */ ds_clear(&match); ds_put_format(&match, "ip && ip4.src == %s" @@ -6643,6 +6645,7 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, if (!distributed && od->l3redirect_port) { /* Flows for NAT rules that are centralized are only * programmed on the "redirect-chassis". */ + priority += 128; ds_put_format(&match, " && is_chassis_resident(%s)", od->l3redirect_port->json_key); } @@ -6657,8 +6660,8 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, * nat->logical_ip with the longest mask gets a higher * priority. */ ovn_lflow_add(lflows, od, S_ROUTER_OUT_SNAT, - count_1bits(ntohl(mask)) + 1, - ds_cstr(&match), ds_cstr(&actions)); + priority, ds_cstr(&match), + ds_cstr(&actions)); } }