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/homa_plumbing.c b/homa_plumbing.c index 3dd4b799..91e09733 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); @@ -644,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) @@ -660,6 +661,13 @@ 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(); +#endif /* See strip.py */ return status; } 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) { diff --git a/test/unit_homa_plumbing.c b/test/unit_homa_plumbing.c index c3e8d929..43325227 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); + + /* 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]); + + /* 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); @@ -142,12 +166,71 @@ 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(); } +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"); 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) {