lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Sat, 20 Oct 2018 07:33:55 -0400
From:   Neil Horman <nhorman@...driver.com>
To:     "Banerjee, Debabrata" <dbanerje@...mai.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

On Fri, Oct 19, 2018 at 08:46:45PM +0000, Banerjee, Debabrata wrote:
> > 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
> 

Cool, thanks
Acked-by: Neil Horman <nhorman@...driver.com>

> > 
> > > 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

Powered by Openwall GNU/*/Linux Powered by OpenVZ