diff mbox

[Precise,SRU] network problems after update to kernel 3.2.0-65 - thunderbird/imap/dovecot

Message ID 53CEB0DE.3010000@canonical.com
State New
Headers show

Commit Message

Tim Gardner July 22, 2014, 6:43 p.m. UTC
Looks like these stable updates caused regressions.

Comments

Stefan Bader July 23, 2014, 6:50 a.m. UTC | #1
On 22.07.2014 20:43, Tim Gardner wrote:
> Looks like these stable updates caused regressions.
> 
> 
> 
Based on the positive tester feedback in the bug.
Stefan Bader July 23, 2014, 6:55 a.m. UTC | #2
On 23.07.2014 08:50, Stefan Bader wrote:
> On 22.07.2014 20:43, Tim Gardner wrote:
>> Looks like these stable updates caused regressions.
>>
>>
>>
> Based on the positive tester feedback in the bug.

Though there is also a reference to Debian where Ben reverted two patches. But I
could not figure out which was the second one, yet.

-Stefan
Stefan Bader July 23, 2014, 6:58 a.m. UTC | #3
On 23.07.2014 08:55, Stefan Bader wrote:
> On 23.07.2014 08:50, Stefan Bader wrote:
>> On 22.07.2014 20:43, Tim Gardner wrote:
>>> Looks like these stable updates caused regressions.
>>>
>>>
>>>
>> Based on the positive tester feedback in the bug.
> 
> Though there is also a reference to Debian where Ben reverted two patches. But I
> could not figure out which was the second one, yet.
> 
> -Stefan
> 
> 
> 
Searched harder...

Subject: Revert "net: ip, ipv6: handle gso skbs in forwarding path"

This reverts commit caa5344994778a2b4725b2d75c74430f76925e4a, which
was commit fe6cc55f3a9a053482a76f5a6b2257cee51b4663 upstream.  In 3.2,
the transport header length is not calculated in the forwarding path,
so skb_gso_network_seglen() returns an incorrect result.  We also have
problems due to the local_df flag not being set correctly.

