Re: [PATCH v7 05/14] qapi: net: add stream and dgram netdevs

[Laurent Vivier]

On 02/08/2022 10:37, Markus Armbruster wrote:
> Laurent Vivier <lvivier@redhat.com> writes:
...
> > diff --git a/qemu-options.hx b/qemu-options.hx
> > index 79e00916a11f..170117e1adf0 100644
> > --- a/qemu-options.hx
> > +++ b/qemu-options.hx
> > @@ -2726,6 +2726,18 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
> >       "-netdev socket,id=str[,fd=h][,udp=host:port][,localaddr=host:port]\n"
> >       "                configure a network backend to connect to another 
> > network\n"
> >       "                using an UDP tunnel\n"
> > +    "-netdev 
> > stream,id=str[,server=on|off],addr.type=inet,addr.host=host,addr.port=port\n"
> > +    "-netdev stream,id=str[,server=on|off],addr.type=fd,addr.str=h\n"
> > +    "                configure a network backend to connect to another 
> > network\n"
> > +    "                using a socket connection in stream mode.\n"

From v6:
> This part needs to match NetdevStreamOptions above.
>
> Missing here: the optional members of InetSocketAddress: numeric, to,
> ipv4, ...  Do we care?

At this patch level, no, because we decode them manually and not using 
socket_connect()/socket_listen(). But the doc should be updated for PATCH 13/14 as I move 
stream.c to QIO.

> The next part needs to match NetdevDgramOptions above.


> > +    "-netdev 
> > dgram,id=str,remote.type=inet,remote.host=maddr,remote.port=port[,local.type=inet,local.host=addr]\n"
> > +    "-netdev 
> > dgram,id=str,remote.type=inet,remote.host=maddr,remote.port=port[,local.type=fd,local.str=h]\n"
> > +    "                configure a network backend to connect to a multicast maddr 
> > and port\n"
> > +    "                use ``local.host=addr`` to specify the host address to send 
> > packets from\n"

From v6:
> I figure this covers table rows
>
>    #    @remote         @local      |   okay?
>    #    ----------------------------+--------
>    #    multicast       absent      |   yes
>    #    multicast       present     |   yes
>
> for remote.type=inet and any local.type.
>
> What about remote.type=fd?

multicast is only supported with remote.type=inet, not fd or unix

In net_dgram_init(), we initiate a multicast connection if remote.type is inet and address 
type is multicast, otherwise it's an unicast connection.

> > +    "-netdev 
> > dgram,id=str,local.type=inet,local.host=addr,local.port=port[,remote.type=inet,remote.host=addr,remote.port=port]\n"

From v6:
> I figure this covers table rows
>
>     #    absent          present     |   yes
>     #    not multicast   present     |   yes
>
> for *.type=inet.


> > +    "-netdev dgram,id=str,local.type=fd,local.str=h\n"
> > +    "                configure a network backend to connect to another 
> > network\n"
> > +    "                using an UDP tunnel\n"

From v6:
> I figure this covers table row
>
>    #    absent          present     |   yes
>
> for local.type=fd.
>
> Together, they cover table row
>
>     #    absent          present     |   yes
>
> for any local.type.  Good.
>
> Table row
>
>     #    not multicast   present     |   yes
>
> is only covered for *.type=inet.  Could either of the types be fd?

In v7, I've update the table to include the case of fd:

    =============  ========  =====
    remote         local     okay?
    =============  ========  =====
    absent         absent    no
    absent         not fd    no
    absent         fd        yes
    multicast      absent    yes
    multicast      present   yes
    not multicast  absent    no
    not multicast  present   yes
    =============  ========  =====

For local, if it's not specified otherwise, fd is supported.
Remote and local type must be the same (inet or unix), if local is fd, remote must not be 
provided.

Thanks,
Laurent