Use of ENOTSUPP in drivers?

Subsystems: broadcom iproc gbit ethernet driver, intel ethernet drivers, marvell mv643xx ethernet driver, marvell mvneta ethernet driver, mellanox ethernet driver (mlx4_en), mellanox mlx4 core vpi driver, mellanox mlx5 core vpi driver, networking drivers, qlogic qlcnic (1/10)gb ethernet driver, the rest, ti ethernet switch driver (cpsw)

6 messages, 5 authors, 2014-01-13 · open the first message on its own page

Use of ENOTSUPP in drivers?

From: Sabrina Dubroca <sd@queasysnail.net>
Date: 2014-01-12 18:57:37

Thu, 2 Jan 2014 12:01:31 +0000, Ben Hutchings wrote:
Never return error code ENOTSUPP; it's *not* the same thing as ENOTSUP
in userland and is not part of the userland ABI.  I would use EINVAL
here.

I've found a few ethernet drivers that return -ENOTSUPP in various
functions. In particular, some ethtool functions or ioctl's.
Ben's message makes me think that the ethtool functions and ioctl's
should be modified.

There are other occurences, mostly in functions related to device
initialization. I didn't manage to track down exactly from where some
of them are called, and I don't know if ENOTSUPP is okay in these.

I've included the complete list of occurences (based on net-next) from
drivers/net/ethernet in patch form at the end, if that's more
convenient than the file/function list. This is not meant to be
applied.


Do these (or part of them) need to be patched? Or is there something
I'm missing?


Thanks,
Sabrina


---

* called from ethtool

  drivers/net/ethernet/intel/igb/igb_ethtool.c
    static int igb_get_sset_count(struct net_device *netdev, int sset)

  drivers/net/ethernet/intel/i40evf/i40evf_ethtool.c
    static int i40evf_get_sset_count(struct net_device *netdev, ...)

  drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c
    static int qlcnic_get_dump_flag(struct net_device *netdev, ...)
    static int qlcnic_get_dump_data(struct net_device *netdev, ...)

  drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_hw.c
    /* called via .self_test in struct ethtool_ops */
    int qlcnic_83xx_loopback_test(struct net_device *netdev, u8 mode)

  drivers/net/ethernet/calxeda/xgmac.c
    static int xgmac_set_wol(struct net_device *dev, ...)


* called as ndo_do_ioctl

  drivers/net/ethernet/ti/cpsw.c
    static int cpsw_ndo_ioctl(struct net_device *dev, ...)

  drivers/net/ethernet/marvell/mvneta.c
    static int mvneta_ioctl(struct net_device *dev, ...)

  drivers/net/ethernet/marvell/mv643xx_eth.c
    static int mv643xx_eth_ioctl(struct net_device *dev, ...)

  drivers/net/ethernet/freescale/fec_mpc52xx.c
    static int mpc52xx_fec_ioctl(struct net_device *dev, ...)


* called as ndo_init

  drivers/net/ethernet/lantiq_etop.c
    static int ltq_etop_hw_init(struct net_device *dev)


* called from ndo_open (return value ignored)/ not called

  drivers/net/ethernet/ti/davinci_cpdma.c
    int cpdma_control_get(struct cpdma_ctlr *ctlr, ...)
    int cpdma_control_set(struct cpdma_ctlr *ctlr, ...)


* not sure, probably fine

  drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
    /* stored as adapter->ptp_caps.enable, ie struct ptp_clock_info */
    static int ixgbe_ptp_enable(struct ptp_clock_info *ptp, ...)

  drivers/net/ethernet/mellanox/mlx4/en_dcb_nl.c
    /* called from dcbnl_rtnl_ops.ieee_setets */
    static int mlx4_en_ets_validate(struct mlx4_en_priv *priv, ...)

  drivers/net/ethernet/mellanox/mlx5/core/cmd.c
    /* called from pci_driver.probe in drivers/infiniband/hw/mlx5/main.c */
    int mlx5_cmd_init(struct mlx5_core_dev *dev)

  drivers/net/ethernet/mellanox/mlx4/mr.c
    int mlx4_mw_alloc(struct mlx4_dev *dev, u32 pd, ...)

  drivers/net/ethernet/broadcom/bgmac.c
    static int bgmac_probe(struct bcma_device *core)
    /* called by bgmac_probe */
    static int bgmac_dma_alloc(struct bgmac *bgmac)

