Repository navigation
Conversation
Contributor
Author
Revision history
Generated by jj-stack from this pull request's force-push history. |
taspelund
force-pushed
the
jj-stack/bgp-add-composable-wrappers-for-get-set-sockopt-xqyysnkr
branch
from
September 2, 2026 19:20
543da50 to
9d83e29
Compare
taspelund
force-pushed
the
jj-stack/bgp-add-composable-wrappers-for-get-set-sockopt-xqyysnkr
branch
2 times, most recently
from
September 2, 2026 20:40
0bf3422 to
109cc76
Compare
taspelund
requested review from
cfzimmerman,
internet-diglett,
nicolaskagami,
rcgoodfellow and
zeeshanlakhani
September 2, 2026 20:43
taspelund
added
bgp
taspelund
force-pushed
the
jj-stack/bgp-add-composable-wrappers-for-get-set-sockopt-xqyysnkr
branch
2 times, most recently
from
September 4, 2026 21:27
d1ff6f0 to
1abfb30
Compare
taspelund
force-pushed
the
jj-stack/bgp-add-composable-wrappers-for-get-set-sockopt-xqyysnkr
branch
from
September 22, 2026 22:57
1abfb30 to
b431f3f
Compare
taspelund
force-pushed
the
jj-stack/bgp-add-composable-wrappers-for-get-set-sockopt-xqyysnkr
branch
from
September 23, 2026 17:07
b431f3f to
519832a
Compare
| } | ||
|
|
||
| #[cfg(any(target_os = "linux", target_os = "illumos"))] | ||
| fn get_sockopt( |
Collaborator
There was a problem hiding this comment.
Should this be
get_sockopt<T>(...) -> Result<T, Error> { ... }
Contributor
Author
There was a problem hiding this comment.
Initially I had thought so too, but getsockopt(2) always returns int so there's no need for generics. I made set_sockopt generic so that plain Rust refs can be passed in without forcing the caller to cast it to a c_void pointer ahead of time
Collaborator
There was a problem hiding this comment.
I don't think thats right for illumos getsockopt(3socket), perhaps on linux. For example, IPV6_NEXTHOP and IPV6_PATHMTU return structs.
Contributor
Author
There was a problem hiding this comment.
Ah, yeah ok. I was looking at the C return type and overlooked the void pointer passed in for the kernel to write to
Contributor
Author
There was a problem hiding this comment.
I've pushed a change (it's force-pushed, sorry... the diff is here) that addresses this, adds some safety comments, and adds in the round-trip socket option tests that I'd started playing with for a different branch a while back.
Assuming the ipv4-mapped sockopts work in whatever helios version CI runs, the tests should all pass and we'll have actual regression coverage.
taspelund
force-pushed
the
jj-stack/bgp-add-composable-wrappers-for-get-set-sockopt-xqyysnkr
branch
from
September 30, 2026 23:08
519832a to
030c409
Compare
taspelund
force-pushed
the
jj-stack/bgp-add-composable-wrappers-for-get-set-sockopt-xqyysnkr
branch
from
October 13, 2026 21:59
030c409 to
3735cc0
Compare
taspelund
force-pushed
the
jj-stack/bgp-add-composable-wrappers-for-get-set-sockopt-xqyysnkr
branch
from
October 20, 2026 16:53
3735cc0 to
60264fc
Compare
taspelund
force-pushed
the
jj-stack/bgp-add-composable-wrappers-for-get-set-sockopt-xqyysnkr
branch
from
October 20, 2026 19:37
60264fc to
9678231
Compare
taspelund
changed the base branch from
jj-stack/falcon-lab-add-focused-bgp-md5-regression-lvopsxpz
to
jj-stack/bgp-fix-length-truncation-for-enhe-capability-and-capabilities-parameter-qoplztqq
October 20, 2026 19:37
taspelund
force-pushed
the
jj-stack/bgp-add-composable-wrappers-for-get-set-sockopt-xqyysnkr
branch
from
October 20, 2026 19:43
9678231 to
fd1f122
Compare
taspelund
changed the base branch from
jj-stack/bgp-fix-length-truncation-for-enhe-capability-and-capabilities-parameter-qoplztqq
to
jj-stack/bgp-split-message-history-into-all-major-buffers-urwxzymv
October 20, 2026 23:09
taspelund
force-pushed
the
jj-stack/bgp-add-composable-wrappers-for-get-set-sockopt-xqyysnkr
branch
from
October 20, 2026 23:09
fd1f122 to
04b1453
Compare
taspelund
changed the base branch from
jj-stack/bgp-split-message-history-into-all-major-buffers-urwxzymv
to
jj-stack/workspace-remove-unused-dependencies-rsvyqtum
October 21, 2026 06:46
taspelund
force-pushed
the
jj-stack/bgp-add-composable-wrappers-for-get-set-sockopt-xqyysnkr
branch
from
October 21, 2026 06:46
04b1453 to
bbea8da
Compare
taspelund
force-pushed
the
jj-stack/bgp-add-composable-wrappers-for-get-set-sockopt-xqyysnkr
branch
from
October 21, 2026 17:04
bbea8da to
b621dc0
Compare
taspelund
force-pushed
the
jj-stack/bgp-add-composable-wrappers-for-get-set-sockopt-xqyysnkr
branch
from
October 21, 2026 23:22
b621dc0 to
aabfeed
Compare
Base automatically changed from
jj-stack/workspace-remove-unused-dependencies-rsvyqtum
to
main
October 22, 2026 17:46
taspelund
force-pushed
the
jj-stack/bgp-add-composable-wrappers-for-get-set-sockopt-xqyysnkr
branch
from
October 22, 2026 17:46
aabfeed to
db96ca8
Compare
taspelund
force-pushed
the
jj-stack/bgp-add-composable-wrappers-for-get-set-sockopt-xqyysnkr
branch
3 times, most recently
from
November 6, 2026 20:59
cabb066 to
21947ea
Compare
rcgoodfellow
approved these changes
Nov 9, 2026
Instead of jamming unsafe calls to libc::{get,set}sockopt, centralize it
to just one wrapper for each method and use option-specific wrappers.
Also fix incorrect sockopt name for IPv6 TTL:
IP_MINTTL -> IPV6_MINHOPCOUNT
Fixes: #647
Signed-off-by: Trey Aspelund <trey@oxidecomputer.com>
taspelund
force-pushed
the
jj-stack/bgp-add-composable-wrappers-for-get-set-sockopt-xqyysnkr
branch
from
November 9, 2026 20:31
21947ea to
99e826f
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
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.
Instead of jamming unsafe calls to libc::{get,set}sockopt, centralize it to just one wrapper for each method and use option-specific wrappers.
Also fix incorrect sockopt name for IPv6 TTL: IP_MINTTL -> IPV6_MINHOPCOUNT
Fixes: #647
Signed-off-by: Trey Aspelund trey@oxidecomputer.com