* Re: [patch 7/9] lguest: the net driver [not found] <200705090951.l499pdOr020406@shell0.pdx.osdl.net> @ 2007-05-09 10:12 ` Herbert Xu 2007-05-09 11:55 ` Rusty Russell 0 siblings, 1 reply; 4+ messages in thread From: Herbert Xu @ 2007-05-09 10:12 UTC (permalink / raw) To: akpm; +Cc: linux-kernel, virtualization, akpm, rusty, ak, jeff, jmorris, netdev akpm@linux-foundation.org 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 such changes would be desirable in this driver (i.e., not even generating a partial checksum). Also, there doesn't seem to be any passing of metadata to the backend which means that neither NO_CSUM/HW_CSUM can work if somebody needs to look at the checksum field. You could use IP_CSUM if you do the same hack on the backend that Xen does. Otherwise you'll have to make do with no checksum offload at all. I think you'd also need a change_mtu function if the SG feature is going to be of some use since the default Ethernet one limits the MTU to 1500. Cheers, -- Visit Openswan at http://www.openswan.org/ Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [patch 7/9] lguest: the net driver 2007-05-09 10:12 ` [patch 7/9] lguest: the net driver Herbert Xu @ 2007-05-09 11:55 ` Rusty Russell 2007-05-09 12:00 ` Herbert Xu 0 siblings, 1 reply; 4+ messages in thread From: Rusty Russell @ 2007-05-09 11:55 UTC (permalink / raw) To: Herbert Xu; +Cc: akpm, linux-kernel, virtualization, ak, jeff, jmorris, netdev On Wed, 2007-05-09 at 20:12 +1000, Herbert Xu wrote: > akpm@linux-foundation.org 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 such changes would > be desirable in this driver (i.e., not even generating a partial > checksum). Hi Herbert, 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, though ("don't do that"). > Also, there doesn't seem to be any passing of metadata to the > backend which means that neither NO_CSUM/HW_CSUM can work if > somebody needs to look at the checksum field. You could use > IP_CSUM if you do the same hack on the backend that Xen does. > Otherwise you'll have to make do with no checksum offload at all. Yeah, definitely good future work. This is far simpler: external-facing networks don't get marked with the LGUEST_NET_F_NOCSUM but set. KVM is going to have a paravirt network driver too: it'd be nice to share code here. > I think you'd also need a change_mtu function if the SG feature > is going to be of some use since the default Ethernet one limits > the MTU to 1500. Indeed, that would be a new feature, and is certainly a consideration for more efficient inter-guest networking. However, I consider that somewhat cheating: it's nice to know that 1500 doesn't suck too hard. Remember, "Features kill puppies!" 8) Thanks, Rusty. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [patch 7/9] lguest: the net driver 2007-05-09 11:55 ` Rusty Russell @ 2007-05-09 12:00 ` Herbert Xu 2007-05-09 12:13 ` Rusty Russell 0 siblings, 1 reply; 4+ messages in thread From: Herbert Xu @ 2007-05-09 12:00 UTC (permalink / raw) To: Rusty Russell Cc: akpm, linux-kernel, virtualization, ak, jeff, jmorris, netdev 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, though ("don't do that"). While I can see that this is good in keeping things simple, I think it's something that you want to be able to support since the user may wish to setup a guest as a firewall appliance which would involve passing packets from another guest to the outside world. > Indeed, that would be a new feature, and is certainly a consideration > for more efficient inter-guest networking. However, I consider that > somewhat cheating: it's nice to know that 1500 doesn't suck too hard. > > Remember, "Features kill puppies!" 8) Heh :) Cheers, -- Visit Openswan at http://www.openswan.org/ Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [patch 7/9] lguest: the net driver 2007-05-09 12:00 ` Herbert Xu @ 2007-05-09 12:13 ` Rusty Russell 0 siblings, 0 replies; 4+ messages in thread From: Rusty Russell @ 2007-05-09 12:13 UTC (permalink / raw) To: Herbert Xu; +Cc: akpm, linux-kernel, virtualization, ak, jeff, jmorris, netdev 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 packets outside the machine, this would be > > an issue, though ("don't do that"). > > While I can see that this is good in keeping things simple, I think > it's something that you want to be able to support since the user > may wish to setup a guest as a firewall appliance which would involve > passing packets from another guest to the outside world. Indeed, you understand the tradeoff. The example launcher could have an option not to set the LGUEST_NET_F_NOCSUM in this case. That said, one significant purpose of lguest is to serve as an example of how to do things. So if you feel really strongly that there's a Right Way, we could look at the patch... Thanks, Rusty. ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2007-05-09 12:13 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <200705090951.l499pdOr020406@shell0.pdx.osdl.net>
2007-05-09 10:12 ` [patch 7/9] lguest: the net driver Herbert Xu
2007-05-09 11:55 ` Rusty Russell
2007-05-09 12:00 ` Herbert Xu
2007-05-09 12:13 ` Rusty Russell
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).