On Mon, Feb 15, 2016 at 03:46:38PM -0500, Ramu Ramamurthy wrote: > Currently, ovn-controller does not record the > lport->zoneid map, and so, after ovn-controller restart, > zone-ids may get set inconsistently on lports, resulting > in possible hits to already established connections. > > Set zone-id as an external-id of the interface record, > and recover 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> > --- > Changes v3 to v4: update as per code-review > * simplify code to update zone-id only when needed > * update documentation for external id > * update authors > Changes v4 to v5: fix formatting
I'm resending some of these comments; others are new. From this part of the patch, it looks like external-ids:zone-id is accepted even if there are duplicates, or if the value is not valid. I think that it should reject such cases: + zone_id = smap_get_int(&iface_rec->external_ids, OVN_ZONE_ID, 0); + if (zone_id && !simap_contains(ct_zones, iface_id)) { + /* get zone-id from the local interface record */ + bitmap_set1(ct_zone_bitmap, zone_id); + simap_put(ct_zones, iface_id, zone_id); + } With that in mind, update_local_zone_ids() should also update external-ids:zone-id if it needs to change, instead of leaving it the same. Here, please use INT_STRLEN(int) + 1 as the size of zone[]. + char zone[12]; + zone_id = simap_get(ct_zones, iface_id); + snprintf(zone, sizeof zone, "%d", zone_id); Thanks, Ben. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev