[ovs-dev,v2] OVN: fix DVR Floating IP support
diff mbox series

Message ID c4d12f354d0167d0c2626d4fd9d143d1a8d42f30.1554565194.git.lorenzo.bianconi@redhat.com
State Accepted
Headers show
Series
  • [ovs-dev,v2] OVN: fix DVR Floating IP support
Related show

Commit Message

Lorenzo Bianconi April 6, 2019, 3:42 p.m. UTC
When DVR is enabled FIP traffic need to be forwarded directly using
external connection to the underlay network and not be distributed
through geneve tunnels.
Fix this adding new logical flows to take care of distributed DNAT/SNAT

Acked-by: Mark Michelson <mmichels@redhat.com>
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
---
Changes since v1:
- minor optimization in add_distributed_nat_routes
---
 ovn/northd/ovn-northd.8.xml |  51 ++++++++++++++++++
 ovn/northd/ovn-northd.c     | 105 ++++++++++++++++++++++++++++++++++++
 2 files changed, 156 insertions(+)

Comments

Ben Pfaff April 16, 2019, 5:40 p.m. UTC | #1
On Sat, Apr 06, 2019 at 05:42:52PM +0200, Lorenzo Bianconi wrote:
> When DVR is enabled FIP traffic need to be forwarded directly using
> external connection to the underlay network and not be distributed
> through geneve tunnels.
> Fix this adding new logical flows to take care of distributed DNAT/SNAT
> 
> Acked-by: Mark Michelson <mmichels@redhat.com>
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>

Thanks, applied to master.
Daniel Alvarez Sanchez April 23, 2019, 2:07 p.m. UTC | #2
Even I successfully tested this before getting merged, I just hit some
scenario where traffic goes to the gateway node. If we have logical
port lp1 with a dnat_and_snat NAT rule to fip1 and from lp1 you ping
fip1, the traffic is leaving the hypervisor to the gateway node and
comes back to lp1.

I'd expect this traffic to be recirculated within the OVN bridge and
not being pushed out through the localnet port. It may be a corner
case but I believe it should be taken into account. What do you folks
think?

Cheers,
Daniel

On Tue, Apr 16, 2019 at 7:41 PM Ben Pfaff <blp@ovn.org> wrote:
>
> On Sat, Apr 06, 2019 at 05:42:52PM +0200, Lorenzo Bianconi wrote:
> > When DVR is enabled FIP traffic need to be forwarded directly using
> > external connection to the underlay network and not be distributed
> > through geneve tunnels.
> > Fix this adding new logical flows to take care of distributed DNAT/SNAT
> >
> > Acked-by: Mark Michelson <mmichels@redhat.com>
> > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
>
> Thanks, applied to master.
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Ben Pfaff April 23, 2019, 4:20 p.m. UTC | #3
It does sounds like this is a case that could be handled better without
going to the gateway.

On Tue, Apr 23, 2019 at 04:07:06PM +0200, Daniel Alvarez Sanchez wrote:
> Even I successfully tested this before getting merged, I just hit some
> scenario where traffic goes to the gateway node. If we have logical
> port lp1 with a dnat_and_snat NAT rule to fip1 and from lp1 you ping
> fip1, the traffic is leaving the hypervisor to the gateway node and
> comes back to lp1.
> 
> I'd expect this traffic to be recirculated within the OVN bridge and
> not being pushed out through the localnet port. It may be a corner
> case but I believe it should be taken into account. What do you folks
> think?
> 
> Cheers,
> Daniel
> 
> On Tue, Apr 16, 2019 at 7:41 PM Ben Pfaff <blp@ovn.org> wrote:
> >
> > On Sat, Apr 06, 2019 at 05:42:52PM +0200, Lorenzo Bianconi wrote:
> > > When DVR is enabled FIP traffic need to be forwarded directly using
> > > external connection to the underlay network and not be distributed
> > > through geneve tunnels.
> > > Fix this adding new logical flows to take care of distributed DNAT/SNAT
> > >
> > > Acked-by: Mark Michelson <mmichels@redhat.com>
> > > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
> >
> > Thanks, applied to master.
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Patch
diff mbox series

diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml
index 392a5efc9..1cec0a69d 100644
--- a/ovn/northd/ovn-northd.8.xml
+++ b/ovn/northd/ovn-northd.8.xml
@@ -1828,6 +1828,37 @@  output;
     </p>
 
     <ul>
