> From: Stefan Hajnoczi [mailto:stefa...@redhat.com]
> On Fri, Aug 18, 2017 at 10:23:54PM +0000, Dexuan Cui wrote:
> > > > +static bool hvs_stream_allow(u32 cid, u32 port)
> > > > +{
> > > > + static const u32 valid_cids[] = {
> > > > + VMADDR_CID_ANY,
> > >
> > > Is this for loopback?
> >
> > No, we don't support lookback in Linux VM, at least for now.
> > In our Linux implementation, Linux VM can only connect to the host, and
> > here when Linux VM calls connect(), I treat VMADDR_CID_ANY
> > the same as VMADDR_CID_HOST.
>
> VMCI and virtio-vsock do not treat connect(VMADDR_CID_ANY) the same as
> connect(VMADDR_CID_HOST). It is an error to connect to VMADDR_CID_ANY.
Ok. Then I'll only allow VMADDR_CID_HOST as the destination CID, since
we don't support loopback mode.
> > > > + /* The host's port range [MIN_HOST_EPHEMERAL_PORT, 0xFFFFFFFF)
> > > is
> > > > + * reserved as ephemeral ports, which are used as the host's
> > > > ports
> > > > + * when the host initiates connections.
> > > > + */
> > > > + if (port > MAX_HOST_LISTEN_PORT)
> > > > + return false;
> > >
> > > Without this if statement the guest will attempt to connect. I guess
> > > there will be no listen sockets above MAX_HOST_LISTEN_PORT, so the
> > > connection attempt will fail.
> >
> > You're correct.
> > To use the vsock common infrastructure, we have to map Hyper-V's
> > GUID <VM_ID, Service_ID> to int <cid, port>, and hence we must limit
> > the port range we can listen() on to [0, MAX_LISTEN_PORT], i.e.
> > we can only use half of the whole 32-bit port space for listen().
> > This is detailed in the long comments starting at about Line 100.
> >
> > > ...but hardcode this knowledge into the guest driver?
> > I'd like the guest's connect() to fail immediately here.
> > IMO this is better than a connect timeout. :-)
>
> Thanks for explaining. Perhaps the comment could be updated:
>
> /* The host's port range [MIN_HOST_EPHEMERAL_PORT, 0xFFFFFFFF) is
> * reserved as ephemeral ports, which are used as the host's ports when
> * the host initiates connections.
> *
> * Perform this check in the guest so an immediate error is produced
> * instead of a timeout.
> */
>
> Stefan
Thank you, Stefan!
Please see the below for the updated version of the function:
static bool hvs_stream_allow(u32 cid, u32 port)
{
/* The host's port range [MIN_HOST_EPHEMERAL_PORT, 0xFFFFFFFF) is
* reserved as ephemeral ports, which are used as the host's ports
* when the host initiates connections.
*
* Perform this check in the guest so an immediate error is produced
* instead of a timeout.
*/
if (port > MAX_HOST_LISTEN_PORT)
return false;
if (cid == VMADDR_CID_HOST)
return true;
return false;
}
I'll send a v2 of the patch later today.
-- Dexuan
