From 9c6adecb84e654f96a16a2509aae549941f7dde8 Mon Sep 17 00:00:00 2001 From: Geliang Tang Date: Fri, 17 Jan 2025 18:16:29 +0800 Subject: [PATCH] Revert "bpf local list 2" This reverts commit fff5c9c900dc5667a52c7871884ed62fc8d04f13. --- net/mptcp/bpf.c | 9 - tools/testing/selftests/bpf/Makefile | 2 +- .../testing/selftests/bpf/prog_tests/mptcp.c | 115 +++++----- .../bpf/progs/mptcp_bpf_userspace_pm.c | 209 ++++++++++-------- 4 files changed, 173 insertions(+), 162 deletions(-) diff --git a/net/mptcp/bpf.c b/net/mptcp/bpf.c index 0d6571d06bfa..17596406b455 100644 --- a/net/mptcp/bpf.c +++ b/net/mptcp/bpf.c @@ -75,15 +75,9 @@ static int bpf_mptcp_pm_btf_struct_access(struct bpf_verifier_log *log, case offsetof(struct mptcp_pm_addr_entry, addr.port): end = offsetofend(struct mptcp_pm_addr_entry, addr.port); break; - case offsetof(struct mptcp_pm_addr_entry, addr.addr.s_addr): - end = offsetofend(struct mptcp_pm_addr_entry, addr.addr.s_addr); - break; case offsetof(struct mptcp_pm_addr_entry, flags): end = offsetofend(struct mptcp_pm_addr_entry, flags); break; - case offsetof(struct mptcp_pm_addr_entry, ifindex): - end = offsetofend(struct mptcp_pm_addr_entry, ifindex); - break; default: bpf_log(log, "no write support to mptcp_pm_addr_entry at off %d\n", off); @@ -100,9 +94,6 @@ static int bpf_mptcp_pm_btf_struct_access(struct bpf_verifier_log *log, case offsetof(struct mptcp_addr_info, port): end = offsetofend(struct mptcp_addr_info, port); break; - case offsetof(struct mptcp_addr_info, addr.s_addr): - end = offsetofend(struct mptcp_addr_info, addr.s_addr); - break; default: bpf_log(log, "no write support to mptcp_addr_info at off %d\n", off); diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index d04de379e424..3bd1247a4c9c 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -42,7 +42,7 @@ srctree := $(patsubst %/,%,$(dir $(srctree))) endif CFLAGS += -g $(OPT_FLAGS) -rdynamic \ - -Werror -fno-omit-frame-pointer \ + -Wall -Werror -fno-omit-frame-pointer \ $(GENFLAGS) $(SAN_CFLAGS) $(LIBELF_CFLAGS) \ -I$(CURDIR) -I$(INCLUDE_DIR) -I$(GENDIR) -I$(LIBDIR) \ -I$(TOOLSINCDIR) -I$(TOOLSARCHINCDIR) -I$(APIDIR) -I$(OUTPUT) diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c index f7fdde36f920..482c13cc4a18 100644 --- a/tools/testing/selftests/bpf/prog_tests/mptcp.c +++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c @@ -960,41 +960,36 @@ static void run_userspace_pm(enum mptcp_pm_family family) send_byte(accept_fd); recv_byte(client_fd); -// sprintf(expect, "id 100 flags subflow %s\n", addr); -// err = userspace_pm_get_addr(token, 100, output); -// if (!ASSERT_OK(err, "userspace_pm_get_addr 100") || -// !ASSERT_STRNEQ(output, expect, sizeof(expect), "get_addr")) -// goto close_accept; - -// err = userspace_pm_set_flags(token, addr, "backup"); -// if (!ASSERT_OK(err, "userspace_pm_set_flags backup")) -// goto close_accept; - -// send_byte(client_fd); -// recv_byte(accept_fd); - -// sprintf(expect, "id 100 flags subflow,backup %s\n", addr); -// err = userspace_pm_get_addr(token, 100, output); -// if (!ASSERT_OK(err, "userspace_pm_get_addr 100") || -// !ASSERT_STRNEQ(output, expect, sizeof(expect), "get_addr")) -// goto close_accept; - -// err = userspace_pm_set_flags_by_id(token, 100, "nobackup"); -// if (!ASSERT_OK(err, "userspace_pm_set_flags_by_id nobackup")) -// goto close_accept; - -// send_byte(accept_fd); -// recv_byte(client_fd); - -// sprintf(expect, "id 100 flags subflow %s\n", addr); -// err = userspace_pm_get_addr(token, 100, output); -// if (!ASSERT_OK(err, "userspace_pm_get_addr 100") || -// !ASSERT_STRNEQ(output, expect, sizeof(expect), "get_addr")) -// goto close_accept; + sprintf(expect, "id 100 flags subflow %s\n", addr); + err = userspace_pm_get_addr(token, 100, output); + if (!ASSERT_OK(err, "userspace_pm_get_addr 100") || + !ASSERT_STRNEQ(output, expect, sizeof(expect), "get_addr")) + goto close_accept; - err = userspace_pm_dump_addr(token, output); - if (!ASSERT_OK(err, "userspace_pm_dump_addr") || - !ASSERT_STRNEQ(output, "", sizeof(output), "dump_addr")) + err = userspace_pm_set_flags(token, addr, "backup"); + if (!ASSERT_OK(err, "userspace_pm_set_flags backup")) + goto close_accept; + + send_byte(client_fd); + recv_byte(accept_fd); + + sprintf(expect, "id 100 flags subflow,backup %s\n", addr); + err = userspace_pm_get_addr(token, 100, output); + if (!ASSERT_OK(err, "userspace_pm_get_addr 100") || + !ASSERT_STRNEQ(output, expect, sizeof(expect), "get_addr")) + goto close_accept; + + err = userspace_pm_set_flags_by_id(token, 100, "nobackup"); + if (!ASSERT_OK(err, "userspace_pm_set_flags_by_id nobackup")) + goto close_accept; + + send_byte(accept_fd); + recv_byte(client_fd); + + sprintf(expect, "id 100 flags subflow %s\n", addr); + err = userspace_pm_get_addr(token, 100, output); + if (!ASSERT_OK(err, "userspace_pm_get_addr 100") || + !ASSERT_STRNEQ(output, expect, sizeof(expect), "get_addr")) goto close_accept; err = userspace_pm_rm_subflow(token, addr, 100); @@ -1009,29 +1004,29 @@ static void run_userspace_pm(enum mptcp_pm_family family) !ASSERT_STRNEQ(output, "", sizeof(output), "dump_addr")) goto close_accept; -// addr = ipv6 ? (ipv4mapped ? "::ffff:"ADDR_3 : ADDR6_3) : ADDR_3; -// err = userspace_pm_add_addr(token, addr, 200); -// if (!ASSERT_OK(err, "userspace_pm_add_addr 200")) -// goto close_accept; + addr = ipv6 ? (ipv4mapped ? "::ffff:"ADDR_3 : ADDR6_3) : ADDR_3; + err = userspace_pm_add_addr(token, addr, 200); + if (!ASSERT_OK(err, "userspace_pm_add_addr 200")) + goto close_accept; -// send_byte(accept_fd); -// recv_byte(client_fd); + send_byte(accept_fd); + recv_byte(client_fd); -// sprintf(expect, "id 200 flags signal %s\n", addr); -// err = userspace_pm_dump_addr(token, output); -// if (!ASSERT_OK(err, "userspace_pm_dump_addr") || -// !ASSERT_STRNEQ(output, expect, sizeof(expect), "dump_addr")) -// goto close_accept; + sprintf(expect, "id 200 flags signal %s\n", addr); + err = userspace_pm_dump_addr(token, output); + if (!ASSERT_OK(err, "userspace_pm_dump_addr") || + !ASSERT_STRNEQ(output, expect, sizeof(expect), "dump_addr")) + goto close_accept; -// err = userspace_pm_rm_addr(token, 200); -// if (!ASSERT_OK(err, "userspace_pm_rm_addr 200")) -// goto close_accept; + err = userspace_pm_rm_addr(token, 200); + if (!ASSERT_OK(err, "userspace_pm_rm_addr 200")) + goto close_accept; send_byte(client_fd); recv_byte(accept_fd); -// err = userspace_pm_rm_addr(token, 0); -// ASSERT_OK(err, "userspace_pm_rm_addr 0"); + err = userspace_pm_rm_addr(token, 0); + ASSERT_OK(err, "userspace_pm_rm_addr 0"); close_accept: close(accept_fd); @@ -1071,16 +1066,16 @@ static void test_bpf_path_manager(void) if (!ASSERT_OK_PTR(skel, "open: userspace_pm")) return; - //err = bpf_program__set_flags(skel->progs.mptcp_pm_address_announce, - // BPF_F_SLEEPABLE); - //err = err ?: bpf_program__set_flags(skel->progs.mptcp_pm_address_remove, - // BPF_F_SLEEPABLE); - err = bpf_program__set_flags(skel->progs.mptcp_pm_subflow_create, + err = bpf_program__set_flags(skel->progs.mptcp_pm_address_announce, + BPF_F_SLEEPABLE); + err = err ?: bpf_program__set_flags(skel->progs.mptcp_pm_address_remove, + BPF_F_SLEEPABLE); + err = err ?: bpf_program__set_flags(skel->progs.mptcp_pm_subflow_create, BPF_F_SLEEPABLE); err = err ?: bpf_program__set_flags(skel->progs.mptcp_pm_subflow_destroy, BPF_F_SLEEPABLE); - //err = err ?: bpf_program__set_flags(skel->progs.mptcp_pm_set_flags, - // BPF_F_SLEEPABLE); + err = err ?: bpf_program__set_flags(skel->progs.mptcp_pm_set_flags, + BPF_F_SLEEPABLE); if (!ASSERT_OK(err, "set sleepable flags")) goto skel_destroy; @@ -1099,7 +1094,7 @@ static void test_bpf_path_manager(void) if (!ASSERT_OK(err, "userspace_pm_init: bpf pm")) goto close_netns; - run_userspace_pm(IPV4MAPPED); + run_userspace_pm(skel->kconfig->CONFIG_MPTCP_IPV6 ? IPV6 : IPV4); userspace_pm_cleanup(); close_netns: @@ -1571,9 +1566,9 @@ static void test_stale(void) void test_mptcp(void) { -#if 0 if (test__start_subtest("connect")) test_connect(); +#if 1 if (test__start_subtest("base")) test_base(); if (test__start_subtest("mptcpify")) @@ -1586,10 +1581,8 @@ void test_mptcp(void) test_iters_address(); if (test__start_subtest("userspace_pm")) test_userspace_pm(); -#endif if (test__start_subtest("bpf_path_manager")) test_bpf_path_manager(); -#if 0 if (test__start_subtest("sockopt")) test_sockopt(); if (test__start_subtest("default")) diff --git a/tools/testing/selftests/bpf/progs/mptcp_bpf_userspace_pm.c b/tools/testing/selftests/bpf/progs/mptcp_bpf_userspace_pm.c index cc04b9a0997c..e2af17c7ae48 100644 --- a/tools/testing/selftests/bpf/progs/mptcp_bpf_userspace_pm.c +++ b/tools/testing/selftests/bpf/progs/mptcp_bpf_userspace_pm.c @@ -8,13 +8,6 @@ char _license[] SEC("license") = "GPL"; extern bool CONFIG_MPTCP_IPV6 __kconfig __weak; -struct { - __uint(type, BPF_MAP_TYPE_ARRAY); - __uint(max_entries, 256); - __type(key, int); - __type(value, struct mptcp_pm_addr_entry); -} mptcp_pm_list SEC(".maps"); - SEC("struct_ops") void BPF_PROG(mptcp_pm_init, struct mptcp_sock *msk) { @@ -27,54 +20,57 @@ void BPF_PROG(mptcp_pm_release, struct mptcp_sock *msk) { } -static void mptcp_pm_copy_entry(struct mptcp_pm_addr_entry *dst, - struct mptcp_pm_addr_entry *src) -{ - dst->flags = src->flags; - dst->ifindex = src->ifindex; - dst->addr.id = src->addr.id; - dst->addr.family = src->addr.family; - dst->addr.port = src->addr.port; - dst->addr.addr.s_addr = src->addr.addr.s_addr; -} - -static bool bpf_mptcp_addresses_equal(const struct mptcp_addr_info *a, - const struct mptcp_addr_info *b, bool use_port) -{ - bool addr_equals = false; - - if (a->family == b->family) { - //addr_equals = a->addr.s_addr == b->addr.s_addr; - addr_equals = a->id == b->id; - } - - if (!addr_equals) - return false; - if (!use_port) - return true; - - return a->port == b->port; -} - static int mptcp_userspace_pm_append_new_local_addr(struct mptcp_sock *msk, struct mptcp_pm_addr_entry *entry, bool needs_id) { + struct mptcp_pm_addr_id_bitmap id_bitmap; + struct sock *sk = (struct sock *)msk; struct mptcp_pm_addr_entry *e; - struct mptcp_pm_addr_entry new_entry; - __u32 key; + bool addr_match = false; + bool id_match = false; + int ret = -EINVAL; + + bpf_bitmap_zero(&id_bitmap, MPTCP_PM_MAX_ADDR_ID + 1); + + bpf_spin_lock_bh(&msk->pm.lock); + bpf_for_each(mptcp_userspace_pm_addr, e, msk) { + addr_match = mptcp_addresses_equal(&e->addr, &entry->addr, true); + if (addr_match && entry->addr.id == 0 && needs_id) + entry->addr.id = e->addr.id; + id_match = (e->addr.id == entry->addr.id); + if (addr_match || id_match) + break; + bpf_set_bit(e->addr.id, &id_bitmap); + } + + if (!addr_match && !id_match) { + /* Memory for the entry is allocated from the + * sock option buffer. + */ + e = bpf_sock_kmalloc_entry(sk, sizeof(*e), GFP_ATOMIC); + if (!e) { + ret = -ENOMEM; + goto append_err; + } - e = bpf_map_lookup_elem(&mptcp_pm_list, &key); - if (e) - return e->addr.id; + bpf_pm_copy_entry(e, entry); + if (!e->addr.id && needs_id) + e->addr.id = bpf_find_next_zero_bit(&id_bitmap, + MPTCP_PM_MAX_ADDR_ID + 1, + 1); + bpf_list_add_tail_rcu(&e->list, &msk->pm.userspace_pm_local_addr_list); + msk->pm.local_addr_used++; + ret = e->addr.id; + } else if (addr_match && id_match) { + ret = entry->addr.id; + } - mptcp_pm_copy_entry(&new_entry, entry); - key = new_entry.addr.id; - bpf_map_update_elem(&mptcp_pm_list, &key, &new_entry, 0); - return entry->addr.id; +append_err: + bpf_spin_unlock_bh(&msk->pm.lock); + return ret; } -#if 0 SEC("struct_ops") int BPF_PROG(mptcp_pm_address_announce, struct mptcp_sock *msk, struct mptcp_pm_addr_entry *local) @@ -99,53 +95,80 @@ int BPF_PROG(mptcp_pm_address_announce, struct mptcp_sock *msk, return 0; } -#endif + +static int mptcp_pm_remove_id_zero_address(struct mptcp_sock *msk) +{ + struct mptcp_rm_list list = { .nr = 0 }; + struct mptcp_subflow_context *subflow; + bool has_id_0 = false; + + mptcp_for_each_subflow(msk, subflow) { + subflow = bpf_core_cast(subflow, struct mptcp_subflow_context); + if (subflow->local_id == 0) { + has_id_0 = true; + break; + } + } + if (!has_id_0) + return -EINVAL; + + list.ids[list.nr++] = 0; + + bpf_spin_lock_bh(&msk->pm.lock); + mptcp_pm_remove_addr(msk, &list); + bpf_spin_unlock_bh(&msk->pm.lock); + + return 0; +} static struct mptcp_pm_addr_entry * mptcp_userspace_pm_lookup_addr_by_id(struct mptcp_sock *msk, unsigned int id) { struct mptcp_pm_addr_entry *entry; - __u32 key = id; - - entry = bpf_map_lookup_elem(&mptcp_pm_list, &key); - if (entry) - return entry; + bpf_for_each(mptcp_userspace_pm_addr, entry, msk) { + if (entry->addr.id == id) + return entry; + } return NULL; } -#if 0 SEC("struct_ops") int BPF_PROG(mptcp_pm_address_remove, struct mptcp_sock *msk, u8 id) { struct mptcp_pm_addr_entry *entry; - __u32 key = id; - entry = bpf_map_lookup_elem(&mptcp_pm_list, &key); - if (entry) { - //mptcp_pm_remove_addr_entry(msk, entry); - bpf_map_delete_elem(&mptcp_pm_list, &key); + if (id == 0) + return mptcp_pm_remove_id_zero_address(msk); + + bpf_spin_lock_bh(&msk->pm.lock); + entry = mptcp_userspace_pm_lookup_addr_by_id(msk, id); + if (!entry) { + bpf_spin_unlock_bh(&msk->pm.lock); + return -EINVAL; } + bpf_list_del_rcu(&entry->list); + bpf_spin_unlock_bh(&msk->pm.lock); + + mptcp_pm_remove_addr_entry(msk, entry); + + bpf_sock_kfree_entry((struct sock *)msk, entry, sizeof(*entry)); + bpf_printk("2 mptcp_pm_address_remove done"); return 0; } -#endif static struct mptcp_pm_addr_entry * mptcp_userspace_pm_lookup_addr(struct mptcp_sock *msk, const struct mptcp_addr_info *addr) { struct mptcp_pm_addr_entry *entry; - __u32 key; - for (key = 0; key < 256 && can_loop; key++) { - entry = bpf_map_lookup_elem(&mptcp_pm_list, &key); - if (entry) { - if (bpf_mptcp_addresses_equal(&entry->addr, addr, false)) - return entry; - } + bpf_for_each(mptcp_userspace_pm_addr, entry, msk) { + if (mptcp_addresses_equal(&entry->addr, addr, false)) + return entry; } return NULL; } @@ -153,15 +176,15 @@ mptcp_userspace_pm_lookup_addr(struct mptcp_sock *msk, static int mptcp_userspace_pm_delete_local_addr(struct mptcp_sock *msk, struct mptcp_pm_addr_entry *addr) { + struct sock *sk = (struct sock *)msk; struct mptcp_pm_addr_entry *entry; - __u32 key = addr->addr.id; entry = mptcp_userspace_pm_lookup_addr(msk, &addr->addr); if (!entry) - ;//return -EINVAL; + return -EINVAL; - bpf_printk("mptcp_userspace_pm_delete_local_addr %d", addr->addr.id); - bpf_map_delete_elem(&mptcp_pm_list, &key); + bpf_list_del_rcu(&entry->list); + bpf_sock_kfree_entry(sk, entry, sizeof(*entry)); msk->pm.local_addr_used--; return 0; } @@ -185,7 +208,7 @@ int BPF_PROG(mptcp_pm_subflow_create, struct mptcp_sock *msk, msk->pm.subflows++; bpf_spin_unlock_bh(&msk->pm.lock); - bpf_printk("3 mptcp_pm_subflow_create done err=%d", err); + bpf_printk("3 mptcp_pm_subflow_create done"); return err; } @@ -252,7 +275,6 @@ int BPF_PROG(mptcp_pm_subflow_destroy, struct mptcp_sock *msk, return -EINVAL; bpf_spin_lock_bh(&msk->pm.lock); - bpf_printk("4 mptcp_pm_subflow_destroy id=%d", local->addr.id); mptcp_userspace_pm_delete_local_addr(msk, local); bpf_spin_unlock_bh(&msk->pm.lock); mptcp_subflow_shutdown(sk, ssk, RCV_SHUTDOWN | SEND_SHUTDOWN); @@ -264,7 +286,6 @@ int BPF_PROG(mptcp_pm_subflow_destroy, struct mptcp_sock *msk, return 0; } -#if 0 SEC("struct_ops") int BPF_PROG(mptcp_pm_get_local_id, struct mptcp_sock *msk, struct mptcp_pm_addr_entry *skc) @@ -319,29 +340,26 @@ int BPF_PROG(mptcp_pm_set_flags, struct mptcp_sock *msk, entry = lookup_by_id ? mptcp_userspace_pm_lookup_addr_by_id(msk, local->addr.id) : mptcp_userspace_pm_lookup_addr(msk, &local->addr); if (entry) { -#if 0 if (bkup) entry->flags |= MPTCP_PM_ADDR_FLAG_BACKUP; else entry->flags &= ~MPTCP_PM_ADDR_FLAG_BACKUP; -#endif } bpf_spin_unlock_bh(&msk->pm.lock); return mptcp_pm_nl_mp_prio_send_ack(msk, entry ? &entry->addr : &local->addr, remote, bkup); } -#endif SEC(".struct_ops.link") struct mptcp_pm_ops userspace_pm = { - //.address_announce = (void *)mptcp_pm_address_announce, - //.address_remove = (void *)mptcp_pm_address_remove, + .address_announce = (void *)mptcp_pm_address_announce, + .address_remove = (void *)mptcp_pm_address_remove, .subflow_create = (void *)mptcp_pm_subflow_create, .subflow_destroy = (void *)mptcp_pm_subflow_destroy, - //.get_local_id = (void *)mptcp_pm_get_local_id, - //.get_flags = (void *)mptcp_pm_get_flags, - //.set_flags = (void *)mptcp_pm_set_flags, + .get_local_id = (void *)mptcp_pm_get_local_id, + .get_flags = (void *)mptcp_pm_get_flags, + .set_flags = (void *)mptcp_pm_set_flags, .init = (void *)mptcp_pm_init, .release = (void *)mptcp_pm_release, .type = MPTCP_PM_TYPE_BPF_USERSPACE, @@ -362,7 +380,7 @@ int BPF_PROG(mptcp_pm_get_addr, struct mptcp_sock *msk, u8 id, bpf_spin_lock_bh(&msk->pm.lock); entry = mptcp_userspace_pm_lookup_addr_by_id(msk, id); if (entry) - ;//mptcp_pm_copy_entry(addr, entry); + bpf_pm_copy_entry(addr, entry); bpf_spin_unlock_bh(&msk->pm.lock); bpf_mptcp_sock_release(msk); @@ -373,19 +391,28 @@ SEC("fmod_ret/mptcp_userspace_pm_dump_addr_msk") int BPF_PROG(mptcp_pm_dump_addr, struct mptcp_sock *msk, struct sk_buff *msg, struct netlink_callback *cb) { + struct mptcp_pm_addr_id_bitmap *bitmap; struct mptcp_pm_addr_entry *entry; - __u32 key; - bpf_printk("9 mptcp_pm_dump_addr"); + bitmap = netlink_callback_get_bitmap(cb); - for (key = 0; key < 256 && can_loop; key++) { - entry = bpf_map_lookup_elem(&mptcp_pm_list, &key); - if (entry && entry->addr.id) { - bpf_printk("id=%d", entry->addr.id); - ;//if (mptcp_pm_genl_fill_addr(msg, cb, entry)) - // break; - } + msk = bpf_mptcp_sock_acquire(msk); + if (!msk) + return 0; + + bpf_spin_lock_bh(&msk->pm.lock); + bpf_for_each(mptcp_userspace_pm_addr, entry, msk) { + if (bpf_test_bit(entry->addr.id, bitmap)) + continue; + + bpf_printk("9 mptcp_pm_dump_addr id=%d", entry->addr.id); + if (mptcp_pm_genl_fill_addr(msg, cb, entry)) + break; + + bpf_set_bit(entry->addr.id, bitmap); } + bpf_spin_unlock_bh(&msk->pm.lock); + bpf_mptcp_sock_release(msk); return msg->len; }