| Message ID | 1244247391.1526.171.camel@w-sridhar.beaverton.ibm.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
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,
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
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,
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
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
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
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,
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, };
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