[Bridge] [PATCH] BRIDGE: Do not suppress FDB learning after topology change when forward_delay is 0.

richardvoigt at gmail.com richardvoigt at gmail.com
Fri May 15 10:04:28 PDT 2009


On Thu, May 14, 2009 at 10:35 AM, Stephen Hemminger
<shemminger at linux-foundation.org> wrote:
> On Thu, 14 May 2009 12:01:44 +0100
> Ian Campbell <ian.campbell at citrix.com> wrote:
>
>> I recently noticed that my bridges were flooding traffic for a period of time
>> after a topology change. These bridges are part of a Xen host and therefore
>> have spanning tree disabled and forward_delay of zero. In this situation there
>> is no reason not to begin relearning immediately after a topology change.
>>
>> The existing code uses hold_time == 0 to suppress learning in br_fdb_update.
>> hold_time() returns forward_delay if a topology change has recently occurred
>> and ageing_time if not. Setting each of those to zero has slightly different
>> semantics: Setting forward_delay to zero effectively means forward immediately
>> while setting ageing_time to zero effectively means do not learn.
>>
>> The solution is to not learn after a topology change only if forward_delay is
>> non-zero but to retain the existing behaviour based on ageing_time if a
>> topology change has not occured.
>
>
> Unless STP is enabled, br_topology_change is bogus.  It looks like,
> the following would avoid the problem?
>
> --- a/net/bridge/br_stp.c       2009-05-14 08:33:01.795909321 -0700
> +++ b/net/bridge/br_stp.c       2009-05-14 08:34:32.839883992 -0700
> @@ -375,7 +375,8 @@ static void br_make_forwarding(struct ne
>
>        if (br->forward_delay == 0) {
>                p->state = BR_STATE_FORWARDING;
> -               br_topology_change_detection(br);
> +               if (p->br->stp_enable == BR_KERNEL_STP)
> +                       br_topology_change_detection(br);
>                del_timer(&p->forward_delay_timer);
>        }
>        else if (p->br->stp_enabled == BR_KERNEL_STP)

Comparing the new line to the last line of the snippet, it looks like
this change is wrong.  Are there really two members "stp_enable" and
"stp_enabled" of the same structure?

>
>
> _______________________________________________
> Bridge mailing list
> Bridge at lists.linux-foundation.org
> https://lists.linux-foundation.org/mailman/listinfo/bridge
>


More information about the Bridge mailing list