netdev - Re: [PATCH] tcp: properly update lost_cnt

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAB_+Fg5ZzC43XcVjoByepZvA4Mknu6PkRWzpUju+KAQnoMb8JQ@mail.gmail.com>
Date:	Wed, 28 Sep 2011 17:06:50 -0700
From:	Nandita Dukkipati <nanditad@...gle.com>
To:	Ilpo Järvinen <ilpo.jarvinen@...sinki.fi>,
	"Yan, Zheng" <zheng.z.yan@...el.com>
Cc:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	Yuchung Cheng <ycheng@...gle.com>
Subject: 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
<ilpo.jarvinen@...sinki.fi> wrote:
> On Wed, 28 Sep 2011, Yan, Zheng wrote:
>
>> > 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 <ilpo.jarvinen@...sinki.fi>
>
> ...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.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html