[PATCH 3/3] tun/tap GSO/partial csum support
This implements partial checksum and GSO support for tun/tap.
We use the virtio_net_hdr: it is an ABI already and designed to
encapsulate such metadata as GSO and partial checksums.
lguest performance (160MB sendfile, worst/best/avg, 20 runs):
Before: 5.06/3.39/3.82
After: 4.69/0.84/2.84
Note that the way tun works, you have to use the TUNSETIFF ioctl to set
this if you want to detect older kernels which don't have support.
Questions:
1) Should we rename/move virtio_net_hdr to something more generic?
2) Is this the right way to build a paged skb from user pages?
3) Do we need more checking for invalid GSO fields?
Signed-off-by: Rusty Russell <[EMAIL PROTECTED]>
---
drivers/net/tun.c| 213 +--
include/linux/if_tun.h |2
3 files changed, 189 insertions(+), 28 deletions(-)
diff -r 1057851c060f drivers/net/tun.c
--- a/drivers/net/tun.c Tue Jan 15 16:59:58 2008 +1100
+++ b/drivers/net/tun.c Tue Jan 15 20:47:41 2008 +1100
@@ -62,6 +62,7 @@
#include
#include
#include
+#include
#include
#include
@@ -238,35 +239,195 @@ static unsigned int tun_chr_poll(struct
return mask;
}
+static struct sk_buff *copy_user_skb(size_t align, struct iovec *iv, size_t
len)
+{
+ struct sk_buff *skb;
+
+ if (!(skb = alloc_skb(len + align, GFP_KERNEL)))
+ return ERR_PTR(-ENOMEM);
+
+ if (align)
+ skb_reserve(skb, align);
+
+ if (memcpy_fromiovec(skb_put(skb, len), iv, len)) {
+ kfree_skb(skb);
+ return ERR_PTR(-EFAULT);
+ }
+ return skb;
+}
+
+/* This will fail if they give us a crazy iovec, but that's their own fault. */
+static int get_user_skb_frags(const struct iovec *iv, size_t count,
+ struct skb_frag_struct *f)
+{
+ unsigned int i, j, num_pg = 0;
+ int err;
+ struct page *pages[MAX_SKB_FRAGS];
+
+ down_read(¤t->mm->mmap_sem);
+ for (i = 0; i < count; i++) {
+ int n, npages;
+ unsigned long base, len;
+ base = (unsigned long)iv[i].iov_base;
+ len = (unsigned long)iv[i].iov_len;
+
+ if (len == 0)
+ continue;
+
+ /* How many pages will this take? */
+ npages = 1 + (base + len - 1)/PAGE_SIZE - base/PAGE_SIZE;
+ if (unlikely(num_pg + npages > MAX_SKB_FRAGS)) {
+ err = -ENOSPC;
+ goto fail;
+ }
+ n = get_user_pages(current, current->mm, base, npages,
+ 0, 0, pages, NULL);
+ if (unlikely(n < 0)) {
+ err = n;
+ goto fail;
+ }
+
+ /* Transfer pages to the frag array */
+ for (j = 0; j < n; j++) {
+ f[num_pg].page = pages[j];
+ if (j == 0) {
+ f[num_pg].page_offset = offset_in_page(base);
+ f[num_pg].size = min(len, PAGE_SIZE -
+f[num_pg].page_offset);
+ } else {
+ f[num_pg].page_offset = 0;
+ f[num_pg].size = min(len, PAGE_SIZE);
+ }
+ len -= f[num_pg].size;
+ base += f[num_pg].size;
+ num_pg++;
+ }
+
+ if (unlikely(n != npages)) {
+ err = -EFAULT;
+ goto fail;
+ }
+ }
+ up_read(¤t->mm->mmap_sem);
+ return num_pg;
+
+fail:
+ for (i = 0; i < num_pg; i++)
+ put_page(f[i].page);
+ up_read(¤t->mm->mmap_sem);
+ return err;
+}
+
+
+static struct sk_buff *map_user_skb(const struct virtio_net_hdr *gso,
+ size_t align, struct iovec *iv,
+ size_t count, size_t len)
+{
+ struct sk_buff *skb;
+ struct skb_shared_info *sinfo;
+ int err;
+
+ if (gso->gso_hdr_len > len)
+ return ERR_PTR(-EINVAL);
+
+ if (!(skb = alloc_skb(gso->gso_hdr_len + align, GFP_KERNEL)))
+ return ERR_PTR(-ENOMEM);
+
+ if (align)
+ skb_reserve(skb, align);
+
+ sinfo = skb_shinfo(skb);
+ sinfo->gso_size = gso->gso_size;
+ sinfo->gso_type = SKB_GSO_DODGY;
+ switch (gso->gso_type) {
+ case VIRTIO_NET_HDR_GSO_TCPV4_ECN:
+ sinfo->gso_type |= SKB_GSO_TCP_ECN;
+ /* fall through */
+ case VIRTIO_NET_HDR_GSO_TCPV4:
+ sinfo->gso_type |= SKB_GSO_TCPV4;
+ break;
+ case VIRTIO_NET_HDR_GSO_TCPV6:
+ sinfo->gso_type |= SKB_GSO_TCPV6;
+ break;
+ case VIRTIO_NET_HDR_GSO_UDP:
+
[PATCH 2/3] virtio: Net header needs gso_hdr_len
It's far easier to deal with GSO if we don't have to parse the packet
to figure out the header length. Add the field to the virtio_net_hdr
struct (and fix the spaces that somehow crept in there).
Signed-off-by: Rusty Russell <[EMAIL PROTECTED]>
---
drivers/net/virtio_net.c |4 +++-
include/linux/virtio_net.h | 11 ++-
2 files changed, 9 insertions(+), 6 deletions(-)
diff -r 24ef33a4ab14 drivers/net/virtio_net.c
--- a/drivers/net/virtio_net.c Tue Jan 15 16:59:58 2008 +1100
+++ b/drivers/net/virtio_net.c Tue Jan 15 21:21:40 2008 +1100
@@ -126,6 +126,7 @@ static void receive_skb(struct net_devic
/* Header must be checked, and gso_segs computed. */
skb_shinfo(skb)->gso_type |= SKB_GSO_DODGY;
skb_shinfo(skb)->gso_segs = 0;
+ skb_set_transport_header(skb, hdr->gso_hdr_len);
}
netif_receive_skb(skb);
@@ -247,6 +248,7 @@ static int start_xmit(struct sk_buff *sk
}
if (skb_is_gso(skb)) {
+ hdr->gso_hdr_len = skb_transport_header(skb) - skb->data;
hdr->gso_size = skb_shinfo(skb)->gso_size;
if (skb_shinfo(skb)->gso_type & SKB_GSO_TCP_ECN)
hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV4_ECN;
@@ -260,7 +262,7 @@ static int start_xmit(struct sk_buff *sk
BUG();
} else {
hdr->gso_type = VIRTIO_NET_HDR_GSO_NONE;
- hdr->gso_size = 0;
+ hdr->gso_size = hdr->gso_hdr_len = 0;
}
vnet_hdr_to_sg(sg, skb);
diff -r 24ef33a4ab14 include/linux/virtio_net.h
--- a/include/linux/virtio_net.hTue Jan 15 16:59:58 2008 +1100
+++ b/include/linux/virtio_net.hTue Jan 15 21:21:40 2008 +1100
@@ -24,16 +24,17 @@ struct virtio_net_hdr
struct virtio_net_hdr
{
#define VIRTIO_NET_HDR_F_NEEDS_CSUM1 // Use csum_start, csum_offset
- __u8 flags;
+ __u8 flags;
#define VIRTIO_NET_HDR_GSO_NONE0 // Not a GSO frame
#define VIRTIO_NET_HDR_GSO_TCPV4 1 // GSO frame, IPv4 TCP (TSO)
/* FIXME: Do we need this? If they said they can handle ECN, do they care? */
#define VIRTIO_NET_HDR_GSO_TCPV4_ECN 2 // GSO frame, IPv4 TCP w/ ECN
#define VIRTIO_NET_HDR_GSO_UDP 3 // GSO frame, IPv4 UDP (UFO)
#define VIRTIO_NET_HDR_GSO_TCPV6 4 // GSO frame, IPv6 TCP
- __u8 gso_type;
- __u16 gso_size;
- __u16 csum_start;
- __u16 csum_offset;
+ __u8 gso_type;
+ __u16 gso_hdr_len; /* Ethernet + IP + tcp/udp hdrs */
+ __u16 gso_size; /* Bytes to append to gso_hdr_len per frame */
+ __u16 csum_start; /* Position to start checksumming from */
+ __u16 csum_offset; /* Offset after that to place checksum */
};
#endif /* _LINUX_VIRTIO_NET_H */
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization
[PATCH 1/3] skb_partial_csum_set
Implement skb_partial_csum_set, for setting partial csums on untrusted packets.
Use it in virtio_net (replacing buggy version there), it's also going
to be used by TAP for partial csum support.
Signed-off-by: Rusty Russell <[EMAIL PROTECTED]>
---
drivers/net/virtio_net.c | 11 +--
include/linux/skbuff.h |1 +
net/core/skbuff.c| 29 +
3 files changed, 31 insertions(+), 10 deletions(-)
diff -r 72be3d596d31 include/linux/skbuff.h
--- a/include/linux/skbuff.hWed Jan 09 15:57:40 2008 +1100
+++ b/include/linux/skbuff.hWed Jan 09 16:56:41 2008 +1100
@@ -1804,5 +1804,6 @@ static inline void skb_forward_csum(stru
skb->ip_summed = CHECKSUM_NONE;
}
+bool skb_partial_csum_set(struct sk_buff *skb, u16 start, u16 off);
#endif /* __KERNEL__ */
#endif /* _LINUX_SKBUFF_H */
diff -r 72be3d596d31 net/core/skbuff.c
--- a/net/core/skbuff.c Wed Jan 09 15:57:40 2008 +1100
+++ b/net/core/skbuff.c Wed Jan 09 16:56:41 2008 +1100
@@ -2214,6 +2214,34 @@ int skb_cow_data(struct sk_buff *skb, in
return elt;
}
+/**
+ * skb_partial_csum_set - set up and verify partial csum values for packet
+ * @skb: the skb to set
+ * @start: the number of bytes after skb->data to start checksumming.
+ * @off: the offset from start to place the checksum.
+ *
+ * For untrusted partially-checksummed packets, we need to make sure the values
+ * for skb->csum_start and skb->csum_offset are valid so we don't oops.
+ *
+ * This function checks and sets those values and skb->ip_summed: if this
+ * returns false you should drop the packet.
+ */
+bool skb_partial_csum_set(struct sk_buff *skb, u16 start, u16 off)
+{
+ if (unlikely(start > skb->len - 2) ||
+ unlikely((int)start + off > skb->len - 2)) {
+ if (net_ratelimit())
+ printk(KERN_WARNING
+ "bad partial csum: csum=%u/%u len=%u\n",
+ start, off, skb->len);
+ return false;
+ }
+ skb->ip_summed = CHECKSUM_PARTIAL;
+ skb->csum_start = skb_headroom(skb) + start;
+ skb->csum_offset = off;
+ return true;
+}
+
EXPORT_SYMBOL(___pskb_trim);
EXPORT_SYMBOL(__kfree_skb);
EXPORT_SYMBOL(kfree_skb);
@@ -2250,3 +2278,4 @@ EXPORT_SYMBOL(skb_append_datato_frags);
EXPORT_SYMBOL_GPL(skb_to_sgvec);
EXPORT_SYMBOL_GPL(skb_cow_data);
+EXPORT_SYMBOL_GPL(skb_partial_csum_set);
diff -r 72be3d596d31 drivers/net/virtio_net.c
--- a/drivers/net/virtio_net.c Wed Jan 09 15:57:40 2008 +1100
+++ b/drivers/net/virtio_net.c Wed Jan 09 16:56:41 2008 +1100
@@ -89,17 +89,8 @@ static void receive_skb(struct net_devic
if (hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) {
pr_debug("Needs csum!\n");
- skb->ip_summed = CHECKSUM_PARTIAL;
- skb->csum_start = hdr->csum_start;
- skb->csum_offset = hdr->csum_offset;
- if (skb->csum_start > skb->len - 2
- || skb->csum_offset > skb->len - 2) {
- if (net_ratelimit())
- printk(KERN_WARNING "%s: csum=%u/%u len=%u\n",
- dev->name, skb->csum_start,
- skb->csum_offset, skb->len);
+ if (!skb_partial_csum_set(skb,hdr->csum_start,hdr->csum_offset))
goto frame_err;
- }
}
if (hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) {
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization
Re: [PATCH] KVM virtio balloon driver
On Mon, Jan 14, 2008 at 03:29:45PM -0600, Anthony Liguori wrote:
> Marcelo Tosatti wrote:
> >Hi Rusty,
> >
> >It was agreed that the balloon driver should be merged through the
> >virtio tree, so here it goes. It depends on the config_changed patch
> >posted earlier.
> >
> >
> >-
> >
> >Following patch adds the KVM balloon driver.
> >
> >Changes from last version:
> >- Get rid of global variables/structure
> >- Use page->lru to link ballooned pages
> >- Use dev_dbg/dev_printk
> >- Proper kthread_should_stop handling
> >- Move shared definitions to separate header
> >- Use ->config_changed method for notification
> >
> >This depends on Rusty's config_changed patch.
> >
> >Signed-off-by: Marcelo Tosatti <[EMAIL PROTECTED]>
> >
> >
> >Index: linux-2.6-nv/drivers/virtio/Kconfig
> >===
> >--- linux-2.6-nv.orig/drivers/virtio/Kconfig
> >+++ linux-2.6-nv/drivers/virtio/Kconfig
> >@@ -23,3 +23,12 @@ config VIRTIO_PCI
> >
> > If unsure, say M.
> >
> >+config KVM_BALLOON
> >+tristate "KVM balloon driver (EXPERIMENTAL)"
> >+depends on VIRTIO_PCI
> >+---help---
> >+ This driver provides support for ballooning memory in/out of a
> >+ KVM paravirt guest.
> >+
> >+ If unsure, say M.
> >+
>
>
> Please rename from KVM_BALLOON to VIRTIO_BALLOON. Also, it doesn't
> depend on VIRTIO_PCI. It should select VIRTIO and VIRTIO_RING.
>
>
>
> >Index: linux-2.6-nv/drivers/virtio/Makefile
> >===
> >--- linux-2.6-nv.orig/drivers/virtio/Makefile
> >+++ linux-2.6-nv/drivers/virtio/Makefile
> >@@ -1,3 +1,4 @@
> > obj-$(CONFIG_VIRTIO) += virtio.o
> > obj-$(CONFIG_VIRTIO_RING) += virtio_ring.o
> > obj-$(CONFIG_VIRTIO_PCI) += virtio_pci.o
> >+obj-$(CONFIG_KVM_BALLOON) += kvm_balloon.o
> >Index: linux-2.6-nv/drivers/virtio/kvm_balloon.c
> >===
> >--- /dev/null
> >+++ linux-2.6-nv/drivers/virtio/kvm_balloon.c
> >@@ -0,0 +1,537 @@
> >+/*
> >+ * KVM guest balloon driver
> >+ *
> >+ * Copyright (C) 2007, Qumranet, Inc., Dor Laor <[EMAIL PROTECTED]>
> >+ * Copyright (C) 2007, Red Hat, Inc., Marcelo Tosatti
> ><[EMAIL PROTECTED]>
> >+ *
> >+ * This work is licensed under the terms of the GNU GPL, version 2. See
> >+ * the COPYING file in the top-level directory.
> >+ */
> >+
> >+#define DEBUG
> >+#include
> >+#include
> >+#include
> >+#include
> >+#include
> >+#include
> >+#include
> >+#include
> >+#include
> >+#include
> >+#include
> >+#include
> >+#include
> >+#include
> >+#include
> >+#include
> >+#include
> >+#include
>
> Please don't include kvm_types or kvm_host.
>
> >+
> >+MODULE_AUTHOR ("Dor Laor");
> >+MODULE_DESCRIPTION ("Implements guest ballooning support");
> >+MODULE_LICENSE("GPL");
> >+MODULE_VERSION("1");
> >+
> >+static int kvm_balloon_debug;
> >+
> >+#define dprintk(dev, str...) if (kvm_balloon_debug) dev_dbg(dev, str)
>
> This can go away.
I think this is useful for debugging problems on field.
Fixed the previous comments, thanks.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization
Re: [PATCH 1/3] skb_partial_csum_set
From: Rusty Russell <[EMAIL PROTECTED]> Date: Tue, 15 Jan 2008 21:41:55 +1100 > Implement skb_partial_csum_set, for setting partial csums on untrusted > packets. > > Use it in virtio_net (replacing buggy version there), it's also going > to be used by TAP for partial csum support. > > Signed-off-by: Rusty Russell <[EMAIL PROTECTED]> Looks fine to me. Acked-by: David S. Miller <[EMAIL PROTECTED]> If you like I can merge this into my net-2.6.25 tree, or alternatively if it makes your life easier you then you can handle it yourself. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
Re: [PATCH] KVM virtio balloon driver
On Tue, Jan 15, 2008 at 10:32:08AM +1100, Rusty Russell wrote:
> On Tuesday 15 January 2008 07:03:57 Marcelo Tosatti wrote:
> > Hi Rusty,
> >
> > It was agreed that the balloon driver should be merged through the
> > virtio tree, so here it goes. It depends on the config_changed patch
> > posted earlier.
>
> Hi Marcelo,
>
> Excellent! Although the main user will be kvm, it'd be nice to have a
> demonstration in-tree using lguest; any chance of you conjuring up an
> appropriate patch?
>
> If not, I can whip something up.
>
> > +config KVM_BALLOON
> > + tristate "KVM balloon driver (EXPERIMENTAL)"
> > + depends on VIRTIO_PCI
> > + ---help---
> > +This driver provides support for ballooning memory in/out of a
> > +KVM paravirt guest.
> > +
> > +If unsure, say M.
>
> Please don't define "balloon" in terms of "ballooning". How about "This
> driver supports increasing and decreasing the amount of memory within a KVM
> guest." ?
>
> > + uint32_t target_nrpages;
>
> I prefer u32 within the kernel, but no big deal.
>
> > +static int send_balloon_buf(struct virtballoon *v, uint8_t cmd,
> > + struct balloon_buf *buf)
> > +{
> > + struct scatterlist sg[VIRTIO_MAX_SG];
> > + int err = 0;
> > +
> > + buf->hdr.cmd = cmd;
> > +
> > + sg_init_table(sg, VIRTIO_MAX_SG);
> > + sg_set_buf(&sg[0], &buf->hdr, sizeof(buf->hdr));
> > + sg_set_buf(&sg[1], &buf->data, sizeof(buf->data));
>
> Since these are adjacent, can't you just combine them into one sg element?
> Or
> does the kvm code rely on the sg as a boundary between header and data?
>
> > +static int kvm_balloon_inflate(struct virtballoon *v, int32_t npages)
> > +{
> > + LIST_HEAD(tmp_list);
> > + struct page *page, *tmp;
> > + struct balloon_buf *buf;
> > + u32 *pfn;
> > + int allocated = 0;
> > + int i, r = -ENOMEM;
> > +
> > + buf = alloc_balloon_buf(v->vdev, GFP_KERNEL);
> > + if (!buf)
> > + return r;
> > +
> > + pfn = (u32 *)&buf->data;
> > + *pfn++ = (u32)npages;
>
> OK, this seems strange. You always use the data portion as an array of u32s,
> yet you declare it as char[200]. You have a perfectly good header, but you
> put the number of pages in the first element of the data array: and you hand
> the entire data array even though you normally only use part of it.
>
> You can intuit the number from the sg len, I suggest you do that instead.
>
> Looking at this driver, I just don't think this needs to be so complicated:
> it's not a high speed device, after all. Perhaps allocate one buffer up
> front, and just reuse that. One simple thread loop, less locking needed.
>
> > + for (i = 0; i < npages; i++) {
> > + page = alloc_page(GFP_HIGHUSER | __GFP_NORETRY);
> > + if (!page)
> > + goto out_free;
> > + list_add(&page->lru, &tmp_list);
> > + allocated++;
> > + *pfn = page_to_pfn(page);
> > + pfn++;
> > + }
>
> I think it might be simpler to defer adding to any list until after the
> callback is done? Helpers to add an array to the balloon array (on
> successful inflate, or unsuccessful deflate) and to free them (unsuccessful
> inflate, or successful deflate) would also make the code more readable.
>
> > + gfns_per_buf = MAX_BALLOON_PAGES_PER_OP;
> > +
> > + /*
> > +* Call the balloon in PAGE_SIZE*pfns-per-buf
> > +* iterations
> > +*/
> > + iterations = DIV_ROUND_UP(abspages, gfns_per_buf);
> > + dprintk(&v->vdev->dev, "%s: iterations=%d\n", __func__, iterations);
> > +
> > + for (i = 0; i < iterations; i++) {
>
> This logic seems overly complex. How about in the main thread:
>
> /* We're woken when target changes, in config_changed() */
> if (wait_event_interruptible(&v->wq,
> (diff = atomic_read(&v->target_pages) - total_pages)) == 0) {
>
> /* If we submit inflate/deflate request, wait for it to finish.
> */
> if (xflate(v, diff) == 0)
> wait_for_completion(&v->complete);
> }
>
> xflate just does as much as it can (up to "diff"), and then the interrupt
> handler adds/removes from the linked list, frees pages, etc and fires the
> completion.
>
> No need for locking, since there's only one pending request at any time.
OK, thats simpler. How about this:
[PATCH] Virtio balloon driver
Add a balloon driver for KVM, host<->guest communication is performed
via virtio.
Signed-off-by: Marcelo Tosatti <[EMAIL PROTECTED]>
Index: linux-2.6-nv/drivers/virtio/Kconfig
===
--- linux-2.6-nv.orig/drivers/virtio/Kconfig
+++ linux-2.6-nv/drivers/virtio/Kconfig
@@ -23,3 +23,13 @@ config VIRTIO_PCI
If unsure, say M.
+config VIRTIO_BALLOON
+ tristate "Virtio balloon driver (EXPERIMENTAL)"
+ select VIRTIO
+ select VIRTIO_RING
+ ---help---
+This driver supports in
Re: [PATCH 1/3] skb_partial_csum_set
On Tuesday 15 January 2008 22:14:22 David Miller wrote: > From: Rusty Russell <[EMAIL PROTECTED]> > Date: Tue, 15 Jan 2008 21:41:55 +1100 > > > Implement skb_partial_csum_set, for setting partial csums on untrusted > > packets. > > > > Use it in virtio_net (replacing buggy version there), it's also going > > to be used by TAP for partial csum support. > > > > Signed-off-by: Rusty Russell <[EMAIL PROTECTED]> > > Looks fine to me. > > Acked-by: David S. Miller <[EMAIL PROTECTED]> > > If you like I can merge this into my net-2.6.25 tree, or alternatively > if it makes your life easier you then you can handle it yourself. Thanks, that will reduce coordination pain. Cheers, Rusty. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
Re: [PATCH 2/3] virtio: Net header needs gso_hdr_len
Rusty Russell <[EMAIL PROTECTED]> wrote:
> It's far easier to deal with GSO if we don't have to parse the packet
> to figure out the header length. Add the field to the virtio_net_hdr
> struct (and fix the spaces that somehow crept in there).
>
> Signed-off-by: Rusty Russell <[EMAIL PROTECTED]>
> ---
> drivers/net/virtio_net.c |4 +++-
> include/linux/virtio_net.h | 11 ++-
> 2 files changed, 9 insertions(+), 6 deletions(-)
>
> diff -r 24ef33a4ab14 drivers/net/virtio_net.c
> --- a/drivers/net/virtio_net.c Tue Jan 15 16:59:58 2008 +1100
> +++ b/drivers/net/virtio_net.c Tue Jan 15 21:21:40 2008 +1100
> @@ -126,6 +126,7 @@ static void receive_skb(struct net_devic
>/* Header must be checked, and gso_segs computed. */
>skb_shinfo(skb)->gso_type |= SKB_GSO_DODGY;
>skb_shinfo(skb)->gso_segs = 0;
> + skb_set_transport_header(skb, hdr->gso_hdr_len);
Why do we need this? When receiving GSO packets from an untrusted
source the network stack will fill in the transport header offset
after verifying that the headers are sane.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[EMAIL PROTECTED]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization
Re: [PATCH 2/3] virtio: Net header needs gso_hdr_len
On Wednesday 16 January 2008 11:06:21 Herbert Xu wrote: > Rusty Russell <[EMAIL PROTECTED]> wrote: > > It's far easier to deal with GSO if we don't have to parse the packet > > to figure out the header length. Add the field to the virtio_net_hdr > > struct (and fix the spaces that somehow crept in there). > > > > Signed-off-by: Rusty Russell <[EMAIL PROTECTED]> > > --- > > drivers/net/virtio_net.c |4 +++- > > include/linux/virtio_net.h | 11 ++- > > 2 files changed, 9 insertions(+), 6 deletions(-) > > > > diff -r 24ef33a4ab14 drivers/net/virtio_net.c > > --- a/drivers/net/virtio_net.c Tue Jan 15 16:59:58 2008 +1100 > > +++ b/drivers/net/virtio_net.c Tue Jan 15 21:21:40 2008 +1100 > > @@ -126,6 +126,7 @@ static void receive_skb(struct net_devic > >/* Header must be checked, and gso_segs computed. */ > >skb_shinfo(skb)->gso_type |= SKB_GSO_DODGY; > >skb_shinfo(skb)->gso_segs = 0; > > + skb_set_transport_header(skb, hdr->gso_hdr_len); > > Why do we need this? When receiving GSO packets from an untrusted > source the network stack will fill in the transport header offset > after verifying that the headers are sane. Thanks for clarifying; it simplifies things. I'll re-test and resend. Rusty. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
