Ben Pfaff <b...@ovn.org> wrote on 02/25/2016 03:31:56 PM:
> From: Ben Pfaff <b...@ovn.org>
> To: Ryan Moats/Omaha/IBM@IBMUS
> Cc: dev@openvswitch.org
> Date: 02/25/2016 03:32 PM
> Subject: Re: [ovs-dev] [PATCH v7 4/6] [ovn-controller] Persist ports
> simap in logical_datapath
>
> On Fri, Feb 19, 2016 at 11:25:10AM -0600, Ryan Moats wrote:
> > From: RYAN D. MOATS <rmo...@us.ibm.com>
> >
> > Persist across runs so that a change to this simap can be used
> > as a trigger for resetting incremental processing.
> >
> > Signed-off-by: RYAN D. MOATS <rmo...@us.ibm.com>
> > ---
> > ovn/controller/lflow.c | 18 ++++++++++++------
> > 1 files changed, 12 insertions(+), 6 deletions(-)
> >
> > diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
> > index d53213c..18f0970 100644
> > --- a/ovn/controller/lflow.c
> > +++ b/ovn/controller/lflow.c
> > @@ -231,21 +231,27 @@ static void
> > ldp_run(struct controller_ctx *ctx)
> > {
> > struct logical_datapath *ldp;
> > - HMAP_FOR_EACH (ldp, hmap_node, &logical_datapaths) {
> > - simap_clear(&ldp->ports);
> > - }
> > +// HMAP_FOR_EACH (ldp, hmap_node, &logical_datapaths) {
> > +// simap_clear(&ldp->ports);
> > +// }
>
> Please don't comment out code. Just remove it, if it is no longer
> needed.
Yes, that's a mistake that will get fixed in v8.
>
> > const struct sbrec_port_binding *binding;
> > SBREC_PORT_BINDING_FOR_EACH (binding, ctx->ovnsb_idl) {
> > struct logical_datapath *ldp = ldp_lookup_or_create
> (binding->datapath);
> > -
> > - simap_put(&ldp->ports, binding->logical_port, binding->
tunnel_key);
> > + struct simap_node *old = simap_find(&ldp->ports,
> > + binding->logical_port);
> > + if (!old || old->data != binding->tunnel_key) {
> > + simap_put(&ldp->ports, binding->logical_port,
> binding->tunnel_key);
> > + }
> > }
> >
> > const struct sbrec_multicast_group *mc;
> > SBREC_MULTICAST_GROUP_FOR_EACH (mc, ctx->ovnsb_idl) {
> > struct logical_datapath *ldp = ldp_lookup_or_create(mc->
datapath);
> > - simap_put(&ldp->ports, mc->name, mc->tunnel_key);
> > + struct simap_node *old = simap_find(&ldp->ports, mc->name);
> > + if (!old || old->data != mc->tunnel_key) {
> > + simap_put(&ldp->ports, mc->name, mc->tunnel_key);
> > + }
> > }
>
> I don't see anything in the new logic that will ever clear a port or a
> multicast group that has disappeared.
Hmm... I remember having something that took care of that case, but since I
don't
see it in the above, that is now on the gating list for the v8 patch set.
Ryan
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
