Skip to content

Commit

Permalink
Merge branch 'libbpf: use func name when pinning programs with LIBBPF…
Browse files Browse the repository at this point in the history
…_STRICT_SEC_NAME'

Stanislav Fomichev says:

====================

Commit 15669e1 ("selftests/bpf: Normalize all the rest SEC() uses")
broke flow dissector tests. With the strict section names, bpftool isn't
able to pin all programs of the objects (all section names are the
same now). To bring it back to life let's do the following:

- teach libbpf to pin by func name with LIBBPF_STRICT_SEC_NAME
- enable strict mode in bpftool (breaking cli change)
- fix custom flow_dissector loader to use strict mode
- fix flow_dissector tests to use new pin names (func vs sec)

v5:
- get rid of error when retrying with '/' (Quentin Monnet)

v4:
- fix comment spelling (Quentin Monnet)
- retry progtype without / (Quentin Monnet)

v3:
- clarify program pinning in LIBBPF_STRICT_SEC_NAME,
  for real this time (Andrii Nakryiko)
- fix possible segfault in __bpf_program__pin_name (Andrii Nakryiko)

v2:
- add github issue (Andrii Nakryiko)
- remove sec_name from bpf_program.pin_name comment (Andrii Nakryiko)
- add cover letter (Andrii Nakryiko)
====================

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
  • Loading branch information
Andrii Nakryiko committed Oct 22, 2021
2 parents e89ef63 + d132120 commit a33f607
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 22 deletions.
13 changes: 11 additions & 2 deletions tools/lib/bpf/libbpf.c
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ struct bpf_program {
size_t sub_insn_off;

char *name;
/* sec_name with / replaced by _; makes recursive pinning
/* name with / replaced by _; makes recursive pinning
* in bpf_object__pin_programs easier
*/
char *pin_name;
Expand Down Expand Up @@ -618,7 +618,16 @@ static char *__bpf_program__pin_name(struct bpf_program *prog)
{
char *name, *p;

name = p = strdup(prog->sec_name);
if (libbpf_mode & LIBBPF_STRICT_SEC_NAME)
name = strdup(prog->name);
else
name = strdup(prog->sec_name);

if (!name)
return NULL;

p = name;

while ((p = strchr(p, '/')))
*p = '_';

Expand Down
3 changes: 3 additions & 0 deletions tools/lib/bpf/libbpf_legacy.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@ enum libbpf_strict_mode {
* allowed, with LIBBPF_STRICT_SEC_PREFIX this will become
* unrecognized by libbpf and would have to be just SEC("xdp") and
* SEC("xdp") and SEC("perf_event").
*
* Note, in this mode the program pin path will be based on the
* function name instead of section name.
*/
LIBBPF_STRICT_SEC_NAME = 0x04,

Expand Down
18 changes: 11 additions & 7 deletions tools/testing/selftests/bpf/flow_dissector_load.c
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,19 @@
const char *cfg_pin_path = "/sys/fs/bpf/flow_dissector";
const char *cfg_map_name = "jmp_table";
bool cfg_attach = true;
char *cfg_section_name;
char *cfg_prog_name;
char *cfg_path_name;

static void load_and_attach_program(void)
{
int prog_fd, ret;
struct bpf_object *obj;

ret = bpf_flow_load(&obj, cfg_path_name, cfg_section_name,
ret = libbpf_set_strict_mode(LIBBPF_STRICT_ALL);
if (ret)
error(1, 0, "failed to enable libbpf strict mode: %d", ret);

ret = bpf_flow_load(&obj, cfg_path_name, cfg_prog_name,
cfg_map_name, NULL, &prog_fd, NULL);
if (ret)
error(1, 0, "bpf_flow_load %s", cfg_path_name);
Expand Down Expand Up @@ -75,15 +79,15 @@ static void parse_opts(int argc, char **argv)
break;
case 'p':
if (cfg_path_name)
error(1, 0, "only one prog name can be given");
error(1, 0, "only one path can be given");

cfg_path_name = optarg;
break;
case 's':
if (cfg_section_name)
error(1, 0, "only one section can be given");
if (cfg_prog_name)
error(1, 0, "only one prog can be given");

cfg_section_name = optarg;
cfg_prog_name = optarg;
break;
}
}
Expand All @@ -94,7 +98,7 @@ static void parse_opts(int argc, char **argv)
if (cfg_attach && !cfg_path_name)
error(1, 0, "must provide a path to the BPF program");

if (cfg_attach && !cfg_section_name)
if (cfg_attach && !cfg_prog_name)
error(1, 0, "must provide a section name");
}

Expand Down
10 changes: 2 additions & 8 deletions tools/testing/selftests/bpf/flow_dissector_load.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

static inline int bpf_flow_load(struct bpf_object **obj,
const char *path,
const char *section_name,
const char *prog_name,
const char *map_name,
const char *keys_map_name,
int *prog_fd,
Expand All @@ -23,13 +23,7 @@ static inline int bpf_flow_load(struct bpf_object **obj,
if (ret)
return ret;

main_prog = NULL;
bpf_object__for_each_program(prog, *obj) {
if (strcmp(section_name, bpf_program__section_name(prog)) == 0) {
main_prog = prog;
break;
}
}
main_prog = bpf_object__find_program_by_name(*obj, prog_name);
if (!main_prog)
return -1;

Expand Down
10 changes: 5 additions & 5 deletions tools/testing/selftests/bpf/test_flow_dissector.sh
Original file line number Diff line number Diff line change
Expand Up @@ -26,22 +26,22 @@ if [[ -z $(ip netns identify $$) ]]; then
type flow_dissector

if ! unshare --net $bpftool prog attach pinned \
/sys/fs/bpf/flow/flow_dissector flow_dissector; then
/sys/fs/bpf/flow/_dissect flow_dissector; then
echo "Unexpected unsuccessful attach in namespace" >&2
err=1
fi

$bpftool prog attach pinned /sys/fs/bpf/flow/flow_dissector \
$bpftool prog attach pinned /sys/fs/bpf/flow/_dissect \
flow_dissector

if unshare --net $bpftool prog attach pinned \
/sys/fs/bpf/flow/flow_dissector flow_dissector; then
/sys/fs/bpf/flow/_dissect flow_dissector; then
echo "Unexpected successful attach in namespace" >&2
err=1
fi

if ! $bpftool prog detach pinned \
/sys/fs/bpf/flow/flow_dissector flow_dissector; then
/sys/fs/bpf/flow/_dissect flow_dissector; then
echo "Failed to detach flow dissector" >&2
err=1
fi
Expand Down Expand Up @@ -95,7 +95,7 @@ else
fi

# Attach BPF program
./flow_dissector_load -p bpf_flow.o -s flow_dissector
./flow_dissector_load -p bpf_flow.o -s _dissect

# Setup
tc qdisc add dev lo ingress
Expand Down

0 comments on commit a33f607

Please sign in to comment.