---

ethtool functions


 drivers/net/ethernet/calxeda/xgmac.c                | 2 +-
 drivers/net/ethernet/intel/i40evf/i40evf_ethtool.c  | 2 +-
 drivers/net/ethernet/intel/igb/igb_ethtool.c        | 2 +-
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_hw.c | 2 +-
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c | 4 ++--
 5 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/calxeda/xgmac.c b/drivers/net/ethernet/calxeda/xgmac.c
index 4fc5c8e..3055a48 100644
--- a/drivers/net/ethernet/calxeda/xgmac.c
+++ b/drivers/net/ethernet/calxeda/xgmac.c
@@ -1677,7 +1677,7 @@ static int xgmac_set_wol(struct net_device *dev,
 	u32 support = WAKE_MAGIC | WAKE_UCAST;
 
 	if (!device_can_wakeup(priv->device))
-		return -ENOTSUPP;
+		return -EINVAL;
 
 	if (wol->wolopts & ~support)
 		return -EINVAL;
diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_ethtool.c b/drivers/net/ethernet/intel/i40evf/i40evf_ethtool.c
index b0b1f4b..604ffa0 100644
--- a/drivers/net/ethernet/intel/i40evf/i40evf_ethtool.c
+++ b/drivers/net/ethernet/intel/i40evf/i40evf_ethtool.c
@@ -96,7 +96,7 @@ static int i40evf_get_sset_count(struct net_device *netdev, int sset)
 	if (sset == ETH_SS_STATS)
 		return I40EVF_STATS_LEN;
 	else
-		return -ENOTSUPP;
+		return -EINVAL;
 }
 
 /**
diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
index 1df0237..c1cac5c 100644
--- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
+++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
@@ -2242,7 +2242,7 @@ static int igb_get_sset_count(struct net_device *netdev, int sset)
 	case ETH_SS_TEST:
 		return IGB_TEST_LEN;
 	default:
-		return -ENOTSUPP;
+		return -EINVAL;
 	}
 }
 
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_hw.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_hw.c
index 03eb2ad..bb9f4ec 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_hw.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_hw.c
@@ -1641,7 +1641,7 @@ int qlcnic_83xx_loopback_test(struct net_device *netdev, u8 mode)
 	if (ahw->op_mode == QLCNIC_NON_PRIV_FUNC) {
 		netdev_warn(netdev,
 			    "Loopback test not supported in non privileged mode\n");
-		return -ENOTSUPP;
+		return -EINVAL;
 	}
 
 	if (test_bit(__QLCNIC_RESETTING, &adapter->state)) {
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c
index 45fa6ef..727be4e 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c
@@ -1681,7 +1681,7 @@ qlcnic_get_dump_flag(struct net_device *netdev, struct ethtool_dump *dump)
 
 	if (!fw_dump->tmpl_hdr) {
 		netdev_err(adapter->netdev, "FW Dump not supported\n");
-		return -ENOTSUPP;
+		return -EINVAL;
 	}
 
 	if (fw_dump->clr)
@@ -1710,7 +1710,7 @@ qlcnic_get_dump_data(struct net_device *netdev, struct ethtool_dump *dump,
 
 	if (!fw_dump->tmpl_hdr) {
 		netdev_err(netdev, "FW Dump not supported\n");
-		return -ENOTSUPP;
+		return -EINVAL;
 	}
 
 	if (!fw_dump->clr) {

---
ndo_do_ioctl functions


 drivers/net/ethernet/freescale/fec_mpc52xx.c | 2 +-
 drivers/net/ethernet/marvell/mv643xx_eth.c   | 2 +-
 drivers/net/ethernet/marvell/mvneta.c        | 2 +-
 drivers/net/ethernet/ti/cpsw.c               | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec_mpc52xx.c b/drivers/net/ethernet/freescale/fec_mpc52xx.c
index 9947765..224f8db 100644
--- a/drivers/net/ethernet/freescale/fec_mpc52xx.c
+++ b/drivers/net/ethernet/freescale/fec_mpc52xx.c
@@ -810,7 +810,7 @@ static int mpc52xx_fec_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
 	struct mpc52xx_fec_priv *priv = netdev_priv(dev);
 
 	if (!priv->phydev)
-		return -ENOTSUPP;
+		return -EINVAL;
 
 	return phy_mii_ioctl(priv->phydev, rq, cmd);
 }
diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c
index a2565ce..7ef4388 100644
--- a/drivers/net/ethernet/marvell/mv643xx_eth.c
+++ b/drivers/net/ethernet/marvell/mv643xx_eth.c
@@ -2299,7 +2299,7 @@ static int mv643xx_eth_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 	int ret;
 
 	if (mp->phy == NULL)
-		return -ENOTSUPP;
+		return -EINVAL;
 
 	ret = phy_mii_ioctl(mp->phy, ifr, cmd);
 	if (!ret)
diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index d5f0d72..9bd8be5 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -2490,7 +2490,7 @@ static int mvneta_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 	int ret;
 
 	if (!pp->phy_dev)
-		return -ENOTSUPP;
+		return -EINVAL;
 
 	ret = phy_mii_ioctl(pp->phy_dev, ifr, cmd);
 	if (!ret)
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index e8bb77d..18395d1 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -1432,7 +1432,7 @@ static int cpsw_ndo_ioctl(struct net_device *dev, struct ifreq *req, int cmd)
 		data->phy_id = priv->slaves[slave_no].phy->addr;
 		break;
 	default:
-		return -ENOTSUPP;
+		return -EINVAL;
 	}
 
 	return 0;


---
other ndo_* functions


 drivers/net/ethernet/lantiq_etop.c      | 2 +-
 drivers/net/ethernet/ti/davinci_cpdma.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/lantiq_etop.c b/drivers/net/ethernet/lantiq_etop.c
index 974a007..fe2521c 100644
--- a/drivers/net/ethernet/lantiq_etop.c
+++ b/drivers/net/ethernet/lantiq_etop.c
@@ -265,7 +265,7 @@ ltq_etop_hw_init(struct net_device *dev)
 	default:
 		netdev_err(dev, "unknown mii mode %d\n",
 			priv->pldata->mii_mode);
-		return -ENOTSUPP;
+		return -EINVAL;
 	}
 
 	/* enable crc generation */
