[ovs-dev] [PATCH 05/14] ofp-util: Allow use of OpenFlow 12 flow format

[Ben Pfaff]

On Mon, Nov 12, 2012 at 09:57:26AM -0800, Ben Pfaff wrote:
> On Wed, Nov 07, 2012 at 05:03:03PM +0900, Simon Horman wrote:
> > This enables the use of the OpenFlow 12 flow format.
> > 
> > Signed-off-by: Simon Horman <horms at verge.net.au>
> 
> Based on "grep", I'm nervous that we're missing important handling for
> OFPUTIL_P_OF12 in a number of places.  For example, it looks like
> ofputil_usable_protocols(), ofputil_encode_set_protocol(), and
> ofputil_flow_mod_usable_protocols() don't have any provisions for
> OF1.2.  Do any of the later patches in this series fix those up?  If so,
> then I guess we should try to fold them into this patch.  If not, then
> we should probably fix them.

After further reflection, I've realized that there are two possible ways
that we can continue:

    1. OFPUTIL_P_OF12 means OpenFlow 1.2 with OXM flow format.  If we
       need to support the OXM flow format on top of other OpenFlow
       versions, we add other OFPUTIL_P_* constants to represent those.

       Advantages: any one given "enum ofputil_protocol" bit has a very
       specific meaning, so that we can generally pass around a single
       enum ofputil_protocol to any code that needs to encode a message.
       Also, it's easy to use bitmasks for set operations like
       membership tests, intersection, and union.

       Disadvantages: we may end up with many possible enum
       ofputil_protocol values, something like the cross-product of
       OpenFlow version and supported flow formats.  (I don't think that
       this should really happen, because I think we can reasonably
       limit the flow formats supported in each OpenFlow version.)

    2. OFPUTIL_P_OF12 means the OXM flow format and doesn't imply a
       particular OpenFlow version.  (This means that we might in some
       places have to know both an "enum ofputil_protocol" and an
       OpenFlow version.)

I'm not sure which one you actually have in mind here.  I guess #1 makes
a little more sense to me.  Can you elaborate?

Thanks,

Ben.