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

Improve tunnel writer trait #19

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pinkisemils
Copy link

The tunnel reader trait as used on macOS (#18) forces a copy of the received traffic since macOS's utun requires extra header to be passed into the tunnel interface when receiving tunnel traffic. Since the Writer::write method takes a single parameter that is the slice of bytes that should be written to the utun device, and the slice doesn't contain the headers utun expects, the slice has to be copied and a buffer needs to be allocated. Since the header on macOS are always 4 bytes long, it will always be smaller than the wireguard header that's preceding the payload in the buffer that's originally backing the slice.

This seems like a bit of a hack, and I'm open to better suggestions to reducing the amount of copying required.

@rot256
Copy link
Member

rot256 commented May 1, 2021

I remember this exact issue when writing wireguard-go, I have yet to come up with a good fix.

Maybe a slightly cleaner work-around is making it an associated constant, since it should never change.

@pinkisemils
Copy link
Author

Ultimately, it's the caller of the trait that has to make sure that the message is offset by the correct amount of bytes, so the associated constant approach also feels slightly cumbersome. Then again, I'm ok with having the changes.

@faern
Copy link
Contributor

faern commented Jul 21, 2021

How about making the Writer::write method accept a custom PacketBuffer type. This type has PacketBuffer::buffer_mut() -> &mut [u8] where the packet data can be written and that is automatically offset by 4 bytes on macOS. That way all platforms write into the buffer with the same code, but some compile time constant makes sure macOS has the data offset in the underlying buffer.

Ultimately, it's the caller of the trait that has to make sure that the message is offset by the correct amount of bytes

Yeah, but this can be forced with the type system rather than having them do it manually. By having the method only accept a custom type that only expose an offset buffer slice on the platforms that need it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants