Re: [PATCH v3] selftests: af_unix: Add tests for ECONNRESET and EOF semantics

[Sunday Adelodun <adelodunolaoluwa@yahoo.com>]

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
>>