datastorage: do not reset rcpi/rsni on expiry - #276
Merged
PolynomialDivision merged 1 commit intoAug 10, 2026
Merged
Conversation
remove_old_probe_entries() runs periodically and, when a probe entry's
rcpi_timestamp is older than the threshold, marks the entry rcpi_expired
so it can be reaped once the RSSI side has also expired.
Before the reap decision it also overwrites rcpi and rsni with -1.
That mutation is only safe if nothing else is reading those fields at
the same time, but eval_probe_metric() -- called from the scoring path
under a different lock -- does exactly that:
if (!signal_available && probe_entry->rcpi <= 220) {
signal = rcpi_to_rssi(probe_entry->rcpi);
...
}
If the expiry pass writes rcpi = -1 between the range check and the
subsequent read (or a later reader takes the "valid RCPI" branch based
on a still-valid rcpi and then observes -1 after the -1 store becomes
visible), rcpi_to_rssi() is fed a bogus value and the score for that
probe is silently corrupted.
The -1 write is also unnecessary for the reap itself: entries whose
rcpi_timestamp has aged out are already tracked via rcpi_expired, and
once both rcpi_expired and rssi_expired are set the entry is unlinked
and freed a few lines below. Fresh beacon reports overwrite rcpi/rsni
directly, so there is no observer that requires the -1 sentinel.
Drop the writes and rely solely on rcpi_expired to drive the reap.
Signed-off-by: Michael Pfeifroth <micpf@westermo.com>
PolynomialDivision
merged commit Aug 10, 2026
a11dd33
into
berlin-open-wireless-lab:master
1 check passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
remove_old_probe_entries()marks entries asrcpi_expiredwhen theirrcpi_timestampis older than the threshold, and — before doing so — overwritesrcpiandrsniwith-1.The overwrite is unsafe:
eval_probe_metric()readsprobe_entry->rcpion the scoring path (src/storage/datastorage.caround line 215):If the expiry pass writes
rcpi = -1while a scorer is between the range check and thercpi_to_rssi()call — or afterwards, when the scorer picks up the stale-then--1value —rcpi_to_rssi()gets a bogus argument and the score is silently corrupted.The overwrite is also unnecessary for the reap itself: entries whose
rcpi_timestamphas aged out are already tracked viarcpi_expired, and once bothrcpi_expiredandrssi_expiredare set the entry is unlinked and freed a few lines below. Incoming beacon reports overwritercpi/rsniunconditionally, so no observer depends on the-1sentinel.This drops the two writes and relies solely on
rcpi_expiredto drive the reap.