On Wed, Jul 25, 2012 at 09:03:15AM +0900, Simon Horman wrote:
> On Tue, Jul 24, 2012 at 02:01:12PM -0700, Ben Pfaff wrote:
> > On Mon, Jul 23, 2012 at 03:16:38PM +0900, Simon Horman wrote:
> > > +    enum ofputil_protocol protocol;
> > > +
> > > +    switch(oh->version) {
> > > +    case OFP10_VERSION:
> > > +          protocol = OFPUTIL_P_OF10_TID;
> > > +          break;
> > > +    default:
> > > +          protocol = ofputil_protocol_from_ofp_version(oh->version);
> > > +          break;
> > > +    }
> > >  
> > >      ofpbuf_init(&ofpacts, 64);
> > > -    error = ofputil_decode_flow_mod(&fm, oh, OFPUTIL_P_OF10_TID, 
> > > &ofpacts);
> > > +    error = ofputil_decode_flow_mod(&fm, oh, protocol, &ofpacts);
> > 
> > I think it'd be better to calculate the protocol to use as something
> > more like:
> > 
> >         protocol = ofputil_protocol_from_ofp_version(oh->version);
> >         protocol = ofputil_protocol_set_tid(protocol, true);
> 
> I'm not sure that I understand the implications of that,
> but I'll see about updating my patch accordingly.

Well, it just makes sure that the protocol we pass in supports
table-id, because that's more useful for dumping flow_mods than ones
that don't.  If you get it wrong in the other direction, then the
flow_mod output can be hard to read.  And it seems cleaner than
writing out the logic explicitly.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to