[ovs-dev] [PATCH v6 2/3] [ovn-controller] Make flow table persistent in ovn controller

Han Zhou zhouhan at gmail.com
Thu Feb 18 18:10:09 UTC 2016


On Thu, Feb 18, 2016 at 6:52 AM, Ryan Moats <rmoats at us.ibm.com> wrote:
>
> Han Zhou <zhouhan at gmail.com> wrote on 02/18/2016 01:34:03 AM:
>
> [snipped for BW]
>
>
>
> > > +    /* loop through all the flows to see if there is an old flow to
be
> > > +     * removed - do so if the old flow has the same priority,
> > table, and match
> > > +     * but a different action or if the old flow has the same
> > priority, table
> > > +     * and action, but the new match is either a superset or subset
of the
> > > +     * old match */
> > > +
> > > +    struct ovn_flow *d, *next;
> > > +    HMAP_FOR_EACH_SAFE (d, next, hmap_node, desired_flows) {
> > > +        if (f->table_id == d->table_id && f->priority ==
d->priority) {
> > > +            if ((match_equal(&f->match, &d->match)
> > > +                 && !ofpacts_equal(f->ofpacts, f->ofpacts_len,
> > > +                                   d->ofpacts, d->ofpacts_len))
> > > +                || (ofpacts_equal(f->ofpacts, f->ofpacts_len,
> > > +                                  d->ofpacts, d->ofpacts_len)
> > > +                    && ((flow_wildcards_has_extra
> > (&f->match.wc,&d->match.wc)
> > > +                         && flow_equal_except(&f->match.flow,
> > &d->match.flow,
> > > +                                              &f->match.wc))
> > > +                        || (flow_wildcards_has_extra
> > (&d->match.wc,&f->match.wc)
> > > +                            && flow_equal_except(&d->match.flow,
> > > +                                                 &f->match.flow,
> > > +                                                 &d->match.wc))))) {
> > > +                static struct vlog_rate_limit rl =
> > VLOG_RATE_LIMIT_INIT(5, 5);
> > > +                if (!VLOG_DROP_INFO(&rl)) {
> > > +                    char *s = ovn_flow_to_string(d);
> > > +                    VLOG_INFO("removing superceded flow: %s", s);
> > > +                    free(s);
> > > +                }
> > >
> > > -        ovn_flow_destroy(f);
> > > -        return;
> > > +                hmap_remove(desired_flows, &d->hmap_node);
> > > +                ovn_flow_destroy(d);
> > > +            }
> > > +
> > > +            /* if this is a complete duplicate, remove the new flow
*/
> > > +            if (match_equal(&f->match, &d->match)
> > > +                && ofpacts_equal(f->ofpacts, f->ofpacts_len,
> > > +                                 d->ofpacts, d->ofpacts_len)) {
> > > +                static struct vlog_rate_limit rl =
> > VLOG_RATE_LIMIT_INIT(5, 5);
> > > +                if (!VLOG_DROP_INFO(&rl)) {
> > > +                    char *s = ovn_flow_to_string(f);
> > > +                    VLOG_INFO("dropping duplicate flow: %s", s);
> > > +                    free(s);
> > > +                }
> > > +
> > > +                ovn_flow_destroy(f);
> > > +                return;
> > > +            }
> > > +        }
> > >      }
> > >
>
> > Ryan, sorry that I don't quite understand the loop here. Could you
> > explain why we need to examine existing flows one by one to identify
> > the old flows to be removed? Shouldn't the ovsdb track deleted
> > entries and then we can just delete whatever is informed by ovsdb?
>
> > Otherwise, I wonder if we go with this approach, it maybe less
> > efficient in circumstances when there are big changes in SB lflow
> > table, e.g. during some fault-recovery when SB db are rebuilt.
> >
> > Maybe I missed some point here, correct me if I am wrong :)
>
> Here's the deal - when we initially find a port, a desired OF flow
> gets created without any port security (match regex is
> /reg6=.,metadata=0x1/ IIRC). After we set port security via
> ovn-nbctl, that OF flow gets created with a match pattern that
> includes the mac address (match regex is
> /reg6=.,metadata=0x1,dl_src=<mac address>/ IIRC).  If the desired
> OF flow table gets rebuilt each time, the first OF flow gets purged
> from the switch after this change because it doesn't exist in the
> rebuilt desired OF flow table.  To persist the desired OF flow table
> across cycles, we have to add the ability to detect and remove that
> old desired flow by hand, otherwise both flows end up in the switch
> and we lose port security.
>
> Now, I absolutely admit that this may be less efficient in cases where
> there are big changes, but I *believe* that this is far more efficient
> in the "sunny day" case once we layer the part 3 patch of incremental
> logical flow processing on to this (I say believe because I'm running
> performance tests this morning to try and verify it).
>
> Lastly, this wasn't my initial choice of how to do things, but I still
> haven't thought of a way to add incremental logical flow processing
> that doesn't require some memory of what OF flows have been asked for
> during previous cycles and still works correctly.
>
> Ryan

Ryan, thanks for explain. What I meant is to rely on the delta reported by
OVSDB_IDL_CHANGE_DELETE to do the job of removing old flows. But I need to
study more on the "track" feature of ovsdb, maybe it is not capable of
doing this yet?


--
Best regards,
Han



More information about the dev mailing list