| Message ID | 20180116020920.20232-3-dja@axtens.net |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | Check gso_size of packets when forwarding | expand |
On Tue, Jan 16, 2018 at 01:09:19PM +1100, Daniel Axtens wrote: > is_skb_forwardable attempts to detect if a packet is too large to > be sent to the destination device. However, this test does not > consider GSO packets, and it is possible that a GSO packet, when > resegmented, will be larger than the device can transmit. > > Add detection for packets which will be too large by creating a > skb_gso_validate_len() routine which is similar to > skb_gso_validate_mtu(), but which considers L2 headers, and wire Why not add a 3rd parameter to skb_gso_validate_mtu() instead, hlen? After all it's the only difference between both. And maybe rename _mtu if you think the term doesn't fit in the new usecase or add a wrapper around it for each case. As it is here, it gets very confusing on which function does what, IMHO. > it up in is_skb_forwardable(). > > Signed-off-by: Daniel Axtens <dja@axtens.net> > --- > include/linux/skbuff.h | 1 + > net/core/dev.c | 7 ++++--- > net/core/skbuff.c | 34 ++++++++++++++++++++++++++++++++++ > 3 files changed, 39 insertions(+), 3 deletions(-) > > diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h > index b8acafd73272..6a9e4b6f80a7 100644 > --- a/include/linux/skbuff.h > +++ b/include/linux/skbuff.h > @@ -3287,6 +3287,7 @@ int skb_shift(struct sk_buff *tgt, struct sk_buff *skb, int shiftlen); > void skb_scrub_packet(struct sk_buff *skb, bool xnet); > unsigned int skb_gso_transport_seglen(const struct sk_buff *skb); > bool skb_gso_validate_mtu(const struct sk_buff *skb, unsigned int mtu); > +bool skb_gso_validate_len(const struct sk_buff *skb, unsigned int len); > struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features); > struct sk_buff *skb_vlan_untag(struct sk_buff *skb); > int skb_ensure_writable(struct sk_buff *skb, int write_len); > diff --git a/net/core/dev.c b/net/core/dev.c > index 94435cd09072..5af04be128c1 100644 > --- a/net/core/dev.c > +++ b/net/core/dev.c > @@ -1841,11 +1841,12 @@ bool is_skb_forwardable(const struct net_device *dev, const struct sk_buff *skb) > if (skb->len <= len) > return true; > > - /* if TSO is enabled, we don't care about the length as the packet > - * could be forwarded without being segmented before > + /* > + * if TSO is enabled, we need to check the size of the > + * segmented packets > */ > if (skb_is_gso(skb)) > - return true; > + return skb_gso_validate_len(skb, len); > > return false; > } > diff --git a/net/core/skbuff.c b/net/core/skbuff.c > index 01e8285aea73..aefe049e4b0c 100644 > --- a/net/core/skbuff.c > +++ b/net/core/skbuff.c > @@ -4945,6 +4945,40 @@ bool skb_gso_validate_mtu(const struct sk_buff *skb, unsigned int mtu) > } > EXPORT_SYMBOL_GPL(skb_gso_validate_mtu); > > +/** > + * skb_gso_validate_len - Will a split GSO skb fit in a given length? > + * > + * @skb: GSO skb > + * @len: length to validate against > + * > + * skb_gso_validate_len validates if a given skb will fit a wanted > + * length once split, including L2, L3 and L4 headers. > + * > + * Similar to skb_gso_validate_mtu, but inclusive of L2 headers. > + */ > +bool skb_gso_validate_len(const struct sk_buff *skb, unsigned int len) > +{ > + const struct skb_shared_info *shinfo = skb_shinfo(skb); > + const struct sk_buff *iter; > + unsigned int hlen; > + > + hlen = skb_gso_mac_seglen(skb); > + > + if (shinfo->gso_size != GSO_BY_FRAGS) > + return hlen <= len; > + > + /* Undo this so we can re-use header sizes */ > + hlen -= GSO_BY_FRAGS; > + > + skb_walk_frags(skb, iter) { > + if (hlen + skb_headlen(iter) > len) > + return false; > + } > + > + return true; > +} > +EXPORT_SYMBOL_GPL(skb_gso_validate_len); > + > static struct sk_buff *skb_reorder_vlan_header(struct sk_buff *skb) > { > if (skb_cow(skb, skb_headroom(skb)) < 0) { > -- > 2.14.1 >
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h index b8acafd73272..6a9e4b6f80a7 100644 --- a/include/linux/skbuff.h +++ b/include/linux/skbuff.h @@ -3287,6 +3287,7 @@ int skb_shift(struct sk_buff *tgt, struct sk_buff *skb, int shiftlen); void skb_scrub_packet(struct sk_buff *skb, bool xnet); unsigned int skb_gso_transport_seglen(const struct sk_buff *skb); bool skb_gso_validate_mtu(const struct sk_buff *skb, unsigned int mtu); +bool skb_gso_validate_len(const struct sk_buff *skb, unsigned int len); struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features); struct sk_buff *skb_vlan_untag(struct sk_buff *skb); int skb_ensure_writable(struct sk_buff *skb, int write_len); diff --git a/net/core/dev.c b/net/core/dev.c index 94435cd09072..5af04be128c1 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -1841,11 +1841,12 @@ bool is_skb_forwardable(const struct net_device *dev, const struct sk_buff *skb) if (skb->len <= len) return true; - /* if TSO is enabled, we don't care about the length as the packet - * could be forwarded without being segmented before + /* + * if TSO is enabled, we need to check the size of the + * segmented packets */ if (skb_is_gso(skb)) - return true; + return skb_gso_validate_len(skb, len); return false; } diff --git a/net/core/skbuff.c b/net/core/skbuff.c index 01e8285aea73..aefe049e4b0c 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -4945,6 +4945,40 @@ bool skb_gso_validate_mtu(const struct sk_buff *skb, unsigned int mtu) } EXPORT_SYMBOL_GPL(skb_gso_validate_mtu); +/** + * skb_gso_validate_len - Will a split GSO skb fit in a given length? + * + * @skb: GSO skb + * @len: length to validate against + * + * skb_gso_validate_len validates if a given skb will fit a wanted + * length once split, including L2, L3 and L4 headers. + * + * Similar to skb_gso_validate_mtu, but inclusive of L2 headers. + */ +bool skb_gso_validate_len(const struct sk_buff *skb, unsigned int len) +{ + const struct skb_shared_info *shinfo = skb_shinfo(skb); + const struct sk_buff *iter; + unsigned int hlen; + + hlen = skb_gso_mac_seglen(skb); + + if (shinfo->gso_size != GSO_BY_FRAGS) + return hlen <= len; + + /* Undo this so we can re-use header sizes */ + hlen -= GSO_BY_FRAGS; + + skb_walk_frags(skb, iter) { + if (hlen + skb_headlen(iter) > len) + return false; + } + + return true; +} +EXPORT_SYMBOL_GPL(skb_gso_validate_len); + static struct sk_buff *skb_reorder_vlan_header(struct sk_buff *skb) { if (skb_cow(skb, skb_headroom(skb)) < 0) {
is_skb_forwardable attempts to detect if a packet is too large to be sent to the destination device. However, this test does not consider GSO packets, and it is possible that a GSO packet, when resegmented, will be larger than the device can transmit. Add detection for packets which will be too large by creating a skb_gso_validate_len() routine which is similar to skb_gso_validate_mtu(), but which considers L2 headers, and wire it up in is_skb_forwardable(). Signed-off-by: Daniel Axtens <dja@axtens.net> --- include/linux/skbuff.h | 1 + net/core/dev.c | 7 ++++--- net/core/skbuff.c | 34 ++++++++++++++++++++++++++++++++++ 3 files changed, 39 insertions(+), 3 deletions(-)