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

Author: pfalcon

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);