Re: USB Xen Summit status summary

[Anthony Liguori <aliguori@us.ibm.com>]

Harry Butterworth wrote:

>Would someone please explain why people thought the xenidc code is
>unlikely to get accepted into mainline?
>
It's the sheer volume of code.  The size of your xenidc patch is on par 
with early versions of the hypervisor in terms of SLOCs.

Abstractions without a clear need for the abstraction is heavily frowned 
upon in Linux.  It's just way too much code without a compelling 
justification for what it's needed.

Regards,

Anthony Liguori

>  It seems to me that Linux has a
>common network stack for the different network card drivers and a common
>USB stack for the different USB devices---I don't see why Xen shouldn't
>have a common inter-domain communication stack for the different virtual
>xen devices.  Is it that there are style issues remaining?  Or perhaps
>it's the problem of better integrating the code with the current Xen
>infrastructure.
>
>I did spend a week or so trying to work out what the best way to
>integrate was and came to the conclusion that the best bet (with minimum
>impact to ongoing development) was to leave the existing xen drivers and
>API in place and write a new set of drivers to the new API on the side
>making the choice of old or new driver selectable in the Kconfig.  After
>the new drivers were stable it would be possible to remove the old ones
>and then remove the old API and coalesce the infrastructure code.  The
>other approach of attempting to make atomic incremental changes to the
>existing API and all the drivers is just going to destabilise the tree
>and impact out-of-tree development too much.
>
>  
>
>>- API code is orthogonal to USB driver piece, should be
>>   a seperate patch/discussion [consensus]
>>    
>>
>
>I needed a stable API to code the driver to which is why I did both
>together.  Now the native Xen API is more stable I'm doing a version of
>the USB driver with the xenidc code factored out.
>
>  
>
>>- Best option is (2), rewrite code to leaner, simpler
>>   USB driver with minimal functionality, and get that into
>>   tree
>>    
>>
>
>Factoring out the xenidc code just means duplicating the function it
>provided in the usbback and usbfront directories, replacing the xenidc
>channel code with a version constructed using the ring macros and making
>use of Ewan's state change info to drive connect/disconnect.
>
>Whilst the sum of xenidc and the USB driver code will be reduced I'm not
>sure that the USB driver itself is actually going to get that much
>smaller with the low-level comms code put back into it.
>
>  
>
>>- Noone in session had looked at USBoverIp patches
>>- There were some good ideas in the IDC API that needed
>>   to be discussed/incorporated in Xen
>>
>>Other input/questions received:
>>- Need to get USB community input
>>    
>>
>
>I have had some input on the design which has been helpful. I posted a
>code snippet too which led to a discussion about changing Linux to make
>it possible to bind to devices rather than interfaces.
>
>  
>
>>- What were the issues that were left? Are they resolved?
>>   If so, what's the current working state of the patch?
>>    
>>
>
>No, I have wasted too much time on the Xen APIs to have got around to
>the remaining issues yet.  In particular, a correct solution to the
>problem of stalling and retrying URBs during error recovery looks fairly
>tricky.  Also changes are required in Linux to allow binding to a USB
>device rather than an interface so as to allow the correct handling of
>devices with multiple configurations.
>
>Having said this, I don't think the remaining USB issues should be the
>thing that stops the driver from going into the tree.  My intention was
>always just to get the basic function working and then let it get
>improved afterwards.  The problem has been syncing up with the xen
>driver infrastructure.
>
>The current state of the patch is that when I last posted it it was
>working for my test USB devices (USB key, keyboard) against the then
>current unstable tree.  Since then the unstable tree has moved on---the
>xenbus API has changed slightly, all the header files have been moved
>and the Kconfig file has been split up---the version of the driver that
>I'm factoring xenidc out of compiles but is currently untested.
>
>  
>
>>- Keir: rewrite to a simpler driver without the IDC API
>>   as the xenbus/store stuff is pretty baked into Xen now,
>>    
>>
>
>The xenbus/xenstore APIs are pretty bad from the device driver side.  It
>would be a shame to have to live with them indefinitely.
>
>In particular, I think that the following issues need to be addressed:
>
>1) The RING macros and interrupt handlers for inter-domain messages are
>too difficult to use. The interrupt handler code to service the ring
>queue correctly is pretty subtle and it's totally unnecessary to have
>this functionality duplicated in each driver.
>
>2) The low-level memory manipulation in drivers is unnecessary.  Linux
>has DMA APIs to do DMA.  Xen should have equivalent high-level APIs for
>inter-domain bulk data transport.  I pointed this out at the first Xen
>summit and Christian claimed it was impossible because each driver
>required specific performance optimisations but I think the xenidc code
>demonstrates that it is actually quite easy to achieve without having to
>sacrifice performance.
>
>3) Marshalling parameters from xenstore.  This wasn't addressed by
>xenidc but it's quite clear that the driver-side xenstore API is not a
>good match for the C programming language.  On device create, it would
>be much easier to get parameters in a struct.  If it's necessary to
>register an XML definition of the struct expected that would be OK.
>
>4) Sequencing operations using xenstore. This is a real problem because
>of the level-sensitive nature of watches on the store.  From the
>driver's perspective if there's going to be a hot configuration change,
>the easiest interface to deal with is receiving the reconfiguration
>request and all the parameters in one operation in a struct.  Again,
>having to marshall the parameters individually and then work out which
>ones have changed and what configuration request that implies is just
>too painful.
>
>5) The xenbus otherend_changed state machine API.  This API is largely a
>result of having half of the communication channel implementation in
>xenbus and the other half in the drivers.  Xenidc demonstrates a better
>channel implementation and a better driver API here.
>
>6) I'm not sure that the function in the xenbus state machine which
>drives a clean shutdown from the back-end closing is useful because the
>FE can just flush if it wants to do a clean shutdown.  On the other
>hand, for multi-pathing device drivers it's more normal for FE's to have
>to deal cleanly with unclean termination of a channel BE which isn't
>catered for in the xenbus state machine.
>
>7) Exposing the implementation details of everything in xenstore isn't
>good from the point of view of creating a well defined API for third
>parties.  I get the impression this is being addressed by using XML-RPC
>to define a public interface to xend which I think is probably a good
>thing.
>
>  
>
>>   might want to do some cleanups in this area.
>>- Ian: look at USBoverIP, tried it and it seems to
>>   work, but not sure if that's the right solution
>>
>>Current Issues/Design questions:
>>- Harry's code supports back/front module load/unload
>>   (useful during development, if nothing else).
>>    
>>
>
>Without working driver domains I was finding module unload of the
>back-end useful because it saves a 5 min reboot.  Driver domains are not
>quite as useful as module unload because you have to reboot both the FE
>and BE domains whereas with module unload you can leave the FE set up.
>
>  
>
>>- Harry's code is not written to Ewan's last common
>>   code pullout API
>>- What other code functionality can be dropped in order
>>   to make it smaller?
>>    
>>
>
>A fair amount of code comes from doing pre-allocation of resources and
>resource management.  This means that once the module is initialised it
>has all the resources required to guarantee progress is made with normal
>operation.  If I drop all the resource management stuff and just do
>allocations on the I/O path and allow I/O failures due to ENOMEM then I
>can make the code simpler.
>
>I think the current bounded approach is better though.
>
>  
>
>>[All misrepresentations and errors are mine, I'm operating
>>  from memory and on occasion what I heard over the crowd noise :)]
>>
>>Hope that initiates the necessary conversation on this...
>>    
>>
>
>Thanks Niv, very useful.
>
>  
>
>>thanks,
>>Nivedita
>>
>>
>>_______________________________________________
>>Xen-devel mailing list
>>Xen-devel@lists.xensource.com
>>http://lists.xensource.com/xen-devel
>>
>>    
>>
>
>
>_______________________________________________
>Xen-devel mailing list
>Xen-devel@lists.xensource.com
>http://lists.xensource.com/xen-devel
>
>  
>