* [PATCH 01/14] netpoll: use GFP_ATOMIC in slave_enable_netpoll() and __netpoll_setup()
2012-08-10 7:10 [PATCH v4 00/14] some netpoll and netconsole fixes Cong Wang
@ 2012-08-10 7:10 ` Cong Wang
2012-08-10 7:10 ` [PATCH 02/14] netpoll: make __netpoll_cleanup non-block Cong Wang
` (12 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Cong Wang @ 2012-08-10 7:10 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, Eric Dumazet, David S. Miller, Eric Dumazet
slave_enable_netpoll() and __netpoll_setup() may be called
with read_lock() held, so should use GFP_ATOMIC to allocate
memory. Eric suggested to pass gfp flags to __netpoll_setup().
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: "David S. Miller" <davem@davemloft.net>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Cong Wang <amwang@redhat.com>
---
drivers/net/bonding/bond_main.c | 6 +++---
drivers/net/team/team.c | 16 +++++++++-------
include/linux/netdevice.h | 3 ++-
include/linux/netpoll.h | 2 +-
net/8021q/vlan_dev.c | 7 ++++---
net/bridge/br_device.c | 12 ++++++------
net/bridge/br_if.c | 2 +-
net/bridge/br_private.h | 4 ++--
net/core/netpoll.c | 8 ++++----
9 files changed, 32 insertions(+), 28 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 6fae5f3..8697136 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1235,12 +1235,12 @@ static inline int slave_enable_netpoll(struct slave *slave)
struct netpoll *np;
int err = 0;
- np = kzalloc(sizeof(*np), GFP_KERNEL);
+ np = kzalloc(sizeof(*np), GFP_ATOMIC);
err = -ENOMEM;
if (!np)
goto out;
- err = __netpoll_setup(np, slave->dev);
+ err = __netpoll_setup(np, slave->dev, GFP_ATOMIC);
if (err) {
kfree(np);
goto out;
@@ -1292,7 +1292,7 @@ static void bond_netpoll_cleanup(struct net_device *bond_dev)
read_unlock(&bond->lock);
}
-static int bond_netpoll_setup(struct net_device *dev, struct netpoll_info *ni)
+static int bond_netpoll_setup(struct net_device *dev, struct netpoll_info *ni, gfp_t gfp)
{
struct bonding *bond = netdev_priv(dev);
struct slave *slave;
diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index 87707ab..341b65d 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -795,16 +795,17 @@ static void team_port_leave(struct team *team, struct team_port *port)
}
#ifdef CONFIG_NET_POLL_CONTROLLER
-static int team_port_enable_netpoll(struct team *team, struct team_port *port)
+static int team_port_enable_netpoll(struct team *team, struct team_port *port,
+ gfp_t gfp)
{
struct netpoll *np;
int err;
- np = kzalloc(sizeof(*np), GFP_KERNEL);
+ np = kzalloc(sizeof(*np), gfp);
if (!np)
return -ENOMEM;
- err = __netpoll_setup(np, port->dev);
+ err = __netpoll_setup(np, port->dev, gfp);
if (err) {
kfree(np);
return err;
@@ -833,7 +834,8 @@ static struct netpoll_info *team_netpoll_info(struct team *team)
}
#else
-static int team_port_enable_netpoll(struct team *team, struct team_port *port)
+static int team_port_enable_netpoll(struct team *team, struct team_port *port,
+ gfp_t gfp)
{
return 0;
}
@@ -913,7 +915,7 @@ static int team_port_add(struct team *team, struct net_device *port_dev)
}
if (team_netpoll_info(team)) {
- err = team_port_enable_netpoll(team, port);
+ err = team_port_enable_netpoll(team, port, GFP_KERNEL);
if (err) {
netdev_err(dev, "Failed to enable netpoll on device %s\n",
portname);
@@ -1443,7 +1445,7 @@ static void team_netpoll_cleanup(struct net_device *dev)
}
static int team_netpoll_setup(struct net_device *dev,
- struct netpoll_info *npifo)
+ struct netpoll_info *npifo, gfp_t gfp)
{
struct team *team = netdev_priv(dev);
struct team_port *port;
@@ -1451,7 +1453,7 @@ static int team_netpoll_setup(struct net_device *dev,
mutex_lock(&team->lock);
list_for_each_entry(port, &team->port_list, list) {
- err = team_port_enable_netpoll(team, port);
+ err = team_port_enable_netpoll(team, port, gfp);
if (err) {
__team_netpoll_cleanup(team);
break;
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index a9db4f3..3560d68 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -953,7 +953,8 @@ struct net_device_ops {
#ifdef CONFIG_NET_POLL_CONTROLLER
void (*ndo_poll_controller)(struct net_device *dev);
int (*ndo_netpoll_setup)(struct net_device *dev,
- struct netpoll_info *info);
+ struct netpoll_info *info,
+ gfp_t gfp);
void (*ndo_netpoll_cleanup)(struct net_device *dev);
#endif
int (*ndo_set_vf_mac)(struct net_device *dev,
diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h
index 28f5389..bf2d51e 100644
--- a/include/linux/netpoll.h
+++ b/include/linux/netpoll.h
@@ -43,7 +43,7 @@ struct netpoll_info {
void netpoll_send_udp(struct netpoll *np, const char *msg, int len);
void netpoll_print_options(struct netpoll *np);
int netpoll_parse_options(struct netpoll *np, char *opt);
-int __netpoll_setup(struct netpoll *np, struct net_device *ndev);
+int __netpoll_setup(struct netpoll *np, struct net_device *ndev, gfp_t gfp);
int netpoll_setup(struct netpoll *np);
int netpoll_trap(void);
void netpoll_set_trap(int trap);
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index 73a2a83..ee4ae09 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -669,19 +669,20 @@ static void vlan_dev_poll_controller(struct net_device *dev)
return;
}
-static int vlan_dev_netpoll_setup(struct net_device *dev, struct netpoll_info *npinfo)
+static int vlan_dev_netpoll_setup(struct net_device *dev, struct netpoll_info *npinfo,
+ gfp_t gfp)
{
struct vlan_dev_priv *info = vlan_dev_priv(dev);
struct net_device *real_dev = info->real_dev;
struct netpoll *netpoll;
int err = 0;
- netpoll = kzalloc(sizeof(*netpoll), GFP_KERNEL);
+ netpoll = kzalloc(sizeof(*netpoll), gfp);
err = -ENOMEM;
if (!netpoll)
goto out;
- err = __netpoll_setup(netpoll, real_dev);
+ err = __netpoll_setup(netpoll, real_dev, gfp);
if (err) {
kfree(netpoll);
goto out;
diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index 3334845..ed0e0f9 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -213,7 +213,8 @@ static void br_netpoll_cleanup(struct net_device *dev)
}
}
-static int br_netpoll_setup(struct net_device *dev, struct netpoll_info *ni)
+static int br_netpoll_setup(struct net_device *dev, struct netpoll_info *ni,
+ gfp_t gfp)
{
struct net_bridge *br = netdev_priv(dev);
struct net_bridge_port *p, *n;
@@ -222,8 +223,7 @@ static int br_netpoll_setup(struct net_device *dev, struct netpoll_info *ni)
list_for_each_entry_safe(p, n, &br->port_list, list) {
if (!p->dev)
continue;
-
- err = br_netpoll_enable(p);
+ err = br_netpoll_enable(p, gfp);
if (err)
goto fail;
}
@@ -236,17 +236,17 @@ fail:
goto out;
}
-int br_netpoll_enable(struct net_bridge_port *p)
+int br_netpoll_enable(struct net_bridge_port *p, gfp_t gfp)
{
struct netpoll *np;
int err = 0;
- np = kzalloc(sizeof(*p->np), GFP_KERNEL);
+ np = kzalloc(sizeof(*p->np), gfp);
err = -ENOMEM;
if (!np)
goto out;
- err = __netpoll_setup(np, p->dev);
+ err = __netpoll_setup(np, p->dev, gfp);
if (err) {
kfree(np);
goto out;
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index e1144e1..171fd6b 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -361,7 +361,7 @@ int br_add_if(struct net_bridge *br, struct net_device *dev)
if (err)
goto err2;
- if (br_netpoll_info(br) && ((err = br_netpoll_enable(p))))
+ if (br_netpoll_info(br) && ((err = br_netpoll_enable(p, GFP_KERNEL))))
goto err3;
err = netdev_set_master(dev, br->dev);
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index a768b24..f507d2a 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -316,7 +316,7 @@ static inline void br_netpoll_send_skb(const struct net_bridge_port *p,
netpoll_send_skb(np, skb);
}
-extern int br_netpoll_enable(struct net_bridge_port *p);
+extern int br_netpoll_enable(struct net_bridge_port *p, gfp_t gfp);
extern void br_netpoll_disable(struct net_bridge_port *p);
#else
static inline struct netpoll_info *br_netpoll_info(struct net_bridge *br)
@@ -329,7 +329,7 @@ static inline void br_netpoll_send_skb(const struct net_bridge_port *p,
{
}
-static inline int br_netpoll_enable(struct net_bridge_port *p)
+static inline int br_netpoll_enable(struct net_bridge_port *p, gfp_t gfp)
{
return 0;
}
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index b4c90e4..37cc854 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -715,7 +715,7 @@ int netpoll_parse_options(struct netpoll *np, char *opt)
}
EXPORT_SYMBOL(netpoll_parse_options);
-int __netpoll_setup(struct netpoll *np, struct net_device *ndev)
+int __netpoll_setup(struct netpoll *np, struct net_device *ndev, gfp_t gfp)
{
struct netpoll_info *npinfo;
const struct net_device_ops *ops;
@@ -734,7 +734,7 @@ int __netpoll_setup(struct netpoll *np, struct net_device *ndev)
}
if (!ndev->npinfo) {
- npinfo = kmalloc(sizeof(*npinfo), GFP_KERNEL);
+ npinfo = kmalloc(sizeof(*npinfo), gfp);
if (!npinfo) {
err = -ENOMEM;
goto out;
@@ -752,7 +752,7 @@ int __netpoll_setup(struct netpoll *np, struct net_device *ndev)
ops = np->dev->netdev_ops;
if (ops->ndo_netpoll_setup) {
- err = ops->ndo_netpoll_setup(ndev, npinfo);
+ err = ops->ndo_netpoll_setup(ndev, npinfo, gfp);
if (err)
goto free_npinfo;
}
@@ -857,7 +857,7 @@ int netpoll_setup(struct netpoll *np)
refill_skbs();
rtnl_lock();
- err = __netpoll_setup(np, ndev);
+ err = __netpoll_setup(np, ndev, GFP_KERNEL);
rtnl_unlock();
if (err)
--
1.7.7.6
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 02/14] netpoll: make __netpoll_cleanup non-block
2012-08-10 7:10 [PATCH v4 00/14] some netpoll and netconsole fixes Cong Wang
2012-08-10 7:10 ` [PATCH 01/14] netpoll: use GFP_ATOMIC in slave_enable_netpoll() and __netpoll_setup() Cong Wang
@ 2012-08-10 7:10 ` Cong Wang
2012-08-10 7:10 ` [PATCH 03/14] netconsole: do not release spin_lock when calling __netpoll_cleanup Cong Wang
` (11 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Cong Wang @ 2012-08-10 7:10 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, David S. Miller
Like the previous patch, slave_disable_netpoll() and __netpoll_cleanup()
may be called with read_lock() held too, so we should make them
non-block, by moving the cleanup and kfree() to call_rcu_bh() callbacks.
Cc: "David S. Miller" <davem@davemloft.net>
Signed-off-by: Cong Wang <amwang@redhat.com>
---
drivers/net/bonding/bond_main.c | 4 +--
include/linux/netpoll.h | 3 ++
net/8021q/vlan_dev.c | 6 +----
net/bridge/br_device.c | 6 +----
net/core/netpoll.c | 42 +++++++++++++++++++++++++++++---------
5 files changed, 38 insertions(+), 23 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 8697136..e428916 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1257,9 +1257,7 @@ static inline void slave_disable_netpoll(struct slave *slave)
return;
slave->np = NULL;
- synchronize_rcu_bh();
- __netpoll_cleanup(np);
- kfree(np);
+ __netpoll_free_rcu(np);
}
static inline bool slave_dev_support_netpoll(struct net_device *slave_dev)
{
diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h
index bf2d51e..907812e 100644
--- a/include/linux/netpoll.h
+++ b/include/linux/netpoll.h
@@ -23,6 +23,7 @@ struct netpoll {
u8 remote_mac[ETH_ALEN];
struct list_head rx; /* rx_np list element */
+ struct rcu_head rcu;
};
struct netpoll_info {
@@ -38,6 +39,7 @@ struct netpoll_info {
struct delayed_work tx_work;
struct netpoll *netpoll;
+ struct rcu_head rcu;
};
void netpoll_send_udp(struct netpoll *np, const char *msg, int len);
@@ -48,6 +50,7 @@ int netpoll_setup(struct netpoll *np);
int netpoll_trap(void);
void netpoll_set_trap(int trap);
void __netpoll_cleanup(struct netpoll *np);
+void __netpoll_free_rcu(struct netpoll *np);
void netpoll_cleanup(struct netpoll *np);
int __netpoll_rx(struct sk_buff *skb);
void netpoll_send_skb_on_dev(struct netpoll *np, struct sk_buff *skb,
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index ee4ae09..b65623f 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -704,11 +704,7 @@ static void vlan_dev_netpoll_cleanup(struct net_device *dev)
info->netpoll = NULL;
- /* Wait for transmitting packets to finish before freeing. */
- synchronize_rcu_bh();
-
- __netpoll_cleanup(netpoll);
- kfree(netpoll);
+ __netpoll_free_rcu(netpoll);
}
#endif /* CONFIG_NET_POLL_CONTROLLER */
diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index ed0e0f9..f41ba40 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -267,11 +267,7 @@ void br_netpoll_disable(struct net_bridge_port *p)
p->np = NULL;
- /* Wait for transmitting packets to finish before freeing. */
- synchronize_rcu_bh();
-
- __netpoll_cleanup(np);
- kfree(np);
+ __netpoll_free_rcu(np);
}
#endif
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index 37cc854..dc17f1d 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -878,6 +878,24 @@ static int __init netpoll_init(void)
}
core_initcall(netpoll_init);
+static void rcu_cleanup_netpoll_info(struct rcu_head *rcu_head)
+{
+ struct netpoll_info *npinfo =
+ container_of(rcu_head, struct netpoll_info, rcu);
+
+ skb_queue_purge(&npinfo->arp_tx);
+ skb_queue_purge(&npinfo->txq);
+
+ /* we can't call cancel_delayed_work_sync here, as we are in softirq */
+ cancel_delayed_work(&npinfo->tx_work);
+
+ /* clean after last, unfinished work */
+ __skb_queue_purge(&npinfo->txq);
+ /* now cancel it again */
+ cancel_delayed_work(&npinfo->tx_work);
+ kfree(npinfo);
+}
+
void __netpoll_cleanup(struct netpoll *np)
{
struct netpoll_info *npinfo;
@@ -903,20 +921,24 @@ void __netpoll_cleanup(struct netpoll *np)
ops->ndo_netpoll_cleanup(np->dev);
RCU_INIT_POINTER(np->dev->npinfo, NULL);
+ call_rcu_bh(&npinfo->rcu, rcu_cleanup_netpoll_info);
+ }
+}
+EXPORT_SYMBOL_GPL(__netpoll_cleanup);
- /* avoid racing with NAPI reading npinfo */
- synchronize_rcu_bh();
+static void rcu_cleanup_netpoll(struct rcu_head *rcu_head)
+{
+ struct netpoll *np = container_of(rcu_head, struct netpoll, rcu);
- skb_queue_purge(&npinfo->arp_tx);
- skb_queue_purge(&npinfo->txq);
- cancel_delayed_work_sync(&npinfo->tx_work);
+ __netpoll_cleanup(np);
+ kfree(np);
+}
- /* clean after last, unfinished work */
- __skb_queue_purge(&npinfo->txq);
- kfree(npinfo);
- }
+void __netpoll_free_rcu(struct netpoll *np)
+{
+ call_rcu_bh(&np->rcu, rcu_cleanup_netpoll);
}
-EXPORT_SYMBOL_GPL(__netpoll_cleanup);
+EXPORT_SYMBOL_GPL(__netpoll_free_rcu);
void netpoll_cleanup(struct netpoll *np)
{
--
1.7.7.6
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 03/14] netconsole: do not release spin_lock when calling __netpoll_cleanup
2012-08-10 7:10 [PATCH v4 00/14] some netpoll and netconsole fixes Cong Wang
2012-08-10 7:10 ` [PATCH 01/14] netpoll: use GFP_ATOMIC in slave_enable_netpoll() and __netpoll_setup() Cong Wang
2012-08-10 7:10 ` [PATCH 02/14] netpoll: make __netpoll_cleanup non-block Cong Wang
@ 2012-08-10 7:10 ` Cong Wang
2012-08-10 7:10 ` [PATCH 04/14] netpoll: take rcu_read_lock_bh() in netpoll_rx() Cong Wang
` (10 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Cong Wang @ 2012-08-10 7:10 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, David S. Miller
With the previous patch applied, __netpoll_cleanup() is non-block now,
so we don't need to release the spin_lock before calling it.
Cc: "David S. Miller" <davem@davemloft.net>
Signed-off-by: Cong Wang <amwang@redhat.com>
---
drivers/net/netconsole.c | 5 -----
1 files changed, 0 insertions(+), 5 deletions(-)
diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index f9347ea..f0ad56c 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -640,12 +640,7 @@ static int netconsole_netdev_event(struct notifier_block *this,
* rtnl_lock already held
*/
if (nt->np.dev) {
- spin_unlock_irqrestore(
- &target_list_lock,
- flags);
__netpoll_cleanup(&nt->np);
- spin_lock_irqsave(&target_list_lock,
- flags);
dev_put(nt->np.dev);
nt->np.dev = NULL;
netconsole_target_put(nt);
--
1.7.7.6
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 04/14] netpoll: take rcu_read_lock_bh() in netpoll_rx()
2012-08-10 7:10 [PATCH v4 00/14] some netpoll and netconsole fixes Cong Wang
` (2 preceding siblings ...)
2012-08-10 7:10 ` [PATCH 03/14] netconsole: do not release spin_lock when calling __netpoll_cleanup Cong Wang
@ 2012-08-10 7:10 ` Cong Wang
2012-08-10 7:10 ` [PATCH 05/14] netpoll: use netpoll_rx_on() " Cong Wang
` (9 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Cong Wang @ 2012-08-10 7:10 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, David S. Miller
In __netpoll_rx(), it dereferences ->npinfo without rcu_dereference_bh(),
this patch fixes it by using the 'npinfo' passed from netpoll_rx()
where it is already dereferenced with rcu_dereference_bh().
Cc: "David S. Miller" <davem@davemloft.net>
Signed-off-by: Cong Wang <amwang@redhat.com>
---
include/linux/netpoll.h | 4 ++--
net/core/netpoll.c | 3 +--
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h
index 907812e..5d881c3 100644
--- a/include/linux/netpoll.h
+++ b/include/linux/netpoll.h
@@ -52,7 +52,7 @@ void netpoll_set_trap(int trap);
void __netpoll_cleanup(struct netpoll *np);
void __netpoll_free_rcu(struct netpoll *np);
void netpoll_cleanup(struct netpoll *np);
-int __netpoll_rx(struct sk_buff *skb);
+int __netpoll_rx(struct sk_buff *skb, struct netpoll_info *npinfo);
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)
@@ -77,7 +77,7 @@ static inline bool netpoll_rx(struct sk_buff *skb)
spin_lock(&npinfo->rx_lock);
/* check rx_flags again with the lock held */
- if (npinfo->rx_flags && __netpoll_rx(skb))
+ if (npinfo->rx_flags && __netpoll_rx(skb, npinfo))
ret = true;
spin_unlock(&npinfo->rx_lock);
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index dc17f1d..d055bb0 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -543,13 +543,12 @@ static void arp_reply(struct sk_buff *skb)
spin_unlock_irqrestore(&npinfo->rx_lock, flags);
}
-int __netpoll_rx(struct sk_buff *skb)
+int __netpoll_rx(struct sk_buff *skb, struct netpoll_info *npinfo)
{
int proto, len, ulen;
int hits = 0;
const struct iphdr *iph;
struct udphdr *uh;
- struct netpoll_info *npinfo = skb->dev->npinfo;
struct netpoll *np, *tmp;
if (list_empty(&npinfo->rx_np))
--
1.7.7.6
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 05/14] netpoll: use netpoll_rx_on() in netpoll_rx()
2012-08-10 7:10 [PATCH v4 00/14] some netpoll and netconsole fixes Cong Wang
` (3 preceding siblings ...)
2012-08-10 7:10 ` [PATCH 04/14] netpoll: take rcu_read_lock_bh() in netpoll_rx() Cong Wang
@ 2012-08-10 7:10 ` Cong Wang
2012-08-10 7:10 ` [PATCH 06/14] netpoll: take rcu_read_lock_bh() in netpoll_send_skb_on_dev() Cong Wang
` (8 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Cong Wang @ 2012-08-10 7:10 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, David S. Miller
The logic of the code is same, just call netpoll_rx_on().
Cc: "David S. Miller" <davem@davemloft.net>
Signed-off-by: Cong Wang <amwang@redhat.com>
---
include/linux/netpoll.h | 18 +++++++++---------
1 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h
index 5d881c3..2d178ba 100644
--- a/include/linux/netpoll.h
+++ b/include/linux/netpoll.h
@@ -63,6 +63,13 @@ static inline void netpoll_send_skb(struct netpoll *np, struct sk_buff *skb)
#ifdef CONFIG_NETPOLL
+static inline int netpoll_rx_on(struct sk_buff *skb)
+{
+ struct netpoll_info *npinfo = rcu_dereference_bh(skb->dev->npinfo);
+
+ return npinfo && (!list_empty(&npinfo->rx_np) || npinfo->rx_flags);
+}
+
static inline bool netpoll_rx(struct sk_buff *skb)
{
struct netpoll_info *npinfo;
@@ -70,11 +77,11 @@ static inline bool netpoll_rx(struct sk_buff *skb)
bool ret = false;
local_irq_save(flags);
- npinfo = rcu_dereference_bh(skb->dev->npinfo);
- if (!npinfo || (list_empty(&npinfo->rx_np) && !npinfo->rx_flags))
+ if (!netpoll_rx_on(skb))
goto out;
+ npinfo = rcu_dereference_bh(skb->dev->npinfo);
spin_lock(&npinfo->rx_lock);
/* check rx_flags again with the lock held */
if (npinfo->rx_flags && __netpoll_rx(skb, npinfo))
@@ -86,13 +93,6 @@ out:
return ret;
}
-static inline int netpoll_rx_on(struct sk_buff *skb)
-{
- struct netpoll_info *npinfo = rcu_dereference_bh(skb->dev->npinfo);
-
- return npinfo && (!list_empty(&npinfo->rx_np) || npinfo->rx_flags);
-}
-
static inline int netpoll_receive_skb(struct sk_buff *skb)
{
if (!list_empty(&skb->dev->napi_list))
--
1.7.7.6
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 06/14] netpoll: take rcu_read_lock_bh() in netpoll_send_skb_on_dev()
2012-08-10 7:10 [PATCH v4 00/14] some netpoll and netconsole fixes Cong Wang
` (4 preceding siblings ...)
2012-08-10 7:10 ` [PATCH 05/14] netpoll: use netpoll_rx_on() " Cong Wang
@ 2012-08-10 7:10 ` Cong Wang
2012-08-10 7:10 ` [PATCH 07/14] bridge: add some comments for NETDEV_RELEASE Cong Wang
` (7 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Cong Wang @ 2012-08-10 7:10 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, David S. Miller
This patch fixes several problems in the call path of
netpoll_send_skb_on_dev():
1. Disable IRQ's before calling netpoll_send_skb_on_dev().
2. All the callees of netpoll_send_skb_on_dev() should use
rcu_dereference_bh() to dereference ->npinfo.
3. Rename arp_reply() to netpoll_arp_reply(), the former is too generic.
Cc: "David S. Miller" <davem@davemloft.net>
Signed-off-by: Cong Wang <amwang@redhat.com>
---
include/linux/netpoll.h | 3 +++
net/core/netpoll.c | 31 +++++++++++++++++--------------
2 files changed, 20 insertions(+), 14 deletions(-)
diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h
index 2d178ba..61aee86 100644
--- a/include/linux/netpoll.h
+++ b/include/linux/netpoll.h
@@ -57,7 +57,10 @@ 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)
{
+ unsigned long flags;
+ local_irq_save(flags);
netpoll_send_skb_on_dev(np, skb, np->dev);
+ local_irq_restore(flags);
}
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index d055bb0..174346a 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -54,7 +54,7 @@ static atomic_t trapped;
MAX_UDP_CHUNK)
static void zap_completion_queue(void);
-static void arp_reply(struct sk_buff *skb);
+static void netpoll_arp_reply(struct sk_buff *skb, struct netpoll_info *npinfo);
static unsigned int carrier_timeout = 4;
module_param(carrier_timeout, uint, 0644);
@@ -170,7 +170,8 @@ static void poll_napi(struct net_device *dev)
list_for_each_entry(napi, &dev->napi_list, dev_list) {
if (napi->poll_owner != smp_processor_id() &&
spin_trylock(&napi->poll_lock)) {
- budget = poll_one_napi(dev->npinfo, napi, budget);
+ budget = poll_one_napi(rcu_dereference_bh(dev->npinfo),
+ napi, budget);
spin_unlock(&napi->poll_lock);
if (!budget)
@@ -185,13 +186,14 @@ static void service_arp_queue(struct netpoll_info *npi)
struct sk_buff *skb;
while ((skb = skb_dequeue(&npi->arp_tx)))
- arp_reply(skb);
+ netpoll_arp_reply(skb, npi);
}
}
static void netpoll_poll_dev(struct net_device *dev)
{
const struct net_device_ops *ops;
+ struct netpoll_info *ni = rcu_dereference_bh(dev->npinfo);
if (!dev || !netif_running(dev))
return;
@@ -206,17 +208,18 @@ static void netpoll_poll_dev(struct net_device *dev)
poll_napi(dev);
if (dev->flags & IFF_SLAVE) {
- if (dev->npinfo) {
+ if (ni) {
struct net_device *bond_dev = dev->master;
struct sk_buff *skb;
- while ((skb = skb_dequeue(&dev->npinfo->arp_tx))) {
+ struct netpoll_info *bond_ni = rcu_dereference_bh(bond_dev->npinfo);
+ while ((skb = skb_dequeue(&ni->arp_tx))) {
skb->dev = bond_dev;
- skb_queue_tail(&bond_dev->npinfo->arp_tx, skb);
+ skb_queue_tail(&bond_ni->arp_tx, skb);
}
}
}
- service_arp_queue(dev->npinfo);
+ service_arp_queue(ni);
zap_completion_queue();
}
@@ -302,6 +305,7 @@ static int netpoll_owner_active(struct net_device *dev)
return 0;
}
+/* call with IRQ disabled */
void netpoll_send_skb_on_dev(struct netpoll *np, struct sk_buff *skb,
struct net_device *dev)
{
@@ -309,8 +313,11 @@ void netpoll_send_skb_on_dev(struct netpoll *np, struct sk_buff *skb,
unsigned long tries;
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;
+ struct netpoll_info *npinfo;
+
+ WARN_ON_ONCE(!irqs_disabled());
+ npinfo = rcu_dereference_bh(np->dev->npinfo);
if (!npinfo || !netif_running(dev) || !netif_device_present(dev)) {
__kfree_skb(skb);
return;
@@ -319,11 +326,9 @@ void netpoll_send_skb_on_dev(struct netpoll *np, struct sk_buff *skb,
/* don't get messages out of order, and no recursion */
if (skb_queue_len(&npinfo->txq) == 0 && !netpoll_owner_active(dev)) {
struct netdev_queue *txq;
- unsigned long flags;
txq = netdev_get_tx_queue(dev, skb_get_queue_mapping(skb));
- local_irq_save(flags);
/* try until next clock tick */
for (tries = jiffies_to_usecs(1)/USEC_PER_POLL;
tries > 0; --tries) {
@@ -347,10 +352,9 @@ void netpoll_send_skb_on_dev(struct netpoll *np, struct sk_buff *skb,
}
WARN_ONCE(!irqs_disabled(),
- "netpoll_send_skb(): %s enabled interrupts in poll (%pF)\n",
+ "netpoll_send_skb_on_dev(): %s enabled interrupts in poll (%pF)\n",
dev->name, ops->ndo_start_xmit);
- local_irq_restore(flags);
}
if (status != NETDEV_TX_OK) {
@@ -423,9 +427,8 @@ void netpoll_send_udp(struct netpoll *np, const char *msg, int len)
}
EXPORT_SYMBOL(netpoll_send_udp);
-static void arp_reply(struct sk_buff *skb)
+static void netpoll_arp_reply(struct sk_buff *skb, struct netpoll_info *npinfo)
{
- struct netpoll_info *npinfo = skb->dev->npinfo;
struct arphdr *arp;
unsigned char *arp_ptr;
int size, type = ARPOP_REPLY, ptype = ETH_P_ARP;
--
1.7.7.6
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 07/14] bridge: add some comments for NETDEV_RELEASE
2012-08-10 7:10 [PATCH v4 00/14] some netpoll and netconsole fixes Cong Wang
` (5 preceding siblings ...)
2012-08-10 7:10 ` [PATCH 06/14] netpoll: take rcu_read_lock_bh() in netpoll_send_skb_on_dev() Cong Wang
@ 2012-08-10 7:10 ` Cong Wang
2012-08-10 7:10 ` [PATCH 08/14] bridge: use list_for_each_entry() in netpoll functions Cong Wang
` (6 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Cong Wang @ 2012-08-10 7:10 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, David Miller, Stephen Hemminger
Add comments on why we don't notify NETDEV_RELEASE.
Cc: David Miller <davem@davemloft.net>
Cc: Stephen Hemminger <shemminger@vyatta.com>
Signed-off-by: Cong Wang <amwang@redhat.com>
---
net/bridge/br_if.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index 171fd6b..1c8fdc3 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -427,6 +427,10 @@ int br_del_if(struct net_bridge *br, struct net_device *dev)
if (!p || p->br != br)
return -EINVAL;
+ /* Since more than one interface can be attached to a bridge,
+ * there still maybe an alternate path for netconsole to use;
+ * therefore there is no reason for a NETDEV_RELEASE event.
+ */
del_nbp(p);
spin_lock_bh(&br->lock);
--
1.7.7.6
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 08/14] bridge: use list_for_each_entry() in netpoll functions
2012-08-10 7:10 [PATCH v4 00/14] some netpoll and netconsole fixes Cong Wang
` (6 preceding siblings ...)
2012-08-10 7:10 ` [PATCH 07/14] bridge: add some comments for NETDEV_RELEASE Cong Wang
@ 2012-08-10 7:10 ` Cong Wang
2012-08-10 7:10 ` [PATCH 09/14] netpoll: check netpoll tx status on the right device Cong Wang
` (5 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Cong Wang @ 2012-08-10 7:10 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, David Miller, Stephen Hemminger
We don't delete 'p' from the list in the loop,
so we can just use list_for_each_entry().
Cc: David Miller <davem@davemloft.net>
Cc: Stephen Hemminger <shemminger@vyatta.com>
Signed-off-by: Cong Wang <amwang@redhat.com>
---
net/bridge/br_device.c | 9 ++++-----
1 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index f41ba40..32211fa 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -206,21 +206,20 @@ static void br_poll_controller(struct net_device *br_dev)
static void br_netpoll_cleanup(struct net_device *dev)
{
struct net_bridge *br = netdev_priv(dev);
- struct net_bridge_port *p, *n;
+ struct net_bridge_port *p;
- list_for_each_entry_safe(p, n, &br->port_list, list) {
+ list_for_each_entry(p, &br->port_list, list)
br_netpoll_disable(p);
- }
}
static int br_netpoll_setup(struct net_device *dev, struct netpoll_info *ni,
gfp_t gfp)
{
struct net_bridge *br = netdev_priv(dev);
- struct net_bridge_port *p, *n;
+ struct net_bridge_port *p;
int err = 0;
- list_for_each_entry_safe(p, n, &br->port_list, list) {
+ list_for_each_entry(p, &br->port_list, list) {
if (!p->dev)
continue;
err = br_netpoll_enable(p, gfp);
--
1.7.7.6
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 09/14] netpoll: check netpoll tx status on the right device
2012-08-10 7:10 [PATCH v4 00/14] some netpoll and netconsole fixes Cong Wang
` (7 preceding siblings ...)
2012-08-10 7:10 ` [PATCH 08/14] bridge: use list_for_each_entry() in netpoll functions Cong Wang
@ 2012-08-10 7:10 ` Cong Wang
2012-08-10 7:20 ` Jiri Pirko
2012-08-10 7:10 ` [PATCH 10/14] netpoll: convert several functions to bool Cong Wang
` (4 subsequent siblings)
13 siblings, 1 reply; 21+ messages in thread
From: Cong Wang @ 2012-08-10 7:10 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, Jiri Pirko, David Miller
Although this doesn't matter actually, because netpoll_tx_running()
doesn't use the parameter, the code will be more readable.
For team_dev_queue_xmit() we have to move it down to avoid
compile errors.
Cc: Jiri Pirko <jpirko@redhat.com>
Cc: David Miller <davem@davemloft.net>
Signed-off-by: Cong Wang <amwang@redhat.com>
---
drivers/net/bonding/bond_main.c | 2 +-
include/linux/if_team.h | 30 +++++++++++++++---------------
net/bridge/br_forward.c | 2 +-
3 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index e428916..d688a8a 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -398,7 +398,7 @@ int bond_dev_queue_xmit(struct bonding *bond, struct sk_buff *skb,
sizeof(qdisc_skb_cb(skb)->slave_dev_queue_mapping));
skb->queue_mapping = qdisc_skb_cb(skb)->slave_dev_queue_mapping;
- if (unlikely(netpoll_tx_running(slave_dev)))
+ if (unlikely(netpoll_tx_running(bond->dev)))
bond_netpoll_send_skb(bond_get_slave_by_dev(bond, slave_dev), skb);
else
dev_queue_xmit(skb);
diff --git a/include/linux/if_team.h b/include/linux/if_team.h
index 6960fc1..aa2e167 100644
--- a/include/linux/if_team.h
+++ b/include/linux/if_team.h
@@ -96,21 +96,6 @@ static inline void team_netpoll_send_skb(struct team_port *port,
}
#endif
-static inline int team_dev_queue_xmit(struct team *team, struct team_port *port,
- struct sk_buff *skb)
-{
- BUILD_BUG_ON(sizeof(skb->queue_mapping) !=
- sizeof(qdisc_skb_cb(skb)->slave_dev_queue_mapping));
- skb_set_queue_mapping(skb, qdisc_skb_cb(skb)->slave_dev_queue_mapping);
-
- skb->dev = port->dev;
- if (unlikely(netpoll_tx_running(port->dev))) {
- team_netpoll_send_skb(port, skb);
- return 0;
- }
- return dev_queue_xmit(skb);
-}
-
struct team_mode_ops {
int (*init)(struct team *team);
void (*exit)(struct team *team);
@@ -200,6 +185,21 @@ struct team {
long mode_priv[TEAM_MODE_PRIV_LONGS];
};
+static inline int team_dev_queue_xmit(struct team *team, struct team_port *port,
+ struct sk_buff *skb)
+{
+ BUILD_BUG_ON(sizeof(skb->queue_mapping) !=
+ sizeof(qdisc_skb_cb(skb)->slave_dev_queue_mapping));
+ skb_set_queue_mapping(skb, qdisc_skb_cb(skb)->slave_dev_queue_mapping);
+
+ skb->dev = port->dev;
+ if (unlikely(netpoll_tx_running(team->dev))) {
+ team_netpoll_send_skb(port, skb);
+ return 0;
+ }
+ return dev_queue_xmit(skb);
+}
+
static inline struct hlist_head *team_port_index_hash(struct team *team,
int port_index)
{
diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c
index e9466d4..02015a5 100644
--- a/net/bridge/br_forward.c
+++ b/net/bridge/br_forward.c
@@ -65,7 +65,7 @@ static void __br_deliver(const struct net_bridge_port *to, struct sk_buff *skb)
{
skb->dev = to->dev;
- if (unlikely(netpoll_tx_running(to->dev))) {
+ if (unlikely(netpoll_tx_running(to->br->dev))) {
if (packet_length(skb) > skb->dev->mtu && !skb_is_gso(skb))
kfree_skb(skb);
else {
--
1.7.7.6
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 09/14] netpoll: check netpoll tx status on the right device
2012-08-10 7:10 ` [PATCH 09/14] netpoll: check netpoll tx status on the right device Cong Wang
@ 2012-08-10 7:20 ` Jiri Pirko
2012-08-10 7:52 ` Cong Wang
0 siblings, 1 reply; 21+ messages in thread
From: Jiri Pirko @ 2012-08-10 7:20 UTC (permalink / raw)
To: Cong Wang; +Cc: netdev, Jiri Pirko, David Miller
Fri, Aug 10, 2012 at 09:10:32AM CEST, amwang@redhat.com wrote:
>Although this doesn't matter actually, because netpoll_tx_running()
>doesn't use the parameter, the code will be more readable.
>
>For team_dev_queue_xmit() we have to move it down to avoid
>compile errors.
>
>Cc: Jiri Pirko <jpirko@redhat.com>
>Cc: David Miller <davem@davemloft.net>
>Signed-off-by: Cong Wang <amwang@redhat.com>
>---
> drivers/net/bonding/bond_main.c | 2 +-
> include/linux/if_team.h | 30 +++++++++++++++---------------
> net/bridge/br_forward.c | 2 +-
> 3 files changed, 17 insertions(+), 17 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index e428916..d688a8a 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -398,7 +398,7 @@ int bond_dev_queue_xmit(struct bonding *bond, struct sk_buff *skb,
> sizeof(qdisc_skb_cb(skb)->slave_dev_queue_mapping));
> skb->queue_mapping = qdisc_skb_cb(skb)->slave_dev_queue_mapping;
>
>- if (unlikely(netpoll_tx_running(slave_dev)))
>+ if (unlikely(netpoll_tx_running(bond->dev)))
> bond_netpoll_send_skb(bond_get_slave_by_dev(bond, slave_dev), skb);
> else
> dev_queue_xmit(skb);
>diff --git a/include/linux/if_team.h b/include/linux/if_team.h
>index 6960fc1..aa2e167 100644
>--- a/include/linux/if_team.h
>+++ b/include/linux/if_team.h
>@@ -96,21 +96,6 @@ static inline void team_netpoll_send_skb(struct team_port *port,
> }
> #endif
>
>-static inline int team_dev_queue_xmit(struct team *team, struct team_port *port,
>- struct sk_buff *skb)
>-{
>- BUILD_BUG_ON(sizeof(skb->queue_mapping) !=
>- sizeof(qdisc_skb_cb(skb)->slave_dev_queue_mapping));
>- skb_set_queue_mapping(skb, qdisc_skb_cb(skb)->slave_dev_queue_mapping);
>-
>- skb->dev = port->dev;
>- if (unlikely(netpoll_tx_running(port->dev))) {
>- team_netpoll_send_skb(port, skb);
>- return 0;
>- }
>- return dev_queue_xmit(skb);
>-}
>-
> struct team_mode_ops {
> int (*init)(struct team *team);
> void (*exit)(struct team *team);
>@@ -200,6 +185,21 @@ struct team {
> long mode_priv[TEAM_MODE_PRIV_LONGS];
> };
>
>+static inline int team_dev_queue_xmit(struct team *team, struct team_port *port,
>+ struct sk_buff *skb)
>+{
>+ BUILD_BUG_ON(sizeof(skb->queue_mapping) !=
>+ sizeof(qdisc_skb_cb(skb)->slave_dev_queue_mapping));
>+ skb_set_queue_mapping(skb, qdisc_skb_cb(skb)->slave_dev_queue_mapping);
>+
>+ skb->dev = port->dev;
>+ if (unlikely(netpoll_tx_running(team->dev))) {
>+ team_netpoll_send_skb(port, skb);
>+ return 0;
>+ }
>+ return dev_queue_xmit(skb);
>+}
>+
Is there any particular reason for moving the whole function?
> static inline struct hlist_head *team_port_index_hash(struct team *team,
> int port_index)
> {
>diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c
>index e9466d4..02015a5 100644
>--- a/net/bridge/br_forward.c
>+++ b/net/bridge/br_forward.c
>@@ -65,7 +65,7 @@ static void __br_deliver(const struct net_bridge_port *to, struct sk_buff *skb)
> {
> skb->dev = to->dev;
>
>- if (unlikely(netpoll_tx_running(to->dev))) {
>+ if (unlikely(netpoll_tx_running(to->br->dev))) {
> if (packet_length(skb) > skb->dev->mtu && !skb_is_gso(skb))
> kfree_skb(skb);
> else {
>--
>1.7.7.6
>
>--
>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
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 09/14] netpoll: check netpoll tx status on the right device
2012-08-10 7:20 ` Jiri Pirko
@ 2012-08-10 7:52 ` Cong Wang
2012-08-10 8:32 ` Jiri Pirko
0 siblings, 1 reply; 21+ messages in thread
From: Cong Wang @ 2012-08-10 7:52 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, Jiri Pirko, David Miller
On Fri, 2012-08-10 at 09:20 +0200, Jiri Pirko wrote:
> >
> >+static inline int team_dev_queue_xmit(struct team *team, struct team_port *port,
> >+ struct sk_buff *skb)
> >+{
> >+ BUILD_BUG_ON(sizeof(skb->queue_mapping) !=
> >+ sizeof(qdisc_skb_cb(skb)->slave_dev_queue_mapping));
> >+ skb_set_queue_mapping(skb, qdisc_skb_cb(skb)->slave_dev_queue_mapping);
> >+
> >+ skb->dev = port->dev;
> >+ if (unlikely(netpoll_tx_running(team->dev))) {
> >+ team_netpoll_send_skb(port, skb);
> >+ return 0;
> >+ }
> >+ return dev_queue_xmit(skb);
> >+}
> >+
>
> Is there any particular reason for moving the whole function?
In the changelog I mentioned:
"For team_dev_queue_xmit() we have to move it down to avoid
compile errors."
Or you mean this isn't clear? As I changed 'port->dev' to 'team->dev',
so the definition of 'struct team' has to be visual to
team_dev_queue_xmit(), therefore I moved to down after that definition.
Thanks!
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 09/14] netpoll: check netpoll tx status on the right device
2012-08-10 7:52 ` Cong Wang
@ 2012-08-10 8:32 ` Jiri Pirko
0 siblings, 0 replies; 21+ messages in thread
From: Jiri Pirko @ 2012-08-10 8:32 UTC (permalink / raw)
To: Cong Wang; +Cc: netdev, Jiri Pirko, David Miller
Fri, Aug 10, 2012 at 09:52:36AM CEST, amwang@redhat.com wrote:
>On Fri, 2012-08-10 at 09:20 +0200, Jiri Pirko wrote:
>> >
>> >+static inline int team_dev_queue_xmit(struct team *team, struct team_port *port,
>> >+ struct sk_buff *skb)
>> >+{
>> >+ BUILD_BUG_ON(sizeof(skb->queue_mapping) !=
>> >+ sizeof(qdisc_skb_cb(skb)->slave_dev_queue_mapping));
>> >+ skb_set_queue_mapping(skb, qdisc_skb_cb(skb)->slave_dev_queue_mapping);
>> >+
>> >+ skb->dev = port->dev;
>> >+ if (unlikely(netpoll_tx_running(team->dev))) {
>> >+ team_netpoll_send_skb(port, skb);
>> >+ return 0;
>> >+ }
>> >+ return dev_queue_xmit(skb);
>> >+}
>> >+
>>
>> Is there any particular reason for moving the whole function?
>
>In the changelog I mentioned:
>
>"For team_dev_queue_xmit() we have to move it down to avoid
>compile errors."
>
>Or you mean this isn't clear? As I changed 'port->dev' to 'team->dev',
>so the definition of 'struct team' has to be visual to
>team_dev_queue_xmit(), therefore I moved to down after that definition.
Right. Thanks!
Signed-off-by: Jiri Pirko <jiri@resnulli.us>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 10/14] netpoll: convert several functions to bool
2012-08-10 7:10 [PATCH v4 00/14] some netpoll and netconsole fixes Cong Wang
` (8 preceding siblings ...)
2012-08-10 7:10 ` [PATCH 09/14] netpoll: check netpoll tx status on the right device Cong Wang
@ 2012-08-10 7:10 ` Cong Wang
2012-08-10 7:10 ` [PATCH 11/14] vlan: clean up vlan_dev_hard_start_xmit() Cong Wang
` (3 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Cong Wang @ 2012-08-10 7:10 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, David Miller
These functions are just boolean, let them return
bool instead of int.
Cc: David Miller <davem@davemloft.net>
Signed-off-by: Cong Wang <amwang@redhat.com>
---
include/linux/netpoll.h | 14 +++++++-------
1 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h
index 61aee86..66d5379 100644
--- a/include/linux/netpoll.h
+++ b/include/linux/netpoll.h
@@ -66,7 +66,7 @@ static inline void netpoll_send_skb(struct netpoll *np, struct sk_buff *skb)
#ifdef CONFIG_NETPOLL
-static inline int netpoll_rx_on(struct sk_buff *skb)
+static inline bool netpoll_rx_on(struct sk_buff *skb)
{
struct netpoll_info *npinfo = rcu_dereference_bh(skb->dev->npinfo);
@@ -125,7 +125,7 @@ static inline void netpoll_poll_unlock(void *have)
}
}
-static inline int netpoll_tx_running(struct net_device *dev)
+static inline bool netpoll_tx_running(struct net_device *dev)
{
return irqs_disabled();
}
@@ -133,11 +133,11 @@ static inline int netpoll_tx_running(struct net_device *dev)
#else
static inline bool netpoll_rx(struct sk_buff *skb)
{
- return 0;
+ return false;
}
-static inline int netpoll_rx_on(struct sk_buff *skb)
+static inline bool netpoll_rx_on(struct sk_buff *skb)
{
- return 0;
+ return false;
}
static inline int netpoll_receive_skb(struct sk_buff *skb)
{
@@ -153,9 +153,9 @@ static inline void netpoll_poll_unlock(void *have)
static inline void netpoll_netdev_init(struct net_device *dev)
{
}
-static inline int netpoll_tx_running(struct net_device *dev)
+static inline bool netpoll_tx_running(struct net_device *dev)
{
- return 0;
+ return false;
}
#endif
--
1.7.7.6
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 11/14] vlan: clean up vlan_dev_hard_start_xmit()
2012-08-10 7:10 [PATCH v4 00/14] some netpoll and netconsole fixes Cong Wang
` (9 preceding siblings ...)
2012-08-10 7:10 ` [PATCH 10/14] netpoll: convert several functions to bool Cong Wang
@ 2012-08-10 7:10 ` Cong Wang
2012-08-10 7:14 ` Patrick McHardy
2012-08-10 7:10 ` [PATCH 12/14] netpoll: handle vlan tags in netpoll tx and rx path Cong Wang
` (2 subsequent siblings)
13 siblings, 1 reply; 21+ messages in thread
From: Cong Wang @ 2012-08-10 7:10 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, Benjamin LaHaise, Patrick McHardy, David Miller
Clean up vlan_dev_hard_start_xmit() function.
Cc: Benjamin LaHaise <bcrl@kvack.org>
Cc: Patrick McHardy <kaber@trash.net>
Cc: David Miller <davem@davemloft.net>
Signed-off-by: Cong Wang <amwang@redhat.com>
---
net/8021q/vlan_dev.c | 27 ++++++++++++++++++++-------
1 files changed, 20 insertions(+), 7 deletions(-)
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index b65623f..dedc56c 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -137,9 +137,21 @@ static int vlan_dev_hard_header(struct sk_buff *skb, struct net_device *dev,
return rc;
}
+static inline netdev_tx_t vlan_netpoll_send_skb(struct vlan_dev_priv *info, struct sk_buff *skb)
+{
+#ifdef CONFIG_NET_POLL_CONTROLLER
+ if (info->netpoll)
+ netpoll_send_skb(info->netpoll, skb);
+#else
+ BUG();
+#endif
+ return NETDEV_TX_OK;
+}
+
static netdev_tx_t vlan_dev_hard_start_xmit(struct sk_buff *skb,
struct net_device *dev)
{
+ struct vlan_dev_priv *info = vlan_dev_priv(dev);
struct vlan_ethhdr *veth = (struct vlan_ethhdr *)(skb->data);
unsigned int len;
int ret;
@@ -150,29 +162,30 @@ static netdev_tx_t vlan_dev_hard_start_xmit(struct sk_buff *skb,
* OTHER THINGS LIKE FDDI/TokenRing/802.3 SNAPs...
*/
if (veth->h_vlan_proto != htons(ETH_P_8021Q) ||
- vlan_dev_priv(dev)->flags & VLAN_FLAG_REORDER_HDR) {
+ info->flags & VLAN_FLAG_REORDER_HDR) {
u16 vlan_tci;
- vlan_tci = vlan_dev_priv(dev)->vlan_id;
+ vlan_tci = info->vlan_id;
vlan_tci |= vlan_dev_get_egress_qos_mask(dev, skb);
skb = __vlan_hwaccel_put_tag(skb, vlan_tci);
}
- skb->dev = vlan_dev_priv(dev)->real_dev;
+ skb->dev = info->real_dev;
len = skb->len;
- if (netpoll_tx_running(dev))
- return skb->dev->netdev_ops->ndo_start_xmit(skb, skb->dev);
+ if (unlikely(netpoll_tx_running(dev)))
+ return vlan_netpoll_send_skb(info, skb);
+
ret = dev_queue_xmit(skb);
if (likely(ret == NET_XMIT_SUCCESS || ret == NET_XMIT_CN)) {
struct vlan_pcpu_stats *stats;
- stats = this_cpu_ptr(vlan_dev_priv(dev)->vlan_pcpu_stats);
+ stats = this_cpu_ptr(info->vlan_pcpu_stats);
u64_stats_update_begin(&stats->syncp);
stats->tx_packets++;
stats->tx_bytes += len;
u64_stats_update_end(&stats->syncp);
} else {
- this_cpu_inc(vlan_dev_priv(dev)->vlan_pcpu_stats->tx_dropped);
+ this_cpu_inc(info->vlan_pcpu_stats->tx_dropped);
}
return ret;
--
1.7.7.6
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 11/14] vlan: clean up vlan_dev_hard_start_xmit()
2012-08-10 7:10 ` [PATCH 11/14] vlan: clean up vlan_dev_hard_start_xmit() Cong Wang
@ 2012-08-10 7:14 ` Patrick McHardy
2012-08-10 7:47 ` Cong Wang
0 siblings, 1 reply; 21+ messages in thread
From: Patrick McHardy @ 2012-08-10 7:14 UTC (permalink / raw)
To: Cong Wang; +Cc: netdev, Benjamin LaHaise, David Miller
On Fri, 10 Aug 2012, Cong Wang wrote:
> Clean up vlan_dev_hard_start_xmit() function.
>
> Cc: Benjamin LaHaise <bcrl@kvack.org>
> Cc: Patrick McHardy <kaber@trash.net>
> Cc: David Miller <davem@davemloft.net>
> Signed-off-by: Cong Wang <amwang@redhat.com>
> ---
> net/8021q/vlan_dev.c | 27 ++++++++++++++++++++-------
> 1 files changed, 20 insertions(+), 7 deletions(-)
>
> diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
> index b65623f..dedc56c 100644
> --- a/net/8021q/vlan_dev.c
> +++ b/net/8021q/vlan_dev.c
> @@ -137,9 +137,21 @@ static int vlan_dev_hard_header(struct sk_buff *skb, struct net_device *dev,
> return rc;
> }
>
> +static inline netdev_tx_t vlan_netpoll_send_skb(struct vlan_dev_priv *info, struct sk_buff *skb)
> +{
> +#ifdef CONFIG_NET_POLL_CONTROLLER
> + if (info->netpoll)
> + netpoll_send_skb(info->netpoll, skb);
> +#else
> + BUG();
> +#endif
> + return NETDEV_TX_OK;
> +}
> +
> static netdev_tx_t vlan_dev_hard_start_xmit(struct sk_buff *skb,
> struct net_device *dev)
> {
> + struct vlan_dev_priv *info = vlan_dev_priv(dev);
We're consistently using "vlan" for the private data throughout
the code. Please keep it that way.
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 11/14] vlan: clean up vlan_dev_hard_start_xmit()
2012-08-10 7:14 ` Patrick McHardy
@ 2012-08-10 7:47 ` Cong Wang
0 siblings, 0 replies; 21+ messages in thread
From: Cong Wang @ 2012-08-10 7:47 UTC (permalink / raw)
To: Patrick McHardy; +Cc: netdev, Benjamin LaHaise, David Miller
On Fri, 2012-08-10 at 09:14 +0200, Patrick McHardy wrote:
> On Fri, 10 Aug 2012, Cong Wang wrote:
> > static netdev_tx_t vlan_dev_hard_start_xmit(struct sk_buff *skb,
> > struct net_device *dev)
> > {
> > + struct vlan_dev_priv *info = vlan_dev_priv(dev);
>
> We're consistently using "vlan" for the private data throughout
> the code. Please keep it that way.
>
Ok, I will update it.
Thanks!
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 12/14] netpoll: handle vlan tags in netpoll tx and rx path
2012-08-10 7:10 [PATCH v4 00/14] some netpoll and netconsole fixes Cong Wang
` (10 preceding siblings ...)
2012-08-10 7:10 ` [PATCH 11/14] vlan: clean up vlan_dev_hard_start_xmit() Cong Wang
@ 2012-08-10 7:10 ` Cong Wang
2012-08-10 7:10 ` [PATCH 13/14] netpoll: re-enable irq in poll_napi() Cong Wang
2012-08-10 7:10 ` [PATCH 14/14] netconsole.txt: revision of examples for the receiver of kernel messages Cong Wang
13 siblings, 0 replies; 21+ messages in thread
From: Cong Wang @ 2012-08-10 7:10 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, Benjamin LaHaise, Patrick McHardy, David Miller
Without this patch, I can't get netconsole logs remotely over
vlan. The reason is probably we don't handle vlan tags in either
netpoll tx or rx path.
I am not sure if I use these vlan functions correctly, at
least this patch works.
Cc: Benjamin LaHaise <bcrl@kvack.org>
Cc: Patrick McHardy <kaber@trash.net>
Cc: David Miller <davem@davemloft.net>
Signed-off-by: Cong Wang <amwang@redhat.com>
---
net/core/netpoll.c | 15 +++++++++++++++
1 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index 174346a..e4ba3e7 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -26,6 +26,7 @@
#include <linux/workqueue.h>
#include <linux/slab.h>
#include <linux/export.h>
+#include <linux/if_vlan.h>
#include <net/tcp.h>
#include <net/udp.h>
#include <asm/unaligned.h>
@@ -334,6 +335,14 @@ void netpoll_send_skb_on_dev(struct netpoll *np, struct sk_buff *skb,
tries > 0; --tries) {
if (__netif_tx_trylock(txq)) {
if (!netif_xmit_stopped(txq)) {
+ if (vlan_tx_tag_present(skb) &&
+ !(netif_skb_features(skb) & NETIF_F_HW_VLAN_TX)) {
+ skb = __vlan_put_tag(skb, vlan_tx_tag_get(skb));
+ if (unlikely(!skb))
+ break;
+ skb->vlan_tci = 0;
+ }
+
status = ops->ndo_start_xmit(skb, dev);
if (status == NETDEV_TX_OK)
txq_trans_update(txq);
@@ -567,6 +576,12 @@ int __netpoll_rx(struct sk_buff *skb, struct netpoll_info *npinfo)
return 1;
}
+ if (skb->protocol == cpu_to_be16(ETH_P_8021Q)) {
+ skb = vlan_untag(skb);
+ if (unlikely(!skb))
+ goto out;
+ }
+
proto = ntohs(eth_hdr(skb)->h_proto);
if (proto != ETH_P_IP)
goto out;
--
1.7.7.6
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 13/14] netpoll: re-enable irq in poll_napi()
2012-08-10 7:10 [PATCH v4 00/14] some netpoll and netconsole fixes Cong Wang
` (11 preceding siblings ...)
2012-08-10 7:10 ` [PATCH 12/14] netpoll: handle vlan tags in netpoll tx and rx path Cong Wang
@ 2012-08-10 7:10 ` Cong Wang
2012-08-10 7:10 ` [PATCH 14/14] netconsole.txt: revision of examples for the receiver of kernel messages Cong Wang
13 siblings, 0 replies; 21+ messages in thread
From: Cong Wang @ 2012-08-10 7:10 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, David Miller
napi->poll() needs IRQ enabled, so we have to re-enable IRQ before
calling it.
Cc: David Miller <davem@davemloft.net>
Signed-off-by: Cong Wang <amwang@redhat.com>
---
net/core/netpoll.c | 10 +++++++++-
1 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index e4ba3e7..346b1eb 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -168,16 +168,24 @@ static void poll_napi(struct net_device *dev)
struct napi_struct *napi;
int budget = 16;
+ WARN_ON_ONCE(!irqs_disabled());
+
list_for_each_entry(napi, &dev->napi_list, dev_list) {
+ local_irq_enable();
if (napi->poll_owner != smp_processor_id() &&
spin_trylock(&napi->poll_lock)) {
+ rcu_read_lock_bh();
budget = poll_one_napi(rcu_dereference_bh(dev->npinfo),
napi, budget);
+ rcu_read_unlock_bh();
spin_unlock(&napi->poll_lock);
- if (!budget)
+ if (!budget) {
+ local_irq_disable();
break;
+ }
}
+ local_irq_disable();
}
}
--
1.7.7.6
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 14/14] netconsole.txt: revision of examples for the receiver of kernel messages
2012-08-10 7:10 [PATCH v4 00/14] some netpoll and netconsole fixes Cong Wang
` (12 preceding siblings ...)
2012-08-10 7:10 ` [PATCH 13/14] netpoll: re-enable irq in poll_napi() Cong Wang
@ 2012-08-10 7:10 ` Cong Wang
13 siblings, 0 replies; 21+ messages in thread
From: Cong Wang @ 2012-08-10 7:10 UTC (permalink / raw)
To: netdev; +Cc: Dirk Gouders, Cong Wang
From: Dirk Gouders <gouders@et.bocholt.fh-gelsenkirchen.de>
There are at least 4 implementations of netcat with the BSD-based
being the only one that has to be used without the -p switch to
specify the listening port.
Jan Engelhardt suggested to add an example for socat(1).
Signed-off-by: Dirk Gouders <gouders@et.bocholt.fh-gelsenkirchen.de>
Signed-off-by: Cong Wang <amwang@redhat.com>
---
Documentation/networking/netconsole.txt | 19 +++++++++++++++++--
1 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/Documentation/networking/netconsole.txt b/Documentation/networking/netconsole.txt
index 8d02207..2e9e0ae2 100644
--- a/Documentation/networking/netconsole.txt
+++ b/Documentation/networking/netconsole.txt
@@ -51,8 +51,23 @@ Built-in netconsole starts immediately after the TCP stack is
initialized and attempts to bring up the supplied dev at the supplied
address.
-The remote host can run either 'netcat -u -l -p <port>',
-'nc -l -u <port>' or syslogd.
+The remote host has several options to receive the kernel messages,
+for example:
+
+1) syslogd
+
+2) netcat
+
+ On distributions using a BSD-based netcat version (e.g. Fedora,
+ openSUSE and Ubuntu) the listening port must be specified without
+ the -p switch:
+
+ 'nc -u -l -p <port>' / 'nc -u -l <port>' or
+ 'netcat -u -l -p <port>' / 'netcat -u -l <port>'
+
+3) socat
+
+ 'socat udp-recv:<port> -'
Dynamic reconfiguration:
========================
--
1.7.7.6
^ permalink raw reply related [flat|nested] 21+ messages in thread