[ovs-dev] [PATCH 1/5] Add error codes for Open Flow v1.2

Ben Pfaff blp at nicira.com
Mon Mar 26 20:57:09 UTC 2012


On Mon, Mar 26, 2012 at 02:30:23PM +0900, Simon Horman wrote:
> Signed-off-by: Simon Horman <horms at verge.net.au>
> 
> ---
> v2
> As suggested by Ben Pfaff
> * Where Open Flow 1.2 breaks apart error codes defined
>   in previous versions, provide all new definitions to
>   previous versions and map the numeric error code to
>   the first first definition supplied in ofp-errors.h.
>   The case handled so far is:
>   OFPERR_OFPBIC_BAD_EXP_TYPE -> { OFPERR_OFPBIC_BAD_EXPERIMENTER,
>                                   OFPERR_OFPBIC_BAD_EXP_TYPE }
> * Correct name of OFPERR_OFPERR_BAD_ROLE, it should be
>   OFPERR_OFPRRFC_BAD_ROLE.
> * Where Open Flow 1.2 adds error codes that were previously
>   defined as Nicira extension errors define the later in terms
>   of the new codes.
> 
> Add some missing common openflow definitions
> 
> Signed-off-by: Simon Horman <horms at verge.net.au>

So, the more I looked at this the more a few things bothered me.
First, entirely getting rid of the checks for duplicates seemed risky;
it seemed like an invitation to get things wrong again later.  Indeed,
when I added it back temporarily I found very quickly a typo here that
was obvious in retrospect:

    /* NX1.0(1,258), NX1.1(1,258), OF1.1(4,7).  Unsupported value in a match field. */
    OFPERR_OFPBMC_BAD_VALUE,

(s/OF1.1/OF1.2/ in case it's still not obvious.)

So I decided to improve that, by making explicit declarations of
expected duplicates mandatory.

And then I realized that the forms of the targets started to really
confuse me.  OF, OF1.0, OF1.1, OF1.1only, OF1.2, and so on, with
meanings that aren't obvious.  I decided that it's better to be more
consistent, so I changed these to, respectively, OF1.0+, OF1.0,
OF1.1+, OF1.1, and OF1.2, where the "+" consistently means "this
version and all later versions" and the absence means "this version
only".

Along the way I discovered and fixed some unit test failures and bugs
in extract-ofp-errors.

Anyway, I'll send this out a new series derived from this patch in a
minute.  Simon, please review it.

Once we have that series in I'll look at the rest of this series.

Thanks,

Ben.



More information about the dev mailing list