ovn: Gratuitous ARP for centralized NAT rules on a distributed router

This patch extends gratuitous ARP support for NAT addresses so that it
applies to centralized NAT rules on a distributed router, in addition to
the existing gratuitous ARP support for NAT addresses on gateway routers.
Centralized NAT rules have type other than "dnat_and_snat", or have type
"dnat_and_snat" but do not specify external_mac or logical_port.  These
NAT rules apply on the redirect-chassis.

Gratuitous ARP packets for centralized NAT rules on a distributed router
are only generated on the redirect-chassis.  This is achieved by extending
the syntax for "options:nat-addresses" in the southbound database,
allowing the condition 'is_chassis_resident("LPORT_NAME")' to be appended
after the MAC and IP addresses.  This condition is automatically inserted
by ovn-northd when the northbound "options:nat-addresses" is set to
"router" and the peer is a distributed gateway port.

A separate patch will be required to support gratuitous ARP for
distributed NAT rules that specify logical_port and external_mac.  Since
the MAC address differs and the logical port often resides on a different
chassis from the redirect-chassis, these addresses cannot be included in
the same "nat-addresses" string as for centralized NAT rules.

Signed-off-by: Mickey Spiegel <[email protected]>
Signed-off-by: Gurucharan Shetty <[email protected]>

ovn/controller/pinctrl.c

@@ -37,6 +37,7 @@
 #include "lib/dhcp.h"
 #include "ovn-controller.h"
 #include "ovn/actions.h"
+#include "ovn/lex.h"
 #include "ovn/lib/logical-fields.h"
 #include "ovn/lib/ovn-dhcp.h"
 #include "ovn/lib/ovn-util.h"
@@ -1048,8 +1049,12 @@ send_garp_update(const struct sbrec_port_binding *binding_rec,
                                              ld->localnet_port->logical_port));
 
     volatile struct garp_data *garp = NULL;
