[ovs-dev] - ovn-controller: Assign zone-ids consistently

Ben Pfaff blp at ovn.org
Tue Feb 23 00:03:03 UTC 2016


On Mon, Feb 08, 2016 at 10:12:50PM +0000, Suryanarayan Ramamurthy wrote:
> Currently, conntrack zone-id is assigned to lport by ovn-controller,
> but the ovn-controller does not remember what was earlier assigned
> to the same lport (possibly in an earlier run across restart).
> 
> So, after ovn-controller restart, the zone-ids may get set 
> inconsistently on lports, resulting in possible hits to 
> already established connections.
> 
> Fix is to remember the zone-id as an external-id of the interface record
> in the local ovs-db, and recover zone-ids assigned earlier to lports 
> from that record.
> 
> This patch fixes:
> https://bugs.launchpad.net/networking-ovn/+bug/1538696
> 
> Signed-off-by: Ramu Ramamurthy <ramu.ramamurthy at us.ibm.com>

Thank you for the patch.

>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 = smap_get(&iface_rec->external_ids, "zone-id");
> +            if (zone && ovs_scan(zone, "%d", &zone_id)) {
> +                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.

This commit removes the XXX comment below.  Does it fix the problem that
comment points out?

> @@ -112,10 +171,8 @@ update_ct_zones(struct sset *lports, struct simap *ct_zones,
>          bitmap_set1(ct_zone_bitmap, zone);
>          simap_put(ct_zones, iface_id, zone);
>  
> -        /* xxx We should erase any old entries for this
> -         * xxx zone, but we need a generic interface to the conntrack
> -         * xxx table. */
>      }
> +    update_local_zone_ids(br_int, ct_zones, ctx);
>  }

Thanks,

Ben.



More information about the dev mailing list