[PATCH v2 0/4] Check size of packets before sending

[PATCH v2 0/4] Check size of packets before sending

[Daniel Axtens]

There are a few ways we can send packets that are too large to a
network driver.

When non-GSO packets are forwarded, we validate their size, based on
the MTU of the destination device. However, when GSO packets are
forwarded, we do not validate their size. We implicitly assume that
when they are segmented, the resultant packets will be correctly
sized.

This is not always the case.

We observed a case where a packet received on an ibmveth device had a
GSO size of around 10kB. This was forwarded by Open vSwitch to a bnx2x
device, where it caused a firmware assert. This is described in detail
at [0] and was the genesis of this series.

Rather than fixing this in the driver, this series fixes the
core path. It does it in 2 steps:

 1) make is_skb_forwardable check GSO packets - this catches bridges
 
 2) make validate_xmit_skb check the size of all packets, so as to
    catch everything else (e.g. macvlan, tc mired, OVS)

I am a bit nervous about how this series will interact with nested
VLANs, as the existing code only allows for one VLAN_HLEN. (Previously
these packets would sail past unchecked.) But I thought it would be
prudent to get more eyes on this sooner rather than later.

Thanks,
Daniel

v1: https://www.spinics.net/lists/netdev/msg478634.html
Changes in v2:

 - improve names, thanks Marcelo Ricardo Leitner

 - add check to xmit_validate_skb; thanks to everyone who participated
   in the discussion.

 - drop extra check in Open vSwitch. Bad packets will be caught by
   validate_xmit_skb for now and we can come back and add it later if
   OVS people would like the extra logging.
   
[0]: https://patchwork.ozlabs.org/patch/859410/

Cc: Jason Wang <jasowang-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: Pravin Shelar <pshelar-LZ6Gd1LRuIk@public.gmane.org>
Cc: Marcelo Ricardo Leitner <marcelo.leitner-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Manish.Chopra-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org
Cc: dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org

Daniel Axtens (4):
  net: rename skb_gso_validate_mtu -> skb_gso_validate_network_len
  net: move skb_gso_mac_seglen to skbuff.h
  net: is_skb_forwardable: check the size of GSO segments
  net: check the size of a packet in validate_xmit_skb

 include/linux/skbuff.h                  | 18 ++++++++-
 net/core/dev.c                          | 24 ++++++++----
 net/core/skbuff.c                       | 66 ++++++++++++++++++++++++++-------
 net/ipv4/ip_forward.c                   |  2 +-
 net/ipv4/ip_output.c                    |  2 +-
 net/ipv4/netfilter/nf_flow_table_ipv4.c |  2 +-
 net/ipv6/ip6_output.c                   |  2 +-
 net/ipv6/netfilter/nf_flow_table_ipv6.c |  2 +-
 net/mpls/af_mpls.c                      |  2 +-
 net/sched/sch_tbf.c                     | 10 -----
 net/xfrm/xfrm_device.c                  |  2 +-
 11 files changed, 93 insertions(+), 39 deletions(-)

-- 
2.14.1

[PATCH v2 1/4] net: rename skb_gso_validate_mtu -> skb_gso_validate_network_len

[Daniel Axtens]

If you take a GSO skb, and split it into packets, will the network
length (L3 headers + L4 headers + payload) of those packets be small
enough to fit within a given MTU?

skb_gso_validate_mtu gives you the answer to that question. However,
we're about to add a way to validate the MAC length of a split GSO
skb (L2+L3+L4+payload), and the names get confusing, so rename
skb_gso_validate_mtu to skb_gso_validate_network_len

