From: nhorman@tuxdriver.com
To: netdev@vger.kernel.org
Cc: bonding-devel@lists.sourceforge.net, fubar@us.ibm.com,
davem@davemloft.net, andy@greyhouse.net, amwang@redhat.com,
nhorman@tuxdriver.com
Subject: [PATCH 1/5] Fix bonding drivers improper modification of netpoll structure
Date: Wed, 13 Oct 2010 22:01:49 -0400 [thread overview]
Message-ID: <1287021713-1750-2-git-send-email-nhorman@tuxdriver.com> (raw)
In-Reply-To: <1287021713-1750-1-git-send-email-nhorman@tuxdriver.com>
From: Neil Horman <nhorman@tuxdriver.com>
The bonding driver currently modifies the netpoll structure in its xmit path
while sending frames from netpoll. This is racy, as other cpus can access the
netpoll structure in parallel. Since the bonding driver points np->dev to a
slave device, other cpus can inadvertently attempt to send data directly to
slave devices, leading to improper locking with the bonding master, lost frames,
and deadlocks. This patch fixes that up.
This patch also removes the real_dev pointer from the netpoll structure as that
data is really only used by bonding in the poll_controller, and we can emulate
its behavior by check each slave for IS_UP.
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
---
drivers/net/bonding/bond_main.c | 15 +++++++++------
include/linux/netpoll.h | 9 +++++++--
net/core/netpoll.c | 6 +++---
3 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index a0bf35d..eb7d089 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -449,11 +449,9 @@ int bond_dev_queue_xmit(struct bonding *bond, struct sk_buff *skb,
if (unlikely(bond->dev->priv_flags & IFF_IN_NETPOLL)) {
struct netpoll *np = bond->dev->npinfo->netpoll;
slave_dev->npinfo = bond->dev->npinfo;
- np->real_dev = np->dev = skb->dev;
slave_dev->priv_flags |= IFF_IN_NETPOLL;
- netpoll_send_skb(np, skb);
+ netpoll_send_skb_on_dev(np, skb, slave_dev);
slave_dev->priv_flags &= ~IFF_IN_NETPOLL;
- np->dev = bond->dev;
} else
#endif
dev_queue_xmit(skb);
@@ -1332,9 +1330,14 @@ static bool slaves_support_netpoll(struct net_device *bond_dev)
static void bond_poll_controller(struct net_device *bond_dev)
{
- struct net_device *dev = bond_dev->npinfo->netpoll->real_dev;
- if (dev != bond_dev)
- netpoll_poll_dev(dev);
+ struct bonding *bond = netdev_priv(bond_dev);
+ struct slave *slave;
+ int i;
+
+ bond_for_each_slave(bond, slave, i) {
+ if (slave->dev && IS_UP(slave->dev))
+ netpoll_poll_dev(slave->dev);
+ }
}
static void bond_netpoll_cleanup(struct net_device *bond_dev)
diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h
index 50d8009..79358bb 100644
--- a/include/linux/netpoll.h
+++ b/include/linux/netpoll.h
@@ -14,7 +14,6 @@
struct netpoll {
struct net_device *dev;
- struct net_device *real_dev;
char dev_name[IFNAMSIZ];
const char *name;
void (*rx_hook)(struct netpoll *, int, char *, int);
@@ -53,7 +52,13 @@ void netpoll_set_trap(int trap);
void __netpoll_cleanup(struct netpoll *np);
void netpoll_cleanup(struct netpoll *np);
int __netpoll_rx(struct sk_buff *skb);
-void netpoll_send_skb(struct netpoll *np, struct sk_buff *skb);
+void netpoll_send_skb_on_dev(struct netpoll *np, struct sk_buff *skb,
+ struct net_device *dev);
+static inline void netpoll_send_skb(struct netpoll *np, struct sk_buff *skb)
+{
+ netpoll_send_skb_on_dev(np, skb, np->dev);
+}
+
#ifdef CONFIG_NETPOLL
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index 537e01a..4e98ffa 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -288,11 +288,11 @@ static int netpoll_owner_active(struct net_device *dev)
return 0;
}
-void netpoll_send_skb(struct netpoll *np, struct sk_buff *skb)
+void netpoll_send_skb_on_dev(struct netpoll *np, struct sk_buff *skb,
+ struct net_device *dev)
{
int status = NETDEV_TX_BUSY;
unsigned long tries;
- struct net_device *dev = np->dev;
const struct net_device_ops *ops = dev->netdev_ops;
/* It is up to the caller to keep npinfo alive. */
struct netpoll_info *npinfo = np->dev->npinfo;
@@ -346,7 +346,7 @@ void netpoll_send_skb(struct netpoll *np, struct sk_buff *skb)
schedule_delayed_work(&npinfo->tx_work,0);
}
}
-EXPORT_SYMBOL(netpoll_send_skb);
+EXPORT_SYMBOL(netpoll_send_skb_on_dev);
void netpoll_send_udp(struct netpoll *np, const char *msg, int len)
{
--
1.7.2.3
next prev parent reply other threads:[~2010-10-14 2:02 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-10-14 2:01 [PATCH] bonding: various fixes for bonding, netpoll & netconsole (v3) nhorman
2010-10-14 2:01 ` nhorman [this message]
2010-10-14 2:01 ` [PATCH 2/5] Fix deadlock in bonding driver resulting from internal locking when using netpoll nhorman
2010-10-14 2:01 ` [PATCH 3/5] Fix napi poll for bonding driver nhorman
2010-10-14 2:01 ` [PATCH 4/5] Fix netconsole to not deadlock on rmmod nhorman
2010-10-14 2:01 ` [PATCH 5/5] Re-enable netpoll over bonding nhorman
2010-10-18 15:25 ` [PATCH] bonding: various fixes for bonding, netpoll & netconsole (v3) David Miller
2010-10-18 15:27 ` David Miller
2010-10-18 15:57 ` Neil Horman
2010-10-19 5:21 ` Cong Wang
2010-10-19 8:06 ` David Miller
-- strict thread matches above, loose matches on Subject: below --
2010-10-13 12:35 [PATCH] bonding: various fixes for bonding, netpoll & netconsole (v2) nhorman
2010-10-13 12:35 ` [PATCH 1/5] Fix bonding drivers improper modification of netpoll structure nhorman
2010-10-12 21:55 [PATCH] bonding: various fixes for bonding, netpoll & netconsole nhorman
2010-10-12 21:55 ` [PATCH 1/5] Fix bonding drivers improper modification of netpoll structure nhorman
2010-10-12 20:29 [PATCH] bonding: various fixes for bonding, netpoll & netconsole nhorman
2010-10-12 20:29 ` [PATCH 1/5] Fix bonding drivers improper modification of netpoll structure nhorman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1287021713-1750-2-git-send-email-nhorman@tuxdriver.com \
--to=nhorman@tuxdriver.com \
--cc=amwang@redhat.com \
--cc=andy@greyhouse.net \
--cc=bonding-devel@lists.sourceforge.net \
--cc=davem@davemloft.net \
--cc=fubar@us.ibm.com \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).