diff mbox

[7/8] ethtool: Add GGRO and SGRO ops

Message ID 20081214210916.GA26325@gondor.apana.org.au
State Not Applicable, archived
Delegated to: David Miller
Headers show

Commit Message

Herbert Xu Dec. 14, 2008, 9:09 p.m. UTC 
On Sun, Dec 14, 2008 at 11:36:14AM -0800, Waskiewicz Jr, Peter P wrote:
>
> I'm confused then.  You're adding two ethtool entry points with 
> ETHTOOL_GGRO and ETHTOOL_SGRO, adding the callpoints in dev_ethtool with 
> set_gro and get_gro, but how do you manipulate this without adding to the 
> userspace application?  Adding this functionality to the set/get_flags 
> will keep the userspace app from needing a patch to support the new 
> callbacks.

Huh? Whether you use get_flags or not you still need to patch the
user-space application so that it knows how to handle

	ethtool -K eth0 gro on/off

There is no way around that.

As I said earlier, I didn't use get_flags/set_flags because of the
need to depend on RX checksum offload.

BTW, here is the patch for ethtool to set the GRO flags.


Cheers,

Comments

Waskiewicz Jr, Peter P Dec. 14, 2008, 10 p.m. UTC | #1 
On Sun, 14 Dec 2008, Herbert Xu wrote:

> On Sun, Dec 14, 2008 at 11:36:14AM -0800, Waskiewicz Jr, Peter P wrote:
> >
> > I'm confused then.  You're adding two ethtool entry points with
> > ETHTOOL_GGRO and ETHTOOL_SGRO, adding the callpoints in dev_ethtool with
> > set_gro and get_gro, but how do you manipulate this without adding to the
> > userspace application?  Adding this functionality to the set/get_flags
> > will keep the userspace app from needing a patch to support the new
> > callbacks.
> 
> Huh? Whether you use get_flags or not you still need to patch the
> user-space application so that it knows how to handle
> 
>         ethtool -K eth0 gro on/off
> 
> There is no way around that.
> 
> As I said earlier, I didn't use get_flags/set_flags because of the
> need to depend on RX checksum offload.

Ok, that makes sense.  

> 
> BTW, here is the patch for ethtool to set the GRO flags.

Thanks Herbert.  I was slightly confused when you said you wouldn't need 
to affect the ethtool_ops struct, which you don't.  But my initial read 
didn't think to just pop it into the existing offload ethtool_ops entry 
point after seeing the get/set ops you had in the kernel interface.  I've 
been enlightened now (or my brain has finally slipped into gear).  :-)

> -static int dump_offload(int rx, int tx, int sg, int tso, int ufo, int gso, int lro)
> +static int dump_offload(int rx, int tx, int sg, int tso, int ufo, int gso,
> +                       int gro, int lro)

Would it make better sense to add GRO after LRO?  I suppose it doesn't 
make any difference, but I'm thinking of any possible way to provide ABI 
compatibility.  I don't think it's possible though; just thinking out loud 
here.

Cheers,
-PJ Waskiewicz
--
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
Herbert Xu Dec. 15, 2008, 3:40 a.m. UTC | #2 
On Sun, Dec 14, 2008 at 02:00:18PM -0800, Waskiewicz Jr, Peter P wrote:
>
> > -static int dump_offload(int rx, int tx, int sg, int tso, int ufo, int gso, int lro)
> > +static int dump_offload(int rx, int tx, int sg, int tso, int ufo, int gso,
> > +                       int gro, int lro)
> 
> Would it make better sense to add GRO after LRO?  I suppose it doesn't 
> make any difference, but I'm thinking of any possible way to provide ABI 
> compatibility.  I don't think it's possible though; just thinking out loud 
> here.

This is an internal iproute function, not part of any ABI :)

Longer term my plan is to first have LRO restricted to hardware
offload which cannot coexist with forwarding/bridging, and eventually
phased out completely.

Cheers,
diff mbox

Patch

