Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

bpf: use common instruction history across all states #7990

Open
wants to merge 1 commit into
base: bpf-next_base
Choose a base branch
from
Open

bpf: use common instruction history across all states #7990

wants to merge 1 commit into from
+62 −62

Conversation

kernel-patches-daemon-bpf[bot]
Copy link

Pull request for series with
subject: bpf: use common instruction history across all states
version: 1
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=904858

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: e626a13
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=904858
version: 1

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: e5e4799
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=904858
version: 1

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 4d99e50
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=904858
version: 1

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 77017b9
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=904858
version: 1

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: f2daa5a
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=904858
version: 1

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 9a78313
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=904858
version: 1

@anakryiko
bpf: use common instruction history across all states
8e1368d 
Instead of allocating and copying instruction history each time we
enqueue child verifier state, switch to a model where we use one common
dynamically sized array of instruction history entries across all states.

The key observation for proving this is correct is that instruction
history is only relevant while state is active, which means it either is
a current state (and thus we are actively modifying instruction history
and no other state can interfere with us) or we are checkpointed state
with some children still active (either enqueued or being current).

In the latter case our portion of instruction history is finalized and
won't change or grow, so as long as we keep it immutable until the state
is finalized, we are good.

Now, when state is finalized and is put into state hash for potentially
future pruning lookups, instruction history is not used anymore. This is
because instruction history is only used by precision marking logic, and
we never modify precision markings for finalized states.

So, instead of each state having its own small instruction history, we
keep a global dynamically-sized instruction history, where each state in
current DFS path from root to active state remembers its portion of
instruction history. Current state can append to this history, but
cannot modify any of its parent histories.

Async callback state enqueing, while logically detached from parent
state, still is part of verification backtracking tree, so has to follow
the same schema as normal state checkpoints.

Because the insn_hist array can be grown through realloc, states don't
keep pointers, they instead maintain two indices, [start, end), into
global instruction history array. End is exclusive index, so
`start == end` means there is no relevant instruction history.

This eliminates a lot of allocations and minimizes overall memory usage.

For instance, running a worst-case test from [0] (but without the
heuristics-based fix [1]), it took 12.5 minutes until we get -ENOMEM.
With the changes in this patch the whole test succeeds in 10 minutes
(very slow, so heuristics from [1] is important, of course).

To further validate correctness, veristat-based comparison was performed for
Meta production BPF objects and BPF selftests objects. In both cases there
were no differences *at all* in terms of verdict or instruction and state
counts, providing a good confidence in the change.

Having this low-memory-overhead solution of keeping dynamic
per-instruction history cheaply opens up some new possibilities, like
keeping extra information for literally every single validated
instruction. This will be used for simplifying precision backpropagation
logic in follow up patches.

  [0] https://lore.kernel.org/bpf/[email protected]/
  [1] https://lore.kernel.org/bpf/[email protected]/

Acked-by: Eduard Zingerman <[email protected]>
Signed-off-by: Andrii Nakryiko <[email protected]>
@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 1850ce1
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=904858
version: 1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
bpf-next new V1-ci-pass V1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@anakryiko