On 5/19/17 5:48 PM, Florian Westphal wrote:
Nikolay Aleksandrov <niko...@cumulusnetworks.com> wrote:
On 5/19/17 5:20 PM, Xin Long wrote:
Since commit 76b91c32dd86 ("bridge: stp: when using userspace stp stop
kernel hello and hold timers"), bridge would not start hello_timer if
stp_enabled is not KERNEL_STP when br_dev_open.

The problem is even if users set stp_enabled with KERNEL_STP later,
the timer will still not be started. It causes that KERNEL_STP can
not really work. Users have to re-ifup the bridge to avoid this.

This patch is to fix it by starting br->hello_timer when enabling
KERNEL_STP in br_stp_start.

As an improvement, it's also to start hello_timer again only when
br->stp_enabled is KERNEL_STP in br_hello_timer_expired, there is
no reason to start the timer again when it's NO_STP.

Fixes: 76b91c32dd86 ("bridge: stp: when using userspace stp stop kernel hello and 
hold timers")
Reported-by: Haidong Li <ha...@redhat.com>
Signed-off-by: Xin Long <lucien....@gmail.com>
---
  net/bridge/br_stp_if.c    | 1 +
  net/bridge/br_stp_timer.c | 2 +-
  2 files changed, 2 insertions(+), 1 deletion(-)


This doesn't make much sense to me, how do you change from USER_STP to
KERNEL_STP without first going through NO_STP ?

This is easily rerpoduceable via:

ip link add vethin1 type veth peer name vethout1
ip link add vethin2 type veth peer name vethout2

ip link set vethin1 up
ip link set vethin2 up

ip link set vethout1 up
ip link set vethout2 up

brctl addbr br0
brctl addbr br1

brctl stp br0 on
brctl stp br1 on

I think this step with moving NO_STP -> KERNEL_STP should be last, then
I can see how the timer won't be started.


brctl addif br0 vethin1
brctl addif br0 vethin2

brctl addif br1 vethout1
brctl addif br1 vethout2

ip link set br0 up
ip link set br1 up


Reply via email to