[ovs-dev] [non-NORMAL learning 3/3] vswitchd: Only re-learn from flows that output to OFPP_NORMAL.

[Ben Pfaff]

On Thu, Aug 05, 2010 at 01:41:27PM -0400, Jesse Gross wrote:
> On Wed, Aug 4, 2010 at 10:27 AM, Ben Pfaff <blp at nicira.com> wrote:
> 
> >     COVERAGE_INC(bridge_process_flow);
> > -    return process_flow(br, flow, packet, actions, tags, nf_output_iface);
> > +
> > +    retval = process_flow(br, flow, packet, actions, tags,
> > nf_output_iface);
> > +    if (*tags == 0) {
> > +        /* Ensure that '*tags' is nonzero, so
> > bridge_account_flow_ofhook_cb()
> > +         * knows that it should use this flow to update the learning
> > table. */
> > +        *tags = tag_arbitrary_1bit();
> > +    }
> >
> 
> Every flow that is interesting to learn from later on will already have a
> tag set on it.  mac_learning_lookup_tag() will add a tag for every flow
> except two categories: inadmissible flows and flows that are on non-learning
> vlans.  Both of these types of flows will be ignored later on when we update
> the table.  Put it another way: if we didn't run it through the MAC learning
> code on flow setup, it probably isn't something that we want to run through
> later on.

You're right.

I dropped this part.

> 1 bit tags seem like a weird hack to me that breaks the abstraction of the
> tag class (what if we decided that tags should be 1 bit?) and generally
> pollutes the tag space with unrelated information.  If we really want to
> store this information separately, I would just add a bool to the rule in
> ofproto.  After all, if we didn't create the flow using the normal action,
> it probably isn't very useful to call the account_flow callback at all.

I think that I could make an argument in favor of 1-bit tags, but you've
shown a better way anyhow, so there's no need.  Thanks for the comments.