Skip to content

Conversation

@nan-mu
Copy link
Contributor

@nan-mu nan-mu commented Jul 22, 2025

For #606

Copy link
Collaborator

@Thomasdezeeuw Thomasdezeeuw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have a test for this as well? Since it requires CAP_NET_ADMIN, we probably need to ignore it and try it locally, see for example set_mark.

src/socket.rs Outdated
/// - Requires `CAP_NET_ADMIN` capability to increase the value
/// - Default value is controlled by `/proc/sys/net/core/busy_read`
/// - Available since Linux 3.11
#[cfg(target_os = "linux")]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be behind the all feature, see other functions that have limited cross-OS availability.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not familiar with all the targets, but I'll try?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I manually searched and used the API and found that none of the targets listed in the README support busy polling. Here is the link to the documentation I found manually. So, I guess there is nothing else.

Apple
Windows
android
FreeBSD
Fuchsia
illumos
NetBSD
Redox
Solaris
OpenHarmony

I found a great list btw: https://docs.rs/rustix/latest/rustix/net/sockopt/index.html

src/socket.rs Outdated
/// - Default value is controlled by `/proc/sys/net/core/busy_read`
/// - Available since Linux 3.11
#[cfg(target_os = "linux")]
pub fn set_busy_poll(&self, busy_poll: u16) -> io::Result<()> {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why u16?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, this is my question. When I saw that most C code used int, I was confused why it was not unsigned int. I thought this type problem should be corrected, so I filled in u16. Now I will change it to u32 next time.

src/socket.rs Outdated
#[cfg(target_os = "linux")]
pub fn set_busy_poll(&self, busy_poll: u16) -> io::Result<()> {
sys::set_busy_poll(self.as_raw(), busy_poll)
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have a getting for this as well?

Copy link
Contributor Author

@nan-mu nan-mu Jul 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, if you could specify I'd appreciate it, basically I copied the code from set_nonblocking.

Oh, set_mark is that one, that makes things easier.

src/socket.rs Outdated
Comment on lines 390 to 393
///
/// - Requires `CAP_NET_ADMIN` capability to increase the value
/// - Default value is controlled by `/proc/sys/net/core/busy_read`
/// - Available since Linux 3.11
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we replace these notes with only calling out SO_BUSY_POLL, similar to the other functions. I don't want to copy all notes from the man page.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, if it's your wish.

nan-mu added 3 commits July 23, 2025 15:32
  * Change the documentation comment of `set_busy_poll`
  * Move setter into unix under `all` feature
  * Add a getter
@Thomasdezeeuw Thomasdezeeuw merged commit 15ade51 into rust-lang:master Jul 23, 2025
47 checks passed
@Thomasdezeeuw
Copy link
Collaborator

Thanks @nan-mu

@Darksonn Darksonn mentioned this pull request Oct 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants