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

Fix mDNS INBOUND_BUFFER_SIZE. Fixes Windows misbehaving. #515

Merged

Conversation

tklajnscek
Copy link
Contributor

@tklajnscek tklajnscek commented Nov 21, 2023

Currently, the INBOUND_BUFFER_SIZE is 512 bytes, which means that the data gets truncated in the recv_from call in start when the incoming payload is larger (happens a lot in our office network, for example).

The problem is that this happens silently on Unix systems and everything works OK (well, the messages are truncated, but still). Unfortunately on Windows socket.recv_from actually returns an error in this case - WSAEMSGSIZE (10040) and mDNS starts logging errors and doesn't behave the same.

This is currently blocking us using using WebRTC via this crate on Windows.

For more info, see:
https://docs.rs/mio/latest/mio/net/struct.UdpSocket.html#method.recv_from as excerpted here:
image

The UDP buffer can be up to 64k so this PR bumps it to the maximum size to avoid this difference in behavoir. Realistically, it'll almost never be above 1500 unless using jumbo frames or some other exotic network setup, but better safe than sorry.

The alternative would be to check the error message for WSAEMSGSIZE and treat it as success to behave like Unix, but unfortunately once an Error is returned from socket.recv_from we can't get the length and address that Ok contains.

Copy link

codecov bot commented Dec 4, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (361acc4) 61.64% compared to head (3579e8d) 61.53%.
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #515      +/-   ##
==========================================
- Coverage   61.64%   61.53%   -0.12%     
==========================================
  Files         529      529              
  Lines       48865    48865              
  Branches    12383    12367      -16     
==========================================
- Hits        30122    30068      -54     
- Misses       9575     9593      +18     
- Partials     9168     9204      +36     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rainliu rainliu merged commit 7d09375 into webrtc-rs:master Dec 4, 2023
4 of 5 checks passed
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.

2 participants