From ade160c8a293f10d088f2cdab725bd1ddbe5725c Mon Sep 17 00:00:00 2001 From: Paul Sokolovsky Date: Thu, 23 Aug 2018 14:41:56 +0300 Subject: [PATCH] net: tcp: Properly queue FIN packets for retransmission In TCP protocol, any packet is subject to retransmission if not ACKed in expected time. Thus, any packet, including FIN (and SYN for that matter) should be added to the retransmission queue. In our case, despite its name, queue_fin() function didn't add FIN packet to rexmit queue, so do that. Then, in net_tcp_ack_received() which handles ACKs, make sure that we can handle FIN packets: calculate its sequence number properly, don't make adhoc adjustments to retransmission logic (it's handled centrally in restart_timer() already), etc. Fixes: #8188 Signed-off-by: Paul Sokolovsky --- subsys/net/ip/tcp.c | 46 +++++++++++++++++++++++++++++++++++++-------- 1 file changed, 38 insertions(+), 8 deletions(-) diff --git a/subsys/net/ip/tcp.c b/subsys/net/ip/tcp.c index f6189f5dde027..45a0de1287786 100644 --- a/subsys/net/ip/tcp.c +++ b/subsys/net/ip/tcp.c @@ -39,6 +39,8 @@ #define ALLOC_TIMEOUT K_MSEC(500) +static int net_tcp_queue_pkt(struct net_context *context, struct net_pkt *pkt); + /* * Each TCP connection needs to be tracked by net_context, so * we need to allocate equal number of control structures here. @@ -205,7 +207,7 @@ static void tcp_retry_expired(struct k_work *work) struct net_tcp *tcp = CONTAINER_OF(work, struct net_tcp, retry_timer); struct net_pkt *pkt; - /* Double the retry period for exponential backoff and resent + /* Double the retry period for exponential backoff and resend * the first (only the first!) unack'd packet. */ if (!sys_slist_is_empty(&tcp->sent_list)) { @@ -826,6 +828,14 @@ int net_tcp_queue_data(struct net_context *context, struct net_pkt *pkt) net_stats_update_tcp_sent(net_pkt_iface(pkt), data_len); + return net_tcp_queue_pkt(context, pkt); +} + +/* This function is the sole point of *adding* packets to tcp->sent_list, + * and should remain such. + */ +static int net_tcp_queue_pkt(struct net_context *context, struct net_pkt *pkt) +{ sys_slist_append(&context->tcp->sent_list, &pkt->sent_list); /* We need to restart retry_timer if it is stopped. */ @@ -1005,7 +1015,6 @@ bool net_tcp_ack_received(struct net_context *ctx, u32_t ack) sys_slist_t *list = &ctx->tcp->sent_list; sys_snode_t *head; struct net_pkt *pkt; - u32_t seq; bool valid_ack = false; if (net_tcp_seq_greater(ack, ctx->tcp->send_seq)) { @@ -1020,6 +1029,8 @@ bool net_tcp_ack_received(struct net_context *ctx, u32_t ack) while (!sys_slist_is_empty(list)) { struct net_tcp_hdr hdr, *tcp_hdr; + u32_t last_seq; + u32_t seq_len; head = sys_slist_peek_head(list); pkt = CONTAINER_OF(head, struct net_pkt, sent_list); @@ -1035,9 +1046,26 @@ bool net_tcp_ack_received(struct net_context *ctx, u32_t ack) continue; } - seq = sys_get_be32(tcp_hdr->seq) + net_pkt_appdatalen(pkt) - 1; + seq_len = net_pkt_appdatalen(pkt); + + /* Each of SYN and FIN flags are counted + * as one sequence number. + */ + if (tcp_hdr->flags & NET_TCP_SYN) { + seq_len += 1; + } + if (tcp_hdr->flags & NET_TCP_FIN) { + seq_len += 1; + } + + /* Last sequence number in this packet. */ + last_seq = sys_get_be32(tcp_hdr->seq) + seq_len - 1; - if (!net_tcp_seq_greater(ack, seq)) { + /* Ack number should be strictly greater to acknowleged numbers + * below it. For example, ack no. 10 acknowledges all numbers up + * to and including 9. + */ + if (!net_tcp_seq_greater(ack, last_seq)) { break; } @@ -1056,13 +1084,13 @@ bool net_tcp_ack_received(struct net_context *ctx, u32_t ack) valid_ack = true; } - /* Restart the timer on a valid inbound ACK. This isn't quite the - * same behavior as per-packet retry timers, but is close in practice - * (it starts retries one timer period after the connection + /* Restart the timer (if needed) on a valid inbound ACK. This isn't + * quite the same behavior as per-packet retry timers, but is close in + * practice (it starts retries one timer period after the connection * "got stuck") and avoids the need to track per-packet timers or * sent times. */ - if (valid_ack && net_tcp_get_state(tcp) == NET_TCP_ESTABLISHED) { + if (valid_ack) { restart_timer(ctx->tcp); } @@ -1452,6 +1480,8 @@ static void queue_fin(struct net_context *ctx) return; } + net_tcp_queue_pkt(ctx, pkt); + ret = net_tcp_send_pkt(pkt); if (ret < 0) { net_pkt_unref(pkt);