[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <9074672b62cd43278884db522bfe877e@usma1ex-dag1mb2.msg.corp.akamai.com>
Date: Fri, 19 Oct 2018 20:46:45 +0000
From: "Banerjee, Debabrata" <dbanerje@...mai.com>
To: 'Neil Horman' <nhorman@...driver.com>
CC: "David S . Miller" <davem@...emloft.net>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [PATCH net-next v2] netpoll: allow cleanup to be synchronous
> From: Neil Horman <nhorman@...driver.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
>
> > CC: Neil Horman <nhorman@...driver.com>
> > CC: "David S. Miller" <davem@...emloft.net>
> > Signed-off-by: Debabrata Banerjee <dbanerje@...mai.com>
> > ---
> > 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)
> > 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)
> > 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)
> > 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
> > +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);
> > + 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
> >
> >
Powered by blists - more mailing lists