-
Notifications
You must be signed in to change notification settings - Fork 14k
socket set_mark addition.
#96334
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
socket set_mark addition.
#96334
Conversation
|
Hey! It looks like you've submitted a new PR for the library teams! If this PR contains changes to any Examples of
|
|
r? @thomcc (rust-highfive has picked a reviewer for you, use r? to override) |
|
r? rust-lang/libs-api @rustbot label +T-libs-api -T-libs |
dtolnay
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems okay to me. Needs documentation and a tracking issue; I assume that's why the PR is still marked Draft.
|
Not only that but waited to see if the idea itself would get traction. ok will do these. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
d9b1a9e to
57024fb
Compare
|
ping from triage: FYI: when a PR is ready for review, send a message containing |
|
@rustbot ready |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not clear to me what useful information "and is only a setter" is meant to convey here. Can this sentence be just the following?—
Set the id of the socket for network filtering purposes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's odd to see expect here in a function that returns io::Result anyway. How about returning the error?
| /// fn main() -> std::io::Result<()> { | |
| /// let sock = UnixDatagram::unbound()?; | |
| /// sock.set_mark(32 as u32).expect("set_mark function failed"); | |
| /// fn main() -> std::io::Result<()> { | |
| /// let sock = UnixDatagram::unbound()?; | |
| /// sock.set_mark(32 as u32)?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for opening the tracking issue. You need to put the issue number here inside the issue attribute. Check out the other unstable attributes in this file.
Same applies to the other new unstable attribute below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
32 as u32 is redundant. This can just be sock.set_mark(32) and the integer will be of type u32 automatically.
library/std/src/sys/unix/net.rs
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The signature is identical across all of these platforms, and the implementation is nearly identical. How about deduplicating it this way?—
pub fn set_mark(&self, mark: u32) -> io::Result<()> {
#[cfg(target_os = "linux")]
let option = libc::SO_MARK;
#[cfg(target_os = "linux")]
let option = libc::SO_USER_COOKIE;
#[cfg(target_os = "linux")]
let option = libc::SO_RTABLE;
setsockopt(self, libc::SOL_SOCKET, option, mark as libc::c_int)
}or maybe this:
pub fn set_mark(&self, mark: u32) -> io::Result<()> {
setsockopt(
self,
libc::SOL_SOCKET,
#[cfg(target_os = "linux")]
libc::SO_MARK,
#[cfg(target_os = "freebsd")]
libc::SO_USER_COOKIE,
#[cfg(target_os = "openbsd")]
libc::SO_RTABLE,
mark as libc::c_int,
)
}
Triage: FYI: when a PR is ready for review, send a message containing @rustbot ready |
dtolnay
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
|
@bors r+ |
socket `set_mark` addition. to be able to set a marker/id on the socket for network filtering (iptables/ipfw here) purpose.
socket `set_mark` addition. to be able to set a marker/id on the socket for network filtering (iptables/ipfw here) purpose.
socket `set_mark` addition. to be able to set a marker/id on the socket for network filtering (iptables/ipfw here) purpose.
socket `set_mark` addition. to be able to set a marker/id on the socket for network filtering (iptables/ipfw here) purpose.
socket `set_mark` addition. to be able to set a marker/id on the socket for network filtering (iptables/ipfw here) purpose.
…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#94890 (Support parsing IP addresses from a byte string) - rust-lang#96334 (socket `set_mark` addition.) - rust-lang#99027 (Replace `Body::basic_blocks()` with field access) - rust-lang#100437 (Improve const mismatch `FulfillmentError`) - rust-lang#100843 (Migrate part of rustc_infer to session diagnostic) - rust-lang#100897 (extra sanity check against consts pointing to mutable memory) - rust-lang#100959 (translations: rename warn_ to warning) - rust-lang#101111 (Use the declaration's SourceInfo for FnEntry retags, not the outermost) - rust-lang#101116 ([rustdoc] Remove Attrs type alias) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
to be able to set a marker/id on the socket for network filtering
(iptables/ipfw here) purpose.