From mboxrd@z Thu Jan  1 00:00:00 1970
From: Bill Fink <billfink@mindspring.com>
Subject: Re: [IPV6]: Fix IPsec datagram fragmentation
Date: Fri, 15 Feb 2008 01:13:13 -0500
Message-ID: <20080215011313.7f5c035f.billfink@mindspring.com>
References: <20080213000437.GA19976@gondor.apana.org.au>
	<20080212.180828.262893392.davem@davemloft.net>
	<20080215015505.GA9098@gondor.apana.org.au>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Cc: David Miller <davem@davemloft.net>, yoshfuji@linux-ipv6.org,
	netdev@vger.kernel.org, dlstevens@us.ibm.com, kazunori@miyazawa.org
To: Herbert Xu <herbert@gondor.apana.org.au>
Return-path: <netdev-owner@vger.kernel.org>
Received: from elasmtp-mealy.atl.sa.earthlink.net ([209.86.89.69]:45045 "EHLO
	elasmtp-mealy.atl.sa.earthlink.net" rhost-flags-OK-OK-OK-OK)
	by vger.kernel.org with ESMTP id S1757625AbYBOGNl (ORCPT
	<rfc822;netdev@vger.kernel.org>); Fri, 15 Feb 2008 01:13:41 -0500
In-Reply-To: <20080215015505.GA9098@gondor.apana.org.au>
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

Hi Herbert,

On Fri, 15 Feb 2008, Herbert Xu wrote:

> On Tue, Feb 12, 2008 at 06:08:28PM -0800, David Miller wrote:
> > 
> > > [IPV6]: Fix IPsec datagram fragmentation
> >
> > Applied, and I'll queue this up to -stable as well.
> 
> Sorry, David Stevens just told me that it doesn't work as intended.
> 
> [IPV6]: Fix reversed local_df test in ip6_fragment
> 
> I managed to reverse the local_df test when forward-porting this
> patch so it actually makes things worse by never fragmenting at
> all.
> 
> Thanks to David Stevens for testing and reporting this bug.
> 
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
> 
> --
> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> index 4e9a2fe..35ba693 100644
> --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c
> @@ -621,7 +621,7 @@ static int ip6_fragment(struct sk_buff *skb, int (*output)(struct sk_buff *))
>  	 * or if the skb it not generated by a local socket.  (This last
>  	 * check should be redundant, but it's free.)
>  	 */
> -	if (skb->local_df) {
> +	if (!skb->local_df) {
>  		skb->dev = skb->dst->dev;
>  		icmpv6_send(skb, ICMPV6_PKT_TOOBIG, 0, mtu, skb->dev);
>  		IP6_INC_STATS(ip6_dst_idev(skb->dst), IPSTATS_MIB_FRAGFAILS);

I think the setting of skb->local_def is still backwards in your
original patch:

> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> index 9ac6ca2..4e9a2fe 100644
> --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c

...

> @@ -1420,6 +1420,10 @@ int ip6_push_pending_frames(struct sock *sk)
> 		tmp_skb->sk = NULL;
> 	}
> 
> +	/* Allow local fragmentation. */
> +	if (np->pmtudisc >= IPV6_PMTUDISC_DO)
> +		skb->local_df = 1;
> +
> 	ipv6_addr_copy(final_dst, &fl->fl6_dst);
> 	__skb_pull(skb, skb_network_header_len(skb));
> 	if (opt && opt->opt_flen)

I think the test should be:

	if (np->pmtudisc < IPV6_PMTUDISC_DO)

as it is in net/ipv4/ip_output.c:

/* Unless user demanded real pmtu discovery (IP_PMTUDISC_DO), we allow
 * to fragment the frame generated here. No matter, what transforms
 * how transforms change size of the packet, it will come out.
 */
	if (inet->pmtudisc < IP_PMTUDISC_DO)
		skb->local_df = 1;

Or perhaps I'm just missing something obvious.

						-Bill