* [PATCH] bridge: push blocking slaves to forwarding when turning stp off
@ 2011-12-13 9:36 Vitalii Demianets
2011-12-14 0:16 ` Stephen Hemminger
0 siblings, 1 reply; 6+ messages in thread
From: Vitalii Demianets @ 2011-12-13 9:36 UTC (permalink / raw)
To: netdev; +Cc: Stephen Hemminger, bridge
If there is a slave in blocking state when stp is turned off, that slave will
remain in blocking state for indefinitely long time until interface state
changed. We should push all blocking slaves into forwarding state after
turning stp off.
Signed-off-by: Vitalii Demianets <vitas@nppfactor.kiev.ua>
---
net/bridge/br_stp.c | 5 ++++-
net/bridge/br_stp_if.c | 10 +++++-----
2 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/net/bridge/br_stp.c b/net/bridge/br_stp.c
index dd147d7..aed7e21 100644
--- a/net/bridge/br_stp.c
+++ b/net/bridge/br_stp.c
@@ -415,7 +415,10 @@ void br_port_state_selection(struct net_bridge *br)
} else {
p->config_pending = 0;
p->topology_change_ack = 0;
- br_make_blocking(p);
+ if(br->stp_enabled == BR_NO_STP)
+ br_make_forwarding(p);
+ else
+ br_make_blocking(p);
}
}
diff --git a/net/bridge/br_stp_if.c b/net/bridge/br_stp_if.c
index 19308e3..38d8dd7 100644
--- a/net/bridge/br_stp_if.c
+++ b/net/bridge/br_stp_if.c
@@ -153,14 +153,14 @@ static void br_stp_stop(struct net_bridge *br)
if (br->stp_enabled == BR_USER_STP) {
r = call_usermodehelper(BR_STP_PROG, argv, envp, UMH_WAIT_PROC);
br_info(br, "userspace STP stopped, return code %d\n", r);
-
- /* To start timers on any ports left in blocking */
- spin_lock_bh(&br->lock);
- br_port_state_selection(br);
- spin_unlock_bh(&br->lock);
}
br->stp_enabled = BR_NO_STP;
+
+ /* To push in forwarding state any ports left in blocking */
+ spin_lock_bh(&br->lock);
+ br_port_state_selection(br);
+ spin_unlock_bh(&br->lock);
}
void br_stp_set_enabled(struct net_bridge *br, unsigned long val)
--
1.7.3.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] bridge: push blocking slaves to forwarding when turning stp off
2011-12-13 9:36 [PATCH] bridge: push blocking slaves to forwarding when turning stp off Vitalii Demianets
@ 2011-12-14 0:16 ` Stephen Hemminger
2011-12-14 9:32 ` Vitalii Demianets
0 siblings, 1 reply; 6+ messages in thread
From: Stephen Hemminger @ 2011-12-14 0:16 UTC (permalink / raw)
To: Vitalii Demianets; +Cc: netdev, bridge
On Tue, 13 Dec 2011 11:36:25 +0200
Vitalii Demianets <vitas@nppfactor.kiev.ua> wrote:
> If there is a slave in blocking state when stp is turned off, that slave will
> remain in blocking state for indefinitely long time until interface state
> changed. We should push all blocking slaves into forwarding state after
> turning stp off.
>
> Signed-off-by: Vitalii Demianets <vitas@nppfactor.kiev.ua>
Maybe. But if the port was in the blocking state then STP must have
decided there was a loop in the network if that port was used.
Therefore blindly putting the port into forwarding state could cause
disastrous network flood.
The user can force the port back out of blocking state (via sysfs).
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] bridge: push blocking slaves to forwarding when turning stp off
2011-12-14 0:16 ` Stephen Hemminger
@ 2011-12-14 9:32 ` Vitalii Demianets
2011-12-20 10:59 ` Vitalii Demianets
0 siblings, 1 reply; 6+ messages in thread
From: Vitalii Demianets @ 2011-12-14 9:32 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, bridge
On Wednesday 14 December 2011 02:16:13 Stephen Hemminger wrote:
> On Tue, 13 Dec 2011 11:36:25 +0200
>
> Vitalii Demianets <vitas@nppfactor.kiev.ua> wrote:
> > If there is a slave in blocking state when stp is turned off, that slave
> > will remain in blocking state for indefinitely long time until interface
> > state changed. We should push all blocking slaves into forwarding state
> > after turning stp off.
> >
> > Signed-off-by: Vitalii Demianets <vitas@nppfactor.kiev.ua>
>
> Maybe. But if the port was in the blocking state then STP must have
> decided there was a loop in the network if that port was used.
> Therefore blindly putting the port into forwarding state could cause
> disastrous network flood.
>
>
> The user can force the port back out of blocking state (via sysfs).
>
1) That blocking state in the absence of STP is not stable. It will eventually
flip to forwarding sooner or later on the first call of
br_port_state_selection(). For example, when user changes MAC address on
another slave. Or even worse - when any other slave of the bridge changes its
carrier state. Don't think user wants such unpredictable state changes.
2) There is also another drawback of not pushing ports into forwarding state
after turning off USER_STP mode. Possible scenario is:
a) bridge in USER_STP mode, all ports are in non-forwarding state (blocking,
learning)
b) user turns off STP. Without the patch ports are not advanced to the
forwarding state and are left in the states they are (the timers do not work
because of USER_STP mode)
c) The bridge stays in no-carrier state until something happens (carrier
state transition on one of the slaves, MAC address change etc)
You can say again that in the above two cases user can manually set the state
of the slaves to forwarding. But to account all possible cases one should
always unconditionally do it for all the slaves each time when stp is being
turned off. So why not to automate the task?
3) The initial intention of the code in br_stp_stop() was to get ports out of
blocking state when stp is being turned off. It fails to achieve the goal,
and patch just fixes it.
4) If user turns stp off he clearly indicates that she wants all ports to work
in stateless mode and that he will deal with possible network loops on
himself. Should we in that case guess network topology basing on loose
assumptions and leave ports in unstable blocking state (and they will flip
eventually to the forwarding state in unpredictable times as mentioned
above)?
--
Vitalii Demianets
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] bridge: push blocking slaves to forwarding when turning stp off
2011-12-14 9:32 ` Vitalii Demianets
@ 2011-12-20 10:59 ` Vitalii Demianets
2011-12-20 18:11 ` Stephen Hemminger
0 siblings, 1 reply; 6+ messages in thread
From: Vitalii Demianets @ 2011-12-20 10:59 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, bridge
Hello, Stephen!
I can not understand your silence.
There are issues fixed by the patch in question. For example, if the interface
is left in blocking state after stp was turned off, that state is not
stable - it can flip to forwarding state in unpredictable times, e.g. when
_any other_ slave of the bridge goes up or down. Do you think user wants
exactly that unpredictable state change?
Also, the code in question in function br_stp_stop(), namely
br_port_state_selection(br) call, does exactly nothing except wasting cpu
cycles. Isn't it worth fixing?
--
Vitalii Demianets
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] bridge: push blocking slaves to forwarding when turning stp off
2011-12-20 10:59 ` Vitalii Demianets
@ 2011-12-20 18:11 ` Stephen Hemminger
2011-12-20 18:27 ` Vitalii Demianets
0 siblings, 1 reply; 6+ messages in thread
From: Stephen Hemminger @ 2011-12-20 18:11 UTC (permalink / raw)
To: Vitalii Demianets; +Cc: netdev, bridge, Michael Tremer
On Tue, 20 Dec 2011 12:59:11 +0200
Vitalii Demianets <vitas@nppfactor.kiev.ua> wrote:
> Hello, Stephen!
> I can not understand your silence.
> There are issues fixed by the patch in question. For example, if the interface
> is left in blocking state after stp was turned off, that state is not
> stable - it can flip to forwarding state in unpredictable times, e.g. when
> _any other_ slave of the bridge goes up or down. Do you think user wants
> exactly that unpredictable state change?
> Also, the code in question in function br_stp_stop(), namely
> br_port_state_selection(br) call, does exactly nothing except wasting cpu
> cycles. Isn't it worth fixing?
>
I had to go do real work last week.
Let me test and look at it more detail.
There is no urgency, the problem has existed for many years.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] bridge: push blocking slaves to forwarding when turning stp off
2011-12-20 18:11 ` Stephen Hemminger
@ 2011-12-20 18:27 ` Vitalii Demianets
0 siblings, 0 replies; 6+ messages in thread
From: Vitalii Demianets @ 2011-12-20 18:27 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, bridge
On Tuesday 20 December 2011 20:11:17 Stephen Hemminger wrote:
> I had to go do real work last week.
> Let me test and look at it more detail.
> There is no urgency, the problem has existed for many years.
Please excuse me. It was only my impatience and "deferred" status on the
David's patchwork site that worried me.
I agree that problem is not urgent.
--
Vitalii Demianets
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-12-20 18:27 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-12-13 9:36 [PATCH] bridge: push blocking slaves to forwarding when turning stp off Vitalii Demianets
2011-12-14 0:16 ` Stephen Hemminger
2011-12-14 9:32 ` Vitalii Demianets
2011-12-20 10:59 ` Vitalii Demianets
2011-12-20 18:11 ` Stephen Hemminger
2011-12-20 18:27 ` Vitalii Demianets
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).