From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jakub Kicinski Subject: Re: [PATCH bpf-next 2/3] bpf: btf: add btf json print functionality Date: Fri, 22 Jun 2018 16:32:00 -0700 Message-ID: <20180622163200.20564ec4@cakuba.netronome.com> References: <20180620203703.101156292@fb.com> <20180621145935.41ff8974@cakuba.netronome.com> <20180621225117.dhrkrtmkfbeihbe4@kafai-mbp.dhcp.thefacebook.com> <20180621160719.2cfb4b58@cakuba.netronome.com> <20180621235746.dfq6kdtkogftw3ws@kafai-mbp.dhcp.thefacebook.com> <20180621172523.6cd00ed1@cakuba.netronome.com> <20180622012052.htkvholi674x6i4f@kafai-mbp.dhcp.thefacebook.com> <20180622114032.162b2a76@cakuba.netronome.com> <20180622205535.c6vjhdwt5er4wc32@kafai-mbp.dhcp.thefacebook.com> <20180622142743.2b890d0f@cakuba.netronome.com> <20180622225408.jor7lpvsksnwiiec@kafai-mbp.dhcp.thefacebook.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: Okash Khawaja , Daniel Borkmann , "Alexei Starovoitov" , Yonghong Song , Quentin Monnet , "David S. Miller" , , , To: Martin KaFai Lau Return-path: In-Reply-To: <20180622225408.jor7lpvsksnwiiec@kafai-mbp.dhcp.thefacebook.com> Sender: linux-kernel-owner@vger.kernel.org List-Id: netdev.vger.kernel.org On Fri, 22 Jun 2018 15:54:08 -0700, Martin KaFai Lau wrote: > > > > > > > > > > "value": ["0x02","0x00","0x00","0x00","0x00","0x00","0x00","0x00" > > > > > > > > > > ], > > > > > > > > > > "value_struct":{ > > > > > > > > > > "src_ip":2, > > > > > If for the same map the user changes the "src_ip" to an array of int[4] > > > > > later (e.g. to support ipv6), it will become "src_ip": [1, 2, 3, 4]. > > > > > Is it breaking backward compat? > > > > > i.e. > > > > > struct five_tuples { > > > > > - int src_ip; > > > > > + int src_ip[4]; > > > > > /* ... */ > > > > > }; > > > > > > > > Well, it is breaking backward compat, but it's the program doing it, > > > > not bpftool :) BTF changes so does the output. > > > As we see, the key/value's btf-output is inherently not backward compat. > > > Hence, "-j" and "-p" will stay as is. The whole existing json will > > > be backward compat instead of only partly backward compat. > > > > No. There is a difference between user of a facility changing their > > input and kernel/libraries providing different output in response to > > that, and the libraries suddenly changing the output on their own. > > > > Your example is like saying if user started using IPv6 addresses > > instead of IPv4 the netlink attributes in dumps will be different so > > kernel didn't keep backwards compat. While what you're doing is more > > equivalent to dropping support for old ioctl interfaces because there > > is a better mechanism now. > Sorry, I don't follow this. I don't see netlink suffer json issue like > the one on "key" and "value". > > All I can grasp is, the json should normally be backward compat but now > we are saying anything added by btf-output is an exception because > the script parsing it will treat it differently than "key" and "value" Backward compatibility means that if I run *the same* program against different kernels/libraries it continues to work. If someone decides to upgrade their program to work with IPv6 (which was your example) obviously there is no way system as a whole will look 1:1 the same. > > BTF in JSON is very useful, and will help people who writes simple > > orchestration/scripts based on bpftool *a* *lot*. I really appreciate > Can you share what the script will do? I want to understand why > it cannot directly use the BTF format and the map data. Think about a python script which wants to read a counter in a map. Right now it would have to get the BTF, find out which bytes are the counter, then convert the bytes into a larger int. With JSON BTF if just does entry["formatted"]["value"]["counter"]. Real life example from my test code (conversion of 3 element counter array): def str2int(strtab): inttab = [] for i in strtab: inttab.append(int(i, 16)) ba = bytearray(inttab) if len(strtab) == 4: fmt = "I" elif len(strtab) == 8: fmt = "Q" else: raise Exception("String array of len %d can't be unpacked to an int" % (len(strtab))) return struct.unpack(fmt, ba)[0] def convert(elems, idx): val = [] for i in range(3): part = elems[idx]["value"][i * length:(i + 1) * length] val.append(str2int(part)) return val With BTF it would be: elems[idx]["formatted"]["value"] Which is fairly awesome. > > this addition to bpftool and will start using it myself as soon as it > > lands. I'm not sure why the reluctance to slightly change the output > > format? > The initial change argument is because the json has to be backward compat. > > Then we show that btf-output is inherently not backward compat, so > printing it in json does not make sense at all. > > However, now it is saying part of it does not have to be backward compat. Compatibility of "formatted" member is defined as -> fields broken out according to BTF. So it is backward compatible. The definition of "value" member is -> an array of unfortunately formatted array of ugly hex strings :( > I am fine putting it under "formatted" for "-j" or "-p" if that is the > case, other than the double output is still confusing. Lets wait for > Okash's input. > > At the same time, the same output will be used as the default plaintext > output when BTF is available. Then the plaintext BTF output > will not be limited by the json restrictions when we want > to improve human readability later. Apparently, the > improvements on plaintext will not be always applicable > to json output.