git: b837ead1d46f - stable/14 - tftpd: Address flaky tests

The branch stable/14 has been updated by markj:

URL: https://cgit.FreeBSD.org/src/commit/?id=b837ead1d46ff7ae8f715b18277929d8579496c7

commit b837ead1d46ff7ae8f715b18277929d8579496c7
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2024-11-03 14:43:38 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
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