[ovs-dev] [PATCH 04/18] ofp-util: Add version bitmap to hello messages

[Ben Pfaff]

On Thu, Oct 18, 2012 at 02:58:04PM +0900, Simon Horman wrote:
> Allow encoding and decoding of version bitmap in hello messages
> as specified in Open Flow 1.3.1.
> 
> Signed-off-by: Simon Horman <horms at verge.net.au>

Thanks for doing this.

It looks like ofputil_decode_hello() only processes a single hello
element, only if that one is an OFPHET_VERSIONBITMAP element.  OF1.3.1
says:

    Implementations must ignore (skip) all elements of a Hello message
    that they do not support.

so I'd be inclined to say that, instead, we should iterate over each
element that is present, looking for an OFPHET_VERSIONBITMAP element and
silently ignoring any others that we find.

It looks like the declarations of ofputil_hello, ofputil_decode_hello(),
and ofputil_encode_hello() should be moved from patch 2 to this patch,
because I don't see any reference to them before this patch.

I'm not certain that ofputil_hello actually needs the 'version' member.
To decode a hello message without a bitmap, one can initialize the
bitmap as every version up to the one specified; to decode a hello
message with a bitmap, one initializes the bitmap from the included
bitmap.  To encode a hello message given only a bitmap is equally
straightforward.  (And if I'm right about all that, then maybe we don't
need a struct at all--perhaps a single uint32_t bitmap is sufficient?)

Thanks,

Ben.