From d1a7440ecddaa2ee4c7c5c7e66c247ab2a442dde Mon Sep 17 00:00:00 2001 From: Alex Becker Date: Thu, 17 Sep 2026 10:01:50 -0500 Subject: [PATCH 1/5] fix(test): remove uninitialized var causing test crash --- .gitignore | 3 ++- test/unit_homa_rpc.c | 3 --- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/.gitignore b/.gitignore index 3d4c0094..d646a9cd 100644 --- a/.gitignore +++ b/.gitignore @@ -2,6 +2,7 @@ \#*# *.pyc *.o +*.d *.hi *.dump *.log @@ -22,4 +23,4 @@ reports/ traces/ users/ -saved_traces/ \ No newline at end of file +saved_traces/ diff --git a/test/unit_homa_rpc.c b/test/unit_homa_rpc.c index 0b042958..d80f532d 100644 --- a/test/unit_homa_rpc.c +++ b/test/unit_homa_rpc.c @@ -328,7 +328,6 @@ TEST_F(homa_rpc, homa_rpc_ack__multiple_acks) TEST_F(homa_rpc, homa_rpc_ack__release_rpc_lock) { struct homa_rpc *srpc; - struct homa_sock hsk; srpc = unit_server_rpc(&self->hsk, UNIT_OUTGOING, self->client_ip, self->server_ip, self->client_port, self->server_id, @@ -345,8 +344,6 @@ TEST_F(homa_rpc, homa_rpc_ack__release_rpc_lock) homa_rpc_ack(&self->hsk, NULL, self->client_ip, 0, NULL); EXPECT_EQ(1, mock_total_spin_locks); homa_rpc_unlock(srpc); - - unit_sock_destroy(&hsk); } TEST_F(homa_rpc, homa_rpc_ack__lookup_socket) { From 626dcba061a56017c7142d78d6ecc5219275c08a Mon Sep 17 00:00:00 2001 From: Alex Becker Date: Thu, 17 Sep 2026 10:05:58 -0500 Subject: [PATCH 2/5] fix: call tt_destroy in homa_load error block --- homa_plumbing.c | 4 ++++ test/unit_homa_plumbing.c | 9 ++++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/homa_plumbing.c b/homa_plumbing.c index 3dd4b799..3107fc7c 100644 --- a/homa_plumbing.c +++ b/homa_plumbing.c @@ -660,6 +660,10 @@ int __init homa_load(void) proto_unregister(&homav6_prot); if (init_net_ops) unregister_pernet_subsys(&homa_net_ops); +#ifndef __UPSTREAM__ /* See strip.py */ + /* Remove proc callbacks before a failed load releases module memory. */ + tt_destroy(); +#endif /* See strip.py */ return status; } diff --git a/test/unit_homa_plumbing.c b/test/unit_homa_plumbing.c index c3e8d929..e2704e85 100644 --- a/test/unit_homa_plumbing.c +++ b/test/unit_homa_plumbing.c @@ -142,8 +142,15 @@ TEST_F(homa_plumbing, homa_load__error_in_inet6_register_protosw) mock_register_protosw_errors = 1; EXPECT_EQ(EINVAL, -homa_load()); + /* Failed loading must release every timetrace buffer before + * another load is attempted. A successful retry followed by + * unload could otherwise hide missing failure-path cleanup. + */ + for (int i = 0; i < nr_cpu_ids; i++) + EXPECT_EQ(NULL, tt_buffers[i]); + /* Second attempt succeeds. */ - EXPECT_EQ(0, -homa_load()); + ASSERT_EQ(0, homa_load()); homa_unload(); } From 1f0e83a5c541db0f7e261d4e4243986b949f2301 Mon Sep 17 00:00:00 2001 From: Alex Becker Date: Thu, 17 Sep 2026 10:16:56 -0500 Subject: [PATCH 3/5] fix: ignored status from tt_init --- homa_plumbing.c | 5 ++++- test/unit_homa_plumbing.c | 24 ++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/homa_plumbing.c b/homa_plumbing.c index 3107fc7c..f1d16ff0 100644 --- a/homa_plumbing.c +++ b/homa_plumbing.c @@ -526,7 +526,10 @@ int __init homa_load(void) #endif /* See strip.py */ #ifndef __UPSTREAM__ /* See strip.py */ - tt_init("timetrace"); + status = tt_init("timetrace"); + /* tt_init cleans up its own partial initialization on failure. */ + if (status) + return status; #endif /* See strip.py */ status = homa_init(homa); diff --git a/test/unit_homa_plumbing.c b/test/unit_homa_plumbing.c index e2704e85..4b2bec4d 100644 --- a/test/unit_homa_plumbing.c +++ b/test/unit_homa_plumbing.c @@ -134,6 +134,30 @@ static void create_rpcs_hook(char *id) saved_self = NULL; } +#ifndef __UPSTREAM__ /* See strip.py */ +TEST_F(homa_plumbing, homa_load__error_in_tt_init) +{ + int result; + + homa_destroy(&self->homa); + + /* Fail the first timetrace-buffer allocation. homa_load must + * propagate tt_init's error instead of continuing initialization. + */ + mock_kmalloc_errors = 1; + result = homa_load(); + EXPECT_EQ(-1, result); + + /* Validate tt_buffers are cleaned up downstream by tt_destroy */ + for (int i = 0; i < nr_cpu_ids; i++) + EXPECT_EQ(NULL, tt_buffers[i]); + + /* Clean up if a regression allowed loading to succeed. */ + if (result == 0) + homa_unload(); +} +#endif /* See strip.py */ + TEST_F(homa_plumbing, homa_load__error_in_inet6_register_protosw) { homa_destroy(&self->homa); From 68462c21387c5145cc514fb79ce4c5a74b70e7b9 Mon Sep 17 00:00:00 2001 From: Alex Becker Date: Thu, 17 Sep 2026 11:01:29 -0500 Subject: [PATCH 4/5] fix: add unit logs to mocks for test assertions --- homa_utils.c | 2 ++ test/mock.c | 20 ++++++++++++++++---- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/homa_utils.c b/homa_utils.c index 89e165f4..f600d7e8 100644 --- a/homa_utils.c +++ b/homa_utils.c @@ -122,6 +122,8 @@ int homa_init(struct homa *homa) */ void homa_destroy(struct homa *homa) { + UNIT_LOG("; ", "homa_destroy"); + /* The order of the following cleanups matters! */ if (homa->socktab) { homa_socktab_destroy(homa->socktab, NULL); diff --git a/test/mock.c b/test/mock.c index eca8207c..5da7e122 100644 --- a/test/mock.c +++ b/test/mock.c @@ -675,6 +675,7 @@ int inet6_del_offload(const struct net_offload *prot, unsigned char protocol) int inet6_del_protocol(const struct inet6_protocol *prot, unsigned char num) { + UNIT_LOG("; ", "inet6_del_protocol %u", num); return 0; } @@ -700,7 +701,10 @@ int inet6_release(struct socket *sock) return 0; } -void inet6_unregister_protosw(struct inet_protosw *p) {} +void inet6_unregister_protosw(struct inet_protosw *p) +{ + UNIT_LOG("; ", "inet6_unregister_protosw %u", p->protocol); +} int inet_add_offload(const struct net_offload *prot, unsigned char protocol) { @@ -719,6 +723,7 @@ int inet_del_offload(const struct net_offload *prot, unsigned char protocol) int inet_del_protocol(const struct net_protocol *prot, unsigned char num) { + UNIT_LOG("; ", "inet_del_protocol %u", num); return 0; } @@ -758,7 +763,9 @@ int inet_sendmsg(struct socket *sock, struct msghdr *msg, size_t size) } void inet_unregister_protosw(struct inet_protosw *p) -{} +{ + UNIT_LOG("; ", "inet_unregister_protosw %u", p->protocol); +} void __init_swait_queue_head(struct swait_queue_head *q, const char *name, struct lock_class_key *key) @@ -1328,7 +1335,10 @@ int proto_register(struct proto *prot, int alloc_slab) return 0; } -void proto_unregister(struct proto *prot) {} +void proto_unregister(struct proto *prot) +{ + UNIT_LOG("; ", "proto_unregister %s", prot->name); +} void *__pskb_pull_tail(struct sk_buff *skb, int delta) { @@ -1767,7 +1777,9 @@ void unregister_net_sysctl_table(struct ctl_table_header *header) } void unregister_pernet_subsys(struct pernet_operations *) -{} +{ + UNIT_LOG("; ", "unregister_pernet_subsys"); +} void unregister_qdisc(struct Qdisc_ops *qops) { From e9efa99055972e6f70d061f8e4951f491b319921 Mon Sep 17 00:00:00 2001 From: Alex Becker Date: Thu, 17 Sep 2026 11:01:41 -0500 Subject: [PATCH 5/5] fix: move homa_destroy after shared resource cleanup --- homa_plumbing.c | 5 ++-- test/unit_homa_plumbing.c | 54 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/homa_plumbing.c b/homa_plumbing.c index f1d16ff0..91e09733 100644 --- a/homa_plumbing.c +++ b/homa_plumbing.c @@ -647,8 +647,6 @@ int __init homa_load(void) if (init_metrics) homa_metrics_end(); #endif /* See strip.py */ - if (init_homa) - homa_destroy(homa); if (init_protocol) inet_del_protocol(&homa_protocol, IPPROTO_HOMA); if (init_protocol6) @@ -663,6 +661,9 @@ int __init homa_load(void) proto_unregister(&homav6_prot); if (init_net_ops) unregister_pernet_subsys(&homa_net_ops); + /* Namespace cleanup still needs homa's socket and peer tables. */ + if (init_homa) + homa_destroy(homa); #ifndef __UPSTREAM__ /* See strip.py */ /* Remove proc callbacks before a failed load releases module memory. */ tt_destroy(); diff --git a/test/unit_homa_plumbing.c b/test/unit_homa_plumbing.c index 4b2bec4d..43325227 100644 --- a/test/unit_homa_plumbing.c +++ b/test/unit_homa_plumbing.c @@ -148,7 +148,7 @@ TEST_F(homa_plumbing, homa_load__error_in_tt_init) result = homa_load(); EXPECT_EQ(-1, result); - /* Validate tt_buffers are cleaned up downstream by tt_destroy */ + /* tt_init cleans up its own partial initialization on failure. */ for (int i = 0; i < nr_cpu_ids; i++) EXPECT_EQ(NULL, tt_buffers[i]); @@ -179,6 +179,58 @@ TEST_F(homa_plumbing, homa_load__error_in_inet6_register_protosw) homa_unload(); } +TEST_F(homa_plumbing, homa_load__destroy_after_unregister) +{ + const char *events[] = { + "inet_del_protocol ", + "inet6_del_protocol ", + "inet_unregister_protosw ", + "inet6_unregister_protosw ", + "proto_unregister HOMA;", + "proto_unregister HOMAv6;", + "unregister_pernet_subsys;" + }; + const char *log; + const char *destroy; + int result; + + homa_destroy(&self->homa); + + /* Exclude destruction of the fixture's Homa instance. */ + unit_log_clear(); + + /* Fail after all registrations have succeeded, so loading must + * unwind them before destroying the shared Homa state. + */ + mock_kthread_create_errors = 1; + result = homa_load(); + EXPECT_EQ(-EACCES, result); + + log = unit_log_get(); + destroy = strstr(log, "homa_destroy"); + EXPECT_NE(NULL, destroy); + + /* Require each unregistration to precede destruction, without + * constraining the order of the unregistrations themselves. + */ + for (int i = 0; i < ARRAY_SIZE(events); i++) { + const char *event = strstr(log, events[i]); + + EXPECT_NE(NULL, event); + if (event && destroy) + EXPECT_TRUE(event < destroy); + } + + /* The global Homa instance must be destroyed exactly once. */ + if (destroy) + EXPECT_EQ(NULL, strstr(destroy + strlen("homa_destroy"), + "homa_destroy")); + + /* Clean up if a regression allowed loading to succeed. */ + if (result == 0) + homa_unload(); +} + TEST_F(homa_plumbing, homa_net_exit__free_peers) { struct in6_addr addr1 = unit_get_in_addr("1.2.3.4");