| Message ID | 20220627112824.2879641-1-xsimonar@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,branch-21.12] northd: avoid snat on reply packets | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | fail | apply and check: fail |
| ovsrobot/github-robot-_Build_and_Test | fail | github build: failed |
| ovsrobot/github-robot-_ovn-kubernetes | fail | github build: failed |
Bleep bloop. Greetings Xavier Simonart, I am a robot and I have tried out your patch.
Thanks for your contribution.
I encountered some error that I wasn't expecting. See the details below.
checkpatch:
WARNING: Unexpected sign-offs from developers who are not authors or co-authors or committers: Numan Siddique <numans@ovn.org>
Lines checked: 275, Warnings: 1, Errors: 0
build:
PYTHONPATH=/var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs/python":"$PYTHONPATH PYTHONDONTWRITEBYTECODE=yes /usr/bin/python3 /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs/ovsdb/ovsdb-idlc.in annotate ./ovn-ic-sb.ovsschema ./lib/ovn-ic-sb-idl.ann > lib/ovn-ic-sb-idl.ovsidl.tmp && \
mv lib/ovn-ic-sb-idl.ovsidl.tmp lib/ovn-ic-sb-idl.ovsidl
PYTHONPATH=/var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs/python":"$PYTHONPATH PYTHONDONTWRITEBYTECODE=yes /usr/bin/python3 /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs/ovsdb/ovsdb-idlc.in c-idl-source lib/ovn-ic-sb-idl.ovsidl > lib/ovn-ic-sb-idl.c.tmp && mv lib/ovn-ic-sb-idl.c.tmp lib/ovn-ic-sb-idl.c
PYTHONPATH=/var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs/python":"$PYTHONPATH PYTHONDONTWRITEBYTECODE=yes /usr/bin/python3 /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs/ovsdb/ovsdb-idlc.in c-idl-header lib/ovn-ic-sb-idl.ovsidl > lib/ovn-ic-sb-idl.h.tmp && mv lib/ovn-ic-sb-idl.h.tmp lib/ovn-ic-sb-idl.h
make all-am
make[1]: Entering directory `/var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace'
depbase=`echo lib/acl-log.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
/bin/sh ./libtool --tag=CC --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./ovn -I ./include -I ./lib -I ./lib -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs/include -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs/include -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs/lib -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs/lib -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror -g -O2 -MT lib/acl-log.lo -MD -MP -MF $depbase.Tpo -c -o li
b/acl-log.lo lib/acl-log.c &&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./ovn -I ./include -I ./lib -I ./lib -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs/include -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs/include -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs/lib -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs/lib -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror -g -O2 -MT lib/acl-log.lo -MD -MP -MF lib/.deps/acl-log.Tpo -c lib/acl-log.c -o lib/acl-log.
o
depbase=`echo lib/actions.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
/bin/sh ./libtool --tag=CC --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./ovn -I ./include -I ./lib -I ./lib -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs/include -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs/include -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs/lib -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs/lib -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror -g -O2 -MT lib/actions.lo -MD -MP -MF $depbase.Tpo -c -o li
b/actions.lo lib/actions.c &&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./ovn -I ./include -I ./lib -I ./lib -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs/include -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs/include -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs/lib -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs/lib -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror -g -O2 -MT lib/actions.lo -MD -MP -MF lib/.deps/actions.Tpo -c lib/actions.c -o lib/actions.
o
lib/actions.c: In function 'validate_empty_lb_backends':
lib/actions.c:2439:13: error: too few arguments to function 'inet_parse_active'
if (!inet_parse_active(c->string, 0, &ss, false)) {
^
In file included from lib/actions.c:42:0:
/var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs/lib/socket-util.h:51:6: note: declared here
bool inet_parse_active(const char *target, int default_port,
^
make[1]: *** [lib/actions.lo] Error 1
make[1]: Leaving directory `/var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace'
make: *** [all] Error 2
Please check this out. If you feel there has been an error, please email aconole@redhat.com
Thanks,
0-day Robot
Thank you for the backport, Xavier. I pushed the change to branch-21.12. On 6/27/22 07:28, Xavier Simonart wrote: > On gateway routers egress packets might be both: > - unDNATted > - SNATted > Reply packets should not be SNATted (they must of course be UnDNATted > if DNAT was applied). > > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2061593 > > Signed-off-by: Xavier Simonart <xsimonar@redhat.com> > Acked-by: Mark Michelson <mmichels@redhat.com> > Signed-off-by: Numan Siddique <numans@ovn.org> > (cherry picked from commit 8b3e1afc30f3cf0ef9857fdc68f619b6fbed10dc) > --- > northd/northd.c | 1 + > northd/ovn-northd.8.xml | 3 +- > tests/ovn-northd.at | 33 +++++------ > tests/system-ovn.at | 119 ++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 139 insertions(+), 17 deletions(-) > > diff --git a/northd/northd.c b/northd/northd.c > index ce78f03de..f51962c4b 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -12991,6 +12991,7 @@ build_lrouter_out_snat_flow(struct hmap *lflows, struct ovn_datapath *od, > ds_put_format(actions, "ip%s.src=%s; next;", > is_v6 ? "6" : "4", nat->external_ip); > } else { > + ds_put_format(match, " && (!ct.trk || !ct.rpl)"); > ds_put_format(actions, "ct_snat(%s", nat->external_ip); > > if (nat->external_port_range[0]) { > diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml > index 2b307cef3..2594c6d3b 100644 > --- a/northd/ovn-northd.8.xml > +++ b/northd/ovn-northd.8.xml > @@ -4417,7 +4417,8 @@ nd_ns { > to change the source IP address of a packet from an IP address of > <var>A</var> or to change the source IP address of a packet that > belongs to network <var>A</var> to <var>B</var>, a flow matches > - <code>ip && ip4.src == <var>A</var></code> with an action > + <code>ip && ip4.src == <var>A</var> && > + (!ct.trk || !ct.rpl)</code> with an action > <code>ct_snat(<var>B</var>);</code>. The priority of the flow > is calculated based on the mask of <var>A</var>, with matches > having larger masks getting higher priorities. If the NAT rule is > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index d5701a72b..a8689dfb0 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -1018,7 +1018,7 @@ AT_CHECK([grep -e "lr_out_snat" drflows | sed 's/table=../table=??/' | sort], [0 > AT_CHECK([grep -e "lr_out_snat" crflows | sed 's/table=../table=??/' | sort], [0], [dnl > table=??(lr_out_snat ), priority=0 , match=(1), action=(next;) > table=??(lr_out_snat ), priority=120 , match=(nd_ns), action=(next;) > - table=??(lr_out_snat ), priority=33 , match=(ip && ip4.src == 50.0.0.11 && ip4.dst == $allowed_range), action=(ct_snat(172.16.1.1);) > + table=??(lr_out_snat ), priority=33 , match=(ip && ip4.src == 50.0.0.11 && ip4.dst == $allowed_range && (!ct.trk || !ct.rpl)), action=(ct_snat(172.16.1.1);) > ]) > > > @@ -1050,7 +1050,7 @@ AT_CHECK([grep -e "lr_out_snat" drflows2 | sed 's/table=../table=??/' | sort], [ > AT_CHECK([grep -e "lr_out_snat" crflows2 | sed 's/table=../table=??/' | sort], [0], [dnl > table=??(lr_out_snat ), priority=0 , match=(1), action=(next;) > table=??(lr_out_snat ), priority=120 , match=(nd_ns), action=(next;) > - table=??(lr_out_snat ), priority=33 , match=(ip && ip4.src == 50.0.0.11), action=(ct_snat(172.16.1.1);) > + table=??(lr_out_snat ), priority=33 , match=(ip && ip4.src == 50.0.0.11 && (!ct.trk || !ct.rpl)), action=(ct_snat(172.16.1.1);) > table=??(lr_out_snat ), priority=35 , match=(ip && ip4.src == 50.0.0.11 && ip4.dst == $disallowed_range), action=(next;) > ]) > > @@ -1079,7 +1079,7 @@ AT_CHECK([grep -e "lr_out_snat" drflows3 | sed 's/table=../table=??/' | sort], [ > AT_CHECK([grep -e "lr_out_snat" crflows3 | sed 's/table=../table=??/' | sort], [0], [dnl > table=??(lr_out_snat ), priority=0 , match=(1), action=(next;) > table=??(lr_out_snat ), priority=120 , match=(nd_ns), action=(next;) > - table=??(lr_out_snat ), priority=33 , match=(ip && ip4.src == 50.0.0.11 && ip4.dst == $allowed_range), action=(ct_snat(172.16.1.2);) > + table=??(lr_out_snat ), priority=33 , match=(ip && ip4.src == 50.0.0.11 && ip4.dst == $allowed_range && (!ct.trk || !ct.rpl)), action=(ct_snat(172.16.1.2);) > ]) > > # Stateful FIP with DISALLOWED_IPs > @@ -1108,7 +1108,7 @@ AT_CHECK([grep -e "lr_out_snat" drflows4 | sed 's/table=../table=??/' | sort], [ > AT_CHECK([grep -e "lr_out_snat" crflows4 | sed 's/table=../table=??/' | sort], [0], [dnl > table=??(lr_out_snat ), priority=0 , match=(1), action=(next;) > table=??(lr_out_snat ), priority=120 , match=(nd_ns), action=(next;) > - table=??(lr_out_snat ), priority=33 , match=(ip && ip4.src == 50.0.0.11), action=(ct_snat(172.16.1.2);) > + table=??(lr_out_snat ), priority=33 , match=(ip && ip4.src == 50.0.0.11 && (!ct.trk || !ct.rpl)), action=(ct_snat(172.16.1.2);) > table=??(lr_out_snat ), priority=35 , match=(ip && ip4.src == 50.0.0.11 && ip4.dst == $disallowed_range), action=(next;) > ]) > > @@ -5082,11 +5082,12 @@ AT_CHECK([grep "lr_out_post_undnat" lr0flows | sed 's/table=./table=?/' | sort], > AT_CHECK([grep "lr_out_snat" lr0flows | sed 's/table=./table=?/' | sort], [0], [dnl > table=? (lr_out_snat ), priority=0 , match=(1), action=(next;) > table=? (lr_out_snat ), priority=120 , match=(nd_ns), action=(next;) > - table=? (lr_out_snat ), priority=25 , match=(ip && ip4.src == 10.0.0.0/24), action=(ct_snat(172.168.0.10);) > - table=? (lr_out_snat ), priority=33 , match=(ip && ip4.src == 10.0.0.10), action=(ct_snat(172.168.0.30);) > - table=? (lr_out_snat ), priority=33 , match=(ip && ip4.src == 10.0.0.3), action=(ct_snat(172.168.0.20);) > + table=? (lr_out_snat ), priority=25 , match=(ip && ip4.src == 10.0.0.0/24 && (!ct.trk || !ct.rpl)), action=(ct_snat(172.168.0.10);) > + table=? (lr_out_snat ), priority=33 , match=(ip && ip4.src == 10.0.0.10 && (!ct.trk || !ct.rpl)), action=(ct_snat(172.168.0.30);) > + table=? (lr_out_snat ), priority=33 , match=(ip && ip4.src == 10.0.0.3 && (!ct.trk || !ct.rpl)), action=(ct_snat(172.168.0.20);) > ]) > > + > # Set lb force snat logical router. > check ovn-nbctl --wait=sb set logical_router lr0 options:lb_force_snat_ip="router_ip" > check ovn-nbctl --wait=sb sync > @@ -5143,9 +5144,9 @@ AT_CHECK([grep "lr_out_snat" lr0flows | sed 's/table=./table=?/' | sort], [0], [ > table=? (lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && ip4 && outport == "lr0-public"), action=(ct_snat(172.168.0.10);) > table=? (lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && ip4 && outport == "lr0-sw0"), action=(ct_snat(10.0.0.1);) > table=? (lr_out_snat ), priority=120 , match=(nd_ns), action=(next;) > - table=? (lr_out_snat ), priority=25 , match=(ip && ip4.src == 10.0.0.0/24), action=(ct_snat(172.168.0.10);) > - table=? (lr_out_snat ), priority=33 , match=(ip && ip4.src == 10.0.0.10), action=(ct_snat(172.168.0.30);) > - table=? (lr_out_snat ), priority=33 , match=(ip && ip4.src == 10.0.0.3), action=(ct_snat(172.168.0.20);) > + table=? (lr_out_snat ), priority=25 , match=(ip && ip4.src == 10.0.0.0/24 && (!ct.trk || !ct.rpl)), action=(ct_snat(172.168.0.10);) > + table=? (lr_out_snat ), priority=33 , match=(ip && ip4.src == 10.0.0.10 && (!ct.trk || !ct.rpl)), action=(ct_snat(172.168.0.30);) > + table=? (lr_out_snat ), priority=33 , match=(ip && ip4.src == 10.0.0.3 && (!ct.trk || !ct.rpl)), action=(ct_snat(172.168.0.20);) > ]) > > # Add a LB VIP same as router ip. > @@ -5208,9 +5209,9 @@ AT_CHECK([grep "lr_out_snat" lr0flows | sed 's/table=./table=?/' | sort], [0], [ > table=? (lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && ip4 && outport == "lr0-public"), action=(ct_snat(172.168.0.10);) > table=? (lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && ip4 && outport == "lr0-sw0"), action=(ct_snat(10.0.0.1);) > table=? (lr_out_snat ), priority=120 , match=(nd_ns), action=(next;) > - table=? (lr_out_snat ), priority=25 , match=(ip && ip4.src == 10.0.0.0/24), action=(ct_snat(172.168.0.10);) > - table=? (lr_out_snat ), priority=33 , match=(ip && ip4.src == 10.0.0.10), action=(ct_snat(172.168.0.30);) > - table=? (lr_out_snat ), priority=33 , match=(ip && ip4.src == 10.0.0.3), action=(ct_snat(172.168.0.20);) > + table=? (lr_out_snat ), priority=25 , match=(ip && ip4.src == 10.0.0.0/24 && (!ct.trk || !ct.rpl)), action=(ct_snat(172.168.0.10);) > + table=? (lr_out_snat ), priority=33 , match=(ip && ip4.src == 10.0.0.10 && (!ct.trk || !ct.rpl)), action=(ct_snat(172.168.0.30);) > + table=? (lr_out_snat ), priority=33 , match=(ip && ip4.src == 10.0.0.3 && (!ct.trk || !ct.rpl)), action=(ct_snat(172.168.0.20);) > ]) > > # Add IPv6 router port and LB. > @@ -5288,9 +5289,9 @@ AT_CHECK([grep "lr_out_snat" lr0flows | sed 's/table=./table=?/' | sort], [0], [ > table=? (lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && ip6 && outport == "lr0-public"), action=(ct_snat(def0::10);) > table=? (lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && ip6 && outport == "lr0-sw0"), action=(ct_snat(aef0::1);) > table=? (lr_out_snat ), priority=120 , match=(nd_ns), action=(next;) > - table=? (lr_out_snat ), priority=25 , match=(ip && ip4.src == 10.0.0.0/24), action=(ct_snat(172.168.0.10);) > - table=? (lr_out_snat ), priority=33 , match=(ip && ip4.src == 10.0.0.10), action=(ct_snat(172.168.0.30);) > - table=? (lr_out_snat ), priority=33 , match=(ip && ip4.src == 10.0.0.3), action=(ct_snat(172.168.0.20);) > + table=? (lr_out_snat ), priority=25 , match=(ip && ip4.src == 10.0.0.0/24 && (!ct.trk || !ct.rpl)), action=(ct_snat(172.168.0.10);) > + table=? (lr_out_snat ), priority=33 , match=(ip && ip4.src == 10.0.0.10 && (!ct.trk || !ct.rpl)), action=(ct_snat(172.168.0.30);) > + table=? (lr_out_snat ), priority=33 , match=(ip && ip4.src == 10.0.0.3 && (!ct.trk || !ct.rpl)), action=(ct_snat(172.168.0.20);) > ]) > > check ovn-nbctl lrp-del lr0-sw0 > diff --git a/tests/system-ovn.at b/tests/system-ovn.at > index 0c33a43c1..25cc1d4ef 100644 > --- a/tests/system-ovn.at > +++ b/tests/system-ovn.at > @@ -7629,3 +7629,122 @@ OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d > > AT_CLEANUP > ]) > + > +OVN_FOR_EACH_NORTHD([ > +AT_SETUP([East-West traffic with gateway router if DNAT configured]) > +AT_KEYWORDS([ovnnat]) > + > +CHECK_CONNTRACK() > +CHECK_CONNTRACK_NAT() > +ovn_start > +OVS_TRAFFIC_VSWITCHD_START() > +ADD_BR([br-int]) > +# Set external-ids in br-int needed for ovn-controller > +ovs-vsctl \ > + -- set Open_vSwitch . external-ids:system-id=hv1 \ > + -- set Open_vSwitch . external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \ > + -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \ > + -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \ > + -- set bridge br-int fail-mode=secure other-config:disable-in-band=true > + > +# Start ovn-controller > +start_daemon ovn-controller > +# Logical network: > +# One LR - R1 has two switches: sw0 and sw1 > +# sw0 -- R1 -- sw1 > +# Logical port 'sw01' in switch 'sw0'. > +# Logical port 'sw11' in switch 'sw1'. > +# nc server running in sw01 > +# nc client running on sw11 > + > +check ovn-nbctl lr-add R1 > +check ovn-nbctl ls-add sw0 > +check ovn-nbctl ls-add sw1 > + > +check ovn-nbctl lrp-add R1 rp-sw0 00:00:01:01:02:03 192.168.1.1/24 > +check ovn-nbctl lrp-add R1 rp-sw1 00:00:03:01:02:03 192.168.2.1/24 > +check ovn-nbctl set logical_router R1 options:chassis=hv1 > + > +check ovn-nbctl lsp-add sw0 sw0-rp -- set Logical_Switch_Port sw0-rp \ > + type=router options:router-port=rp-sw0 \ > + -- lsp-set-addresses sw0-rp router > +check ovn-nbctl lsp-add sw1 sw1-rp -- set Logical_Switch_Port sw1-rp \ > + type=router options:router-port=rp-sw1 \ > + -- lsp-set-addresses sw1-rp router > + > +ADD_NAMESPACES(sw01) > +ADD_VETH(sw01, sw01, br-int, "192.168.1.2/24", "f0:00:00:01:02:03", \ > + "192.168.1.1") > +check ovn-nbctl lsp-add sw0 sw01 \ > + -- lsp-set-addresses sw01 "f0:00:00:01:02:03 192.168.1.2" > + > +ADD_NAMESPACES(sw11) > +ADD_VETH(sw11, sw11, br-int, "192.168.2.2/24", "f0:00:00:02:02:03", \ > + "192.168.2.1") > +check ovn-nbctl lsp-add sw1 sw11 \ > + -- lsp-set-addresses sw11 "f0:00:00:02:02:03 192.168.2.2" > + > +NETNS_DAEMONIZE([sw01], [nc -k -l 8000], [nc-sw01.pid]) > + > +test_ping() { > + NS_CHECK_EXEC([$1], [ping -q -c 1 $2 -w 2 | FORMAT_PING], \ > +[0], [dnl > +1 packets transmitted, 1 received, 0% packet loss, time 0ms > +]) > +} > + > +# Only SNAT > +check ovn-nbctl --wait=hv lr-nat-add R1 snat 172.16.1.21 192.168.2.0/24 > + > +echo "foo" > foo > +NS_CHECK_EXEC([sw11], [nc 192.168.1.2 8000 < foo]) > +test_ping sw11 192.168.1.2 > + > +# Ensure nat has been hit > +OVS_WAIT_UNTIL([ovs-ofctl dump-flows br-int | grep -v "n_packets=0" | grep 'nat(src=172.16.1.21)']) > +# Ensure conntrack entry is present > +OVS_WAIT_FOR_OUTPUT([ > + ovs-appctl dpctl/dump-conntrack | FORMAT_CT(192.168.2.2) | \ > + sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl > +icmp,orig=(src=192.168.2.2,dst=192.168.1.2,id=<cleared>,type=8,code=0),reply=(src=192.168.1.2,dst=192.168.2.2,id=<cleared>,type=0,code=0),zone=<cleared> > +tcp,orig=(src=192.168.2.2,dst=192.168.1.2,sport=<cleared>,dport=<cleared>),reply=(src=192.168.1.2,dst=192.168.2.2,sport=<cleared>,dport=<cleared>),zone=<cleared>,protoinfo=(state=<cleared>) > +]) > + > +AT_CHECK([ovs-appctl dpctl/flush-conntrack]) > + > +# SNAT and DNAT. using Logical IP > +ovn-nbctl --wait=hv lr-nat-add R1 dnat_and_snat 172.16.1.2 192.168.1.2 > +NS_CHECK_EXEC([sw11], [nc 192.168.1.2 8000 < foo ]) > +test_ping sw11 192.168.1.2 > + > +# Ensure conntrack entry is present > +OVS_WAIT_FOR_OUTPUT([ > + ovs-appctl dpctl/dump-conntrack | FORMAT_CT(192.168.2.2) | \ > + sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl > +icmp,orig=(src=192.168.2.2,dst=192.168.1.2,id=<cleared>,type=8,code=0),reply=(src=192.168.1.2,dst=192.168.2.2,id=<cleared>,type=0,code=0),zone=<cleared> > +tcp,orig=(src=192.168.2.2,dst=192.168.1.2,sport=<cleared>,dport=<cleared>),reply=(src=192.168.1.2,dst=192.168.2.2,sport=<cleared>,dport=<cleared>),zone=<cleared>,protoinfo=(state=<cleared>) > +]) > + > +AT_CHECK([ovs-appctl dpctl/flush-conntrack]) > + > +# SNAT and DNAT. using floating IP > +NS_CHECK_EXEC([sw11], [nc 172.16.1.2 8000 < foo ]) > +test_ping sw11 172.16.1.2 > + > +# Ensure conntrack entry is present > +OVS_WAIT_FOR_OUTPUT([ > + ovs-appctl dpctl/dump-conntrack | FORMAT_CT(192.168.2.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=192.168.1.2,dst=192.168.2.2,id=<cleared>,type=0,code=0),zone=<cleared> > +tcp,orig=(src=192.168.2.2,dst=172.16.1.2,sport=<cleared>,dport=<cleared>),reply=(src=192.168.1.2,dst=192.168.2.2,sport=<cleared>,dport=<cleared>),zone=<cleared>,protoinfo=(state=<cleared>) > +]) > + > +AT_CHECK([ovs-appctl dpctl/flush-conntrack]) > + > +OVS_APP_EXIT_AND_WAIT([ovn-controller]) > +as > +OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d > +/connection dropped.*/d"]) > + > +AT_CLEANUP > +]) >
diff --git a/northd/northd.c b/northd/northd.c index ce78f03de..f51962c4b 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -12991,6 +12991,7 @@ build_lrouter_out_snat_flow(struct hmap *lflows, struct ovn_datapath *od, ds_put_format(actions, "ip%s.src=%s; next;", is_v6 ? "6" : "4", nat->external_ip); } else { + ds_put_format(match, " && (!ct.trk || !ct.rpl)"); ds_put_format(actions, "ct_snat(%s", nat->external_ip); if (nat->external_port_range[0]) { diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml index 2b307cef3..2594c6d3b 100644 --- a/northd/ovn-northd.8.xml +++ b/northd/ovn-northd.8.xml @@ -4417,7 +4417,8 @@ nd_ns { to change the source IP address of a packet from an IP address of <var>A</var> or to change the source IP address of a packet that belongs to network <var>A</var> to <var>B</var>, a flow matches - <code>ip && ip4.src == <var>A</var></code> with an action + <code>ip && ip4.src == <var>A</var> && + (!ct.trk || !ct.rpl)</code> with an action <code>ct_snat(<var>B</var>);</code>. The priority of the flow is calculated based on the mask of <var>A</var>, with matches having larger masks getting higher priorities. If the NAT rule is diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index d5701a72b..a8689dfb0 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -1018,7 +1018,7 @@ AT_CHECK([grep -e "lr_out_snat" drflows | sed 's/table=../table=??/' | sort], [0 AT_CHECK([grep -e "lr_out_snat" crflows | sed 's/table=../table=??/' | sort], [0], [dnl table=??(lr_out_snat ), priority=0 , match=(1), action=(next;) table=??(lr_out_snat ), priority=120 , match=(nd_ns), action=(next;) - table=??(lr_out_snat ), priority=33 , match=(ip && ip4.src == 50.0.0.11 && ip4.dst == $allowed_range), action=(ct_snat(172.16.1.1);) + table=??(lr_out_snat ), priority=33 , match=(ip && ip4.src == 50.0.0.11 && ip4.dst == $allowed_range && (!ct.trk || !ct.rpl)), action=(ct_snat(172.16.1.1);) ]) @@ -1050,7 +1050,7 @@ AT_CHECK([grep -e "lr_out_snat" drflows2 | sed 's/table=../table=??/' | sort], [ AT_CHECK([grep -e "lr_out_snat" crflows2 | sed 's/table=../table=??/' | sort], [0], [dnl table=??(lr_out_snat ), priority=0 , match=(1), action=(next;) table=??(lr_out_snat ), priority=120 , match=(nd_ns), action=(next;) - table=??(lr_out_snat ), priority=33 , match=(ip && ip4.src == 50.0.0.11), action=(ct_snat(172.16.1.1);) + table=??(lr_out_snat ), priority=33 , match=(ip && ip4.src == 50.0.0.11 && (!ct.trk || !ct.rpl)), action=(ct_snat(172.16.1.1);) table=??(lr_out_snat ), priority=35 , match=(ip && ip4.src == 50.0.0.11 && ip4.dst == $disallowed_range), action=(next;) ]) @@ -1079,7 +1079,7 @@ AT_CHECK([grep -e "lr_out_snat" drflows3 | sed 's/table=../table=??/' | sort], [ AT_CHECK([grep -e "lr_out_snat" crflows3 | sed 's/table=../table=??/' | sort], [0], [dnl table=??(lr_out_snat ), priority=0 , match=(1), action=(next;) table=??(lr_out_snat ), priority=120 , match=(nd_ns), action=(next;) - table=??(lr_out_snat ), priority=33 , match=(ip && ip4.src == 50.0.0.11 && ip4.dst == $allowed_range), action=(ct_snat(172.16.1.2);) + table=??(lr_out_snat ), priority=33 , match=(ip && ip4.src == 50.0.0.11 && ip4.dst == $allowed_range && (!ct.trk || !ct.rpl)), action=(ct_snat(172.16.1.2);) ]) # Stateful FIP with DISALLOWED_IPs @@ -1108,7 +1108,7 @@ AT_CHECK([grep -e "lr_out_snat" drflows4 | sed 's/table=../table=??/' | sort], [ AT_CHECK([grep -e "lr_out_snat" crflows4 | sed 's/table=../table=??/' | sort], [0], [dnl table=??(lr_out_snat ), priority=0 , match=(1), action=(next;) table=??(lr_out_snat ), priority=120 , match=(nd_ns), action=(next;) - table=??(lr_out_snat ), priority=33 , match=(ip && ip4.src == 50.0.0.11), action=(ct_snat(172.16.1.2);) + table=??(lr_out_snat ), priority=33 , match=(ip && ip4.src == 50.0.0.11 && (!ct.trk || !ct.rpl)), action=(ct_snat(172.16.1.2);) table=??(lr_out_snat ), priority=35 , match=(ip && ip4.src == 50.0.0.11 && ip4.dst == $disallowed_range), action=(next;) ]) @@ -5082,11 +5082,12 @@ AT_CHECK([grep "lr_out_post_undnat" lr0flows | sed 's/table=./table=?/' | sort], AT_CHECK([grep "lr_out_snat" lr0flows | sed 's/table=./table=?/' | sort], [0], [dnl table=? (lr_out_snat ), priority=0 , match=(1), action=(next;) table=? (lr_out_snat ), priority=120 , match=(nd_ns), action=(next;) - table=? (lr_out_snat ), priority=25 , match=(ip && ip4.src == 10.0.0.0/24), action=(ct_snat(172.168.0.10);) - table=? (lr_out_snat ), priority=33 , match=(ip && ip4.src == 10.0.0.10), action=(ct_snat(172.168.0.30);) - table=? (lr_out_snat ), priority=33 , match=(ip && ip4.src == 10.0.0.3), action=(ct_snat(172.168.0.20);) + table=? (lr_out_snat ), priority=25 , match=(ip && ip4.src == 10.0.0.0/24 && (!ct.trk || !ct.rpl)), action=(ct_snat(172.168.0.10);) + table=? (lr_out_snat ), priority=33 , match=(ip && ip4.src == 10.0.0.10 && (!ct.trk || !ct.rpl)), action=(ct_snat(172.168.0.30);) + table=? (lr_out_snat ), priority=33 , match=(ip && ip4.src == 10.0.0.3 && (!ct.trk || !ct.rpl)), action=(ct_snat(172.168.0.20);) ]) + # Set lb force snat logical router. check ovn-nbctl --wait=sb set logical_router lr0 options:lb_force_snat_ip="router_ip" check ovn-nbctl --wait=sb sync @@ -5143,9 +5144,9 @@ AT_CHECK([grep "lr_out_snat" lr0flows | sed 's/table=./table=?/' | sort], [0], [ table=? (lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && ip4 && outport == "lr0-public"), action=(ct_snat(172.168.0.10);) table=? (lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && ip4 && outport == "lr0-sw0"), action=(ct_snat(10.0.0.1);) table=? (lr_out_snat ), priority=120 , match=(nd_ns), action=(next;) - table=? (lr_out_snat ), priority=25 , match=(ip && ip4.src == 10.0.0.0/24), action=(ct_snat(172.168.0.10);) - table=? (lr_out_snat ), priority=33 , match=(ip && ip4.src == 10.0.0.10), action=(ct_snat(172.168.0.30);) - table=? (lr_out_snat ), priority=33 , match=(ip && ip4.src == 10.0.0.3), action=(ct_snat(172.168.0.20);) + table=? (lr_out_snat ), priority=25 , match=(ip && ip4.src == 10.0.0.0/24 && (!ct.trk || !ct.rpl)), action=(ct_snat(172.168.0.10);) + table=? (lr_out_snat ), priority=33 , match=(ip && ip4.src == 10.0.0.10 && (!ct.trk || !ct.rpl)), action=(ct_snat(172.168.0.30);) + table=? (lr_out_snat ), priority=33 , match=(ip && ip4.src == 10.0.0.3 && (!ct.trk || !ct.rpl)), action=(ct_snat(172.168.0.20);) ]) # Add a LB VIP same as router ip. @@ -5208,9 +5209,9 @@ AT_CHECK([grep "lr_out_snat" lr0flows | sed 's/table=./table=?/' | sort], [0], [ table=? (lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && ip4 && outport == "lr0-public"), action=(ct_snat(172.168.0.10);) table=? (lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && ip4 && outport == "lr0-sw0"), action=(ct_snat(10.0.0.1);) table=? (lr_out_snat ), priority=120 , match=(nd_ns), action=(next;) - table=? (lr_out_snat ), priority=25 , match=(ip && ip4.src == 10.0.0.0/24), action=(ct_snat(172.168.0.10);) - table=? (lr_out_snat ), priority=33 , match=(ip && ip4.src == 10.0.0.10), action=(ct_snat(172.168.0.30);) - table=? (lr_out_snat ), priority=33 , match=(ip && ip4.src == 10.0.0.3), action=(ct_snat(172.168.0.20);) + table=? (lr_out_snat ), priority=25 , match=(ip && ip4.src == 10.0.0.0/24 && (!ct.trk || !ct.rpl)), action=(ct_snat(172.168.0.10);) + table=? (lr_out_snat ), priority=33 , match=(ip && ip4.src == 10.0.0.10 && (!ct.trk || !ct.rpl)), action=(ct_snat(172.168.0.30);) + table=? (lr_out_snat ), priority=33 , match=(ip && ip4.src == 10.0.0.3 && (!ct.trk || !ct.rpl)), action=(ct_snat(172.168.0.20);) ]) # Add IPv6 router port and LB. @@ -5288,9 +5289,9 @@ AT_CHECK([grep "lr_out_snat" lr0flows | sed 's/table=./table=?/' | sort], [0], [ table=? (lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && ip6 && outport == "lr0-public"), action=(ct_snat(def0::10);) table=? (lr_out_snat ), priority=110 , match=(flags.force_snat_for_lb == 1 && ip6 && outport == "lr0-sw0"), action=(ct_snat(aef0::1);) table=? (lr_out_snat ), priority=120 , match=(nd_ns), action=(next;) - table=? (lr_out_snat ), priority=25 , match=(ip && ip4.src == 10.0.0.0/24), action=(ct_snat(172.168.0.10);) - table=? (lr_out_snat ), priority=33 , match=(ip && ip4.src == 10.0.0.10), action=(ct_snat(172.168.0.30);) - table=? (lr_out_snat ), priority=33 , match=(ip && ip4.src == 10.0.0.3), action=(ct_snat(172.168.0.20);) + table=? (lr_out_snat ), priority=25 , match=(ip && ip4.src == 10.0.0.0/24 && (!ct.trk || !ct.rpl)), action=(ct_snat(172.168.0.10);) + table=? (lr_out_snat ), priority=33 , match=(ip && ip4.src == 10.0.0.10 && (!ct.trk || !ct.rpl)), action=(ct_snat(172.168.0.30);) + table=? (lr_out_snat ), priority=33 , match=(ip && ip4.src == 10.0.0.3 && (!ct.trk || !ct.rpl)), action=(ct_snat(172.168.0.20);) ]) check ovn-nbctl lrp-del lr0-sw0 diff --git a/tests/system-ovn.at b/tests/system-ovn.at index 0c33a43c1..25cc1d4ef 100644 --- a/tests/system-ovn.at +++ b/tests/system-ovn.at @@ -7629,3 +7629,122 @@ OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d AT_CLEANUP ]) + +OVN_FOR_EACH_NORTHD([ +AT_SETUP([East-West traffic with gateway router if DNAT configured]) +AT_KEYWORDS([ovnnat]) + +CHECK_CONNTRACK() +CHECK_CONNTRACK_NAT() +ovn_start +OVS_TRAFFIC_VSWITCHD_START() +ADD_BR([br-int]) +# Set external-ids in br-int needed for ovn-controller +ovs-vsctl \ + -- set Open_vSwitch . external-ids:system-id=hv1 \ + -- set Open_vSwitch . external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \ + -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \ + -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \ + -- set bridge br-int fail-mode=secure other-config:disable-in-band=true + +# Start ovn-controller +start_daemon ovn-controller +# Logical network: +# One LR - R1 has two switches: sw0 and sw1 +# sw0 -- R1 -- sw1 +# Logical port 'sw01' in switch 'sw0'. +# Logical port 'sw11' in switch 'sw1'. +# nc server running in sw01 +# nc client running on sw11 + +check ovn-nbctl lr-add R1 +check ovn-nbctl ls-add sw0 +check ovn-nbctl ls-add sw1 + +check ovn-nbctl lrp-add R1 rp-sw0 00:00:01:01:02:03 192.168.1.1/24 +check ovn-nbctl lrp-add R1 rp-sw1 00:00:03:01:02:03 192.168.2.1/24 +check ovn-nbctl set logical_router R1 options:chassis=hv1 + +check ovn-nbctl lsp-add sw0 sw0-rp -- set Logical_Switch_Port sw0-rp \ + type=router options:router-port=rp-sw0 \ + -- lsp-set-addresses sw0-rp router +check ovn-nbctl lsp-add sw1 sw1-rp -- set Logical_Switch_Port sw1-rp \ + type=router options:router-port=rp-sw1 \ + -- lsp-set-addresses sw1-rp router + +ADD_NAMESPACES(sw01) +ADD_VETH(sw01, sw01, br-int, "192.168.1.2/24", "f0:00:00:01:02:03", \ + "192.168.1.1") +check ovn-nbctl lsp-add sw0 sw01 \ + -- lsp-set-addresses sw01 "f0:00:00:01:02:03 192.168.1.2" + +ADD_NAMESPACES(sw11) +ADD_VETH(sw11, sw11, br-int, "192.168.2.2/24", "f0:00:00:02:02:03", \ + "192.168.2.1") +check ovn-nbctl lsp-add sw1 sw11 \ + -- lsp-set-addresses sw11 "f0:00:00:02:02:03 192.168.2.2" + +NETNS_DAEMONIZE([sw01], [nc -k -l 8000], [nc-sw01.pid]) + +test_ping() { + NS_CHECK_EXEC([$1], [ping -q -c 1 $2 -w 2 | FORMAT_PING], \ +[0], [dnl +1 packets transmitted, 1 received, 0% packet loss, time 0ms +]) +} + +# Only SNAT +check ovn-nbctl --wait=hv lr-nat-add R1 snat 172.16.1.21 192.168.2.0/24 + +echo "foo" > foo +NS_CHECK_EXEC([sw11], [nc 192.168.1.2 8000 < foo]) +test_ping sw11 192.168.1.2 + +# Ensure nat has been hit +OVS_WAIT_UNTIL([ovs-ofctl dump-flows br-int | grep -v "n_packets=0" | grep 'nat(src=172.16.1.21)']) +# Ensure conntrack entry is present +OVS_WAIT_FOR_OUTPUT([ + ovs-appctl dpctl/dump-conntrack | FORMAT_CT(192.168.2.2) | \ + sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl +icmp,orig=(src=192.168.2.2,dst=192.168.1.2,id=<cleared>,type=8,code=0),reply=(src=192.168.1.2,dst=192.168.2.2,id=<cleared>,type=0,code=0),zone=<cleared> +tcp,orig=(src=192.168.2.2,dst=192.168.1.2,sport=<cleared>,dport=<cleared>),reply=(src=192.168.1.2,dst=192.168.2.2,sport=<cleared>,dport=<cleared>),zone=<cleared>,protoinfo=(state=<cleared>) +]) + +AT_CHECK([ovs-appctl dpctl/flush-conntrack]) + +# SNAT and DNAT. using Logical IP +ovn-nbctl --wait=hv lr-nat-add R1 dnat_and_snat 172.16.1.2 192.168.1.2 +NS_CHECK_EXEC([sw11], [nc 192.168.1.2 8000 < foo ]) +test_ping sw11 192.168.1.2 + +# Ensure conntrack entry is present +OVS_WAIT_FOR_OUTPUT([ + ovs-appctl dpctl/dump-conntrack | FORMAT_CT(192.168.2.2) | \ + sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl +icmp,orig=(src=192.168.2.2,dst=192.168.1.2,id=<cleared>,type=8,code=0),reply=(src=192.168.1.2,dst=192.168.2.2,id=<cleared>,type=0,code=0),zone=<cleared> +tcp,orig=(src=192.168.2.2,dst=192.168.1.2,sport=<cleared>,dport=<cleared>),reply=(src=192.168.1.2,dst=192.168.2.2,sport=<cleared>,dport=<cleared>),zone=<cleared>,protoinfo=(state=<cleared>) +]) + +AT_CHECK([ovs-appctl dpctl/flush-conntrack]) + +# SNAT and DNAT. using floating IP +NS_CHECK_EXEC([sw11], [nc 172.16.1.2 8000 < foo ]) +test_ping sw11 172.16.1.2 + +# Ensure conntrack entry is present +OVS_WAIT_FOR_OUTPUT([ + ovs-appctl dpctl/dump-conntrack | FORMAT_CT(192.168.2.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=192.168.1.2,dst=192.168.2.2,id=<cleared>,type=0,code=0),zone=<cleared> +tcp,orig=(src=192.168.2.2,dst=172.16.1.2,sport=<cleared>,dport=<cleared>),reply=(src=192.168.1.2,dst=192.168.2.2,sport=<cleared>,dport=<cleared>),zone=<cleared>,protoinfo=(state=<cleared>) +]) + +AT_CHECK([ovs-appctl dpctl/flush-conntrack]) + +OVS_APP_EXIT_AND_WAIT([ovn-controller]) +as +OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d +/connection dropped.*/d"]) + +AT_CLEANUP +])