[ovs-dev] [PATCH v2] ovn-controller: Restore ct zone assignment.

[Ryan Moats]

"dev" <dev-bounces at openvswitch.org> wrote on 07/29/2016 10:18:49 AM:

> From: Russell Bryant <russell at ovn.org>
> To: dev at openvswitch.org
> Date: 07/29/2016 10:19 AM
> Subject: [ovs-dev] [PATCH v2] ovn-controller: Restore ct zone assignment.
> Sent by: "dev" <dev-bounces at openvswitch.org>
>
> From: Babu Shanmugam <bschanmu at redhat.com>
>
> 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 <bschanmu at redhat.com>
> Co-authored-by: Russell Bryant <russell at ovn.org>
> Signed-off-by: Russell Bryant <russell at ovn.org>
> ---

<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...

Otherwise, this looks good (modulo the debugging log statement :) )

Ryan