[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