+      <li>
+        <p>
+          For distributed logical routers where one of the logical router
+          ports specifies a <code>redirect-chassis</code>, a priority-400
+          logical flow for each ip source/destination couple that matches the
+          <code>dnat_and_snat</code> NAT rules configured. These flows will
+          allow to properly forward traffic to the external connections if
+          available and avoid sending it through the tunnel.
+          Assuming the two following NAT rules have been configured:
+        </p>
+
+        <pre>
+external_ip{0,1} = <var>EIP{0,1}</var>;
+external_mac{0,1} = <var>MAC{0,1}</var>;
+logical_ip{0,1} = <var>LIP{0,1}</var>;
+        </pre>
+
+        <p>
+            the following action will be applied:
+        </p>
+
+        <pre>
+eth.dst = <var>MAC0</var>;
+eth.src = <var>MAC1</var>;
+reg0 = ip4.dst;
+reg1 = <var>EIP1</var>;
+outport = <code>redirect-chassis-port</code>;
+<code>REGBIT_DISTRIBUTED_NAT = 1; next;</code>.
+        </pre>
+      </li>
+
       <li>
         <p>
           For distributed logical routers where one of the logical router
@@ -1924,6 +1955,12 @@  next;
 
     <ul>
       <li>
+        <p>
+          For distributed logical routers where one of the logical router
+          ports specifies a <code>redirect-chassis</code>, a priority-400
+          logical flow with match <code>REGBIT_DISTRIBUTED_NAT == 1</code>
+          has action <code>next;</code>
+        </p>
         <p>
           For distributed logical routers where one of the logical router
           ports specifies a <code>redirect-chassis</code>, a priority-200
@@ -2012,6 +2049,11 @@  next;
     </p>
 
     <ul>
+      <li>
+        A priority-300 logical flow with match
+        <code>REGBIT_DISTRIBUTED_NAT == 1</code> has action
+        <code>next;</code>
+      </li>
       <li>
         A priority-200 logical flow with match
         <code>REGBIT_NAT_REDIRECT == 1</code> has actions
