Re: [PATCH bpf-next] bpf: Add drgn script to list progs/maps

[Omar Sandoval <osandov@fb.com>]

On 2/27/20 1:32 PM, Daniel Borkmann wrote:
> On 2/27/20 10:11 PM, Daniel Borkmann wrote:
>> [ +tj ]
>>
>> On 2/27/20 7:26 PM, Andrey Ignatov wrote:
>>> Stanislav Fomichev <sdf@fomichev.me> [Thu, 2020-02-27 10:01 -0800]:
>>>> On 02/26, Andrey Ignatov wrote:
>>>>> drgn is a debugger that reads kernel memory and uses DWARF to get types
>>>>> and symbols. See [1], [2] and [3] for more details on drgn.
>>>>>
>>>>> Since drgn operates on kernel memory it has access to kernel internals
>>>>> that user space doesn't. It allows to get extended info about various
>>>>> kernel data structures.
>>>>>
>>>>> Introduce bpf.py drgn script to list BPF programs and maps and their
>>>>> properties unavailable to user space via kernel API.
>>>> Any reason this is not pushed to https://github.com/osandov/drgn/ ?
>>>> I have a bunch of networking helpers for drgn as well, but I was
>>>> thinking about contributing them to the drgn github, not the kernel.
>>>> IMO, seems like a better place to consolidate all drgn stuff.
>>>
>>> I have this part in the commit message:
>>>
>>>>> The script can be sent to drgn repo where it's easier to maintain its
>>>>> "drgn-ness", but in kernel tree it should be easier to maintain BPF
>>>>> functionality itself what can be more important in this case.
>>>
>>> That's being said it's debatable which place is better and I'm still
>>> trying to figure it out myself since, from what i see, there is no
>>> widely adopted practice.
>>>
>>> I've been contributing to drgn as well mostly in two forms:
>>> * helpers [1];
>>> * examples [2]
>>>
>>> And so far I used examples/ dir as a place to keep small useful "tools"
>>> (tcp_sock.py, cgroup.py, bpf.py).
>>>
>>> But there is no place for bigger "scripts" or "tools" in drgn (yet?). On
>>> the other hand I see two drgn scripts in kernel tree already:
>>> * tools/cgroup/iocost_coef_gen.py
>>> * tools/cgroup/iocost_monitor.py
>>>
>>> So maybe it's time to discuss where to keep tools like this in the
>>> future.
>>>
>>> In this specifc case I'd love to see feedback from Omar and BPF
>>> maintainers.
>>
>> I can certainly see both sides given that drgn tools have been added to
>> tools/cgroup/ already. I presume if so, then these could live in tools/drgn/
>> which would also make it more clear what is needed to run as dependency
>> plus there should be be a proper high-level readme to document what developers
>> need to run in order to run them. But from looking at [1], I can also see that
>> those scripts would depend on new helpers being added/updated/deleted, so it
>> might be easier to add drgn/tools/ directory where scripts could be updated
>> in one go with updates to drgn helpers. Either way, I think it would be nice
>> to add documentation somewhere for getting people started.
> 
> One example that should definitely be avoided is 9ea37e24d4a9 ("iocost: Fix
> iocost_monitor.py due to helper type mismatch") due to both living in separate
> places. A third option to think about (if this is to be adapted by more subsystems)
> could be to have all the kernel-specific helpers from drgn/helpers/linux under
> tools/drgn/helpers/ in the kernel tree and the tools living under
> tools/drgn/<subsys>/ e.g. tools/drgn/bpf/.
> 
>>> [1] https://github.com/osandov/drgn/tree/master/drgn/helpers/linux
>>> [2] https://github.com/osandov/drgn/tree/master/examples/linux
> 
I can think of a few benefits of having this tool (and others like it)
in the drgn repository:

* Easier to keep in sync with new helpers/API changes
* More examples in one centralized place for people building new tools
* Potential to identify pain points in the API and possible new helpers

I think this would benefit the drgn project as a whole.

The downsides:

* More maintenance for me
* Tools will have to support multiple kernel versions (as opposed to
  only supporting the kernel that they shipped with)
* Less visibility for kernel developers

That second point is true of the helpers bundled with drgn anyways, so I
don't think it's a big deal. The third point will improve over time as
we get more people on the drgn train :)

I may come to regret the first point, but I think the upsides are worth
it. Andrey, feel free to submit a PR adding this to the drgn repository
under a new top-level tools/ directory.