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.ramamur...@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.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
