From 9b87551d40030c928a235af95f8e4efbf84ec1ab Mon Sep 17 00:00:00 2001 From: Arun S Date: Tue, 21 Apr 2020 22:15:25 +0530 Subject: [PATCH] Remove ethernet interface logic for SoftAP and STA Issue: The problem is that there is a race condition introduced in that the LWIP thread is relying on the interface as it is taken down by a application thread while calling disconnect. In disconnect api called from application context, whd_emac_wifi_link_state_changed() will refer to netif interface structure in its callback api netif_link_irq(netif). This netif will be cleared by remove_etherent_interface(). whd_emac_wifi_link_state_changed will post message to tcpip_thread. tcpip_thread will process the message and call the callback api netif_link_irq(netif) Calling sequence is whd_emac_wifi_link_state_changed -> remove_etherent_interface(). Hence there is a timing issue that netif might be cleared first before tcpip thread process the message netif_link_irq(netif) Fix: remove_etherent_interface() will post message to tcpip thread and tcpip: thread process the message delete_interface() which will actually remove the inferface from the netif_list. Calling sequence is whd_emac_wifi_link_state_changed() message post -> remove_etherent_interface() message post. message processing order netif_link_irq(netif) -> delete_interface(). Since both the processing is handled in single thread, processing of message is handled sequentially. --- features/lwipstack/LWIPInterface.cpp | 48 +++++++++++++++++++ features/lwipstack/LWIPStack.h | 11 +++++ features/netsocket/OnboardNetworkStack.h | 5 ++ .../interface/WhdSTAInterface.cpp | 9 ++++ .../interface/WhdSoftAPInterface.cpp | 9 ++++ 5 files changed, 82 insertions(+) diff --git a/features/lwipstack/LWIPInterface.cpp b/features/lwipstack/LWIPInterface.cpp index 1fe8f4f92e..3f82e52994 100644 --- a/features/lwipstack/LWIPInterface.cpp +++ b/features/lwipstack/LWIPInterface.cpp @@ -494,6 +494,11 @@ LWIP::Interface::Interface() : attr.cb_mem = &has_any_addr_sem; attr.cb_size = sizeof has_any_addr_sem; has_any_addr = osSemaphoreNew(UINT16_MAX, 0, &attr); + + attr.cb_mem = &remove_interface_sem; + attr.cb_size = sizeof remove_interface_sem; + remove_interface = osSemaphoreNew(UINT16_MAX, 0, &attr); + #if PREF_ADDR_TIMEOUT attr.cb_mem = &has_pref_addr_sem; attr.cb_size = sizeof has_pref_addr_sem; @@ -565,6 +570,49 @@ nsapi_error_t LWIP::add_ethernet_interface(EMAC &emac, bool default_if, OnboardN #endif //LWIP_ETHERNET } +void LWIP::Interface::delete_interface(OnboardNetworkStack::Interface **interface_out) +{ +#if LWIP_ETHERNET + if ((interface_out != NULL) && (*interface_out != NULL)) { + LWIP::Interface *lwip = static_cast(*interface_out); + LWIP::Interface *node = lwip->list; + + if (lwip->list != NULL) { + if (lwip->list == lwip) { + + lwip->list = lwip->list->next; + netif_remove(&node->netif); + *interface_out = NULL; + delete node; + } else { + while (node->next != NULL && node->next != lwip) { + node = node->next; + } + if (node->next != NULL && node->next == lwip) { + Interface *remove = node->next; + node->next = node->next->next; + netif_remove(&remove->netif); + *interface_out = NULL; + delete remove; + } + } + } + osSemaphoreRelease(lwip->remove_interface); + } +#endif +} + +nsapi_error_t LWIP::remove_ethernet_interface(OnboardNetworkStack::Interface **interface_out) +{ +#if LWIP_ETHERNET + LWIP::Interface *lwip = static_cast(*interface_out); + tcpip_callback_with_block((tcpip_callback_fn)&LWIP::Interface::delete_interface, interface_out, 1); + osSemaphoreAcquire(lwip->remove_interface, osWaitForever); + return NSAPI_ERROR_OK; +#else + return NSAPI_ERROR_UNSUPPORTED; +#endif //LWIP_ETHERNET +} nsapi_error_t LWIP::add_l3ip_interface(L3IP &l3ip, bool default_if, OnboardNetworkStack::Interface **interface_out) { diff --git a/features/lwipstack/LWIPStack.h b/features/lwipstack/LWIPStack.h index 18cf9d54f6..a5fa44a8f0 100644 --- a/features/lwipstack/LWIPStack.h +++ b/features/lwipstack/LWIPStack.h @@ -149,6 +149,7 @@ public: static void netif_link_irq(struct netif *netif); static void netif_status_irq(struct netif *netif); static Interface *our_if_from_netif(struct netif *netif); + static void delete_interface(OnboardNetworkStack::Interface **interface_out); #if LWIP_ETHERNET static err_t emac_low_level_output(struct netif *netif, struct pbuf *p); @@ -208,6 +209,8 @@ public: void *hw; /**< alternative implementation pointer - used for PPP */ }; + mbed_rtos_storage_semaphore_t remove_interface_sem; + osSemaphoreId_t remove_interface; mbed_rtos_storage_semaphore_t linked_sem; osSemaphoreId_t linked; mbed_rtos_storage_semaphore_t unlinked_sem; @@ -282,6 +285,14 @@ public: */ virtual nsapi_error_t add_ppp_interface(PPP &ppp, bool default_if, OnboardNetworkStack::Interface **interface_out); + /** Remove a network interface from IP stack + * + * Removes layer 3 IP objects,network interface from stack list . + * @param[out] interface_out pointer to stack interface object controlling the EMAC + * @return NSAPI_ERROR_OK on success, or error code + */ + nsapi_error_t remove_ethernet_interface(OnboardNetworkStack::Interface **interface_out) override; + /** Remove a network interface from IP stack * * Removes layer 3 IP objects,network interface from stack list, and shutdown device driver . diff --git a/features/netsocket/OnboardNetworkStack.h b/features/netsocket/OnboardNetworkStack.h index 7a3f25d9ea..16f1a79107 100644 --- a/features/netsocket/OnboardNetworkStack.h +++ b/features/netsocket/OnboardNetworkStack.h @@ -169,6 +169,11 @@ public: return NSAPI_ERROR_UNSUPPORTED; }; + virtual nsapi_error_t remove_ethernet_interface(Interface **interface_out) + { + return NSAPI_ERROR_OK; + }; + virtual nsapi_error_t remove_l3ip_interface(Interface **interface_out) { return NSAPI_ERROR_OK; diff --git a/features/netsocket/emac-drivers/TARGET_Cypress/COMPONENT_WHD/interface/WhdSTAInterface.cpp b/features/netsocket/emac-drivers/TARGET_Cypress/COMPONENT_WHD/interface/WhdSTAInterface.cpp index 1cbe7ebc2a..3c23d7de1a 100644 --- a/features/netsocket/emac-drivers/TARGET_Cypress/COMPONENT_WHD/interface/WhdSTAInterface.cpp +++ b/features/netsocket/emac-drivers/TARGET_Cypress/COMPONENT_WHD/interface/WhdSTAInterface.cpp @@ -359,6 +359,15 @@ nsapi_error_t WhdSTAInterface::disconnect() } whd_emac_wifi_link_state_changed(_whd_emac.ifp, WHD_FALSE); + // remove the interface added in connect + if (_interface) { + nsapi_error_t err = _stack.remove_ethernet_interface(&_interface); + if (err != NSAPI_ERROR_OK) { + return err; + } + _iface_shared.iface_sta = NULL; + } + res = whd_wifi_deregister_event_handler(_whd_emac.ifp, sta_link_update_entry); if (res != WHD_SUCCESS) { return whd_toerror(res); diff --git a/features/netsocket/emac-drivers/TARGET_Cypress/COMPONENT_WHD/interface/WhdSoftAPInterface.cpp b/features/netsocket/emac-drivers/TARGET_Cypress/COMPONENT_WHD/interface/WhdSoftAPInterface.cpp index 6a764750ee..e3c499c0e7 100644 --- a/features/netsocket/emac-drivers/TARGET_Cypress/COMPONENT_WHD/interface/WhdSoftAPInterface.cpp +++ b/features/netsocket/emac-drivers/TARGET_Cypress/COMPONENT_WHD/interface/WhdSoftAPInterface.cpp @@ -201,6 +201,15 @@ int WhdSoftAPInterface::stop(void) if (res != WHD_SUCCESS) { return whd_toerror(res); } + + // remove the interface added in start + if (_interface) { + nsapi_error_t err = _stack.remove_ethernet_interface(&_interface); + if (err != NSAPI_ERROR_OK) { + return err; + } + _iface_shared.iface_softap = NULL; + } return NSAPI_ERROR_OK; }