* [PATCH net-next] bridge: respect RFC2863 operational state
@ 2012-12-28 6:28 Stephen Hemminger
2012-12-28 8:40 ` Jiri Pirko
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Stephen Hemminger @ 2012-12-28 6:28 UTC (permalink / raw)
To: David Miller; +Cc: netdev
The bridge link detection should follow the operational state
of the lower device, rather than the carrier bit. This allows devices
like tunnels that are controlled by userspace control plane to work
with bridge STP link management.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
--- a/net/bridge/br_if.c 2012-10-25 09:11:15.627272524 -0700
+++ b/net/bridge/br_if.c 2012-12-14 08:58:14.329847361 -0800
@@ -66,14 +66,14 @@ void br_port_carrier_check(struct net_br
struct net_device *dev = p->dev;
struct net_bridge *br = p->br;
- if (netif_running(dev) && netif_carrier_ok(dev))
+ if (netif_running(dev) && netif_oper_up(dev))
p->path_cost = port_cost(dev);
if (!netif_running(br->dev))
return;
spin_lock_bh(&br->lock);
- if (netif_running(dev) && netif_carrier_ok(dev)) {
+ if (netif_running(dev) && netif_oper_up(dev))
if (p->state == BR_STATE_DISABLED)
br_stp_enable_port(p);
} else {
--- a/net/bridge/br_notify.c 2012-10-25 09:11:15.631272484 -0700
+++ b/net/bridge/br_notify.c 2012-12-14 08:57:36.954222724 -0800
@@ -82,7 +82,7 @@ static int br_device_event(struct notifi
break;
case NETDEV_UP:
- if (netif_carrier_ok(dev) && (br->dev->flags & IFF_UP)) {
+ if (netif_running(br->dev) && netif_oper_up(dev)) {
spin_lock_bh(&br->lock);
br_stp_enable_port(p);
spin_unlock_bh(&br->lock);
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next] bridge: respect RFC2863 operational state
2012-12-28 6:28 [PATCH net-next] bridge: respect RFC2863 operational state Stephen Hemminger
@ 2012-12-28 8:40 ` Jiri Pirko
2012-12-28 13:03 ` Flavio Leitner
2012-12-28 23:27 ` David Miller
2 siblings, 0 replies; 7+ messages in thread
From: Jiri Pirko @ 2012-12-28 8:40 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David Miller, netdev
Fri, Dec 28, 2012 at 07:28:54AM CET, shemminger@vyatta.com wrote:
>The bridge link detection should follow the operational state
>of the lower device, rather than the carrier bit. This allows devices
>like tunnels that are controlled by userspace control plane to work
>with bridge STP link management.
>
>
>Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
>
>
>--- a/net/bridge/br_if.c 2012-10-25 09:11:15.627272524 -0700
>+++ b/net/bridge/br_if.c 2012-12-14 08:58:14.329847361 -0800
>@@ -66,14 +66,14 @@ void br_port_carrier_check(struct net_br
> struct net_device *dev = p->dev;
> struct net_bridge *br = p->br;
>
>- if (netif_running(dev) && netif_carrier_ok(dev))
>+ if (netif_running(dev) && netif_oper_up(dev))
> p->path_cost = port_cost(dev);
>
> if (!netif_running(br->dev))
> return;
>
> spin_lock_bh(&br->lock);
>- if (netif_running(dev) && netif_carrier_ok(dev)) {
>+ if (netif_running(dev) && netif_oper_up(dev))
> if (p->state == BR_STATE_DISABLED)
> br_stp_enable_port(p);
> } else {
>--- a/net/bridge/br_notify.c 2012-10-25 09:11:15.631272484 -0700
>+++ b/net/bridge/br_notify.c 2012-12-14 08:57:36.954222724 -0800
>@@ -82,7 +82,7 @@ static int br_device_event(struct notifi
> break;
>
> case NETDEV_UP:
>- if (netif_carrier_ok(dev) && (br->dev->flags & IFF_UP)) {
>+ if (netif_running(br->dev) && netif_oper_up(dev)) {
> spin_lock_bh(&br->lock);
> br_stp_enable_port(p);
> spin_unlock_bh(&br->lock);
>--
>To unsubscribe from this list: send the line "unsubscribe netdev" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at http://vger.kernel.org/majordomo-info.html
Reviewed-by: Jiri Pirko <jiri@resnulli.us>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next] bridge: respect RFC2863 operational state
2012-12-28 6:28 [PATCH net-next] bridge: respect RFC2863 operational state Stephen Hemminger
2012-12-28 8:40 ` Jiri Pirko
@ 2012-12-28 13:03 ` Flavio Leitner
2012-12-28 23:27 ` David Miller
2 siblings, 0 replies; 7+ messages in thread
From: Flavio Leitner @ 2012-12-28 13:03 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David Miller, netdev
On Thu, Dec 27, 2012 at 10:28:54PM -0800, Stephen Hemminger wrote:
> The bridge link detection should follow the operational state
> of the lower device, rather than the carrier bit. This allows devices
> like tunnels that are controlled by userspace control plane to work
> with bridge STP link management.
>
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
>
>
> --- a/net/bridge/br_if.c 2012-10-25 09:11:15.627272524 -0700
> +++ b/net/bridge/br_if.c 2012-12-14 08:58:14.329847361 -0800
> @@ -66,14 +66,14 @@ void br_port_carrier_check(struct net_br
> struct net_device *dev = p->dev;
> struct net_bridge *br = p->br;
>
> - if (netif_running(dev) && netif_carrier_ok(dev))
> + if (netif_running(dev) && netif_oper_up(dev))
> p->path_cost = port_cost(dev);
>
> if (!netif_running(br->dev))
> return;
>
> spin_lock_bh(&br->lock);
> - if (netif_running(dev) && netif_carrier_ok(dev)) {
> + if (netif_running(dev) && netif_oper_up(dev))
> if (p->state == BR_STATE_DISABLED)
> br_stp_enable_port(p);
I found this piece still using netif_carrier_ok():
321 int br_add_if(struct net_bridge *br, struct net_device *dev)
322 {
...
385
386 if ((dev->flags & IFF_UP) && netif_carrier_ok(dev) &&
387 (br->dev->flags & IFF_UP))
388 br_stp_enable_port(p);
389 spin_unlock_bh(&br->lock);
390
Is there any reason for enabling stp on a port using operstate
in br_port_carrier_check() but not in br_add_if() ?
The same thing happens with br_stp_enable_bridge():
56 list_for_each_entry(p, &br->port_list, list) {
57 if ((p->dev->flags & IFF_UP) && netif_carrier_ok(p->dev))
58 br_stp_enable_port(p);
Also, as operstate UP means that packets are flowing, there is no need to
check if the device is opened, so checking only for operstate should be
enough, right? I.e.
- if ((p->dev->flags & IFF_UP) && netif_carrier_ok(p->dev))
+ if (netif_oper_up(dev))
> } else {
> --- a/net/bridge/br_notify.c 2012-10-25 09:11:15.631272484 -0700
> +++ b/net/bridge/br_notify.c 2012-12-14 08:57:36.954222724 -0800
> @@ -82,7 +82,7 @@ static int br_device_event(struct notifi
> break;
>
> case NETDEV_UP:
> - if (netif_carrier_ok(dev) && (br->dev->flags & IFF_UP)) {
> + if (netif_running(br->dev) && netif_oper_up(dev)) {
> spin_lock_bh(&br->lock);
> br_stp_enable_port(p);
> spin_unlock_bh(&br->lock);
You are not just changing to use operstate, but also to check another
flag - before it was IFF_UP and now __LINK_STATE_START. Any reason
for that besides being consistent with other checks?
thanks!
--
fbl
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next] bridge: respect RFC2863 operational state
2012-12-28 6:28 [PATCH net-next] bridge: respect RFC2863 operational state Stephen Hemminger
2012-12-28 8:40 ` Jiri Pirko
2012-12-28 13:03 ` Flavio Leitner
@ 2012-12-28 23:27 ` David Miller
2012-12-29 4:15 ` [PATCHv2 " Stephen Hemminger
2 siblings, 1 reply; 7+ messages in thread
From: David Miller @ 2012-12-28 23:27 UTC (permalink / raw)
To: shemminger; +Cc: netdev
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Thu, 27 Dec 2012 22:28:54 -0800
> spin_lock_bh(&br->lock);
> - if (netif_running(dev) && netif_carrier_ok(dev)) {
> + if (netif_running(dev) && netif_oper_up(dev))
> if (p->state == BR_STATE_DISABLED)
> br_stp_enable_port(p);
> } else {
You didn't even try to compile this.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCHv2 net-next] bridge: respect RFC2863 operational state
2012-12-28 23:27 ` David Miller
@ 2012-12-29 4:15 ` Stephen Hemminger
2012-12-30 2:01 ` Flavio Leitner
2012-12-30 10:32 ` David Miller
0 siblings, 2 replies; 7+ messages in thread
From: Stephen Hemminger @ 2012-12-29 4:15 UTC (permalink / raw)
To: David Miller; +Cc: netdev
The bridge link detection should follow the operational state
of the lower device, rather than the carrier bit. This allows devices
like tunnels that are controlled by userspace control plane to work
with bridge STP link management.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
---
v2 - add more places and fix typo
net/bridge/br_if.c | 6 +++---
net/bridge/br_netlink.c | 5 ++++-
net/bridge/br_notify.c | 2 +-
net/bridge/br_stp_if.c | 2 +-
4 files changed, 9 insertions(+), 6 deletions(-)
--- a/net/bridge/br_if.c 2012-12-27 22:30:53.634796003 -0800
+++ b/net/bridge/br_if.c 2012-12-28 19:54:14.198973683 -0800
@@ -66,14 +66,14 @@ void br_port_carrier_check(struct net_br
struct net_device *dev = p->dev;
struct net_bridge *br = p->br;
- if (netif_running(dev) && netif_carrier_ok(dev))
+ if (netif_running(dev) && netif_oper_up(dev))
p->path_cost = port_cost(dev);
if (!netif_running(br->dev))
return;
spin_lock_bh(&br->lock);
- if (netif_running(dev) && netif_carrier_ok(dev)) {
+ if (netif_running(dev) && netif_oper_up(dev)) {
if (p->state == BR_STATE_DISABLED)
br_stp_enable_port(p);
} else {
@@ -383,7 +383,7 @@ int br_add_if(struct net_bridge *br, str
spin_lock_bh(&br->lock);
changed_addr = br_stp_recalculate_bridge_id(br);
- if ((dev->flags & IFF_UP) && netif_carrier_ok(dev) &&
+ if (netif_running(dev) && netif_oper_up(dev) &&
(br->dev->flags & IFF_UP))
br_stp_enable_port(p);
spin_unlock_bh(&br->lock);
--- a/net/bridge/br_notify.c 2012-12-27 22:30:23.547098173 -0800
+++ b/net/bridge/br_notify.c 2012-12-28 19:47:57.022761664 -0800
@@ -82,7 +82,7 @@ static int br_device_event(struct notifi
break;
case NETDEV_UP:
- if (netif_carrier_ok(dev) && (br->dev->flags & IFF_UP)) {
+ if (netif_running(br->dev) && netif_oper_up(dev)) {
spin_lock_bh(&br->lock);
br_stp_enable_port(p);
spin_unlock_bh(&br->lock);
--- a/net/bridge/br_netlink.c 2012-12-27 11:36:34.813072288 -0800
+++ b/net/bridge/br_netlink.c 2012-12-28 19:52:24.180078602 -0800
@@ -181,8 +181,11 @@ static int br_set_port_state(struct net_
if (p->br->stp_enabled == BR_KERNEL_STP)
return -EBUSY;
+ /* if device is not up, change is not allowed
+ * if link is not present, only allowable state is disabled
+ */
if (!netif_running(p->dev) ||
- (!netif_carrier_ok(p->dev) && state != BR_STATE_DISABLED))
+ (!netif_oper_up(p->dev) && state != BR_STATE_DISABLED))
return -ENETDOWN;
p->state = state;
--- a/net/bridge/br_stp_if.c 2012-10-29 09:07:36.618530493 -0700
+++ b/net/bridge/br_stp_if.c 2012-12-28 19:53:37.207345191 -0800
@@ -54,7 +54,7 @@ void br_stp_enable_bridge(struct net_bri
br_config_bpdu_generation(br);
list_for_each_entry(p, &br->port_list, list) {
- if ((p->dev->flags & IFF_UP) && netif_carrier_ok(p->dev))
+ if (netif_running(p->dev) && netif_oper_up(p->dev))
br_stp_enable_port(p);
}
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCHv2 net-next] bridge: respect RFC2863 operational state
2012-12-29 4:15 ` [PATCHv2 " Stephen Hemminger
@ 2012-12-30 2:01 ` Flavio Leitner
2012-12-30 10:32 ` David Miller
1 sibling, 0 replies; 7+ messages in thread
From: Flavio Leitner @ 2012-12-30 2:01 UTC (permalink / raw)
To: netdev
On Fri, Dec 28, 2012 at 08:15:22PM -0800, Stephen Hemminger wrote:
> The bridge link detection should follow the operational state
> of the lower device, rather than the carrier bit. This allows devices
> like tunnels that are controlled by userspace control plane to work
> with bridge STP link management.
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
>
> ---
Although I can't test this right now, it does look good to me.
Reviewed-by: Flavio Leitner <fbl@redhat.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCHv2 net-next] bridge: respect RFC2863 operational state
2012-12-29 4:15 ` [PATCHv2 " Stephen Hemminger
2012-12-30 2:01 ` Flavio Leitner
@ 2012-12-30 10:32 ` David Miller
1 sibling, 0 replies; 7+ messages in thread
From: David Miller @ 2012-12-30 10:32 UTC (permalink / raw)
To: shemminger; +Cc: netdev
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Fri, 28 Dec 2012 20:15:22 -0800
> The bridge link detection should follow the operational state
> of the lower device, rather than the carrier bit. This allows devices
> like tunnels that are controlled by userspace control plane to work
> with bridge STP link management.
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
Applied.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2012-12-30 10:32 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-12-28 6:28 [PATCH net-next] bridge: respect RFC2863 operational state Stephen Hemminger
2012-12-28 8:40 ` Jiri Pirko
2012-12-28 13:03 ` Flavio Leitner
2012-12-28 23:27 ` David Miller
2012-12-29 4:15 ` [PATCHv2 " Stephen Hemminger
2012-12-30 2:01 ` Flavio Leitner
2012-12-30 10:32 ` David Miller
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).