* [PATCH] tcp: fix tcp_shifted_skb() adjustment of lost_cnt_hint for FACK
@ 2012-02-14 6:22 Neal Cardwell
2012-02-14 19:40 ` David Miller
0 siblings, 1 reply; 10+ messages in thread
From: Neal Cardwell @ 2012-02-14 6:22 UTC (permalink / raw)
To: David Miller
Cc: netdev, ilpo.jarvinen, Nandita Dukkipati, Yuchung Cheng,
Tom Herbert, Vijay Subramanian, Neal Cardwell
This commit ensures that lost_cnt_hint is correctly updated in
tcp_shifted_skb() for FACK TCP senders. The lost_cnt_hint adjustment
in tcp_sacktag_one() only applies to non-FACK senders, so FACK senders
need their own adjustment.
This applies the spirit of 1e5289e121372a3494402b1b131b41bfe1cf9b7f -
except now that the sequence range passed into tcp_sacktag_one() is
correct we need only have a special case adjustment for FACK.
Signed-off-by: Neal Cardwell <ncardwell@google.com>
---
net/ipv4/tcp_input.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 8116d06..53c8ce4 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -1403,6 +1403,10 @@ static int tcp_shifted_skb(struct sock *sk, struct sk_buff *skb,
BUG_ON(!pcount);
+ /* Adjust hint for FACK. Non-FACK is handled in tcp_sacktag_one(). */
+ if (tcp_is_fack(tp) && (skb == tp->lost_skb_hint))
+ tp->lost_cnt_hint += pcount;
+
TCP_SKB_CB(prev)->end_seq += shifted;
TCP_SKB_CB(skb)->seq += shifted;
--
1.7.7.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] tcp: fix tcp_shifted_skb() adjustment of lost_cnt_hint for FACK
2012-02-14 6:22 [PATCH] tcp: fix tcp_shifted_skb() adjustment of lost_cnt_hint for FACK Neal Cardwell
@ 2012-02-14 19:40 ` David Miller
2012-02-14 19:41 ` David Miller
2012-02-27 23:39 ` Ilpo Järvinen
0 siblings, 2 replies; 10+ messages in thread
From: David Miller @ 2012-02-14 19:40 UTC (permalink / raw)
To: ncardwell
Cc: netdev, ilpo.jarvinen, nanditad, ycheng, therbert,
subramanian.vijay
From: Neal Cardwell <ncardwell@google.com>
Date: Tue, 14 Feb 2012 01:22:08 -0500
> This commit ensures that lost_cnt_hint is correctly updated in
> tcp_shifted_skb() for FACK TCP senders. The lost_cnt_hint adjustment
> in tcp_sacktag_one() only applies to non-FACK senders, so FACK senders
> need their own adjustment.
>
> This applies the spirit of 1e5289e121372a3494402b1b131b41bfe1cf9b7f -
> except now that the sequence range passed into tcp_sacktag_one() is
> correct we need only have a special case adjustment for FACK.
>
> Signed-off-by: Neal Cardwell <ncardwell@google.com>
Applied, and queued up for -stable, thanks Neil.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] tcp: fix tcp_shifted_skb() adjustment of lost_cnt_hint for FACK
2012-02-14 19:40 ` David Miller
@ 2012-02-14 19:41 ` David Miller
2012-02-27 23:39 ` Ilpo Järvinen
1 sibling, 0 replies; 10+ messages in thread
From: David Miller @ 2012-02-14 19:41 UTC (permalink / raw)
To: ncardwell
Cc: netdev, ilpo.jarvinen, nanditad, ycheng, therbert,
subramanian.vijay
From: David Miller <davem@davemloft.net>
Date: Tue, 14 Feb 2012 14:40:33 -0500 (EST)
> Applied, and queued up for -stable, thanks Neil.
Sorry for the typo, I truly meant to say "Neal" :-)
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] tcp: fix tcp_shifted_skb() adjustment of lost_cnt_hint for FACK
2012-02-14 19:40 ` David Miller
2012-02-14 19:41 ` David Miller
@ 2012-02-27 23:39 ` Ilpo Järvinen
2012-02-27 23:43 ` Ilpo Järvinen
1 sibling, 1 reply; 10+ messages in thread
From: Ilpo Järvinen @ 2012-02-27 23:39 UTC (permalink / raw)
To: David Miller, ncardwell, Greg KH
Cc: Netdev, nanditad, ycheng, therbert, subramanian.vijay
On Tue, 14 Feb 2012, David Miller wrote:
> From: Neal Cardwell <ncardwell@google.com>
> Date: Tue, 14 Feb 2012 01:22:08 -0500
>
> > This commit ensures that lost_cnt_hint is correctly updated in
> > tcp_shifted_skb() for FACK TCP senders. The lost_cnt_hint adjustment
> > in tcp_sacktag_one() only applies to non-FACK senders, so FACK senders
> > need their own adjustment.
> >
> > This applies the spirit of 1e5289e121372a3494402b1b131b41bfe1cf9b7f -
> > except now that the sequence range passed into tcp_sacktag_one() is
> > correct we need only have a special case adjustment for FACK.
> >
> > Signed-off-by: Neal Cardwell <ncardwell@google.com>
>
> Applied, and queued up for -stable, thanks Neil.
There has been lots of TCP related warnings posted recently to netdev.
Finding out which of these three recent changes caused which warning and
if they're all cured by the full series is bit challenging. ...IMHO we
should wait a bit more until these recent fixes are put to stable in order
to verify that the dust has really settled.
--
i.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] tcp: fix tcp_shifted_skb() adjustment of lost_cnt_hint for FACK
2012-02-27 23:39 ` Ilpo Järvinen
@ 2012-02-27 23:43 ` Ilpo Järvinen
2012-02-28 2:40 ` Neal Cardwell
0 siblings, 1 reply; 10+ messages in thread
From: Ilpo Järvinen @ 2012-02-27 23:43 UTC (permalink / raw)
To: Greg KH
Cc: David Miller, ncardwell, Netdev, nanditad, ycheng, therbert,
subramanian.vijay
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1239 bytes --]
...Grr, I picked up Greg's new address but ended still using his old one.
The correct is one now added.
On Tue, 28 Feb 2012, Ilpo Järvinen wrote:
> On Tue, 14 Feb 2012, David Miller wrote:
>
> > From: Neal Cardwell <ncardwell@google.com>
> > Date: Tue, 14 Feb 2012 01:22:08 -0500
> >
> > > This commit ensures that lost_cnt_hint is correctly updated in
> > > tcp_shifted_skb() for FACK TCP senders. The lost_cnt_hint adjustment
> > > in tcp_sacktag_one() only applies to non-FACK senders, so FACK senders
> > > need their own adjustment.
> > >
> > > This applies the spirit of 1e5289e121372a3494402b1b131b41bfe1cf9b7f -
> > > except now that the sequence range passed into tcp_sacktag_one() is
> > > correct we need only have a special case adjustment for FACK.
> > >
> > > Signed-off-by: Neal Cardwell <ncardwell@google.com>
> >
> > Applied, and queued up for -stable, thanks Neil.
>
> There has been lots of TCP related warnings posted recently to netdev.
> Finding out which of these three recent changes caused which warning and
> if they're all cured by the full series is bit challenging. ...IMHO we
> should wait a bit more until these recent fixes are put to stable in order
> to verify that the dust has really settled.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] tcp: fix tcp_shifted_skb() adjustment of lost_cnt_hint for FACK
2012-02-27 23:43 ` Ilpo Järvinen
@ 2012-02-28 2:40 ` Neal Cardwell
2012-02-28 2:58 ` David Miller
0 siblings, 1 reply; 10+ messages in thread
From: Neal Cardwell @ 2012-02-28 2:40 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: Greg KH, David Miller, Netdev, nanditad, ycheng, therbert,
subramanian.vijay
Yes, I agree that it would make sense to hold off on adding these
three 3.3-rc4 commits to the stable tree:
cc9a672ee522d4805495b98680f4a3db5d0a0af9
daef52bab1fd26e24e8e9578f8fb33ba1d0cb412
0af2a0d0576205dda778d25c6c344fc6508fc81d
It would make sense to let the dust settle a bit more before adding
those to the stable tree.
The same goes for the http://patchwork.ozlabs.org/patch/143114/ patch
(under review) that is in the same area of the code.
neal
On Mon, Feb 27, 2012 at 6:43 PM, Ilpo Järvinen
<ilpo.jarvinen@helsinki.fi> wrote:
> ...Grr, I picked up Greg's new address but ended still using his old one.
> The correct is one now added.
>
> On Tue, 28 Feb 2012, Ilpo Järvinen wrote:
>
>> On Tue, 14 Feb 2012, David Miller wrote:
>>
>> > From: Neal Cardwell <ncardwell@google.com>
>> > Date: Tue, 14 Feb 2012 01:22:08 -0500
>> >
>> > > This commit ensures that lost_cnt_hint is correctly updated in
>> > > tcp_shifted_skb() for FACK TCP senders. The lost_cnt_hint adjustment
>> > > in tcp_sacktag_one() only applies to non-FACK senders, so FACK senders
>> > > need their own adjustment.
>> > >
>> > > This applies the spirit of 1e5289e121372a3494402b1b131b41bfe1cf9b7f -
>> > > except now that the sequence range passed into tcp_sacktag_one() is
>> > > correct we need only have a special case adjustment for FACK.
>> > >
>> > > Signed-off-by: Neal Cardwell <ncardwell@google.com>
>> >
>> > Applied, and queued up for -stable, thanks Neil.
>>
>> There has been lots of TCP related warnings posted recently to netdev.
>> Finding out which of these three recent changes caused which warning and
>> if they're all cured by the full series is bit challenging. ...IMHO we
>> should wait a bit more until these recent fixes are put to stable in order
>> to verify that the dust has really settled.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] tcp: fix tcp_shifted_skb() adjustment of lost_cnt_hint for FACK
2012-02-28 2:40 ` Neal Cardwell
@ 2012-02-28 2:58 ` David Miller
2012-02-28 3:45 ` Neal Cardwell
0 siblings, 1 reply; 10+ messages in thread
From: David Miller @ 2012-02-28 2:58 UTC (permalink / raw)
To: ncardwell
Cc: ilpo.jarvinen, gregkh, netdev, nanditad, ycheng, therbert,
subramanian.vijay
From: Neal Cardwell <ncardwell@google.com>
Date: Mon, 27 Feb 2012 21:40:03 -0500
> Yes, I agree that it would make sense to hold off on adding these
> three 3.3-rc4 commits to the stable tree:
>
> cc9a672ee522d4805495b98680f4a3db5d0a0af9
>
> daef52bab1fd26e24e8e9578f8fb33ba1d0cb412
>
> 0af2a0d0576205dda778d25c6c344fc6508fc81d
>
> It would make sense to let the dust settle a bit more before adding
> those to the stable tree.
>
> The same goes for the http://patchwork.ozlabs.org/patch/143114/ patch
> (under review) that is in the same area of the code.
I totally disagree and I have submitted those fixes to -stable
already and do not plan to rescind those submissions at all.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] tcp: fix tcp_shifted_skb() adjustment of lost_cnt_hint for FACK
2012-02-28 2:58 ` David Miller
@ 2012-02-28 3:45 ` Neal Cardwell
2012-02-28 4:31 ` David Miller
0 siblings, 1 reply; 10+ messages in thread
From: Neal Cardwell @ 2012-02-28 3:45 UTC (permalink / raw)
To: David Miller
Cc: ilpo.jarvinen, gregkh, netdev, nanditad, ycheng, therbert,
subramanian.vijay
On Mon, Feb 27, 2012 at 9:58 PM, David Miller <davem@davemloft.net> wrote:
> From: Neal Cardwell <ncardwell@google.com>
> Date: Mon, 27 Feb 2012 21:40:03 -0500
>
>> Yes, I agree that it would make sense to hold off on adding these
>> three 3.3-rc4 commits to the stable tree:
>>
>> cc9a672ee522d4805495b98680f4a3db5d0a0af9
>>
>> daef52bab1fd26e24e8e9578f8fb33ba1d0cb412
>>
>> 0af2a0d0576205dda778d25c6c344fc6508fc81d
>>
>> It would make sense to let the dust settle a bit more before adding
>> those to the stable tree.
>>
>> The same goes for the http://patchwork.ozlabs.org/patch/143114/ patch
>> (under review) that is in the same area of the code.
>
> I totally disagree and I have submitted those fixes to -stable
> already and do not plan to rescind those submissions at all.
OK, sounds good to me. In that case I think it would be good to have
the http://patchwork.ozlabs.org/patch/143114/ patch in -stable as
well, since having just the three patches above would leave us with a
known reordering issue.
neal
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] tcp: fix tcp_shifted_skb() adjustment of lost_cnt_hint for FACK
2012-02-28 3:45 ` Neal Cardwell
@ 2012-02-28 4:31 ` David Miller
2012-02-28 4:42 ` Neal Cardwell
0 siblings, 1 reply; 10+ messages in thread
From: David Miller @ 2012-02-28 4:31 UTC (permalink / raw)
To: ncardwell
Cc: ilpo.jarvinen, gregkh, netdev, nanditad, ycheng, therbert,
subramanian.vijay
From: Neal Cardwell <ncardwell@google.com>
Date: Mon, 27 Feb 2012 22:45:31 -0500
> On Mon, Feb 27, 2012 at 9:58 PM, David Miller <davem@davemloft.net> wrote:
>> From: Neal Cardwell <ncardwell@google.com>
>> Date: Mon, 27 Feb 2012 21:40:03 -0500
>>
>>> Yes, I agree that it would make sense to hold off on adding these
>>> three 3.3-rc4 commits to the stable tree:
>>>
>>> cc9a672ee522d4805495b98680f4a3db5d0a0af9
>>>
>>> daef52bab1fd26e24e8e9578f8fb33ba1d0cb412
>>>
>>> 0af2a0d0576205dda778d25c6c344fc6508fc81d
>>>
>>> It would make sense to let the dust settle a bit more before adding
>>> those to the stable tree.
>>>
>>> The same goes for the http://patchwork.ozlabs.org/patch/143114/ patch
>>> (under review) that is in the same area of the code.
>>
>> I totally disagree and I have submitted those fixes to -stable
>> already and do not plan to rescind those submissions at all.
>
> OK, sounds good to me. In that case I think it would be good to have
> the http://patchwork.ozlabs.org/patch/143114/ patch in -stable as
> well, since having just the three patches above would leave us with a
> known reordering issue.
I'll make sure it hits the -stable release after the one Greg is
trying to put out right now.
The change you reference rarely matters as most of the time it passes
a degreee of reordering of one.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] tcp: fix tcp_shifted_skb() adjustment of lost_cnt_hint for FACK
2012-02-28 4:31 ` David Miller
@ 2012-02-28 4:42 ` Neal Cardwell
0 siblings, 0 replies; 10+ messages in thread
From: Neal Cardwell @ 2012-02-28 4:42 UTC (permalink / raw)
To: David Miller
Cc: ilpo.jarvinen, gregkh, netdev, nanditad, ycheng, therbert,
subramanian.vijay
On Mon, Feb 27, 2012 at 11:31 PM, David Miller <davem@davemloft.net> wrote:
>> OK, sounds good to me. In that case I think it would be good to have
>> the http://patchwork.ozlabs.org/patch/143114/ patch in -stable as
>> well, since having just the three patches above would leave us with a
>> known reordering issue.
>
> I'll make sure it hits the -stable release after the one Greg is
> trying to put out right now.
Great. Thanks!
> The change you reference rarely matters as most of the time it passes
> a degreee of reordering of one.
Yep.
neal
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2012-02-28 4:42 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-02-14 6:22 [PATCH] tcp: fix tcp_shifted_skb() adjustment of lost_cnt_hint for FACK Neal Cardwell
2012-02-14 19:40 ` David Miller
2012-02-14 19:41 ` David Miller
2012-02-27 23:39 ` Ilpo Järvinen
2012-02-27 23:43 ` Ilpo Järvinen
2012-02-28 2:40 ` Neal Cardwell
2012-02-28 2:58 ` David Miller
2012-02-28 3:45 ` Neal Cardwell
2012-02-28 4:31 ` David Miller
2012-02-28 4:42 ` Neal Cardwell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).