Skip to content

Commit eb78d70

Browse files
mniestrojcarlescufi
authored andcommitted
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 <[email protected]>
1 parent b671cf7 commit eb78d70

File tree

3 files changed

+45
-17
lines changed

3 files changed

+45
-17
lines changed

drivers/wifi/esp_at/esp.h

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,8 +165,12 @@ struct esp_socket {
165165
/* work */
166166
struct k_work connect_work;
167167
struct k_work recvdata_work;
168+
struct k_work send_work;
168169
struct k_work close_work;
169170

171+
/* TX packets fifo */
172+
struct k_fifo tx_fifo;
173+
170174
/* net context */
171175
struct net_context *context;
172176
net_context_connect_cb_t connect_cb;
@@ -335,6 +339,22 @@ static inline int esp_socket_work_submit(struct esp_socket *sock,
335339
return ret;
336340
}
337341

342+
static inline int esp_socket_queue_tx(struct esp_socket *sock,
343+
struct net_pkt *pkt)
344+
{
345+
int ret = -EBUSY;
346+
347+
k_mutex_lock(&sock->lock, K_FOREVER);
348+
if (!(esp_socket_flags(sock) & ESP_SOCK_WORKQ_STOPPED)) {
349+
k_fifo_put(&sock->tx_fifo, pkt);
350+
__esp_socket_work_submit(sock, &sock->send_work);
351+
ret = 0;
352+
}
353+
k_mutex_unlock(&sock->lock);
354+
355+
return ret;
356+
}
357+
338358
static inline bool esp_socket_connected(struct esp_socket *sock)
339359
{
340360
return (esp_socket_flags(sock) & ESP_SOCK_CONNECTED) != 0;
@@ -378,6 +398,7 @@ static inline int esp_cmd_send(struct esp_data *data,
378398
void esp_connect_work(struct k_work *work);
379399
void esp_recvdata_work(struct k_work *work);
380400
void esp_close_work(struct k_work *work);
401+
void esp_send_work(struct k_work *work);
381402

382403
#ifdef __cplusplus
383404
}

drivers/wifi/esp_at/esp_offload.c

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -290,24 +290,32 @@ static int _sock_send(struct esp_socket *sock, struct net_pkt *pkt)
290290
return ret;
291291
}
292292

293-
static void esp_send_work(struct k_work *work)
293+
void esp_send_work(struct k_work *work)
294294
{
295-
struct net_pkt *pkt = CONTAINER_OF(work, struct net_pkt, work);
296-
struct net_context *context = pkt->context;
297-
struct esp_socket *sock = context->offload_context;
295+
struct esp_socket *sock = CONTAINER_OF(work, struct esp_socket,
296+
send_work);
297+
struct net_context *context = sock->context;
298+
struct net_pkt *pkt;
298299
int ret = 0;
299300

300-
ret = _sock_send(sock, pkt);
301-
if (ret < 0) {
302-
LOG_ERR("Failed to send data: link %d, ret %d", sock->link_id,
303-
ret);
304-
}
301+
while (true) {
302+
pkt = k_fifo_get(&sock->tx_fifo, K_NO_WAIT);
303+
if (!pkt) {
304+
break;
305+
}
305306

306-
if (context->send_cb) {
307-
context->send_cb(context, ret, context->user_data);
308-
}
307+
ret = _sock_send(sock, pkt);
308+
if (ret < 0) {
309+
LOG_ERR("Failed to send data: link %d, ret %d",
310+
sock->link_id, ret);
311+
}
312+
313+
if (context->send_cb) {
314+
context->send_cb(context, ret, context->user_data);
315+
}
309316

310-
net_pkt_unref(pkt);
317+
net_pkt_unref(pkt);
318+
}
311319
}
312320

313321
static int esp_sendto(struct net_pkt *pkt,
@@ -359,10 +367,7 @@ static int esp_sendto(struct net_pkt *pkt,
359367
}
360368
}
361369

362-
k_work_init(&pkt->work, esp_send_work);
363-
esp_socket_work_submit(sock, &pkt->work);
364-
365-
return 0;
370+
return esp_socket_queue_tx(sock, pkt);
366371
}
367372

368373
static int esp_send(struct net_pkt *pkt,

drivers/wifi/esp_at/esp_socket.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,8 @@ void esp_socket_init(struct esp_data *data)
9393
k_work_init(&sock->connect_work, esp_connect_work);
9494
k_work_init(&sock->recvdata_work, esp_recvdata_work);
9595
k_work_init(&sock->close_work, esp_close_work);
96+
k_work_init(&sock->send_work, esp_send_work);
97+
k_fifo_init(&sock->tx_fifo);
9698
}
9799
}
98100

0 commit comments

Comments
 (0)