-
Notifications
You must be signed in to change notification settings - Fork 349
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
Bump pcap
to address TODO comment in split tunnel implementation
#6685
Bump pcap
to address TODO comment in split tunnel implementation
#6685
Conversation
7003ab9
to
cad48fc
Compare
cad48fc
to
c6a8ceb
Compare
pcap
to address TODO comment in split tunnel implementationpcap
to address TODO comment in split tunnel implementation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great cleanup!
Reviewed 4 of 4 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @MarkusPettersson98)
talpid-core/src/split_tunnel/macos/bindings.rs
line 1 at r1 (raw file):
// automatically generated by rust-bindgen 0.69.2
Note that this file is generated by a bash script/header. So it should be updated.
talpid-core/src/split_tunnel/macos/tun.rs
line 810 at r1 (raw file):
// libpcap will do the heavy lifting for us if we simply request a "pktap" device. // // TODO: upstream exposure of a raw handle to pcap_t on Capture<Inactive>
I think we don't need the raw pointer, so we can probably remove this TODO comment as well.
c6a8ceb
to
6d98b68
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 2 of 6 files reviewed, 1 unresolved discussion (waiting on @dlon)
talpid-core/src/split_tunnel/macos/bindings.rs
line 1 at r1 (raw file):
Previously, dlon (David Lönnhager) wrote…
Note that this file is generated by a bash script. So it should be updated.
Oh, of course! So satisfying 👍
talpid-core/src/split_tunnel/macos/tun.rs
line 810 at r1 (raw file):
Previously, dlon (David Lönnhager) wrote…
I think we don't need the raw pointer, so we can probably remove this TODO comment as well.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 4 of 4 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved
Use the safe `pcap` function `Direction::want_pktap` to initialize the packet capture in the macOS split tunnel implemenation. This replaces parts of our auto-generated FFI to interface with `libpcap`, thus getting rid of some use of `unsafe`.
5e77045
to
43b276e
Compare
This PR replaces some auto-generated
libpcap
bindings for macOS with recently (WIP) released APIs in thepcap
crate. By usingCapture::want_pktap
we can get rid of 3 uses ofunsafe
in the implementation of split tunneling on macOS, while removing some no-longer needed bindings.This change is