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


+#ifdef CONFIG_XEN_BALLOON
+#include <xen/balloon.h>
+#endif

such ifdefs are ***-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 ?


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/