-
-
Notifications
You must be signed in to change notification settings - Fork 431
Sending/receiving of a >~100KB of data fails on some Android devices #2246
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
Comments
Might be related, I ran the other tests on the Samsung phone as well and these fail:
I ran those on one other (Google Pixel) device where
The output of the
|
Is this using the very latest version of |
I forked from 9f008ad which was the latest |
I just tried to add |
Can you share the logs from the test when it is failing? In my experience, Android kernels are a lot more restrictive than general Linux. It would be interesting to see which syscalls are failing. |
You might have to initialise a logger: let _guard = tracing_subscriber::fmt().with_test_writer().with_env_filter("trace").set_default(); |
Here is I replaced these lines with - tracing::subscriber::set_global_default(
- tracing_subscriber::FmtSubscriber::builder()
- .with_env_filter(tracing_subscriber::EnvFilter::from_default_env())
- .finish(),
- )
- .unwrap();
+ let _guard_fmt = tracing_subscriber::fmt()
+ .with_test_writer()
+ .with_env_filter("trace")
+ .set_default(); I'm not sure that's exactly what you asked for though. I'm here looking into it so feel free to ask me to check other things, if you like send me an email and we can set up a more direct channel to debug this. |
Please note that in the original post I attached samsung-with-trace.log, perhaps that's what you asked for? |
The tests you linked above are
Unfortunately, that doesn't seem to contain any logs from
Sorry for being so brief here. The logger would need to go on top of the quinn/quinn-udp/tests/tests.rs Line 187 in 4f8a0f1
If you patch the logger in and run the tests again, it should give you an output directly in the terminal where you typed |
I think this issue is fine. I am mostly familiar with |
This is the answer. UDP GSO was only added to Linux in 4.18: torvalds/linux@cb586c6 |
It would still be helpful to see the logs of the tests. We already have some checks in place to detect situations where GSO is not available. Seems like there might be another one or it is not working for some reason. |
There doesn't seem to be any output from @@ -184,6 +186,19 @@ fn ecn_v4_mapped_v6() {
ignore
)]
fn gso() {
+ let _guard = tracing_subscriber::fmt()
+ .with_test_writer()
+ .with_env_filter("trace")
+ .set_default();
+
let send = UdpSocket::bind((Ipv6Addr::LOCALHOST, 0))
.or_else(|_| UdpSocket::bind((Ipv4Addr::LOCALHOST, 0)))
.unwrap(); And added a test line output to --- a/quinn-udp/src/unix.rs
+++ b/quinn-udp/src/unix.rs
@@ -84,6 +84,7 @@ pub struct UdpSocketState {
impl UdpSocketState {
pub fn new(sock: UdpSockRef<'_>) -> io::Result<Self> {
+ crate::log::trace!("Test line");
let io = sock.0;
let mut cmsg_platform_space = 0;
if cfg!(target_os = "linux") Then push to the Android and run with adb -d shell RUST_LOG=trace /data/local/tmp/test-quinn gso --nocapture (Note that the name of the executable is just how I renamed it so it's easier for me to type. That is, it's not the name that But the only output I see is the test line and the assertion:
|
Interesting, so that means the existing checks for GSO don't work. Can you log this error? Line 339 in 4f8a0f1
|
Actually, the fact that it fails on the comparison means we never get to this error path? That is odd. It would mean that Android silently fails despite it not supporting GSO. I would have expexted to see the following line: Line 825 in 4f8a0f1
|
Yes, the execution won't reach the Line 336 in 4f8a0f1
The value of |
Which is obviously way too big for a single UDP packet on the public Internet. I am surprised that the kernel doesn't complain at all about the unsupported cmsgs. I am afraid the only thing you can do is disable GSO entirely. If we can find a way of reading the kernel version, we could also disable GSO based on that within |
Maybe the |
On the affected device this code let mut n = unsafe { std::mem::zeroed() };
let r = unsafe { libc::uname(&mut n) };
assert_eq!(r, 0);
let release = unsafe {
std::ffi::CStr::from_ptr(n.release[..].as_ptr())
.to_string_lossy()
.into_owned()
};
crate::log::trace!("{release:?}"); prints |
Maybe something like this? inetic@26e3950
Apologies for the noise, I got nerd sniped, I'll wait for the opinions from here on :-). In the mean time I can use the above kernel version check in our code to disable GSO. |
+1 to surprise that Linux doesn't generate an error on unknown cmsg. If that's happening, then it's totally reasonable for quinn-udp to check the kernel version. I don't love parsing a string for it, but if that's the best option, so be it. |
Great debugging work! +1 for checking the kernel version at runtime. I can not think of a better option. |
I think this is a good start. The fallback value IMO when we have a parsing error should perhaps be Wanna open a PR to discuss these details?
I wonder if it is specific to Android and stock Linux 4.14 does error? If so, then I think it would make sense to guard such a check on |
On a particular Samsung Android phone sending and receiving of ~100KB data fails with
ConnectionLost(TimedOut)
.We wrote a test for it here send_receive.rs (also attaching for posterity send_receive.rs.txt).
The test creates a connected pair of sockets, sender and a receiver. The sender sends 100 messages of size 1000B to the receiver (the exact number of messages and sizes doesn't seem to matter as long as it's above some threshold).
Everywhere apart of that Samsung phone the test runs as expected and the receiver receives all the messages. On the Samsung phone we observe that the sender sends all the messages, but the receiver receives only few of them and then times out (samsung-no-trace.log, samsung-with-trace.log).
May be worth noting that adding
tokio::task::yield_now().await
right below thewrite_to_peer
function resolves the issue and the test passes.Some info on the affected phone:
samsung-cpuinfo.txt
samsung-meminfo.txt
To run the test I use
The text was updated successfully, but these errors were encountered: