* [PATCH v3] selftests: af_unix: Add tests for ECONNRESET and EOF semantics [not found] <20251101172230.10179-1-adelodunolaoluwa.ref@yahoo.com> @ 2025-11-01 17:22 ` Sunday Adelodun 2025-11-02 7:32 ` Kuniyuki Iwashima 0 siblings, 1 reply; 7+ messages in thread From: Sunday Adelodun @ 2025-11-01 17:22 UTC (permalink / raw) To: kuniyu, davem, edumazet, kuba, pabeni, horms, shuah Cc: linux-kernel, netdev, linux-kselftest, skhan, david.hunter.linux, 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 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 <kuniyu@google.com> Signed-off-by: Sunday Adelodun <adelodunolaoluwa@yahoo.com> --- 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 \ # 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 */ +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) { + err = listen(self->server, 1); + ASSERT_EQ(0, err); + + self->client = socket(AF_UNIX, variant->socket_type, 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 || + 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)); + 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 */ +TEST_F(unix_sock, dgram_reset) +{ + char buf[16] = {}; + ssize_t n; + + 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)); + 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_HARNESS_MAIN + -- 2.43.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v3] selftests: af_unix: Add tests for ECONNRESET and EOF semantics 2025-11-01 17:22 ` [PATCH v3] selftests: af_unix: Add tests for ECONNRESET and EOF semantics Sunday Adelodun @ 2025-11-02 7:32 ` Kuniyuki Iwashima 2025-11-04 0:08 ` Sunday Adelodun 0 siblings, 1 reply; 7+ messages in thread From: Kuniyuki Iwashima @ 2025-11-02 7:32 UTC (permalink / raw) To: Sunday Adelodun Cc: davem, edumazet, kuba, pabeni, horms, shuah, linux-kernel, netdev, linux-kselftest, skhan, david.hunter.linux, linux-kernel-mentees On Sat, Nov 1, 2025 at 10:23 AM 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 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 <kuniyu@google.com> > Signed-off-by: Sunday Adelodun <adelodunolaoluwa@yahoo.com> > --- > 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/20251101172230.10179-1-adelodunolaoluwa@yahoo.com/ Could you add it to this file ? tools/testing/selftests/net/.gitignore > # 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 ? > +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 ? > + 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. > + 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 ;) Thanks! > +TEST_F(unix_sock, dgram_reset) > +{ > + char buf[16] = {}; > + ssize_t n; > + > + 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)); > + 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_HARNESS_MAIN > + > -- > 2.43.0 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3] selftests: af_unix: Add tests for ECONNRESET and EOF semantics 2025-11-02 7:32 ` Kuniyuki Iwashima @ 2025-11-04 0:08 ` Sunday Adelodun 2025-11-04 0:30 ` Kuniyuki Iwashima 0 siblings, 1 reply; 7+ messages in thread From: Sunday Adelodun @ 2025-11-04 0:08 UTC (permalink / raw) To: Kuniyuki Iwashima Cc: davem, edumazet, kuba, pabeni, horms, shuah, linux-kernel, netdev, linux-kselftest, skhan, david.hunter.linux, linux-kernel-mentees On 11/2/25 08:32, Kuniyuki Iwashima wrote: > On Sat, Nov 1, 2025 at 10:23 AM 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 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 <kuniyu@google.com> >> Signed-off-by: Sunday Adelodun <adelodunolaoluwa@yahoo.com> >> --- >> 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/20251101172230.10179-1-adelodunolaoluwa@yahoo.com/ > > 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? >> Thanks! >> >> +TEST_F(unix_sock, dgram_reset) >> +{ >> + char buf[16] = {}; >> + ssize_t n; >> + >> + 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)); >> + 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_HARNESS_MAIN >> + >> -- >> 2.43.0 >> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3] selftests: af_unix: Add tests for ECONNRESET and EOF semantics 2025-11-04 0:08 ` Sunday Adelodun @ 2025-11-04 0:30 ` Kuniyuki Iwashima 2025-11-04 10:42 ` Sunday Adelodun 0 siblings, 1 reply; 7+ messages in thread From: Kuniyuki Iwashima @ 2025-11-04 0:30 UTC (permalink / raw) To: Sunday Adelodun Cc: davem, edumazet, kuba, pabeni, horms, shuah, linux-kernel, netdev, linux-kselftest, skhan, david.hunter.linux, linux-kernel-mentees On Mon, Nov 3, 2025 at 4:08 PM Sunday Adelodun <adelodunolaoluwa@yahoo.com> wrote: > > On 11/2/25 08:32, Kuniyuki Iwashima wrote: > > On Sat, Nov 1, 2025 at 10:23 AM 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 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 <kuniyu@google.com> > >> Signed-off-by: Sunday Adelodun <adelodunolaoluwa@yahoo.com> > >> --- > >> 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/20251101172230.10179-1-adelodunolaoluwa@yahoo.com/ > > > > 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/20251028024339.2028774-1-kuniyu@google.com/ Thanks! ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3] selftests: af_unix: Add tests for ECONNRESET and EOF semantics 2025-11-04 0:30 ` Kuniyuki Iwashima @ 2025-11-04 10:42 ` Sunday Adelodun 2025-11-07 7:05 ` Kuniyuki Iwashima 0 siblings, 1 reply; 7+ messages in thread From: Sunday Adelodun @ 2025-11-04 10:42 UTC (permalink / raw) To: Kuniyuki Iwashima Cc: davem, edumazet, kuba, pabeni, horms, shuah, linux-kernel, netdev, linux-kselftest, skhan, david.hunter.linux, linux-kernel-mentees On 11/4/25 01:30, Kuniyuki Iwashima wrote: > On Mon, Nov 3, 2025 at 4:08 PM Sunday Adelodun > <adelodunolaoluwa@yahoo.com> wrote: >> On 11/2/25 08:32, Kuniyuki Iwashima wrote: >>> On Sat, Nov 1, 2025 at 10:23 AM 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 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 <kuniyu@google.com> >>>> Signed-off-by: Sunday Adelodun <adelodunolaoluwa@yahoo.com> >>>> --- >>>> 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/20251101172230.10179-1-adelodunolaoluwa@yahoo.com/ >>> >>> 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/20251028024339.2028774-1-kuniyu@google.com/ > > Thanks! Thank you! kindly check these if any conforms to what it should be: TEST_F(unix_sock, reset_unread_behavior) { char buf[16] = {}; ssize_t n; /* Send data that will remain unread by client */ send(self->client, "hello", 5, 0); if (variant->socket_type == SOCK_DGRAM) { close(self->server); } else { if (!self->child) SKIP(return); close(self->child); } n = recv(self->client, buf, sizeof(buf), 0); ASSERT_EQ(-1, n); if (variant->socket_type == SOCK_STREAM || variant->socket_type == SOCK_SEQPACKET) do { ASSERT_EQ(ECONNRESET, errno); } while (0); else ASSERT_EQ(EAGAIN, errno); } OR TEST_F(unix_sock, reset_unread_behavior) { char buf[16] = {}; ssize_t n; /* Send data that will remain unread by client */ send(self->client, "hello", 5, 0); if (variant->socket_type == SOCK_DGRAM) { close(self->server); } else { if (self->child) close(self->child); else close(self->server); } n = recv(self->client, buf, sizeof(buf), 0); ASSERT_EQ(-1, n); if (variant->socket_type == SOCK_STREAM || variant->socket_type == SOCK_SEQPACKET) do { ASSERT_EQ(ECONNRESET, errno); } while (0); else ASSERT_EQ(EAGAIN, errno); } OR is there a better way to handle this? I ran the KCOV_OUTPUT command using the first *TEST_F above* as the Test 2 and got the output below: *$ KCOV_OUTPUT=kcov KCOV_SLOTS=8192 ./tools/testing/selftests/net/af_unix/unix_connreset* TAP version 13 1..6 # Starting 6 tests from 3 test cases. # RUN unix_sock.stream.eof ... # OK unix_sock.stream.eof ok 1 unix_sock.stream.eof # RUN unix_sock.stream.reset_unread_behavior ... # OK unix_sock.stream.reset_unread_behavior ok 2 unix_sock.stream.reset_unread_behavior # RUN unix_sock.dgram.eof ... # OK unix_sock.dgram.eof ok 3 unix_sock.dgram.eof # RUN unix_sock.dgram.reset_unread_behavior ... # OK unix_sock.dgram.reset_unread_behavior ok 4 unix_sock.dgram.reset_unread_behavior # RUN unix_sock.seqpacket.eof ... # OK unix_sock.seqpacket.eof ok 5 unix_sock.seqpacket.eof # RUN unix_sock.seqpacket.reset_unread_behavior ... # OK unix_sock.seqpacket.reset_unread_behavior ok 6 unix_sock.seqpacket.reset_unread_behavior # PASSED: 6 / 6 tests passed. # Totals: pass:6 fail:0 xfail:0 xpass:0 skip:0 error:0 Thank you once again for your continuous guidance and patience. It's a worthy and rewarding learning period for me. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3] selftests: af_unix: Add tests for ECONNRESET and EOF semantics 2025-11-04 10:42 ` Sunday Adelodun @ 2025-11-07 7:05 ` Kuniyuki Iwashima 2025-11-12 20:32 ` Sunday Adelodun 0 siblings, 1 reply; 7+ messages in thread From: Kuniyuki Iwashima @ 2025-11-07 7:05 UTC (permalink / raw) To: adelodunolaoluwa Cc: davem, david.hunter.linux, edumazet, horms, kuba, kuniyu, linux-kernel-mentees, linux-kernel, linux-kselftest, netdev, pabeni, shuah, skhan From: Sunday Adelodun <adelodunolaoluwa@yahoo.com> Date: Tue, 4 Nov 2025 11:42:47 +0100 > On 11/4/25 01:30, Kuniyuki Iwashima wrote: > > On Mon, Nov 3, 2025 at 4:08 PM Sunday Adelodun > > <adelodunolaoluwa@yahoo.com> wrote: > >> On 11/2/25 08:32, Kuniyuki Iwashima wrote: > >>> On Sat, Nov 1, 2025 at 10:23 AM 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 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 <kuniyu@google.com> > >>>> Signed-off-by: Sunday Adelodun <adelodunolaoluwa@yahoo.com> > >>>> --- > >>>> 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/20251101172230.10179-1-adelodunolaoluwa@yahoo.com/ > >>> > >>> 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/20251028024339.2028774-1-kuniyu@google.com/ > > > > Thanks! > Thank you! > > kindly check these if any conforms to what it should be: > > TEST_F(unix_sock, reset_unread_behavior) > { > char buf[16] = {}; > ssize_t n; > > /* Send data that will remain unread by client */ > send(self->client, "hello", 5, 0); > > if (variant->socket_type == SOCK_DGRAM) { > close(self->server); > } > else { > if (!self->child) > SKIP(return); > > close(self->child); > } > > n = recv(self->client, buf, sizeof(buf), 0); > > ASSERT_EQ(-1, n); > > if (variant->socket_type == SOCK_STREAM || > variant->socket_type == SOCK_SEQPACKET) > do { ASSERT_EQ(ECONNRESET, errno); } while (0); > else > ASSERT_EQ(EAGAIN, errno); > } > > OR > > TEST_F(unix_sock, reset_unread_behavior) > { > char buf[16] = {}; > ssize_t n; > > /* Send data that will remain unread by client */ > send(self->client, "hello", 5, 0); > > if (variant->socket_type == SOCK_DGRAM) { > close(self->server); > } > else { > if (self->child) > close(self->child); > else > close(self->server); > } > > n = recv(self->client, buf, sizeof(buf), 0); > > ASSERT_EQ(-1, n); > > if (variant->socket_type == SOCK_STREAM || > variant->socket_type == SOCK_SEQPACKET) > do { ASSERT_EQ(ECONNRESET, errno); } while (0); > else > ASSERT_EQ(EAGAIN, errno); > } > > OR > > is there a better way to handle this? Sorry for late! What I had in mind is to move accept() in FIXTURE_SETUP() to Test 1 & 2 (then, only listen() is conditional in FIXTURE_SETUP()) and rewrite Test 3 to cover the last ECONNRESET case caused by close()ing un-accept()ed sockets: TEST_F(unix_sock, reset_closed_embryo) { if (variant->socket_type == SOCK_DGRAM) SKIP(return, "This test only applies to SOCK_STREAM and SOCK_SEQPACKET"); close(self->server); n = recv(self->client, ...); ASSERT_EQ(-1, n); ASSERT_EQ(ECONNRESET, errno); } > > I ran the KCOV_OUTPUT command using the first *TEST_F above* as the Test > 2 and got the output below: > *$ KCOV_OUTPUT=kcov KCOV_SLOTS=8192 > ./tools/testing/selftests/net/af_unix/unix_connreset* You should be able to see line-by-line coverage by decoding files under kcov with addr2line or vock/report.py. Thanks! ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3] selftests: af_unix: Add tests for ECONNRESET and EOF semantics 2025-11-07 7:05 ` Kuniyuki Iwashima @ 2025-11-12 20:32 ` Sunday Adelodun 0 siblings, 0 replies; 7+ messages in thread From: Sunday Adelodun @ 2025-11-12 20:32 UTC (permalink / raw) To: Kuniyuki Iwashima Cc: davem, david.hunter.linux, edumazet, horms, kuba, linux-kernel-mentees, linux-kernel, linux-kselftest, netdev, pabeni, shuah, skhan On 11/7/25 08:05, Kuniyuki Iwashima wrote: > From: Sunday Adelodun <adelodunolaoluwa@yahoo.com> > Date: Tue, 4 Nov 2025 11:42:47 +0100 >> On 11/4/25 01:30, Kuniyuki Iwashima wrote: >>> On Mon, Nov 3, 2025 at 4:08 PM Sunday Adelodun >>> <adelodunolaoluwa@yahoo.com> wrote: >>>> On 11/2/25 08:32, Kuniyuki Iwashima wrote: >>>>> On Sat, Nov 1, 2025 at 10:23 AM 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 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 <kuniyu@google.com> >>>>>> Signed-off-by: Sunday Adelodun <adelodunolaoluwa@yahoo.com> >>>>>> --- >>>>>> 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/20251101172230.10179-1-adelodunolaoluwa@yahoo.com/ >>>>> >>>>> 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/20251028024339.2028774-1-kuniyu@google.com/ >>> >>> Thanks! >> Thank you! >> >> kindly check these if any conforms to what it should be: >> >> TEST_F(unix_sock, reset_unread_behavior) >> { >> char buf[16] = {}; >> ssize_t n; >> >> /* Send data that will remain unread by client */ >> send(self->client, "hello", 5, 0); >> >> if (variant->socket_type == SOCK_DGRAM) { >> close(self->server); >> } >> else { >> if (!self->child) >> SKIP(return); >> >> close(self->child); >> } >> >> n = recv(self->client, buf, sizeof(buf), 0); >> >> ASSERT_EQ(-1, n); >> >> if (variant->socket_type == SOCK_STREAM || >> variant->socket_type == SOCK_SEQPACKET) >> do { ASSERT_EQ(ECONNRESET, errno); } while (0); >> else >> ASSERT_EQ(EAGAIN, errno); >> } >> >> OR >> >> TEST_F(unix_sock, reset_unread_behavior) >> { >> char buf[16] = {}; >> ssize_t n; >> >> /* Send data that will remain unread by client */ >> send(self->client, "hello", 5, 0); >> >> if (variant->socket_type == SOCK_DGRAM) { >> close(self->server); >> } >> else { >> if (self->child) >> close(self->child); >> else >> close(self->server); >> } >> >> n = recv(self->client, buf, sizeof(buf), 0); >> >> ASSERT_EQ(-1, n); >> >> if (variant->socket_type == SOCK_STREAM || >> variant->socket_type == SOCK_SEQPACKET) >> do { ASSERT_EQ(ECONNRESET, errno); } while (0); >> else >> ASSERT_EQ(EAGAIN, errno); >> } >> >> OR >> >> is there a better way to handle this? > Sorry for late! > > What I had in mind is to move accept() in FIXTURE_SETUP() to > Test 1 & 2 (then, only listen() is conditional in FIXTURE_SETUP()) > and rewrite Test 3 to cover the last ECONNRESET case caused by > close()ing un-accept()ed sockets: > > TEST_F(unix_sock, reset_closed_embryo) > { > if (variant->socket_type == SOCK_DGRAM) > SKIP(return, "This test only applies to SOCK_STREAM and SOCK_SEQPACKET"); > > close(self->server); > > n = recv(self->client, ...); > ASSERT_EQ(-1, n); > ASSERT_EQ(ECONNRESET, errno); > } > > >> I ran the KCOV_OUTPUT command using the first *TEST_F above* as the Test >> 2 and got the output below: >> *$ KCOV_OUTPUT=kcov KCOV_SLOTS=8192 >> ./tools/testing/selftests/net/af_unix/unix_connreset* > You should be able to see line-by-line coverage by decoding > files under kcov with addr2line or vock/report.py. > > Thanks! > So sorry for the late reply. I have noted all and I will send v4 shortly Thanks once again ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-11-12 20:33 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20251101172230.10179-1-adelodunolaoluwa.ref@yahoo.com>
2025-11-01 17:22 ` [PATCH v3] selftests: af_unix: Add tests for ECONNRESET and EOF semantics Sunday Adelodun
2025-11-02 7:32 ` Kuniyuki Iwashima
2025-11-04 0:08 ` Sunday Adelodun
2025-11-04 0:30 ` Kuniyuki Iwashima
2025-11-04 10:42 ` Sunday Adelodun
2025-11-07 7:05 ` Kuniyuki Iwashima
2025-11-12 20:32 ` Sunday Adelodun
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox