[Qemu-devel] [PULL 0/9] slirp updates

[Qemu-devel] [PULL 0/9] slirp updates

[Samuel Thibault]

The following changes since commit c181ddaa176856b3cd2dfd12bbcf25fa9c884a97:

  Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20180531-1' into staging (2018-05-31 17:00:55 +0100)

are available in the Git repository at:

  http://people.debian.org/~sthibault/qemu.git tags/samuel-thibault

for you to fetch changes up to 528e913612bf22661b169018780d8a6fc678c655:

  slirp/ncsi: add checksum support (2018-05-31 20:45:02 +0200)

----------------------------------------------------------------
slirp updates

Alexey Kardashevskiy
  slirp: Improve debugging messages

Andreas Gustafsson, Samuel Thibault, James Clarke
  slirp: Improve bandwidth in GDB remote debugging and FreeBSD guests

Benjamin Drung:
  slirp/dhcp: Add domainname option

Cédric Le Goater (3):
  slirp/ncsi: fix "Get Version ID" payload length
  slirp/ncsi: add a "Get Parameters" response
  slirp/ncsi: add checksum support

Nia Alarie:
  net/slirp: Convert atoi to qemu_strtoi to allow error checking

----------------------------------------------------------------
Alexey Kardashevskiy (1):
      slirp/debug: Print IP addresses in human readable form

Andreas Gustafsson (1):
      slirp: disable Nagle in outgoing connections

Benjamin Drung (1):
      slirp: Add domainname option to slirp's DHCP server

Cédric Le Goater (3):
      slirp/ncsi: fix "Get Version ID" payload length
      slirp/ncsi: add a "Get Parameters" response
      slirp/ncsi: add checksum support

James Clarke (1):
      slirp: Send window updates to guest after window was closed

Nia Alarie (1):
      net/slirp: Convert atoi to qemu_strtoi to allow error checking

Samuel Thibault (1):
      slirp: disable Nagle in ingoing connections

 net/slirp.c       | 16 ++++++++++++----
 qapi/net.json     |  4 ++++
 qemu-options.hx   |  7 +++++--
 slirp/arp_table.c |  4 ++--
 slirp/bootp.c     |  8 ++++++++
 slirp/libslirp.h  |  2 +-
 slirp/ncsi.c      | 51 ++++++++++++++++++++++++++++++++++++++++++++-------
 slirp/slirp.c     | 16 +++++++++-------
 slirp/slirp.h     |  1 +
 slirp/socket.c    |  6 ++++--
 slirp/tcp_subr.c  |  2 ++
 11 files changed, 92 insertions(+), 25 deletions(-)

[Qemu-devel] [PULL 1/9] slirp: Add domainname option to slirp's DHCP server

[Samuel Thibault]

From: Benjamin Drung <benjamin.drung@profitbricks.com>

This patch will allow the user to include the domainname option in
replies from the built-in DHCP server.

Signed-off-by: Benjamin Drung <benjamin.drung@profitbricks.com>
Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
---
 net/slirp.c      | 12 +++++++++---
 qapi/net.json    |  4 ++++
 qemu-options.hx  |  7 +++++--
 slirp/bootp.c    |  8 ++++++++
 slirp/libslirp.h |  2 +-
 slirp/slirp.c    |  4 +++-
 slirp/slirp.h    |  1 +
 7 files changed, 31 insertions(+), 7 deletions(-)

