Re: [Qemu-devel] [PATCH RFC 2/4] slirp: add helper for udp6 socket creation
Ditto :) Samuel
Re: [Qemu-devel] [PATCH RFC 1/4] slirp: add helper for tcp6 socket creation
Samuel Thibault, le sam. 27 oct. 2018 13:11:41 +0200, a ecrit:
> struct socket *
> tcp_listen(Slirp *slirp, uint32_t haddr, u_int hport, uint32_t laddr,
>u_int lport, int flags)
> {
> struct sockaddr_in addr;
> struct socket *so;
> socklen_t addrsize = sizeof(addr);
Also memset(&addr, 0, sizeof(addr);
> addr.sin_family = AF_INET;
> addr.sin_addr.s_addr = haddr;
> addr.sin_port = hport;
Re: [Qemu-devel] [PATCH RFC 1/4] slirp: add helper for tcp6 socket creation
Hello,
Thanks for working on this!
There is a lot of overlap with tcp_listen. It'd be much better to
refactor it this way:
struct socket *
tcpx_listen(Slirp *slirp, struct sockaddr *addr, socklen_t *addrlen, int flags)
{
... The current content of tcp_listen, with all heading and
trailing addr manipulations removed...
... so->so_lfamily = addr->sa_family;...
... s = qemu_socket(addr->sa_family, SOCK_STREAM, 0);...
... (bind(s, addr, *addrlen) < 0) ||...
... getsockname(s, addr, addrlen);
}
struct socket *
tcp_listen(Slirp *slirp, uint32_t haddr, u_int hport, uint32_t laddr,
u_int lport, int flags)
{
struct sockaddr_in addr;
struct socket *so;
socklen_t addrsize = sizeof(addr);
addr.sin_family = AF_INET;
addr.sin_addr.s_addr = haddr;
addr.sin_port = hport;
so = tcpx_listen(slirp, (struct sockaddr *) &addr, &addrsize, flags);
so->so_lport = lport; /* Kept in network format */
so->so_laddr.s_addr = laddr; /* Ditto */
so->so_ffamily = AF_INET;
so->so_fport = addr.sin_port;
if (addr.sin_addr.s_addr == 0 || addr.sin_addr.s_addr ==
loopback_addr.s_addr)
so->so_faddr = slirp->vhost_addr;
else
so->so_faddr = addr.sin_addr;
}
The v6 version then boils down to
struct socket *
tcp6_listen(Slirp *slirp, struct in6_addr haddr, u_int hport, struct
in6_addr laddr, u_int lport, int flags)
{
struct sockaddr_in6 addr;
struct socket *so;
socklen_t addrsize = sizeof(addr);
addr.sin6_family = AF_INET6;
addr.sin6_addr = haddr;
addr.sin6_port = hport;
so = tcpx_listen(slirp, (struct sockaddr *) &addr, &addrsize, flags);
so->so_lport6 = lport; /* Kept in network format */
so->so_laddr6 = laddr; /* Ditto */
so->fhost.sin6 = addr;
if (!memcmp(&addr.sin6_addr, &in6addr_any, sizeof(in6addr_any)) ||
!memcmp(&addr.sin6_addr, &in6addr_loopback,
sizeof(in6addr_loopback))) {
memcpy(&so->so_faddr6, &slirp->vhost_addr6,
sizeof(slirp->vhost_addr6));
}
}
modulo all typos etc. I may have done.
Maxim Samoylov, le ven. 26 oct. 2018 03:03:40 +0300, a ecrit:
> +qemu_setsockopt(s, SOL_SOCKET, SO_OOBINLINE, &opt, sizeof(int));
Is there a reason why you set SO_OOBINLINE, but not TCP_NODELAY? That's
the kind of discrepancy we don't want to let unseen, thus the reason for
a shared implementation :)
Samuel
Re: [Qemu-devel] [PATCH v3 2/2] slirp: Implement RFC2132 TFTP server name
Hello, Fam Zheng, le ven. 14 sept. 2018 15:26:16 +0800, a ecrit: > This new usernet option can be used to add data for option 66 (tftp > server name) in the BOOTP reply, which is useful in PXE based automatic > OS install such as OpenBSD. Applied to my tree, thanks! Samuel
Re: [Qemu-devel] [PATCH v3 1/2] slirp: Add sanity check for str option length
Hello, Fam Zheng, le ven. 14 sept. 2018 15:26:15 +0800, a ecrit: > When user provides a long domainname or hostname that doesn't fit in the > DHCP packet, we mustn't overflow the response packet buffer. Instead, > report errors, following the g_warning() in the slirp->vdnssearch > branch. > > Also check the strlen against 256 when initializing slirp, which limit > is also from the protocol where one byte represents the string length. > This gives an early error before the warning which is harder to notice > or diagnose. Applied to my tree, thanks! Samuel
[Qemu-devel] [PULL 0/2] slirp updates
The following changes since commit b312532fd03413d0e6ae6767ec793a3e30f487b8: Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2018-10-19 19:01:07 +0100) are available in the Git repository at: https://people.debian.org/~sthibault/qemu.git tags/samuel-thibault for you to fetch changes up to 0fca92b9077af9817c04545cdfc519fe95c6fde9: slirp: Implement RFC2132 TFTP server name (2018-10-21 21:24:55 +0200) Fam Zheng (2): slirp: Add sanity check for str option length slirp: Implement RFC2132 TFTP server name Fam Zheng (2): slirp: Add sanity check for str option length slirp: Implement RFC2132 TFTP server name net/slirp.c | 21 +++-- qapi/net.json| 5 - qemu-options.hx | 7 ++- slirp/bootp.c| 45 +++-- slirp/bootp.h| 1 + slirp/libslirp.h | 1 + slirp/slirp.c| 2 ++ slirp/slirp.h| 1 + 8 files changed, 69 insertions(+), 14 deletions(-)
[Qemu-devel] [PULL 1/2] slirp: Add sanity check for str option length
From: Fam Zheng
When user provides a long domainname or hostname that doesn't fit in the
DHCP packet, we mustn't overflow the response packet buffer. Instead,
report errors, following the g_warning() in the slirp->vdnssearch
branch.
Also check the strlen against 256 when initializing slirp, which limit
is also from the protocol where one byte represents the string length.
This gives an early error before the warning which is harder to notice
or diagnose.
Reported-by: Thomas Huth
Reviewed-by: Thomas Huth
Cc: qemu-sta...@nongnu.org
Signed-off-by: Fam Zheng
Tested-by: Gerd Hoffmann
Signed-off-by: Samuel Thibault
---
net/slirp.c | 9 +
slirp/bootp.c | 32 ++--
2 files changed, 31 insertions(+), 10 deletions(-)
diff --git a/net/slirp.c b/net/slirp.c
index 99884de204..da6c0a1a5c 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -350,6 +350,15 @@ static int net_slirp_init(NetClientState *peer, const char
*model,
return -1;
}
+if (vdomainname && strlen(vdomainname) > 255) {
+error_setg(errp, "'domainname' parameter cannot exceed 255 bytes");
+return -1;
+}
+
+if (vhostname && strlen(vhostname) > 255) {
+error_setg(errp, "'vhostname' parameter cannot exceed 255 bytes");
+return -1;
+}
nc = qemu_new_net_client(&net_slirp_info, peer, model, name);
diff --git a/slirp/bootp.c b/slirp/bootp.c
index 9e7b53ba94..1e8185f0ec 100644
--- a/slirp/bootp.c
+++ b/slirp/bootp.c
@@ -159,6 +159,7 @@ static void bootp_reply(Slirp *slirp, const struct bootp_t
*bp)
struct in_addr preq_addr;
int dhcp_msg_type, val;
uint8_t *q;
+uint8_t *end;
uint8_t client_ethaddr[ETH_ALEN];
/* extract exact DHCP msg type */
@@ -240,6 +241,7 @@ static void bootp_reply(Slirp *slirp, const struct bootp_t
*bp)
rbp->bp_siaddr = saddr.sin_addr; /* Server IP address */
q = rbp->bp_vend;
+end = (uint8_t *)&rbp[1];
memcpy(q, rfc1533_cookie, 4);
q += 4;
@@ -292,24 +294,33 @@ static void bootp_reply(Slirp *slirp, const struct
bootp_t *bp)
if (*slirp->client_hostname) {
val = strlen(slirp->client_hostname);
-*q++ = RFC1533_HOSTNAME;
-*q++ = val;
-memcpy(q, slirp->client_hostname, val);
-q += val;
+if (q + val + 2 >= end) {
+g_warning("DHCP packet size exceeded, "
+"omitting host name option.");
+} else {
+*q++ = RFC1533_HOSTNAME;
+*q++ = val;
+memcpy(q, slirp->client_hostname, val);
+q += val;
+}
}
if (slirp->vdomainname) {
val = strlen(slirp->vdomainname);
-*q++ = RFC1533_DOMAINNAME;
-*q++ = val;
-memcpy(q, slirp->vdomainname, val);
-q += val;
+if (q + val + 2 >= end) {
+g_warning("DHCP packet size exceeded, "
+"omitting domain name option.");
+} else {
+*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;
-if (val + 1 > spaceleft) {
+if (q + val >= end) {
g_warning("DHCP packet size exceeded, "
"omitting domain-search option.");
} else {
@@ -331,6 +342,7 @@ static void bootp_reply(Slirp *slirp, const struct bootp_t
*bp)
memcpy(q, nak_msg, sizeof(nak_msg) - 1);
q += sizeof(nak_msg) - 1;
}
+assert(q < end);
*q = RFC1533_END;
daddr.sin_addr.s_addr = 0xu;
--
2.19.1
[Qemu-devel] [PULL 2/2] slirp: Implement RFC2132 TFTP server name
From: Fam Zheng
This new usernet option can be used to add data for option 66 (tftp
server name) in the BOOTP reply, which is useful in PXE based automatic
OS install such as OpenBSD.
Signed-off-by: Fam Zheng
Reviewed-by: Thomas Huth
Tested-by: Gerd Hoffmann
Signed-off-by: Samuel Thibault
---
net/slirp.c | 12 ++--
qapi/net.json| 5 -
qemu-options.hx | 7 ++-
slirp/bootp.c| 13 +
slirp/bootp.h| 1 +
slirp/libslirp.h | 1 +
slirp/slirp.c| 2 ++
slirp/slirp.h| 1 +
8 files changed, 38 insertions(+), 4 deletions(-)
diff --git a/net/slirp.c b/net/slirp.c
index da6c0a1a5c..f6dc03963a 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -150,6 +150,7 @@ static int net_slirp_init(NetClientState *peer, const char
*model,
const char *vnameserver, const char *vnameserver6,
const char *smb_export, const char *vsmbserver,
const char **dnssearch, const char *vdomainname,
+ const char *tftp_server_name,
Error **errp)
{
/* default settings according to historic slirp */
@@ -360,6 +361,11 @@ static int net_slirp_init(NetClientState *peer, const char
*model,
return -1;
}
+if (tftp_server_name && strlen(tftp_server_name) > 255) {
+error_setg(errp, "'tftp-server-name' parameter cannot exceed 255
bytes");
+return -1;
+}
+
nc = qemu_new_net_client(&net_slirp_info, peer, model, name);
snprintf(nc->info_str, sizeof(nc->info_str),
@@ -370,7 +376,8 @@ 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,
+ vhostname, tftp_server_name,
+ tftp_export, bootfile, dhcp,
dns, ip6_dns, dnssearch, vdomainname, s);
QTAILQ_INSERT_TAIL(&slirp_stacks, s, entry);
@@ -907,7 +914,8 @@ 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, user->domainname, errp);
+ user->smbserver, dnssearch, user->domainname,
+ user->tftp_server_name, errp);
while (slirp_configs) {
config = slirp_configs;
diff --git a/qapi/net.json b/qapi/net.json
index c86f351161..8f99fd911d 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -174,6 +174,8 @@
#
# @guestfwd: forward guest TCP connections
#
+# @tftp-server-name: RFC2132 "TFTP server name" string (Since 3.1)
+#
# Since: 1.2
##
{ 'struct': 'NetdevUserOptions',
@@ -198,7 +200,8 @@
'*smb': 'str',
'*smbserver': 'str',
'*hostfwd': ['String'],
-'*guestfwd': ['String'] } }
+'*guestfwd': ['String'],
+'*tftp-server-name': 'str' } }
##
# @NetdevTapOptions:
diff --git a/qemu-options.hx b/qemu-options.hx
index 214ce396f9..08f8516a9a 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1823,7 +1823,7 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
" [,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][,domainname=domain]\n"
-" [,tftp=dir][,bootfile=f][,hostfwd=rule][,guestfwd=rule]"
+"
[,tftp=dir][,tftp-server-name=name][,bootfile=f][,hostfwd=rule][,guestfwd=rule]"
#ifndef _WIN32
"[,smb=dir[,smbserver=addr]]\n"
#endif
@@ -2060,6 +2060,11 @@ server. The files in @var{dir} will be exposed as the
root of a TFTP server.
The TFTP client on the guest must be configured in binary mode (use the command
@code{bin} of the Unix TFTP client).
+@item tftp-server-name=@var{name}
+In BOOTP reply, broadcast @var{name} as the "TFTP server name" (RFC2132 option
+66). This can be used to advise the guest to load boot files or configurations
+from a different server than the host address.
+
@item bootfile=@var{file}
When using the user mode network stack, broadcast @var{file} as the BOOTP
filename. In conjunction with @option{tftp}, this can be used to network boot
diff --git a/slirp/bootp.c b/slirp/bootp.c
index 1e8185f0ec..7b1af73c95 100644
--- a/slirp/bootp.c
+++ b/slirp/bootp.c
@@ -318,6 +318,19 @@
[Qemu-devel] [PULL 0/3] Slirp updates
The following changes since commit 3c2d3042849686969add641bd38b08b9877b9e8f: Merge remote-tracking branch 'remotes/mcayland/tags/qemu-openbios.for-upstream-20181005' into staging (2018-10-05 17:55:22 +0100) are available in the Git repository at: https://people.debian.org/~sthibault/qemu.git tags/samuel-thibault for you to fetch changes up to 93a972f8548571d35c718ca3a94d5ab1507b2443: slirp: Propagate host TCP RST packet to the guest after socket disconnected (2018-10-07 19:50:48 +0200) slirp updates Andrew Oates (1): slirp: fix ICMP handling on macOS hosts Gavin Grant (1): slirp: Propagate host TCP RST packet to the guest after socket disconnected Peter Maydell (1): slirp: document mbuf pointers and sizes Andrew Oates (1): slirp: fix ICMP handling on macOS hosts Gavin Grant (1): slirp: Propagate host TCP RST packet to the guest after socket disconnected Peter Maydell (1): slirp: document mbuf pointers and sizes slirp/ip_icmp.c | 27 ++- slirp/mbuf.c| 14 +++--- slirp/mbuf.h| 13 + slirp/socket.c | 13 ++--- 4 files changed, 56 insertions(+), 11 deletions(-)
[Qemu-devel] [PULL 2/3] slirp: fix ICMP handling on macOS hosts
From: Andrew Oates
On Linux, SOCK_DGRAM+IPPROTO_ICMP sockets give only the ICMP packet when
read from. On macOS, however, the socket acts like a SOCK_RAW socket
and includes the IP header as well.
This change strips the extra IP header from the received packet on macOS
before sending it to the guest. SOCK_DGRAM ICMP sockets aren't
supported on other BSDs, but we enable this behavior for them as well to
treat the sockets the same as raw sockets.
Signed-off-by: Andrew Oates
Signed-off-by: Samuel Thibault
---
slirp/ip_icmp.c | 27 ++-
1 file changed, 26 insertions(+), 1 deletion(-)
diff --git a/slirp/ip_icmp.c b/slirp/ip_icmp.c
index 0b667a429a..da100d1f55 100644
--- a/slirp/ip_icmp.c
+++ b/slirp/ip_icmp.c
@@ -420,7 +420,32 @@ void icmp_receive(struct socket *so)
icp = mtod(m, struct icmp *);
id = icp->icmp_id;
-len = qemu_recv(so->s, icp, m->m_len, 0);
+len = qemu_recv(so->s, icp, M_ROOM(m), 0);
+/*
+ * The behavior of reading SOCK_DGRAM+IPPROTO_ICMP sockets is inconsistent
+ * between host OSes. On Linux, only the ICMP header and payload is
+ * included. On macOS/Darwin, the socket acts like a raw socket and
+ * includes the IP header as well. On other BSDs, SOCK_DGRAM+IPPROTO_ICMP
+ * sockets aren't supported at all, so we treat them like raw sockets. It
+ * isn't possible to detect this difference at runtime, so we must use an
+ * #ifdef to determine if we need to remove the IP header.
+ */
+#ifdef CONFIG_BSD
+if (len >= sizeof(struct ip)) {
+struct ip *inner_ip = mtod(m, struct ip *);
+int inner_hlen = inner_ip->ip_hl << 2;
+if (inner_hlen > len) {
+len = -1;
+errno = -EINVAL;
+} else {
+len -= inner_hlen;
+memmove(icp, (unsigned char *)icp + inner_hlen, len);
+}
+} else {
+ len = -1;
+ errno = -EINVAL;
+}
+#endif
icp->icmp_id = id;
m->m_data -= hlen;
--
2.19.0
[Qemu-devel] [PULL 3/3] slirp: Propagate host TCP RST packet to the guest after socket disconnected
From: Gavin Grant
Commit 27d92ebc5ed1bb0b518d0ebc4c609182ad20a799 handled the case where the TCP
connection is abruptly closed via a RST packet, by checking for the ECONNRESET
errno. However it does not consider the case where the connection has been
half-closed by the host (FIN/ACK), then the host socket is disconnected. For
example, if the host application calls close() on the socket, then the
application exits.
In this case, the socket still exists due to the file descriptor in SLIRP, but
it is disconnected. recv() does not indicate an error since an orderly socket
close has previously occurred. The socket will then be stuck in FIN_WAIT_2,
until the peer sends FIN/ACK or a timeout occurs. Instead we can send a RST
to the peer and transition to the CLOSED state.
Signed-off-by: Gavin Grant
Signed-off-by: Samuel Thibault
---
slirp/socket.c | 13 ++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/slirp/socket.c b/slirp/socket.c
index 08fe98907d..322383a1f9 100644
--- a/slirp/socket.c
+++ b/slirp/socket.c
@@ -204,12 +204,19 @@ soread(struct socket *so)
return 0;
else {
int err;
- socklen_t slen = sizeof err;
+ socklen_t elen = sizeof err;
+ struct sockaddr_storage addr;
+ struct sockaddr *paddr = (struct sockaddr *) &addr;
+ socklen_t alen = sizeof addr;
err = errno;
if (nn == 0) {
- getsockopt(so->s, SOL_SOCKET, SO_ERROR,
- &err, &slen);
+ if (getpeername(so->s, paddr, &alen) < 0) {
+ err = errno;
+ } else {
+ getsockopt(so->s, SOL_SOCKET, SO_ERROR,
+ &err, &elen);
+ }
}
DEBUG_MISC((dfd, " --- soread() disconnected, nn = %d,
errno = %d-%s\n", nn, errno,strerror(errno)));
--
2.19.0
[Qemu-devel] [PULL 1/3] slirp: document mbuf pointers and sizes
From: Peter Maydell
and fix confusing datasize name into gapsize in m_inc.
Signed-off-by: Peter Maydell
Signed-off-by: Samuel Thibault
---
slirp/mbuf.c | 14 +++---
slirp/mbuf.h | 13 +
2 files changed, 20 insertions(+), 7 deletions(-)
diff --git a/slirp/mbuf.c b/slirp/mbuf.c
index 1b7868355a..aa1f28afb1 100644
--- a/slirp/mbuf.c
+++ b/slirp/mbuf.c
@@ -151,7 +151,7 @@ m_cat(struct mbuf *m, struct mbuf *n)
void
m_inc(struct mbuf *m, int size)
{
-int datasize;
+int gapsize;
/* some compilers throw up on gotos. This one we can fake. */
if (M_ROOM(m) > size) {
@@ -159,17 +159,17 @@ m_inc(struct mbuf *m, int size)
}
if (m->m_flags & M_EXT) {
-datasize = m->m_data - m->m_ext;
-m->m_ext = g_realloc(m->m_ext, size + datasize);
+gapsize = m->m_data - m->m_ext;
+m->m_ext = g_realloc(m->m_ext, size + gapsize);
} else {
-datasize = m->m_data - m->m_dat;
-m->m_ext = g_malloc(size + datasize);
+gapsize = m->m_data - m->m_dat;
+m->m_ext = g_malloc(size + gapsize);
memcpy(m->m_ext, m->m_dat, m->m_size);
m->m_flags |= M_EXT;
}
-m->m_data = m->m_ext + datasize;
-m->m_size = size + datasize;
+m->m_data = m->m_ext + gapsize;
+m->m_size = size + gapsize;
}
diff --git a/slirp/mbuf.h b/slirp/mbuf.h
index 33b84485d6..bfdf8c4577 100644
--- a/slirp/mbuf.h
+++ b/slirp/mbuf.h
@@ -47,6 +47,19 @@
* free the m_ext. This is inefficient memory-wise, but who cares.
*/
+/*
+ * mbufs allow to have a gap between the start of the allocated buffer (m_ext
if
+ * M_EXT is set, m_dat otherwise) and the in-use data:
+ *
+ * |--gapsize->|---m_len--->
+ * |--m_size-->
+ * |M_ROOM>
+ * |-M_FREEROOM-->
+ *
+ * ^ ^ ^
+ * m_dat/m_ext m_data end of buffer
+ */
+
/*
* How much room is in the mbuf, from m_data to the end of the mbuf
*/
--
2.19.0
Re: [Qemu-devel] [PATCH] slirp: Propagate host TCP RST packet to the guest after socket disconnected
Hello, Gavin Grant, le jeu. 30 août 2018 16:57:57 +0100, a ecrit: > Commit 27d92ebc5ed1bb0b518d0ebc4c609182ad20a799 handled the case where the TCP > connection is abruptly closed via a RST packet, by checking for the ECONNRESET > errno. However it does not consider the case where the connection has been > half-closed by the host (FIN/ACK), then the host socket is disconnected. For > example, if the host application calls close() on the socket, then the > application exits. > > In this case, the socket still exists due to the file descriptor in SLIRP, but > it is disconnected. recv() does not indicate an error since an orderly socket > close has previously occurred. The socket will then be stuck in FIN_WAIT_2, > until the peer sends FIN/ACK or a timeout occurs. Instead we can send a RST > to the peer and transition to the CLOSED state. > > Signed-off-by: Gavin Grant I have fixed it a bit and pushed to my tree, thanks! Samuel
Re: [Qemu-devel] [PATCH v6 24/25] slirp: fix ipv6 timers
Pavel Dovgalyuk, le mer. 12 sept. 2018 11:20:07 +0300, a ecrit:
> ICMP implementation for IPv6 uses timers based on virtual clock.
> This is incorrect because this service is not related to the guest state,
> and its events should not be recorded and replayed.
> This patch changes using virtual clock to the new virtual_ext clock.
>
> Signed-off-by: Pavel Dovgalyuk
As discussed previously
Reviewed-by: Samuel Thibault
> ---
> slirp/ip6_icmp.c |7 ---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/slirp/ip6_icmp.c b/slirp/ip6_icmp.c
> index ee333d0..3f41187 100644
> --- a/slirp/ip6_icmp.c
> +++ b/slirp/ip6_icmp.c
> @@ -17,7 +17,7 @@ static void ra_timer_handler(void *opaque)
> {
> Slirp *slirp = opaque;
> timer_mod(slirp->ra_timer,
> - qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + NDP_Interval);
> + qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL_EXT) + NDP_Interval);
> ndp_send_ra(slirp);
> }
>
> @@ -27,9 +27,10 @@ void icmp6_init(Slirp *slirp)
> return;
> }
>
> -slirp->ra_timer = timer_new_ms(QEMU_CLOCK_VIRTUAL, ra_timer_handler,
> slirp);
> +slirp->ra_timer = timer_new_ms(QEMU_CLOCK_VIRTUAL_EXT,
> + ra_timer_handler, slirp);
> timer_mod(slirp->ra_timer,
> - qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + NDP_Interval);
> + qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL_EXT) + NDP_Interval);
> }
>
> void icmp6_cleanup(Slirp *slirp)
>
>
--
Samuel
csp.tar.gz: ascii text
-+- #ens-mim - vive les browsers qui prennent des initiatives à la con -+-
Re: [Qemu-devel] [PATCH] slirp: Propagate host TCP RST packet to the guest after socket disconnected
Hello,
The principle seems sane, I'll have a look.
Thanks,
Samuel
Gavin Grant, le jeu. 30 août 2018 16:57:57 +0100, a ecrit:
> Commit 27d92ebc5ed1bb0b518d0ebc4c609182ad20a799 handled the case where the TCP
> connection is abruptly closed via a RST packet, by checking for the ECONNRESET
> errno. However it does not consider the case where the connection has been
> half-closed by the host (FIN/ACK), then the host socket is disconnected. For
> example, if the host application calls close() on the socket, then the
> application exits.
>
> In this case, the socket still exists due to the file descriptor in SLIRP, but
> it is disconnected. recv() does not indicate an error since an orderly socket
> close has previously occurred. The socket will then be stuck in FIN_WAIT_2,
> until the peer sends FIN/ACK or a timeout occurs. Instead we can send a RST
> to the peer and transition to the CLOSED state.
>
> Signed-off-by: Gavin Grant
> ---
> slirp/socket.c | 9 +++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/slirp/socket.c b/slirp/socket.c
> index 08fe98907d..543bd5feaf 100644
> --- a/slirp/socket.c
> +++ b/slirp/socket.c
> @@ -204,12 +204,17 @@ soread(struct socket *so)
> return 0;
> else {
> int err;
> + struct sockaddr addr;
> socklen_t slen = sizeof err;
>
> err = errno;
> if (nn == 0) {
> - getsockopt(so->s, SOL_SOCKET, SO_ERROR,
> -&err, &slen);
> + if (getpeername(so->s, &addr, &slen) < 0) {
> + err = errno;
> + } else {
> + getsockopt(so->s, SOL_SOCKET, SO_ERROR,
> + &err, &slen);
> + }
> }
>
> DEBUG_MISC((dfd, " --- soread() disconnected, nn = %d,
> errno = %d-%s\n", nn, errno,strerror(errno)));
> --
> 2.11.0
>
--
Samuel
***e trouve un .xls
***e passe un moment à se demander quelle version de xml c'est ça, le .xls
e: donc j'ai fait un file
-+- #sos - on n'a pas forcément les mêmes références que tout le monde -+-
Re: [Qemu-devel] [PATCH 4/4] net: Remove the deprecated -tftp, -bootp, -redir and -smb options
Thomas Huth, le lun. 27 août 2018 16:54:12 +0200, a ecrit:
> These options likely do not work as expected as soon as the user
> tries to use more than one network interface at once. The parameters
> have been marked as deprecated since QEMU v2.6, so users had plenty
> of time to move their scripts to the new syntax. Time to remove the
> old parameters now.
>
> Signed-off-by: Thomas Huth
Reviewed-by: Samuel Thibault
> ---
> include/net/net.h| 3 --
> include/net/slirp.h | 4 --
> net/slirp.c | 132
> +++
> os-posix.c | 8
> qemu-deprecated.texi | 34 -
> qemu-options.hx | 15 --
> vl.c | 18 ---
> 7 files changed, 29 insertions(+), 185 deletions(-)
>
> diff --git a/include/net/net.h b/include/net/net.h
> index 1425960..7936d53 100644
> --- a/include/net/net.h
> +++ b/include/net/net.h
> @@ -201,9 +201,6 @@ extern NICInfo nd_table[MAX_NICS];
> extern const char *host_net_devices[];
>
> /* from net.c */
> -extern const char *legacy_tftp_prefix;
> -extern const char *legacy_bootp_filename;
> -
> int net_client_parse(QemuOptsList *opts_list, const char *str);
> int net_init_clients(Error **errp);
> void net_check_clients(void);
> diff --git a/include/net/slirp.h b/include/net/slirp.h
> index 4d63d74..bad3e1e 100644
> --- a/include/net/slirp.h
> +++ b/include/net/slirp.h
> @@ -30,10 +30,6 @@
> void hmp_hostfwd_add(Monitor *mon, const QDict *qdict);
> void hmp_hostfwd_remove(Monitor *mon, const QDict *qdict);
>
> -int net_slirp_redir(const char *redir_str);
> -
> -int net_slirp_smb(const char *exported_dir);
> -
> void hmp_info_usernet(Monitor *mon, const QDict *qdict);
>
> #endif
> diff --git a/net/slirp.c b/net/slirp.c
> index 1e14318..c18060f 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -67,13 +67,11 @@ static int get_str_sep(char *buf, int buf_size, const
> char **pp, int sep)
> /* slirp network adapter */
>
> #define SLIRP_CFG_HOSTFWD 1
> -#define SLIRP_CFG_LEGACY 2
>
> struct slirp_config_str {
> struct slirp_config_str *next;
> int flags;
> char str[1024];
> -int legacy_format;
> };
>
> typedef struct SlirpState {
> @@ -87,19 +85,13 @@ typedef struct SlirpState {
> } SlirpState;
>
> static struct slirp_config_str *slirp_configs;
> -const char *legacy_tftp_prefix;
> -const char *legacy_bootp_filename;
> static QTAILQ_HEAD(slirp_stacks, SlirpState) slirp_stacks =
> QTAILQ_HEAD_INITIALIZER(slirp_stacks);
>
> -static int slirp_hostfwd(SlirpState *s, const char *redir_str,
> - int legacy_format, Error **errp);
> -static int slirp_guestfwd(SlirpState *s, const char *config_str,
> - int legacy_format, Error **errp);
> +static int slirp_hostfwd(SlirpState *s, const char *redir_str, Error **errp);
> +static int slirp_guestfwd(SlirpState *s, const char *config_str, Error
> **errp);
>
> #ifndef _WIN32
> -static const char *legacy_smb_export;
> -
> static int slirp_smb(SlirpState *s, const char *exported_dir,
> struct in_addr vserver_addr, Error **errp);
> static void slirp_smb_cleanup(SlirpState *s);
> @@ -196,13 +188,6 @@ static int net_slirp_init(NetClientState *peer, const
> char *model,
> return -1;
> }
>
> -if (!tftp_export) {
> -tftp_export = legacy_tftp_prefix;
> -}
> -if (!bootfile) {
> -bootfile = legacy_bootp_filename;
> -}
> -
> if (vnetwork) {
> if (get_str_sep(buf, sizeof(buf), &vnetwork, '/') < 0) {
> if (!inet_aton(vnetwork, &net)) {
> @@ -382,21 +367,16 @@ static int net_slirp_init(NetClientState *peer, const
> char *model,
>
> for (config = slirp_configs; config; config = config->next) {
> if (config->flags & SLIRP_CFG_HOSTFWD) {
> -if (slirp_hostfwd(s, config->str,
> - config->flags & SLIRP_CFG_LEGACY, errp) < 0) {
> +if (slirp_hostfwd(s, config->str, errp) < 0) {
> goto error;
> }
> } else {
> -if (slirp_guestfwd(s, config->str,
> - config->flags & SLIRP_CFG_LEGACY, errp) < 0) {
> +if (slirp_guestfwd(s, config->str, errp) < 0) {
> goto error;
> }
> }
> }
> #ifndef _WIN32
> -if (!smb_export) {
> -smb_export = legacy_smb_export;
> -}
> if (smb_export) {
> if (slirp_smb(s,
Re: [Qemu-devel] [PATCH] slirp: Implement RFC2132 TFTP server name
Hello,
Fam Zheng, le ven. 24 août 2018 21:53:12 +0800, a ecrit:
>const char *vnameserver, const char *vnameserver6,
>const char *smb_export, const char *vsmbserver,
>const char **dnssearch, const char *vdomainname,
> + const char *tftp_server_name,
I'd say rather put it between the vhostname and tftp_export parameters.
> @@ -321,6 +322,9 @@ Slirp *slirp_init(int restricted, bool in_enabled, struct
> in_addr vnetwork,
> slirp->vdhcp_startaddr = vdhcp_start;
> slirp->vnameserver_addr = vnameserver;
> slirp->vnameserver_addr6 = vnameserver6;
> +if (tftp_server_name) {
> +slirp->tftp_server_name = g_strdup(tftp_server_name);
> +}
I'd say do not bother testing for tftp_server_name != NULL, just always
use g_strdup, as is done for other values.
Samuel
Re: [Qemu-devel] [PATCH v4] slirp: fix ICMP handling on macOS hosts
and...@andrewoates.com, le mer. 15 août 2018 20:18:45 -0400, a ecrit: > v4: drop packets that are too short for an IP header Applied to my tree, thanks! Samuel
Re: [Qemu-devel] [PATCH v3] slirp: fix ICMP handling on macOS hosts
Andrew Oates, le mar. 14 août 2018 22:35:21 -0400, a ecrit:
> On Linux, SOCK_DGRAM+IPPROTO_ICMP sockets give only the ICMP packet when
> read from. On macOS, however, the socket acts like a SOCK_RAW socket
> and includes the IP header as well.
>
> This change strips the extra IP header from the received packet on macOS
> before sending it to the guest. SOCK_DGRAM ICMP sockets aren't
> supported on other BSDs, but we enable this behavior for them as well to
> treat the sockets the same as raw sockets.
>
> Signed-off-by: Andrew Oates
> ---
> v2: check validity of inner_hlen and update len appropriately
> v3: CONFIG_DARWIN -> CONFIG_BSD; add comment explaining #ifdef
>
> slirp/ip_icmp.c | 24 +++-
> 1 file changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/slirp/ip_icmp.c b/slirp/ip_icmp.c
> index 0b667a429a..0e289fd9d9 100644
> --- a/slirp/ip_icmp.c
> +++ b/slirp/ip_icmp.c
> @@ -420,7 +420,29 @@ void icmp_receive(struct socket *so)
> icp = mtod(m, struct icmp *);
>
> id = icp->icmp_id;
> -len = qemu_recv(so->s, icp, m->m_len, 0);
> +len = qemu_recv(so->s, icp, M_ROOM(m), 0);
> +/*
> + * The behavior of reading SOCK_DGRAM+IPPROTO_ICMP sockets is
> inconsistent
> + * between host OSes. On Linux, only the ICMP header and payload is
> + * included. On macOS/Darwin, the socket acts like a raw socket and
> + * includes the IP header as well. On other BSDs,
> SOCK_DGRAM+IPPROTO_ICMP
> + * sockets aren't supported at all, so we treat them like raw sockets.
> It
> + * isn't possible to detect this difference at runtime, so we must use an
> + * #ifdef to determine if we need to remove the IP header.
> + */
> +#ifdef CONFIG_BSD
> +if (len > 0) {
Looking at it again, this used to be
if (len >= sizeof(struct ip)) {
shouldn't be it that way so that
> +struct ip *inner_ip = mtod(m, struct ip *);
> +int inner_hlen = inner_ip->ip_hl << 2;
Reading in the header doesn't give uninitialized values?
I guess that to be on the safe side and trigger explicit warnings, we
should reject with EINVAL packets which are smaller than an IP header?
> +if (inner_hlen > len) {
> +len = -1;
> +errno = -EINVAL;
> +} else {
> +len -= inner_hlen;
> +memmove(icp, (unsigned char *)icp + inner_hlen, len);
> +}
> +}
> +#endif
> icp->icmp_id = id;
>
> m->m_data -= hlen;
> --
> 2.18.0.865.gffc8e1a3cd6-goog
>
--
Samuel
(* If you have a precise idea of the intended use of the following code,
please
write to eduardo.gime...@inria.fr and ask for the prize :-)
-- Eduardo (11/8/97) *)
-+- N sur #ens-mim - et c'était un des développeurs -+-
Re: [Qemu-devel] [PATCH v3] slirp: fix ICMP handling on macOS hosts
Andrew Oates, le mar. 14 août 2018 22:35:21 -0400, a ecrit:
> On Linux, SOCK_DGRAM+IPPROTO_ICMP sockets give only the ICMP packet when
> read from. On macOS, however, the socket acts like a SOCK_RAW socket
> and includes the IP header as well.
>
> This change strips the extra IP header from the received packet on macOS
> before sending it to the guest. SOCK_DGRAM ICMP sockets aren't
> supported on other BSDs, but we enable this behavior for them as well to
> treat the sockets the same as raw sockets.
Applied to my tree, thanks!
> Signed-off-by: Andrew Oates
> ---
> v2: check validity of inner_hlen and update len appropriately
> v3: CONFIG_DARWIN -> CONFIG_BSD; add comment explaining #ifdef
>
> slirp/ip_icmp.c | 24 +++-
> 1 file changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/slirp/ip_icmp.c b/slirp/ip_icmp.c
> index 0b667a429a..0e289fd9d9 100644
> --- a/slirp/ip_icmp.c
> +++ b/slirp/ip_icmp.c
> @@ -420,7 +420,29 @@ void icmp_receive(struct socket *so)
> icp = mtod(m, struct icmp *);
>
> id = icp->icmp_id;
> -len = qemu_recv(so->s, icp, m->m_len, 0);
> +len = qemu_recv(so->s, icp, M_ROOM(m), 0);
> +/*
> + * The behavior of reading SOCK_DGRAM+IPPROTO_ICMP sockets is
> inconsistent
> + * between host OSes. On Linux, only the ICMP header and payload is
> + * included. On macOS/Darwin, the socket acts like a raw socket and
> + * includes the IP header as well. On other BSDs,
> SOCK_DGRAM+IPPROTO_ICMP
> + * sockets aren't supported at all, so we treat them like raw sockets.
> It
> + * isn't possible to detect this difference at runtime, so we must use an
> + * #ifdef to determine if we need to remove the IP header.
> + */
> +#ifdef CONFIG_BSD
> +if (len > 0) {
> +struct ip *inner_ip = mtod(m, struct ip *);
> +int inner_hlen = inner_ip->ip_hl << 2;
> +if (inner_hlen > len) {
> +len = -1;
> +errno = -EINVAL;
> +} else {
> +len -= inner_hlen;
> +memmove(icp, (unsigned char *)icp + inner_hlen, len);
> +}
> +}
> +#endif
> icp->icmp_id = id;
>
> m->m_data -= hlen;
> --
> 2.18.0.865.gffc8e1a3cd6-goog
>
--
Samuel
As usual, this being a 1.3.x release, I haven't even compiled this
kernel yet. So if it works, you should be doubly impressed.
(Linus Torvalds, announcing kernel 1.3.3 on the linux-kernel mailing list.)
[Qemu-devel] [PATCH] slirp: document mbuf pointers and sizes
From: Peter Maydell
and fix confusing datasize name into gapsize in m_inc.
Signed-off-by: Peter Maydell
Signed-off-by: Samuel Thibault
---
slirp/mbuf.c | 14 +++---
slirp/mbuf.h | 13 +
2 files changed, 20 insertions(+), 7 deletions(-)
diff --git a/slirp/mbuf.c b/slirp/mbuf.c
index 1b7868355a..aa1f28afb1 100644
--- a/slirp/mbuf.c
+++ b/slirp/mbuf.c
@@ -151,7 +151,7 @@ m_cat(struct mbuf *m, struct mbuf *n)
void
m_inc(struct mbuf *m, int size)
{
-int datasize;
+int gapsize;
/* some compilers throw up on gotos. This one we can fake. */
if (M_ROOM(m) > size) {
@@ -159,17 +159,17 @@ m_inc(struct mbuf *m, int size)
}
if (m->m_flags & M_EXT) {
-datasize = m->m_data - m->m_ext;
-m->m_ext = g_realloc(m->m_ext, size + datasize);
+gapsize = m->m_data - m->m_ext;
+m->m_ext = g_realloc(m->m_ext, size + gapsize);
} else {
-datasize = m->m_data - m->m_dat;
-m->m_ext = g_malloc(size + datasize);
+gapsize = m->m_data - m->m_dat;
+m->m_ext = g_malloc(size + gapsize);
memcpy(m->m_ext, m->m_dat, m->m_size);
m->m_flags |= M_EXT;
}
-m->m_data = m->m_ext + datasize;
-m->m_size = size + datasize;
+m->m_data = m->m_ext + gapsize;
+m->m_size = size + gapsize;
}
diff --git a/slirp/mbuf.h b/slirp/mbuf.h
index 33b84485d6..bfdf8c4577 100644
--- a/slirp/mbuf.h
+++ b/slirp/mbuf.h
@@ -47,6 +47,19 @@
* free the m_ext. This is inefficient memory-wise, but who cares.
*/
+/*
+ * mbufs allow to have a gap between the start of the allocated buffer (m_ext
if
+ * M_EXT is set, m_dat otherwise) and the in-use data:
+ *
+ * |--gapsize->|---m_len--->
+ * |--m_size-->
+ * |M_ROOM>
+ * |-M_FREEROOM-->
+ *
+ * ^ ^ ^
+ * m_dat/m_ext m_data end of buffer
+ */
+
/*
* How much room is in the mbuf, from m_data to the end of the mbuf
*/
--
2.18.0
Re: [Qemu-devel] [PATCH for-3.0] slirp: Correct size check in m_inc()
Peter Maydell, le ven. 10 août 2018 10:02:37 +0100, a ecrit: > (a) you should add a comment describing what 'gapsize' > is, ie that there may be a gap between the in-use data and the > start of the allocated buffer, and > (b) m_inc() should change its variable name to match. Right, I have done so in my tree. Do you prefer to have it pushed for qemu 3.0? Samuel
Re: [Qemu-devel] [PATCH for-3.0] slirp: Correct size check in m_inc()
Dr. David Alan Gilbert, le jeu. 09 août 2018 12:32:05 +0100, a ecrit:
> >|--datasize>|---m_len--->
> >|--m_size-->
> >|M_ROOM>
> > |-M_FREEROOM-->
> >
> >^ ^ ^
> >m_dat m_data end of buffer
> >
> > ("datasize" is a bit misnamed, as it's "size of the leading
> > gap between the start of the buffer and the data"; "gapsize"
> > would be more helpful.)
> >
> > Anyway, we allocate size + datasize, and
> > m_size == datasize + M_ROOM. We know that size >= M_ROOM,
> > so the allocated buffer must be at least m_size big.
>
> Ah OK, thanks.
> (That ascii art could do with being in a comment somewhere!)
Indeed. Peter, maybe your Signed-off-by on this? :)
Samuel
commit 4be85a1eeb6b19e91491e689d4d0d054030cbb49
Author: Peter Maydell
Date: Thu Aug 9 23:52:59 2018 +0200
slirp: document mbuf pointers and sizes
Signed-off-by: Samuel Thibault
diff --git a/slirp/mbuf.h b/slirp/mbuf.h
index 33b84485d6..a5bb3f9e66 100644
--- a/slirp/mbuf.h
+++ b/slirp/mbuf.h
@@ -47,6 +47,16 @@
* free the m_ext. This is inefficient memory-wise, but who cares.
*/
+/*
+ * |--gapsize->|---m_len--->
+ * |--m_size-->
+ * |M_ROOM>
+ * |-M_FREEROOM-->
+ *
+ * ^ ^ ^
+ * m_dat/m_ext m_data end of buffer
+ */
+
/*
* How much room is in the mbuf, from m_data to the end of the mbuf
*/
Re: [Qemu-devel] [PATCH for-3.0] slirp: Correct size check in m_inc()
Peter Maydell, le mar. 07 août 2018 12:45:01 +0100, a ecrit:
> The data in an mbuf buffer is not necessarily at the start of the
> allocated buffer. (For instance m_adj() allows data to be trimmed
> from the start by just advancing the pointer and reducing the length.)
> This means that the allocated buffer size (m->m_size) and the
> amount of space from the m_data pointer to the end of the
> buffer (M_ROOM(m)) are not necessarily the same.
>
> Commit 864036e251f54c9 tried to change the m_inc() function from
> taking the new allocated-buffer-size to taking the new room-size,
> but forgot to change the initial "do we already have enough space"
> check. This meant that if we were trying to extend a buffer which
> had a leading gap between the buffer start and the data, we might
> incorrectly decide it didn't need to be extended, and then
> overrun the end of the buffer, causing memory corruption and
> an eventual crash.
>
> Change the "already big enough?" condition from checking the
> argument against m->m_size to checking against M_ROOM().
> This only makes a difference for the callsite in m_cat();
> the other three callsites all start with a freshly allocated
> mbuf from m_get(), which will have m->m_size == M_ROOM(m).
>
> Fixes: 864036e251f54c9
> Fixes: https://bugs.launchpad.net/qemu/+bug/1785670
> Signed-off-by: Peter Maydell
Reviewed-by: Samuel Thibault
> ---
> slirp/mbuf.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/slirp/mbuf.c b/slirp/mbuf.c
> index 0c189e1a7bf..1b7868355a3 100644
> --- a/slirp/mbuf.c
> +++ b/slirp/mbuf.c
> @@ -154,7 +154,7 @@ m_inc(struct mbuf *m, int size)
> int datasize;
>
> /* some compilers throw up on gotos. This one we can fake. */
> -if (m->m_size > size) {
> +if (M_ROOM(m) > size) {
> return;
> }
>
> --
> 2.17.1
>
>
--
Samuel
"And the next time you consider complaining that running Lucid Emacs
19.05 via NFS from a remote Linux machine in Paraguay doesn't seem to
get the background colors right, you'll know who to thank."
(By Matt Welsh)
Re: [Qemu-devel] [PATCH v5 24/24] slirp: fix ipv6 timers
Pavel Dovgalyuk, le mar. 31 juil. 2018 09:58:26 +0300, a ecrit: > > From: Samuel Thibault [mailto:samuel.thiba...@gnu.org] > > Pavel Dovgalyuk, le jeu. 26 juil. 2018 11:37:57 +0300, a ecrit: > > > Or the timers are related to the network devices (e.g., servers in the > > > outer world)? > > > > No. > > > > > > > > > this service is not related to the guest state. > > > > > > > > seems incorrect. At the moment the ip6_icmp timer's current value is not > > > > saved in the guest state, but in principle it should, so that the guest > > > > does see the RAs at a regular rate. In practice we don't care because > > > > the timing is randomized anyway. > > > > > > Isn't this just a side effect? > > > I mean that slirp may be replaced by, say, tap, and the guest should not > > > notice > > > the difference. > > > > Well, if a guest is connected through a tap, the virtual time should > > really run as fast as the realtime, and it should not be paused. > > Otherwise TCP connections will break since the guest won't be able to > > reply fast enough, without even knowing about the issue. Slirp can > > compensate this thanks to a buffer between what happens in the real > > world and what happens in the virtual world. Real world timings are > > handled by the OS socket implementation, and virtual world timings are > > handled with the qemu timer. > > Then maybe the solution is the new clock with the frequency of the virtual > clock, but which does not affect the replayed core? > This clock should stop when VM is paused. > It also could be saved in vmstate. As it does not affect the replay, > saving and restoring its state won't break anything. I guess so. Samuel
[Qemu-devel] [PULL 0/1] slirp fix for qemu 3.0
The following changes since commit 18a398f6a39df4b08ff86ac0d38384193ca5f4cc: Update version for v3.0.0-rc2 release (2018-07-24 22:06:31 +0100) are available in the Git repository at: https://people.debian.org/~sthibault/qemu.git tags/samuel-thibault for you to fetch changes up to 176e8672834d2038e744d7f230c3d75ecfec66aa: slirp: fix ICMP handling on macOS hosts (2018-07-29 16:28:58 +0200) slirp fixes Andrew Oates (1): slirp: fix ICMP handling on macOS hosts Andrew Oates (1): slirp: fix ICMP handling on macOS hosts slirp/ip_icmp.c | 16 +++- 1 file changed, 15 insertions(+), 1 deletion(-)
Re: [Qemu-devel] [PATCH v2] slirp: fix ICMP handling on macOS hosts
Hello,
aoa...@google.com, le sam. 28 juil. 2018 21:54:26 -0400, a ecrit:
> From: Andrew Oates
>
> On Linux, SOCK_DGRAM+IPPROTO_ICMP sockets give only the ICMP packet when
> read from. On macOS, however, the socket acts like a SOCK_RAW socket
> and includes the IP header as well.
>
> This change strips the extra IP header from the received packet on macOS
> before sending it to the guest.
>
> Signed-off-by: Andrew Oates
Applied to my tree, thanks!
> ---
> v2: check validity of inner_hlen and update len appropriately
>
> slirp/ip_icmp.c | 16 +++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/slirp/ip_icmp.c b/slirp/ip_icmp.c
> index 0b667a429a..76c6d54b11 100644
> --- a/slirp/ip_icmp.c
> +++ b/slirp/ip_icmp.c
> @@ -420,7 +420,21 @@ void icmp_receive(struct socket *so)
> icp = mtod(m, struct icmp *);
>
> id = icp->icmp_id;
> -len = qemu_recv(so->s, icp, m->m_len, 0);
> +len = qemu_recv(so->s, icp, M_ROOM(m), 0);
> +#ifdef CONFIG_DARWIN
> +if (len > 0) {
> +/* Skip the IP header that OS X (unlike Linux) includes. */
> +struct ip *inner_ip = mtod(m, struct ip *);
> +int inner_hlen = inner_ip->ip_hl << 2;
> +if (inner_hlen > len) {
> +len = -1;
> +errno = -EINVAL;
> +} else {
> +len -= inner_hlen;
> +memmove(icp, (unsigned char *)icp + inner_hlen, len);
> +}
> +}
> +#endif
> icp->icmp_id = id;
>
> m->m_data -= hlen;
> --
> 2.17.0
>
--
Samuel
"...Unix, MS-DOS, and Windows NT (also known as the Good, the Bad, and
the Ugly)."
(By Matt Welsh)
[Qemu-devel] [PULL 1/1] slirp: fix ICMP handling on macOS hosts
From: Andrew Oates
On Linux, SOCK_DGRAM+IPPROTO_ICMP sockets give only the ICMP packet when
read from. On macOS, however, the socket acts like a SOCK_RAW socket
and includes the IP header as well.
This change strips the extra IP header from the received packet on macOS
before sending it to the guest.
Signed-off-by: Andrew Oates
Signed-off-by: Samuel Thibault
---
slirp/ip_icmp.c | 16 +++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/slirp/ip_icmp.c b/slirp/ip_icmp.c
index 0b667a429a..6316427ed3 100644
--- a/slirp/ip_icmp.c
+++ b/slirp/ip_icmp.c
@@ -420,7 +420,21 @@ void icmp_receive(struct socket *so)
icp = mtod(m, struct icmp *);
id = icp->icmp_id;
-len = qemu_recv(so->s, icp, m->m_len, 0);
+len = qemu_recv(so->s, icp, M_ROOM(m), 0);
+#ifdef CONFIG_DARWIN
+if (len >= sizeof(struct ip)) {
+/* Skip the IP header that OS X (unlike Linux) includes. */
+struct ip *inner_ip = mtod(m, struct ip *);
+int inner_hlen = inner_ip->ip_hl << 2;
+if (inner_hlen > len) {
+len = -1;
+errno = -EINVAL;
+} else {
+len -= inner_hlen;
+memmove(icp, (unsigned char *)icp + inner_hlen, len);
+}
+}
+#endif
icp->icmp_id = id;
m->m_data -= hlen;
--
2.18.0
Re: [Qemu-devel] [PATCH] slirp: fix ICMP handling on macOS hosts
Hello,
aoa...@google.com, le mer. 25 juil. 2018 21:08:12 -0400, a ecrit:
> From: Andrew Oates
>
> On Linux, SOCK_DGRAM+IPPROTO_ICMP sockets give only the ICMP packet when
> read from. On macOS, however, the socket acts like a SOCK_RAW socket
> and includes the IP header as well.
>
> This change strips the extra IP header from the received packet on macOS
> before sending it to the guest.
>
> Signed-off-by: Andrew Oates
> ---
> slirp/ip_icmp.c | 10 +-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/slirp/ip_icmp.c b/slirp/ip_icmp.c
> index 0b667a429a..5fa67814f4 100644
> --- a/slirp/ip_icmp.c
> +++ b/slirp/ip_icmp.c
> @@ -420,7 +420,15 @@ void icmp_receive(struct socket *so)
> icp = mtod(m, struct icmp *);
>
> id = icp->icmp_id;
> -len = qemu_recv(so->s, icp, m->m_len, 0);
> +len = qemu_recv(so->s, icp, M_ROOM(m), 0);
> +#ifdef CONFIG_DARWIN
> +if (len > 0) {
> +/* Skip the IP header that OS X (unlike Linux) includes. */
> +struct ip *inner_ip = mtod(m, struct ip *);
> +int inner_hlen = inner_ip->ip_hl << 2;
> +memmove(icp, (unsigned char *)icp + inner_hlen, len - inner_hlen);
Please also check that the provided inner_hlen is not bigger than len.
(otherwise it'd be a security issue). Also, substract inner_len from len
for coherency. In case inner_len is bigger than len, you'd set len to -1
and set errno to EINVAL, handled below.
Thanks,
Samuel
Re: [Qemu-devel] [PATCH v5 24/24] slirp: fix ipv6 timers
Pavel Dovgalyuk, le jeu. 26 juil. 2018 11:37:57 +0300, a ecrit: > > From: Samuel Thibault [mailto:samuel.thiba...@gnu.org] > > Pavel Dovgalyuk, le jeu. 26 juil. 2018 10:08:29 +0300, a ecrit: > > > virtual clock should be used by the virtual devices. > > > slirp module is not the virtual device. Therefore processed packets > > > become visible to the guest after passing to the virtual network card. > > > Before that it can create any timers that should not change the state of > > > the guest. > > > > I'm not sure I understand that part correctly. slirp is not a "device" > > strictly speaking, but it has a whole foot in the virtual world. All > > TCP/UDP/ARP/RA timings are related to the guest timing, so > > I don't know all details of slirp, so let me ask: > if the virtual timer runs very slowly (when it configured this way with > icount option), > should the timings relate this speed? Yes. Otherwise the guest will not be fast enough to answer promptly according to slirp's TCP delays. > Or the timers are related to the network devices (e.g., servers in the > outer world)? No. > > > > > this service is not related to the guest state. > > > > seems incorrect. At the moment the ip6_icmp timer's current value is not > > saved in the guest state, but in principle it should, so that the guest > > does see the RAs at a regular rate. In practice we don't care because > > the timing is randomized anyway. > > Isn't this just a side effect? > I mean that slirp may be replaced by, say, tap, and the guest should not > notice > the difference. Well, if a guest is connected through a tap, the virtual time should really run as fast as the realtime, and it should not be paused. Otherwise TCP connections will break since the guest won't be able to reply fast enough, without even knowing about the issue. Slirp can compensate this thanks to a buffer between what happens in the real world and what happens in the virtual world. Real world timings are handled by the OS socket implementation, and virtual world timings are handled with the qemu timer. > > > intended to be used for the internal QEMU purposes, but stops when VM > > > is stopped. > > > > I again don't understand this. The ip6_icmp timing is not for internal > > QEMU purpose, its whole point is how often RAs are sent to the guest. > > > > slirp's guest part is not a device as directly seen by guest I/O, but > > it's a router device as seen through guest packets. Think of it like a > > USB device, which is seen by the guest through USB packets. > > Record/replay implementation creates a line between the guest state and > the outer world. Everything crossing this line is saved in the log replayed. > In case of network, this line is implemented with the network filter. > It takes packets from slirp(or anything) and passes(or not) them to the guest > nic. > When replaying, the saved packets are injected into the filter directly. > Slirp is the part of the outer world, In normal uses it is not. It is a virtual world (its DHCP server, tftp server, TCP connexions, etc.) that lives along the guest. Now, I understand that for record/replay it's simpler to put the line after slirp. Ideally slirp's state should ideally be split it two: the part connected to the real world (data from/to the sockets), and the part connected to the virtual world (TCP buffering with the guest). So that when pausing, going back, going forward etc. the slirp buffers act accordingly, TCP knowing exactly what is supposed to be sent or not (otherwise, TCP would for instance be really astonished if the guest happens to insist requesting old data that it has already ACKed). But that's tricky, and I understand it's simpler to just put the line after slirp, and let the replay of frames provide the guest (which for instance has been reset to an older time) with the missing data, and TCP will nicely cope with duplicate ACKs and spurious re-emissions from the guest. That being said, there will be problems with TCP connections if you pause the guest for a long time: slirp's TCP will timeout and reset the connexion. Yes, that happens with tap devices anyway, but slirp acting as a buffer seems more useful to me. Samuel
Re: [Qemu-devel] [PATCH v5 24/24] slirp: fix ipv6 timers
Pavel Dovgalyuk, le jeu. 26 juil. 2018 10:08:29 +0300, a ecrit: > virtual clock should be used by the virtual devices. > slirp module is not the virtual device. Therefore processed packets > become visible to the guest after passing to the virtual network card. > Before that it can create any timers that should not change the state of the > guest. I'm not sure I understand that part correctly. slirp is not a "device" strictly speaking, but it has a whole foot in the virtual world. All TCP/UDP/ARP/RA timings are related to the guest timing, so > > > this service is not related to the guest state. seems incorrect. At the moment the ip6_icmp timer's current value is not saved in the guest state, but in principle it should, so that the guest does see the RAs at a regular rate. In practice we don't care because the timing is randomized anyway. > intended to be used for the internal QEMU purposes, but stops when VM > is stopped. I again don't understand this. The ip6_icmp timing is not for internal QEMU purpose, its whole point is how often RAs are sent to the guest. slirp's guest part is not a device as directly seen by guest I/O, but it's a router device as seen through guest packets. Think of it like a USB device, which is seen by the guest through USB packets. Samuel
Re: [Qemu-devel] [PATCH v5 24/24] slirp: fix ipv6 timers
Pavel Dovgalyuk, le jeu. 26 juil. 2018 10:37:03 +0300, a ecrit: > > From: Samuel Thibault [mailto:samuel.thiba...@gnu.org] > > Pavel Dovgalyuk, le jeu. 26 juil. 2018 10:08:29 +0300, a ecrit: > > > > As documented: > > > > > > > > * @QEMU_CLOCK_REALTIME: Real time clock > > > > * > > > > * The real time clock should be used only for stuff which does not > > > > * change the virtual machine state, as it runs even if the virtual > > > > * machine is stopped. > > > > > > > > There is no reason to "send RAs" while the machine is stopped. > > > > > > I see. > > > Then we'll need one more clock. Which works like realtime+virtual: > > > intended to be used for the internal QEMU purposes, but stops when > > > VM is stopped. > > > > Just to be sure: what is meant by "is stopped"? Is it a pause (thus time > > does not advance within the guest), or is it just sleeping because it > > has nothing to do? > > Paused with HMP/QMP command. > As virtual clock runs only if VM is not paused. Then all other uses of qemu_clock in slirp are bogus and need to be fixed like ip6_icmp: they are using QEMU_CLOCK_REALTIME, but they want it not to progress while the guest time is not advancing. Otherwise on guest resume after a long pause basically all TCP/UDP/ARP timings will have expired. Samuel
Re: [Qemu-devel] [PATCH v5 24/24] slirp: fix ipv6 timers
Pavel Dovgalyuk, le jeu. 26 juil. 2018 10:08:29 +0300, a ecrit: > > As documented: > > > > * @QEMU_CLOCK_REALTIME: Real time clock > > * > > * The real time clock should be used only for stuff which does not > > * change the virtual machine state, as it runs even if the virtual > > * machine is stopped. > > > > There is no reason to "send RAs" while the machine is stopped. > > I see. > Then we'll need one more clock. Which works like realtime+virtual: > intended to be used for the internal QEMU purposes, but stops when > VM is stopped. Just to be sure: what is meant by "is stopped"? Is it a pause (thus time does not advance within the guest), or is it just sleeping because it has nothing to do? Samuel
Re: [Qemu-devel] [PATCH v5 24/24] slirp: fix ipv6 timers
Pavel Dovgalyuk, le mer. 25 juil. 2018 15:17:06 +0300, a ecrit: > ICMP implementation for IPv6 uses timers based on virtual clock. > This is incorrect because this service is not related to the guest state. ? Why not? The RAs are seen by the guest. As documented: * @QEMU_CLOCK_REALTIME: Real time clock * * The real time clock should be used only for stuff which does not * change the virtual machine state, as it runs even if the virtual * machine is stopped. There is no reason to "send RAs" while the machine is stopped. Samuel
[Qemu-devel] [PULL 4/5] slirp: correct size computation while concatenating mbuf
From: Prasad J Pandit
While reassembling incoming fragmented datagrams, 'm_cat' routine
extends the 'mbuf' buffer, if it has insufficient room. It computes
a wrong buffer size, which leads to overwriting adjacent heap buffer
area. Correct this size computation in m_cat.
Reported-by: ZDI Disclosures
Signed-off-by: Prasad J Pandit
Signed-off-by: Samuel Thibault
---
slirp/mbuf.c | 11 +--
slirp/mbuf.h | 8 +++-
2 files changed, 8 insertions(+), 11 deletions(-)
diff --git a/slirp/mbuf.c b/slirp/mbuf.c
index 5ff24559fd..18cbf759a7 100644
--- a/slirp/mbuf.c
+++ b/slirp/mbuf.c
@@ -138,7 +138,7 @@ m_cat(struct mbuf *m, struct mbuf *n)
* If there's no room, realloc
*/
if (M_FREEROOM(m) < n->m_len)
- m_inc(m,m->m_size+MINCSIZE);
+ m_inc(m, m->m_len + n->m_len);
memcpy(m->m_data+m->m_len, n->m_data, n->m_len);
m->m_len += n->m_len;
@@ -147,7 +147,7 @@ m_cat(struct mbuf *m, struct mbuf *n)
}
-/* make m size bytes large */
+/* make m 'size' bytes large from m_data */
void
m_inc(struct mbuf *m, int size)
{
@@ -158,12 +158,12 @@ m_inc(struct mbuf *m, int size)
if (m->m_flags & M_EXT) {
datasize = m->m_data - m->m_ext;
- m->m_ext = g_realloc(m->m_ext, size);
+ m->m_ext = g_realloc(m->m_ext, size + datasize);
m->m_data = m->m_ext + datasize;
} else {
char *dat;
datasize = m->m_data - m->m_dat;
- dat = g_malloc(size);
+ dat = g_malloc(size + datasize);
memcpy(dat, m->m_dat, m->m_size);
m->m_ext = dat;
@@ -171,8 +171,7 @@ m_inc(struct mbuf *m, int size)
m->m_flags |= M_EXT;
}
-m->m_size = size;
-
+m->m_size = size + datasize;
}
diff --git a/slirp/mbuf.h b/slirp/mbuf.h
index 893601ff9d..33b84485d6 100644
--- a/slirp/mbuf.h
+++ b/slirp/mbuf.h
@@ -33,8 +33,6 @@
#ifndef MBUF_H
#define MBUF_H
-#define MINCSIZE 4096 /* Amount to increase mbuf if too small */
-
/*
* Macros for type conversion
* mtod(m,t) - convert mbuf pointer to data pointer of correct type
@@ -72,11 +70,11 @@ struct mbuf {
struct mbuf *m_prevpkt;/* Flags aren't used in the output
queue */
int m_flags;/* Misc flags */
- int m_size; /* Size of data */
+ int m_size; /* Size of mbuf, from m_dat or m_ext */
struct socket *m_so;
- caddr_t m_data; /* Location of data */
- int m_len; /* Amount of data in this mbuf */
+ caddr_t m_data; /* Current location of data */
+ int m_len; /* Amount of data in this mbuf, from
m_data */
Slirp *slirp;
boolresolution_requested;
--
2.17.1
[Qemu-devel] [PULL 1/5] slirp: Fix spurious error report when sending directly
Move check to where it actually is useful, and reduce scope of 'len'
variable along the way.
Signed-off-by: Samuel Thibault
Reviewed-by: Philippe Mathieu-Daudé
---
slirp/socket.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/slirp/socket.c b/slirp/socket.c
index e2a71c9b04..08fe98907d 100644
--- a/slirp/socket.c
+++ b/slirp/socket.c
@@ -340,7 +340,7 @@ sosendoob(struct socket *so)
struct sbuf *sb = &so->so_rcv;
char buff[2048]; /* XXX Shouldn't be sending more oob data than this */
- int n, len;
+ int n;
DEBUG_CALL("sosendoob");
DEBUG_ARG("so = %p", so);
@@ -359,7 +359,7 @@ sosendoob(struct socket *so)
* send it all
*/
uint32_t urgc = so->so_urgc;
- len = (sb->sb_data + sb->sb_datalen) - sb->sb_rptr;
+ int len = (sb->sb_data + sb->sb_datalen) - sb->sb_rptr;
if (len > urgc) {
len = urgc;
}
@@ -374,13 +374,13 @@ sosendoob(struct socket *so)
len += n;
}
n = slirp_send(so, buff, len, (MSG_OOB)); /* |MSG_DONTWAIT)); */
- }
-
#ifdef DEBUG
- if (n != len) {
- DEBUG_ERROR((dfd, "Didn't send all data urgently X\n"));
- }
+ if (n != len) {
+ DEBUG_ERROR((dfd, "Didn't send all data urgently
X\n"));
+ }
#endif
+ }
+
if (n < 0) {
return n;
}
--
2.17.1
[Qemu-devel] [PULL 0/5] slirp updates
The following changes since commit 9be4af13305f24d2dabf94bb53e6b65c76d08bb2: Merge remote-tracking branch 'remotes/thibault/tags/samuel-thibault' into staging (2018-06-01 14:58:53 +0100) are available in the Git repository at: https://people.debian.org/~sthibault/qemu.git tags/samuel-thibault for you to fetch changes up to c22098c74a09164797fae6511c5eaf68f32c4dd8: slirp: reformat m_inc routine (2018-06-08 09:08:30 +0300) slirp updates Prasad J Pandit (2): slirp: Fix buffer overflow on packet reassembling Samuel Thibault (3): slirp: Add Samuel Thibault's staging tree for slirp slirp: fix domainname version availability Prasad J Pandit (2): slirp: correct size computation while concatenating mbuf slirp: reformat m_inc routine Samuel Thibault (3): slirp: Fix spurious error report when sending directly slirp: Add Samuel Thibault's staging tree for slirp slirp: fix domainname version availability MAINTAINERS| 1 + qapi/net.json | 2 +- slirp/mbuf.c | 39 ++- slirp/mbuf.h | 8 +++- slirp/socket.c | 14 +++--- 5 files changed, 30 insertions(+), 34 deletions(-)
[Qemu-devel] [PULL 3/5] slirp: fix domainname version availability
The change missed the 2.12 deadline. Signed-off-by: Samuel Thibault Reviewed-by: Eric Blake --- qapi/net.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qapi/net.json b/qapi/net.json index 32681a1af7..6b7d93cb59 100644 --- a/qapi/net.json +++ b/qapi/net.json @@ -161,7 +161,7 @@ # to the guest # # @domainname: guest-visible domain name of the virtual nameserver -# (since 2.12) +# (since 3.0) # # @ipv6-prefix: IPv6 network prefix (default is fec0::) (since # 2.6). The network prefix is given in the usual -- 2.17.1
[Qemu-devel] [PULL 5/5] slirp: reformat m_inc routine
From: Prasad J Pandit
Coding style changes to the m_inc routine and minor refactoring.
Reported-by: ZDI Disclosures
Signed-off-by: Prasad J Pandit
Signed-off-by: Samuel Thibault
---
slirp/mbuf.c | 34 --
1 file changed, 16 insertions(+), 18 deletions(-)
diff --git a/slirp/mbuf.c b/slirp/mbuf.c
index 18cbf759a7..0c189e1a7b 100644
--- a/slirp/mbuf.c
+++ b/slirp/mbuf.c
@@ -151,27 +151,25 @@ m_cat(struct mbuf *m, struct mbuf *n)
void
m_inc(struct mbuf *m, int size)
{
- int datasize;
+int datasize;
- /* some compiles throw up on gotos. This one we can fake. */
-if(m->m_size>size) return;
+/* some compilers throw up on gotos. This one we can fake. */
+if (m->m_size > size) {
+return;
+}
-if (m->m_flags & M_EXT) {
- datasize = m->m_data - m->m_ext;
- m->m_ext = g_realloc(m->m_ext, size + datasize);
- m->m_data = m->m_ext + datasize;
-} else {
- char *dat;
- datasize = m->m_data - m->m_dat;
- dat = g_malloc(size + datasize);
- memcpy(dat, m->m_dat, m->m_size);
-
- m->m_ext = dat;
- m->m_data = m->m_ext + datasize;
- m->m_flags |= M_EXT;
-}
+if (m->m_flags & M_EXT) {
+datasize = m->m_data - m->m_ext;
+m->m_ext = g_realloc(m->m_ext, size + datasize);
+} else {
+datasize = m->m_data - m->m_dat;
+m->m_ext = g_malloc(size + datasize);
+memcpy(m->m_ext, m->m_dat, m->m_size);
+m->m_flags |= M_EXT;
+}
-m->m_size = size + datasize;
+m->m_data = m->m_ext + datasize;
+m->m_size = size + datasize;
}
--
2.17.1
[Qemu-devel] [PULL 2/5] slirp: Add Samuel Thibault's staging tree for slirp
Signed-off-by: Samuel Thibault Acked-by: Thomas Huth --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 41cd3736a9..4c73c16fee 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1675,6 +1675,7 @@ S: Maintained F: slirp/ F: net/slirp.c F: include/net/slirp.h +T: git https://people.debian.org/~sthibault/qemu.git slirp T: git git://git.kiszka.org/qemu.git queues/slirp Stubs -- 2.17.1
Re: [Qemu-devel] [PATCH 2/2] slirp: reformat m_cat routine
P J P, le mar. 05 juin 2018 23:38:36 +0530, a ecrit: > From: Prasad J Pandit > > Coding style changes to the m_cat routine and minor refactoring. Fixed a bit and applied to my tree, thanks! Samuel
Re: [Qemu-devel] [PATCH 1/2] slirp: correct size computation while concatenating mbuf
Hello, P J P, le mar. 05 juin 2018 23:38:35 +0530, a ecrit: > From: Prasad J Pandit > > While reassembling incoming fragmented datagrams, 'm_cat' routine > extends the 'mbuf' buffer, if it has insufficient room. It computes > a wrong buffer size, which leads to overwriting adjacent heap buffer > area. Correct this size computation in m_cat. > > Reported-by: ZDI Disclosures > Signed-off-by: Prasad J Pandit Applied to my tree with a couple documentation improvements, thanks! Samuel
Re: [Qemu-devel] [PATCH] slirp: Add Samuel Thibault's staging tree for slirp
Ping? I'm not sure who I am supposed to get a review from, or if I have to have one at all? Samuel Samuel Thibault, le jeu. 31 mai 2018 21:48:43 +0200, a ecrit: > Signed-off-by: Samuel Thibault > --- > MAINTAINERS | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 41cd3736a9..8822ae9b70 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -1675,6 +1675,7 @@ S: Maintained > F: slirp/ > F: net/slirp.c > F: include/net/slirp.h > +T: git http://people.debian.org/~sthibault/qemu.git slirp > T: git git://git.kiszka.org/qemu.git queues/slirp > > Stubs > -- > 2.17.0 >
[Qemu-devel] [Bug 1724590] Re: Usermode networking hostfwd only listens on IPv4
Hello, This is indeed not implemented, patches welcome :) Samuel -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1724590 Title: Usermode networking hostfwd only listens on IPv4 Status in QEMU: New Bug description: When forwarding ports in usermode networking (-net user,hostfwd=), QEMU binds to IPv4 only. Therefore, connecting to the port over IPv6 results in 'connection refused'. I experienced this in QEMU 2.10.1, but it looks to still be present in the current master (861cd431c99e56ddb5953ca1da164a9c32b477ca), since slirp_hostfwd in net/slirp.c uses in_addr instead of in6_addr. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1724590/+subscriptions
Re: [Qemu-devel] [PULL 0/9 v2] slirp updates
Eric Blake, le ven. 01 juin 2018 09:45:25 -0500, a ecrit: > On 06/01/2018 08:56 AM, Samuel Thibault 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: > > > > https://people.debian.org/~sthibault/qemu.git tags/samuel-thibault > > > > for you to fetch changes up to 47335eeea8f1d14b7c6a1dd585a25a9166721168: > > > >slirp/ncsi: add checksum support (2018-05-31 21:19:24 +0200) > > As long as you're respinning a v2, do you want a v3 that fixes the issue I > pointed out on 1/9 (s/2.12/3.0/)? Too late :) Anyway, I'll send a PR as soon as the MAINTAINER patch I sent on http://lists.nongnu.org/archive/html/qemu-devel/2018-05/msg07244.html gets reviewed. Samuel
Re: [Qemu-devel] [PULL 0/9] slirp updates
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
[Qemu-devel] [PULL 0/9 v2] slirp updates
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: https://people.debian.org/~sthibault/qemu.git tags/samuel-thibault for you to fetch changes up to 47335eeea8f1d14b7c6a1dd585a25a9166721168: slirp/ncsi: add checksum support (2018-05-31 21:19:24 +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] [PATCHv2] slirp: Fix spurious error report when sending directly
Move check to where it actually is useful, and reduce scope of 'len'
variable along the way.
Signed-off-by: Samuel Thibault
---
Difference from v1:
- move check instead of initializing len.
slirp/socket.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/slirp/socket.c b/slirp/socket.c
index e2a71c9b04..08fe98907d 100644
--- a/slirp/socket.c
+++ b/slirp/socket.c
@@ -340,7 +340,7 @@ sosendoob(struct socket *so)
struct sbuf *sb = &so->so_rcv;
char buff[2048]; /* XXX Shouldn't be sending more oob data than this */
- int n, len;
+ int n;
DEBUG_CALL("sosendoob");
DEBUG_ARG("so = %p", so);
@@ -359,7 +359,7 @@ sosendoob(struct socket *so)
* send it all
*/
uint32_t urgc = so->so_urgc;
- len = (sb->sb_data + sb->sb_datalen) - sb->sb_rptr;
+ int len = (sb->sb_data + sb->sb_datalen) - sb->sb_rptr;
if (len > urgc) {
len = urgc;
}
@@ -374,13 +374,13 @@ sosendoob(struct socket *so)
len += n;
}
n = slirp_send(so, buff, len, (MSG_OOB)); /* |MSG_DONTWAIT)); */
- }
-
#ifdef DEBUG
- if (n != len) {
- DEBUG_ERROR((dfd, "Didn't send all data urgently X\n"));
- }
+ if (n != len) {
+ DEBUG_ERROR((dfd, "Didn't send all data urgently
X\n"));
+ }
#endif
+ }
+
if (n < 0) {
return n;
}
--
2.17.0
[Qemu-devel] [PATCH] slirp: fix domainname version availability
The change missed the 2.12 deadline. Signed-off-by: Samuel Thibault --- qapi/net.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qapi/net.json b/qapi/net.json index 32681a1af7..6b7d93cb59 100644 --- a/qapi/net.json +++ b/qapi/net.json @@ -161,7 +161,7 @@ # to the guest # # @domainname: guest-visible domain name of the virtual nameserver -# (since 2.12) +# (since 3.0) # # @ipv6-prefix: IPv6 network prefix (default is fec0::) (since # 2.6). The network prefix is given in the usual -- 2.17.0
[Qemu-devel] [PATCH] slirp: Add Samuel Thibault's staging tree for slirp
Signed-off-by: Samuel Thibault --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 41cd3736a9..8822ae9b70 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1675,6 +1675,7 @@ S: Maintained F: slirp/ F: net/slirp.c F: include/net/slirp.h +T: git http://people.debian.org/~sthibault/qemu.git slirp T: git git://git.kiszka.org/qemu.git queues/slirp Stubs -- 2.17.0
[Qemu-devel] [PULL 9/9] slirp/ncsi: add checksum support
From: Cédric Le Goater
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
Reviewed-by: Philippe Mathieu-Daudé
Tested-by: Joel Stanley
Signed-off-by: Samuel Thibault
---
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
[Qemu-devel] [PATCH] slirp: Fix spurious error report when sending directly
When DEBUG is enabled, len needs to be updated.
Signed-off-by: Samuel Thibault
---
slirp/socket.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/slirp/socket.c b/slirp/socket.c
index e2a71c9b04..3182477ff5 100644
--- a/slirp/socket.c
+++ b/slirp/socket.c
@@ -351,7 +351,7 @@ sosendoob(struct socket *so)
if (sb->sb_rptr < sb->sb_wptr) {
/* We can send it directly */
- n = slirp_send(so, sb->sb_rptr, so->so_urgc, (MSG_OOB)); /*
|MSG_DONTWAIT)); */
+ len = n = slirp_send(so, sb->sb_rptr, so->so_urgc, (MSG_OOB));
/* |MSG_DONTWAIT)); */
} else {
/*
* Since there's no sendv or sendtov like writev,
--
2.17.0
[Qemu-devel] [PULL 3/9] slirp: disable Nagle in ingoing connections
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
Reviewed-by: Philippe Mathieu-Daudé
---
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 0/9] slirp updates
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 2/9] slirp: disable Nagle in outgoing connections
From: Andreas Gustafsson
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
Reviewed-by: Kamil Rytarowski
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Samuel Thibault
---
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 7/9] slirp/ncsi: fix "Get Version ID" payload length
From: Cédric Le Goater
Signed-off-by: Cédric Le Goater
Reviewed-by: Philippe Mathieu-Daudé
Tested-by: Joel Stanley
Signed-off-by: Samuel Thibault
---
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 1/9] slirp: Add domainname option to slirp's DHCP server
From: Benjamin Drung
This patch will allow the user to include the domainname option in
replies from the built-in DHCP server.
Signed-off-by: Benjamin Drung
Signed-off-by: Samuel Thibault
---
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 st
[Qemu-devel] [PULL 5/9] net/slirp: Convert atoi to qemu_strtoi to allow error checking
From: Nia Alarie
Signed-off-by: Nia Alarie
Signed-off-by: Samuel Thibault
---
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 4/9] slirp/debug: Print IP addresses in human readable form
From: Alexey Kardashevskiy
Signed-off-by: Alexey Kardashevskiy
Signed-off-by: Samuel Thibault
---
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 == 0x || 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 6/9] slirp: Send window updates to guest after window was closed
From: James Clarke
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
Signed-off-by: Samuel Thibault
---
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));
+}
}
-/*
- * X 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 8/9] slirp/ncsi: add a "Get Parameters" response
From: Cédric Le Goater
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
Reviewed-by: Philippe Mathieu-Daudé
Tested-by: Joel Stanley
Signed-off-by: Samuel Thibault
---
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
Re: [Qemu-devel] [PATCH 23/67] slirp: add include directory headers
Hello, Michael S. Tsirkin, le jeu. 03 mai 2018 22:51:01 +0300, a ecrit: > +#include_next "../slirp/ip6.h" Mmm, this is the first time #include_next would be used in qemu, and I don't think this is a standard thing... Samuel
Re: [Qemu-devel] [PATCH v2 0/3] slirp: NC-SI enhancements
Hello, Cédric Le Goater, le mer. 30 mai 2018 08:10:32 +0200, a ecrit: > Here is a couple of enhancements and fixes for the NC-SI backend used > on the Aspeed SoC when soldered on OpenPOWER boards. Applied to my tree, thanks! (my train is 2h late, so I'll probably have the time to pull-request it into master). Samuel
Re: [Qemu-devel] [PATCH qemu v2] slirp/debug: Print IP addresses in human readable form
Alexey Kardashevskiy, le lun. 14 mai 2018 17:00:08 +1000, a ecrit: > On 13/3/18 6:44 pm, Samuel Thibault wrote: > > Alexey Kardashevskiy, on mar. 13 mars 2018 15:49:44 +1100, wrote: > >> Signed-off-by: Alexey Kardashevskiy > > > > Applied to my tree, thanks! > > > And what is your tree, is this something to be merged sometime later > somewhere? :) When I manage to take the time to, yes. Samuel
Re: [Qemu-devel] [PATCH for-2.13 v3 1/2] slirp: Add "query-usernet" QMP command
Fam Zheng, le mer. 02 mai 2018 15:29:36 +0800, a ecrit:
> On Tue, 05/01 22:41, Samuel Thibault wrote:
> > I'm sorry I didn't find the time to have a look at it before.
> >
> > In general it looks good, just a few things:
> >
> > Samuel
> >
> > Fam Zheng, le ven. 16 mars 2018 14:28:21 +0800, a ecrit:
> > > +if (!net_hub_id_for_client(&s->nc, &vlan)) {
> >
> > That makes me think that IIUC, we shouldn't call them vlan, as mentioned
> > in the recent vlan->hub renaming patches.
> >
> > > +{ 'struct': 'UsernetInfo',
> > > + 'data': {
> > > +'id': 'str',
> > > +'vlan':'int',
> >
> > and here as well, I guess.
>
> OK, so rename this to hub? (I'm not familiar with the naming issue you pointed
> out).
I am not either :)
Please check with the thread
http://lists.nongnu.org/archive/html/qemu-devel/2018-04/msg02126.html
Samuel
Re: [Qemu-devel] [PATCH for-2.13 v3 1/2] slirp: Add "query-usernet" QMP command
Hello,
I'm sorry I didn't find the time to have a look at it before.
In general it looks good, just a few things:
Samuel
Fam Zheng, le ven. 16 mars 2018 14:28:21 +0800, a ecrit:
> +if (!net_hub_id_for_client(&s->nc, &vlan)) {
That makes me think that IIUC, we shouldn't call them vlan, as mentioned
in the recent vlan->hub renaming patches.
> +{ 'struct': 'UsernetInfo',
> + 'data': {
> +'id': 'str',
> +'vlan':'int',
and here as well, I guess.
> +void slirp_connection_info(Slirp *slirp, Monitor *mon)
> +{
> +const char *state;
> +char buf[64];
> +UsernetInfo info = { };
> +UsernetConnectionList *cl;
> +
> +monitor_printf(mon, " Protocol[State]FD Source Address Port "
> +"Dest. Address Port RecvQ SendQ\n");
> +
> +usernet_get_info(slirp, &info);
Mmm, I don't see the content of info being freed here?
Samuel
Re: [Qemu-devel] [PATCH v3 00/46] fix building of tests/tcg
Philippe Mathieu-Daudé, le mar. 24 avril 2018 22:25:18 -0300, a ecrit: > > This is starting to shape up pretty nicely. I was able to add a whole > > bunch of additional architectures thanks to cross compilers in Debian > > Sid which are there to support the Debian "ports". These may not be > > around for ever, most "ports" are on the way out, but they will be the > > last thing to drop out of the Sid repo. Maybe when Debian stops > > caring (and no other distro does) maybe we should to? > > I *think* working with Sid is not recommended as very unstable and not > reproducible. A reproducible way is to use the Debian Snapshot Archive > (http://snapshot.debian.org/) eventually using package specific version > and holding packages at this version. > > I found an example in the following post: > https://blog.sleeplessbeastie.eu/2017/07/17/how-to-install-packages-using-repository-snapshot/ > > (I Cc'ed Debian experts who might have a better idea). That looks like the correct idea :) (basically what we use to make "releases" of the hurd port, as sid snapshots). Samuel
Re: [Qemu-devel] [PATCH] slirp: Send window updates to guest after window was closed
James Clarke, le mar. 17 avril 2018 15:10:58 +0100, a ecrit:
> 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
Makes sense indeed, I have applied it to my tree.
Thanks!
Samuel
> ---
>
> Hi,
> I encountered this whilst running a (k)FreeBSD build server for Debian.
> The host's upload link is over ADSL and thus rather slow, so slirp's
> outgoing buffer soon fills up, causing it to close its receive window.
> However, without this patch, slirp does not send a window update back to
> the guest once it starts draining its outgoing buffer and thus opening
> its receive window, causing the guest to remain blocked until its
> persist timer next fires and it sends a zero window probe. In the case
> of a Linux guest, this starts at ~200ms and grows exponentially, though
> most of the time the receive window has already opened by then and thus
> the unnecessary delay doesn't have too much of an effect, but the
> FreeBSD network stack defaults to a 5s minimum for the persist timer and
> thus spends the vast majority of its time not transmitting data, with
> the upload speed achieved around 10 KiB/s for this particular guest and
> link.
>
> VirtualBox, which uses slirp for its NAT networking mode, also
> encountered this and fixed it back in 2014 with [1], but interestingly
> decided to set its own delayed ACK flag and wait for its own timer to
> fire, rather than calling tcp_output directly. I don't know what their
> motivation for that decision was, but to me that seems to add an
> unnecessary delay of up to a few hundred ms (though that is of course
> still much better than 5s).
>
> I have been testing this patch for the past few days with the build
> server uploading its huge backlog of packages one at a time. It is now
> reaching 1.3 Mbit/s and networking has remained seemingly stable.
>
> Regards,
> James
>
> [1]
> https://github.com/mirror/vbox/commit/87c7aa2e8d26b989da6e85d532695f1e3b050aaa
>
> slirp/slirp.c | 12 ++--
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/slirp/slirp.c b/slirp/slirp.c
> index 1cb6b07004..238fb04115 100644
> --- a/slirp/slirp.c
> +++ b/slirp/slirp.c
> @@ -676,13 +676,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));
> +}
> }
> -/*
> - * X 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
>
--
Samuel
"...Unix, MS-DOS, and Windows NT (also known as the Good, the Bad, and
the Ugly)."
(By Matt Welsh)
Re: [Qemu-devel] [PATCH] net/slirp: Convert atoi to qemu_strtoi to allow error checking
Nia Alarie, on ven. 16 mars 2018 14:39:21 +, wrote:
> Signed-off-by: Nia Alarie
Applied to my tree, thanks!
> ---
> net/slirp.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/net/slirp.c b/net/slirp.c
> index 8991816bbf..e938944bd4 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -486,7 +486,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.16.2
>
--
Samuel
"...[Linux's] capacity to talk via any medium except smoke signals."
(By Dr. Greg Wettstein, Roger Maris Cancer Center)
Re: [Qemu-devel] [PATCH qemu v2] slirp/debug: Print IP addresses in human readable form
Alexey Kardashevskiy, on mar. 13 mars 2018 15:49:44 +1100, wrote:
> Signed-off-by: Alexey Kardashevskiy
Applied to my tree, thanks!
> ---
>
> checkpatch.pl complains on every single changed line as it keeps
> using tabs - do I need to post 's/\t//g'?
>
> ---
> Changes:
> v2:
> * replaced static cast to (in_addr*) with temporary structs
> ---
> slirp/arp_table.c | 4 ++--
> slirp/socket.c| 8
> slirp/udp.c | 4 ++--
> 3 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/slirp/arp_table.c b/slirp/arp_table.c
> index 3547043..f81963b 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 = 0x%x", 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 = 0x%x", ip_addr);
> +DEBUG_ARG("ip = %s", inet_ntoa((struct in_addr){.s_addr = ip_addr}));
>
> /* If broadcast address */
> if (ip_addr == 0x || ip_addr == broadcast_addr) {
> diff --git a/slirp/socket.c b/slirp/socket.c
> index cb7b5b6..318301f 100644
> --- a/slirp/socket.c
> +++ b/slirp/socket.c
> @@ -701,10 +701,10 @@ tcp_listen(Slirp *slirp, uint32_t haddr, u_int hport,
> uint32_t laddr,
> memset(&addr, 0, addrlen);
>
> DEBUG_CALL("tcp_listen");
> - DEBUG_ARG("haddr = %x", haddr);
> - DEBUG_ARG("hport = %d", hport);
> - DEBUG_ARG("laddr = %x", laddr);
> - DEBUG_ARG("lport = %d", lport);
> + 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){.s_addr = laddr}));
> + DEBUG_ARG("lport = %d", ntohs(lport));
> DEBUG_ARG("flags = %x", flags);
>
> so = socreate(slirp);
> diff --git a/slirp/udp.c b/slirp/udp.c
> index 227d779..e5bf065 100644
> --- a/slirp/udp.c
> +++ b/slirp/udp.c
> @@ -241,8 +241,8 @@ int udp_output(struct socket *so, struct mbuf *m,
> DEBUG_CALL("udp_output");
> DEBUG_ARG("so = %p", so);
> DEBUG_ARG("m = %p", m);
> - DEBUG_ARG("saddr = %lx", (long)saddr->sin_addr.s_addr);
> - DEBUG_ARG("daddr = %lx", (long)daddr->sin_addr.s_addr);
> + DEBUG_ARG("saddr = %s", inet_ntoa(saddr->sin_addr));
> + DEBUG_ARG("daddr = %s", inet_ntoa(daddr->sin_addr));
>
> /*
>* Adjust for header
> --
> 2.11.0
>
--
Samuel
"I don't know why, but first C programs tend to look a lot worse than
first programs in any other language (maybe except for fortran, but then
I suspect all fortran programs look like `firsts')"
(By Olaf Kirch)
Re: [Qemu-devel] [PATCH] slirp: disable Nagle in outgoing connections
Philippe Mathieu-Daudé, on mer. 07 mars 2018 19:57:28 -0300, wrote: > Reviewed-by: Philippe Mathieu-Daudé Thanks!
Re: [Qemu-devel] [PATCH qemu] slirp/debug: Print IP addresses in human readable form
Hello,
Remember to Cc the maintainer, I just can't read qemu-devel fully to
find slirp mails.
Thomas Huth, on mer. 07 mars 2018 07:24:16 +0100, wrote:
> >> diff --git a/slirp/arp_table.c b/slirp/arp_table.c
> >> index 3547043..bac608f 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 = 0x%x", ip_addr);
> >> +DEBUG_ARG("ip = %s", inet_ntoa(*(struct in_addr *)&ip_addr));
I never like casts :)
And it happens that the standard doesn't say that s_addr is necessarily
the first field of struct in_addr, so better really initialize a struct
in_addr variable and use that (ditto for arp_table_search and
tcp_listen).
Samuel
Re: [Qemu-devel] [PATCH] slirp: disable Nagle in outgoing connections
Hello,
Thanks for the Cc.
I have applied it to my tree. I don't think there is any reason to
avoid the same change for ingoing connections? Could one of your review
the attached patch doing it?
Samuel
commit 99a9a5028e0e15aa3b17d6f884c1e5f48dccea90
Author: Samuel Thibault
Date: Wed Mar 7 23:29:41 2018 +0100
slirp: disable Nagle in ingoing connections
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
diff --git a/slirp/socket.c b/slirp/socket.c
index cb7b5b608d..81f67b5702 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;
Re: [Qemu-devel] [RFC PATCH qemu] slirp: Update forwarding IP address if guest receiver non-default IP
Hello,
Thomas Huth, on mer. 07 mars 2018 07:30:29 +0100, wrote:
> On 07.03.2018 04:39, Alexey Kardashevskiy wrote:
> > On 08/02/18 15:29, Alexey Kardashevskiy wrote:
> >> On 01/02/18 20:36, Alexey Kardashevskiy wrote:
> >>> If we run QEMU with -netdev user,id=USER0,hostfwd=tcp::-:22, it starts
> >>> a DHCP server and starts allocating client IPs from 10.0.2.15 so
> >>> this is what the guest normally receives. Since QEMU automatically adds
> >>> the DHCP starting address into the forwarding table, everything works.
> >>> This is the table before guest started:
> >>>
> >>> (qemu) info usernet
> >>> VLAN -1 (USER0):
> >>> Protocol[State]FD Source Address Port Dest. Address Port RecvQ
> >>> SendQ
> >>> TCP[HOST_FORWARD] 11 * 10.0.2.1522 0
> >>> 0
> >>>
> >>> However if the guest happens to have DHCP lease (for example, 10.0.2.16),
> >>> the forwarding stops working. The guest can still reach the outer world
> >>> (which is expected).
> >>>
> >>> This updates the forwarding table when QEMU confirms the requested IP
> >>> to the guest.
> >>>
> >>> Signed-off-by: Alexey Kardashevskiy
> >>> ---
> >>>
> >>> Does this look any useful?
> >
> > Ping, anyone?
>
> Maybe you should make sure to put the SLIRP maintainer on CC: ?
That would work much better to catch my attention indeed :)
I'm afraid this will be a nack. What you basically propose is "the last
DHCP lease wins". There can be setups where it is expected that it's the
first DHCP lease which should get the forward, as documented actually
("If guestaddr is not specified, its value is x.x.x.15"). In your case,
you can always set to hostfwd=tcp::-10.0.2.16:22. If your guest
doesn't have predictable DHCP behavior, better use a static IP
assignment rather than introducing into qemu something which looks
rather undefined to me ("last DHCP lease wins").
Samuel
Re: [Qemu-devel] [PATCH 2/2] slirp: Add classless static routes support to DHCP server
Benjamin Drung, on mar. 27 févr. 2018 17:06:02 +0100, wrote:
> +int i = 0;
Rather unsigned?
> char *end;
> +unsigned int route_count = 0;
> struct slirp_config_str *config;
> +struct StaticRoute *routes = NULL;
> +const StringList *iter;
>
> if (!ipv4 && (vnetwork || vhost || vnameserver)) {
> error_setg(errp, "IPv4 disabled but netmask/host/dns provided");
> @@ -365,6 +369,58 @@ static int net_slirp_init(NetClientState *peer, const
> char *model,
> return -1;
> }
>
> +iter = vroutes;
Rather initialize route_count to 0 here.
> +while (iter) {
> +route_count++;
> +iter = iter->next;
> +}
> +iter = vroutes;
Rather initialize i to 0 here.
> +while(iter != NULL) {
> +// Split "subnet/mask:gateway" into its components
> +if (get_str_sep(buf2, sizeof(buf2), &gateway, ':') < 0) {
[etc.]
You should make the gateway host.s_addr by default, so the gateway or
even :gateway part can be optional.
> -" [,tftp=dir][,bootfile=f][,hostfwd=rule][,guestfwd=rule]"
> +"
> [,route=addr/mask:gateway][,tftp=dir][,bootfile=f][,hostfwd=rule][,guestfwd=rule]"
And document optionality in the help, of course.
> +if (slirp->route_count > 0) {
> +uint8_t option_length = 0;
> +uint8_t significant_octets;
> +
> +for (int i = 0; i < slirp->route_count; i++) {
> +significant_octets = slirp->routes[i].mask_width / 8
> + + (slirp->routes[i].mask_width % 8 > 0);
Rather use (slirp->routes[i].mask_width + 7) / 8 (and ditto below)
> +option_length += significant_octets + 5;
> +}
> +
> +*q++ = RFC3442_CLASSLESS_STATIC_ROUTE;
I'd say instead of computing the option_length twice, create a variable
uint8_t *size which you make point at the size byte here, and you can
fill it after the loop.
Apart from that it looks good.
Samuel
Re: [Qemu-devel] [PATCH 1/2 v4] slirp: Add domainname option to slirp's DHCP server
Benjamin Drung, on mar. 27 févr. 2018 17:06:01 +0100, wrote:
> This patch will allow the user to include the domainname option in
> replies from the built-in DHCP server.
>
> Signed-off-by: Benjamin Drung
Reviewed-by: Samuel Thibault
and applied to my tree.
Samuel
> ---
> 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 8991816bbf..8c08e5644f 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 1238ba5de1..9dfd34cafa 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 8ccd5dcaa6..c000ef454e 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
> @@ -2116,6 +2116,
Re: [Qemu-devel] [PATCH 0/1] slirp: Add domainname option to slirp's DHCP server
Hello, Benjamin Drung, on ven. 16 févr. 2018 13:55:03 +0100, wrote: > Or should the command line option be simpler, but how should it be specified > then? Maybe > > -net > staticroute=10.0.2.0/24via10.0.2.2,staticroute=192.168.0.0/16via10.0.2.2 I guess > -net staticroute=10.0.2.0/24:10.0.2.2,staticroute=192.168.0.0/16:10.0.2.2 would be more mainstream. I'm also wondering to which extent we want to extend our dhcp server, when a tap device can be used to plug (or proxy) an actual dhcp serveR. samuel
[Qemu-devel] [PATCHv2] linux-user: Fix sched_getaffinity mask size
We properly computed the capped mask size to be put to the application
buffer, but didn't actually used it. Also, we need to return the capped mask
size instead of 0 on success.
Signed-off-by: Samuel Thibault
---
Difference from v1:
- simplify fix
---
linux-user/syscall.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 82b35a6bdf..bcda3362fc 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -10493,7 +10493,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
arg1,
ret = arg2;
}
-ret = host_to_target_cpu_mask(mask, mask_size, arg3, arg2);
+if (host_to_target_cpu_mask(mask, mask_size, arg3, ret)) {
+goto efault;
+}
}
}
break;
--
2.15.1
Re: [Qemu-devel] [PATCH] linux-user: Fix sched_getaffinity mask size
Laurent Vivier, on ven. 26 janv. 2018 21:34:38 +0100, wrote:
> Le 26/01/2018 à 19:36, Samuel Thibault a écrit :
> > We properly computed the capped mask size to be put to the application
> > buffer, but didn't actually it. Also, we need to return the capped mask
> > size instead of 0 on success.
> >
> > Signed-off-by: Samuel Thibault
> > ---
> > linux-user/syscall.c | 7 ++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> > index 74378947f0..cf2369aac2 100644
> > --- a/linux-user/syscall.c
> > +++ b/linux-user/syscall.c
> > @@ -10496,10 +10496,15 @@ abi_long do_syscall(void *cpu_env, int num,
> > abi_long arg1,
> > ret = -TARGET_EINVAL;
> > break;
> > }
> > -ret = arg2;
> > +} else if (arg2 > ret) {
> > +arg2 = ret;
> > }
> >
> > ret = host_to_target_cpu_mask(mask, mask_size, arg3, arg2);
> > +
> > +if (ret == 0) {
> > +ret = arg2;
> > +}
> > }
> > }
> > break;
> >
>
> Modified code was:
>
> if (copy_to_user(arg3, mask, ret)) {
> goto efault;
> }
>
> so the change should only be:
>
> if (host_to_target_cpu_mask(mask, mask_size, arg3, ret)) {
> goto efault;
> }
That depends whether host_to_target_cpu_mask wants to return something
else than -TARGET_EFAULT, but at some point I don't care about
bikeshedding.
Samuel
Re: [Qemu-devel] [PULL 07/13] linux-user: Fix sched_get/setaffinity conversion
Peter Maydell, on ven. 26 janv. 2018 18:23:02 +, wrote:
> On 23 January 2018 at 14:48, Laurent Vivier wrote:
> > From: Samuel Thibault
> >
> > sched_get/setaffinity linux-user syscalls were missing conversions for
> > little/big endian, which is hairy since longs may not be the same size
> > either.
> >
> > For simplicity, this just introduces loops to convert bit by bit like is
> > done for select.
> >
> > Signed-off-by: Samuel Thibault
> > Reviewed-by: Laurent Vivier
> > Message-Id: <20180109201643.1479-1-samuel.thiba...@ens-lyon.org>
> > Signed-off-by: Laurent Vivier
> > ---
>
> > @@ -10395,9 +10463,7 @@ abi_long do_syscall(void *cpu_env, int num,
> > abi_long arg1,
> > ret = arg2;
> > }
> >
> > -if (copy_to_user(arg3, mask, ret)) {
> > -goto efault;
> > -}
> > +ret = host_to_target_cpu_mask(mask, mask_size, arg3, arg2);
> > }
> > }
> > break;
>
> Hi -- Coverity spots that in this change, we now have a case
> where we set "ret = arg2;" which then immediately is replaced
> by "ret = host_to_target_cpu_mask(mask, mask_size, arg3, arg2);",
> making the first assignment pointless.
>
> It looks like we're now ignoring the host filled buffer size
> that is returned by sys_sched_getaffinity() and then adjusted
> by this bit of code. Shouldn't we be using that value in this
> new host_to_target_cpu_mask() code?
Indeed, will send a patch against this.
Samuel
[Qemu-devel] [PATCH] linux-user: Fix sched_getaffinity mask size
We properly computed the capped mask size to be put to the application
buffer, but didn't actually it. Also, we need to return the capped mask
size instead of 0 on success.
Signed-off-by: Samuel Thibault
---
linux-user/syscall.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 74378947f0..cf2369aac2 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -10496,10 +10496,15 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
arg1,
ret = -TARGET_EINVAL;
break;
}
-ret = arg2;
+} else if (arg2 > ret) {
+arg2 = ret;
}
ret = host_to_target_cpu_mask(mask, mask_size, arg3, arg2);
+
+if (ret == 0) {
+ret = arg2;
+}
}
}
break;
--
2.15.1
[Qemu-devel] [PULL 0/5] slirp updates
warning: redirection vers https://people.debian.org/~sthibault/qemu.git/ The following changes since commit 7398166ddf7c6dbbc9cae6ac69bb2feda14b40ac: Merge remote-tracking branch 'remotes/kraxel/tags/vnc-20180112-pull-request' into staging (2018-01-12 16:01:30 +) are available in the Git repository at: http://people.debian.org/~sthibault/qemu.git tags/samuel-thibault for you to fetch changes up to 318116a6ff36bee13c725a247a9395e80bcfbd6b: slirp: add in6_dhcp_multicast() (2018-01-14 18:16:13 +0100) slirp updates Philippe Mathieu-Daudé (5): slirp: avoid IN6_IS_ADDR_UNSPECIFIED(), rather use in6_zero() slirp: remove unused header slirp: remove unnecessary struct declaration slirp: removed unused code slirp: add in6_dhcp_multicast() slirp/dhcpv6.h| 3 +++ slirp/ip.h| 13 - slirp/ip6_icmp.c | 6 +++--- slirp/libslirp.h | 1 - slirp/ndp_table.c | 4 ++-- slirp/slirp.h | 1 - slirp/udp6.c | 2 +- 7 files changed, 9 insertions(+), 21 deletions(-)
[Qemu-devel] [PULL 4/5] slirp: removed unused code
From: Philippe Mathieu-Daudé
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Thomas Huth
Signed-off-by: Samuel Thibault
---
slirp/ip.h | 13 -
1 file changed, 13 deletions(-)
diff --git a/slirp/ip.h b/slirp/ip.h
index 1df6723357..59cf4aa918 100644
--- a/slirp/ip.h
+++ b/slirp/ip.h
@@ -233,17 +233,4 @@ struct ipasfrag {
#define ipf_next ipf_link.next
#define ipf_prev ipf_link.prev
-/*
- * Structure stored in mbuf in inpcb.ip_options
- * and passed to ip_output when ip options are in use.
- * The actual length of the options (including ipopt_dst)
- * is in m_len.
- */
-#define MAX_IPOPTLEN 40
-
-struct ipoption {
- struct in_addr ipopt_dst; /* first-hop dst if source routed */
- int8_t ipopt_list[MAX_IPOPTLEN]; /* options proper */
-} QEMU_PACKED;
-
#endif
--
2.15.1
[Qemu-devel] [PULL 1/5] slirp: avoid IN6_IS_ADDR_UNSPECIFIED(), rather use in6_zero()
From: Philippe Mathieu-Daudé
Host: Mac OS 10.12.5
Compiler: Apple LLVM version 8.1.0 (clang-802.0.42)
slirp/ip6_icmp.c:80:38: warning: taking address of packed member 'ip_src' of
class or
structure 'ip6' may result in an unaligned pointer value
[-Waddress-of-packed-member]
IN6_IS_ADDR_UNSPECIFIED(&ip->ip_src)) {
^~
/usr/include/netinet6/in6.h:238:42: note: expanded from macro
'IN6_IS_ADDR_UNSPECIFIED'
((*(const __uint32_t *)(const void *)(&(a)->s6_addr[0]) == 0) && \
^
Reported-by: John Arbuckle
Signed-off-by: Philippe Mathieu-Daudé
Signed-off-by: Samuel Thibault
---
slirp/ip6_icmp.c | 6 +++---
slirp/ndp_table.c | 4 ++--
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/slirp/ip6_icmp.c b/slirp/ip6_icmp.c
index 777eb574be..ee333d05a2 100644
--- a/slirp/ip6_icmp.c
+++ b/slirp/ip6_icmp.c
@@ -77,7 +77,7 @@ void icmp6_send_error(struct mbuf *m, uint8_t type, uint8_t
code)
DEBUG_ARGS((dfd, " type = %d, code = %d\n", type, code));
if (IN6_IS_ADDR_MULTICAST(&ip->ip_src) ||
-IN6_IS_ADDR_UNSPECIFIED(&ip->ip_src)) {
+in6_zero(&ip->ip_src)) {
/* TODO icmp error? */
return;
}
@@ -272,7 +272,7 @@ static void ndp_send_na(Slirp *slirp, struct ip6 *ip,
struct icmp6 *icmp)
struct mbuf *t = m_get(slirp);
struct ip6 *rip = mtod(t, struct ip6 *);
rip->ip_src = icmp->icmp6_nns.target;
-if (IN6_IS_ADDR_UNSPECIFIED(&ip->ip_src)) {
+if (in6_zero(&ip->ip_src)) {
rip->ip_dst = (struct in6_addr)ALLNODES_MULTICAST;
} else {
rip->ip_dst = ip->ip_src;
@@ -350,7 +350,7 @@ static void ndp_input(struct mbuf *m, Slirp *slirp, struct
ip6 *ip,
&& icmp->icmp6_code == 0
&& !IN6_IS_ADDR_MULTICAST(&icmp->icmp6_nns.target)
&& ntohs(ip->ip_pl) >= ICMP6_NDP_NS_MINLEN
-&& (!IN6_IS_ADDR_UNSPECIFIED(&ip->ip_src)
+&& (!in6_zero(&ip->ip_src)
|| in6_solicitednode_multicast(&ip->ip_dst))) {
if (in6_equal_host(&icmp->icmp6_nns.target)) {
/* Gratuitous NDP */
diff --git a/slirp/ndp_table.c b/slirp/ndp_table.c
index 9d4c39b45c..e1676a0a7b 100644
--- a/slirp/ndp_table.c
+++ b/slirp/ndp_table.c
@@ -23,7 +23,7 @@ void ndp_table_add(Slirp *slirp, struct in6_addr ip_addr,
ethaddr[0], ethaddr[1], ethaddr[2],
ethaddr[3], ethaddr[4], ethaddr[5]));
-if (IN6_IS_ADDR_MULTICAST(&ip_addr) || IN6_IS_ADDR_UNSPECIFIED(&ip_addr)) {
+if (IN6_IS_ADDR_MULTICAST(&ip_addr) || in6_zero(&ip_addr)) {
/* Do not register multicast or unspecified addresses */
DEBUG_CALL(" abort: do not register multicast or unspecified address");
return;
@@ -60,7 +60,7 @@ bool ndp_table_search(Slirp *slirp, struct in6_addr ip_addr,
DEBUG_ARG("ip = %s", addrstr);
#endif
-assert(!IN6_IS_ADDR_UNSPECIFIED(&ip_addr));
+assert(!in6_zero(&ip_addr));
/* Multicast address: fec0::abcd:efgh/8 -> 33:33:ab:cd:ef:gh */
if (IN6_IS_ADDR_MULTICAST(&ip_addr)) {
--
2.15.1
[Qemu-devel] [PULL 3/5] slirp: remove unnecessary struct declaration
From: Philippe Mathieu-Daudé Signed-off-by: Philippe Mathieu-Daudé Signed-off-by: Samuel Thibault --- slirp/libslirp.h | 1 - 1 file changed, 1 deletion(-) diff --git a/slirp/libslirp.h b/slirp/libslirp.h index f90f0f524c..540b3e5903 100644 --- a/slirp/libslirp.h +++ b/slirp/libslirp.h @@ -3,7 +3,6 @@ #include "qemu-common.h" -struct Slirp; typedef struct Slirp Slirp; int get_dns_addr(struct in_addr *pdns_addr); -- 2.15.1
[Qemu-devel] [PULL 5/5] slirp: add in6_dhcp_multicast()
From: Philippe Mathieu-Daudé
Signed-off-by: Philippe Mathieu-Daudé
Signed-off-by: Samuel Thibault
---
slirp/dhcpv6.h | 3 +++
slirp/udp6.c | 2 +-
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/slirp/dhcpv6.h b/slirp/dhcpv6.h
index 9189cd3f2d..3373f6cb89 100644
--- a/slirp/dhcpv6.h
+++ b/slirp/dhcpv6.h
@@ -17,6 +17,9 @@
0x00, 0x00, 0x00, 0x00,\
0x00, 0x01, 0x00, 0x02 } }
+#define in6_dhcp_multicast(a)\
+in6_equal(a, &(struct in6_addr)ALLDHCP_MULTICAST)
+
void dhcpv6_input(struct sockaddr_in6 *srcsas, struct mbuf *m);
#endif
diff --git a/slirp/udp6.c b/slirp/udp6.c
index 9fa314bc2d..7c4a6b003a 100644
--- a/slirp/udp6.c
+++ b/slirp/udp6.c
@@ -65,7 +65,7 @@ void udp6_input(struct mbuf *m)
/* handle DHCPv6 */
if (ntohs(uh->uh_dport) == DHCPV6_SERVER_PORT &&
(in6_equal(&ip->ip_dst, &slirp->vhost_addr6) ||
- in6_equal(&ip->ip_dst, &(struct in6_addr)ALLDHCP_MULTICAST))) {
+ in6_dhcp_multicast(&ip->ip_dst))) {
m->m_data += iphlen;
m->m_len -= iphlen;
dhcpv6_input(&lhost, m);
--
2.15.1
[Qemu-devel] [PULL 2/5] slirp: remove unused header
From: Philippe Mathieu-Daudé Signed-off-by: Philippe Mathieu-Daudé Tested-by: Thomas Huth Signed-off-by: Samuel Thibault --- slirp/slirp.h | 1 - 1 file changed, 1 deletion(-) diff --git a/slirp/slirp.h b/slirp/slirp.h index 898ec9516d..06febfc78b 100644 --- a/slirp/slirp.h +++ b/slirp/slirp.h @@ -1,7 +1,6 @@ #ifndef SLIRP_H #define SLIRP_H -#include "qemu/host-utils.h" #include "slirp_config.h" #ifdef _WIN32 -- 2.15.1
[Qemu-devel] [PATCHv4] linux-user: Add getcpu() support
Signed-off-by: Samuel Thibault
---
Difference between v1 and v2: handle failure of put_user_u32 with goto efault;
Difference between v2 and v3: handle failure of sys_getcpu system call
Difference between v3 and v4: use is_error
---
linux-user/syscall.c | 19 +++
1 file changed, 19 insertions(+)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 11c9116c4a..26403d7e5c 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -296,6 +296,8 @@ _syscall3(int, sys_sched_getaffinity, pid_t, pid, unsigned
int, len,
#define __NR_sys_sched_setaffinity __NR_sched_setaffinity
_syscall3(int, sys_sched_setaffinity, pid_t, pid, unsigned int, len,
unsigned long *, user_mask_ptr);
+#define __NR_sys_getcpu __NR_getcpu
+_syscall3(int, sys_getcpu, unsigned *, cpu, unsigned *, node, void *, tcache);
_syscall4(int, reboot, int, magic1, int, magic2, unsigned int, cmd,
void *, arg);
_syscall2(int, capget, struct __user_cap_header_struct *, header,
@@ -10403,6 +10405,23 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
arg1,
ret = get_errno(sys_sched_setaffinity(arg1, mask_size, mask));
}
break;
+case TARGET_NR_getcpu:
+{
+unsigned cpu, node;
+ret = get_errno(sys_getcpu(arg1 ? &cpu : NULL,
+ arg2 ? &node : NULL,
+ NULL));
+if (is_error(ret)) {
+goto fail;
+}
+if (arg1 && put_user_u32(cpu, arg1)) {
+goto efault;
+}
+if (arg2 && put_user_u32(node, arg2)) {
+goto efault;
+}
+}
+break;
case TARGET_NR_sched_setparam:
{
struct sched_param *target_schp;
--
2.15.1
[Qemu-devel] [PATCHv3] linux-user: Add getcpu() support
Signed-off-by: Samuel Thibault
---
Difference between v1 and v2: handle failure of put_user_u32 with goto efault;
Difference between v2 and v3: handle failure of sys_getcpu system call
---
linux-user/syscall.c | 19 +++
1 file changed, 19 insertions(+)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 11c9116c4a..f8cfaf043a 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -296,6 +296,8 @@ _syscall3(int, sys_sched_getaffinity, pid_t, pid, unsigned
int, len,
#define __NR_sys_sched_setaffinity __NR_sched_setaffinity
_syscall3(int, sys_sched_setaffinity, pid_t, pid, unsigned int, len,
unsigned long *, user_mask_ptr);
+#define __NR_sys_getcpu __NR_getcpu
+_syscall3(int, sys_getcpu, unsigned *, cpu, unsigned *, node, void *, tcache);
_syscall4(int, reboot, int, magic1, int, magic2, unsigned int, cmd,
void *, arg);
_syscall2(int, capget, struct __user_cap_header_struct *, header,
@@ -10403,6 +10405,23 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
arg1,
ret = get_errno(sys_sched_setaffinity(arg1, mask_size, mask));
}
break;
+case TARGET_NR_getcpu:
+{
+unsigned cpu, node;
+ret = get_errno(sys_getcpu(arg1 ? &cpu : NULL,
+ arg2 ? &node : NULL,
+ NULL));
+if (ret < 0) {
+goto fail;
+}
+if (arg1 && put_user_u32(cpu, arg1)) {
+goto efault;
+}
+if (arg2 && put_user_u32(node, arg2)) {
+goto efault;
+}
+}
+break;
case TARGET_NR_sched_setparam:
{
struct sched_param *target_schp;
--
2.15.1
[Qemu-devel] [PATCHv2] linux-user: Add getcpu() support
Signed-off-by: Samuel Thibault
---
Difference from v1: handle failure of put_user_u32 with goto efault;
---
linux-user/syscall.c | 16
1 file changed, 16 insertions(+)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 11c9116c4a..89d78b7b48 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -296,6 +296,8 @@ _syscall3(int, sys_sched_getaffinity, pid_t, pid, unsigned
int, len,
#define __NR_sys_sched_setaffinity __NR_sched_setaffinity
_syscall3(int, sys_sched_setaffinity, pid_t, pid, unsigned int, len,
unsigned long *, user_mask_ptr);
+#define __NR_sys_getcpu __NR_getcpu
+_syscall3(int, sys_getcpu, unsigned *, cpu, unsigned *, node, void *, tcache);
_syscall4(int, reboot, int, magic1, int, magic2, unsigned int, cmd,
void *, arg);
_syscall2(int, capget, struct __user_cap_header_struct *, header,
@@ -10403,6 +10405,20 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
arg1,
ret = get_errno(sys_sched_setaffinity(arg1, mask_size, mask));
}
break;
+case TARGET_NR_getcpu:
+{
+unsigned cpu, node;
+ret = get_errno(sys_getcpu(arg1 ? &cpu : NULL,
+ arg2 ? &node : NULL,
+ NULL));
+if (arg1 && put_user_u32(cpu, arg1)) {
+goto efault;
+}
+if (arg2 && put_user_u32(node, arg2)) {
+goto efault;
+}
+}
+break;
case TARGET_NR_sched_setparam:
{
struct sched_param *target_schp;
--
2.15.1
Re: [Qemu-devel] [PATCH] linux-user: Add getcpu() support
Laurent Vivier, on mer. 10 janv. 2018 16:53:47 +0100, wrote:
> Le 28/12/2017 à 18:39, Laurent Vivier a écrit :
> > Le 28/12/2017 à 16:00, Samuel Thibault a écrit :
> >> Signed-off-by: Samuel Thibault
> >> ---
> >> linux-user/syscall.c | 16
> >> 1 file changed, 16 insertions(+)
> >>
> >> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> >> index 8ec7de96ce..bb8cb726f5 100644
> >> --- a/linux-user/syscall.c
> >> +++ b/linux-user/syscall.c
> >> @@ -296,6 +296,8 @@ _syscall3(int, sys_sched_getaffinity, pid_t, pid,
> >> unsigned int, len,
> >> #define __NR_sys_sched_setaffinity __NR_sched_setaffinity
> >> _syscall3(int, sys_sched_setaffinity, pid_t, pid, unsigned int, len,
> >>unsigned long *, user_mask_ptr);
> >> +#define __NR_sys_getcpu __NR_getcpu
> >> +_syscall3(int, sys_getcpu, unsigned *, cpu, unsigned *, node, void *,
> >> tcache);
> >> _syscall4(int, reboot, int, magic1, int, magic2, unsigned int, cmd,
> >>void *, arg);
> >> _syscall2(int, capget, struct __user_cap_header_struct *, header,
> >> @@ -10443,6 +10445,20 @@ abi_long do_syscall(void *cpu_env, int num,
> >> abi_long arg1,
> >> ret = get_errno(sys_sched_setaffinity(arg1, mask_size, mask));
> >> }
> >> break> +case TARGET_NR_getcpu:
> >> +{
> >> +unsigned cpu, node;
> >> +ret = get_errno(sys_getcpu(arg1 ? &cpu : NULL,
> >> + arg2 ? &node : NULL,
> >> + NULL));
> >> +if (arg1) {
> >> +put_user(cpu, arg1, abi_uint);
> >> +}
> >> +if (arg2) {
> >> +put_user(node, arg2, abi_uint);
> >> +}
> >
> > You must check for EFAULT.
> >
> > if (arg1 && put_user_u32(cpu, arg1)) {
> > goto efault;
> > }
> > ...
> >
> > I don't think you need to use the abi_uint type as we are using
> > put_user_u32() for all the other syscalls for unsigned int.
>
> could you send an updated version of your patch?
Oops, I had missed that mail. Will work on it.
Samuel
Re: [Qemu-devel] [RFC PATCH 12/12] slirp: use HOST_SUPPORTS_UNALIGNED_ACCESS
Philippe Mathieu-Daudé, on lun. 08 janv. 2018 14:29:04 -0300, wrote: > Access struct in6_addr with 'void *', then cast to 'u8 *' to avoid alignment > issues. Err, I don't understand the point. in6_addr's s6_addr is already a uint8_t by the standard. There is no non-byte access in the existing code. Samuel
Re: [Qemu-devel] [PATCH 09/12] slirp: add in6_dhcp_multicast()
Philippe Mathieu-Daudé, on lun. 08 janv. 2018 14:29:01 -0300, wrote:
> Signed-off-by: Philippe Mathieu-Daudé
Applied to my tree.
Thanks,
Samuel
> ---
> slirp/dhcpv6.h | 3 +++
> slirp/udp6.c | 2 +-
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/slirp/dhcpv6.h b/slirp/dhcpv6.h
> index 9189cd3f2d..3373f6cb89 100644
> --- a/slirp/dhcpv6.h
> +++ b/slirp/dhcpv6.h
> @@ -17,6 +17,9 @@
> 0x00, 0x00, 0x00, 0x00,\
> 0x00, 0x01, 0x00, 0x02 } }
>
> +#define in6_dhcp_multicast(a)\
> +in6_equal(a, &(struct in6_addr)ALLDHCP_MULTICAST)
> +
> void dhcpv6_input(struct sockaddr_in6 *srcsas, struct mbuf *m);
>
> #endif
> diff --git a/slirp/udp6.c b/slirp/udp6.c
> index 9fa314bc2d..7c4a6b003a 100644
> --- a/slirp/udp6.c
> +++ b/slirp/udp6.c
> @@ -65,7 +65,7 @@ void udp6_input(struct mbuf *m)
> /* handle DHCPv6 */
> if (ntohs(uh->uh_dport) == DHCPV6_SERVER_PORT &&
> (in6_equal(&ip->ip_dst, &slirp->vhost_addr6) ||
> - in6_equal(&ip->ip_dst, &(struct in6_addr)ALLDHCP_MULTICAST))) {
> + in6_dhcp_multicast(&ip->ip_dst))) {
> m->m_data += iphlen;
> m->m_len -= iphlen;
> dhcpv6_input(&lhost, m);
> --
> 2.15.1
>
--
Samuel
Créer une hiérarchie supplementaire pour remedier à un problème (?) de
dispersion est d'une logique digne des Shadocks.
* BT in: Guide du Cabaliste Usenet - La Cabale vote oui (les Shadocks aussi) *
Re: [Qemu-devel] [PATCH 08/12] slirp: removed unused code
Thomas Huth, on lun. 08 janv. 2018 20:02:14 +0100, wrote:
> On 08.01.2018 18:29, Philippe Mathieu-Daudé wrote:
> > Signed-off-by: Philippe Mathieu-Daudé
> > ---
> > slirp/ip.h | 13 -
> > 1 file changed, 13 deletions(-)
> >
> > diff --git a/slirp/ip.h b/slirp/ip.h
> > index e29ccd3c9f..71c3642cfe 100644
> > --- a/slirp/ip.h
> > +++ b/slirp/ip.h
> > @@ -233,17 +233,4 @@ struct ipasfrag {
> > #define ipf_next ipf_link.next
> > #define ipf_prev ipf_link.prev
> >
> > -/*
> > - * Structure stored in mbuf in inpcb.ip_options
> > - * and passed to ip_output when ip options are in use.
> > - * The actual length of the options (including ipopt_dst)
> > - * is in m_len.
> > - */
> > -#define MAX_IPOPTLEN 40
> > -
> > -struct ipoption {
> > - struct in_addr ipopt_dst; /* first-hop dst if source routed */
> > - int8_t ipopt_list[MAX_IPOPTLEN]; /* options proper */
> > -} QEMU_PACKED;
> > -
> > #endif
> >
>
> Reviewed-by: Thomas Huth
Applied to my tree.
Thanks,
Samuel
Re: [Qemu-devel] [PATCH 07/12] slirp: remove unnecessary
Thomas Huth, on lun. 08 janv. 2018 20:01:10 +0100, wrote: > The subject is missing a word or two. Applied to my tree with a more complete subject :) Samuel
Re: [Qemu-devel] [PATCH 06/12] slirp: remove unused header
Thomas Huth, on lun. 08 janv. 2018 19:54:27 +0100, wrote: > On 08.01.2018 18:28, Philippe Mathieu-Daudé wrote: > > Signed-off-by: Philippe Mathieu-Daudé > > --- > > slirp/slirp.h | 1 - > > 1 file changed, 1 deletion(-) > > > > diff --git a/slirp/slirp.h b/slirp/slirp.h > > index 9a7287e7cc..447dc045a8 100644 > > --- a/slirp/slirp.h > > +++ b/slirp/slirp.h > > @@ -1,7 +1,6 @@ > > #ifndef SLIRP_H > > #define SLIRP_H > > > > -#include "qemu/host-utils.h" > > This had been added by commit 87776ab72b02e3c99a042ab7a0a378bc457cc069 > which stated "There are some inclusions of qemu/host-utils.h in headers, > but they are all necessary." ... I wonder why it is not necessary > anymore today...? > > Anyway, seems like it compiles now fine without this: > > Tested-by: Thomas Huth Applied to my tree, thanks! Samuel
Re: [Qemu-devel] [PATCH 04/12] slirp: add in6_multicast() and use it instead of IN6_IS_ADDR_MULTICAST()
Richard Henderson, on mar. 09 janv. 2018 08:33:22 -0800, wrote: > On 01/08/2018 09:28 AM, Philippe Mathieu-Daudé wrote: > > Host: Mac OS 10.12.5 > > Compiler: Apple LLVM version 8.1.0 (clang-802.0.42) > > > > slirp/ip6_icmp.c:79:32: warning: taking address of packed member 'ip_src' > > of class or > > structure 'ip6' may result in an unaligned pointer value > > [-Waddress-of-packed-member] > > if (IN6_IS_ADDR_MULTICAST(&ip->ip_src) || > > ^~ > > /usr/include/netinet6/in6.h:299:36: note: expanded from macro > > 'IN6_IS_ADDR_MULTICAST' > > #define IN6_IS_ADDR_MULTICAST(a)((a)->s6_addr[0] == 0xff) > > The fact that you replace a macro with a function of exactly the same code in > order to avoid a diagnostic sure looks like a compiler bug to me. The compiler could be smarter to realize that it's actually a byte access indeed. There are two things for me: - The OS-provided implementation of IN6_IS_ADDR_MULTICAST could be doing 32bit accesses and whatnot, so we are not supposed to use it on packed fields. - With the proposal patch, the compiler could still emit the warning, since we are basically still passing the address of the packed field to a function which the compiler might not see either. We are however sure that the function makes an aligned access. So I'd say we should still rather do it to be on the safe side. With Thomas Huth's comments applied. BTW, Thomas Huth wrote: > I think this might also be a bug in the macro definitions. > > > #define IN6_IS_ADDR_MULTICAST(a)((a)->s6_addr[0] == 0xff) > On Linux, it is defined like this: > > #define IN6_IS_ADDR_MULTICAST(a) (((const uint8_t *) (a))[0] == 0xff) Well, the standard says that IN6_IS_ADDR_MULTICAST takes a struct in6_addr*, so it can use the s6_addr field. Samuel
Re: [Qemu-devel] [PATCH 03/12] slirp: avoid IN6_IS_ADDR_UNSPECIFIED(), rather use in6_zero()
Philippe Mathieu-Daudé, on lun. 08 janv. 2018 14:28:55 -0300, wrote:
> Host: Mac OS 10.12.5
> Compiler: Apple LLVM version 8.1.0 (clang-802.0.42)
>
> slirp/ip6_icmp.c:80:38: warning: taking address of packed member 'ip_src'
> of class or
> structure 'ip6' may result in an unaligned pointer value
> [-Waddress-of-packed-member]
> IN6_IS_ADDR_UNSPECIFIED(&ip->ip_src)) {
>^~
> /usr/include/netinet6/in6.h:238:42: note: expanded from macro
> 'IN6_IS_ADDR_UNSPECIFIED'
> ((*(const __uint32_t *)(const void *)(&(a)->s6_addr[0]) == 0) && \
I have applied it to my tree.
Thanks,
Samuel
Re: [Qemu-devel] [PATCH 01/12] slirp: remove QEMU_PACKED from structures with don't require it
Hello,
Philippe Mathieu-Daudé, on lun. 08 janv. 2018 14:28:53 -0300, wrote:
> struct mbuf_ptr {
> struct mbuf *mptr;
> uint32_t dummy;
> -} QEMU_PACKED;
> +};
> #else
> struct mbuf_ptr {
> struct mbuf *mptr;
> -} QEMU_PACKED;
> +};
> @@ -199,7 +199,7 @@ struct ipovly {
> uint16_tih_len; /* protocol length */
> struct in_addr ih_src; /* source internet address */
> struct in_addr ih_dst; /* destination internet address */
> -} QEMU_PACKED;
> +};
Did you actually try to change these structures? The presence of the
"dummy" field should have hinted that it's not a trivial structure :)
mbuf_ptr is used in struct ipovly which is to have the same size as the
ipv4 header. One would have to untangle that before removing the packed
attribute.
> @@ -215,7 +215,7 @@ struct ipq {
> uint8_t ipq_p; /* protocol of this fragment */
> uint16_tipq_id; /* sequence id for reassembly */
> struct in_addr ipq_src,ipq_dst;
> -} QEMU_PACKED;
> +};
This one seems safe to me. I'd still rather see an actual test with
added holes in the structure to be sure :)
> @@ -225,7 +225,7 @@ struct ipq {
> struct ipasfrag {
> struct qlink ipf_link;
> struct ip ipf_ip;
> -} QEMU_PACKED;
> +};
Please see iptofrag and fragtoip in ip_input.c, they assume that ipf_ip
immediately follows ipf_link. I guess using offsetof there should avoid
the issue. Note however that slirp assumes that in a 32bit-aligned
ethernet frame it has enough room before the IP header to stuff the
ipf_link things. We could add a build-time check that offsetof(ipf_ip)
<= 2 + ETH_HLEN
> @@ -136,7 +136,7 @@ bool arp_table_search(Slirp *slirp, uint32_t ip_addr,
> struct ndpentry {
> unsigned char eth_addr[ETH_ALEN]; /* sender hardware address */
> struct in6_addr ip_addr;/* sender IP address */
> -} QEMU_PACKED;
> +};
This one should be safe.
Samuel
[Qemu-devel] [PATCHv2] linux-user: Fix sched_get/setaffinity conversion
sched_get/setaffinity linux-user syscalls were missing conversions for
little/big endian, which is hairy since longs may not be the same size
either.
For simplicity, this just introduces loops to convert bit by bit like is
done for select.
Signed-off-by: Samuel Thibault
Reviewed-by: Laurent Vivier
---
Difference from v1: bitmask computation was separated out into
target_to_host_cpu_mask()/host_to_target_cpu_mask().
linux-user/syscall.c | 81 ++--
1 file changed, 73 insertions(+), 8 deletions(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 11c9116c4a..cac07419aa 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -7716,6 +7716,73 @@ static TargetFdTrans target_inotify_trans = {
};
#endif
+static int target_to_host_cpu_mask(unsigned long *host_mask,
+ size_t host_size,
+ abi_ulong target_addr,
+ size_t target_size)
+{
+unsigned target_bits = sizeof(abi_ulong) * 8;
+unsigned host_bits = sizeof(*host_mask) * 8;
+abi_ulong *target_mask;
+unsigned i, j;
+
+assert(host_size >= target_size);
+
+target_mask = lock_user(VERIFY_READ, target_addr, target_size, 1);
+if (!target_mask) {
+return -TARGET_EFAULT;
+}
+memset(host_mask, 0, host_size);
+
+for (i = 0 ; i < target_size / sizeof(abi_ulong); i++) {
+unsigned bit = i * target_bits;
+abi_ulong val;
+
+__get_user(val, &target_mask[i]);
+for (j = 0; j < target_bits; j++, bit++) {
+if (val & (1UL << j)) {
+host_mask[bit / host_bits] |= 1UL << (bit % host_bits);
+}
+}
+}
+
+unlock_user(target_mask, target_addr, 0);
+return 0;
+}
+
+static int host_to_target_cpu_mask(const unsigned long *host_mask,
+ size_t host_size,
+ abi_ulong target_addr,
+ size_t target_size)
+{
+unsigned target_bits = sizeof(abi_ulong) * 8;
+unsigned host_bits = sizeof(*host_mask) * 8;
+abi_ulong *target_mask;
+unsigned i, j;
+
+assert(host_size >= target_size);
+
+target_mask = lock_user(VERIFY_WRITE, target_addr, target_size, 0);
+if (!target_mask) {
+return -TARGET_EFAULT;
+}
+
+for (i = 0 ; i < target_size / sizeof(abi_ulong); i++) {
+unsigned bit = i * target_bits;
+abi_ulong val = 0;
+
+for (j = 0; j < target_bits; j++, bit++) {
+if (host_mask[bit / host_bits] & (1UL << (bit % host_bits))) {
+val |= 1UL << j;
+}
+}
+__put_user(val, &target_mask[i]);
+}
+
+unlock_user(target_mask, target_addr, target_size);
+return 0;
+}
+
/* do_syscall() should always have a single exit point at the end so
that actions, such as logging of syscall results, can be performed.
All errnos that do_syscall() returns must be -TARGET_. */
@@ -10353,6 +10420,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
arg1,
mask_size = (arg2 + (sizeof(*mask) - 1)) & ~(sizeof(*mask) - 1);
mask = alloca(mask_size);
+memset(mask, 0, mask_size);
ret = get_errno(sys_sched_getaffinity(arg1, mask_size, mask));
if (!is_error(ret)) {
@@ -10372,9 +10440,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
arg1,
ret = arg2;
}
-if (copy_to_user(arg3, mask, ret)) {
-goto efault;
-}
+ret = host_to_target_cpu_mask(mask, mask_size, arg3, arg2);
}
}
break;
@@ -10392,13 +10458,12 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
arg1,
break;
}
mask_size = (arg2 + (sizeof(*mask) - 1)) & ~(sizeof(*mask) - 1);
-
mask = alloca(mask_size);
-if (!lock_user_struct(VERIFY_READ, p, arg3, 1)) {
-goto efault;
+
+ret = target_to_host_cpu_mask(mask, mask_size, arg3, arg2);
+if (ret) {
+break;
}
-memcpy(mask, p, arg2);
-unlock_user_struct(p, arg2, 0);
ret = get_errno(sys_sched_setaffinity(arg1, mask_size, mask));
}
--
2.15.1
