linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v4] selftests: net: add test for ipv6 fragmentation
@ 2025-09-01 12:37 Brett A C Sheffield
  2025-09-01 13:45 ` Willem de Bruijn
  2025-09-01 15:34 ` Jakub Kicinski
  0 siblings, 2 replies; 3+ messages in thread
From: Brett A C Sheffield @ 2025-09-01 12:37 UTC (permalink / raw)
  To: willemdebruijn.kernel
  Cc: bacs, davem, edumazet, gregkh, horms, kuba, linux-kernel,
	linux-kselftest, netdev, pabeni, shuah, willemb

Add selftest for the IPv6 fragmentation regression which affected
several stable kernels.

Commit a18dfa9925b9 ("ipv6: save dontfrag in cork") was backported to
stable without some prerequisite commits.  This caused a regression when
sending IPv6 UDP packets by preventing fragmentation and instead
returning -1 (EMSGSIZE).

Add selftest to check for this issue by attempting to send a packet
larger than the interface MTU. The packet will be fragmented on a
working kernel, with sendmsg(2) correctly returning the expected number
of bytes sent.  When the regression is present, sendmsg returns -1 and
sets errno to EMSGSIZE.

Link: https://lore.kernel.org/stable/aElivdUXqd1OqgMY@karahi.gladserv.com
Signed-off-by: Brett A C Sheffield <bacs@librecast.net>
---
v4 changes:
 - fix "else should follow close brace" (checkpatch ERROR)

v3 changes:
 - add usleep instead of busy polling on sendmsg
 - simplify error handling by using error() and leaving cleanup to O/S
 - use loopback interface - don't bother creating TAP
 - send to localhost (::1)

v2 changes:
 - remove superfluous namespace calls - unshare(2) suffices
 - remove usleep(). Don't wait for the interface to be ready, just send, and
   handle the (less likely) error case by retrying.
 - set destination address only once
 - document our use of the IPv6 link-local source address
 - send to port 9 (DISCARD) instead of 4242 (DONT PANIC)
 - ensure sockets are closed on failure paths
 - use KSFT exit codes for clarity

v3: https://lore.kernel.org/netdev/20250901112248.5218-1-bacs@librecast.net
v2: https://lore.kernel.org/netdev/20250831102908.14655-1-bacs@librecast.net
v1: https://lore.kernel.org/netdev/20250825092548.4436-3-bacs@librecast.net

 tools/testing/selftests/net/.gitignore        |   1 +
 tools/testing/selftests/net/Makefile          |   1 +
 .../selftests/net/ipv6_fragmentation.c        | 144 ++++++++++++++++++
 3 files changed, 146 insertions(+)
 create mode 100644 tools/testing/selftests/net/ipv6_fragmentation.c

diff --git a/tools/testing/selftests/net/.gitignore b/tools/testing/selftests/net/.gitignore
index 47c293c2962f..3d4b4a53dfda 100644
--- a/tools/testing/selftests/net/.gitignore
+++ b/tools/testing/selftests/net/.gitignore
@@ -16,6 +16,7 @@ ip_local_port_range
 ipsec
 ipv6_flowlabel
 ipv6_flowlabel_mgr
+ipv6_fragmentation
 log.txt
 msg_oob
 msg_zerocopy
diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index eef0b8f8a7b0..276e0481d996 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -117,6 +117,7 @@ TEST_GEN_FILES += tfo
 TEST_PROGS += tfo_passive.sh
 TEST_PROGS += broadcast_pmtu.sh
 TEST_PROGS += ipv6_force_forwarding.sh
+TEST_GEN_PROGS += ipv6_fragmentation
 TEST_PROGS += route_hint.sh
 
 # YNL files, must be before "include ..lib.mk"