diff --git a/net/slirp.c b/net/slirp.c
index 692252445a..005c2675e6 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -157,7 +157,8 @@ static int net_slirp_init(NetClientState *peer, const char *model,
                           const char *bootfile, const char *vdhcp_start,
                           const char *vnameserver, const char *vnameserver6,
                           const char *smb_export, const char *vsmbserver,
-                          const char **dnssearch, Error **errp)
+                          const char **dnssearch, const char *vdomainname,
+                          Error **errp)
 {
     /* default settings according to historic slirp */
     struct in_addr net  = { .s_addr = htonl(0x0a000200) }; /* 10.0.2.0 */
@@ -359,6 +360,11 @@ static int net_slirp_init(NetClientState *peer, const char *model,
         ip6_dns.s6_addr[15] |= 3;
     }
 
+    if (vdomainname && !*vdomainname) {
+        error_setg(errp, "'domainname' parameter cannot be empty");
+        return -1;
+    }
+
 
     nc = qemu_new_net_client(&net_slirp_info, peer, model, name);
 
@@ -371,7 +377,7 @@ static int net_slirp_init(NetClientState *peer, const char *model,
     s->slirp = slirp_init(restricted, ipv4, net, mask, host,
                           ipv6, ip6_prefix, vprefix6_len, ip6_host,
                           vhostname, tftp_export, bootfile, dhcp,
-                          dns, ip6_dns, dnssearch, s);
+                          dns, ip6_dns, dnssearch, vdomainname, s);
     QTAILQ_INSERT_TAIL(&slirp_stacks, s, entry);
 
     for (config = slirp_configs; config; config = config->next) {
@@ -958,7 +964,7 @@ int net_init_slirp(const Netdev *netdev, const char *name,
                          user->ipv6_host, user->hostname, user->tftp,
                          user->bootfile, user->dhcpstart,
                          user->dns, user->ipv6_dns, user->smb,
-                         user->smbserver, dnssearch, errp);
+                         user->smbserver, dnssearch, user->domainname, errp);
 
     while (slirp_configs) {
         config = slirp_configs;
diff --git a/qapi/net.json b/qapi/net.json
index 5c1dc48890..32681a1af7 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -160,6 +160,9 @@
 # @dnssearch: list of DNS suffixes to search, passed as DHCP option
 #             to the guest
 #
+# @domainname: guest-visible domain name of the virtual nameserver
+#              (since 2.12)
+#
 # @ipv6-prefix: IPv6 network prefix (default is fec0::) (since
 #               2.6). The network prefix is given in the usual
 #               hexadecimal IPv6 address notation.
@@ -197,6 +200,7 @@
     '*dhcpstart': 'str',
     '*dns':       'str',
     '*dnssearch': ['String'],
+    '*domainname': 'str',
     '*ipv6-prefix':      'str',
     '*ipv6-prefixlen':   'int',
     '*ipv6-host':        'str',
diff --git a/qemu-options.hx b/qemu-options.hx
index 2f61ea42ee..c0d3951e9f 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1906,8 +1906,8 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
     "-netdev user,id=str[,ipv4[=on|off]][,net=addr[/mask]][,host=addr]\n"
     "         [,ipv6[=on|off]][,ipv6-net=addr[/int]][,ipv6-host=addr]\n"
     "         [,restrict=on|off][,hostname=host][,dhcpstart=addr]\n"
-    "         [,dns=addr][,ipv6-dns=addr][,dnssearch=domain][,tftp=dir]\n"
-    "         [,bootfile=f][,hostfwd=rule][,guestfwd=rule]"
+    "         [,dns=addr][,ipv6-dns=addr][,dnssearch=domain][,domainname=domain]\n"
+    "         [,tftp=dir][,bootfile=f][,hostfwd=rule][,guestfwd=rule]"
 #ifndef _WIN32
                                              "[,smb=dir[,smbserver=addr]]\n"
 #endif
@@ -2135,6 +2135,9 @@ Example:
 qemu-system-i386 -nic user,dnssearch=mgmt.example.org,dnssearch=example.org
 @end example
 
+@item domainname=@var{domain}
+Specifies the client domain name reported by the built-in DHCP server.
+
 @item tftp=@var{dir}
 When using the user mode network stack, activate a built-in TFTP
 server. The files in @var{dir} will be exposed as the root of a TFTP server.
diff --git a/slirp/bootp.c b/slirp/bootp.c
index 5dd1a415b5..9e7b53ba94 100644
--- a/slirp/bootp.c
+++ b/slirp/bootp.c
@@ -298,6 +298,14 @@ static void bootp_reply(Slirp *slirp, const struct bootp_t *bp)
             q += val;
         }
 
+        if (slirp->vdomainname) {
+            val = strlen(slirp->vdomainname);
+            *q++ = RFC1533_DOMAINNAME;
+            *q++ = val;
+            memcpy(q, slirp->vdomainname, val);
+            q += val;
+        }
+
         if (slirp->vdnssearch) {
             size_t spaceleft = sizeof(rbp->bp_vend) - (q - rbp->bp_vend);
             val = slirp->vdnssearch_len;
diff --git a/slirp/libslirp.h b/slirp/libslirp.h
index 540b3e5903..740408a96e 100644
--- a/slirp/libslirp.h
+++ b/slirp/libslirp.h
@@ -16,7 +16,7 @@ Slirp *slirp_init(int restricted, bool in_enabled, struct in_addr vnetwork,
                   const char *tftp_path, const char *bootfile,
                   struct in_addr vdhcp_start, struct in_addr vnameserver,
                   struct in6_addr vnameserver6, const char **vdnssearch,
-                  void *opaque);
+                  const char *vdomainname, void *opaque);
 void slirp_cleanup(Slirp *slirp);
 
 void slirp_pollfds_fill(GArray *pollfds, uint32_t *timeout);
diff --git a/slirp/slirp.c b/slirp/slirp.c
index 1cb6b07004..4f29753444 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -286,7 +286,7 @@ Slirp *slirp_init(int restricted, bool in_enabled, struct in_addr vnetwork,
                   const char *tftp_path, const char *bootfile,
                   struct in_addr vdhcp_start, struct in_addr vnameserver,
                   struct in6_addr vnameserver6, const char **vdnssearch,
-                  void *opaque)
+                  const char *vdomainname, void *opaque)
 {
     Slirp *slirp = g_malloc0(sizeof(Slirp));
 
@@ -317,6 +317,7 @@ Slirp *slirp_init(int restricted, bool in_enabled, struct in_addr vnetwork,
     }
     slirp->tftp_prefix = g_strdup(tftp_path);
     slirp->bootp_filename = g_strdup(bootfile);
+    slirp->vdomainname = g_strdup(vdomainname);
     slirp->vdhcp_startaddr = vdhcp_start;
     slirp->vnameserver_addr = vnameserver;
     slirp->vnameserver_addr6 = vnameserver6;
@@ -349,6 +350,7 @@ void slirp_cleanup(Slirp *slirp)
     g_free(slirp->vdnssearch);
     g_free(slirp->tftp_prefix);
     g_free(slirp->bootp_filename);
+    g_free(slirp->vdomainname);
     g_free(slirp);
 }
 
