From nobody Sat May 4 09:59:33 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org Return-Path: Received: from lists.gnu.org (208.118.235.17 [208.118.235.17]) by mx.zohomail.com with SMTPS id 1506276672103938.6914146235307; Sun, 24 Sep 2017 11:11:12 -0700 (PDT) Received: from localhost ([::1]:38977 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dwBMs-0005vl-Co for importer@patchew.org; Sun, 24 Sep 2017 14:11:02 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33920) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dwBKp-0004Z1-7f for qemu-devel@nongnu.org; Sun, 24 Sep 2017 14:08:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dwBKm-000580-55 for qemu-devel@nongnu.org; Sun, 24 Sep 2017 14:08:55 -0400 Received: from hera.aquilenet.fr ([141.255.128.1]:49409) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dwBKl-00057W-Re for qemu-devel@nongnu.org; Sun, 24 Sep 2017 14:08:52 -0400 Received: from localhost (localhost [127.0.0.1]) by hera.aquilenet.fr (Postfix) with ESMTP id A48A8E9D0; Sun, 24 Sep 2017 20:08:51 +0200 (CEST) Received: from hera.aquilenet.fr ([127.0.0.1]) by localhost (hera.aquilenet.fr [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id zPXlgqOq-JUh; Sun, 24 Sep 2017 20:08:49 +0200 (CEST) Received: from var.youpi.perso.aquilenet.fr (unknown [IPv6:2a01:cb19:181:c200:3602:86ff:fe2c:6a19]) by hera.aquilenet.fr (Postfix) with ESMTPSA id BF17AE9D5; Sun, 24 Sep 2017 20:08:49 +0200 (CEST) Received: from samy by var.youpi.perso.aquilenet.fr with local (Exim 4.89) (envelope-from ) id 1dwBKi-0004zw-B8; Sun, 24 Sep 2017 20:08:48 +0200 X-Virus-Scanned: Debian amavisd-new at aquilenet.fr From: Samuel Thibault To: qemu-devel@nongnu.org Date: Sun, 24 Sep 2017 20:08:46 +0200 Message-Id: <20170924180848.19168-2-samuel.thibault@ens-lyon.org> X-Mailer: git-send-email 2.14.1 In-Reply-To: <20170924180848.19168-1-samuel.thibault@ens-lyon.org> References: <20170924180848.19168-1-samuel.thibault@ens-lyon.org> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 141.255.128.1 Subject: [Qemu-devel] [PULL 1/3] slirp: Add explanation for hostfwd parsing failure X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: jan.kiszka@siemens.com, "Dr. David Alan Gilbert" , stefanha@redhat.com, Samuel Thibault Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" From: "Dr. David Alan Gilbert" e.g. ./x86_64-softmmu/qemu-system-x86_64 -nographic -netdev 'user,id=3Dvnet,host= fwd=3D:555.0.0.0:0-:22' qemu-system-x86_64: -netdev user,id=3Dvnet,hostfwd=3D:555.0.0.0:0-:22: Inva= lid host forwarding rule ':555.0.0.0:0-:22' (Bad host address) Signed-off-by: Dr. David Alan Gilbert Reviewed-by: Philippe Mathieu-Daud=C3=A9 Signed-off-by: Samuel Thibault --- net/slirp.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/net/slirp.c b/net/slirp.c index 01ed21c006..318a26e892 100644 --- a/net/slirp.c +++ b/net/slirp.c @@ -496,9 +496,11 @@ static int slirp_hostfwd(SlirpState *s, const char *re= dir_str, char buf[256]; int is_udp; char *end; + const char *fail_reason =3D "Unknown reason"; =20 p =3D redir_str; if (!p || get_str_sep(buf, sizeof(buf), &p, ':') < 0) { + fail_reason =3D "No : separators"; goto fail_syntax; } if (!strcmp(buf, "tcp") || buf[0] =3D=3D '\0') { @@ -506,35 +508,43 @@ static int slirp_hostfwd(SlirpState *s, const char *r= edir_str, } else if (!strcmp(buf, "udp")) { is_udp =3D 1; } else { + fail_reason =3D "Bad protocol name"; goto fail_syntax; } =20 if (!legacy_format) { if (get_str_sep(buf, sizeof(buf), &p, ':') < 0) { + fail_reason =3D "Missing : separator"; goto fail_syntax; } if (buf[0] !=3D '\0' && !inet_aton(buf, &host_addr)) { + fail_reason =3D "Bad host address"; goto fail_syntax; } } =20 if (get_str_sep(buf, sizeof(buf), &p, legacy_format ? ':' : '-') < 0) { + fail_reason =3D "Bad host port separator"; goto fail_syntax; } host_port =3D strtol(buf, &end, 0); if (*end !=3D '\0' || host_port < 0 || host_port > 65535) { + fail_reason =3D "Bad host port"; goto fail_syntax; } =20 if (get_str_sep(buf, sizeof(buf), &p, ':') < 0) { + fail_reason =3D "Missing guest address"; goto fail_syntax; } if (buf[0] !=3D '\0' && !inet_aton(buf, &guest_addr)) { + fail_reason =3D "Bad guest address"; goto fail_syntax; } =20 guest_port =3D strtol(p, &end, 0); if (*end !=3D '\0' || guest_port < 1 || guest_port > 65535) { + fail_reason =3D "Bad guest port"; goto fail_syntax; } =20 @@ -547,7 +557,8 @@ static int slirp_hostfwd(SlirpState *s, const char *red= ir_str, return 0; =20 fail_syntax: - error_setg(errp, "Invalid host forwarding rule '%s'", redir_str); + error_setg(errp, "Invalid host forwarding rule '%s' (%s)", redir_str, + fail_reason); return -1; } =20 --=20 2.14.1 From nobody Sat May 4 09:59:33 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1506276671684678.8548367906468; Sun, 24 Sep 2017 11:11:11 -0700 (PDT) Received: from localhost ([::1]:38976 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dwBMp-0005uY-Vl for importer@patchew.org; Sun, 24 Sep 2017 14:11:00 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33921) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dwBKp-0004Z2-7o for qemu-devel@nongnu.org; Sun, 24 Sep 2017 14:08:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dwBKm-00058O-PO for qemu-devel@nongnu.org; Sun, 24 Sep 2017 14:08:55 -0400 Received: from hera.aquilenet.fr ([141.255.128.1]:49414) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dwBKm-00057g-F8 for qemu-devel@nongnu.org; Sun, 24 Sep 2017 14:08:52 -0400 Received: from localhost (localhost [127.0.0.1]) by hera.aquilenet.fr (Postfix) with ESMTP id 36CC8E9D5; Sun, 24 Sep 2017 20:08:52 +0200 (CEST) Received: from hera.aquilenet.fr ([127.0.0.1]) by localhost (hera.aquilenet.fr [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 4HhN09IebuHx; Sun, 24 Sep 2017 20:08:50 +0200 (CEST) Received: from var.youpi.perso.aquilenet.fr (unknown [IPv6:2a01:cb19:181:c200:3602:86ff:fe2c:6a19]) by hera.aquilenet.fr (Postfix) with ESMTPSA id CE2EFE9D6; Sun, 24 Sep 2017 20:08:49 +0200 (CEST) Received: from samy by var.youpi.perso.aquilenet.fr with local (Exim 4.89) (envelope-from ) id 1dwBKi-0004zy-Bn; Sun, 24 Sep 2017 20:08:48 +0200 X-Virus-Scanned: Debian amavisd-new at aquilenet.fr From: Samuel Thibault To: qemu-devel@nongnu.org Date: Sun, 24 Sep 2017 20:08:47 +0200 Message-Id: <20170924180848.19168-3-samuel.thibault@ens-lyon.org> X-Mailer: git-send-email 2.14.1 In-Reply-To: <20170924180848.19168-1-samuel.thibault@ens-lyon.org> References: <20170924180848.19168-1-samuel.thibault@ens-lyon.org> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 141.255.128.1 Subject: [Qemu-devel] [PULL 2/3] slirp: Fix intermittent send queue hangs on a socket X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: jan.kiszka@siemens.com, Kevin Cernekee , stefanha@redhat.com, Samuel Thibault Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" From: Kevin Cernekee if_output() originally sent one mbuf per call and used the slirp->next_m variable to keep track of where it left off. But nowadays it tries to send all of the mbufs from the fastq, and one mbuf from each session on the batchq. The next_m variable is both redundant and harmful: there is a case[0] involving delayed packets in which next_m ends up pointing to &slirp->if_batchq when an active session still exists, and this blocks all traffic for that session until qemu is restarted. The test case was created to reproduce a problem that was seen on long-running Chromium OS VM tests[1] which rapidly create and destroy ssh connections through hostfwd. [0] https://pastebin.com/NNy6LreF [1] https://bugs.chromium.org/p/chromium/issues/detail?id=3D766323 Signed-off-by: Kevin Cernekee Signed-off-by: Samuel Thibault --- slirp/if.c | 51 +++++++++++++++++---------------------------------- slirp/slirp.h | 1 - 2 files changed, 17 insertions(+), 35 deletions(-) diff --git a/slirp/if.c b/slirp/if.c index 51ae0d0e9a..6262d77495 100644 --- a/slirp/if.c +++ b/slirp/if.c @@ -30,7 +30,6 @@ if_init(Slirp *slirp) { slirp->if_fastq.qh_link =3D slirp->if_fastq.qh_rlink =3D &slirp->if_fa= stq; slirp->if_batchq.qh_link =3D slirp->if_batchq.qh_rlink =3D &slirp->if_= batchq; - slirp->next_m =3D (struct mbuf *) &slirp->if_batchq; } =20 /* @@ -100,10 +99,6 @@ if_output(struct socket *so, struct mbuf *ifm) } } else { ifq =3D (struct mbuf *) slirp->if_batchq.qh_rlink; - /* Set next_m if the queue was empty so far */ - if ((struct quehead *) slirp->next_m =3D=3D &slirp->if_bat= chq) { - slirp->next_m =3D ifm; - } } =20 /* Create a new doubly linked list for this session */ @@ -143,21 +138,18 @@ diddit: } =20 /* - * Send a packet - * We choose a packet based on its position in the output queues; + * Send one packet from each session. * If there are packets on the fastq, they are sent FIFO, before - * everything else. Otherwise we choose the first packet from the - * batchq and send it. the next packet chosen will be from the session - * after this one, then the session after that one, and so on.. So, - * for example, if there are 3 ftp session's fighting for bandwidth, + * everything else. Then we choose the first packet from each + * batchq session (socket) and send it. + * For example, if there are 3 ftp sessions fighting for bandwidth, * one packet will be sent from the first session, then one packet - * from the second session, then one packet from the third, then back - * to the first, etc. etc. + * from the second session, then one packet from the third. */ void if_start(Slirp *slirp) { uint64_t now =3D qemu_clock_get_ns(QEMU_CLOCK_REALTIME); - bool from_batchq, next_from_batchq; + bool from_batchq =3D false; struct mbuf *ifm, *ifm_next, *ifqt; =20 DEBUG_CALL("if_start"); @@ -167,26 +159,29 @@ void if_start(Slirp *slirp) } slirp->if_start_busy =3D true; =20 + struct mbuf *batch_head =3D NULL; + if (slirp->if_batchq.qh_link !=3D &slirp->if_batchq) { + batch_head =3D (struct mbuf *) slirp->if_batchq.qh_link; + } + if (slirp->if_fastq.qh_link !=3D &slirp->if_fastq) { ifm_next =3D (struct mbuf *) slirp->if_fastq.qh_link; - next_from_batchq =3D false; - } else if ((struct quehead *) slirp->next_m !=3D &slirp->if_batchq) { - /* Nothing on fastq, pick up from batchq via next_m */ - ifm_next =3D slirp->next_m; - next_from_batchq =3D true; + } else if (batch_head) { + /* Nothing on fastq, pick up from batchq */ + ifm_next =3D batch_head; + from_batchq =3D true; } else { ifm_next =3D NULL; } =20 while (ifm_next) { ifm =3D ifm_next; - from_batchq =3D next_from_batchq; =20 ifm_next =3D ifm->ifq_next; if ((struct quehead *) ifm_next =3D=3D &slirp->if_fastq) { /* No more packets in fastq, switch to batchq */ - ifm_next =3D slirp->next_m; - next_from_batchq =3D true; + ifm_next =3D batch_head; + from_batchq =3D true; } if ((struct quehead *) ifm_next =3D=3D &slirp->if_batchq) { /* end of batchq */ @@ -199,11 +194,6 @@ void if_start(Slirp *slirp) continue; } =20 - if (ifm =3D=3D slirp->next_m) { - /* Set which packet to send on next iteration */ - slirp->next_m =3D ifm->ifq_next; - } - /* Remove it from the queue */ ifqt =3D ifm->ifq_prev; remque(ifm); @@ -214,15 +204,8 @@ void if_start(Slirp *slirp) =20 insque(next, ifqt); ifs_remque(ifm); - if (!from_batchq) { - /* Next packet in fastq is from the same session */ ifm_next =3D next; - next_from_batchq =3D false; - } else if ((struct quehead *) slirp->next_m =3D=3D &slirp->if_= batchq) { - /* Set next_m and ifm_next if the session packet is now the - * only one on batchq */ - slirp->next_m =3D ifm_next =3D next; } } =20 diff --git a/slirp/slirp.h b/slirp/slirp.h index 5af4f482b5..898ec9516d 100644 --- a/slirp/slirp.h +++ b/slirp/slirp.h @@ -183,7 +183,6 @@ struct Slirp { /* if states */ struct quehead if_fastq; /* fast queue (for interactive data) */ struct quehead if_batchq; /* queue for non-interactive data */ - struct mbuf *next_m; /* pointer to next mbuf to output */ bool if_start_busy; /* avoid if_start recursion */ =20 /* ip states */ --=20 2.14.1 From nobody Sat May 4 09:59:33 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1506276789845824.8762956544625; Sun, 24 Sep 2017 11:13:09 -0700 (PDT) Received: from localhost ([::1]:38987 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dwBOq-0007jn-6O for importer@patchew.org; Sun, 24 Sep 2017 14:13:04 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33922) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dwBKp-0004Z3-7q for qemu-devel@nongnu.org; Sun, 24 Sep 2017 14:08:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dwBKn-00058e-Ep for qemu-devel@nongnu.org; Sun, 24 Sep 2017 14:08:55 -0400 Received: from hera.aquilenet.fr ([141.255.128.1]:49421) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dwBKn-00058A-7y for qemu-devel@nongnu.org; Sun, 24 Sep 2017 14:08:53 -0400 Received: from localhost (localhost [127.0.0.1]) by hera.aquilenet.fr (Postfix) with ESMTP id 3821BE9D6; Sun, 24 Sep 2017 20:08:53 +0200 (CEST) Received: from hera.aquilenet.fr ([127.0.0.1]) by localhost (hera.aquilenet.fr [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id H3LI8an-rBnm; Sun, 24 Sep 2017 20:08:51 +0200 (CEST) Received: from var.youpi.perso.aquilenet.fr (unknown [IPv6:2a01:cb19:181:c200:3602:86ff:fe2c:6a19]) by hera.aquilenet.fr (Postfix) with ESMTPSA id D54C9E9D7; Sun, 24 Sep 2017 20:08:49 +0200 (CEST) Received: from samy by var.youpi.perso.aquilenet.fr with local (Exim 4.89) (envelope-from ) id 1dwBKi-000500-CU; Sun, 24 Sep 2017 20:08:48 +0200 X-Virus-Scanned: Debian amavisd-new at aquilenet.fr From: Samuel Thibault To: qemu-devel@nongnu.org Date: Sun, 24 Sep 2017 20:08:48 +0200 Message-Id: <20170924180848.19168-4-samuel.thibault@ens-lyon.org> X-Mailer: git-send-email 2.14.1 In-Reply-To: <20170924180848.19168-1-samuel.thibault@ens-lyon.org> References: <20170924180848.19168-1-samuel.thibault@ens-lyon.org> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 141.255.128.1 Subject: [Qemu-devel] [PULL 3/3] slirp: Add a special case for the NULL socket X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: jan.kiszka@siemens.com, Kevin Cernekee , stefanha@redhat.com, Samuel Thibault Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" From: Kevin Cernekee NULL sockets are used for NDP, BOOTP, and other critical operations. If the topmost mbuf in a NULL session is blocked pending resolution, it may cause problems if it blocks other packets with a NULL socket. So do not add mbufs with a NULL socket field to the same session. Signed-off-by: Kevin Cernekee Signed-off-by: Samuel Thibault --- slirp/if.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/slirp/if.c b/slirp/if.c index 6262d77495..590753c658 100644 --- a/slirp/if.c +++ b/slirp/if.c @@ -73,14 +73,16 @@ if_output(struct socket *so, struct mbuf *ifm) * We mustn't put this packet back on the fastq (or we'll send it out of = order) * XXX add cache here? */ - for (ifq =3D (struct mbuf *) slirp->if_batchq.qh_rlink; - (struct quehead *) ifq !=3D &slirp->if_batchq; - ifq =3D ifq->ifq_prev) { - if (so =3D=3D ifq->ifq_so) { - /* A match! */ - ifm->ifq_so =3D so; - ifs_insque(ifm, ifq->ifs_prev); - goto diddit; + if (so) { + for (ifq =3D (struct mbuf *) slirp->if_batchq.qh_rlink; + (struct quehead *) ifq !=3D &slirp->if_batchq; + ifq =3D ifq->ifq_prev) { + if (so =3D=3D ifq->ifq_so) { + /* A match! */ + ifm->ifq_so =3D so; + ifs_insque(ifm, ifq->ifs_prev); + goto diddit; + } } } =20 --=20 2.14.1