* [PATCH v2] selftests: af_unix: Add tests for ECONNRESET and EOF semantics [not found] <20251025190256.11352-1-adelodunolaoluwa.ref@yahoo.com> @ 2025-10-25 19:02 ` Sunday Adelodun 2025-10-28 18:28 ` Kuniyuki Iwashima 0 siblings, 1 reply; 5+ messages in thread From: Sunday Adelodun @ 2025-10-25 19:02 UTC (permalink / raw) To: Kuniyuki Iwashima, =David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, Shuah Khan Cc: Shuah Khan, David Hunter, linux-kernel, netdev, linux-kselftest, linux-kernel-mentees, Sunday Adelodun 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 cover: 1. EOF returned when a SOCK_STREAM peer closes normally. 2. ECONNRESET returned when a SOCK_STREAM peer closes with unread data. 3. SOCK_DGRAM sockets not returning ECONNRESET on peer close. This follows up on review feedback suggesting a selftest to clarify Linux’s semantics. Suggested-by: Kuniyuki Iwashima <kuniyu@google.com> Signed-off-by: Sunday Adelodun <adelodunolaoluwa@yahoo.com> --- Changelog: Changes made from v1: - Patch prefix updated to selftest: af_unix:. - All mentions of “UNIX” changed to AF_UNIX. - Removed BSD references from comments. - Shared setup refactored using FIXTURE_VARIANT(). - Cleanup moved to FIXTURE_TEARDOWN() to always run. - Tests consolidated to reduce duplication: EOF, ECONNRESET, SOCK_DGRAM peer close. - Corrected ASSERT usage and initialization style. - Makefile updated for new directory af_unix. tools/testing/selftests/net/af_unix/Makefile | 1 + .../selftests/net/af_unix/unix_connreset.c | 161 ++++++++++++++++++ 2 files changed, 162 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 \ # 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..c65ec997d77d --- /dev/null +++ b/tools/testing/selftests/net/af_unix/unix_connreset.c @@ -0,0 +1,161 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Selftest for AF_UNIX socket close and ECONNRESET behaviour. + * + * This test verifies that: + * 1. SOCK_STREAM sockets return EOF when peer closes normally. + * 2. SOCK_STREAM sockets return ECONNRESET if peer closes with unread data. + * 3. SOCK_DGRAM sockets do not return ECONNRESET when 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 */ +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_SETUP(unix_sock) +{ + struct sockaddr_un addr = {}; + int err; + + addr.sun_family = AF_UNIX; + strcpy(addr.sun_path, SOCK_PATH); + + 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) { + err = listen(self->server, 1); + ASSERT_EQ(0, err); + + self->client = socket(AF_UNIX, SOCK_STREAM, 0); + 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) + 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; + + if (variant->socket_type != SOCK_STREAM) + SKIP(return, "This test only applies to SOCK_STREAM"); + + /* Peer closes normally */ + 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 (n == -1) + ASSERT_EQ(ECONNRESET, errno); + + if (n != -1) + ASSERT_EQ(0, n); +} + +/* Test 2: peer closes with unread data */ +TEST_F(unix_sock, reset_unread) +{ + char buf[16] = {}; + ssize_t n; + + if (variant->socket_type != SOCK_STREAM) + SKIP(return, "This test only applies to SOCK_STREAM"); + + /* 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)); + ASSERT_EQ(-1, n); + ASSERT_EQ(ECONNRESET, errno); +} + +/* Test 3: SOCK_DGRAM peer close */ +TEST_F(unix_sock, dgram_reset) +{ + char buf[16] = {}; + ssize_t n; + + if (variant->socket_type != SOCK_DGRAM) + SKIP(return, "This test only applies to SOCK_DGRAM"); + + send(self->client, "hello", 5, 0); + 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)); + /* Expect EAGAIN because there is no datagram and peer is closed. */ + ASSERT_EQ(-1, n); + ASSERT_EQ(EAGAIN, errno); +} + +TEST_HARNESS_MAIN + -- 2.43.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] selftests: af_unix: Add tests for ECONNRESET and EOF semantics 2025-10-25 19:02 ` [PATCH v2] selftests: af_unix: Add tests for ECONNRESET and EOF semantics Sunday Adelodun @ 2025-10-28 18:28 ` Kuniyuki Iwashima 2025-10-30 13:45 ` Sunday Adelodun 0 siblings, 1 reply; 5+ messages in thread From: Kuniyuki Iwashima @ 2025-10-28 18:28 UTC (permalink / raw) To: Sunday Adelodun Cc: =David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, Shuah Khan, Shuah Khan, David Hunter, linux-kernel, netdev, linux-kselftest, linux-kernel-mentees On Sat, Oct 25, 2025 at 12:03 PM Sunday Adelodun <adelodunolaoluwa@yahoo.com> 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 cover: > > 1. EOF returned when a SOCK_STREAM peer closes normally. > 2. ECONNRESET returned when a SOCK_STREAM peer closes with unread data. > 3. SOCK_DGRAM sockets not returning ECONNRESET on peer close. > > This follows up on review feedback suggesting a selftest to clarify > Linux’s semantics. > > Suggested-by: Kuniyuki Iwashima <kuniyu@google.com> > Signed-off-by: Sunday Adelodun <adelodunolaoluwa@yahoo.com> > --- > Changelog: > > Changes made from v1: > > - Patch prefix updated to selftest: af_unix:. > > - All mentions of “UNIX” changed to AF_UNIX. > > - Removed BSD references from comments. > > - Shared setup refactored using FIXTURE_VARIANT(). > > - Cleanup moved to FIXTURE_TEARDOWN() to always run. > > - Tests consolidated to reduce duplication: EOF, ECONNRESET, SOCK_DGRAM peer close. > > - Corrected ASSERT usage and initialization style. > > - Makefile updated for new directory af_unix. > > tools/testing/selftests/net/af_unix/Makefile | 1 + > .../selftests/net/af_unix/unix_connreset.c | 161 ++++++++++++++++++ > 2 files changed, 162 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 \ > # 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..c65ec997d77d > --- /dev/null > +++ b/tools/testing/selftests/net/af_unix/unix_connreset.c > @@ -0,0 +1,161 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Selftest for AF_UNIX socket close and ECONNRESET behaviour. > + * > + * This test verifies that: > + * 1. SOCK_STREAM sockets return EOF when peer closes normally. > + * 2. SOCK_STREAM sockets return ECONNRESET if peer closes with unread data. > + * 3. SOCK_DGRAM sockets do not return ECONNRESET when 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 */ > +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", > +}; Let's add coverage for SOCK_SEQPACKET, which needs listen() / connect() but other semantics are similar to SOCK_DGRAM. > + > +FIXTURE_SETUP(unix_sock) > +{ > + struct sockaddr_un addr = {}; > + int err; > + > + addr.sun_family = AF_UNIX; > + strcpy(addr.sun_path, SOCK_PATH); > + > + 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) { > + err = listen(self->server, 1); > + ASSERT_EQ(0, err); > + > + self->client = socket(AF_UNIX, SOCK_STREAM, 0); > + 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) > + 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; > + > + if (variant->socket_type != SOCK_STREAM) > + SKIP(return, "This test only applies to SOCK_STREAM"); Instead of skipping, let's define final ASSERT() results for each type. Same for other 2 tests. > + > + /* Peer closes normally */ > + 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 (n == -1) > + ASSERT_EQ(ECONNRESET, errno); ... otherwise, we don't see an error here > + > + if (n != -1) > + ASSERT_EQ(0, n); and this can be checked unconditionally. > +} > + > +/* Test 2: peer closes with unread data */ > +TEST_F(unix_sock, reset_unread) > +{ > + char buf[16] = {}; > + ssize_t n; > + > + if (variant->socket_type != SOCK_STREAM) > + SKIP(return, "This test only applies to SOCK_STREAM"); > + > + /* 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)); > + ASSERT_EQ(-1, n); > + ASSERT_EQ(ECONNRESET, errno); > +} > + > +/* Test 3: SOCK_DGRAM peer close */ > +TEST_F(unix_sock, dgram_reset) > +{ > + char buf[16] = {}; > + ssize_t n; > + > + if (variant->socket_type != SOCK_DGRAM) > + SKIP(return, "This test only applies to SOCK_DGRAM"); > + > + send(self->client, "hello", 5, 0); > + 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)); > + /* Expect EAGAIN because there is no datagram and peer is closed. */ > + ASSERT_EQ(-1, n); > + ASSERT_EQ(EAGAIN, errno); > +} > + > +TEST_HARNESS_MAIN > + > -- > 2.43.0 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] selftests: af_unix: Add tests for ECONNRESET and EOF semantics 2025-10-28 18:28 ` Kuniyuki Iwashima @ 2025-10-30 13:45 ` Sunday Adelodun 2025-10-31 19:41 ` Kuniyuki Iwashima 0 siblings, 1 reply; 5+ messages in thread From: Sunday Adelodun @ 2025-10-30 13:45 UTC (permalink / raw) To: Kuniyuki Iwashima Cc: =David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, Shuah Khan, Shuah Khan, David Hunter, linux-kernel, netdev, linux-kselftest, linux-kernel-mentees On 10/28/25 19:28, Kuniyuki Iwashima wrote: > On Sat, Oct 25, 2025 at 12:03 PM Sunday Adelodun > <adelodunolaoluwa@yahoo.com> 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 cover: >> >> 1. EOF returned when a SOCK_STREAM peer closes normally. >> 2. ECONNRESET returned when a SOCK_STREAM peer closes with unread data. >> 3. SOCK_DGRAM sockets not returning ECONNRESET on peer close. >> >> This follows up on review feedback suggesting a selftest to clarify >> Linux’s semantics. >> >> Suggested-by: Kuniyuki Iwashima <kuniyu@google.com> >> Signed-off-by: Sunday Adelodun <adelodunolaoluwa@yahoo.com> >> --- >> Changelog: >> >> Changes made from v1: >> >> - Patch prefix updated to selftest: af_unix:. >> >> - All mentions of “UNIX” changed to AF_UNIX. >> >> - Removed BSD references from comments. >> >> - Shared setup refactored using FIXTURE_VARIANT(). >> >> - Cleanup moved to FIXTURE_TEARDOWN() to always run. >> >> - Tests consolidated to reduce duplication: EOF, ECONNRESET, SOCK_DGRAM peer close. >> >> - Corrected ASSERT usage and initialization style. >> >> - Makefile updated for new directory af_unix. >> >> tools/testing/selftests/net/af_unix/Makefile | 1 + >> .../selftests/net/af_unix/unix_connreset.c | 161 ++++++++++++++++++ >> 2 files changed, 162 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 \ >> # 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..c65ec997d77d >> --- /dev/null >> +++ b/tools/testing/selftests/net/af_unix/unix_connreset.c >> @@ -0,0 +1,161 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Selftest for AF_UNIX socket close and ECONNRESET behaviour. >> + * >> + * This test verifies that: >> + * 1. SOCK_STREAM sockets return EOF when peer closes normally. >> + * 2. SOCK_STREAM sockets return ECONNRESET if peer closes with unread data. >> + * 3. SOCK_DGRAM sockets do not return ECONNRESET when 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 */ >> +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", >> +}; > Let's add coverage for SOCK_SEQPACKET, > which needs listen() / connect() but other semantics > are similar to SOCK_DGRAM. I will add it through: if (variant->socket_type == SOCK_STREAM || variant->socket_type == SOCK_SEQPACKET) in both the setup and teardown fixtures with a little bit of modification where necessary (especially in the setup fixture). And also the fixture_variant_add macro. >> + >> +FIXTURE_SETUP(unix_sock) >> +{ >> + struct sockaddr_un addr = {}; >> + int err; >> + >> + addr.sun_family = AF_UNIX; >> + strcpy(addr.sun_path, SOCK_PATH); >> + >> + 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) { >> + err = listen(self->server, 1); >> + ASSERT_EQ(0, err); >> + >> + self->client = socket(AF_UNIX, SOCK_STREAM, 0); >> + 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) >> + 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; >> + >> + if (variant->socket_type != SOCK_STREAM) >> + SKIP(return, "This test only applies to SOCK_STREAM"); > Instead of skipping, let's define final ASSERT() results > for each type. > > Same for other 2 tests. can I use a switch statement in all the tests? say, for example test1: ... switch (variant->socket_type) { case SOCK_STREAM: case SOCK_SEQPACKET: ASSERT_EQ(0, n); case SOCK_DGRAM: ASSERT(-1, n); ASSERT_EQ(EAGAIN, errno); break; } ... test2: ... switch (variant->socket_type) { case SOCK_STREAM: case SOCK_SEQPACKET: ASSERT_EQ(-1, n); ASSERT_EQ(ECONNRESET, errno); break; case SOCK_DGRAM: ASSERT(-1, n); ASSERT_EQ(EAGAIN, errno); break; } ... test 3: ... switch (variant->socket_type) { case SOCK_STREAM: case SOCK_SEQPACKET: ASSERT_EQ(-1, n); ASSERT_EQ(ECONNRESET, errno); break; case SOCK_DGRAM: ASSERT(-1, n); ASSERT_EQ(EAGAIN, errno); break; } ... if not these, could you kindly shed more light to what you meant > > >> + >> + /* Peer closes normally */ >> + 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 (n == -1) >> + ASSERT_EQ(ECONNRESET, errno); > ... otherwise, we don't see an error here > >> + >> + if (n != -1) >> + ASSERT_EQ(0, n); > and this can be checked unconditionally. did you mean I should remove the if (n != -1) ASSERT_EQ(0, n); part? > >> +} >> + >> +/* Test 2: peer closes with unread data */ >> +TEST_F(unix_sock, reset_unread) >> +{ >> + char buf[16] = {}; >> + ssize_t n; >> + >> + if (variant->socket_type != SOCK_STREAM) >> + SKIP(return, "This test only applies to SOCK_STREAM"); >> + >> + /* 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)); >> + ASSERT_EQ(-1, n); >> + ASSERT_EQ(ECONNRESET, errno); >> +} >> + >> +/* Test 3: SOCK_DGRAM peer close */ >> +TEST_F(unix_sock, dgram_reset) >> +{ >> + char buf[16] = {}; >> + ssize_t n; >> + >> + if (variant->socket_type != SOCK_DGRAM) >> + SKIP(return, "This test only applies to SOCK_DGRAM"); >> + >> + send(self->client, "hello", 5, 0); >> + 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)); >> + /* Expect EAGAIN because there is no datagram and peer is closed. */ >> + ASSERT_EQ(-1, n); >> + ASSERT_EQ(EAGAIN, errno); >> +} >> + >> +TEST_HARNESS_MAIN >> + >> -- >> 2.43.0 >> Thank you very much for the review and comments. I will send v3 after you review the questions/clarifications I asked in my previous reply. Apologies for the delay in responding. Thanks again for the guidance and patience, really appreciate it. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] selftests: af_unix: Add tests for ECONNRESET and EOF semantics 2025-10-30 13:45 ` Sunday Adelodun @ 2025-10-31 19:41 ` Kuniyuki Iwashima 2025-10-31 20:01 ` Sunday Adelodun 0 siblings, 1 reply; 5+ messages in thread From: Kuniyuki Iwashima @ 2025-10-31 19:41 UTC (permalink / raw) To: Sunday Adelodun Cc: =David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, Shuah Khan, Shuah Khan, David Hunter, linux-kernel, netdev, linux-kselftest, linux-kernel-mentees On Thu, Oct 30, 2025 at 6:45 AM Sunday Adelodun <adelodunolaoluwa@yahoo.com> wrote: > > On 10/28/25 19:28, Kuniyuki Iwashima wrote: > > On Sat, Oct 25, 2025 at 12:03 PM Sunday Adelodun > > <adelodunolaoluwa@yahoo.com> 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 cover: > >> > >> 1. EOF returned when a SOCK_STREAM peer closes normally. > >> 2. ECONNRESET returned when a SOCK_STREAM peer closes with unread data. > >> 3. SOCK_DGRAM sockets not returning ECONNRESET on peer close. > >> > >> This follows up on review feedback suggesting a selftest to clarify > >> Linux’s semantics. > >> > >> Suggested-by: Kuniyuki Iwashima <kuniyu@google.com> > >> Signed-off-by: Sunday Adelodun <adelodunolaoluwa@yahoo.com> > >> --- > >> Changelog: > >> > >> Changes made from v1: > >> > >> - Patch prefix updated to selftest: af_unix:. > >> > >> - All mentions of “UNIX” changed to AF_UNIX. > >> > >> - Removed BSD references from comments. > >> > >> - Shared setup refactored using FIXTURE_VARIANT(). > >> > >> - Cleanup moved to FIXTURE_TEARDOWN() to always run. > >> > >> - Tests consolidated to reduce duplication: EOF, ECONNRESET, SOCK_DGRAM peer close. > >> > >> - Corrected ASSERT usage and initialization style. > >> > >> - Makefile updated for new directory af_unix. > >> > >> tools/testing/selftests/net/af_unix/Makefile | 1 + > >> .../selftests/net/af_unix/unix_connreset.c | 161 ++++++++++++++++++ > >> 2 files changed, 162 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 \ > >> # 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..c65ec997d77d > >> --- /dev/null > >> +++ b/tools/testing/selftests/net/af_unix/unix_connreset.c > >> @@ -0,0 +1,161 @@ > >> +// SPDX-License-Identifier: GPL-2.0 > >> +/* > >> + * Selftest for AF_UNIX socket close and ECONNRESET behaviour. > >> + * > >> + * This test verifies that: > >> + * 1. SOCK_STREAM sockets return EOF when peer closes normally. > >> + * 2. SOCK_STREAM sockets return ECONNRESET if peer closes with unread data. > >> + * 3. SOCK_DGRAM sockets do not return ECONNRESET when 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 */ > >> +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", > >> +}; > > Let's add coverage for SOCK_SEQPACKET, > > which needs listen() / connect() but other semantics > > are similar to SOCK_DGRAM. > > I will add it through: > if (variant->socket_type == SOCK_STREAM || > variant->socket_type == SOCK_SEQPACKET) > > > in both the setup and teardown fixtures with a little bit of modification > > where necessary (especially in the setup fixture). > > And also the fixture_variant_add macro. > > >> + > >> +FIXTURE_SETUP(unix_sock) > >> +{ > >> + struct sockaddr_un addr = {}; > >> + int err; > >> + > >> + addr.sun_family = AF_UNIX; > >> + strcpy(addr.sun_path, SOCK_PATH); > >> + > >> + 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) { > >> + err = listen(self->server, 1); > >> + ASSERT_EQ(0, err); > >> + > >> + self->client = socket(AF_UNIX, SOCK_STREAM, 0); > >> + 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) > >> + 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; > >> + > >> + if (variant->socket_type != SOCK_STREAM) > >> + SKIP(return, "This test only applies to SOCK_STREAM"); > > Instead of skipping, let's define final ASSERT() results > > for each type. > > > > Same for other 2 tests. > > can I use a switch statement in all the tests? say, for example switch() is completely fine, but I guess "if" will be shorter :) > > test1: > > ... > > switch (variant->socket_type) { > > case SOCK_STREAM: > > case SOCK_SEQPACKET: > > ASSERT_EQ(0, n); You need break; here. > > case SOCK_DGRAM: > > ASSERT(-1, n); > > ASSERT_EQ(EAGAIN, errno); > > break; And also please make sure the compiler will not complain without default: depending on inherited build options. > > } > > ... > > test2: > > ... > > switch (variant->socket_type) { > > case SOCK_STREAM: > > case SOCK_SEQPACKET: > > ASSERT_EQ(-1, n); > > ASSERT_EQ(ECONNRESET, errno); > > break; > > case SOCK_DGRAM: > > ASSERT(-1, n); > > ASSERT_EQ(EAGAIN, errno); > > break; > > } > ... > > test 3: > > ... > > switch (variant->socket_type) { > > case SOCK_STREAM: > > case SOCK_SEQPACKET: > > ASSERT_EQ(-1, n); > > ASSERT_EQ(ECONNRESET, errno); > > break; > > case SOCK_DGRAM: > > ASSERT(-1, n); > > ASSERT_EQ(EAGAIN, errno); > > break; > > } > > ... > > if not these, could you kindly shed more light to what you meant > > > > > > >> + > >> + /* Peer closes normally */ > >> + 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 (n == -1) > >> + ASSERT_EQ(ECONNRESET, errno); > > ... otherwise, we don't see an error here > > > >> + > >> + if (n != -1) > >> + ASSERT_EQ(0, n); > > and this can be checked unconditionally. > did you mean I should remove the if (n != -1) ASSERT_EQ(0, n); part? If SOCK_DGRAM does not reuse this test, yes. The point is we do not want to miss future regression by preparing both if (n == -1) case and if (n == 0) case, one of which should never happen at this point. Thanks! ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] selftests: af_unix: Add tests for ECONNRESET and EOF semantics 2025-10-31 19:41 ` Kuniyuki Iwashima @ 2025-10-31 20:01 ` Sunday Adelodun 0 siblings, 0 replies; 5+ messages in thread From: Sunday Adelodun @ 2025-10-31 20:01 UTC (permalink / raw) To: Kuniyuki Iwashima Cc: =David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, Shuah Khan, Shuah Khan, David Hunter, linux-kernel, netdev, linux-kselftest, linux-kernel-mentees On 10/31/25 20:41, Kuniyuki Iwashima wrote: > On Thu, Oct 30, 2025 at 6:45 AM Sunday Adelodun > <adelodunolaoluwa@yahoo.com> wrote: >> On 10/28/25 19:28, Kuniyuki Iwashima wrote: >>> On Sat, Oct 25, 2025 at 12:03 PM Sunday Adelodun >>> <adelodunolaoluwa@yahoo.com> 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 cover: >>>> >>>> 1. EOF returned when a SOCK_STREAM peer closes normally. >>>> 2. ECONNRESET returned when a SOCK_STREAM peer closes with unread data. >>>> 3. SOCK_DGRAM sockets not returning ECONNRESET on peer close. >>>> >>>> This follows up on review feedback suggesting a selftest to clarify >>>> Linux’s semantics. >>>> >>>> Suggested-by: Kuniyuki Iwashima <kuniyu@google.com> >>>> Signed-off-by: Sunday Adelodun <adelodunolaoluwa@yahoo.com> >>>> --- >>>> Changelog: >>>> >>>> Changes made from v1: >>>> >>>> - Patch prefix updated to selftest: af_unix:. >>>> >>>> - All mentions of “UNIX” changed to AF_UNIX. >>>> >>>> - Removed BSD references from comments. >>>> >>>> - Shared setup refactored using FIXTURE_VARIANT(). >>>> >>>> - Cleanup moved to FIXTURE_TEARDOWN() to always run. >>>> >>>> - Tests consolidated to reduce duplication: EOF, ECONNRESET, SOCK_DGRAM peer close. >>>> >>>> - Corrected ASSERT usage and initialization style. >>>> >>>> - Makefile updated for new directory af_unix. >>>> >>>> tools/testing/selftests/net/af_unix/Makefile | 1 + >>>> .../selftests/net/af_unix/unix_connreset.c | 161 ++++++++++++++++++ >>>> 2 files changed, 162 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 \ >>>> # 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..c65ec997d77d >>>> --- /dev/null >>>> +++ b/tools/testing/selftests/net/af_unix/unix_connreset.c >>>> @@ -0,0 +1,161 @@ >>>> +// SPDX-License-Identifier: GPL-2.0 >>>> +/* >>>> + * Selftest for AF_UNIX socket close and ECONNRESET behaviour. >>>> + * >>>> + * This test verifies that: >>>> + * 1. SOCK_STREAM sockets return EOF when peer closes normally. >>>> + * 2. SOCK_STREAM sockets return ECONNRESET if peer closes with unread data. >>>> + * 3. SOCK_DGRAM sockets do not return ECONNRESET when 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 */ >>>> +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", >>>> +}; >>> Let's add coverage for SOCK_SEQPACKET, >>> which needs listen() / connect() but other semantics >>> are similar to SOCK_DGRAM. >> I will add it through: >> if (variant->socket_type == SOCK_STREAM || >> variant->socket_type == SOCK_SEQPACKET) >> >> >> in both the setup and teardown fixtures with a little bit of modification >> >> where necessary (especially in the setup fixture). >> >> And also the fixture_variant_add macro. >> >>>> + >>>> +FIXTURE_SETUP(unix_sock) >>>> +{ >>>> + struct sockaddr_un addr = {}; >>>> + int err; >>>> + >>>> + addr.sun_family = AF_UNIX; >>>> + strcpy(addr.sun_path, SOCK_PATH); >>>> + >>>> + 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) { >>>> + err = listen(self->server, 1); >>>> + ASSERT_EQ(0, err); >>>> + >>>> + self->client = socket(AF_UNIX, SOCK_STREAM, 0); >>>> + 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) >>>> + 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; >>>> + >>>> + if (variant->socket_type != SOCK_STREAM) >>>> + SKIP(return, "This test only applies to SOCK_STREAM"); >>> Instead of skipping, let's define final ASSERT() results >>> for each type. >>> >>> Same for other 2 tests. >> can I use a switch statement in all the tests? say, for example > switch() is completely fine, but I guess "if" will be shorter :) I will go for if then. > >> test1: >> >> ... >> >> switch (variant->socket_type) { >> >> case SOCK_STREAM: >> >> case SOCK_SEQPACKET: >> >> ASSERT_EQ(0, n); > You need break; here. Thank you. It was an omission > >> case SOCK_DGRAM: >> >> ASSERT(-1, n); >> >> ASSERT_EQ(EAGAIN, errno); >> >> break; > And also please make sure the compiler will not complain > without default: depending on inherited build options. I will look into this > >> } >> >> ... >> >> test2: >> >> ... >> >> switch (variant->socket_type) { >> >> case SOCK_STREAM: >> >> case SOCK_SEQPACKET: >> >> ASSERT_EQ(-1, n); >> >> ASSERT_EQ(ECONNRESET, errno); >> >> break; >> >> case SOCK_DGRAM: >> >> ASSERT(-1, n); >> >> ASSERT_EQ(EAGAIN, errno); >> >> break; >> >> } >> ... >> >> test 3: >> >> ... >> >> switch (variant->socket_type) { >> >> case SOCK_STREAM: >> >> case SOCK_SEQPACKET: >> >> ASSERT_EQ(-1, n); >> >> ASSERT_EQ(ECONNRESET, errno); >> >> break; >> >> case SOCK_DGRAM: >> >> ASSERT(-1, n); >> >> ASSERT_EQ(EAGAIN, errno); >> >> break; >> >> } >> >> ... >> >> if not these, could you kindly shed more light to what you meant >> >>> >>>> + >>>> + /* Peer closes normally */ >>>> + 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 (n == -1) >>>> + ASSERT_EQ(ECONNRESET, errno); >>> ... otherwise, we don't see an error here >>> >>>> + >>>> + if (n != -1) >>>> + ASSERT_EQ(0, n); >>> and this can be checked unconditionally. >> did you mean I should remove the if (n != -1) ASSERT_EQ(0, n); part? > If SOCK_DGRAM does not reuse this test, yes. > > The point is we do not want to miss future regression by > preparing both if (n == -1) case and if (n == 0) case, one > of which should never happen at this point. > > Thanks! > Thank you for these. I will work on them and send v3 shortly. Thanks once again. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-10-31 20:02 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20251025190256.11352-1-adelodunolaoluwa.ref@yahoo.com>
2025-10-25 19:02 ` [PATCH v2] selftests: af_unix: Add tests for ECONNRESET and EOF semantics Sunday Adelodun
2025-10-28 18:28 ` Kuniyuki Iwashima
2025-10-30 13:45 ` Sunday Adelodun
2025-10-31 19:41 ` Kuniyuki Iwashima
2025-10-31 20:01 ` Sunday Adelodun
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox