lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <000001d850e8$0c5743f0$2505cbd0$@wangsu.com> Date: Sat, 16 Apr 2022 00:44:16 +0800 From: "Pengcheng Yang" <yangpc@...gsu.com> To: "'Neal Cardwell'" <ncardwell@...gle.com> Cc: "'Eric Dumazet'" <edumazet@...gle.com>, "'Yuchung Cheng'" <ycheng@...gle.com>, <netdev@...r.kernel.org> Subject: Re: [PATCH net-next] tcp: ensure to use the most recently sent skb when filling the rate sample On Fri, Apr 15, 2022 at 12:12 AM Neal Cardwell <ncardwell@...gle.com> wrote: > > On Wed, Apr 13, 2022 at 6:54 AM Pengcheng Yang <yangpc@...gsu.com> wrote: > > > > If an ACK (s)acks multiple skbs, we favor the information > > from the most recently sent skb by choosing the skb with > > the highest prior_delivered count. > > But prior_delivered may be equal, because tp->delivered only > > changes when receiving, which requires further comparison of > > skb timestamp. > > > > Signed-off-by: Pengcheng Yang <yangpc@...gsu.com> > > Thank you for posting this patch! I agree there is a bug there, and > your patch is an improvement. However, I think this patch is not a > complete solution, since it does not handle the case where there are > multiple skbs with the tcp_skb_timestamp_us() (which can happen if a > outgoing buffered TSO/GSO skb is later split into multiple skbs with > the same timestamp). > My initial thought was that this case would not affect the correctness of rate_sample, since the timestamp of these skbs are the same. So I am confused whether we have to find the *real* most recently (with the highest seq) skb (at the cost of a little extra cost)? > RACK has to deal with the same question "which skb was sent first?", > and already has a solution in tcp_rack_sent_after(). I suggest we > share code between RACK and tcp_rate_skb_delivered() to make this > check. This might involve making a copy of tcp_rack_sent_after() in > include/net/tcp.h, naming the .h copy to tcp_skb_sent_after(), and > reworking this logic to save and use the sequence number and timestamp > so that it can use the new tcp_skb_sent_after() helper. After this fix > propagates to net-next we could later then change RACK to use the new > tcp_skb_sent_after() function, so we have a single helper used in two > places. > Ok. I will send the V2 later according to your suggestion. Thanks neal. > If you want to post a version of this patch that uses some approach > like that, IMHO that would be welcome. If you do not have cycles, I am > happy to post one when I get a moment. > > thanks, > neal
Powered by blists - more mailing lists