Re: [Xen-devel] Re: [PATCH 3/4] [Net] Support Xen accelerated network plugin modules
On 22/5/07 08:28, Kieran Mansley [EMAIL PROTECTED] wrote: On Tue, 2007-05-22 at 08:15 +0100, Kieran Mansley wrote: RCU on its own wouldn't prevent the accelerated plugin being unloaded while netfront was using one of the hooks. Hmm, actually I think it could be used to do that. I'll take a look. Eagerly zap the function pointers, then wait one RCU period so every CPU goes through a quiescent point before unloading the module? -- Keir - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Xen-devel] Re: [PATCH 3/4] [Net] Support Xen accelerated network plugin modules
On 22/5/07 13:44, Kieran Mansley [EMAIL PROTECTED] wrote: Eagerly zap the function pointers, then wait one RCU period so every CPU goes through a quiescent point before unloading the module? -- Keir Am I right in thinking that if one of the functions that was protected by RCU was to block, that would be a bad thing? Clearly the data path hooks can't/don't block, but I'm not sure it's so obvious for things like probing a new device. Are there still module reference counts? If so, functions which may block can manipulate their module's reference count. Or if not, I guess the accelerator module can have a private reference count checked by whatever unload function gets called from the RCU subsystem. So that unload becomes deferred until *both* an RCU phase has passed *and* a reference count has fallen to zero. -- Keir - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Xen-devel] [PATCH 0/4] [Net] Support accelerated network plugin modules
On 15/6/07 11:46, Kieran Mansley [EMAIL PROTECTED] wrote:
This is a repost of some earlier patches to the xen-devel mailing list,
with a number of changes thanks to some useful suggestions from others.
The coding style needs fixing in various ways.
Hard tabs need to be used, no spaces inside brackets, but should include
spaces between if/while/for and bracket, and bracket and brace:
if (foo) {
Not
if( foo ){
if(foo ) {
Or various other possibilities.
No use of the following please:
If (foo) return 1; else return 0;
Is clearer as:
Return foo;
There's a (good) idiom for error handling in Linux:
If (!error) {
page of code;
} else {
oh no, error, print and return;
}
Should be changed to:
If (error) {
handle error, bail if necessary
}
Page of code, no longer indented by error check;
Apart from the various coding style bits, accelerator_probe_vifs_on_load()
walks the vif_states list with no lock held. Is this safe against the list
changing?
-- Keir
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Xen-devel] [PATCH 0/4] [Net] Support accelerated network plugin modules
On 15/6/07 13:11, Michael Buesch [EMAIL PROTECTED] wrote: No use of the following please: If (foo) return 1; else return 0; Is clearer as: Return foo; But it's not the same. return !!foo; would be the same. And yes, it does matter: True in general, but not the cases I've seen in this patchset, where 'foo' is a predicate. -- Keir - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Xen-devel] [PATCH 0/4] [Net] Support accelerated network plugin modules
On 15/6/07 13:21, Michael Buesch [EMAIL PROTECTED] wrote: True in general, but not the cases I've seen in this patchset, where 'foo' is a predicate. Ok, if foo is a variable containing an error code, it's better to return that error code. I assumed that foo is a variable containing some value (counter or something). I mean specifically things like: if (be-accelerator == NULL) return 1; else return 0; K. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Xen-devel] [PATCH 4/4] [Net] Support accelerated network plugin modules
On 15/6/07 17:22, Kieran Mansley [EMAIL PROTECTED] wrote: The lock protects the use_count variable. Yes, that's one thing I noticed -- can you use atomic_t for reference counts and hence reduce the number of times you need to lock/unlock? At least the open-coded lock-decrement-test-maybe-free-unlock sequences could be abstracted into a put_foo() function. -- Keir - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 34/35] Add the Xen virtual network device driver.
On 9 May 2006, at 21:25, Stephen Hemminger wrote: + memcpy(netdev-dev_addr, info-mac, ETH_ALEN); + network_connect(netdev); + info-irq = bind_evtchn_to_irqhandler( + info-evtchn, netif_int, SA_SAMPLE_RANDOM, netdev-name, This doesn't look like a real random entropy source. packets arriving from another domain are easily timed. Where should we get our entropy from in a VM environment? Leaving the pool empty can cause processes to hang. -- Keir - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Xen-devel] [RFC PATCH 34/35] Add the Xen virtual network device driver.
On 10 May 2006, at 00:51, Chris Wright wrote: * Herbert Xu ([EMAIL PROTECTED]) wrote: Chris Wright [EMAIL PROTECTED] wrote: + netdev-features= NETIF_F_IP_CSUM; Any reason why IP_CSUM was chosen instead of HW_CSUM? Doing the latter would seem to be in fact easier for a virtual driver, no? That, I really don't know. Checksum offload was added late to the virtual transport and currently not enough info is carried to identify protocol checksum fields in arbitrary locations. When we rev the virtual interface, and include a proper checksum-offset field, we'll be able to switch to NETIF_F_HW_CSUM. -- Keir - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 34/35] Add the Xen virtual network device driver.
On 11 May 2006, at 01:33, Herbert Xu wrote: But if sampling virtual events for randomness is really unsafe (is it really?) then native guests in Xen would also get bad random numbers and this would need to be somehow addressed. Good point. I wonder what VMWare does in this situation. Well, there's not much they can do except maybe jitter interrupt delivery. I doubt they do that though. The original complaint in our case was that we take entropy from interrupts caused by other local VMs, as well as external sources. There was a feeling that the former was more predictable and could form the basis of an attack. I have to say I'm unconvinced: I don't really see that it's significantly easier to inject precisely-timed interrupts into a local VM. Certainly not to better than +/- a few microseconds. As long as you add cycle-counter info to the entropy pool, the least significant bits of that will always be noise. The alternatives are unattractive: 1. We have no good way to distinguish interrupts caused by packets from local VMs versus packets from remote hosts. Both get muxed on the same virtual interface. 2. An entropy front/back is tricky -- how do we decide how much entropy to pull from domain0? How much should domain0 be prepared to give other domains? How easy is it to DoS domain0 by draining its entropy pool? Yuk. -- Keir - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Arp announce (for Xen)
On 2/3/07 01:30, Stephen Hemminger [EMAIL PROTECTED] wrote: What about implementing the unused arp_announce flag on the inetdevice? Something like the following. Totally untested... Looks like it either was there (and got removed) or was planned but never implemented. This would be acceptable. It only needs a simple script to poke the correct value, and that could be easily integrated into vendor scripts. The general idea of being able to have a callback to userspace is okay but we'd like something lighter weight for live migration, which is typically done only within the scope of a single layer-2 network and hence all that is really required is the gratuitous ARP to kick the switch hardware. -- Keir - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 32/33] Add the Xen virtual network device driver.
On 18 Jul 2006, at 11:27, Arjan van de Ven wrote: Hmmm maybe it's me, but something bugs me if a NIC driver is going to send IP level ARP packets... that just feels very very wrong and is a blatant layering violation shouldn't the ifup/ifconfig scripts just be fixed instead if this is critical behavior? Maybe we should be faking this out from our hotplug scripts in the control VM, although triggering this from user space is probably a bit of a pain. Regardless, the function can be removed from the driver if it's too distasteful: it's only a performance 'hack' to get network traffic more quickly redirected when migrating a VM between physical hosts. Things won't break horribly if it's removed. -- Keir - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
