ovn: Fix localnet ports on the same chassis.

Multiple logical ports on the same chassis that were connected to the
same physical network via localnet ports were not able to send packets
to each other.  This was because ovn-controller created a single patch
port between br-int and the physical network access bridge and used it
for all localnet ports.

The fix implemented here is to create a separate patch port for every
logical port of type=localnet.  An optimization is included where these
ports are only created if the localnet port is on a logical switch with
another logical port with an associated local VIF.

A nice side effect of this fix is that the code in physical.c got a lot
simpler, as localnet ports are now handled mostly like local VIFs.

Fixes: c028192 ("ovn: Add "localnet" logical port type.")
Reported-by: Han Zhou <[email protected]>
Reported-at: http://openvswitch.org/pipermail/dev/2016-January/064413.html
Signed-off-by: Russell Bryant <[email protected]>
Tested-by: Kyle Mestery <[email protected]
Acked-By: Kyle Mestery <[email protected]>
Tested-by: Han Zhou <[email protected]>
Tested-by: Michael Arnaldi <[email protected]>
Acked-by: Ben Pfaff <[email protected]>

ovn/controller/ovn-controller.8.xml

@@ -167,9 +167,10 @@
           The presence of this key identifies a patch port as one created by
           <code>ovn-controller</code> to connect the integration bridge and
           another bridge to implement a <code>localnet</code> logical port.
-          Its value is the name of the physical network that the port
-          implements.  See <code>external_ids:ovn-bridge-mappings</code>,
-          above, for more information.
+          Its value is the name of the logical port with type=localnet that
+          the port implements.
+          See <code>external_ids:ovn-bridge-mappings</code>, above,
+          for more information.
         </p>
 
         <p>

ovn/controller/ovn-controller.c

@@ -285,11 +285,6 @@ main(int argc, char *argv[])
         const struct ovsrec_bridge *br_int = get_br_int(&ctx);
         const char *chassis_id = get_chassis_id(ctx.ovs_idl);
 
-        /* Map bridges to local nets from ovn-bridge-mappings */
-        if (br_int) {
-            patch_run(&ctx, br_int);
-        }
-
         if (chassis_id) {
             chassis_run(&ctx, chassis_id);
             encaps_run(&ctx, br_int, chassis_id);
@@ -298,6 +293,8 @@ main(int argc, char *argv[])
         }
 
         if (br_int) {
+            patch_run(&ctx, br_int, &local_datapaths);
+
             enum mf_field_id mff_ovn_geneve = ofctrl_run(br_int);
 
             pinctrl_run(&ctx, br_int);
@@ -306,8 +303,7 @@ main(int argc, char *argv[])
             lflow_run(&ctx, &flow_table, &ct_zones);
             if (chassis_id) {
                 physical_run(&ctx, mff_ovn_geneve,
-                             br_int, chassis_id, &ct_zones, &flow_table,
-                             &local_datapaths);
+                             br_int, chassis_id, &ct_zones, &flow_table);
             }
             ofctrl_put(&flow_table);
             hmap_destroy(&flow_table);

ovn/controller/patch.c

@@ -18,6 +18,7 @@
 #include "patch.h"
 
 #include "hash.h"
+#include "lib/hmap.h"
 #include "lib/vswitch-idl.h"
 #include "openvswitch/vlog.h"
 #include "ovn-controller.h"
@@ -95,27 +96,6 @@ create_patch_port(struct controller_ctx *ctx,
     free(ports);
 }
 
-/* Creates a pair of patch ports that connect bridges 'b1' and 'b2', using a
- * port named 'name1' and 'name2' in each respective bridge.
- * external-ids:'key' in each port is initialized to 'value'.
- *
- * If one or both of the ports already exists, leaves it there and removes it
- * from 'existing_ports'. */
-static void
-create_patch_ports(struct controller_ctx *ctx,
-                   const char *key, const char *value,
-                   const struct ovsrec_bridge *b1,
-                   const struct ovsrec_bridge *b2,
-                   struct shash *existing_ports)
-{
-    char *name1 = patch_port_name(b1->name, b2->name);
-    char *name2 = patch_port_name(b2->name, b1->name);
-    create_patch_port(ctx, key, value, b1, name1, b2, name2, existing_ports);
-    create_patch_port(ctx, key, value, b2, name2, b1, name1, existing_ports);
-    free(name2);
-    free(name1);
-}
-
 static void
 remove_port(struct controller_ctx *ctx,
             const struct ovsrec_port *port)
