From: Numan Siddique <num...@ovn.org>

When a Gateway router is configured with a load balancer
and it is also configured with options:lb_force_snat_ip=<IP>,
OVN after load balancing the destination IP to one of the
backend also does a NAT on the source ip with the
lb_force_snat_ip if the packet is destined to a load balancer
VIP.

There is a problem with the snat of source ip to 'lb_force_snat_ip'
in one particular usecase.  When the packet enters the Gateway router
from a provider logical switch destined to the load balancer VIP,
then it is first load balanced to one of the backend and then
the source ip is snatted to 'lb_force_snat_ip'.  If the chosen
backend is reachable via the provider logical switch, then the
packet is hairpinned back and it may hit the wire with
the source ip 'lb_force_snat_ip'.  If 'lb_force_snat_ip' happens
to be an OVN internal IP then the packet may be dropped.

This patch addresses this issue by providing the option to
set the option - 'lb_force_snat_ip=router_ip'.  If 'router_ip'
is set, then OVN will snat the load balanced packet to the
router ip of the logical router port which chosen as 'outport'
in lr_in_ip_routing stage.

Example.

If the gateway router is

ovn-nbctl show lr0
router 68f20092-5563-44b8-9ccb-b11de3e3a66c (lr0)
    port lr0-sw0
        mac: "00:00:00:00:ff:01"
        networks: ["10.0.0.1/24"]
    port lr0-public
        mac: "00:00:20:20:12:13"
        networks: ["172.168.0.100/24"]

Then the below logical flows are added if 'lb_force_snat_ip'
is configured to 'router_ip'.

table=1 (lr_out_snat), priority=110
   match=(flags.force_snat_for_lb == 1 && ip4 && outport == "lr0-public"),
   action=(ct_snat(172.168.0.100);)

table=1 (lr_out_snat), priority=110
   match=(flags.force_snat_for_lb == 1 && ip4 && outport == "lr0-sw0")
   action=(ct_snat(10.0.0.1);)

For the above described scenario, the packet will have source ip as
172.168.0.100 which belongs to the provider logical switch CIDR.

Reported-by: Tim Rozet <tro...@redhat.com>
Signed-off-by: Numan Siddique <num...@ovn.org>
---
 northd/ovn-northd.8.xml | 35 ++++++++++++++++++
 northd/ovn-northd.c     | 66 ++++++++++++++++++++++++++++++++--
 tests/ovn-northd.at     | 79 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 177 insertions(+), 3 deletions(-)

diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index 70065a36d9..27b28aff93 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -3653,6 +3653,32 @@ nd_ns {
           <code>flags.force_snat_for_dnat == 1 &amp;&amp; ip</code> with an
           action <code>ct_snat(<var>B</var>);</code>.
         </p>
+      </li>
+
+      <li>
+        <p>
+          If the Gateway router in the OVN Northbound database has been
+          configured to force SNAT a packet (that has been previously
+          load-balanced) using router IP (i.e <ref column="options"
+          table="Logical_Router"/>:lb_force_snat_ip=router_ip), then for
+          each logical router port <var>P</var> attached to the Gateway
+          router, a priority-110 flow matches
+          <code>flags.force_snat_for_lb == 1 &amp;&amp; outport == <var>P</var>
+          </code> with an action <code>ct_snat(<var>R</var>);</code>
+          where <var>R</var> is the router port IP configured.
+          If <code>R</code> is an IPv4 address then the match will also
+          include <code>ip4</code> and if it is an IPv6 address, then the
+          match will also include <code>ip6</code>.
+        </p>
+
+        <p>
+          If the logical router port <var>P</var> is configured with multiple
+          IPv4 and multiple IPv6 addresses, only the first IPv4 and first IPv6
+          address is considered.
+        </p>
+      </li>
+
+      <li>
         <p>
           If the Gateway router in the OVN Northbound database has been
           configured to force SNAT a packet (that has been previously
@@ -3660,6 +3686,9 @@ nd_ns {
           <code>flags.force_snat_for_lb == 1 &amp;&amp; ip</code> with an
           action <code>ct_snat(<var>B</var>);</code>.
         </p>
+      </li>
+
+      <li>
         <p>
           For each configuration in the OVN Northbound database, that asks
           to change the source IP address of a packet from an IP address of
@@ -3673,14 +3702,18 @@ nd_ns {
           options, then the action would be <code>ip4/6.src=
           (<var>B</var>)</code>.
         </p>
+      </li>
 
+      <li>
         <p>
           If the NAT rule has <code>allowed_ext_ips</code> configured, then
           there is an additional match <code>ip4.dst == <var>allowed_ext_ips
           </var></code>. Similarly, for IPV6, match would be <code>ip6.dst ==
           <var>allowed_ext_ips</var></code>.
         </p>
+      </li>
 
+      <li>
         <p>
           If the NAT rule has <code>exempted_ext_ips</code> set, then
           there is an additional flow configured at the priority + 1 of
@@ -3689,7 +3722,9 @@ nd_ns {
           </code>. This flow is used to bypass the ct_snat action for a packet
           which is destinted to <code>exempted_ext_ips</code>.
         </p>
+      </li>
 
+      <li>
         <p>
           A priority-0 logical flow with match <code>1</code> has actions
           <code>next;</code>.
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index db6572a62b..ece158b71e 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -622,6 +622,7 @@ struct ovn_datapath {
 
     struct lport_addresses dnat_force_snat_addrs;
     struct lport_addresses lb_force_snat_addrs;
+    bool lb_force_snat_router_ip;
 
     struct ovn_port **localnet_ports;
     size_t n_localnet_ports;
@@ -721,6 +722,17 @@ init_nat_entries(struct ovn_datapath *od)
             snat_ip_add(od, od->lb_force_snat_addrs.ipv6_addrs[0].addr_s,
                         NULL);
         }
+    } else {
+        const char *lb_force_snat =
+            smap_get(&od->nbr->options, "lb_force_snat_ip");
+        if (lb_force_snat && !strcmp(lb_force_snat, "router_ip")
+                && smap_get(&od->nbr->options, "chassis")) {
+            /* Set it to true only if its gateway router and
+             * options:lb_force_snat_ip=router_ip. */
+            od->lb_force_snat_router_ip = true;
+        } else {
+            od->lb_force_snat_router_ip = false;
+        }
     }
 
     if (!od->nbr->n_nat) {
@@ -8365,9 +8377,12 @@ get_force_snat_ip(struct ovn_datapath *od, const char 
*key_type,
     }
 
     if (!extract_ip_address(addresses, laddrs)) {
-        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
-        VLOG_WARN_RL(&rl, "bad ip %s in options of router "UUID_FMT"",
-                     addresses, UUID_ARGS(&od->key));
+        if (strcmp(addresses, "router_ip") || strcmp(key_type, "lb")) {
+            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
+            VLOG_WARN_RL(&rl, "bad ip %s in options of router "UUID_FMT"",
+                         addresses, UUID_ARGS(&od->key));
+        }
+
         return false;
     }
 
@@ -8943,6 +8958,48 @@ build_lrouter_force_snat_flows(struct hmap *lflows, 
struct ovn_datapath *od,
     ds_destroy(&actions);
 }
 
+static void
+build_lrouter_force_snat_flows_op(struct ovn_port *op,
+                                  struct hmap *lflows,
+                                  struct ds *match, struct ds *actions)
+{
+    if (!op->nbrp || !op->peer || !op->od->lb_force_snat_router_ip) {
+        return;
+    }
+
+    if (op->lrp_networks.n_ipv4_addrs) {
+        ds_clear(match);
+        ds_clear(actions);
+
+        /* Higher priority rules to force SNAT with the router port ip.
+         * This only takes effect when the packet has already been
+         * load balanced once. */
+        ds_put_format(match, "flags.force_snat_for_lb == 1 && ip4 && "
+                      "outport == %s", op->json_key);
+        ds_put_format(actions, "ct_snat(%s);",
+                      op->lrp_networks.ipv4_addrs[0].addr_s);
+        ovn_lflow_add(lflows, op->od, S_ROUTER_OUT_SNAT, 110,
+                      ds_cstr(match), ds_cstr(actions));
+    }
+
+    /* op->lrp_networks.ipv6_addrs will always have LLA and that will be
+     * last in the list. So add the flows only if n_ipv6_addrs > 1. */
+    if (op->lrp_networks.n_ipv6_addrs > 1) {
+        ds_clear(match);
+        ds_clear(actions);
+
+        /* Higher priority rules to force SNAT with the router port ip.
+         * This only takes effect when the packet has already been
+         * load balanced once. */
+        ds_put_format(match, "flags.force_snat_for_lb == 1 && ip6 && "
+                      "outport == %s", op->json_key);
+        ds_put_format(actions, "ct_snat(%s);",
+                      op->lrp_networks.ipv6_addrs[0].addr_s);
+        ovn_lflow_add(lflows, op->od, S_ROUTER_OUT_SNAT, 110,
+                      ds_cstr(match), ds_cstr(actions));
+    }
+}
+
 static void
 build_lrouter_bfd_flows(struct hmap *lflows, struct ovn_port *op)
 {
@@ -11278,6 +11335,7 @@ build_lrouter_nat_defrag_and_lb(struct ovn_datapath *od,
                         "dnat");
                 }
             }
+
             if (lb_force_snat_ip) {
                 if (od->lb_force_snat_addrs.n_ipv4_addrs) {
                     build_lrouter_force_snat_flows(lflows, od, "4",
@@ -11490,6 +11548,8 @@ build_lswitch_and_lrouter_iterate_by_op(struct ovn_port 
*op,
                                             &lsi->match, &lsi->actions);
     build_lrouter_ipv4_ip_input(op, lsi->lflows,
                                 &lsi->match, &lsi->actions);
+    build_lrouter_force_snat_flows_op(op, lsi->lflows, &lsi->match,
+                                      &lsi->actions);
 }
 
 static void
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 7240e22baf..fd03b1fb66 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -2443,3 +2443,82 @@ check ovn-sbctl set chassis hv1 
other_config:port-up-notif=true
 wait_row_count nb:Logical_Switch_Port 1 up=false name=lsp1
 
 AT_CLEANUP
+
+AT_SETUP([ovn -- lb_force_snat_ip for Gateway Routers])
+ovn_start
+
+check ovn-nbctl ls-add sw0
+check ovn-nbctl ls-add sw1
+
+# Create a logical router and attach both logical switches
+check ovn-nbctl lr-add lr0
+check ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24
+check ovn-nbctl lsp-add sw0 sw0-lr0
+check ovn-nbctl lsp-set-type sw0-lr0 router
+check ovn-nbctl lsp-set-addresses sw0-lr0 00:00:00:00:ff:01
+check ovn-nbctl lsp-set-options sw0-lr0 router-port=lr0-sw0
+
+check ovn-nbctl lrp-add lr0 lr0-sw1 00:00:00:00:ff:02 20.0.0.1/24
+check ovn-nbctl lsp-add sw1 sw1-lr0
+check ovn-nbctl lsp-set-type sw1-lr0 router
+check ovn-nbctl lsp-set-addresses sw1-lr0 00:00:00:00:ff:02
+check ovn-nbctl lsp-set-options sw1-lr0 router-port=lr0-sw1
+
+check ovn-nbctl ls-add public
+check ovn-nbctl lrp-add lr0 lr0-public 00:00:20:20:12:13 172.168.0.100/24
+check ovn-nbctl lsp-add public public-lr0
+check ovn-nbctl lsp-set-type public-lr0 router
+check ovn-nbctl lsp-set-addresses public-lr0 router
+check ovn-nbctl lsp-set-options public-lr0 router-port=lr0-public
+
+check ovn-nbctl set logical_router lr0 options:chassis=ch1
+
+ovn-sbctl dump-flows lr0 > lr0flows
+AT_CAPTURE_FILE([lr0flows])
+
+AT_CHECK([grep "lr_out_snat" lr0flows | grep force_snat_for_lb | sort], [0], 
[dnl
+])
+
+check ovn-nbctl --wait=sb set logical_router lr0 
options:lb_force_snat_ip="20.0.0.4 aef0::4"
+
+ovn-sbctl dump-flows lr0 > lr0flows
+AT_CAPTURE_FILE([lr0flows])
+
+AT_CHECK([grep "lr_out_snat" lr0flows | grep force_snat_for_lb | sort], [0], 
[dnl
+  table=1 (lr_out_snat        ), priority=100  , 
match=(flags.force_snat_for_lb == 1 && ip4), action=(ct_snat(20.0.0.4);)
+  table=1 (lr_out_snat        ), priority=100  , 
match=(flags.force_snat_for_lb == 1 && ip6), action=(ct_snat(aef0::4);)
+])
+
+check ovn-nbctl --wait=sb set logical_router lr0 
options:lb_force_snat_ip="router_ip"
+
+ovn-sbctl dump-flows lr0 > lr0flows
+AT_CAPTURE_FILE([lr0flows])
+
+AT_CHECK([grep "lr_out_snat" lr0flows | grep force_snat_for_lb | sort], [0], 
[dnl
+  table=1 (lr_out_snat        ), priority=110  , 
match=(flags.force_snat_for_lb == 1 && ip4 && outport == "lr0-public"), 
action=(ct_snat(172.168.0.100);)
+  table=1 (lr_out_snat        ), priority=110  , 
match=(flags.force_snat_for_lb == 1 && ip4 && outport == "lr0-sw0"), 
action=(ct_snat(10.0.0.1);)
+  table=1 (lr_out_snat        ), priority=110  , 
match=(flags.force_snat_for_lb == 1 && ip4 && outport == "lr0-sw1"), 
action=(ct_snat(20.0.0.1);)
+])
+
+check ovn-nbctl --wait=sb remove logical_router lr0 options chassis
+
+ovn-sbctl dump-flows lr0 > lr0flows
+AT_CAPTURE_FILE([lr0flows])
+
+AT_CHECK([grep "lr_out_snat" lr0flows | grep force_snat_for_lb | sort], [0], 
[dnl
+])
+
+check ovn-nbctl set logical_router lr0 options:chassis=ch1
+check ovn-nbctl --wait=sb add logical_router_port lr0-sw1 networks 
"bef0\:\:1/64"
+
+ovn-sbctl dump-flows lr0 > lr0flows
+AT_CAPTURE_FILE([lr0flows])
+
+AT_CHECK([grep "lr_out_snat" lr0flows | grep force_snat_for_lb | sort], [0], 
[dnl
+  table=1 (lr_out_snat        ), priority=110  , 
match=(flags.force_snat_for_lb == 1 && ip4 && outport == "lr0-public"), 
action=(ct_snat(172.168.0.100);)
+  table=1 (lr_out_snat        ), priority=110  , 
match=(flags.force_snat_for_lb == 1 && ip4 && outport == "lr0-sw0"), 
action=(ct_snat(10.0.0.1);)
+  table=1 (lr_out_snat        ), priority=110  , 
match=(flags.force_snat_for_lb == 1 && ip4 && outport == "lr0-sw1"), 
action=(ct_snat(20.0.0.1);)
+  table=1 (lr_out_snat        ), priority=110  , 
match=(flags.force_snat_for_lb == 1 && ip6 && outport == "lr0-sw1"), 
action=(ct_snat(bef0::1);)
+])
+
+AT_CLEANUP
-- 
2.29.2

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to