Re: [PATCH v2 net-next 4/4] bpftool: implement cgroup bpf operations

[Quentin Monnet]

2017-12-08 14:12 UTC+0000 ~ Roman Gushchin <guro@xxxxxx>
> On Fri, Dec 08, 2017 at 10:34:16AM +0000, Quentin Monnet wrote:
>> 2017-12-07 18:39 UTC+0000 ~ Roman Gushchin <guro@xxxxxx>
>>> This patch adds basic cgroup bpf operations to bpftool:
>>> cgroup list, attach and detach commands.
>>>
>>> Usage is described in the corresponding man pages,
>>> and examples are provided.
> [...]
>>> +MAP COMMANDS
>>> +=============
>>> +
>>> +|	**bpftool** **cgroup list** *CGROUP*
>>> +|	**bpftool** **cgroup attach** *CGROUP* *ATTACH_TYPE* *PROG* [*ATTACH_FLAGS*]
>>> +|	**bpftool** **cgroup detach** *CGROUP* *ATTACH_TYPE* *PROG*
>>> +|	**bpftool** **cgroup help**
>>> +|
>>> +|	*PROG* := { **id** *PROG_ID* | **pinned** *FILE* | **tag** *PROG_TAG* }
>>
>> Could you please give the different possible values for ATTACH_TYPE and
>> ATTACH_FLAGS, and provide some documentation for the flags?
> 
> I intentionally didn't include the list of possible values, as it depends
> on the exact kernel version, and other bpftool docs are carefully avoiding
> specifying such things.

Do they? As far as I can tell the only other bpftool command that uses
flags is the `bpftool map update`, and it does specify the possible
values for UPDATE_FLAGS (and document them) in the man page.

I don't believe compatibility is an issue here, since the program and
its documentation come together (so they should stay in sync) and are
part of the kernel tree (so the tool should be compatible with the
kernel sources it comes with). My concern is that there is no way to
guess from the current description what the values for ATTACH_FLAG or
ATTACH_TYPE can be, without reading the source code of the programâwhich
is not exactly user-friendly.

> 
> It would be nice to have a way to ask the kernel about provided bpf program types,
> attach types, etc; but I'm not sure that hardcoding it in bpftool docs is
> a good idea.

They are coded into the bpftool that comes with the docs anyway :).

Quentin