diff --git a/slirp/slirp.h b/slirp/slirp.h
index 06febfc78b..10b410898a 100644
--- a/slirp/slirp.h
+++ b/slirp/slirp.h
@@ -193,6 +193,7 @@ struct Slirp {
     char *bootp_filename;
     size_t vdnssearch_len;
     uint8_t *vdnssearch;
+    char *vdomainname;
 
     /* tcp states */
     struct socket tcb;
-- 
2.17.0

Re: [Qemu-devel] [PULL 1/9] slirp: Add domainname option to slirp's DHCP server

[Eric Blake]

On 05/31/2018 02:22 PM, Samuel Thibault wrote:
> From: Benjamin Drung <benjamin.drung@profitbricks.com>
> 
> This patch will allow the user to include the domainname option in
> replies from the built-in DHCP server.
> 
> Signed-off-by: Benjamin Drung <benjamin.drung@profitbricks.com>
> Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> ---
>   net/slirp.c      | 12 +++++++++---
>   qapi/net.json    |  4 ++++
>   qemu-options.hx  |  7 +++++--
>   slirp/bootp.c    |  8 ++++++++
>   slirp/libslirp.h |  2 +-
>   slirp/slirp.c    |  4 +++-
>   slirp/slirp.h    |  1 +
>   7 files changed, 31 insertions(+), 7 deletions(-)

> +++ b/qapi/net.json
> @@ -160,6 +160,9 @@
>   # @dnssearch: list of DNS suffixes to search, passed as DHCP option
>   #             to the guest
>   #
> +# @domainname: guest-visible domain name of the virtual nameserver
> +#              (since 2.12)

This missed 2.12; we'll need a followup patch that updates it to 3.0.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

[Qemu-devel] [PULL 2/9] slirp: disable Nagle in outgoing connections

[Samuel Thibault]

From: Andreas Gustafsson <gson@gson.org>

When setting up an outgoing user mode networking TCP connection,
disable the Nagle algorithm in the host-side connection.  Either the
guest is already doing Nagle, in which case there is no point in doing
it twice, or it has chosen to disable it, in which case we should
respect that choice.

This change speeds up GDB remote debugging over TCP over user mode
networking (with GDB runing on the guest) by multiple orders of
magnitude, and has been part of the local patches applied by pkgsrc
since 2012 with no reported ill effects.

Signed-off-by: Andreas Gustafsson <gson@gson.org>
Reviewed-by: Kamil Rytarowski <n54@gmx.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
---
 slirp/tcp_subr.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/slirp/tcp_subr.c b/slirp/tcp_subr.c
index da0d53743f..8d0f94b75f 100644
--- a/slirp/tcp_subr.c
+++ b/slirp/tcp_subr.c
@@ -416,6 +416,8 @@ int tcp_fconnect(struct socket *so, unsigned short af)
     socket_set_fast_reuse(s);
     opt = 1;
     qemu_setsockopt(s, SOL_SOCKET, SO_OOBINLINE, &opt, sizeof(opt));
+    opt = 1;
+    qemu_setsockopt(s, IPPROTO_TCP, TCP_NODELAY, &opt, sizeof(opt));
 
     addr = so->fhost.ss;
     DEBUG_CALL(" connect()ing")
-- 
2.17.0

[Qemu-devel] [PULL 3/9] slirp: disable Nagle in ingoing connections

[Samuel Thibault]

This follows 3929766fb3e4 ('slirp: disable Nagle in outgoing connections'):
for the same reasons, ingoing connections should have the Nagle algorithm disabled.

Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 slirp/socket.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/slirp/socket.c b/slirp/socket.c
index 61347d1a0c..6f18e157a5 100644
--- a/slirp/socket.c
+++ b/slirp/socket.c
@@ -754,6 +754,8 @@ tcp_listen(Slirp *slirp, uint32_t haddr, u_int hport, uint32_t laddr,
 		return NULL;
 	}
 	qemu_setsockopt(s, SOL_SOCKET, SO_OOBINLINE, &opt, sizeof(int));
+	opt = 1;
+	qemu_setsockopt(s, IPPROTO_TCP, TCP_NODELAY, &opt, sizeof(int));
 
 	getsockname(s,(struct sockaddr *)&addr,&addrlen);
 	so->so_ffamily = AF_INET;
-- 
2.17.0

[Qemu-devel] [PULL 4/9] slirp/debug: Print IP addresses in human readable form

[Samuel Thibault]

From: Alexey Kardashevskiy <aik@ozlabs.ru>

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
---
 slirp/arp_table.c | 4 ++--
 slirp/socket.c    | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/slirp/arp_table.c b/slirp/arp_table.c
index bac608f97f..f81963bb88 100644
--- a/slirp/arp_table.c
+++ b/slirp/arp_table.c
@@ -33,7 +33,7 @@ void arp_table_add(Slirp *slirp, uint32_t ip_addr, uint8_t ethaddr[ETH_ALEN])
     int i;
 
     DEBUG_CALL("arp_table_add");
-    DEBUG_ARG("ip = %s", inet_ntoa(*(struct in_addr *)&ip_addr));
+    DEBUG_ARG("ip = %s", inet_ntoa((struct in_addr){.s_addr = ip_addr}));
     DEBUG_ARGS((dfd, " hw addr = %02x:%02x:%02x:%02x:%02x:%02x\n",
                 ethaddr[0], ethaddr[1], ethaddr[2],
                 ethaddr[3], ethaddr[4], ethaddr[5]));
@@ -67,7 +67,7 @@ bool arp_table_search(Slirp *slirp, uint32_t ip_addr,
     int i;
 
     DEBUG_CALL("arp_table_search");
-    DEBUG_ARG("ip = %s", inet_ntoa(*(struct in_addr *)&ip_addr));
+    DEBUG_ARG("ip = %s", inet_ntoa((struct in_addr){.s_addr = ip_addr}));
 
     /* If broadcast address */
     if (ip_addr == 0xffffffff || ip_addr == broadcast_addr) {
diff --git a/slirp/socket.c b/slirp/socket.c
index 6f18e157a5..e2a71c9b04 100644
--- a/slirp/socket.c
+++ b/slirp/socket.c
@@ -701,9 +701,9 @@ tcp_listen(Slirp *slirp, uint32_t haddr, u_int hport, uint32_t laddr,
 	memset(&addr, 0, addrlen);
 
 	DEBUG_CALL("tcp_listen");
-	DEBUG_ARG("haddr = %s", inet_ntoa(*(struct in_addr *)&haddr));
+	DEBUG_ARG("haddr = %s", inet_ntoa((struct in_addr){.s_addr = haddr}));
 	DEBUG_ARG("hport = %d", ntohs(hport));
-	DEBUG_ARG("laddr = %s", inet_ntoa(*(struct in_addr *)&laddr));
+	DEBUG_ARG("laddr = %s", inet_ntoa((struct in_addr){.s_addr = laddr}));
 	DEBUG_ARG("lport = %d", ntohs(lport));
 	DEBUG_ARG("flags = %x", flags);
 
-- 
2.17.0

[Qemu-devel] [PULL 5/9] net/slirp: Convert atoi to qemu_strtoi to allow error checking

[Samuel Thibault]

From: Nia Alarie <nia.alarie@gmail.com>

Signed-off-by: Nia Alarie <nia.alarie@gmail.com>
Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
---
 net/slirp.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/slirp.c b/net/slirp.c
index 005c2675e6..1e14318b4d 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -492,7 +492,9 @@ void hmp_hostfwd_remove(Monitor *mon, const QDict *qdict)
         goto fail_syntax;
     }
 
-    host_port = atoi(p);
+    if (qemu_strtoi(p, NULL, 10, &host_port)) {
+        goto fail_syntax;
+    }
 
     err = slirp_remove_hostfwd(s->slirp, is_udp, host_addr, host_port);
 
-- 
2.17.0

[Qemu-devel] [PULL 6/9] slirp: Send window updates to guest after window was closed

[Samuel Thibault]

From: James Clarke <jrtc27@jrtc27.com>

If the receive window presented to the guest closes, slirp should send a
window update once the window reopens sufficiently, rather than forcing
the guest to send a window probe, which can take several seconds.

Signed-off-by: James Clarke <jrtc27@jrtc27.com>
Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
---
 slirp/slirp.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/slirp/slirp.c b/slirp/slirp.c
index 4f29753444..5c3bd6163f 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -678,13 +678,13 @@ void slirp_pollfds_poll(GArray *pollfds, int select_error)
                         /* continue; */
                     } else {
                         ret = sowrite(so);
+                        if (ret > 0) {
+                            /* Call tcp_output in case we need to send a window
+                             * update to the guest, otherwise it will be stuck
+                             * until it sends a window probe. */
+                            tcp_output(sototcpcb(so));
+                        }
                     }
-                    /*
-                     * XXXXX If we wrote something (a lot), there
-                     * could be a need for a window update.
-                     * In the worst case, the remote will send
-                     * a window probe to get things going again
-                     */
                 }
 
                 /*
-- 
2.17.0

[Qemu-devel] [PULL 7/9] slirp/ncsi: fix "Get Version ID" payload length

[Samuel Thibault]

From: Cédric Le Goater <clg@kaod.org>

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
---
 slirp/ncsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/slirp/ncsi.c b/slirp/ncsi.c
index d12ba3e494..02d0e9def3 100644
--- a/slirp/ncsi.c
+++ b/slirp/ncsi.c
@@ -60,7 +60,7 @@ static const struct ncsi_rsp_handler {
         { NCSI_PKT_RSP_EGMF,    4, NULL },
         { NCSI_PKT_RSP_DGMF,    4, NULL },
         { NCSI_PKT_RSP_SNFC,    4, NULL },
-        { NCSI_PKT_RSP_GVI,    36, NULL },
+        { NCSI_PKT_RSP_GVI,    40, NULL },
         { NCSI_PKT_RSP_GC,     32, ncsi_rsp_handler_gc },
         { NCSI_PKT_RSP_GP,     -1, NULL },
         { NCSI_PKT_RSP_GCPS,  172, NULL },
-- 
2.17.0

[Qemu-devel] [PULL 8/9] slirp/ncsi: add a "Get Parameters" response

[Samuel Thibault]

From: Cédric Le Goater <clg@kaod.org>

Command 0x17 'Get Parameters' is used to get configuration parameter
values currently in effect on the controller and it is mandatory in
the NS-CI specification.

Provide a minimum response to exercise the kernel.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
---
 slirp/ncsi.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/slirp/ncsi.c b/slirp/ncsi.c
index 02d0e9def3..7b3fff207a 100644
--- a/slirp/ncsi.c
+++ b/slirp/ncsi.c
@@ -35,6 +35,20 @@ static int ncsi_rsp_handler_gls(struct ncsi_rsp_pkt_hdr *rnh)
     return 0;
 }
 
+/* Get Parameters */
+static int ncsi_rsp_handler_gp(struct ncsi_rsp_pkt_hdr *rnh)
+{
+    struct ncsi_rsp_gp_pkt *rsp = (struct ncsi_rsp_gp_pkt *) rnh;
+
+    /* no MAC address filters or VLAN filters on the channel */
+    rsp->mac_cnt = 0;
+    rsp->mac_enable = 0;
+    rsp->vlan_cnt = 0;
+    rsp->vlan_enable = 0;
+
+    return 0;
+}
+
 static const struct ncsi_rsp_handler {
         unsigned char   type;
         int             payload;
@@ -62,7 +76,7 @@ static const struct ncsi_rsp_handler {
         { NCSI_PKT_RSP_SNFC,    4, NULL },
         { NCSI_PKT_RSP_GVI,    40, NULL },
         { NCSI_PKT_RSP_GC,     32, ncsi_rsp_handler_gc },
-        { NCSI_PKT_RSP_GP,     -1, NULL },
+        { NCSI_PKT_RSP_GP,     40, ncsi_rsp_handler_gp },
         { NCSI_PKT_RSP_GCPS,  172, NULL },
         { NCSI_PKT_RSP_GNS,   172, NULL },
         { NCSI_PKT_RSP_GNPTS, 172, NULL },
-- 
2.17.0

[Qemu-devel] [PULL 9/9] slirp/ncsi: add checksum support

[Samuel Thibault]

From: Cédric Le Goater <clg@kaod.org>

The checksum field of a NC-SI packet contains a value that may be
included in each command and response. The verification is optional
but the Linux driver does so when a non-zero value is provided. Let's
extend the model to compute the checksum value and exercise a little
more the Linux driver.

See section "8.2.2.3 - 2's Complement Checksum Compensation" in the
Network Controller Sideband Interface (NC-SI) Specification for more
details.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
---
 slirp/ncsi.c | 33 ++++++++++++++++++++++++++++-----
 1 file changed, 28 insertions(+), 5 deletions(-)

diff --git a/slirp/ncsi.c b/slirp/ncsi.c
index 7b3fff207a..7116034afc 100644
--- a/slirp/ncsi.c
+++ b/slirp/ncsi.c
@@ -1,7 +1,7 @@
 /*
  * NC-SI (Network Controller Sideband Interface) "echo" model
  *
- * Copyright (C) 2016 IBM Corp.
+ * Copyright (C) 2016-2018 IBM Corp.
  *
  * This code is licensed under the GPL version 2 or later. See the
  * COPYING file in the top-level directory.
@@ -11,6 +11,23 @@
 
 #include "ncsi-pkt.h"
 
+static uint32_t ncsi_calculate_checksum(uint16_t *data, int len)
+{
+    uint32_t checksum = 0;
+    int i;
+
+    /*
+     * 32-bit unsigned sum of the NC-SI packet header and NC-SI packet
+     * payload interpreted as a series of 16-bit unsigned integer values.
+     */
+    for (i = 0; i < len; i++) {
+        checksum += htons(data[i]);
+    }
+
+    checksum = (~checksum + 1);
+    return checksum;
+}
+
 /* Get Capabilities */
 static int ncsi_rsp_handler_gc(struct ncsi_rsp_pkt_hdr *rnh)
 {
@@ -101,6 +118,9 @@ void ncsi_input(Slirp *slirp, const uint8_t *pkt, int pkt_len)
         (ncsi_reply + ETH_HLEN);
     const struct ncsi_rsp_handler *handler = NULL;
     int i;
+    int ncsi_rsp_len = sizeof(*nh);
+    uint32_t checksum;
+    uint32_t *pchecksum;
 
     memset(ncsi_reply, 0, sizeof(ncsi_reply));
 
@@ -130,15 +150,18 @@ void ncsi_input(Slirp *slirp, const uint8_t *pkt, int pkt_len)
             /* TODO: handle errors */
             handler->handler(rnh);
         }
+        ncsi_rsp_len += handler->payload;
     } else {
         rnh->common.length = 0;
         rnh->code          = htons(NCSI_PKT_RSP_C_UNAVAILABLE);
         rnh->reason        = htons(NCSI_PKT_RSP_R_UNKNOWN);
     }
 
-    /* TODO: add a checksum at the end of the frame but the specs
-     * allows it to be zero */
+    /* Add the optional checksum at the end of the frame. */
+    checksum = ncsi_calculate_checksum((uint16_t *) rnh, ncsi_rsp_len);
+    pchecksum = (uint32_t *)((void *) rnh + ncsi_rsp_len);
+    *pchecksum = htonl(checksum);
+    ncsi_rsp_len += 4;
 
-    slirp_output(slirp->opaque, ncsi_reply, ETH_HLEN + sizeof(*nh) +
-                 (handler ? handler->payload : 0) + 4);
+    slirp_output(slirp->opaque, ncsi_reply, ETH_HLEN + ncsi_rsp_len);
 }
