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 <d...@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
>
