michaelh/zephyr
1
0
Fork 0
Code Releases Activity

net: tcp: Clean up net_context lifecycle

Browse source 
The death of a network context was sort of a mess.  There was one
function, net_context_put(), which was used both by the user as a way
to "close" the connection and by the internals to delete it and to
"clean up" a TCP connection at the end of its life.

This has led to repeated gotchas where contexts die before you are
ready for them (one example: when a user callback decides the
transation is complete and calls net_context_put() underneath the
receive callback for the EOF, which then returns and tries to inspect
the now-freed memory inside the TCP internals).  I've now stepped into
this mess four times now, and it's time to fix the architecture:

Swap the solitary put() call for a more conventional reference
counting implementation.  The put() call now is a pure user API (and
maybe should be renamed "close" or "shutdown").  For compatibility,
it still calls unref() where appropriate (i.e. when the context can be
synchronously deleted) and the FIN processing will still do an unref()
when the FIN packets have been both transmitted and acked.  The
context will start with a refcount of 1, and all TCP callbacks made on
it will increment the refcount around the callback to prevent
premature deletion.

Note that this gives the user a "destroy" mechanism for an in-progress
connection that doesn't require a network round trip.  That might be
useful in some circumstances.

Change-Id: I44cb355e42941605913b2f84eb14d4eb3c134570
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
This commit is contained in:
Andy Ross 2017-01-25 15:14:59 -08:00 • committed by Jukka Rissanen
commit 5d6e0d425b
3 changed files with 120 additions and 47 deletions
Download patch file Download diff file Expand all files Collapse all files

52
include/net/net_context.h
View file

