Omar Sandoval <osandov@xxxxxx> [Thu, 2020-02-27 14:19 -0800]: > 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@xxxxxxxxxxx> [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. Daniel, Omar, thanks for your input. This all makes sense and it seems we're coming to an agreement that it'll be easier to support tools like this in drgn repo. That works for me and with this in mind I see the followig way forward: - I'll submit PR to drgn repo to create a new tools/ directory and place the script there using the bpf_inspect.py name suggested by Andrii. This way the script will be in sync with all the recent changes to drgn helpers; - As soon as PR is merged I'll add documentation to kernel tree to make folks aware that such a tool exists, if there are any recommendation where to add the documentation to I'd appreciate it. - I'll try to make the script less dependent on kernel version (Quentin's feedba) since it'll exist separate from the kernel sources. Hope it works for everyone, please lmk if it doesn't for some reason of if I missed something. Thanks. -- Andrey Ignatov
