Repository navigation
device: return an error instead of panicking when updating an existing peer - #499
Open
miguelcorderopamphile wants to merge 1 commit into
Open
miguelcorderopamphile wants to merge 1 commit into
miguelcorderopamphile wants to merge 1 commit into
Conversation
…g peer
A routine UAPI set request that includes a peer which already exists (for
example re-applying a configuration or changing an endpoint) hits a panic in
update_peer ("Modifying existing peers is not yet supported"). The panic
kills the thread that services the UAPI listener and the anonymous UDP
socket, so the daemon stops processing all network traffic until it is
restarted (verified live: the UDP path stops answering after the panic).
Return a bool from update_peer (false when the peer already exists) and have
the UAPI return EOPNOTSUPP to the caller, keeping the device alive and
matching the guidance in the original panic message ("Remove and add again
instead").
Verified live with the daemon release build: before, the second set request
panicked and the daemon stopped processing packets; after, the request
returns errno=95, get=1 keeps working, and the UDP path keeps processing.
Signed-off-by: Miguel Cordero Pamphile <miguelcorderopamphile@gmail.com>
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
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.
Fixes #498.
What
update_peerpanicked when the UAPIset=1request included a peer that already exists. The panic runs inside the device event handler (UAPI listener + anonymous UDP socket), so the daemon kept running but stopped processing network traffic until restarted.This PR returns a
boolfromupdate_peer(falsewhen the peer already exists) and the UAPI returnsEOPNOTSUPPto the caller, keeping the device alive and matching the guidance in the original panic message ("Remove and add again instead").Verification
cargo test -p boringtun --lib: 10 passed.set=1for the same peer -> no response,get=1times out, UDP path stops answering (200 -> 0 cookie replies under the same flood).set=1->errno=95(EOPNOTSUPP),get=1returns the peer list, UDP path keeps answering (200 cookie replies).