Skip to content

Commit

Permalink
Many more fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
@larseggert
larseggert committed Nov 20, 2023
1 parent f8a3d7e commit 7f91f4b
Show file tree
Hide file tree
Showing 17 changed files with 175 additions and 81 deletions.
7 changes: 4 additions & 3 deletions neqo-client/src/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -870,6 +870,7 @@ mod old {
time::Instant, os::fd::AsRawFd,
};

use neqo_helper::recv_datagram;
use url::Url;

use super::{qlog_new, KeyUpdateState, Res};
Expand Down Expand Up @@ -1106,7 +1107,8 @@ mod old {
return Ok(client.state().clone());
}

match socket.recv_from(&mut buf[..]) {
let mut tos = 0;
match recv_datagram(socket.as_raw_fd(), &mut buf[..], &mut tos) {
Err(err) => {
if err.kind() != ErrorKind::WouldBlock && err.kind() != ErrorKind::Interrupted {
eprintln!("UDP error: {}", err);
Expand All @@ -1119,8 +1121,7 @@ mod old {
continue;
}
if sz > 0 {
// FIXME: deal with tos
let d = Datagram::new(addr, *local_addr, 0, &buf[..sz]);
let d = Datagram::new(addr, *local_addr, tos, &buf[..sz]);
client.process_input(d, Instant::now());
handler.maybe_key_update(client)?;
}
Expand Down
52 changes: 52 additions & 0 deletions neqo-common/src/datagram.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,58 @@ use std::ops::Deref;

use crate::hex_with_len;

// ECN (Explicit Congestion Notification) codepoints mapped to the
// lower 2 bits of the TOS field.
// https://www.iana.org/assignments/dscp-registry/dscp-registry.xhtml
// #[derive(Copy, Clone)]
pub enum IpTosEcn {
EcnNotEct = 0b00, // Not-ECT (Not ECN-Capable Transport) [RFC3168]
EcnEct1 = 0b01, // ECT(1) (ECN-Capable Transport(1))[1] [RFC8311][RFC Errata 5399][RFC9331]
EcnEct0 = 0b10, // ECT(0) (ECN-Capable Transport(0)) [RFC3168]
EcnCe = 0b11, // CE (Congestion Experienced) [RFC3168]
}

// impl From<IpTosEcn> for u8 {
// fn from(ecn: IpTosEcn) -> Self {
// ecn as u8
// }
// }

// DiffServ Codepoints, mapped to the upper six bits of the TOS field.
// https://www.iana.org/assignments/dscp-registry/dscp-registry.xhtml
// #[derive(Copy, Clone)]
pub enum IpTosDscp {
DscpCs0 = 0b00000000, // [RFC2474]
DscpCs1 = 0b00100000, // [RFC2474]
DscpCs2 = 0b01000000, // [RFC2474]
DscpCs3 = 0b01100000, // [RFC2474]
DscpCs4 = 0b10000000, // [RFC2474]
DscpCs5 = 0b10100000, // [RFC2474]
DscpCs6 = 0b11000000, // [RFC2474]
DscpCs7 = 0b11100000, // [RFC2474]
DscpAf11 = 0b00101000, // [RFC2597]
DscpAf12 = 0b00110000, // [RFC2597]
DscpAf13 = 0b00111000, // [RFC2597]
DscpAf21 = 0b01001000, // [RFC2597]
DscpAf22 = 0b01010000, // [RFC2597]
DscpAf23 = 0b01011000, // [RFC2597]
DscpAf31 = 0b01101000, // [RFC2597]
DscpAf32 = 0b01110000, // [RFC2597]
DscpAf33 = 0b01111000, // [RFC2597]
DscpAf41 = 0b10001000, // [RFC2597]
DscpAf42 = 0b10010000, // [RFC2597]
DscpAf43 = 0b10011000, // [RFC2597]
DscpEf = 0b10111000, // [RFC3246]
DscpVoiceAdmit = 0b10110000, // [RFC5865]
DscpLe = 0b00000100, // [RFC8622]
}

// impl From<IpTosDscp> for u8 {
// fn from(dscp: IpTosDscp) -> Self {
// (dscp as u8) << 2
// }
// }

#[derive(PartialEq, Eq, Clone)]
pub struct Datagram {
src: SocketAddr,
Expand Down
2 changes: 1 addition & 1 deletion neqo-common/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ pub mod qlog;
pub mod timer;

pub use self::codec::{Decoder, Encoder};
pub use self::datagram::Datagram;
pub use self::datagram::{Datagram, IpTosEcn, IpTosDscp};
pub use self::header::Header;
pub use self::incrdecoder::{
IncrementalDecoderBuffer, IncrementalDecoderIgnore, IncrementalDecoderUint,
Expand Down
26 changes: 8 additions & 18 deletions neqo-helper/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use nix::{
},
};

use neqo_common::{Datagram, qdebug};
use neqo_common::{qdebug, Datagram};

// Bind a UDPO socket and set some default socket options.
pub fn bind(local_addr: SocketAddr) -> io::Result<UdpSocket> {
Expand All @@ -38,26 +38,13 @@ pub fn bind(local_addr: SocketAddr) -> io::Result<UdpSocket> {
SocketAddr::V6(..) => setsockopt(&s, Ipv6DontFrag, &true),
};
assert!(res.is_ok());

// Mark all outgoing IP packets with ECT(0). This is a kludge until proper ECN
// validation is supported (RFC9000, Section 13.4.2).
let res = match local_addr {
SocketAddr::V4(..) => setsockopt(&s, IpTos, &0b00000010),
SocketAddr::V6(..) => setsockopt(&s, Ipv6TClass, &0b00000010),
};
assert!(res.is_ok());

// Request IPv4 type-of-service (TOS) and IPv6 traffic class
// information for all incoming packets.
let res = match local_addr {
SocketAddr::V4(..) => setsockopt(&s, IpRecvTos, &true),
SocketAddr::V6(..) => setsockopt(&s, Ipv6RecvTClass, &true),
};
assert!(res.is_ok());

// let res = match local_addr {
// SocketAddr::V4(..) => setsockopt(&s, IpRecvTtl, &true),
// SocketAddr::V6(..) => setsockopt(&s, Ipv6HopLimit, &true),
// };
// assert!(res.is_ok());
s
}
};
Expand All @@ -70,8 +57,11 @@ fn to_sockaddr(addr: SocketAddr) -> SockaddrStorage {

pub fn emit_datagram(fd: i32, d: Datagram) -> io::Result<()> {
let iov = [IoSlice::new(&d[..])];
let tos = d.tos();
let cmsg = ControlMessage::IpTos(&tos);
let tos = d.tos() as i32;
let cmsg = match d.destination() {
SocketAddr::V4(..) => ControlMessage::IpTos(&tos),
SocketAddr::V6(..) => ControlMessage::Ipv6TClass(&tos),
};
let sent = sendmsg(
fd,
&iov,
Expand Down
1 change: 1 addition & 0 deletions neqo-interop/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ neqo-transport = { path = "./../neqo-transport" }
neqo-common = { path="./../neqo-common" }
neqo-http3 = { path = "./../neqo-http3" }
neqo-qpack = { path = "./../neqo-qpack" }
neqo-helper = { path = "./../neqo-helper" }

structopt = "0.3.7"
lazy_static = "1.3.0"
Expand Down
33 changes: 15 additions & 18 deletions neqo-interop/src/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

use neqo_common::{event::Provider, hex, Datagram};
use neqo_crypto::{init, AuthenticationStatus, ResumptionToken};
use neqo_helper::{bind, emit_datagram, recv_datagram};
use neqo_http3::{Header, Http3Client, Http3ClientEvent, Http3Parameters, Http3State, Priority};
use neqo_transport::{
Connection, ConnectionError, ConnectionEvent, ConnectionParameters, EmptyConnectionIdGenerator,
Expand All @@ -21,6 +22,7 @@ use std::{
collections::HashSet,
mem,
net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr, ToSocketAddrs, UdpSocket},
os::fd::AsRawFd,
rc::Rc,
};
// use std::path::PathBuf;
Expand Down Expand Up @@ -60,13 +62,6 @@ trait Handler {
}
}

fn emit_datagram(socket: &UdpSocket, d: Datagram) {
let sent = socket.send(&d[..]).expect("Error sending datagram");
if sent != d.len() {
eprintln!("Unable to send all {} bytes of datagram", d.len());
}
}

lazy_static::lazy_static! {
static ref TEST_TIMEOUT: Mutex<Duration> = Mutex::new(Duration::from_secs(5));
}
Expand Down Expand Up @@ -116,7 +111,10 @@ fn process_loop(
match output {
Output::Datagram(dgram) => {
let dgram = handler.rewrite_out(&dgram).unwrap_or(dgram);
emit_datagram(&nctx.socket, dgram);
if let Err(e) = emit_datagram(nctx.socket.as_raw_fd(), dgram) {
eprintln!("UDP write error: {}", e);
continue;
}
}
Output::Callback(duration) => {
let delay = min(timer.check()?, duration);
Expand All @@ -133,7 +131,8 @@ fn process_loop(
return Ok(client.state().clone());
}

let sz = match nctx.socket.recv(&mut buf[..]) {
let mut tos = 0;
let (sz, _) = match recv_datagram(nctx.socket.as_raw_fd(), &mut buf[..], &mut tos) {
Ok(sz) => sz,
Err(e) => {
return Err(String::from(match e.kind() {
Expand All @@ -148,8 +147,7 @@ fn process_loop(
continue;
}
if sz > 0 {
// FIXME: deal with tos
let received = Datagram::new(nctx.remote_addr, nctx.local_addr, 0, &buf[..sz]);
let received = Datagram::new(nctx.remote_addr, nctx.local_addr, tos, &buf[..sz]);
client.process_input(received, Instant::now());
}
}
Expand Down Expand Up @@ -280,7 +278,7 @@ fn process_loop_h3(
loop {
let output = handler.h3.conn().process_output(Instant::now());
match output {
Output::Datagram(dgram) => emit_datagram(&nctx.socket, dgram),
Output::Datagram(dgram) => emit_datagram(nctx.socket.as_raw_fd(), dgram).unwrap(),
Output::Callback(duration) => {
let delay = min(timer.check()?, duration);
nctx.socket.set_read_timeout(Some(delay)).unwrap();
Expand All @@ -295,7 +293,8 @@ fn process_loop_h3(
return Ok(handler.h3.conn().state().clone());
}

let sz = match nctx.socket.recv(&mut buf[..]) {
let mut tos = 0;
let (sz, _) = match recv_datagram(nctx.socket.as_raw_fd(), &mut buf[..], &mut tos) {
Ok(sz) => sz,
Err(e) => {
return Err(String::from(match e.kind() {
Expand All @@ -310,8 +309,7 @@ fn process_loop_h3(
continue;
}
if sz > 0 {
// FIXME: deal with tos
let received = Datagram::new(nctx.remote_addr, nctx.local_addr, 0, &buf[..sz]);
let received = Datagram::new(nctx.remote_addr, nctx.local_addr, tos, &buf[..sz]);
handler.h3.process_input(received, Instant::now());
}
}
Expand Down Expand Up @@ -684,8 +682,7 @@ impl Handler for VnHandler {
fn rewrite_out(&mut self, d: &Datagram) -> Option<Datagram> {
let mut payload = d[..].to_vec();
payload[1] = 0x1a;
// FIXME: deal with tos
Some(Datagram::new(d.source(), d.destination(), 0, payload))
Some(Datagram::new(d.source(), d.destination(), d.tos(), payload))
}
}

Expand All @@ -707,7 +704,7 @@ fn test_vn(nctx: &NetworkCtx, peer: &Peer) -> Connection {
}

fn run_test<'t>(peer: &Peer, test: &'t Test) -> (&'t Test, String) {
let socket = UdpSocket::bind(peer.bind()).expect("Unable to bind UDP socket");
let socket = bind(peer.bind()).expect("Unable to bind UDP socket");
socket.connect(peer).expect("Unable to connect UDP socket");

let local_addr = socket.local_addr().expect("Socket local address not bound");
Expand Down
3 changes: 1 addition & 2 deletions neqo-server/src/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -568,8 +568,7 @@ fn read_dgram(
eprintln!("zero length datagram received?");
Ok(None)
} else {
// FIXME: deal with tos
Ok(Some(Datagram::new(remote_addr, *local_address, 0, &buf[..sz])))
Ok(Some(Datagram::new(remote_addr, *local_address, tos, &buf[..sz])))
}
}

Expand Down
7 changes: 5 additions & 2 deletions neqo-transport/src/connection/tests/close.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use super::{connect, connect_force_idle, default_client, default_server, send_so
use crate::tparams::{self, TransportParameter};
use crate::{AppError, ConnectionError, Error, ERROR_APPLICATION_CLOSE};

use neqo_common::Datagram;
use neqo_common::{Datagram, IpTosEcn};
use std::time::Duration;
use test_fixture::{self, addr, now};

Expand Down Expand Up @@ -201,6 +201,9 @@ fn stateless_reset_client() {
.unwrap();
connect_force_idle(&mut client, &mut server);

client.process_input(Datagram::new(addr(), addr(), vec![77; 21]), now());
client.process_input(
Datagram::new(addr(), addr(), IpTosEcn::EcnEct0 as u8, vec![77; 21]),
now(),
);
assert_draining(&client, &Error::StatelessReset);
}
9 changes: 6 additions & 3 deletions neqo-transport/src/connection/tests/handshake.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use crate::{
ConnectionError, ConnectionParameters, EmptyConnectionIdGenerator, Error, StreamType, Version,
};

use neqo_common::IpTosEcn;
use neqo_common::{event::Provider, qdebug, Datagram};
use neqo_crypto::{
constants::TLS_CHACHA20_POLY1305_SHA256, generate_ech_keys, AuthenticationStatus,
Expand Down Expand Up @@ -615,7 +616,7 @@ fn corrupted_initial() {
.find(|(_, &v)| v != 0)
.unwrap();
corrupted[idx] ^= 0x76;
let dgram = Datagram::new(d.source(), d.destination(), corrupted);
let dgram = Datagram::new(d.source(), d.destination(), d.tos(), corrupted);
server.process_input(dgram, now());
// The server should have received two packets,
// the first should be dropped, the second saved.
Expand Down Expand Up @@ -711,7 +712,7 @@ fn extra_initial_invalid_cid() {
let mut copy = hs.to_vec();
assert_ne!(copy[5], 0); // The DCID should be non-zero length.
copy[6] ^= 0xc4;
let dgram_copy = Datagram::new(hs.destination(), hs.source(), copy);
let dgram_copy = Datagram::new(hs.destination(), hs.source(), hs.tos(), copy);
let nothing = client.process(Some(dgram_copy), now).dgram();
assert!(nothing.is_none());
}
Expand Down Expand Up @@ -814,7 +815,7 @@ fn garbage_initial() {
let mut corrupted = Vec::from(&initial[..initial.len() - 1]);
corrupted.push(initial[initial.len() - 1] ^ 0xb7);
corrupted.extend_from_slice(rest.as_ref().map_or(&[], |r| &r[..]));
let garbage = Datagram::new(addr(), addr(), corrupted);
let garbage = Datagram::new(addr(), addr(), IpTosEcn::EcnEct0 as u8, corrupted);
assert_eq!(Output::None, server.process(Some(garbage), now()));
}

Expand All @@ -832,6 +833,7 @@ fn drop_initial_packet_from_wrong_address() {
let dgram = Datagram::new(
SocketAddr::new(IpAddr::V6(Ipv6Addr::new(0xfe80, 0, 0, 0, 0, 0, 0, 2)), 443),
p.destination(),
IpTosEcn::EcnEct0 as u8,
&p[..],
);

Expand All @@ -858,6 +860,7 @@ fn drop_handshake_packet_from_wrong_address() {
let dgram = Datagram::new(
SocketAddr::new(IpAddr::V6(Ipv6Addr::new(0xfe80, 0, 0, 0, 0, 0, 0, 2)), 443),
p.destination(),
IpTosEcn::EcnEct0 as u8,
&p[..],
);

Expand Down
4 changes: 2 additions & 2 deletions neqo-transport/src/connection/tests/migration.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ fn loopback() -> SocketAddr {
}

fn change_path(d: &Datagram, a: SocketAddr) -> Datagram {
Datagram::new(a, a, &d[..])
Datagram::new(a, a, d.tos(), &d[..])
}

fn new_port(a: SocketAddr) -> SocketAddr {
Expand All @@ -61,7 +61,7 @@ fn new_port(a: SocketAddr) -> SocketAddr {
}

fn change_source_port(d: &Datagram) -> Datagram {
Datagram::new(new_port(d.source()), d.destination(), &d[..])
Datagram::new(new_port(d.source()), d.destination(), d.tos(), &d[..])
}

/// As these tests use a new path, that path often has a non-zero RTT.
Expand Down
Loading

0 comments on commit 7f91f4b

Please sign in to comment.