lkml.org 
[lkml]   [2014]   [Sep]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH 3.2 105/131] net: Correctly set segment mac_len in skb_segment().
    From
    Date
    On Thu, 2014-09-11 at 08:48 -0400, Vlad Yasevich wrote:
    > On 09/11/2014 08:32 AM, Ben Hutchings wrote:
    > > 3.2.63-rc1 review patch. If anyone has any objections, please let me know.
    > >
    > > ------------------
    > >
    > > From: Vlad Yasevich <vyasevic@redhat.com>
    > >
    > > [ Upstream commit fcdfe3a7fa4cb74391d42b6a26dc07c20dab1d82 ]
    > >
    > > When performing segmentation, the mac_len value is copied right
    > > out of the original skb. However, this value is not always set correctly
    > > (like when the packet is VLAN-tagged) and we'll end up copying a bad
    > > value.
    > >
    > > One way to demonstrate this is to configure a VM which tags
    > > packets internally and turn off VLAN acceleration on the forwarding
    > > bridge port. The packets show up corrupt like this:
    > > 16:18:24.985548 52:54:00:ab:be:25 > 52:54:00:26:ce:a3, ethertype 802.1Q
    > > (0x8100), length 1518: vlan 100, p 0, ethertype 0x05e0,
    > > 0x0000: 8cdb 1c7c 8cdb 0064 4006 b59d 0a00 6402 ...|...d@.....d.
    > > 0x0010: 0a00 6401 9e0d b441 0a5e 64ec 0330 14fa ..d....A.^d..0..
    > > 0x0020: 29e3 01c9 f871 0000 0101 080a 000a e833)....q.........3
    > > 0x0030: 000f 8c75 6e65 7470 6572 6600 6e65 7470 ...unetperf.netp
    > > 0x0040: 6572 6600 6e65 7470 6572 6600 6e65 7470 erf.netperf.netp
    > > 0x0050: 6572 6600 6e65 7470 6572 6600 6e65 7470 erf.netperf.netp
    > > 0x0060: 6572 6600 6e65 7470 6572 6600 6e65 7470 erf.netperf.netp
    > > ...
    > >
    > > This also leads to awful throughput as GSO packets are dropped and
    > > cause retransmissions.
    > >
    > > The solution is to set the mac_len using the values already available
    > > in then new skb. We've already adjusted all of the header offset, so we
    > > might as well correctly figure out the mac_len using skb_reset_mac_len().
    > > After this change, packets are segmented correctly and performance
    > > is restored.
    > >
    > > CC: Eric Dumazet <edumazet@google.com>
    > > Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
    > > Signed-off-by: David S. Miller <davem@davemloft.net>
    > > Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
    > > ---
    > > net/core/skbuff.c | 2 +-
    > > 1 file changed, 1 insertion(+), 1 deletion(-)
    > >
    > > diff --git a/net/core/skbuff.c b/net/core/skbuff.c
    > > index 7121d9b..0ccfb53 100644
    > > --- a/net/core/skbuff.c
    > > +++ b/net/core/skbuff.c
    > > @@ -2669,7 +2669,6 @@ struct sk_buff *skb_segment(struct sk_buff *skb, u32 features)
    > > tail = nskb;
    > >
    > > __copy_skb_header(nskb, skb);
    > > - nskb->mac_len = skb->mac_len;
    > >
    > > /* nskb and skb might have different headroom */
    > > if (nskb->ip_summed == CHECKSUM_PARTIAL)
    > > @@ -2679,6 +2678,7 @@ struct sk_buff *skb_segment(struct sk_buff *skb, u32 features)
    > > skb_set_network_header(nskb, skb->mac_len);
    > > nskb->transport_header = (nskb->network_header +
    > > skb_network_header_len(skb));
    > > + skb_reset_mac_len(nskb);
    >
    > This will not fix the problem here because the network header above will
    > already be set incorrectly based on the old mac_len.
    >
    > This patch depends on
    > commit 030737bcc3c404e273e97dbe06fe9561699a411b
    > Author: Eric Dumazet <edumazet@google.com>
    > Date: Sat Oct 19 11:42:54 2013 -0700
    >
    > net: generalize skb_segment()
    >
    > that correctly populates the header offsets in the new segment.

    I can't apply that because 3.2 doesn't even have the
    skb_headers_offset_update() function. So I'm going to drop this patch
    for now, but if you or David can provide a complete backport for this
    fix I would very much appreciate it.

    Ben.

    --
    Ben Hutchings
    I'm always amazed by the number of people who take up solipsism because
    they heard someone else explain it. - E*Borg on alt.fan.pratchett
    [unhandled content-type:application/pgp-signature]
    \
     
     \ /
      Last update: 2014-09-14 01:21    [W:4.252 / U:0.040 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site