From nobody Tue Nov 19 14:31:19 2024 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4Xt6PM3lkfz5f5X6; Tue, 19 Nov 2024 14:31:19 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R10" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Xt6PM1jTsz436r; Tue, 19 Nov 2024 14:31:19 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1732026679; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=RWJzV2N86RkVbbUlEKm+TPIW5ifXcRy+31AerWpUmHE=; b=TCX9fdl51BmydykWbJJLM5r/nraroO4Ka1kOpkk+aMjtE/Df5RAknnNBuVfcYNsG7A8516 +tN0TvvhjrsQdWOL2NwMaA4ANJwVylpOdchaYEcijVPdNKvjQ7RiEaBtHCHaTZsTvIP2JP UjIJdlBpmt9unVHrg5wIMFIouAFbCiyNfcd6/LhlwBi2LtMHfbuu41wSERYiK7H2Vow92Q kMhb3QKm8rsCdkYjt9cqY+r9Kg3+XIKDRNx19T6l5Aj5ZlKHVdUOlYP3ZDndhsGwybcQMQ Cm7XrfjOU42AtE1vIcGjdWMZUPgMeCrtnfH0KlU5ADet16swIEaaP8Fj8Hd0ZA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1732026679; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=RWJzV2N86RkVbbUlEKm+TPIW5ifXcRy+31AerWpUmHE=; b=EY1rErY3E17I7M6w05sfCHrJMbr7PjXaCfizOT80ke5Ngz3TNPFVlDtmoPyPiaWLnGta4v oaB11yEPLDRHHxnJj+ISJTrVuAzh2mTi6SUrGF8q9OgAyzav3N383g1zCWctobs9pTlsbN XZgQ/Hx6FqUi7RJt3/Y1CPGbysAklQ/lt9rMZEyXxKSQXgntbIM8FKJtE1mwan7ACdhDe5 icCjgs+b07PcrX5YtysdJQ8PIvuqfWuXKOCB3ltW5ryXD894xPQ1T1rDY3Ycevgu+daM/3 CQfhmZmgb174yYgnxJPeBuIzuxRkynG83Z/cEQVMXiaHEED2IGqh1aEp69o8AA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1732026679; a=rsa-sha256; cv=none; b=WP+o/jK0FjOcVliP3zqeCeBO7R6mw1xo+STu+DHkmixCzXuAw+7LIrc6bOHytfs0ZQuPlZ LCMcUbTOHNZwhhR0rAHG0bFYbzux77SUyrFSX7ZRzV9/konbF6Vnd96AEQzlKjlJDSe46Z mPNRwRJGqih4Cv2y1GRQ8kXSbN7wjRmW3Y3Q2JtrcNy9kkRHNogSQXl+Vf+63q394L+CWX Vvxh6V/u85dy8dqfD6CrjAW3c+IBdR6V+aQNdUJYAl3HGDib4HMMYJ4lhZVfSTsNYJrPo7 Iu6IDOhrXAOoAyJ7sSRfI9I38CQPA5yUxlPP+RV4El4L0ezTIifk2Lyt0W8yKg== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4Xt6PM10NSzDdC; Tue, 19 Nov 2024 14:31:19 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.18.1/8.18.1) with ESMTP id 4AJEVJug040821; Tue, 19 Nov 2024 14:31:19 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 4AJEVJ65040818; Tue, 19 Nov 2024 14:31:19 GMT (envelope-from git) Date: Tue, 19 Nov 2024 14:31:19 GMT Message-Id: <202411191431.4AJEVJ65040818@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Mark Johnston Subject: git: b837ead1d46f - stable/14 - tftpd: Address flaky tests List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: markj X-Git-Repository: src X-Git-Refname: refs/heads/stable/14 X-Git-Reftype: branch X-Git-Commit: b837ead1d46ff7ae8f715b18277929d8579496c7 Auto-Submitted: auto-generated The branch stable/14 has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=b837ead1d46ff7ae8f715b18277929d8579496c7 commit b837ead1d46ff7ae8f715b18277929d8579496c7 Author: Mark Johnston AuthorDate: 2024-11-03 14:43:38 +0000 Commit: Mark Johnston CommitDate: 2024-11-19 14:15:06 +0000 tftpd: Address flaky tests The tftpd tests all follow the same pattern: 1. open a UDP socket, 2. fork a child to exec tftpd, which subsequently handles requests on the socket, 3. use a client socket to send some message to the tftpd daemon. However, tftpd's first action is to mark its socket as non-blocking and then read a request from it. If no data is present in the socket, tftpd exits immediately with an error. So, there is a race; we often see tftpd test timeouts when running tests in parallel. These timeouts also arise periodically in CI runs. One solution is to restructure each test to create the server socket, then write the request to the client socket, then fork tftpd. This closes the race. However, this involves a lot of churn. This patch fixes the problem a different way, by adding a new -b flag to tftpd which makes it block to read the initial request. Each test is modified to use -b, closing the race. Reviewed by: imp, asomers MFC after: 2 weeks Differential Revision: https://reviews.freebsd.org/D47404 (cherry picked from commit 79c342aaf86feb4efbd15383f54e4fe7bdc9da7b) --- libexec/tftpd/tests/functional.c | 2 ++ libexec/tftpd/tftpd.8 | 14 ++++++++++++-- libexec/tftpd/tftpd.c | 19 +++++++++++-------- 3 files changed, 25 insertions(+), 10 deletions(-) diff --git a/libexec/tftpd/tests/functional.c b/libexec/tftpd/tests/functional.c index 32e5f85cf421..3194f266faa5 100644 --- a/libexec/tftpd/tests/functional.c +++ b/libexec/tftpd/tests/functional.c @@ -324,6 +324,7 @@ setup(struct sockaddr_storage *to, uint16_t idx) { int client_s, server_s, pid, argv_idx; char execname[] = "/usr/libexec/tftpd"; + char b_flag_str[] = "-b"; char s_flag_str[] = "-s"; char w_flag_str[] = "-w"; char pwd[MAXPATHLEN]; @@ -383,6 +384,7 @@ setup(struct sockaddr_storage *to, uint16_t idx) bzero(argv, sizeof(argv)); argv[0] = execname; argv_idx = 1; + argv[argv_idx++] = b_flag_str; if (w_flag) argv[argv_idx++] = w_flag_str; if (s_flag) diff --git a/libexec/tftpd/tftpd.8 b/libexec/tftpd/tftpd.8 index 24a743a92e14..1cbbe6975944 100644 --- a/libexec/tftpd/tftpd.8 +++ b/libexec/tftpd/tftpd.8 @@ -27,7 +27,7 @@ .\" .\" @(#)tftpd.8 8.1 (Berkeley) 6/4/93 .\" -.Dd May 8, 2024 +.Dd November 3, 2024 .Dt TFTPD 8 .Os .Sh NAME @@ -35,7 +35,7 @@ .Nd Internet Trivial File Transfer Protocol server .Sh SYNOPSIS .Nm tftpd -.Op Fl CcdlnoSw +.Op Fl bCcdlnoSw .Op Fl F Ar strftime-format .Op Fl s Ar directory .Op Fl U Ar umask @@ -121,6 +121,16 @@ option is specified. .Pp The options are: .Bl -tag -width Ds +.It Fl b +By default, +.Nm +expects an initial message to be available on its input socket. +If no data is available, the server exits immediately. +If +.Fl b +is specified, +.Nm +will block waiting for the initial message. .It Fl c Changes the default root directory of a connecting host via .Xr chroot 2 diff --git a/libexec/tftpd/tftpd.c b/libexec/tftpd/tftpd.c index cef8d0278a55..81bc79fc2c12 100644 --- a/libexec/tftpd/tftpd.c +++ b/libexec/tftpd/tftpd.c @@ -133,15 +133,18 @@ main(int argc, char *argv[]) struct passwd *nobody; const char *chuser = "nobody"; char recvbuffer[MAXPKTSIZE]; - int allow_ro = 1, allow_wo = 1, on = 1; + int allow_ro = 1, allow_wo = 1, block = 0, on = 1; pid_t pid; tzset(); /* syslog in localtime */ acting_as_client = 0; tftp_openlog("tftpd", LOG_PID | LOG_NDELAY, LOG_FTP); - while ((ch = getopt(argc, argv, "cCd::F:lnoOp:s:Su:U:wW")) != -1) { + while ((ch = getopt(argc, argv, "bcCd::F:lnoOp:s:Su:U:wW")) != -1) { switch (ch) { + case 'b': + block = 1; + break; case 'c': ipchroot = 1; break; @@ -225,14 +228,9 @@ main(int argc, char *argv[]) umask(mask); - if (ioctl(0, FIONBIO, &on) < 0) { - tftp_log(LOG_ERR, "ioctl(FIONBIO): %s", strerror(errno)); - exit(1); - } - /* Find out who we are talking to and what we are going to do */ peerlen = sizeof(peer_sock); - n = recvfrom(0, recvbuffer, MAXPKTSIZE, 0, + n = recvfrom(0, recvbuffer, MAXPKTSIZE, block ? 0 : MSG_DONTWAIT, (struct sockaddr *)&peer_sock, &peerlen); if (n < 0) { tftp_log(LOG_ERR, "recvfrom: %s", strerror(errno)); @@ -246,6 +244,11 @@ main(int argc, char *argv[]) exit(1); } + if (ioctl(0, FIONBIO, &on) < 0) { + tftp_log(LOG_ERR, "ioctl(FIONBIO): %s", strerror(errno)); + exit(1); + } + /* * Now that we have read the message out of the UDP * socket, we fork and exit. Thus, inetd will go back