facebook / mvfst

An implementation of the QUIC transport protocol.
MIT License
1.5k stars 242 forks source link

Is this a bug in Copa.cpp? #268

Closed yangfurong closed 2 years ago

yangfurong commented 2 years ago

Hi. I have found the code that you use to calculate the congestion window adjustment component may be problematic.

    uint64_t reduction = (ack.ackedPackets.size() * conn_.udpSendPacketLen *
                          conn_.udpSendPacketLen * velocityState_.velocity) /
        (deltaParam_ * cwndBytes_);

Is it correct to multiply conn_.udpSendPacketLen twice?

yangfurong commented 2 years ago

In the original paper, they use v/(d*cwnd_in_pkts) to update cwnd for every single acked pkt. If the cwnd is in bytes and the update of cwnd is called for multiple acked pkts, we could multiply the numerator and denominator by MSS. Then we get the adjustment component as (v * total_acked_bytes) / (d * cwnd_in_bytes), where total_acked_bytes should be total_acked_pkt * MSS.

yangfurong commented 2 years ago

Oh. Sorry. I found this is already a closed issue. Now I understand.