ovn: Support for GARP for NAT IPs via localnet

In cases where a DNAT IP is moved to a new router or the SNAT IP is reused
with a new mac address, the NAT IPs become unreachable because the external
switches/routers have stale ARP entries. This commit
aims to fix the problem by sending GARPs for NAT IPs via locanet. There are
two parts to this patch.

[1] Adding the datapath of the l3 gateway port to local datapaths in
ovn-controller. This will result in creation of patch ports between
br-int and the physical bridge (that provides connectivity to local network
via localnet port) and will enable gateway router to have external
connectivity

[2] A new options key "nat-addresses" is added to the logical switch port of
type router, the logical switch that has this port is the one that provides
connectivity to local network via localnet port. The value for the key
"nat-addresses" is the MAC address of the port followed by a list of
SNAT & DNAT IPs. When ovn-controller sees a new IP in nat-addrress option,
it sends a GARP message for the IP via the localnet port. nat-addresses
option is added to the logical switch port of type router and not to the
logical router port, because the logical switch datapath has the localnet
port. Adding nat-addresses option to the router port will involve more
changes to get to the local net port.

Signed-off-by: Chandra Sekhar Vejendla <csvejend@us.ibm.com>
Acked-by: Ryan Moats <rmoats@us.ibm.com>
Signed-off-by: Gurucharan Shetty <guru@ovn.org>

ovn/controller/binding.c

@@ -232,8 +232,13 @@ consider_local_datapath(struct controller_ctx *ctx,
             sset_add(all_lports, binding_rec->logical_port);
             add_local_datapath(local_datapaths, binding_rec);
         }
