On Fri, Feb 04, 2011 at 06:35:50PM -0800, Ethan Jackson wrote: > Much of the LACP status information attached to an interface is > moved from an enum to a bit mask in this commit. The main reason > to do this is to allow a link to be concurrently expired and > defaulted. With this commit, if a link enters an expired state, > but has never had its partner information update, it will properly > set the defaulted flag in its LACP messages.
Looks good, but I have a few nitpicks. I liked the longer names. They were easier to understand at a glance. Would you mind spelling them out again, e.g. maybe LACP_CURRENT, LCAP_EXPIRED, LACP_DEFAULTED, LACP_ATTACHED? Usually we've been using an enum for this kind of thing, instead of macros. Occasionally this is convenient in GDB, for example. I see a few instances of 'x & a || x & b'. Would you mind writing these as 'x & (a | b)'? I guess that GCC probably outputs exactly the same code in each case but the latter seems more canonical to me. Thanks, Ben. _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev_openvswitch.org
