Re: [PATCH] tcp: properly update lost_cnt_hint during shifting

Could you please clarify this case for me-

skb == tp->lost_skb_hint
If skb is sacked, doesn't tcp_mark_head_lost() already increment
lost_cnt_hint, in which case you won't need to do it here?

Nandita

On Wed, Sep 28, 2011 at 4:29 AM, Ilpo Järvinen
[off-list ref] wrote:

On Wed, 28 Sep 2011, Yan, Zheng wrote:

quoted

quoted

But is the non-SACKed case really handled right when hint == skb by the
sacktag_one. We move the seqno in between and then before(x->newseq,
x->newseq) check returns false?

you are right, thank you.

really hope my patch is correct this time :)
---

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 21fab3e..a04622e 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c

@@ -1390,8 +1390,7 @@ static int tcp_shifted_skb(struct sock *sk, struct sk_buff *skb,

      BUG_ON(!pcount);

      /* Tweak before seqno plays */
-     if (!tcp_is_fack(tp) && tcp_is_sack(tp) && tp->lost_skb_hint &&
-         !before(TCP_SKB_CB(tp->lost_skb_hint)->seq, TCP_SKB_CB(skb)->seq))
+     if (!tcp_is_fack(tp) && tcp_is_sack(tp) && tp->lost_skb_hint == skb)
              tp->lost_cnt_hint += pcount;

      TCP_SKB_CB(prev)->end_seq += shifted;

It also looks a lot nicer now and more obvious. According to my current
understanding, feel free to add this once doing the proper submission with
Signed-off etc. (please also remove the comment too as seqnos have no
longer any significance here):

Acked-by: Ilpo Järvinen <redacted>

...but it certainly wouldn't hurt if also somebody else has pair of eyes
to spare to confirm that we (both) are now in agreement what the code
really says.


--
 i.