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

tcp: connect: Add source_address and bind_device options #154

Merged
merged 2 commits into from
May 5, 2023

Conversation

kgraefe
Copy link
Contributor

@kgraefe kgraefe commented May 2, 2023

This change allows binding an outgoing TCP connection to a specific network interface. My use-case for that is that I need to support situations where two interfaces are configured for link-local addressing. In that case the system has two different routes into the link-local subnet.

On Windows the interface is determined by the IP address passed to bind(). On Linux this is not sufficient as the routing table takes precedence. Therefore we must explicitly bind the socket to the interface.

Depends on #153

@kgraefe kgraefe marked this pull request as draft May 2, 2023 08:51
@kgraefe kgraefe force-pushed the tcp-connect-add-source-address branch from c9e4b29 to 7fc14e0 Compare May 2, 2023 08:53
@kgraefe kgraefe changed the title tcp: Add source address option tcp: connect: Add source_address and bind_device options May 3, 2023
@kgraefe kgraefe force-pushed the tcp-connect-add-source-address branch 6 times, most recently from fac3e82 to 07fbb1d Compare May 3, 2023 10:42
@kgraefe kgraefe marked this pull request as ready for review May 3, 2023 10:42
Copy link
Owner

@lemunozm lemunozm left a comment

Choose a reason for hiding this comment

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

Thanks for this hard-to-implement feature, I left a couple of comments

Comment on lines 40 to 41
/// Bind the TCP connection to a specific interface, identified by its name. On other systems
/// this opion will be ignored.
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
/// Bind the TCP connection to a specific interface, identified by its name. On other systems
/// this opion will be ignored.
/// Bind the TCP connection to a specific interface, identified by its name.
/// This option works in Unix, on other systems, it will be ignored.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh yes, I changed that sentence multiple times. At some point I must have dropped the Unix part accidentally .

Copy link
Owner

Choose a reason for hiding this comment

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

No worries! usually happens 😆

Comment on lines +135 to +138
#[cfg(unix)]
Err(e) if e.raw_os_error() != Some(libc::EINPROGRESS) => return Err(e),
#[cfg(windows)]
Err(e) if e.kind() != io::ErrorKind::WouldBlock => return Err(e),
Copy link
Owner

Choose a reason for hiding this comment

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

Ask from the unknowledge, could a WouldBlock be obtained on Unix at this point (it seems like it's totally fine returning that for a socket implementation at this point)? In this case, an error will be returned when we shouldn't

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Owner

Choose a reason for hiding this comment

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

Then it should be ok 👍🏻

@kgraefe kgraefe force-pushed the tcp-connect-add-source-address branch from 07fbb1d to 9f783b6 Compare May 5, 2023 08:10
kgraefe added 2 commits May 5, 2023 10:12
This option allows to specify the source address and port for outgoing
TCP connections.

Signed-off-by: Konrad Gräfe <[email protected]>
Add option to bind an outgoing TCP connection to a specific device,
identified by its name.

Signed-off-by: Konrad Gräfe <[email protected]>
@kgraefe kgraefe force-pushed the tcp-connect-add-source-address branch from 9f783b6 to 41a9ef8 Compare May 5, 2023 08:12
@lemunozm
Copy link
Owner

lemunozm commented May 5, 2023

Thanks again for another awesome contribution! 🚀

@lemunozm lemunozm merged commit 6c1c58f into lemunozm:master May 5, 2023
@lemunozm lemunozm mentioned this pull request May 5, 2023
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