On Thu, Feb 18, 2016 at 6:52 AM, Ryan Moats <rmo...@us.ibm.com> wrote:
>
> Han Zhou <zhou...@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
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
