ipv6: Fix ip6_xmit to send fragments if ipfragok is true

PATCH Superseded Wei Yongjun 3 participants 7 messages Jul 31, 2008 – Aug 1, 2008

Subsystems (3)

Networking Network drivers IPv4/IPv6

lore

Share & tools

lore

Wei Yongjun

author

2008-07-31 05:57 UTC #0 lore raw

PATCH 1/1 ipv6: Fix ip6_xmit to send fragments if ipfragok is true

SCTP used ip6_xmit() to send fragments after received ICMP packet too big message. But while send packet used ip6_xmit, the skb->local_df is not initialized. So when skb if enter ip6_fragment <../cgi-bin/global.cgi?pattern=ip6_fragment&type=reference>(), the following code will discard the skb.

ip6_fragment(...)

{
    if (!skb->local_df) {
        ...
        return -EMSGSIZE;
    }
    ...
}

SCTP do the following step:
1. send packet ip6_xmit(skb, ipfragok=0)
2. received ICMP packet too big message
3. if PMTUD_ENABLE: ip6_xmit(skb, ipfragok=1)

This bug come from patch (b5c15fc004ac83b7ad280acbe0fd4bbed7e2c8d4) [IPV6]: Fix reversed local_df test in ip6_fragment
which commit by Herbert Xu.

This patch fixed the problem.

Signed-off-by Wei Yongjun <[email protected]>

---
 net/ipv6/ip6_output.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

net/ipv6/ip6_output.c +4 -0