diff --git a/ethtool-copy.h b/ethtool-copy.h
index eadba25..3ca4e2c 100644
--- a/ethtool-copy.h
+++ b/ethtool-copy.h
@@ -336,6 +336,8 @@  struct ethtool_rxnfc {
 
 #define	ETHTOOL_GRXFH		0x00000029 /* Get RX flow hash configuration */
 #define	ETHTOOL_SRXFH		0x0000002a /* Set RX flow hash configuration */
+#define	ETHTOOL_GGRO		0x0000002b /* Get GRO enable (ethtool_value) */
+#define	ETHTOOL_SGRO		0x0000002c /* Set GRO enable (ethtool_value) */
 
 /* compatibility with older code */
 #define SPARC_ETH_GSET		ETHTOOL_GSET
diff --git a/ethtool.c b/ethtool.c
index a7c02d0..502fc8f 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -160,6 +160,7 @@  static struct option {
 	        "		[ tso on|off ]\n"
 	        "		[ ufo on|off ]\n"
 		"		[ gso on|off ]\n"
+		"		[ gro on|off ]\n"
 		"               [ lro on|off ]\n"
     },
     { "-i", "--driver", MODE_GDRV, "Show driver information" },
@@ -218,6 +219,7 @@  static int off_sg_wanted = -1;
 static int off_tso_wanted = -1;
 static int off_ufo_wanted = -1;
 static int off_gso_wanted = -1;
+static int off_gro_wanted = -1;
 static int off_lro_wanted = -1;
 
 static struct ethtool_pauseparam epause;
@@ -333,6 +335,7 @@  static struct cmdline_info cmdline_offload[] = {
 	{ "tso", CMDL_BOOL, &off_tso_wanted, NULL },
 	{ "ufo", CMDL_BOOL, &off_ufo_wanted, NULL },
 	{ "gso", CMDL_BOOL, &off_gso_wanted, NULL },
+	{ "gro", CMDL_BOOL, &off_gro_wanted, NULL },
 	{ "lro", CMDL_BOOL, &off_lro_wanted, NULL },
 };
 
@@ -1387,7 +1390,8 @@  static int dump_coalesce(void)
 	return 0;
 }
 
-static int dump_offload(int rx, int tx, int sg, int tso, int ufo, int gso, int lro)
+static int dump_offload(int rx, int tx, int sg, int tso, int ufo, int gso,
+			int gro, int lro)
 {
 	fprintf(stdout,
 		"rx-checksumming: %s\n"
@@ -1396,6 +1400,7 @@  static int dump_offload(int rx, int tx, int sg, int tso, int ufo, int gso, int l
 		"tcp segmentation offload: %s\n"
 		"udp fragmentation offload: %s\n"
 		"generic segmentation offload: %s\n"
+		"generic receive offload: %s\n"
 		"large receive offload: %s\n",
 		rx ? "on" : "off",
 		tx ? "on" : "off",
@@ -1403,6 +1408,7 @@  static int dump_offload(int rx, int tx, int sg, int tso, int ufo, int gso, int l
 		tso ? "on" : "off",
 		ufo ? "on" : "off",
 		gso ? "on" : "off",
+		gro ? "on" : "off",
 		lro ? "on" : "off");
 
 	return 0;
@@ -1714,7 +1720,7 @@  static int do_goffload(int fd, struct ifreq *ifr)
 {
 	struct ethtool_value eval;
 	int err, allfail = 1, rx = 0, tx = 0, sg = 0;
-	int tso = 0, ufo = 0, gso = 0, lro = 0;
+	int tso = 0, ufo = 0, gso = 0, gro = 0, lro = 0;
 
 	fprintf(stdout, "Offload parameters for %s:\n", devname);
 
@@ -1778,6 +1784,16 @@  static int do_goffload(int fd, struct ifreq *ifr)
 		allfail = 0;
 	}
 
+	eval.cmd = ETHTOOL_GGRO;
+	ifr->ifr_data = (caddr_t)&eval;
+	err = ioctl(fd, SIOCETHTOOL, ifr);
+	if (err)
+		perror("Cannot get device generic receive offload settings");
+	else {
+		gro = eval.data;
+		allfail = 0;
+	}
+
 	eval.cmd = ETHTOOL_GFLAGS;
 	ifr->ifr_data = (caddr_t)&eval;
 	err = ioctl(fd, SIOCETHTOOL, ifr);
@@ -1793,7 +1809,7 @@  static int do_goffload(int fd, struct ifreq *ifr)
 		return 83;
 	}
 
-	return dump_offload(rx, tx, sg, tso, ufo, gso, lro);
+	return dump_offload(rx, tx, sg, tso, ufo, gso, gro, lro);
 }
 
 static int do_soffload(int fd, struct ifreq *ifr)
@@ -1870,6 +1886,17 @@  static int do_soffload(int fd, struct ifreq *ifr)
 			return 90;
 		}
 	}
+	if (off_gro_wanted >= 0) {
+		changed = 1;
+		eval.cmd = ETHTOOL_SGRO;
+		eval.data = (off_gro_wanted == 1);
+		ifr->ifr_data = (caddr_t)&eval;
+		err = ioctl(fd, SIOCETHTOOL, ifr);
+		if (err) {
+			perror("Cannot set device generic receive offload settings");
+			return 93;
+		}
+	}
 	if (off_lro_wanted >= 0) {
 		changed = 1;
 		eval.cmd = ETHTOOL_GFLAGS;