-- 
2.17.0

Re: [Qemu-devel] [PULL 0/9] slirp updates

[no-reply]

Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20180531192237.3994-1-samuel.thibault@ens-lyon.org
Subject: [Qemu-devel] [PULL 0/9] slirp updates

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]               patchew/20180531192237.3994-1-samuel.thibault@ens-lyon.org -> patchew/20180531192237.3994-1-samuel.thibault@ens-lyon.org
Switched to a new branch 'test'
100c703503 slirp/ncsi: add checksum support
41dba64dbb slirp/ncsi: add a "Get Parameters" response
b0eef29e1d slirp/ncsi: fix "Get Version ID" payload length
9704af1c8c slirp: Send window updates to guest after window was closed
0829b29dd1 net/slirp: Convert atoi to qemu_strtoi to allow error checking
b25abd1163 slirp/debug: Print IP addresses in human readable form
b333e33553 slirp: disable Nagle in ingoing connections
b5523b84a3 slirp: disable Nagle in outgoing connections
9ecad88828 slirp: Add domainname option to slirp's DHCP server

=== OUTPUT BEGIN ===
Checking PATCH 1/9: slirp: Add domainname option to slirp's DHCP server...
Checking PATCH 2/9: slirp: disable Nagle in outgoing connections...
Checking PATCH 3/9: slirp: disable Nagle in ingoing connections...
ERROR: code indent should never use tabs
#24: FILE: slirp/socket.c:757:
+^Iopt = 1;$

