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

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

On 28/06 01:36, Ian Campbell wrote:
> On Thu, 2012-06-28 at 13:10 +0100, Jean Guyader wrote:
> > On 28/06 12:58, Ian Campbell wrote:
> > > On Thu, 2012-06-28 at 12:43 +0100, Jean Guyader wrote:
> > > > 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.
> > > 
> > > An application using TCP doesn't need to worry about choosing its own
> > > source port though.
> > > 
> > > Or does this only effect destination / listening ports?
> > > 
> > 
> > The guest v4v driver knows which port are in used so if you put port 0
> > we will pick a random unused number for the source port.
> 
> Except when there are two such drivers each doesn't know which the other
> one is using.
> 

Then the kernel will try to register the ring and the hypercall will fail
because it's already registered.

Jean