-
Notifications
You must be signed in to change notification settings - Fork 250
feat: Add UDP GRO option #536
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
base: master
Are you sure you want to change the base?
Conversation
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.
Since this Linux only, can you put in the sys/unix.rs file:
Line 763 in 3a93893
impl SockAddr { |
Could also you make sure it's order by option name.
Can you add the appropriate attributes, e.g. see
Lines 791 to 795 in 3a93893
#[cfg(all(feature = "all", any(target_os = "android", target_os = "linux")))] | |
#[cfg_attr( | |
docsrs, | |
doc(cfg(all(feature = "all", any(target_os = "android", target_os = "linux")))) | |
)] |
Finally can you add a test? A simple set and get test is sufficient, see
Line 1357 in 3a93893
test!(reuse_port, set_reuse_port(true)); |
@Thomasdezeeuw Thank you, I've implemented your feedback, the only thing I wasn't 100% sure on was "Could also you make sure it's order by option name." let me know if I need to make more changes |
Thanks @XAMPPRocky |
@XAMPPRocky sorry for leaving this so long. I'm not sure why I didn't merge after I approved it. I tried rebase the branch, but GitHub won't allow me (I think it's because it's the master branch, but not sure). I've opened #577 which is the same patch, but rebased. |
It seems like the CI is failing, even with a rebase. @XAMPPRocky do you have time to look at it? I totally understand if you've lost interest after all this time. |
Hey, yeah my initial need for the option has gone as the project I was using it in now uses XDP sockets which bypass this setting. I'lll try to fixup the PR next week though. |
Allows setting UDP_GRO on the socket.
https://www.man7.org/linux/man-pages/man7/udp.7.html