diff mbox

[RFC,04/12] net: introduce NETIF_F_RXCSUM

Message ID fcee74e8b71430c0e0c267ac180f5861c471dc41.1292451560.git.mirq-linux@rere.qmqm.pl
State RFC, archived
Delegated to: David Miller
Headers show

Commit Message

Michał Mirosław Dec. 15, 2010, 10:24 p.m. UTC 
Introduce NETIF_F_RXCSUM to replace device-private flags for RX checksum
offload. Integrate it with ndo_fix_features.

ethtool_op_get_rx_csum() is removed altogether as nothing in-tree uses it.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 include/linux/ethtool.h   |    1 -
 include/linux/netdevice.h |    3 +++
 net/core/ethtool.c        |   34 +++++++++++-----------------------
 3 files changed, 14 insertions(+), 24 deletions(-)

Comments

Ben Hutchings Dec. 16, 2010, 11:27 p.m. UTC | #1 
On Wed, 2010-12-15 at 23:24 +0100, Michał Mirosław wrote:
> Introduce NETIF_F_RXCSUM to replace device-private flags for RX checksum
> offload. Integrate it with ndo_fix_features.
> 
> ethtool_op_get_rx_csum() is removed altogether as nothing in-tree uses it.

Not explicitly, but what about drivers that turn TX and RX checksumming
on and off together?  They are implicitly covered by the case which
you're removing:

[...]
> -	case ETHTOOL_GRXCSUM:
> -		rc = ethtool_get_value(dev, useraddr, ethcmd,
> -				       (dev->ethtool_ops->get_rx_csum ?
> -					dev->ethtool_ops->get_rx_csum :
> -					ethtool_op_get_rx_csum));
[...]

Ben.
Michał Mirosław Dec. 19, 2010, 12:57 a.m. UTC | #2 
On Thu, Dec 16, 2010 at 11:27:29PM +0000, Ben Hutchings wrote:
> On Wed, 2010-12-15 at 23:24 +0100, Michał Mirosław wrote:
> > Introduce NETIF_F_RXCSUM to replace device-private flags for RX checksum
> > offload. Integrate it with ndo_fix_features.
> > 
> > ethtool_op_get_rx_csum() is removed altogether as nothing in-tree uses it.
> Not explicitly, but what about drivers that turn TX and RX checksumming
> on and off together?  They are implicitly covered by the case which
> you're removing:
> [...]
> > -	case ETHTOOL_GRXCSUM:
> > -		rc = ethtool_get_value(dev, useraddr, ethcmd,
> > -				       (dev->ethtool_ops->get_rx_csum ?
> > -					dev->ethtool_ops->get_rx_csum :
> > -					ethtool_op_get_rx_csum));
> [...]

I had an impression that drivers not defining get_rx_csum() had no
RX checksumming or didn't allow it to be toggled. I'll have a closer look
on this.

Best Regards,
Michał Mirosław
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Michał Mirosław Jan. 4, 2011, 6:33 p.m. UTC | #3 
On Sun, Dec 19, 2010 at 01:57:15AM +0100, Michał Mirosław wrote:
> On Thu, Dec 16, 2010 at 11:27:29PM +0000, Ben Hutchings wrote:
> > On Wed, 2010-12-15 at 23:24 +0100, Michał Mirosław wrote:
> > > Introduce NETIF_F_RXCSUM to replace device-private flags for RX checksum
> > > offload. Integrate it with ndo_fix_features.
> > > 
> > > ethtool_op_get_rx_csum() is removed altogether as nothing in-tree uses it.
> > Not explicitly, but what about drivers that turn TX and RX checksumming
> > on and off together?  They are implicitly covered by the case which
> > you're removing:
> > [...]
> > > -	case ETHTOOL_GRXCSUM:
> > > -		rc = ethtool_get_value(dev, useraddr, ethcmd,
> > > -				       (dev->ethtool_ops->get_rx_csum ?
> > > -					dev->ethtool_ops->get_rx_csum :
> > > -					ethtool_op_get_rx_csum));
> > [...]
> I had an impression that drivers not defining get_rx_csum() had no
> RX checksumming or didn't allow it to be toggled. I'll have a closer look
> on this.

All drivers that have set_rx_csum() also define matching get_rx_csum().

Anyway, the code this is removing is broken in that it assumes TX csum
capability implying RX csum capability.

As a side note, GRO receive functions (TCPv4 and v6 - no other protocols
use it) are checking skb->ip_summed != CHECKSUM_NONE before allowing the
packet to be passed on, so it's not even necessary to force disabling GRO
when RX csum is off.

Best Regards,
Michał Mirosław
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 0267d45..2475b41 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -568,7 +568,6 @@  struct net_device;
 
 /* Some generic methods drivers may use in their ethtool_ops */
 u32 ethtool_op_get_link(struct net_device *dev);
