From eb78d70805af125995c8e5c3e6fc9a47fb6655d7 Mon Sep 17 00:00:00 2001 From: Marcin Niestroj Date: Thu, 6 May 2021 02:04:36 +0200 Subject: [PATCH] drivers: wifi: esp: stop using pkt->work in TX path Usage of k_work object from within net_pkt results in undefined behavior in case when net_pkt is deallocated (by net_pkt_unref()) before work has been finished. Use per socket k_work object (sock->send_work) to submit send work and put net_pkt objects onto k_fifo (sock->tx_fifo). Add a helper function esp_socket_queue_tx() for that, which will make sure that packets are enqueued only when send work handler will be successfully submitted (so that all packets are consumed/dereferenced). Signed-off-by: Marcin Niestroj --- drivers/wifi/esp_at/esp.h | 21 +++++++++++++++++ drivers/wifi/esp_at/esp_offload.c | 39 +++++++++++++++++-------------- drivers/wifi/esp_at/esp_socket.c | 2 ++ 3 files changed, 45 insertions(+), 17 deletions(-) diff --git a/drivers/wifi/esp_at/esp.h b/drivers/wifi/esp_at/esp.h index 52cda02bc13..bbdf339da0b 100644 --- a/drivers/wifi/esp_at/esp.h +++ b/drivers/wifi/esp_at/esp.h @@ -165,8 +165,12 @@ struct esp_socket { /* work */ struct k_work connect_work; struct k_work recvdata_work; + struct k_work send_work; struct k_work close_work; + /* TX packets fifo */ + struct k_fifo tx_fifo; + /* net context */ struct net_context *context; net_context_connect_cb_t connect_cb; @@ -335,6 +339,22 @@ static inline int esp_socket_work_submit(struct esp_socket *sock, return ret; } +static inline int esp_socket_queue_tx(struct esp_socket *sock, + struct net_pkt *pkt) +{ + int ret = -EBUSY; + + k_mutex_lock(&sock->lock, K_FOREVER); + if (!(esp_socket_flags(sock) & ESP_SOCK_WORKQ_STOPPED)) { + k_fifo_put(&sock->tx_fifo, pkt); + __esp_socket_work_submit(sock, &sock->send_work); + ret = 0; + } + k_mutex_unlock(&sock->lock); + + return ret; +} + static inline bool esp_socket_connected(struct esp_socket *sock) { return (esp_socket_flags(sock) & ESP_SOCK_CONNECTED) != 0; @@ -378,6 +398,7 @@ static inline int esp_cmd_send(struct esp_data *data, void esp_connect_work(struct k_work *work); void esp_recvdata_work(struct k_work *work); void esp_close_work(struct k_work *work); +void esp_send_work(struct k_work *work); #ifdef __cplusplus } diff --git a/drivers/wifi/esp_at/esp_offload.c b/drivers/wifi/esp_at/esp_offload.c index f60d7ceac50..23867c1ea7d 100644 --- a/drivers/wifi/esp_at/esp_offload.c +++ b/drivers/wifi/esp_at/esp_offload.c @@ -290,24 +290,32 @@ out: return ret; } -static void esp_send_work(struct k_work *work) +void esp_send_work(struct k_work *work) { - struct net_pkt *pkt = CONTAINER_OF(work, struct net_pkt, work); - struct net_context *context = pkt->context; - struct esp_socket *sock = context->offload_context; + struct esp_socket *sock = CONTAINER_OF(work, struct esp_socket, + send_work); + struct net_context *context = sock->context; + struct net_pkt *pkt; int ret = 0; - ret = _sock_send(sock, pkt); - if (ret < 0) { - LOG_ERR("Failed to send data: link %d, ret %d", sock->link_id, - ret); - } + while (true) { + pkt = k_fifo_get(&sock->tx_fifo, K_NO_WAIT); + if (!pkt) { + break; + } - if (context->send_cb) { - context->send_cb(context, ret, context->user_data); - } + ret = _sock_send(sock, pkt); + if (ret < 0) { + LOG_ERR("Failed to send data: link %d, ret %d", + sock->link_id, ret); + } - net_pkt_unref(pkt); + if (context->send_cb) { + context->send_cb(context, ret, context->user_data); + } + + net_pkt_unref(pkt); + } } static int esp_sendto(struct net_pkt *pkt, @@ -359,10 +367,7 @@ static int esp_sendto(struct net_pkt *pkt, } } - k_work_init(&pkt->work, esp_send_work); - esp_socket_work_submit(sock, &pkt->work); - - return 0; + return esp_socket_queue_tx(sock, pkt); } static int esp_send(struct net_pkt *pkt, diff --git a/drivers/wifi/esp_at/esp_socket.c b/drivers/wifi/esp_at/esp_socket.c index 5c805f7b9b5..7c5394d5d51 100644 --- a/drivers/wifi/esp_at/esp_socket.c +++ b/drivers/wifi/esp_at/esp_socket.c @@ -93,6 +93,8 @@ void esp_socket_init(struct esp_data *data) k_work_init(&sock->connect_work, esp_connect_work); k_work_init(&sock->recvdata_work, esp_recvdata_work); k_work_init(&sock->close_work, esp_close_work); + k_work_init(&sock->send_work, esp_send_work); + k_fifo_init(&sock->tx_fifo); } }