@ -14,6 +14,7 @@
#define __NET_CONTEXT_H
#include <kernel.h>
#include <atomic.h>
#include <net/net_ip.h>
#include <net/net_if.h>
@ -139,6 +140,10 @@ struct net_conn_handle;
* anyway. This saves 12 bytes / context in IPv6.
*/
struct net_context {
/** Reference count
*/
atomic_t refcount;
/** Local IP address. Note that the values are in network byte order.
*/
struct sockaddr_ptr local;
@ -402,10 +407,10 @@ static inline void net_context_set_iface(struct net_context *context,
/**
* @brief Get network context.
*
* @details Network context is used to define the connection
* 5-tuple (protocol, remote address, remote port, source
* address and source port). This is similar as BSD socket()
* function.
* @details Network context is used to define the connection 5-tuple
* (protocol, remote address, remote port, source address and source
* port). This is similar as BSD socket() function. The context will
* be created with a reference count of 1.
*
* @param family IP address family (AF_INET or AF_INET6)
* @param type Type of the socket, SOCK_STREAM or SOCK_DGRAM
@ -420,11 +425,13 @@ int net_context_get(sa_family_t family,
struct net_context **context);
/**
* @brief Free/close a network context.
* @brief Close and unref a network context.
*
* @details This releases the context. It is not possible to
* send or receive data via this context after this call.
* This is similar as BSD shutdown() function.
* @details This releases the context. It is not possible to send or
* receive data via this context after this call. This is similar as
* BSD shutdown() function. For legacy compatibility, this function
* will implicitly decrement the reference count and possibly destroy
* the context either now or when it reaches a final state.
*
* @param context The context to be closed.
*
@ -432,6 +439,35 @@ int net_context_get(sa_family_t family,
*/
int net_context_put(struct net_context *context);
/**
* @brief Take a reference count to a net_context, preventing destruction
*
* @details Network contexts are not recycled until their reference
* count reaches zero. Note that this does not prevent any "close"
* behavior that results from errors or net_context_put. It simply
* prevents the context from being recycled for further use.
*
* @param context The context on which to increment the reference count
*
* @return The new reference count
*/
int net_context_ref(struct net_context *context);
/**
* @brief Decrement the reference count to a network context
*
* @details Decrements the refcount. If it reaches zero, the context
* will be recycled. Note that this does not cause any
* network-visible "close" behavior (i.e. future packets to this
* connection may see TCP RST or ICMP port unreachable responses). See
* net_context_put() for that.
*
* @param context The context on which to decrement the reference count
*
* @return The new reference count, zero if the context was destroyed
*/
int net_context_unref(struct net_context *context);
/**
* @brief Assign a socket a local address.
*

113
subsys/net/ip/net_context.c
View file

@ -35,6 +35,32 @@
#define NET_MAX_CONTEXT CONFIG_NET_MAX_CONTEXTS
/* Declares a wrapper function for a net_conn callback that refs the
* context around the invocation (to protect it from premature
* deletion). Long term would be nice to see this feature be part of
* the connection type itself, but right now it has opaque "user_data"
* pointers and doesn't understand what a net_context is.
*/
#define NET_CONN_CB(name) \
static enum net_verdict _##name(struct net_conn *conn, \
struct net_buf *buf, \
void *user_data); \
static enum net_verdict name(struct net_conn *conn, \
struct net_buf *buf, \
void *user_data) \
{ \
enum net_verdict result; \
\
net_context_ref(user_data); \
result = _##name(conn, buf, user_data); \
net_context_unref(user_data); \
return result; \
} \
static enum net_verdict _##name(struct net_conn *conn, \
struct net_buf *buf, \
void *user_data) \
static struct net_context contexts[NET_MAX_CONTEXT];
/* We need to lock the contexts array as these APIs are typically called
@ -182,6 +208,7 @@ int net_context_get(sa_family_t family,
#endif /* CONFIG_NET_TCP */
contexts[i].flags = 0;
atomic_set(&contexts[i].refcount, 1);
net_context_set_family(&contexts[i], family);
net_context_set_type(&contexts[i], type);
@ -255,6 +282,40 @@ static bool send_fin_if_active_close(struct net_context *context)
}
#endif /* CONFIG_NET_TCP */
int net_context_ref(struct net_context *context)
{
int old_rc = atomic_inc(&context->refcount);
return old_rc + 1;
}
int net_context_unref(struct net_context *context)
{
int old_rc = atomic_dec(&context->refcount);
if (old_rc != 1) {
return old_rc - 1;
}
k_sem_take(&contexts_lock, K_FOREVER);
#if defined(CONFIG_NET_TCP)
if (context->tcp) {
net_tcp_release(context->tcp);
}
#endif /* CONFIG_NET_TCP */
if (context->conn_handler) {
net_conn_unregister(context->conn_handler);
}
context->flags &= ~NET_CONTEXT_IN_USE;
k_sem_give(&contexts_lock);
return 0;
}
int net_context_put(struct net_context *context)
{
NET_ASSERT(context);
@ -263,10 +324,9 @@ int net_context_put(struct net_context *context)
return -EINVAL;
}
k_sem_take(&contexts_lock, K_FOREVER);
#if defined(CONFIG_NET_L2_OFFLOAD_IP)
if (net_if_is_ip_offloaded(net_context_get_iface(context))) {
k_sem_take(&contexts_lock, K_FOREVER);
context->flags &= ~NET_CONTEXT_IN_USE;
return net_l2_offload_ip_put(
net_context_get_iface(context), context);
@ -278,27 +338,12 @@ int net_context_put(struct net_context *context)
if (send_fin_if_active_close(context)) {
NET_DBG("TCP connection in active close, not "
"disposing yet");
goto still_in_use;
}
if (context->tcp) {
net_tcp_release(context->tcp);
return 0;
}
}
#endif /* CONFIG_NET_TCP */
if (context->conn_handler) {
net_conn_unregister(context->conn_handler);
}
context->flags &= ~NET_CONTEXT_IN_USE;
#if defined(CONFIG_NET_TCP)
still_in_use:
#endif /* CONFIG_NET_TCP */
k_sem_give(&contexts_lock);
net_context_unref(context);
return 0;
}
@ -694,9 +739,7 @@ static int tcp_hdr_len(struct net_buf *buf)
return 4 * (hdr->offset >> 4);
}
static enum net_verdict tcp_passive_close(struct net_conn *conn,
struct net_buf *buf,
void *user_data)
NET_CONN_CB(tcp_passive_close)
{
struct net_context *context = (struct net_context *)user_data;
@ -717,7 +760,7 @@ static enum net_verdict tcp_passive_close(struct net_conn *conn,
if (net_tcp_get_state(context->tcp) == NET_TCP_LAST_ACK &&
NET_TCP_FLAGS(buf) & NET_TCP_ACK) {
NET_DBG("ACK received in LAST_ACK, disposing of connection");
net_context_put(context);
net_context_unref(context);
}
return NET_DROP;
@ -726,9 +769,7 @@ static enum net_verdict tcp_passive_close(struct net_conn *conn,
/* This is called when we receive data after the connection has been
* established. The core TCP logic is located here.
*/
static enum net_verdict tcp_established(struct net_conn *conn,
struct net_buf *buf,
void *user_data)
NET_CONN_CB(tcp_established)
{
struct net_context *context = (struct net_context *)user_data;
enum net_verdict ret;
@ -784,9 +825,7 @@ static enum net_verdict tcp_established(struct net_conn *conn,
return ret;
}
static enum net_verdict tcp_active_close(struct net_conn *conn,
struct net_buf *buf,
void *user_data)
NET_CONN_CB(tcp_active_close)
{
struct net_context *context = (struct net_context *)user_data;
struct net_tcp *tcp;
@ -825,9 +864,7 @@ static enum net_verdict tcp_active_close(struct net_conn *conn,
return NET_DROP;
}
static enum net_verdict tcp_synack_received(struct net_conn *conn,
struct net_buf *buf,
void *user_data)
NET_CONN_CB(tcp_synack_received)
{
struct net_context *context = (struct net_context *)user_data;
int ret;
@ -947,6 +984,7 @@ static enum net_verdict tcp_synack_received(struct net_conn *conn,
return NET_DROP;
}
#endif /* CONFIG_NET_TCP */
int net_context_connect(struct net_context *context,
@ -1183,9 +1221,7 @@ static void buf_get_sockaddr(sa_family_t family, struct net_buf *buf,
* The ACK could also be received, in which case we have an established
* connection.
*/
static enum net_verdict tcp_syn_rcvd(struct net_conn *conn,
struct net_buf *buf,
void *user_data)
NET_CONN_CB(tcp_syn_rcvd)
{
struct net_context *context = (struct net_context *)user_data;
struct net_tcp *tcp;
@ -1345,7 +1381,7 @@ static enum net_verdict tcp_syn_rcvd(struct net_conn *conn,
{
NET_ASSERT_INFO(false, "Invalid protocol family %d",
net_context_get_family(context));
net_context_put(new_context);
net_context_unref(new_context);
return NET_DROP;
}
@ -1354,7 +1390,7 @@ static enum net_verdict tcp_syn_rcvd(struct net_conn *conn,
if (ret < 0) {
NET_DBG("Cannot bind accepted context, "
"connection reset");
net_context_put(new_context);
net_context_unref(new_context);
goto reset;
}
@ -1373,7 +1409,7 @@ static enum net_verdict tcp_syn_rcvd(struct net_conn *conn,
if (ret < 0) {
NET_DBG("Cannot register accepted TCP handler (%d)",
ret);
net_context_put(new_context);
net_context_unref(new_context);
goto reset;
}
@ -1415,6 +1451,7 @@ reset:
return NET_DROP;
}
#endif /* CONFIG_NET_TCP */
int net_context_accept(struct net_context *context,

2
subsys/net/ip/tcp.c
View file

@ -729,7 +729,7 @@ static void fin_timeout(struct k_work *work)
NET_DBG("Remote peer didn't confirm connection close");
rc = net_context_put(tcp->context);
rc = net_context_unref(tcp->context);
if (rc < 0) {
NET_DBG("Cannot close TCP context");
}