[ovs-dev] [PATCH v6 1/3] ovn: Assign zone-id consistently
Ramu Ramamurthy
ramu.ramamurthy at gmail.com
Tue Mar 22 20:03:57 UTC 2016
Currently, ovn-controller does not record the
zoneid assigned to logical port. Tests indicate that
zoneids can be different on the same logical port
after ovn-controller restart.
The fix sets the zone-id as an external-id of the interface record,
and recovers the zone-id from that record.
Reported-by: Russell Bryant <russell at ovn.org>
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 | 157 ++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 141 insertions(+), 16 deletions(-)
diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index d3ca9c9..77d0a2d 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -19,6 +19,7 @@
#include "lib/bitmap.h"
#include "lib/hmap.h"
#include "lib/sset.h"
+#include "lib/type-props.h"
#include "lib/util.h"
#include "lib/vswitch-idl.h"
#include "openvswitch/vlog.h"
@@ -50,7 +51,8 @@ binding_register_ovs_idl(struct ovsdb_idl *ovs_idl)
}
static void
-get_local_iface_ids(const struct ovsrec_bridge *br_int, struct shash *lports)
+get_local_iface_ids(const struct ovsrec_bridge *br_int, struct shash *lports,
+ struct shash *localnet_ports)
{
int i;
@@ -63,10 +65,19 @@ get_local_iface_ids(const struct ovsrec_bridge *br_int, struct shash *lports)
continue;
}
+ const char *localnet = smap_get(&port_rec->external_ids,
+ "ovn-localnet-port");
+
for (j = 0; j < port_rec->n_interfaces; j++) {
const struct ovsrec_interface *iface_rec;
iface_rec = port_rec->interfaces[j];
+
+ if (!strcmp(iface_rec->type, "patch") && localnet) {
+ shash_add(localnet_ports, localnet, iface_rec);
+ break;
+ }
+
iface_id = smap_get(&iface_rec->external_ids, "iface-id");
if (!iface_id) {
continue;
@@ -76,30 +87,127 @@ get_local_iface_ids(const struct ovsrec_bridge *br_int, struct shash *lports)
}
}
+static void get_zones_on_ifaces(struct shash *lports,
+ struct simap *iface_zones,
+ struct shash *parent_ports)
+{
+ const char OVN_ZONE_ID[] = "ovn-zone-id-";
+ const struct ovsrec_interface *iface_rec;
+ struct shash_node *node;
+ struct shash already_added = SHASH_INITIALIZER(&already_added);
+
+ /* get zone-ids set in external-ids of iface records, and
+ * also get parent ports for logical ports
+ * for efficient iteration on child ports, maintain already_added
+ */
+ SHASH_FOR_EACH(node, lports) {
+ iface_rec = node->data;
+ if (!iface_rec || shash_find(&already_added, iface_rec->name)) {
+ continue;
+ }
+ shash_add(&already_added, iface_rec->name, iface_rec);
+ struct smap_node *zone;
+ int zone_id;
+ SMAP_FOR_EACH(zone, &iface_rec->external_ids) {
+ zone_id = smap_get_int(&iface_rec->external_ids, zone->key, 0);
+ if (!strncmp(zone->key, OVN_ZONE_ID, strlen(OVN_ZONE_ID)) &&
+ zone_id) {
+ simap_put(iface_zones, zone->key+strlen(OVN_ZONE_ID), zone_id);
+ shash_add(parent_ports, zone->key+strlen(OVN_ZONE_ID), iface_rec);
+ }
+ }
+ }
+ shash_destroy(&already_added);
+}
+
+static void
+update_zone_on_iface_rec(const struct ovsrec_interface *iface_rec,
+ const char *iface_id, int zone_id, struct controller_ctx *ctx)
+{
+
+ if (!iface_rec || !ctx->ovs_idl_txn) {
+ return;
+ }
+ struct smap new;
+ char *zone_key = xasprintf("ovn-zone-id-%s",iface_id);
+ char zone[INT_STRLEN(int)+1];
+ snprintf(zone, sizeof zone, "%d", zone_id);
+ smap_clone(&new, &iface_rec->external_ids);
+ smap_replace(&new, zone_key, zone);
+ ovsrec_interface_verify_external_ids(iface_rec);
+ ovsrec_interface_set_external_ids(iface_rec, &new);
+ smap_destroy(&new);
+ free(zone_key);
+}
+
+static void
+delete_zone_on_iface_rec(const struct ovsrec_interface *iface_rec,
+ const char *key, struct controller_ctx *ctx)
+{
+ if (!iface_rec || !ctx->ovs_idl_txn) {
+ return;
+ }
+ struct smap new;
+ char *zone_key = xasprintf("ovn-zone-id-%s", key);
+ smap_clone(&new, &iface_rec->external_ids);
+ smap_remove(&new, zone_key);
+ ovsrec_interface_verify_external_ids(iface_rec);
+ ovsrec_interface_set_external_ids(iface_rec, &new);
+ smap_destroy(&new);
+ free(zone_key);
+}
+
static void
-update_ct_zones(struct sset *lports, struct simap *ct_zones,
- unsigned long *ct_zone_bitmap)
+update_ct_zones(struct shash *lports, struct simap *ct_zones,
+ unsigned long *ct_zone_bitmap, struct controller_ctx *ctx)
{
struct simap_node *ct_zone, *ct_zone_next;
- const char *iface_id;
int scan_start = 1;
+ struct shash_node *node;
+ const struct ovsrec_interface *iface_rec;
/* xxx This is wasteful to assign a zone to each port--even if no
* xxx security policy is applied. */
/* Delete any zones that are associated with removed ports. */
SIMAP_FOR_EACH_SAFE(ct_zone, ct_zone_next, ct_zones) {
- if (!sset_contains(lports, ct_zone->name)) {
+ if (!shash_find(lports, ct_zone->name)) {
bitmap_set0(ct_zone_bitmap, ct_zone->data);
simap_delete(ct_zones, ct_zone);
}
}
+ /* collect zones on iface records
+ * Mark those that are used, and delete unmarked at end */
+ struct simap iface_zones = SIMAP_INITIALIZER(&iface_zones);
+ struct shash parent_ports = SHASH_INITIALIZER(&parent_ports);
+
+ get_zones_on_ifaces(lports, &iface_zones, &parent_ports);
/* Assign a unique zone id for each logical port. */
- SSET_FOR_EACH(iface_id, lports) {
- size_t zone;
+ SHASH_FOR_EACH (node, lports) {
+ size_t zone, zone_in_iface;
+ const char *iface_id;
- if (simap_contains(ct_zones, iface_id)) {
+ iface_rec = node->data;
+ iface_id = node->name;
+
+ zone_in_iface=simap_get(&iface_zones, iface_id);
+ simap_find_and_delete(&iface_zones, iface_id);
+ zone = simap_get(ct_zones, iface_id);
+
+ if (zone_in_iface && (zone == zone_in_iface)) {
+ continue;
+ }
+ if (zone_in_iface && (zone != zone_in_iface)) {
+ /* recover already assigned zone ids from interface record */
+ bitmap_set1(ct_zone_bitmap, zone_in_iface);
+ simap_put(ct_zones, iface_id, zone_in_iface);
+ continue;
+ }
+ if (zone && !zone_in_iface) {
+ /* update zone in interface record */
+ zone = simap_get(ct_zones, iface_id);
+ update_zone_on_iface_rec(iface_rec, iface_id, zone, ctx);
continue;
}
@@ -115,10 +223,22 @@ update_ct_zones(struct sset *lports, struct simap *ct_zones,
bitmap_set1(ct_zone_bitmap, zone);
simap_put(ct_zones, iface_id, zone);
+ /* update the zone id in the interface-record */
+ update_zone_on_iface_rec(iface_rec, iface_id, zone, ctx);
+
/* xxx We should erase any old entries for this
* xxx zone, but we need a generic interface to the conntrack
* xxx table. */
}
+ /* remove unused zones on iface records
+ * this is possible on parent interfaces for deleted child ports */
+ struct simap_node *iface_zone;
+ SIMAP_FOR_EACH(iface_zone, &iface_zones) {
+ iface_rec = shash_find_data(&parent_ports, iface_zone->name);
+ delete_zone_on_iface_rec(iface_rec, iface_zone->name, ctx);
+ }
+ simap_destroy(&iface_zones);
+ shash_destroy(&parent_ports);
}
static void
@@ -160,17 +280,19 @@ binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
}
struct shash lports = SHASH_INITIALIZER(&lports);
+ struct shash localnet_ports = SHASH_INITIALIZER(&localnet_ports);
+
if (br_int) {
- get_local_iface_ids(br_int, &lports);
+ get_local_iface_ids(br_int, &lports, &localnet_ports);
} else {
/* We have no integration bridge, therefore no local logical ports.
* We'll remove our chassis from all port binding records below. */
}
- struct sset all_lports = SSET_INITIALIZER(&all_lports);
+ struct shash all_lports = SHASH_INITIALIZER(&all_lports);
struct shash_node *node;
SHASH_FOR_EACH (node, &lports) {
- sset_add(&all_lports, node->name);
+ shash_add(&all_lports, node->name, node->data);
}
/* Run through each binding record to see if it is resident on this
@@ -181,10 +303,11 @@ binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
= shash_find_and_delete(&lports, binding_rec->logical_port);
if (iface_rec
|| (binding_rec->parent_port && binding_rec->parent_port[0] &&
- sset_contains(&all_lports, binding_rec->parent_port))) {
+ shash_find(&all_lports, binding_rec->parent_port))) {
if (binding_rec->parent_port && binding_rec->parent_port[0]) {
/* Add child logical port to the set of all local ports. */
- sset_add(&all_lports, binding_rec->logical_port);
+ shash_add(&all_lports, binding_rec->logical_port,
+ shash_find_data(&all_lports, binding_rec->parent_port));
}
add_local_datapath(local_datapaths, binding_rec);
if (iface_rec && ctx->ovs_idl_txn) {
@@ -217,7 +340,8 @@ binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
* to list them in all_lports because we want to allocate
* a conntrack zone ID for each one, as we'll be creating
* a patch port for each one. */
- sset_add(&all_lports, binding_rec->logical_port);
+ shash_add(&all_lports, binding_rec->logical_port,
+ shash_find_data(&localnet_ports, binding_rec->logical_port));
}
}
@@ -225,10 +349,11 @@ binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
VLOG_DBG("No port binding record for lport %s", node->name);
}
- update_ct_zones(&all_lports, ct_zones, ct_zone_bitmap);
+ update_ct_zones(&all_lports, ct_zones, ct_zone_bitmap, ctx);
shash_destroy(&lports);
- sset_destroy(&all_lports);
+ shash_destroy(&all_lports);
+ shash_destroy(&localnet_ports);
}
/* Returns true if the database is all cleaned up, false if more work is
--
2.3.9
More information about the dev
mailing list