On Tue, Mar 7, 2023 at 3:33 PM Kui-Feng Lee <kuifeng@xxxxxxxx> wrote:
>
> Flags a struct_ops is to back a bpf_link by putting it to the
> ".struct_ops.link" section. Once it is flagged, the created
> struct_ops can be used to create a bpf_link or update a bpf_link that
> has been backed by another struct_ops.
>
> Signed-off-by: Kui-Feng Lee <kuifeng@xxxxxxxx>
> ---
> tools/lib/bpf/libbpf.c | 64 +++++++++++++++++++++++++++++++++---------
> 1 file changed, 50 insertions(+), 14 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 247de39d136f..d66acd2fdbaa 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -467,6 +467,7 @@ struct bpf_struct_ops {
> #define KCONFIG_SEC ".kconfig"
> #define KSYMS_SEC ".ksyms"
> #define STRUCT_OPS_SEC ".struct_ops"
> +#define STRUCT_OPS_LINK_SEC ".struct_ops.link"
>
> enum libbpf_map_type {
> LIBBPF_MAP_UNSPEC,
> @@ -596,6 +597,7 @@ struct elf_state {
> Elf64_Ehdr *ehdr;
> Elf_Data *symbols;
> Elf_Data *st_ops_data;
> + Elf_Data *st_ops_link_data;
> size_t shstrndx; /* section index for section name strings */
> size_t strtabidx;
> struct elf_sec_desc *secs;
> @@ -605,6 +607,7 @@ struct elf_state {
> int text_shndx;
> int symbols_shndx;
> int st_ops_shndx;
> + int st_ops_link_shndx;
> };
>
> struct usdt_manager;
> @@ -1119,7 +1122,7 @@ static int bpf_object__init_kern_struct_ops_maps(struct bpf_object *obj)
> return 0;
> }
>
> -static int bpf_object__init_struct_ops_maps(struct bpf_object *obj)
> +static int bpf_object__init_struct_ops_maps_link(struct bpf_object *obj, bool link)
let's shorten it and not use double underscores, as this is not a
public bpf_object API, just "init_struct_ops_maps" probably is fine
> {
> const struct btf_type *type, *datasec;
> const struct btf_var_secinfo *vsi;
> @@ -1127,18 +1130,33 @@ static int bpf_object__init_struct_ops_maps(struct bpf_object *obj)
> const char *tname, *var_name;
> __s32 type_id, datasec_id;
> const struct btf *btf;
> + const char *sec_name;
> struct bpf_map *map;
> - __u32 i;
> + __u32 i, map_flags;
> + Elf_Data *data;
> + int shndx;
>
> - if (obj->efile.st_ops_shndx == -1)
> + if (link) {
> + sec_name = STRUCT_OPS_LINK_SEC;
> + shndx = obj->efile.st_ops_link_shndx;
> + data = obj->efile.st_ops_link_data;
> + map_flags = BPF_F_LINK;
> + } else {
> + sec_name = STRUCT_OPS_SEC;
> + shndx = obj->efile.st_ops_shndx;
> + data = obj->efile.st_ops_data;
> + map_flags = 0;
> + }
let's pass these as function arguments instead
> +
> + if (shndx == -1)
> return 0;
>
> btf = obj->btf;
> - datasec_id = btf__find_by_name_kind(btf, STRUCT_OPS_SEC,
> + datasec_id = btf__find_by_name_kind(btf, sec_name,
> BTF_KIND_DATASEC);
> if (datasec_id < 0) {
> pr_warn("struct_ops init: DATASEC %s not found\n",
> - STRUCT_OPS_SEC);
> + sec_name);
> return -EINVAL;
> }
>
> @@ -1151,7 +1169,7 @@ static int bpf_object__init_struct_ops_maps(struct bpf_object *obj)
> type_id = btf__resolve_type(obj->btf, vsi->type);
> if (type_id < 0) {
> pr_warn("struct_ops init: Cannot resolve var type_id %u in DATASEC %s\n",
> - vsi->type, STRUCT_OPS_SEC);
> + vsi->type, sec_name);
> return -EINVAL;
> }
>
> @@ -1170,7 +1188,7 @@ static int bpf_object__init_struct_ops_maps(struct bpf_object *obj)
> if (IS_ERR(map))
> return PTR_ERR(map);
>
> - map->sec_idx = obj->efile.st_ops_shndx;
> + map->sec_idx = shndx;
> map->sec_offset = vsi->offset;
> map->name = strdup(var_name);
> if (!map->name)
> @@ -1180,6 +1198,7 @@ static int bpf_object__init_struct_ops_maps(struct bpf_object *obj)
> map->def.key_size = sizeof(int);
> map->def.value_size = type->size;
> map->def.max_entries = 1;
> + map->def.map_flags = map_flags;
>
> map->st_ops = calloc(1, sizeof(*map->st_ops));
> if (!map->st_ops)
> @@ -1192,14 +1211,14 @@ static int bpf_object__init_struct_ops_maps(struct bpf_object *obj)
> if (!st_ops->data || !st_ops->progs || !st_ops->kern_func_off)
> return -ENOMEM;
>
> - if (vsi->offset + type->size > obj->efile.st_ops_data->d_size) {
> + if (vsi->offset + type->size > data->d_size) {
> pr_warn("struct_ops init: var %s is beyond the end of DATASEC %s\n",
> - var_name, STRUCT_OPS_SEC);
> + var_name, sec_name);
> return -EINVAL;
> }
>
> memcpy(st_ops->data,
> - obj->efile.st_ops_data->d_buf + vsi->offset,
> + data->d_buf + vsi->offset,
> type->size);
> st_ops->tname = tname;
> st_ops->type = type;
> @@ -1212,6 +1231,15 @@ static int bpf_object__init_struct_ops_maps(struct bpf_object *obj)
> return 0;
> }
>
> +static int bpf_object__init_struct_ops_maps(struct bpf_object *obj)
let's name this bpf_object_init_struct_ops, no double underscores
> +{
> + int err;
> +
> + err = bpf_object__init_struct_ops_maps_link(obj, false);
> + err = err ?: bpf_object__init_struct_ops_maps_link(obj, true);
> + return err;
> +}
> +
> static struct bpf_object *bpf_object__new(const char *path,
> const void *obj_buf,
> size_t obj_buf_sz,
> @@ -1248,6 +1276,7 @@ static struct bpf_object *bpf_object__new(const char *path,
> obj->efile.obj_buf_sz = obj_buf_sz;
> obj->efile.btf_maps_shndx = -1;
> obj->efile.st_ops_shndx = -1;
> + obj->efile.st_ops_link_shndx = -1;
> obj->kconfig_map_idx = -1;
>
> obj->kern_version = get_kernel_version();
> @@ -1265,6 +1294,7 @@ static void bpf_object__elf_finish(struct bpf_object *obj)
> obj->efile.elf = NULL;
> obj->efile.symbols = NULL;
> obj->efile.st_ops_data = NULL;
> + obj->efile.st_ops_link_data = NULL;
>
> zfree(&obj->efile.secs);
> obj->efile.sec_cnt = 0;
> @@ -2753,12 +2783,13 @@ static bool libbpf_needs_btf(const struct bpf_object *obj)
> {
> return obj->efile.btf_maps_shndx >= 0 ||
> obj->efile.st_ops_shndx >= 0 ||
> + obj->efile.st_ops_link_shndx >= 0 ||
> obj->nr_extern > 0;
> }
>
> static bool kernel_needs_btf(const struct bpf_object *obj)
> {
> - return obj->efile.st_ops_shndx >= 0;
> + return obj->efile.st_ops_shndx >= 0 || obj->efile.st_ops_link_shndx >= 0;
> }
>
> static int bpf_object__init_btf(struct bpf_object *obj,
> @@ -3451,6 +3482,9 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
> } else if (strcmp(name, STRUCT_OPS_SEC) == 0) {
> obj->efile.st_ops_data = data;
> obj->efile.st_ops_shndx = idx;
> + } else if (strcmp(name, STRUCT_OPS_LINK_SEC) == 0) {
> + obj->efile.st_ops_link_data = data;
> + obj->efile.st_ops_link_shndx = idx;
> } else {
> pr_info("elf: skipping unrecognized data section(%d) %s\n",
> idx, name);
> @@ -3465,6 +3499,7 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
> /* Only do relo for section with exec instructions */
> if (!section_have_execinstr(obj, targ_sec_idx) &&
> strcmp(name, ".rel" STRUCT_OPS_SEC) &&
> + strcmp(name, ".rel" STRUCT_OPS_LINK_SEC) &&
> strcmp(name, ".rel" MAPS_ELF_SEC)) {
> pr_info("elf: skipping relo section(%d) %s for section(%d) %s\n",
> idx, name, targ_sec_idx,
> @@ -6611,7 +6646,7 @@ static int bpf_object__collect_relos(struct bpf_object *obj)
> return -LIBBPF_ERRNO__INTERNAL;
> }
>
> - if (idx == obj->efile.st_ops_shndx)
> + if (idx == obj->efile.st_ops_shndx || idx == obj->efile.st_ops_link_shndx)
> err = bpf_object__collect_st_ops_relos(obj, shdr, data);
this function calls find_struct_ops_map_by_offset() which assumes we
only have one struct_ops section. This won't work now, please double
check all this, there should be no assumption about specific section
index
> else if (idx == obj->efile.btf_maps_shndx)
> err = bpf_object__collect_map_relos(obj, shdr, data);
> @@ -8954,8 +8989,9 @@ static int bpf_object__collect_st_ops_relos(struct bpf_object *obj,
> }
>
> /* struct_ops BPF prog can be re-used between multiple
> - * .struct_ops as long as it's the same struct_ops struct
> - * definition and the same function pointer field
> + * .struct_ops & .struct_ops.link as long as it's the
> + * same struct_ops struct definition and the same
> + * function pointer field
> */
> if (prog->attach_btf_id != st_ops->type_id ||
> prog->expected_attach_type != member_idx) {
> --
> 2.34.1
>
