[RFC] net: fix ethtool->set_flags not intended -EINVAL return value

netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

From: Stanislaw Gruszka <sgruszka@redhat.com>
To: Ben Hutchings <bhutchings@solarflare.com>
Cc: Amit Salecha <amit.salecha@qlogic.com>,
	Eric Dumazet <eric.dumazet@gmail.com>,
	Jesper Dangaard Brouer <hawk@comx.dk>,
	Alexander Duyck <alexander.h.duyck@intel.com>,
	netdev <netdev@vger.kernel.org>,
	Neil Horman <nhorman@tuxdriver.com>
Subject: [RFC] net: fix ethtool->set_flags not intended -EINVAL return value
Date: Mon, 21 Mar 2011 16:10:16 +0100	[thread overview]
Message-ID: <20110321151015.GA2209@redhat.com> (raw)
In-Reply-To: <1300715543.26693.349.camel@localhost>

After commit d5dbda23804156ae6f35025ade5307a49d1db6d7 "ethtool: Add
support for vlan accleration.", drivers that have NETIF_F_HW_VLAN_TX,
and/or NETIF_F_HW_VLAN_RX feature, but do not allow enable/disable vlan
acceleration via ethtool set_flags, always return -EINVAL from that
function. Fix by returning -EINVAL only if requested features do
not match current settings and can not be changed by driver.

RFC for now, compile tested only.

diff --git a/drivers/net/netxen/netxen_nic_ethtool.c b/drivers/net/netxen/netxen_nic_ethtool.c
index 653d308..3bdcc80 100644
--- a/drivers/net/netxen/netxen_nic_ethtool.c
+++ b/drivers/net/netxen/netxen_nic_ethtool.c
@@ -871,7 +871,7 @@ static int netxen_nic_set_flags(struct net_device *netdev, u32 data)
 	struct netxen_adapter *adapter = netdev_priv(netdev);
 	int hw_lro;
 
-	if (data & ~ETH_FLAG_LRO)
+	if (ethtool_invalid_flags(netdev, data, ETH_FLAG_LRO))
 		return -EINVAL;
 
 	if (!(adapter->capabilities & NX_FW_CAPABILITY_HW_LRO))
diff --git a/drivers/net/qlcnic/qlcnic_ethtool.c b/drivers/net/qlcnic/qlcnic_ethtool.c
index 4c14510..45b2755 100644
--- a/drivers/net/qlcnic/qlcnic_ethtool.c
+++ b/drivers/net/qlcnic/qlcnic_ethtool.c
@@ -1003,7 +1003,7 @@ static int qlcnic_set_flags(struct net_device *netdev, u32 data)
 	struct qlcnic_adapter *adapter = netdev_priv(netdev);
 	int hw_lro;
 
-	if (data & ~ETH_FLAG_LRO)
+	if (ethtool_invalid_flags(netdev, data, ETH_FLAG_LRO))
 		return -EINVAL;
 
 	if (!(adapter->capabilities & QLCNIC_FW_CAPABILITY_HW_LRO))
diff --git a/drivers/net/s2io.c b/drivers/net/s2io.c
index 2ad6364..356e74d 100644
--- a/drivers/net/s2io.c
+++ b/drivers/net/s2io.c
@@ -6726,7 +6726,7 @@ static int s2io_ethtool_set_flags(struct net_device *dev, u32 data)
 	int rc = 0;
 	int changed = 0;
 
