RE: [PATCH net-next v2] netpoll: allow cleanup to be synchronous

From: Neil Horman <nhorman@tuxdriver.com>

I presume you've tested this with some of the stacked devices?  I think I'm
ok with this change, but I'd like confirmation that its worked.

Neil

Yes I've tested this on a bond device with vlan stacked on top.

-Deb

quoted

CC: Neil Horman <nhorman@tuxdriver.com>
CC: "David S. Miller" <davem@davemloft.net>
Signed-off-by: Debabrata Banerjee <redacted>
---
 drivers/net/bonding/bond_main.c |  3 ++-
 drivers/net/macvlan.c           |  2 +-
 drivers/net/team/team.c         |  5 +----
 include/linux/netpoll.h         |  4 +---
 net/8021q/vlan_dev.c            |  3 +--
 net/bridge/br_device.c          |  2 +-
 net/core/netpoll.c              | 20 +++++---------------
 net/dsa/slave.c                 |  2 +-
 8 files changed, 13 insertions(+), 28 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c

b/drivers/net/bonding/bond_main.c index ee28ec9e0aba..ffa37adb7681
100644

--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c

@@ -963,7 +963,8 @@ static inline void slave_disable_netpoll(struct slave

*slave)

quoted

 		return;

 	slave->np = NULL;
-	__netpoll_free_async(np);
+
+	__netpoll_free(np);
 }

 static void bond_poll_controller(struct net_device *bond_dev) diff
--git a/drivers/net/macvlan.c b/drivers/net/macvlan.c index
cfda146f3b3b..fc8d5f1ee1ad 100644

--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c

@@ -1077,7 +1077,7 @@ static void macvlan_dev_netpoll_cleanup(struct

net_device *dev)

 	vlan->netpoll = NULL;

-	__netpoll_free_async(netpoll);
+	__netpoll_free(netpoll);
 }
 #endif	/* CONFIG_NET_POLL_CONTROLLER */

diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c index

d887016e54b6..db633ae9f784 100644

--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c

@@ -1104,10 +1104,7 @@ static void team_port_disable_netpoll(struct

team_port *port)

quoted

 		return;
 	port->np = NULL;

-	/* Wait for transmitting packets to finish before freeing. */
-	synchronize_rcu_bh();
-	__netpoll_cleanup(np);
-	kfree(np);
+	__netpoll_free(np);
 }
 #else
 static int team_port_enable_netpoll(struct team_port *port) diff
--git a/include/linux/netpoll.h b/include/linux/netpoll.h index
3ef82d3a78db..676f1ff161a9 100644

--- a/include/linux/netpoll.h
+++ b/include/linux/netpoll.h

@@ -31,8 +31,6 @@ struct netpoll {
 	bool ipv6;
 	u16 local_port, remote_port;
 	u8 remote_mac[ETH_ALEN];
-
-	struct work_struct cleanup_work;
 };

 struct netpoll_info {

@@ -63,7 +61,7 @@ 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);  void
__netpoll_cleanup(struct netpoll *np); -void
__netpoll_free_async(struct netpoll *np);
+void __netpoll_free(struct netpoll *np);
 void netpoll_cleanup(struct netpoll *np);  void
netpoll_send_skb_on_dev(struct netpoll *np, struct sk_buff *skb,
 			     struct net_device *dev);

diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c index

546af0e73ac3..ff720f1ebf73 100644

--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c

@@ -756,8 +756,7 @@ static void vlan_dev_netpoll_cleanup(struct

net_device *dev)

quoted

 		return;

 	vlan->netpoll = NULL;
-
-	__netpoll_free_async(netpoll);
+	__netpoll_free(netpoll);
 }
 #endif /* CONFIG_NET_POLL_CONTROLLER */

diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c index

e053a4e43758..c6abf927f0c9 100644

--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c

@@ -344,7 +344,7 @@ void br_netpoll_disable(struct net_bridge_port *p)

 	p->np = NULL;

-	__netpoll_free_async(np);
+	__netpoll_free(np);
 }

 #endif

diff --git a/net/core/netpoll.c b/net/core/netpoll.c index

de1d1ba92f2d..6ac71624ead4 100644

--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c

@@ -591,7 +591,6 @@ int __netpoll_setup(struct netpoll *np, struct

net_device *ndev)

 	np->dev = ndev;
 	strlcpy(np->dev_name, ndev->name, IFNAMSIZ);
-	INIT_WORK(&np->cleanup_work, netpoll_async_cleanup);

 	if (ndev->priv_flags & IFF_DISABLE_NETPOLL) {
 		np_err(np, "%s doesn't support polling, aborting\n", @@ -

790,10

quoted

+789,6 @@ void __netpoll_cleanup(struct netpoll *np)  {
 	struct netpoll_info *npinfo;

-	/* rtnl_dereference would be preferable here but
-	 * rcu_cleanup_netpoll path can put us in here safely without
-	 * holding the rtnl, so plain rcu_dereference it is
-	 */
 	npinfo = rtnl_dereference(np->dev->npinfo);
 	if (!npinfo)
 		return;

@@ -814,21 +809,16 @@ void __netpoll_cleanup(struct netpoll *np)  }

EXPORT_SYMBOL_GPL(__netpoll_cleanup);

-static void netpoll_async_cleanup(struct work_struct *work)
+void __netpoll_free(struct netpoll *np)
 {
-	struct netpoll *np = container_of(work, struct netpoll,

cleanup_work);

quoted

+	ASSERT_RTNL();

-	rtnl_lock();
+	/* Wait for transmitting packets to finish before freeing. */
+	synchronize_rcu_bh();
 	__netpoll_cleanup(np);
-	rtnl_unlock();
 	kfree(np);
 }
-
-void __netpoll_free_async(struct netpoll *np) -{
-	schedule_work(&np->cleanup_work);
-}
-EXPORT_SYMBOL_GPL(__netpoll_free_async);
+EXPORT_SYMBOL_GPL(__netpoll_free);

 void netpoll_cleanup(struct netpoll *np)  { diff --git
a/net/dsa/slave.c b/net/dsa/slave.c index 3f840b6eea69..3679e13b2ead
100644

--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c

@@ -722,7 +722,7 @@ static void dsa_slave_netpoll_cleanup(struct

net_device *dev)

 	p->netpoll = NULL;

-	__netpoll_free_async(netpoll);
+	__netpoll_free(netpoll);
 }

 static void dsa_slave_poll_controller(struct net_device *dev)
--
2.19.1