[ovs-dev] [nxm 26/42] ofproto: Make flow format specific to an OpenFlow connection.
blp at nicira.com
Fri Nov 5 19:02:35 UTC 2010
On Fri, Nov 05, 2010 at 11:18:16AM -0700, Justin Pettit wrote:
> On Oct 28, 2010, at 10:27 AM, Ben Pfaff wrote:
> > Until now the flow format has been a global property of an ofproto.
> > This is no longer appropriate, because the Nicira Extended Match
> > flow format that upcoming commits will add is not compatible with
> > OpenFlow 1.0-only controllers. This commit changes the flow format
> > to a property of an individual OpenFlow connection.connection
> I assume you didn't want the extra "connection" there.
> > @@ -3297,7 +3297,7 @@ flow_stats_ds_cb(struct cls_rule *rule_, void *cbdata_)
> > query_stats(cbdata->ofproto, rule, &packet_count, &byte_count);
> > flow_to_match(&rule->cr.flow, rule->cr.wc.wildcards,
> > - cbdata->ofproto->flow_format, &match);
> > + NXFF_OPENFLOW10, &match);
> If the handle_flow_stats_request() function is going to use the
> "flow_format" argument, it seems like this should as well. However,
> as I mentioned in the previous patch, I think we may want to force it
> to use NXFF_OPENFLOW10, too.
I think you are saying that this part of the patch looks correct. I
agree. I guess maybe it should have been called out separately though.
> > static struct rule *
> > -find_flow_strict(struct ofproto *p, const struct ofp_flow_mod *ofm)
> > +find_flow_strict(struct ofproto *p, struct ofconn *ofconn,
> > + const struct ofp_flow_mod *ofm)
> Since "ofproto" is in "ofconn", do you think there's any benefit to
> just be passing in "ofconn" in some of these functions?
It is? So it is. I'd forgotten.
I'll fix that up.
> Having some connections using a different match structure than others
> seems like it's going to cause some interesting interactions. I can
> see the need, but I'm curious to see how you handled it.
It's interesting, yes. I probably didn't handle it particularly well.
More information about the dev