Re: [PATCH bpf-next v4 6/8] bpftool: Extend net dump with tcx progs

[Daniel Borkmann <daniel@xxxxxxxxxxxxx>]

On 7/11/23 4:19 PM, Quentin Monnet wrote:
> 2023-07-10 22:12 UTC+0200 ~ Daniel Borkmann <daniel@xxxxxxxxxxxxx>
> > Add support to dump fd-based attach types via bpftool. This includes both
> > the tc BPF link and attach ops programs. Dumped information contain the
> > attach location, function entry name, program ID and link ID when applicable.
> >
> > Example with tc BPF link:
> >
> >    # ./bpftool net
> >    xdp:
> >
> >    tc:
> >    bond0(4) tcx/ingress cil_from_netdev prog id 784 link id 10
> >    bond0(4) tcx/egress cil_to_netdev prog id 804 link id 11
> >
> >    flow_dissector:
> >
> >    netfilter:
> >
> > Example with tc BPF attach ops:
> >
> >    # ./bpftool net
> >    xdp:
> >
> >    tc:
> >    bond0(4) tcx/ingress cil_from_netdev prog id 654
> >    bond0(4) tcx/egress cil_to_netdev prog id 672
> >
> >    flow_dissector:
> >
> >    netfilter:
> >
> > Signed-off-by: Daniel Borkmann <daniel@xxxxxxxxxxxxx>
>
> Reviewed-by: Quentin Monnet <quentin@xxxxxxxxxxxxx>
>
> Thank you!
>
> If you respin, would you mind updating the docs please
> (Documentation/bpftool-net.rst), I realise it says that "bpftool net"
> only dumps for tc and XDP, but that's not true any more since we have
> the flow dissector, netfilter programs, and now tcx. The examples are
> out-of-date too, but updating them doesn't have to be part of this PR.

Good point, I updated the docs and help usage to reflect that.

> >   tools/bpf/bpftool/net.c | 86 +++++++++++++++++++++++++++++++++++++++--
> >   1 file changed, 82 insertions(+), 4 deletions(-)
> >
> > diff --git a/tools/bpf/bpftool/net.c b/tools/bpf/bpftool/net.c
> > index 26a49965bf71..22af0a81458c 100644
> > --- a/tools/bpf/bpftool/net.c
> > +++ b/tools/bpf/bpftool/net.c
> > @@ -76,6 +76,11 @@ static const char * const attach_type_strings[] = {
> >   	[NET_ATTACH_TYPE_XDP_OFFLOAD]	= "xdpoffload",
> >   };
> >   
> > +static const char * const attach_loc_strings[] = {
> > +	[BPF_TCX_INGRESS]		= "tcx/ingress",
> > +	[BPF_TCX_EGRESS]		= "tcx/egress",
> > +};
> > +
> >   const size_t net_attach_type_size = ARRAY_SIZE(attach_type_strings);
> >   
> >   static enum net_attach_type parse_attach_type(const char *str)
> > @@ -422,8 +427,80 @@ static int dump_filter_nlmsg(void *cookie, void *msg, struct nlattr **tb)
> >   			      filter_info->devname, filter_info->ifindex);
> >   }
> >   
> > -static int show_dev_tc_bpf(int sock, unsigned int nl_pid,
> > -			   struct ip_devname_ifindex *dev)
> > +static const char *flags_strings(__u32 flags)
> > +{
> > +	return json_output ? "none" : "";
> > +}
> > +
> > +static int __show_dev_tc_bpf_name(__u32 id, char *name, size_t len)
> > +{
> > +	struct bpf_prog_info info = {};
> > +	__u32 ilen = sizeof(info);
> > +	int fd, ret;
> > +
> > +	fd = bpf_prog_get_fd_by_id(id);
> > +	if (fd < 0)
> > +		return fd;
> > +	ret = bpf_obj_get_info_by_fd(fd, &info, &ilen);
> > +	if (ret < 0)
> > +		goto out;
> > +	ret = -ENOENT;
> > +	if (info.name[0]) {
> > +		get_prog_full_name(&info, fd, name, len);
> > +		ret = 0;
> > +	}
> > +out:
> > +	close(fd);
> > +	return ret;
> > +}
> > +
> > +static void __show_dev_tc_bpf(const struct ip_devname_ifindex *dev,
> > +			      const enum bpf_attach_type loc)
> > +{
> > +	__u32 prog_flags[64] = {}, link_flags[64] = {}, i;
> > +	__u32 prog_ids[64] = {}, link_ids[64] = {};
> > +	LIBBPF_OPTS(bpf_prog_query_opts, optq);
> > +	char prog_name[MAX_PROG_FULL_NAME];
> > +	int ret;
> > +
> > +	optq.prog_ids = prog_ids;
> > +	optq.prog_attach_flags = prog_flags;
> > +	optq.link_ids = link_ids;
> > +	optq.link_attach_flags = link_flags;
> > +	optq.count = ARRAY_SIZE(prog_ids);
> > +
> > +	ret = bpf_prog_query_opts(dev->ifindex, loc, &optq);
> > +	if (ret)
> > +		return;
> > +	for (i = 0; i < optq.count; i++) {
> > +		NET_START_OBJECT;
> > +		NET_DUMP_STR("devname", "%s", dev->devname);
> > +		NET_DUMP_UINT("ifindex", "(%u)", dev->ifindex);
> > +		NET_DUMP_STR("kind", " %s", attach_loc_strings[loc]);
> > +		ret = __show_dev_tc_bpf_name(prog_ids[i], prog_name,
> > +					     sizeof(prog_name));
> > +		if (!ret)
> > +			NET_DUMP_STR("name", " %s", prog_name);
> > +		NET_DUMP_UINT("prog_id", " prog id %u", prog_ids[i]);
>
> I was unsure at first about having two words for "prog id", or "link id"
> below (we use "prog_id" for netfilter, for example), but I see it leaves
> you the opportunity to append the flags, if any, without additional
> keywords so... why not.

Ok, I'll change it into prog_id, link_id for consistency for the human readable output.

And some like flow dissector just dump 'id'. After sync with Quentin, I tracked this
in [0] to more streamline the net dump output for other types.

  [0] https://github.com/libbpf/bpftool/issues/106

Thanks,
Daniel