Skip to content

Commit

Permalink
net: tcp: Fix ref counting for the net_pkt
Browse files Browse the repository at this point in the history
The network packet ref count was not properly increased when
the TCP was retried. This meant that the second time the packet
was sent, the device driver managed to release the TCP frame even
if we had not got ACK to it.

Somewhat long debug log follows:

The net_pkt 0x08072d5c is created, we write 1K data into it, initial ref
count is 1.

net_pkt_write: pkt 0x08072d5c data 0x08075d40 length 1024
net_tcp_queue_data: Queue 0x08072d5c len 1024
net_tcp_trace: pkt 0x08072d5c src 5001 dst 5001
net_tcp_trace:    seq 0x15d2aa09 (366127625) ack 0x7f67d918
net_tcp_trace:    flags uAPrsf
net_tcp_trace:    win 1280 chk 0x0bea
net_tcp_queue_pkt: pkt 0x08072d5c new ref 2 (net_tcp_queue_pkt:850)

At this point, the ref is 2. Then the packet is sent as you see below.

net_pkt_ref_debug: TX [13] pkt 0x08072d5c ref 2 net_tcp_queue_pkt():850
net_tcp_send_data: Sending pkt 0x08072d5c (1084 bytes)
net_pkt_unref_debug: TX [13] pkt 0x08072d5c ref 1 (ethernet_send():597)

Ref is still correct, packet is still alive. We have not received ACK,
so the packet is resent.

tcp_retry_expired: ref pkt 0x08072d5c new ref 2 (tcp_retry_expired:233)
net_pkt_ref_debug: TX [10] pkt 0x08072d5c ref 2 tcp_retry_expired():233
net_pkt_unref_debug: TX [10] pkt 0x08072d5c ref 1 ... (net_if_tx():173)
net_pkt_unref_debug: TX [10] pkt 0x08072d5c ref 0 ... (net_if_tx():173)

Reference count is now wrong, it should have been 1. This is because we
did not increase the ref count when packet was placed first time into
sent list in tcp.c:tcp_retry_expired().

The fix is quite simple as you can see from this commit.

Signed-off-by: Jukka Rissanen <[email protected]>
  • Loading branch information
jukkar authored and galak committed Mar 26, 2019
1 parent 25d0404 commit 9cd547f
Showing 1 changed file with 11 additions and 4 deletions.
15 changes: 11 additions & 4 deletions subsys/net/ip/tcp.c
Original file line number Diff line number Diff line change
Expand Up @@ -229,10 +229,17 @@ static void tcp_retry_expired(struct k_work *work)
pkt = CONTAINER_OF(sys_slist_peek_head(&tcp->sent_list),
struct net_pkt, sent_list);

if (net_pkt_sent(pkt)) {
do_ref_if_needed(tcp, pkt);
net_pkt_set_sent(pkt, false);
}
/* In the retry case, the original ref (when the packet
* was created) is set to 1. That original ref was
* decremented when the packet was sent by the driver.
* We need to restore that original ref so that the
* device driver will not remove the retry packet that
* we just sent. Earlier we also checked net_pkt_sent(pkt)
* here but that is not correct as then the packet that was
* sent first time, was removed by the driver and we got
* access to memory already freed.
*/
do_ref_if_needed(tcp, pkt);

net_pkt_set_queued(pkt, true);

Expand Down

0 comments on commit 9cd547f

Please sign in to comment.