Skip to content
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

inix::sys::socket::sockopt add Linux's TCP_DEFER_ACCEPT. #2106

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

devnexen
Copy link
Contributor

No description provided.

@@ -797,6 +797,17 @@ sockopt_impl!(
libc::accept_filter_arg
);
#[cfg(target_os = "linux")]
sockopt_impl!(
/// Defers completion on accept so only when data arrives.
Copy link
Member

Choose a reason for hiding this comment

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

Could you try to reword this? The first sentence isn't grammatically correct. The second sentence's x refers to a variable that isn't anywhere in the prototype. Probably "the number of seconds" would be better. And how would one use this after listen(2), given that listen blocks?

@devnexen
Copy link
Contributor Author

To your last point and why this option can be set before or after listen :
1/ one example is HAProxy listen is done below.
2/ another example is NGINX listen is in this case above.

Note however SO_ACCEPTFILTER is always after listen.

@asomers
Copy link
Member

asomers commented Aug 27, 2023

Sorry; when I said "use this after listen", I was thinking "use this after accept". Yes, it can be used after listen(), but what does that have to do with portability?

@devnexen
Copy link
Contributor Author

I wrote it for two reasons, to highlight this difference between TCP_DEFER_ACCEPT and SO_ACCEPTFILTER but also I think when you want to implement this feature for those platforms it s somehow better to do both after listen like NGINX does.

@asomers
Copy link
Member

asomers commented Aug 27, 2023

Why would it be better do to it after listen than before, if it only has an effect on the behavior of accept?

@devnexen
Copy link
Contributor Author

feature wise it changes nothing indeed, just grouping implementations look better in my opinion.

@asomers
Copy link
Member

asomers commented Aug 28, 2023

feature wise it changes nothing indeed, just grouping implementations look better in my opinion.

Then why did you say "the latter is better for portable workflows"? It isn't portable at all, regardless of the order.

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.

None yet

2 participants