[ovs-dev] [ovs-dev, v5] ovn-controller: Assign zone-id consistently
Ben Pfaff
blp at ovn.org
Wed Feb 24 00:14:45 UTC 2016
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 <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>
> ---
> 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.
More information about the dev
mailing list