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

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