Re: [PATCH 2.6]: Fix suboptimal fragment sizing for last fragment

[YOSHIFUJI Hideaki / 吉藤英明 <yoshfuji@xxxxxxxxxxxxxx>]

In article <20040907231557.GA2254@xxxxxxxxxxxxxxxxxxx> (at Wed, 8 Sep 2004 
09:15:57 +1000), Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx> says:

> > +           len = mtu - skb->len;
> > +           if (len > size)
> > +                   len = maxfraglen - skb->len;
> 
> I think that should be len < size, right?

oh, right, thanks.

Signed-off-by: Hideaki YOSHIFUJI <yoshfuji@xxxxxxxxxxxxxx>

===== net/ipv4/ip_output.c 1.67 vs edited =====
--- 1.67/net/ipv4/ip_output.c   2004-09-03 06:50:20 +09:00
+++ edited/net/ipv4/ip_output.c 2004-09-08 08:22:29 +09:00
@@ -811,26 +811,33 @@
                goto alloc_new_skb;
 
        while (length > 0) {
-               if ((copy = mtu - skb->len) <= 0) {
+               /* Check if the remaining data fits into current packet. */
+               copy = mtu - skb->len;
+               if (copy < length)
+                       copy = maxfraglen - skb->len;
+               if (copy <= 0) {
                        char *data;
                        unsigned int datalen;
                        unsigned int fraglen;
                        unsigned int fraggap;
                        unsigned int alloclen;
                        struct sk_buff *skb_prev;
-                       BUG_TRAP(copy == 0);
-
 alloc_new_skb:
                        skb_prev = skb;
-                       fraggap = 0;
                        if (skb_prev)
-                               fraggap = mtu - maxfraglen;
-
-                       datalen = mtu - fragheaderlen;
-                       if (datalen > length + fraggap)
-                               datalen = length + fraggap;
+                               fraggap = skb_prev->len - maxfraglen;
+                       else
+                               fraggap = 0;
 
+                       /*
+                        * If remaining data exceeds the mtu,
+                        * we know we need more fragment(s).
+                        */
+                       datalen = length + fraggap;
+                       if (datalen > mtu - fragheaderlen)
+                               datalen = maxfraglen - fragheaderlen;
                        fraglen = datalen + fragheaderlen;
+
                        if ((flags & MSG_MORE) && 
                            !(rt->u.dst.dev->features&NETIF_F_SG))
                                alloclen = mtu;
@@ -1026,18 +1033,22 @@
 
        while (size > 0) {
                int i;
-               if ((len = mtu - skb->len) <= 0) {
+
+               /* Check if the remaining data fits into current packet. */
+               len = mtu - skb->len;
+               if (len < size)
+                       len = maxfraglen - skb->len;
+               if (len <= 0) {
                        struct sk_buff *skb_prev;
                        char *data;
                        struct iphdr *iph;
                        int alloclen;
 
-                       BUG_TRAP(len == 0);
-
                        skb_prev = skb;
-                       fraggap = 0;
                        if (skb_prev)
-                               fraggap = mtu - maxfraglen;
+                               fraggap = skb_prev->len - maxfraglen;
+                       else
+                               fraggap = 0;
 
                        alloclen = fragheaderlen + hh_len + fraggap + 15;
                        skb = sock_wmalloc(sk, alloclen, 1, sk->sk_allocation);
===== net/ipv6/ip6_output.c 1.72 vs edited =====
--- 1.72/net/ipv6/ip6_output.c  2004-09-03 06:50:20 +09:00
+++ edited/net/ipv6/ip6_output.c        2004-09-08 08:18:28 +09:00
@@ -898,26 +898,34 @@
                goto alloc_new_skb;
 
        while (length > 0) {
-               if ((copy = mtu - skb->len) <= 0) {
+               /* Check if the remaining data fits into current packet. */
+               copy = mtu - skb->len;
+               if (copy < length)
+                       copy = maxfraglen - skb->len;
+
+               if (copy <= 0) {
                        char *data;
                        unsigned int datalen;
                        unsigned int fraglen;
                        unsigned int fraggap;
                        unsigned int alloclen;
                        struct sk_buff *skb_prev;
-                       BUG_TRAP(copy == 0);
 alloc_new_skb:
                        skb_prev = skb;
 
                        /* There's no room in the current skb */
-                       fraggap = 0;
                        if (skb_prev)
-                               fraggap = mtu - maxfraglen;
-
-                       datalen = mtu - fragheaderlen;
+                               fraggap = skb_prev->len - maxfraglen;
+                       else
+                               fraggap = 0;
 
-                       if (datalen > length + fraggap)
-                               datalen = length + fraggap;
+                       /*
+                        * If remaining data exceeds the mtu,
+                        * we know we need more fragment(s).
+                        */
+                       datalen = length + fraggap;
+                       if (datalen > mtu - fragheaderlen)
+                               datalen = maxfraglen - fragheaderlen;
 
                        fraglen = datalen + fragheaderlen;
                        if ((flags & MSG_MORE) &&


-- 
Hideaki YOSHIFUJI @ USAGI Project <yoshfuji@xxxxxxxxxxxxxx>
GPG FP: 9022 65EB 1ECF 3AD1 0BDF  80D8 4807 F894 E062 0EEA