Currently, conntrack zone-id is assigned to lport by ovn-controller,
    but the ovn-controller does not record this map (lport->zoneid)

    So, after ovn-controller restart, zone-ids may get set
    inconsistently on lports, resulting in possible hits to
    already established connections.

    The fix is to set the zone-id as an external-id of the interface record
    in the local ovs-db, and recover zone-ids assigned earlier to lports
    from that record.

    Changes in v2:
      - add a check for null br_int in update_local_zone_ids

Reported-by: Russell Bryant <russell.bryant at gmail.com>
Reported-at: https://bugs.launchpad.net/networking-ovn/+bug/1538696
Signed-off-by: Ramu Ramamurthy <ramu.ramamurthy at us.ibm.com>

---
 ovn/controller/binding.c | 71 +++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 64 insertions(+), 7 deletions(-)

diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index ce9cccf..aa3efa4 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -46,10 +46,61 @@ binding_register_ovs_idl(struct ovsdb_idl *ovs_idl)
     ovsdb_idl_add_column(ovs_idl, &ovsrec_interface_col_external_ids);
 }
 
+
 static void
-get_local_iface_ids(const struct ovsrec_bridge *br_int, struct sset *lports)
+update_local_zone_ids(const struct ovsrec_bridge *br_int, struct simap 
*ct_zones,
+                      struct controller_ctx *ctx)
 {
     int i;
+    struct smap new;
+    int zone_id;
+    char *zone;
+
+    if (!ctx->ovs_idl_txn || !br_int) {
+        return;
+    }
+
+    for (i = 0; i < br_int->n_ports; i++) {
+        const struct ovsrec_port *port_rec = br_int->ports[i];
+        const char *iface_id;
+        int j;
+
+        if (!strcmp(port_rec->name, br_int->name)) {
+            continue;
+        }
+
+        for (j = 0; j < port_rec->n_interfaces; j++) {
+            const struct ovsrec_interface *iface_rec;
+
+            iface_rec = port_rec->interfaces[j];
+            iface_id = smap_get(&iface_rec->external_ids, "iface-id");
+
+            if (!iface_id || 
+                smap_get(&iface_rec->external_ids, "zone-id") ||
+                !simap_contains(ct_zones, iface_id)) {
+                continue;
+            }
+
+            zone_id = simap_get(ct_zones, iface_id);
+            zone = xasprintf("%d", zone_id);
+            smap_clone(&new, &iface_rec->external_ids); 
+            smap_replace(&new, "zone-id", zone); 
+            ovsrec_interface_verify_external_ids(iface_rec);
+            ovsrec_interface_set_external_ids(iface_rec, &new);
+            free(zone);
+            smap_destroy(&new);
+        }
+    }
+}
+
+
+static void
+get_local_iface_ids(const struct ovsrec_bridge *br_int, struct sset *lports, 
+                    struct simap *ct_zones, unsigned long *ct_zone_bitmap)
+{
+    int i;
+    const char *zone;
+    int zone_id;
 
     for (i = 0; i < br_int->n_ports; i++) {
         const struct ovsrec_port *port_rec = br_int->ports[i];
@@ -69,13 +120,21 @@ get_local_iface_ids(const struct ovsrec_bridge *br_int, 
struct sset *lports)
                 continue;
             }
             sset_add(lports, iface_id);
+            zone = smap_get(&iface_rec->external_ids, "zone-id");
+            if (zone && ovs_scan(zone, "%d", &zone_id)) {
+                bitmap_set1(ct_zone_bitmap, zone_id);
+                simap_put(ct_zones, iface_id, zone_id);
+            }
         }
     }
 }
 
+
 static void
 update_ct_zones(struct sset *lports, struct simap *ct_zones,
-                unsigned long *ct_zone_bitmap)
+                unsigned long *ct_zone_bitmap, 
+                const struct ovsrec_bridge *br_int,
+                struct controller_ctx *ctx)
 {
     struct simap_node *ct_zone, *ct_zone_next;
     const char *iface_id;
@@ -112,10 +171,8 @@ update_ct_zones(struct sset *lports, struct simap 
*ct_zones,
         bitmap_set1(ct_zone_bitmap, zone);
         simap_put(ct_zones, iface_id, zone);
 
-        /* xxx We should erase any old entries for this
-         * xxx zone, but we need a generic interface to the conntrack
-         * xxx table. */
     }
+    update_local_zone_ids(br_int, ct_zones, ctx);
 }
 
 static void
@@ -154,7 +211,7 @@ binding_run(struct controller_ctx *ctx, const struct 
ovsrec_bridge *br_int,
     sset_init(&lports);
     sset_init(&all_lports);
     if (br_int) {
-        get_local_iface_ids(br_int, &lports);
+        get_local_iface_ids(br_int, &lports, ct_zones, ct_zone_bitmap);
     } else {
         /* We have no integration bridge, therefore no local logical ports.
          * We'll remove our chassis from all port binding records below. */
@@ -203,7 +260,7 @@ binding_run(struct controller_ctx *ctx, const struct 
ovsrec_bridge *br_int,
         VLOG_DBG("No port binding record for lport %s", name);
     }
 
-    update_ct_zones(&all_lports, ct_zones, ct_zone_bitmap);
+    update_ct_zones(&all_lports, ct_zones, ct_zone_bitmap, br_int, ctx);
 
     sset_destroy(&lports);
     sset_destroy(&all_lports);
-- 
1.8.3.1


_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to