* [PATCH bpf-next v6 0/5] bpf: Add bpf_link support for sk_msg and sk_skb progs
@ 2024-04-08 15:24 Yonghong Song
2024-04-08 15:24 ` [PATCH bpf-next v6 1/5] " Yonghong Song
` (4 more replies)
0 siblings, 5 replies; 9+ messages in thread
From: Yonghong Song @ 2024-04-08 15:24 UTC (permalink / raw
To: bpf
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Jakub Sitnicki, John Fastabend, kernel-team, Martin KaFai Lau
One of our internal services started to use sk_msg program and currently
it used existing prog attach/detach2 as demonstrated in selftests.
But attach/detach of all other bpf programs are based on bpf_link.
Consistent attach/detach APIs for all programs will make things easy to
undersand and less error prone. So this patch added bpf_link
support for BPF_PROG_TYPE_SK_MSG. Based on comments from
previous RFC patch, I added BPF_PROG_TYPE_SK_SKB support as well
as both program types have similar treatment w.r.t. bpf_link
handling.
For the patch series, patch 1 added kernel support. Patch 2
added libbpf support. Patch 3 added bpftool support and
patches 4/5 added some new tests.
Changelogs:
v5 -> v6:
- resolve libbpf conflict due to recent upstream change.
- add a bpf_link_create() test.
- some code refactoring for better code quality.
v4 -> v5:
- increase scope of mutex protection in link_release.
- remove previous-leftover entry in libbpf.map.
- make some code changes for better understanding.
v3 -> v4:
- use a single mutex lock to protect both attach/detach/update
and release/fill_info/show_fdinfo.
- simplify code for sock_map_link_lookup().
- fix a few bugs.
- add more tests.
v2 -> v3:
- consolidate link types of sk_msg and sk_skb to
a single link type BPF_PROG_TYPE_SOCKMAP.
- fix bpf_link lifecycle issue. in v2, after bpf_link
is attached, a subsequent prog_attach could change
that bpf_link. this patch makes bpf_link having
correct behavior such that it won't go away facing
other prog/link attach for the same map and the same
attach type.
Yonghong Song (5):
bpf: Add bpf_link support for sk_msg and sk_skb progs
libbpf: Add bpf_link support for BPF_PROG_TYPE_SOCKMAP
bpftool: Add link dump support for BPF_LINK_TYPE_SOCKMAP
selftests/bpf: Refactor out helper functions for a few tests
selftests/bpf: Add some tests with new bpf_program__attach_sockmap()
APIs
include/linux/bpf.h | 6 +
include/linux/skmsg.h | 4 +
include/uapi/linux/bpf.h | 5 +
kernel/bpf/syscall.c | 4 +
net/core/sock_map.c | 261 ++++++++++++++++--
tools/bpf/bpftool/link.c | 9 +
tools/include/uapi/linux/bpf.h | 5 +
tools/lib/bpf/libbpf.c | 7 +
tools/lib/bpf/libbpf.h | 2 +
tools/lib/bpf/libbpf.map | 1 +
.../selftests/bpf/prog_tests/sockmap_basic.c | 171 +++++++++++-
.../selftests/bpf/prog_tests/sockmap_listen.c | 38 +++
.../bpf/progs/test_skmsg_load_helpers.c | 27 +-
.../bpf/progs/test_sockmap_pass_prog.c | 17 +-
.../progs/test_sockmap_skb_verdict_attach.c | 2 +-
15 files changed, 524 insertions(+), 35 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH bpf-next v6 1/5] bpf: Add bpf_link support for sk_msg and sk_skb progs
2024-04-08 15:24 [PATCH bpf-next v6 0/5] bpf: Add bpf_link support for sk_msg and sk_skb progs Yonghong Song
@ 2024-04-08 15:24 ` Yonghong Song
2024-04-09 9:52 ` Eduard Zingerman
2024-04-08 15:24 ` [PATCH bpf-next v6 2/5] libbpf: Add bpf_link support for BPF_PROG_TYPE_SOCKMAP Yonghong Song
` (3 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: Yonghong Song @ 2024-04-08 15:24 UTC (permalink / raw
To: bpf
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Jakub Sitnicki, John Fastabend, kernel-team, Martin KaFai Lau
Add bpf_link support for sk_msg and sk_skb programs. We have an
internal request to support bpf_link for sk_msg programs so user
space can have a uniform handling with bpf_link based libbpf
APIs. Using bpf_link based libbpf API also has a benefit which
makes system robust by decoupling prog life cycle and
attachment life cycle.
Reviewed-by: John Fastabend <john.fastabend@gmail.com>
Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
include/linux/bpf.h | 6 +
include/linux/skmsg.h | 4 +
include/uapi/linux/bpf.h | 5 +
kernel/bpf/syscall.c | 4 +
net/core/sock_map.c | 261 +++++++++++++++++++++++++++++++--
tools/include/uapi/linux/bpf.h | 5 +
6 files changed, 269 insertions(+), 16 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 62762390c93d..5034c1b4ded7 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -2996,6 +2996,7 @@ int sock_map_prog_detach(const union bpf_attr *attr, enum bpf_prog_type ptype);
int sock_map_update_elem_sys(struct bpf_map *map, void *key, void *value, u64 flags);
int sock_map_bpf_prog_query(const union bpf_attr *attr,
union bpf_attr __user *uattr);
+int sock_map_link_create(const union bpf_attr *attr, struct bpf_prog *prog);
void sock_map_unhash(struct sock *sk);
void sock_map_destroy(struct sock *sk);
@@ -3094,6 +3095,11 @@ static inline int sock_map_bpf_prog_query(const union bpf_attr *attr,
{
return -EINVAL;
}
+
+static inline int sock_map_link_create(const union bpf_attr *attr, struct bpf_prog *prog)
+{
+ return -EOPNOTSUPP;
+}
#endif /* CONFIG_BPF_SYSCALL */
#endif /* CONFIG_NET && CONFIG_BPF_SYSCALL */
diff --git a/include/linux/skmsg.h b/include/linux/skmsg.h
index e65ec3fd2799..9c8dd4c01412 100644
--- a/include/linux/skmsg.h
+++ b/include/linux/skmsg.h
@@ -58,6 +58,10 @@ struct sk_psock_progs {
struct bpf_prog *stream_parser;
struct bpf_prog *stream_verdict;
struct bpf_prog *skb_verdict;
+ struct bpf_link *msg_parser_link;
+ struct bpf_link *stream_parser_link;
+ struct bpf_link *stream_verdict_link;
+ struct bpf_link *skb_verdict_link;
};
enum sk_psock_state_bits {
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 6fe9f11c8abe..cee0a7915c08 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -1135,6 +1135,7 @@ enum bpf_link_type {
BPF_LINK_TYPE_TCX = 11,
BPF_LINK_TYPE_UPROBE_MULTI = 12,
BPF_LINK_TYPE_NETKIT = 13,
+ BPF_LINK_TYPE_SOCKMAP = 14,
__MAX_BPF_LINK_TYPE,
};
@@ -6724,6 +6725,10 @@ struct bpf_link_info {
__u32 ifindex;
__u32 attach_type;
} netkit;
+ struct {
+ __u32 map_id;
+ __u32 attach_type;
+ } sockmap;
};
} __attribute__((aligned(8)));
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index e44c276e8617..7d392ec83655 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -5213,6 +5213,10 @@ static int link_create(union bpf_attr *attr, bpfptr_t uattr)
case BPF_PROG_TYPE_SK_LOOKUP:
ret = netns_bpf_link_create(attr, prog);
break;
+ case BPF_PROG_TYPE_SK_MSG:
+ case BPF_PROG_TYPE_SK_SKB:
+ ret = sock_map_link_create(attr, prog);
+ break;
#ifdef CONFIG_NET
case BPF_PROG_TYPE_XDP:
ret = bpf_xdp_link_attach(attr, prog);
diff --git a/net/core/sock_map.c b/net/core/sock_map.c
index 27d733c0f65e..2deda1637752 100644
--- a/net/core/sock_map.c
+++ b/net/core/sock_map.c
@@ -24,8 +24,16 @@ struct bpf_stab {
#define SOCK_CREATE_FLAG_MASK \
(BPF_F_NUMA_NODE | BPF_F_RDONLY | BPF_F_WRONLY)
+/* This mutex is used to
+ * - protect race between prog/link attach/detach and link prog update, and
+ * - protect race between releasing and accessing map in bpf_link.
+ * A single global mutex lock is used since it is expected contention is low.
+ */
+static DEFINE_MUTEX(sockmap_mutex);
+
static int sock_map_prog_update(struct bpf_map *map, struct bpf_prog *prog,
- struct bpf_prog *old, u32 which);
+ struct bpf_prog *old, struct bpf_link *link,
+ u32 which);
static struct sk_psock_progs *sock_map_progs(struct bpf_map *map);
static struct bpf_map *sock_map_alloc(union bpf_attr *attr)
@@ -71,7 +79,9 @@ int sock_map_get_from_fd(const union bpf_attr *attr, struct bpf_prog *prog)
map = __bpf_map_get(f);
if (IS_ERR(map))
return PTR_ERR(map);
- ret = sock_map_prog_update(map, prog, NULL, attr->attach_type);
+ mutex_lock(&sockmap_mutex);
+ ret = sock_map_prog_update(map, prog, NULL, NULL, attr->attach_type);
+ mutex_unlock(&sockmap_mutex);
fdput(f);
return ret;
}
@@ -103,7 +113,9 @@ int sock_map_prog_detach(const union bpf_attr *attr, enum bpf_prog_type ptype)
goto put_prog;
}
- ret = sock_map_prog_update(map, NULL, prog, attr->attach_type);
+ mutex_lock(&sockmap_mutex);
+ ret = sock_map_prog_update(map, NULL, prog, NULL, attr->attach_type);
+ mutex_unlock(&sockmap_mutex);
put_prog:
bpf_prog_put(prog);
put_map:
@@ -1454,55 +1466,84 @@ static struct sk_psock_progs *sock_map_progs(struct bpf_map *map)
return NULL;
}
-static int sock_map_prog_lookup(struct bpf_map *map, struct bpf_prog ***pprog,
- u32 which)
+static int sock_map_prog_link_lookup(struct bpf_map *map, struct bpf_prog ***pprog,
+ struct bpf_link ***plink, u32 which)
{
struct sk_psock_progs *progs = sock_map_progs(map);
+ struct bpf_prog **cur_pprog;
+ struct bpf_link **cur_plink;
if (!progs)
return -EOPNOTSUPP;
switch (which) {
case BPF_SK_MSG_VERDICT:
- *pprog = &progs->msg_parser;
+ cur_pprog = &progs->msg_parser;
+ cur_plink = &progs->msg_parser_link;
break;
#if IS_ENABLED(CONFIG_BPF_STREAM_PARSER)
case BPF_SK_SKB_STREAM_PARSER:
- *pprog = &progs->stream_parser;
+ cur_pprog = &progs->stream_parser;
+ cur_plink = &progs->stream_parser_link;
break;
#endif
case BPF_SK_SKB_STREAM_VERDICT:
if (progs->skb_verdict)
return -EBUSY;
- *pprog = &progs->stream_verdict;
+ cur_pprog = &progs->stream_verdict;
+ cur_plink = &progs->stream_verdict_link;
break;
case BPF_SK_SKB_VERDICT:
if (progs->stream_verdict)
return -EBUSY;
- *pprog = &progs->skb_verdict;
+ cur_pprog = &progs->skb_verdict;
+ cur_plink = &progs->skb_verdict_link;
break;
default:
return -EOPNOTSUPP;
}
+ *pprog = cur_pprog;
+ if (plink)
+ *plink = cur_plink;
return 0;
}
+/* Handle the following four cases:
+ * prog_attach: prog != NULL, old == NULL, link == NULL
+ * prog_detach: prog == NULL, old != NULL, link == NULL
+ * link_attach: prog != NULL, old == NULL, link != NULL
+ * link_detach: prog == NULL, old != NULL, link != NULL
+ */
static int sock_map_prog_update(struct bpf_map *map, struct bpf_prog *prog,
- struct bpf_prog *old, u32 which)
+ struct bpf_prog *old, struct bpf_link *link,
+ u32 which)
{
struct bpf_prog **pprog;
+ struct bpf_link **plink;
int ret;
- ret = sock_map_prog_lookup(map, &pprog, which);
+ ret = sock_map_prog_link_lookup(map, &pprog, &plink, which);
if (ret)
return ret;
- if (old)
- return psock_replace_prog(pprog, prog, old);
+ /* for prog_attach/prog_detach/link_attach, return error if a bpf_link
+ * exists for that prog.
+ */
+ if ((!link || prog) && *plink)
+ return -EBUSY;
- psock_set_prog(pprog, prog);
- return 0;
+ if (old) {
+ ret = psock_replace_prog(pprog, prog, old);
+ if (!ret)
+ *plink = NULL;
+ } else {
+ psock_set_prog(pprog, prog);
+ if (link)
+ *plink = link;
+ }
+
+ return ret;
}
int sock_map_bpf_prog_query(const union bpf_attr *attr,
@@ -1527,7 +1568,7 @@ int sock_map_bpf_prog_query(const union bpf_attr *attr,
rcu_read_lock();
- ret = sock_map_prog_lookup(map, &pprog, attr->query.attach_type);
+ ret = sock_map_prog_link_lookup(map, &pprog, NULL, attr->query.attach_type);
if (ret)
goto end;
@@ -1657,6 +1698,194 @@ void sock_map_close(struct sock *sk, long timeout)
}
EXPORT_SYMBOL_GPL(sock_map_close);
+struct sockmap_link {
+ struct bpf_link link;
+ struct bpf_map *map;
+ enum bpf_attach_type attach_type;
+};
+
+static void sock_map_link_release(struct bpf_link *link)
+{
+ struct sockmap_link *sockmap_link = container_of(link, struct sockmap_link, link);
+
+ mutex_lock(&sockmap_mutex);
+ if (!sockmap_link->map)
+ goto out;
+
+ WARN_ON_ONCE(sock_map_prog_update(sockmap_link->map, NULL, link->prog, link,
+ sockmap_link->attach_type));
+
+ bpf_map_put_with_uref(sockmap_link->map);
+ sockmap_link->map = NULL;
+out:
+ mutex_unlock(&sockmap_mutex);
+}
+
+static int sock_map_link_detach(struct bpf_link *link)
+{
+ sock_map_link_release(link);
+ return 0;
+}
+
+static void sock_map_link_dealloc(struct bpf_link *link)
+{
+ kfree(link);
+}
+
+/* Handle the following two cases:
+ * case 1: link != NULL, prog != NULL, old != NULL
+ * case 2: link != NULL, prog != NULL, old == NULL
+ */
+static int sock_map_link_update_prog(struct bpf_link *link,
+ struct bpf_prog *prog,
+ struct bpf_prog *old)
+{
+ const struct sockmap_link *sockmap_link = container_of(link, struct sockmap_link, link);
+ struct bpf_prog **pprog, *old_link_prog;
+ struct bpf_link **plink;
+ int ret = 0;
+
+ mutex_lock(&sockmap_mutex);
+
+ /* If old prog is not NULL, ensure old prog is the same as link->prog. */
+ if (old && link->prog != old) {
+ ret = -EPERM;
+ goto out;
+ }
+ /* Ensure link->prog has the same type/attach_type as the new prog. */
+ if (link->prog->type != prog->type ||
+ link->prog->expected_attach_type != prog->expected_attach_type) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ ret = sock_map_prog_link_lookup(sockmap_link->map, &pprog, &plink,
+ sockmap_link->attach_type);
+ if (ret)
+ goto out;
+
+ /* return error if the stored bpf_link does not match the incoming bpf_link. */
+ if (link != *plink)
+ return -EBUSY;
+
+ if (old) {
+ ret = psock_replace_prog(pprog, prog, old);
+ if (ret)
+ goto out;
+ } else {
+ psock_set_prog(pprog, prog);
+ }
+
+ bpf_prog_inc(prog);
+ old_link_prog = xchg(&link->prog, prog);
+ bpf_prog_put(old_link_prog);
+
+out:
+ mutex_unlock(&sockmap_mutex);
+ return ret;
+}
+
+static u32 sock_map_link_get_map_id(const struct sockmap_link *sockmap_link)
+{
+ u32 map_id = 0;
+
+ mutex_lock(&sockmap_mutex);
+ if (sockmap_link->map)
+ map_id = sockmap_link->map->id;
+ mutex_unlock(&sockmap_mutex);
+ return map_id;
+}
+
+static int sock_map_link_fill_info(const struct bpf_link *link,
+ struct bpf_link_info *info)
+{
+ const struct sockmap_link *sockmap_link = container_of(link, struct sockmap_link, link);
+ u32 map_id = sock_map_link_get_map_id(sockmap_link);
+
+ info->sockmap.map_id = map_id;
+ info->sockmap.attach_type = sockmap_link->attach_type;
+ return 0;
+}
+
+static void sock_map_link_show_fdinfo(const struct bpf_link *link,
+ struct seq_file *seq)
+{
+ const struct sockmap_link *sockmap_link = container_of(link, struct sockmap_link, link);
+ u32 map_id = sock_map_link_get_map_id(sockmap_link);
+
+ seq_printf(seq, "map_id:\t%u\n", map_id);
+ seq_printf(seq, "attach_type:\t%u\n", sockmap_link->attach_type);
+}
+
+static const struct bpf_link_ops sock_map_link_ops = {
+ .release = sock_map_link_release,
+ .dealloc = sock_map_link_dealloc,
+ .detach = sock_map_link_detach,
+ .update_prog = sock_map_link_update_prog,
+ .fill_link_info = sock_map_link_fill_info,
+ .show_fdinfo = sock_map_link_show_fdinfo,
+};
+
+int sock_map_link_create(const union bpf_attr *attr, struct bpf_prog *prog)
+{
+ struct bpf_link_primer link_primer;
+ struct sockmap_link *sockmap_link;
+ enum bpf_attach_type attach_type;
+ struct bpf_map *map;
+ int ret;
+
+ if (attr->link_create.flags)
+ return -EINVAL;
+
+ map = bpf_map_get_with_uref(attr->link_create.target_fd);
+ if (IS_ERR(map))
+ return PTR_ERR(map);
+ if (map->map_type != BPF_MAP_TYPE_SOCKMAP && map->map_type != BPF_MAP_TYPE_SOCKHASH) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ sockmap_link = kzalloc(sizeof(*sockmap_link), GFP_USER);
+ if (!sockmap_link) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ attach_type = attr->link_create.attach_type;
+ bpf_link_init(&sockmap_link->link, BPF_LINK_TYPE_SOCKMAP, &sock_map_link_ops, prog);
+ sockmap_link->map = map;
+ sockmap_link->attach_type = attach_type;
+
+ ret = bpf_link_prime(&sockmap_link->link, &link_primer);
+ if (ret) {
+ kfree(sockmap_link);
+ goto out;
+ }
+
+ mutex_lock(&sockmap_mutex);
+ ret = sock_map_prog_update(map, prog, NULL, &sockmap_link->link, attach_type);
+ mutex_unlock(&sockmap_mutex);
+ if (ret) {
+ bpf_link_cleanup(&link_primer);
+ goto out;
+ }
+
+ /* Increase refcnt for the prog since when old prog is replaced with
+ * psock_replace_prog() and psock_set_prog() its refcnt will be decreased.
+ *
+ * Actually, we do not need to increase refcnt for the prog since bpf_link
+ * will hold a reference. But in order to have less complexity w.r.t.
+ * replacing/setting prog, let us increase the refcnt to make things simpler.
+ */
+ bpf_prog_inc(prog);
+
+ return bpf_link_settle(&link_primer);
+
+out:
+ bpf_map_put_with_uref(map);
+ return ret;
+}
+
static int sock_map_iter_attach_target(struct bpf_prog *prog,
union bpf_iter_link_info *linfo,
struct bpf_iter_aux_info *aux)
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 6fe9f11c8abe..cee0a7915c08 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -1135,6 +1135,7 @@ enum bpf_link_type {
BPF_LINK_TYPE_TCX = 11,
BPF_LINK_TYPE_UPROBE_MULTI = 12,
BPF_LINK_TYPE_NETKIT = 13,
+ BPF_LINK_TYPE_SOCKMAP = 14,
__MAX_BPF_LINK_TYPE,
};
@@ -6724,6 +6725,10 @@ struct bpf_link_info {
__u32 ifindex;
__u32 attach_type;
} netkit;
+ struct {
+ __u32 map_id;
+ __u32 attach_type;
+ } sockmap;
};
} __attribute__((aligned(8)));
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH bpf-next v6 2/5] libbpf: Add bpf_link support for BPF_PROG_TYPE_SOCKMAP
2024-04-08 15:24 [PATCH bpf-next v6 0/5] bpf: Add bpf_link support for sk_msg and sk_skb progs Yonghong Song
2024-04-08 15:24 ` [PATCH bpf-next v6 1/5] " Yonghong Song
@ 2024-04-08 15:24 ` Yonghong Song
2024-04-08 15:24 ` [PATCH bpf-next v6 3/5] bpftool: Add link dump support for BPF_LINK_TYPE_SOCKMAP Yonghong Song
` (2 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Yonghong Song @ 2024-04-08 15:24 UTC (permalink / raw
To: bpf
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Jakub Sitnicki, John Fastabend, kernel-team, Martin KaFai Lau,
Eduard Zingerman
Introduce a libbpf API function bpf_program__attach_sockmap()
which allow user to get a bpf_link for their corresponding programs.
Acked-by: Andrii Nakryiko <andrii@kernel.org>
Acked-by: Eduard Zingerman <eddyz87@gmail.com>
Reviewed-by: John Fastabend <john.fastabend@gmail.com>
Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
tools/lib/bpf/libbpf.c | 7 +++++++
tools/lib/bpf/libbpf.h | 2 ++
tools/lib/bpf/libbpf.map | 1 +
3 files changed, 10 insertions(+)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index b091154bc5b5..97eb6e5dd7c8 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -149,6 +149,7 @@ static const char * const link_type_name[] = {
[BPF_LINK_TYPE_TCX] = "tcx",
[BPF_LINK_TYPE_UPROBE_MULTI] = "uprobe_multi",
[BPF_LINK_TYPE_NETKIT] = "netkit",
+ [BPF_LINK_TYPE_SOCKMAP] = "sockmap",
};
static const char * const map_type_name[] = {
@@ -12533,6 +12534,12 @@ bpf_program__attach_netns(const struct bpf_program *prog, int netns_fd)
return bpf_program_attach_fd(prog, netns_fd, "netns", NULL);
}
+struct bpf_link *
+bpf_program__attach_sockmap(const struct bpf_program *prog, int map_fd)
+{
+ return bpf_program_attach_fd(prog, map_fd, "sockmap", NULL);
+}
+
struct bpf_link *bpf_program__attach_xdp(const struct bpf_program *prog, int ifindex)
{
/* target_fd/target_ifindex use the same field in LINK_CREATE */
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 4f775a6dcaa0..1333ae20ebe6 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -795,6 +795,8 @@ bpf_program__attach_cgroup(const struct bpf_program *prog, int cgroup_fd);
LIBBPF_API struct bpf_link *
bpf_program__attach_netns(const struct bpf_program *prog, int netns_fd);
LIBBPF_API struct bpf_link *
+bpf_program__attach_sockmap(const struct bpf_program *prog, int map_fd);
+LIBBPF_API struct bpf_link *
bpf_program__attach_xdp(const struct bpf_program *prog, int ifindex);
LIBBPF_API struct bpf_link *
bpf_program__attach_freplace(const struct bpf_program *prog,
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 23d82bba021a..c1ce8aa3520b 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -419,6 +419,7 @@ LIBBPF_1.4.0 {
LIBBPF_1.5.0 {
global:
+ bpf_program__attach_sockmap;
ring__consume_n;
ring_buffer__consume_n;
} LIBBPF_1.4.0;
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH bpf-next v6 3/5] bpftool: Add link dump support for BPF_LINK_TYPE_SOCKMAP
2024-04-08 15:24 [PATCH bpf-next v6 0/5] bpf: Add bpf_link support for sk_msg and sk_skb progs Yonghong Song
2024-04-08 15:24 ` [PATCH bpf-next v6 1/5] " Yonghong Song
2024-04-08 15:24 ` [PATCH bpf-next v6 2/5] libbpf: Add bpf_link support for BPF_PROG_TYPE_SOCKMAP Yonghong Song
@ 2024-04-08 15:24 ` Yonghong Song
2024-04-08 15:24 ` [PATCH bpf-next v6 4/5] selftests/bpf: Refactor out helper functions for a few tests Yonghong Song
2024-04-08 15:24 ` [PATCH bpf-next v6 5/5] selftests/bpf: Add some tests with new bpf_program__attach_sockmap() APIs Yonghong Song
4 siblings, 0 replies; 9+ messages in thread
From: Yonghong Song @ 2024-04-08 15:24 UTC (permalink / raw
To: bpf
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Jakub Sitnicki, John Fastabend, kernel-team, Martin KaFai Lau,
Quentin Monnet
An example output looks like:
$ bpftool link
1776: sk_skb prog 49730
map_id 0 attach_type sk_skb_verdict
pids test_progs(8424)
1777: sk_skb prog 49755
map_id 0 attach_type sk_skb_stream_verdict
pids test_progs(8424)
1778: sk_msg prog 49770
map_id 8208 attach_type sk_msg_verdict
pids test_progs(8424)
Reviewed-by: John Fastabend <john.fastabend@gmail.com>
Reviewed-by: Quentin Monnet <qmo@kernel.org>
Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
tools/bpf/bpftool/link.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/tools/bpf/bpftool/link.c b/tools/bpf/bpftool/link.c
index afde9d0c2ea1..5cd503b763d7 100644
--- a/tools/bpf/bpftool/link.c
+++ b/tools/bpf/bpftool/link.c
@@ -526,6 +526,10 @@ static int show_link_close_json(int fd, struct bpf_link_info *info)
show_link_ifindex_json(info->netkit.ifindex, json_wtr);
show_link_attach_type_json(info->netkit.attach_type, json_wtr);
break;
+ case BPF_LINK_TYPE_SOCKMAP:
+ jsonw_uint_field(json_wtr, "map_id", info->sockmap.map_id);
+ show_link_attach_type_json(info->sockmap.attach_type, json_wtr);
+ break;
case BPF_LINK_TYPE_XDP:
show_link_ifindex_json(info->xdp.ifindex, json_wtr);
break;
@@ -915,6 +919,11 @@ static int show_link_close_plain(int fd, struct bpf_link_info *info)
show_link_ifindex_plain(info->netkit.ifindex);
show_link_attach_type_plain(info->netkit.attach_type);
break;
+ case BPF_LINK_TYPE_SOCKMAP:
+ printf("\n\t");
+ printf("map_id %u ", info->sockmap.map_id);
+ show_link_attach_type_plain(info->sockmap.attach_type);
+ break;
case BPF_LINK_TYPE_XDP:
printf("\n\t");
show_link_ifindex_plain(info->xdp.ifindex);
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH bpf-next v6 4/5] selftests/bpf: Refactor out helper functions for a few tests
2024-04-08 15:24 [PATCH bpf-next v6 0/5] bpf: Add bpf_link support for sk_msg and sk_skb progs Yonghong Song
` (2 preceding siblings ...)
2024-04-08 15:24 ` [PATCH bpf-next v6 3/5] bpftool: Add link dump support for BPF_LINK_TYPE_SOCKMAP Yonghong Song
@ 2024-04-08 15:24 ` Yonghong Song
2024-04-08 15:24 ` [PATCH bpf-next v6 5/5] selftests/bpf: Add some tests with new bpf_program__attach_sockmap() APIs Yonghong Song
4 siblings, 0 replies; 9+ messages in thread
From: Yonghong Song @ 2024-04-08 15:24 UTC (permalink / raw
To: bpf
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Jakub Sitnicki, John Fastabend, kernel-team, Martin KaFai Lau,
Eduard Zingerman
These helper functions will be used later new tests as well.
There are no functionality change.
Acked-by: Eduard Zingerman <eddyz87@gmail.com>
Reviewed-by: John Fastabend <john.fastabend@gmail.com>
Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
.../selftests/bpf/prog_tests/sockmap_basic.c | 38 +++++++++++--------
.../bpf/progs/test_skmsg_load_helpers.c | 9 ++++-
2 files changed, 30 insertions(+), 17 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
index 77e26ecffa9d..63fb2da7930a 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
@@ -475,30 +475,19 @@ static void test_sockmap_skb_verdict_fionread(bool pass_prog)
test_sockmap_drop_prog__destroy(drop);
}
-static void test_sockmap_skb_verdict_peek(void)
+static void test_sockmap_skb_verdict_peek_helper(int map)
{
- int err, map, verdict, s, c1, p1, zero = 0, sent, recvd, avail;
- struct test_sockmap_pass_prog *pass;
+ int err, s, c1, p1, zero = 0, sent, recvd, avail;
char snd[256] = "0123456789";
char rcv[256] = "0";
- pass = test_sockmap_pass_prog__open_and_load();
- if (!ASSERT_OK_PTR(pass, "open_and_load"))
- return;
- verdict = bpf_program__fd(pass->progs.prog_skb_verdict);
- map = bpf_map__fd(pass->maps.sock_map_rx);
-
- err = bpf_prog_attach(verdict, map, BPF_SK_SKB_STREAM_VERDICT, 0);
- if (!ASSERT_OK(err, "bpf_prog_attach"))
- goto out;
-
s = socket_loopback(AF_INET, SOCK_STREAM);
if (!ASSERT_GT(s, -1, "socket_loopback(s)"))
- goto out;
+ return;
err = create_pair(s, AF_INET, SOCK_STREAM, &c1, &p1);
if (!ASSERT_OK(err, "create_pairs(s)"))
- goto out;
+ return;
err = bpf_map_update_elem(map, &zero, &c1, BPF_NOEXIST);
if (!ASSERT_OK(err, "bpf_map_update_elem(c1)"))
@@ -520,6 +509,25 @@ static void test_sockmap_skb_verdict_peek(void)
out_close:
close(c1);
close(p1);
+}
+
+static void test_sockmap_skb_verdict_peek(void)
+{
+ struct test_sockmap_pass_prog *pass;
+ int err, map, verdict;
+
+ pass = test_sockmap_pass_prog__open_and_load();
+ if (!ASSERT_OK_PTR(pass, "open_and_load"))
+ return;
+ verdict = bpf_program__fd(pass->progs.prog_skb_verdict);
+ map = bpf_map__fd(pass->maps.sock_map_rx);
+
+ err = bpf_prog_attach(verdict, map, BPF_SK_SKB_STREAM_VERDICT, 0);
+ if (!ASSERT_OK(err, "bpf_prog_attach"))
+ goto out;
+
+ test_sockmap_skb_verdict_peek_helper(map);
+
out:
test_sockmap_pass_prog__destroy(pass);
}
diff --git a/tools/testing/selftests/bpf/progs/test_skmsg_load_helpers.c b/tools/testing/selftests/bpf/progs/test_skmsg_load_helpers.c
index 45e8fc75a739..b753672f04c9 100644
--- a/tools/testing/selftests/bpf/progs/test_skmsg_load_helpers.c
+++ b/tools/testing/selftests/bpf/progs/test_skmsg_load_helpers.c
@@ -24,8 +24,7 @@ struct {
__type(value, __u64);
} socket_storage SEC(".maps");
-SEC("sk_msg")
-int prog_msg_verdict(struct sk_msg_md *msg)
+static int prog_msg_verdict_common(struct sk_msg_md *msg)
{
struct task_struct *task = (struct task_struct *)bpf_get_current_task();
int verdict = SK_PASS;
@@ -44,4 +43,10 @@ int prog_msg_verdict(struct sk_msg_md *msg)
return verdict;
}
+SEC("sk_msg")
+int prog_msg_verdict(struct sk_msg_md *msg)
+{
+ return prog_msg_verdict_common(msg);
+}
+
char _license[] SEC("license") = "GPL";
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH bpf-next v6 5/5] selftests/bpf: Add some tests with new bpf_program__attach_sockmap() APIs
2024-04-08 15:24 [PATCH bpf-next v6 0/5] bpf: Add bpf_link support for sk_msg and sk_skb progs Yonghong Song
` (3 preceding siblings ...)
2024-04-08 15:24 ` [PATCH bpf-next v6 4/5] selftests/bpf: Refactor out helper functions for a few tests Yonghong Song
@ 2024-04-08 15:24 ` Yonghong Song
2024-04-09 7:55 ` Eduard Zingerman
4 siblings, 1 reply; 9+ messages in thread
From: Yonghong Song @ 2024-04-08 15:24 UTC (permalink / raw
To: bpf
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Jakub Sitnicki, John Fastabend, kernel-team, Martin KaFai Lau
Add a few more tests in sockmap_basic.c and sockmap_listen.c to
test bpf_link based APIs for SK_MSG and SK_SKB programs.
Link attach/detach/update are all tested.
All tests are passed.
Reviewed-by: John Fastabend <john.fastabend@gmail.com>
Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
.../selftests/bpf/prog_tests/sockmap_basic.c | 133 ++++++++++++++++++
.../selftests/bpf/prog_tests/sockmap_listen.c | 38 +++++
.../bpf/progs/test_skmsg_load_helpers.c | 18 +++
.../bpf/progs/test_sockmap_pass_prog.c | 17 ++-
.../progs/test_sockmap_skb_verdict_attach.c | 2 +-
5 files changed, 206 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
index 63fb2da7930a..1337153eb0ad 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
@@ -131,6 +131,65 @@ static void test_skmsg_helpers(enum bpf_map_type map_type)
test_skmsg_load_helpers__destroy(skel);
}
+static void test_skmsg_helpers_with_link(enum bpf_map_type map_type)
+{
+ struct bpf_program *prog, *prog_clone, *prog_clone2;
+ DECLARE_LIBBPF_OPTS(bpf_link_update_opts, opts);
+ struct test_skmsg_load_helpers *skel;
+ struct bpf_link *link, *link2;
+ int err, map;
+
+ skel = test_skmsg_load_helpers__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "test_skmsg_load_helpers__open_and_load"))
+ return;
+
+ prog = skel->progs.prog_msg_verdict;
+ prog_clone = skel->progs.prog_msg_verdict_clone;
+ prog_clone2 = skel->progs.prog_msg_verdict_clone2;
+ map = bpf_map__fd(skel->maps.sock_map);
+
+ link = bpf_program__attach_sockmap(prog, map);
+ if (!ASSERT_OK_PTR(link, "bpf_program__attach_sockmap"))
+ goto out;
+
+ /* Fail since bpf_link for the same prog has been created. */
+ err = bpf_prog_attach(bpf_program__fd(prog), map, BPF_SK_MSG_VERDICT, 0);
+ if (!ASSERT_ERR(err, "bpf_prog_attach"))
+ goto out;
+
+ /* Fail since bpf_link for the same prog type has been created. */
+ link2 = bpf_program__attach_sockmap(prog_clone, map);
+ if (!ASSERT_ERR_PTR(link2, "bpf_program__attach_sockmap")) {
+ bpf_link__detach(link2);
+ goto out;
+ }
+
+ err = bpf_link__update_program(link, prog_clone);
+ if (!ASSERT_OK(err, "bpf_link__update_program"))
+ goto out;
+
+ /* Fail since a prog with different type attempts to do update. */
+ err = bpf_link__update_program(link, skel->progs.prog_skb_verdict);
+ if (!ASSERT_ERR(err, "bpf_link__update_program"))
+ goto out;
+
+ /* Fail since the old prog does not match the one in the kernel. */
+ opts.old_prog_fd = bpf_program__fd(prog_clone2);
+ opts.flags = BPF_F_REPLACE;
+ err = bpf_link_update(bpf_link__fd(link), bpf_program__fd(prog), &opts);
+ if (!ASSERT_ERR(err, "bpf_link_update"))
+ goto out;
+
+ opts.old_prog_fd = bpf_program__fd(prog_clone);
+ opts.flags = BPF_F_REPLACE;
+ err = bpf_link_update(bpf_link__fd(link), bpf_program__fd(prog), &opts);
+ if (!ASSERT_OK(err, "bpf_link_update"))
+ goto out;
+out:
+ bpf_link__detach(link);
+ test_skmsg_load_helpers__destroy(skel);
+}
+
static void test_sockmap_update(enum bpf_map_type map_type)
{
int err, prog, src;
@@ -298,6 +357,40 @@ static void test_sockmap_skb_verdict_attach(enum bpf_attach_type first,
test_sockmap_skb_verdict_attach__destroy(skel);
}
+static void test_sockmap_skb_verdict_attach_with_link(void)
+{
+ struct test_sockmap_skb_verdict_attach *skel;
+ struct bpf_program *prog;
+ struct bpf_link *link;
+ int err, map;
+
+ skel = test_sockmap_skb_verdict_attach__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "open_and_load"))
+ return;
+ prog = skel->progs.prog_skb_verdict;
+ map = bpf_map__fd(skel->maps.sock_map);
+ link = bpf_program__attach_sockmap(prog, map);
+ if (!ASSERT_OK_PTR(link, "bpf_program__attach_sockmap"))
+ goto out;
+
+ bpf_link__detach(link);
+
+ err = bpf_prog_attach(bpf_program__fd(prog), map, BPF_SK_SKB_STREAM_VERDICT, 0);
+ if (!ASSERT_OK(err, "bpf_prog_attach"))
+ goto out;
+
+ /* Fail since attaching with the same prog/map has been done. */
+ link = bpf_program__attach_sockmap(prog, map);
+ if (!ASSERT_ERR_PTR(link, "bpf_program__attach_sockmap"))
+ bpf_link__detach(link);
+
+ err = bpf_prog_detach2(bpf_program__fd(prog), map, BPF_SK_SKB_STREAM_VERDICT);
+ if (!ASSERT_OK(err, "bpf_prog_detach2"))
+ goto out;
+out:
+ test_sockmap_skb_verdict_attach__destroy(skel);
+}
+
static __u32 query_prog_id(int prog_fd)
{
struct bpf_prog_info info = {};
@@ -532,6 +625,38 @@ static void test_sockmap_skb_verdict_peek(void)
test_sockmap_pass_prog__destroy(pass);
}
+static void test_sockmap_skb_verdict_peek_with_link(void)
+{
+ struct test_sockmap_pass_prog *pass;
+ struct bpf_program *prog;
+ struct bpf_link *link;
+ int err, map;
+
+ pass = test_sockmap_pass_prog__open_and_load();
+ if (!ASSERT_OK_PTR(pass, "open_and_load"))
+ return;
+ prog = pass->progs.prog_skb_verdict;
+ map = bpf_map__fd(pass->maps.sock_map_rx);
+ link = bpf_program__attach_sockmap(prog, map);
+ if (!ASSERT_OK_PTR(link, "bpf_program__attach_sockmap"))
+ goto out;
+
+ err = bpf_link__update_program(link, pass->progs.prog_skb_verdict_clone);
+ if (!ASSERT_OK(err, "bpf_link__update_program"))
+ goto out;
+
+ /* Fail since a prog with different attach type attempts to do update. */
+ err = bpf_link__update_program(link, pass->progs.prog_skb_parser);
+ if (!ASSERT_ERR(err, "bpf_link__update_program"))
+ goto out;
+
+ test_sockmap_skb_verdict_peek_helper(map);
+ ASSERT_EQ(pass->bss->clone_called, 1, "clone_called");
+out:
+ bpf_link__detach(link);
+ test_sockmap_pass_prog__destroy(pass);
+}
+
static void test_sockmap_unconnected_unix(void)
{
int err, map, stream = 0, dgram = 0, zero = 0;
@@ -796,6 +921,8 @@ void test_sockmap_basic(void)
test_sockmap_skb_verdict_attach(BPF_SK_SKB_STREAM_VERDICT,
BPF_SK_SKB_VERDICT);
}
+ if (test__start_subtest("sockmap skb_verdict attach_with_link"))
+ test_sockmap_skb_verdict_attach_with_link();
if (test__start_subtest("sockmap msg_verdict progs query"))
test_sockmap_progs_query(BPF_SK_MSG_VERDICT);
if (test__start_subtest("sockmap stream_parser progs query"))
@@ -812,6 +939,8 @@ void test_sockmap_basic(void)
test_sockmap_skb_verdict_fionread(false);
if (test__start_subtest("sockmap skb_verdict msg_f_peek"))
test_sockmap_skb_verdict_peek();
+ if (test__start_subtest("sockmap skb_verdict msg_f_peek with link"))
+ test_sockmap_skb_verdict_peek_with_link();
if (test__start_subtest("sockmap unconnected af_unix"))
test_sockmap_unconnected_unix();
if (test__start_subtest("sockmap one socket to many map entries"))
@@ -820,4 +949,8 @@ void test_sockmap_basic(void)
test_sockmap_many_maps();
if (test__start_subtest("sockmap same socket replace"))
test_sockmap_same_sock();
+ if (test__start_subtest("sockmap sk_msg attach sockmap helpers with link"))
+ test_skmsg_helpers_with_link(BPF_MAP_TYPE_SOCKMAP);
+ if (test__start_subtest("sockhash sk_msg attach sockhash helpers with link"))
+ test_skmsg_helpers_with_link(BPF_MAP_TYPE_SOCKHASH);
}
diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c b/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
index a92807bfcd13..e91b59366030 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
@@ -767,6 +767,24 @@ static void test_msg_redir_to_connected(struct test_sockmap_listen *skel,
xbpf_prog_detach2(verdict, sock_map, BPF_SK_MSG_VERDICT);
}
+static void test_msg_redir_to_connected_with_link(struct test_sockmap_listen *skel,
+ struct bpf_map *inner_map, int family,
+ int sotype)
+{
+ int prog_msg_verdict = bpf_program__fd(skel->progs.prog_msg_verdict);
+ int verdict_map = bpf_map__fd(skel->maps.verdict_map);
+ int sock_map = bpf_map__fd(inner_map);
+ int link_fd;
+
+ link_fd = bpf_link_create(prog_msg_verdict, sock_map, BPF_SK_MSG_VERDICT, NULL);
+ if (!ASSERT_GE(link_fd, 0, "bpf_link_create"))
+ return;
+
+ redir_to_connected(family, sotype, sock_map, verdict_map, REDIR_EGRESS);
+
+ close(link_fd);
+}
+
static void redir_to_listening(int family, int sotype, int sock_mapfd,
int verd_mapfd, enum redir_mode mode)
{
@@ -869,6 +887,24 @@ static void test_msg_redir_to_listening(struct test_sockmap_listen *skel,
xbpf_prog_detach2(verdict, sock_map, BPF_SK_MSG_VERDICT);
}
+static void test_msg_redir_to_listening_with_link(struct test_sockmap_listen *skel,
+ struct bpf_map *inner_map, int family,
+ int sotype)
+{
+ struct bpf_program *verdict = skel->progs.prog_msg_verdict;
+ int verdict_map = bpf_map__fd(skel->maps.verdict_map);
+ int sock_map = bpf_map__fd(inner_map);
+ struct bpf_link *link;
+
+ link = bpf_program__attach_sockmap(verdict, sock_map);
+ if (!ASSERT_OK_PTR(link, "bpf_program__attach_sockmap"))
+ return;
+
+ redir_to_listening(family, sotype, sock_map, verdict_map, REDIR_EGRESS);
+
+ bpf_link__detach(link);
+}
+
static void redir_partial(int family, int sotype, int sock_map, int parser_map)
{
int s, c0 = -1, c1 = -1, p0 = -1, p1 = -1;
@@ -1316,7 +1352,9 @@ static void test_redir(struct test_sockmap_listen *skel, struct bpf_map *map,
TEST(test_skb_redir_to_listening),
TEST(test_skb_redir_partial),
TEST(test_msg_redir_to_connected),
+ TEST(test_msg_redir_to_connected_with_link),
TEST(test_msg_redir_to_listening),
+ TEST(test_msg_redir_to_listening_with_link),
};
const char *family_name, *map_name;
const struct redir_test *t;
diff --git a/tools/testing/selftests/bpf/progs/test_skmsg_load_helpers.c b/tools/testing/selftests/bpf/progs/test_skmsg_load_helpers.c
index b753672f04c9..996b177324ba 100644
--- a/tools/testing/selftests/bpf/progs/test_skmsg_load_helpers.c
+++ b/tools/testing/selftests/bpf/progs/test_skmsg_load_helpers.c
@@ -49,4 +49,22 @@ int prog_msg_verdict(struct sk_msg_md *msg)
return prog_msg_verdict_common(msg);
}
+SEC("sk_msg")
+int prog_msg_verdict_clone(struct sk_msg_md *msg)
+{
+ return prog_msg_verdict_common(msg);
+}
+
+SEC("sk_msg")
+int prog_msg_verdict_clone2(struct sk_msg_md *msg)
+{
+ return prog_msg_verdict_common(msg);
+}
+
+SEC("sk_skb/stream_verdict")
+int prog_skb_verdict(struct __sk_buff *skb)
+{
+ return SK_PASS;
+}
+
char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/progs/test_sockmap_pass_prog.c b/tools/testing/selftests/bpf/progs/test_sockmap_pass_prog.c
index 1d86a717a290..69aacc96db36 100644
--- a/tools/testing/selftests/bpf/progs/test_sockmap_pass_prog.c
+++ b/tools/testing/selftests/bpf/progs/test_sockmap_pass_prog.c
@@ -23,10 +23,25 @@ struct {
__type(value, int);
} sock_map_msg SEC(".maps");
-SEC("sk_skb")
+SEC("sk_skb/stream_verdict")
int prog_skb_verdict(struct __sk_buff *skb)
{
return SK_PASS;
}
+int clone_called;
+
+SEC("sk_skb/stream_verdict")
+int prog_skb_verdict_clone(struct __sk_buff *skb)
+{
+ clone_called = 1;
+ return SK_PASS;
+}
+
+SEC("sk_skb/stream_parser")
+int prog_skb_parser(struct __sk_buff *skb)
+{
+ return SK_PASS;
+}
+
char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/progs/test_sockmap_skb_verdict_attach.c b/tools/testing/selftests/bpf/progs/test_sockmap_skb_verdict_attach.c
index 3c69aa971738..d25b0bb30fc0 100644
--- a/tools/testing/selftests/bpf/progs/test_sockmap_skb_verdict_attach.c
+++ b/tools/testing/selftests/bpf/progs/test_sockmap_skb_verdict_attach.c
@@ -9,7 +9,7 @@ struct {
__type(value, __u64);
} sock_map SEC(".maps");
-SEC("sk_skb")
+SEC("sk_skb/verdict")
int prog_skb_verdict(struct __sk_buff *skb)
{
return SK_DROP;
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next v6 5/5] selftests/bpf: Add some tests with new bpf_program__attach_sockmap() APIs
2024-04-08 15:24 ` [PATCH bpf-next v6 5/5] selftests/bpf: Add some tests with new bpf_program__attach_sockmap() APIs Yonghong Song
@ 2024-04-09 7:55 ` Eduard Zingerman
0 siblings, 0 replies; 9+ messages in thread
From: Eduard Zingerman @ 2024-04-09 7:55 UTC (permalink / raw
To: Yonghong Song, bpf
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Jakub Sitnicki, John Fastabend, kernel-team, Martin KaFai Lau
On Mon, 2024-04-08 at 08:24 -0700, Yonghong Song wrote:
> Add a few more tests in sockmap_basic.c and sockmap_listen.c to
> test bpf_link based APIs for SK_MSG and SK_SKB programs.
> Link attach/detach/update are all tested.
>
> All tests are passed.
>
> Reviewed-by: John Fastabend <john.fastabend@gmail.com>
> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
> ---
Acked-by: Eduard Zingerman <eddyz87@gmail.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next v6 1/5] bpf: Add bpf_link support for sk_msg and sk_skb progs
2024-04-08 15:24 ` [PATCH bpf-next v6 1/5] " Yonghong Song
@ 2024-04-09 9:52 ` Eduard Zingerman
2024-04-09 15:28 ` Yonghong Song
0 siblings, 1 reply; 9+ messages in thread
From: Eduard Zingerman @ 2024-04-09 9:52 UTC (permalink / raw
To: Yonghong Song, bpf
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Jakub Sitnicki, John Fastabend, kernel-team, Martin KaFai Lau
On Mon, 2024-04-08 at 08:24 -0700, Yonghong Song wrote:
[...]
> +/* Handle the following two cases:
> + * case 1: link != NULL, prog != NULL, old != NULL
> + * case 2: link != NULL, prog != NULL, old == NULL
> + */
> +static int sock_map_link_update_prog(struct bpf_link *link,
> + struct bpf_prog *prog,
> + struct bpf_prog *old)
> +{
> + const struct sockmap_link *sockmap_link = container_of(link, struct sockmap_link, link);
> + struct bpf_prog **pprog, *old_link_prog;
> + struct bpf_link **plink;
> + int ret = 0;
> +
> + mutex_lock(&sockmap_mutex);
> +
> + /* If old prog is not NULL, ensure old prog is the same as link->prog. */
> + if (old && link->prog != old) {
> + ret = -EPERM;
> + goto out;
> + }
> + /* Ensure link->prog has the same type/attach_type as the new prog. */
> + if (link->prog->type != prog->type ||
> + link->prog->expected_attach_type != prog->expected_attach_type) {
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + ret = sock_map_prog_link_lookup(sockmap_link->map, &pprog, &plink,
> + sockmap_link->attach_type);
> + if (ret)
> + goto out;
> +
> + /* return error if the stored bpf_link does not match the incoming bpf_link. */
> + if (link != *plink)
> + return -EBUSY;
Hi Yonghong,
Sorry, this was a mistake on my side,
this needs a 'goto out' in order to unlock the mutex.
Thanks,
Eduard
> +
> + if (old) {
> + ret = psock_replace_prog(pprog, prog, old);
> + if (ret)
> + goto out;
> + } else {
> + psock_set_prog(pprog, prog);
> + }
> +
> + bpf_prog_inc(prog);
> + old_link_prog = xchg(&link->prog, prog);
> + bpf_prog_put(old_link_prog);
> +
> +out:
> + mutex_unlock(&sockmap_mutex);
> + return ret;
> +}
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next v6 1/5] bpf: Add bpf_link support for sk_msg and sk_skb progs
2024-04-09 9:52 ` Eduard Zingerman
@ 2024-04-09 15:28 ` Yonghong Song
0 siblings, 0 replies; 9+ messages in thread
From: Yonghong Song @ 2024-04-09 15:28 UTC (permalink / raw
To: Eduard Zingerman, bpf
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Jakub Sitnicki, John Fastabend, kernel-team, Martin KaFai Lau
On 4/9/24 2:52 AM, Eduard Zingerman wrote:
> On Mon, 2024-04-08 at 08:24 -0700, Yonghong Song wrote:
>
> [...]
>
>
>> +/* Handle the following two cases:
>> + * case 1: link != NULL, prog != NULL, old != NULL
>> + * case 2: link != NULL, prog != NULL, old == NULL
>> + */
>> +static int sock_map_link_update_prog(struct bpf_link *link,
>> + struct bpf_prog *prog,
>> + struct bpf_prog *old)
>> +{
>> + const struct sockmap_link *sockmap_link = container_of(link, struct sockmap_link, link);
>> + struct bpf_prog **pprog, *old_link_prog;
>> + struct bpf_link **plink;
>> + int ret = 0;
>> +
>> + mutex_lock(&sockmap_mutex);
>> +
>> + /* If old prog is not NULL, ensure old prog is the same as link->prog. */
>> + if (old && link->prog != old) {
>> + ret = -EPERM;
>> + goto out;
>> + }
>> + /* Ensure link->prog has the same type/attach_type as the new prog. */
>> + if (link->prog->type != prog->type ||
>> + link->prog->expected_attach_type != prog->expected_attach_type) {
>> + ret = -EINVAL;
>> + goto out;
>> + }
>> +
>> + ret = sock_map_prog_link_lookup(sockmap_link->map, &pprog, &plink,
>> + sockmap_link->attach_type);
>> + if (ret)
>> + goto out;
>> +
>> + /* return error if the stored bpf_link does not match the incoming bpf_link. */
>> + if (link != *plink)
>> + return -EBUSY;
> Hi Yonghong,
>
> Sorry, this was a mistake on my side,
> this needs a 'goto out' in order to unlock the mutex.
It is my fault as well since I missed it too. Will send a revision later today
just in case there are more comments.
>
> Thanks,
> Eduard
>
>> +
>> + if (old) {
>> + ret = psock_replace_prog(pprog, prog, old);
>> + if (ret)
>> + goto out;
>> + } else {
>> + psock_set_prog(pprog, prog);
>> + }
>> +
>> + bpf_prog_inc(prog);
>> + old_link_prog = xchg(&link->prog, prog);
>> + bpf_prog_put(old_link_prog);
>> +
>> +out:
>> + mutex_unlock(&sockmap_mutex);
>> + return ret;
>> +}
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-04-09 15:28 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-08 15:24 [PATCH bpf-next v6 0/5] bpf: Add bpf_link support for sk_msg and sk_skb progs Yonghong Song
2024-04-08 15:24 ` [PATCH bpf-next v6 1/5] " Yonghong Song
2024-04-09 9:52 ` Eduard Zingerman
2024-04-09 15:28 ` Yonghong Song
2024-04-08 15:24 ` [PATCH bpf-next v6 2/5] libbpf: Add bpf_link support for BPF_PROG_TYPE_SOCKMAP Yonghong Song
2024-04-08 15:24 ` [PATCH bpf-next v6 3/5] bpftool: Add link dump support for BPF_LINK_TYPE_SOCKMAP Yonghong Song
2024-04-08 15:24 ` [PATCH bpf-next v6 4/5] selftests/bpf: Refactor out helper functions for a few tests Yonghong Song
2024-04-08 15:24 ` [PATCH bpf-next v6 5/5] selftests/bpf: Add some tests with new bpf_program__attach_sockmap() APIs Yonghong Song
2024-04-09 7:55 ` Eduard Zingerman
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.