Re: [PATCH bpf-next 2/3] bpf: btf: add btf json print functionality

From: Martin KaFai Lau
Date: Thu Jun 21 2018 - 18:51:53 EST


On Thu, Jun 21, 2018 at 02:59:35PM -0700, Jakub Kicinski wrote:
> On Wed, 20 Jun 2018 13:30:53 -0700, Okash Khawaja wrote:
> > $ sudo bpftool map dump -p id 14
> > [{
> > "key": 0
> > },{
> > "value": {
> > "m": 1,
> > "n": 2,
> > "o": "c",
> > "p": [15,16,17,18,15,16,17,18
> > ],
> > "q": [[25,26,27,28,25,26,27,28
> > ],[35,36,37,38,35,36,37,38
> > ],[45,46,47,48,45,46,47,48
> > ],[55,56,57,58,55,56,57,58
> > ]
> > ],
> > "r": 1,
> > "s": 0x7ffff6f70568,
> > "t": {
> > "x": 5,
> > "y": 10
> > },
> > "u": 100,
> > "v": 20,
> > "w1": 0x7,
> > "w2": 0x3
> > }
> > }
> > ]
>
> I don't think this format is okay, JSON output is an API you shouldn't
> break. You can change the non-JSON output whatever way you like, but
> JSON must remain backwards compatible.
>
> The dump today has object per entry, e.g.:
>
> {
> "key":["0x00","0x00","0x00","0x00",
> ],
> "value": ["0x02","0x00","0x00","0x00","0x00","0x00","0x00","0x00"
> ]
> }
>
> This format must remain, you may only augment it with new fields. E.g.:
>
> {
> "key":["0x00","0x00","0x00","0x00",
> ],
> "key_struct":{
> "index":0
> },
> "value": ["0x02","0x00","0x00","0x00","0x00","0x00","0x00","0x00"
> ],
> "value_struct":{
> "src_ip":2,
> "dst_ip:0
> }
> }
I am not sure how useful to have both "key|value" and "(key|value)_struct"
while most people would prefer "key_struct"/"value_struct" if it is
available.

How about introducing a new option, like "-b", to print the
map with BTF (if available) such that it won't break the existing
one (-j or -p) while the "-b" output can keep using the "key"
and "value".

The existing json can be kept as is.

>
> The name XYZ_struct may not be the best, perhaps you can come up with a
> better one?
>
> Does that make sense? Am I missing what you're doing here?
>
> One process note - please make sure you run checkpatch.pl --strict on
> bpftool patches before posting.
>
> Thanks for working on this!