Re: [PATCH net-next v2 06/12] udp: Support gro_ipv4_max_size > 65536

[Willem de Bruijn <willemdebruijn.kernel@gmail.com>]

Willem de Bruijn wrote:
> Alice Mikityanska wrote:
> > From: Alice Mikityanska <alice@isovalent.com>
> > 
> > Currently, gro_max_size and gro_ipv4_max_size can be set to values
> > bigger than 65536, and GRO will happily aggregate UDP to the configured
> > size (for example, with TCP traffic in VXLAN tunnels). However,
> > udp_gro_complete uses the 16-bit length field in the UDP header to store
> > the length of the aggregated packet. It leads to the packet truncation
> > later in __udp4_lib_rcv.
> > 
> > Fix this by storing 0 to the UDP length field and by restoring the real
> > length from skb->len in __udp4_lib_rcv.
> > 
> > Signed-off-by: Alice Mikityanska <alice@isovalent.com>
> > ---
> >  net/ipv4/udp.c         | 5 ++++-
> >  net/ipv4/udp_offload.c | 4 ++--
> >  net/ipv6/udp_offload.c | 2 +-
> >  3 files changed, 7 insertions(+), 4 deletions(-)
> > 
> > diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> > index 345ef93001fc..870b35107ede 100644
> > --- a/net/ipv4/udp.c
> > +++ b/net/ipv4/udp.c
> > @@ -2690,7 +2690,7 @@ int __udp4_lib_rcv(struct sk_buff *skb, struct udp_table *udptable,
> >  {
> >  	struct sock *sk = NULL;
> >  	struct udphdr *uh;
> > -	unsigned short ulen;
> > +	unsigned int ulen;
> >  	struct rtable *rt = skb_rtable(skb);
> >  	__be32 saddr, daddr;
> >  	struct net *net = dev_net(skb->dev);
> > @@ -2714,6 +2714,9 @@ int __udp4_lib_rcv(struct sk_buff *skb, struct udp_table *udptable,
> >  		goto short_packet;
> >  
> >  	if (proto == IPPROTO_UDP) {
> > +		if (!ulen)
> > +			ulen = skb->len;
> > +
> 
> For normal packets, ip_rcv_core truncates skbs to their IP length.
> 
> I don't immediate see the GRO layer taking care of this. Which is fine
> if it can be done later, but not if these protocol fields are zeroed.
> 
> Should we validate that packets have no padding before we coalesce?

I should have read ahead. This is addressed in patch 8. Great! Never mind this comment.