@@ -153,7 +133,8 @@ remove_port(struct controller_ctx *ctx,
 static void
 add_bridge_mappings(struct controller_ctx *ctx,
                     const struct ovsrec_bridge *br_int,
-                    struct shash *existing_ports)
+                    struct shash *existing_ports,
+                    struct hmap *local_datapaths)
 {
     /* Get ovn-bridge-mappings. */
     const char *mappings_cfg = "";
@@ -166,7 +147,8 @@ add_bridge_mappings(struct controller_ctx *ctx,
         }
     }
 
-    /* Create patch ports. */
+    /* Parse bridge mappings. */
+    struct shash bridge_mappings = SHASH_INITIALIZER(&bridge_mappings);
     char *cur, *next, *start;
     next = start = xstrdup(mappings_cfg);
     while ((cur = strsep(&next, ",")) && *cur) {
@@ -187,10 +169,53 @@ add_bridge_mappings(struct controller_ctx *ctx,
             continue;
         }
 
-        create_patch_ports(ctx, "ovn-localnet-port", network,
-                           br_int, ovs_bridge, existing_ports);
+        shash_add(&bridge_mappings, network, ovs_bridge);
     }
     free(start);
+
+    const struct sbrec_port_binding *binding;
+    SBREC_PORT_BINDING_FOR_EACH (binding, ctx->ovnsb_idl) {
+        if (strcmp(binding->type, "localnet")) {
+            /* Not a binding for a localnet port. */
+            continue;
+        }
+
+        struct hmap_node *ld;
+        ld = hmap_first_with_hash(local_datapaths,
+                                  binding->datapath->tunnel_key);
+        if (!ld) {
+            /* This localnet port is on a datapath with no
+             * logical ports bound to this chassis, so there's no need
+             * to create patch ports for it. */
+            continue;
+        }
+
+        const char *network = smap_get(&binding->options, "network_name");
+        if (!network) {
+            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
+            VLOG_ERR_RL(&rl, "localnet port '%s' has no network name.",
+                         binding->logical_port);
+            continue;
+        }
+        struct ovsrec_bridge *br_ln = shash_find_data(&bridge_mappings, network);
+        if (!br_ln) {
+            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
+            VLOG_ERR_RL(&rl, "bridge not found for localnet port '%s' "
+                    "with network name '%s'", binding->logical_port, network);
+            continue;
+        }
+
+        char *name1 = patch_port_name(br_int->name, binding->logical_port);
+        char *name2 = patch_port_name(binding->logical_port, br_int->name);
+        create_patch_port(ctx, "ovn-localnet-port", binding->logical_port,
+                          br_int, name1, br_ln, name2, existing_ports);
+        create_patch_port(ctx, "ovn-localnet-port", binding->logical_port,
+                          br_ln, name2, br_int, name1, existing_ports);
+        free(name1);
+        free(name2);
+    }
+
+    shash_destroy(&bridge_mappings);
 }
 
 /* Add one OVS patch port for each OVN logical patch port.
@@ -241,7 +266,8 @@ add_logical_patch_ports(struct controller_ctx *ctx,
 }
 
 void
-patch_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int)
+patch_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
+          struct hmap *local_datapaths)
 {
     if (!ctx->ovs_idl_txn) {
         return;
@@ -260,7 +286,7 @@ 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);
+    add_bridge_mappings(ctx, br_int, &existing_ports, local_datapaths);
     add_logical_patch_ports(ctx, br_int, &existing_ports);
 
     /* Now 'existing_ports' only still contains patch ports that exist in the

ovn/controller/patch.h

@@ -23,8 +23,10 @@
  * physical bridges, as directed by other-config:ovn-bridge-mappings. */
 
 struct controller_ctx;
+struct hmap;
 struct ovsrec_bridge;
 
-void patch_run(struct controller_ctx *, const struct ovsrec_bridge *br_int);
+void patch_run(struct controller_ctx *, const struct ovsrec_bridge *br_int,
+               struct hmap *local_datapaths);
 
 #endif /* ovn/patch.h */