diff --git a/tools/testing/selftests/net/ipv6_fragmentation.c b/tools/testing/selftests/net/ipv6_fragmentation.c
new file mode 100644
index 000000000000..6d1311e26501
--- /dev/null
+++ b/tools/testing/selftests/net/ipv6_fragmentation.c
@@ -0,0 +1,144 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Author: Brett A C Sheffield <bacs@librecast.net>
+ *
+ * Kernel selftest for the IPv6 fragmentation regression which affected stable
+ * kernels:
+ *
+ *   https://lore.kernel.org/stable/aElivdUXqd1OqgMY@karahi.gladserv.com
+ *
+ * Commit: a18dfa9925b9 ("ipv6: save dontfrag in cork") was backported to stable
+ * without some prerequisite commits.
+ *
+ * This caused a regression when sending IPv6 UDP packets by preventing
+ * fragmentation and instead returning -1 (EMSGSIZE).
+ *
+ * This selftest demonstrates the issue by sending an IPv6 UDP packet to
+ * localhost (::1) on the loopback interface from the autoconfigured link-local
+ * address.
+ *
+ * sendmsg(2) returns bytes sent correctly on a working kernel, and returns -1
+ * (EMSGSIZE) when the regression is present.
+ *
+ * The regression was not present in the mainline kernel, but add this test to
+ * catch similar breakage in future.
+ */
+
+#define _GNU_SOURCE
+
+#include <error.h>
+#include <fcntl.h>
+#include <linux/if_tun.h>
+#include <net/if.h>
+#include <netinet/in.h>
+#include <sched.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/ioctl.h>
+#include <sys/socket.h>
+#include <unistd.h>
+#include "../kselftest.h"
+
+#define MTU 1500
+#define LARGER_THAN_MTU 8192
+
+/* bring up interface */
+static int interface_up(int ctl, struct ifreq *ifr)
+{
+	if (ioctl(ctl, SIOCGIFFLAGS, ifr) == -1)
+		error(KSFT_FAIL, errno, "ioctl SIOCGIFFLAGS");
+	ifr->ifr_flags = ifr->ifr_flags | IFF_UP;
+	return ioctl(ctl, SIOCSIFFLAGS, ifr);
+}
+
+/* no need to wait for DAD in our namespace */
+static int disable_dad(char *ifname)
+{
+	char sysvar[] = "/proc/sys/net/ipv6/conf/%s/accept_dad";
+	char fname[IFNAMSIZ + sizeof(sysvar)];
+	int fd;
+
+	snprintf(fname, sizeof(fname), sysvar, ifname);
+	fd = open(fname, O_WRONLY);
+	if (fd == -1)
+		error(KSFT_FAIL, errno, "open accept_dad");
+	if (write(fd, "0", 1) != 1)
+		error(KSFT_FAIL, errno, "write accept_dad");
+
+	return close(fd);
+}
+
+static int setup(void)
+{
+	struct ifreq ifr = {
+		.ifr_name = "lo"
+	};
+	int fd = -1;
+	int ctl;
+
+	/* we need to set MTU, so do this in a namespace to play nicely */
+	if (unshare(CLONE_NEWNET) == -1)
+		error(KSFT_FAIL, errno, "unshare");
+
+	ctl = socket(AF_LOCAL, SOCK_STREAM, 0);
+	if (ctl == -1)
+		error(KSFT_FAIL, errno, "socket");
+
+	/* ensure MTU is smaller than what we plan to send */
+	ifr.ifr_mtu = MTU;
+	if (ioctl(ctl, SIOCSIFMTU, &ifr) == -1)
+		error(KSFT_FAIL, errno, "ioctl: set MTU");
+
+	disable_dad("lo");
+	interface_up(ctl, &ifr);
+
+	close(ctl);
+	return fd;
+}
+
+int main(void)
+{
+	struct in6_addr addr = {
+		.s6_addr[15] = 0x01, /* ::1 */
+	};
+	struct sockaddr_in6 sa = {
+		.sin6_family = AF_INET6,
+		.sin6_addr = addr,
+		.sin6_port = 9      /* port 9/udp (DISCARD) */
+	};
+	char buf[LARGER_THAN_MTU] = {0};
+	struct iovec iov = { .iov_base = buf, .iov_len = sizeof(buf)};
+	struct msghdr msg = {
+		.msg_iov = &iov,
+		.msg_iovlen = 1,
+		.msg_name = (struct sockaddr *)&sa,
+		.msg_namelen = sizeof(sa),
+	};
+	ssize_t rc;
+	int ns_fd;
+	int err = KSFT_FAIL;
+	int s;
+
+	printf("Testing IPv6 fragmentation\n");
+	ns_fd = setup();
+	s = socket(AF_INET6, SOCK_DGRAM, 0);
+send_again:
+	rc = sendmsg(s, &msg, 0);
+	if (rc == -1) {
+		/* if interface wasn't ready, try again */
+		if (errno == EADDRNOTAVAIL) {
+			usleep(1000);
+			goto send_again;
+		}
+		printf("[FAIL] sendmsg: %s\n", strerror(errno));
+	} else if (rc != LARGER_THAN_MTU) {
+		printf("[FAIL] sendmsg() returned %zi, expected %i\n", rc, LARGER_THAN_MTU);
+	} else {
+		printf("[PASS] sendmsg() returned %zi\n", rc);
+		err = KSFT_PASS;
+	}
+	close(s);
+	close(ns_fd);
+	return err;
+}

