[Xen-devel] Re: RX_COPY_THRESHOLD in netfront

[Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>]

Hi Keir:

On Fri, Aug 11, 2006 at 05:23:20PM +0100, Keir Fraser wrote:
> 
> Does the RX_COPY_THRESHOLD you added to netfront need to be as large as 256?
> It seems rather large: for example, the copy length in netback for packets
> being transmitted is just 64.

That was fairly arbitrary, but 64 bytes is probably too small since it'll
keep IPv6 TCP headers out of the head area (2 + 14 + 40 + 28 > 64).  So we
want at least 128 bytes for both front and back.

> Also, your patch to netfront allocates a full page for each receive slot.
> The skbuff 'header' area is really not much used, except for the first
> RX_COPY_THRESHOLD bytes of each packet. Rather than allocating lots of
> skbuffs up front and then freeing most of them when you receive jumbo
> packets, why not just allocate the skbuff part in netif_poll, for each
> complete packet that you detect you have received, rather than doing it in
> network_alloc_rx_buffers?

The main reason for preallocation is to avoid unnecessary work.  When
the system is under memory pressure, if we postpone the allocation then
all the work done to move the packet from dom0 to the point where we are
would be wasted should the allocation fail at that point.

On the other, if the failure occurs during pre-allocation, then dom0
would not even get to place the skb into the ring since there would
be no room there.

I do concede that we're wasting effort in repeatedly initialising skb's
that we throw away in the case of jumbo packets.  We can remove that
waste by maintaining our own list of unused skb's that we can simply
put back on the ring in network_alloc_rx_buffers without going through
alloc_skb again.

> Alternatively we could go back to the old method of rx buffer allocation
> which allocates page-sized skbuff header areas, then unmaps the page
> containing skb->head. This would avoid the need for RX_COPY_THRESHOLD, but
> would constrain the location of the first packet fragment (since the skb
> header area needs 16 bytes headroom and tailroom for skb_shared_info) or
> we'd need a slow path to relocate the header fragment. What do you think?

I'd like avoid that because this relies on the internal structure of
sk_buff which may not stay the same as Linux evolves.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@xxxxxxxxxxxxxxxxxxx>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel