From 0d86104b86d7dbede25f3b15d53efe8ce0498aa0 Mon Sep 17 00:00:00 2001 From: Jukka Rissanen Date: Fri, 12 May 2017 20:48:23 +0300 Subject: [PATCH] net: context: Call send_cb for TCP after ACK is received Change the TCP send_cb call to be done after we have received ACK to sent packet. It did not make much sense to call the callback immediately (synchronously) after we sent the net_pkt. Signed-off-by: Jukka Rissanen --- include/net/net_pkt.h | 3 +++ subsys/net/ip/net_context.c | 53 ++++++++++++++++++++++++++++++++----- subsys/net/ip/net_if.c | 14 ---------- subsys/net/ip/net_pkt.c | 35 ++++++++++++++++++++++++ subsys/net/ip/net_private.h | 2 ++ subsys/net/ip/tcp.c | 13 +++++++++ 6 files changed, 99 insertions(+), 21 deletions(-) diff --git a/include/net/net_pkt.h b/include/net/net_pkt.h index c0da4b2dcc100..d5283989d5d20 100644 --- a/include/net/net_pkt.h +++ b/include/net/net_pkt.h @@ -69,6 +69,9 @@ struct net_pkt { #if defined(CONFIG_NET_TCP) sys_snode_t sent_list; + + /* Send callback timer, this is set in net_context when data is sent. */ + struct k_delayed_work send_cb_timer; #endif u8_t sent : 1; /* Is this sent or not diff --git a/subsys/net/ip/net_context.c b/subsys/net/ip/net_context.c index 3518f499fcbf6..fef9c7545bdfa 100644 --- a/subsys/net/ip/net_context.c +++ b/subsys/net/ip/net_context.c @@ -33,6 +33,8 @@ #include "udp.h" #include "tcp.h" +#include "net_stats.h" + #define NET_MAX_CONTEXT CONFIG_NET_MAX_CONTEXTS /* Declares a wrapper function for a net_conn callback that refs the @@ -1595,14 +1597,18 @@ static int send_data(struct net_context *context, if (net_context_get_ip_proto(context) == IPPROTO_TCP) { int ret = net_tcp_send_data(context); - /* Just make the callback synchronously even if it didn't - * go over the wire. In theory it would be nice to track - * specific ACK locations in the stream and make the - * callback at that time, but there's nowhere to store the - * potentially-separate token/user_data values right now. + /* If there is no timeout, then we can only call the send_cb + * immediately, otherwise wait until we have received ACK + * to the packet or timeout happens. If the timeout is + * K_FOREVER, then do not install cancel worker but wait + * until we have received the ACK. */ - if (cb) { - cb(context, ret, token, user_data); + if (timeout > 0) { + k_delayed_work_submit(&pkt->send_cb_timer, timeout); + } else if (timeout == K_NO_WAIT) { + if (cb) { + cb(context, ret, token, user_data); + } } return ret; @@ -1814,6 +1820,39 @@ int net_context_sendto(struct net_pkt *pkt, return sendto(pkt, dst_addr, addrlen, cb, timeout, token, user_data); } +void net_context_send_cb(struct net_context *context, void *token, int status) +{ + if (!context) { + return; + } + + if (status < 0) { + /* If there was an error, then call the callback always. + * For success call it only for UDP as TCP one will be called + * after we have received ACK. + */ + if (context->send_cb) { + context->send_cb(context, status, token, + context->user_data); + } + } else { +#if defined(CONFIG_NET_UDP) + if (net_context_get_ip_proto(context) == IPPROTO_UDP) { + if (context->send_cb) { + context->send_cb(context, status, token, + context->user_data); + } + } +#endif + } + +#if defined(CONFIG_NET_UDP) + if (net_context_get_ip_proto(context) == IPPROTO_UDP) { + net_stats_update_udp_sent(); + } +#endif +} + static void set_appdata_values(struct net_pkt *pkt, enum net_ip_protocol proto) { size_t total_len = net_pkt_get_len(pkt); diff --git a/subsys/net/ip/net_if.c b/subsys/net/ip/net_if.c index 489564f0ba655..d79959d47cb75 100644 --- a/subsys/net/ip/net_if.c +++ b/subsys/net/ip/net_if.c @@ -60,20 +60,6 @@ static struct k_thread tx_thread_data; #define debug_check_packet(...) #endif /* CONFIG_NET_DEBUG_IF */ -static inline void net_context_send_cb(struct net_context *context, - void *token, int status) -{ - if (context->send_cb) { - context->send_cb(context, status, token, context->user_data); - } - -#if defined(CONFIG_NET_UDP) - if (net_context_get_ip_proto(context) == IPPROTO_UDP) { - net_stats_update_udp_sent(); - } -#endif -} - static bool net_if_tx(struct net_if *iface) { const struct net_if_api *api = iface->dev->driver_api; diff --git a/subsys/net/ip/net_pkt.c b/subsys/net/ip/net_pkt.c index f501b149a6074..41cc09ad5497f 100644 --- a/subsys/net/ip/net_pkt.c +++ b/subsys/net/ip/net_pkt.c @@ -88,6 +88,26 @@ NET_BUF_POOL_DEFINE(rx_bufs, NET_BUF_RX_COUNT, NET_BUF_DATA_LEN, NET_BUF_POOL_DEFINE(tx_bufs, NET_BUF_TX_COUNT, NET_BUF_DATA_LEN, NET_BUF_USER_DATA_LEN, NULL); +#if defined(CONFIG_NET_TCP) +static void send_cb_timeout(struct k_work *work) +{ + /* This means that we did not receive ACK response in time. */ + struct net_pkt *pkt = CONTAINER_OF(work, struct net_pkt, send_cb_timer); + void *token; + + token = net_pkt_token(pkt); + + /* Clear the token so that we do not call the send callback + * again. This must be done before we call the callback so that + * any received ACK will not call the callback again while we + * are running inside the send callback. + */ + net_pkt_set_token(pkt, NULL); + + net_context_send_cb(net_pkt_context(pkt), token, -ETIMEDOUT); +} +#endif + #if defined(CONFIG_NET_DEBUG_NET_PKT) #define NET_FRAG_CHECK_IF_NOT_IN_USE(frag, ref) \ @@ -319,6 +339,10 @@ struct net_pkt *net_pkt_get_reserve(struct k_mem_slab *slab, pkt->ref = 1; pkt->slab = slab; +#if defined(CONFIG_NET_TCP) + k_delayed_work_init(&pkt->send_cb_timer, send_cb_timeout); +#endif + #if defined(CONFIG_NET_DEBUG_NET_PKT) net_pkt_alloc_add(pkt, true, caller, line); @@ -737,6 +761,17 @@ void net_pkt_unref(struct net_pkt *pkt) return; } +#if defined(CONFIG_NET_TCP) + net_pkt_set_token(pkt, NULL); + + /* If we do not have context, then the send_cb timer cannot be + * running. + */ + if (pkt->context) { + k_delayed_work_cancel(&pkt->send_cb_timer); + } +#endif + if (pkt->frags) { net_pkt_frag_unref(pkt->frags); } diff --git a/subsys/net/ip/net_private.h b/subsys/net/ip/net_private.h index 5d441e972b9f7..6b3f14c226b60 100644 --- a/subsys/net/ip/net_private.h +++ b/subsys/net/ip/net_private.h @@ -19,6 +19,8 @@ extern void net_pkt_init(void); extern void net_if_init(struct k_sem *startup_sync); extern void net_if_post_init(void); extern void net_context_init(void); +extern void net_context_send_cb(struct net_context *context, void *token, + int status); enum net_verdict net_ipv4_process_pkt(struct net_pkt *pkt); enum net_verdict net_ipv6_process_pkt(struct net_pkt *pkt); extern void net_ipv6_init(void); diff --git a/subsys/net/ip/tcp.c b/subsys/net/ip/tcp.c index 0447b81c07a68..8c792fcbe16da 100644 --- a/subsys/net/ip/tcp.c +++ b/subsys/net/ip/tcp.c @@ -790,6 +790,8 @@ void net_tcp_ack_received(struct net_context *ctx, u32_t ack) bool valid_ack = false; while (!sys_slist_is_empty(list)) { + void *token; + head = sys_slist_peek_head(list); pkt = CONTAINER_OF(head, struct net_pkt, sent_list); tcphdr = NET_TCP_HDR(pkt); @@ -811,6 +813,17 @@ void net_tcp_ack_received(struct net_context *ctx, u32_t ack) } sys_slist_remove(list, NULL, head); + + /* Call the network context send callback after we have + * received the ACK for it. + */ + token = net_pkt_token(pkt); + net_pkt_set_token(pkt, NULL); + + k_delayed_work_cancel(&pkt->send_cb_timer); + + net_context_send_cb(net_pkt_context(pkt), token, 0); + net_pkt_unref(pkt); valid_ack = true; }