[PATCH 0/2] Fix handling of GRO skbs in forwarding path

[PATCH 0/2] Fix handling of GRO skbs in forwarding path

[Florian Westphal]

Marcelo Ricardo Leitner reported problems when the forwarding link
path has a lower mtu than the incoming link if the inbound interface
supports GRO.

Currently GSO/GRO skbs bypass all dst MTU checks, i.e. forwarding of
such skbs fails in case the outgoing link mtu is smaller than the one
of the incoming interface: We neither generate an icmp error nor will
the packet be fragmented if ipv4 would permit it.

The first patch moves part of Eric Dumazets skb_gso_seglen helper from
sch_tbf to skbuff core for re-use in forwarding path.

The 2nd change then alters forwarding path to handle GRO skbs.

It is not 100% correct, since the icmp error will contain the headers
of the GRO skb instead of the original/segmented one, but it seems to
work fine in my (limited) tests.

Software segmentation is done for ipv4 if the DF bit is not set.

If you think this is -next material just set patchwork state to
"deferred", I'll resend then once -next is open again.

 include/linux/skbuff.h |   18 +++++++++++++++
 net/core/skbuff.c      |   26 +++++++++++++++++++++
 net/ipv4/ip_forward.c  |   58 +++++++++++++++++++++++++++++++++++++++++++++++--
 net/ipv6/ip6_output.c  |   18 +++++++++++++--
 net/sched/sch_tbf.c    |   12 ++--------
 5 files changed, 119 insertions(+), 13 deletions(-)

[PATCH 1/2] net: add and use skb_gso_transport_seglen()

[Florian Westphal]

This moves part of Eric Dumazets skb_gso_seglen helper from tbf sched to
skbuff core so it may be reused by upcoming ip forwarding path patch.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/linux/skbuff.h |  1 +
 net/core/skbuff.c      | 26 ++++++++++++++++++++++++++
 net/sched/sch_tbf.c    | 12 +++---------
 3 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 6f69b3f..3d76066 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2371,6 +2371,7 @@ void skb_copy_and_csum_dev(const struct sk_buff *skb, u8 *to);
 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);
 struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features);
 
 struct skb_checksum_ops {
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 06e72d3..2e1fd75 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -45,6 +45,7 @@
 #include <linux/mm.h>
 #include <linux/interrupt.h>
 #include <linux/in.h>
+#include <linux/tcp.h>
 #include <linux/inet.h>
 #include <linux/slab.h>
 #include <linux/netdevice.h>
@@ -71,6 +72,8 @@
 #include <trace/events/skb.h>
 #include <linux/highmem.h>
 
+#include <uapi/linux/udp.h>
+
 struct kmem_cache *skbuff_head_cache __read_mostly;
 static struct kmem_cache *skbuff_fclone_cache __read_mostly;
 
@@ -3592,3 +3595,26 @@ void skb_scrub_packet(struct sk_buff *skb, bool xnet)
 	nf_reset_trace(skb);
 }
 EXPORT_SYMBOL_GPL(skb_scrub_packet);
