[Xen-devel] Re: [RFC PATCH 34/35] Add the Xen virtual network device driver.

[Arjan van de Ven <arjan@xxxxxxxxxxxxx>]

> +#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 ?



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