On Mon, Nov 3, 2025 at 4:08 PM Sunday Adelodun <[email protected]> wrote: > > On 11/2/25 08:32, Kuniyuki Iwashima wrote: > > On Sat, Nov 1, 2025 at 10:23 AM Sunday Adelodun > > <[email protected]> wrote: > >> Add selftests to verify and document Linux’s intended behaviour for > >> UNIX domain sockets (SOCK_STREAM and SOCK_DGRAM) when a peer closes. > >> The tests verify that: > >> > >> 1. SOCK_STREAM returns EOF when the peer closes normally. > >> 2. SOCK_STREAM returns ECONNRESET if the peer closes with unread data. > >> 3. SOCK_SEQPACKET returns EOF when the peer closes normally. > >> 4. SOCK_SEQPACKET returns ECONNRESET if the peer closes with unread data. > >> 5. SOCK_DGRAM does not return ECONNRESET when the peer closes. > >> > >> This follows up on review feedback suggesting a selftest to clarify > >> Linux’s semantics. > >> > >> Suggested-by: Kuniyuki Iwashima <[email protected]> > >> Signed-off-by: Sunday Adelodun <[email protected]> > >> --- > >> tools/testing/selftests/net/af_unix/Makefile | 1 + > >> .../selftests/net/af_unix/unix_connreset.c | 179 ++++++++++++++++++ > >> 2 files changed, 180 insertions(+) > >> create mode 100644 tools/testing/selftests/net/af_unix/unix_connreset.c > >> > >> diff --git a/tools/testing/selftests/net/af_unix/Makefile > >> b/tools/testing/selftests/net/af_unix/Makefile > >> index de805cbbdf69..5826a8372451 100644 > >> --- a/tools/testing/selftests/net/af_unix/Makefile > >> +++ b/tools/testing/selftests/net/af_unix/Makefile > >> @@ -7,6 +7,7 @@ TEST_GEN_PROGS := \ > >> scm_pidfd \ > >> scm_rights \ > >> unix_connect \ > >> + unix_connreset \ > > patchwork caught this test is not added to .gitignore. > > https://patchwork.kernel.org/project/netdevbpf/patch/[email protected]/ > > > > Could you add it to this file ? > > > > tools/testing/selftests/net/.gitignore > Oh, thank you for this. will add it > > > > > >> # end of TEST_GEN_PROGS > >> > >> include ../../lib.mk > >> diff --git a/tools/testing/selftests/net/af_unix/unix_connreset.c > >> b/tools/testing/selftests/net/af_unix/unix_connreset.c > >> new file mode 100644 > >> index 000000000000..6f43435d96e2 > >> --- /dev/null > >> +++ b/tools/testing/selftests/net/af_unix/unix_connreset.c > >> @@ -0,0 +1,179 @@ > >> +// SPDX-License-Identifier: GPL-2.0 > >> +/* > >> + * Selftest for AF_UNIX socket close and ECONNRESET behaviour. > >> + * > >> + * This test verifies: > >> + * 1. SOCK_STREAM returns EOF when the peer closes normally. > >> + * 2. SOCK_STREAM returns ECONNRESET if peer closes with unread data. > >> + * 3. SOCK_SEQPACKET returns EOF when the peer closes normally. > >> + * 4. SOCK_SEQPACKET returns ECONNRESET if the peer closes with unread > >> data. > >> + * 5. SOCK_DGRAM does not return ECONNRESET when the peer closes. > >> + * > >> + * These tests document the intended Linux behaviour. > >> + * > >> + */ > >> + > >> +#define _GNU_SOURCE > >> +#include <stdlib.h> > >> +#include <string.h> > >> +#include <fcntl.h> > >> +#include <unistd.h> > >> +#include <errno.h> > >> +#include <sys/socket.h> > >> +#include <sys/un.h> > >> +#include "../../kselftest_harness.h" > >> + > >> +#define SOCK_PATH "/tmp/af_unix_connreset.sock" > >> + > >> +static void remove_socket_file(void) > >> +{ > >> + unlink(SOCK_PATH); > >> +} > >> + > >> +FIXTURE(unix_sock) > >> +{ > >> + int server; > >> + int client; > >> + int child; > >> +}; > >> + > >> +FIXTURE_VARIANT(unix_sock) > >> +{ > >> + int socket_type; > >> + const char *name; > >> +}; > >> + > >> +/* Define variants: stream and datagram */ > > nit: outdated, maybe simply remove ? > oh..skipped me. > will do so. > > > >> +FIXTURE_VARIANT_ADD(unix_sock, stream) { > >> + .socket_type = SOCK_STREAM, > >> + .name = "SOCK_STREAM", > >> +}; > >> + > >> +FIXTURE_VARIANT_ADD(unix_sock, dgram) { > >> + .socket_type = SOCK_DGRAM, > >> + .name = "SOCK_DGRAM", > >> +}; > >> + > >> +FIXTURE_VARIANT_ADD(unix_sock, seqpacket) { > >> + .socket_type = SOCK_SEQPACKET, > >> + .name = "SOCK_SEQPACKET", > >> +}; > >> + > >> +FIXTURE_SETUP(unix_sock) > >> +{ > >> + struct sockaddr_un addr = {}; > >> + int err; > >> + > >> + addr.sun_family = AF_UNIX; > >> + strcpy(addr.sun_path, SOCK_PATH); > >> + remove_socket_file(); > >> + > >> + self->server = socket(AF_UNIX, variant->socket_type, 0); > >> + ASSERT_LT(-1, self->server); > >> + > >> + err = bind(self->server, (struct sockaddr *)&addr, sizeof(addr)); > >> + ASSERT_EQ(0, err); > >> + > >> + if (variant->socket_type == SOCK_STREAM || > >> + variant->socket_type == SOCK_SEQPACKET) { > > patchwork caught mis-alignment here and other places. > > > > I'm using this for emacs, and other editors will have a similar config. > > > > (setq-default c-default-style "linux") > > > > You can check if lines are aligned properly by > > > > $ git show --format=email | ./scripts/checkpatch.pl > > > > > >> + err = listen(self->server, 1); > >> + ASSERT_EQ(0, err); > >> + > >> + self->client = socket(AF_UNIX, variant->socket_type, 0); > > Could you add SOCK_NONBLOCK here too ? > This is noted > > > >> + ASSERT_LT(-1, self->client); > >> + > >> + err = connect(self->client, (struct sockaddr *)&addr, > >> sizeof(addr)); > >> + ASSERT_EQ(0, err); > >> + > >> + self->child = accept(self->server, NULL, NULL); > >> + ASSERT_LT(-1, self->child); > >> + } else { > >> + /* Datagram: bind and connect only */ > >> + self->client = socket(AF_UNIX, SOCK_DGRAM | SOCK_NONBLOCK, > >> 0); > >> + ASSERT_LT(-1, self->client); > >> + > >> + err = connect(self->client, (struct sockaddr *)&addr, > >> sizeof(addr)); > >> + ASSERT_EQ(0, err); > >> + } > >> +} > >> + > >> +FIXTURE_TEARDOWN(unix_sock) > >> +{ > >> + if (variant->socket_type == SOCK_STREAM || > >> + variant->socket_type == SOCK_SEQPACKET) > >> + close(self->child); > >> + > >> + close(self->client); > >> + close(self->server); > >> + remove_socket_file(); > >> +} > >> + > >> +/* Test 1: peer closes normally */ > >> +TEST_F(unix_sock, eof) > >> +{ > >> + char buf[16] = {}; > >> + ssize_t n; > >> + > >> + /* Peer closes normally */ > >> + if (variant->socket_type == SOCK_STREAM || > >> + variant->socket_type == SOCK_SEQPACKET) > >> + close(self->child); > >> + else > >> + close(self->server); > >> + > >> + n = recv(self->client, buf, sizeof(buf), 0); > >> + TH_LOG("%s: recv=%zd errno=%d (%s)", variant->name, n, errno, > >> strerror(errno)); > > errno is undefined if not set, and same for strerror(errno). > > > > Also, if ASSERT_XXX() below fails, the same information > > (test case, errno) is logged. So, TH_LOG() seems unnecessary. > > > > Maybe try modifying the condition below and see how the > > assertion is logged. > Oh..thank you. Didn't it through that way. > I understand. > I will remove the TH_LOG()'s > > > >> + if (variant->socket_type == SOCK_STREAM || > >> + variant->socket_type == SOCK_SEQPACKET) { > >> + ASSERT_EQ(0, n); > >> + } else { > >> + ASSERT_EQ(-1, n); > >> + ASSERT_EQ(EAGAIN, errno); > >> + } > >> +} > >> + > >> +/* Test 2: peer closes with unread data */ > >> +TEST_F(unix_sock, reset_unread) > >> +{ > >> + char buf[16] = {}; > >> + ssize_t n; > >> + > >> + /* Send data that will remain unread by client */ > >> + send(self->client, "hello", 5, 0); > >> + close(self->child); > >> + > >> + n = recv(self->client, buf, sizeof(buf), 0); > >> + TH_LOG("%s: recv=%zd errno=%d (%s)", variant->name, n, errno, > >> strerror(errno)); > >> + if (variant->socket_type == SOCK_STREAM || > >> + variant->socket_type == SOCK_SEQPACKET) { > >> + ASSERT_EQ(-1, n); > >> + ASSERT_EQ(ECONNRESET, errno); > >> + } else { > >> + ASSERT_EQ(-1, n); > >> + ASSERT_EQ(EAGAIN, errno); > >> + } > >> +} > >> + > >> +/* Test 3: SOCK_DGRAM peer close */ > >> Now Test 2 and Test 3 look identical ;) > > seems so, but the only difference is: > > close(self->child); is used in Test 2, while > close(self->server); is used in Test 3. > Maybe I should find a way to collapse Tests 2 and 3 (if statement might > work) > > I am just afraid the tests to run will reduce to 6 from 9 and we will have 6 > cases passed as against 7 as before. > > What do you think?
The name of Test 2 is a bit confusing, which is not true for SOCK_DGRAM. So, I'd use "if" to change which fd to close() depending on the socket type. Also, close(self->server) does nothing for SOCK_STREAM and SOCK_SEQPACKET after accept(). Rather, that close() should have the same effect if self->child is not accept()ed. (In this case, Skip() for SOCK_DGRAM makes sense) I think covering that scenario would be nicer. If interested, you can check the test coverage with this patch. https://lore.kernel.org/linux-kselftest/[email protected]/ Thanks!
