yonghong-song added a comment. @rsmith @eli.friedman Thanks for your comments. I fully agree that it seems awkward that we have both GEP and intrinsic generation. I will try to do some experiments here to only have intrinsic generation. My only concern is possible performance degradation. I will post here once I got concrete implementation and numbers.
@rsmith regarding to your concerns about struct size change. Yes, the structure size may indeed change. Currently, we plan only to support field fields (non-struct/union type with 1/2/4/8 bytes, and strings). These are most common use cases. Let me explain a little bit more. Here, we are targeting the bpf_probe_read (https://github.com/torvalds/linux/blob/master/include/uapi/linux/bpf.h#L532-L537) int bpf_probe_read(void *dst, u32 size, const void *src) which tries to copy some data from kernel (src pointer) to the stack of the bpf program. Here, if "src" points to a structure, and user intends to copy the whole structure out of kernel, then "size" may need to be different for different kernels if different kernels indeed have different structure size. But first, in all our use cases, users typically do not read the whole structure. Second, if "size" does change, we request users to use multiple versioning (using patchable global variables) with possibly different "size" values for different kernel versions. Note that "size" must be constant for bpf verifier to succeed. (https://github.com/torvalds/linux/blob/master/kernel/trace/bpf_trace.c#L138-L156) Just gave another two examples here about reading kernel memories in iovisor/bcc. Note that bcc will transform certain pointer access "b->c" to "bpf_probe_read(&dest, size, &b->c)" internally. The bcc tool biosnoop.py contain several bpf probe read: https://github.com/iovisor/bcc/blob/master/tools/biosnoop.py#L110 data.len = req->__data_len; https://github.com/iovisor/bcc/blob/master/tools/biosnoop.py#L117 data.len = req->__data_len; https://github.com/iovisor/bcc/blob/master/tools/biosnoop.py#L118 data.sector = req->__sector; https://github.com/iovisor/bcc/blob/master/tools/biosnoop.py#L120 struct gendisk *rq_disk = req->rq_disk; https://github.com/iovisor/bcc/blob/master/tools/biosnoop.py#L121-L122 bpf_probe_read(&data.disk_name, sizeof(data.disk_name), rq_disk->disk_name); They are all primitive types and strings. Another example for bcc tool tcpconnect.py https://github.com/iovisor/bcc/blob/master/tools/tcpconnect.py#L128 u16 dport = skp->__sk_common.skc_dport; https://github.com/iovisor/bcc/blob/master/tools/tcpconnect.py#L136-L137 data4.saddr = skp->__sk_common.skc_rcv_saddr; data4.daddr = skp->__sk_common.skc_daddr; ..... In summary, right now, the to-be-read kernel memory size (through a specific bpf_probe_read() call) won't change between different kernel versions. So we do not need to handle structure allocation size change here. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61809/new/ https://reviews.llvm.org/D61809 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
