On Sun, 1 Jul 2018 11:31:47 +0100, Okash Khawaja wrote:
> On Wed, Jun 27, 2018 at 02:56:49PM +0200, Daniel Borkmann wrote:
> > On 06/27/2018 01:47 PM, Okash Khawaja wrote:
> > > On Wed, Jun 27, 2018 at 12:34:35PM +0200, Daniel Borkmann wrote:
> > >> On 06/27/2018 12:35 AM, Jakub Kicinski wrote:
> > >>> On Tue, 26 Jun 2018 15:27:09 -0700, Martin KaFai Lau wrote:
> > >>>> On Tue, Jun 26, 2018 at 01:31:33PM -0700, Jakub Kicinski wrote:
> > >> [...]
> > >>>>> Implementing both outputs in one series will help you structure your
> > >>>>> code to best suit both of the formats up front.
> > >>>> hex and "formatted" are the only things missing? As always, things
> > >>>> can be refactored when new use case comes up. Lets wait for
> > >>>> Okash input.
> > >>>>
> > >>>> Regardless, plaintext is our current use case. Having the current
> > >>>> patchset in does not stop us or others from contributing other use
> > >>>> cases (json, "bpftool map find"...etc), and IMO it is actually
> > >>>> the opposite. Others may help us get there faster than us alone.
> > >>>> We should not stop making forward progress and take this patch
> > >>>> as hostage because "abc" and "xyz" are not done together.
> > >>>
> > >>> Parity between JSON and plain text output is non negotiable.
> > >>
> > >> Longish discussion and some confusion in this thread. :-) First of all
> > >> thanks a lot for working on it, very useful!
> > > Thanks :)
> > >
> > >> My $0.02 on it is that so far
> > >> great care has been taken in bpftool to indeed have feature parity
> > >> between
> > >> JSON and plain text, so it would be highly desirable to keep continuing
> > >> this practice if the consensus is that it indeed is feasible and makes
> > >> sense wrt BTF data. There has been mentioned that given BTF data can be
> > >> dynamic depending on what the user loads via bpf(2) so a potential JSON
> > >> output may look different/break each time anyway. This however could all
> > >> be
> > >> embedded under a container object that has a fixed key like 'formatted'
> > >> where tools like jq(1) can query into it. I think this would be fine
> > >> since
> > >> the rest of the (non-dynamic) output is still retained as-is and then
> > >> wouldn't confuse or collide with existing users, and anyone
> > >> programmatically
> > >> parsing deeper into the BTF data under such JSON container object needs
> > >> to have awareness of what specific data it wants to query from it; so
> > >> there's no conflict wrt breaking anything here. Imho, both outputs would
> > >> be very valuable.
> > > Okay I can add "formatted" object under json output.
> > >
> > > One thing to note here is that the fixed output will change if the map
> > > itself changes. So someone writing a program that consumes that fixed
> > > output will have to account for his program breaking in future, thus
> >
> > Yes, that aspect is fine though, any program/script parsing this would need
> > to be aware of the underlying map type to make sense of it (e.g. per-cpu vs
> > non per-cpu maps to name one). But that info it could query/verify already
> > beforehand via bpftool as well (via normal map info dump for a given id).
> >
> > > breaking backward compatibility anyway as far as the developer is
> > > concerned :)
> > >
> > > I will go ahead with work on "formatted" object.
> >
> > Cool, thanks,
> > Daniel
>
>
> hi,
>
> couple of questions:
>
> 1. just to be sure, formatted section will be on the same level as "key"
> and "value"? so something like following:
>
>
> $ bpftool map dump -p id 8
> [{
> "key": ["0x00","0x00","0x00","0x00"
> ],
> "value": [...
> ],
> "formatted": {
> "key": 0,
> "value": {
> "int_field": 3,
> "pointerfield": 2152930552,
> ...
> }
> }
> }]
Looks good, yes!
> 2. i noticed that the ouput in v1 has all the keys and values on the
> same level. in v2, i'll change them so that each key-value pair is a
> separate object. let me know what you think.
For non-JSON output? No preference, whatever looks better :) Empty
line between key/value pairs to visually separate them could also
work. But up to you.
> finally, i noticed there is a map lookup command which also prints map
> entries. do want that to also be btf-printed in this patchset?
It would be nice to share the printing code for the two, yes.
