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.

Sign up for GitHub

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

Jump to bottom

Add functions that accept socket options #6

Closed
wants to merge 1 commit into from
Closed

Add functions that accept socket options #6

wants to merge 1 commit into from

Conversation

FinleyMcIlwaine
Copy link
Contributor

@FinleyMcIlwaine FinleyMcIlwaine commented Jun 25, 2024 •
edited
Loading

We have run into a situation where it would be nice to be able to use openServerSocket/runTCPClient and set some socket options on the sockets before the bind/connect occurs. This PR adds generalizations of the existing functions which set given options on the sockets, and reimplements the originals in terms of the generalizations (e.g. openServerSocket <=> openServerSocketWithOptions []).

Bumps version to 0.3.2. Thanks!

@FinleyMcIlwaine FinleyMcIlwaine force-pushed the finley/add-open-sock-with-opts branch 2 times, most recently from 04b588c to 333203f Compare June 25, 2024 23:32
@FinleyMcIlwaine FinleyMcIlwaine changed the title Add openServerSocketWithOptions and openClientSocketWithOptions Add functions that accept socket options Jun 25, 2024
@FinleyMcIlwaine FinleyMcIlwaine force-pushed the finley/add-open-sock-with-opts branch 3 times, most recently from 2948d05 to 755ea82 Compare June 25, 2024 23:43
@FinleyMcIlwaine
Copy link
Contributor Author

I guess an alternative approach would be to generalize runTCPClient in the same way @edsko generalized runTCPServer and create a runTCPClientWithSocket, leaving openClientSocketWithOptions to fulfill the purpose of this PR. Let me know if you would prefer that!

@FinleyMcIlwaine FinleyMcIlwaine force-pushed the finley/add-open-sock-with-opts branch from 755ea82 to e02d9de Compare June 28, 2024 17:25
@FinleyMcIlwaine
Copy link
Contributor Author

I decided it was cleaner to introduce runTCPClientWithSocket to be more symmetrical with runTCPServerWithSocket, so I've updated this.

@FinleyMcIlwaine
Add functions that accept socket options
4ffdc61 
Adds:

- `openServerSocketWithOptions`
- `openClientSocketWithOptions`
- `runTCPServerWithSocketOptions` x2, one in `Timeout` one in `TCP`
- `runTCPClientWithSocket`
- `runTCPClientWithSocketOptions`

Bump version to 0.3.2
@FinleyMcIlwaine FinleyMcIlwaine force-pushed the finley/add-open-sock-with-opts branch from e02d9de to 4ffdc61 Compare July 1, 2024 22:51
@edsko
Copy link
Contributor

edsko commented Jul 2, 2024

@kazu-yamamoto Sorry for the churn with the various pushes to this PR, but we think this is ready now :) Context: it turns out to be really important for us to be able to set TCP_NODELAY.

By the way, it might be useful to enable TCP_QUICKACK by default in network-run; see https://news.ycombinator.com/item?id=10608356 by John Nagle (he of TCP fame) himself.

@edsko edsko mentioned this pull request Jul 2, 2024
Closed
@kazu-yamamoto kazu-yamamoto self-requested a review July 3, 2024 23:58
@kazu-yamamoto kazu-yamamoto mentioned this pull request Jul 4, 2024
Closed
@edsko
Copy link
Contributor

edsko commented Jul 4, 2024

I'm actually not sure if all functions that should be exported have been exported, I think they have not. Let me check and maybe fix, will report back soon.

This was referenced Jul 4, 2024
Merged
Closed
@edsko
Copy link
Contributor

edsko commented Jul 4, 2024

Superseded by #8.

@edsko edsko mentioned this pull request Jul 9, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kazu-yamamoto kazu-yamamoto Awaiting requested review from kazu-yamamoto

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@FinleyMcIlwaine @edsko