[ovs-dev] [PATCH 19/19] ofp-util: Allow decoding of Open Flow 1.2 Packet In Messages

[Ben Pfaff]

On Fri, Jun 15, 2012 at 08:58:18AM +0900, Simon Horman wrote:
> On Thu, Jun 14, 2012 at 08:56:27AM -0700, Ben Pfaff wrote:
> > On Thu, Jun 14, 2012 at 09:40:55AM +0900, Simon Horman wrote:
> > > > -    if (code == OFPUTIL_OFPT_PACKET_IN) {
> > > > +    if (code == OFPUTIL_OFPT_PACKET_IN && oh->version == OFP10_VERSION) {
> > > 
> > > I am wondering if it would be better to rename OFPUTIL_OFPT_PACKET_IN as
> > > OFPUTIL_OFPT10_PACKET_IN and add OFPUTIL_OFPT12_PACKET_IN and avoid
> > > checking oh->version directly here. The same probably applies to other
> > > the decoders for other messages that differ between Open Flow 1.0 and 1.2.
> > 
> > I'm not sure; I'm split on a similar issue for the flow_mod message.
> > 
> > A single code has the advantage that client code doesn't have to
> > unnecessarily distinguish messages that are, from its perspective,
> > equivalent.  Separate codes have the advantage that the decode function
> > doesn't have to distinguish the different versions of a message.
> > 
> > On the whole, I'm leaning toward using a single code, on the principle
> > that one should isolate complexity in libraries, not in their clients.
> 
> Thanks, I have mostly used a single code and I think that ends up being
> simpler overall. I'll move all of my code in that direction and see what happens.
> 
> > (This also argues for, e.g., merging OFPUTIL_NXT_FLOW_MOD into
> > OFPUTIL_OFPT_FLOW_MOD.)
> 
> Yes, that would make the treatment of different versions of
> Flow Mod a bit more consistent.

Take a look at the approach that becomes possible with the ofp-msgs
library that I posted:
http://openvswitch.org/pipermail/dev/2012-July/018725.html

I think it's a lot cleaner, because you can get both separate views
for each wire format type for the places where that is needed (the
OFPRAW_* constants) and the abstract view for the places where that is
needed (the OFPTYPE_* constants).  What do you think?

Thanks,

Ben.