> +#ifdef CONFIG_XEN_BALLOON > +#include <xen/balloon.h> > +#endif such ifdefs are butt-ugly and should be inside the header at least > +#ifndef __GFP_NOWARN > +#define __GFP_NOWARN 0 > +#endif eehhh why? > +#define alloc_xen_skb(_l) __dev_alloc_skb((_l), GFP_ATOMIC|__GFP_NOWARN) why does xen need it's own alloc_skb variant? > +static unsigned long rx_pfn_array[NET_RX_RING_SIZE]; > +static struct multicall_entry rx_mcl[NET_RX_RING_SIZE+1]; > +static struct mmu_update rx_mmu[NET_RX_RING_SIZE]; does this mean you can only have 1 network interface? that sounds silly > + > +/* Access macros for acquiring freeing slots in {tx,rx}_skbs[]. */ > +#define ADD_ID_TO_FREELIST(_list, _id) \ > + (_list)[(_id)] = (_list)[0]; \ > + (_list)[0] = (void *)(unsigned long)(_id); > +#define GET_ID_FROM_FREELIST(_list) \ > + ({ unsigned long _id = (unsigned long)(_list)[0]; \ > + (_list)[0] = (_list)[_id]; \ > + (unsigned short)_id; }) these should be inlines at leas > + > +/** Send a packet on a net device to encourage switches to learn the > + * MAC. We send a fake ARP request. why is this inside a driver????? > + if (unlikely((((unsigned long)skb->data & ~PAGE_MASK) + skb->len) >= > + PAGE_SIZE)) { > + struct sk_buff *nskb; > + if (unlikely((nskb = alloc_xen_skb(skb->len)) == NULL)) > + goto drop; > + skb_put(nskb, skb->len); > + memcpy(nskb->data, skb->data, skb->len); > + nskb->dev = skb->dev; > + dev_kfree_skb(skb); > + skb = nskb; > + } hmm this smells fishy > + > +static int xennet_proc_read( why is this device driver adding new own /proc crud ?