Skip to content

Unit test fix - #91

Open
axkum10 wants to merge 1 commit into
PlatformLab:mainfrom
axkum10:main
Open

axkum10 wants to merge 1 commit into
PlatformLab:mainfrom
axkum10:main

Conversation

@axkum10

@axkum10 axkum10 commented Sep 14, 2026

Copy link
Copy Markdown

HomaModule_main unit test fixes for newer kernels (6.12+)

Context: the unit test suite in test/ failed to build/pass on a newer
kernel (6.12). Starting point: build failed to link (pcpu_hot undefined
references), and after a first workaround only 308/804 tests passed. End
result after the fixes below: 804/804 tests pass (with ASan disabled;
see the ASan section at the end for its current status).

Files changed

  • test/Makefile
  • test/mock.h
  • test/mock.c

1. pcpu_hot undefined reference (mock.c)

Kernel 6.12 introduced struct pcpu_hot (cache-hot per-CPU fields:
current_task, preempt_count, cpu_number, top_of_stack, ...) in
arch/x86/include/asm/current.h. raw_smp_processor_id() and other inline
kernel functions reference this per-CPU variable directly, so the unit test
binary needs real storage for the symbol to link. Added to mock.c:

#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 12, 0)
struct pcpu_hot pcpu_hot;
#endif

2. Preempt-count-imbalance failures (mock.h restructure + Makefile)

Static inline functions and macros in C bake in whichever macro definition
is textually active at their definition point, not at the call site.
Many kernel headers (skbuff.h, kthread.h, sched/signal.h, etc.,
transitively pulled in by homa_impl.h before it reached the old
#include "mock.h" line) define inline functions using the real
preempt_disable()/preempt_enable()/smp_processor_id()/raw_smp_processor_id(),
because mock.h's overrides weren't active yet when those headers were first
parsed. This caused ~495 tests to fail with "preempt_disables still active
after test".

Fix:

  • Moved the WARN/WARN_ON/WARN_ON_ONCE/WARN_ONCE overrides and the
    preempt_disable/preempt_enable/smp_processor_id/raw_smp_processor_id
    overrides to the very top of mock.h (right after the include guard).
  • Added -include mock.h to the Makefile's CINCLUDES (right after
    -include $(KDIR)/include/linux/kconfig.h), forcing mock.h to be the
    first header processed for every compiled .c file (test files and
    production ../*.c files alike). CCINCLUDES (used only for
    ccutils.cc) was left unchanged.

3. Macro-redefinition-as-error (mock.h)

Once mock.h ran first, its own later object-style overrides
(HOMA_BPAGE_SIZE, HOMA_MIN_DEFAULT_PORT, net_generic,
register_net_sysctl, tcp_v4_check, tcp_v6_check, this_cpu_ptr) were
applied before the real headers/homa.h defined those symbols. When
homa_impl.h's normal include chain later pulled in the real headers, the
real definitions silently redefined the mocks — fatal under -Werror.

Fix: added early includes of the real headers, right after the new
preempt/smp override block and before mock.h's own later overrides:

#include <net/tcp.h>
#include <net/ip6_checksum.h>
#include <linux/filter.h>
#include <net/icmp.h>
#include <net/ip6_route.h>
#include <net/netns/generic.h>
#include "homa.h"
#include "homa_wire.h"

homa_wire.h was needed for a related but distinct reason: mock.h's own
function prototypes further down (e.g. mock_skb_alloc(..., struct homa_common_hdr *h, ...)) were the first-ever reference to
struct homa_common_hdr in the translation unit. In C, a struct tag whose
first appearance is inside a function prototype's parameter list gets only
function-prototype scope, not file scope — so it became a distinct,
incompatible type from the real struct homa_common_hdr later defined by
homa_wire.h, causing "incompatible pointer type" errors where the
compiler printed the identical type name on both sides of the diagnostic.
Including homa_wire.h early (so the real, complete, file-scope definition
exists first) fixed this.

4. Unused-variable warning from a kernel header (Makefile)

Pulling net/tcp.h in earlier than before (via the include chain above)
caused net/sch_generic.h's qdisc_cb_private_validate() to be parsed
earlier too. Its local variable qcb is only referenced inside
BUILD_BUG_ON(sizeof(...)), which the compiler counts as unused — a real
warning that -Wall -Werror turned into a fatal error. Added
-Wno-unused-variable to WARNS.

5. Runtime segfault in rt6_get_cookie/fib6_get_cookie_safe

After the build succeeded, ./unit segfaulted immediately
(homa_peer_gethoma_peer_allochoma_peer_reset_dst
rt6_get_cookie). mock.c's ip6_dst_lookup_flow() returns a
partially-initialized mock struct rtable (only dst.__rcuref, dst.ops,
dst.dev, dst.obsolete are set); calling the real
rt6_get_cookie() inline function on it dereferences uninitialized/garbage
fields. Diagnosed via a core dump + backtrace.

Fix: added a mock override to mock.h so the real function is never
called in unit tests:

#define rt6_get_cookie(...) 999

6. ASan removed, then restored

ASan (-fsanitize=address -fno-omit-frame-pointer in CFLAGS/CCFLAGS,
-lasan on the unit/s_unit link targets) was temporarily removed to
get a clean build/test signal while debugging the issues above, then
restored afterward. With ASan restored, compilation succeeds but linking
currently fails on the test machine used:
Please Test with asan lib on your system

@johnousterhout

Copy link
Copy Markdown
Member

The main branch is currently target at 6.17.8, so your kernel (6.12) is actually older than main's target, not newer, and it looks like most of the changes in this PR are to account for kernel interface changes. At this point I'd prefer not to add support for 6.12 (even 6.17 is getting a bit old; I'll probably switch to something newer in the not-too-distant future). Any chance that you could upgrade to 6.17.8 for your development work?

@axkum10

axkum10 commented Sep 14, 2026

Copy link
Copy Markdown
Author

Thanks @johnousterhout . I will proceed with UDP changes.
Leave this PR open for now, as I will check on Kernel upgrade

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants