Skip to contentSkip to conversation

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

PATCH In review Li Yu 4 participants 5 messages Mar 28, 2012

Subsystems (3)
lore
Share & tools
lore

Li Yu

author
#0 lore raw

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

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 <[email protected]>

net/ipv4/tcp_input.c +0 -0
index e886e2f..b2f8ada 100644
@@ -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)
tp->undo_marker = 0;

#1 lore raw
Quoted 15 lines · Li Yu

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 <[email protected]>

net/ipv4/tcp_input.c +0 -0
index e886e2f..b2f8ada 100644

net/ipv4/tcp_input.c static void tcp_enter_frto_loss(struct sock *sk, · line 2174

Quoted 6 lines
net/ipv4/tcp_input.c +0 -2
@@ -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.

Quoted 7 lines
net/ipv4/tcp_input.c +0 -0
                } 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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

maintainer
#2 lore raw
In reply to Li Yu
Quoted 4 lines · Li Yu

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

author
#3 lore raw
Quoted 20 lines · Neal Cardwell
Quoted 4 lines · Li Yu

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 {

Quoted 5 lines

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

Quoted 2 lines

neal

#4 lore raw
Quoted 2 lines · Li Yu

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