netdev - Re: [PATCH bpf-next 4/4] tools/bpf: bpftool: add net support

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] [day] [month] [year] [list]

Message-ID: <0c7af0da-e812-600b-d18c-482726099902@fb.com>
Date:   Wed, 12 Sep 2018 22:01:02 -0700
From:   Yonghong Song <yhs@...com>
To:     Daniel Borkmann <daniel@...earbox.net>, <ast@...com>,
        <netdev@...r.kernel.org>
CC:     <kernel-team@...com>
Subject: Re: [PATCH bpf-next 4/4] tools/bpf: bpftool: add net support



On 9/12/18 3:40 PM, Daniel Borkmann wrote:
> On 09/13/2018 12:29 AM, 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. :(
>>
>>>    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?
>>
>> Should we have a small indicator here if the tc prog was offloaded?
>>
>> Does the dump work with tc shared blocks?
>>
>> Should we also dump networking related cgroup BPF progs here under bpftool net?
> 
> One more thought, I think it would make sense to also explicitly document current
> limitations of the progs that this listing does not show where user should consult
> other tools aka iproute2 e.g. lwt, seg6, tc bpf actions, etc, so the current scope
> would be more clear for users.

I will do a followup patch to address a few issues mentioned in my 
previous email (mostly more concise output, briefly mentioning 
limitation of the tool, etc.) and also add more information in the
documentation and also in the 'bpf net help' output to set user's
expectation right about what this tool can do and how to find more
information.

> 
>> Thanks,
>> Daniel
>>
>>    [0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b04df400c30235fa347313c9e2a0695549bd2c8e
>>
>