mirror of
https://github.com/freebsd/freebsd-src.git
synced 2024-11-26 20:12:44 +00:00
tftpd: Address flaky tests
Some checks are pending
Cross-build Kernel / ${{ matrix.target_arch }} ${{ matrix.os }} (${{ matrix.compiler }}) (clang-14, /usr/lib/llvm-14/bin, ubuntu-22.04, bmake libarchive-dev clang-14 lld-14, amd64, amd64) (push) Waiting to run
Cross-build Kernel / ${{ matrix.target_arch }} ${{ matrix.os }} (${{ matrix.compiler }}) (clang-14, /usr/lib/llvm-14/bin, ubuntu-22.04, bmake libarchive-dev clang-14 lld-14, arm64, aarch64) (push) Waiting to run
Cross-build Kernel / ${{ matrix.target_arch }} ${{ matrix.os }} (${{ matrix.compiler }}) (clang-18, /opt/homebrew/opt/llvm@18/bin, macos-latest, bmake libarchive llvm@18, amd64, amd64) (push) Waiting to run
Cross-build Kernel / ${{ matrix.target_arch }} ${{ matrix.os }} (${{ matrix.compiler }}) (clang-18, /opt/homebrew/opt/llvm@18/bin, macos-latest, bmake libarchive llvm@18, arm64, aarch64) (push) Waiting to run
Cross-build Kernel / ${{ matrix.target_arch }} ${{ matrix.os }} (${{ matrix.compiler }}) (clang-18, /usr/lib/llvm-18/bin, ubuntu-24.04, bmake libarchive-dev clang-18 lld-18, amd64, amd64) (push) Waiting to run
Cross-build Kernel / ${{ matrix.target_arch }} ${{ matrix.os }} (${{ matrix.compiler }}) (clang-18, /usr/lib/llvm-18/bin, ubuntu-24.04, bmake libarchive-dev clang-18 lld-18, arm64, aarch64) (push) Waiting to run
Some checks are pending
Cross-build Kernel / ${{ matrix.target_arch }} ${{ matrix.os }} (${{ matrix.compiler }}) (clang-14, /usr/lib/llvm-14/bin, ubuntu-22.04, bmake libarchive-dev clang-14 lld-14, amd64, amd64) (push) Waiting to run
Cross-build Kernel / ${{ matrix.target_arch }} ${{ matrix.os }} (${{ matrix.compiler }}) (clang-14, /usr/lib/llvm-14/bin, ubuntu-22.04, bmake libarchive-dev clang-14 lld-14, arm64, aarch64) (push) Waiting to run
Cross-build Kernel / ${{ matrix.target_arch }} ${{ matrix.os }} (${{ matrix.compiler }}) (clang-18, /opt/homebrew/opt/llvm@18/bin, macos-latest, bmake libarchive llvm@18, amd64, amd64) (push) Waiting to run
Cross-build Kernel / ${{ matrix.target_arch }} ${{ matrix.os }} (${{ matrix.compiler }}) (clang-18, /opt/homebrew/opt/llvm@18/bin, macos-latest, bmake libarchive llvm@18, arm64, aarch64) (push) Waiting to run
Cross-build Kernel / ${{ matrix.target_arch }} ${{ matrix.os }} (${{ matrix.compiler }}) (clang-18, /usr/lib/llvm-18/bin, ubuntu-24.04, bmake libarchive-dev clang-18 lld-18, amd64, amd64) (push) Waiting to run
Cross-build Kernel / ${{ matrix.target_arch }} ${{ matrix.os }} (${{ matrix.compiler }}) (clang-18, /usr/lib/llvm-18/bin, ubuntu-24.04, bmake libarchive-dev clang-18 lld-18, arm64, aarch64) (push) Waiting to run
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
This commit is contained in:
parent
2b8c3a05e0
commit
79c342aaf8
@ -323,6 +323,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];
|
||||
@ -382,6 +383,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)
|
||||
|
@ -25,7 +25,7 @@
|
||||
.\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
|
||||
.\" SUCH DAMAGE.
|
||||
.\"
|
||||
.Dd May 8, 2024
|
||||
.Dd November 3, 2024
|
||||
.Dt TFTPD 8
|
||||
.Os
|
||||
.Sh NAME
|
||||
@ -33,7 +33,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
|
||||
@ -119,6 +119,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
|
||||
|
@ -121,15 +121,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;
|
||||
@ -213,14 +216,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));
|
||||
@ -234,6 +232,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
|
||||
|
Loading…
Reference in New Issue
Block a user