The following claim seems wrong, too:
> NOTE that [frags..IPOIB_CM_RX_SG-1] are *not* copied to newskb and
> thus leaked. Also, newskb's nr_frags is set based on the message size
> rather than the full size.
Not really. nr_frags *starts* at the message size.
skb_fill_page_desc will fill in the fargment in toskb, and
increment nr_frags until it matches the full size.
> @@ -378,23 +382,26 @@ static void skb_put_frags(struct sk_buff *skb, unsigned
> int hdr_space,
> skb->len += size;
> length -= size;
>
> - num_frags = skb_shinfo(skb)->nr_frags;
> + num_frags = skb_shinfo(toskb)->nr_frags;
> for (i = 0; i < num_frags; i++) {
> skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
>
> - if (length == 0) {
> - /* don't need this page */
> - skb_fill_page_desc(toskb, i, frag->page, 0, PAGE_SIZE);
> - --skb_shinfo(skb)->nr_frags;
> - } else {
> - size = min(length, (unsigned) PAGE_SIZE);
> -
> - frag->size = size;
> - skb->data_len += size;
> - skb->truesize += size;
> - skb->len += size;
> - length -= size;
> - }
> + size = min(length, (unsigned) PAGE_SIZE);
> + WARN_ON(size == 0);
> + frag->size = size;
> + skb->data_len += size;
> + skb->truesize += size;
> + skb->len += size;
> + length -= size;
> + }
> + WARN_ON(length != 0);
> + skb_shinfo(skb)->nr_frags = num_frags;
> +
> + /* move unused pages from skb to toskb */
> + for (; i < IPOIB_CM_RX_SG - 1; i++) {
> + skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
> +
> + skb_fill_page_desc(toskb, i, frag->page, 0, PAGE_SIZE);
> }
> }
>
I don't see how this chunk changes anything.
It seems you have basically replaced a single loop with
a condition by two loops.
--
MST
_______________________________________________
general mailing list
[email protected]
http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
