diff mbox

tcp: remove useless update for flag parameter in tcp_enter_frto_loss()

Message ID 4F729235.8040706@gmail.com
State Rejected, archived
Delegated to: David Miller
Headers show

Commit Message

Li Yu March 28, 2012, 4:23 a.m. UTC
It seem that we forget remove below two lines after copying code :)

This update never impacts others.

Sorry for last mail with legal notices improperly, I used wrong
mail.

Signed-off-by Li Yu <bingtian.ly@taobao.com>

 				tp->undo_marker = 0;
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Yuchung Cheng March 28, 2012, 5:27 a.m. UTC | #1
On Tue, Mar 27, 2012 at 9:23 PM, Li Yu <raise.sail@gmail.com> wrote:
>
> It seem that we forget remove below two lines after copying code :)
>
> This update never impacts others.
>
> Sorry for last mail with legal notices improperly, I used wrong
> mail.
>
> Signed-off-by Li Yu <bingtian.ly@taobao.com>
>
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index e886e2f..b2f8ada 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -2169,8 +2169,6 @@ static void tcp_enter_frto_loss(struct sock *sk,
> int allowed_segments, int flag)
>                        /* For some reason this R-bit might get cleared? */
>                        if (TCP_SKB_CB(skb)->sacked & TCPCB_SACKED_RETRANS)
>                                tp->retrans_out += tcp_skb_pcount(skb);
> -                       /* ...enter this if branch just for the first segment */
> -                       flag |= FLAG_DATA_ACKED;

The comment explicitly explains why the flag is marked for.


>                } else {
>                        if (TCP_SKB_CB(skb)->sacked & TCPCB_RETRANS)
>                                tp->undo_marker = 0;
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Neal Cardwell March 28, 2012, 5:29 a.m. UTC | #2
On Wed, Mar 28, 2012 at 12:23 AM, Li Yu <raise.sail@gmail.com> wrote:
>
> It seem that we forget remove below two lines after copying code :)
>
> This update never impacts others.

The 'flag' variable will be used in later iterations of the
tcp_for_write_queue() loop. The comment seems to quite sensibly
indicate that the FLAG_DATA_ACKED bit is being set to avoid entering
that branch of the if statement in later segments in the loop:

2168                 if ((tp->frto_counter == 1) && !(flag & FLAG_DATA_ACKED)) {
2169                         /* For some reason this R-bit might get cleared? */
2170                         if (TCP_SKB_CB(skb)->sacked & TCPCB_SACKED_RETRANS)
2171                                 tp->retrans_out += tcp_skb_pcount(skb);
2172                         /* ...enter this if branch just for the
first segment */
2173                         flag |= FLAG_DATA_ACKED;
2174                 } else {

The structure of the logic seems intact from the original commit that
added this:

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=d1a54c6a0a3f9c2c4ef71982d89b8571bd9eaa51

neal
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Li Yu March 28, 2012, 6:54 a.m. UTC | #3
于 2012年03月28日 13:29, Neal Cardwell 写道:
> On Wed, Mar 28, 2012 at 12:23 AM, Li Yu<raise.sail@gmail.com>  wrote:
>>
>> It seem that we forget remove below two lines after copying code :)
>>
>> This update never impacts others.
>
> The 'flag' variable will be used in later iterations of the
> tcp_for_write_queue() loop. The comment seems to quite sensibly
> indicate that the FLAG_DATA_ACKED bit is being set to avoid entering
> that branch of the if statement in later segments in the loop:
>
> 2168                 if ((tp->frto_counter == 1)&&  !(flag&  FLAG_DATA_ACKED)) {
> 2169                         /* For some reason this R-bit might get cleared? */
> 2170                         if (TCP_SKB_CB(skb)->sacked&  TCPCB_SACKED_RETRANS)
> 2171                                 tp->retrans_out += tcp_skb_pcount(skb);
> 2172                         /* ...enter this if branch just for the
> first segment */
> 2173                         flag |= FLAG_DATA_ACKED;
> 2174                 } else {
>
> The structure of the logic seems intact from the original commit that
> added this:
>
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=d1a54c6a0a3f9c2c4ef71982d89b8571bd9eaa51
>

Sorry for spent time on this, it it account for F-RTO retransmitted
segment here.

Yu

> neal
>

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Dumazet March 28, 2012, 7:34 a.m. UTC | #4
On Wed, 2012-03-28 at 14:54 +0800, Li Yu wrote:

> Sorry for spent time on this, it it account for F-RTO retransmitted
> segment here.

I dont believe its waste of time having several eyes on this code.

Thanks


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index e886e2f..b2f8ada 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -2169,8 +2169,6 @@  static void tcp_enter_frto_loss(struct sock *sk,
int allowed_segments, int flag)
 			/* For some reason this R-bit might get cleared? */
 			if (TCP_SKB_CB(skb)->sacked & TCPCB_SACKED_RETRANS)
 				tp->retrans_out += tcp_skb_pcount(skb);
-			/* ...enter this if branch just for the first segment */
-			flag |= FLAG_DATA_ACKED;
 		} else {
 			if (TCP_SKB_CB(skb)->sacked & TCPCB_RETRANS)