[PATCH] drivers: set_sg should not be set for !F_SG drivers

[PATCH] drivers: set_sg should not be set for !F_SG drivers

[Krishna Kumar]

From: Krishna Kumar <krkumar2@in.ibm.com>

set_sg should not be set to ethtool_op_set_sg() by drivers
that do not support SG. It is also not required for these
drivers to set get_sg since it is handled by the generic
ethtool_op_get_sg.

Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
---
 drivers/net/8139cp.c |    1 -
 drivers/net/r8169.c  |    1 -
 drivers/net/veth.c   |    2 --
 3 files changed, 4 deletions(-)

diff -ruNp org/drivers/net/8139cp.c new/drivers/net/8139cp.c
--- org/drivers/net/8139cp.c	2009-12-29 18:32:57.000000000 +0530
+++ new/drivers/net/8139cp.c	2009-12-29 18:34:29.000000000 +0530
@@ -1570,7 +1570,6 @@ static const struct ethtool_ops cp_ethto
 	.get_rx_csum		= cp_get_rx_csum,
 	.set_rx_csum		= cp_set_rx_csum,
 	.set_tx_csum		= ethtool_op_set_tx_csum, /* local! */
-	.set_sg			= ethtool_op_set_sg,
 	.set_tso		= ethtool_op_set_tso,
 	.get_regs		= cp_get_regs,
 	.get_wol		= cp_get_wol,
diff -ruNp org/drivers/net/r8169.c new/drivers/net/r8169.c
--- org/drivers/net/r8169.c	2009-12-29 18:32:57.000000000 +0530
+++ new/drivers/net/r8169.c	2009-12-29 18:34:32.000000000 +0530
@@ -1253,7 +1253,6 @@ static const struct ethtool_ops rtl8169_
 	.get_rx_csum		= rtl8169_get_rx_csum,
 	.set_rx_csum		= rtl8169_set_rx_csum,
 	.set_tx_csum		= ethtool_op_set_tx_csum,
-	.set_sg			= ethtool_op_set_sg,
 	.set_tso		= ethtool_op_set_tso,
 	.get_regs		= rtl8169_get_regs,
 	.get_wol		= rtl8169_get_wol,
diff -ruNp org/drivers/net/veth.c new/drivers/net/veth.c
--- org/drivers/net/veth.c	2009-12-29 18:32:57.000000000 +0530
+++ new/drivers/net/veth.c	2009-12-29 18:34:38.000000000 +0530
@@ -137,8 +137,6 @@ static const struct ethtool_ops veth_eth
 	.set_rx_csum		= veth_set_rx_csum,
 	.get_tx_csum		= veth_get_tx_csum,
 	.set_tx_csum		= veth_set_tx_csum,
-	.get_sg			= ethtool_op_get_sg,
-	.set_sg			= ethtool_op_set_sg,
 	.get_strings		= veth_get_strings,
 	.get_sset_count		= veth_get_sset_count,
 	.get_ethtool_stats	= veth_get_ethtool_stats,

Re: [PATCH] drivers: set_sg should not be set for !F_SG drivers

[David Miller]

From: Krishna Kumar <krkumar2@in.ibm.com>
Date: Fri, 15 Jan 2010 11:02:29 +0530

> From: Krishna Kumar <krkumar2@in.ibm.com>
> 
> set_sg should not be set to ethtool_op_set_sg() by drivers
> that do not support SG. It is also not required for these
> drivers to set get_sg since it is handled by the generic
> ethtool_op_get_sg.
> 
> Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
 ...
> diff -ruNp org/drivers/net/r8169.c new/drivers/net/r8169.c
> --- org/drivers/net/r8169.c	2009-12-29 18:32:57.000000000 +0530
> +++ new/drivers/net/r8169.c	2009-12-29 18:34:32.000000000 +0530
> @@ -1253,7 +1253,6 @@ static const struct ethtool_ops rtl8169_
>  	.get_rx_csum		= rtl8169_get_rx_csum,
>  	.set_rx_csum		= rtl8169_set_rx_csum,
>  	.set_tx_csum		= ethtool_op_set_tx_csum,
> -	.set_sg			= ethtool_op_set_sg,
>  	.set_tso		= ethtool_op_set_tso,
>  	.get_regs		= rtl8169_get_regs,
>  	.get_wol		= rtl8169_get_wol,

I think you have some false positives here, actually.

As in my reply to your tcp_sendmsg() patch, r8169.c's transmit
function does fully support segmented packets.

So I think what it's doing here is supporting SG but having
it off by default.

And that's exactly what you get by setting the ethtool_op_set_sg()
function here, and it might be the case that this is what the
other two drivers in your patch are trying to do.

At a minimum, the r8169.c part of this patch is wrong.