Patchwork [net-next,2/4] udp: Handle large UFO packets from untrusted sources

login
register
mail settings
Submitter Sridhar Samudrala
Date June 6, 2009, 12:16 a.m.
Message ID <1244247391.1526.171.camel@w-sridhar.beaverton.ibm.com>
Download mbox | patch
Permalink /patch/28170/
State Changes Requested
Delegated to: David Miller
Headers show

Comments

Sridhar Samudrala - June 6, 2009, 12:16 a.m.
Setup partial checksum and add gso checks to handle large UFO packets from
untrusted sources.

Signed-off-by: Sridhar Samudrala <sri@us.ibm.com>

---


 include/net/udp.h  |    3 +++
 net/ipv4/af_inet.c |    2 ++
 net/ipv4/udp.c     |   60 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 net/ipv6/udp.c     |   22 +++++++++++++++++++
 4 files changed, 87 insertions(+), 0 deletions(-)



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Herbert Xu - June 8, 2009, 5:16 a.m.
On Fri, Jun 05, 2009 at 05:16:31PM -0700, Sridhar Samudrala wrote:
>
> +	/* Software UFO is not yet supported */
> +	segs = ERR_PTR(-EPROTONOSUPPORT);

Hmm, we need to fill this in before you start using it for virt.
After all, it's very difficult for the guest to know whether the
output device on the host is going to be able to do UFO or not.

The whole reason I did GSO in the first place is to allow the guest
not to worry about whether the host supported TSO in hardware.

Cheers,
Sridhar Samudrala - June 8, 2009, 5:04 p.m.
On Mon, 2009-06-08 at 15:16 +1000, Herbert Xu wrote:
> On Fri, Jun 05, 2009 at 05:16:31PM -0700, Sridhar Samudrala wrote:
> >
> > +	/* Software UFO is not yet supported */
> > +	segs = ERR_PTR(-EPROTONOSUPPORT);
> 
> Hmm, we need to fill this in before you start using it for virt.
> After all, it's very difficult for the guest to know whether the
> output device on the host is going to be able to do UFO or not.

OK. Can we use skb_segment() to do IP fragmentation of UDP packets?
Or does it only support TCP segementation?
The function itself looks protocol independent and if it is simply
splitting large skb into a list of mtu sized skb's, can we fixup the
ip header differently for UDP packets(id and frag_off) in inet_gso_segment
and ipv6_gso_segment?


Thanks
Sridhar

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Herbert Xu - June 8, 2009, 11:37 p.m.
On Mon, Jun 08, 2009 at 10:04:47AM -0700, Sridhar Samudrala wrote:
>
> OK. Can we use skb_segment() to do IP fragmentation of UDP packets?

It should be able to.

> The function itself looks protocol independent and if it is simply
> splitting large skb into a list of mtu sized skb's, can we fixup the
> ip header differently for UDP packets(id and frag_off) in inet_gso_segment
> and ipv6_gso_segment?

Yes that's the idea.

Cheers,
Rusty Russell - June 10, 2009, 3:40 a.m.
On Mon, 8 Jun 2009 02:46:08 pm Herbert Xu wrote:
> On Fri, Jun 05, 2009 at 05:16:31PM -0700, Sridhar Samudrala wrote:
> > +	/* Software UFO is not yet supported */
> > +	segs = ERR_PTR(-EPROTONOSUPPORT);
>
> Hmm, we need to fill this in before you start using it for virt.
> After all, it's very difficult for the guest to know whether the
> output device on the host is going to be able to do UFO or not.

Yep, no point telling the guest we support UFO if we don't!

Cheers,
Rusty.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sridhar Samudrala - June 10, 2009, 5:51 a.m.
Herbert Xu wrote:
> On Mon, Jun 08, 2009 at 10:04:47AM -0700, Sridhar Samudrala wrote:
>   
>> OK. Can we use skb_segment() to do IP fragmentation of UDP packets?
>>     
>
> It should be able to.
>   
Unfortunately, this doesn't work for UDP without any changes. 
skb_segment() currently adds transport header to
each segmented skb. But when we are using IP fragmentation, only the 
first fragment should include the UDP
header.
We either need to fix skb_segment() to handle IP fragmentation or write 
a new skb_fragment(). I will look into
this when i get some time.

Thanks
Sridhar

>   
>> The function itself looks protocol independent and if it is simply
>> splitting large skb into a list of mtu sized skb's, can we fixup the
>> ip header differently for UDP packets(id and frag_off) in inet_gso_segment
>> and ipv6_gso_segment?
>>     
>
> Yes that's the idea.
>
> Cheers,
>   


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sridhar Samudrala - June 10, 2009, 5:52 a.m.
Rusty Russell wrote:
> On Mon, 8 Jun 2009 02:46:08 pm Herbert Xu wrote:
>   
>> On Fri, Jun 05, 2009 at 05:16:31PM -0700, Sridhar Samudrala wrote:
>>     
>>> +	/* Software UFO is not yet supported */
>>> +	segs = ERR_PTR(-EPROTONOSUPPORT);
>>>       
>> Hmm, we need to fill this in before you start using it for virt.
>> After all, it's very difficult for the guest to know whether the
>> output device on the host is going to be able to do UFO or not.
>>     
>
> Yep, no point telling the guest we support UFO if we don't!
>   
This path is taken only when the packet needs to go out a device that 
doesn't support UFO.
Until the software UFO is supported, I thought it may be useful to 
provide a way to enable
UFO inter-guest, between host to guest and to an external host via an 
outgoing device that supports UFO.

