Re: [patch 7/9] lguest: the net driver

2007-05-10 Thread Rusty Russell
On Thu, 2007-05-10 at 01:33 -0400, Jeff Garzik wrote: Rusty Russell wrote: I realize your continual battle with this, but adding a layer of indirection doesn't seem like it will add clarity. The issues with __pa() are reasonably known (don't hand it a vmalloc address, for example).

Re: [patch 7/9] lguest: the net driver

2007-05-09 Thread Herbert Xu
[EMAIL PROTECTED] wrote: + if (desc-features LGUEST_NET_F_NOCSUM) + dev-features |= NETIF_F_NO_CSUM; Any reason why you're using NO_CSUM here instead of HW_CSUM? Practically there is no difference but NO_CSUM could be treated differently in future and I'm not sure whether

[patch 7/9] lguest: the net driver

2007-05-09 Thread akpm
From: Rusty Russell [EMAIL PROTECTED] Lguest net driver A simple net driver for lguest. Signed-off-by: Rusty Russell [EMAIL PROTECTED] Cc: Andi Kleen [EMAIL PROTECTED] Cc: Jeff Garzik [EMAIL PROTECTED] Acked-by: James Morris [EMAIL PROTECTED] Signed-off-by: Andrew Morton [EMAIL PROTECTED] ---

Re: [patch 7/9] lguest: the net driver

2007-05-09 Thread Rusty Russell
On Wed, 2007-05-09 at 20:12 +1000, Herbert Xu wrote: [EMAIL PROTECTED] wrote: + if (desc-features LGUEST_NET_F_NOCSUM) + dev-features |= NETIF_F_NO_CSUM; Any reason why you're using NO_CSUM here instead of HW_CSUM? Practically there is no difference but NO_CSUM

Re: [patch 7/9] lguest: the net driver

2007-05-09 Thread Andi Kleen
__pa() should not be used in any driver. Besides that was always not supposed to work. RELOC_HIDE or __pa_symbol. -Andi ___ Virtualization mailing list Virtualization@lists.linux-foundation.org

Re: [patch 7/9] lguest: the net driver

2007-05-09 Thread Jeff Garzik
[EMAIL PROTECTED] wrote: +static void transfer_packet(struct net_device *dev, + struct sk_buff *skb, + unsigned int peernum) +{ + struct lguestnet_info *info = dev-priv; + struct lguest_dma dma; + + skb_to_dma(skb,

Re: [patch 7/9] lguest: the net driver

2007-05-09 Thread Herbert Xu
Hi Rusty: On Wed, May 09, 2007 at 09:55:25PM +1000, Rusty Russell wrote: NO_CSUM because it really doesn't need a checksum. The LGUEST_NET_F_NOCSUM is only set for local inter-guest networking. If some guest were to route the packets outside the machine, this would be an issue,

Re: [patch 7/9] lguest: the net driver

2007-05-09 Thread Rusty Russell
On Wed, 2007-05-09 at 22:00 +1000, Herbert Xu wrote: Hi Rusty: On Wed, May 09, 2007 at 09:55:25PM +1000, Rusty Russell wrote: NO_CSUM because it really doesn't need a checksum. The LGUEST_NET_F_NOCSUM is only set for local inter-guest networking. If some guest were to route the

Re: [patch 7/9] lguest: the net driver

2007-05-09 Thread Rusty Russell
Hi Jeff, Thanks for your review. Questions below. On Wed, 2007-05-09 at 08:28 -0400, Jeff Garzik wrote: [EMAIL PROTECTED] wrote: +static void transfer_packet(struct net_device *dev, ... + hcall(LHCALL_SEND_DMA, peer_key(info,peernum), __pa(dma), 0); __pa() should not be used

Re: [patch 7/9] lguest: the net driver

2007-05-09 Thread Christoph Hellwig
On Thu, May 10, 2007 at 01:14:55AM +1000, Rusty Russell wrote: + info-peer = (void *)ioremap(info-peer_phys, info-mapsize); check for NULL Erk, good catch! Also the cast is bogus. ioremap already returns void already. Even more importantly the lack of the __iomem annotations shows

Re: [patch 7/9] lguest: the net driver

2007-05-09 Thread Jeff Garzik
Rusty Russell wrote: Hi Jeff, Thanks for your review. Questions below. On Wed, 2007-05-09 at 08:28 -0400, Jeff Garzik wrote: [EMAIL PROTECTED] wrote: +static void transfer_packet(struct net_device *dev, ... + hcall(LHCALL_SEND_DMA, peer_key(info,peernum), __pa(dma), 0);