Signed-off-by: Daniel Axtens <dja@axtens.net>
---
 include/linux/skbuff.h                  | 2 +-
 net/core/skbuff.c                       | 9 +++++----
 net/ipv4/ip_forward.c                   | 2 +-
 net/ipv4/ip_output.c                    | 2 +-
 net/ipv4/netfilter/nf_flow_table_ipv4.c | 2 +-
 net/ipv6/ip6_output.c                   | 2 +-
 net/ipv6/netfilter/nf_flow_table_ipv6.c | 2 +-
 net/mpls/af_mpls.c                      | 2 +-
 net/xfrm/xfrm_device.c                  | 2 +-
 9 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index b8e0da6c27d6..b137c79bf88d 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -3286,7 +3286,7 @@ void skb_split(struct sk_buff *skb, struct sk_buff *skb1, const u32 len);
 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_network_len(const struct sk_buff *skb, unsigned int mtu);
 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/skbuff.c b/net/core/skbuff.c
index 01e8285aea73..a93e5c7aa5b2 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -4914,15 +4914,16 @@ unsigned int skb_gso_transport_seglen(const struct sk_buff *skb)
 EXPORT_SYMBOL_GPL(skb_gso_transport_seglen);
 
 /**
- * skb_gso_validate_mtu - Return in case such skb fits a given MTU
+ * skb_gso_validate_network_len - Will a split GSO skb fit into a given MTU?
  *
  * @skb: GSO skb
  * @mtu: MTU to validate against
  *
- * skb_gso_validate_mtu validates if a given skb will fit a wanted MTU
- * once split.
+ * skb_gso_validate_network_len validates if a given skb will fit a
+ * wanted MTU once split. It considers L3 headers, L4 headers, and the
+ * payload.
  */
