Merge neqo-client and neqo-server

[mxinden]

neqo-client and neqo-server have a lot of logic in common, yet each duplicates it.

A couple of examples

• A lot of command line arguments are shared between client and server:

  • neqo/neqo-client/src/main.rs

    Lines 123 to 219 in ad027cf

    	#[derive(Debug, Parser)]
    	#[command(author, version, about, long_about = None)]
    	#[allow(clippy::struct_excessive_bools)] // Not a good use of that lint.
    	pub struct Args {
    	#[arg(short = 'a', long, default_value = "h3")]
    	/// ALPN labels to negotiate.
    	///
    	/// This client still only does HTTP/3 no matter what the ALPN says.
    	alpn: String,
    	urls: Vec<Url>,
    	#[arg(short = 'm', default_value = "GET")]
    	method: String,
    	#[arg(short = 'H', long, number_of_values = 2)]
    	header: Vec<String>,
    	#[arg(name = "encoder-table-size", long, default_value = "16384")]
    	max_table_size_encoder: u64,
    	#[arg(name = "decoder-table-size", long, default_value = "16384")]
    	max_table_size_decoder: u64,
    	#[arg(name = "max-blocked-streams", short = 'b', long, default_value = "10")]
    	max_blocked_streams: u16,
    	#[arg(name = "max-push", short = 'p', long, default_value = "10")]
    	max_concurrent_push_streams: u64,
    	#[arg(name = "use-old-http", short = 'o', long)]
    	/// Use http 0.9 instead of HTTP/3
    	use_old_http: bool,
    	#[arg(name = "download-in-series", long)]
    	/// Download resources in series using separate connections.
    	download_in_series: bool,
    	#[arg(name = "concurrency", long, default_value = "100")]
    	/// The maximum number of requests to have outstanding at one time.
    	concurrency: usize,
    	#[arg(name = "output-read-data", long)]
    	/// Output received data to stdout
    	output_read_data: bool,
    	#[arg(name = "qlog-dir", long)]
    	/// Enable QLOG logging and QLOG traces to this directory
    	qlog_dir: Option<PathBuf>,
    	#[arg(name = "output-dir", long)]
    	/// Save contents of fetched URLs to a directory
    	output_dir: Option<PathBuf>,
    	#[arg(name = "qns-test", long)]
    	/// Enable special behavior for use with QUIC Network Simulator
    	qns_test: Option<String>,
    	#[arg(short = 'r', long)]
    	/// Client attempts to resume by making multiple connections to servers.
    	/// Requires that 2 or more URLs are listed for each server.
    	/// Use this for 0-RTT: the stack always attempts 0-RTT on resumption.
    	resume: bool,
    	#[arg(name = "key-update", long)]
    	/// Attempt to initiate a key update immediately after confirming the connection.
    	key_update: bool,
    	#[arg(short = 'c', long, number_of_values = 1)]
    	/// The set of TLS cipher suites to enable.
    	/// From: TLS_AES_128_GCM_SHA256, TLS_AES_256_GCM_SHA384, TLS_CHACHA20_POLY1305_SHA256.
    	ciphers: Vec<String>,
    	#[arg(name = "ech", long, value_parser = |s: &str| hex::decode(s))]
    	/// Enable encrypted client hello (ECH).
    	/// This takes an encoded ECH configuration in hexadecimal format.
    	ech: Option<Vec<u8>>,
    	#[command(flatten)]
    	quic_parameters: QuicParameters,
    	#[arg(name = "ipv4-only", short = '4', long)]
    	/// Connect only over IPv4
    	ipv4_only: bool,
    	#[arg(name = "ipv6-only", short = '6', long)]
    	/// Connect only over IPv6
    	ipv6_only: bool,
    	/// The test that this client will run. Currently, we only support "upload".
    	#[arg(name = "test", long)]
    	test: Option<String>,
    	/// The request size that will be used for upload test.
    	#[arg(name = "upload-size", long, default_value = "100")]
    	upload_size: usize,
    	}

  • neqo/neqo-server/src/main.rs

    Lines 92 to 151 in ad027cf

    	#[derive(Debug, Parser)]
    	#[command(author, version, about, long_about = None)]
    	struct Args {
    	/// List of IP:port to listen on
    	#[arg(default_value = "[::]:4433")]
    	hosts: Vec<String>,
    	#[arg(name = "encoder-table-size", long, default_value = "16384")]
    	max_table_size_encoder: u64,
    	#[arg(name = "decoder-table-size", long, default_value = "16384")]
    	max_table_size_decoder: u64,
    	#[arg(short = 'b', long, default_value = "10")]
    	max_blocked_streams: u16,
    	#[arg(short = 'd', long, default_value = "./test-fixture/db")]
    	/// NSS database directory.
    	db: PathBuf,
    	#[arg(short = 'k', long, default_value = "key")]
    	/// Name of key from NSS database.
    	key: String,
    	#[arg(short = 'a', long, default_value = "h3")]
    	/// ALPN labels to negotiate.
    	///
    	/// This server still only does HTTP3 no matter what the ALPN says.
    	alpn: String,
    	#[arg(name = "qlog-dir", long, value_parser=clap::value_parser!(PathBuf))]
    	/// Enable QLOG logging and QLOG traces to this directory
    	qlog_dir: Option<PathBuf>,
    	#[arg(name = "qns-test", long)]
    	/// Enable special behavior for use with QUIC Network Simulator
    	qns_test: Option<String>,
    	#[arg(name = "use-old-http", short = 'o', long)]
    	/// Use http 0.9 instead of HTTP/3
    	use_old_http: bool,
    	#[command(flatten)]
    	quic_parameters: QuicParameters,
    	#[arg(name = "retry", long)]
    	/// Force a retry
    	retry: bool,
    	#[arg(short = 'c', long, number_of_values = 1)]
    	/// The set of TLS cipher suites to enable.
    	/// From: TLS_AES_128_GCM_SHA256, TLS_AES_256_GCM_SHA384, TLS_CHACHA20_POLY1305_SHA256.
    	ciphers: Vec<String>,
    	#[arg(name = "ech", long)]
    	/// Enable encrypted client hello (ECH).
    	/// This generates a new set of ECH keys when it is invoked.
    	/// The resulting configuration is printed to stdout in hexadecimal format.
    	ech: bool,
    	}

