Non-deterministic output starting in libbpf-cargo 0.8+

[jyn514]

I tried the following:

$ cargo libbpf --version
cargo-libbpf-libbpf 0.7.2
$ cargo libbpf make
$ git add src
$ cargo libbpf make
$ git diff --stat # no output

$ cargo install libbpf-cargo
$ cargo libbpf --version
cargo-libbpf-libbpf 0.8.0
$ cargo libbpf make
$ git add src
$ cargo libbpf make
$ git diff
diff --git a/src/my_file.skel.rs b/src/my_file.skel.rs
index 9bd1e5d..43e3718 100644
--- a/src/my_file.skel.rs
+++ b/src/my_file.skel.rs
@@ -487,7 +487,7 @@ const DATA: &[u8] = &[
     0, 0, 0, 0, 0, 0, 208, 1, 0, 0, 0, 0, 0, 0, 64, 1, 0, 0, 4, 0, 155, 0, 0, 0, 8, 1, 1, 251, 14,
     13, 0, 1, 1, 1, 1, 0, 0, 0, 1, 0, 0, 1, 115, 114, 99, 47, 98, 112, 102, 0, 47, 117, 115, 114,
     47, 105, 110, 99, 108, 117, 100, 101, 47, 97, 115, 109, 45, 103, 101, 110, 101, 114, 105, 99,
-    0, 47, 116, 109, 112, 47, 46, 116, 109, 112, 70, 80, 118, 116, 74, 84, 47, 98, 112, 102, 0, 47,
+    0, 47, 116, 109, 112, 47, 46, 116, 109, 112, 57, 98, 80, 117, 48, 105, 47, 98, 112, 102, 0, 47,
     117, 115, 114, 47, 105, 110, 99, 108, 117, 100, 101, 47, 108, 105, 110, 117, 120, 0, 0, 117,
     100, 112, 95, 114, 111, 117, 116, 101, 46, 98, 112, 102, 46, 99, 0, 1, 0, 0, 105, 110, 116, 45,
     108, 108, 54, 52, 46, 104, 0, 2, 0, 0, 98, 112, 102, 95, 104, 101, 108, 112, 101, 114, 95, 100,

This makes it annoying to commit the generated file to git, since it will always be marked as out of date.

[jyn514]

I've bisected this to 46ca228.

[jyn514]

I think what's going wrong is that clang sometimes chooses the system headers and sometimes chooses the vendored headers. I'm not sure how to make that deterministic - I tried using -iquote instead of -I and changing my downstream .c program to use "bpf/bpf_endian.h", but it didn't help for some reason.

[jyn514]

I think what's going wrong is that clang sometimes chooses the system headers and sometimes chooses the vendored headers.

This can't be it - I removed my system headers altogether and I'm still getting non-determinism. So I guess something in the vendored headers themselves causes non-determinism?

[jyn514]

Here's the diff between the system headers and the vendored headers:

$ diff /usr/include/bpf/bpf_helpers.h /home/jnelson/.local/lib/cargo/registry/src/github.com-1ecc6299db9ec823/libbpf-sys-0.6.0-1/libbpf/src/bpf_helpers.h 
17,24d16
< /* Helper macro to print out debug messages */
< #define bpf_printk(fmt, ...)				\
< ({							\
< 	char ____fmt[] = fmt;				\
< 	bpf_trace_printk(____fmt, sizeof(____fmt),	\
< 			 ##__VA_ARGS__);		\
< })
< 
159a152,260
> 
> #ifndef ___bpf_concat
> #define ___bpf_concat(a, b) a ## b
> #endif
> #ifndef ___bpf_apply
> #define ___bpf_apply(fn, n) ___bpf_concat(fn, n)
> #endif
> #ifndef ___bpf_nth
> #define ___bpf_nth(_, _1, _2, _3, _4, _5, _6, _7, _8, _9, _a, _b, _c, N, ...) N
> #endif
> #ifndef ___bpf_narg
> #define ___bpf_narg(...) \
> 	___bpf_nth(_, ##__VA_ARGS__, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0)
> #endif
> 
> #define ___bpf_fill0(arr, p, x) do {} while (0)
> #define ___bpf_fill1(arr, p, x) arr[p] = x
> #define ___bpf_fill2(arr, p, x, args...) arr[p] = x; ___bpf_fill1(arr, p + 1, args)
> #define ___bpf_fill3(arr, p, x, args...) arr[p] = x; ___bpf_fill2(arr, p + 1, args)
> #define ___bpf_fill4(arr, p, x, args...) arr[p] = x; ___bpf_fill3(arr, p + 1, args)
> #define ___bpf_fill5(arr, p, x, args...) arr[p] = x; ___bpf_fill4(arr, p + 1, args)
> #define ___bpf_fill6(arr, p, x, args...) arr[p] = x; ___bpf_fill5(arr, p + 1, args)
> #define ___bpf_fill7(arr, p, x, args...) arr[p] = x; ___bpf_fill6(arr, p + 1, args)
> #define ___bpf_fill8(arr, p, x, args...) arr[p] = x; ___bpf_fill7(arr, p + 1, args)
> #define ___bpf_fill9(arr, p, x, args...) arr[p] = x; ___bpf_fill8(arr, p + 1, args)
> #define ___bpf_fill10(arr, p, x, args...) arr[p] = x; ___bpf_fill9(arr, p + 1, args)
> #define ___bpf_fill11(arr, p, x, args...) arr[p] = x; ___bpf_fill10(arr, p + 1, args)
> #define ___bpf_fill12(arr, p, x, args...) arr[p] = x; ___bpf_fill11(arr, p + 1, args)
> #define ___bpf_fill(arr, args...) \
> 	___bpf_apply(___bpf_fill, ___bpf_narg(args))(arr, 0, args)
> 
> /*
>  * BPF_SEQ_PRINTF to wrap bpf_seq_printf to-be-printed values
>  * in a structure.
>  */
> #define BPF_SEQ_PRINTF(seq, fmt, args...)			\
> ({								\
> 	static const char ___fmt[] = fmt;			\
> 	unsigned long long ___param[___bpf_narg(args)];		\
> 								\
> 	_Pragma("GCC diagnostic push")				\
> 	_Pragma("GCC diagnostic ignored \"-Wint-conversion\"")	\
> 	___bpf_fill(___param, args);				\
> 	_Pragma("GCC diagnostic pop")				\
> 								\
> 	bpf_seq_printf(seq, ___fmt, sizeof(___fmt),		\
> 		       ___param, sizeof(___param));		\
> })
> 
> /*
>  * BPF_SNPRINTF wraps the bpf_snprintf helper with variadic arguments instead of
>  * an array of u64.
>  */
> #define BPF_SNPRINTF(out, out_size, fmt, args...)		\
> ({								\
> 	static const char ___fmt[] = fmt;			\
> 	unsigned long long ___param[___bpf_narg(args)];		\
> 								\
> 	_Pragma("GCC diagnostic push")				\
> 	_Pragma("GCC diagnostic ignored \"-Wint-conversion\"")	\
> 	___bpf_fill(___param, args);				\
> 	_Pragma("GCC diagnostic pop")				\
> 								\
> 	bpf_snprintf(out, out_size, ___fmt,			\
> 		     ___param, sizeof(___param));		\
> })
> 
> #ifdef BPF_NO_GLOBAL_DATA
> #define BPF_PRINTK_FMT_MOD
> #else
> #define BPF_PRINTK_FMT_MOD static const
> #endif
> 
> #define __bpf_printk(fmt, ...)				\
> ({							\
> 	BPF_PRINTK_FMT_MOD char ____fmt[] = fmt;	\
> 	bpf_trace_printk(____fmt, sizeof(____fmt),	\
> 			 ##__VA_ARGS__);		\
> })
> 
> /*
>  * __bpf_vprintk wraps the bpf_trace_vprintk helper with variadic arguments
>  * instead of an array of u64.
>  */
> #define __bpf_vprintk(fmt, args...)				\
> ({								\
> 	static const char ___fmt[] = fmt;			\
> 	unsigned long long ___param[___bpf_narg(args)];		\
> 								\
> 	_Pragma("GCC diagnostic push")				\
> 	_Pragma("GCC diagnostic ignored \"-Wint-conversion\"")	\
> 	___bpf_fill(___param, args);				\
> 	_Pragma("GCC diagnostic pop")				\
> 								\
> 	bpf_trace_vprintk(___fmt, sizeof(___fmt),		\
> 			  ___param, sizeof(___param));		\
> })
> 
> /* Use __bpf_printk when bpf_printk call has 3 or fewer fmt args
>  * Otherwise use __bpf_vprintk
>  */
> #define ___bpf_pick_printk(...) \
> 	___bpf_nth(_, ##__VA_ARGS__, __bpf_vprintk, __bpf_vprintk, __bpf_vprintk,	\
> 		   __bpf_vprintk, __bpf_vprintk, __bpf_vprintk, __bpf_vprintk,		\
> 		   __bpf_vprintk, __bpf_vprintk, __bpf_printk /*3*/, __bpf_printk /*2*/,\
> 		   __bpf_printk /*1*/, __bpf_printk /*0*/)
> 
> /* Helper macro to print out debug messages */
> #define bpf_printk(fmt, args...) ___bpf_pick_printk(args)(fmt, ##args)
$ diff /usr/include/bpf/bpf_helper_defs.h /home/jnelson/.local/lib/cargo/registry/src/github.com-1ecc6299db9ec823/libbpf-sys-0.6.0-1/libbpf/src/bpf_helper_defs.h 
29a30
> struct unix_sock;
38a40
> struct bpf_timer;
192c194
<  * 	all programs run with preemption disabled, which means that the
---
>  * 	all programs run with migration disabled, which means that the
315c317
<  * 	which is currently set to 32.
---
>  * 	which is currently set to 33.
1280,1281c1282,1287
<  * 	by the caller. Any higher bits in the *flags* argument must be
<  * 	unset.
---
>  * 	by the caller. The higher bits of *flags* can be set to
>  * 	BPF_F_BROADCAST or BPF_F_EXCLUDE_INGRESS as defined below.
>  *
>  * 	With BPF_F_BROADCAST the packet will be broadcasted to all the
>  * 	interfaces in the map, with BPF_F_EXCLUDE_INGRESS the ingress
>  * 	interface will be excluded when do broadcasting.
2093c2099
<  * 	**BPF_MAP_TYPE_REUSEPORT_ARRAY** *map*.
---
>  * 	**BPF_MAP_TYPE_REUSEPORT_SOCKARRAY** *map*.
3010c3016
<  * 	The *data_len* is the size of *data* in bytes.
---
>  * 	The *data_len* is the size of *data* in bytes - must be a multiple of 8.
3871c3877,3878
<  * 	array. The *data_len* is the size of *data* in bytes.
---
>  * 	array. The *data_len* is the size of *data* in bytes - must be
>  * 	a multiple of 8.
3890a3898,4137
> 
> /*
>  * bpf_sys_bpf
>  *
>  * 	Execute bpf syscall with given arguments.
>  *
>  * Returns
>  * 	A syscall result.
>  */
> static long (*bpf_sys_bpf)(__u32 cmd, void *attr, __u32 attr_size) = (void *) 166;
> 
> /*
>  * bpf_btf_find_by_name_kind
>  *
>  * 	Find BTF type with given name and kind in vmlinux BTF or in module's BTFs.
>  *
>  * Returns
>  * 	Returns btf_id and btf_obj_fd in lower and upper 32 bits.
>  */
> static long (*bpf_btf_find_by_name_kind)(char *name, int name_sz, __u32 kind, int flags) = (void *) 167;
> 
> /*
>  * bpf_sys_close
>  *
>  * 	Execute close syscall for given FD.
>  *
>  * Returns
>  * 	A syscall result.
>  */
> static long (*bpf_sys_close)(__u32 fd) = (void *) 168;
> 
> /*
>  * bpf_timer_init
>  *
>  * 	Initialize the timer.
>  * 	First 4 bits of *flags* specify clockid.
>  * 	Only CLOCK_MONOTONIC, CLOCK_REALTIME, CLOCK_BOOTTIME are allowed.
>  * 	All other bits of *flags* are reserved.
>  * 	The verifier will reject the program if *timer* is not from
>  * 	the same *map*.
>  *
>  * Returns
>  * 	0 on success.
>  * 	**-EBUSY** if *timer* is already initialized.
>  * 	**-EINVAL** if invalid *flags* are passed.
>  * 	**-EPERM** if *timer* is in a map that doesn't have any user references.
>  * 	The user space should either hold a file descriptor to a map with timers
>  * 	or pin such map in bpffs. When map is unpinned or file descriptor is
>  * 	closed all timers in the map will be cancelled and freed.
>  */
> static long (*bpf_timer_init)(struct bpf_timer *timer, void *map, __u64 flags) = (void *) 169;
> 
> /*
>  * bpf_timer_set_callback
>  *
>  * 	Configure the timer to call *callback_fn* static function.
>  *
>  * Returns
>  * 	0 on success.
>  * 	**-EINVAL** if *timer* was not initialized with bpf_timer_init() earlier.
>  * 	**-EPERM** if *timer* is in a map that doesn't have any user references.
>  * 	The user space should either hold a file descriptor to a map with timers
>  * 	or pin such map in bpffs. When map is unpinned or file descriptor is
>  * 	closed all timers in the map will be cancelled and freed.
>  */
> static long (*bpf_timer_set_callback)(struct bpf_timer *timer, void *callback_fn) = (void *) 170;
> 
> /*
>  * bpf_timer_start
>  *
>  * 	Set timer expiration N nanoseconds from the current time. The
>  * 	configured callback will be invoked in soft irq context on some cpu
>  * 	and will not repeat unless another bpf_timer_start() is made.
>  * 	In such case the next invocation can migrate to a different cpu.
>  * 	Since struct bpf_timer is a field inside map element the map
>  * 	owns the timer. The bpf_timer_set_callback() will increment refcnt
>  * 	of BPF program to make sure that callback_fn code stays valid.
>  * 	When user space reference to a map reaches zero all timers
>  * 	in a map are cancelled and corresponding program's refcnts are
>  * 	decremented. This is done to make sure that Ctrl-C of a user
>  * 	process doesn't leave any timers running. If map is pinned in
>  * 	bpffs the callback_fn can re-arm itself indefinitely.
>  * 	bpf_map_update/delete_elem() helpers and user space sys_bpf commands
>  * 	cancel and free the timer in the given map element.
>  * 	The map can contain timers that invoke callback_fn-s from different
>  * 	programs. The same callback_fn can serve different timers from
>  * 	different maps if key/value layout matches across maps.
>  * 	Every bpf_timer_set_callback() can have different callback_fn.
>  *
>  *
>  * Returns
>  * 	0 on success.
>  * 	**-EINVAL** if *timer* was not initialized with bpf_timer_init() earlier
>  * 	or invalid *flags* are passed.
>  */
> static long (*bpf_timer_start)(struct bpf_timer *timer, __u64 nsecs, __u64 flags) = (void *) 171;
> 
> /*
>  * bpf_timer_cancel
>  *
>  * 	Cancel the timer and wait for callback_fn to finish if it was running.
>  *
>  * Returns
>  * 	0 if the timer was not active.
>  * 	1 if the timer was active.
>  * 	**-EINVAL** if *timer* was not initialized with bpf_timer_init() earlier.
>  * 	**-EDEADLK** if callback_fn tried to call bpf_timer_cancel() on its
>  * 	own timer which would have led to a deadlock otherwise.
>  */
> static long (*bpf_timer_cancel)(struct bpf_timer *timer) = (void *) 172;
> 
> /*
>  * bpf_get_func_ip
>  *
>  * 	Get address of the traced function (for tracing and kprobe programs).
>  *
>  * Returns
>  * 	Address of the traced function.
>  */
> static __u64 (*bpf_get_func_ip)(void *ctx) = (void *) 173;
> 
> /*
>  * bpf_get_attach_cookie
>  *
>  * 	Get bpf_cookie value provided (optionally) during the program
>  * 	attachment. It might be different for each individual
>  * 	attachment, even if BPF program itself is the same.
>  * 	Expects BPF program context *ctx* as a first argument.
>  *
>  * 	Supported for the following program types:
>  * 		- kprobe/uprobe;
>  * 		- tracepoint;
>  * 		- perf_event.
>  *
>  * Returns
>  * 	Value specified by user at BPF link creation/attachment time
>  * 	or 0, if it was not specified.
>  */
> static __u64 (*bpf_get_attach_cookie)(void *ctx) = (void *) 174;
> 
> /*
>  * bpf_task_pt_regs
>  *
>  * 	Get the struct pt_regs associated with **task**.
>  *
>  * Returns
>  * 	A pointer to struct pt_regs.
>  */
> static long (*bpf_task_pt_regs)(struct task_struct *task) = (void *) 175;
> 
> /*
>  * bpf_get_branch_snapshot
>  *
>  * 	Get branch trace from hardware engines like Intel LBR. The
>  * 	hardware engine is stopped shortly after the helper is
>  * 	called. Therefore, the user need to filter branch entries
>  * 	based on the actual use case. To capture branch trace
>  * 	before the trigger point of the BPF program, the helper
>  * 	should be called at the beginning of the BPF program.
>  *
>  * 	The data is stored as struct perf_branch_entry into output
>  * 	buffer *entries*. *size* is the size of *entries* in bytes.
>  * 	*flags* is reserved for now and must be zero.
>  *
>  *
>  * Returns
>  * 	On success, number of bytes written to *buf*. On error, a
>  * 	negative value.
>  *
>  * 	**-EINVAL** if *flags* is not zero.
>  *
>  * 	**-ENOENT** if architecture does not support branch records.
>  */
> static long (*bpf_get_branch_snapshot)(void *entries, __u32 size, __u64 flags) = (void *) 176;
> 
> /*
>  * bpf_trace_vprintk
>  *
>  * 	Behaves like **bpf_trace_printk**\ () helper, but takes an array of u64
>  * 	to format and can handle more format args as a result.
>  *
>  * 	Arguments are to be used as in **bpf_seq_printf**\ () helper.
>  *
>  * Returns
>  * 	The number of bytes written to the buffer, or a negative error
>  * 	in case of failure.
>  */
> static long (*bpf_trace_vprintk)(const char *fmt, __u32 fmt_size, const void *data, __u32 data_len) = (void *) 177;
> 
> /*
>  * bpf_skc_to_unix_sock
>  *
>  * 	Dynamically cast a *sk* pointer to a *unix_sock* pointer.
>  *
>  * Returns
>  * 	*sk* if casting is valid, or **NULL** otherwise.
>  */
> static struct unix_sock *(*bpf_skc_to_unix_sock)(void *sk) = (void *) 178;
> 
> /*
>  * bpf_kallsyms_lookup_name
>  *
>  * 	Get the address of a kernel symbol, returned in *res*. *res* is
>  * 	set to 0 if the symbol is not found.
>  *
>  * Returns
>  * 	On success, zero. On error, a negative value.
>  *
>  * 	**-EINVAL** if *flags* is not zero.
>  *
>  * 	**-EINVAL** if string *name* is not the same size as *name_sz*.
>  *
>  * 	**-ENOENT** if symbol is not found.
>  *
>  * 	**-EPERM** if caller does not have permission to obtain kernel address.
>  */
> static long (*bpf_kallsyms_lookup_name)(const char *name, int name_sz, int flags, __u64 *res) = (void *) 179;
> 
> /*
>  * bpf_find_vma
>  *
>  * 	Find vma of *task* that contains *addr*, call *callback_fn*
>  * 	function with *task*, *vma*, and *callback_ctx*.
>  * 	The *callback_fn* should be a static function and
>  * 	the *callback_ctx* should be a pointer to the stack.
>  * 	The *flags* is used to control certain aspects of the helper.
>  * 	Currently, the *flags* must be 0.
>  *
>  * 	The expected callback signature is
>  *
>  * 	long (\*callback_fn)(struct task_struct \*task, struct vm_area_struct \*vma, void \*callback_ctx);
>  *
>  *
>  * Returns
>  * 	0 on success.
>  * 	**-ENOENT** if *task->mm* is NULL, or no vma contains *addr*.
>  * 	**-EBUSY** if failed to try lock mmap_lock.
>  * 	**-EINVAL** for invalid **flags**.
>  */
> static long (*bpf_find_vma)(struct task_struct *task, __u64 addr, void *callback_fn, void *callback_ctx, __u64 flags) = (void *) 180;

(there might be other differences but my program doesn't include any other headers)

[mimullin-bbry]

@jyn514 The headers which come from libbpf-sys (via libbpf) may be updated between versions. Could this be the cause if your non-determinism?

Note that beyond the DATA slice, libbpf-cargo has been undergoing improvements, and the actual Rust Code generated may change between versions (eg 3b072c8).

Thus as libbpf-cargo improves, the skeletons it outputs is going to change.

[jyn514]

@mimullin-bbry no, I specifically mean changes within the same version, i.e. running the same command twice gives different outputs.

[jyn514]

Here's an MCVE:

#include <linux/bpf.h>
#include <bpf/bpf_helpers.h>

struct {
	__uint(type, BPF_MAP_TYPE_REUSEPORT_SOCKARRAY);
} sock_map SEC(".maps");

SEC("sk_reuseport/reuse_pass")
long prog_select_sk(struct sk_reuseport_md *reuse_md)
{
	unsigned int index = 0;
	bpf_sk_select_reuseport(reuse_md, &sock_map, &index, 0);
	return SK_PASS;
}

If you comment out bpf_sk_select_reuseport it's deterministic; if it's uncommented, it's nondeterministic.

[jyn514]

I compared the .o files clang generates between two runs and they're different, but I'm not sure how, the symbol tables match.

$ readelf --all udp_route.bpf.o  | diff - <(readelf --all target/bpf/udp_route.bpf.o)
$ cmp udp_route.bpf.o target/bpf/udp_route.bpf.o
udp_route.bpf.o target/bpf/udp_route.bpf.o differ: byte 2984, line 6

[jyn514]

ah, but the section contents are different:

$ readelf -x .debug_line udp_route.bpf.o | diff - <(readelf -x .debug_line target/bpf/udp_route.bpf.o)
9,10c9,10
<   0x00000050 6e657269 63002f74 6d702f2e 746d7073 neric./tmp/.tmps
<   0x00000060 52564f4b 312f6270 66000077 6f726b2f RVOK1/bpf..work/
---
>   0x00000050 6e657269 63002f74 6d702f2e 746d704b neric./tmp/.tmpK
>   0x00000060 766d3566 762f6270 66000077 6f726b2f vm5fv/bpf..work/

[jyn514]

/tmp/.tmpsRVOK1 looks like the name of the temporary directory libbpf-cargo uses for storing the section headers. Is there a way to tell clang not to include that in the binary? Or, barring that, to using a consistent directory name?

[insearchoflosttime]

As a summary of the conversation in #169, the approach we're going to go with is statically linking BPF object files, which will strip out DWARF info as a side-effect