diff --git a/drivers/net/ethernet/ti/davinci_cpdma.c b/drivers/net/ethernet/ti/davinci_cpdma.c
index 364d0c7..99985db 100644
--- a/drivers/net/ethernet/ti/davinci_cpdma.c
+++ b/drivers/net/ethernet/ti/davinci_cpdma.c
@@ -993,7 +993,7 @@ int cpdma_control_get(struct cpdma_ctlr *ctlr, int control)
 
 	spin_lock_irqsave(&ctlr->lock, flags);
 
-	ret = -ENOTSUPP;
+	ret = -EINVAL;
 	if (!ctlr->params.has_ext_regs)
 		goto unlock_ret;
 
@@ -1025,7 +1025,7 @@ int cpdma_control_set(struct cpdma_ctlr *ctlr, int control, int value)
 
 	spin_lock_irqsave(&ctlr->lock, flags);
 
-	ret = -ENOTSUPP;
+	ret = -EINVAL;
 	if (!ctlr->params.has_ext_regs)
 		goto unlock_ret;


---
other functions


 drivers/net/ethernet/broadcom/bgmac.c          | 10 +++++-----
 drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c   |  2 +-
 drivers/net/ethernet/mellanox/mlx4/en_dcb_nl.c |  2 +-
 drivers/net/ethernet/mellanox/mlx4/mr.c        |  2 +-
 drivers/net/ethernet/mellanox/mlx5/core/cmd.c  |  2 +-
 5 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index 7f968a9..649c99e 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -487,7 +487,7 @@ static int bgmac_dma_alloc(struct bgmac *bgmac)
 
 	if (!(bcma_aread32(bgmac->core, BCMA_IOST) & BCMA_IOST_DMA64)) {
 		bgmac_err(bgmac, "Core does not report 64-bit DMA\n");
-		return -ENOTSUPP;
+		return -EINVAL;
 	}
 
 	for (i = 0; i < BGMAC_MAX_TX_RINGS; i++) {
@@ -1421,7 +1421,7 @@ static int bgmac_probe(struct bcma_device *core)
 	/* We don't support 2nd, 3rd, ... units, SPROM has to be adjusted */
 	if (core->core_unit > 1) {
 		pr_err("Unsupported core_unit %d\n", core->core_unit);
-		return -ENOTSUPP;
+		return -EINVAL;
 	}
 
 	if (!is_valid_ether_addr(mac)) {
@@ -1467,7 +1467,7 @@ static int bgmac_probe(struct bcma_device *core)
 
 	if (core->bus->hosttype == BCMA_HOSTTYPE_PCI) {
 		bgmac_err(bgmac, "PCI setup not implemented\n");
-		err = -ENOTSUPP;
+		err = -EINVAL;
 		goto err_netdev_free;
 	}
 
@@ -1518,14 +1518,14 @@ static int bgmac_probe(struct bcma_device *core)
 	err = bgmac_mii_register(bgmac);
 	if (err) {
 		bgmac_err(bgmac, "Cannot register MDIO\n");
-		err = -ENOTSUPP;
+		err = -EINVAL;
 		goto err_dma_free;
 	}
 
 	err = register_netdev(bgmac->net_dev);
 	if (err) {
 		bgmac_err(bgmac, "Cannot register net device\n");
-		err = -ENOTSUPP;
+		err = -EINVAL;
 		goto err_mii_unregister;
 	}
 
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
index 5184e2a..b26667e 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
@@ -368,7 +368,7 @@ static int ixgbe_ptp_enable(struct ptp_clock_info *ptp,
 		}
 	}
 
-	return -ENOTSUPP;
+	return -EINVAL;
 }
 
 /**
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_dcb_nl.c b/drivers/net/ethernet/mellanox/mlx4/en_dcb_nl.c
index b4881b6..17355ba 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_dcb_nl.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_dcb_nl.c
@@ -78,7 +78,7 @@ static int mlx4_en_ets_validate(struct mlx4_en_priv *priv, struct ieee_ets *ets)
 		default:
 			en_err(priv, "TC[%d]: Not supported TSA: %d\n",
 					i, ets->tc_tsa[i]);
-			return -ENOTSUPP;
+			return -EINVAL;
 		}
 	}
 
diff --git a/drivers/net/ethernet/mellanox/mlx4/mr.c b/drivers/net/ethernet/mellanox/mlx4/mr.c
index 0558ddd..a877a35 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mr.c
+++ b/drivers/net/ethernet/mellanox/mlx4/mr.c
@@ -660,7 +660,7 @@ int mlx4_mw_alloc(struct mlx4_dev *dev, u32 pd, enum mlx4_mw_type type,
 	     !(dev->caps.flags & MLX4_DEV_CAP_FLAG_MEM_WINDOW)) ||
 	     (type == MLX4_MW_TYPE_2 &&
 	     !(dev->caps.bmme_flags & MLX4_BMME_FLAG_TYPE_2_WIN)))
-		return -ENOTSUPP;
+		return -EINVAL;
 
 	index = mlx4_mpt_reserve(dev);
 	if (index == -1)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/cmd.c b/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
index 8675d26..5398fbb 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
@@ -1422,7 +1422,7 @@ int mlx5_cmd_init(struct mlx5_core_dev *dev)
 	if (cmd->cmdif_rev > CMD_IF_REV) {
 		dev_err(&dev->pdev->dev, "driver does not support command interface version. driver %d, firmware %d\n",
 			CMD_IF_REV, cmd->cmdif_rev);
-		err = -ENOTSUPP;
+		err = -EINVAL;
 		goto err_map;
 	}

Re: Use of ENOTSUPP in drivers?

From: Ben Hutchings <hidden>
Date: 2014-01-12 20:47:06

On Sun, 2014-01-12 at 19:57 +0100, Sabrina Dubroca wrote:
Thu, 2 Jan 2014 12:01:31 +0000, Ben Hutchings wrote:
quoted
Never return error code ENOTSUPP; it's *not* the same thing as ENOTSUP
in userland and is not part of the userland ABI.  I would use EINVAL
here.

I've found a few ethernet drivers that return -ENOTSUPP in various
functions. In particular, some ethtool functions or ioctl's.
Ben's message makes me think that the ethtool functions and ioctl's
should be modified.

There are other occurences, mostly in functions related to device
initialization. I didn't manage to track down exactly from where some
of them are called, and I don't know if ENOTSUPP is okay in these.

I've included the complete list of occurences (based on net-next) from
drivers/net/ethernet in patch form at the end, if that's more
convenient than the file/function list. This is not meant to be
applied.


Do these (or part of them) need to be patched? Or is there something
I'm missing?
[...]

I believe they should all be patched.  According to
include/linux/errno.h, ENOTSUPP is meant for use in the NFSv3 code only.
(But it's apparently erroneously used *all over* the tree, not just in
net drivers!)

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: Use of ENOTSUPP in drivers?

From: Florian Fainelli <f.fainelli@gmail.com>
Date: 2014-01-12 21:19:40

Le dimanche 12 janvier 2014, 20:47:02 Ben Hutchings a écrit :
On Sun, 2014-01-12 at 19:57 +0100, Sabrina Dubroca wrote:
quoted
Thu, 2 Jan 2014 12:01:31 +0000, Ben Hutchings wrote:
quoted
Never return error code ENOTSUPP; it's *not* the same thing as ENOTSUP
in userland and is not part of the userland ABI.  I would use EINVAL
here.
I've found a few ethernet drivers that return -ENOTSUPP in various
functions. In particular, some ethtool functions or ioctl's.
Ben's message makes me think that the ethtool functions and ioctl's
should be modified.

There are other occurences, mostly in functions related to device
initialization. I didn't manage to track down exactly from where some
of them are called, and I don't know if ENOTSUPP is okay in these.

I've included the complete list of occurences (based on net-next) from
drivers/net/ethernet in patch form at the end, if that's more
convenient than the file/function list. This is not meant to be
applied.


Do these (or part of them) need to be patched? Or is there something
I'm missing?
[...]

I believe they should all be patched.  According to
include/linux/errno.h, ENOTSUPP is meant for use in the NFSv3 code only.
(But it's apparently erroneously used *all over* the tree, not just in
net drivers!)
Most other drivers use -EOPNOTSUPP, which is arguably as bad as -ENOTSUPP, 
since the comment about it says:

* Operation not supported on transport endpoint *

But at least changing -ENOTSUPP to -EOPNOTSUPP until something better which is 
not protocol/endpoint specific is agreed on might be better for consistency?
-- 
Florian

Re: Use of ENOTSUPP in drivers?

From: Ben Hutchings <hidden>
Date: 2014-01-12 21:39:25

On Sun, 2014-01-12 at 13:19 -0800, Florian Fainelli wrote:
Le dimanche 12 janvier 2014, 20:47:02 Ben Hutchings a écrit :
quoted
On Sun, 2014-01-12 at 19:57 +0100, Sabrina Dubroca wrote:
quoted
Thu, 2 Jan 2014 12:01:31 +0000, Ben Hutchings wrote:
quoted
Never return error code ENOTSUPP; it's *not* the same thing as ENOTSUP
in userland and is not part of the userland ABI.  I would use EINVAL
here.
I've found a few ethernet drivers that return -ENOTSUPP in various
functions. In particular, some ethtool functions or ioctl's.
Ben's message makes me think that the ethtool functions and ioctl's
should be modified.

There are other occurences, mostly in functions related to device
initialization. I didn't manage to track down exactly from where some
of them are called, and I don't know if ENOTSUPP is okay in these.

I've included the complete list of occurences (based on net-next) from
drivers/net/ethernet in patch form at the end, if that's more
convenient than the file/function list. This is not meant to be
applied.


Do these (or part of them) need to be patched? Or is there something
I'm missing?
[...]

I believe they should all be patched.  According to
include/linux/errno.h, ENOTSUPP is meant for use in the NFSv3 code only.
(But it's apparently erroneously used *all over* the tree, not just in
net drivers!)
Most other drivers use -EOPNOTSUPP, which is arguably as bad as -ENOTSUPP, 
since the comment about it says:

* Operation not supported on transport endpoint *

But at least changing -ENOTSUPP to -EOPNOTSUPP until something better which is 
not protocol/endpoint specific is agreed on might be better for consistency?
It is not as bad, because EOPNOTSUPP is actually named in userland (in
fact it has two names, the other being ENOTSUP spelt with *one* P).

glibc's strerror() returns these strings for ENOTSUPP and EOPNOTSUPP
respectively:

"Unknown error 524"
"Operation not supported"

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

RE: Use of ENOTSUPP in drivers?

From: Shahed Shaikh <hidden>
Date: 2014-01-13 06:45:49

-----Original Message-----
From: netdev-owner@vger.kernel.org [mailto:netdev-
owner@vger.kernel.org] On Behalf Of Sabrina Dubroca
Sent: Monday, January 13, 2014 12:27 AM
To: netdev
Cc: Ben Hutchings
Subject: Use of ENOTSUPP in drivers?


Thu, 2 Jan 2014 12:01:31 +0000, Ben Hutchings wrote:
quoted
Never return error code ENOTSUPP; it's *not* the same thing as ENOTSUP
in userland and is not part of the userland ABI.  I would use EINVAL
here.

I've found a few ethernet drivers that return -ENOTSUPP in various
functions. In particular, some ethtool functions or ioctl's.
Ben's message makes me think that the ethtool functions and ioctl's should
be modified.

There are other occurences, mostly in functions related to device
initialization. I didn't manage to track down exactly from where some of
them are called, and I don't know if ENOTSUPP is okay in these.

I've included the complete list of occurences (based on net-next) from
drivers/net/ethernet in patch form at the end, if that's more convenient
than the file/function list. This is not meant to be applied.


Do these (or part of them) need to be patched? Or is there something I'm
missing?


Thanks,
Sabrina


---
Hi Sabrina,

Using -EOPNOTSUPP instead of -ENOTSUPP,  ethtool is giving the expected error message.

# ethtool -w p6p1
Can not get dump level
: Operation not supported
quoted hunk
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_hw.c
b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_hw.c
index 03eb2ad..bb9f4ec 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_hw.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_hw.c
@@ -1641,7 +1641,7 @@ int qlcnic_83xx_loopback_test(struct net_device
*netdev, u8 mode)
 	if (ahw->op_mode == QLCNIC_NON_PRIV_FUNC) {
 		netdev_warn(netdev,
 			    "Loopback test not supported in non privileged
mode\n");
-		return -ENOTSUPP;
+		return -EINVAL;
Please use -EOPNOTSUPP
quoted hunk
 	}

 	if (test_bit(__QLCNIC_RESETTING, &adapter->state)) { diff --git
a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c
b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c
index 45fa6ef..727be4e 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c
@@ -1681,7 +1681,7 @@ qlcnic_get_dump_flag(struct net_device *netdev,
struct ethtool_dump *dump)

 	if (!fw_dump->tmpl_hdr) {
 		netdev_err(adapter->netdev, "FW Dump not supported\n");
-		return -ENOTSUPP;
+		return -EINVAL;
Please use -EOPNOTSUPP
quoted hunk
 	}

 	if (fw_dump->clr)
@@ -1710,7 +1710,7 @@ qlcnic_get_dump_data(struct net_device *netdev,
struct ethtool_dump *dump,

 	if (!fw_dump->tmpl_hdr) {
 		netdev_err(netdev, "FW Dump not supported\n");
-		return -ENOTSUPP;
+		return -EINVAL;
Please use -EOPNOTSUPP
 	}

 	if (!fw_dump->clr) {
Thanks,
Shahed
  

RE: Use of ENOTSUPP in drivers?

From: David Laight <hidden>
Date: 2014-01-13 09:43:58

From: Ben Hutchings
I believe they should all be patched.  According to
include/linux/errno.h, ENOTSUPP is meant for use in the NFSv3 code only.
(But it's apparently erroneously used *all over* the tree, not just in
net drivers!)
The 'real' problem is that there are quite a few very specific errno
values, but very few general ones.
I suspect that once (way back in the 1980s - the definitions predate linux)
adding new errno values was easy, and some that might have been general
got documented for their single use - the text for EAGAIN used to be
"No more processes".
New errno values were added for some subsystems (like NFS) but documented
with subsystem-specific texts.
So there are few generic errno codes except EINVAL and ENXIO - neither
of which is helpful.
Nothing for thinks like:
- I don't understand the request.
- I don't support the request.
- Some internal limit reached.
etc.

	David
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help