| Message ID | 20250129143601.16035-2-annaemesenyiri@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | selftests: net: Add support for testing SO_RCVMARK and SO_RCVPRIORITY | expand |
On Wed, 29 Jan 2025 15:36:01 +0100 Anna Emese Nyiri wrote: > diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile > index 73ee88d6b043..98f05473e672 100644 > --- a/tools/testing/selftests/net/Makefile > +++ b/tools/testing/selftests/net/Makefile > @@ -33,6 +33,7 @@ TEST_PROGS += gro.sh > TEST_PROGS += gre_gso.sh > TEST_PROGS += cmsg_so_mark.sh > TEST_PROGS += cmsg_so_priority.sh > +TEST_PROGS += test_so_rcv.sh You need to add the C part to the TEST_GEN_PROGS, otherwise it won't get built. We're seeing: ./test_so_rcv.sh: line 25: ./so_rcv_listener: No such file or directory in the CI. > + memset(&recv_addr, 0, sizeof(recv_addr)); > + recv_addr.sin_family = AF_INET; > + recv_addr.sin_port = htons(atoi(opt.service)); > + > + if (inet_pton(AF_INET, opt.host, &recv_addr.sin_addr) <= 0) { > + perror("Invalid address"); > + ret_value = -errno; > + goto cleanup; > + } Any reason not to use getaddrinfo() ? Otherwise LGTM, thanks for following up!
Jakub Kicinski <kuba@kernel.org> ezt írta (időpont: 2025. jan. 29., Sze, 21:05): > > On Wed, 29 Jan 2025 15:36:01 +0100 Anna Emese Nyiri wrote: > > diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile > > index 73ee88d6b043..98f05473e672 100644 > > --- a/tools/testing/selftests/net/Makefile > > +++ b/tools/testing/selftests/net/Makefile > > @@ -33,6 +33,7 @@ TEST_PROGS += gro.sh > > TEST_PROGS += gre_gso.sh > > TEST_PROGS += cmsg_so_mark.sh > > TEST_PROGS += cmsg_so_priority.sh > > +TEST_PROGS += test_so_rcv.sh > > You need to add the C part to the TEST_GEN_PROGS, otherwise it won't > get built. We're seeing: > > ./test_so_rcv.sh: line 25: ./so_rcv_listener: No such file or directory > > in the CI. > > > + memset(&recv_addr, 0, sizeof(recv_addr)); > > + recv_addr.sin_family = AF_INET; > > + recv_addr.sin_port = htons(atoi(opt.service)); > > + > > + if (inet_pton(AF_INET, opt.host, &recv_addr.sin_addr) <= 0) { > > + perror("Invalid address"); > > + ret_value = -errno; > > + goto cleanup; > > + } > > Any reason not to use getaddrinfo() ? I chose inet_pton() over getaddrinfo() because getaddrinfo() depends on libnss, which can cause warnings and linking issues in static builds. In contrast, inet_pton() is fully part of libc, so it seemed like a safer choice. > Otherwise LGTM, thanks for following up! > -- > pw-bot: cr
Jakub Kicinski wrote: > On Wed, 29 Jan 2025 15:36:01 +0100 Anna Emese Nyiri wrote: > > diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile > > index 73ee88d6b043..98f05473e672 100644 > > --- a/tools/testing/selftests/net/Makefile > > +++ b/tools/testing/selftests/net/Makefile > > @@ -33,6 +33,7 @@ TEST_PROGS += gro.sh > > TEST_PROGS += gre_gso.sh > > TEST_PROGS += cmsg_so_mark.sh > > TEST_PROGS += cmsg_so_priority.sh > > +TEST_PROGS += test_so_rcv.sh > > You need to add the C part to the TEST_GEN_PROGS, otherwise it won't > get built. We're seeing: and to .gitignore > ./test_so_rcv.sh: line 25: ./so_rcv_listener: No such file or directory > > in the CI. > > > + memset(&recv_addr, 0, sizeof(recv_addr)); > > + recv_addr.sin_family = AF_INET; > > + recv_addr.sin_port = htons(atoi(opt.service)); > > + > > + if (inet_pton(AF_INET, opt.host, &recv_addr.sin_addr) <= 0) { > > + perror("Invalid address"); > > + ret_value = -errno; > > + goto cleanup; > > + } > > Any reason not to use getaddrinfo() ? > > Otherwise LGTM, thanks for following up! > -- > pw-bot: cr
Anna Emese Nyiri wrote: > Introduce tests to verify the correct functionality of the SO_RCVMARK and > SO_RCVPRIORITY socket options. > > Key changes include: > > - so_rcv_listener.c: Implements a receiver application to test the correct > behavior of the SO_RCVMARK and SO_RCVPRIORITY options. > - test_so_rcv.sh: Provides a shell script to automate testing for these options. > - Makefile: Integrates test_so_rcv.sh into the kernel selftests. > > Suggested-by: Jakub Kicinski <kuba@kernel.org> > Suggested-by: Ferenc Fejes <fejes@inf.elte.hu> > Signed-off-by: Anna Emese Nyiri <annaemesenyiri@gmail.com> No need for a cover letter to a single patch. net-next is closed. > > --- > tools/testing/selftests/net/Makefile | 1 + > tools/testing/selftests/net/so_rcv_listener.c | 147 ++++++++++++++++++ > tools/testing/selftests/net/test_so_rcv.sh | 56 +++++++ > 3 files changed, 204 insertions(+) > create mode 100644 tools/testing/selftests/net/so_rcv_listener.c > create mode 100755 tools/testing/selftests/net/test_so_rcv.sh > > diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile > index 73ee88d6b043..98f05473e672 100644 > --- a/tools/testing/selftests/net/Makefile > +++ b/tools/testing/selftests/net/Makefile > @@ -33,6 +33,7 @@ TEST_PROGS += gro.sh > TEST_PROGS += gre_gso.sh > TEST_PROGS += cmsg_so_mark.sh > TEST_PROGS += cmsg_so_priority.sh > +TEST_PROGS += test_so_rcv.sh > TEST_PROGS += cmsg_time.sh cmsg_ipv6.sh > TEST_PROGS += netns-name.sh > TEST_PROGS += nl_netdev.py > diff --git a/tools/testing/selftests/net/so_rcv_listener.c b/tools/testing/selftests/net/so_rcv_listener.c > new file mode 100644 > index 000000000000..53b09582a7e3 > --- /dev/null > +++ b/tools/testing/selftests/net/so_rcv_listener.c > @@ -0,0 +1,147 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +#include <errno.h> > +#include <netdb.h> > +#include <stdbool.h> > +#include <stdio.h> > +#include <stdlib.h> > +#include <string.h> > +#include <unistd.h> > +#include <linux/types.h> > +#include <sys/socket.h> > +#include <netinet/in.h> > +#include <arpa/inet.h> > + > +#ifndef SO_RCVPRIORITY > +#define SO_RCVPRIORITY 82 > +#endif > + > +struct options { > + __u32 val; > + int name; > + int rcvname; > + const char *host; > + const char *service; > +} opt; > + > +static void __attribute__((noreturn)) usage(const char *bin) > +{ > + printf("Usage: %s [opts] <dst host> <dst port / service>\n", bin); > + printf("Options:\n" > + "\t\t-M val Test SO_RCVMARK\n" > + "\t\t-P val Test SO_RCVPRIORITY\n" > + ""); > + exit(EXIT_FAILURE); > +} > + > +static void parse_args(int argc, char *argv[]) > +{ > + int o; > + > + while ((o = getopt(argc, argv, "M:P:")) != -1) { > + switch (o) { > + case 'M': > + opt.val = atoi(optarg); > + opt.name = SO_MARK; > + opt.rcvname = SO_RCVMARK; > + break; > + case 'P': > + opt.val = atoi(optarg); > + opt.name = SO_PRIORITY; > + opt.rcvname = SO_RCVPRIORITY; > + break; > + default: > + usage(argv[0]); > + break; > + } > + } > + > + if (optind != argc - 2) > + usage(argv[0]); > + > + opt.host = argv[optind]; > + opt.service = argv[optind + 1]; > +} > + > +int main(int argc, char *argv[]) > +{ > + int err = 0; > + int recv_fd = -1; > + int ret_value = 0; > + __u32 recv_val; > + struct cmsghdr *cmsg; > + char cbuf[1024]; Please use CMSG_SPACE and anticipate the minimum required space needed. > + char recv_buf[1024]; > + struct iovec iov[1]; > + struct msghdr msg; > + struct sockaddr_in recv_addr; In general code should not be IPv4 only. In this case the logic in the kernel is the same for IPv4 and IPv6, so I guess it's okay. If explicitly testing only one of the two, I would suggest IPv6. > + > + parse_args(argc, argv); > + > + recv_fd = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP); > + if (recv_fd < 0) { > + perror("Can't open recv socket"); > + ret_value = -errno; > + goto cleanup; > + } > + > + err = setsockopt(recv_fd, SOL_SOCKET, opt.rcvname, &opt.val, sizeof(opt.val)); > + if (err < 0) { > + perror("Recv setsockopt error"); > + ret_value = -errno; > + goto cleanup; > + } > + > + memset(&recv_addr, 0, sizeof(recv_addr)); > + recv_addr.sin_family = AF_INET; > + recv_addr.sin_port = htons(atoi(opt.service)); > + > + if (inet_pton(AF_INET, opt.host, &recv_addr.sin_addr) <= 0) { > + perror("Invalid address"); > + ret_value = -errno; > + goto cleanup; > + } > + > + err = bind(recv_fd, (struct sockaddr *)&recv_addr, sizeof(recv_addr)); > + if (err < 0) { > + perror("Recv bind error"); > + ret_value = -errno; > + goto cleanup; > + } > + > + iov[0].iov_base = recv_buf; > + iov[0].iov_len = sizeof(recv_buf); > + > + memset(&msg, 0, sizeof(msg)); > + msg.msg_iov = iov; > + msg.msg_iovlen = 1; > + msg.msg_control = cbuf; > + msg.msg_controllen = sizeof(cbuf); > + > + err = recvmsg(recv_fd, &msg, 0); > + if (err < 0) { > + perror("Message receive error"); > + ret_value = -errno; > + goto cleanup; > + } > + > + for (cmsg = CMSG_FIRSTHDR(&msg); cmsg != NULL; cmsg = CMSG_NXTHDR(&msg, cmsg)) { > + if (cmsg->cmsg_level == SOL_SOCKET && cmsg->cmsg_type == opt.name) { > + recv_val = *(__u32 *)CMSG_DATA(cmsg); > + printf("Received value: %u\n", recv_val); > + > + if (recv_val != opt.val) { > + fprintf(stderr, "Error: expected value: %u, got: %u\n", > + opt.val, recv_val); > + ret_value = -EINVAL; > + goto cleanup; > + } > + } > + } > + > +cleanup: > + if (recv_fd >= 0) > + close(recv_fd); > + > + return ret_value; > +} > diff --git a/tools/testing/selftests/net/test_so_rcv.sh b/tools/testing/selftests/net/test_so_rcv.sh > new file mode 100755 > index 000000000000..12d37f9ab905 > --- /dev/null > +++ b/tools/testing/selftests/net/test_so_rcv.sh > @@ -0,0 +1,56 @@ > +#!/bin/bash > +# SPDX-License-Identifier: GPL-2.0 > + > +HOST=127.0.0.1 > +PORT=1234 > +TOTAL_TESTS=0 > +FAILED_TESTS=0 > + > +declare -A TESTS=( > + ["SO_RCVPRIORITY"]="-P 2" > + ["SO_RCVMARK"]="-M 3" > +) > + > +check_result() { > + ((TOTAL_TESTS++)) > + if [ "$1" -ne 0 ]; then > + ((FAILED_TESTS++)) > + fi > +} > + > +for test_name in "${!TESTS[@]}"; do > + echo "Running $test_name test" > + arg=${TESTS[$test_name]} > + > + ./so_rcv_listener $arg $HOST $PORT & > + LISTENER_PID=$! > + > + if ./cmsg_sender $arg $HOST $PORT; then > + echo "Sender succeeded for $test_name" nit: such verbose comments are not very informative. > + else > + echo "Sender failed for $test_name" > + kill "$LISTENER_PID" 2>/dev/null > + wait "$LISTENER_PID" > + check_result 1 > + continue > + fi > + > + wait "$LISTENER_PID" > + LISTENER_EXIT_CODE=$? > + > + if [ "$LISTENER_EXIT_CODE" -eq 0 ]; then > + echo "Rcv test OK for $test_name" > + check_result 0 > + else > + echo "Rcv test FAILED for $test_name" > + check_result 1 > + fi > +done > + > +if [ "$FAILED_TESTS" -ne 0 ]; then > + echo "FAIL - $FAILED_TESTS/$TOTAL_TESTS tests failed" > + exit 1 please use kselftest exit codes: KSFT_FAIL > +else > + echo "OK - All $TOTAL_TESTS tests passed" > + exit 0 KSFT_PASS > +fi > -- > 2.43.0 >
diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile index 73ee88d6b043..98f05473e672 100644 --- a/tools/testing/selftests/net/Makefile +++ b/tools/testing/selftests/net/Makefile @@ -33,6 +33,7 @@ TEST_PROGS += gro.sh TEST_PROGS += gre_gso.sh TEST_PROGS += cmsg_so_mark.sh TEST_PROGS += cmsg_so_priority.sh +TEST_PROGS += test_so_rcv.sh TEST_PROGS += cmsg_time.sh cmsg_ipv6.sh TEST_PROGS += netns-name.sh TEST_PROGS += nl_netdev.py diff --git a/tools/testing/selftests/net/so_rcv_listener.c b/tools/testing/selftests/net/so_rcv_listener.c new file mode 100644 index 000000000000..53b09582a7e3 --- /dev/null +++ b/tools/testing/selftests/net/so_rcv_listener.c @@ -0,0 +1,147 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include <errno.h> +#include <netdb.h> +#include <stdbool.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <unistd.h> +#include <linux/types.h> +#include <sys/socket.h> +#include <netinet/in.h> +#include <arpa/inet.h> + +#ifndef SO_RCVPRIORITY +#define SO_RCVPRIORITY 82 +#endif + +struct options { + __u32 val; + int name; + int rcvname; + const char *host; + const char *service; +} opt; + +static void __attribute__((noreturn)) usage(const char *bin) +{ + printf("Usage: %s [opts] <dst host> <dst port / service>\n", bin); + printf("Options:\n" + "\t\t-M val Test SO_RCVMARK\n" + "\t\t-P val Test SO_RCVPRIORITY\n" + ""); + exit(EXIT_FAILURE); +} + +static void parse_args(int argc, char *argv[]) +{ + int o; + + while ((o = getopt(argc, argv, "M:P:")) != -1) { + switch (o) { + case 'M': + opt.val = atoi(optarg); + opt.name = SO_MARK; + opt.rcvname = SO_RCVMARK; + break; + case 'P': + opt.val = atoi(optarg); + opt.name = SO_PRIORITY; + opt.rcvname = SO_RCVPRIORITY; + break; + default: + usage(argv[0]); + break; + } + } + + if (optind != argc - 2) + usage(argv[0]); + + opt.host = argv[optind]; + opt.service = argv[optind + 1]; +} + +int main(int argc, char *argv[]) +{ + int err = 0; + int recv_fd = -1; + int ret_value = 0; + __u32 recv_val; + struct cmsghdr *cmsg; + char cbuf[1024]; + char recv_buf[1024]; + struct iovec iov[1]; + struct msghdr msg; + struct sockaddr_in recv_addr; + + parse_args(argc, argv); + + recv_fd = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP); + if (recv_fd < 0) { + perror("Can't open recv socket"); + ret_value = -errno; + goto cleanup; + } + + err = setsockopt(recv_fd, SOL_SOCKET, opt.rcvname, &opt.val, sizeof(opt.val)); + if (err < 0) { + perror("Recv setsockopt error"); + ret_value = -errno; + goto cleanup; + } + + memset(&recv_addr, 0, sizeof(recv_addr)); + recv_addr.sin_family = AF_INET; + recv_addr.sin_port = htons(atoi(opt.service)); + + if (inet_pton(AF_INET, opt.host, &recv_addr.sin_addr) <= 0) { + perror("Invalid address"); + ret_value = -errno; + goto cleanup; + } + + err = bind(recv_fd, (struct sockaddr *)&recv_addr, sizeof(recv_addr)); + if (err < 0) { + perror("Recv bind error"); + ret_value = -errno; + goto cleanup; + } + + iov[0].iov_base = recv_buf; + iov[0].iov_len = sizeof(recv_buf); + + memset(&msg, 0, sizeof(msg)); + msg.msg_iov = iov; + msg.msg_iovlen = 1; + msg.msg_control = cbuf; + msg.msg_controllen = sizeof(cbuf); + + err = recvmsg(recv_fd, &msg, 0); + if (err < 0) { + perror("Message receive error"); + ret_value = -errno; + goto cleanup; + } + + for (cmsg = CMSG_FIRSTHDR(&msg); cmsg != NULL; cmsg = CMSG_NXTHDR(&msg, cmsg)) { + if (cmsg->cmsg_level == SOL_SOCKET && cmsg->cmsg_type == opt.name) { + recv_val = *(__u32 *)CMSG_DATA(cmsg); + printf("Received value: %u\n", recv_val); + + if (recv_val != opt.val) { + fprintf(stderr, "Error: expected value: %u, got: %u\n", + opt.val, recv_val); + ret_value = -EINVAL; + goto cleanup; + } + } + } + +cleanup: + if (recv_fd >= 0) + close(recv_fd); + + return ret_value; +} diff --git a/tools/testing/selftests/net/test_so_rcv.sh b/tools/testing/selftests/net/test_so_rcv.sh new file mode 100755 index 000000000000..12d37f9ab905 --- /dev/null +++ b/tools/testing/selftests/net/test_so_rcv.sh @@ -0,0 +1,56 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 + +HOST=127.0.0.1 +PORT=1234 +TOTAL_TESTS=0 +FAILED_TESTS=0 + +declare -A TESTS=( + ["SO_RCVPRIORITY"]="-P 2" + ["SO_RCVMARK"]="-M 3" +) + +check_result() { + ((TOTAL_TESTS++)) + if [ "$1" -ne 0 ]; then + ((FAILED_TESTS++)) + fi +} + +for test_name in "${!TESTS[@]}"; do + echo "Running $test_name test" + arg=${TESTS[$test_name]} + + ./so_rcv_listener $arg $HOST $PORT & + LISTENER_PID=$! + + if ./cmsg_sender $arg $HOST $PORT; then + echo "Sender succeeded for $test_name" + else + echo "Sender failed for $test_name" + kill "$LISTENER_PID" 2>/dev/null + wait "$LISTENER_PID" + check_result 1 + continue + fi + + wait "$LISTENER_PID" + LISTENER_EXIT_CODE=$? + + if [ "$LISTENER_EXIT_CODE" -eq 0 ]; then + echo "Rcv test OK for $test_name" + check_result 0 + else + echo "Rcv test FAILED for $test_name" + check_result 1 + fi +done + +if [ "$FAILED_TESTS" -ne 0 ]; then + echo "FAIL - $FAILED_TESTS/$TOTAL_TESTS tests failed" + exit 1 +else + echo "OK - All $TOTAL_TESTS tests passed" + exit 0 +fi
Introduce tests to verify the correct functionality of the SO_RCVMARK and SO_RCVPRIORITY socket options. Key changes include: - so_rcv_listener.c: Implements a receiver application to test the correct behavior of the SO_RCVMARK and SO_RCVPRIORITY options. - test_so_rcv.sh: Provides a shell script to automate testing for these options. - Makefile: Integrates test_so_rcv.sh into the kernel selftests. Suggested-by: Jakub Kicinski <kuba@kernel.org> Suggested-by: Ferenc Fejes <fejes@inf.elte.hu> Signed-off-by: Anna Emese Nyiri <annaemesenyiri@gmail.com> --- tools/testing/selftests/net/Makefile | 1 + tools/testing/selftests/net/so_rcv_listener.c | 147 ++++++++++++++++++ tools/testing/selftests/net/test_so_rcv.sh | 56 +++++++ 3 files changed, 204 insertions(+) create mode 100644 tools/testing/selftests/net/so_rcv_listener.c create mode 100755 tools/testing/selftests/net/test_so_rcv.sh