base-commit: 864ecc4a6dade82d3f70eab43dad0e277aa6fc78
-- 
2.49.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH net-next v4] selftests: net: add test for ipv6 fragmentation
  2025-09-01 12:37 [PATCH net-next v4] selftests: net: add test for ipv6 fragmentation Brett A C Sheffield
@ 2025-09-01 13:45 ` Willem de Bruijn
  2025-09-01 15:34 ` Jakub Kicinski
  1 sibling, 0 replies; 3+ messages in thread
From: Willem de Bruijn @ 2025-09-01 13:45 UTC (permalink / raw)
  To: Brett A C Sheffield, willemdebruijn.kernel
  Cc: bacs, davem, edumazet, gregkh, horms, kuba, linux-kernel,
	linux-kselftest, netdev, pabeni, shuah, willemb

Brett A C Sheffield wrote:
> Add selftest for the IPv6 fragmentation regression which affected
> several stable kernels.
> 
> Commit a18dfa9925b9 ("ipv6: save dontfrag in cork") was backported to
> stable without some prerequisite commits.  This caused a regression when
> sending IPv6 UDP packets by preventing fragmentation and instead
> returning -1 (EMSGSIZE).
> 
> Add selftest to check for this issue by attempting to send a packet
> larger than the interface MTU. The packet will be fragmented on a
> working kernel, with sendmsg(2) correctly returning the expected number
> of bytes sent.  When the regression is present, sendmsg returns -1 and
> sets errno to EMSGSIZE.
> 
> Link: https://lore.kernel.org/stable/aElivdUXqd1OqgMY@karahi.gladserv.com
> Signed-off-by: Brett A C Sheffield <bacs@librecast.net>

Reviewed-by: Willem de Bruijn <willemb@google.com>

> ---
> v4 changes:
>  - fix "else should follow close brace" (checkpatch ERROR)

Reminder to send only only iteration to netdev per 24 hrs.
 
> v3 changes:
>  - add usleep instead of busy polling on sendmsg
>  - simplify error handling by using error() and leaving cleanup to O/S
>  - use loopback interface - don't bother creating TAP
>  - send to localhost (::1)
 

> +/* no need to wait for DAD in our namespace */
> +static int disable_dad(char *ifname)
> +{
> +	char sysvar[] = "/proc/sys/net/ipv6/conf/%s/accept_dad";
> +	char fname[IFNAMSIZ + sizeof(sysvar)];
> +	int fd;
> +
> +	snprintf(fname, sizeof(fname), sysvar, ifname);
> +	fd = open(fname, O_WRONLY);
> +	if (fd == -1)
> +		error(KSFT_FAIL, errno, "open accept_dad");
> +	if (write(fd, "0", 1) != 1)
> +		error(KSFT_FAIL, errno, "write accept_dad");
> +
> +	return close(fd);
> +}

Is this needed with loopback?