@@ -2321,6 +2363,15 @@  nd_ns {
 
     <ul>
       <li>
+        <p>
+          For each <code>dnat_and_snat</code> NAT rule couple in the
+          OVN Northbound database on a distributed router,
+          a priority-200 logical with match
+          <code>ip4.dst == <var>external_ip0</var> &amp;&amp;
+          ip4.src == <var>external_ip1</var></code>, has action
+          <code>next;</code>
+        </p>
+
         <p>
           For each NAT rule in the OVN Northbound database on a
           distributed router, a priority-100 logical flow with match
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 05b8aad4f..44fb92528 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -179,6 +179,7 @@  enum ovn_stage {
  * logical router dropping packets with source IP address equals
  * one of the logical router's own IP addresses. */
 #define REGBIT_EGRESS_LOOPBACK  "reg9[1]"
+#define REGBIT_DISTRIBUTED_NAT  "reg9[2]"
 
 /* Returns an "enum ovn_stage" built from the arguments. */
 static enum ovn_stage
@@ -4678,6 +4679,66 @@  find_lrp_member_ip(const struct ovn_port *op, const char *ip_s)
     return NULL;
 }
 
+static void
+add_distributed_nat_routes(struct hmap *lflows, const struct ovn_port *op)
+{
+    struct ds actions = DS_EMPTY_INITIALIZER;
+    struct ds match = DS_EMPTY_INITIALIZER;
+
+    if (!op->od->l3dgw_port) {
+        return;
+    }
+
+    if (!op->peer || !op->peer->od->nbs) {
+        return;
+    }
+
+    for (size_t i = 0; i < op->od->nbr->n_nat; i++) {
+        const struct nbrec_nat *nat = op->od->nbr->nat[i];
+        bool found = false;
+
+        if (strcmp(nat->type, "dnat_and_snat") ||
+            !nat->external_mac  || !nat->external_ip) {
+            continue;
+        }
+
+        const struct ovn_datapath *peer_dp = op->peer->od;
+        for (size_t j = 0; j < peer_dp->nbs->n_ports; j++) {
+            if (!strcmp(peer_dp->nbs->ports[j]->name, nat->logical_port)) {
+                found = true;
+                break;
+            }
+        }
+        if (!found) {
+            continue;
+        }
+
+        for (size_t j = 0; j < op->od->nbr->n_nat; j++) {
+            const struct nbrec_nat *nat2 = op->od->nbr->nat[j];
+
+            if (nat == nat2 || strcmp(nat2->type, "dnat_and_snat") ||
+                !nat2->external_mac || !nat2->external_ip)
+                continue;
+
+            ds_put_format(&match, "inport == %s && "
+                          "ip4.src == %s && ip4.dst == %s",
+                          op->json_key, nat->logical_ip, nat2->external_ip);
+            ds_put_format(&actions, "outport = %s; "
+                          "eth.src = %s; eth.dst = %s; "
+                          "reg0 = ip4.dst; reg1 = %s; "
+                          REGBIT_DISTRIBUTED_NAT" = 1; "
+                          REGBIT_NAT_REDIRECT" = 0; next;",
+                          op->od->l3dgw_port->json_key,
+                          op->od->l3dgw_port->lrp_networks.ea_s,
+                          nat2->external_mac, nat->external_ip);
+            ovn_lflow_add(lflows, op->od, S_ROUTER_IN_IP_ROUTING, 400,
+                          ds_cstr(&match), ds_cstr(&actions));
+            ds_clear(&match);
+            ds_clear(&actions);
+        }
+    }
+}
+
 static void
 add_route(struct hmap *lflows, const struct ovn_port *op,
           const char *lrp_addr_s, const char *network_s, int plen,
@@ -6061,6 +6122,41 @@  build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
              * ingress pipeline with inport = outport. */
             if (od->l3dgw_port) {
                 /* Distributed router. */
+                if (!strcmp(nat->type, "dnat_and_snat") &&
+                    nat->external_mac && nat->external_ip) {
+                    for (int j = 0; j < od->nbr->n_nat; j++) {
+                        const struct nbrec_nat *nat2 = od->nbr->nat[j];
+
+                        if (nat2 == nat ||
+                            strcmp(nat2->type, "dnat_and_snat") ||
+                            !nat2->external_mac || !nat2->external_ip) {
+                            continue;
+                        }
+
+                        ds_clear(&match);
+                        ds_put_format(&match, "is_chassis_resident(\"%s\") && "
+                                      "ip4.src == %s && ip4.dst == %s",
+                                      nat->logical_port, nat2->external_ip,
+                                      nat->external_ip);
+                        ds_clear(&actions);
+                        ds_put_format(&actions,
+                                      "inport = outport; outport = \"\"; "
+                                      "flags = 0; flags.loopback = 1; "
+                                      REGBIT_EGRESS_LOOPBACK" = 1; "
+                                      "next(pipeline=ingress, table=0); ");
+                        ovn_lflow_add(lflows, od, S_ROUTER_OUT_EGR_LOOP, 300,
+                                      ds_cstr(&match),  ds_cstr(&actions));
+
+                        ds_clear(&match);
+                        ds_put_format(&match,
+                                      "ip4.src == %s && ip4.dst == %s",
+                                      nat2->external_ip, nat->external_ip);
+                        ovn_lflow_add(lflows, od, S_ROUTER_OUT_EGR_LOOP, 200,
+                                      ds_cstr(&match), "next;");
+                        ds_clear(&match);
+                    }
+                }
+
                 ds_clear(&match);
                 ds_put_format(&match, "ip4.dst == %s && outport == %s",
                               nat->external_ip,
@@ -6132,6 +6228,9 @@  build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
             ovn_lflow_add(lflows, od, S_ROUTER_IN_DNAT, 50,
                           "ip", "flags.loopback = 1; ct_dnat;");
         } else {
+            ovn_lflow_add(lflows, od, S_ROUTER_IN_ARP_RESOLVE, 400,
+                          REGBIT_DISTRIBUTED_NAT" == 1", "next;");
+
             /* For NAT on a distributed router, add flows to Ingress
              * IP Routing table, Ingress ARP Resolution table, and
              * Ingress Gateway Redirect Table that are not specific to a
@@ -6367,6 +6466,9 @@  build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
             continue;
         }
 
+        /* create logical flows for DVR floating IPs */
+        add_distributed_nat_routes(lflows, op);
+
         for (int i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) {
             add_route(lflows, op, op->lrp_networks.ipv4_addrs[i].addr_s,
                       op->lrp_networks.ipv4_addrs[i].network_s,
@@ -6607,6 +6709,9 @@  build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
             continue;
         }
         if (od->l3dgw_port && od->l3redirect_port) {
+            ovn_lflow_add(lflows, od, S_ROUTER_IN_GW_REDIRECT, 300,
+                          REGBIT_DISTRIBUTED_NAT" == 1", "next;");
+
             /* For traffic with outport == l3dgw_port, if the
              * packet did not match any higher priority redirect
              * rule, then the traffic is redirected to the central