From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: passt.top; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20230601 header.b=Knl8g7i0; dkim-atps=neutral Received: from mail-io1-xd2c.google.com (mail-io1-xd2c.google.com [IPv6:2607:f8b0:4864:20::d2c]) by passt.top (Postfix) with ESMTPS id DD16A5A004E for ; Sat, 24 Aug 2024 01:44:41 +0200 (CEST) Received: by mail-io1-xd2c.google.com with SMTP id ca18e2360f4ac-824ee14f7bfso93971539f.1 for ; Fri, 23 Aug 2024 16:44:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1724456680; x=1725061480; darn=passt.top; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=KistXHioizcW9FYbSUI1OlFUfi5tftYK70zDq1HxHBE=; b=Knl8g7i0Z+luiopPy2PZaNBUo2Wo+ne13TNbDc5angkZbKNszgVE5HWCYvV60ejBwr Du0pRr3ormojK0YFzqV8Dt3LIs6t4c1th8e1xrHp9Gfy+rWXPv3HXieuHk2iHZEFxLoX yMRKMGpWe8x7zv5eCfq4ugSX1SUHYGRagFPOCJxRmr/QSCB1iopvFVWXoWHh/uC/W4fs HJzKIULMFOW8c61nRJH9Q5XyOvEgugU4ppCmk5tOmQh4llmgY2JgHEUow9hecmZyKZC2 AIU2ngBaPEL8WvoYY4ysgRbMl8v6h+WSL5WN/XP2P+cVMu1eCzHfM+72WQJo3bEl+Jo8 /CTg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1724456680; x=1725061480; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=KistXHioizcW9FYbSUI1OlFUfi5tftYK70zDq1HxHBE=; b=IOhXjjc4L7LXJQimo2WCZr6PKgTBudXqdyJKAMsXfXHa5ZOfQPvLuoa3ug0G/ajmTC d8XY0GWWlQxjMtJXnEeWzCV+JuzQsouL6xlAS/Yszd0ukO6JXL6ndCTbI7sRRjJV1NOY 0YJlu9le1tN7PO00ZcRp3gmWuHtQM3aHBqkg9/ekZYSRj75mCoYu3amzKLRAXX2+Yhw+ EKX3/l+oH4i8jMyJOWpioyxooZ6NAR7PZ/XlIvMQxR37HFtelv4w5zpgQtxfHn6ECMyY dP0BEBJJusYr+D7D6NKLvtXW9IwyS0/nNxTXfdXqFyGULlFOsdoWpvBlm3iBDQJi8Eix 9sRw== X-Forwarded-Encrypted: i=1; AJvYcCXXwHm7NZlJM3mER/DoByrJXmMYfYwGPfwpFuDfgidzhmGdnT6CY01DFDYZGore6t9lJqJ/U+Xbpwg=@passt.top X-Gm-Message-State: AOJu0YzyMvr7HEwtmwaxbmsBZ9BXzCA2VnJwmLG6Zs4lvVReplql6kDa hetwsIoVsabvQApMJwrg6T4fdhI5yvWc/VMEL3cy9H3uVtnS81wnsgIbq61P83KteAuDZT5pkjI /+tmTvei1rSVMAHqd56XdTAbdRe8= X-Google-Smtp-Source: AGHT+IHCScWcM1ds0gB3lNaC69Mano1ry0Gq37G1600EUnNhIVZ7f/NfUYnJapCmBiihOhMOyBGmsZp2IEtZE0PVG+o= X-Received: by 2002:a05:6e02:12ee:b0:39d:3c85:9b0 with SMTP id e9e14a558f8ab-39e3c98c707mr43076015ab.15.1724456680471; Fri, 23 Aug 2024 16:44:40 -0700 (PDT) MIME-Version: 1.0 References: <20240823211902.143210-1-jmaloy@redhat.com> <20240823211902.143210-3-jmaloy@redhat.com> In-Reply-To: <20240823211902.143210-3-jmaloy@redhat.com> From: Jason Xing Date: Sat, 24 Aug 2024 07:44:04 +0800 Message-ID: Subject: Re: [PATCH 2/2] selftests: add selftest for tcp SO_PEEK_OFF support To: jmaloy@redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-MailFrom: kerneljasonxing@gmail.com X-Mailman-Rule-Hits: nonmember-moderation X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation Message-ID-Hash: EVSF2M2PJ4URH47R2KSJN7EFJTYBPRO3 X-Message-ID-Hash: EVSF2M2PJ4URH47R2KSJN7EFJTYBPRO3 X-Mailman-Approved-At: Sat, 24 Aug 2024 17:54:08 +0200 CC: netdev@vger.kernel.org, linux-kselftest@vger.kernel.org, davem@davemloft.net, kuba@kernel.org, passt-dev@passt.top, sbrivio@redhat.com, lvivier@redhat.com, dgibson@redhat.com, eric.dumazet@gmail.com, edumazet@google.com X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Hello Jon, On Sat, Aug 24, 2024 at 5:19=E2=80=AFAM wrote: > > From: Jon Maloy > > We add a selftest to check that the new feature added in > commit 05ea491641d3 ("tcp: add support for SO_PEEK_OFF socket option") > works correctly. > > Reviewed-by: Stefano Brivio > Tested-by: Stefano Brivio > Signed-off-by: Jon Maloy Thanks for working on this. Sorry that I just noticed I missed your previous reply :( > --- > tools/testing/selftests/net/Makefile | 1 + > tools/testing/selftests/net/tcp_so_peek_off.c | 181 ++++++++++++++++++ > 2 files changed, 182 insertions(+) > create mode 100644 tools/testing/selftests/net/tcp_so_peek_off.c > > diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftes= ts/net/Makefile > index 8eaffd7a641c..1179e3261bef 100644 > --- a/tools/testing/selftests/net/Makefile > +++ b/tools/testing/selftests/net/Makefile > @@ -80,6 +80,7 @@ TEST_PROGS +=3D io_uring_zerocopy_tx.sh > TEST_GEN_FILES +=3D bind_bhash > TEST_GEN_PROGS +=3D sk_bind_sendto_listen > TEST_GEN_PROGS +=3D sk_connect_zero_addr > +TEST_GEN_PROGS +=3D tcp_so_peek_off > TEST_PROGS +=3D test_ingress_egress_chaining.sh > TEST_GEN_PROGS +=3D so_incoming_cpu > TEST_PROGS +=3D sctp_vrf.sh > diff --git a/tools/testing/selftests/net/tcp_so_peek_off.c b/tools/testin= g/selftests/net/tcp_so_peek_off.c > new file mode 100644 > index 000000000000..8379ea02e3d7 > --- /dev/null > +++ b/tools/testing/selftests/net/tcp_so_peek_off.c > @@ -0,0 +1,181 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include "../kselftest.h" > + > +static char *afstr(int af) > +{ > + return af =3D=3D AF_INET ? "TCP/IPv4" : "TCP/IPv6"; > +} > + > +int tcp_peek_offset_probe(sa_family_t af) > +{ > + int optv =3D 0; > + int ret =3D 0; > + int s; > + > + s =3D socket(af, SOCK_STREAM | SOCK_CLOEXEC, IPPROTO_TCP); > + if (s < 0) { > + ksft_perror("Temporary TCP socket creation failed"); > + } else { > + if (!setsockopt(s, SOL_SOCKET, SO_PEEK_OFF, &optv, sizeof= (int))) > + ret =3D 1; > + else > + printf("%s does not support SO_PEEK_OFF\n", afstr= (af)); > + close(s); > + } > + return ret; > +} > + > +static void tcp_peek_offset_set(int s, int offset) > +{ > + if (setsockopt(s, SOL_SOCKET, SO_PEEK_OFF, &offset, sizeof(offset= ))) > + ksft_perror("Failed to set SO_PEEK_OFF value\n"); > +} > + > +static int tcp_peek_offset_get(int s) > +{ > + int offset; > + socklen_t len =3D sizeof(offset); > + > + if (getsockopt(s, SOL_SOCKET, SO_PEEK_OFF, &offset, &len)) > + ksft_perror("Failed to get SO_PEEK_OFF value\n"); > + return offset; > +} > + > +static int tcp_peek_offset_test(sa_family_t af) > +{ > + union { > + struct sockaddr sa; > + struct sockaddr_in a4; > + struct sockaddr_in6 a6; > + } a; > + int res =3D 0; > + int s[2] =3D {0, 0}; > + int recv_sock =3D 0; > + int offset =3D 0; > + ssize_t len; > + char buf; > + > + memset(&a, 0, sizeof(a)); > + a.sa.sa_family =3D af; > + > + s[0] =3D socket(af, SOCK_STREAM, IPPROTO_TCP); > + s[1] =3D socket(af, SOCK_STREAM | SOCK_NONBLOCK, IPPROTO_TCP); > + > + if (s[0] < 0 || s[1] < 0) { > + ksft_perror("Temporary probe socket creation failed\n"); > + goto out; Nit: I wonder if we can use more proper test statements to avoid such hiding failure[1] when closing a invalid file descriptor, even though it doesn't harm the test itself? [1]: "EBADF (Bad file descriptor)" > + } > + if (bind(s[0], &a.sa, sizeof(a)) < 0) { > + ksft_perror("Temporary probe socket bind() failed\n"); > + goto out; > + } > + if (getsockname(s[0], &a.sa, &((socklen_t) { sizeof(a) })) < 0) { > + ksft_perror("Temporary probe socket getsockname() failed\= n"); > + goto out; > + } > + if (listen(s[0], 0) < 0) { > + ksft_perror("Temporary probe socket listen() failed\n"); > + goto out; > + } > + if (connect(s[1], &a.sa, sizeof(a)) >=3D 0 || errno !=3D EINPROGR= ESS) { > + ksft_perror("Temporary probe socket connect() failed\n"); > + goto out; > + } > + recv_sock =3D accept(s[0], NULL, NULL); > + if (recv_sock <=3D 0) { > + ksft_perror("Temporary probe socket accept() failed\n"); > + goto out; Same here. > + } > + > + /* Some basic tests of getting/setting offset */ > + offset =3D tcp_peek_offset_get(recv_sock); > + if (offset !=3D -1) { > + ksft_perror("Initial value of socket offset not -1\n"); > + goto out; > + } > + tcp_peek_offset_set(recv_sock, 0); > + offset =3D tcp_peek_offset_get(recv_sock); > + if (offset !=3D 0) { > + ksft_perror("Failed to set socket offset to 0\n"); > + goto out; > + } > + > + /* Transfer a message */ > + if (send(s[1], (char *)("ab"), 2, 0) <=3D 0 || errno !=3D EINPROG= RESS) { > + ksft_perror("Temporary probe socket send() failed\n"); > + goto out; > + } > + /* Read first byte */ > + len =3D recv(recv_sock, &buf, 1, MSG_PEEK); > + if (len !=3D 1 || buf !=3D 'a') { > + ksft_perror("Failed to read first byte of message\n"); > + goto out; > + } > + offset =3D tcp_peek_offset_get(recv_sock); > + if (offset !=3D 1) { > + ksft_perror("Offset not forwarded correctly at first byte= \n"); > + goto out; > + } > + /* Try to read beyond last byte */ > + len =3D recv(recv_sock, &buf, 2, MSG_PEEK); > + if (len !=3D 1 || buf !=3D 'b') { > + ksft_perror("Failed to read last byte of message\n"); > + goto out; > + } > + offset =3D tcp_peek_offset_get(recv_sock); > + if (offset !=3D 2) { > + ksft_perror("Offset not forwarded correctly at last byte\= n"); > + goto out; > + } > + /* Flush message */ > + len =3D recv(recv_sock, NULL, 2, MSG_TRUNC); > + if (len !=3D 2) { > + ksft_perror("Failed to flush message\n"); > + goto out; > + } > + offset =3D tcp_peek_offset_get(recv_sock); > + if (offset !=3D 0) { > + ksft_perror("Offset not reverted correctly after flush\n"= ); > + goto out; > + } > + > + printf("%s with MSG_PEEK_OFF works correctly\n", afstr(af)); > + res =3D 1; > +out: > + close(recv_sock); > + close(s[1]); > + close(s[0]); > + return res; > +} > + > +int main(void) > +{ > + int res4, res6; > + > + res4 =3D tcp_peek_offset_probe(AF_INET); > + res6 =3D tcp_peek_offset_probe(AF_INET6); > + > + if (!res4 && !res6) > + return KSFT_SKIP; > + > + if (res4) > + res4 =3D tcp_peek_offset_test(AF_INET); > + > + if (res6) > + res6 =3D tcp_peek_offset_test(AF_INET6); > + > + if (!res4 || !res6) What if res6 is NULL after checking tcp_peek_offset_probe() while res4 is always working correctly, then we will get notified with a KSFT_FAIL failure instead of KSFT_SKIP. The thing could happen because you reuse the same return value for v4/v6 mo= de. Thanks, Jason > + return KSFT_FAIL; > + > + return KSFT_PASS; > +} > + > -- > 2.45.2 > >