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

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

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

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?