-bool skb_gso_validate_mtu(const struct sk_buff *skb, unsigned int mtu)
+bool skb_gso_validate_network_len(const struct sk_buff *skb, unsigned int mtu)
 {
 	const struct skb_shared_info *shinfo = skb_shinfo(skb);
 	const struct sk_buff *iter;
diff --git a/net/ipv4/ip_forward.c b/net/ipv4/ip_forward.c
index 2dd21c3281a1..b54b948b0596 100644
--- a/net/ipv4/ip_forward.c
+++ b/net/ipv4/ip_forward.c
@@ -55,7 +55,7 @@ static bool ip_exceeds_mtu(const struct sk_buff *skb, unsigned int mtu)
 	if (skb->ignore_df)
 		return false;
 
-	if (skb_is_gso(skb) && skb_gso_validate_mtu(skb, mtu))
+	if (skb_is_gso(skb) && skb_gso_validate_network_len(skb, mtu))
 		return false;
 
 	return true;
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index e8e675be60ec..66340ab750e6 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -248,7 +248,7 @@ static int ip_finish_output_gso(struct net *net, struct sock *sk,
 
 	/* common case: seglen is <= mtu
 	 */
-	if (skb_gso_validate_mtu(skb, mtu))
+	if (skb_gso_validate_network_len(skb, mtu))
 		return ip_finish_output2(net, sk, skb);
 
 	/* Slowpath -  GSO segment length exceeds the egress MTU.
diff --git a/net/ipv4/netfilter/nf_flow_table_ipv4.c b/net/ipv4/netfilter/nf_flow_table_ipv4.c
index b2d01eb25f2c..cdf2625dc277 100644
--- a/net/ipv4/netfilter/nf_flow_table_ipv4.c
+++ b/net/ipv4/netfilter/nf_flow_table_ipv4.c
@@ -185,7 +185,7 @@ static bool __nf_flow_exceeds_mtu(const struct sk_buff *skb, unsigned int mtu)
 	if ((ip_hdr(skb)->frag_off & htons(IP_DF)) == 0)
 		return false;
 
-	if (skb_is_gso(skb) && skb_gso_validate_mtu(skb, mtu))
+	if (skb_is_gso(skb) && skb_gso_validate_network_len(skb, mtu))
 		return false;
 
 	return true;
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 18547a44bdaf..4e888328d4dd 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -412,7 +412,7 @@ static bool ip6_pkt_too_big(const struct sk_buff *skb, unsigned int mtu)
 	if (skb->ignore_df)
 		return false;
 
-	if (skb_is_gso(skb) && skb_gso_validate_mtu(skb, mtu))
+	if (skb_is_gso(skb) && skb_gso_validate_network_len(skb, mtu))
 		return false;
 
 	return true;
diff --git a/net/ipv6/netfilter/nf_flow_table_ipv6.c b/net/ipv6/netfilter/nf_flow_table_ipv6.c
index 0c3b9d32f64f..f1ab4e03df7d 100644
--- a/net/ipv6/netfilter/nf_flow_table_ipv6.c
+++ b/net/ipv6/netfilter/nf_flow_table_ipv6.c
@@ -179,7 +179,7 @@ static bool __nf_flow_exceeds_mtu(const struct sk_buff *skb, unsigned int mtu)
 	if (skb->len <= mtu)
 		return false;
 
-	if (skb_is_gso(skb) && skb_gso_validate_mtu(skb, mtu))
+	if (skb_is_gso(skb) && skb_gso_validate_network_len(skb, mtu))
 		return false;
 
 	return true;
diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c
index 5dce8336d33f..ec2a62c1d296 100644
--- a/net/mpls/af_mpls.c
+++ b/net/mpls/af_mpls.c
@@ -121,7 +121,7 @@ bool mpls_pkt_too_big(const struct sk_buff *skb, unsigned int mtu)
 	if (skb->len <= mtu)
 		return false;
 
-	if (skb_is_gso(skb) && skb_gso_validate_mtu(skb, mtu))
+	if (skb_is_gso(skb) && skb_gso_validate_network_len(skb, mtu))
 		return false;
 
 	return true;
diff --git a/net/xfrm/xfrm_device.c b/net/xfrm/xfrm_device.c
index 75982506617b..a6dd9cb17d1d 100644
--- a/net/xfrm/xfrm_device.c
+++ b/net/xfrm/xfrm_device.c
@@ -209,7 +209,7 @@ bool xfrm_dev_offload_ok(struct sk_buff *skb, struct xfrm_state *x)
 		if (skb->len <= mtu)
 			goto ok;
 
-		if (skb_is_gso(skb) && skb_gso_validate_mtu(skb, mtu))
+		if (skb_is_gso(skb) && skb_gso_validate_network_len(skb, mtu))
 			goto ok;
 	}
 
-- 
2.14.1

[PATCH v2 2/4] net: move skb_gso_mac_seglen to skbuff.h

[Daniel Axtens]

We're about to use this elsewhere, so move it into the header with
the other related functions like skb_gso_network_seglen().

Signed-off-by: Daniel Axtens <dja@axtens.net>
---
 include/linux/skbuff.h | 15 +++++++++++++++
 net/sched/sch_tbf.c    | 10 ----------
 2 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index b137c79bf88d..4b3ca6a5ec0a 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -4120,6 +4120,21 @@ static inline unsigned int skb_gso_network_seglen(const struct sk_buff *skb)
 	return hdr_len + skb_gso_transport_seglen(skb);
 }
 
+/**
+ * skb_gso_mac_seglen - Return length of individual segments of a gso packet
+ *
+ * @skb: GSO skb
+ *
+ * skb_gso_mac_seglen is used to determine the real size of the
+ * individual segments, including MAC/L2, Layer3 (IP, IPv6) and L4
+ * headers (TCP/UDP).
+ */
+static inline unsigned int skb_gso_mac_seglen(const struct sk_buff *skb)
+{
+	unsigned int hdr_len = skb_transport_header(skb) - skb_mac_header(skb);
+	return hdr_len + skb_gso_transport_seglen(skb);
+}
+
 /* Local Checksum Offload.
  * Compute outer checksum based on the assumption that the
  * inner checksum will be offloaded later.
diff --git a/net/sched/sch_tbf.c b/net/sched/sch_tbf.c
index 83e76d046993..229172d509cc 100644
--- a/net/sched/sch_tbf.c
+++ b/net/sched/sch_tbf.c
@@ -142,16 +142,6 @@ static u64 psched_ns_t2l(const struct psched_ratecfg *r,
 	return len;
 }
 
-/*
- * Return length of individual segments of a gso packet,
- * including all headers (MAC, IP, TCP/UDP)
- */
-static unsigned int skb_gso_mac_seglen(const struct sk_buff *skb)
-{
-	unsigned int hdr_len = skb_transport_header(skb) - skb_mac_header(skb);
-	return hdr_len + skb_gso_transport_seglen(skb);
-}
-
 /* GSO packet is too big, segment it so that tbf can transmit
  * each segment in time
  */
-- 
2.14.1

[PATCH v2 3/4] net: is_skb_forwardable: check the size of GSO segments

[Daniel Axtens]

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 skbs, and it is possible that a GSO skb, when
segmented, will be larger than the device can transmit.

Create skb_gso_validate_mac_len, and use that to check.

Signed-off-by: Daniel Axtens <dja@axtens.net>
---
 include/linux/skbuff.h |  1 +
 net/core/dev.c         |  7 +++---
 net/core/skbuff.c      | 67 +++++++++++++++++++++++++++++++++++++++-----------
 3 files changed, 57 insertions(+), 18 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 4b3ca6a5ec0a..ec9c47b5a1c8 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_network_len(const struct sk_buff *skb, unsigned int mtu);
+bool skb_gso_validate_mac_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..6c96c26aadbf 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_mac_len(skb, len);
 
 	return false;
 }
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index a93e5c7aa5b2..93f66725c32d 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -4914,37 +4914,74 @@ unsigned int skb_gso_transport_seglen(const struct sk_buff *skb)
 EXPORT_SYMBOL_GPL(skb_gso_transport_seglen);
 
 /**
- * skb_gso_validate_network_len - Will a split GSO skb fit into a given MTU?
+ * skb_gso_size_check - check the skb size, considering GSO_BY_FRAGS
  *
- * @skb: GSO skb
- * @mtu: MTU to validate against
+ * There are a couple of instances where we have a GSO skb, and we
+ * want to determine what size it would be after it is segmented.
  *
- * skb_gso_validate_network_len validates if a given skb will fit a
- * wanted MTU once split. It considers L3 headers, L4 headers, and the
- * payload.
+ * We might want to check:
+ * -    L3+L4+payload size (e.g. IP forwarding)
+ * - L2+L3+L4+payload size (e.g. sanity check before passing to driver)
+ *
+ * This is a helper to do that correctly considering GSO_BY_FRAGS.
+ *
+ * @seg_len: The segmented length (from skb_gso_*_seglen). In the
+ *           GSO_BY_FRAGS case this will be [header sizes + GSO_BY_FRAGS].
+ *
+ * @max_len: The maximum permissible length.
+ *
+ * Returns true if the segmented length <= max length.
  */
-bool skb_gso_validate_network_len(const struct sk_buff *skb, unsigned int mtu)
-{
+static inline bool skb_gso_size_check(const struct sk_buff *skb,
+				      unsigned int seg_len,
+				      unsigned int max_len) {
 	const struct skb_shared_info *shinfo = skb_shinfo(skb);
 	const struct sk_buff *iter;
-	unsigned int hlen;
-
-	hlen = skb_gso_network_seglen(skb);
 
 	if (shinfo->gso_size != GSO_BY_FRAGS)
-		return hlen <= mtu;
+		return seg_len <= max_len;
 
 	/* Undo this so we can re-use header sizes */
-	hlen -= GSO_BY_FRAGS;
+	seg_len -= GSO_BY_FRAGS;
 
 	skb_walk_frags(skb, iter) {
-		if (hlen + skb_headlen(iter) > mtu)
+		if (seg_len + skb_headlen(iter) > max_len)
 			return false;
 	}
 
 	return true;
 }
-EXPORT_SYMBOL_GPL(skb_gso_validate_mtu);
+
+/**
+ * skb_gso_validate_network_len - Does an skb fit a given MTU?
+ *
+ * @skb: GSO skb
+ * @mtu: MTU to validate against
+ *
+ * skb_gso_validate_network_len validates if a given skb will fit a
+ * wanted MTU once split. It considers L3 headers, L4 headers, and the
+ * payload.
+ */
+bool skb_gso_validate_network_len(const struct sk_buff *skb, unsigned int mtu)
+{
+	return skb_gso_size_check(skb, skb_gso_network_seglen(skb), mtu);
+}
+EXPORT_SYMBOL_GPL(skb_gso_validate_network_len);
+
+/**
+ * skb_gso_validate_mac_len - Will a split GSO skb fit in a given length?
+ *
+ * @skb: GSO skb
+ * @len: length to validate against
+ *
+ * skb_gso_validate_mac_len validates if a given skb will fit a wanted
+ * length once split, including L2, L3 and L4 headers and the payload.
+ */
+bool skb_gso_validate_mac_len(const struct sk_buff *skb, unsigned int len)
+{
+	return skb_gso_size_check(skb, skb_gso_mac_seglen(skb), len);
+}
+EXPORT_SYMBOL_GPL(skb_gso_validate_mac_len);
 
 static struct sk_buff *skb_reorder_vlan_header(struct sk_buff *skb)
 {
-- 
2.14.1

[PATCH v2 4/4] net: check the size of a packet in validate_xmit_skb

[Daniel Axtens]

There are a number of paths where an oversize skb could be sent to
a driver. The driver should not be required to check for this - the
core layer should do it instead.

Add a check to validate_xmit_skb that checks both GSO and non-GSO
packets and drops them if they are too large.

Signed-off-by: Daniel Axtens <dja@axtens.net>
---
 net/core/dev.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 6c96c26aadbf..f09eece2cd21 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1830,13 +1830,11 @@ static inline void net_timestamp_set(struct sk_buff *skb)
 			__net_timestamp(SKB);		\
 	}						\
 
-bool is_skb_forwardable(const struct net_device *dev, const struct sk_buff *skb)
+static inline bool skb_mac_len_fits_dev(const struct net_device *dev,
+					const struct sk_buff *skb)
 {
 	unsigned int len;
 
-	if (!(dev->flags & IFF_UP))
-		return false;
-
 	len = dev->mtu + dev->hard_header_len + VLAN_HLEN;
 	if (skb->len <= len)
 		return true;
@@ -1850,6 +1848,14 @@ bool is_skb_forwardable(const struct net_device *dev, const struct sk_buff *skb)
 
 	return false;
 }
+
+bool is_skb_forwardable(const struct net_device *dev, const struct sk_buff *skb)
+{
+	if (!(dev->flags & IFF_UP))
+		return false;
+
+	return skb_mac_len_fits_dev(dev, skb);
+}
 EXPORT_SYMBOL_GPL(is_skb_forwardable);
 
 int __dev_forward_skb(struct net_device *dev, struct sk_buff *skb)
@@ -3081,6 +3087,9 @@ static struct sk_buff *validate_xmit_skb(struct sk_buff *skb, struct net_device
 	if (unlikely(!skb))
 		goto out_null;
 
+	if (unlikely(!skb_mac_len_fits_dev(dev, skb)))
+		goto out_kfree_skb;
+
 	if (netif_needs_gso(skb, features)) {
 		struct sk_buff *segs;
 
-- 
2.14.1

Re: [PATCH v2 0/4] Check size of packets before sending

[Eric Dumazet]

On Thu, 2018-01-25 at 15:31 +1100, Daniel Axtens wrote:
> There are a few ways we can send packets that are too large to a
> network driver.
> 
> When non-GSO packets are forwarded, we validate their size, based on
> the MTU of the destination device. However, when GSO packets are
> forwarded, we do not validate their size. We implicitly assume that
> when they are segmented, the resultant packets will be correctly
> sized.
> 
> This is not always the case.
> 
> We observed a case where a packet received on an ibmveth device had a
> GSO size of around 10kB. This was forwarded by Open vSwitch to a bnx2x
> device, where it caused a firmware assert. This is described in detail
> at [0] and was the genesis of this series.
> 
> Rather than fixing this in the driver, this series fixes the
> core path. It does it in 2 steps:
> 
>  1) make is_skb_forwardable check GSO packets - this catches bridges
>  
>  2) make validate_xmit_skb check the size of all packets, so as to
>     catch everything else (e.g. macvlan, tc mired, OVS)
> 
> I am a bit nervous about how this series will interact with nested
> VLANs, as the existing code only allows for one VLAN_HLEN. (Previously
> these packets would sail past unchecked.) But I thought it would be
> prudent to get more eyes on this sooner rather than later.
> 
> Thanks,
> Daniel
> 
> v1: https://www.spinics.net/lists/netdev/msg478634.html
> Changes in v2:
> 
>  - improve names, thanks Marcelo Ricardo Leitner
> 
>  - add check to xmit_validate_skb; thanks to everyone who participated
>    in the discussion.
> 
>  - drop extra check in Open vSwitch. Bad packets will be caught by
>    validate_xmit_skb for now and we can come back and add it later if
>    OVS people would like the extra logging.
>    
> [0]: https://patchwork.ozlabs.org/patch/859410/
> 
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: Pravin Shelar <pshelar@ovn.org>
> Cc: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> Cc: Manish.Chopra@cavium.com
> Cc: dev@openvswitch.org
> 
> Daniel Axtens (4):
>   net: rename skb_gso_validate_mtu -> skb_gso_validate_network_len
>   net: move skb_gso_mac_seglen to skbuff.h
>   net: is_skb_forwardable: check the size of GSO segments
>   net: check the size of a packet in validate_xmit_skb
> 
>  include/linux/skbuff.h                  | 18 ++++++++-
>  net/core/dev.c                          | 24 ++++++++----
>  net/core/skbuff.c                       | 66 ++++++++++++++++++++++++++-------
>  net/ipv4/ip_forward.c                   |  2 +-
>  net/ipv4/ip_output.c                    |  2 +-
>  net/ipv4/netfilter/nf_flow_table_ipv4.c |  2 +-
>  net/ipv6/ip6_output.c                   |  2 +-
>  net/ipv6/netfilter/nf_flow_table_ipv6.c |  2 +-
>  net/mpls/af_mpls.c                      |  2 +-
>  net/sched/sch_tbf.c                     | 10 -----
>  net/xfrm/xfrm_device.c                  |  2 +-
>  11 files changed, 93 insertions(+), 39 deletions(-)
> 

May I ask which tree are you targeting ?

( Documentation/networking/netdev-FAQ.txt )

Anything touching GSO is very risky and should target net-next,
especially considering 4.15 is released this week end.

Are we really willing to backport this intrusive series in stable
trees, or do we have a smaller fix for bnx2x ?

Thanks.

Re: [PATCH v2 0/4] Check size of packets before sending

[Daniel Axtens]

Hi Eric,

> May I ask which tree are you targeting ?
>
> ( Documentation/networking/netdev-FAQ.txt )

I have been targeting net-next, but I haven't pulled for about two
weeks. I will rebase and if there are conflicts I will resend early next
week.

> Anything touching GSO is very risky and should target net-next,
> especially considering 4.15 is released this week end.
>
> Are we really willing to backport this intrusive series in stable
> trees, or do we have a smaller fix for bnx2x ?

I do actually have a smaller fix for bnx2x, although it would need more work:
https://patchwork.ozlabs.org/patch/859410/

It leaves open the possibility of too-large packets causing issues on
other drivers. DaveM wasn't a fan: https://patchwork.ozlabs.org/patch/859410/#1839429

Regards,
Daniel
>
> Thanks.

Re: [PATCH v2 0/4] Check size of packets before sending

[Eric Dumazet]

On Fri, 2018-01-26 at 00:44 +1100, Daniel Axtens wrote:
> Hi Eric,
> 
> > May I ask which tree are you targeting ?
> > 
> > ( Documentation/networking/netdev-FAQ.txt )
> 
> I have been targeting net-next, but I haven't pulled for about two
> weeks. I will rebase and if there are conflicts I will resend early next
> week.
> 
> > Anything touching GSO is very risky and should target net-next,
> > especially considering 4.15 is released this week end.
> > 
> > Are we really willing to backport this intrusive series in stable
> > trees, or do we have a smaller fix for bnx2x ?
> 
> I do actually have a smaller fix for bnx2x, although it would need more work:
> https://patchwork.ozlabs.org/patch/859410/
> 
> It leaves open the possibility of too-large packets causing issues on
> other drivers. DaveM wasn't a fan: https://patchwork.ozlabs.org/patch/859410/#1839429

Yes, I know he prefers a generic solution, but I am pragmatic here.
Old kernels are very far from current GSO stack in net-next.

Backporting all the dependencies is going to be very boring/risky.

Re: [PATCH v2 0/4] Check size of packets before sending

[Marcelo Ricardo Leitner]

On Thu, Jan 25, 2018 at 03:31:05PM +1100, Daniel Axtens wrote:
> There are a few ways we can send packets that are too large to a
> network driver.
> 
> When non-GSO packets are forwarded, we validate their size, based on
> the MTU of the destination device. However, when GSO packets are
> forwarded, we do not validate their size. We implicitly assume that
> when they are segmented, the resultant packets will be correctly
> sized.
...

Patchset LGTM, but I also think it's risky merging this one in by now.

  Marcelo

Re: [PATCH v2 0/4] Check size of packets before sending

[Daniel Axtens]

Eric Dumazet <eric.dumazet@gmail.com> writes:

> On Fri, 2018-01-26 at 00:44 +1100, Daniel Axtens wrote:
>> Hi Eric,
>> 
>> > May I ask which tree are you targeting ?
>> > 
>> > ( Documentation/networking/netdev-FAQ.txt )
>> 
>> I have been targeting net-next, but I haven't pulled for about two
>> weeks. I will rebase and if there are conflicts I will resend early next
>> week.
>> 
>> > Anything touching GSO is very risky and should target net-next,
>> > especially considering 4.15 is released this week end.
>> > 
>> > Are we really willing to backport this intrusive series in stable
>> > trees, or do we have a smaller fix for bnx2x ?
>> 
>> I do actually have a smaller fix for bnx2x, although it would need more work:
>> https://patchwork.ozlabs.org/patch/859410/
>> 
>> It leaves open the possibility of too-large packets causing issues on
>> other drivers. DaveM wasn't a fan: https://patchwork.ozlabs.org/patch/859410/#1839429
>
> Yes, I know he prefers a generic solution, but I am pragmatic here.
> Old kernels are very far from current GSO stack in net-next.
>
> Backporting all the dependencies is going to be very boring/risky.

OK, so how about:

 - first, a series that introduces skb_gso_validate_mac_len and uses it
   in bnx2x. This should be backportable without difficulty.

 - then, a series that wires skb_gso_validate_mac_len into the core -
   validate_xmit_skb for instance, and reverts the bnx2x fix. This would
   not need to be backported.

If that's an approach we can agree on, I am ready to send it when
net-next opens again.

Regards,
Daniel

Re: [PATCH v2 0/4] Check size of packets before sending

[David Miller]

From: Daniel Axtens <dja@axtens.net>
Date: Mon, 29 Jan 2018 14:20:58 +1100

> OK, so how about:
> 
>  - first, a series that introduces skb_gso_validate_mac_len and uses it
>    in bnx2x. This should be backportable without difficulty.
> 
>  - then, a series that wires skb_gso_validate_mac_len into the core -
>    validate_xmit_skb for instance, and reverts the bnx2x fix. This would
>    not need to be backported.
> 
> If that's an approach we can agree on, I am ready to send it when
> net-next opens again.

Please send the bnx2x specific fix now, thank you.