Re: [PATCH] Fixed zero copy GSO without orphaning the fragments

On Tue, May 20, 2014 at 5:28 PM, Eric Dumazet [off-list ref] wrote:

On Tue, 2014-05-20 at 14:24 +0300, Igor Royzis wrote:

quoted

Fix accessing GSO fragments memory (and a possible corruption therefore) after
reporting completion in a zero copy callback. The previous fix in the commit 1fd819ec
orphaned frags which eliminates zero copy advantages. The fix makes the completion
called after all the fragments were processed avoiding unnecessary orphaning/copying
from userspace.

The GSO fragments corruption issue was observed in a typical QEMU/KVM VM setup that
hosts a Windows guest (since QEMU virtio-net Windows driver doesn't support GRO).
The fix has been verified by running the HCK OffloadLSO test.

Signed-off-by: Igor Royzis <redacted>
Signed-off-by: Anton Nayshtut <redacted>
---
 include/linux/skbuff.h |    1 +
 net/core/skbuff.c      |   18 +++++++++++++-----
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 08074a8..8c49edc 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h

@@ -287,6 +287,7 @@ struct skb_shared_info {
      struct sk_buff  *frag_list;
      struct skb_shared_hwtstamps hwtstamps;
      __be32          ip6_frag_id;
+     struct sk_buff  *zcopy_src;

Before your patch :

sizeof(struct skb_shared_info)=0x140
offsetof(struct skb_shared_info, frags[1])=0x40

SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) -> 0x140

After your patch :

sizeof(struct skb_shared_info)=0x148
offsetof(struct skb_shared_info, frags[1])=0x48

SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) -> 0x180

Thats a serious bump, because it increases all skb truesizes, and
typical skb with one fragment will use 2 cache lines instead of one in
struct skb_shared_info, so this adds memory pressure in fast path.

So while this patch might increase performance for some workloads,
it generally decreases performance on many others.

Would it "ease" the memory cache penalty if we moved the parent
fragment pointer from skb_shared_info to skbuff itself?