| Message ID | m1wrjsi2tf.fsf@fess.ebiederm.org |
|---|---|
| State | Deferred, archived |
| Delegated to: | David Miller |
| Headers | show |
Le lundi 21 mars 2011 à 14:25 -0700, Eric W. Biederman a écrit : > Today when gso packets reach dev_forward_skb through the macvlan driver > we drop them on the floor because they exceed the device mtu. Ouch! > > I don't undersand the subtelties of gso but I think it is sufficient to > simply relax the checks and let gso packets through without an mtu > check, and it works in my test case. > > If needed we can split the gso packets into multiple packets here but > that just seems like a wast of memory and time. > > Signed-off-by: Eric W. Biederman <ebiederm@aristanetworks.com> > --- > net/core/dev.c | 16 ++++++++++------ > 1 files changed, 10 insertions(+), 6 deletions(-) > > diff --git a/net/core/dev.c b/net/core/dev.c > index 0b88eba..2e26606 100644 > --- a/net/core/dev.c > +++ b/net/core/dev.c > @@ -1527,17 +1527,21 @@ int dev_forward_skb(struct net_device *dev, struct sk_buff *skb) > skb_orphan(skb); > nf_reset(skb); > > - if (unlikely(!(dev->flags & IFF_UP) || > - (skb->len > (dev->mtu + dev->hard_header_len + VLAN_HLEN)))) { > - atomic_long_inc(&dev->rx_dropped); > - kfree_skb(skb); > - return NET_RX_DROP; > - } > + if (unlikely(!(dev->flags & IFF_UP))) > + goto kfree_skb; > + /* Don't check mtu on gso packets... */ > + if (unlikely(!skb_is_gso(skb) && > + (skb->len > (dev->mtu + dev->hard_header_len + VLAN_HLEN)))) > + goto kfree_skb; > skb_set_dev(skb, dev); > skb->tstamp.tv64 = 0; > skb->pkt_type = PACKET_HOST; > skb->protocol = eth_type_trans(skb, dev); > return netif_rx(skb); > +kfree_skb: > + atomic_long_inc(&dev->rx_dropped); > + kfree_skb(skb); > + return NET_RX_DROP; > } > EXPORT_SYMBOL_GPL(dev_forward_skb); > Hmm, did you follow http://patchwork.ozlabs.org/patch/86815/ discussion ? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Eric Dumazet <eric.dumazet@gmail.com> writes: > Hmm, did you follow http://patchwork.ozlabs.org/patch/86815/ > discussion ? No. It seems I missed it, but I do have the same problem, and essentially the same fix. Eric -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: ebiederm@xmission.com (Eric W. Biederman) Date: Mon, 21 Mar 2011 15:00:56 -0700 > Eric Dumazet <eric.dumazet@gmail.com> writes: > >> Hmm, did you follow http://patchwork.ozlabs.org/patch/86815/ >> discussion ? > > No. It seems I missed it, but I do have the same problem, and > essentially the same fix. Can someone work on getting this straightened out and resubmit the final patch so I can apply something? Thanks. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Le dimanche 27 mars 2011 à 18:09 -0700, David Miller a écrit : > From: ebiederm@xmission.com (Eric W. Biederman) > Date: Mon, 21 Mar 2011 15:00:56 -0700 > > > Eric Dumazet <eric.dumazet@gmail.com> writes: > > > >> Hmm, did you follow http://patchwork.ozlabs.org/patch/86815/ > >> discussion ? > > > > No. It seems I missed it, but I do have the same problem, and > > essentially the same fix. > > Can someone work on getting this straightened out and resubmit the > final patch so I can apply something? > Apparently Daniel is busy. I guess we can use Eric B. patch, then ? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Eric Dumazet <eric.dumazet@gmail.com> writes: > Le dimanche 27 mars 2011 à 18:09 -0700, David Miller a écrit : >> From: ebiederm@xmission.com (Eric W. Biederman) >> Date: Mon, 21 Mar 2011 15:00:56 -0700 >> >> > Eric Dumazet <eric.dumazet@gmail.com> writes: >> > >> >> Hmm, did you follow http://patchwork.ozlabs.org/patch/86815/ >> >> discussion ? >> > >> > No. It seems I missed it, but I do have the same problem, and >> > essentially the same fix. >> >> Can someone work on getting this straightened out and resubmit the >> final patch so I can apply something? >> > > Apparently Daniel is busy. > > I guess we can use Eric B. patch, then ? That sounds good to me. After reviewing the other thread to the best of my knowledge there are no issues with the patch I submitted. At worst I am 4 bytes extra permissive about the mtu because of the way that path handles vlan headers and that isn't something my patch changed. Eric -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/core/dev.c b/net/core/dev.c index 0b88eba..2e26606 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -1527,17 +1527,21 @@ int dev_forward_skb(struct net_device *dev, struct sk_buff *skb) skb_orphan(skb); nf_reset(skb); - if (unlikely(!(dev->flags & IFF_UP) || - (skb->len > (dev->mtu + dev->hard_header_len + VLAN_HLEN)))) { - atomic_long_inc(&dev->rx_dropped); - kfree_skb(skb); - return NET_RX_DROP; - } + if (unlikely(!(dev->flags & IFF_UP))) + goto kfree_skb; + /* Don't check mtu on gso packets... */ + if (unlikely(!skb_is_gso(skb) && + (skb->len > (dev->mtu + dev->hard_header_len + VLAN_HLEN)))) + goto kfree_skb; skb_set_dev(skb, dev); skb->tstamp.tv64 = 0; skb->pkt_type = PACKET_HOST; skb->protocol = eth_type_trans(skb, dev); return netif_rx(skb); +kfree_skb: + atomic_long_inc(&dev->rx_dropped); + kfree_skb(skb); + return NET_RX_DROP; } EXPORT_SYMBOL_GPL(dev_forward_skb);
Today when gso packets reach dev_forward_skb through the macvlan driver we drop them on the floor because they exceed the device mtu. Ouch! I don't undersand the subtelties of gso but I think it is sufficient to simply relax the checks and let gso packets through without an mtu check, and it works in my test case. If needed we can split the gso packets into multiple packets here but that just seems like a wast of memory and time. Signed-off-by: Eric W. Biederman <ebiederm@aristanetworks.com> --- net/core/dev.c | 16 ++++++++++------ 1 files changed, 10 insertions(+), 6 deletions(-)