Build failure with test_add_ra_header on aarch64 #200
Comments
|
### Alpine abuild terminal output
```
+ doas -u builder abuild -r
>>> radvd: Building opt/radvd 9000-r1 (using abuild 3.10.0-r0) started Fri, 06 Jan 2023 21:11:56 +0000
>>> radvd: Checking sanity of /opt/radvd-git/APKBUILD...
>>> radvd: Analyzing dependencies...
>>> radvd: Installing for build: build-base flex bison libdaemon-dev linux-headers check-dev libbsd-dev autoconf automake
WARNING: Ignoring /home/builder/packages//opt: No such file or directory
(1/19) Installing m4 (1.4.19-r2)
(2/19) Installing flex (2.6.4-r3)
(3/19) Installing bison (3.8.2-r0)
...
(14/19) Installing autoconf (2.71-r2)
(15/19) Installing automake (1.16.5-r1)
...
… autoreconf: export WARNINGS=
autoreconf: Entering directory '.'
autoreconf: configure.ac: not using Gettext
autoreconf: running: aclocal --force
autoreconf: configure.ac: tracing
autoreconf: configure.ac: not using Libtool
autoreconf: configure.ac: not using Intltool
autoreconf: configure.ac: not using Gtkdoc
autoreconf: running: /usr/bin/autoconf --force
configure.ac:18: warning: The macro `AC_CANONICAL_SYSTEM' is obsolete.
configure.ac:18: You should run autoupdate.
./lib/autoconf/general.m4:2081: AC_CANONICAL_SYSTEM is expanded from...
configure.ac:18: the top level
configure.ac:50: warning: The macro `AC_PROG_CC_C99' is obsolete.
configure.ac:50: You should run autoupdate.
./lib/autoconf/c.m4:1659: AC_PROG_CC_C99 is expanded from...
configure.ac:50: the top level
configure.ac:67: warning: The macro `AC_TRY_COMPILE' is obsolete.
configure.ac:67: You should run autoupdate.
./lib/autoconf/general.m4:2847: AC_TRY_COMPILE is expanded from...
configure.ac:67: the top level
configure.ac:112: warning: AC_PROG_LEX without either yywrap or noyywrap is obsolete
./lib/autoconf/programs.m4:716: _AC_PROG_LEX is expanded from...
./lib/autoconf/programs.m4:709: AC_PROG_LEX is expanded from...
aclocal.m4:1074: AM_PROG_LEX is expanded from...
configure.ac:112: the top level
configure.ac:162: warning: The macro `AC_HEADER_STDC' is obsolete.
configure.ac:162: You should run autoupdate.
./lib/autoconf/headers.m4:704: AC_HEADER_STDC is expanded from...
configure.ac:162: the top level
configure.ac:177: warning: The macro `AC_HEADER_TIME' is obsolete.
configure.ac:177: You should run autoupdate.
./lib/autoconf/headers.m4:743: AC_HEADER_TIME is expanded from...
configure.ac:177: the top level
configure.ac:181: warning: The macro `AC_TRY_COMPILE' is obsolete.
configure.ac:181: You should run autoupdate.
./lib/autoconf/general.m4:2847: AC_TRY_COMPILE is expanded from...
configure.ac:181: the top level
configure.ac:188: warning: The macro `AC_TRY_COMPILE' is obsolete.
configure.ac:188: You should run autoupdate.
./lib/autoconf/general.m4:2847: AC_TRY_COMPILE is expanded from...
configure.ac:188: the top level
configure.ac:245: warning: 'AM_CONFIG_HEADER': this macro is obsolete.
configure.ac:245: You should use the 'AC_CONFIG_HEADERS' macro instead.
./lib/autoconf/general.m4:2434: AC_DIAGNOSE is expanded from...
aclocal.m4:1169: AM_CONFIG_HEADER is expanded from...
configure.ac:245: the top level
configure.ac:246: warning: AC_OUTPUT should be used without arguments.
configure.ac:246: You should run autoupdate.
|
|
Hi @stappersg Thanks for the reply. I'm struggling to see any comments you added so it's possible that GitHub didn't preserve them too well from the email. There's definitely a number of warnings produced during the build process which I think(?) you've highlighted here, though the main thing I would note is that the same code (and APKBUILD) does build and test successfully on amd64 including the same warning output, so I am mostly interested in what might be different between aarch64 and amd64 that's causing the ARM build to fail during the test phase. |
|
Can you do this on your Alpine AArch64 env? I wonder if we've got a subtle host byte ordering bug that crept in. |
|
As for the cdefs warning, that's clearly something musl-specific. I can't reproduce on glibc and I don't have a musl env handy right now to compare. |
|
Ah, FUN. radvd itself doesn't include cdefs.h, but |
Thanks for looking into this! Full output from that command below:
That's fair. I'll see if I can report the bug upstream or at least notify the maintainer of libbsd-dev in Alpine, but I suspect I might not know enough about the issue to produce a helpful bug report, so might just have to direct them to this thread and hope. What's interesting though is that the cdefs.h compiler warning disappears if I build the older commit that merged the code I want (#179). Building a646066 doesn't produce that warning, but building the latest 5482717 does. So it seems that although the issue might belong to libbsd upstream, something changed in radvd somewhere between those two commits to cause it to start manifesting. |
FWIW: In #199 is a change with |
|
That's weird. The expected output is exactly what it should be. I'm wondering what else is going on. |
|
Ok, I think I have a reproduction, and it's lack of isolation in testing that is causing the failure. In your environment, does changing |
|
Ah I think you've found it here! All my previous tests have been with IPv6 forwarding enabled (predictably, this host is intended for routing so I've been testing it in this build environment). Once I disabled it as above, the package builds successfully. We still get the |
|
Great success! Thanks @robbat2 👍 Fully built and working on aarch64 musl Alpine with the latest 5482717 commit. I didn't actually need to change anything from the original build attempt in my first post, apart from building with the The built package installs happily, and is working as expected to send the pref64 that I've been wanting, as I've validated on-the-wire with Wireshark:
|
|
Yup, so false positive because the testcase didn't properly isolate itself from the system state. I'll improve the tests. |
robbat2
added a commit
to robbat2/radvd
that referenced
this issue
Jan 9, 2023
The add_ra_header function had a hidden dependency on system state: If cease_adv was not set, the outcome was dependent on the sysctl knob "net.ipv6.conf.all.forwarding". This was visible only during testing with net.ipv6.conf.all.forwarding=1. If tested with net.ipv6.conf.all.forwarding=0, the tests passed. Hoist the sysctl dependency out of the add_ra_header function, to enable testing of both code paths, and add the extra test to validate both states of the cease_adv input. Closes: radvd-project#200 Signed-off-by: Robin H. Johnson <[email protected]>
robbat2
added a commit
to robbat2/radvd
that referenced
this issue
Jan 9, 2023
The add_ra_header function had a hidden dependency on system state: If cease_adv was not set, the outcome was dependent on the sysctl knob "net.ipv6.conf.all.forwarding". This was visible only during testing with net.ipv6.conf.all.forwarding=1. If tested with net.ipv6.conf.all.forwarding=0, the tests passed. Hoist the sysctl dependency out of the add_ra_header function, to enable testing of both code paths, and add the extra test to validate both states of the cease_adv input. Closes: radvd-project#200 Signed-off-by: Robin H. Johnson <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Issue Overview
With the lack of release for a while (noted in #185), I'm trying to build a release of
radvdwhich is new enough to include at least #179.I can get this to work on x86_64, including using the latest 5482717 commit, but I'm having an issue with the
test_add_ra_headertest failing when I try and build it onaarch64- specifically Alpine Linux Edge on a Raspberry Pi 4.The following test seems to be the one that fails according to
test-suite.log:If anyone is interested in the APKBUILD I am using to try and build this on Alpine, along with the source package, I uploaded it here: https://github.com/alexhaydock/radvd-git
I've tried building both the latest commit, as well as the commit that merged #179, but both fail with the same error on aarch64.
Full
test-suite.logoutputAlpine abuild terminal output
The text was updated successfully, but these errors were encountered: