Commit 50d31037 authored by David S. Miller's avatar David S. Miller
Browse files

Merge branch 'ethtool-allow-nesting-of-begin-and-complete-callbacks'



Michal Kubecek says:

====================
ethtool: allow nesting of begin() and complete() callbacks

The ethtool ioctl interface used to guarantee that ethtool_ops callbacks
were always called in a block between calls to ->begin() and ->complete()
(if these are defined) and that this whole block was executed with RTNL
lock held:

	rtnl_lock();
	ops->begin();
	/* other ethtool_ops calls */
	ops->complete();
	rtnl_unlock();

This prevented any nesting or crossing of the begin-complete blocks.
However, this is no longer guaranteed even for ioctl interface as at least
ethtool_phys_id() releases RTNL lock while waiting for a timer. With the
introduction of netlink ethtool interface, the begin-complete pairs are
naturally nested e.g. when a request triggers a netlink notification.

Fortunately, only minority of networking drivers implements begin() and
complete() callbacks and most of those that do, fall into three groups:

  - wrappers for pm_runtime_get_sync() and pm_runtime_put()
  - wrappers for clk_prepare_enable() and clk_disable_unprepare()
  - begin() checks netif_running() (fails if false), no complete()

First two have their own refcounting, third is safe w.r.t. nesting of the
blocks.

Only three in-tree networking drivers need an update to deal with nesting
of begin() and complete() calls: via-velocity and epic100 perform resume
and suspend on their own and wil6210 completely serializes the calls using
its own mutex (which would lead to a deadlock if a request request
triggered a netlink notification). The series addresses these problems.

changes between v1 and v2:
  - fix inverted condition in epic100 ethtool_begin() (thanks to Andrew
    Lunn)
====================

Reviewed-by: default avatarSimon Horman <simon.horman@netronome.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parents 17aa23ee 4ac0ac84
Loading
Loading
Loading
Loading
+5 −2
Original line number Diff line number Diff line
@@ -280,6 +280,7 @@ struct epic_private {
	signed char phys[4];				/* MII device addresses. */
	u16 advertising;					/* NWay media advertisement */
	int mii_phy_cnt;
	u32 ethtool_ops_nesting;
	struct mii_if_info mii;
	unsigned int tx_full:1;				/* The Tx queue is full. */
	unsigned int default_port:4;		/* Last dev->if_port value. */
@@ -1435,8 +1436,10 @@ static int ethtool_begin(struct net_device *dev)
	struct epic_private *ep = netdev_priv(dev);
	void __iomem *ioaddr = ep->ioaddr;

	if (ep->ethtool_ops_nesting == U32_MAX)
		return -EBUSY;
	/* power-up, if interface is down */
	if (!netif_running(dev)) {
	if (!ep->ethtool_ops_nesting++ && !netif_running(dev)) {
		ew32(GENCTL, 0x0200);
		ew32(NVCTL, (er32(NVCTL) & ~0x003c) | 0x4800);
	}
@@ -1449,7 +1452,7 @@ static void ethtool_complete(struct net_device *dev)
	void __iomem *ioaddr = ep->ioaddr;

	/* power-down, if interface is down */
	if (!netif_running(dev)) {
	if (!--ep->ethtool_ops_nesting && !netif_running(dev)) {
		ew32(GENCTL, 0x0008);
		ew32(NVCTL, (er32(NVCTL) & ~0x483c) | 0x0000);
	}
+10 −4
Original line number Diff line number Diff line
@@ -3257,12 +3257,16 @@ static struct platform_driver velocity_platform_driver = {
 *	@dev: network device
 *
 *	Called before an ethtool operation. We need to make sure the
 *	chip is out of D3 state before we poke at it.
 *	chip is out of D3 state before we poke at it. In case of ethtool
 *	ops nesting, only wake the device up in the outermost block.
 */
static int velocity_ethtool_up(struct net_device *dev)
{
	struct velocity_info *vptr = netdev_priv(dev);
	if (!netif_running(dev))

	if (vptr->ethtool_ops_nesting == U32_MAX)
		return -EBUSY;
	if (!vptr->ethtool_ops_nesting++ && !netif_running(dev))
		velocity_set_power_state(vptr, PCI_D0);
	return 0;
}
@@ -3272,12 +3276,14 @@ static int velocity_ethtool_up(struct net_device *dev)
 *	@dev: network device
 *
 *	Called after an ethtool operation. Restore the chip back to D3
 *	state if it isn't running.
 *	state if it isn't running. In case of ethtool ops nesting, only
 *	put the device to sleep in the outermost block.
 */
static void velocity_ethtool_down(struct net_device *dev)
{
	struct velocity_info *vptr = netdev_priv(dev);
	if (!netif_running(dev))

	if (!--vptr->ethtool_ops_nesting && !netif_running(dev))
		velocity_set_power_state(vptr, PCI_D3hot);
}

+1 −0
Original line number Diff line number Diff line
@@ -1483,6 +1483,7 @@ struct velocity_info {
	struct velocity_context context;

	u32 ticks;
	u32 ethtool_ops_nesting;

	u8 rev_id;

+16 −27
Original line number Diff line number Diff line
@@ -11,26 +11,6 @@

#include "wil6210.h"

static int wil_ethtoolops_begin(struct net_device *ndev)
{
	struct wil6210_priv *wil = ndev_to_wil(ndev);

	mutex_lock(&wil->mutex);

	wil_dbg_misc(wil, "ethtoolops_begin\n");

	return 0;
}

static void wil_ethtoolops_complete(struct net_device *ndev)
{
	struct wil6210_priv *wil = ndev_to_wil(ndev);

	wil_dbg_misc(wil, "ethtoolops_complete\n");

	mutex_unlock(&wil->mutex);
}

static int wil_ethtoolops_get_coalesce(struct net_device *ndev,
				       struct ethtool_coalesce *cp)
{
@@ -39,11 +19,12 @@ static int wil_ethtoolops_get_coalesce(struct net_device *ndev,
	u32 rx_itr_en, rx_itr_val = 0;
	int ret;

	mutex_lock(&wil->mutex);
	wil_dbg_misc(wil, "ethtoolops_get_coalesce\n");

	ret = wil_pm_runtime_get(wil);
	if (ret < 0)
		return ret;
		goto out;

	tx_itr_en = wil_r(wil, RGF_DMA_ITR_TX_CNT_CTL);
	if (tx_itr_en & BIT_DMA_ITR_TX_CNT_CTL_EN)
@@ -57,7 +38,11 @@ static int wil_ethtoolops_get_coalesce(struct net_device *ndev,

	cp->tx_coalesce_usecs = tx_itr_val;
	cp->rx_coalesce_usecs = rx_itr_val;
	return 0;
	ret = 0;

out:
	mutex_unlock(&wil->mutex);
	return ret;
}

static int wil_ethtoolops_set_coalesce(struct net_device *ndev,
@@ -67,12 +52,14 @@ static int wil_ethtoolops_set_coalesce(struct net_device *ndev,
	struct wireless_dev *wdev = ndev->ieee80211_ptr;
	int ret;

	mutex_lock(&wil->mutex);
	wil_dbg_misc(wil, "ethtoolops_set_coalesce: rx %d usec, tx %d usec\n",
		     cp->rx_coalesce_usecs, cp->tx_coalesce_usecs);

	if (wdev->iftype == NL80211_IFTYPE_MONITOR) {
		wil_dbg_misc(wil, "No IRQ coalescing in monitor mode\n");
		return -EINVAL;
		ret = -EINVAL;
		goto out;
	}

	/* only @rx_coalesce_usecs and @tx_coalesce_usecs supported,
@@ -88,24 +75,26 @@ static int wil_ethtoolops_set_coalesce(struct net_device *ndev,

	ret = wil_pm_runtime_get(wil);
	if (ret < 0)
		return ret;
		goto out;

	wil->txrx_ops.configure_interrupt_moderation(wil);

	wil_pm_runtime_put(wil);
	ret = 0;

	return 0;
out:
	mutex_unlock(&wil->mutex);
	return ret;

out_bad:
	wil_dbg_misc(wil, "Unsupported coalescing params. Raw command:\n");
	print_hex_dump_debug("DBG[MISC] coal ", DUMP_PREFIX_OFFSET, 16, 4,
			     cp, sizeof(*cp), false);
	mutex_unlock(&wil->mutex);
	return -EINVAL;
}

static const struct ethtool_ops wil_ethtool_ops = {
	.begin		= wil_ethtoolops_begin,
	.complete	= wil_ethtoolops_complete,
	.get_drvinfo	= cfg80211_get_drvinfo,
	.get_coalesce	= wil_ethtoolops_get_coalesce,
	.set_coalesce	= wil_ethtoolops_set_coalesce,