On Fri, Apr 08, 2011 at 06:35:22PM -0700, Ethan Jackson wrote:
> The newly created autopath action will be the way OpenFlow
> interacts with the existing bonding infrastructure.

Looks good.

In autopath_parse(), you can omit the extra " " in the first line of:
> +        ovs_fatal(0, "%s: autopath id %d " "is not in valid range "
> +                  "1 to %"PRIu32, s_, id_int, UINT32_MAX);

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

What should happen if the port number is too big for the register?  It
can't happen now with the currently supported datapaths but the range
of port numbers could increase in the future.  At least I think that 
autopath_execute() should do something consistent with too-big port
numbers, maybe by changing
    *reg = (*reg & ~(mask << ofs)) | (ofp_port << ofs);
to
    *reg = (*reg & ~(mask << ofs)) | ((ofp_port & mask) << ofs);
to truncate them.

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

Reply via email to