feat(congestion control): add CongestionController trait and example impl

[ytakano]

Add CongestionController trait for congestion controllers of TCP.

By default, this PR uses NoControl, which do not control congestion, and the PR does not affect conventional behavior.

To use Cubic or Reno congestion controllers, socket-tcp-cubic or socket-tcp-reno features must be set or tcp::Socket::new_with_cc() must be used.

Users can implement custom congestion controllers by using CongestionController trait and tcp::Socket::new_with_cc().

Cubic or Reno can be tested as follows.

$ cargo test --features socket-tcp-cubic
$ cargo test --features socket-tcp-reno

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 98.41270% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 79.96%. Comparing base (f0d1fd9) to head (833399f). Report is 4 commits behind head on main.

Files	Patch %	Lines
src/socket/tcp/congestion.rs	97.67%	1 Missing :warning:
src/socket/tcp/congestion/no_control.rs	75.00%	1 Missing :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #907      +/-   ##
==========================================
+ Coverage   79.92%   79.96%   +0.04%     
==========================================
  Files          80       82       +2     
  Lines       28234    28378     +144     
==========================================
+ Hits        22566    22693     +127     
- Misses       5668     5685      +17     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[ytakano]

Avoid cbrt() and powi() for no_std.

[pothos]

Just for your reference, there was also https://github.com/smoltcp-rs/smoltcp/pull/450 and https://github.com/cbranch/smoltcp/commits/cbranch/cc (other non-upstreamed things are also public: https://github.com/cbranch/smoltcp/branches/stale, and if you are looking into BBR there is https://github.com/quinn-rs/quinn/tree/0.8.0)

[ytakano]

When I was testing smoltcp, it occupied traffic bandwidth, and other TCP connections starved due to the lack of congestion control.

By the way, I think delay based congestion control mechanisms like BBR should be also implemented by using CongestionController, because it can know time and bytes of sending/receiving data and RTT by using following methods.

pub trait CongestionController: Default {
    #[allow(unused_variables)]
    fn on_ack(&mut self, now: Instant, len: usize, rtt: &RttEstimator) {}

    #[allow(unused_variables)]
    fn post_transmit(&mut self, now: Instant, len: usize) {}
}

This is similar to quinn. https://github.com/quinn-rs/quinn/blob/0.8.0/quinn-proto/src/congestion.rs

I know theory of BBR, but I cannot evaluate it sufficiently because it requires emulation of delays.

[ytakano]

I think we shouldn't add this with non-additive features. I think it's OK to have an enum of all possible congestion control methods, with some sensible default selection, but still allowing two crates that both use smoltcp to compile without needing to coordinate their feature sets.

Right. A general enum type of congestion control can be prepared. I implemented it before, but discarded. I will fix it later.

Thank you for your review.

[whitequark]

To expand a bit, I'm thinking that the enum could include all of the necessary state, therefore making tcp::Socket as much bigger as the biggest possible to choose congestion controller (0 bytes if none are enabled).

The way to create it would probably be methods on the CongestionController enum, whose fields are all private. Then a Default implementation would pick the "best" one available if several are enabled. (The decision on which congestion controller is the "best" one is probably contentious but I don't expect it to matter that much since most people will pick one.)

Once this PR is in, we should strongly recommend picking a congestion controller in the README, to be a good netizen.

[ytakano]

I have defined set_congestion_control() and get_congestion_control() methods for tcp::Socket as follows.

pub enum CongestionControl {
    None,
    Cubic,
    Reno,
}

impl tcp::Scoket {
    /// Set an algorithm for congestion control.
    ///
    /// The default setting is `CongestionControl::None`, indicating that no congestion control is applied.
    /// Options `CongestionControl::Cubic` and `CongestionControl::Reno` are also available.
    ///
    /// `CongestionControl::Cubic` represents a modern congestion control algorithm designed to
    /// be more efficient and fair compared to `CongestionControl::Reno`.
    /// It is the default choice for Linux, Windows, and macOS.
    ///
    /// `CongestionControl::Reno` is a classic congestion control algorithm valued for its simplicity.
    /// Despite having a lower algorithmic complexity than `Cubic`,
    /// it is less efficient in terms of bandwidth usage.
    pub fn set_congestion_control(&mut self, congestion_control: CongestionControl) { /* omitted */ }

    /// Return the current congestion control algorithm.
    pub fn get_congestion_control(&self) -> CongestionControl { /* omitted */ }
}

CongestionController enum, whose fields are all private.

Variants of a public enum cannot be private. So, I have defined these 2 methods and CongestionControl enum to tcp::Socket.

If the best performance is required, I think it should be a type parameter of tcp::Socket.

pub struct Socket<'a, CC: CongestionController> {
    congestion_controller: CC
}

However, it is less compatible. See https://github.com/smoltcp-rs/smoltcp/pull/907#discussion_r1493859449

If it is a type parameter, the required bytes of the empty congestion controller must be 0 as follows.

let socket = tcp::Socket<EmptyCongestionController>();

[whitequark]

Yeah it's not actually possible to use a type parameter here. A enum is perfectly fine.

[ytakano]

Now we can test Cubic and Reno by specifying the features as follows.

$ cargo test --features=socket-tcp-reno
$ cargo test --features=socket-tcp-cubic

[whitequark]

Can you squash everything into one commit please?

[ytakano]

Squashed.

[whitequark]

I think this includes some unrelated commits?

[ytakano]

Sorry. How about this?

[whitequark]

Thanks!

[whitequark]

@Dirbaio Once this is merged I think we should strongly recommend that Reno be selected unless there is an extremely good reason not to use any congestion control.