[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