[ovs-dev] [PATCH] autopath: Create the autopath action.

Ethan Jackson ethan at nicira.com
Mon Apr 11 17:48:41 UTC 2011


> autopath_parse() rejects nonpositive 'id' values but the
> NXAST_AUTOPATH action description doesn't say that only positive
> values are valid.

I'm not sure here whether the correct thing to do is document that
positive ID  values are required, or loosen the requirement.  From the
action's perspective, it's just a 4byte id value.  It doesn't
particularly care if that's interpreted as a two's compliment integer,
or an unsigned integer. It's just used as a key.  Do you have a
preference?

>    *reg = (*reg & ~(mask << ofs)) | (ofp_port << ofs);
> to
>    *reg = (*reg & ~(mask << ofs)) | ((ofp_port & mask) << ofs);

Good idea.  I made this change.

> I see that OFPP_NONE doesn't fit in 10 bits (unless you regard it as a
> negative number, -1 instead of 65535 (in which case OFPP_NONE ==
> ODPP_NONE)).
>
> Maybe we should just require at least 16 bits for the register?  That
> should help for a while.

I'm fine with that.  16 is a cleaner number anyways.  I changed it in the patch.

Ethan



More information about the dev mailing list