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 <russ...@ovn.org> Reported-at: https://bugs.launchpad.net/networking-ovn/+bug/1538696 Signed-off-by: Ramu Ramamurthy <ramu.ramamur...@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 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev