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

feat(tun): windows auto-route #561

Merged
merged 8 commits into from
Aug 31, 2024
Merged

feat(tun): windows auto-route #561

merged 8 commits into from
Aug 31, 2024

Conversation

ibigbug
Copy link
Member

@ibigbug ibigbug commented Aug 29, 2024

🤔 This is a ...

  • New feature
  • Bug fix
  • Performance optimization
  • Enhancement feature
  • Refactoring
  • Code style optimization
  • Test Case
  • Branch merge
  • Workflow
  • Other (about what?)

🔗 Related issue link

#559

💡 Background and solution

https://learn.microsoft.com/en-us/windows/win32/rras/add-and-update-routes-using-rtmaddroutetodest

📝 Changelog

tun:
  enable: true
  device-id: "dev://utun1989"
  route-all: false
  routes:
    - 0.0.0.0/1
    - 128.0.0.0/1

☑️ Self-Check before Merge

⚠️ Please check all items below before requesting a reviewing. ⚠️

  • Doc is updated/provided or not needed
  • Changelog is provided or not needed

@ibigbug ibigbug self-assigned this Aug 30, 2024
@ibigbug ibigbug added enhancement New feature or request core labels Aug 30, 2024
@ibigbug ibigbug marked this pull request as ready for review August 30, 2024 14:26
@Itsusinn
Copy link
Member

实测能通吗?windows api那部分实在是看不懂

@ibigbug
Copy link
Member Author

ibigbug commented Aug 30, 2024

通。那部分别看了直接看cmd的吧。留着以后知道啥原因修好了再用那个。

@Itsusinn
Copy link
Member

别修了吧,感觉没啥用,微软的文档都不全

Itsusinn
Itsusinn previously approved these changes Aug 30, 2024
IpNet::new(std::net::IpAddr::V4(Ipv4Addr::new(128, 0, 0, 0)), 1)
.unwrap(),
];
for r in default_routes {
Copy link
Contributor

Choose a reason for hiding this comment

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

shall we make this a two step operation of build_routes & add_routes, thus is can be separated into the routes module

Copy link
Member Author

Choose a reason for hiding this comment

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

i'm hoping the 0/1 and 128/1 can work on all platforms so the build_route will just be the same 2 entries

/// https://learn.microsoft.com/en-us/windows/win32/rras/add-and-update-routes-using-rtmaddroutetodest
/// FIXME: figure out why this doesn't work https://stackoverflow.com/questions/43632619/how-to-properly-use-rtmv2-and-rtmaddroutetodest
#[allow(dead_code)]
pub fn add_route_that_does_not_work(
Copy link
Contributor

Choose a reason for hiding this comment

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

this is how net-route handled the routing table: https://github.com/johnyburd/net-route/blob/f83a447bd13404225c8a78db71af208ee8e7885c/src/platform_impl/windows.rs#L178. it seems that they took a different approach

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah i checked their code they used a different set of API. which makes sense. maybe I'll switch to using the crate for this too when I do the macos one

@ibigbug ibigbug merged commit d6171fe into master Aug 31, 2024
23 checks passed
@ibigbug ibigbug deleted the win-route branch August 31, 2024 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants