Re: [PATCH v8 07/14] net: stream: add unix socket

[Laurent Vivier]

On 9/14/22 07:16, David Gibson wrote:
> On Tue, Sep 13, 2022 at 08:39:53AM +0200, Laurent Vivier wrote:
> > Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> > Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
> > ---
> >   net/stream.c    | 108 +++++++++++++++++++++++++++++++++++++++++++++---
> >   qapi/net.json   |   2 +-
> >   qemu-options.hx |   1 +
> >   3 files changed, 105 insertions(+), 6 deletions(-)
> >
> > diff --git a/net/stream.c b/net/stream.c
> > index e8afbaca50b6..0f91ff20df61 100644
> > --- a/net/stream.c
> > +++ b/net/stream.c
> > @@ -235,7 +235,7 @@ static NetStreamState 
> > *net_stream_fd_init_stream(NetClientState *peer,
> >   static void net_stream_accept(void *opaque)
> >   {
> >       NetStreamState *s = opaque;
> > -    struct sockaddr_in saddr;
> > +    struct sockaddr_storage saddr;
> >       socklen_t len;
> >       int fd;
> >   
> > @@ -253,9 +253,27 @@ static void net_stream_accept(void *opaque)
> >       s->fd = fd;
> >       s->nc.link_down = false;
> >       net_stream_connect(s);
> > -    snprintf(s->nc.info_str, sizeof(s->nc.info_str),
> > -             "connection from %s:%d",
> > -             inet_ntoa(saddr.sin_addr), ntohs(saddr.sin_port));
> > +    switch (saddr.ss_family) {
> > +    case AF_INET: {
> > +        struct sockaddr_in *saddr_in = (struct sockaddr_in *)&saddr;
> > +
> > +        snprintf(s->nc.info_str, sizeof(s->nc.info_str),
> > +                 "connection from %s:%d",
> > +                 inet_ntoa(saddr_in->sin_addr), ntohs(saddr_in->sin_port));
> > +        break;
> > +    }
> > +    case AF_UNIX: {
> > +        struct sockaddr_un saddr_un;
> > +
> > +        len = sizeof(saddr_un);
> > +        getsockname(s->listen_fd, (struct sockaddr *)&saddr_un, &len);
>
> It seems like there's an asymmetry here in that we've gotten the
> connection address outside the switch for inet sockets, but we need an
> additional getsockname() for unix sockets.  Am I missing something?

This is because unix socket are not symmetric.

connect() returns the peer socket address.

With inet, server is waiting on, for instance, 127.0.0.1:1234 and peer address will appear 
to be something like 127.0.0.1:33946. So info_str will be set with "127.0.0.1:33946".

In case of unix, server is waiting on, for instance, /tmp/qemu0 and there is no peer 
address as the client connect to this same file. So the sun_path is empty when it returns 
from accept(). So to set the path of the socket (sun_path) in info_str I use the 
getsockname() to retrieve the server sun_path.

Thanks,
Laurent