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

Fix MTU detection below 1280 with IPv6 on windows #5805

Merged
merged 1 commit into from
Feb 13, 2024

Conversation

Serock3
Copy link
Contributor

@Serock3 Serock3 commented Feb 12, 2024


This change is Reviewable

@Serock3 Serock3 self-assigned this Feb 12, 2024
@Serock3 Serock3 requested a review from dlon February 12, 2024 14:50
@Serock3 Serock3 added bug Windows Issues related to Windows labels Feb 12, 2024
Copy link
Member

@dlon dlon left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Serock3)


talpid-windows/src/net.rs line 339 at r1 (raw file):

    let mut row = match get_ip_interface_entry(ip_family, &luid) {
        Ok(row) => row,
        Err(error) if error.raw_os_error() == Some(ERROR_NOT_FOUND as i32) => return Ok(()),

I think we should return this error. It makes little sense to ignore the error given that the caller is specifically asking to set the MTU for the given family. Also, both interfaces/families should always exist when we call this.

@Serock3 Serock3 force-pushed the mtu-detection-windows-patch branch from ef407fb to 93493d1 Compare February 12, 2024 15:04
Copy link
Member

@dlon dlon left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@dlon dlon force-pushed the mtu-detection-windows-patch branch from 93493d1 to fd502da Compare February 13, 2024 08:50
@dlon dlon merged commit b0e4315 into main Feb 13, 2024
23 checks passed
@dlon dlon deleted the mtu-detection-windows-patch branch February 13, 2024 08:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Windows Issues related to Windows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants