Skip to content

Commit

Permalink
net: shell: Fix TCP connect behavior
Browse files Browse the repository at this point in the history
Fix two issues with net tcp command:

* The `net tcp` commands are still based on net_context API. For TCP,
  the API caller (net shell) should add one extra reference to the
  allocated net context, to prevent premature context release in case of
  connection teardown. Currently that was not the case, and the context
  was released too early, resulting in missing final ACK from the Zephyr
  side on connection close.

* The net context API should not be called from the registered connect
  callback, as this creates a temporary deadlock situation. The
  net_context_connect() function blocks until the connection is
  established, or an error or timeout occurs. For that time the
  net_context mutex is being locked. In case of connection error (for
  example after receiving RST packet) the connect callback is called,
  indicating an error. If we try to call net_context API from within, a
  deadlock situation takes place, as the context mutex is still locked
  by the net_context_connect() (called from the shell thread). This
  blocks the further execution of the TCP stack and can result in an
  unexpected behavior (like for example retransmitting the SYN packet,
  which takes place from yet another thread, TCP work queue).
  Fix this, by releasing the net context not from the callback directly,
  but based on the return value from net_context_connect().

Signed-off-by: Robert Lubos <[email protected]>
  • Loading branch information
rlubos authored and carlescufi committed Oct 23, 2023
1 parent ad1b1cf commit 05361ed
Showing 1 changed file with 9 additions and 6 deletions.
15 changes: 9 additions & 6 deletions subsys/net/lib/shell/tcp.c
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,6 @@ static void tcp_connected(struct net_context *context,
{
if (status < 0) {
PR_SHELL(tcp_shell, "TCP connection failed (%d)\n", status);

net_context_put(context);

tcp_ctx = NULL;
} else {
PR_SHELL(tcp_shell, "TCP connected\n");
}
Expand Down Expand Up @@ -207,8 +203,15 @@ static void tcp_connect(const struct shell *sh, char *host, uint16_t port,
#define CONNECT_TIMEOUT K_SECONDS(3)
#endif

net_context_connect(*ctx, &addr, addrlen, tcp_connected,
CONNECT_TIMEOUT, NULL);
net_context_ref(*ctx);

ret = net_context_connect(*ctx, &addr, addrlen, tcp_connected,
CONNECT_TIMEOUT, NULL);
if (ret < 0) {
PR_WARNING("Connect failed!\n");
net_context_put(*ctx);
tcp_ctx = NULL;
}
}

static void tcp_sent_cb(struct net_context *context,
Expand Down

0 comments on commit 05361ed

Please sign in to comment.