netdev - Re: [PATCH bpf-next] tools/bpf: bpftool: improve output format for bpftool net

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]

Message-ID: <5e65458e-aec5-baf7-9e0f-50e3647542ff@iogearbox.net>
Date:   Mon, 17 Sep 2018 12:19:40 +0200
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Yonghong Song <yhs@...com>, ast@...com, netdev@...r.kernel.org
Cc:     kernel-team@...com
Subject: Re: [PATCH bpf-next] tools/bpf: bpftool: improve output format for
 bpftool net

On 09/14/2018 11:49 PM, Yonghong Song wrote:
> This is a followup patch for Commit f6f3bac08ff9
> ("tools/bpf: bpftool: add net support").
> Some improvements are made for the bpftool net output.
> Specially, plain output is more concise such that
> per attachment should nicely fit in one line.
> Compared to previous output, the prog tag is removed
> since it can be easily obtained with program id.
> Similar to xdp attachments, the device name is added
> to tc_filters attachments.
> 
> The bpf program attached through shared block
> mechanism is supported as well.
>   $ ip link add dev v1 type veth peer name v2
>   $ tc qdisc add dev v1 ingress_block 10 egress_block 20 clsact
>   $ tc qdisc add dev v2 ingress_block 10 egress_block 20 clsact
>   $ tc filter add block 10 protocol ip prio 25 bpf obj bpf_shared.o sec ingress flowid 1:1
>   $ tc filter add block 20 protocol ip prio 30 bpf obj bpf_cyclic.o sec classifier flowid 1:1
>   $ bpftool net
>   xdp [
>   ]
>   tc_filters [
>    v2(7) qdisc_clsact_ingress bpf_shared.o:[ingress] id 23
>    v2(7) qdisc_clsact_egress bpf_cyclic.o:[classifier] id 24
>    v1(8) qdisc_clsact_ingress bpf_shared.o:[ingress] id 23
>    v1(8) qdisc_clsact_egress bpf_cyclic.o:[classifier] id 24

Just one minor note for this one here, do we even need the "qdisc_" prefix? Couldn't it just simply
be "clsact/ingress", "clsact/egress", "htb" etc?

>   ]
> 
> The documentation and "bpftool net help" are updated
> to make it clear that current implementation only
> supports xdp and tc attachments. For programs
> attached to cgroups, "bpftool cgroup" can be used
> to dump attachments. For other programs e.g.
> sk_{filter,skb,msg,reuseport} and lwt/seg6,
> iproute2 tools should be used.
> 
> The new output:
>   $ bpftool net
>   xdp [
>    eth0(2) id/drv 198

Could we change the "id/{drv,offload,generic} xyz" into e.g. "eth0(2) {driver,offload,generic} id 198",
meaning, the "id xyz" being a child of either "driver", "offload" or "generic". Reason would be two-fold:
i) we can keep the "id xyz" notion consistent as used under "tc_filters", and ii) it allows to put further
information aside from just "id" member under "driver", "offload" or "generic" in the future.

>   ]
>   tc_filters [

Nit: can we use just "tc" for the above? Main use case would be clsact with one of its two hooks anyway,
and the term "filter" is sort of tc historic; while being correct bpf progs would do much more than just
filtering, and context is pretty clear anyway from qdisc that we subsequently dump.

>    eth0(2) qdisc_clsact_ingress fbflow_icmp id 335 act [{icmp_action id 336}]
>    eth0(2) qdisc_clsact_egress fbflow_egress id 334
>   ]
>   $ bpftool -jp net
>   [{
>         "xdp": [{
>                 "devname": "eth0",
>                 "ifindex": 2,
>                 "id/drv": 198
>             }
>         ],
>         "tc_filters": [{
>                 "devname": "eth0",
>                 "ifindex": 2,
>                 "kind": "qdisc_clsact_ingress",
>                 "name": "fbflow_icmp",
>                 "id": 335,
>                 "act": [{
>                         "name": "icmp_action",
>                         "id": 336
>                     }
>                 ]
>             },{
>                 "devname": "eth0",
>                 "ifindex": 2,
>                 "kind": "qdisc_clsact_egress",
>                 "name": "fbflow_egress",
>                 "id": 334
>             }
>         ]
>     }
>   ]
> 
> Signed-off-by: Yonghong Song <yhs@...com>