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

All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.