Skip to content

Commit 99c5ec2

Browse files
nealcardwellAshok Vairavan
authored andcommitted
tcp: fix tcp_mark_head_lost to check skb len before fragmenting
This commit fixes a corner case in tcp_mark_head_lost() which was causing the WARN_ON(len > skb->len) in tcp_fragment() to fire. tcp_mark_head_lost() was assuming that if a packet has tcp_skb_pcount(skb) of N, then it's safe to fragment off a prefix of M*mss bytes, for any M < N. But with the tricky way TCP pcounts are maintained, this is not always true. For example, suppose the sender sends 4 1-byte packets and have the last 3 packet sacked. It will merge the last 3 packets in the write queue into an skb with pcount = 3 and len = 3 bytes. If another recovery happens after a sack reneging event, tcp_mark_head_lost() may attempt to split the skb assuming it has more than 2*MSS bytes. This sounds very counterintuitive, but as the commit description for the related commit c0638c2 ("tcp: don't fragment SACKed skbs in tcp_mark_head_lost()") notes, this is because tcp_shifted_skb() coalesces adjacent regions of SACKed skbs, and when doing this it preserves the sum of their packet counts in order to reflect the real-world dynamics on the wire. The c0638c2 commit tried to avoid problems by not fragmenting SACKed skbs, since SACKed skbs are where the non-proportionality between pcount and skb->len/mss is known to be possible. However, that commit did not handle the case where during a reneging event one of these weird SACKed skbs becomes an un-SACKed skb, which tcp_mark_head_lost() can then try to fragment. The fix is to simply mark the entire skb lost when this happens. This makes the recovery slightly more aggressive in such corner cases before we detect reordering. But once we detect reordering this code path is by-passed because FACK is disabled. Signed-off-by: Neal Cardwell <[email protected]> Signed-off-by: Yuchung Cheng <[email protected]> Signed-off-by: Eric Dumazet <[email protected]> Signed-off-by: David S. Miller <[email protected]> (cherry picked from commit d88270e) Orabug: 26646104 Conflicts: tcp_skb_mss is not used in UEK4. Hence, skb_shinfo() is used to get the mss size. Signed-off-by: Ashok Vairavan <[email protected]> Reviewed-by: Jack Vogel <[email protected]>
1 parent 4dfcf8a commit 99c5ec2

File tree

1 file changed

+5
-5
lines changed

1 file changed

+5
-5
lines changed

net/ipv4/tcp_input.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2216,8 +2216,7 @@ static void tcp_mark_head_lost(struct sock *sk, int packets, int mark_head)
22162216
{
22172217
struct tcp_sock *tp = tcp_sk(sk);
22182218
struct sk_buff *skb;
2219-
int cnt, oldcnt;
2220-
int err;
2219+
int cnt, oldcnt, lost;
22212220
unsigned int mss;
22222221
/* Use SACK to deduce losses of new sequences sent during recovery */
22232222
const u32 loss_high = tcp_is_sack(tp) ? tp->snd_nxt : tp->high_seq;
@@ -2257,9 +2256,10 @@ static void tcp_mark_head_lost(struct sock *sk, int packets, int mark_head)
22572256
break;
22582257

22592258
mss = skb_shinfo(skb)->gso_size;
2260-
err = tcp_fragment(sk, skb, (packets - oldcnt) * mss,
2261-
mss, GFP_ATOMIC);
2262-
if (err < 0)
2259+
/* If needed, chop off the prefix to mark as lost. */
2260+
lost = (packets - oldcnt) * mss;
2261+
if (lost < skb->len &&
2262+
tcp_fragment(sk, skb, lost, mss, GFP_ATOMIC) < 0)
22632263
break;
22642264
cnt = packets;
22652265
}

0 commit comments

Comments
 (0)