* [PATCH 1/2] Issue NETDEV_CHANGE notification when bridge changes state
@ 2011-03-06 5:18 Adam Majer
2011-03-06 5:20 ` [PATCH 2/2] Retry autoconfiguration on interface after NETDEV_CHANGE notification Adam Majer
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Adam Majer @ 2011-03-06 5:18 UTC (permalink / raw
To: netdev
Cc: Stephen Hemminger, David S. Miller, Alexey Kuznetsov,
Pekka Savola (ipv6), James Morris, Hideaki YOSHIFUJI,
Patrick McHardy, bridge, linux-kernel
IPv6 address autoconfiguration relies on an UP interface to processes
traffic normally. A bridge that is UP enters LEARNING state and this
state "eats" any Router Advertising packets sent to the
bridge. Issuing a NETDEV_CHANGE notification on the bridge interface
when it changes state allows autoconfiguration code to retry
querying router information.
Signed-off-by: Adam Majer <adamm@zombino.com>
---
net/bridge/br_if.c | 20 ++++++++++++++++++++
net/bridge/br_private.h | 2 ++
net/bridge/br_stp.c | 2 ++
3 files changed, 24 insertions(+), 0 deletions(-)
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index d9d1e2b..9604d52 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -25,6 +25,22 @@
#include "br_private.h"
+
+/*
+ * Notifies that the bridge has changed state
+ */
+static void br_port_change_notifier_handler(struct work_struct *work)
+{
+ struct net_bridge *br = container_of(work,
+ struct net_bridge,
+ change_notification_worker);
+
+ rtnl_lock();
+ netdev_state_change(br->dev);
+ rtnl_unlock();
+}
+
+
/*
* Determine initial path cost based on speed.
* using recommendations from 802.1d standard
@@ -163,6 +179,7 @@ static void del_br(struct net_bridge *br, struct list_head *head)
{
struct net_bridge_port *p, *n;
+ flush_work(&br->change_notification_worker);
list_for_each_entry_safe(p, n, &br->port_list, list) {
del_nbp(p);
}
@@ -203,6 +220,9 @@ static struct net_device *new_bridge_dev(struct net *net, const char *name)
memcpy(br->group_addr, br_group_address, ETH_ALEN);
+ INIT_WORK(&br->change_notification_worker,
+ &br_port_change_notifier_handler);
+
br->feature_mask = dev->features;
br->stp_enabled = BR_NO_STP;
br->designated_root = br->bridge_id;
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 4e1b620..7f8ef41 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -179,6 +179,8 @@ struct net_bridge
struct list_head port_list;
struct net_device *dev;
+ struct work_struct change_notification_worker;
+
struct br_cpu_netstats __percpu *stats;
spinlock_t hash_lock;
struct hlist_head hash[BR_HASH_SIZE];
diff --git a/net/bridge/br_stp.c b/net/bridge/br_stp.c
index 57186d8..b5be3e3 100644
--- a/net/bridge/br_stp.c
+++ b/net/bridge/br_stp.c
@@ -296,6 +296,8 @@ void br_topology_change_detection(struct net_bridge *br)
{
int isroot = br_is_root_bridge(br);
+ queue_work(system_long_wq, &br->change_notification_worker);
+
if (br->stp_enabled != BR_KERNEL_STP)
return;
--
1.7.2.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] Retry autoconfiguration on interface after NETDEV_CHANGE notification
2011-03-06 5:18 [PATCH 1/2] Issue NETDEV_CHANGE notification when bridge changes state Adam Majer
@ 2011-03-06 5:20 ` Adam Majer
2011-03-06 5:31 ` [PATCH 1/2] Issue NETDEV_CHANGE notification when bridge changes state Adam Majer
2011-03-09 15:09 ` Américo Wang
2 siblings, 0 replies; 5+ messages in thread
From: Adam Majer @ 2011-03-06 5:20 UTC (permalink / raw
To: netdev
Cc: Stephen Hemminger, David S. Miller, Alexey Kuznetsov,
Pekka Savola (ipv6), James Morris, Hideaki YOSHIFUJI,
Patrick McHardy, bridge, linux-kernel
A bridged interface will timeout trying to receive Router Advert as
these packets are not forwarded when bridge is UP but not in the
FORWARDING state (eg. LEARNING state). Bridge code issues
NETDEV_CHANGE when bridge's internal state is changed. It is then
possible to retry Router Solicitation.
Signed-off-by: Adam Majer <adamm@zombino.com>
---
net/ipv6/addrconf.c | 74 +++++++++++++++++++++++++++++++++-----------------
1 files changed, 49 insertions(+), 25 deletions(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index fd6782e..f8018b3 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -152,6 +152,7 @@ static void addrconf_dad_start(struct inet6_ifaddr *ifp, u32 flags);
static void addrconf_dad_timer(unsigned long data);
static void addrconf_dad_completed(struct inet6_ifaddr *ifp);
static void addrconf_dad_run(struct inet6_dev *idev);
+static void addrconf_rs_start(struct inet6_ifaddr *ifp);
static void addrconf_rs_timer(unsigned long data);
static void __ipv6_ifa_notify(int event, struct inet6_ifaddr *ifa);
static void ipv6_ifa_notify(int event, struct inet6_ifaddr *ifa);
@@ -2368,6 +2369,18 @@ static void addrconf_add_linklocal(struct inet6_dev *idev, struct in6_addr *addr
addrconf_prefix_route(&ifp->addr, ifp->prefix_len, idev->dev, 0, 0);
addrconf_dad_start(ifp, 0);
in6_ifa_put(ifp);
+ } else if (PTR_ERR(ifp) == -EEXIST &&
+ list_is_singular(&idev->addr_list)) {
+ /*
+ * first address must be link local, but no router - re-solicit.
+ * This code path is called when bridge exits LEARNING state
+ */
+
+ ifp = list_first_entry(&idev->addr_list,
+ struct inet6_ifaddr,
+ if_list);
+
+ addrconf_rs_start(ifp);
}
}
@@ -2532,8 +2545,11 @@ static int addrconf_notify(struct notifier_block *this, unsigned long event,
}
if (idev) {
- if (idev->if_flags & IF_READY)
- /* device is already configured. */
+ if (idev->if_flags & IF_READY &&
+ idev->if_flags & IF_RA_RCVD)
+ /* device is already configured and
+ * RA was received.
+ */
break;
idev->if_flags |= IF_READY;
}
@@ -2775,6 +2791,35 @@ static int addrconf_ifdown(struct net_device *dev, int how)
return 0;
}
+static void addrconf_rs_start(struct inet6_ifaddr *ifp)
+{
+ /* If added prefix is link local and forwarding is off,
+ start sending router solicitations.
+ */
+ struct net_device *dev = ifp->idev->dev;
+
+ if ((ifp->idev->cnf.forwarding == 0 ||
+ ifp->idev->cnf.forwarding == 2) &&
+ ifp->idev->cnf.rtr_solicits > 0 &&
+ (dev->flags&IFF_LOOPBACK) == 0 &&
+ (ipv6_addr_type(&ifp->addr) & IPV6_ADDR_LINKLOCAL)) {
+ /*
+ * If a host as already performed a random delay
+ * [...] as part of DAD [...] there is no need
+ * to delay again before sending the first RS
+ */
+ ndisc_send_rs(ifp->idev->dev, &ifp->addr,
+ &in6addr_linklocal_allrouters);
+
+ spin_lock_bh(&ifp->lock);
+ ifp->probes = 1;
+ ifp->idev->if_flags |= IF_RS_SENT;
+ addrconf_mod_timer(ifp, AC_RS,
+ ifp->idev->cnf.rtr_solicit_interval);
+ spin_unlock_bh(&ifp->lock);
+ }
+}
+
static void addrconf_rs_timer(unsigned long data)
{
struct inet6_ifaddr *ifp = (struct inet6_ifaddr *) data;
@@ -2935,36 +2980,15 @@ out:
static void addrconf_dad_completed(struct inet6_ifaddr *ifp)
{
- struct net_device *dev = ifp->idev->dev;
-
/*
* Configure the address for reception. Now it is valid.
*/
ipv6_ifa_notify(RTM_NEWADDR, ifp);
- /* If added prefix is link local and forwarding is off,
- start sending router solicitations.
- */
+ /* start sending router solicitations. */
- if ((ifp->idev->cnf.forwarding == 0 ||
- ifp->idev->cnf.forwarding == 2) &&
- ifp->idev->cnf.rtr_solicits > 0 &&
- (dev->flags&IFF_LOOPBACK) == 0 &&
- (ipv6_addr_type(&ifp->addr) & IPV6_ADDR_LINKLOCAL)) {
- /*
- * If a host as already performed a random delay
- * [...] as part of DAD [...] there is no need
- * to delay again before sending the first RS
- */
- ndisc_send_rs(ifp->idev->dev, &ifp->addr, &in6addr_linklocal_allrouters);
-
- spin_lock_bh(&ifp->lock);
- ifp->probes = 1;
- ifp->idev->if_flags |= IF_RS_SENT;
- addrconf_mod_timer(ifp, AC_RS, ifp->idev->cnf.rtr_solicit_interval);
- spin_unlock_bh(&ifp->lock);
- }
+ addrconf_rs_start(ifp);
}
static void addrconf_dad_run(struct inet6_dev *idev)
--
1.7.2.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] Issue NETDEV_CHANGE notification when bridge changes state
2011-03-06 5:18 [PATCH 1/2] Issue NETDEV_CHANGE notification when bridge changes state Adam Majer
2011-03-06 5:20 ` [PATCH 2/2] Retry autoconfiguration on interface after NETDEV_CHANGE notification Adam Majer
@ 2011-03-06 5:31 ` Adam Majer
2011-03-09 15:09 ` Américo Wang
2 siblings, 0 replies; 5+ messages in thread
From: Adam Majer @ 2011-03-06 5:31 UTC (permalink / raw
To: netdev
Cc: Stephen Hemminger, David S. Miller, Alexey Kuznetsov,
Pekka Savola (ipv6), James Morris, Hideaki YOSHIFUJI,
Patrick McHardy, bridge, linux-kernel
Hello,
As background for these 2 patches, the current bridge code does not
play very nicely with IPv6 autoconfiguration. What happens is,
[ 35.117030] device eth0 entered promiscuous mode
[ 35.120898] br0: port 1(eth0) entering learning state
[ 35.120901] br0: port 1(eth0) entering learning state
Here bridge is setup and both interface eth0 and br0 are indicated as
up via NETDEV_UP notification. The IPv6 autoconfiguration code
configures linklocal address and start to issue RS packets. These
timeout because bridge is in learning state, not forwarding state.
[ 45.168007] br0: no IPv6 routers present
[ 45.312020] eth0: no IPv6 routers present
RS timeouts and IPv6 is abandoned at this point.
[ 50.144011] br0: port 1(eth0) entering forwarding state
Now bridge entered forwarding state. Userland sets up IPv4 via DHCP or
otherwise.
[ 50.144487] ADDRCONF(NETDEV_CHANGE): br0: link becomes ready
With the patches, NETDEV_CHANGE is issued by the bridge once it
changes state. The autoconfiguration code then checks that no RA
packets are received and only linklocal interface is setup. It then
retries soliciting router information.
This is my first attempt at a kernel patch so please be gentle :) Part
of the autoconf patch splits out the addrconf_rs_start() code from the
addrconf_dad_completed() function.
Cheers,
Adam
--
Adam Majer
adamm@zombino.com
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] Issue NETDEV_CHANGE notification when bridge changes state
2011-03-06 5:18 [PATCH 1/2] Issue NETDEV_CHANGE notification when bridge changes state Adam Majer
2011-03-06 5:20 ` [PATCH 2/2] Retry autoconfiguration on interface after NETDEV_CHANGE notification Adam Majer
2011-03-06 5:31 ` [PATCH 1/2] Issue NETDEV_CHANGE notification when bridge changes state Adam Majer
@ 2011-03-09 15:09 ` Américo Wang
2011-03-09 16:44 ` Adam Majer
2 siblings, 1 reply; 5+ messages in thread
From: Américo Wang @ 2011-03-09 15:09 UTC (permalink / raw
To: Adam Majer
Cc: netdev, Stephen Hemminger, David S. Miller, Alexey Kuznetsov,
Pekka Savola (ipv6), James Morris, Hideaki YOSHIFUJI,
Patrick McHardy, bridge, linux-kernel
On Sat, Mar 05, 2011 at 11:18:33PM -0600, Adam Majer wrote:
>
>IPv6 address autoconfiguration relies on an UP interface to processes
>traffic normally. A bridge that is UP enters LEARNING state and this
>state "eats" any Router Advertising packets sent to the
>bridge. Issuing a NETDEV_CHANGE notification on the bridge interface
>when it changes state allows autoconfiguration code to retry
>querying router information.
>
...
>+static void br_port_change_notifier_handler(struct work_struct *work)
>+{
>+ struct net_bridge *br = container_of(work,
>+ struct net_bridge,
>+ change_notification_worker);
>+
>+ rtnl_lock();
>+ netdev_state_change(br->dev);
>+ rtnl_unlock();
>+}
>+
Do you really want user-space to get this notification too?
Why do you put it into a workqueue? Maybe it has to be called in
process-context?
Thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] Issue NETDEV_CHANGE notification when bridge changes state
2011-03-09 15:09 ` Américo Wang
@ 2011-03-09 16:44 ` Adam Majer
0 siblings, 0 replies; 5+ messages in thread
From: Adam Majer @ 2011-03-09 16:44 UTC (permalink / raw
To: Américo Wang; +Cc: netdev, Stephen Hemminger, bridge, linux-kernel
On Wed, Mar 09, 2011 at 11:09:49PM +0800, Américo Wang wrote:
> On Sat, Mar 05, 2011 at 11:18:33PM -0600, Adam Majer wrote:
> >
> >IPv6 address autoconfiguration relies on an UP interface to processes
> >traffic normally. A bridge that is UP enters LEARNING state and this
> >state "eats" any Router Advertising packets sent to the
> >bridge. Issuing a NETDEV_CHANGE notification on the bridge interface
> >when it changes state allows autoconfiguration code to retry
> >querying router information.
> >
> ...
>
> >+static void br_port_change_notifier_handler(struct work_struct *work)
> >+{
> >+ struct net_bridge *br = container_of(work,
> >+ struct net_bridge,
> >+ change_notification_worker);
> >+
> >+ rtnl_lock();
> >+ netdev_state_change(br->dev);
> >+ rtnl_unlock();
> >+}
> >+
>
> Do you really want user-space to get this notification too?
> Why do you put it into a workqueue? Maybe it has to be called in
> process-context?
Stephen Hemminger's patch (posted as a reply in this thread but only
on the netdev mailing list) is much better as it only brings the link
up on the bridge interface when the bridge enters forwarding mode.
Effectively it does similar thing via NETDEV_CHANGE but requires no
other hacks in other code. It should make userland much happier too.
As for my patch, I've put that in the workqueue because the
notification cannot be called in interrupt context. Is there a better
way of calling code that can sleep from interrupt context?
- Adam
--
Adam Majer
adamm@zombino.com
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-03-09 16:45 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-03-06 5:18 [PATCH 1/2] Issue NETDEV_CHANGE notification when bridge changes state Adam Majer
2011-03-06 5:20 ` [PATCH 2/2] Retry autoconfiguration on interface after NETDEV_CHANGE notification Adam Majer
2011-03-06 5:31 ` [PATCH 1/2] Issue NETDEV_CHANGE notification when bridge changes state Adam Majer
2011-03-09 15:09 ` Américo Wang
2011-03-09 16:44 ` Adam Majer
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 read-only IMAP folder(s) and NNTP newsgroup(s).