From 43729f1f9f572935c1e0cca38d8626c2ead1cd96 Mon Sep 17 00:00:00 2001 From: Ravi kumar Veeramally Date: Thu, 8 Feb 2018 11:35:53 +0200 Subject: [PATCH 1/4] net: context: Remove duplicate call k_delayed_work_cancel(&context->tcp->fin_timer) called twice immediately one after other. Signed-off-by: Ravi kumar Veeramally --- subsys/net/ip/net_context.c | 1 - 1 file changed, 1 deletion(-) diff --git a/subsys/net/ip/net_context.c b/subsys/net/ip/net_context.c index 3a3942c2d061b..b797f5b96796d 100644 --- a/subsys/net/ip/net_context.c +++ b/subsys/net/ip/net_context.c @@ -1203,7 +1203,6 @@ NET_CONN_CB(tcp_established) /* Received FIN on FIN_WAIT1, so cancel the timer */ k_delayed_work_cancel(&context->tcp->fin_timer); /* Active close: step to FIN_WAIT_2 */ - k_delayed_work_cancel(&context->tcp->fin_timer); net_tcp_change_state(context->tcp, NET_TCP_FIN_WAIT_2); } else if (net_tcp_get_state(context->tcp) == NET_TCP_LAST_ACK) { From 583b7c7fcf47837092c3d366fb404e74c03cd856 Mon Sep 17 00:00:00 2001 From: Ravi kumar Veeramally Date: Thu, 8 Feb 2018 11:40:50 +0200 Subject: [PATCH 2/4] net: context: Handle failure case of tcp backlog entry In case of failed to get source address from the net packet, release the assgined tcp backlog entry. Otherwise it will never be freed. Signed-off-by: Ravi kumar Veeramally --- subsys/net/ip/net_context.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/subsys/net/ip/net_context.c b/subsys/net/ip/net_context.c index b797f5b96796d..a1bfd3321ce79 100644 --- a/subsys/net/ip/net_context.c +++ b/subsys/net/ip/net_context.c @@ -199,6 +199,9 @@ static int tcp_backlog_syn(struct net_pkt *pkt, struct net_context *context, ret = net_pkt_get_src_addr(pkt, &tcp_backlog[empty_slot].remote, sizeof(tcp_backlog[empty_slot].remote)); if (ret < 0) { + /* Release the assigned empty slot */ + tcp_backlog[empty_slot].tcp = NULL; + return ret; } From 81645b5469e673a10e047fbebb9f65c34f89e4fe Mon Sep 17 00:00:00 2001 From: Ravi kumar Veeramally Date: Thu, 8 Feb 2018 12:17:47 +0200 Subject: [PATCH 3/4] net: tcp: Do not handle packet re-transmission in TCP ACK Zephyr doesn't have luxury to create re-transmit timer per packet. So it has different methods to handle packets in queue. But re-sending packets on valid ack messages causing issues. E.g. A TCP node sent two packets (packet-1, packet-2). Peer replied two ACKs (ACK-1 and ACK-2), and these two ACK's are at rx_thread queue. Now ACK-1 is handled and reference of packet-1 is freed from sent list. Then if condiftion (valid ACK and connection state is ESTABLISHED) notices that, sent list is not empty. Restart the timer, modify sent flag and resend packets in a list. Here packet-2 is sent again, even though ACK-2 is already received. Situation is worse if there are more packets in the list. So only start the re-transmit timer in-case queue is not empty. It allows rx_thread to handle all incoming packets (in this e.g ACKs). When the re-trasmit timer expires, it sends the packets which are left in queue. Signed-off-by: Ravi kumar Veeramally --- subsys/net/ip/tcp.c | 30 ++++++------------------------ 1 file changed, 6 insertions(+), 24 deletions(-) diff --git a/subsys/net/ip/tcp.c b/subsys/net/ip/tcp.c index cf2cdaeaeff74..03e69a5976809 100644 --- a/subsys/net/ip/tcp.c +++ b/subsys/net/ip/tcp.c @@ -961,32 +961,14 @@ bool net_tcp_ack_received(struct net_context *ctx, u32_t ack) valid_ack = true; } - /* No need to re-send stuff we are closing down */ + /* 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 + * "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) { - /* 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 "got stuck") - * and avoids the need to track per-packet timers or - * sent times. - */ restart_timer(ctx->tcp); - - /* And, if we had been retrying, mark all packets - * untransmitted and then resend them. The stalled - * pipe is uncorked again. - */ - if (ctx->tcp->flags & NET_TCP_RETRYING) { - SYS_SLIST_FOR_EACH_CONTAINER(&ctx->tcp->sent_list, pkt, - sent_list) { - if (net_pkt_sent(pkt)) { - do_ref_if_needed(ctx->tcp, pkt); - net_pkt_set_sent(pkt, false); - } - } - - net_tcp_send_data(ctx); - } } return true; From d3960d369a54a62b91f6dceaa6994c7217c46c9b Mon Sep 17 00:00:00 2001 From: Ravi kumar Veeramally Date: Thu, 8 Feb 2018 12:43:23 +0200 Subject: [PATCH 4/4] net: tcp: Provide local address in TCP reset message preparation If context is bound to IPv6 unspecified addresss and some port number, then unspecified address is passed in TCP reset packet message preparation. Eventually packet dropped at the peer. Signed-off-by: Ravi kumar Veeramally --- subsys/net/ip/net_context.c | 17 ++++++++++++----- subsys/net/ip/tcp.c | 35 ++++++++++++++++++++++++++++++++++- subsys/net/ip/tcp.h | 5 ++++- 3 files changed, 50 insertions(+), 7 deletions(-) diff --git a/subsys/net/ip/net_context.c b/subsys/net/ip/net_context.c index a1bfd3321ce79..997bbea8478a9 100644 --- a/subsys/net/ip/net_context.c +++ b/subsys/net/ip/net_context.c @@ -94,7 +94,8 @@ static enum net_verdict packet_received(struct net_conn *conn, static void set_appdata_values(struct net_pkt *pkt, enum net_ip_protocol proto); #if defined(CONFIG_NET_TCP) -static int send_reset(struct net_context *context, struct sockaddr *remote); +static int send_reset(struct net_context *context, struct sockaddr *local, + struct sockaddr *remote); static struct tcp_backlog_entry { struct net_tcp *tcp; @@ -112,7 +113,12 @@ static void backlog_ack_timeout(struct k_work *work) NET_DBG("Did not receive ACK in %dms", ACK_TIMEOUT); - send_reset(backlog->tcp->context, &backlog->remote); + /* TODO: If net_context is bound to unspecified IPv6 address + * and some port number, local address is not available. + * RST packet might be invalid. Cache local address + * and use it in RST message preparation. + */ + send_reset(backlog->tcp->context, NULL, &backlog->remote); memset(backlog, 0, sizeof(struct tcp_backlog_entry)); } @@ -1083,12 +1089,13 @@ static int send_ack(struct net_context *context, } static int send_reset(struct net_context *context, + struct sockaddr *local, struct sockaddr *remote) { struct net_pkt *pkt = NULL; int ret; - ret = net_tcp_prepare_reset(context->tcp, remote, &pkt); + ret = net_tcp_prepare_reset(context->tcp, local, remote, &pkt); if (ret || !pkt) { return ret; } @@ -1364,7 +1371,7 @@ NET_CONN_CB(tcp_synack_received) &context->conn_handler); if (ret < 0) { NET_DBG("Cannot register TCP handler (%d)", ret); - send_reset(context, &remote_addr); + send_reset(context, &local_addr, &remote_addr); return NET_DROP; } @@ -1847,7 +1854,7 @@ NET_CONN_CB(tcp_syn_rcvd) net_stats_update_tcp_seg_conndrop(); reset: - send_reset(tcp->context, &remote_addr); + send_reset(tcp->context, &local_addr, &remote_addr); return NET_DROP; } diff --git a/subsys/net/ip/tcp.c b/subsys/net/ip/tcp.c index 03e69a5976809..1f7c0417dfdcb 100644 --- a/subsys/net/ip/tcp.c +++ b/subsys/net/ip/tcp.c @@ -661,12 +661,37 @@ int net_tcp_prepare_ack(struct net_tcp *tcp, const struct sockaddr *remote, return -EINVAL; } +static inline void copy_sockaddr_to_sockaddr_ptr(struct net_tcp *tcp, + const struct sockaddr *local, + struct sockaddr_ptr *addr) +{ + memset(addr, 0, sizeof(struct sockaddr_ptr)); + +#if defined(CONFIG_NET_IPV4) + if (local->sa_family == AF_INET) { + net_sin_ptr(addr)->sin_family = AF_INET; + net_sin_ptr(addr)->sin_port = net_sin(local)->sin_port; + net_sin_ptr(addr)->sin_addr = &net_sin(local)->sin_addr; + } +#endif + +#if defined(CONFIG_NET_IPV6) + if (local->sa_family == AF_INET6) { + net_sin6_ptr(addr)->sin6_family = AF_INET6; + net_sin6_ptr(addr)->sin6_port = net_sin6(local)->sin6_port; + net_sin6_ptr(addr)->sin6_addr = &net_sin6(local)->sin6_addr; + } +#endif +} + int net_tcp_prepare_reset(struct net_tcp *tcp, + const struct sockaddr *local, const struct sockaddr *remote, struct net_pkt **pkt) { struct tcp_segment segment = { 0 }; int status = 0; + struct sockaddr_ptr src_addr_ptr; if ((net_context_get_state(tcp->context) != NET_CONTEXT_UNCONNECTED) && (net_tcp_get_state(tcp) != NET_TCP_SYN_SENT) && @@ -675,7 +700,15 @@ int net_tcp_prepare_reset(struct net_tcp *tcp, segment.ack = tcp->send_ack; segment.flags = NET_TCP_RST | NET_TCP_ACK; segment.seq = tcp->send_seq; - segment.src_addr = &tcp->context->local; + + if (!local) { + segment.src_addr = &tcp->context->local; + } else { + copy_sockaddr_to_sockaddr_ptr(tcp, local, + &src_addr_ptr); + segment.src_addr = &src_addr_ptr; + } + segment.dst_addr = remote; segment.wnd = 0; segment.options = NULL; diff --git a/subsys/net/ip/tcp.h b/subsys/net/ip/tcp.h index fe892a3c4e742..6ed2d4f94fd58 100644 --- a/subsys/net/ip/tcp.h +++ b/subsys/net/ip/tcp.h @@ -302,12 +302,15 @@ int net_tcp_prepare_ack(struct net_tcp *tcp, const struct sockaddr *remote, * @brief Prepare a TCP RST message that can be send to peer. * * @param tcp TCP context + * @param local Source address * @param remote Peer address * @param pkt Network buffer * * @return 0 if ok, < 0 if error */ -int net_tcp_prepare_reset(struct net_tcp *tcp, const struct sockaddr *remote, +int net_tcp_prepare_reset(struct net_tcp *tcp, + const struct sockaddr *local, + const struct sockaddr *remote, struct net_pkt **pkt); typedef void (*net_tcp_cb_t)(struct net_tcp *tcp, void *user_data);