From 06aa26fcf398f5d19ab46e42996190d7f95e837a Mon Sep 17 00:00:00 2001 From: Stefano Brivio Date: Thu, 29 Sep 2022 10:59:38 +0200 Subject: [PATCH] Makefile: Hack for optimised-away store in ndp() before checksum calculation With gcc 11 and 12, passing -flto, or -flto=auto, and -O2, intra-procedural optimisation gets rid of a fundamental bit in ndp(): the store of hop_limit in the IPv6 header, before the checksum is calculated, which on x86_64 looks like this: ip6hr->hop_limit = IPPROTO_ICMPV6; b8c0: c6 44 24 35 3a movb $0x3a,0x35(%rsp) Here, hop_limit is temporarily set to the protocol number, to conveniently get the IPv6 pseudo-header for ICMPv6 checksum calculation in memory. With LTO, the assignment just disappears from the binary. This is rather visible as NDP messages get a wrong checksum, namely the expected checksum plus 58, and they're ignored by the guest or in the namespace, meaning we can't get any IPv6 routes, as reported by Wenli Quan. The issue affects a significant number of distribution builds, including the ones for CentOS Stream 9, EPEL 9, Fedora >= 35, Mageia Cauldron, and openSUSE Tumbleweed. As a quick workaround, declare csum_unaligned() as "noipa" for gcc 11 and 12, with -flto and -O2. This disables inlining and cloning, which causes the assignment to be compiled again. Leave a TODO item: we should figure out if a gcc issue has already been reported, and report one otherwise. There's no apparent justification as to why the store could go away. Reported-by: Wenli Quan Link: https://bugzilla.redhat.com/show_bug.cgi?id=2129713 Signed-off-by: Stefano Brivio --- Makefile | 7 +++++++ checksum.c | 3 +++ 2 files changed, 10 insertions(+) diff --git a/Makefile b/Makefile index 1d45f17..d4b623f 100644 --- a/Makefile +++ b/Makefile @@ -50,11 +50,18 @@ HEADERS = $(PASST_HEADERS) seccomp.h # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78993 # from the pointer arithmetic used from the tcp_tap_handler() path to get the # remote connection address. +# +# TODO: With the same combination, in ndp(), gcc optimises away the store of +# hop_limit in the IPv6 header (temporarily set to the protocol number for +# convenience, to mimic the ICMPv6 checksum pseudo-header) before the call to +# csum_unaligned(). Mark csum_unaligned() as "noipa" as a quick work-around, +# while we figure out if a corresponding gcc issue has already been reported. ifeq (,$(filter-out 11 12, $(shell $(CC) -dumpversion))) ifneq (,$(filter -flto%,$(FLAGS) $(CFLAGS))) ifneq (,$(filter -O2,$(FLAGS) $(CFLAGS))) FLAGS += -DTCP_HASH_NOINLINE FLAGS += -DSIPHASH_20B_NOINLINE + FLAGS += -DCSUM_UNALIGNED_NO_IPA endif endif endif diff --git a/checksum.c b/checksum.c index acb1e3e..56ad01e 100644 --- a/checksum.c +++ b/checksum.c @@ -97,6 +97,9 @@ uint16_t csum_fold(uint32_t sum) * * Return: 16-bit IPv4-style checksum */ +#if CSUM_UNALIGNED_NO_IPA +__attribute__((__noipa__)) /* See comment in Makefile */ +#endif uint16_t csum_unaligned(const void *buf, size_t len, uint32_t init) { return (uint16_t)~csum_fold(sum_16b(buf, len) + init);