Re: [PATCH net-next V2 1/3] net: Add GRO support for UDP encapsulating protocols

On Wed, Jan 8, 2014 at 12:02 AM, Or Gerlitz [off-list ref] wrote:

On 08/01/2014 00:11, Jerry Chu wrote:

quoted

On Tue, Jan 7, 2014 at 12:37 PM, Or Gerlitz [off-list ref] wrote:

quoted

On Tue, Jan 7, 2014 at 10:32 PM, Eric Dumazet [off-list ref]
wrote:

quoted

On Tue, 2014-01-07 at 22:19 +0200, Or Gerlitz wrote:

quoted

On Tue, Jan 7, 2014 at 6:33 PM, Eric Dumazet [off-list ref]
wrote:

quoted

On Tue, 2014-01-07 at 17:29 +0200, Or Gerlitz wrote:

quoted

+
+#define MAX_UDP_PORT (1 << 16)
+extern const struct net_offload __rcu *udp_offloads[MAX_UDP_PORT];

Thats 512 KB of memory.
This will greatly impact forwarding performance of UDP packets with
random ports, and will increase kernel memory size for embedded
devices.

Re forwarding, are you referring to the case where the forwarded
packets are encapsulated? packets which are not encapusalted will be
flushed in the gro receive handler (this went out by mistake in V2 but
exists in V1)  if skb->encapsulation isn't set.

How do you know encapsulation must be tried for a given incoming
packet ? NIC do not magically sets skb->encapsulation I think...

So here's the thing, per my understanding we want to GRO only received
**encapsulated** packets whose checksum status is != CHECKSUM_NONE

What's wrong with GRO'ing pkts whose csum == CHECKSUM_NONE?

I am not sure, intuitively it sounds a bit wrong to me, empirically, it
doesn't work for udp encapsulated  / vxlan
traffic, I got drops from the tcp stack in tcp_rcv_established() -- if
GRO-ed packets carry CHECKSUM_NONE
we arrive to the csum_error label, which means that
tcp_checksum_complete_user() failed for them

This is odd because if pkts have been aggregated successfully,
tcp4_gro_receive() should've skb_checksum() and turned CHECKSUM_NONE
into CHECKSUM_UNNECESSARY. (I think i've already tested this
case with my GRE-GRO patch on a NIC that sends up pkts w/ CHECKSUM_NONE.

But granted there are a lot of csum related bugs in the current code. I just
spent half a day scratching my head on a very low thruput number with
my GRE patch over a GRE tunnel w/ csum flag on. I just tracked it down
to be buggy TSO/GRE code that will produce bad csum on the tx side.

quoted

Also "udp_offload" is a little misleading - you are not trying to GRO UDP
pkts where UDP is the real transport. You are only trying to GRO UDP
encapped TCP pkts.

Indeed -- however, I just plugged into what was there for GSO, e.g stack
will not do GSO for plain UDP
packets, only for those who encapsulate something the code that does this is
udp_offloads.c -- any suggestion
how to phrase/frame the change you envision?

There is already udpv4_offload for real udp gso (ufo) offload. How about
"udp_encap_offload" for your stuff?

quoted