[PATCH 1/2] net-test: Adds --disable_gso option to packetdrill

[Matthieu Baerts]

With this option, offload flags will not be added to the 'tun' interface
and then GSO support will not be enabled to not be like a typical
ethernet device.
It's useful when testing TCP implementation from a kernel point of view.

Here at the university, we see TCP implementation from a kernel point
of view: when sending 4 packets, we expect receiving 4 ACKs. It's easy
to understand the advantages of GSO but it's not explained in the
course's notes (because it's done by the hardware, can be optional and
it adds additional complexities).
This is why it can be interesting to add this new option when using
packetdrill to check that students correctly understand TCP stack
without thinking to hardware accelerations and other things added to
optimise the connexion.

Thanks to Fabien Duchêne for his help and his explanations about GSO.

Tested on GNU/Linux (Ubuntu Trusty 14.04), this modification only
affects GNU/Linux users.

Signed-off-by: Matthieu Baerts <mat...@gmail.com>
---
gtests/net/packetdrill/config.c | 6 ++++++
gtests/net/packetdrill/config.h | 2 ++
gtests/net/packetdrill/netdev.c | 3 ++-
3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/gtests/net/packetdrill/config.c b/gtests/net/packetdrill/config.c
index a286b26..b33c80a 100644
--- a/gtests/net/packetdrill/config.c
+++ b/gtests/net/packetdrill/config.c
@@ -56,6 +56,7 @@ enum option_codes {
OPT_WIRE_CLIENT_DEV,
OPT_WIRE_SERVER_DEV,
OPT_TCP_TS_TICK_USECS,
+ OPT_DISABLE_GSO,
OPT_NON_FATAL,
OPT_DRY_RUN,
OPT_VERBOSE = 'v', /* our only single-letter option */
@@ -84,6 +85,7 @@ struct option options[] = {
{ "wire_client_dev", .has_arg = true, NULL, OPT_WIRE_CLIENT_DEV },
{ "wire_server_dev", .has_arg = true, NULL, OPT_WIRE_SERVER_DEV },
{ "tcp_ts_tick_usecs", .has_arg = true, NULL, OPT_TCP_TS_TICK_USECS },
+ { "disable_gso", .has_arg = false, NULL, OPT_DISABLE_GSO },
{ "non_fatal", .has_arg = true, NULL, OPT_NON_FATAL },
{ "dry_run", .has_arg = false, NULL, OPT_DRY_RUN },
{ "verbose", .has_arg = false, NULL, OPT_VERBOSE },
@@ -108,6 +110,7 @@ void show_usage(void)
"\t[--mtu=<MTU in bytes>\n"
"\t[--tolerance_usecs=tolerance_usecs]\n"
"\t[--tcp_ts_tick_usecs=<microseconds per TCP TS val tick>]\n"
+ "\t[--disable_gso]\n"
"\t[--non_fatal=<comma separated types: packet,syscall>]\n"
"\t[--wire_client]\n"
"\t[--wire_server]\n"
@@ -423,6 +426,9 @@ static void process_option(int opt, char *optarg, struct config *config,
config->tcp_ts_tick_usecs > 1000000)
die("%s: bad --tcp_ts_tick_usecs: %s\n", where, optarg);
break;
+ case OPT_DISABLE_GSO:
+ config->disable_gso = true;
+ break;
case OPT_WIRE_CLIENT:
config->is_wire_client = true;
break;
diff --git a/gtests/net/packetdrill/config.h b/gtests/net/packetdrill/config.h
index 55190ce..bb2407d 100644
--- a/gtests/net/packetdrill/config.h
+++ b/gtests/net/packetdrill/config.h
@@ -78,6 +78,8 @@ struct config {
bool non_fatal_packet; /* treat packet asserts as non-fatal */
bool non_fatal_syscall; /* treat syscall asserts as non-fatal */

+ bool disable_gso; /* to not be like eth devices: w/ GSO */
+
bool dry_run; /* parse script but don't execute? */

bool verbose; /* print detailed debug info? */
diff --git a/gtests/net/packetdrill/netdev.c b/gtests/net/packetdrill/netdev.c
index 8cee434..bb278de 100644
--- a/gtests/net/packetdrill/netdev.c
+++ b/gtests/net/packetdrill/netdev.c
@@ -276,7 +276,8 @@ struct netdev *local_netdev_new(struct config *config)

check_remote_address(config, netdev);
create_device(config, netdev);
- set_device_offload_flags(netdev);
+ if (!config->disable_gso)
+ set_device_offload_flags(netdev);
bring_up_device(netdev);

net_setup_dev_address(netdev->name,
--
1.8.5.2

[Matthieu Baerts]

Fixed typos in the 'usage' message: a few ']' were missing

Signed-off-by: Matthieu Baerts <mat...@gmail.com>
---

gtests/net/packetdrill/config.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/gtests/net/packetdrill/config.c b/gtests/net/packetdrill/config.c
index b33c80a..5ea55a5 100644
--- a/gtests/net/packetdrill/config.c
+++ b/gtests/net/packetdrill/config.c
@@ -105,9 +105,9 @@ void show_usage(void)
"\t[--local_ip=local_ip]\n"
"\t[--gateway_ip=gateway_ip]\n"
"\t[--netmask_ip=netmask_ip]\n"
- "\t[--init_scripts=<comma separated filenames>\n"
- "\t[--speed=<speed in Mbps>\n"
- "\t[--mtu=<MTU in bytes>\n"
+ "\t[--init_scripts=<comma separated filenames>]\n"
+ "\t[--speed=<speed in Mbps>]\n"
+ "\t[--mtu=<MTU in bytes>]\n"

"\t[--tolerance_usecs=tolerance_usecs]\n"
"\t[--tcp_ts_tick_usecs=<microseconds per TCP TS val tick>]\n"

"\t[--disable_gso]\n"
--
1.8.5.2

[Matthieu Baerts]

Hello,

Is it only me or we only see here my second patch and not the first one (the most important one :-) )?

Anyway... if I have to resend this email, feel free to ask me.

BTW thank you for having released this tool under GPL2 license!

This second patch just fixes typos, nothing very important. Maybe should I split them?

Best regards,

Matt

[Matthieu Baerts]

Ok now I also see the first patch, sorry :-)

[Neal Cardwell]

Hi Matthieu,

I've applied this patch to fix the usage message syntax.

Thanks!

neal

> --
> You received this message because you are subscribed to the Google Groups "packetdrill" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to packetdrill...@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.

[Neal Cardwell]

Hi Matthieu,

Thanks for sending this patch. I agree that there's huge value in
being able to somehow get rid of the offload features that are
currently configured on the tun device in the Linux version of
packetdrill.

I am tempted by an alternate approach:

(1) Completely remove the set_device_offload_flags() function, which
currently only exists on Linux.

(2) Set an environment variable ($PKT_DEV or somesuch) to allow the
untimed/optional `foo` init shell command at the top of the script to
configure whatever offloads it wants on the device under test, whether
that device is a tun, Ethernet, wifi, etc. For example, on Linux, you
could do something like the following at the top of a script:

`ethtool -K $PKT_DEV gso on tso on`

This approach has some nice features:

+ makes default behavior more similar across different platforms (e.g.
no GSO/TSO on tun on Linux and *BSD)

+ makes default behavior more similar between remote and local modes
(tests see 1-MSS units, not TSO/GSO jumbograms)

+ allows arbitrary config of the device under test (change offload
features, or change queuing discipline)

+ allows scripts to make some kinds of device config changes
independent of the physical name of the device, so a single script
could be used across multiple devices (like tun0 or eth0, or the
various names of an Ethernet NIC under various flavors of *BSD)

Linux users who want to still see TSO/GSO jumbograms can use something
like the ethtool snippet above to enable their favorite offload
features again.

What do folks think?

neal


On Wed, Dec 18, 2013 at 12:35 PM, Matthieu Baerts <mat...@gmail.com> wrote:

[fabien....@uclouvain.be]

Hi guys,

Definitely, the second approach, even though I don't really like the fact that it's adding a dep. to some third party tools (tc, ethtool, ..) if you want to activate TSO/GSO.

But it's clearly more flexible, and so I stand for the second approach.