index 6407c64..a678c71 100644
@@ -236,6 +236,10 @@ int ip6_xmit(struct sock *sk, struct sk_buff *skb, struct flowi *fl,
 	skb_reset_network_header(skb);
 	hdr = ipv6_hdr(skb);
 
+	/* Allow local fragmentation. */
+	if (np && np->pmtudisc < IPV6_PMTUDISC_DO)
+		skb->local_df = 1;
+
 	/*
 	 *	Fill in the IPv6 header
 	 */

Herbert Xu

2008-07-31 08:21 UTC #1 lore raw

Quoted 5 lines · Wei Yongjun

SCTP used ip6_xmit() to send fragments after received ICMP packet too big message. But while send packet used ip6_xmit, the skb->local_df is not initialized. So when skb if enter ip6_fragment <../cgi-bin/global.cgi?pattern=ip6_fragment&type=reference>(), the following code will discard the skb.

That sounds broken. ip6_xmit is for TCP-like protocols that perform PMTU-discovery. Therefore it should never fragment locally.

So either SCTP needs to do the same, or it should switch to something other than ip6_xmit.

Cheers,

Signature · 4 lines

Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Wei Yongjun

author

2008-07-31 09:13 UTC #2 lore raw

Quoted 9 lines · Herbert Xu

Quoted 6 lines · Wei Yongjun
SCTP used ip6_xmit() to send fragments after received ICMP packet too big message. But while send packet used ip6_xmit, the skb->local_df is not initialized. So when skb if enter ip6_fragment <../cgi-bin/global.cgi?pattern=ip6_fragment&type=reference>(), the following code will discard the skb.

Quoted 7 lines

That sounds broken. ip6_xmit is for TCP-like protocols that perform PMTU-discovery. Therefore it should never fragment locally.
So either SCTP needs to do the same, or it should switch to something other than ip6_xmit.

So what does the ipfragok paramater means?
TCP used ip6_xmit always set ipfragok = 0. But if ipfragok=1, what will happend?

If ipfragok = 0, and skb->len > mtu, ip6_xmit will discard the skb.

If ipfragok=1, ip6_xmit will pass skb to ip6_output() and send skb used ip6_fragment(), but skb will be discard since local_df is 0, it seems that ipfragok has no effect.

skb->local_df is set to 1 if np->pmtudisc < IPV6_PMTUDISC_DO in other place, so I think it may also set to 1 if np->pmtudisc < IPV6_PMTUDISC_DO, and send the skb used ip6_fragment(). Is this correct?

As I know, if I send skb in IPv4 used ip_queue_xmit(ipfragok=1), skb will be send in fragments if len > mtu.

int ip6_xmit(struct sock *sk, struct sk_buff *skb, struct flowi *fl,
struct ipv6_txoptions *opt, int ipfragok)

{
       ...
       mtu = dst_mtu(dst);
       if ((skb->len <= mtu) || ipfragok || skb_is_gso(skb)) {
               IP6_INC_STATS(ip6_dst_idev(skb->dst),
                             IPSTATS_MIB_OUTREQUESTS);
               return NF_HOOK(PF_INET6, NF_INET_LOCAL_OUT, skb, NULL, dst->dev,
                               dst_output);
       }

       if (net_ratelimit())
               printk(KERN_DEBUG "IPv6: sending pkt_too_big to self\n");
       ...
}

int ip6_output(struct sk_buff *skb)

{
       if ((skb->len > ip6_skb_dst_mtu(skb) && !skb_is_gso(skb)) ||
                               dst_allfrag(skb->dst))
               return ip6_fragment(skb, ip6_output2);
       else
               return ip6_output2(skb);
}

Herbert Xu

2008-07-31 09:23 UTC #3 lore raw

Quoted 4 lines · Wei Yongjun

So what does the ipfragok paramater means?
TCP used ip6_xmit always set ipfragok = 0. But if ipfragok=1, what will happend?

OK it's already been butchered for SCTP :)

In that case you should set local_df iff ipfragok is true.

Cheers,

Signature · 4 lines

Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Wei Yongjun

author

2008-07-31 09:57 UTC #4 lore raw

PATCH v2 1/1 ipv6: Fix ip6_xmit to send fragments if ipfragok is true

ip6_fragment(...)
{

   if (!skb->local_df) {
       ...
       return -EMSGSIZE;

}
...
}

SCTP do the following step:
1. send packet ip6_xmit(skb, ipfragok=0)
2. received ICMP packet too big message
3. if PMTUD_ENABLE: ip6_xmit(skb, ipfragok=1)

This patch fixed the problem by set local_df if ipfragok is true.

Signed-off-by Wei Yongjun <[email protected]>

---
net/ipv6/ip6_output.c | 4 ++++

1 files changed, 4 insertions(+), 0 deletions(-)

net/ipv6/ip6_output.c +0 -0

index 6407c64..0969f2a 100644
@@ -236,6 +236,10 @@ int ip6_xmit(struct sock *sk, struct sk_buff *skb, struct flowi *fl,

	skb_reset_network_header(skb);
	hdr = ipv6_hdr(skb);

+ /* Allow local fragmentation. */
+ if (ipfragok)
+ skb->local_df = 1;
+

	/*
	 *	Fill in the IPv6 header
	 */

Signature · 1 line

1.5.3.8

Herbert Xu

Acked-by on v2 1/1

2008-07-31 10:41 UTC #5 lore raw

Quoted 14 lines · Wei Yongjun

SCTP used ip6_xmit() to send fragments after received ICMP packet too big message. But while send packet used ip6_xmit, the skb->local_df is not initialized. So when skb if enter ip6_fragment(), the following code will discard the skb.
ip6_fragment(...)
{
   if (!skb->local_df) {
       ...
       return -EMSGSIZE;
   }
   ...
}

Quoted 8 lines

SCTP do the following step:
1. send packet ip6_xmit(skb, ipfragok=0)
2. received ICMP packet too big message
3. if PMTUD_ENABLE: ip6_xmit(skb, ipfragok=1)
This patch fixed the problem by set local_df if ipfragok is true.
Signed-off-by Wei Yongjun <[email protected]>

Acked-by Herbert Xu <[email protected]>

Thanks,

Signature · 4 lines

Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

David Miller

maintainer Applied

2008-08-01 03:47 UTC #6 lore raw

From: Herbert Xu <[email protected]>
Date: Thu, 31 Jul 2008 18:41:50 +0800

Quoted 20 lines · Wei Yongjun

Quoted 19 lines
SCTP used ip6_xmit() to send fragments after received ICMP packet too big message. But while send packet used ip6_xmit, the skb->local_df is not initialized. So when skb if enter ip6_fragment(), the following code will discard the skb.
ip6_fragment(...)
{
   if (!skb->local_df) {
       ...
       return -EMSGSIZE;
   }
   ...
}
SCTP do the following step:
1. send packet ip6_xmit(skb, ipfragok=0)
2. received ICMP packet too big message
3. if PMTUD_ENABLE: ip6_xmit(skb, ipfragok=1)

Quoted 5 lines

Quoted 3 lines
This patch fixed the problem by set local_df if ipfragok is true.
Signed-off-by Wei Yongjun <[email protected]>
Acked-by Herbert Xu <[email protected]>

Applied, and I'll queue this up for -stable.

Thanks everyone.