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. */
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
