On 2/16/23 14:40, Andrii Nakryiko wrote:
On Tue, Feb 14, 2023 at 2:17 PM Kui-Feng Lee <kuifeng@xxxxxxxx> wrote:
bpf_map__attach_struct_ops() was creating a dummy bpf_link as a
placeholder, but now it is constructing an authentic one by calling
bpf_link_create() if the map has the BPF_F_LINK flag.
You can flag a struct_ops map with BPF_F_LINK by calling
bpf_map__set_map_flags().
Signed-off-by: Kui-Feng Lee <kuifeng@xxxxxxxx>
---
tools/lib/bpf/libbpf.c | 73 +++++++++++++++++++++++++++++++++---------
1 file changed, 58 insertions(+), 15 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 75ed95b7e455..1eff6a03ddd9 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -11430,29 +11430,41 @@ struct bpf_link *bpf_program__attach(const struct bpf_program *prog)
return link;
}
+struct bpf_link_struct_ops_map {
let's drop the "_map" suffix? struct_ops is always a map, so no need
to point this out
Sure!
+ struct bpf_link link;
+ int map_fd;
+};
+
static int bpf_link__detach_struct_ops(struct bpf_link *link)
{
+ struct bpf_link_struct_ops_map *st_link;
__u32 zero = 0;
- if (bpf_map_delete_elem(link->fd, &zero))
+ st_link = container_of(link, struct bpf_link_struct_ops_map, link);
+
+ if (st_link->map_fd < 0) {
+ /* Fake bpf_link */
+ if (bpf_map_delete_elem(link->fd, &zero))
+ return -errno;
+ return 0;
+ }
+
+ if (bpf_map_delete_elem(st_link->map_fd, &zero))
+ return -errno;
+
+ if (close(link->fd))
return -errno;
return 0;
}
-struct bpf_link *bpf_map__attach_struct_ops(const struct bpf_map *map)
+/*
+ * Update the map with the prepared vdata.
+ */
+static int bpf_map__update_vdata(const struct bpf_map *map)
this is internal helper, so let's not use double underscores, just
bpf_map_update_vdata()
Ok!
{
struct bpf_struct_ops *st_ops;
- struct bpf_link *link;
__u32 i, zero = 0;
- int err;
-
- if (!bpf_map__is_struct_ops(map) || map->fd == -1)
- return libbpf_err_ptr(-EINVAL);
-
- link = calloc(1, sizeof(*link));
- if (!link)
- return libbpf_err_ptr(-EINVAL);
st_ops = map->st_ops;
for (i = 0; i < btf_vlen(st_ops->type); i++) {
@@ -11468,17 +11480,48 @@ struct bpf_link *bpf_map__attach_struct_ops(const struct bpf_map *map)
*(unsigned long *)kern_data = prog_fd;
}
- err = bpf_map_update_elem(map->fd, &zero, st_ops->kern_vdata, 0);
+ return bpf_map_update_elem(map->fd, &zero, st_ops->kern_vdata, 0);
+}
+
+struct bpf_link *bpf_map__attach_struct_ops(const struct bpf_map *map)
+{
+ struct bpf_link_struct_ops_map *link;
+ int err, fd;
+
+ if (!bpf_map__is_struct_ops(map) || map->fd == -1)
+ return libbpf_err_ptr(-EINVAL);
+
+ link = calloc(1, sizeof(*link));
+ if (!link)
+ return libbpf_err_ptr(-EINVAL);
+
+ err = bpf_map__update_vdata(map);
if (err) {
err = -errno;
free(link);
return libbpf_err_ptr(err);
}
- link->detach = bpf_link__detach_struct_ops;
- link->fd = map->fd;
+ link->link.detach = bpf_link__detach_struct_ops;
- return link;
+ if (!(map->def.map_flags & BPF_F_LINK)) {
So this will always require a programmatic bpf_map__set_map_flags()
call, there is currently no declarative way to do this, right?
Is there any way to avoid this BPF_F_LINK flag approach? How bad would
it be if kernel just always created bpf_link-backed struct_ops?
Alternatively, should we think about SEC(".struct_ops.link") or
something like that to instruct libbpf to add this BPF_F_LINK flag
automatically?
Agree!
The other solution is to add a flag when declare a struct_ops.
SEC(".struct_ops")
tcp_congestion_ops ops = {
...
.flags = WITH_LINK,
}
+ /* Fake bpf_link */
+ link->link.fd = map->fd;
+ link->map_fd = -1;
+ return &link->link;
+ }
+
