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

chore(packages): use pnet fork until they fix the pselect bug #83

Merged
merged 2 commits into from
Jan 7, 2020

Conversation

imsnif
Copy link
Owner

@imsnif imsnif commented Jan 7, 2020

This fixes: #51

I forked a few of the libpnet repositories until they merge @zhangxp1998's PR: libpnet/libpnet#394

@zhangxp1998
Copy link
Collaborator

👍LGTM, finally we can fix #51 . Maybe we should also decrease read_timeout for better responsiveness?

@imsnif
Copy link
Owner Author

imsnif commented Jan 7, 2020

Hmm - I thought about that. I removed it on my machine and the CPU rose a little ~9%, but I didn't look very deeply at it.

Tbh, I'm a little afraid to lug this in as well. Looks like this next version will have lots of core changes. As I understand it, the timeout doesn't run the risk of us losing packets or anything like that, right? I think the only change right now would be that we might quit faster? Or am I missing something?

Cargo.toml Show resolved Hide resolved
src/network/sniffer.rs Show resolved Hide resolved
@ebroto
Copy link
Collaborator

ebroto commented Jan 7, 2020

I think the only change right now would be that we might quit faster? Or am I missing something?

That's my understanding, the faster we exit select the more times we will read the atomic bool, which takes CPU. It's not an easy decision :)

We can play with the values in a followup if 1s of quitting is not problematic for now.

@imsnif imsnif merged commit 351e4dd into master Jan 7, 2020
@zhangxp1998
Copy link
Collaborator

A little bit late... but I just found out that we can patch dependencies by using this feature of cargo or that feature of cargo

zhangxp1998 added a commit to zhangxp1998/bandwhich that referenced this pull request Jan 7, 2020
@imsnif
Copy link
Owner Author

imsnif commented Jan 8, 2020

@zhangxp1998 - that was my first thought, but according to our friends on ##rust on freenode, this does not work for binary crates installed from cargo. :(

I saw the other issue you opened and will fix the various forks, ugh! :)

@imsnif imsnif deleted the libpnet-patch branch September 13, 2020 16:28
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.

No traffic shown on some macs
3 participants