-u32 ethtool_op_get_rx_csum(struct net_device *dev);
 u32 ethtool_op_get_tx_csum(struct net_device *dev);
 int ethtool_op_set_tx_csum(struct net_device *dev, u32 data);
 int ethtool_op_set_tx_hw_csum(struct net_device *dev, u32 data);
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 7634cab..ce35514 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -920,6 +920,7 @@  struct net_device {
 #define NETIF_F_FCOE_MTU	(1 << 26) /* Supports max FCoE MTU, 2158 bytes*/
 #define NETIF_F_NTUPLE		(1 << 27) /* N-tuple filters supported */
 #define NETIF_F_RXHASH		(1 << 28) /* Receive hashing offload */
+#define NETIF_F_RXCSUM		(1 << 29) /* Receive checksumming offload */
 
 	/* Segmentation offload features */
 #define NETIF_F_GSO_SHIFT	16
@@ -2379,6 +2380,8 @@  static inline int dev_ethtool_get_settings(struct net_device *dev,
 
 static inline u32 dev_ethtool_get_rx_csum(struct net_device *dev)
 {
+	if (dev->hw_features & NETIF_F_RXCSUM)
+		return !!(dev->features & NETIF_F_RXCSUM);
 	if (!dev->ethtool_ops || !dev->ethtool_ops->get_rx_csum)
 		return 0;
 	return dev->ethtool_ops->get_rx_csum(dev);
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index b068738..15e00b8 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -34,12 +34,6 @@  u32 ethtool_op_get_link(struct net_device *dev)
 }
 EXPORT_SYMBOL(ethtool_op_get_link);
 
-u32 ethtool_op_get_rx_csum(struct net_device *dev)
-{
-	return (dev->features & NETIF_F_ALL_CSUM) != 0;
-}
-EXPORT_SYMBOL(ethtool_op_get_rx_csum);
-
 u32 ethtool_op_get_tx_csum(struct net_device *dev)
 {
 	return (dev->features & NETIF_F_ALL_CSUM) != 0;
@@ -227,6 +221,9 @@  static unsigned long ethtool_get_feature_mask(u32 eth_cmd)
 	case ETHTOOL_GTXCSUM:
 	case ETHTOOL_STXCSUM:
 		return NETIF_F_ALL_CSUM|NETIF_F_SCTP_CSUM;
+	case ETHTOOL_GRXCSUM:
+	case ETHTOOL_SRXCSUM:
+		return NETIF_F_RXCSUM;
 	case ETHTOOL_GSG:
 	case ETHTOOL_SSG:
 		return NETIF_F_SG;
@@ -261,6 +258,7 @@  static int ethtool_get_one_feature(struct net_device *dev, char __user *useraddr
 }
 
 static int __ethtool_set_tx_csum(struct net_device *dev, u32 data);
+static int __ethtool_set_rx_csum(struct net_device *dev, u32 data);
 static int __ethtool_set_sg(struct net_device *dev, u32 data);
 static int __ethtool_set_tso(struct net_device *dev, u32 data);
 static int __ethtool_set_ufo(struct net_device *dev, u32 data);
@@ -289,6 +287,8 @@  static int ethtool_set_one_feature(struct net_device *dev,
 	switch (ethcmd) {
 	case ETHTOOL_STXCSUM:
 		return __ethtool_set_tx_csum(dev, edata.data);
+	case ETHTOOL_SRXCSUM:
+		return __ethtool_set_rx_csum(dev, edata.data);
 	case ETHTOOL_SSG:
 		return __ethtool_set_sg(dev, edata.data);
 	case ETHTOOL_STSO:
@@ -1253,20 +1253,15 @@  static int __ethtool_set_tx_csum(struct net_device *dev, u32 data)
 	return dev->ethtool_ops->set_tx_csum(dev, data);
 }
 
-static int ethtool_set_rx_csum(struct net_device *dev, char __user *useraddr)
+static int __ethtool_set_rx_csum(struct net_device *dev, u32 data)
 {
-	struct ethtool_value edata;
-
 	if (!dev->ethtool_ops->set_rx_csum)
 		return -EOPNOTSUPP;
 
-	if (copy_from_user(&edata, useraddr, sizeof(edata)))
-		return -EFAULT;
-
-	if (!edata.data && dev->ethtool_ops->set_sg)
+	if (!data)
 		dev->features &= ~NETIF_F_GRO;
 
-	return dev->ethtool_ops->set_rx_csum(dev, edata.data);
+	return dev->ethtool_ops->set_rx_csum(dev, data);
 }
 
 static int __ethtool_set_tso(struct net_device *dev, u32 data)
@@ -1618,15 +1613,6 @@  int dev_ethtool(struct net *net, struct ifreq *ifr)
 	case ETHTOOL_SPAUSEPARAM:
 		rc = ethtool_set_pauseparam(dev, useraddr);
 		break;
-	case ETHTOOL_GRXCSUM:
-		rc = ethtool_get_value(dev, useraddr, ethcmd,
-				       (dev->ethtool_ops->get_rx_csum ?
-					dev->ethtool_ops->get_rx_csum :
-					ethtool_op_get_rx_csum));
-		break;
-	case ETHTOOL_SRXCSUM:
-		rc = ethtool_set_rx_csum(dev, useraddr);
-		break;
 	case ETHTOOL_TEST:
 		rc = ethtool_self_test(dev, useraddr);
 		break;
@@ -1700,6 +1686,7 @@  int dev_ethtool(struct net *net, struct ifreq *ifr)
 		rc = ethtool_set_features(dev, useraddr);
 		break;
 	case ETHTOOL_GTXCSUM:
+	case ETHTOOL_GRXCSUM:
 	case ETHTOOL_GSG:
 	case ETHTOOL_GTSO:
 	case ETHTOOL_GUFO:
@@ -1708,6 +1695,7 @@  int dev_ethtool(struct net *net, struct ifreq *ifr)
 		rc = ethtool_get_one_feature(dev, useraddr, ethcmd);
 		break;
 	case ETHTOOL_STXCSUM:
+	case ETHTOOL_SRXCSUM:
 	case ETHTOOL_SSG:
 	case ETHTOOL_STSO:
 	case ETHTOOL_SUFO: