On Mon, Nov 20, 2023 at 04:22:19PM -0800, Andrii Nakryiko wrote:
> include it here. But the reduction in states is due to the following
> piece of C code:
>
> unsigned long ino;
>
> ...
>
> sk = s->sk_socket;
> if (!sk) {
> ino = 0;
> } else {
> inode = SOCK_INODE(sk);
> bpf_probe_read_kernel(&ino, sizeof(ino), &inode->i_ino);
> }
> BPF_SEQ_PRINTF(seq, "%-8u %-8lu\n", s->sk_drops.counter, ino);
> return 0;
>
> You can see that in some situations `ino` is zero-initialized, while in
> others it's unknown value filled out by bpf_probe_read_kernel(). Before
> this change both branches have to be validated twice. Once with
I think you wanted to say that the code _after_ both branches converge
had to be validated twice.
With or without this patch both branches (ino = 0 and probe_read)
will be validated only once. It's the code that after the branch
that gets state pruned after this patch.
> (precise) ino == 0, due to eager STACK_ZERO logic, and then again for
> when ino is just STACK_MISC. But BPF_SEQ_PRINTF() doesn't care about
> precise value of ino, so with the change in this patch verifier is able
> to prune states from one of the branches, reducing number of total
> states (and instructions) required for successful validation.
This part is good.
