From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Dumazet Subject: Re: [PATCH] tcp: do not update snd_una if it is same with ack_seq Date: Sat, 3 Nov 2018 17:40:39 -0700 Message-ID: <5f585304-b6f5-5f49-adcf-eaed471c0d76@gmail.com> References: <1541264071-9905-1-git-send-email-laoar.shao@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org To: Yafang Shao , davem@davemloft.net, edumazet@google.com Return-path: In-Reply-To: <1541264071-9905-1-git-send-email-laoar.shao@gmail.com> Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org List-Id: netdev.vger.kernel.org On 11/03/2018 09:54 AM, Yafang Shao wrote: > In the slow path, TCP_SKB_SB(skb)->ack_seq may be same with tp->snd_una, > and under this condition we don't need to update the snd_una. > > Furthermore, tcp_ack_update_window() is only called in slow path, > so introducing this check won't affect the fast path processing. > > By the way, '&' is a little faster than '-', so I replaced after() with > "flag & FLAG_SND_UNA_ADVANCED". > > Signed-off-by: Yafang Shao > --- > net/ipv4/tcp_input.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c > index 2868ef2..db5a6b7 100644 > --- a/net/ipv4/tcp_input.c > +++ b/net/ipv4/tcp_input.c > @@ -3376,7 +3376,8 @@ static int tcp_ack_update_window(struct sock *sk, const struct sk_buff *skb, u32 > } > } > > - tcp_snd_una_update(tp, ack); > + if (after(ack, tp->snd_una)) > + tcp_snd_una_update(tp, ack); > Adding this after() here is confusing, how ack could be before snd_una ? That would be a serious bug. I do not see why another conditional has any gain here. You are trying to avoid very cheap operations : u32 delta = ack - tp->snd_una; tp->bytes_acked += delta; tp->snd_una = ack; Maybe the real reason for this patch is not explained in the changelog ?