On Wed, 10 Oct 2018 09:44:26 -0700, John Fastabend wrote:
> Sock map/hash introduce support for attaching programs to maps. To
> date I have been doing this with custom tooling but this is less than
> ideal as we shift to using bpftool as the single CLI for our BPF uses.
> This patch adds new sub commands 'attach' and 'detach' to the 'prog'
> command to attach programs to maps and then detach them.
>
> Signed-off-by: John Fastabend <john.fastab...@gmail.com>
> ---
> tools/bpf/bpftool/main.h | 1 +
> tools/bpf/bpftool/prog.c | 92
> ++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 92 insertions(+), 1 deletion(-)
>
> diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
> index 40492cd..9ceb2b6 100644
> --- a/tools/bpf/bpftool/main.h
> +++ b/tools/bpf/bpftool/main.h
> @@ -137,6 +137,7 @@ int cmd_select(const struct cmd *cmds, int argc, char
> **argv,
> int do_cgroup(int argc, char **arg);
> int do_perf(int argc, char **arg);
> int do_net(int argc, char **arg);
> +int do_attach_cmd(int argc, char **arg);
Looks like a leftover?
> int prog_parse_fd(int *argc, char ***argv);
> int map_parse_fd(int *argc, char ***argv);
> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> index b1cd3bc..280881d 100644
> --- a/tools/bpf/bpftool/prog.c
> +++ b/tools/bpf/bpftool/prog.c
> @@ -77,6 +77,26 @@
> [BPF_PROG_TYPE_FLOW_DISSECTOR] = "flow_dissector",
> };
>
> +static const char * const attach_type_strings[] = {
> + [BPF_SK_SKB_STREAM_PARSER] = "stream_parser",
> + [BPF_SK_SKB_STREAM_VERDICT] = "stream_verdict",
> + [BPF_SK_MSG_VERDICT] = "msg_verdict",
> + [__MAX_BPF_ATTACH_TYPE] = NULL,
> +};
> +
> +enum bpf_attach_type parse_attach_type(const char *str)
> +{
> + enum bpf_attach_type type;
> +
> + for (type = 0; type < __MAX_BPF_ATTACH_TYPE; type++) {
> + if (attach_type_strings[type] &&
> + is_prefix(str, attach_type_strings[type]))
> + return type;
> + }
> +
> + return __MAX_BPF_ATTACH_TYPE;
> +}
> +
> static void print_boot_time(__u64 nsecs, char *buf, unsigned int size)
> {
> struct timespec real_time_ts, boot_time_ts;
> @@ -697,6 +717,71 @@ int map_replace_compar(const void *p1, const void *p2)
> return a->idx - b->idx;
> }
>
> +static int do_attach(int argc, char **argv)
> +{
> + enum bpf_attach_type attach_type;
> + int err, mapfd, progfd;
> +
> + if (!REQ_ARGS(4)) {
Hm, 4 or 5? id $prog $type id $map ?
> + p_err("too few parameters for map attach");
> + return -EINVAL;
> + }
> +
> + progfd = prog_parse_fd(&argc, &argv);
> + if (progfd < 0)
> + return progfd;
> +
> + attach_type = parse_attach_type(*argv);
> + if (attach_type == __MAX_BPF_ATTACH_TYPE) {
> + p_err("invalid attach type");
> + return -EINVAL;
> + }
> +
> + NEXT_ARG();
nit: maybe NEXT_ARG() should be grouped with the code that consumes the
parameter, i.e. new line after not before?
> + mapfd = map_parse_fd(&argc, &argv);
> + if (mapfd < 0)
> + return mapfd;
> +
> + err = bpf_prog_attach(progfd, mapfd, attach_type, 0);
> + if (err) {
> + p_err("failed prog attach to map");
> + return -EINVAL;
> + }
Could you plunk a
if (json_output)
jsonw_null(json_wtr);
here to always produce valid JSON even for commands with no output
today?
Same comments for detach.
> + return 0;
> +}
> +
> +static int do_detach(int argc, char **argv)
> +{
> +}
> static int do_load(int argc, char **argv)
> {
> enum bpf_attach_type expected_attach_type;
> @@ -942,6 +1027,7 @@ static int do_help(int argc, char **argv)
> " %s %s pin PROG FILE\n"
> " %s %s load OBJ FILE [type TYPE] [dev NAME] \\\n"
> " [map { idx IDX | name NAME } MAP]\n"
> + " %s %s attach PROG ATTACH_TYPE MAP\n"
> " %s %s help\n"
> "\n"
> " " HELP_SPEC_MAP "\n"
> @@ -953,10 +1039,12 @@ static int do_help(int argc, char **argv)
> " cgroup/bind4 | cgroup/bind6 |
> cgroup/post_bind4 |\n"
> " cgroup/post_bind6 | cgroup/connect4 |
> cgroup/connect6 |\n"
> " cgroup/sendmsg4 | cgroup/sendmsg6 }\n"
> + " ATTACH_TYPE := { msg_verdict | skb_verdict | skb_parse
> }\n"
> " " HELP_SPEC_OPTIONS "\n"
> "",
> bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2],
> - bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2]);
> + bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2],
> + bin_name, argv[-2]);
>
> return 0;
> }
> @@ -968,6 +1056,8 @@ static int do_help(int argc, char **argv)
> { "dump", do_dump },
> { "pin", do_pin },
> { "load", do_load },
> + { "attach", do_attach },
> + { "detach", do_detach },
> { 0 }
> };
Would you mind updating the man page and the bash completions?
