Re: socket.c added support for unix domain socket datagram transport

[Stefano Brivio <sbrivio@redhat.com>]

On Fri, 23 Apr 2021 17:21:38 +0100
Daniel P. Berrangé <berrange@redhat.com> wrote:

> On Fri, Apr 23, 2021 at 05:29:40PM +0200, Stefano Brivio wrote:
> > Hi Ralph,
> > 
> > On Fri, 23 Apr 2021 08:56:48 +0200
> > Ralph Schmieder <ralph.schmieder@gmail.com> wrote:
> >   
> > > Hey...  new to this list.  I was looking for a way to use Unix domain
> > > sockets as a network transport between local VMs.
> > > 
> > > I'm part of a team where we run dozens if not hundreds of VMs on a
> > > single compute instance which are highly interconnected.
> > > 
> > > In the current implementation, I use UDP sockets (e.g. something like 
> > > 
> > > -netdev
> > > id=bla,type=socket,udp=localhost:1234,localaddr=localhost:5678) 
> > > 
> > > which works great.
> > > 
> > > The downside of this approach is that I need to keep track of all the
> > > UDP ports in use and that there's a potential for clashes.  Clearly,
> > > having Unix domain sockets where I could store the sockets in the
> > > VM's directory would be much easier to manage.
> > > 
> > > However, even though there is some AF_UNIX support in net/socket.c,
> > > it's
> > > 
> > > - not configurable
> > > - it doesn't work  
> > 
> > I hate to say this, but I've been working on something very similar
> > just in the past days, with the notable difference that I'm using
> > stream-oriented AF_UNIX sockets instead of datagram-oriented.
> > 
> > I have a similar use case, and after some experiments I picked a
> > stream-oriented socket over datagram-oriented because:
> > 
> > - overhead appears to be the same
> > 
> > - message boundaries make little sense here -- you already have a
> >   32-bit vnet header with the message size defining the message
> >   boundaries
> > 
> > - datagram-oriented AF_UNIX sockets are actually reliable and there's
> >   no packet reordering on Linux, but this is not "formally" guaranteed
> > 
> > - it's helpful for me to know when a qemu instance disconnects for any
> >   reason  
> 
> The current IP socket impl for the net socket backend uses SOCK_DGRAM,
> so from a consistency POV it feels sensible todo the same for UNIX
> sockets too.

That's just for UDP though -- it also supports TCP with the "connect="
parameter, and given that a stream-oriented AF_UNIX socket behaves very
similarly, I recycled that parameter and just extended that bit of
documentation.

> None the less, your last point in particular about wanting to know
> about disconnects feels valid, and if its useful to you for UNIX
> sockets, then it ought to be useful for IP sockets too.
> 
> IOW, I wonder if  we should use DGRAM for UNIX sockets too by default
> to match current behaviour, but then also add a CLI option that allows
> choice of DGRAM vs STREAM, and wire that up for IP & UNIX sockets.

The choice would only apply to AF_UNIX (that is, not to TCP and UDP).

The current situation isn't entirely consistent, because for TCP you
have "connect=", for UDP it's "udp=" or "mcast=", and I'm extending the
"connect=" case to support stream-oriented AF_UNIX, which I think is
consistent.

However, to have it symmetric for the datagram-oriented case
(UDP and AF_UNIX), ideally it should be changed to
"dgram=host:port|path" -- which I guess we can't do.

I see two alternatives:

1.
  - "connect=" (TCP only)
  - "unix=path,type=dgram|stream"
  - "udp=" (UDP only)

2.
  - "connect=" (TCP and AF_UNIX stream)
  - "unix_dgram="
  - "udp=" (UDP only)

The major thing I like of 2. is that we save some code and a further
option, but other than that I don't have a strong preference.

-- 
Stefano