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 <y...@fb.com>
