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@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. 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
Attachment:
signature.asc
Description: This is a digitally signed message part