On Sat, Mar 27, 2021 at 04:17:16PM +0100, Toke Høiland-Jørgensen wrote:
Alexei Starovoitov alexei.starovoitov@gmail.com writes:
On Thu, Mar 25, 2021 at 05:30:03PM +0530, Kumar Kartikeya Dwivedi wrote:
This adds some basic tests for the low level bpf_tc_* API and its bpf_program__attach_tc_* wrapper on top.
*_block() apis from patch 3 and 4 are not covered by this selftest. Why were they added ? And how were they tested?
Pls trim your cc. bpf@vger and netdev@vger would have been enough.
My main concern with this set is that it adds netlink apis to libbpf while we already agreed to split xdp manipulation pieces out of libbpf. It would be odd to add tc apis now only to split them later.
We're not removing the ability to attach an XDP program via netlink from libxdp, though. This is the equivalent for TC: the minimum support to attach a program, and if you want to do more, you pull in another library or roll your own.
I'm fine with cutting out more stuff and making this even more minimal (e.g., remove the block stuff and only support attach/detach on ifaces), but we figured we'd err on the side of including too much and getting some feedback from others on which bits are the essential ones to keep, and which can be dropped.
This is up to you. I'm trying to understand the motivation for *_block() apis. I'm not taking a stance for/against them.
I think it's better to start with new library for tc/xdp and have libbpf as a dependency on that new lib. For example we can add it as subdir in tools/lib/bpf/.
I agree for the higher-level stuff (though I'm not sure what that would be for TC), but right now TC programs are the only ones that cannot be attached by libbpf, which is annoying; that's what we're trying to fix.
Sure. I wasn't saying that there is no place for these APIs in libbpf+. Just that existing libbpf is already became a kitchen sink of features that users are not going to use like static linking. tc-api was a straw that broke the camel's back. I think we must move static linking and skeleton out of libbpf before the next release.