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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Tue, 11 Jul 2023 18:46:29 +0200
From: Daniel Borkmann <daniel@...earbox.net>
To: Quentin Monnet <quentin@...valent.com>, ast@...nel.org
Cc: andrii@...nel.org, martin.lau@...ux.dev, razor@...ckwall.org,
 sdf@...gle.com, john.fastabend@...il.com, kuba@...nel.org, dxu@...uu.xyz,
 joe@...ium.io, toke@...nel.org, davem@...emloft.net, bpf@...r.kernel.org,
 netdev@...r.kernel.org
Subject: Re: [PATCH bpf-next v4 6/8] bpftool: Extend net dump with tcx progs

On 7/11/23 4:19 PM, Quentin Monnet wrote:
> 2023-07-10 22:12 UTC+0200 ~ Daniel Borkmann <daniel@...earbox.net>
>> 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@...earbox.net>
> 
> Reviewed-by: Quentin Monnet <quentin@...valent.com>
> 
> 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