On Mon Sep 14, 2026 at 6:02 PM UTC, Vineet Gupta wrote: > > > On 9/10/26 11:22 PM, [email protected] wrote: > >> diff --git a/kernel/bpf/states.c b/kernel/bpf/states.c > >> index 66fb11b6c6a7..d974baad37ee 100644 > >> --- a/kernel/bpf/states.c > >> +++ b/kernel/bpf/states.c > > [ ... ] > > > >> @@ -369,13 +369,6 @@ static bool check_ids(u32 old_id, u32 cur_id, struct > >> bpf_idmap *idmap) > >> * and r7.id=0 (both independent), without temp IDs both would map > >> old_id=X > >> * to cur_id=0 and pass. With temp IDs: r6 maps X->temp1, r7 tries to map > >> * X->temp2, but X is already mapped to temp1, so the check fails > >> correctly. > >> - * > >> - * When old_id has BPF_ADD_CONST set, the compound id (base | flag) and > >> the > >> - * base id (flag stripped) must both map consistently. Example: old has > >> - * r2.id=A, r3.id=A|flag (r3 = r2 + delta), cur has r2.id=B, r3.id=C|flag > >> - * (r3 derived from unrelated r4). Without the base check, idmap gets two > >> - * independent entries A->B and A|flag->C|flag, missing that A->C > >> conflicts > >> - * with A->B. The base ID cross-check catches this. > >> */ > >> static bool check_scalar_ids(u32 old_id, u32 cur_id, struct bpf_idmap > >> *idmap) > >> { > >> @@ -384,15 +377,7 @@ static bool check_scalar_ids(u32 old_id, u32 cur_id, > >> struct bpf_idmap *idmap) > >> > >> cur_id = cur_id ? cur_id : ++idmap->tmp_id_gen; > >> > >> - if (!check_ids(old_id, cur_id, idmap)) > >> - return false; > >> - if (old_id & BPF_ADD_CONST) { > >> - old_id &= ~BPF_ADD_CONST; > >> - cur_id &= ~BPF_ADD_CONST; > >> - if (!check_ids(old_id, cur_id, idmap)) > >> - return false; > >> - } > >> - return true; > >> + return check_ids(old_id, cur_id, idmap); > >> } > > [ ... ] > > > >> diff --git a/tools/testing/selftests/bpf/progs/verifier_linked_scalars.c > >> b/tools/testing/selftests/bpf/progs/verifier_linked_scalars.c > >> index d571fbfc86a3..da6cb961a520 100644 > >> --- a/tools/testing/selftests/bpf/progs/verifier_linked_scalars.c > >> +++ b/tools/testing/selftests/bpf/progs/verifier_linked_scalars.c > > [ ... ] > > > >> @@ -648,10 +648,10 @@ l_exit_%=: > >> \ > >> } > >> > >> /* > >> - * Test that regsafe() verifies base_id consistency for BPF_ADD_CONST > >> + * Test that regsafe() verifies base_id consistency for ADD_CONST_* > >> * linked scalars during state pruning. > >> * > >> - * The false branch (explored first) links R3 to R2 via ADD_CONST. > >> + * The false branch (explored first) links R3 to R2 via ADD_CONST_64. > >> * The true branch (runtime path) links R3 to R4 (unrelated base_id). > >> * At the merge point, pruning must fail because the linkage topology > >> * differs. > > Does this comment still match what the code does? The patch removes the > > "base_id" concept from kernel/bpf/states.c (deleted the paragraph that > > defined it, removed the two-level compound/base id check from > > check_scalar_ids), but the comment here still describes the test as > > verifying "base_id consistency" and refers to "unrelated base_id" on the > > next line. > > > > Looking at the function name and __description below: > > > >> */ > >> __description("add_const base_id must be consistent for pruning") > >> __failure __msg("invalid variable-offset") > >> __naked void add_const_base_id_pruning(void) > > and later at line 695: > > > >> /* Runtime: R2.id=A, R4.id=C: base_ids A vs C inconsistent, > >> * pruning should have failed. */ > > Should these be updated to use current terminology, or does "base_id" > > still have meaning in this context? The test itself still works correctly > > after the patch, so this appears to be a documentation consistency > > question rather than a functional issue. > > Indeed base_id as a term is gone, but just id alone might be confusing > as well. > How about link id as a replacement for base_id
I don't think "link id" fits either. Just use id.
