| Message ID | 20190423232200.101627-2-sdf@google.com |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | [bpf-next,1/2] bpf: support BPF_PROG_QUERY for BPF_FLOW_DISSECTOR attach_type | expand |
On Tue, 23 Apr 2019 16:22:00 -0700, Stanislav Fomichev wrote: > +static int query_flow_dissector(struct bpf_attach_info *attach_info) > +{ > + __u32 prog_ids[1] = {0}; > + __u32 prog_cnt = ARRAY_SIZE(prog_ids); > + __u32 attach_flags; > + int fd; > + int err; reverse christmas tree, please :) Move the init into the code if necessary. > + fd = open("/proc/self/ns/net", O_RDONLY); > + if (fd < 0) { > + p_err("can't open /proc/self/ns/net: %d", > + strerror(errno)); > + return -1; > + } > + err = bpf_prog_query(fd, BPF_FLOW_DISSECTOR, 0, > + &attach_flags, prog_ids, &prog_cnt); > + close(fd); > + if (err) { > + if (errno == EINVAL) { > + /* Older kernel's don't support querying > + * flow dissector programs. > + */ > + return 0; > + } > + p_err("can't query prog: %s", strerror(errno)); > + return -1; > + } > + > + if (prog_cnt == 1) > + attach_info->flow_dissector_id = prog_ids[0]; So the count can only be 0 or 1? Hm. > + return 0; > +}
On 04/23, Jakub Kicinski wrote: > On Tue, 23 Apr 2019 16:22:00 -0700, Stanislav Fomichev wrote: > > +static int query_flow_dissector(struct bpf_attach_info *attach_info) > > +{ > > + __u32 prog_ids[1] = {0}; > > + __u32 prog_cnt = ARRAY_SIZE(prog_ids); > > + __u32 attach_flags; > > + int fd; > > + int err; > > reverse christmas tree, please :) > > Move the init into the code if necessary. Ack, initialization didn't let me do that. But I think I can drop it, I don't look prog_cnt in the syscall, maybe I should :-/ > > + fd = open("/proc/self/ns/net", O_RDONLY); > > + if (fd < 0) { > > + p_err("can't open /proc/self/ns/net: %d", > > + strerror(errno)); > > + return -1; > > + } > > + err = bpf_prog_query(fd, BPF_FLOW_DISSECTOR, 0, > > + &attach_flags, prog_ids, &prog_cnt); > > + close(fd); > > + if (err) { > > + if (errno == EINVAL) { > > + /* Older kernel's don't support querying > > + * flow dissector programs. > > + */ > > + return 0; > > + } > > + p_err("can't query prog: %s", strerror(errno)); > > + return -1; > > + } > > + > > + if (prog_cnt == 1) > > + attach_info->flow_dissector_id = prog_ids[0]; > > So the count can only be 0 or 1? Hm. Yes, its either attached or not; there is nothing like prog_array. > > + return 0; > > +}
2019-04-23 16:22 UTC-0700 ~ Stanislav Fomichev <sdf@google.com> > Right now there is no way to query whether BPF flow_dissector program > is attached to a network namespace or not. In previous commit, I added > support for querying that info, show it when doing `bpftool net`: > > $ bpftool prog loadall ./bpf_flow.o \ > /sys/fs/bpf/flow type flow_dissector \ > pinmaps /sys/fs/bpf/flow > $ bpftool prog > 3: flow_dissector name _dissect tag 8c9e917b513dd5cc gpl > loaded_at 2019-04-23T16:14:48-0700 uid 0 > xlated 656B jited 461B memlock 4096B map_ids 1,2 > btf_id 1 > ... > > $ bpftool net -j > [{"xdp":[],"tc":[],"flow_dissector":[]}] > > $ bpftool prog attach pinned \ > /sys/fs/bpf/flow/flow_dissector flow_dissector > $ bpftool net -j > [{"xdp":[],"tc":[],"flow_dissector":["id":3]}] > > Doesn't show up in a different net namespace: > $ ip netns add test > $ ip netns exec test bpftool net -j > [{"xdp":[],"tc":[],"flow_dissector":[]}] > > Non-json output: > $ bpftool net > xdp: > > tc: > > flow_dissector: > id 3 > > Signed-off-by: Stanislav Fomichev <sdf@google.com> > --- > tools/bpf/bpftool/net.c | 52 +++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 52 insertions(+) > > diff --git a/tools/bpf/bpftool/net.c b/tools/bpf/bpftool/net.c > index db0e7de49d49..afe0903201e2 100644 > --- a/tools/bpf/bpftool/net.c > +++ b/tools/bpf/bpftool/net.c > @@ -48,6 +51,10 @@ struct bpf_filter_t { > int ifindex; > }; > > +struct bpf_attach_info { > + __u32 flow_dissector_id; > +}; > + > static int dump_link_nlmsg(void *cookie, void *msg, struct nlattr **tb) > { > struct bpf_netdev_t *netinfo = cookie; > @@ -180,8 +187,43 @@ static int show_dev_tc_bpf(int sock, unsigned int nl_pid, > return 0; > } > > +static int query_flow_dissector(struct bpf_attach_info *attach_info) > +{ > + __u32 prog_ids[1] = {0}; > + __u32 prog_cnt = ARRAY_SIZE(prog_ids); > + __u32 attach_flags; > + int fd; > + int err; > + > + fd = open("/proc/self/ns/net", O_RDONLY); > + if (fd < 0) { > + p_err("can't open /proc/self/ns/net: %d", > + strerror(errno)); > + return -1; > + } > + err = bpf_prog_query(fd, BPF_FLOW_DISSECTOR, 0, > + &attach_flags, prog_ids, &prog_cnt); > + close(fd); > + if (err) { > + if (errno == EINVAL) { > + /* Older kernel's don't support querying > + * flow dissector programs. > + */ > + return 0; Hi Stanislav, If we handle the "error" gracefully here, should we maybe reset errno to 0 before returning? Batch mode, for example, stops processing commands when it sees that errno is set, but we probably want it to continue in the current case (commit 39c9f10639a3 addressed something similar for "bpftool cgroup tree"). > + } > + p_err("can't query prog: %s", strerror(errno)); > + return -1; > + } > + > + if (prog_cnt == 1) > + attach_info->flow_dissector_id = prog_ids[0]; > + > + return 0; > +} > + > static int do_show(int argc, char **argv) > { > + struct bpf_attach_info attach_info = {}; > int i, sock, ret, filter_idx = -1; > struct bpf_netdev_t dev_array; > unsigned int nl_pid;
On 04/24, Quentin Monnet wrote: > 2019-04-23 16:22 UTC-0700 ~ Stanislav Fomichev <sdf@google.com> > > Right now there is no way to query whether BPF flow_dissector program > > is attached to a network namespace or not. In previous commit, I added > > support for querying that info, show it when doing `bpftool net`: > > > > $ bpftool prog loadall ./bpf_flow.o \ > > /sys/fs/bpf/flow type flow_dissector \ > > pinmaps /sys/fs/bpf/flow > > $ bpftool prog > > 3: flow_dissector name _dissect tag 8c9e917b513dd5cc gpl > > loaded_at 2019-04-23T16:14:48-0700 uid 0 > > xlated 656B jited 461B memlock 4096B map_ids 1,2 > > btf_id 1 > > ... > > > > $ bpftool net -j > > [{"xdp":[],"tc":[],"flow_dissector":[]}] > > > > $ bpftool prog attach pinned \ > > /sys/fs/bpf/flow/flow_dissector flow_dissector > > $ bpftool net -j > > [{"xdp":[],"tc":[],"flow_dissector":["id":3]}] > > > > Doesn't show up in a different net namespace: > > $ ip netns add test > > $ ip netns exec test bpftool net -j > > [{"xdp":[],"tc":[],"flow_dissector":[]}] > > > > Non-json output: > > $ bpftool net > > xdp: > > > > tc: > > > > flow_dissector: > > id 3 > > > > Signed-off-by: Stanislav Fomichev <sdf@google.com> > > --- > > tools/bpf/bpftool/net.c | 52 +++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 52 insertions(+) > > > > diff --git a/tools/bpf/bpftool/net.c b/tools/bpf/bpftool/net.c > > index db0e7de49d49..afe0903201e2 100644 > > --- a/tools/bpf/bpftool/net.c > > +++ b/tools/bpf/bpftool/net.c > > > @@ -48,6 +51,10 @@ struct bpf_filter_t { > > int ifindex; > > }; > > +struct bpf_attach_info { > > + __u32 flow_dissector_id; > > +}; > > + > > static int dump_link_nlmsg(void *cookie, void *msg, struct nlattr **tb) > > { > > struct bpf_netdev_t *netinfo = cookie; > > @@ -180,8 +187,43 @@ static int show_dev_tc_bpf(int sock, unsigned int nl_pid, > > return 0; > > } > > +static int query_flow_dissector(struct bpf_attach_info *attach_info) > > +{ > > + __u32 prog_ids[1] = {0}; > > + __u32 prog_cnt = ARRAY_SIZE(prog_ids); > > + __u32 attach_flags; > > + int fd; > > + int err; > > + > > + fd = open("/proc/self/ns/net", O_RDONLY); > > + if (fd < 0) { > > + p_err("can't open /proc/self/ns/net: %d", > > + strerror(errno)); > > + return -1; > > + } > > + err = bpf_prog_query(fd, BPF_FLOW_DISSECTOR, 0, > > + &attach_flags, prog_ids, &prog_cnt); > > + close(fd); > > + if (err) { > > + if (errno == EINVAL) { > > + /* Older kernel's don't support querying > > + * flow dissector programs. > > + */ > > + return 0; > > Hi Stanislav, > > If we handle the "error" gracefully here, should we maybe reset errno to 0 > before returning? Batch mode, for example, stops processing commands when it > sees that errno is set, but we probably want it to continue in the current > case (commit 39c9f10639a3 addressed something similar for "bpftool cgroup > tree"). Oh, I didn't know that. Sure, will do! > > + } > > + p_err("can't query prog: %s", strerror(errno)); > > + return -1; > > + } > > + > > + if (prog_cnt == 1) > > + attach_info->flow_dissector_id = prog_ids[0]; > > + > > + return 0; > > +} > > + > > static int do_show(int argc, char **argv) > > { > > + struct bpf_attach_info attach_info = {}; > > int i, sock, ret, filter_idx = -1; > > struct bpf_netdev_t dev_array; > > unsigned int nl_pid;
diff --git a/tools/bpf/bpftool/net.c b/tools/bpf/bpftool/net.c index db0e7de49d49..afe0903201e2 100644 --- a/tools/bpf/bpftool/net.c +++ b/tools/bpf/bpftool/net.c @@ -3,6 +3,7 @@ #define _GNU_SOURCE #include <errno.h> +#include <fcntl.h> #include <stdlib.h> #include <string.h> #include <unistd.h> @@ -12,6 +13,8 @@ #include <linux/rtnetlink.h> #include <linux/tc_act/tc_bpf.h> #include <sys/socket.h> +#include <sys/stat.h> +#include <sys/types.h> #include <bpf.h> #include <nlattr.h> @@ -48,6 +51,10 @@ struct bpf_filter_t { int ifindex; }; +struct bpf_attach_info { + __u32 flow_dissector_id; +}; + static int dump_link_nlmsg(void *cookie, void *msg, struct nlattr **tb) { struct bpf_netdev_t *netinfo = cookie; @@ -180,8 +187,43 @@ static int show_dev_tc_bpf(int sock, unsigned int nl_pid, return 0; } +static int query_flow_dissector(struct bpf_attach_info *attach_info) +{ + __u32 prog_ids[1] = {0}; + __u32 prog_cnt = ARRAY_SIZE(prog_ids); + __u32 attach_flags; + int fd; + int err; + + fd = open("/proc/self/ns/net", O_RDONLY); + if (fd < 0) { + p_err("can't open /proc/self/ns/net: %d", + strerror(errno)); + return -1; + } + err = bpf_prog_query(fd, BPF_FLOW_DISSECTOR, 0, + &attach_flags, prog_ids, &prog_cnt); + close(fd); + if (err) { + if (errno == EINVAL) { + /* Older kernel's don't support querying + * flow dissector programs. + */ + return 0; + } + p_err("can't query prog: %s", strerror(errno)); + return -1; + } + + if (prog_cnt == 1) + attach_info->flow_dissector_id = prog_ids[0]; + + return 0; +} + static int do_show(int argc, char **argv) { + struct bpf_attach_info attach_info = {}; int i, sock, ret, filter_idx = -1; struct bpf_netdev_t dev_array; unsigned int nl_pid; @@ -199,6 +241,10 @@ static int do_show(int argc, char **argv) usage(); } + ret = query_flow_dissector(&attach_info); + if (ret) + return -1; + sock = libbpf_netlink_open(&nl_pid); if (sock < 0) { fprintf(stderr, "failed to open netlink sock\n"); @@ -227,6 +273,12 @@ static int do_show(int argc, char **argv) } NET_END_ARRAY("\n"); } + + NET_START_ARRAY("flow_dissector", "%s:\n"); + if (attach_info.flow_dissector_id > 0) + NET_DUMP_UINT("id", "id %u", attach_info.flow_dissector_id); + NET_END_ARRAY("\n"); + NET_END_OBJECT; if (json_output) jsonw_end_array(json_wtr);
Right now there is no way to query whether BPF flow_dissector program is attached to a network namespace or not. In previous commit, I added support for querying that info, show it when doing `bpftool net`: $ bpftool prog loadall ./bpf_flow.o \ /sys/fs/bpf/flow type flow_dissector \ pinmaps /sys/fs/bpf/flow $ bpftool prog 3: flow_dissector name _dissect tag 8c9e917b513dd5cc gpl loaded_at 2019-04-23T16:14:48-0700 uid 0 xlated 656B jited 461B memlock 4096B map_ids 1,2 btf_id 1 ... $ bpftool net -j [{"xdp":[],"tc":[],"flow_dissector":[]}] $ bpftool prog attach pinned \ /sys/fs/bpf/flow/flow_dissector flow_dissector $ bpftool net -j [{"xdp":[],"tc":[],"flow_dissector":["id":3]}] Doesn't show up in a different net namespace: $ ip netns add test $ ip netns exec test bpftool net -j [{"xdp":[],"tc":[],"flow_dissector":[]}] Non-json output: $ bpftool net xdp: tc: flow_dissector: id 3 Signed-off-by: Stanislav Fomichev <sdf@google.com> --- tools/bpf/bpftool/net.c | 52 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+)