netdev - Re: [PATCH net-next 2/2] tcp: remove a redundant check for SKBTX_ACK

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] [day] [month] [year] [list]

Message-ID: <CACSApvZ-i+q3iR3LXk_VF1XgNefRTx41H9hOFtUBHKD+pj-eSg@mail.gmail.com>
Date:	Wed, 27 Apr 2016 15:25:51 -0400
From:	Soheil Hassas Yeganeh <soheil@...gle.com>
To:	Martin KaFai Lau <kafai@...com>
Cc:	Soheil Hassas Yeganeh <soheil.kdev@...il.com>,
	David Miller <davem@...emloft.net>,
	netdev <netdev@...r.kernel.org>,
	Willem de Bruijn <willemb@...gle.com>,
	Eric Dumazet <edumazet@...gle.com>,
	Yuchung Cheng <ycheng@...gle.com>,
	Neal Cardwell <ncardwell@...gle.com>
Subject: Re: [PATCH net-next 2/2] tcp: remove a redundant check for SKBTX_ACK_TSTAMP

On Wed, Apr 27, 2016 at 2:19 PM, Martin KaFai Lau <kafai@...com> wrote:
> On Mon, Apr 25, 2016 at 04:51:13PM -0400, Soheil Hassas Yeganeh wrote:
>> From: Soheil Hassas Yeganeh <soheil@...gle.com>
>>
>> txstamp_ack in tcp_skb_cb is set iff the SKBTX_ACK_TSTAMP
>> flag is set for an skb. Thus, it is not required to check
>> shinfo->tx_flags if the txstamp_ack bit is checked.
>>
>> Remove the check on shinfo->tx_flags & SKBTX_ACK_TSTAMP, since
>> it has already been checked using the txstamp_ack bit.
>>
>> Signed-off-by: Soheil Hassas Yeganeh <soheil@...gle.com>
>> ---
>>  net/ipv4/tcp_input.c | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
>> index 967520d..2f3fd92 100644
>> --- a/net/ipv4/tcp_input.c
>> +++ b/net/ipv4/tcp_input.c
>> @@ -3087,8 +3087,7 @@ static void tcp_ack_tstamp(struct sock *sk, struct sk_buff *skb,
>>               return;
>>
>>       shinfo = skb_shinfo(skb);
>> -     if ((shinfo->tx_flags & SKBTX_ACK_TSTAMP) &&
>> -         !before(shinfo->tskey, prior_snd_una) &&
>> +     if (!before(shinfo->tskey, prior_snd_una) &&
>>           before(shinfo->tskey, tcp_sk(sk)->snd_una))
>>               __skb_tstamp_tx(skb, NULL, sk, SCM_TSTAMP_ACK);
>>  }
>> --
>> 2.8.0.rc3.226.g39d4020
>>
> Acked-by: Martin KaFai Lau <kafai@...com>
>
> Can it be one step further and completely remove SKBTX_ACK_TSTAMP?
> like what Willem has also suggested here:
> http://www.spinics.net/lists/netdev/msg374231.html
>
> It seems no one else is using the SKBTX_ACK_TSTAMP except TCP.

Ah, good point. Will update the patch then. Thanks!