qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Stefano Brivio <sbrivio@redhat.com>
Cc: Laurent Vivier <lvivier@redhat.com>,
	qemu-devel@nongnu.org,
	Ralph Schmieder <ralph.schmieder@gmail.com>,
	Markus Armbruster <armbru@redhat.com>
Subject: Re: [RFC PATCH 0/6] qapi: net: add unix socket type support to netdev backend
Date: Tue, 10 May 2022 10:22:48 +0100	[thread overview]
Message-ID: <Ynou6ASuaaP9aVCB@redhat.com> (raw)
In-Reply-To: <20220510105908.607c4de7@elisabeth>

On Tue, May 10, 2022 at 10:59:08AM +0200, Stefano Brivio wrote:
> On Tue, 10 May 2022 09:26:39 +0100
> Daniel P. Berrangé <berrange@redhat.com> wrote:
> 
> > On Mon, May 09, 2022 at 07:36:12PM +0200, Laurent Vivier wrote:
> > > "-netdev socket" only supports inet sockets.
> > > 
> > > It's not a complex task to add support for unix sockets, but
> > > the socket netdev parameters are not defined to manage well unix
> > > socket parameters.
> > > 
> > > As discussed in:
> > > 
> > >   "socket.c added support for unix domain socket datagram transport"
> > >   https://lore.kernel.org/qemu-devel/1C0E1BC5-904F-46B0-8044-68E43E67BE60@gmail.com/
> > > 
> > > This series adds support of unix socket type using SocketAddress QAPI structure.
> > > 
> > > A new netdev backend "socket-ng" is added, that is barely a copy of "socket"
> > > backend but it uses the SocketAddress QAPI to provide socket parameters.
> > > And then it also implement unix sockets (TCP and UDP).  
> > 
> > So pulling in the QAPI from patch 2
> > 
> >    { 'enum': 'NetdevSocketNGMode',
> >      'data':  [ 'dgram', 'server', 'client' ] }
> > 
> >    { 'struct': 'NetdevSocketNGOptions',
> >      'data': {
> >        'mode':    'NetdevSocketNGMode',
> >        '*addr':   'SocketAddress',
> >        '*remote': 'SocketAddress',
> >        '*local':  'SocketAddress' } }
> > 
> > > Some examples of CLI syntax:
> > > 
> > >   for TCP:
> > > 
> > >   -netdev socket-ng,id=socket0,mode=server,addr.type=inet,addr.host=localhost,addr.port=1234
> > >   -netdev socket-ng,id=socket0,mode=client,addr.type=inet,addr.host=localhost,addr.port=1234
> > > 
> > >   -netdev socket-ng,id=socket0,mode=dgram,\
> > >           local.type=inet,local.host=localhost,local.port=1234,\
> > >           remote.type=inet,remote.host=localhost,remote.port=1235
> > > 
> > >   for UNIX:
> > > 
> > >   -netdev socket-ng,id=socket0,mode=server,addr.type=unix,addr.path=/tmp/qemu0
> > >   -netdev socket-ng,id=socket0,mode=client,addr.type=unix,addr.path=/tmp/qemu0
> > > 
> > >   -netdev socket-ng,id=socket0,mode=dgram,\
> > >           local.type=unix,local.path=/tmp/qemu0,\
> > >           remote.type=unix,remote.path=/tmp/qemu1
> > > 
> > >   for FD:
> > > 
> > >   -netdev socket-ng,id=socket0,mode=server,addr.type=fd,addr.str=4
> > >   -netdev socket-ng,id=socket0,mode=client,addr.type=fd,addr.str=5
> > > 
> > >   -netdev socket-ng,id=socket0,mode=dgram,local.type=fd,addr.str=4  
> > 
> >                                                           ^^^ local.str=4
> > 
> > I notice that in all these examples, mode=client/server always use
> > the 'addr' field, and mode=dgram always uses the 'local'/'remote'
> > fields. IOW, there is almost no commonality between the dgram scenario
> > and the stream scenario, which feels sub-optimal.
> > 
> > Two alternatives come to mind
> > 
> >  - mode=client could use 'remote' and mode=server could use 'local',
> >    removing the 'addr' field entirely
> 
> To me, "mode is client, address is x" sounds more intuitive than "mode
> is client, remote is x". I mean, of course it's the remote address --
> that's a bit redundant.
> 
> >  - Have completely separate backends, ie '-netdev stream' for
> >    client/server TCP/UNIX sockets, and '-netdev dgram' for UDP
> >    sockets, removing 'mode' field.
> 
> ...this won't work, though, because UNIX domain sockets can be
> stream-oriented or datagram-oriented.

Sure it can work, both the 'stream' and 'dgram' backends would
allow the full range of addr types as they're independant config
dimensions


    -netdev stream,server=no,addr.type=inet,addr.host=localhost,addr.port=1234
    -netdev stream,server=no,addr.type=unix,addr.path=/some/stream/sock


    -netdev dgram,id=ndev0,\
            local.type=inet,local.host=localhost,local.port=1234,\
            remote.type=inet,remote.host=localhost,remote.port=1235
    -netdev dgram,id=ndev0,\
            local.type=unix,local.path=/some/dgram/sock0,
            remote.type=unix,remote.path=/some/dgram/sock1


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



  reply	other threads:[~2022-05-10  9:36 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-09 17:36 [RFC PATCH 0/6] qapi: net: add unix socket type support to netdev backend Laurent Vivier
2022-05-09 17:36 ` [RFC PATCH 1/6] net: introduce convert_host_port() Laurent Vivier
2022-05-10 21:24   ` Stefano Brivio
2022-05-11 15:54     ` Laurent Vivier
2022-05-09 17:36 ` [RFC PATCH 2/6] qapi: net: add socket-ng netdev Laurent Vivier
2022-05-10 21:24   ` Stefano Brivio
2022-05-11 14:33     ` Laurent Vivier
2022-05-09 17:36 ` [RFC PATCH 3/6] net: socket-ng: add unix socket for server and client mode Laurent Vivier
2022-05-09 17:36 ` [RFC PATCH 4/6] net: socket-ng: make dgram_dst generic Laurent Vivier
2022-05-10 21:24   ` Stefano Brivio
2022-05-09 17:36 ` [RFC PATCH 5/6] net: socket-ng: move mcast specific code from net_socket_fd_init_dgram() Laurent Vivier
2022-05-09 17:36 ` [RFC PATCH 6/6] net: socket-ng: add unix socket for dgram mode Laurent Vivier
2022-05-10  8:26 ` [RFC PATCH 0/6] qapi: net: add unix socket type support to netdev backend Daniel P. Berrangé
2022-05-10  8:59   ` Stefano Brivio
2022-05-10  9:22     ` Daniel P. Berrangé [this message]
2022-05-10 10:09       ` Stefano Brivio
2022-05-10 10:10       ` Ralph Schmieder
2022-05-10  9:47   ` Laurent Vivier

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Ynou6ASuaaP9aVCB@redhat.com \
    --to=berrange@redhat.com \
    --cc=armbru@redhat.com \
    --cc=lvivier@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=ralph.schmieder@gmail.com \
    --cc=sbrivio@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).