There isn't a Debian bug report for this, but it needed a further
fix ("net: ipv4: ip_forward: fix inverted local_df test") which in
turn caused more obvious regressions (#754173).
Luis Henriques July 23, 2014, 9:31 a.m. UTC | #4
On Tue, Jul 22, 2014 at 12:43:42PM -0600, Tim Gardner wrote:
> Looks like these stable updates caused regressions.
> -- 
> Tim Gardner tim.gardner@canonical.com

> From 44325cca2e0e064b9c1925cdf07d112c25bb0835 Mon Sep 17 00:00:00 2001
> From: Tim Gardner <tim.gardner@canonical.com>
> Date: Tue, 22 Jul 2014 12:38:48 -0600
> Subject: [PATCH Precise SRU 1/2] Revert "net: ipv4: ip_forward: fix inverted local_df test"
> 
> BugLink: http://bugs.launchpad.net/bugs/1337281
> 
> This reverts commit 5be90996677a55d3c2371debce40c45066587c9b.
> 
> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
> ---
>  net/ipv4/ip_forward.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/ipv4/ip_forward.c b/net/ipv4/ip_forward.c
> index 7593f3a..e0d9f02 100644
> --- a/net/ipv4/ip_forward.c
> +++ b/net/ipv4/ip_forward.c
> @@ -42,12 +42,12 @@
>  static bool ip_may_fragment(const struct sk_buff *skb)
>  {
>  	return unlikely((ip_hdr(skb)->frag_off & htons(IP_DF)) == 0) ||
> -		skb->local_df;
> +	       !skb->local_df;
>  }
>  
>  static bool ip_exceeds_mtu(const struct sk_buff *skb, unsigned int mtu)
>  {
> -	if (skb->len <= mtu)
> +	if (skb->len <= mtu || skb->local_df)
>  		return false;
>  
>  	if (skb_is_gso(skb) && skb_gso_network_seglen(skb) <= mtu)
> -- 
> 2.0.1
> 

> From 44a5d3a9ff784632ff4d6dccdaaed0ad965711e1 Mon Sep 17 00:00:00 2001
> From: Tim Gardner <tim.gardner@canonical.com>
> Date: Tue, 22 Jul 2014 12:39:02 -0600
> Subject: [PATCH Precise SRU 2/2] Revert "net: ip, ipv6: handle gso skbs in forwarding path"
> 
> BugLink: http://bugs.launchpad.net/bugs/1337281
> 
> This reverts commit 440c80397cea0e49cd249852cd2d66c223a345d6.
> 
> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
> ---
>  include/linux/skbuff.h | 17 -------------
>  net/ipv4/ip_forward.c  | 68 ++------------------------------------------------
>  net/ipv6/ip6_output.c  | 13 +---------
>  3 files changed, 3 insertions(+), 95 deletions(-)
> 
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 40c2726..1b4ea29 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -2583,22 +2583,5 @@ static inline bool skb_is_recycleable(const struct sk_buff *skb, int skb_size)
>  
>  	return true;
>  }
> -
> -/**
> - * 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 e0d9f02..29a07b6 100644
> --- a/net/ipv4/ip_forward.c
> +++ b/net/ipv4/ip_forward.c
> @@ -39,68 +39,6 @@
>  #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_exceeds_mtu(const struct sk_buff *skb, unsigned int mtu)
> -{
> -	if (skb->len <= mtu || skb->local_df)
> -		return false;
> -
> -	if (skb_is_gso(skb) && skb_gso_network_seglen(skb) <= mtu)
> -		return false;
> -
> -	return true;
> -}
> -
> -static bool ip_gso_exceeds_dst_mtu(const struct sk_buff *skb)
> -{
> -	unsigned int mtu;
> -
> -	if (skb->local_df || !skb_is_gso(skb))
> -		return false;
> -
> -	mtu = dst_mtu(skb_dst(skb));
> -
> -	/* if seglen > mtu, do software segmentation for IP fragmentation on
> -	 * output.  DF bit cannot be set since ip_forward would have sent
> -	 * icmp error.
> -	 */
> -	return skb_gso_network_seglen(skb) > 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;
> -	int ret = 0;
> -
> -	segs = skb_gso_segment(skb, 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);
> @@ -110,9 +48,6 @@ 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);
>  }
>  
> @@ -152,7 +87,8 @@ int ip_forward(struct sk_buff *skb)
>  	if (opt->is_strictroute && opt->nexthop != rt->rt_gateway)
>  		goto sr_failed;
>  
> -	if (!ip_may_fragment(skb) && ip_exceeds_mtu(skb, dst_mtu(&rt->dst))) {
> +	if (unlikely(skb->len > dst_mtu(&rt->dst) && !skb_is_gso(skb) &&
> +		     (ip_hdr(skb)->frag_off & htons(IP_DF))) && !skb->local_df) {
>  		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 7871cc6..b720710 100644
> --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c
> @@ -381,17 +381,6 @@ 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)
> -{
> -	if (skb->len <= mtu || skb->local_df)
> -		return false;
> -
> -	if (skb_is_gso(skb) && skb_gso_network_seglen(skb) <= mtu)
> -		return false;
> -
> -	return true;
> -}
> -
>  int ip6_forward(struct sk_buff *skb)
>  {
>  	struct dst_entry *dst = skb_dst(skb);
> @@ -515,7 +504,7 @@ int ip6_forward(struct sk_buff *skb)
>  	if (mtu < IPV6_MIN_MTU)
>  		mtu = IPV6_MIN_MTU;
>  
> -	if (ip6_pkt_too_big(skb, mtu)) {
> +	if (skb->len > mtu && !skb_is_gso(skb)) {
>  		/* Again, force OUTPUT device used as source address */
>  		skb->dev = dst->dev;
>  		icmpv6_send(skb, ICMPV6_PKT_TOOBIG, 0, mtu);
> -- 
> 2.0.1
> 

> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team


Cheers,
--
Luís
Tim Gardner July 23, 2014, 12:22 p.m. UTC | #5

diff mbox

Patch

From 44a5d3a9ff784632ff4d6dccdaaed0ad965711e1 Mon Sep 17 00:00:00 2001
From: Tim Gardner <tim.gardner@canonical.com>
Date: Tue, 22 Jul 2014 12:39:02 -0600
Subject: [PATCH Precise SRU 2/2] Revert "net: ip, ipv6: handle gso skbs in forwarding path"

BugLink: http://bugs.launchpad.net/bugs/1337281

This reverts commit 440c80397cea0e49cd249852cd2d66c223a345d6.

Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
---
 include/linux/skbuff.h | 17 -------------
 net/ipv4/ip_forward.c  | 68 ++------------------------------------------------
 net/ipv6/ip6_output.c  | 13 +---------
 3 files changed, 3 insertions(+), 95 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 40c2726..1b4ea29 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2583,22 +2583,5 @@  static inline bool skb_is_recycleable(const struct sk_buff *skb, int skb_size)
 
 	return true;
 }
-
-/**
- * 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 e0d9f02..29a07b6 100644
--- a/net/ipv4/ip_forward.c
+++ b/net/ipv4/ip_forward.c
@@ -39,68 +39,6 @@ 
 #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_exceeds_mtu(const struct sk_buff *skb, unsigned int mtu)
-{
-	if (skb->len <= mtu || skb->local_df)
-		return false;
-
-	if (skb_is_gso(skb) && skb_gso_network_seglen(skb) <= mtu)
-		return false;
-
-	return true;
-}
-
-static bool ip_gso_exceeds_dst_mtu(const struct sk_buff *skb)
-{
-	unsigned int mtu;
-
-	if (skb->local_df || !skb_is_gso(skb))
-		return false;
-
-	mtu = dst_mtu(skb_dst(skb));
-
-	/* if seglen > mtu, do software segmentation for IP fragmentation on
-	 * output.  DF bit cannot be set since ip_forward would have sent
-	 * icmp error.
-	 */
-	return skb_gso_network_seglen(skb) > 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;
-	int ret = 0;
-
-	segs = skb_gso_segment(skb, 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);
@@ -110,9 +48,6 @@  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);
 }
 
@@ -152,7 +87,8 @@  int ip_forward(struct sk_buff *skb)
 	if (opt->is_strictroute && opt->nexthop != rt->rt_gateway)
 		goto sr_failed;
 
-	if (!ip_may_fragment(skb) && ip_exceeds_mtu(skb, dst_mtu(&rt->dst))) {
+	if (unlikely(skb->len > dst_mtu(&rt->dst) && !skb_is_gso(skb) &&
+		     (ip_hdr(skb)->frag_off & htons(IP_DF))) && !skb->local_df) {
 		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 7871cc6..b720710 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -381,17 +381,6 @@  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)
-{
-	if (skb->len <= mtu || skb->local_df)
-		return false;
-
-	if (skb_is_gso(skb) && skb_gso_network_seglen(skb) <= mtu)
-		return false;
-
-	return true;
-}
-
 int ip6_forward(struct sk_buff *skb)
 {
 	struct dst_entry *dst = skb_dst(skb);
@@ -515,7 +504,7 @@  int ip6_forward(struct sk_buff *skb)
 	if (mtu < IPV6_MIN_MTU)
 		mtu = IPV6_MIN_MTU;
 
-	if (ip6_pkt_too_big(skb, mtu)) {
+	if (skb->len > mtu && !skb_is_gso(skb)) {
 		/* Again, force OUTPUT device used as source address */
 		skb->dev = dst->dev;
 		icmpv6_send(skb, ICMPV6_PKT_TOOBIG, 0, mtu);
-- 
2.0.1