From 1ebf7b5196b069c9efefe558d5b5abf492244511 Mon Sep 17 00:00:00 2001 From: Grissiom Date: Sat, 11 Sep 2021 13:11:57 +0800 Subject: [PATCH] [Netif] Fix the eth_tx_msg protection The ethernetif use semaphore netif->tx_ack to protect the local variable `struct eth_tx_msg msg` in function `ethernetif_linkoutput`. But the function could be called in multiple context: LwIP "tcpip" thread, "erx" thread(on linkup) and any user thread that call `udp_sendto`. So the global semaphore protection is not sufficient. It could only wakeup the top priority waiting thread instead of the thread that owns the `msg` and may result the `msg` next in the mailbox got destructed. So to use a `rt_completion` within the `struct eth_tx_msg` and synchronize on that. This could deal the ownership issue in an easy way. --- .../net/lwip-2.1.2/src/include/netif/ethernetif.h | 1 - components/net/lwip-2.1.2/src/netif/ethernetif.c | 13 +++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/components/net/lwip-2.1.2/src/include/netif/ethernetif.h b/components/net/lwip-2.1.2/src/include/netif/ethernetif.h index 1459e3241d..6d5044b1b7 100644 --- a/components/net/lwip-2.1.2/src/include/netif/ethernetif.h +++ b/components/net/lwip-2.1.2/src/include/netif/ethernetif.h @@ -22,7 +22,6 @@ struct eth_device /* network interface for lwip */ struct netif *netif; - struct rt_semaphore tx_ack; rt_uint16_t flags; rt_uint8_t link_changed; diff --git a/components/net/lwip-2.1.2/src/netif/ethernetif.c b/components/net/lwip-2.1.2/src/netif/ethernetif.c index bd4db225ee..e9c2a20a15 100644 --- a/components/net/lwip-2.1.2/src/netif/ethernetif.c +++ b/components/net/lwip-2.1.2/src/netif/ethernetif.c @@ -39,6 +39,7 @@ * 2013-02-28 aozima fixed list_tcps bug: ipaddr_ntoa isn't reentrant. * 2016-08-18 Bernard port to lwIP 2.0.0 * 2018-11-02 MurphyZhao port to lwIP 2.1.0 + * 2021-09-07 Grissiom fix eth_tx_msg ack bug */ #include "lwip/opt.h" @@ -58,6 +59,8 @@ #include "lwip/inet.h" +#include + #if LWIP_IPV6 #include "lwip/ethip6.h" #endif /* LWIP_IPV6 */ @@ -79,6 +82,7 @@ struct eth_tx_msg { struct netif *netif; struct pbuf *buf; + struct rt_completion ack; }; static struct rt_mailbox eth_tx_thread_mb; @@ -387,18 +391,17 @@ static err_t ethernetif_linkoutput(struct netif *netif, struct pbuf *p) { #ifndef LWIP_NO_TX_THREAD struct eth_tx_msg msg; - struct eth_device* enetif; RT_ASSERT(netif != RT_NULL); - enetif = (struct eth_device*)netif->state; /* send a message to eth tx thread */ msg.netif = netif; msg.buf = p; + rt_completion_init(&msg.ack); if (rt_mb_send(ð_tx_thread_mb, (rt_uint32_t) &msg) == RT_EOK) { /* waiting for ack */ - rt_sem_take(&(enetif->tx_ack), RT_WAITING_FOREVER); + rt_completion_wait(&msg.ack, RT_WAITING_FOREVER); } #else struct eth_device* enetif; @@ -519,7 +522,6 @@ rt_err_t eth_device_init_with_flag(struct eth_device *dev, const char *name, rt_ dev->parent.type = RT_Device_Class_NetIf; /* register to RT-Thread device manager */ rt_device_register(&(dev->parent), name, RT_DEVICE_FLAG_RDWR); - rt_sem_init(&(dev->tx_ack), name, 0, RT_IPC_FLAG_FIFO); /* set name */ netif->name[0] = name[0]; @@ -595,7 +597,6 @@ void eth_device_deinit(struct eth_device *dev) #endif rt_device_close(&(dev->parent)); rt_device_unregister(&(dev->parent)); - rt_sem_detach(&(dev->tx_ack)); rt_free(netif); } @@ -673,7 +674,7 @@ static void eth_tx_thread_entry(void* parameter) } /* send ACK */ - rt_sem_release(&(enetif->tx_ack)); + rt_completion_done(&msg->ack); } } }