mirrors/passt
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Makefile: Hack for optimised-away store in ndp() before checksum calculation

Browse source 
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 <wquan@redhat.com>
Link: https://bugzilla.redhat.com/show_bug.cgi?id=2129713
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
This commit is contained in:
Stefano Brivio 2022-09-29 10:59:38 +02:00
parent 5290b6f13e
commit 06aa26fcf3
2 changed files with 10 additions and 0 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

7
Makefile
View file

@ -50,11 +50,18 @@ HEADERS = $(PASST_HEADERS) seccomp.h
# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78993 # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78993
# from the pointer arithmetic used from the tcp_tap_handler() path to get the # from the pointer arithmetic used from the tcp_tap_handler() path to get the
# remote connection address. # 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))) ifeq (,$(filter-out 11 12, $(shell $(CC) -dumpversion)))
ifneq (,$(filter -flto%,$(FLAGS) $(CFLAGS))) ifneq (,$(filter -flto%,$(FLAGS) $(CFLAGS)))
ifneq (,$(filter -O2,$(FLAGS) $(CFLAGS))) ifneq (,$(filter -O2,$(FLAGS) $(CFLAGS)))
FLAGS += -DTCP_HASH_NOINLINE FLAGS += -DTCP_HASH_NOINLINE
FLAGS += -DSIPHASH_20B_NOINLINE FLAGS += -DSIPHASH_20B_NOINLINE
FLAGS += -DCSUM_UNALIGNED_NO_IPA
endif endif
endif endif
endif endif

3
checksum.c
View file

@ -97,6 +97,9 @@ uint16_t csum_fold(uint32_t sum)
* *
* Return: 16-bit IPv4-style checksum * 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) uint16_t csum_unaligned(const void *buf, size_t len, uint32_t init)
{ {
return (uint16_t)~csum_fold(sum_16b(buf, len) + init); return (uint16_t)~csum_fold(sum_16b(buf, len) + init);