qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v5 1/2] slirp: Add "query-usernet" QMP command


From: Fam Zheng
Subject: Re: [Qemu-devel] [PATCH v5 1/2] slirp: Add "query-usernet" QMP command
Date: Thu, 3 May 2018 17:27:35 +0800
User-agent: Mutt/1.9.2 (2017-12-15)

On Thu, 05/03 10:12, Daniel P. Berrangé wrote:
> On Thu, May 03, 2018 at 09:25:45AM +0800, Fam Zheng wrote:
> > HMP "info usernet" has been available but it isn't ideal for programmed
> > use cases. This closes the gap in QMP by adding a counterpart
> > "query-usernet" command. It is basically translated from
> > the HMP slirp_connection_info() loop, which now calls the QMP
> > implementation and prints the data, just like other HMP info_* commands.
> > 
> > The TCPS_* macros are now defined as a QAPI enum.
> > 
> > Signed-off-by: Fam Zheng <address@hidden>
> 
> 
> > diff --git a/qapi/net.json b/qapi/net.json
> > index 9117c56972..83757d0ed0 100644
> > --- a/qapi/net.json
> > +++ b/qapi/net.json
> > @@ -689,3 +689,217 @@
> >  ##
> >  { 'event': 'NIC_RX_FILTER_CHANGED',
> >    'data': { '*name': 'str', 'path': 'str' } }
> > +
> > +##
> > +# @TCPS:
> > +#
> > +# TCP States of a SLIRP connection.
> > +#
> > +# - States where connections are not established: none, closed, listen, 
> > syn-sent,
> > +#   syn-received
> > +#
> > +# - States where user has closed: fin-wait-1, closing, last-ack, 
> > fin-wait-2,
> > +#   time-wait
> > +#
> > +# - States awaiting ACK of FIN: fin-wait-1, closing, last-ack
> > +#
> > +# 'none' state is used only when host forwarding
> > +#
> > +# Since 2.13
> > +#
> > +##
> > +{ 'enum': 'TCPS',
> 
> I'd suggest avoiding the abbreviation, IOW use  TCPState as
> the name. Yes that will require changnig the constants in
> the SLIRP code, but that's worthwhile to get this QAPI
> naming clearer IMHO.
> 

Yes, it feels cleaner to me. I wanted to keep this patch short-ish but that
change can be split into a separate patch which introduces the QAPI enum and
replaces the TCPS_ macros.

I will do it in v6, thanks.

Fam

> I wonder if we should have a common prefix too eg UsernetTCPState
> as this is a global QAPI namespace after all .
> 
> > +  'data':
> > +   ['closed',
> > +    'listen',
> > +    'syn-sent',
> > +    'syn-received',
> > +    'established',
> > +    'close-wait',
> > +    'fin-wait-1',
> > +    'closing',
> > +    'last-ack',
> > +    'fin-wait-2',
> > +    'time-wait',
> > +    'none'
> > +   ] }
> > +
> > +##
> > +# @UsernetTCPConnection:
> > +#
> > +# SLIRP TCP information.
> > +#
> > +# @state: tcp connection state
> > +#
> > +# @hostfwd: whether this connection has host port forwarding
> > +#
> > +# @fd: the file descriptor of the connection
> > +#
> > +# @src-addr: source address of host port forwarding
> > +#
> > +# @src-port: source port of host port forwarding
> > +#
> > +# @dest-addr: destination address of host port forwarding
> > +#
> > +# @dest-port: destination port of host port forwarding
> > +#
> > +# @recv-buffered: number of bytes queued in the receive buffer
> > +#
> > +# @send-buffered: number of bytes queued in the send buffer
> > +#
> > +# Since: 2.13
> > +##
> > +{ 'struct': 'UsernetTCPConnection',
> > +  'data': {
> > +    'state': 'TCPS',
> > +    'hostfwd': 'bool',
> > +    'fd': 'int',
> > +    'src-addr': 'str',
> > +    'src-port': 'int',
> > +    'dest-addr': 'str',
> > +    'dest-port': 'int',
> > +    'recv-buffered': 'int',
> > +    'send-buffered': 'int'
> > +  } }
> > +
> > +##
> > +# @UsernetUDPConnection:
> > +#
> > +# SLIRP UDP information.
> > +#
> > +# @hostfwd: whether this connection has host port forwarding
> > +#
> > +# @expire-time-ms: time in microseconds after which this connection will 
> > expire
> > +#
> > +# @fd: the file descriptor of the connection
> > +#
> > +# @src-addr: source address of host port forwarding
> > +#
> > +# @src-port: source port of host port forwarding
> > +#
> > +# @dest-addr: destination address of host port forwarding
> > +#
> > +# @dest-port: destination port of host port forwarding
> > +#
> > +# @recv-buffered: number of bytes queued in the receive buffer
> > +#
> > +# @send-buffered: number of bytes queued in the send buffer
> > +#
> > +# Since: 2.13
> > +##
> > +{ 'struct': 'UsernetUDPConnection',
> > +  'data': {
> > +    'hostfwd': 'bool',
> > +    'expire-time-ms': 'int',
> > +    'fd': 'int',
> > +    'src-addr': 'str',
> > +    'src-port': 'int',
> > +    'dest-addr': 'str',
> > +    'dest-port': 'int',
> > +    'recv-buffered': 'int',
> > +    'send-buffered': 'int'
> > +    } }
> > +
> > +##
> > +# @UsernetICMPConnection:
> > +#
> > +# SLIRP ICMP information.
> > +#
> > +# @expire-time-ms: time in microseconds after which this connection will 
> > expire
> > +#
> > +# @fd: the file descriptor of the connection
> > +#
> > +# @src-addr: source address of host port forwarding
> > +#
> > +# @dest-addr: destination address of host port forwarding
> > +#
> > +# @recv-buffered: number of bytes queued in the receive buffer
> > +#
> > +# @send-buffered: number of bytes queued in the send buffer
> > +#
> > +# Since: 2.13
> > +##
> > +{ 'struct': 'UsernetICMPConnection',
> > +  'data': {
> > +    'expire-time-ms': 'int',
> > +    'fd': 'int',
> > +    'src-addr': 'str',
> > +    'dest-addr': 'str',
> > +    'recv-buffered': 'int',
> > +    'send-buffered': 'int'
> > +    } }
> > +
> > +##
> > +# @UsernetType:
> > +#
> > +# Available netdev drivers.
> > +#
> > +# Since: 2.13
> > +##
> > +{ 'enum': 'UsernetType',
> > +  'data': [ 'tcp', 'udp', 'icmp' ] }
> > +
> > +##
> > +# @UsernetConnection:
> > +#
> > +# SLIRP usernet connection information.
> > +#
> > +# Since: 2.13
> > +##
> > +{ 'union': 'UsernetConnection',
> > +  'discriminator': 'type',
> > +  'base': { 'type': 'UsernetType' },
> > +  'data': {
> > +    'tcp':     'UsernetTCPConnection',
> > +    'udp':     'UsernetUDPConnection',
> > +    'icmp':    'UsernetICMPConnection'
> > +    } }
> > +
> > +##
> > +# @UsernetInfo:
> > +#
> > +# SLIRP usernet information.
> > +#
> > +# Since: 2.13
> > +##
> > +{ 'struct': 'UsernetInfo',
> > +  'data': {
> > +    'id':              'str',
> > +    'hub':             'int',
> > +    'connections':     ['UsernetConnection']
> > +} }
> > +
> > +##
> > +# @query-usernet:
> > +#
> > +# Return SLIRP network information.
> > +#
> > +# Since: 2.13
> > +#
> > +# Example:
> > +#
> > +# -> { "execute": "query-usernet", "arguments": { } }
> > +# <- { "return": [
> > +#      {
> > +#        "hub": -1,
> > +#        "connections": [
> > +#          {
> > +#            "dest-addr": "10.0.2.15",
> > +#            "recv-buffered": 0,
> > +#            "src-port": 10022,
> > +#            "state": "closed",
> > +#            "fd": 16,
> > +#            "src-addr": "*",
> > +#            "send-buffered": 0,
> > +#            "dest-port": 22,
> > +#            "type": "tcp",
> > +#            "hostfwd": true
> > +#          }
> > +#        ],
> > +#        "id": "vnet"
> > +#      }
> > +#   ]}
> > +#
> > +##
> > +{ 'command': 'query-usernet',
> > +  'returns': ['UsernetInfo'] }
> > diff --git a/slirp/libslirp.h b/slirp/libslirp.h
> > index 540b3e5903..3ba361ea41 100644
> > --- a/slirp/libslirp.h
> > +++ b/slirp/libslirp.h
> > @@ -2,6 +2,7 @@
> >  #define LIBSLIRP_H
> >  
> >  #include "qemu-common.h"
> > +#include "qapi/qapi-commands-net.h"
> >  
> >  typedef struct Slirp Slirp;
> >  
> > @@ -37,6 +38,7 @@ int slirp_add_exec(Slirp *slirp, int do_pty, const void 
> > *args,
> >                     struct in_addr *guest_addr, int guest_port);
> >  
> >  void slirp_connection_info(Slirp *slirp, Monitor *mon);
> > +void usernet_get_info(Slirp *slirp, UsernetInfo *info);
> >  
> >  void slirp_socket_recv(Slirp *slirp, struct in_addr guest_addr,
> >                         int guest_port, const uint8_t *buf, int size);
> > diff --git a/slirp/misc.c b/slirp/misc.c
> > index 260187b6b6..335b49aad8 100644
> > --- a/slirp/misc.c
> > +++ b/slirp/misc.c
> > @@ -205,39 +205,28 @@ fork_exec(struct socket *so, const char *ex, int 
> > do_pty)
> >  }
> >  #endif
> >  
> > -void slirp_connection_info(Slirp *slirp, Monitor *mon)
> > +void usernet_get_info(Slirp *slirp, UsernetInfo *info)
> >  {
> > -    const char * const tcpstates[] = {
> > -        [TCPS_CLOSED]       = "CLOSED",
> > -        [TCPS_LISTEN]       = "LISTEN",
> > -        [TCPS_SYN_SENT]     = "SYN_SENT",
> > -        [TCPS_SYN_RECEIVED] = "SYN_RCVD",
> > -        [TCPS_ESTABLISHED]  = "ESTABLISHED",
> > -        [TCPS_CLOSE_WAIT]   = "CLOSE_WAIT",
> > -        [TCPS_FIN_WAIT_1]   = "FIN_WAIT_1",
> > -        [TCPS_CLOSING]      = "CLOSING",
> > -        [TCPS_LAST_ACK]     = "LAST_ACK",
> > -        [TCPS_FIN_WAIT_2]   = "FIN_WAIT_2",
> > -        [TCPS_TIME_WAIT]    = "TIME_WAIT",
> > -    };
> >      struct in_addr dst_addr;
> >      struct sockaddr_in src;
> >      socklen_t src_len;
> >      uint16_t dst_port;
> >      struct socket *so;
> > -    const char *state;
> > -    char buf[20];
> > -
> > -    monitor_printf(mon, "  Protocol[State]    FD  Source Address  Port   "
> > -                        "Dest. Address  Port RecvQ SendQ\n");
> > +    UsernetConnectionList **p_next = &info->connections;
> >  
> >      for (so = slirp->tcb.so_next; so != &slirp->tcb; so = so->so_next) {
> > +        UsernetConnection *conn = g_new0(UsernetConnection, 1);
> > +        UsernetTCPConnection *tcp = &conn->u.tcp;
> > +        UsernetConnectionList *list = g_new0(UsernetConnectionList, 1);
> > +
> > +        list->value = conn;
> >          if (so->so_state & SS_HOSTFWD) {
> > -            state = "HOST_FORWARD";
> > +            tcp->hostfwd = true;
> > +            tcp->state = so->so_tcpcb->t_state;
> >          } else if (so->so_tcpcb) {
> > -            state = tcpstates[so->so_tcpcb->t_state];
> > +            tcp->state = so->so_tcpcb->t_state;
> >          } else {
> > -            state = "NONE";
> > +            tcp->state = TCPS_NONE;
> >          }
> >          if (so->so_state & (SS_HOSTFWD | SS_INCOMING)) {
> >              src_len = sizeof(src);
> > @@ -250,46 +239,125 @@ void slirp_connection_info(Slirp *slirp, Monitor 
> > *mon)
> >              dst_addr = so->so_faddr;
> >              dst_port = so->so_fport;
> >          }
> > -        snprintf(buf, sizeof(buf), "  TCP[%s]", state);
> > -        monitor_printf(mon, "%-19s %3d %15s %5d ", buf, so->s,
> > -                       src.sin_addr.s_addr ? inet_ntoa(src.sin_addr) : "*",
> > -                       ntohs(src.sin_port));
> > -        monitor_printf(mon, "%15s %5d %5d %5d\n",
> > -                       inet_ntoa(dst_addr), ntohs(dst_port),
> > -                       so->so_rcv.sb_cc, so->so_snd.sb_cc);
> > +        tcp->fd = so->s;
> > +        tcp->src_addr =
> > +            g_strdup(src.sin_addr.s_addr ? inet_ntoa(src.sin_addr) : "*");
> > +        tcp->src_port = ntohs(src.sin_port);
> > +        tcp->dest_addr = g_strdup(inet_ntoa(dst_addr));
> > +        tcp->dest_port = ntohs(dst_port);
> > +        tcp->recv_buffered = so->so_rcv.sb_cc;
> > +        tcp->send_buffered = so->so_snd.sb_cc;
> > +        *p_next = list;
> > +        p_next = &list->next;
> >      }
> > -
> >      for (so = slirp->udb.so_next; so != &slirp->udb; so = so->so_next) {
> > +        UsernetConnection *conn = g_new0(UsernetConnection, 1);
> > +        UsernetUDPConnection *udp = &conn->u.udp;
> > +        UsernetConnectionList *list = g_new0(UsernetConnectionList, 1);
> > +
> > +        list->value = conn;
> >          if (so->so_state & SS_HOSTFWD) {
> > -            snprintf(buf, sizeof(buf), "  UDP[HOST_FORWARD]");
> > +            udp->hostfwd = true;
> >              src_len = sizeof(src);
> >              getsockname(so->s, (struct sockaddr *)&src, &src_len);
> >              dst_addr = so->so_laddr;
> >              dst_port = so->so_lport;
> >          } else {
> > -            snprintf(buf, sizeof(buf), "  UDP[%d sec]",
> > -                         (so->so_expire - curtime) / 1000);
> > +            udp->expire_time_ms = so->so_expire - curtime;
> >              src.sin_addr = so->so_laddr;
> >              src.sin_port = so->so_lport;
> >              dst_addr = so->so_faddr;
> >              dst_port = so->so_fport;
> >          }
> > -        monitor_printf(mon, "%-19s %3d %15s %5d ", buf, so->s,
> > -                       src.sin_addr.s_addr ? inet_ntoa(src.sin_addr) : "*",
> > -                       ntohs(src.sin_port));
> > -        monitor_printf(mon, "%15s %5d %5d %5d\n",
> > -                       inet_ntoa(dst_addr), ntohs(dst_port),
> > -                       so->so_rcv.sb_cc, so->so_snd.sb_cc);
> > +        udp->fd = so->s;
> > +        udp->src_addr =
> > +            g_strdup(src.sin_addr.s_addr ? inet_ntoa(src.sin_addr) : "*");
> > +        udp->src_port = ntohs(src.sin_port);
> > +        udp->dest_addr = g_strdup(inet_ntoa(dst_addr));
> > +        udp->dest_port = ntohs(dst_port);
> > +        udp->recv_buffered = so->so_rcv.sb_cc;
> > +        udp->send_buffered = so->so_snd.sb_cc;
> > +        *p_next = list;
> > +        p_next = &list->next;
> >      }
> >  
> >      for (so = slirp->icmp.so_next; so != &slirp->icmp; so = so->so_next) {
> > -        snprintf(buf, sizeof(buf), "  ICMP[%d sec]",
> > -                     (so->so_expire - curtime) / 1000);
> > +        UsernetConnection *conn = g_new0(UsernetConnection, 1);
> > +        UsernetICMPConnection *icmp = &conn->u.icmp;
> > +        UsernetConnectionList *list = g_new0(UsernetConnectionList, 1);
> > +
> > +        icmp->expire_time_ms = so->so_expire - curtime;
> >          src.sin_addr = so->so_laddr;
> >          dst_addr = so->so_faddr;
> > -        monitor_printf(mon, "%-19s %3d %15s  -    ", buf, so->s,
> > -                       src.sin_addr.s_addr ? inet_ntoa(src.sin_addr) : 
> > "*");
> > -        monitor_printf(mon, "%15s  -    %5d %5d\n", inet_ntoa(dst_addr),
> > -                       so->so_rcv.sb_cc, so->so_snd.sb_cc);
> > +        icmp->fd = so->s;
> > +        icmp->src_addr =
> > +            g_strdup(src.sin_addr.s_addr ? inet_ntoa(src.sin_addr) : "*");
> > +        icmp->dest_addr = g_strdup(inet_ntoa(dst_addr));
> > +        icmp->recv_buffered = so->so_rcv.sb_cc;
> > +        icmp->send_buffered = so->so_snd.sb_cc;
> > +        *p_next = list;
> > +        p_next = &list->next;
> >      }
> >  }
> > +
> > +
> > +void slirp_connection_info(Slirp *slirp, Monitor *mon)
> > +{
> > +    const char *state;
> > +    char buf[64];
> > +    UsernetInfo *info = g_new0(UsernetInfo, 1);
> > +    UsernetConnectionList *cl;
> > +
> > +    monitor_printf(mon, "  Protocol[State]    FD  Source Address  Port   "
> > +                        "Dest. Address  Port RecvQ SendQ\n");
> > +
> > +    usernet_get_info(slirp, info);
> > +    for (cl = info->connections; cl && cl->value; cl = cl->next) {
> > +        UsernetConnection *conn = cl->value;
> > +
> > +        if (conn->type == USERNET_TYPE_TCP) {
> > +            UsernetTCPConnection *tcp = &conn->u.tcp;
> > +
> > +            if (tcp->hostfwd) {
> > +                state = "HOST_FORWARD";
> > +            } else {
> > +                state = TCPS_str(tcp->state);
> > +            }
> > +            snprintf(buf, sizeof(buf), "  TCP[%s]", state);
> > +            monitor_printf(mon, "%-19s %3" PRId64 " %15s %5" PRId64 " ",
> > +                           buf, tcp->fd,
> > +                           tcp->src_addr, tcp->src_port);
> > +            monitor_printf(mon, "%15s %5" PRId64 " %5" PRId64 " %5" PRId64 
> > "\n",
> > +                           tcp->dest_addr, tcp->dest_port,
> > +                           tcp->recv_buffered, tcp->send_buffered);
> > +        } else if (conn->type == USERNET_TYPE_UDP) {
> > +            UsernetUDPConnection *udp = &conn->u.udp;
> > +
> > +            if (udp->hostfwd) {
> > +                snprintf(buf, sizeof(buf), "  UDP[HOST_FORWARD]");
> > +            } else {
> > +                snprintf(buf, sizeof(buf), "  UDP[%" PRId64 " sec]",
> > +                         udp->expire_time_ms / 1000);
> > +            }
> > +            monitor_printf(mon, "%-19s %3" PRId64 " %15s %5" PRId64 " ",
> > +                           buf, udp->fd,
> > +                           udp->src_addr, udp->src_port);
> > +            monitor_printf(mon, "%15s %5" PRId64 " %5" PRId64 " %5" PRId64 
> > "\n",
> > +                           udp->dest_addr, udp->dest_port,
> > +                           udp->recv_buffered, udp->send_buffered);
> > +        } else {
> > +            UsernetICMPConnection *icmp = &conn->u.icmp;
> > +
> > +            assert(conn->type == USERNET_TYPE_ICMP);
> > +            snprintf(buf, sizeof(buf), "  ICMP[%" PRId64 " sec]",
> > +                     icmp->expire_time_ms / 1000);
> > +            monitor_printf(mon, "%-19s %3" PRId64 " %15s  -    ", buf, 
> > icmp->fd,
> > +                           icmp->src_addr);
> > +            monitor_printf(mon, "%15s  -    %5" PRId64 " %5" PRId64 "\n",
> > +                           icmp->dest_addr,
> > +                           icmp->recv_buffered, icmp->send_buffered);
> > +        }
> > +    }
> > +
> > +    qapi_free_UsernetInfo(info);
> > +}
> > diff --git a/slirp/tcp.h b/slirp/tcp.h
> > index 174d3d960c..155569f380 100644
> > --- a/slirp/tcp.h
> > +++ b/slirp/tcp.h
> > @@ -133,21 +133,6 @@ struct tcphdr {
> >  
> >  #define TCP_NSTATES     11
> >  
> > -#define TCPS_CLOSED             0       /* closed */
> > -#define TCPS_LISTEN             1       /* listening for connection */
> > -#define TCPS_SYN_SENT           2       /* active, have sent syn */
> > -#define TCPS_SYN_RECEIVED       3       /* have send and received syn */
> > -/* states < TCPS_ESTABLISHED are those where connections not established */
> > -#define TCPS_ESTABLISHED        4       /* established */
> > -#define TCPS_CLOSE_WAIT         5       /* rcvd fin, waiting for close */
> > -/* states > TCPS_CLOSE_WAIT are those where user has closed */
> > -#define TCPS_FIN_WAIT_1         6       /* have closed, sent fin */
> > -#define TCPS_CLOSING            7       /* closed xchd FIN; await FIN ACK 
> > */
> > -#define TCPS_LAST_ACK           8       /* had fin and close; await FIN 
> > ACK */
> > -/* states > TCPS_CLOSE_WAIT && < TCPS_FIN_WAIT_2 await ACK of FIN */
> > -#define TCPS_FIN_WAIT_2         9       /* have closed, fin is acked */
> > -#define TCPS_TIME_WAIT          10      /* in 2*msl quiet wait after close 
> > */
> > -
> >  #define TCPS_HAVERCVDSYN(s)     ((s) >= TCPS_SYN_RECEIVED)
> >  #define TCPS_HAVEESTABLISHED(s) ((s) >= TCPS_ESTABLISHED)
> >  #define TCPS_HAVERCVDFIN(s)     ((s) >= TCPS_TIME_WAIT)
> > -- 
> > 2.14.3
> > 
> > 
> 
> 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 via email to

[Prev in Thread] Current Thread [Next in Thread]