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

clean code #74

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

clean code #74

wants to merge 7 commits into from

Conversation

ssrlive
Copy link
Contributor

@ssrlive ssrlive commented Nov 5, 2023

due to libc crate updated, ifreq struct added

due to libc crate updated, ifreq struct added
Copy link
Contributor

Choose a reason for hiding this comment

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

wintun::Adapter::create(&wintun, tun_name, tun_name, guid)? The last argument should be set as None here. I find the given guid can make the creation of the tun device fail, especially, the next time after the device was created successfully.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This fixed value prevents the number of device name aliases from growing indefinitely. so needn't to set to None.

Copy link
Contributor

Choose a reason for hiding this comment

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

This fixed value prevents the number of device name aliases from growing indefinitely. so needn't to set to None.

However, the device can fail to be created with the current arguments, during my practice test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

post your log info here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

chat from telegram?

Copy link
Contributor

Choose a reason for hiding this comment

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

result

Run the example, then key ctrl+c to terminate the program, I repeated the process three times, and I got the error Error: WintunError(String("Failed to create adapter")). You can try to do this process

Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure this error can be stably reproduced within three times

Copy link
Contributor Author

Choose a reason for hiding this comment

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

emm, I can't meet this issue. my OS is windows 11.
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This number will not changed while we set a guid.
image

Copy link
Contributor Author

@ssrlive ssrlive Dec 2, 2023

Choose a reason for hiding this comment

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

I have add a switch for tun, now you can set guid to Some(u128) or None in the examples.

@xmh0511
Copy link
Contributor

xmh0511 commented Dec 26, 2023

@meh Please take a look at this patch for enhancing read/write performance on Windows platform.

@fujiapple852
Copy link

Stumbled on this PR whilst debugging an issue in CI.

Assuming the changes here are the same as those from https://github.com/ssrlive/rust-tun e944758, then this PR fixes the issues I've been seeing using this crate with async code on Windows from Github CI (see fujiapple852/trippy#891 for WIP code which work with the fork and fails sporadically with rust-run).

I haven't bisected exactly which change here fixes the issue, as I've only just discovered this to be the cause, but I will.

@ssrlive
Copy link
Contributor Author

ssrlive commented Jan 7, 2024

Hi @meh , If you really don't have time to maintain this repo and crate, I would like you to add me as a co-contributor and publisher. what do you think?

@ssrlive ssrlive force-pushed the master branch 5 times, most recently from 8f2f109 to dbc1c4e Compare January 12, 2024 15:04
@ssrlive
Copy link
Contributor Author

ssrlive commented Jan 13, 2024

Hi @meh, if this PR has not been responded a week later, I will have to publish tun2 crate, and I don't want this to happen. Hope you can respond in time.

For me, a submitted PR has not been reviewed for a long time, cannot be merged to the main branch, and cannot be published. It is like a patient who has not been sutured on the operating table for a long time. This is a bad experience. I believe that many people feel the same.

@pronebird
Copy link

pronebird commented Mar 4, 2024

The changes are all over the place. It would be easier to accept if you narrowed your scope and instead pushed a few more focused and smaller PRs.

feifeigood added a commit to feifeigood/rust-tun that referenced this pull request Jul 8, 2024
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

4 participants