[ovs-dev] [PATCH] ovn-controller: Add incremental processing for multicast groups

Flaviof flavio at flaviof.com
Mon Jul 25 01:14:19 UTC 2016


On Sun, Jul 24, 2016 at 2:33 PM, Ryan Moats <rmoats at us.ibm.com> wrote:

> The various fixes in handling physical and binding changes
> have addressed the problems that the original attempt to
> incrementally process the multicast group table ran into.
> So, re-add incremental processing of the multicast group
> table.
>
> Notes:
> - This patch renders the short term changes in [1] taken
>   for multicast group processing unnecessary (and so it
>   conflicts with [1]).
> - Running through the ovn unit tests 50 times with -j4
>   didn't show increased test case failures over the
>   baseline code prior to adding this patch.
>
> [1] http://patchwork.ozlabs.org/patch/652117/
>
> Signed-off-by: Ryan Moats <rmoats at us.ibm.com>
>

short of nitpicks below...


Acked-by: Flavio Fernandes <flavio at flaviof.com>



> ---
>  ovn/controller/physical.c | 26 +++++++++++++++++++++++---
>  1 file changed, 23 insertions(+), 3 deletions(-)
>
> diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
> index a104e33..b10baed 100644
> --- a/ovn/controller/physical.c
> +++ b/ovn/controller/physical.c
> @@ -61,11 +61,13 @@ static struct hmap tunnels =
> HMAP_INITIALIZER(&tunnels);
>  /* UUID to identify OF flows not associated with ovsdb rows. */
>  static struct uuid *hc_uuid = NULL;
>  static bool full_binding_processing = false;
> +static bool full_mcgroup_processing = false;
>
>  void
>  physical_reset_processing(void)
>  {
>      full_binding_processing = true;
> +    full_mcgroup_processing = true;
>  }
>
>  /* Maps from a chassis to the OpenFlow port number of the tunnel that can
> be
> @@ -675,6 +677,7 @@ physical_run(struct controller_ctx *ctx, enum
> mf_field_id mff_ovn_geneve,
>                  tun->ofport = u16_to_ofp(ofport);
>                  tun->type = tunnel_type;
>

nitpick...


>                  full_binding_processing = true;
> +                full_mcgroup_processing = true;
>

how about replacing the new line and the line above with:

physical_reset_processing();


>                  binding_reset_processing();
>
>                  /* Reprocess logical flow table immediately. */
> @@ -715,6 +718,7 @@ physical_run(struct controller_ctx *ctx, enum
> mf_field_id mff_ovn_geneve,
>      }
>      if (localvif_map_changed) {
>



>          full_binding_processing = true;
> +        full_mcgroup_processing = true;
>

same as previous comment: just call
physical_reset_processing();



>
>          /* Reprocess logical flow table immediately. */
>          lflow_reset_processing();
> @@ -751,9 +755,25 @@ physical_run(struct controller_ctx *ctx, enum
> mf_field_id mff_ovn_geneve,
>      const struct sbrec_multicast_group *mc;
>      struct ofpbuf remote_ofpacts;
>      ofpbuf_init(&remote_ofpacts, 0);
> -    SBREC_MULTICAST_GROUP_FOR_EACH (mc, ctx->ovnsb_idl) {
> -        consider_mc_group(mff_ovn_geneve, ct_zones,
> -                          local_datapaths, mc, &ofpacts, &remote_ofpacts);
> +    if (full_mcgroup_processing) {
> +        SBREC_MULTICAST_GROUP_FOR_EACH (mc, ctx->ovnsb_idl) {
> +            consider_mc_group(mff_ovn_geneve, ct_zones,
> +                              local_datapaths, mc, &ofpacts,
> &remote_ofpacts);
> +        }
> +        full_mcgroup_processing = false;
> +    } else {
> +        SBREC_MULTICAST_GROUP_FOR_EACH_TRACKED (mc, ctx->ovnsb_idl) {
> +            if (sbrec_multicast_group_is_deleted(mc)) {
> +                ofctrl_remove_flows(&mc->header_.uuid);
> +            } else {
> +                if (!sbrec_multicast_group_is_new(mc)) {
> +                    ofctrl_remove_flows(&mc->header_.uuid);
> +                }
> +                consider_mc_group(mff_ovn_geneve, ct_zones,
> +                                  local_datapaths, mc, &ofpacts,
> +                                  &remote_ofpacts);
> +            }
> +        }
>      }
>
>      ofpbuf_uninit(&remote_ofpacts);
> --
> 1.9.1
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>



More information about the dev mailing list