ERROR: code indent should never use tabs
#25: FILE: slirp/socket.c:758:
+^Iqemu_setsockopt(s, IPPROTO_TCP, TCP_NODELAY, &opt, sizeof(int));$

total: 2 errors, 0 warnings, 8 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 4/9: slirp/debug: Print IP addresses in human readable form...
ERROR: code indent should never use tabs
#41: FILE: slirp/socket.c:704:
+^IDEBUG_ARG("haddr = %s", inet_ntoa((struct in_addr){.s_addr = haddr}));$

ERROR: code indent should never use tabs
#44: FILE: slirp/socket.c:706:
+^IDEBUG_ARG("laddr = %s", inet_ntoa((struct in_addr){.s_addr = laddr}));$

total: 2 errors, 0 warnings, 27 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 5/9: net/slirp: Convert atoi to qemu_strtoi to allow error checking...
Checking PATCH 6/9: slirp: Send window updates to guest after window was closed...
Checking PATCH 7/9: slirp/ncsi: fix "Get Version ID" payload length...
Checking PATCH 8/9: slirp/ncsi: add a "Get Parameters" response...
Checking PATCH 9/9: slirp/ncsi: add checksum support...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [Qemu-devel] [PULL 0/9] slirp updates

[Peter Maydell]

On 31 May 2018 at 20:22, Samuel Thibault <samuel.thibault@ens-lyon.org> wrote:
> The following changes since commit c181ddaa176856b3cd2dfd12bbcf25fa9c884a97:
>
>   Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20180531-1' into staging (2018-05-31 17:00:55 +0100)
>
> are available in the Git repository at:
>
>   http://people.debian.org/~sthibault/qemu.git tags/samuel-thibault
>
> for you to fetch changes up to 528e913612bf22661b169018780d8a6fc678c655:
>
>   slirp/ncsi: add checksum support (2018-05-31 20:45:02 +0200)
>
> ----------------------------------------------------------------
> slirp updates
>
> Alexey Kardashevskiy
>   slirp: Improve debugging messages
>
> Andreas Gustafsson, Samuel Thibault, James Clarke
>   slirp: Improve bandwidth in GDB remote debugging and FreeBSD guests
>
> Benjamin Drung:
>   slirp/dhcp: Add domainname option

Hi; it looks like you forgot to add your signed-off-by to this
patch from Benjamin. Could you add it and respin, please?
(You can just send the pullreq cover letter, no need to re-email
the individual patches.)

thanks
-- PMM

Re: [Qemu-devel] [PULL 0/9] slirp updates

[Samuel Thibault]

Peter Maydell, le ven. 01 juin 2018 14:50:29 +0100, a ecrit:
> Hi; it looks like you forgot to add your signed-off-by to this
> patch from Benjamin.

Oh?  I did have noticed it and rebased, but I guess I forgot to update
the tag.

> Could you add it and respin, please?

I have now fixed the tag and the PR.

Samuel