Re: [iovisor-dev] math between pkt pointer and register with unbounded min value is not allowed #verifier
On Fri, Mar 8, 2019 at 9:22 AM Simon wrote: > > > 35: (69) r3 = *(u16 *)(r7 +38) > 36: (dc) r3 = be16 r3 > > r3 get the value from memory, its value could be any one as permitted > by the type. > > Does it mean that r3 is considered as be16 ? I do not understand why as I > explicitly convert it in u16. The be16 is to convert r3 with big endian encoding. If the host system is big endian, it will do nothing. Otherwise, it will convert from little endian to big endian. > > This output language is a readable format of bpf bytecode, right ? Is there > any documentation to lean/understand it ? Yes, there is no documentation. It intends to be self explanatory. I guess "be16" is special and may need some documentation. Otherwise assembly-style codes should be easy to understand. > > The compiler does the right thing, just verifier is not advanced enough. > > Is it worthy to share this issue of verifier.c with bpf maintainers ? The > compiler which is used here is clang which is called by bcc, right ? I am also a regular kernel/bpf reviewer. The bpf maintainers/community are aware of this limitation. As you mentioned, the verifier is already very complex. To implement complex tracking like described in this thread will make verifier even more complex, hence this is delayed. One of reason is that we have reasonable, although unpleasant, workarounds. Yes, it is compiled with clang. > > Yes, you will need some source workaround. You could try below (untested): > + udp_len = udp_len & 0x1ff; > > I tested it and it seems to work. Thx a lot !! > > But that means I can not use the u16 max value ? You can. I add that because you have a test to limit the range of the value to 511. > > -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#1605): https://lists.iovisor.org/g/iovisor-dev/message/1605 Mute This Topic: https://lists.iovisor.org/mt/30285987/21656 Mute #verifier: https://lists.iovisor.org/mk?hashtag=verifier&subid=2590197 Group Owner: iovisor-dev+ow...@lists.iovisor.org Unsubscribe: https://lists.iovisor.org/g/iovisor-dev/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [iovisor-dev] minutes: IO Visor TSC/Dev Meeting
Hi Guys, My agenda for next meeting: 1) unifying and centralizing XDP statistics accounting [1]. 2) XDP resource management, User API [2]. 3) XDP meta data via btf (in kernel BTF registration). 4) all of the above issues share one common problem, which is the lack of a unified user interface without it, We really can't make a real progress. I just sent a proposal [3] for away to achieve the unified interface, please look it up and let me know your thoughts. [1] https://github.com/xdp-project/xdp-project/blob/master/xdp-project.org#statistics-per-xdp-action [2] https://github.com/xdp-project/xdp-project/blob/master/xdp-project.org#better-ndo_xdp_xmit-resource-management [3] Subject: "[RFC][Proposal] BPF Control MAP" Thanks, Saeed On Wed, Mar 6, 2019 at 12:51 PM Brenden Blanco wrote: > > Hi All, > > Thank you for joining the call today. Here are my notes from the discussion. > > Thanks, > Brenden > > === Discussion === > Brenden: > * Plan to tag release to coincide with kernel 5.0 > > Brendan: > * Speaking this weekend at SCaLE in Los Angeles > > Yonghong: > * LLVM work > * compile once - run anywhere WIP > * support for static variables > > Daniel: > * Global data support work in kernel continues > * Ability to lock maps as read-only > * bugfixes after merge window > > Alexei: > * Some thoughts on future work of BPF > * especially with introduction of BTF > * overall needs concerted effort to improve debuggability > * BTF for programs itself with source/type/layout information > * structures for maps and global data > * suggest to always require type information >(already turned on by default in bcc and supported by llvm) > * Some extra hoops to jump through for driver embedded BPF > * to be enabled with a sysctl > * kernel support is ready > * some long tail of support - e.g. systemd has raw assembly BPF > * kconfig option - eventual deprecation > * if kernel is default strict, llvm should automatically emit BTF as well > * memcg accounting patch status? > * Daniel - still being worked on > * proposal to enable the same accounting for verifier memory > * helps to enable verifier multithreading > > Jakub: > * question regarding global data atomicity > * Daniel - requires read once / write once instructions to work properly > * some todo work on documentation, interpreter + jit implementations > * depends on architecture (machine word size guarantees only) > > Jesper: > * which llvm release supports BTF > * landed in December - will be in 8.0, better in 9.0 > * working on tutorial for xdp at netdev > * https://www.netdevconf.org/0x13/session.html?tutorial-XDP-hands-on > * soliciting feedback > * https://github.com/xdp-project/xdp-tutorial/ > > Saeed: > * request to devote some time in the next meeting to iron out some XDP issues > * please send an agend in reply to the reminder email before next call > * prepare discussion over email in between time > > === Attendees === > Alexei Starovoitov > Marco Leogrande > Mauricio Vasquez > Paul Chaignon > Brenden Blanco > Jiong Wang > Yonghong Song > Daniel Borkmann > Jesper Brouer > Quentin Monnet > Dan Siemon > Jakub Kicinski > Saeed > John > Yutaro > > > -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#1604): https://lists.iovisor.org/g/iovisor-dev/message/1604 Mute This Topic: https://lists.iovisor.org/mt/30289974/21656 Group Owner: iovisor-dev+ow...@lists.iovisor.org Unsubscribe: https://lists.iovisor.org/g/iovisor-dev/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[iovisor-dev] R? min value is negative, either use unsigned or 'var &= const' #verifier
I suspect I fall in another issue with verifier.
This code (taking from katran (
https://github.com/facebookincubator/katran/blob/master/katran/lib/bpf/balancer_helpers.h#L62
) : I just changed data_size type to __u32 ):
__attribute__((__always_inline__)) static inline void ipv4_l4_csum(void
*data_start, __u32 data_size, __u64 *csum, struct iphdr *iph) { __u32 tmp = 0;
*csum = bpf_csum_diff(0, 0, &iph->saddr, sizeof(__be32), *csum); *csum =
bpf_csum_diff(0, 0, &iph->daddr, sizeof(__be32), *csum); tmp =
__builtin_bswap32((__u32)(iph->protocol)); *csum = bpf_csum_diff(0, 0, &tmp,
sizeof(__u32), *csum); tmp = __builtin_bswap32((__u32)(data_size)); *csum =
bpf_csum_diff(0, 0, &tmp, sizeof(__u32), *csum); *csum = bpf_csum_diff(0, 0,
data_start, data_size, *csum); *csum = csum_fold_helper(*csum); } But it brings
to this verifier issue : 93: (67) r0 <<= 32 294: (c7) r0 s>>= 32 295: (b7) r1 =
0 296: (b7) r2 = 0 297: (bf) r3 = r8 298: (79) r4 = *(u64 *)(r10 -40) 299: (bf)
r5 = r0 300: (85) call bpf_csum_diff#28 R4 min value is negative, either use
unsigned or 'var &= const' The whole code is available here (
https://github.com/sbernard31/udploadbalancer ).
This is maybe relative to my previous issue (
https://lists.iovisor.org/g/iovisor-dev/topic/math_between_pkt_pointer_and/30285987?p=,,,20,0,0,0::recentpostdate%2Fsticky,,,20,2,0,30285987
).
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#1603): https://lists.iovisor.org/g/iovisor-dev/message/1603
Mute This Topic: https://lists.iovisor.org/mt/30315706/21656
Mute #verifier: https://lists.iovisor.org/mk?hashtag=verifier&subid=2590197
Group Owner: iovisor-dev+ow...@lists.iovisor.org
Unsubscribe: https://lists.iovisor.org/g/iovisor-dev/unsub
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [iovisor-dev] math between pkt pointer and register with unbounded min value is not allowed #verifier
> > >> 35: (69) r3 = *(u16 *)(r7 +38) >> 36: (dc) r3 = be16 r3 > > r3 get the value from memory, its value could be any one as permitted > by the type. Does it mean that r3 is considered as be16 ? I do not understand why as I explicitly convert it in u16. This output language is a readable format of bpf bytecode, right ? Is there any documentation to lean/understand it ? > > The compiler does the right thing, just verifier is not advanced enough. Is it worthy to share this issue of verifier.c ( https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/kernel/bpf/verifier.c ) with bpf maintainers ( https://www.kernel.org/doc/html/latest/bpf/bpf_devel_QA.html#q-how-do-i-report-bugs-for-bpf-kernel-code ) ? The compiler which is used here is clang which is called by bcc, right ? > > Yes, you will need some source workaround. You could try below (untested): > > + udp_len = udp_len & 0x1ff; I tested it and it seems to work. Thx a lot !! But that means I can not use the u16 max value ? -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#1602): https://lists.iovisor.org/g/iovisor-dev/message/1602 Mute This Topic: https://lists.iovisor.org/mt/30285987/21656 Mute #verifier: https://lists.iovisor.org/mk?hashtag=verifier&subid=2590197 Group Owner: iovisor-dev+ow...@lists.iovisor.org Unsubscribe: https://lists.iovisor.org/g/iovisor-dev/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
