fix(quinn-udp): only set GSO segment size if != content length

[thomaseizinger]

No description provided.

[mxinden]

I am not sure I understand the purpose of this new conditional.

If I understand your comment #2042 (comment) correctly, GSO always fails on Android. If that is indeed always the case, why not disable it all together on Android?

[thomaseizinger]

If that is indeed always the case, why not disable it all together on Android?

It works fine when I am on WiFi and the unit tests also run on Android. It only fails on mobile data! That is the weird bit.

The purpose of the conditional is to actually make the "disable GSO on failure"-mechanism work.

[Ralith]

It works fine when I am on WiFi and the unit tests also run on Android. It only fails on mobile data! That is the weird bit.

We've seen this on other Linux as well, which is why the dynamic max_gso_segments fall-back path exists to begin with. I assume it's due to network driver support for GSO varying. I guess, furthermore, some drivers happily ignore a single segment and others don't want to see that cmsg at all...

[Ralith]

It looks like we already have similar logic in quinn-proto, which is probably why we've never noticed this problem before:

quinn/quinn-proto/src/connection/mod.rs

Lines 1039 to 1042 in 3a9d176

	segment_size: match num_datagrams {
	1 => None,
	_ => Some(segment_size),
	},

I don't particularly mind being a little more robust here, though.

[thomaseizinger]

I assume it's due to network driver support for GSO varying

This is interesting. I don't know anything about how these network drivers are organised in the Linux kernel but the docs page (https://www.man7.org/linux/man-pages/man7/udp.7.html) says:

As late as possible, the large packet is split by segment size into a series of datagrams. This segmentation offload step is deferred to hardware if supported, else performed in software.

I guess, this still doesn't rule out that the particular software support is missing.

[thomaseizinger]

It looks like we already have similar logic in quinn-proto, which is probably why we've never noticed this problem before:

Would you like me to remove this from quinn-proto as part of this patch-set then?

[Ralith]

Would you like me to remove this from quinn-proto as part of this patch-set then?

Nah, I don't think that's necessary. I see it as defense in depth, maybe. In theory quinn-proto can be used without quinn-udp, as well.