[ovs-dev] [PATCH 4/4] Definitions for Open Flow 1.2

[Simon Horman]

On Wed, Mar 28, 2012 at 12:34:53PM -0700, Ben Pfaff wrote:
> On Wed, Mar 28, 2012 at 09:44:17AM +0900, Simon Horman wrote:
> > This is a first pass at adding include/openflow/openflow-1.2.h to
> > include enum and struct definitions for Open Flow 1.2 that
> > are not already covered by Open Flow 1.1.
> > 
> > Signed-off-by: Simon Horman <horms at verge.net.au>
> 
> There are still some "uint<N>_t"s in here.
> 
> I'm not sure why this modifies openflow-1.1.h.  Should that go in the
> previous patch?

Sorry about that x2.

> Also, I think that this new ofp11_port_stats_request
> is a candidate to have the ofp11_stats_msg header incorporated into
> it.

Sure, I'll look into that.

> I think that the differences between ofp11_flow_mod and ofp12_flow_mod
> are illusory.  The only difference is that the OF1.1 version embeds an
> OF1.1 flow match and that the OF1.2 version embeds an OF1.2 flow
> match.  I think it would be better to use a structure that just
> contains everything up to the flow match and deal with the differences
> in the flow match separately.
>
> Similarly, ofp11_flow_stats and ofp12_flow_stats are actually
> identical except for the flow match, so it'd be better to just have a
> structure with the common parts and the flow match part left out.

Yes, I think there are a few cases where the only difference between
an ofp11 and ofp12 structure is the new match format. I like your idea
of consolidating these where possible as it should reduce noise.

> I'd leave out ofp12_aggregate_stats_request.
> 
> I don't think that the ofp12_match structure will really be useful in
> this form.  If we just defined an "ofp_match_header" that just
> contains length and type, we could probably use that somewhere.

That sounds reasonable. Would it be ofp11_match_header or would
it also cover Open Flow 1.0?

> I don't think experimenter stats are actually different in 1.1 and
> 1.2, although the sections are worded differently.

I'll double check that.