Feedback on draft-cheng-iccrg-delivery-rate-estimation-02

[Junho Choi]

(I shared this report with the authors and they advised to share the information with bbr-dev and iccrg, so posting here again)

While implementing this spec (the main purpose is to add BBR on quiche), I think the following part needs some update.

> 3.2. Transmitting or retransmitting a data packet

> SendPacket(Packet P):

       if (SND.NXT == SND.UNA)  /* no packets in flight yet? */
         C.first_sent_time  = C.delivered_time = Now()

-->

       if (SND.NXT == SND.UNA)  /* no packets in flight yet? */
         C.first_sent_time  = C.delivered_time = P.sent_time

Looks like linux already doing the same logic:
https://elixir.bootlin.com/linux/v5.19.14/source/net/ipv4/tcp_rate.c#L59

I also made this change on quiche (https://github.com/cloudflare/quiche/pull/1337) and got some good results as well.

--

Junho Choi <junho dot choi at gmail.com> | https://saturnsoft.net

[Neal Cardwell]

Hi Junho,

Thanks for posting this issue to the bbr-dev and ICCRG lists, and thank you very much for the bug report! 

As noted in our recent e-mail discussion, your fix looks correct to me; as you note, your fixed version is what the Linux code in tcp_rate.c is already doing. :-) And so I have updated our team's internal copy of draft-cheng-iccrg-delivery-rate-estimation so that next time we publish an update this fix will be included. I've also added your name to the acknowledgment section.

Thanks!
neal

--
You received this message because you are subscribed to the Google Groups "BBR Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to bbr-dev+u...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/bbr-dev/CAJ5e%2BHAX6VebtDoGKxb%3Dcm1A27veK%3DwquE3%3Djdp%3DbkeG%3DFx9dg%40mail.gmail.com.