Russell Bryant <[email protected]> wrote on 07/29/2016 11:27:49 AM:
> From: Russell Bryant <[email protected]>
> To: Ryan Moats/Omaha/IBM@IBMUS
> Cc: ovs dev <[email protected]>
> Date: 07/29/2016 11:28 AM
> Subject: Re: [ovs-dev] [PATCH v2] ovn-controller: Restore ct zone
assignment.
>
> On Fri, Jul 29, 2016 at 11:59 AM, Ryan Moats <[email protected]> wrote:
> "dev" <[email protected]> wrote on 07/29/2016 10:18:49 AM:
>
> > From: Russell Bryant <[email protected]>
> > To: [email protected]
> > Date: 07/29/2016 10:19 AM
> > Subject: [ovs-dev] [PATCH v2] ovn-controller: Restore ct zone
assignment.
> > Sent by: "dev" <[email protected]>
> >
> > From: Babu Shanmugam <[email protected]>
> >
> > Recent commits reorganizing bindings handling and also moving ct zone
> > assignment to ovn-controller.c caused ct zone assignment to no longer
> > work. The code relies on an "all_lports" sset that should contain all
> > logical ports that we should be assigning ct zones for. Prior to this
> > change, all_lports was always empty.
> >
> > Signed-off-by: Babu Shanmugam <[email protected]>
> > Co-authored-by: Russell Bryant <[email protected]>
> > Signed-off-by: Russell Bryant <[email protected]>
> > ---
>
> <snip>
>
> > @@ -278,6 +303,14 @@ binding_run(struct controller_ctx *ctx, const
> > struct ovsrec_bridge *br_int,
> > }
> > }
> > hmap_destroy(&keep_local_datapath_by_uuid);
> > +
> > + /* Any remaining entries in removed_lports are logical ports
that
> > + * have been deleted and should also be removed from
all_ports. */
> > + const char *cur_id;
> > + SSET_FOR_EACH(cur_id, &removed_lports) {
> > + sset_find_and_delete(all_lports, cur_id);
> > + }
> > +
> > process_full_binding = false;
> > } else {
> > SBREC_PORT_BINDING_FOR_EACH_TRACKED(binding_rec, ctx->
ovnsb_idl) {
> > @@ -292,7 +325,8 @@ binding_run(struct controller_ctx *ctx, const
> > struct ovsrec_bridge *br_int,
> > }
> > } else {
> > consider_local_datapath(ctx, chassis_rec, binding_rec,
> > - local_datapaths,
&lport_to_iface);
> > + local_datapaths,
&lport_to_iface,
> > + all_lports);
> > }
> > }
> > }
>
> A quick look at this leads me to ask about cleaning up entries
> derived from binding records that are found to be deleted during
> incremental processing? The code leads me to believe that they
> will hang around as stale at least until the next full processing
> cycle and I don't think we want that...
> I think it's handled. See this block. The immediate full bindings
> refresh will get all_lports cleaned up, as well.
>
> SBREC_PORT_BINDING_FOR_EACH_TRACKED(binding_rec, ctx->ovnsb_idl)
{
> if (sbrec_port_binding_is_deleted(binding_rec)) {
> /* If a port binding was bound to this chassis and
> removed before
> * the ovs interface was removed, we'll catch that
> here and trigger
> * a full bindings refresh. This is to see if we
> need to clear
> * an entry out of local_datapaths. */
> if (binding_rec->chassis == chassis_rec) {
> process_full_binding = true;
> poll_immediate_wake();
> }
>
> --
> Russell Bryant
Well, I may not entirely like it but yes, it does do it so ...
Begrudgingly-
Acked-by: Ryan Moats <[email protected]>
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
