Support conditional pending-dataset replaces with entity tags - #274
Draft
LorbusChris wants to merge 2 commits into
Draft
Support conditional pending-dataset replaces with entity tags#274LorbusChris wants to merge 2 commits into
LorbusChris wants to merge 2 commits into
Conversation
LorbusChris
force-pushed
the
pending-dataset-if-match
branch
2 times, most recently
from
August 28, 2026 23:47
855f95f to
4287a2b
Compare
Add set_pending_dataset_tlvs(), the pending-endpoint twin of set_active_dataset_tlvs(), so a controller can hand a border router a new operational dataset as a raw TLV pending set. The TLV parser gains the write-side counterparts callers need to build such a dataset: Timestamp.from_values(), with range checks for the 48-bit seconds and 15-bit ticks fields, and DelayTimer.from_milliseconds(). PENDINGTIMESTAMP and DELAYTIMER now decode to typed items, matching ACTIVETIMESTAMP. Also raise TLVError instead of leaking struct.error on a malformed timestamp or delay timer TLV, and log unknown TLVs by type and length only -- the value may hold network credentials. Like the channel change, the write is refused while a pending dataset is already in place. A not-newer one would be silently ignored by the mesh, and superseding is never safe to do implicitly: the replacement races the delay timer on every device that already holds the old dataset, so a late replacement can split the mesh, and it would also silently undo whatever the in-flight dataset was doing. A caller that finds the write refused should surface that, not retry. The check runs locally first, and again on the border router: unless allow_replace is set the request carries "If-None-Match: *", which a border router with openthread/ot-br-posix#3552 evaluates atomically with the write, closing the race the local check leaves open. Older border routers ignore the header and keep relying on the local check. Both refusals raise PendingDatasetConflictError rather than a plain OTBRError, so a caller can tell them apart from a transport or protocol failure without matching on the message: nothing was written, and the state that made the write wrong is there to be read back. Assisted-By: Claude Fable 5
set_pending_dataset_tlvs() refuses whenever a pending dataset is in place, and deliberately offers no blanket override: superseding races the delay timer on every device holding the old dataset and silently undoes whatever that dataset was doing. What a caller may reasonably want is narrower: "replace exactly the dataset I read", as an explicit, informed action. Add get_pending_dataset_tlvs_with_etag(), returning the dataset together with the entity tag a border router hands out for it, and an if_match keyword on set_pending_dataset_tlvs() that passes the tag back: the router then only replaces the dataset the tag stands for, checked atomically with the write (openthread/ot-br-posix#3553). An older border router hands out no tag and would ignore the header, making the replace there unconditional, which is why callers must only expose this as an explicit choice, never as a default. A conditional replace that the router refuses raises the same PendingDatasetConflictError the guarded write already raises, with a message naming the other reason: the dataset changed since it was read. Assisted-By: Claude Fable 5
LorbusChris
force-pushed
the
pending-dataset-if-match
branch
from
September 1, 2026 14:15
4287a2b to
aebad9b
Compare
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
1 participant
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.
Draft until #269 and openthread/ot-br-posix#3553 are merged -- this branch is stacked on #269 (its commit shows here until it merges), and the border-router side of the API is openthread/ot-br-posix#3553.
set_pending_dataset_tlvs()refuses outright while a pending dataset is in flight (#269), because replacing one implicitly is never safe. But a caller that read the in-flight dataset, showed it to the user, and stamped a successor above it has a legitimate replace -- and still cannot say "replace exactly the one I read": another controller can slip a new pending dataset in between the read and the write.This adds
get_pending_dataset_tlvs_with_etag(), returning the dataset together with the entity tag a border router hands out for it, and anif_matchkeyword onset_pending_dataset_tlvs()that passes the tag back: the router then only replaces the dataset the tag stands for, checked atomically with the write (openthread/ot-br-posix#3553), and a 412 raisesPendingDatasetConflictErrortelling the caller the dataset changed since it was read. This is the only way the library replaces a pending dataset. An older border router hands out no tag (the getter returnsNonefor it) and would ignore theIf-Matchheader, silently making the write unconditional -- which is why a caller must only offerif_matchas an explicit, informed choice, and refuse it when the getter returned no tag.Needed for the replace-pending follow-up to home-assistant/core#178291 (the
otbr.migrate_networkaction gains an explicitreplace_pendingoption that passes the tag back, so a migration only supersedes the in-flight dataset the user actually saw).AI use: Written with AI assistance -- the commit carries an
Assisted-Bytrailer. I've reviewed and understand all of it, and will be answering questions myself.