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

[Ben Pfaff]

On Thu, Mar 22, 2012 at 04:58:53PM +0900, Simon Horman wrote:
> On Tue, Mar 20, 2012 at 10:28:02AM -0700, Ben Pfaff wrote:
> > On Tue, Mar 20, 2012 at 11:43:38AM +0900, Simon Horman wrote:
> > > On Mon, Mar 19, 2012 at 02:31:36PM -0700, Ben Pfaff wrote:
> > > > On Mon, Mar 19, 2012 at 09:54:43AM +0900, Simon Horman wrote:
> 
> [snip]
> 
> > > > Second, in a few cases OF1.2 seems to "break apart" errors defined by
> > > > older versions.  When this happens, it would be ideal if OVS
> > > > internally could use the more-specific error codes of later versions
> > > > and have them translated to less-specific versions for sending in
> > > > older protocols.  Here's an example:
> > > > 
> > > >     /* OF1.1(3,5).  Specific experimenter instruction unsupported. */
> > > >     OFPERR_OFPBIC_UNSUP_EXP_INST,
> > > > 
> > > > in your patch becomes:
> > > > 
> > > >     /* OF1.1only(3,5).  Specific experimenter instruction unsupported. */
> > > >     OFPERR_OFPBIC_UNSUP_EXP_INST,
> > > > 
> > > >     /* OF1.2(3,5).  Unknown experimenter id specified. */
> > > >     OFPERR_OFPBIC_BAD_EXPERIMENTER,
> > > > 
> > > >     /* OF1.2(3,6).  Unknown instruction for experimenter id. */
> > > >     OFPERR_OFPBIC_BAD_EXP_TYPE,
> > > > 
> > > > but it would more usefully become:
> > > > 
> > > >     /* OF1.1only(3,5), OF1.2(3,5).  Unknown experimenter id specified. */
> > > >     OFPERR_OFPBIC_BAD_EXPERIMENTER,
> > > > 
> > > >     /* OF1.1only(3,5), OF1.2(3,6).  Unknown instruction for experimenter id. */
> > > >     OFPERR_OFPBIC_BAD_EXP_TYPE,
> > > > 
> > > > that is, both of the new error codes would get translated to a single
> > > > OF1.1 error code.  My guess is that the little python script disallows
> > > > this at the moment (it makes the error codes non-bijective), but we
> > > > should probably add support.
> > > 
> > > I am a little unsure of how you see this translation working,
> > > could you explain a bit further?
> > 
> > ofperr_encode_reply() would translate OFPERR_OFPBIC_BAD_EXPERIMENTER to
> > 3,5 for OF1.1 and OF1.2.
> > 
> > ofperr_encode_reply() would translate OFPERR_OFPBIC_BAD_EXP_TYPE to
> > 3,5 for OF1.1 and to 3,6 for OF1.2.
> > 
> > The only difficulty is translation the other direction: should
> > ofperr_decode(of1.1, 3, 5) return OFPERR_OFPBIC_BAD_EXPERIMENTER or
> > OFPERR_OFPBIC_BAD_EXP_TYPE?  The answer may not be important.
> 
> I propose that we can handle this by simply using the first value
> that was defined in lib/ofp-errors.h. We can improve on that logic
> if the need arises.
> 
> The following, which applies on top of my series, implements this idea.
> I will merge it into the relevant patch if you think that it is the right
> approach.

I think that it's OK.

Thanks.

> Do you have other examples of "break apart"?

That's the one I saw right away.