lkml.org 
[lkml]   [2006]   [Mar]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [RFC PATCH 34/35] Add the Xen virtual network device driver.
    From
    Date

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


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

    \
     
     \ /
      Last update: 2006-03-22 10:01    [W:0.021 / U:59.864 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site