Skip to content

TCP related and other fixes #6058

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Feb 13, 2018
Merged

TCP related and other fixes #6058

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
43729f1
net: context: Remove duplicate call
rveerama1 Feb 8, 2018
583b7c7
net: context: Handle failure case of tcp backlog entry
rveerama1 Feb 8, 2018
81645b5
net: tcp: Do not handle packet re-transmission in TCP ACK
rveerama1 Feb 8, 2018
d3960d3
net: tcp: Provide local address in TCP reset message preparation
rveerama1 Feb 8, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 15 additions & 6 deletions subsys/net/ip/net_context.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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));
}
Expand Down Expand Up @@ -199,6 +205,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;
}

Expand Down Expand Up @@ -1080,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;
}
Expand Down Expand Up @@ -1203,7 +1213,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) {
Expand Down Expand Up @@ -1362,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;
}

Expand Down Expand Up @@ -1845,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;
}
Expand Down
65 changes: 40 additions & 25 deletions subsys/net/ip/tcp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) &&
Expand All @@ -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;
Expand Down Expand Up @@ -961,32 +994,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 */
Copy link
Contributor

@pfalcon pfalcon Feb 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment for the commit message:

Now ACK's are in rx_thread queue or ACK for first packet received.

I'm not sure I understand this phrase. Can you please clarify the description in commit message?

Copy link
Contributor

@pfalcon pfalcon Feb 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, whole this part is not very clear:

Now ACK's are
in rx_thread queue or ACK for first packet received. Then
net_tcp_ack_received() function will delete the first packet.
Because it's valid ACK and connection state is in ESTABLISHED state
sent list is not empty. It restarts the timer and resend packets
in queue. In e.g. case Zephyr sends second packet in a list
immediately.

/* 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;
Expand Down
5 changes: 4 additions & 1 deletion subsys/net/ip/tcp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down