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/aelivduxqd1oq...@karahi.gladserv.com
> Signed-off-by: Brett A C Sheffield <b...@librecast.net>
Reviewed-by: Willem de Bruijn <will...@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
>
