From 74b7010d676c91b706a458af28cb51bfa4b9056c Mon Sep 17 00:00:00 2001 From: whitequark Date: Thu, 31 Aug 2017 14:12:26 +0000 Subject: [PATCH] runtime: allow safely pulling logs even on TRACE log level. Before this commit, this resulted in a packet flood, because sending a TRACE log message to the host caused more TRACE log messages to be emitted. --- artiq/firmware/Cargo.lock | 6 +++--- artiq/firmware/runtime/Cargo.toml | 2 +- artiq/firmware/runtime/mgmt.rs | 26 +++++++++++++++++++++----- artiq/firmware/runtime/sched.rs | 8 ++++++-- 4 files changed, 31 insertions(+), 11 deletions(-) diff --git a/artiq/firmware/Cargo.lock b/artiq/firmware/Cargo.lock index ce52706bb..7b8e36b3c 100644 --- a/artiq/firmware/Cargo.lock +++ b/artiq/firmware/Cargo.lock @@ -157,7 +157,7 @@ dependencies = [ "log 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)", "logger_artiq 0.0.0", "proto 0.0.0", - "smoltcp 0.4.0-pre (git+https://github.com/m-labs/smoltcp?rev=f76b4be)", + "smoltcp 0.4.0-pre (git+https://github.com/m-labs/smoltcp?rev=7e2dc1a)", "std_artiq 0.0.0", ] @@ -183,7 +183,7 @@ dependencies = [ [[package]] name = "smoltcp" version = "0.4.0-pre" -source = "git+https://github.com/m-labs/smoltcp?rev=f76b4be#f76b4be42d5e80f33000d20979fcc8cccac7c14a" +source = "git+https://github.com/m-labs/smoltcp?rev=7e2dc1a#7e2dc1a6d67278cb68f5e03f0ac21d58e85c92f7" dependencies = [ "byteorder 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)", @@ -221,7 +221,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" "checksum log_buffer 1.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "ec57723b84bbe7bdf76aa93169c9b59e67473317c6de3a83cb2a0f8ccb2aa493" "checksum managed 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "61eb783b4fa77e8fa4d27ec400f97ed9168546b8b30341a120b7ba9cc6571aaf" "checksum rustc-cfg 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "56a596b5718bf5e059d59a30af12f7f462a152de147aa462b70892849ee18704" -"checksum smoltcp 0.4.0-pre (git+https://github.com/m-labs/smoltcp?rev=f76b4be)" = "" +"checksum smoltcp 0.4.0-pre (git+https://github.com/m-labs/smoltcp?rev=7e2dc1a)" = "" "checksum walkdir 1.0.3 (registry+https://github.com/rust-lang/crates.io-index)" = "dd7c16466ecc507c7cb5988db03e6eab4aaeab89a5c37a29251fcfd3ac9b7afe" "checksum winapi 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)" = "167dc9d6949a9b857f3451275e911c3f44255842c1f7a76f33c55103a909087a" "checksum winapi-build 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "2d315eee3b34aca4797b2da6b13ed88266e6d612562a0c46390af8299fc699bc" diff --git a/artiq/firmware/runtime/Cargo.toml b/artiq/firmware/runtime/Cargo.toml index 0f5174001..80ade98b2 100644 --- a/artiq/firmware/runtime/Cargo.toml +++ b/artiq/firmware/runtime/Cargo.toml @@ -32,6 +32,6 @@ features = ["mem"] [dependencies.smoltcp] git = "https://github.com/m-labs/smoltcp" -rev = "f76b4be" +rev = "7e2dc1a" default-features = false features = ["alloc", "collections", "log"]#, "verbose"] diff --git a/artiq/firmware/runtime/mgmt.rs b/artiq/firmware/runtime/mgmt.rs index 0e1819b5e..2078e2a3a 100644 --- a/artiq/firmware/runtime/mgmt.rs +++ b/artiq/firmware/runtime/mgmt.rs @@ -1,4 +1,5 @@ -use std::io::{self, Read}; +use std::io::{self, Read, Write}; +use log::LogLevelFilter; use logger_artiq::BufferLogger; use sched::Io; use sched::{TcpListener, TcpStream}; @@ -43,10 +44,25 @@ fn worker(io: &Io, stream: &mut TcpStream) -> io::Result<()> { io.until(|| BufferLogger::with(|logger| !logger.is_empty()))?; BufferLogger::with(|logger| { - match logger.extract(|log| stream.write_string(log)) { - Ok(()) => Ok(logger.clear()), - Err(e) => Err(e) - } + let log_level = logger.max_log_level(); + logger.extract(|log| { + stream.write_string(log)?; + + if log_level == LogLevelFilter::Trace { + // Hold exclusive access over the logger until we get positive + // acknowledgement; otherwise we get an infinite loop of network + // trace messages being transmitted and causing more network + // trace messages to be emitted. + // + // Any messages unrelated to this management socket that arrive + // while it is flushed are lost, but such is life. + stream.flush() + } else { + Ok(()) + } + })?; + + Ok(logger.clear()) as io::Result<()> })?; } }, diff --git a/artiq/firmware/runtime/sched.rs b/artiq/firmware/runtime/sched.rs index de0dbf4c6..11d5da676 100644 --- a/artiq/firmware/runtime/sched.rs +++ b/artiq/firmware/runtime/sched.rs @@ -528,8 +528,12 @@ impl<'a> Write for TcpStream<'a> { } fn flush(&mut self) -> Result<()> { - // smoltcp always sends all available data when it's possible; nothing to do - Ok(()) + until!(self, TcpSocketLower, |s| s.send_queue() == 0 || !s.may_send())?; + if self.as_lower().send_queue() == 0 { + Ok(()) + } else { + Err(Error::new(ErrorKind::ConnectionAborted, "connection aborted")) + } } }