-    } else if (chassis_rec && binding_rec->chassis == chassis_rec
-               && strcmp(binding_rec->type, "l3gateway")) {
+    } else if (!strcmp(binding_rec->type, "l3gateway")) {
+        const char *chassis = smap_get(&binding_rec->options,
+                                       "l3gateway-chassis");
+        if (!strcmp(chassis, chassis_rec->name) && ctx->ovnsb_idl_txn) {
+            add_local_datapath(local_datapaths, binding_rec);
+        }
+    } else if (chassis_rec && binding_rec->chassis == chassis_rec) {
         if (ctx->ovnsb_idl_txn) {
             VLOG_INFO("Releasing lport %s from this chassis.",
                       binding_rec->logical_port);

ovn/controller/ovn-controller.8.xml

@@ -232,7 +232,7 @@
           <code>ovn-controller</code> to connect the integration bridge and
           another bridge to implement a <code>l2gateway</code> logical port.
           Its value is the name of the logical port with <code>type</code>
-          set to <code>l3gateway</code> that the port implements. See
+          set to <code>l2gateway</code> that the port implements. See
           <code>external_ids:ovn-bridge-mappings</code>, above, for more
           information.
         </p>
@@ -246,7 +246,23 @@
       </dd>
 
       <dt>
-        <code>external_ids:ovn-logical-patch-port</code> in the
+        <code>external-ids:ovn-l3gateway-port</code> in the <code>Port</code>
+        table
+      </dt>
+
+      <dd>
+        <p>
+          This key identifies a patch port as one created by
+          <code>ovn-controller</code> to implement a <code>l3gateway</code>
+          logical port. Its value is the name of the logical port with type
+          set to <code>l3gateway</code>. This patch port is similar to
+          the OVN logical patch port, except that <code>l3gateway</code>
+          port can only be bound to a paticular chassis.
+        </p>
+      </dd>
+
+      <dt>
+        <code>external-ids:ovn-logical-patch-port</code> in the
         <code>Port</code> table
       </dt>
 

ovn/controller/patch.c

@@ -345,12 +345,14 @@ add_logical_patch_ports(struct controller_ctx *ctx,
 
     const struct sbrec_port_binding *binding;
     SBREC_PORT_BINDING_FOR_EACH (binding, ctx->ovnsb_idl) {
+        const char *patch_port_id = "ovn-logical-patch-port";
         bool local_port = false;
         if (!strcmp(binding->type, "l3gateway")) {
             const char *chassis = smap_get(&binding->options,
                                            "l3gateway-chassis");
             if (chassis && !strcmp(local_chassis_id, chassis)) {
                 local_port = true;
+                patch_port_id = "ovn-l3gateway-port";
             }
         }
 
@@ -363,7 +365,7 @@ add_logical_patch_ports(struct controller_ctx *ctx,
 
             char *src_name = patch_port_name(local, peer);
             char *dst_name = patch_port_name(peer, local);
-            create_patch_port(ctx, "ovn-logical-patch-port", local,
+            create_patch_port(ctx, patch_port_id, local,
                               br_int, src_name, br_int, dst_name,
                               existing_ports);
             free(dst_name);
@@ -394,6 +396,7 @@ patch_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
     OVSREC_PORT_FOR_EACH (port, ctx->ovs_idl) {
         if (smap_get(&port->external_ids, "ovn-localnet-port")
             || smap_get(&port->external_ids, "ovn-l2gateway-port")
+            || smap_get(&port->external_ids, "ovn-l3gateway-port")
             || smap_get(&port->external_ids, "ovn-logical-patch-port")) {
             shash_add(&existing_ports, port->name, port);
         }
@@ -402,7 +405,8 @@ patch_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
     /* Create in the database any patch ports that should exist.  Remove from
      * 'existing_ports' any patch ports that do exist in the database and
      * should be there. */
-    add_bridge_mappings(ctx, br_int, &existing_ports, local_datapaths, chassis_id);
+    add_bridge_mappings(ctx, br_int, &existing_ports, local_datapaths,
+                        chassis_id);
     add_logical_patch_ports(ctx, br_int, chassis_id, &existing_ports,
                             patched_datapaths);
 

ovn/controller/physical.c

@@ -709,6 +709,8 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve,
                                         "ovn-localnet-port");
         const char *l2gateway = smap_get(&port_rec->external_ids,
                                         "ovn-l2gateway-port");
+        const char *l3gateway = smap_get(&port_rec->external_ids,
+                                        "ovn-l3gateway-port");
         const char *logpatch = smap_get(&port_rec->external_ids,
                                         "ovn-logical-patch-port");
 
@@ -735,6 +737,10 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve,
                 /* L2 gateway patch ports can be handled just like VIFs. */
                 simap_put(&new_localvif_to_ofport, l2gateway, ofport);
                 break;
+            } else if (is_patch && l3gateway) {
+                /* L3 gateway patch ports can be handled just like VIFs. */
+                simap_put(&new_localvif_to_ofport, l3gateway, ofport);
+                break;
             } else if (is_patch && logpatch) {
                 /* Logical patch ports can be handled just like VIFs. */
                 simap_put(&new_localvif_to_ofport, logpatch, ofport);

ovn/controller/pinctrl.c

@@ -1010,10 +1010,24 @@ destroy_send_garps(void)
     shash_destroy_free_data(&send_garp_data);
 }
 
+static void
+add_garp(const char *name, ofp_port_t ofport,
+         const struct eth_addr ea, ovs_be32 ip)
+{
+    struct garp_data *garp = xmalloc(sizeof *garp);
+    garp->ea = ea;
+    garp->ipv4 = ip;
+    garp->announce_time = time_msec() + 1000;
+    garp->backoff = 1;
+    garp->ofport = ofport;
+    shash_add(&send_garp_data, name, garp);
+}
+
 /* Add or update a vif for which GARPs need to be announced. */
 static void
 send_garp_update(const struct sbrec_port_binding *binding_rec,
-                 struct simap *localnet_ofports, struct hmap *local_datapaths)
+                 struct simap *localnet_ofports, struct hmap *local_datapaths,
+                 struct shash *nat_addresses)
 {
     /* Find the localnet ofport to send this GARP. */
     struct local_datapath *ld
@@ -1025,9 +1039,32 @@ send_garp_update(const struct sbrec_port_binding *binding_rec,
     ofp_port_t ofport = u16_to_ofp(simap_get(localnet_ofports,
                                              ld->localnet_port->logical_port));
 
-    /* Update GARP if it exists. */
-    struct garp_data *garp = shash_find_data(&send_garp_data,
-                                             binding_rec->logical_port);
+    volatile struct garp_data *garp = NULL;
+    /* Update GARP for NAT IP if it exists. */
+    if (!strcmp(binding_rec->type, "l3gateway")) {
+        struct lport_addresses *laddrs = NULL;
+        laddrs = shash_find_data(nat_addresses, binding_rec->logical_port);
+        if (!laddrs) {
+            return;
+        }
+        int i;
+        for (i = 0; i < laddrs->n_ipv4_addrs; i++) {
+            char *name = xasprintf("%s-%s", binding_rec->logical_port,
+                                            laddrs->ipv4_addrs[i].addr_s);
+            garp = shash_find_data(&send_garp_data, name);
+            if (garp) {
+                garp->ofport = ofport;
+            } else {
+                add_garp(name, ofport, laddrs->ea, laddrs->ipv4_addrs[i].addr);
+            }
+            free(name);
+        }
+        destroy_lport_addresses(laddrs);
+        return;
+    }
+
+    /* Update GARP for vif if it exists. */
+    garp = shash_find_data(&send_garp_data, binding_rec->logical_port);
     if (garp) {
         garp->ofport = ofport;
         return;
@@ -1042,13 +1079,8 @@ send_garp_update(const struct sbrec_port_binding *binding_rec,
             continue;
         }
 
-        struct garp_data *garp = xmalloc(sizeof *garp);
-        garp->ea = laddrs.ea;
-        garp->ipv4 = laddrs.ipv4_addrs[0].addr;
-        garp->announce_time = time_msec() + 1000;
-        garp->backoff = 1;
-        garp->ofport = ofport;
-        shash_add(&send_garp_data, binding_rec->logical_port, garp);
+        add_garp(binding_rec->logical_port, ofport,
+                 laddrs.ea, laddrs.ipv4_addrs[0].addr);
 
         destroy_lport_addresses(&laddrs);
         break;
@@ -1107,14 +1139,15 @@ send_garp(struct garp_data *garp, long long int current_time)
     return garp->announce_time;
 }
 
-/* Get localnet vifs, and ofport for localnet patch ports. */
+/* Get localnet vifs, local l3gw ports and ofport for localnet patch ports. */
 static void
-get_localnet_vifs(const struct ovsrec_bridge *br_int,
+get_localnet_vifs_l3gwports(const struct ovsrec_bridge *br_int,
                   const char *this_chassis_id,
                   const struct lport_index *lports,
                   struct hmap *local_datapaths,
                   struct sset *localnet_vifs,
-                  struct simap *localnet_ofports)
+                  struct simap *localnet_ofports,
+                  struct sset *local_l3gw_ports)
 {
     for (int i = 0; i < br_int->n_ports; i++) {
         const struct ovsrec_port *port_rec = br_int->ports[i];
@@ -1128,6 +1161,8 @@ get_localnet_vifs(const struct ovsrec_bridge *br_int,
         }
         const char *localnet = smap_get(&port_rec->external_ids,
                                         "ovn-localnet-port");
+        const char *l3_gateway_port = smap_get(&port_rec->external_ids,
+                                               "ovn-l3gateway-port");
         for (int j = 0; j < port_rec->n_interfaces; j++) {
             const struct ovsrec_interface *iface_rec = port_rec->interfaces[j];
             if (!iface_rec->n_ofport) {
@@ -1141,6 +1176,10 @@ get_localnet_vifs(const struct ovsrec_bridge *br_int,
                 simap_put(localnet_ofports, localnet, ofport);
                 continue;
             }
+            if (l3_gateway_port) {
+                sset_add(local_l3gw_ports, l3_gateway_port);
+                continue;
+            }
             const char *iface_id = smap_get(&iface_rec->external_ids,
                                             "iface-id");
             if (!iface_id) {
@@ -1161,6 +1200,41 @@ get_localnet_vifs(const struct ovsrec_bridge *br_int,
     }
 }
 
+static void
+get_nat_addresses_and_keys(struct sset *nat_address_keys,
+                           struct sset *local_l3gw_ports,
+                           const struct lport_index *lports,
+                           struct shash *nat_addresses)
+{
+    const char *gw_port;
+    SSET_FOR_EACH(gw_port, local_l3gw_ports) {
+        const struct sbrec_port_binding *pb = lport_lookup_by_name(lports,
+                                                                   gw_port);
+        if (!pb) {
+            continue;
+        }
+        const char *nat_addresses_options = smap_get(&pb->options,
+                                                     "nat-addresses");
+        if (!nat_addresses_options) {
+            continue;
+        }
+
+        struct lport_addresses *laddrs = xmalloc(sizeof *laddrs);
+        if (!extract_lsp_addresses(nat_addresses_options, laddrs)) {
+            free(laddrs);
+            continue;
+        }
+        int i;
+        for (i = 0; i < laddrs->n_ipv4_addrs; i++) {
+            char *name = xasprintf("%s-%s", pb->logical_port,
+                                            laddrs->ipv4_addrs[i].addr_s);
+            sset_add(nat_address_keys, name);
+            free(name);
+        }
+        shash_add(nat_addresses, pb->logical_port, laddrs);
+    }
+}
+
 static void
 send_garp_wait(void)
 {
@@ -1173,15 +1247,23 @@ send_garp_run(const struct ovsrec_bridge *br_int, const char *chassis_id,
               struct hmap *local_datapaths)
 {
     struct sset localnet_vifs = SSET_INITIALIZER(&localnet_vifs);
+    struct sset local_l3gw_ports = SSET_INITIALIZER(&local_l3gw_ports);
+    struct sset nat_ip_keys = SSET_INITIALIZER(&nat_ip_keys);
     struct simap localnet_ofports = SIMAP_INITIALIZER(&localnet_ofports);
+    struct shash nat_addresses;
 
-    get_localnet_vifs(br_int, chassis_id, lports, local_datapaths,
-                      &localnet_vifs, &localnet_ofports);
+    shash_init(&nat_addresses);
 
-    /* For deleted ports, remove from send_garp_data. */
+    get_localnet_vifs_l3gwports(br_int, chassis_id, lports, local_datapaths,
+                      &localnet_vifs, &localnet_ofports, &local_l3gw_ports);
+
+    get_nat_addresses_and_keys(&nat_ip_keys, &local_l3gw_ports, lports,
+                               &nat_addresses);
+    /* For deleted ports and deleted nat ips, remove from send_garp_data. */
     struct shash_node *iter, *next;
     SHASH_FOR_EACH_SAFE (iter, next, &send_garp_data) {
-        if (!sset_contains(&localnet_vifs, iter->name)) {
+        if (!sset_contains(&localnet_vifs, iter->name) &&
+            !sset_contains(&nat_ip_keys, iter->name)) {
             send_garp_delete(iter->name);
         }
     }
@@ -1192,7 +1274,19 @@ send_garp_run(const struct ovsrec_bridge *br_int, const char *chassis_id,
         const struct sbrec_port_binding *pb = lport_lookup_by_name(lports,
                                                                    iface_id);
         if (pb) {
-            send_garp_update(pb, &localnet_ofports, local_datapaths);
+            send_garp_update(pb, &localnet_ofports, local_datapaths,
+                             &nat_addresses);
+        }
+    }
+
+    /* Update send_garp_data for nat-addresses. */
+    const char *gw_port;
+    SSET_FOR_EACH (gw_port, &local_l3gw_ports) {
+        const struct sbrec_port_binding *pb = lport_lookup_by_name(lports,
+                                                                gw_port);
+        if (pb) {
+            send_garp_update(pb, &localnet_ofports, local_datapaths,
+                             &nat_addresses);
         }
     }
 
@@ -1206,7 +1300,9 @@ send_garp_run(const struct ovsrec_bridge *br_int, const char *chassis_id,
         }
     }
     sset_destroy(&localnet_vifs);
+    sset_destroy(&local_l3gw_ports);
     simap_destroy(&localnet_ofports);
+    shash_destroy_free_data(&nat_addresses);
 }
 
 static void

ovn/lib/ovn-util.c

@@ -71,13 +71,13 @@ add_ipv6_netaddr(struct lport_addresses *laddrs, struct in6_addr addr,
  *
  * The caller must call destroy_lport_addresses(). */
 bool
-extract_lsp_addresses(char *address, struct lport_addresses *laddrs)
+extract_lsp_addresses(const char *address, struct lport_addresses *laddrs)
 {
     memset(laddrs, 0, sizeof *laddrs);
 
-    char *buf = address;
+    const char *buf = address;
     int buf_index = 0;
-    char *buf_end = buf + strlen(address);
+    const char *buf_end = buf + strlen(address);
     if (!ovs_scan_len(buf, &buf_index, ETH_ADDR_SCAN_FMT,
                       ETH_ADDR_SCAN_ARGS(laddrs->ea))) {
         laddrs->ea = eth_addr_zero;

ovn/lib/ovn-util.h

@@ -53,7 +53,7 @@ struct lport_addresses {
 };
 
 
-bool extract_lsp_addresses(char *address, struct lport_addresses *);
+bool extract_lsp_addresses(const char *address, struct lport_addresses *);
 bool extract_lrp_networks(const struct nbrec_logical_router_port *,
                           struct lport_addresses *);
 void destroy_lport_addresses(struct lport_addresses *);

ovn/northd/ovn-northd.c

@@ -1195,6 +1195,20 @@ ovn_port_update_sbrec(const struct ovn_port *op)
             if (chassis) {
                 smap_add(&new, "l3gateway-chassis", chassis);
             }
+
+            const char *nat_addresses = smap_get(&op->nbsp->options,
+                                           "nat-addresses");
+            if (nat_addresses) {
+                struct lport_addresses laddrs;
+                if (!extract_lsp_addresses(nat_addresses, &laddrs)) {
+                    static struct vlog_rate_limit rl =
+                        VLOG_RATE_LIMIT_INIT(1, 1);
+                    VLOG_WARN_RL(&rl, "Error extracting nat-addresses.");
+                } else {
+                    smap_add(&new, "nat-addresses", nat_addresses);
+                    destroy_lport_addresses(&laddrs);
+                }
+            }
             sbrec_port_binding_set_options(op->sb, &new);
             smap_destroy(&new);
         }

ovn/ovn-nb.xml

@@ -225,6 +225,16 @@
           table="Logical_Router_Port"/> to which this logical switch port is
           connected.
         </column>
+
+        <column name="options" key="nat-addresses">
+          MAC address of the <code>router-port</code> followed by a list of
+          SNAT and DNAT IP addresses. This is used to send gratuitous ARPs for
+          SNAT and DNAT IP addresses via <code>localnet</code> and is valid for
+          only L3 gateway ports.  Example: <code>80:fa:5b:06:72:b7 158.36.44.22
+          158.36.44.24</code>. This would result in generation of gratuitous
+          ARPs for IP addresses 158.36.44.22 and 158.36.44.24 with a MAC
+          address of 80:fa:5b:06:72:b7.
+        </column>
       </group>
 
       <group title="Options for localnet ports">