On Mon, Mar 26, 2012 at 01:57:09PM -0700, Ben Pfaff wrote:
> On Mon, Mar 26, 2012 at 02:30:23PM +0900, Simon Horman wrote:
> > Signed-off-by: Simon Horman <ho...@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 <ho...@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.
Sure, will do. The changes you describe above sound entirely reasonable to me.
> Once we have that series in I'll look at the rest of this series.
I fear there are more points of bother lurking.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
