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

Simon Horman horms at verge.net.au
Thu Mar 22 07:58:53 UTC 2012


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.

Do you have other examples of "break apart"?

diff --git a/build-aux/extract-ofp-errors b/build-aux/extract-ofp-errors
index 765e079..9c84283 100755
--- a/build-aux/extract-ofp-errors
+++ b/build-aux/extract-ofp-errors
@@ -221,8 +221,6 @@ def extract_ofp_errors(filenames):
                 for target in target_map[targets]:
                     if type_ not in domain[target]:
                         domain[target][type_] = {}
-                    if code in domain[target][type_]:
-                        fatal("%s: duplicate assignment in domain" % dst)
                     domain[target][type_][code] = enum
                     reverse[target][enum] = (type_, code)
 
@@ -259,12 +257,17 @@ static enum ofperr
 %s_decode(uint16_t type, uint16_t code)
 {
     switch ((type << 16) | code) {""" % name
+        found = []
         for enum in names:
             if enum not in map:
                 continue
             type_, code = map[enum]
             if code is None:
                 continue
+            value = type_ << 16 | code
+            if value in found:
+                continue
+            found.append(value)
             print "    case (%d << 16) | %d:" % (type_, code)
             print "        return OFPERR_%s;" % enum
         print """\
diff --git a/lib/ofp-errors.h b/lib/ofp-errors.h
index 5062f3e..dc25f38 100644
--- a/lib/ofp-errors.h
+++ b/lib/ofp-errors.h
@@ -219,13 +219,10 @@ enum ofperr {
     /* OF1.1(3,4).  Metadata mask value unsupported by datapath. */
     OFPERR_OFPBIC_UNSUP_METADATA_MASK,
 
-    /* OF1.1only(3,5).  Specific experimenter instruction unsupported. */
-    OFPERR_OFPBIC_UNSUP_EXP_INST,
-
-    /* OF1.2(3,5).  Unknown experimenter id specified. */
+    /* OF1.1only(3,5), OF1.2(3,5).  Unknown experimenter id specified. */
     OFPERR_OFPBIC_BAD_EXPERIMENTER,
 
-    /* OF1.2(3,6).  Unknown instruction for experimenter id. */
+    /* OF1.1only(3,5), OF1.2(3,6).  Unknown instruction for experimenter id. */
     OFPERR_OFPBIC_BAD_EXP_TYPE,
 
     /* OF1.2(3,7).  Length problem in instructions. */



More information about the dev mailing list