Re: [PATCH bpf-next 2/2] bpftool: show flow_dissector attachment status

[Stanislav Fomichev <sdf@fomichev.me>]

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;