-	if (data & ~ETH_FLAG_LRO)
+	if (ethtool_invalid_flags(dev, data, ETH_FLAG_LRO))
 		return -EINVAL;
 
 	if (data & ETH_FLAG_LRO) {
diff --git a/drivers/net/vmxnet3/vmxnet3_ethtool.c b/drivers/net/vmxnet3/vmxnet3_ethtool.c
index 81254be..51f2ef1 100644
--- a/drivers/net/vmxnet3/vmxnet3_ethtool.c
+++ b/drivers/net/vmxnet3/vmxnet3_ethtool.c
@@ -304,8 +304,8 @@ vmxnet3_set_flags(struct net_device *netdev, u32 data)
 	u8 lro_present = (netdev->features & NETIF_F_LRO) == 0 ? 0 : 1;
 	unsigned long flags;
 
-	if (data & ~ETH_FLAG_LRO)
-		return -EOPNOTSUPP;
+	if (ethtool_invalid_flags(netdev, data, ETH_FLAG_LRO))
+		return -EINVAL;
 
 	if (lro_requested ^ lro_present) {
 		/* toggle the LRO feature*/
diff --git a/drivers/net/vxge/vxge-ethtool.c b/drivers/net/vxge/vxge-ethtool.c
index 1dd3a21..c5eb034 100644
--- a/drivers/net/vxge/vxge-ethtool.c
+++ b/drivers/net/vxge/vxge-ethtool.c
@@ -1117,8 +1117,8 @@ static int vxge_set_flags(struct net_device *dev, u32 data)
 	struct vxgedev *vdev = netdev_priv(dev);
 	enum vxge_hw_status status;
 
-	if (data & ~ETH_FLAG_RXHASH)
-		return -EOPNOTSUPP;
+	if (ethtool_invalid_flags(dev, data, ETH_FLAG_RXHASH))
+		return -EINVAL;
 
 	if (!!(data & ETH_FLAG_RXHASH) == vdev->devh->config.rth_en)
 		return 0;
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index aac3e2e..e22b046 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -643,6 +643,7 @@ int ethtool_op_set_ufo(struct net_device *dev, u32 data);
 u32 ethtool_op_get_flags(struct net_device *dev);
 int ethtool_op_set_flags(struct net_device *dev, u32 data, u32 supported);
 void ethtool_ntuple_flush(struct net_device *dev);
+bool ethtool_invalid_flags(struct net_device *dev, u32 data, u32 supported);
 
 /**
  * &ethtool_ops - Alter and report network device settings
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index c1a71bb..514127d 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -141,9 +141,17 @@ u32 ethtool_op_get_flags(struct net_device *dev)
 }
 EXPORT_SYMBOL(ethtool_op_get_flags);
 
+bool ethtool_invalid_flags(struct net_device *dev, u32 data, u32 supported)
+{
+	u32 features = dev->features & flags_dup_features;
+
+	return ((features & ~supported) != (data & ~supported));
+}
+EXPORT_SYMBOL(ethtool_invalid_flags);
+
 int ethtool_op_set_flags(struct net_device *dev, u32 data, u32 supported)
 {
-	if (data & ~supported)
+	if (ethtool_invalid_flags(dev, data, supported))
 		return -EINVAL;
 
 	dev->features = ((dev->features & ~flags_dup_features) |

next prev parent reply	other threads:[~2011-03-21 15:10 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-03-18 11:12 LRO disable warnings on kernel 2.6.38 Jesper Dangaard Brouer
2011-03-18 13:05 ` Eric Dumazet
2011-03-18 14:17   ` Ben Hutchings
2011-03-18 14:33     ` Eric Dumazet
2011-03-18 15:15       ` Stephen Hemminger
2011-03-18 19:52         ` David Miller
2011-03-18 19:58           ` Ben Hutchings
2011-03-18 19:59             ` David Miller
2011-03-18 15:40 ` Ben Hutchings
2011-03-18 16:18 ` Alexander Duyck
2011-03-21  9:21   ` Jesper Dangaard Brouer
2011-03-21  9:45     ` Eric Dumazet
2011-03-21  9:53       ` Eric Dumazet
2011-03-21 10:04         ` Jesper Dangaard Brouer
2011-03-21 10:11           ` Eric Dumazet
2011-03-21 10:27             ` Jesper Dangaard Brouer
2011-03-21 10:00       ` Amit Salecha
2011-03-21 12:34         ` Stanislaw Gruszka
2011-03-21 13:46           ` Stanislaw Gruszka
2011-03-21 13:52             ` Ben Hutchings
2011-03-21 15:10               ` Stanislaw Gruszka [this message]
2011-03-21 15:15                 ` [RFC] net: fix ethtool->set_flags not intended -EINVAL return value Ben Hutchings
2011-03-22  2:41                 ` Jesse Gross

find likely ancestor, descendant, or conflicting patches for this message:
( dfblob:653d308 dfblob:3bdcc80 dfblob:4c14510 dfblob:45b2755
dfblob:2ad6364 dfblob:356e74d dfblob:81254be dfblob:51f2ef1
dfblob:1dd3a21 dfblob:c5eb034 dfblob:aac3e2e dfblob:e22b046
dfblob:c1a71bb dfblob:514127d )
 OR (
bs:"[RFC] net: fix ethtool->set_flags not intended -EINVAL return value" )
	(help)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110321151015.GA2209@redhat.com \
    --to=sgruszka@redhat.com \
    --cc=alexander.h.duyck@intel.com \
    --cc=amit.salecha@qlogic.com \
    --cc=bhutchings@solarflare.com \
    --cc=eric.dumazet@gmail.com \
    --cc=hawk@comx.dk \
    --cc=netdev@vger.kernel.org \
    --cc=nhorman@tuxdriver.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Be sure your reply has a Subject: header at the top and a blank line before the message body.

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).