google / quiche

BSD 3-Clause "New" or "Revised" License
622 stars 132 forks source link

Quic Session Datagram Queue is ignored when considering whether App limited #28

Open mpelletan opened 1 year ago

mpelletan commented 1 year ago

Doing some prototyping using Quiche with WebTransport+BBR2, I found the datagram queue appeared to be ignored when assessing whether the app is limiting the bitrate:

  1. QuicSession::datagramqueue: https://github.com/google/quiche/blob/9b4551033df6483c4bf18c9e70ec14897070aa80/quiche/quic/core/quic_session.h#L979
  2. QuicSession::WillingAndAbleToWrite() does not consider the datagram queue, and therefore returns false when data is being paced by congestion control: https://github.com/google/quiche/blob/9b4551033df6483c4bf18c9e70ec14897070aa80/quiche/quic/core/quic_session.cc#L711
  3. WillingAndAbleToWrite() is used by QuicConnection to determine whether congestion control is app limited: https://github.com/google/quiche/blob/9b4551033df6483c4bf18c9e70ec14897070aa80/quiche/quic/core/quic_connection.cc#L5277

In my test, where the pipe is filled with datagrams, BBR constantly marks datapoints as app limited and never gets out of STARTUP phase.

Simply considering a non-empty datagram queue as willingness to write allows BBR to get into PROBE_BW, although I'm sill seeing poor behavior, where the RTT increases quite dramatically from ~40ms (min RTT) to sustained ~200ms when we hit a bitrate close to congestion, but I haven't looked into what's causing it (no draining on latency increase?)

ianswett commented 1 year ago

Wow, good catch. That is indeed a bug.