+
+/**
+ * skb_gso_transport_seglen - Return length of individual segments of a gso packet
+ *
+ * @skb: GSO skb
+ *
+ * skb_gso_transport_seglen is used to determine the real size of the
+ * individual segments, including Layer4 headers (TCP/UDP).
+ *
+ * The MAC/L2 or network (IP, IPv6) headers are not accounted for.
+ */
+unsigned int skb_gso_transport_seglen(const struct sk_buff *skb)
+{
+	const struct skb_shared_info *shinfo = skb_shinfo(skb);
+	unsigned int hdr_len;
+
+	if (likely(shinfo->gso_type & (SKB_GSO_TCPV4 | SKB_GSO_TCPV6)))
+		hdr_len = tcp_hdrlen(skb);
+	else
+		hdr_len = sizeof(struct udphdr);
+	return hdr_len + shinfo->gso_size;
+}
+EXPORT_SYMBOL_GPL(skb_gso_transport_seglen);
diff --git a/net/sched/sch_tbf.c b/net/sched/sch_tbf.c
index 887e672..837a61b 100644
--- a/net/sched/sch_tbf.c
+++ b/net/sched/sch_tbf.c
@@ -148,16 +148,10 @@ static u64 psched_ns_t2l(const struct psched_ratecfg *r,
  * Return length of individual segments of a gso packet,
  * including all headers (MAC, IP, TCP/UDP)
  */
-static unsigned int skb_gso_seglen(const struct sk_buff *skb)
+static unsigned int skb_gso_mac_seglen(const struct sk_buff *skb)
 {
 	unsigned int hdr_len = skb_transport_header(skb) - skb_mac_header(skb);
-	const struct skb_shared_info *shinfo = skb_shinfo(skb);
-
-	if (likely(shinfo->gso_type & (SKB_GSO_TCPV4 | SKB_GSO_TCPV6)))
-		hdr_len += tcp_hdrlen(skb);
-	else
-		hdr_len += sizeof(struct udphdr);
-	return hdr_len + shinfo->gso_size;
+	return hdr_len + skb_gso_transport_seglen(skb);
 }
 
 /* GSO packet is too big, segment it so that tbf can transmit
@@ -202,7 +196,7 @@ static int tbf_enqueue(struct sk_buff *skb, struct Qdisc *sch)
 	int ret;
 
 	if (qdisc_pkt_len(skb) > q->max_size) {
-		if (skb_is_gso(skb) && skb_gso_seglen(skb) <= q->max_size)
+		if (skb_is_gso(skb) && skb_gso_mac_seglen(skb) <= q->max_size)
 			return tbf_segment(skb, sch);
 		return qdisc_reshape_fail(skb, sch);
 	}
-- 
1.8.1.5

Re: [PATCH 1/2] net: add and use skb_gso_transport_seglen()

[Eric Dumazet]

On Sat, 2014-01-25 at 23:48 +0100, Florian Westphal wrote:
> This moves part of Eric Dumazets skb_gso_seglen helper from tbf sched to
> skbuff core so it may be reused by upcoming ip forwarding path patch.
> 
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
>  include/linux/skbuff.h |  1 +
>  net/core/skbuff.c      | 26 ++++++++++++++++++++++++++
>  net/sched/sch_tbf.c    | 12 +++---------
>  3 files changed, 30 insertions(+), 9 deletions(-)
> 
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 6f69b3f..3d76066 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -2371,6 +2371,7 @@ void skb_copy_and_csum_dev(const struct sk_buff *skb, u8 *to);
>  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);
>  struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features);
>  
>  struct skb_checksum_ops {
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 06e72d3..2e1fd75 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -45,6 +45,7 @@
>  #include <linux/mm.h>
>  #include <linux/interrupt.h>
>  #include <linux/in.h>
> +#include <linux/tcp.h>
>  #include <linux/inet.h>
>  #include <linux/slab.h>
>  #include <linux/netdevice.h>
> @@ -71,6 +72,8 @@
>  #include <trace/events/skb.h>
>  #include <linux/highmem.h>
>  
> +#include <uapi/linux/udp.h>


Normally you should not use uapi/

#include <linux/udp.h>

> +
>  struct kmem_cache *skbuff_head_cache __read_mostly;
>  static struct kmem_cache *skbuff_fclone_cache __read_mostly;
>  
> @@ -3592,3 +3595,26 @@ void skb_scrub_packet(struct sk_buff *skb, bool xnet)
>  	nf_reset_trace(skb);
>  }
>  EXPORT_SYMBOL_GPL(skb_scrub_packet);
> +
> +/**
> + * skb_gso_transport_seglen - Return length of individual segments of a gso packet
> + *
> + * @skb: GSO skb
> + *
> + * skb_gso_transport_seglen is used to determine the real size of the
> + * individual segments, including Layer4 headers (TCP/UDP).
> + *
> + * The MAC/L2 or network (IP, IPv6) headers are not accounted for.
> + */
> +unsigned int skb_gso_transport_seglen(const struct sk_buff *skb)
> +{
> +	const struct skb_shared_info *shinfo = skb_shinfo(skb);
> +	unsigned int hdr_len;
> +
> +	if (likely(shinfo->gso_type & (SKB_GSO_TCPV4 | SKB_GSO_TCPV6)))
> +		hdr_len = tcp_hdrlen(skb);
> +	else
> +		hdr_len = sizeof(struct udphdr);
> +	return hdr_len + shinfo->gso_size;
> +}
> +EXPORT_SYMBOL_GPL(skb_gso_transport_seglen);
> diff --git a/net/sched/sch_tbf.c b/net/sched/sch_tbf.c
> index 887e672..837a61b 100644
> --- a/net/sched/sch_tbf.c
> +++ b/net/sched/sch_tbf.c

It seems you forgot to remove from this file this include :

#include <net/tcp.h>

> @@ -148,16 +148,10 @@ static u64 psched_ns_t2l(const struct psched_ratecfg *r,
>   * Return length of individual segments of a gso packet,
>   * including all headers (MAC, IP, TCP/UDP)
>   */
> -static unsigned int skb_gso_seglen(const struct sk_buff *skb)
> +static unsigned int skb_gso_mac_seglen(const struct sk_buff *skb)
>  {
>  	unsigned int hdr_len = skb_transport_header(skb) - skb_mac_header(skb);
> -	const struct skb_shared_info *shinfo = skb_shinfo(skb);
> -
> -	if (likely(shinfo->gso_type & (SKB_GSO_TCPV4 | SKB_GSO_TCPV6)))
> -		hdr_len += tcp_hdrlen(skb);
> -	else
> -		hdr_len += sizeof(struct udphdr);
> -	return hdr_len + shinfo->gso_size;
> +	return hdr_len + skb_gso_transport_seglen(skb);
>  }
>  
>  /* GSO packet is too big, segment it so that tbf can transmit
> @@ -202,7 +196,7 @@ static int tbf_enqueue(struct sk_buff *skb, struct Qdisc *sch)
>  	int ret;
>  
>  	if (qdisc_pkt_len(skb) > q->max_size) {
> -		if (skb_is_gso(skb) && skb_gso_seglen(skb) <= q->max_size)
> +		if (skb_is_gso(skb) && skb_gso_mac_seglen(skb) <= q->max_size)
>  			return tbf_segment(skb, sch);
>  		return qdisc_reshape_fail(skb, sch);
>  	}


Otherwise, this seems good, thanks !

Re: [PATCH 1/2] net: add and use skb_gso_transport_seglen()

[Florian Westphal]

Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Sat, 2014-01-25 at 23:48 +0100, Florian Westphal wrote:
> > --- a/net/core/skbuff.c
> > +++ b/net/core/skbuff.c
> > @@ -45,6 +45,7 @@
> >  #include <linux/mm.h>
> >  #include <linux/interrupt.h>
> >  #include <linux/in.h>
> > +#include <linux/tcp.h>
> >  #include <linux/inet.h>
> >  #include <linux/slab.h>
> >  #include <linux/netdevice.h>
> > @@ -71,6 +72,8 @@
> >  #include <trace/events/skb.h>
> >  #include <linux/highmem.h>
> >  
> > +#include <uapi/linux/udp.h>
> 
> 
> Normally you should not use uapi/

I added this include to ensure sizeof(struct udphdr) works.
I'll change it to linux/udp.h

> > diff --git a/net/sched/sch_tbf.c b/net/sched/sch_tbf.c
> > index 887e672..837a61b 100644
> > --- a/net/sched/sch_tbf.c
> > +++ b/net/sched/sch_tbf.c
> 
> It seems you forgot to remove from this file this include :
> 
> #include <net/tcp.h>

Indeed, thanks for spotting this.

> Otherwise, this seems good, thanks !

Thanks for reviewing Eric!

[PATCH 2/2] net: ip, ipv6: handle gso skbs in forwarding path

[Florian Westphal]

Given:
Host <mtu1500> R1 <mtu1200> R2

Host sends tcp stream which is routed via R1 and R2.
R1 nics perform GRO.

In this case, the kernel will fail to send ICMP fragmentation needed
messages (or pkt too big for ipv6), as gso packets currently bypass the
dst mtu checks in forward path. Instead, Linux tries to send out packets
exceeding R1-R2 link mtu.

When locking route MTU on Host (i.e., no ipv4 DF bit set), R1 does
not fragment the packets when forwarding, and again tries to send out
packets exceeding R1-R2 link mtu.

This alters the forwarding dstmtu checks to take the individual gso
segment lengths into account.

For ipv6, we send out pkt too big error for gso if the individual
segments are too big.

For ipv4, we either send icmp fragmentation needed, or, if the DF bit
is not set, perform software segmentation and let the output path
create fragments when the packet is leaving the machine.  This is
technically not 100% accurate, as the error message will contain the
gro-skb tcp header (we'd have to segment unconditionally and use first
segment instead), but it does seem to work.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/linux/skbuff.h | 17 +++++++++++++++
 net/ipv4/ip_forward.c  | 58 ++++++++++++++++++++++++++++++++++++++++++++++++--
 net/ipv6/ip6_output.c  | 18 ++++++++++++++--
 3 files changed, 89 insertions(+), 4 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 3d76066..37cb679 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2811,5 +2811,22 @@ static inline bool skb_head_is_locked(const struct sk_buff *skb)
 {
 	return !skb->head_frag || skb_cloned(skb);
 }
+
+/**
+ * skb_gso_network_seglen - Return length of individual segments of a gso packet
+ *
+ * @skb: GSO skb
+ *
+ * skb_gso_network_seglen is used to determine the real size of the
+ * individual segments, including Layer3 (IP, IPv6) and L4 headers (TCP/UDP).
+ *
+ * The MAC/L2 header is not accounted for.
+ */
+static inline unsigned int skb_gso_network_seglen(const struct sk_buff *skb)
+{
+	unsigned int hdr_len = skb_transport_header(skb) -
+			       skb_network_header(skb);
+	return hdr_len + skb_gso_transport_seglen(skb);
+}
 #endif	/* __KERNEL__ */
 #endif	/* _LINUX_SKBUFF_H */
diff --git a/net/ipv4/ip_forward.c b/net/ipv4/ip_forward.c
index 694de3b..78f2e2a 100644
--- a/net/ipv4/ip_forward.c
+++ b/net/ipv4/ip_forward.c
@@ -39,6 +39,58 @@
 #include <net/route.h>
 #include <net/xfrm.h>
 
+static bool ip_may_fragment(const struct sk_buff *skb)
+{
+	return unlikely((ip_hdr(skb)->frag_off & htons(IP_DF)) == 0) ||
+	       !skb->local_df;
+}
+
+static bool ip_gso_exceeds_dst_mtu(const struct sk_buff *skb)
+{
+	if (skb->local_df || !skb_is_gso(skb))
+		return false;
+	return skb_gso_network_seglen(skb) > dst_mtu(skb_dst(skb));
+}
+
+static bool ip_exceeds_mtu(const struct sk_buff *skb, unsigned int mtu)
+{
+	unsigned len;
+
+	if (skb->local_df)
+		return false;
+	len = skb_is_gso(skb) ? skb_gso_network_seglen(skb) : skb->len;
+
+	return len > mtu;
+}
+
+/* called if GSO skb needs to be fragmented on forward.  */
+static int ip_forward_finish_gso(struct sk_buff *skb)
+{
+	struct sk_buff *segs = skb_gso_segment(skb, 0);
+	int ret = 0;
+
+	if (IS_ERR(segs)) {
+		kfree_skb(skb);
+		return -ENOMEM;
+	}
+
+	consume_skb(skb);
+
+	do {
+		struct sk_buff *nskb = segs->next;
+		int err;
+
+		segs->next = NULL;
+		err = dst_output(segs);
+
+		if (err && ret == 0)
+			ret = err;
+		segs = nskb;
+	} while (segs);
+
+	return ret;
+}
+
 static int ip_forward_finish(struct sk_buff *skb)
 {
 	struct ip_options *opt	= &(IPCB(skb)->opt);
@@ -49,6 +101,9 @@ static int ip_forward_finish(struct sk_buff *skb)
 	if (unlikely(opt->optlen))
 		ip_forward_options(skb);
 
+	if (ip_gso_exceeds_dst_mtu(skb))
+		return ip_forward_finish_gso(skb);
+
 	return dst_output(skb);
 }
 
@@ -88,8 +143,7 @@ int ip_forward(struct sk_buff *skb)
 	if (opt->is_strictroute && rt->rt_uses_gateway)
 		goto sr_failed;
 
-	if (unlikely(skb->len > dst_mtu(&rt->dst) && !skb_is_gso(skb) &&
-		     (ip_hdr(skb)->frag_off & htons(IP_DF))) && !skb->local_df) {
+	if (!ip_may_fragment(skb) && ip_exceeds_mtu(skb, dst_mtu(&rt->dst))) {
 		IP_INC_STATS(dev_net(rt->dst.dev), IPSTATS_MIB_FRAGFAILS);
 		icmp_send(skb, ICMP_DEST_UNREACH, ICMP_FRAG_NEEDED,
 			  htonl(dst_mtu(&rt->dst)));
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index e6f9319..e1a5aec 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -321,6 +321,21 @@ static inline int ip6_forward_finish(struct sk_buff *skb)
 	return dst_output(skb);
 }
 
+static bool ip6_pkt_too_big(const struct sk_buff *skb, unsigned int mtu)
+{
+	unsigned int len;
+
+	if (skb->local_df)
+		return false;
+
+	if (IP6CB(skb)->frag_max_size && IP6CB(skb)->frag_max_size > mtu)
+		return true;
+
+	len = skb_is_gso(skb) ? skb_gso_network_seglen(skb) : skb->len;
+
+	return len > mtu;
+}
+
 int ip6_forward(struct sk_buff *skb)
 {
 	struct dst_entry *dst = skb_dst(skb);
@@ -443,8 +458,7 @@ int ip6_forward(struct sk_buff *skb)
 	if (mtu < IPV6_MIN_MTU)
 		mtu = IPV6_MIN_MTU;
 
-	if ((!skb->local_df && skb->len > mtu && !skb_is_gso(skb)) ||
-	    (IP6CB(skb)->frag_max_size && IP6CB(skb)->frag_max_size > mtu)) {
+	if (ip6_pkt_too_big(skb, mtu)) {
 		/* Again, force OUTPUT device used as source address */
 		skb->dev = dst->dev;
 		icmpv6_send(skb, ICMPV6_PKT_TOOBIG, 0, mtu);
-- 
1.8.1.5

Re: [PATCH 2/2] net: ip, ipv6: handle gso skbs in forwarding path

[Eric Dumazet]

On Sat, 2014-01-25 at 23:48 +0100, Florian Westphal wrote:
> Given:
> Host <mtu1500> R1 <mtu1200> R2
> 
> Host sends tcp stream which is routed via R1 and R2.
> R1 nics perform GRO.
> 
> In this case, the kernel will fail to send ICMP fragmentation needed
> messages (or pkt too big for ipv6), as gso packets currently bypass the
> dst mtu checks in forward path. Instead, Linux tries to send out packets
> exceeding R1-R2 link mtu.
> 
> When locking route MTU on Host (i.e., no ipv4 DF bit set), R1 does
> not fragment the packets when forwarding, and again tries to send out
> packets exceeding R1-R2 link mtu.
> 
> This alters the forwarding dstmtu checks to take the individual gso
> segment lengths into account.
> 
> For ipv6, we send out pkt too big error for gso if the individual
> segments are too big.
> 
> For ipv4, we either send icmp fragmentation needed, or, if the DF bit
> is not set, perform software segmentation and let the output path
> create fragments when the packet is leaving the machine.  This is
> technically not 100% accurate, as the error message will contain the
> gro-skb tcp header (we'd have to segment unconditionally and use first
> segment instead), but it does seem to work.
> 
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
>  include/linux/skbuff.h | 17 +++++++++++++++
>  net/ipv4/ip_forward.c  | 58 ++++++++++++++++++++++++++++++++++++++++++++++++--
>  net/ipv6/ip6_output.c  | 18 ++++++++++++++--
>  3 files changed, 89 insertions(+), 4 deletions(-)
> 
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 3d76066..37cb679 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -2811,5 +2811,22 @@ static inline bool skb_head_is_locked(const struct sk_buff *skb)
>  {
>  	return !skb->head_frag || skb_cloned(skb);
>  }
> +
> +/**
> + * skb_gso_network_seglen - Return length of individual segments of a gso packet
> + *
> + * @skb: GSO skb
> + *
> + * skb_gso_network_seglen is used to determine the real size of the
> + * individual segments, including Layer3 (IP, IPv6) and L4 headers (TCP/UDP).
> + *
> + * The MAC/L2 header is not accounted for.
> + */
> +static inline unsigned int skb_gso_network_seglen(const struct sk_buff *skb)
> +{
> +	unsigned int hdr_len = skb_transport_header(skb) -
> +			       skb_network_header(skb);
> +	return hdr_len + skb_gso_transport_seglen(skb);
> +}
>  #endif	/* __KERNEL__ */
>  #endif	/* _LINUX_SKBUFF_H */
> diff --git a/net/ipv4/ip_forward.c b/net/ipv4/ip_forward.c
> index 694de3b..78f2e2a 100644
> --- a/net/ipv4/ip_forward.c
> +++ b/net/ipv4/ip_forward.c
> @@ -39,6 +39,58 @@
>  #include <net/route.h>
>  #include <net/xfrm.h>
>  
> +static bool ip_may_fragment(const struct sk_buff *skb)
> +{
> +	return unlikely((ip_hdr(skb)->frag_off & htons(IP_DF)) == 0) ||
> +	       !skb->local_df;
> +}
> +
> +static bool ip_gso_exceeds_dst_mtu(const struct sk_buff *skb)
> +{
> +	if (skb->local_df || !skb_is_gso(skb))
> +		return false;
> +	return skb_gso_network_seglen(skb) > dst_mtu(skb_dst(skb));
> +}
> +
> +static bool ip_exceeds_mtu(const struct sk_buff *skb, unsigned int mtu)
> +{
> +	unsigned len;
> +
> +	if (skb->local_df)
> +		return false;
> +	len = skb_is_gso(skb) ? skb_gso_network_seglen(skb) : skb->len;
> +
> +	return len > mtu;

The function should avoid extra computation/tests for small packets.

if (skb->len <= mtu || skb->local_df)
	return false;

if (skb_is_gso(skb) && skb_gso_network_seglen(skb) <= mtu)
	return false;

return true;

> +}
> +
> +/* called if GSO skb needs to be fragmented on forward.  */
> +static int ip_forward_finish_gso(struct sk_buff *skb)
> +{
> +	struct sk_buff *segs = skb_gso_segment(skb, 0);

0 is very pessimistic.

Have you tried :

netdev_features_t features = netif_skb_features(skb); 
struct sk_buff *segs = skb_gso_segment(skb, features & ~NETIF_F_GSO_MASK);


> +	int ret = 0;
> +
> +	if (IS_ERR(segs)) {
> +		kfree_skb(skb);
> +		return -ENOMEM;
> +	}
> +
> +	consume_skb(skb);
> +
> +	do {
> +		struct sk_buff *nskb = segs->next;
> +		int err;
> +
> +		segs->next = NULL;
> +		err = dst_output(segs);
> +
> +		if (err && ret == 0)
> +			ret = err;
> +		segs = nskb;
> +	} while (segs);
> +
> +	return ret;
> +}
> +

Re: [PATCH 2/2] net: ip, ipv6: handle gso skbs in forwarding path

[Florian Westphal]

Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > +static bool ip_exceeds_mtu(const struct sk_buff *skb, unsigned int mtu)
> > +{
> > +	unsigned len;
> > +
> > +	if (skb->local_df)
> > +		return false;
> > +	len = skb_is_gso(skb) ? skb_gso_network_seglen(skb) : skb->len;
> > +
> > +	return len > mtu;
> 
> The function should avoid extra computation/tests for small packets.
> 
> if (skb->len <= mtu || skb->local_df)
> 	return false;

Good idea!  Will change it as per your suggestion.

> if (skb_is_gso(skb) && skb_gso_network_seglen(skb) <= mtu)
> 	return false;
> 
> return true;
> 
> > +}
> > +
> > +/* called if GSO skb needs to be fragmented on forward.  */
> > +static int ip_forward_finish_gso(struct sk_buff *skb)
> > +{
> > +	struct sk_buff *segs = skb_gso_segment(skb, 0);
> 
> 0 is very pessimistic.
> 
> Have you tried :
> 
> netdev_features_t features = netif_skb_features(skb); 
> struct sk_buff *segs = skb_gso_segment(skb, features & ~NETIF_F_GSO_MASK);

No.  I'll see if this works for me, then include it in V2.

Thanks Eric.