Thanks
Sridhar

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Herbert Xu - June 10, 2009, 9:17 a.m.
On Tue, Jun 09, 2009 at 10:51:23PM -0700, Sridhar Samudrala wrote:
>
> Unfortunately, this doesn't work for UDP without any changes.  
> skb_segment() currently adds transport header to
> each segmented skb. But when we are using IP fragmentation, only the  
> first fragment should include the UDP
> header.
> We either need to fix skb_segment() to handle IP fragmentation or write  
> a new skb_fragment(). I will look into
> this when i get some time.

Couldn't you get around this by setting skb->transport_header to
skb->network_header before getting into skb_segment?

Cheers,

Patch

diff --git a/include/net/udp.h b/include/net/udp.h
index 90e6ce5..274b764 100644
--- a/include/net/udp.h
+++ b/include/net/udp.h
@@ -207,4 +207,7 @@  extern void udp4_proc_exit(void);
 #endif
 
 extern void udp_init(void);
+
+extern int udp_v4_gso_send_check(struct sk_buff *skb);
+extern struct sk_buff *udp_ufo_fragment(struct sk_buff *skb, int features);
 #endif	/* _UDP_H */
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index d873621..1c1ac7d 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1426,6 +1426,8 @@  static struct net_protocol tcp_protocol = {
 static struct net_protocol udp_protocol = {
 	.handler =	udp_rcv,
 	.err_handler =	udp_err,
+	.gso_send_check = udp_v4_gso_send_check,
+	.gso_segment = udp_ufo_fragment,
 	.no_policy =	1,
 	.netns_ok =	1,
 };
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 8f4158d..678bdf9 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1815,6 +1815,66 @@  void __init udp_init(void)
 	sysctl_udp_wmem_min = SK_MEM_QUANTUM;
 }
 
+int udp_v4_gso_send_check(struct sk_buff *skb)
+{
+	const struct iphdr *iph;
+	struct udphdr *uh;
+
+	if (!pskb_may_pull(skb, sizeof(*uh)))
+		return -EINVAL;
+
+	iph = ip_hdr(skb);
+	uh = udp_hdr(skb);
+
+	uh->check = 0;
+	uh->check = ~csum_tcpudp_magic(iph->saddr, iph->daddr, skb->len, IPPROTO_UDP, 0);
+	skb->csum_start = skb_transport_header(skb) - skb->head;
+	skb->csum_offset = offsetof(struct udphdr, check);
+	skb->ip_summed = CHECKSUM_PARTIAL;
+	return 0;
+}
+
+struct sk_buff *udp_ufo_fragment(struct sk_buff *skb, int features)
+{
+	struct sk_buff *segs = ERR_PTR(-EINVAL);
+	struct udphdr *uh;
+	unsigned uhlen;
+	unsigned int mss;
+
+	if (!pskb_may_pull(skb, sizeof(*uh)))
+		goto out;
+
+	uh = udp_hdr(skb);
+	uhlen = sizeof(*uh);
+
+	__skb_pull(skb, uhlen);
+
+	mss = skb_shinfo(skb)->gso_size;
+	if (unlikely(skb->len <= mss))
+		goto out;
+
+	if (skb_gso_ok(skb, features | NETIF_F_GSO_ROBUST)) {
+		int type = skb_shinfo(skb)->gso_type;
+
+		if (unlikely(type & ~(SKB_GSO_UDP | SKB_GSO_DODGY) ||
+			     !(type & (SKB_GSO_UDP))))
+			goto out;
+
+		skb_shinfo(skb)->gso_segs = DIV_ROUND_UP(skb->len, mss);
+
+		segs = NULL;
+		goto out;
+	}
+
+	/* Software UFO is not yet supported */
+	segs = ERR_PTR(-EPROTONOSUPPORT);
+
+out:
+	return segs;
+}
+EXPORT_SYMBOL(udp_ufo_fragment);
+
+
 EXPORT_SYMBOL(udp_disconnect);
 EXPORT_SYMBOL(udp_ioctl);
 EXPORT_SYMBOL(udp_prot);
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index fad0f5f..ccd42d5 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -1081,9 +1081,31 @@  int compat_udpv6_getsockopt(struct sock *sk, int level, int optname,
 }
 #endif
 
+
+static int udpv6_gso_send_check(struct sk_buff *skb)
+{
+	struct ipv6hdr *ipv6h;
+	struct udphdr *uh;
+
+	if (!pskb_may_pull(skb, sizeof(*uh)))
+		return -EINVAL;
+
+	ipv6h = ipv6_hdr(skb);
+	uh = udp_hdr(skb);
+
+	uh->check = 0;
+	uh->check = ~csum_ipv6_magic(&ipv6h->saddr, &ipv6h->daddr, skb->len, IPPROTO_UDP, 0);
+	skb->csum_start = skb_transport_header(skb) - skb->head;
+	skb->csum_offset = offsetof(struct udphdr, check);
+	skb->ip_summed = CHECKSUM_PARTIAL;
+	return 0;
+}
+
 static struct inet6_protocol udpv6_protocol = {
 	.handler	=	udpv6_rcv,
 	.err_handler	=	udpv6_err,
+	.gso_send_check =	udpv6_gso_send_check,
+	.gso_segment	=	udp_ufo_fragment,
 	.flags		=	INET6_PROTO_NOPOLICY|INET6_PROTO_FINAL,
 };