On Sun, 2007-08-26 at 08:03 +0300, Michael S. Tsirkin wrote:
> 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.
Looking at the orginial code again (after some more sleep :-) ),
I agree that new and old are equivalent. I just found the
original a bit confusing.
_______________________________________________
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
