search

cloudflare / boringtun

Userspace WireGuard® Implementation in Rust
BSD 3-Clause "New" or "Revised" License
5.92k stars 396 forks source link

creation of "connected socket" returns unsupported #395

Open codemonkeylabs-de opened 3 months ago

codemonkeylabs-de commented 3 months ago

Hi, while troubleshooting performance issues regarding the integration of the boringtun library into a project I am working on, I ran into the following like of code:

https://github.com/cloudflare/boringtun/blob/f672bb6c1e1e371240a8d151f15854687eb740bb/boringtun/src/device/peer.rs#L120

Now, creating a UDP socket of type STREAM (as opposed to DGRAM) is quite likely an error since it doesn't make much sense. This call returns an IoError(Os { code: 93, kind: Uncategorized, message: "Protocol not supported" } and the wireguard peer lookup happens on every packet received in the anonymous UDP handler thus reducing throughput to a crawl.

The fix is ofc easy enough (i.e. changing the socket type to DGRAM), I tried this myself and performance is as expected greatly improved to at least the same order of magnitude as the cloudflare warp app.. but this is what bugs me: beyond the actual problem, it strikes me as quite unlikely this is the code that cloudflare uses in production; I mean, a bug like this suggests perhaps I am not using the correct branch? Comments welcome. Thanks!

givascu commented 1 month ago

I've recently run into this too and can confirm that this typo (most likely Type::STREAM wasn't intended there) dramatically reduces the throughput when running boringtun with the "connected UDP socket" config ON (the default). Fortunately the fix is trivial enough such that it should be addressed sooner rather than later.

P.S. A workaround for those unable to patch the library themselves would be to set use_connected_socket to false in the device config or to pass the --disable-connected-udp option when running boringtun-cli (for the executable users).