Thanks for the review, Willem.

On 2025-08-25 12:16, Willem de Bruijn wrote:

Brett A C Sheffield wrote:

quoted

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.

Signed-off-by: Brett A C Sheffield <redacted>
Link: https://lore.kernel.org/stable/aElivdUXqd1OqgMY@karahi.gladserv.com (local)

Thanks for adding a regression test for this.

No problem. I wrote a test for myself when bisecting the problem back in June -
makes sense to convert it to a selftest.

quoted

+/* we need to set MTU, so do this in a namespace to play nicely */
+static int create_namespace(void)
+{
+	const char *netns_path = "/proc/self/ns/net";
+	int fd;
+
+	if (unshare(CLONE_NEWNET) != 0) {
+		perror("unshare");
+		return -1;
+	}

Is this not sufficient to move the current process in its own netns?

Yes. Yes it is. Apparently I did not read the man page properly.

quoted

+	fd = open(netns_path, O_RDONLY);
+	if (fd == -1) {
+		perror("open");
+		return -1;
+	}
+
+	if (setns(fd, CLONE_NEWNET)) {
+		perror("setns");
+		return -1;
+	}
+
+	return 0;
+}
+
+static int setup(void)
+{
+	struct ifreq ifr = {0};
+	char ifname[IFNAMSIZ];
+	int fd = -1;
+	int ctl;
+
+	if (create_namespace() == -1)
+		return -1;
+
+	ctl = socket(AF_LOCAL, SOCK_STREAM, 0);
+	if (ctl == -1)
+		return -1;
+
+	memset(ifname, 0, sizeof(ifname));
+	fd = create_interface(ctl, ifname, &ifr);
+	if (fd == -1)
+		goto err_close_ctl;
+	if (disable_dad(ifname) == -1)
+		goto err_close_fd;
+	if (interface_up(ctl, ifname, &ifr) == -1)
+		goto err_close_fd;
+	if (set_mtu(ctl, ifname, &ifr) == -1)
+		goto err_close_fd;
+	usleep(10000); /* give interface a moment to wake up */

This may be racy. Wait on a more explicit signal? E.g.,
/sys/class/net/$DEV/operstate.

Good thinking. I'll try that.

quoted

+	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 s;
+
+	printf("Testing IPv6 fragmentation\n");
+	ns_fd = setup();
+	if (ns_fd == -1)
+		return 1;
+	s = socket(AF_INET6, SOCK_DGRAM, 0);
+	msg.msg_name = (struct sockaddr *)&sa;
+	msg.msg_namelen = sizeof(sa);

nit: duplicate?

Well spotted. Will fix.

Also, no local address is set. This uses the IPv6 auto assigned
address?

Correct. The test sends to a link-local scope multicast group from the autoconf
link-local address.  I'll clarify that in the comments at the top of the test.

quoted

+	rc = sendmsg(s, &msg, 0);
+	if (rc == -1) {
+		perror("send");
+		return 1;

Probably want to cleanup state both on success and failure.

Ack.

Could use KSFT_.. exit codes, though 0/1 works just as well for
kselftests in practice.

Ok.

quoted

+	} else if (rc != LARGER_THAN_MTU) {
+		fprintf(stderr, "send() returned %zi\n", rc);
+		return 1;
+	}
+	close(s);
+	close(ns_fd);
+
+	return 0;
+}
-- 
2.49.1

Thanks again - expect a v2 when I have that cleaned up and re-tested.

Cheers,


Brett
--