[ovs-dev] [PATCH 3/3] lacp: Implement custom timing mode.

Ethan Jackson ethan at nicira.com
Tue Apr 19 17:49:22 UTC 2011


> It seems to me that there are several instances of code that is
> something like
>   lacp_time == LACP_TIME_CUSTOM ? lacp->custom_time
>   : lacp_time == LACP_TIME_FAST ? LACP_FAST_TIME_TX
>   : LACP_SLOW_TIME_TX
> I'd suggest using a helper function or a member in struct lacp to
> avoid the repetition.  Even two helpers or members, if you want both rx
> and tx rates.

I actually thought about doing this, but it turns out that we only do
this in the code once.  There are three sections of the code I think
you are referencing.  The first is a switch statement in
lacp_process_pdu which does exactly as you describe.  The second is in
lacp_run().  It's similar to what you describe, however if the timing
mode isn't LACP_TIME_CUSTOM then it chooses FAST or SLOW time based on
its *partners* state, not based on the lacp_time field in the lacp
structure. The third is in slave_set_expired() which uses always uses
LACP_FAST_TIME unless in custom timing mode.  Each case is different
enough that I don't think they can be cleanly shoved into one
function.  I may be wrong though, I can think about it more.

I'll incorporate the rest of the comments into the patch, and pending
a response to this email merge.

Ethan



More information about the dev mailing list