diff mbox series

[ovs-dev,v2,1/2] northd: Swap src and dst eth addresses in router egress loop.

Message ID 20210310195507.1214545-1-mmichels@redhat.com
State Superseded
Headers show
Series [ovs-dev,v2,1/2] northd: Swap src and dst eth addresses in router egress loop. | expand

Commit Message

Mark Michelson March 10, 2021, 7:55 p.m. UTC
When a hairpin scenario is detected (i.e. egressing on gateway router
port to a NAT external address), we loop back to the ingress router
pipeline and swap the inport and outport. This is intended to allow for
the traffic to get DNATted. In practice, though, the ethernet
destination has not been updated to be the distributed gateway port's
address, and so the packet is immediately dropped.

This fixes the issue by swapping source and dest eth addressses when
doing the hairpin redirection. This way, the packet appears to be
arriving on the gateway port, and it gets handled as expected.

Reported at: https://bugzilla.redhat.com/show_bug.cgi?id=1929901

Signed-off-by: Mark Michelson <mmichels@redhat.com>
---
 northd/ovn-northd.c  |   1 +
 northd/ovn_northd.dl |   1 +
 tests/system-ovn.at  | 111 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 113 insertions(+)

Comments

Numan Siddique March 15, 2021, 6:14 p.m. UTC | #1
On Thu, Mar 11, 2021 at 1:25 AM Mark Michelson <mmichels@redhat.com> wrote:
>
> When a hairpin scenario is detected (i.e. egressing on gateway router
> port to a NAT external address), we loop back to the ingress router
> pipeline and swap the inport and outport. This is intended to allow for
> the traffic to get DNATted. In practice, though, the ethernet
> destination has not been updated to be the distributed gateway port's
> address, and so the packet is immediately dropped.
>
> This fixes the issue by swapping source and dest eth addressses when
> doing the hairpin redirection. This way, the packet appears to be
> arriving on the gateway port, and it gets handled as expected.
>
> Reported at: https://bugzilla.redhat.com/show_bug.cgi?id=1929901
>
> Signed-off-by: Mark Michelson <mmichels@redhat.com>
> ---
>  northd/ovn-northd.c  |   1 +
>  northd/ovn_northd.dl |   1 +
>  tests/system-ovn.at  | 111 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 113 insertions(+)
>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 7b95a5c7f..cab17f07e 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -11642,6 +11642,7 @@ build_lrouter_nat_defrag_and_lb(struct ovn_datapath *od,
>              ds_put_format(actions,
>                            "clone { ct_clear; "
>                            "inport = outport; outport = \"\"; "
> +                          "eth.dst <-> eth.src; "
>                            "flags = 0; flags.loopback = 1; ");
>              for (int j = 0; j < MFF_N_LOG_REGS; j++) {
>                  ds_put_format(actions, "reg%d = 0; ", j);
> diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl
> index 8d0d55682..227f38157 100644
> --- a/northd/ovn_northd.dl
> +++ b/northd/ovn_northd.dl
> @@ -5923,6 +5923,7 @@ for (r in &Router(.lr = lr,
>              var actions =
>                  "clone { ct_clear; "
>                  "inport = outport; outport = \"\"; "
> +                "eth.dst <-> eth.src ;"

In v1 of this patch, Ben pointed out a typo.

With that typo fixed - Acked-by: Numan Siddique <numans@ovn.org>

Thanks
Numan



>                  "flags = 0; flags.loopback = 1; " ++
>                  regs.join("") ++
>                  "${rEGBIT_EGRESS_LOOPBACK()} = 1; "
> diff --git a/tests/system-ovn.at b/tests/system-ovn.at
> index 4885303d1..36b469c30 100644
> --- a/tests/system-ovn.at
> +++ b/tests/system-ovn.at
> @@ -5903,3 +5903,114 @@ OVS_TRAFFIC_VSWITCHD_STOP(["/.*error receiving.*/d
>  /.*terminating with signal 15.*/d"])
>  AT_CLEANUP
>  ])
> +
> +OVN_FOR_EACH_NORTHD([
> +AT_SETUP(ovn -- DNAT LR hairpin IPv4)
> +AT_KEYWORDS(hairpin)
> +
> +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_daemon ovn-controller
> +
> +# Logical network:
> +# Two VMs
> +#   * VM1 with IP address 192.168.100.5
> +#   * VM2 with IP address 192.168.100.6
> +# The VMs connect to logical switch ls1.
> +#
> +# An external router with IP address 172.18.1.2. We simulate this with a network namespace.
> +# There will be no traffic going here in this test.
> +# The external router connects to logical switch ls-pub
> +#
> +# One logical router (lr1) connects to ls1 and ls-pub. The router port connected to ls-pub is
> +# a gateway port.
> +#   * The subnet connected to ls1 is 192.168.100.0/24. The Router IP address is 192.168.100.1
> +#   * The subnet connected to ls-pub is 172.18.1.0/24. The Router IP address is 172.168.1.1
> +# lr1 has the following attributes:
> +#   * It has a "default" static route that sends traffic out the gateway router port.
> +#   * It has a DNAT rule that translates 172.18.2.10 to 192.168.100.6 (VM2)
> +#
> +# In this test, we want to ensure that a ping from VM1 to IP address 172.18.2.10 reaches VM2.
> +
> +ovn-nbctl ls-add ls1
> +ovn-nbctl lsp-add ls1 vm1 -- lsp-set-addresses vm1 "00:00:00:00:00:05 192.168.100.5"
> +ovn-nbctl lsp-add ls1 vm2 -- lsp-set-addresses vm2 "00:00:00:00:00:06 192.168.100.6"
> +
> +ovn-nbctl ls-add ls-pub
> +ovn-nbctl lsp-add ls-pub ext-router -- lsp-set-addresses ext-router "00:00:00:00:01:02 172.18.1.2"
> +
> +ovn-nbctl lr-add lr1
> +ovn-nbctl lrp-add lr1 lr1-ls1 00:00:00:00:00:01 192.168.100.1/24
> +ovn-nbctl lsp-add ls1 ls1-lr1                      \
> +    -- lsp-set-type ls1-lr1 router                 \
> +    -- lsp-set-addresses ls1-lr1 00:00:00:00:00:01 \
> +    -- lsp-set-options ls1-lr1 router-port=lr1-ls1
> +
> +ovn-nbctl lrp-add lr1 lr1-ls-pub 00:00:00:00:01:01 172.18.1.1/24
> +ovn-nbctl lrp-set-gateway-chassis lr1-ls-pub hv1
> +ovn-nbctl lsp-add ls-pub ls-pub-lr1                      \
> +    -- lsp-set-type ls-pub-lr1 router                    \
> +    -- lsp-set-addresses ls-pub-lr1 00:00:00:00:01:01    \
> +    -- lsp-set-options ls-pub-lr1 router-port=lr1-ls-pub
> +
> +ovn-nbctl lr-nat-add lr1 snat 172.18.1.1 192.168.100.0/24
> +ovn-nbctl lr-nat-add lr1 dnat_and_snat 172.18.2.10 192.168.100.6
> +ovn-nbctl lr-route-add lr1 0.0.0.0/0 172.18.1.2
> +
> +#ls1_uuid=$(fetch_column Port_Binding datapath logical_port=vm1)
> +#ovn-sbctl create MAC_Binding ip=172.18.2.10 datapath=$ls1_uuid logical_port=vm2 mac="00:00:00:00:00:06"
> +
> +OVN_POPULATE_ARP
> +ovn-nbctl --wait=hv sync
> +
> +ADD_NAMESPACES(vm1)
> +ADD_VETH(vm1, vm1, br-int, "192.168.100.5/24", "00:00:00:00:00:05", \
> +         "192.168.100.1")
> +
> +ADD_NAMESPACES(vm2)
> +ADD_VETH(vm2, vm2, br-int, "192.168.100.6/24", "00:00:00:00:00:06", \
> +         "192.168.100.1")
> +
> +ADD_NAMESPACES(ext-router)
> +ADD_VETH(ext-router, ext-router, br-int, "172.18.1.2/24", "00:00:00:00:01:02", \
> +         "172.18.1.1")
> +
> +# Let's take a quick look at the logical flows
> +ovn-sbctl lflow-list
> +
> +# Let's check what ovn-trace says...
> +ovn-trace ls1 'inport == "vm1" && eth.src == 00:00:00:00:00:05 && ip4.src == 192.168.100.5 && eth.dst == 00:00:00:00:00:01 && ip4.dst == 172.18.2.10 && ip.ttl == 32'
> +
> +# A ping from vm1 should hairpin in lr1 and successfully DNAT to vm2
> +NS_CHECK_EXEC([vm1], [ping -q -c 3 -i 0.3 -w 2 172.18.2.10 | FORMAT_PING], \
> +[0], [dnl
> +3 packets transmitted, 3 received, 0% packet loss, time 0ms
> +])
> +kill $(pidof ovn-controller)
> +
> +as ovn-sb
> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> +
> +as ovn-nb
> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> +
> +as northd
> +OVS_APP_EXIT_AND_WAIT([NORTHD_TYPE])
> +
> +as
> +OVS_TRAFFIC_VSWITCHD_STOP(["/.*error receiving.*/d
> +/.*terminating with signal 15.*/d"])
> +
> +AT_CLEANUP
> +])
> --
> 2.29.2
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
diff mbox series

Patch

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 7b95a5c7f..cab17f07e 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -11642,6 +11642,7 @@  build_lrouter_nat_defrag_and_lb(struct ovn_datapath *od,
             ds_put_format(actions,
                           "clone { ct_clear; "
                           "inport = outport; outport = \"\"; "
+                          "eth.dst <-> eth.src; "
                           "flags = 0; flags.loopback = 1; ");
             for (int j = 0; j < MFF_N_LOG_REGS; j++) {
                 ds_put_format(actions, "reg%d = 0; ", j);
diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl
index 8d0d55682..227f38157 100644
--- a/northd/ovn_northd.dl
+++ b/northd/ovn_northd.dl
@@ -5923,6 +5923,7 @@  for (r in &Router(.lr = lr,
             var actions =
                 "clone { ct_clear; "
                 "inport = outport; outport = \"\"; "
+                "eth.dst <-> eth.src ;"
                 "flags = 0; flags.loopback = 1; " ++
                 regs.join("") ++
                 "${rEGBIT_EGRESS_LOOPBACK()} = 1; "
diff --git a/tests/system-ovn.at b/tests/system-ovn.at
index 4885303d1..36b469c30 100644
--- a/tests/system-ovn.at
+++ b/tests/system-ovn.at
@@ -5903,3 +5903,114 @@  OVS_TRAFFIC_VSWITCHD_STOP(["/.*error receiving.*/d
 /.*terminating with signal 15.*/d"])
 AT_CLEANUP
 ])
+
+OVN_FOR_EACH_NORTHD([
+AT_SETUP(ovn -- DNAT LR hairpin IPv4)
+AT_KEYWORDS(hairpin)
+
+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_daemon ovn-controller
+
+# Logical network:
+# Two VMs
+#   * VM1 with IP address 192.168.100.5
+#   * VM2 with IP address 192.168.100.6
+# The VMs connect to logical switch ls1.
+#
+# An external router with IP address 172.18.1.2. We simulate this with a network namespace.
+# There will be no traffic going here in this test.
+# The external router connects to logical switch ls-pub
+#
+# One logical router (lr1) connects to ls1 and ls-pub. The router port connected to ls-pub is
+# a gateway port.
+#   * The subnet connected to ls1 is 192.168.100.0/24. The Router IP address is 192.168.100.1
+#   * The subnet connected to ls-pub is 172.18.1.0/24. The Router IP address is 172.168.1.1
+# lr1 has the following attributes:
+#   * It has a "default" static route that sends traffic out the gateway router port.
+#   * It has a DNAT rule that translates 172.18.2.10 to 192.168.100.6 (VM2)
+#
+# In this test, we want to ensure that a ping from VM1 to IP address 172.18.2.10 reaches VM2.
+
+ovn-nbctl ls-add ls1
+ovn-nbctl lsp-add ls1 vm1 -- lsp-set-addresses vm1 "00:00:00:00:00:05 192.168.100.5"
+ovn-nbctl lsp-add ls1 vm2 -- lsp-set-addresses vm2 "00:00:00:00:00:06 192.168.100.6"
+
+ovn-nbctl ls-add ls-pub
+ovn-nbctl lsp-add ls-pub ext-router -- lsp-set-addresses ext-router "00:00:00:00:01:02 172.18.1.2"
+
+ovn-nbctl lr-add lr1
+ovn-nbctl lrp-add lr1 lr1-ls1 00:00:00:00:00:01 192.168.100.1/24
+ovn-nbctl lsp-add ls1 ls1-lr1                      \
+    -- lsp-set-type ls1-lr1 router                 \
+    -- lsp-set-addresses ls1-lr1 00:00:00:00:00:01 \
+    -- lsp-set-options ls1-lr1 router-port=lr1-ls1
+
+ovn-nbctl lrp-add lr1 lr1-ls-pub 00:00:00:00:01:01 172.18.1.1/24
+ovn-nbctl lrp-set-gateway-chassis lr1-ls-pub hv1
+ovn-nbctl lsp-add ls-pub ls-pub-lr1                      \
+    -- lsp-set-type ls-pub-lr1 router                    \
+    -- lsp-set-addresses ls-pub-lr1 00:00:00:00:01:01    \
+    -- lsp-set-options ls-pub-lr1 router-port=lr1-ls-pub
+
+ovn-nbctl lr-nat-add lr1 snat 172.18.1.1 192.168.100.0/24
+ovn-nbctl lr-nat-add lr1 dnat_and_snat 172.18.2.10 192.168.100.6
+ovn-nbctl lr-route-add lr1 0.0.0.0/0 172.18.1.2
+
+#ls1_uuid=$(fetch_column Port_Binding datapath logical_port=vm1)
+#ovn-sbctl create MAC_Binding ip=172.18.2.10 datapath=$ls1_uuid logical_port=vm2 mac="00:00:00:00:00:06"
+
+OVN_POPULATE_ARP
+ovn-nbctl --wait=hv sync
+
+ADD_NAMESPACES(vm1)
+ADD_VETH(vm1, vm1, br-int, "192.168.100.5/24", "00:00:00:00:00:05", \
+         "192.168.100.1")
+
+ADD_NAMESPACES(vm2)
+ADD_VETH(vm2, vm2, br-int, "192.168.100.6/24", "00:00:00:00:00:06", \
+         "192.168.100.1")
+
+ADD_NAMESPACES(ext-router)
+ADD_VETH(ext-router, ext-router, br-int, "172.18.1.2/24", "00:00:00:00:01:02", \
+         "172.18.1.1")
+
+# Let's take a quick look at the logical flows
+ovn-sbctl lflow-list
+
+# Let's check what ovn-trace says...
+ovn-trace ls1 'inport == "vm1" && eth.src == 00:00:00:00:00:05 && ip4.src == 192.168.100.5 && eth.dst == 00:00:00:00:00:01 && ip4.dst == 172.18.2.10 && ip.ttl == 32'
+
+# A ping from vm1 should hairpin in lr1 and successfully DNAT to vm2
+NS_CHECK_EXEC([vm1], [ping -q -c 3 -i 0.3 -w 2 172.18.2.10 | FORMAT_PING], \
+[0], [dnl
+3 packets transmitted, 3 received, 0% packet loss, time 0ms
+])
+kill $(pidof ovn-controller)
+
+as ovn-sb
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+
+as ovn-nb
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+
+as northd
+OVS_APP_EXIT_AND_WAIT([NORTHD_TYPE])
+
+as
+OVS_TRAFFIC_VSWITCHD_STOP(["/.*error receiving.*/d
+/.*terminating with signal 15.*/d"])
+
+AT_CLEANUP
+])