> +int main(void)
> +{
> +	struct in6_addr addr = {
> +		.s6_addr[15] = 0x01, /* ::1 */
> +	};
> +	struct sockaddr_in6 sa = {
> +		.sin6_family = AF_INET6,
> +		.sin6_addr = addr,
> +		.sin6_port = 9      /* port 9/udp (DISCARD) */

htons

> +	};
> +	char buf[LARGER_THAN_MTU] = {0};

That's a large stack allocation. static char?

> +	ns_fd = setup();
> +	s = socket(AF_INET6, SOCK_DGRAM, 0);
> +send_again:
> +	rc = sendmsg(s, &msg, 0);
> +	if (rc == -1) {
> +		/* if interface wasn't ready, try again */
> +		if (errno == EADDRNOTAVAIL) {
> +			usleep(1000);
> +			goto send_again;
> +		}
> +		printf("[FAIL] sendmsg: %s\n", strerror(errno));
> +	} else if (rc != LARGER_THAN_MTU) {
> +		printf("[FAIL] sendmsg() returned %zi, expected %i\n", rc, LARGER_THAN_MTU);
> +	} else {
> +		printf("[PASS] sendmsg() returned %zi\n", rc);
> +		err = KSFT_PASS;

slight preference to just fail with error() in the two error cases and
let the expected common path be linear and succeed:

    if (rc == -1) {
            if (..)
                    goto send_again;
            error(KSFT_FAIL, ..);
    }
    if (rc != LARGER_THAN_MTU) {
            error(KSFT_FAIL, ..);
    }

    printf(..)
    return KSFT_PASS;

> +	}
> +	close(s);
> +	close(ns_fd);

ns_fd is always -1. Check all system calls return values. Now that
setup internally can fail with error() no need for return values at
all.

> +	return err;
> +}
> 
> base-commit: 864ecc4a6dade82d3f70eab43dad0e277aa6fc78
> -- 
> 2.49.1
> 



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net-next v4] selftests: net: add test for ipv6 fragmentation
  2025-09-01 12:37 [PATCH net-next v4] selftests: net: add test for ipv6 fragmentation Brett A C Sheffield
  2025-09-01 13:45 ` Willem de Bruijn
@ 2025-09-01 15:34 ` Jakub Kicinski
  1 sibling, 0 replies; 3+ messages in thread
From: Jakub Kicinski @ 2025-09-01 15:34 UTC (permalink / raw)
  To: Brett A C Sheffield
  Cc: willemdebruijn.kernel, davem, edumazet, gregkh, horms,
	linux-kernel, linux-kselftest, netdev, pabeni, shuah, willemb

On Mon,  1 Sep 2025 12:37:14 +0000 Brett A C Sheffield wrote:
> +static int setup(void)
> +{
> +	struct ifreq ifr = {
> +		.ifr_name = "lo"
> +	};
> +	int fd = -1;
> +	int ctl;
> +
> +	/* we need to set MTU, so do this in a namespace to play nicely */
> +	if (unshare(CLONE_NEWNET) == -1)
> +		error(KSFT_FAIL, errno, "unshare");
> +
> +	ctl = socket(AF_LOCAL, SOCK_STREAM, 0);
> +	if (ctl == -1)
> +		error(KSFT_FAIL, errno, "socket");
> +
> +	/* ensure MTU is smaller than what we plan to send */
> +	ifr.ifr_mtu = MTU;
> +	if (ioctl(ctl, SIOCSIFMTU, &ifr) == -1)
> +		error(KSFT_FAIL, errno, "ioctl: set MTU");
> +
> +	disable_dad("lo");
> +	interface_up(ctl, &ifr);
> +
> +	close(ctl);
> +	return fd;

fd is unused here
-- 
pw-bot: cr

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-09-01 15:34 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-01 12:37 [PATCH net-next v4] selftests: net: add test for ipv6 fragmentation Brett A C Sheffield
2025-09-01 13:45 ` Willem de Bruijn
2025-09-01 15:34 ` Jakub Kicinski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).