Hi, Ben. I know we've had a lot of mailing list problems so you may have missed some of the follow-up to this patch. It's up to v5. I believe another revision is coming to address saving zone IDs for container-in-vm and localnet ports.
http://openvswitch.org/pipermail/dev/2016-February/066179.html On Mon, Feb 22, 2016 at 7:03 PM, Ben Pfaff <b...@ovn.org> wrote: > 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. > -- Russell Bryant _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
