[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