• Each has (at least) one process implementation calling client.process and handling its return value.

  • neqo/neqo-client/src/main.rs

    Lines 860 to 881 in ad027cf

    	async fn process(&mut self, mut dgram: Option<&Datagram>) -> Result<(), io::Error> {
    	loop {
    	match self.client.process(dgram.take(), Instant::now()) {
    	Output::Datagram(dgram) => {
    	self.socket.writable().await?;
    	self.socket.send(dgram)?;
    	}
    	Output::Callback(new_timeout) => {
    	qinfo!("Setting timeout of {:?}", new_timeout);
    	self.timeout = Some(Box::pin(tokio::time::sleep(new_timeout)));
    	break;
    	}
    	Output::None => {
    	qdebug!("Output::None");
    	break;
    	}
    	}
    	}
    	Ok(())
    	}
    	}

  • neqo/neqo-server/src/main.rs

    Lines 646 to 665 in ad027cf

    	async fn process(&mut self, mut dgram: Option<&Datagram>) -> Result<(), io::Error> {
    	loop {
    	match self.server.process(dgram.take(), self.args.now()) {
    	Output::Datagram(dgram) => {
    	let socket = self.find_socket(dgram.source());
    	socket.writable().await?;
    	socket.send(dgram)?;
    	}
    	Output::Callback(new_timeout) => {
    	qinfo!("Setting timeout of {:?}", new_timeout);
    	self.timeout = Some(Box::pin(tokio::time::sleep(new_timeout)));
    	break;
    	}
    	Output::None => {
    	break;
    	}
    	}
    	}
    	Ok(())
    	}

• Each has a similar run loop.

  • neqo/neqo-client/src/main.rs

    Lines 819 to 858 in ad027cf

    	async fn run(mut self) -> Res<Option<ResumptionToken>> {
    	loop {
    	if !self.handler.handle(&mut self.client)? {
    	break;
    	}
    	self.process(None).await?;
    	match ready(self.socket, self.timeout.as_mut()).await? {
    	Ready::Socket => loop {
    	let dgram = self.socket.recv(&self.local_addr)?;
    	if dgram.is_none() {
    	break;
    	}
    	self.process(dgram.as_ref()).await?;
    	self.handler.maybe_key_update(&mut self.client)?;
    	},
    	Ready::Timeout => {
    	self.timeout = None;
    	}
    	}
    	if let Http3State::Closed(..) = self.client.state() {
    	break;
    	}
    	}
    	let token = if self.args.test.is_none() && self.args.resume {
    	// If we haven't received an event, take a token if there is one.
    	// Lots of servers don't provide NEW_TOKEN, but a session ticket
    	// without NEW_TOKEN is better than nothing.
    	self.handler
    	.token
    	.take()
    	.or_else(|| self.client.take_resumption_token(Instant::now()))
    	} else {
    	None
    	};
    	Ok(token)
    	}

  • neqo/neqo-server/src/main.rs

    Lines 686 to 707 in ad027cf

    	async fn run(&mut self) -> Result<(), io::Error> {
    	loop {
    	match self.ready().await? {
    	Ready::Socket(inx) => loop {
    	let (host, socket) = self.sockets.get_mut(inx).unwrap();
    	let dgram = socket.recv(host)?;
    	if dgram.is_none() {
    	break;
    	}
    	self.process(dgram.as_ref()).await?;
    	},
    	Ready::Timeout => {
    	self.timeout = None;
    	self.process(None).await?;
    	}
    	}
    	self.server.process_events(&self.args, self.args.now());
    	self.process(None).await?;
    	}
    	}
    	}

• ...

Before making any major changes to neqo-client and neqo-server, like e.g. #1693, I thus suggest merging the two into one crate, and thereby deduplicating the shared logic.

Suggestion:

• Introduce neqo-bin/ crate.
• Move neqo-common/src/udp.rs into neqo-bin/src/udp.rs.
• Consolidate shared logic between neqo-client and neqo-server in neqo-bin/src/lib.rs and submodules.
• Move neqo-client/src/main.rs into neqo-bin/bin/client.rs and neqo-server/src/main.rs into neqo-bin/bin/server.rs.

What do folks think? Why have they been split into two separate crates thus far?

[larseggert]

Sounds good.

[larseggert]

Done via #1724