[PATCH net-next 00/12] ethtool: consolidate parameter checking for irq coalescing

[PATCH net-next 00/12] ethtool: consolidate parameter checking for irq coalescing

[Jakub Kicinski]

Hi!

This set aims to simplify and unify the unsupported irq
coalescing parameter handling.

First patch adds a bitmask which drivers should fill in
in their ethtool_ops structs to declare which parameters
they support. Core will then ensure that driver callback
won't see any parameter outside of that set.

This allows us to save some LoC and make sure all drivers
respond the same to unsupported parameters.

If any parameter driver does not support is set to a value
other than 0 core will return -EINVAL. In the future we can
reject any present but unsupported netlink attribute, without
assuming 0 means unset. We can also add some prints or extack,
perhaps a'la Intel's current code.

I started converting the drivers alphabetically but then
realized that for the first set it's probably best to
address a representative mix of actively developed drivers.

According to my unreliable math there are roughly 69 drivers
in the tree which support some form of interrupt coalescing
settings via ethtool. Of these roughly 17 reject parameters
they don't support.

I hope drivers which ignore the parameters don't care, and
won't care about the slight change in behavior. Once all
drivers are converted we can make the checking mandatory.

I've only tested the e1000e and virtio patches, the rest builds.

Jakub Kicinski (12):
  ethtool: add infrastructure for centralized checking of coalescing
    parameters
  xgbe: let core reject the unsupported coalescing parameters
  enic: let core reject the unsupported coalescing parameters
  stmmac: let core reject the unsupported coalescing parameters
  nfp: let core reject the unsupported coalescing parameters
  ionic: let core reject the unsupported coalescing parameters
  hisilicon: let core reject the unsupported coalescing parameters
  ice: let core reject the unsupported coalescing parameters
  bnxt: reject unsupported coalescing params
  mlx5: reject unsupported coalescing params
  e1000e: reject unsupported coalescing params
  virtio_net: reject unsupported coalescing params

 drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c  | 26 +-------
 .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c |  6 ++
 .../net/ethernet/cisco/enic/enic_ethtool.c    | 23 ++-----
 drivers/net/ethernet/hisilicon/hip04_eth.c    | 16 +----
 drivers/net/ethernet/intel/e1000e/ethtool.c   |  1 +
 drivers/net/ethernet/intel/ice/ice_ethtool.c  | 59 +----------------
 .../ethernet/mellanox/mlx5/core/en_ethtool.c  |  3 +
 .../net/ethernet/mellanox/mlx5/core/en_rep.c  |  6 ++
 .../mellanox/mlx5/core/ipoib/ethtool.c        |  3 +
 .../ethernet/netronome/nfp/nfp_net_ethtool.c  | 22 +------
 .../ethernet/pensando/ionic/ionic_ethtool.c   | 23 +------
 .../ethernet/stmicro/stmmac/stmmac_ethtool.c  | 16 +----
 drivers/net/virtio_net.c                      |  1 +
 include/linux/ethtool.h                       | 44 ++++++++++++-
 net/ethtool/ioctl.c                           | 63 +++++++++++++++++++
 15 files changed, 140 insertions(+), 172 deletions(-)

-- 
2.24.1

[PATCH net-next 01/12] ethtool: add infrastructure for centralized checking of coalescing parameters

[Jakub Kicinski]

Linux supports 22 different interrupt coalescing parameters.
No driver implements them all. Some drivers just ignore the
ones they don't support, while others have to carry a long
list of checks to reject unsupported settings.

To simplify the drivers add the ability to specify inside
ethtool_ops which parameters are supported and let the core
reject attempts to set any other one.

This commit makes the mechanism an opt-in, only drivers which
set ethtool_opts->coalesce_types to a non-zero value will have
the checks enforced.

The same mask is used for global and per queue settings.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 include/linux/ethtool.h | 45 +++++++++++++++++++++++++++--
 net/ethtool/ioctl.c     | 63 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 105 insertions(+), 3 deletions(-)

diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 23373978cb3c..3c7328f02ba3 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -177,8 +177,44 @@ void ethtool_convert_legacy_u32_to_link_mode(unsigned long *dst,
 bool ethtool_convert_link_mode_to_legacy_u32(u32 *legacy_u32,
 				     const unsigned long *src);
 
+#define ETHTOOL_COALESCE_RX_USECS		BIT(0)
+#define ETHTOOL_COALESCE_RX_MAX_FRAMES		BIT(1)
+#define ETHTOOL_COALESCE_RX_USECS_IRQ		BIT(2)
+#define ETHTOOL_COALESCE_RX_MAX_FRAMES_IRQ	BIT(3)
+#define ETHTOOL_COALESCE_TX_USECS		BIT(4)
+#define ETHTOOL_COALESCE_TX_MAX_FRAMES		BIT(5)
+#define ETHTOOL_COALESCE_TX_USECS_IRQ		BIT(6)
+#define ETHTOOL_COALESCE_TX_MAX_FRAMES_IRQ	BIT(7)
+#define ETHTOOL_COALESCE_STATS_BLOCK_USECS	BIT(8)
+#define ETHTOOL_COALESCE_USE_ADAPTIVE_RX	BIT(9)
+#define ETHTOOL_COALESCE_USE_ADAPTIVE_TX	BIT(10)
+#define ETHTOOL_COALESCE_PKT_RATE_LOW		BIT(11)
+#define ETHTOOL_COALESCE_RX_USECS_LOW		BIT(12)
+#define ETHTOOL_COALESCE_RX_MAX_FRAMES_LOW	BIT(13)
+#define ETHTOOL_COALESCE_TX_USECS_LOW		BIT(14)
+#define ETHTOOL_COALESCE_TX_MAX_FRAMES_LOW	BIT(15)
+#define ETHTOOL_COALESCE_PKT_RATE_HIGH		BIT(16)
+#define ETHTOOL_COALESCE_RX_USECS_HIGH		BIT(17)
+#define ETHTOOL_COALESCE_RX_MAX_FRAMES_HIGH	BIT(18)
+#define ETHTOOL_COALESCE_TX_USECS_HIGH		BIT(19)
+#define ETHTOOL_COALESCE_TX_MAX_FRAMES_HIGH	BIT(20)
+#define ETHTOOL_COALESCE_RATE_SAMPLE_INTERVAL	BIT(21)
+
+#define ETHTOOL_COALESCE_USECS						\
+	(ETHTOOL_COALESCE_RX_USECS | ETHTOOL_COALESCE_TX_USECS)
+#define ETHTOOL_COALESCE_MAX_FRAMES					\
+	(ETHTOOL_COALESCE_RX_MAX_FRAMES | ETHTOOL_COALESCE_TX_MAX_FRAMES)
+#define ETHTOOL_COALESCE_USECS_IRQ					\
+	(ETHTOOL_COALESCE_RX_USECS_IRQ | ETHTOOL_COALESCE_TX_USECS_IRQ)
+#define ETHTOOL_COALESCE_MAX_FRAMES_IRQ		\
+	(ETHTOOL_COALESCE_RX_MAX_FRAMES_IRQ |	\
+	 ETHTOOL_COALESCE_TX_MAX_FRAMES_IRQ)
+#define ETHTOOL_COALESCE_USE_ADAPTIVE					\
+	(ETHTOOL_COALESCE_USE_ADAPTIVE_RX | ETHTOOL_COALESCE_USE_ADAPTIVE_TX)
+
 /**
  * struct ethtool_ops - optional netdev operations
+ * @coalesce_types: supported types of interrupt coalescing.
  * @get_drvinfo: Report driver/device information.  Should only set the
  *	@driver, @version, @fw_version and @bus_info fields.  If not
  *	implemented, the @driver and @bus_info fields will be filled in
@@ -207,8 +243,9 @@ bool ethtool_convert_link_mode_to_legacy_u32(u32 *legacy_u32,
  *	or zero.
  * @get_coalesce: Get interrupt coalescing parameters.  Returns a negative
  *	error code or zero.
- * @set_coalesce: Set interrupt coalescing parameters.  Returns a negative
- *	error code or zero.
+ * @set_coalesce: Set interrupt coalescing parameters.  Supported coalescing
+ *	types should be set in @coalesce_types.
+ *	Returns a negative error code or zero.
  * @get_ringparam: Report ring sizes
  * @set_ringparam: Set ring sizes.  Returns a negative error code or zero.
  * @get_pauseparam: Report pause parameters
@@ -292,7 +329,8 @@ bool ethtool_convert_link_mode_to_legacy_u32(u32 *legacy_u32,
  * @set_per_queue_coalesce: Set interrupt coalescing parameters per queue.
  *	It must check that the given queue number is valid. If neither a RX nor
  *	a TX queue has this number, return -EINVAL. If only a RX queue or a TX
- *	queue has this number, ignore the inapplicable fields.
+ *	queue has this number, ignore the inapplicable fields. Supported
+ *	coalescing types should be set in @coalesce_types.
  *	Returns a negative error code or zero.
  * @get_link_ksettings: Get various device settings including Ethernet link
  *	settings. The %cmd and %link_mode_masks_nwords fields should be
@@ -323,6 +361,7 @@ bool ethtool_convert_link_mode_to_legacy_u32(u32 *legacy_u32,
  * of the generic netdev features interface.
  */
 struct ethtool_ops {
+	u32	coalesce_types;
 	void	(*get_drvinfo)(struct net_device *, struct ethtool_drvinfo *);
 	int	(*get_regs_len)(struct net_device *);
 	void	(*get_regs)(struct net_device *, struct ethtool_regs *, void *);
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index f2fe8e5896dc..ae1a6eea36f1 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -1544,6 +1544,61 @@ static noinline_for_stack int ethtool_get_coalesce(struct net_device *dev,
 	return 0;
 }
 
+static bool
+ethtool_set_coalesce_supported(struct net_device *dev,
+			       struct ethtool_coalesce *coalesce)
+{
+	u32 used_types = 0;
+
+	if (coalesce->rx_coalesce_usecs)
+		used_types |= ETHTOOL_COALESCE_RX_USECS;
+	if (coalesce->rx_max_coalesced_frames)
+		used_types |= ETHTOOL_COALESCE_RX_MAX_FRAMES;
+	if (coalesce->rx_coalesce_usecs_irq)
+		used_types |= ETHTOOL_COALESCE_RX_USECS_IRQ;
+	if (coalesce->rx_max_coalesced_frames_irq)
+		used_types |= ETHTOOL_COALESCE_RX_MAX_FRAMES_IRQ;
+	if (coalesce->tx_coalesce_usecs)
+		used_types |= ETHTOOL_COALESCE_TX_USECS;
+	if (coalesce->tx_max_coalesced_frames)
+		used_types |= ETHTOOL_COALESCE_TX_MAX_FRAMES;
+	if (coalesce->tx_coalesce_usecs_irq)
+		used_types |= ETHTOOL_COALESCE_TX_USECS_IRQ;
+	if (coalesce->tx_max_coalesced_frames_irq)
+		used_types |= ETHTOOL_COALESCE_TX_MAX_FRAMES_IRQ;
+	if (coalesce->stats_block_coalesce_usecs)
+		used_types |= ETHTOOL_COALESCE_STATS_BLOCK_USECS;
+	if (coalesce->use_adaptive_rx_coalesce)
+		used_types |= ETHTOOL_COALESCE_USE_ADAPTIVE_RX;
+	if (coalesce->use_adaptive_tx_coalesce)
+		used_types |= ETHTOOL_COALESCE_USE_ADAPTIVE_TX;
+	if (coalesce->pkt_rate_low)
+		used_types |= ETHTOOL_COALESCE_PKT_RATE_LOW;
+	if (coalesce->rx_coalesce_usecs_low)
+		used_types |= ETHTOOL_COALESCE_RX_USECS_LOW;
+	if (coalesce->rx_max_coalesced_frames_low)
+		used_types |= ETHTOOL_COALESCE_RX_MAX_FRAMES_LOW;
+	if (coalesce->tx_coalesce_usecs_low)
+		used_types |= ETHTOOL_COALESCE_TX_USECS_LOW;
+	if (coalesce->tx_max_coalesced_frames_low)
+		used_types |= ETHTOOL_COALESCE_TX_MAX_FRAMES_LOW;
+	if (coalesce->pkt_rate_high)
+		used_types |= ETHTOOL_COALESCE_PKT_RATE_HIGH;
+	if (coalesce->rx_coalesce_usecs_high)
+		used_types |= ETHTOOL_COALESCE_RX_USECS_HIGH;
+	if (coalesce->rx_max_coalesced_frames_high)
+		used_types |= ETHTOOL_COALESCE_RX_MAX_FRAMES_HIGH;
+	if (coalesce->tx_coalesce_usecs_high)
+		used_types |= ETHTOOL_COALESCE_TX_USECS_HIGH;
+	if (coalesce->tx_max_coalesced_frames_high)
+		used_types |= ETHTOOL_COALESCE_TX_MAX_FRAMES_HIGH;
+	if (coalesce->rate_sample_interval)
+		used_types |= ETHTOOL_COALESCE_RATE_SAMPLE_INTERVAL;
+
+	return !dev->ethtool_ops->coalesce_types ||
+		(dev->ethtool_ops->coalesce_types & used_types) == used_types;
+}
+
 static noinline_for_stack int ethtool_set_coalesce(struct net_device *dev,
 						   void __user *useraddr)
 {
@@ -1555,6 +1610,9 @@ static noinline_for_stack int ethtool_set_coalesce(struct net_device *dev,
 	if (copy_from_user(&coalesce, useraddr, sizeof(coalesce)))
 		return -EFAULT;
 
+	if (!ethtool_set_coalesce_supported(dev, &coalesce))
+		return -EINVAL;
+
 	return dev->ethtool_ops->set_coalesce(dev, &coalesce);
 }
 
@@ -2336,6 +2394,11 @@ ethtool_set_per_queue_coalesce(struct net_device *dev,
 			goto roll_back;
 		}
 
+		if (!ethtool_set_coalesce_supported(dev, &coalesce)) {
+			ret = -EINVAL;
+			goto roll_back;
+		}
+
 		ret = dev->ethtool_ops->set_per_queue_coalesce(dev, bit, &coalesce);
 		if (ret != 0)
 			goto roll_back;
-- 
2.24.1

[PATCH net-next 02/12] xgbe: let core reject the unsupported coalescing parameters

[Jakub Kicinski]

Set ethtool_ops->coalesce_types to let the core reject
unsupported coalescing parameters.

This driver correctly rejects all unsupported parameters.
We are losing the print, and changing the return value
from EOPNOTSUPP to EINVAL.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c | 26 ++------------------
 1 file changed, 2 insertions(+), 24 deletions(-)

diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c b/drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c
index b23c8ee24ee3..e373991c9905 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c
@@ -450,30 +450,6 @@ static int xgbe_set_coalesce(struct net_device *netdev,
 	unsigned int rx_frames, rx_riwt, rx_usecs;
 	unsigned int tx_frames;
 
-	/* Check for not supported parameters  */
-	if ((ec->rx_coalesce_usecs_irq) ||
-	    (ec->rx_max_coalesced_frames_irq) ||
-	    (ec->tx_coalesce_usecs) ||
-	    (ec->tx_coalesce_usecs_irq) ||
-	    (ec->tx_max_coalesced_frames_irq) ||
-	    (ec->stats_block_coalesce_usecs) ||
-	    (ec->use_adaptive_rx_coalesce) ||
-	    (ec->use_adaptive_tx_coalesce) ||
-	    (ec->pkt_rate_low) ||
-	    (ec->rx_coalesce_usecs_low) ||
-	    (ec->rx_max_coalesced_frames_low) ||
-	    (ec->tx_coalesce_usecs_low) ||
-	    (ec->tx_max_coalesced_frames_low) ||
-	    (ec->pkt_rate_high) ||
-	    (ec->rx_coalesce_usecs_high) ||
-	    (ec->rx_max_coalesced_frames_high) ||
-	    (ec->tx_coalesce_usecs_high) ||
-	    (ec->tx_max_coalesced_frames_high) ||
-	    (ec->rate_sample_interval)) {
-		netdev_err(netdev, "unsupported coalescing parameter\n");
-		return -EOPNOTSUPP;
-	}
-
 	rx_riwt = hw_if->usec_to_riwt(pdata, ec->rx_coalesce_usecs);
 	rx_usecs = ec->rx_coalesce_usecs;
 	rx_frames = ec->rx_max_coalesced_frames;
@@ -837,6 +813,8 @@ static int xgbe_set_channels(struct net_device *netdev,
 }
 
 static const struct ethtool_ops xgbe_ethtool_ops = {
+	.coalesce_types = ETHTOOL_COALESCE_RX_USECS |
+			  ETHTOOL_COALESCE_MAX_FRAMES,
 	.get_drvinfo = xgbe_get_drvinfo,
 	.get_msglevel = xgbe_get_msglevel,
 	.set_msglevel = xgbe_set_msglevel,
-- 
2.24.1

[PATCH net-next 03/12] enic: let core reject the unsupported coalescing parameters

[Jakub Kicinski]

Set ethtool_ops->coalesce_types to let the core reject
unsupported coalescing parameters.

This driver correctly rejects all unsupported parameters.
No functional changes.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 .../net/ethernet/cisco/enic/enic_ethtool.c    | 23 ++++---------------
 1 file changed, 4 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/cisco/enic/enic_ethtool.c b/drivers/net/ethernet/cisco/enic/enic_ethtool.c
index ebd5c2cf1efe..bff487a2c5be 100644
--- a/drivers/net/ethernet/cisco/enic/enic_ethtool.c
+++ b/drivers/net/ethernet/cisco/enic/enic_ethtool.c
@@ -324,25 +324,6 @@ static int enic_coalesce_valid(struct enic *enic,
 	u32 rx_coalesce_usecs_low = min_t(u32, coalesce_usecs_max,
 					  ec->rx_coalesce_usecs_low);
 
-	if (ec->rx_max_coalesced_frames		||
-	    ec->rx_coalesce_usecs_irq		||
-	    ec->rx_max_coalesced_frames_irq	||
-	    ec->tx_max_coalesced_frames		||
-	    ec->tx_coalesce_usecs_irq		||
-	    ec->tx_max_coalesced_frames_irq	||
-	    ec->stats_block_coalesce_usecs	||
-	    ec->use_adaptive_tx_coalesce	||
-	    ec->pkt_rate_low			||
-	    ec->rx_max_coalesced_frames_low	||
-	    ec->tx_coalesce_usecs_low		||
-	    ec->tx_max_coalesced_frames_low	||
-	    ec->pkt_rate_high			||
-	    ec->rx_max_coalesced_frames_high	||
-	    ec->tx_coalesce_usecs_high		||
-	    ec->tx_max_coalesced_frames_high	||
-	    ec->rate_sample_interval)
-		return -EINVAL;
-
 	if ((vnic_dev_get_intr_mode(enic->vdev) != VNIC_DEV_INTR_MODE_MSIX) &&
 	    ec->tx_coalesce_usecs)
 		return -EINVAL;
@@ -636,6 +617,10 @@ static int enic_get_ts_info(struct net_device *netdev,
 }
 
 static const struct ethtool_ops enic_ethtool_ops = {
+	.coalesce_types = ETHTOOL_COALESCE_USECS |
+			  ETHTOOL_COALESCE_USE_ADAPTIVE_RX |
+			  ETHTOOL_COALESCE_RX_USECS_LOW |
+			  ETHTOOL_COALESCE_RX_USECS_HIGH,
 	.get_drvinfo = enic_get_drvinfo,
 	.get_msglevel = enic_get_msglevel,
 	.set_msglevel = enic_set_msglevel,
-- 
2.24.1

[PATCH net-next 04/12] stmmac: let core reject the unsupported coalescing parameters

[Jakub Kicinski]

Set ethtool_ops->coalesce_types to let the core reject
unsupported coalescing parameters.

This driver correctly rejects all unsupported parameters.
As a side effect of these changes the error code for
unsupported params changes from EOPNOTSUPP to EINVAL.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 .../net/ethernet/stmicro/stmmac/stmmac_ethtool.c | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
index b29603ec744c..6f551e75ad72 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
@@ -732,20 +732,6 @@ static int stmmac_set_coalesce(struct net_device *dev,
 	u32 rx_cnt = priv->plat->rx_queues_to_use;
 	unsigned int rx_riwt;
 
-	/* Check not supported parameters  */
-	if ((ec->rx_coalesce_usecs_irq) ||
-	    (ec->rx_max_coalesced_frames_irq) || (ec->tx_coalesce_usecs_irq) ||
-	    (ec->use_adaptive_rx_coalesce) || (ec->use_adaptive_tx_coalesce) ||
-	    (ec->pkt_rate_low) || (ec->rx_coalesce_usecs_low) ||
-	    (ec->rx_max_coalesced_frames_low) || (ec->tx_coalesce_usecs_high) ||
-	    (ec->tx_max_coalesced_frames_low) || (ec->pkt_rate_high) ||
-	    (ec->tx_coalesce_usecs_low) || (ec->rx_coalesce_usecs_high) ||
-	    (ec->rx_max_coalesced_frames_high) ||
-	    (ec->tx_max_coalesced_frames_irq) ||
-	    (ec->stats_block_coalesce_usecs) ||
-	    (ec->tx_max_coalesced_frames_high) || (ec->rate_sample_interval))
-		return -EOPNOTSUPP;
-
 	if (priv->use_riwt && (ec->rx_coalesce_usecs > 0)) {
 		rx_riwt = stmmac_usec2riwt(ec->rx_coalesce_usecs, priv);
 
@@ -914,6 +900,8 @@ static int stmmac_set_tunable(struct net_device *dev,
 }
 
 static const struct ethtool_ops stmmac_ethtool_ops = {
+	.coalesce_types = ETHTOOL_COALESCE_USECS |
+			  ETHTOOL_COALESCE_MAX_FRAMES,
 	.begin = stmmac_check_if_running,
 	.get_drvinfo = stmmac_ethtool_getdrvinfo,
 	.get_msglevel = stmmac_ethtool_getmsglevel,
-- 
2.24.1

[PATCH net-next 05/12] nfp: let core reject the unsupported coalescing parameters

[Jakub Kicinski]

Set ethtool_ops->coalesce_types to let the core reject
unsupported coalescing parameters.

This driver correctly rejects all unsupported parameters.
As a side effect of these changes the error code for
unsupported params changes from EOPNOTSUPP to EINVAL.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 .../ethernet/netronome/nfp/nfp_net_ethtool.c  | 22 ++-----------------
 1 file changed, 2 insertions(+), 20 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
index d648e32c0520..71354ff2fe7d 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
@@ -1343,26 +1343,6 @@ static int nfp_net_set_coalesce(struct net_device *netdev,
 	struct nfp_net *nn = netdev_priv(netdev);
 	unsigned int factor;
 
-	if (ec->rx_coalesce_usecs_irq ||
-	    ec->rx_max_coalesced_frames_irq ||
-	    ec->tx_coalesce_usecs_irq ||
-	    ec->tx_max_coalesced_frames_irq ||
-	    ec->stats_block_coalesce_usecs ||
-	    ec->use_adaptive_rx_coalesce ||
-	    ec->use_adaptive_tx_coalesce ||
-	    ec->pkt_rate_low ||
-	    ec->rx_coalesce_usecs_low ||
-	    ec->rx_max_coalesced_frames_low ||
-	    ec->tx_coalesce_usecs_low ||
-	    ec->tx_max_coalesced_frames_low ||
-	    ec->pkt_rate_high ||
-	    ec->rx_coalesce_usecs_high ||
-	    ec->rx_max_coalesced_frames_high ||
-	    ec->tx_coalesce_usecs_high ||
-	    ec->tx_max_coalesced_frames_high ||
-	    ec->rate_sample_interval)
-		return -EOPNOTSUPP;
-
 	/* Compute factor used to convert coalesce '_usecs' parameters to
 	 * ME timestamp ticks.  There are 16 ME clock cycles for each timestamp
 	 * count.
@@ -1476,6 +1456,8 @@ static int nfp_net_set_channels(struct net_device *netdev,
 }
 
 static const struct ethtool_ops nfp_net_ethtool_ops = {
+	.coalesce_types = ETHTOOL_COALESCE_USECS |
+			  ETHTOOL_COALESCE_MAX_FRAMES,
 	.get_drvinfo		= nfp_net_get_drvinfo,
 	.get_link		= ethtool_op_get_link,
 	.get_ringparam		= nfp_net_get_ringparam,
-- 
2.24.1

[PATCH net-next 06/12] ionic: let core reject the unsupported coalescing parameters

[Jakub Kicinski]

Set ethtool_ops->coalesce_types to let the core reject
unsupported coalescing parameters.

This driver correctly rejects all unsupported parameters.
No functional changes.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 .../ethernet/pensando/ionic/ionic_ethtool.c   | 23 +------------------
 1 file changed, 1 insertion(+), 22 deletions(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
index f778fff034f5..83ea35715533 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
@@ -412,28 +412,6 @@ static int ionic_set_coalesce(struct net_device *netdev,
 	unsigned int i;
 	u32 coal;
 
-	if (coalesce->rx_max_coalesced_frames ||
-	    coalesce->rx_coalesce_usecs_irq ||
-	    coalesce->rx_max_coalesced_frames_irq ||
-	    coalesce->tx_max_coalesced_frames ||
-	    coalesce->tx_coalesce_usecs_irq ||
-	    coalesce->tx_max_coalesced_frames_irq ||
-	    coalesce->stats_block_coalesce_usecs ||
-	    coalesce->use_adaptive_rx_coalesce ||
-	    coalesce->use_adaptive_tx_coalesce ||
-	    coalesce->pkt_rate_low ||
-	    coalesce->rx_coalesce_usecs_low ||
-	    coalesce->rx_max_coalesced_frames_low ||
-	    coalesce->tx_coalesce_usecs_low ||
-	    coalesce->tx_max_coalesced_frames_low ||
-	    coalesce->pkt_rate_high ||
-	    coalesce->rx_coalesce_usecs_high ||
-	    coalesce->rx_max_coalesced_frames_high ||
-	    coalesce->tx_coalesce_usecs_high ||
-	    coalesce->tx_max_coalesced_frames_high ||
-	    coalesce->rate_sample_interval)
-		return -EINVAL;
-
 	ident = &lif->ionic->ident;
 	if (ident->dev.intr_coal_div == 0) {
 		netdev_warn(netdev, "bad HW value in dev.intr_coal_div = %d\n",
@@ -784,6 +762,7 @@ static int ionic_nway_reset(struct net_device *netdev)
 }
 
 static const struct ethtool_ops ionic_ethtool_ops = {
+	.coalesce_types = ETHTOOL_COALESCE_USECS,
 	.get_drvinfo		= ionic_get_drvinfo,
 	.get_regs_len		= ionic_get_regs_len,
 	.get_regs		= ionic_get_regs,
-- 
2.24.1

[PATCH net-next 07/12] hisilicon: let core reject the unsupported coalescing parameters

[Jakub Kicinski]

Set ethtool_ops->coalesce_types to let the core reject
unsupported coalescing parameters.

This driver correctly rejects all unsupported parameters.
As a side effect of these changes the error code for
unsupported params changes from EOPNOTSUPP to EINVAL.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 drivers/net/ethernet/hisilicon/hip04_eth.c | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hip04_eth.c b/drivers/net/ethernet/hisilicon/hip04_eth.c
index d9718b87279d..74d1a5778c3e 100644
--- a/drivers/net/ethernet/hisilicon/hip04_eth.c
+++ b/drivers/net/ethernet/hisilicon/hip04_eth.c
@@ -811,20 +811,6 @@ static int hip04_set_coalesce(struct net_device *netdev,
 {
 	struct hip04_priv *priv = netdev_priv(netdev);
 
-	/* Check not supported parameters  */
-	if ((ec->rx_max_coalesced_frames) || (ec->rx_coalesce_usecs_irq) ||
-	    (ec->rx_max_coalesced_frames_irq) || (ec->tx_coalesce_usecs_irq) ||
-	    (ec->use_adaptive_rx_coalesce) || (ec->use_adaptive_tx_coalesce) ||
-	    (ec->pkt_rate_low) || (ec->rx_coalesce_usecs_low) ||
-	    (ec->rx_max_coalesced_frames_low) || (ec->tx_coalesce_usecs_high) ||
-	    (ec->tx_max_coalesced_frames_low) || (ec->pkt_rate_high) ||
-	    (ec->tx_coalesce_usecs_low) || (ec->rx_coalesce_usecs_high) ||
-	    (ec->rx_max_coalesced_frames_high) || (ec->rx_coalesce_usecs) ||
-	    (ec->tx_max_coalesced_frames_irq) ||
-	    (ec->stats_block_coalesce_usecs) ||
-	    (ec->tx_max_coalesced_frames_high) || (ec->rate_sample_interval))
-		return -EOPNOTSUPP;
-
 	if ((ec->tx_coalesce_usecs > HIP04_MAX_TX_COALESCE_USECS ||
 	     ec->tx_coalesce_usecs < HIP04_MIN_TX_COALESCE_USECS) ||
 	    (ec->tx_max_coalesced_frames > HIP04_MAX_TX_COALESCE_FRAMES ||
@@ -845,6 +831,8 @@ static void hip04_get_drvinfo(struct net_device *netdev,
 }
 
 static const struct ethtool_ops hip04_ethtool_ops = {
+	.coalesce_types		= ETHTOOL_COALESCE_TX_USECS |
+				  ETHTOOL_COALESCE_TX_MAX_FRAMES,
 	.get_coalesce		= hip04_get_coalesce,
 	.set_coalesce		= hip04_set_coalesce,
 	.get_drvinfo		= hip04_get_drvinfo,
-- 
2.24.1

[PATCH net-next 08/12] ice: let core reject the unsupported coalescing parameters

[Jakub Kicinski]

Set ethtool_ops->coalesce_types to let the core reject
unsupported coalescing parameters.

This driver correctly rejects all unsupported parameters.
As a side effect of these changes the info message about
the bad parameter will no longer be printed. We also
always reject the tx_coalesce_usecs_high param, even
if the target queue pair does not have a TX queue.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 drivers/net/ethernet/intel/ice/ice_ethtool.c | 59 +-------------------
 1 file changed, 3 insertions(+), 56 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c b/drivers/net/ethernet/intel/ice/ice_ethtool.c
index ab37dddb225b..d1e9ec790424 100644
--- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
+++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
@@ -3452,12 +3452,6 @@ ice_set_rc_coalesce(enum ice_container_type c_type, struct ethtool_coalesce *ec,
 
 		break;
 	case ICE_TX_CONTAINER:
-		if (ec->tx_coalesce_usecs_high) {
-			netdev_info(vsi->netdev, "setting %s-usecs-high is not supported\n",
-				    c_type_str);
-			return -EINVAL;
-		}
-
 		use_adaptive_coalesce = ec->use_adaptive_tx_coalesce;
 		coalesce_usecs = ec->tx_coalesce_usecs;
 
@@ -3533,53 +3527,6 @@ ice_set_q_coalesce(struct ice_vsi *vsi, struct ethtool_coalesce *ec, int q_num)
 	return 0;
 }
 
-/**
- * ice_is_coalesce_param_invalid - check for unsupported coalesce parameters
- * @netdev: pointer to the netdev associated with this query
- * @ec: ethtool structure to fill with driver's coalesce settings
- *
- * Print netdev info if driver doesn't support one of the parameters
- * and return error. When any parameters will be implemented, remove only
- * this parameter from param array.
- */
-static int
-ice_is_coalesce_param_invalid(struct net_device *netdev,
-			      struct ethtool_coalesce *ec)
-{
-	struct ice_ethtool_not_used {
-		u32 value;
-		const char *name;
-	} param[] = {
-		{ec->stats_block_coalesce_usecs, "stats-block-usecs"},
-		{ec->rate_sample_interval, "sample-interval"},
-		{ec->pkt_rate_low, "pkt-rate-low"},
-		{ec->pkt_rate_high, "pkt-rate-high"},
-		{ec->rx_max_coalesced_frames, "rx-frames"},
-		{ec->rx_coalesce_usecs_irq, "rx-usecs-irq"},
-		{ec->rx_max_coalesced_frames_irq, "rx-frames-irq"},
-		{ec->tx_max_coalesced_frames, "tx-frames"},
-		{ec->tx_coalesce_usecs_irq, "tx-usecs-irq"},
-		{ec->tx_max_coalesced_frames_irq, "tx-frames-irq"},
-		{ec->rx_coalesce_usecs_low, "rx-usecs-low"},
-		{ec->rx_max_coalesced_frames_low, "rx-frames-low"},
-		{ec->tx_coalesce_usecs_low, "tx-usecs-low"},
-		{ec->tx_max_coalesced_frames_low, "tx-frames-low"},
-		{ec->rx_max_coalesced_frames_high, "rx-frames-high"},
-		{ec->tx_max_coalesced_frames_high, "tx-frames-high"}
-	};
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(param); i++) {
-		if (param[i].value) {
-			netdev_info(netdev, "Setting %s not supported\n",
-				    param[i].name);
-			return -EINVAL;
-		}
-	}
-
-	return 0;
-}
-
 /**
  * ice_print_if_odd_usecs - print message if user tries to set odd [tx|rx]-usecs
  * @netdev: netdev used for print
@@ -3620,9 +3567,6 @@ __ice_set_coalesce(struct net_device *netdev, struct ethtool_coalesce *ec,
 	struct ice_netdev_priv *np = netdev_priv(netdev);
 	struct ice_vsi *vsi = np->vsi;
 
-	if (ice_is_coalesce_param_invalid(netdev, ec))
-		return -EINVAL;
-
 	if (q_num < 0) {
 		struct ice_q_vector *q_vector = vsi->q_vectors[0];
 		int v_idx;
@@ -3817,6 +3761,9 @@ ice_get_module_eeprom(struct net_device *netdev,
 }
 
 static const struct ethtool_ops ice_ethtool_ops = {
+	.coalesce_types = ETHTOOL_COALESCE_USECS |
+			  ETHTOOL_COALESCE_USE_ADAPTIVE_RX |
+			  ETHTOOL_COALESCE_RX_USECS_HIGH,
 	.get_link_ksettings	= ice_get_link_ksettings,
 	.set_link_ksettings	= ice_set_link_ksettings,
 	.get_drvinfo		= ice_get_drvinfo,
-- 
2.24.1

[PATCH net-next 09/12] bnxt: reject unsupported coalescing params

[Jakub Kicinski]

Set ethtool_ops->coalesce_types to let the core reject
unsupported coalescing parameters.

This driver did not previously reject unsupported parameters.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index e8fc1671c581..1316432f70e1 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -3473,6 +3473,12 @@ void bnxt_ethtool_free(struct bnxt *bp)
 }
 
 const struct ethtool_ops bnxt_ethtool_ops = {
+	.coalesce_types = ETHTOOL_COALESCE_USECS |
+			  ETHTOOL_COALESCE_MAX_FRAMES |
+			  ETHTOOL_COALESCE_USECS_IRQ |
+			  ETHTOOL_COALESCE_MAX_FRAMES_IRQ |
+			  ETHTOOL_COALESCE_STATS_BLOCK_USECS |
+			  ETHTOOL_COALESCE_USE_ADAPTIVE_RX,
 	.get_link_ksettings	= bnxt_get_link_ksettings,
 	.set_link_ksettings	= bnxt_set_link_ksettings,
 	.get_pauseparam		= bnxt_get_pauseparam,
-- 
2.24.1

[PATCH net-next 10/12] mlx5: reject unsupported coalescing params

[Jakub Kicinski]

Set ethtool_ops->coalesce_types to let the core reject
unsupported coalescing parameters.

This driver did not previously reject unsupported parameters.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c    | 3 +++
 drivers/net/ethernet/mellanox/mlx5/core/en_rep.c        | 6 ++++++
 drivers/net/ethernet/mellanox/mlx5/core/ipoib/ethtool.c | 3 +++
 3 files changed, 12 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
index 06f6f08ff5eb..739a0771944a 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
@@ -1965,6 +1965,9 @@ static int mlx5e_set_rxnfc(struct net_device *dev, struct ethtool_rxnfc *cmd)
 }
 
 const struct ethtool_ops mlx5e_ethtool_ops = {
+	.coalesce_types = ETHTOOL_COALESCE_USECS |
+			  ETHTOOL_COALESCE_MAX_FRAMES |
+			  ETHTOOL_COALESCE_USE_ADAPTIVE,
 	.get_drvinfo       = mlx5e_get_drvinfo,
 	.get_link          = ethtool_op_get_link,
 	.get_strings       = mlx5e_get_strings,
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
index 1a8897f80547..abb8071e58c5 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
@@ -376,6 +376,9 @@ static int mlx5e_uplink_rep_set_link_ksettings(struct net_device *netdev,
 }
 
 static const struct ethtool_ops mlx5e_rep_ethtool_ops = {
+	.coalesce_types = ETHTOOL_COALESCE_USECS |
+			  ETHTOOL_COALESCE_MAX_FRAMES |
+			  ETHTOOL_COALESCE_USE_ADAPTIVE,
 	.get_drvinfo	   = mlx5e_rep_get_drvinfo,
 	.get_link	   = ethtool_op_get_link,
 	.get_strings       = mlx5e_rep_get_strings,
@@ -392,6 +395,9 @@ static const struct ethtool_ops mlx5e_rep_ethtool_ops = {
 };
 
 static const struct ethtool_ops mlx5e_uplink_rep_ethtool_ops = {
+	.coalesce_types = ETHTOOL_COALESCE_USECS |
+			  ETHTOOL_COALESCE_MAX_FRAMES |
+			  ETHTOOL_COALESCE_USE_ADAPTIVE,
 	.get_drvinfo	   = mlx5e_uplink_rep_get_drvinfo,
 	.get_link	   = ethtool_op_get_link,
 	.get_strings       = mlx5e_rep_get_strings,
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ethtool.c
index 90cb50fe17fd..0deeed70f128 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ethtool.c
@@ -235,6 +235,9 @@ static int mlx5i_get_link_ksettings(struct net_device *netdev,
 }
 
 const struct ethtool_ops mlx5i_ethtool_ops = {
+	.coalesce_types = ETHTOOL_COALESCE_USECS |
+			  ETHTOOL_COALESCE_MAX_FRAMES |
+			  ETHTOOL_COALESCE_USE_ADAPTIVE,
 	.get_drvinfo        = mlx5i_get_drvinfo,
 	.get_strings        = mlx5i_get_strings,
 	.get_sset_count     = mlx5i_get_sset_count,
-- 
2.24.1

[PATCH net-next 11/12] e1000e: reject unsupported coalescing params

[Jakub Kicinski]

Set ethtool_ops->coalesce_types to let the core reject
unsupported coalescing parameters.

This driver did not previously reject unsupported parameters.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 drivers/net/ethernet/intel/e1000e/ethtool.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/intel/e1000e/ethtool.c b/drivers/net/ethernet/intel/e1000e/ethtool.c
index 9e7881db7859..9aef691a0e3a 100644
--- a/drivers/net/ethernet/intel/e1000e/ethtool.c
+++ b/drivers/net/ethernet/intel/e1000e/ethtool.c
@@ -2307,6 +2307,7 @@ static int e1000e_get_ts_info(struct net_device *netdev,
 }
 
 static const struct ethtool_ops e1000_ethtool_ops = {
+	.coalesce_types = ETHTOOL_COALESCE_RX_USECS,
 	.get_drvinfo		= e1000_get_drvinfo,
 	.get_regs_len		= e1000_get_regs_len,
 	.get_regs		= e1000_get_regs,
-- 
2.24.1

[PATCH net-next 12/12] virtio_net: reject unsupported coalescing params

[Jakub Kicinski]

Set ethtool_ops->coalesce_types to let the core reject
unsupported coalescing parameters.

This driver did not previously reject unsupported parameters.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 drivers/net/virtio_net.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 84c0d9581f93..cfca3d134d4e 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2273,6 +2273,7 @@ static void virtnet_update_settings(struct virtnet_info *vi)
 }
 
 static const struct ethtool_ops virtnet_ethtool_ops = {
+	.coalesce_types = ETHTOOL_COALESCE_TX_MAX_FRAMES,
 	.get_drvinfo = virtnet_get_drvinfo,
 	.get_link = ethtool_op_get_link,
 	.get_ringparam = virtnet_get_ringparam,
-- 
2.24.1

Re: [PATCH net-next 12/12] virtio_net: reject unsupported coalescing params

[Jakub Kicinski]

On Tue,  3 Mar 2020 19:55:01 -0800 Jakub Kicinski wrote:
> Set ethtool_ops->coalesce_types to let the core reject
> unsupported coalescing parameters.
> 
> This driver did not previously reject unsupported parameters.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
>  drivers/net/virtio_net.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 84c0d9581f93..cfca3d134d4e 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -2273,6 +2273,7 @@ static void virtnet_update_settings(struct virtnet_info *vi)
>  }
>  
>  static const struct ethtool_ops virtnet_ethtool_ops = {
> +	.coalesce_types = ETHTOOL_COALESCE_TX_MAX_FRAMES,

Ugh, the last minute addition backfired, virtio_net does not support
changing RX settings, but they must be always configured to 1. This
should have been:

	.coalesce_types = ETHTOOL_COALESCE_MAX_FRAMES,

And the driver does in fact check for unsupported parameters.

>  	.get_drvinfo = virtnet_get_drvinfo,
>  	.get_link = ethtool_op_get_link,
>  	.get_ringparam = virtnet_get_ringparam,

Re: [PATCH net-next 06/12] ionic: let core reject the unsupported coalescing parameters

[Shannon Nelson]

On 3/3/20 7:54 PM, Jakub Kicinski wrote:
> Set ethtool_ops->coalesce_types to let the core reject
> unsupported coalescing parameters.
>
> This driver correctly rejects all unsupported parameters.
> No functional changes.
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

Thanks!

Acked-by: Shannon Nelson <snelson@pensando.io>


> ---
>   .../ethernet/pensando/ionic/ionic_ethtool.c   | 23 +------------------
>   1 file changed, 1 insertion(+), 22 deletions(-)
>
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
> index f778fff034f5..83ea35715533 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
> @@ -412,28 +412,6 @@ static int ionic_set_coalesce(struct net_device *netdev,
>   	unsigned int i;
>   	u32 coal;
>   
> -	if (coalesce->rx_max_coalesced_frames ||
> -	    coalesce->rx_coalesce_usecs_irq ||
> -	    coalesce->rx_max_coalesced_frames_irq ||
> -	    coalesce->tx_max_coalesced_frames ||
> -	    coalesce->tx_coalesce_usecs_irq ||
> -	    coalesce->tx_max_coalesced_frames_irq ||
> -	    coalesce->stats_block_coalesce_usecs ||
> -	    coalesce->use_adaptive_rx_coalesce ||
> -	    coalesce->use_adaptive_tx_coalesce ||
> -	    coalesce->pkt_rate_low ||
> -	    coalesce->rx_coalesce_usecs_low ||
> -	    coalesce->rx_max_coalesced_frames_low ||
> -	    coalesce->tx_coalesce_usecs_low ||
> -	    coalesce->tx_max_coalesced_frames_low ||
> -	    coalesce->pkt_rate_high ||
> -	    coalesce->rx_coalesce_usecs_high ||
> -	    coalesce->rx_max_coalesced_frames_high ||
> -	    coalesce->tx_coalesce_usecs_high ||
> -	    coalesce->tx_max_coalesced_frames_high ||
> -	    coalesce->rate_sample_interval)
> -		return -EINVAL;
> -
>   	ident = &lif->ionic->ident;
>   	if (ident->dev.intr_coal_div == 0) {
>   		netdev_warn(netdev, "bad HW value in dev.intr_coal_div = %d\n",
> @@ -784,6 +762,7 @@ static int ionic_nway_reset(struct net_device *netdev)
>   }
>   
>   static const struct ethtool_ops ionic_ethtool_ops = {
> +	.coalesce_types = ETHTOOL_COALESCE_USECS,
>   	.get_drvinfo		= ionic_get_drvinfo,
>   	.get_regs_len		= ionic_get_regs_len,
>   	.get_regs		= ionic_get_regs,

Re: [PATCH net-next 08/12] ice: let core reject the unsupported coalescing parameters

[Jakub Kicinski]

On Tue,  3 Mar 2020 19:54:57 -0800 Jakub Kicinski wrote:
> @@ -3817,6 +3761,9 @@ ice_get_module_eeprom(struct net_device *netdev,
>  }
>  
>  static const struct ethtool_ops ice_ethtool_ops = {
> +	.coalesce_types = ETHTOOL_COALESCE_USECS |
> +			  ETHTOOL_COALESCE_USE_ADAPTIVE_RX |

Triple checking now this should have been ETHTOOL_COALESCE_USE_ADAPTIVE,
as TX also works. Let me resend, sorry for the noise.. :(

> +			  ETHTOOL_COALESCE_RX_USECS_HIGH,
>  	.get_link_ksettings	= ice_get_link_ksettings,
>  	.set_link_ksettings	= ice_set_link_ksettings,
>  	.get_drvinfo		= ice_get_drvinfo,

Re: [PATCH net-next 01/12] ethtool: add infrastructure for centralized checking of coalescing parameters

[Andrew Lunn]

On Tue, Mar 03, 2020 at 07:54:50PM -0800, Jakub Kicinski wrote:
> @@ -2336,6 +2394,11 @@ ethtool_set_per_queue_coalesce(struct net_device *dev,
>  			goto roll_back;
>  		}
>  
> +		if (!ethtool_set_coalesce_supported(dev, &coalesce)) {
> +			ret = -EINVAL;
> +			goto roll_back;
> +		}

Hi Jakub

EOPNOTSUPP? 

    Andrew

Re: [PATCH net-next 01/12] ethtool: add infrastructure for centralized checking of coalescing parameters

[Michal Kubecek]

On Wed, Mar 04, 2020 at 03:54:39PM +0100, Andrew Lunn wrote:
> On Tue, Mar 03, 2020 at 07:54:50PM -0800, Jakub Kicinski wrote:
> > @@ -2336,6 +2394,11 @@ ethtool_set_per_queue_coalesce(struct net_device *dev,
> >  			goto roll_back;
> >  		}
> >  
> > +		if (!ethtool_set_coalesce_supported(dev, &coalesce)) {
> > +			ret = -EINVAL;
> > +			goto roll_back;
> > +		}
> 
> Hi Jakub
> 
> EOPNOTSUPP? 

Out of the 11 drivers patched in the rest of the series, 4 used
EOPNOTSUPP, 3 EINVAL and 4 just ignored unsupported parameters so there
doesn't seem to be a clear consensus. Personally I find EOPNOTSUPP more
appropriate but it's quite common that drivers return EINVAL for
parameters they don't understand or support.

BtW, there is a v2 later.

Michal

Re: [PATCH net-next 01/12] ethtool: add infrastructure for centralized checking of coalescing parameters

[Andrew Lunn]

On Wed, Mar 04, 2020 at 04:19:58PM +0100, Michal Kubecek wrote:
> On Wed, Mar 04, 2020 at 03:54:39PM +0100, Andrew Lunn wrote:
> > On Tue, Mar 03, 2020 at 07:54:50PM -0800, Jakub Kicinski wrote:
> > > @@ -2336,6 +2394,11 @@ ethtool_set_per_queue_coalesce(struct net_device *dev,
> > >  			goto roll_back;
> > >  		}
> > >  
> > > +		if (!ethtool_set_coalesce_supported(dev, &coalesce)) {
> > > +			ret = -EINVAL;
> > > +			goto roll_back;
> > > +		}
> > 
> > Hi Jakub
> > 
> > EOPNOTSUPP? 
> 
> Out of the 11 drivers patched in the rest of the series, 4 used
> EOPNOTSUPP, 3 EINVAL and 4 just ignored unsupported parameters so there
> doesn't seem to be a clear consensus. Personally I find EOPNOTSUPP more
> appropriate but it's quite common that drivers return EINVAL for
> parameters they don't understand or support.

Hi Michel

Yes, as i looked through the later patches, it became clear there is
no consensus. I personally prefer EOPNOTSUPP, but don't care too much.

   Andrew

Re: [PATCH net-next 01/12] ethtool: add infrastructure for centralized checking of coalescing parameters

[Jakub Kicinski]

On Wed, 4 Mar 2020 16:22:49 +0100 Andrew Lunn wrote:
> On Wed, Mar 04, 2020 at 04:19:58PM +0100, Michal Kubecek wrote:
> > On Wed, Mar 04, 2020 at 03:54:39PM +0100, Andrew Lunn wrote:  
> > > On Tue, Mar 03, 2020 at 07:54:50PM -0800, Jakub Kicinski wrote:  
> > > > @@ -2336,6 +2394,11 @@ ethtool_set_per_queue_coalesce(struct net_device *dev,
> > > >  			goto roll_back;
> > > >  		}
> > > >  
> > > > +		if (!ethtool_set_coalesce_supported(dev, &coalesce)) {
> > > > +			ret = -EINVAL;
> > > > +			goto roll_back;
> > > > +		}  
> > > 
> > > Hi Jakub
> > > 
> > > EOPNOTSUPP?   
> > 
> > Out of the 11 drivers patched in the rest of the series, 4 used
> > EOPNOTSUPP, 3 EINVAL and 4 just ignored unsupported parameters so there
> > doesn't seem to be a clear consensus. Personally I find EOPNOTSUPP more
> > appropriate but it's quite common that drivers return EINVAL for
> > parameters they don't understand or support.  
> 
> Hi Michel
> 
> Yes, as i looked through the later patches, it became clear there is
> no consensus. I personally prefer EOPNOTSUPP, but don't care too much.

Yeah, looking through the entire tree the tally seems to be:

EOPNOTSUPP  9
EINVAL      7
ENOTSUPP    2

I think EINVAL is traditionally the black box "can't do" for the stack.
But also - no strong feelings, I can switch to EOPNOTSUPP in v3.