[ovs-dev] [PATCH v3] ovn: Support for GARP for NAT IPs via localnet

Chandra S Vejendla csvejend at us.ibm.com
Fri Jul 29 22:26:29 UTC 2016


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

A new options key "nat-addresses" is added to the logical switch port of
type router. The value for the key "nat-addresses" is the MAC address of the
port followed by a list of SNAT & DNAT IPs.

Signed-off-by: Chandra Sekhar Vejendla <csvejend at us.ibm.com>
---
 ovn/controller/binding.c            |   9 ++-
 ovn/controller/ovn-controller.8.xml |  18 ++++-
 ovn/controller/patch.c              |   8 ++-
 ovn/controller/physical.c           |   6 ++
 ovn/controller/pinctrl.c            | 134 +++++++++++++++++++++++++++++++-----
 ovn/lib/ovn-util.c                  |   6 +-
 ovn/lib/ovn-util.h                  |   2 +-
 ovn/northd/ovn-northd.c             |  14 ++++
 ovn/ovn-nb.xml                      |  10 +++
 ovn/ovn-sb.xml                      |  10 +++
 tests/ovn.at                        |  49 +++++++++++++
 11 files changed, 238 insertions(+), 28 deletions(-)

diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index 3073727..bd73da8 100644
--- a/ovn/controller/binding.c
+++ b/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);
diff --git a/ovn/controller/ovn-controller.8.xml b/ovn/controller/ovn-controller.8.xml
index 3fda8e7..713045c 100644
--- a/ovn/controller/ovn-controller.8.xml
+++ b/ovn/controller/ovn-controller.8.xml
@@ -208,7 +208,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>
@@ -222,6 +222,22 @@
       </dd>
 
       <dt>
+        <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>
diff --git a/ovn/controller/patch.c b/ovn/controller/patch.c
index 012e6ba..3a77ed9 100644
--- a/ovn/controller/patch.c
+++ b/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);
 
diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
index ee1da4f..0827d35 100644
--- a/ovn/controller/physical.c
+++ b/ovn/controller/physical.c
@@ -628,6 +628,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");
 
@@ -654,6 +656,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);
diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index 416dad6..4c425f1 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -709,10 +709,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
@@ -724,9 +738,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;
@@ -741,13 +778,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;
@@ -806,14 +838,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];
@@ -827,6 +860,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) {
@@ -840,6 +875,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) {
@@ -861,6 +900,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)
 {
     poll_timer_wait_until(send_garp_time);
@@ -872,15 +946,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);
         }
     }
@@ -891,7 +973,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);
         }
     }
 
@@ -905,7 +999,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
diff --git a/ovn/lib/ovn-util.c b/ovn/lib/ovn-util.c
index de54624..e4e3f51 100644
--- a/ovn/lib/ovn-util.c
+++ b/ovn/lib/ovn-util.c
@@ -72,13 +72,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;
diff --git a/ovn/lib/ovn-util.h b/ovn/lib/ovn-util.h
index e9f3ec2..7056781 100644
--- a/ovn/lib/ovn-util.h
+++ b/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 *);
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 4d9c9e7..d014002 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -1189,6 +1189,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);
+                }
+            }
             sbrec_port_binding_set_options(op->sb, &new);
             smap_destroy(&new);
         }
diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
index 85aa2d3..6715201 100644
--- a/ovn/ovn-nb.xml
+++ b/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 adddresses. 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">
diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml
index c5f236e..8d46170 100644
--- a/ovn/ovn-sb.xml
+++ b/ovn/ovn-sb.xml
@@ -1645,6 +1645,16 @@ tcp.flags = RST;
       <column name="options" key="l3gateway-chassis">
         The <code>chassis</code> in which the port resides.
       </column>
+
+        <column name="options" key="nat-addresses">
+          MAC address of the <code>l3gateway</code> port followed by a list of
+          SNAT and DNAT IP adddresses. 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="Localnet Options">
diff --git a/tests/ovn.at b/tests/ovn.at
index 051b222..1d2af58 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -4010,3 +4010,52 @@ AT_CHECK([cat received2.packets], [0], [expout])
 OVN_CLEANUP([hv1])
 
 AT_CLEANUP
+
+AT_SETUP([ovn -- send gratuitous arp for nat ips in localnet])
+AT_KEYWORDS([ovn])
+AT_SKIP_IF([test $HAVE_PYTHON = no])
+ovn_start
+# Create logical switch
+ovn-nbctl ls-add ls0
+# Create gateway router
+ovn-nbctl create Logical_Router name=lr0 options:chassis=hv1
+# Add router port to gateway router
+ovn-nbctl lrp-add lr0 lrp0 f0:00:00:00:00:01 192.168.0.1/24
+ovn-nbctl lsp-add ls0 lrp0-rp -- set Logical_Switch_Port lrp0-rp \
+    type=router options:router-port=lrp0-rp addresses='"f0:00:00:00:00:01"'
+# Add nat-address option
+ovn-nbctl lsp-set-options lrp0-rp router-port=lrp0 nat-addresses="f0:00:00:00:00:01 192.168.0.2"
+
+net_add n1
+sim_add hv1
+as hv1
+ovs-vsctl \
+    -- add-br br-phys \
+    -- add-br br-eth0
+
+ovn_attach n1 br-phys 192.168.0.1
+
+AT_CHECK([ovs-vsctl set Open_vSwitch . external-ids:ovn-bridge-mappings=physnet1:br-eth0])
+AT_CHECK([ovs-vsctl add-port br-eth0 snoopvif -- set Interface snoopvif options:tx_pcap=hv1/snoopvif-tx.pcap options:rxq_pcap=hv1/snoopvif-rx.pcap])
+
+# Create a localnet port.
+AT_CHECK([ovn-nbctl lsp-add ls0 ln_port])
+AT_CHECK([ovn-nbctl lsp-set-addresses ln_port unknown])
+AT_CHECK([ovn-nbctl lsp-set-type ln_port localnet])
+AT_CHECK([ovn-nbctl lsp-set-options ln_port network_name=physnet1])
+
+
+# Wait for packet to be received.
+OVS_WAIT_UNTIL([test `wc -c < "hv1/snoopvif-tx.pcap"` -ge 50])
+trim_zeros() {
+    sed 's/\(00\)\{1,\}$//'
+}
+$PYTHON "$top_srcdir/utilities/ovs-pcap.in" hv1/snoopvif-tx.pcap | trim_zeros > packets
+expected="fffffffffffff0000000000108060001080006040001f00000000001c0a80002000000000000c0a80002"
+echo $expected > expout
+AT_CHECK([sort packets], [0], [expout])
+cat packets
+
+OVN_CLEANUP([hv1])
+
+AT_CLEANUP
-- 
2.6.1




More information about the dev mailing list