-    /* Update GARP for NAT IP if it exists. */
-    if (!strcmp(binding_rec->type, "l3gateway")) {
+    /* Update GARP for NAT IP if it exists.  Consider port bindings with type
+     * "l3gateway" for logical switch ports attached to gateway routers, and
+     * port bindings with type "patch" for logical switch ports attached to
+     * distributed gateway ports. */
+    if (!strcmp(binding_rec->type, "l3gateway")
+        || !strcmp(binding_rec->type, "patch")) {
         struct lport_addresses *laddrs = NULL;
         laddrs = shash_find_data(nat_addresses, binding_rec->logical_port);
         if (!laddrs) {
@@ -1173,6 +1178,7 @@ get_localnet_vifs_l3gwports(const struct ovsrec_bridge *br_int,
             if (!iface_rec->n_ofport) {
                 continue;
             }
+            /* Get localnet port with its ofport. */
             if (localnet) {
                 int64_t ofport = iface_rec->ofport[0];
                 if (ofport < 1 || ofport > ofp_to_u16(OFPP_MAX)) {
@@ -1181,6 +1187,7 @@ get_localnet_vifs_l3gwports(const struct ovsrec_bridge *br_int,
                 simap_put(localnet_ofports, localnet, ofport);
                 continue;
             }
+            /* Get localnet vif. */
             const char *iface_id = smap_get(&iface_rec->external_ids,
                                             "iface-id");
             if (!iface_id) {
@@ -1202,24 +1209,105 @@ get_localnet_vifs_l3gwports(const struct ovsrec_bridge *br_int,
 
     const struct local_datapath *ld;
     HMAP_FOR_EACH (ld, hmap_node, local_datapaths) {
-        if (!ld->has_local_l3gateway) {
+        if (!ld->localnet_port) {
             continue;
         }
 
+        /* Get l3gw ports.  Consider port bindings with type "l3gateway"
+         * that connect to gateway routers (if local), and consider port
+         * bindings of type "patch" since they might connect to
+         * distributed gateway ports with NAT addresses. */
         for (size_t i = 0; i < ld->ldatapath->n_lports; i++) {
             const struct sbrec_port_binding *pb = ld->ldatapath->lports[i];
-            if (!strcmp(pb->type, "l3gateway")
-                /* && it's on this chassis */) {
+            if ((ld->has_local_l3gateway && !strcmp(pb->type, "l3gateway"))
+                || !strcmp(pb->type, "patch")) {
                 sset_add(local_l3gw_ports, pb->logical_port);
             }
         }
     }
 }
 
+static bool
+pinctrl_is_chassis_resident(const struct lport_index *lports,
+                            const struct sbrec_chassis *chassis,
+                            const char *port_name)
+{
+    const struct sbrec_port_binding *pb
+        = lport_lookup_by_name(lports, port_name);
+    return pb && pb->chassis && pb->chassis == chassis;
+}
+
+/* Extracts the mac, IPv4 and IPv6 addresses, and logical port from
+ * 'addresses' which should be of the format 'MAC [IP1 IP2 ..]
+ * [is_chassis_resident("LPORT_NAME")]', where IPn should be a valid IPv4
+ * or IPv6 address, and stores them in the 'ipv4_addrs' and 'ipv6_addrs'
+ * fields of 'laddrs'.  The logical port name is stored in 'lport'.
+ *
+ * Returns true if at least 'MAC' is found in 'address', false otherwise.
+ *
+ * The caller must call destroy_lport_addresses() and free(*lport). */
+static bool
+extract_addresses_with_port(const char *addresses,
+                            struct lport_addresses *laddrs,
+                            char **lport)
+{
+    int ofs;
+    if (!extract_addresses(addresses, laddrs, &ofs)) {
+        return false;
+    } else if (ofs >= strlen(addresses)) {
+        return true;
+    }
+
+    struct lexer lexer;
+    lexer_init(&lexer, addresses + ofs);
+    lexer_get(&lexer);
+
+    if (lexer.error || lexer.token.type != LEX_T_ID
+        || !lexer_match_id(&lexer, "is_chassis_resident")) {
+        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
+        VLOG_INFO_RL(&rl, "invalid syntax '%s' in address", addresses);
+        lexer_destroy(&lexer);
+        return true;
+    }
+
+    if (!lexer_match(&lexer, LEX_T_LPAREN)) {
+        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
+        VLOG_INFO_RL(&rl, "Syntax error: expecting '(' after "
+                          "'is_chassis_resident' in address '%s'", addresses);
+        lexer_destroy(&lexer);
+        return false;
+    }
+
+    if (lexer.token.type != LEX_T_STRING) {
+        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
+        VLOG_INFO_RL(&rl,
+                    "Syntax error: expecting quoted string after"
+                    " 'is_chassis_resident' in address '%s'", addresses);
+        lexer_destroy(&lexer);
+        return false;
+    }
+
+    *lport = xstrdup(lexer.token.s);
+
+    lexer_get(&lexer);
+    if (!lexer_match(&lexer, LEX_T_RPAREN)) {
+        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
+        VLOG_INFO_RL(&rl, "Syntax error: expecting ')' after quoted string in "
+                          "'is_chassis_resident()' in address '%s'",
+                          addresses);
+        lexer_destroy(&lexer);
+        return false;
+    }
+
+    lexer_destroy(&lexer);
+    return true;
+}
+
 static void
 get_nat_addresses_and_keys(struct sset *nat_address_keys,
                            struct sset *local_l3gw_ports,
                            const struct lport_index *lports,
+                           const struct sbrec_chassis *chassis,
                            struct shash *nat_addresses)
 {
     const char *gw_port;
@@ -1236,10 +1324,23 @@ get_nat_addresses_and_keys(struct sset *nat_address_keys,
         }
 
         struct lport_addresses *laddrs = xmalloc(sizeof *laddrs);
-        if (!extract_lsp_addresses(nat_addresses_options, laddrs)) {
+        char *lport = NULL;
+        if (!extract_addresses_with_port(nat_addresses_options, laddrs, &lport)
+            || (!lport && !strcmp(pb->type, "patch"))) {
             free(laddrs);
+            if (lport) {
+                free(lport);
+            }
             continue;
+        } else if (lport) {
+            if (!pinctrl_is_chassis_resident(lports, chassis, lport)) {
+                free(laddrs);
+                free(lport);
+                continue;
+            }
+            free(lport);
         }
+
         int i;
         for (i = 0; i < laddrs->n_ipv4_addrs; i++) {
             char *name = xasprintf("%s-%s", pb->logical_port,
@@ -1275,7 +1376,7 @@ send_garp_run(const struct ovsrec_bridge *br_int,
                       &localnet_vifs, &localnet_ofports, &local_l3gw_ports);
 
     get_nat_addresses_and_keys(&nat_ip_keys, &local_l3gw_ports, lports,
-                               &nat_addresses);
+                               chassis, &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) {

ovn/lib/ovn-util.c

@@ -83,24 +83,29 @@ is_dynamic_lsp_address(const char *address)
 }
 
 /* Extracts the mac, IPv4 and IPv6 addresses from * 'address' which
- * should be of the format 'MAC [IP1 IP2 ..]" where IPn should be a
+ * should be of the format "MAC [IP1 IP2 ..] .." where IPn should be a
  * valid IPv4 or IPv6 address and stores them in the 'ipv4_addrs' and
- * 'ipv6_addrs' fields of 'laddrs'.
+ * 'ipv6_addrs' fields of 'laddrs'.  There may be additional content in
+ * 'address' after "MAC [IP1 IP2 .. ]".  The value of 'ofs' that is
+ * returned indicates the offset where that additional content begins.
  *
- * Return true if at least 'MAC' is found in 'address', false otherwise.
+ * Returns true if at least 'MAC' is found in 'address', false otherwise.
  *
  * The caller must call destroy_lport_addresses(). */
 bool
-extract_lsp_addresses(const char *address, struct lport_addresses *laddrs)
+extract_addresses(const char *address, struct lport_addresses *laddrs,
+                  int *ofs)
 {
     memset(laddrs, 0, sizeof *laddrs);
 
     const char *buf = address;
+    const char *const start = buf;
     int buf_index = 0;
     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;
+        *ofs = 0;
         return false;
     }
 
@@ -129,17 +134,38 @@ extract_lsp_addresses(const char *address, struct lport_addresses *laddrs)
         if (!error) {
             add_ipv6_netaddr(laddrs, ip6, plen);
         } else {
-            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
-            VLOG_INFO_RL(&rl, "invalid syntax '%s' in address", address);
             free(error);
             break;
         }
         buf += buf_index;
     }
 
+    *ofs = buf - start;
     return true;
 }
 
+/* Extracts the mac, IPv4 and IPv6 addresses from * 'address' which
+ * should be of the format 'MAC [IP1 IP2 ..]" where IPn should be a
+ * valid IPv4 or IPv6 address and stores them in the 'ipv4_addrs' and
+ * 'ipv6_addrs' fields of 'laddrs'.
+ *
+ * Return true if at least 'MAC' is found in 'address', false otherwise.
+ *
+ * The caller must call destroy_lport_addresses(). */
+bool
+extract_lsp_addresses(const char *address, struct lport_addresses *laddrs)
+{
+    int ofs;
+    bool success = extract_addresses(address, laddrs, &ofs);
+
+    if (success && ofs < strlen(address)) {
+        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
+        VLOG_INFO_RL(&rl, "invalid syntax '%s' in address", address);
+    }
+
+    return success;
+}
+
 /* Extracts the mac, IPv4 and IPv6 addresses from the
  * "nbrec_logical_router_port" parameter 'lrp'.  Stores the IPv4 and
  * IPv6 addresses in the 'ipv4_addrs' and 'ipv6_addrs' fields of

ovn/lib/ovn-util.h

@@ -54,6 +54,8 @@ struct lport_addresses {
 };
 
 bool is_dynamic_lsp_address(const char *address);
+bool extract_addresses(const char *address, struct lport_addresses *,
+                       int *ofs);
 bool extract_lsp_addresses(const char *address, struct lport_addresses *);
 bool extract_lrp_networks(const struct nbrec_logical_router_port *,
                           struct lport_addresses *);

ovn/northd/ovn-northd.c

@@ -1384,6 +1384,15 @@ join_logical_ports(struct northd_context *ctx,
                                      "on L3 gateway router", nbrp->name);
                         continue;
                     }
+                    if (od->l3dgw_port || od->l3redirect_port) {
+                        static struct vlog_rate_limit rl
+                            = VLOG_RATE_LIMIT_INIT(1, 1);
+                        VLOG_WARN_RL(&rl, "Bad configuration: multiple ports "
+                                     "with redirect-chassis on same logical "
+                                     "router %s", od->nbr->name);
+                        continue;
+                    }
+
                     char *redirect_name = chassis_redirect_name(nbrp->name);
                     struct ovn_port *crp = ovn_port_find(ports, redirect_name);
                     if (crp) {
@@ -1402,17 +1411,8 @@ join_logical_ports(struct northd_context *ctx,
 
                     /* Set l3dgw_port and l3redirect_port in od, for later
                      * use during flow creation. */
-                    if (od->l3dgw_port || od->l3redirect_port) {
-                        static struct vlog_rate_limit rl
-                            = VLOG_RATE_LIMIT_INIT(1, 1);
-                        VLOG_WARN_RL(&rl, "Bad configuration: multiple ports "
-                                     "with redirect-chassis on same logical "
-                                     "router %s", od->nbr->name);
-                        continue;
-                    } else {
-                        od->l3dgw_port = op;
-                        od->l3redirect_port = crp;
-                    }
+                    od->l3dgw_port = op;
+                    od->l3redirect_port = crp;
                 }
             }
         }
@@ -1536,7 +1536,21 @@ get_nat_addresses(const struct ovn_port *op)
             free(error);
             continue;
         }
-        ds_put_format(&addresses, " %s", nat->external_ip);
+
+        /* Determine whether this NAT rule satisfies the conditions for
+         * distributed NAT processing. */
+        if (op->od->l3redirect_port && !strcmp(nat->type, "dnat_and_snat")
+            && nat->logical_port && nat->external_mac) {
+            /* Distributed NAT rule. */
+            /* XXX This uses a different MAC address, so it cannot go
+             * into the same string as centralized NAT external IP
+             * addresses.  Need to change this function to return an
+             * array of strings. */
+        } else {
+            /* Centralized NAT rule, either on gateway router or distributed
+             * router. */
+            ds_put_format(&addresses, " %s", nat->external_ip);
+        }
     }
 
     /* A set to hold all load-balancer vips. */
@@ -1549,6 +1563,13 @@ get_nat_addresses(const struct ovn_port *op)
     }
     sset_destroy(&all_ips);
 
+    /* Gratuitous ARP for centralized NAT rules on distributed gateway
+     * ports should be restricted to the "redirect-chassis". */
+    if (op->od->l3redirect_port) {
+        ds_put_format(&addresses, " is_chassis_resident(%s)",
+                      op->od->l3redirect_port->json_key);
+    }
+
     return ds_steal_cstr(&addresses);
 }
 
@@ -1642,14 +1663,17 @@ ovn_port_update_sbrec(const struct ovn_port *op,
             const char *nat_addresses = smap_get(&op->nbsp->options,
                                            "nat-addresses");
             if (nat_addresses && !strcmp(nat_addresses, "router")) {
-                if (op->peer && op->peer->nbrp) {
+                if (op->peer && op->peer->od
+                    && (chassis || op->peer->od->l3redirect_port)) {
                     char *nats = get_nat_addresses(op->peer);
                     if (nats) {
                         smap_add(&new, "nat-addresses", nats);
                         free(nats);
                     }
                 }
-            } else if (nat_addresses) {
+            /* Only accept manual specification of ethernet address
+             * followed by IPv4 addresses on type "l3gateway" ports. */
+            } else if (nat_addresses && chassis) {
                 struct lport_addresses laddrs;
                 if (!extract_lsp_addresses(nat_addresses, &laddrs)) {
                     static struct vlog_rate_limit rl =