Re: [RFC][PATCH 0/5] Add V4V to Xen

[Jean Guyader <jean.guyader@citrix.com>]

On 28/06 12:34, Ian Campbell wrote:
> On Thu, 2012-06-28 at 11:38 +0100, Jean Guyader wrote:
> > On 26/06 03:38, Ian Campbell wrote:
> > > Hi,
> > > 
> > > Sorry it's taken me so long to get round to responding to this.
> > > 
> > > On Mon, 2012-06-25 at 10:05 +0100, Tim Deegan wrote:
> > > > At 22:14 +0100 on 14 Jun (1339712061), Jean Guyader wrote:
> > > > > On 14 June 2012 16:35, Tim Deegan <tim@xen.org> wrote:
> > > > > > At 16:10 +0100 on 14 Jun (1339690244), Jean Guyader wrote:
> > > > > >> On 14/06 03:56, Tim Deegan wrote:
> > > > > >> > At 11:55 +0100 on 14 Jun (1339674908), Jean Guyader wrote:
> > > > > >> > > Are you talking about having different version of V4V driver running
> > > > > >> > > in the same VM?
> > > > > >> >
> > > > > >> > Yes.
> > > > > >> >
> > > > > >> > > I don't think that is a problem they both interact with Xen via
> > > > > >> > > hypercall directly so if they follow the v4v hypercall interface it's
> > > > > >> > > all fine.
> > > > > >> >
> > > > > >> > AFAICS if they both try to register the same port then one of them will
> > > > > >> > silently get its ring discarded.  And if they both try to communicate
> > > > > >> > with the same remote port their entries on the pending lists will get
> > > > > >> > merged (which is probably not too bad).  I think the possibility for
> > > > > >> > confusion depends on how you use the service.  Still, it seems better
> > > > > >> > than the xenstore case, anyway. :)
> > > > > >> >
> > > > > >>
> > > > > >> Not silently, register_ring will return an error.
> > > > > >
> > > > > > Will it?  It looks to me like v4v_ring_add just clobbers the old MFN
> > > > > > list.
> > > > > >
> > > > > 
> > > > > Ha yes. It does that now but I think it should return an error
> > > > > informing up the stack that a ring has already been registered.
> > > > 
> > > > Actually, I think it's deliberate, to allow a guest to re-register all
> > > > its rings after a suspend/resume or migration, without having to worry
> > > > about whether it was actually migrated into a new domain or not. 
> > > 
> > > Which takes us back to the original issue Tim asked about with
> > > cohabitation of multiple (perhaps just plain buggy or even malicious)
> > > v4v clients in a single domain, doesn't it?
> > > 
> > 
> > There is nothing wrong the two v4v driver running in the same guest.
> > The probably that Tim reported was about trying to create two connections
> > on the same port. Today with the code that I've submited in the RFC
> > one will overwrite the other silently which isn't a good thing, that can
> > easily be changed to notify which one got registered up the stack.
> 
> So they'd somehow need to randomise (and retry) their use of source
> ports in order to co-exist?
>

That can be assimilated to two userspace programs trying to bind to the
same TCP port. I think it's not v4v's responsability to solve this problem.

> Speaking of ports, is there a registry somewhere of the well known port
> numbers and/or any scheme for administering these? (a text file in the
> repo would be find by me).
> 

Port numbers are 32 bits, by convention the first 65535 will match the TCP onces,
then for the rest we can have a file in the repo to reference them.

> > > If one of the reasons for not using xenstore is the inability for
> > > multiple clients to co-exist then there is no point moving to a
> > > different scheme with the same (even if lesser) issue. Up until now v4v
> > > has only been used by XenClient so it has avoided this issue but if we
> > > take it upstream then presumably the potential for this sort of problem
> > > to creep in comes back.
> > > 
> > > Some other things from my notes...
> > > 
> > > Can you remind me of the reasoning behind using VIRQ_V4V instead of
> > > regular event channels? I can't see any reason why these
> > > shouldn't/couldn't be regular event channels demuxed in the usual way.
> > > 
> > 
> > No good reason, the virq can be changed to a event channel. We thought
> > that event channels required other machinery to function. If we can
> > deal with the event channel in the v4v driver directly I don't have any
> > problem changing it to a event channel. What I have in mind is if one
> > would want to use v4v in a rombios for instance where you can't rely
> > on any of the xen pv driver code to be present.
> 
> I think that will be ok -- under the hood a VIRQ is just an evtchn too
> so if you can make the VIRQ work you can a regular evtchn work too.
> 
> > > Are you going to submit any client code? I think the most important of
> > > these would be code to make libvchan able to use v4v if it is available
> > > (and both ends agree), but any other client code (like the sockets
> > > interface overlay I've heard about) would be interesting to see too.
> > > 
> > 
> > Yes. I still have to submit the kernel driver code and the v4v library that
> > talks to it. But now that you've pointed out that we might be able to use
> > an event channel instead of a virq, I think we might event be able to have
> > a driver in userspace only.
> 
> That would be a nice property to have!
> 
> > > The patches need plenty of documentation adding to them, both in the
> > > interface docs in xen/include/public (marked up such that it comes out
> > > nicely in docs/html aka
> > > http://xenbits.xen.org/docs/unstable/hypercall/index.html) as well as
> > > design docs and any other useful material you have under docs itself.
> > > 
> > 
> > I agree such change need a lot of document. Once we agreed on the interfaces
> > I will start working on a complete documentation for them. 
> 
> Thanks!
> 

Jean