Re: [Xen-devel] [PATCH 2/2] netfront skb padding

[Jon Mason <jdmason@xxxxxxxxxx>]

On Tuesday 08 February 2005 08:12 pm, Ian Pratt wrote:
> 
> > It appears that when alloc'ing a skb, it is bring padded by 
> > an arbitrarily 
> > (and excessive) long value.  The value for this padding 
> > really only needs to 
> > be 24.  24 = 14 for the ethernet header + 2 for the cache 
> > alignment + 4 for 
> > the CRC + 4 for the VLAN flags.
> 
> Given that we're allocating page sized buffers the current situation
> doesn't cost us anything.

Unless it starts using larger packets, e.g. Jumbo Frames (hint hint).  Then
the unnecessary room can be a problem, as the unnecessary pad could cause the
unnecessary allocation of an extra page.

> Infact, what happens if the packet gets encapsulated e.g. by etherip
> etc? Is Linux smart enough to be able to put the extra headers on
> in-place if there is enough head room?

I would assume that they would have to be there.

> If so, the current situation is positively better.
> 
> Ian
> 
> > Tested on Xen 2.0.3 and linux 2.6.10 kernel.
> > 
> > Signed-off-by: Jon Mason <jdmason@xxxxxxxxxx>
> > ---
> > 
> > --- drivers/xen/netfront/netfront.c.new 2005-02-08 
> > 19:09:31.368834000 -0600
> > +++ drivers/xen/netfront/netfront.c 2005-02-08 
> > 19:09:24.738834000 -0600
> > @@ -50,6 +50,8 @@
> >  #include <asm-xen/balloon.h>
> >  #include <asm/page.h>
> >  
> > +#define NETFRONT_VERSION "1.1"
> > +
> >  #ifndef __GFP_NOWARN
> >  #define __GFP_NOWARN 0
> >  #endif
> > @@ -62,8 +64,11 @@
> >          skb_shinfo(_skb)->frag_list = NULL;           \
> >      } while (0)
> >  
> > -/* Allow headroom on each rx pkt for Ethernet header, 
> > alignment padding, ... */
> > -#define RX_HEADROOM 200
> > +/* 
> > + * Allow headroom on each rx pkt for Ethernet header, 
> > alignment padding, 
> > + * CRC, and VLAN 
> > + */
> > +#define RX_HEADROOM ETH_HLEN + NET_IP_ALIGN + 8 
> >  
> >  /*
> >   * If the backend driver is pipelining transmit requests 
> > then we can be very
> > @@ -1248,7 +1253,8 @@ static int __init netif_init(void)
> >      if (xen_start_info.flags & SIF_INITDOMAIN)
> >          return 0;
> >  
> > -    IPRINTK("Initialising virtual ethernet driver.\n");
> > +    IPRINTK("Initialising virtual ethernet driver, version %s\n", 
> > +      NETFRONT_VERSION);
> >      INIT_LIST_HEAD(&dev_list);
> >      (void)register_inetaddr_notifier(&notifier_inetdev);
> >      netctrl_init();
> > 
> > 
> > -------------------------------------------------------
> > SF email is sponsored by - The IT Product Guide
> > Read honest & candid reviews on hundreds of IT Products from 
> > real users.
> > Discover which products truly live up to the hype. Start reading now.
> > http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.sourceforge.net/lists/listinfo/xen-devel
> > 
> 
> 
> -------------------------------------------------------
> SF email is sponsored by - The IT Product Guide
> Read honest & candid reviews on hundreds of IT Products from real users.
> Discover which products truly live up to the hype. Start reading now.
> http://ads.osdn.com/?ad_ide95&alloc_id396&op=Click
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.sourceforge.net/lists/listinfo/xen-devel
> 
> 

-- 
Jon Mason
jdmason@xxxxxxxxxx


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.sourceforge.net/lists/listinfo/xen-devel