From mboxrd@z Thu Jan 1 00:00:00 1970 From: Yonghong Song Subject: Re: [PATCH bpf-next 4/4] tools/bpf: bpftool: add net support Date: Wed, 12 Sep 2018 21:57:29 -0700 Message-ID: <930f9848-5037-df52-5867-ff6fba3eecfe@fb.com> References: <20180905235806.1536396-1-yhs@fb.com> <20180905235806.1536396-5-yhs@fb.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Cc: To: Daniel Borkmann , , Return-path: Received: from mx0a-00082601.pphosted.com ([67.231.145.42]:46242 "EHLO mx0a-00082601.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726427AbeIMKFx (ORCPT ); Thu, 13 Sep 2018 06:05:53 -0400 In-Reply-To: Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 9/12/18 3:29 PM, Daniel Borkmann wrote: > On 09/06/2018 01:58 AM, Yonghong Song wrote: >> Add "bpftool net" support. Networking devices are enumerated >> to dump device index/name associated with xdp progs. >> >> For each networking device, tc classes and qdiscs are enumerated >> in order to check their bpf filters. >> In addition, root handle and clsact ingress/egress are also checked for >> bpf filters. Not all filter information is printed out. Only ifindex, >> kind, filter name, prog_id and tag are printed out, which are good >> enough to show attachment information. If the filter action >> is a bpf action, its bpf program id, bpf name and tag will be >> printed out as well. >> >> For example, >> $ ./bpftool net >> xdp [ >> ifindex 2 devname eth0 prog_id 198 >> ] > > Could we make the output more terse? E.g. the 'ifindex' and 'devname' is basically > zero information but will take lots of space. 'eth0 (2)' would for example make it > shorter. Also info is missing whether the attached prog is driver/hw/generic XDP. :( Right 'eth0 (2)' is a good idea. Similarly to other bpftool plain output, agree we should have concise printout. For the above xdp output, I guess I tested on an old kernel and will test on newer kernel to ensure it works properly. > >> tc_filters [ >> ifindex 2 kind qdisc_htb name prefix_matcher.o:[cls_prefix_matcher_htb] >> prog_id 111727 tag d08fe3b4319bc2fd act [] >> ifindex 2 kind qdisc_clsact_ingress name fbflow_icmp >> prog_id 130246 tag 3f265c7f26db62c9 act [] >> ifindex 2 kind qdisc_clsact_egress name prefix_matcher.o:[cls_prefix_matcher_clsact] >> prog_id 111726 tag 99a197826974c876 >> ifindex 2 kind qdisc_clsact_egress name cls_fg_dscp >> prog_id 108619 tag dc4630674fd72dcc act [] >> ifindex 2 kind qdisc_clsact_egress name fbflow_egress >> prog_id 130245 tag 72d2d830d6888d2c >> ] > > Similar comment here. Do we need the tag here? I think it's not really needed, e.g. > the output of bpftool perf [0] doesn't provide it either and therefore makes the list > as nice one-liners, so overview is much nicer there. Is there a reason that tc progs > do not show dev name as opposed to xdp progs? Can we shorten everything to make it > a one-liner like in bpftool perf? Yes, we should remove 'tag'. Users can use prog_id to find tag. There is no IFNAME attribute in the tc filter return message. But I already got them in previous iplink message, so I can print out here. > Should we have a small indicator here if the tc prog was offloaded? Not sure about this one. As you suggested in the next email, we can have a message like if users want more information they can use more specific tools 'ip link ...', 'tc filter ...' etc. > > Does the dump work with tc shared blocks? I does not.I did not have experiences with tc shared block and that is why I did not add support for it. > > Should we also dump networking related cgroup BPF progs here under bpftool net? probably not, but in additional to the above suggestions about using different tools, we can add a message to suggest check `bpftool cgroup tree` for cgroup related net programs. > > Thanks, > Daniel > > [0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b04df400c30235fa347313c9e2a0695549bd2c8e >