* [PATCH] net: tcp: add correct check for tcp_retransmit_skb()
@ 2018-11-26 9:35 Sharath Chandra Vurukala
2018-11-26 10:16 ` Joey Pabalinas
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Sharath Chandra Vurukala @ 2018-11-26 9:35 UTC (permalink / raw)
To: netdev; +Cc: kapandey, chinagar
when the tcp_retranmission_timer expires and tcp_retranmsit_skb is
called if the retranmsission fails due to local congestion,
backoff should not incremented.
tcp_retransmit_skb() returns non-zero negative value in some cases of
failure but the caller tcp_retransmission_timer() has a check for
failure which checks if the return value is greater than zero.
The check is corrected to check for non-zero value.
Change-Id: I494fed73b2e385216402c91e9558d5c2884add5b
Signed-off-by: Sharath Chandra Vurukala <sharathv@codeaurora.org>
---
net/ipv4/tcp_timer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index 4be66e4..a70b4a9 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -536,7 +536,7 @@ void tcp_retransmit_timer(struct sock *sk)
tcp_enter_loss(sk);
- if (tcp_retransmit_skb(sk, tcp_write_queue_head(sk), 1) > 0) {
+ if (tcp_retransmit_skb(sk, tcp_write_queue_head(sk), 1) != 0) {
/* Retransmission failed because of local congestion,
* do not backoff.
*/
--
1.9.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] net: tcp: add correct check for tcp_retransmit_skb()
2018-11-26 9:35 [PATCH] net: tcp: add correct check for tcp_retransmit_skb() Sharath Chandra Vurukala
@ 2018-11-26 10:16 ` Joey Pabalinas
2018-11-28 0:15 ` David Miller
2018-11-28 0:29 ` Yuchung Cheng
2 siblings, 0 replies; 7+ messages in thread
From: Joey Pabalinas @ 2018-11-26 10:16 UTC (permalink / raw)
To: Sharath Chandra Vurukala; +Cc: netdev, kapandey, chinagar, Joey Pabalinas
[-- Attachment #1: Type: text/plain, Size: 842 bytes --]
On Mon, Nov 26, 2018 at 03:05:50PM +0530, Sharath Chandra Vurukala wrote:
> when the tcp_retranmission_timer expires and tcp_retranmsit_skb is
> called if the retranmsission fails due to local congestion,
> backoff should not incremented.
>
> tcp_retransmit_skb() returns non-zero negative value in some cases of
> failure but the caller tcp_retransmission_timer() has a check for
> failure which checks if the return value is greater than zero.
> The check is corrected to check for non-zero value.
>
> Change-Id: I494fed73b2e385216402c91e9558d5c2884add5b
> Signed-off-by: Sharath Chandra Vurukala <sharathv@codeaurora.org>
You are right, tcp_retransmit_skb() calls `int err = __tcp_retransmit_skb()`
which returns non-zero on error.
Reviewed-by: Joey Pabalinas <joeypabalinas@gmail.com>
--
Cheers,
Joey Pabalinas
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] net: tcp: add correct check for tcp_retransmit_skb()
2018-11-26 9:35 [PATCH] net: tcp: add correct check for tcp_retransmit_skb() Sharath Chandra Vurukala
2018-11-26 10:16 ` Joey Pabalinas
@ 2018-11-28 0:15 ` David Miller
2018-11-28 0:29 ` Yuchung Cheng
2 siblings, 0 replies; 7+ messages in thread
From: David Miller @ 2018-11-28 0:15 UTC (permalink / raw)
To: sharathv; +Cc: netdev, kapandey, chinagar
From: Sharath Chandra Vurukala <sharathv@codeaurora.org>
Date: Mon, 26 Nov 2018 15:05:50 +0530
> when the tcp_retranmission_timer expires and tcp_retranmsit_skb is
> called if the retranmsission fails due to local congestion,
> backoff should not incremented.
>
> tcp_retransmit_skb() returns non-zero negative value in some cases of
> failure but the caller tcp_retransmission_timer() has a check for
> failure which checks if the return value is greater than zero.
> The check is corrected to check for non-zero value.
>
> Change-Id: I494fed73b2e385216402c91e9558d5c2884add5b
No changeids please.
> Signed-off-by: Sharath Chandra Vurukala <sharathv@codeaurora.org>
This patch doesn't apply cleanly to my net tree.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] net: tcp: add correct check for tcp_retransmit_skb()
2018-11-26 9:35 [PATCH] net: tcp: add correct check for tcp_retransmit_skb() Sharath Chandra Vurukala
2018-11-26 10:16 ` Joey Pabalinas
2018-11-28 0:15 ` David Miller
@ 2018-11-28 0:29 ` Yuchung Cheng
2 siblings, 0 replies; 7+ messages in thread
From: Yuchung Cheng @ 2018-11-28 0:29 UTC (permalink / raw)
To: Sharath Chandra Vurukala
Cc: netdev, kapandey, chinagar, Neal Cardwell, Eric Dumazet
On Mon, Nov 26, 2018 at 1:35 AM, Sharath Chandra Vurukala
<sharathv@codeaurora.org> wrote:
> when the tcp_retranmission_timer expires and tcp_retranmsit_skb is
> called if the retranmsission fails due to local congestion,
> backoff should not incremented.
>
> tcp_retransmit_skb() returns non-zero negative value in some cases of
> failure but the caller tcp_retransmission_timer() has a check for
> failure which checks if the return value is greater than zero.
> The check is corrected to check for non-zero value.
Not sure about this fix. The specific check is to handle local
congestion which is only indicated by positive return values.
>
> Change-Id: I494fed73b2e385216402c91e9558d5c2884add5b
> Signed-off-by: Sharath Chandra Vurukala <sharathv@codeaurora.org>
> ---
> net/ipv4/tcp_timer.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
> index 4be66e4..a70b4a9 100644
> --- a/net/ipv4/tcp_timer.c
> +++ b/net/ipv4/tcp_timer.c
> @@ -536,7 +536,7 @@ void tcp_retransmit_timer(struct sock *sk)
>
> tcp_enter_loss(sk);
>
> - if (tcp_retransmit_skb(sk, tcp_write_queue_head(sk), 1) > 0) {
> + if (tcp_retransmit_skb(sk, tcp_write_queue_head(sk), 1) != 0) {
> /* Retransmission failed because of local congestion,
> * do not backoff.
> */
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] net: tcp: add correct check for tcp_retransmit_skb()
2018-11-30 18:28 Sharath Chandra Vurukala
2018-11-30 18:40 ` Eric Dumazet
@ 2018-11-30 18:41 ` Yuchung Cheng
1 sibling, 0 replies; 7+ messages in thread
From: Yuchung Cheng @ 2018-11-30 18:41 UTC (permalink / raw)
To: Sharath Chandra Vurukala; +Cc: netdev, chinagar, kapandey, subashab
On Fri, Nov 30, 2018 at 10:28 AM Sharath Chandra Vurukala
<sharathv@codeaurora.org> wrote:
>
> when the tcp_retranmission_timer expires and tcp_retranmsit_skb is
> called if the retranmsission fails due to local congestion,
> backoff should not incremented.
>
> tcp_retransmit_skb() returns non-zero negative value in some cases of
> failure but the caller tcp_retransmission_timer() has a check for
> failure which checks if the return value is greater than zero.
> The check is corrected to check for non-zero value.
>
> Signed-off-by: Sharath Chandra Vurukala <sharathv@codeaurora.org>
Perhaps my previous comment was not clear: your bug-fix patch is incorrect.
On local congestion, tcp_retransmit_skb returns positive values
*only*. negative values do not indicate local congestion.
> ---
> net/ipv4/tcp_timer.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
> index 091c5392..c19f371 100644
> --- a/net/ipv4/tcp_timer.c
> +++ b/net/ipv4/tcp_timer.c
> @@ -511,7 +511,7 @@ void tcp_retransmit_timer(struct sock *sk)
>
> tcp_enter_loss(sk);
>
> - if (tcp_retransmit_skb(sk, tcp_rtx_queue_head(sk), 1) > 0) {
> + if (tcp_retransmit_skb(sk, tcp_rtx_queue_head(sk), 1) != 0) {
> /* Retransmission failed because of local congestion,
> * do not backoff.
> */
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] net: tcp: add correct check for tcp_retransmit_skb()
2018-11-30 18:28 Sharath Chandra Vurukala
@ 2018-11-30 18:40 ` Eric Dumazet
2018-11-30 18:41 ` Yuchung Cheng
1 sibling, 0 replies; 7+ messages in thread
From: Eric Dumazet @ 2018-11-30 18:40 UTC (permalink / raw)
To: Sharath Chandra Vurukala, netdev; +Cc: chinagar, kapandey, subashab
On 11/30/2018 10:28 AM, Sharath Chandra Vurukala wrote:
> when the tcp_retranmission_timer expires and tcp_retranmsit_skb is
> called if the retranmsission fails due to local congestion,
> backoff should not incremented.
>
> tcp_retransmit_skb() returns non-zero negative value in some cases of
> failure but the caller tcp_retransmission_timer() has a check for
> failure which checks if the return value is greater than zero.
> The check is corrected to check for non-zero value.
>
> Signed-off-by: Sharath Chandra Vurukala <sharathv@codeaurora.org>
>
This looks wrong.
Yuchung has cooked a patch series to really address issues, please wait for it
We are waiting for David Miller to merge a prior patch series into net tree, then
in net-next.
Thanks.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] net: tcp: add correct check for tcp_retransmit_skb()
@ 2018-11-30 18:28 Sharath Chandra Vurukala
2018-11-30 18:40 ` Eric Dumazet
2018-11-30 18:41 ` Yuchung Cheng
0 siblings, 2 replies; 7+ messages in thread
From: Sharath Chandra Vurukala @ 2018-11-30 18:28 UTC (permalink / raw)
To: netdev; +Cc: chinagar, kapandey, subashab
when the tcp_retranmission_timer expires and tcp_retranmsit_skb is
called if the retranmsission fails due to local congestion,
backoff should not incremented.
tcp_retransmit_skb() returns non-zero negative value in some cases of
failure but the caller tcp_retransmission_timer() has a check for
failure which checks if the return value is greater than zero.
The check is corrected to check for non-zero value.
Signed-off-by: Sharath Chandra Vurukala <sharathv@codeaurora.org>
---
net/ipv4/tcp_timer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index 091c5392..c19f371 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -511,7 +511,7 @@ void tcp_retransmit_timer(struct sock *sk)
tcp_enter_loss(sk);
- if (tcp_retransmit_skb(sk, tcp_rtx_queue_head(sk), 1) > 0) {
+ if (tcp_retransmit_skb(sk, tcp_rtx_queue_head(sk), 1) != 0) {
/* Retransmission failed because of local congestion,
* do not backoff.
*/
--
1.9.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-12-01 5:52 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-26 9:35 [PATCH] net: tcp: add correct check for tcp_retransmit_skb() Sharath Chandra Vurukala
2018-11-26 10:16 ` Joey Pabalinas
2018-11-28 0:15 ` David Miller
2018-11-28 0:29 ` Yuchung Cheng
2018-11-30 18:28 Sharath Chandra Vurukala
2018-11-30 18:40 ` Eric Dumazet
2018-11-30 18:41 ` Yuchung Cheng
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.