On Wed, Jul 27, 2016 at 8:42 AM, Ryan Moats <rmo...@us.ibm.com> wrote:
> As [1] indicates, commit 263064a (Convert binding_run > to incremental processing.) incorrectly localized > all_lports to the binding module, leaving an empty > set for update_ct_zone to work with. This patch > restores all_lports processing to what existed prior > to that patch. > > [1] http://openvswitch.org/pipermail/dev/2016-July/076171.html > > Signed-off-by: Ryan Moats <rmo...@us.ibm.com> > --- > v1->v2: Added missing reference to commit message > > ovn/controller/binding.c | 30 +++++++++++++++++++++++++++++- > ovn/controller/binding.h | 3 ++- > ovn/controller/ovn-controller.c | 3 ++- > 3 files changed, 33 insertions(+), 3 deletions(-) > > diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c > index e83c1d5..82ee3ba 100644 > --- a/ovn/controller/binding.c > +++ b/ovn/controller/binding.c > @@ -230,7 +230,8 @@ consider_local_datapath(struct controller_ctx *ctx, > > void > binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge > *br_int, > - const char *chassis_id, struct hmap *local_datapaths) > + const char *chassis_id, struct sset *all_lports, > + struct hmap *local_datapaths) > { > const struct sbrec_chassis *chassis_rec; > const struct sbrec_port_binding *binding_rec; > @@ -251,6 +252,33 @@ binding_run(struct controller_ctx *ctx, const struct > ovsrec_bridge *br_int, > process_full_binding = true; > } > > + struct shash_node *node; > + SHASH_FOR_EACH (node, &lport_to_iface) { > + sset_add(all_lports, node->name); > + } > I think you could just sset_clone() local_ids into all_lports and get the same result. > + > + /* Run through all binding records to collect lists of lports > + for later use in updating ct zones. */ > + SBREC_PORT_BINDING_FOR_EACH(binding_rec, ctx->ovnsb_idl) { > + const struct ovsrec_interface *iface_rec > + = shash_find_data(&lport_to_iface, binding_rec->logical_port); > + if (iface_rec > + || (binding_rec->parent_port && binding_rec->parent_port[0] && > + sset_contains(all_lports, binding_rec->parent_port))) { > + if (binding_rec->parent_port && binding_rec->parent_port[0]) { > + /* Add child logical port to the set of all local ports. > */ > + sset_add(all_lports, binding_rec->logical_port); > + } > + } else if (!binding_rec->chassis > + && !strcmp(binding_rec->type, "localnet")) { > + /* localnet ports will never be bound to a chassis, but we > want > + * to list them in all_lports because we want to allocate > + * a conntrack zone ID for each one, as we'll be creating > + * a patch port for each one. */ > + sset_add(all_lports, binding_rec->logical_port); > + } > + } > + > This seems to undo the benefits of the original change to do "incremental procesing" in binding.c. It seems like we weren't that far from a complete fix in Babu's first patch. -- Russell Bryant _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
