[EMAIL PROTECTED]> <[EMAIL PROTECTED]> <20060822212150. [EMAIL PROTECTED]> <[EMAIL PROTECTED]> In-Reply-To: <[EMAIL PROTECTED]> User-Agent: Mutt/1.5.11+cvs20060126
Hi. Back in August I sent out some patches for the aoe driver, and Alan objected to the direct setting of skb->len in one of them. I asked whether he was suggesting that we use something like this instead of setting skb->len: skb->data_len = 0; skb_trim(skb, 0); skb_put(skb, sizeof *h + sizeof *ah + DEFAULTBCNT); ... and Alan said that was acceptible because it takes care of skb->tail, checks for overflow, and is more future proof. So I took some time to implement the necessary changes, but then it became apparent that there was a problem. The skb_trim and skb_put macros are only for non-linear skbuffs, but we are only using the area inside the skbuff itself for packet headers, not data. When we do something like this: if (bio_data_dir(buf->bio) == WRITE) { skb_fill_page_desc(skb, 0, bv->bv_page, buf->bv_off, bcnt); ah.aflags |= AOEAFL_WRITE; skb->len += bcnt; skb->data_len = bcnt; t->wpkts++; ... skb->tail isn't really relevant, because we are only using the pre-allocated part of the skbuff for headers, and the headers aren't expanding here. Other parts of the kernel that aren't putting data in the skbuff itself set the skb->len directly. e1000_main.c ip_output.c tcp.c ip6_output.c So is it correct for the callers of skb_fill_page_desc to set skb->len or is another interface needed besides skb_put/skb_trim? Such a new interface would be able to maintain the consistency of the skbuff fields without assuming that the data is in the skbuff itself. If a new interface is needed, then it seems like we should set skb->len in this patch until the new interface is ready. -- Ed L Cashin <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html