* [PATCH v3] tcp: fix tp->undo_retrans accounting in tcp_sacktag_one()
@ 2021-08-30 9:00 zhenggy
2021-09-13 15:49 ` Eric Dumazet
2021-09-13 15:52 ` Jakub Kicinski
0 siblings, 2 replies; 6+ messages in thread
From: zhenggy @ 2021-08-30 9:00 UTC (permalink / raw)
To: ncardwell, netdev
Cc: edumazet, davem, yoshfuji, dsahern, kuba, ycheng, qitiepeng,
wujianguo, liyonglong, zhenggy
Commit 10d3be569243 ("tcp-tso: do not split TSO packets at retransmit
time") may directly retrans a multiple segments TSO/GSO packet without
split, Since this commit, we can no longer assume that a retransmitted
packet is a single segment.
This patch fixes the tp->undo_retrans accounting in tcp_sacktag_one()
that use the actual segments(pcount) of the retransmitted packet.
Before that commit (10d3be569243), the assumption underlying the
tp->undo_retrans-- seems correct.
Fixes: 10d3be569243 ("tcp-tso: do not split TSO packets at retransmit time")
Signed-off-by: zhenggy <zhenggy@chinatelecom.cn>
---
net/ipv4/tcp_input.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 3f7bd7a..141e85e 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -1346,7 +1346,7 @@ static u8 tcp_sacktag_one(struct sock *sk,
if (dup_sack && (sacked & TCPCB_RETRANS)) {
if (tp->undo_marker && tp->undo_retrans > 0 &&
after(end_seq, tp->undo_marker))
- tp->undo_retrans--;
+ tp->undo_retrans = max_t(int, 0, tp->undo_retrans - pcount);
if ((sacked & TCPCB_SACKED_ACKED) &&
before(start_seq, state->reord))
state->reord = start_seq;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v3] tcp: fix tp->undo_retrans accounting in tcp_sacktag_one()
2021-08-30 9:00 [PATCH v3] tcp: fix tp->undo_retrans accounting in tcp_sacktag_one() zhenggy
@ 2021-09-13 15:49 ` Eric Dumazet
2021-09-13 16:10 ` Yuchung Cheng
2021-09-13 15:52 ` Jakub Kicinski
1 sibling, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2021-09-13 15:49 UTC (permalink / raw)
To: zhenggy
Cc: Neal Cardwell, netdev, David Miller, Hideaki YOSHIFUJI,
David Ahern, Jakub Kicinski, Yuchung Cheng, qitiepeng, wujianguo,
liyonglong
On Mon, Sep 13, 2021 at 3:51 AM zhenggy <zhenggy@chinatelecom.cn> wrote:
>
> Commit 10d3be569243 ("tcp-tso: do not split TSO packets at retransmit
> time") may directly retrans a multiple segments TSO/GSO packet without
> split, Since this commit, we can no longer assume that a retransmitted
> packet is a single segment.
>
> This patch fixes the tp->undo_retrans accounting in tcp_sacktag_one()
> that use the actual segments(pcount) of the retransmitted packet.
>
> Before that commit (10d3be569243), the assumption underlying the
> tp->undo_retrans-- seems correct.
>
> Fixes: 10d3be569243 ("tcp-tso: do not split TSO packets at retransmit time")
>
nit: We normally do not add an empty line between Fixes: tag and others.
Reviewed-by: Eric Dumazet <edumazet@google.com>
> Signed-off-by: zhenggy <zhenggy@chinatelecom.cn>
> ---
> net/ipv4/tcp_input.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index 3f7bd7a..141e85e 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -1346,7 +1346,7 @@ static u8 tcp_sacktag_one(struct sock *sk,
> if (dup_sack && (sacked & TCPCB_RETRANS)) {
> if (tp->undo_marker && tp->undo_retrans > 0 &&
> after(end_seq, tp->undo_marker))
> - tp->undo_retrans--;
> + tp->undo_retrans = max_t(int, 0, tp->undo_retrans - pcount);
> if ((sacked & TCPCB_SACKED_ACKED) &&
> before(start_seq, state->reord))
> state->reord = start_seq;
> --
> 1.8.3.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v3] tcp: fix tp->undo_retrans accounting in tcp_sacktag_one()
2021-09-13 15:49 ` Eric Dumazet
@ 2021-09-13 16:10 ` Yuchung Cheng
2021-09-13 16:27 ` Neal Cardwell
0 siblings, 1 reply; 6+ messages in thread
From: Yuchung Cheng @ 2021-09-13 16:10 UTC (permalink / raw)
To: Eric Dumazet
Cc: zhenggy, Neal Cardwell, netdev, David Miller, Hideaki YOSHIFUJI,
David Ahern, Jakub Kicinski, qitiepeng, wujianguo, liyonglong
On Mon, Sep 13, 2021 at 8:49 AM Eric Dumazet <edumazet@google.com> wrote:
>
> On Mon, Sep 13, 2021 at 3:51 AM zhenggy <zhenggy@chinatelecom.cn> wrote:
> >
> > Commit 10d3be569243 ("tcp-tso: do not split TSO packets at retransmit
> > time") may directly retrans a multiple segments TSO/GSO packet without
> > split, Since this commit, we can no longer assume that a retransmitted
> > packet is a single segment.
> >
> > This patch fixes the tp->undo_retrans accounting in tcp_sacktag_one()
> > that use the actual segments(pcount) of the retransmitted packet.
> >
> > Before that commit (10d3be569243), the assumption underlying the
> > tp->undo_retrans-- seems correct.
> >
> > Fixes: 10d3be569243 ("tcp-tso: do not split TSO packets at retransmit time")
> >
>
> nit: We normally do not add an empty line between Fixes: tag and others.
>
> Reviewed-by: Eric Dumazet <edumazet@google.com>
Acked-by: Yuchung Cheng <ycheng@google.com>
This is a nice fix that would increase the effectiveness of TCP undo
in high-speed networks.
>
> > Signed-off-by: zhenggy <zhenggy@chinatelecom.cn>
> > ---
> > net/ipv4/tcp_input.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> > index 3f7bd7a..141e85e 100644
> > --- a/net/ipv4/tcp_input.c
> > +++ b/net/ipv4/tcp_input.c
> > @@ -1346,7 +1346,7 @@ static u8 tcp_sacktag_one(struct sock *sk,
> > if (dup_sack && (sacked & TCPCB_RETRANS)) {
> > if (tp->undo_marker && tp->undo_retrans > 0 &&
> > after(end_seq, tp->undo_marker))
> > - tp->undo_retrans--;
> > + tp->undo_retrans = max_t(int, 0, tp->undo_retrans - pcount);
> > if ((sacked & TCPCB_SACKED_ACKED) &&
> > before(start_seq, state->reord))
> > state->reord = start_seq;
> > --
> > 1.8.3.1
> >
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v3] tcp: fix tp->undo_retrans accounting in tcp_sacktag_one()
2021-09-13 16:10 ` Yuchung Cheng
@ 2021-09-13 16:27 ` Neal Cardwell
2021-09-14 0:17 ` zhenggy
0 siblings, 1 reply; 6+ messages in thread
From: Neal Cardwell @ 2021-09-13 16:27 UTC (permalink / raw)
To: Yuchung Cheng
Cc: Eric Dumazet, zhenggy, netdev, David Miller, Hideaki YOSHIFUJI,
David Ahern, Jakub Kicinski, qitiepeng, wujianguo, liyonglong
On Mon, Sep 13, 2021 at 12:11 PM Yuchung Cheng <ycheng@google.com> wrote:
>
> On Mon, Sep 13, 2021 at 8:49 AM Eric Dumazet <edumazet@google.com> wrote:
> >
> > On Mon, Sep 13, 2021 at 3:51 AM zhenggy <zhenggy@chinatelecom.cn> wrote:
> > >
> > > Commit 10d3be569243 ("tcp-tso: do not split TSO packets at retransmit
> > > time") may directly retrans a multiple segments TSO/GSO packet without
> > > split, Since this commit, we can no longer assume that a retransmitted
> > > packet is a single segment.
> > >
> > > This patch fixes the tp->undo_retrans accounting in tcp_sacktag_one()
> > > that use the actual segments(pcount) of the retransmitted packet.
> > >
> > > Before that commit (10d3be569243), the assumption underlying the
> > > tp->undo_retrans-- seems correct.
> > >
> > > Fixes: 10d3be569243 ("tcp-tso: do not split TSO packets at retransmit time")
> > >
> >
> > nit: We normally do not add an empty line between Fixes: tag and others.
> >
> > Reviewed-by: Eric Dumazet <edumazet@google.com>
> Acked-by: Yuchung Cheng <ycheng@google.com>
>
> This is a nice fix that would increase the effectiveness of TCP undo
> in high-speed networks.
Yes, thanks for the fix!
Acked-by: Neal Cardwell <ncardwell@google.com>
neal
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v3] tcp: fix tp->undo_retrans accounting in tcp_sacktag_one()
2021-09-13 16:27 ` Neal Cardwell
@ 2021-09-14 0:17 ` zhenggy
0 siblings, 0 replies; 6+ messages in thread
From: zhenggy @ 2021-09-14 0:17 UTC (permalink / raw)
To: Neal Cardwell, Yuchung Cheng, edumazet, Jakub Kicinski
Cc: netdev, David Miller, Hideaki YOSHIFUJI, David Ahern, qitiepeng,
wujianguo, liyonglong
Thanks very much for all your comments, i will repost a v4 patch later.
在 2021/9/14 0:27, Neal Cardwell 写道:
> On Mon, Sep 13, 2021 at 12:11 PM Yuchung Cheng <ycheng@google.com> wrote:
>>
>> On Mon, Sep 13, 2021 at 8:49 AM Eric Dumazet <edumazet@google.com> wrote:
>>>
>>> On Mon, Sep 13, 2021 at 3:51 AM zhenggy <zhenggy@chinatelecom.cn> wrote:
>>>>
>>>> Commit 10d3be569243 ("tcp-tso: do not split TSO packets at retransmit
>>>> time") may directly retrans a multiple segments TSO/GSO packet without
>>>> split, Since this commit, we can no longer assume that a retransmitted
>>>> packet is a single segment.
>>>>
>>>> This patch fixes the tp->undo_retrans accounting in tcp_sacktag_one()
>>>> that use the actual segments(pcount) of the retransmitted packet.
>>>>
>>>> Before that commit (10d3be569243), the assumption underlying the
>>>> tp->undo_retrans-- seems correct.
>>>>
>>>> Fixes: 10d3be569243 ("tcp-tso: do not split TSO packets at retransmit time")
>>>>
>>>
>>> nit: We normally do not add an empty line between Fixes: tag and others.
>>>
>>> Reviewed-by: Eric Dumazet <edumazet@google.com>
>> Acked-by: Yuchung Cheng <ycheng@google.com>
>>
>> This is a nice fix that would increase the effectiveness of TCP undo
>> in high-speed networks.
>
> Yes, thanks for the fix!
>
> Acked-by: Neal Cardwell <ncardwell@google.com>
>
> neal
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] tcp: fix tp->undo_retrans accounting in tcp_sacktag_one()
2021-08-30 9:00 [PATCH v3] tcp: fix tp->undo_retrans accounting in tcp_sacktag_one() zhenggy
2021-09-13 15:49 ` Eric Dumazet
@ 2021-09-13 15:52 ` Jakub Kicinski
1 sibling, 0 replies; 6+ messages in thread
From: Jakub Kicinski @ 2021-09-13 15:52 UTC (permalink / raw)
To: zhenggy
Cc: ncardwell, netdev, edumazet, davem, yoshfuji, dsahern, ycheng,
qitiepeng, wujianguo, liyonglong
On Mon, 30 Aug 2021 05:00:10 -0400 zhenggy wrote:
> Commit 10d3be569243 ("tcp-tso: do not split TSO packets at retransmit
> time") may directly retrans a multiple segments TSO/GSO packet without
> split, Since this commit, we can no longer assume that a retransmitted
> packet is a single segment.
>
> This patch fixes the tp->undo_retrans accounting in tcp_sacktag_one()
> that use the actual segments(pcount) of the retransmitted packet.
>
> Before that commit (10d3be569243), the assumption underlying the
> tp->undo_retrans-- seems correct.
>
> Fixes: 10d3be569243 ("tcp-tso: do not split TSO packets at retransmit time")
>
Please remove this empty line. There should be no empty lines between
tags.
> Signed-off-by: zhenggy <zhenggy@chinatelecom.cn>
Please fix the data on your system and repost. I'm pretty sure this
wasn't posted on Aug 30th.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-09-14 0:18 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-30 9:00 [PATCH v3] tcp: fix tp->undo_retrans accounting in tcp_sacktag_one() zhenggy
2021-09-13 15:49 ` Eric Dumazet
2021-09-13 16:10 ` Yuchung Cheng
2021-09-13 16:27 ` Neal Cardwell
2021-09-14 0:17 ` zhenggy
2021-09-13 15:52 ` Jakub Kicinski
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.