Re: [IPV6]: Fix IPsec datagram fragmentation

[Bill Fink <billfink@mindspring.com>]

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