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@xxxxxxxxxx>
>
> [ 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@xxxxxxxxxx>
> Signed-off-by: Vlad Yasevich <vyasevic@xxxxxxxxxx>
> Signed-off-by: David S. Miller <davem@xxxxxxxxxxxxx>
> Signed-off-by: Ben Hutchings <ben@xxxxxxxxxxxxxxx>
> ---
> 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@xxxxxxxxxx>
Date: Sat Oct 19 11:42:54 2013 -0700
net: generalize skb_segment()
that correctly populates the header offsets in the new segment.
-vlad
> skb_copy_from_linear_data(skb, nskb->data, doffset);
>
> if (fskb != skb_shinfo(skb)->frag_list)
>
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html