On Tue, Apr 5, 2022 at 2:18 PM Mark Michelson <[email protected]> wrote:
>
> Thanks Xavier. It's amazing this issue has been around so long.
>
The main reason this issue has not been an actual issue is because
generally a gateway router is connected to a join switch
and E-W traffic doesn't reach the gateway router as it will be handled
in the distributed router itself.
Thanks for fixing this issue.
I applied this patch to the main branch and backported to branch-22.03
with the below changes
----------------------
diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index 85a65703db..db4f4d267c 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -4505,7 +4505,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
-------------------------
I couldn't apply the patch cleanly to branch-21.12. If you think this
needs to be backported further please
submit a backport patch.
Numan
> Acked-by: Mark Michelson <[email protected]>
>
> On 4/1/22 12:34, 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 <[email protected]>
> >
> > ---
> > v2: replaced "<<<" in tests as failing on github
> > ---
> > northd/northd.c | 1 +
> > tests/ovn-northd.at | 33 ++++++------
> > tests/system-ovn.at | 119 ++++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 137 insertions(+), 16 deletions(-)
> >
> > diff --git a/northd/northd.c b/northd/northd.c
> > index 2fb0a93c2..511bd3331 100644
> > --- a/northd/northd.c
> > +++ b/northd/northd.c
> > @@ -12919,6 +12919,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/tests/ovn-northd.at b/tests/ovn-northd.at
> > index daa664982..f462fca86 100644
> > --- a/tests/ovn-northd.at
> > +++ b/tests/ovn-northd.at
> > @@ -1030,7 +1030,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);)
> > ])
> >
> >
> > @@ -1062,7 +1062,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;)
> > ])
> >
> > @@ -1091,7 +1091,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
> > @@ -1120,7 +1120,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;)
> > ])
> >
> > @@ -5140,11 +5140,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
> > @@ -5201,9 +5202,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.
> > @@ -5266,9 +5267,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.
> > @@ -5346,9 +5347,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 5d90be1c9..353c8df5d 100644
> > --- a/tests/system-ovn.at
> > +++ b/tests/system-ovn.at
> > @@ -7992,3 +7992,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
> > +])
> >
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
