From 36735f4d775a5ed4d348321c2eaafb4a5d22a460 Mon Sep 17 00:00:00 2001 From: David Cermak Date: Fri, 22 Sep 2023 19:10:36 +0200 Subject: [PATCH] fix(esp_netif): Lock netif list with TCPIP context This commit removes the lock from the list manipulation code in esp_netif_objects.c, because we already have another lock/task context for lwip. So the list manipulation is unsafe and safety must be assured by the stack layer (in esp_netif_lwip). Problems with current locking: * implementation of locking was wrong -- lazy init style of creating the mutex is not thread safe (and destroying it if we have no interface makes the problem exhibit very frequently) * locking only the list won't solve issues when assessing interfaces atomically * maintaining multiple locks is problematic, as we often switch between lwip context and user context in internal implementation of esp_netif_lwip Closes https://github.com/espressif/esp-idf/issues/12261 --- components/esp_netif/esp_netif_objects.c | 75 +---- components/esp_netif/include/esp_netif.h | 6 +- components/esp_netif/lwip/esp_netif_lwip.c | 286 ++++++++++-------- .../esp_netif/lwip/esp_netif_lwip_internal.h | 9 +- .../esp_netif/lwip/esp_netif_lwip_ppp.c | 40 +-- .../esp_netif/lwip/esp_netif_lwip_ppp.h | 14 +- .../private_include/esp_netif_private.h | 21 +- 7 files changed, 217 insertions(+), 234 deletions(-) diff --git a/components/esp_netif/esp_netif_objects.c b/components/esp_netif/esp_netif_objects.c index 0a19c8f1a0..fbfa45b324 100644 --- a/components/esp_netif/esp_netif_objects.c +++ b/components/esp_netif/esp_netif_objects.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2023 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -7,8 +7,6 @@ #include "esp_netif.h" #include "sys/queue.h" #include "esp_log.h" -#include "freertos/FreeRTOS.h" -#include "freertos/semphr.h" #include "esp_netif_private.h" #include @@ -28,65 +26,31 @@ struct slist_netifs_s { SLIST_HEAD(slisthead, slist_netifs_s) s_head = { .slh_first = NULL, }; static size_t s_esp_netif_counter = 0; -static SemaphoreHandle_t s_list_lock = NULL; ESP_EVENT_DEFINE_BASE(IP_EVENT); -esp_err_t esp_netif_list_lock(void) -{ - if (s_list_lock == NULL) { - s_list_lock = xSemaphoreCreateMutex(); - if (s_list_lock == NULL) { - return ESP_ERR_NO_MEM; - } - } - xSemaphoreTake(s_list_lock, portMAX_DELAY); - return ESP_OK; -} - -void esp_netif_list_unlock(void) -{ - assert(s_list_lock); - xSemaphoreGive(s_list_lock); - if (s_esp_netif_counter == 0) { - vQueueDelete(s_list_lock); - s_list_lock = NULL; - } -} - // // List manipulation functions // -esp_err_t esp_netif_add_to_list(esp_netif_t *netif) +esp_err_t esp_netif_add_to_list_unsafe(esp_netif_t *netif) { - esp_err_t ret; struct slist_netifs_s *item = calloc(1, sizeof(struct slist_netifs_s)); - ESP_LOGD(TAG, "%s %p", __func__, netif); + ESP_LOGV(TAG, "%s %p", __func__, netif); if (item == NULL) { return ESP_ERR_NO_MEM; } item->netif = netif; - if ((ret = esp_netif_list_lock()) != ESP_OK) { - free(item); - return ret; - } - SLIST_INSERT_HEAD(&s_head, item, next); ++s_esp_netif_counter; ESP_LOGD(TAG, "%s netif added successfully (total netifs: %" PRIu32 ")", __func__, (uint32_t)s_esp_netif_counter); - esp_netif_list_unlock(); return ESP_OK; } -esp_err_t esp_netif_remove_from_list(esp_netif_t *netif) +esp_err_t esp_netif_remove_from_list_unsafe(esp_netif_t *netif) { struct slist_netifs_s *item; - esp_err_t ret; - if ((ret = esp_netif_list_lock()) != ESP_OK) { - return ret; - } ESP_LOGV(TAG, "%s %p", __func__, netif); SLIST_FOREACH(item, &s_head, next) { @@ -96,11 +60,9 @@ esp_err_t esp_netif_remove_from_list(esp_netif_t *netif) --s_esp_netif_counter; ESP_LOGD(TAG, "%s netif successfully removed (total netifs: %" PRIu32 ")", __func__, (uint32_t)s_esp_netif_counter); free(item); - esp_netif_list_unlock(); return ESP_OK; } } - esp_netif_list_unlock(); return ESP_ERR_NOT_FOUND; } @@ -109,17 +71,11 @@ size_t esp_netif_get_nr_of_ifs(void) return s_esp_netif_counter; } +// This API is inherently unsafe +// suggest that users call from esp_netif_tcpip_exec() esp_netif_t* esp_netif_next(esp_netif_t* netif) { - esp_err_t ret; - esp_netif_t* result; - if ((ret = esp_netif_list_lock()) != ESP_OK) { - ESP_LOGE(TAG, "Failed to lock esp-netif list with %d", ret); - return NULL; - } - result = esp_netif_next_unsafe(netif); - esp_netif_list_unlock(); - return result; + return esp_netif_next_unsafe(netif); } esp_netif_t* esp_netif_next_unsafe(esp_netif_t* netif) @@ -144,38 +100,23 @@ esp_netif_t* esp_netif_next_unsafe(esp_netif_t* netif) bool esp_netif_is_netif_listed(esp_netif_t *esp_netif) { struct slist_netifs_s *item; - esp_err_t ret; - if ((ret = esp_netif_list_lock()) != ESP_OK) { - ESP_LOGE(TAG, "Failed to lock esp-netif list with %d", ret); - return false; - } - SLIST_FOREACH(item, &s_head, next) { if (item->netif == esp_netif) { - esp_netif_list_unlock(); return true; } } - esp_netif_list_unlock(); return false; } -esp_netif_t *esp_netif_get_handle_from_ifkey(const char *if_key) +esp_netif_t *esp_netif_get_handle_from_ifkey_unsafe(const char *if_key) { struct slist_netifs_s *item; - esp_err_t ret; - if ((ret = esp_netif_list_lock()) != ESP_OK) { - ESP_LOGE(TAG, "Failed to lock esp-netif list with %d", ret); - return NULL; - } SLIST_FOREACH(item, &s_head, next) { esp_netif_t *esp_netif = item->netif; if (strcmp(if_key, esp_netif_get_ifkey(esp_netif)) == 0) { - esp_netif_list_unlock(); return esp_netif; } } - esp_netif_list_unlock(); return NULL; } diff --git a/components/esp_netif/include/esp_netif.h b/components/esp_netif/include/esp_netif.h index 2510a1eefe..a1b3368990 100644 --- a/components/esp_netif/include/esp_netif.h +++ b/components/esp_netif/include/esp_netif.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2019-2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2019-2023 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -975,6 +975,10 @@ int32_t esp_netif_get_event_id(esp_netif_t *esp_netif, esp_netif_ip_event_type_t /** * @brief Iterates over list of interfaces. Returns first netif if NULL given as parameter * + * @note This API doesn't lock the list, nor the TCPIP context, as this it's usually required + * to get atomic access between iteration steps rather that within a single iteration. + * Therefore it is recommended to iterate over the interfaces inside esp_netif_tcpip_exec() + * * @param[in] esp_netif Handle to esp-netif instance * * @return First netif from the list if supplied parameter is NULL, next one otherwise diff --git a/components/esp_netif/lwip/esp_netif_lwip.c b/components/esp_netif/lwip/esp_netif_lwip.c index 69fdaf6d1f..0d7d7f47c5 100644 --- a/components/esp_netif/lwip/esp_netif_lwip.c +++ b/components/esp_netif/lwip/esp_netif_lwip.c @@ -144,6 +144,8 @@ static void netif_set_mldv6_flag(struct netif *netif); static void netif_unset_mldv6_flag(struct netif *netif); #endif /* LWIP_IPV6 */ +static esp_err_t esp_netif_destroy_api(esp_netif_api_msg_t *msg); + static void netif_callback_fn(struct netif* netif, netif_nsc_reason_t reason, const netif_ext_callback_args_t* args) { #if LWIP_IPV4 @@ -162,30 +164,6 @@ static void netif_callback_fn(struct netif* netif, netif_nsc_reason_t reason, co #endif /* #if LWIP_IPV6 */ } -#if LWIP_ESP_NETIF_DATA -static esp_err_t alloc_client_data_id(esp_netif_api_msg_t *msg) -{ - uint8_t *client_data_id = msg->data; - *client_data_id = netif_alloc_client_data_id(); - return ESP_OK; -} -#endif // LWIP_ESP_NETIF_DATA - -static esp_err_t set_lwip_netif_callback(struct esp_netif_api_msg_s *msg) -{ - (void)msg; - netif_add_ext_callback(&netif_callback, netif_callback_fn); - return ESP_OK; -} - -static esp_err_t remove_lwip_netif_callback(struct esp_netif_api_msg_s *msg) -{ - (void)msg; - netif_remove_ext_callback(&netif_callback); - memset(&netif_callback, 0, sizeof(netif_callback)); - return ESP_OK; -} - #ifdef CONFIG_LWIP_GARP_TMR_INTERVAL static void netif_send_garp(void *arg) @@ -276,6 +254,16 @@ static inline esp_err_t esp_netif_lwip_ipc_call_fn(esp_netif_api_fn fn, esp_neti return esp_netif_lwip_ipc_call_msg(&msg); } +static inline esp_err_t esp_netif_lwip_ipc_call_get_netif(esp_netif_api_fn fn, esp_netif_t **netif, void *ctx) +{ + esp_netif_api_msg_t msg = { + .p_esp_netif = netif, + .data = ctx, + .api_fn = fn + }; + return esp_netif_lwip_ipc_call_msg(&msg); +} + static inline esp_err_t esp_netif_lwip_ipc_no_args(esp_netif_api_fn fn) { esp_netif_api_msg_t msg = { @@ -364,7 +352,6 @@ static esp_err_t esp_netif_update_default_netif_lwip(esp_netif_api_msg_t *msg) case ESP_NETIF_STOPPED: { s_last_default_esp_netif = NULL; - esp_netif_list_lock(); esp_netif_t *netif = esp_netif_next_unsafe(NULL); while (netif) { if (esp_netif_is_netif_up(netif)) { @@ -379,7 +366,6 @@ static esp_err_t esp_netif_update_default_netif_lwip(esp_netif_api_msg_t *msg) } netif = esp_netif_next_unsafe(netif); } - esp_netif_list_unlock(); if (s_last_default_esp_netif && esp_netif_is_netif_up(s_last_default_esp_netif)) { esp_netif_set_default_netif_internal(s_last_default_esp_netif); } @@ -519,6 +505,13 @@ void* esp_netif_get_netif_impl(esp_netif_t *esp_netif) static void tcpip_init_done(void *arg) { sys_sem_t *init_sem = arg; + +#if LWIP_ESP_NETIF_DATA + if (lwip_netif_client_id == 0xFF) { + lwip_netif_client_id = netif_alloc_client_data_id(); + } +#endif + sys_sem_signal(init_sem); } @@ -570,11 +563,6 @@ esp_err_t esp_netif_init(void) } #endif -#if LWIP_ESP_NETIF_DATA - if (lwip_netif_client_id == 0xFF) { - esp_netif_lwip_ipc_call(alloc_client_data_id, NULL, &lwip_netif_client_id); - } -#endif ESP_LOGD(TAG, "esp-netif has been successfully initialized"); return ESP_OK; } @@ -698,15 +686,16 @@ esp_err_t esp_netif_tcpip_exec(esp_netif_callback_fn fn, void*ctx) return esp_netif_lwip_ipc_call_fn(tcpip_exec_api, fn, ctx); } -esp_netif_t *esp_netif_new(const esp_netif_config_t *esp_netif_config) +static esp_err_t esp_netif_new_api(esp_netif_api_msg_t *msg) { + const esp_netif_config_t *esp_netif_config = msg->data; // mandatory configuration must be provided when creating esp_netif object if (esp_netif_config == NULL || esp_netif_config->base->if_key == NULL || - NULL != esp_netif_get_handle_from_ifkey(esp_netif_config->base->if_key)) { + NULL != esp_netif_get_handle_from_ifkey_unsafe(esp_netif_config->base->if_key)) { ESP_LOGE(TAG, "%s: Failed to configure netif with config=%p (config or if_key is NULL or duplicate key)", __func__, esp_netif_config); - return NULL; + return ESP_FAIL; } #if ESP_DHCPS @@ -715,7 +704,7 @@ esp_netif_t *esp_netif_new(const esp_netif_config_t *esp_netif_config) (esp_netif_config->base->flags & ESP_NETIF_DHCP_CLIENT)) { ESP_LOGE(TAG, "%s: Failed to configure netif with config=%p (DHCP server and client cannot be configured together)", __func__, esp_netif_config); - return NULL; + return ESP_FAIL; } #endif @@ -724,7 +713,7 @@ esp_netif_t *esp_netif_new(const esp_netif_config_t *esp_netif_config) if (!esp_netif) { ESP_LOGE(TAG, "Failed to allocate %" PRIu32 " bytes (free heap size %" PRIu32 ")", (uint32_t)sizeof(struct esp_netif_obj), esp_get_free_heap_size()); - return NULL; + return ESP_FAIL; } // Create ip info @@ -733,7 +722,7 @@ esp_netif_t *esp_netif_new(const esp_netif_config_t *esp_netif_config) ESP_LOGE(TAG, "Failed to allocate %" PRIu32 " bytes (free heap size %" PRIu32 ")", (uint32_t)sizeof(esp_netif_ip_info_t), esp_get_free_heap_size()); free(esp_netif); - return NULL; + return ESP_FAIL; } esp_netif->ip_info = ip_info; @@ -744,19 +733,11 @@ esp_netif_t *esp_netif_new(const esp_netif_config_t *esp_netif_config) esp_get_free_heap_size()); free(esp_netif->ip_info); free(esp_netif); - return NULL; + return ESP_FAIL; } esp_netif->ip_info_old = ip_info; // Create underlying lwip netif -#if LWIP_ESP_NETIF_DATA - // Optionally allocate netif client data for esp-netif ptr - // to allow for running esp_netif_new() before esp_netif_init() - if (lwip_netif_client_id == 0xFF) { - esp_netif_lwip_ipc_call(alloc_client_data_id, NULL, &lwip_netif_client_id); - } -#endif - struct netif * lwip_netif = calloc(1, sizeof(struct netif)); if (!lwip_netif) { ESP_LOGE(TAG, "Failed to allocate %" PRIu32 " bytes (free heap size %" PRIu32 ")", (uint32_t)sizeof(struct netif), @@ -764,12 +745,12 @@ esp_netif_t *esp_netif_new(const esp_netif_config_t *esp_netif_config) free(esp_netif->ip_info_old); free(esp_netif->ip_info); free(esp_netif); - return NULL; + return ESP_FAIL; } esp_netif->lwip_netif = lwip_netif; - esp_netif_add_to_list(esp_netif); + esp_netif_add_to_list_unsafe(esp_netif); #if ESP_DHCPS // Create DHCP server structure @@ -777,8 +758,9 @@ esp_netif_t *esp_netif_new(const esp_netif_config_t *esp_netif_config) esp_netif->dhcps = dhcps_new(); if (esp_netif->dhcps == NULL) { ESP_LOGE(TAG, "Failed to create dhcp server handle"); - esp_netif_destroy(esp_netif); - return NULL; + esp_netif_api_msg_t to_destroy = { .esp_netif = esp_netif}; + esp_netif_destroy_api(&to_destroy); + return ESP_FAIL; } } #endif @@ -787,16 +769,38 @@ esp_netif_t *esp_netif_new(const esp_netif_config_t *esp_netif_config) esp_err_t ret = esp_netif_init_configuration(esp_netif, esp_netif_config); if (ret != ESP_OK) { ESP_LOGE(TAG, "Initial configuration of esp_netif failed with %d", ret); - esp_netif_destroy(esp_netif); - return NULL; + esp_netif_api_msg_t to_destroy = { .esp_netif = esp_netif}; + esp_netif_destroy_api(&to_destroy); + return ESP_FAIL; } lwip_set_esp_netif(lwip_netif, esp_netif); if (netif_callback.callback_fn == NULL ) { - esp_netif_lwip_ipc_no_args(set_lwip_netif_callback); + netif_add_ext_callback(&netif_callback, netif_callback_fn); } - return esp_netif; + *msg->p_esp_netif = esp_netif; + return ESP_OK; +} + +esp_netif_t *esp_netif_new(const esp_netif_config_t *config) +{ + esp_netif_t *netif = NULL; + esp_netif_lwip_ipc_call_get_netif(esp_netif_new_api, &netif, (void *)config); + return netif; +} + +static esp_err_t get_handle_from_ifkey_api(esp_netif_api_msg_t *msg) +{ + *msg->p_esp_netif = esp_netif_get_handle_from_ifkey_unsafe(msg->data); + return ESP_OK; +} + +esp_netif_t *esp_netif_get_handle_from_ifkey(const char *if_key) +{ + esp_netif_t *netif = NULL; + esp_netif_lwip_ipc_call_get_netif(get_handle_from_ifkey_api, &netif, (void*)if_key); + return netif; } static void esp_netif_lwip_remove(esp_netif_t *esp_netif) @@ -887,33 +891,36 @@ static void esp_netif_destroy_related(esp_netif_t *esp_netif) } } -static esp_err_t esp_netif_lwip_remove_api(esp_netif_api_msg_t *msg) +static esp_err_t esp_netif_destroy_api(esp_netif_api_msg_t *msg) { - esp_netif_lwip_remove(msg->esp_netif); + esp_netif_t *esp_netif = msg->esp_netif; + esp_netif_remove_from_list_unsafe(esp_netif); + if (esp_netif_get_nr_of_ifs() == 0) { + netif_remove_ext_callback(&netif_callback); + netif_callback.callback_fn = NULL; + } + free(esp_netif->ip_info); + free(esp_netif->ip_info_old); + free(esp_netif->if_key); + free(esp_netif->if_desc); + esp_netif_lwip_remove(esp_netif); + esp_netif_destroy_related(esp_netif); + free(esp_netif->lwip_netif); + free(esp_netif->hostname); + esp_netif_update_default_netif(esp_netif, ESP_NETIF_STOPPED); +#if ESP_DHCPS + dhcps_delete(esp_netif->dhcps); +#endif + free(esp_netif); return ESP_OK; } void esp_netif_destroy(esp_netif_t *esp_netif) { - if (esp_netif) { - esp_netif_remove_from_list(esp_netif); - if (esp_netif_get_nr_of_ifs() == 0) { - esp_netif_lwip_ipc_no_args(remove_lwip_netif_callback); - } - free(esp_netif->ip_info); - free(esp_netif->ip_info_old); - free(esp_netif->if_key); - free(esp_netif->if_desc); - esp_netif_lwip_ipc_call(esp_netif_lwip_remove_api, esp_netif, NULL); - esp_netif_destroy_related(esp_netif); - free(esp_netif->lwip_netif); - free(esp_netif->hostname); - esp_netif_update_default_netif(esp_netif, ESP_NETIF_STOPPED); -#if ESP_DHCPS - dhcps_delete(esp_netif->dhcps); -#endif - free(esp_netif); + if (esp_netif == NULL) { + return; } + esp_netif_lwip_ipc_call(esp_netif_destroy_api, esp_netif, NULL); } esp_err_t esp_netif_attach(esp_netif_t *esp_netif, esp_netif_iodriver_handle driver_handle) @@ -1034,6 +1041,15 @@ static esp_err_t esp_netif_start_api(esp_netif_api_msg_t *msg) esp_netif_t * esp_netif = msg->esp_netif; ESP_LOGD(TAG, "%s %p", __func__, esp_netif); + if (ESP_NETIF_IS_POINT2POINT_TYPE(esp_netif, PPP_LWIP_NETIF)) { +#if CONFIG_PPP_SUPPORT + esp_err_t ret = esp_netif_start_ppp(esp_netif); + if (ret == ESP_OK) { + esp_netif_update_default_netif(esp_netif, ESP_NETIF_STARTED); + } + return ret; +#endif + } ESP_ERROR_CHECK(esp_netif_config_sanity_check(esp_netif)); @@ -1116,16 +1132,6 @@ static esp_err_t esp_netif_start_api(esp_netif_api_msg_t *msg) esp_err_t esp_netif_start(esp_netif_t *esp_netif) { - if (ESP_NETIF_IS_POINT2POINT_TYPE(esp_netif, PPP_LWIP_NETIF)) { -#if CONFIG_PPP_SUPPORT - // No need to start PPP interface in lwip thread - esp_err_t ret = esp_netif_start_ppp(esp_netif); - if (ret == ESP_OK) { - esp_netif_update_default_netif(esp_netif, ESP_NETIF_STARTED); - } - return ret; -#endif - } return esp_netif_lwip_ipc_call(esp_netif_start_api, esp_netif, NULL); } @@ -1133,6 +1139,16 @@ static esp_err_t esp_netif_stop_api(esp_netif_api_msg_t *msg) { esp_netif_t *esp_netif = msg->esp_netif; + if (ESP_NETIF_IS_POINT2POINT_TYPE(esp_netif, PPP_LWIP_NETIF)) { +#if CONFIG_PPP_SUPPORT + esp_err_t ret = esp_netif_stop_ppp(esp_netif->related_data); + if (ret == ESP_OK) { + esp_netif_update_default_netif(esp_netif, ESP_NETIF_STOPPED); + } + return ret; +#endif + } + struct netif *lwip_netif = esp_netif->lwip_netif; if (lwip_netif == NULL) { return ESP_ERR_ESP_NETIF_IF_NOT_READY; @@ -1175,16 +1191,6 @@ static esp_err_t esp_netif_stop_api(esp_netif_api_msg_t *msg) esp_err_t esp_netif_stop(esp_netif_t *esp_netif) { - if (ESP_NETIF_IS_POINT2POINT_TYPE(esp_netif, PPP_LWIP_NETIF)) { -#if CONFIG_PPP_SUPPORT - // No need to stop PPP interface in lwip thread - esp_err_t ret = esp_netif_stop_ppp(esp_netif->related_data); - if (ret == ESP_OK) { - esp_netif_update_default_netif(esp_netif, ESP_NETIF_STOPPED); - } - return ret; -#endif - } return esp_netif_lwip_ipc_call(esp_netif_stop_api, esp_netif, NULL); } @@ -2398,11 +2404,11 @@ esp_err_t esp_netif_get_netif_impl_name(esp_netif_t *esp_netif, char* name) return esp_netif_lwip_ipc_call(esp_netif_get_netif_impl_name_api, esp_netif, name); } -esp_err_t esp_netif_napt_enable(esp_netif_t *esp_netif) +#if IP_NAPT +static esp_err_t esp_netif_napt_control_api(esp_netif_api_msg_t *msg) { -#if !IP_NAPT - return ESP_ERR_NOT_SUPPORTED; -#else + bool enable = (bool)msg->data; + esp_netif_t *esp_netif = msg->esp_netif; ESP_LOGD(TAG, "%s esp_netif:%p", __func__, esp_netif); /* Check if the interface is up */ @@ -2410,44 +2416,74 @@ esp_err_t esp_netif_napt_enable(esp_netif_t *esp_netif) return ESP_FAIL; } - esp_netif_list_lock(); - /* Disable napt on all other interface */ - esp_netif_t *netif = esp_netif_next_unsafe(NULL); - while (netif) { - if (netif != esp_netif) { - ip_napt_enable_netif(netif->lwip_netif, 0); // Fails only if netif is down - ESP_LOGW(TAG, "napt disabled on esp_netif:%p", esp_netif); + if (enable) { + /* Disable napt on all other interface */ + esp_netif_t *netif = esp_netif_next_unsafe(NULL); + while (netif) { + if (netif != esp_netif) { + ip_napt_enable_netif(netif->lwip_netif, 0); // Fails only if netif is down + ESP_LOGW(TAG, "napt disabled on esp_netif:%p", esp_netif); + } + netif = esp_netif_next_unsafe(netif); } - netif = esp_netif_next_unsafe(netif); + + int ret = ip_napt_enable_netif(esp_netif->lwip_netif, 1); + + if (ret == 0) { + return ESP_FAIL; + } + return ESP_OK; + } else { + ip_napt_enable_netif(esp_netif->lwip_netif, 0); // Fails only if netif is down + ESP_LOGD(TAG, "napt disabled on esp_netif:%p", esp_netif); + + return ESP_OK; } +} +#endif // !IP_NAPT - int ret = ip_napt_enable_netif(esp_netif->lwip_netif, 1); - esp_netif_list_unlock(); - - if (ret == 0) { - return ESP_FAIL; - } - - return ESP_OK; +esp_err_t esp_netif_napt_enable(esp_netif_t *esp_netif) +{ +#if !IP_NAPT + return ESP_ERR_NOT_SUPPORTED; +#else + return esp_netif_lwip_ipc_call(esp_netif_napt_control_api, esp_netif, (void*)true /* Enable */); #endif /* IP_NAPT */ } +typedef struct { + u8_t authtype; + const char *user; + const char *passwd; +} set_auth_msg_t; + +static esp_err_t esp_netif_ppp_set_auth_api(esp_netif_api_msg_t *msg) +{ + set_auth_msg_t *params = msg->data; + return esp_netif_ppp_set_auth_internal(msg->esp_netif, params->authtype, params->user, params->passwd); +} + +esp_err_t esp_netif_ppp_set_auth(esp_netif_t *esp_netif, esp_netif_auth_type_t authtype, const char *user, const char *passwd) +{ + set_auth_msg_t msg = { .authtype = authtype, .user = user, .passwd = passwd }; + return esp_netif_lwip_ipc_call(esp_netif_ppp_set_auth_api, esp_netif, &msg); +#if PPP_AUTH_SUPPORT + lwip_peer2peer_ctx_t *ppp_ctx = (lwip_peer2peer_ctx_t *)netif->related_data; + assert(ppp_ctx->base.netif_type == PPP_LWIP_NETIF); + pppapi_set_auth(ppp_ctx->ppp, authtype, user, passwd); + return ESP_OK; +#else + ESP_LOGE(TAG, "%s failed: No authorisation enabled in menuconfig", __func__); + return ESP_ERR_ESP_NETIF_IF_NOT_READY; +#endif +} + esp_err_t esp_netif_napt_disable(esp_netif_t *esp_netif) { #if !IP_NAPT return ESP_ERR_NOT_SUPPORTED; #else - /* Check if the interface is up */ - if (!netif_is_up(esp_netif->lwip_netif)) { - return ESP_FAIL; - } - - esp_netif_list_lock(); - ip_napt_enable_netif(esp_netif->lwip_netif, 0); // Fails only if netif is down - ESP_LOGD(TAG, "napt disabled on esp_netif:%p", esp_netif); - esp_netif_list_unlock(); - - return ESP_OK; + return esp_netif_lwip_ipc_call(esp_netif_napt_control_api, esp_netif, (void*)false /* Disable */); #endif /* IP_NAPT */ } diff --git a/components/esp_netif/lwip/esp_netif_lwip_internal.h b/components/esp_netif/lwip/esp_netif_lwip_internal.h index 5f749c0306..f1fc1f8bf8 100644 --- a/components/esp_netif/lwip/esp_netif_lwip_internal.h +++ b/components/esp_netif/lwip/esp_netif_lwip_internal.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2023 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -23,9 +23,10 @@ typedef struct esp_netif_api_msg_s { int ret; esp_netif_api_fn api_fn; union { - esp_netif_t *esp_netif; - esp_netif_callback_fn user_fn; - }; + esp_netif_t *esp_netif; /* esp_netif as input param */ + esp_netif_t **p_esp_netif; /* esp_netif as output */ + esp_netif_callback_fn user_fn; /* user callback */ + }; /* Commonly used parameters what calling api_fn */ void *data; } esp_netif_api_msg_t; diff --git a/components/esp_netif/lwip/esp_netif_lwip_ppp.c b/components/esp_netif/lwip/esp_netif_lwip_ppp.c index 4bf697bc27..25a3db6ef1 100644 --- a/components/esp_netif/lwip/esp_netif_lwip_ppp.c +++ b/components/esp_netif/lwip/esp_netif_lwip_ppp.c @@ -8,7 +8,6 @@ #include "esp_netif.h" #include "lwip/dns.h" -#include "netif/ppp/pppapi.h" #include "netif/ppp/pppos.h" #include "esp_log.h" #include "esp_netif_net_stack.h" @@ -34,29 +33,6 @@ typedef struct lwip_peer2peer_ctx { ppp_pcb *ppp; } lwip_peer2peer_ctx_t; -#if PPP_SUPPORT && PPP_AUTH_SUPPORT -typedef struct { - struct tcpip_api_call_data call; - ppp_pcb *ppp; - u8_t authtype; - const char *user; - const char *passwd; -} set_auth_msg_t; - -static err_t pppapi_do_ppp_set_auth(struct tcpip_api_call_data *m) -{ - set_auth_msg_t *msg = (set_auth_msg_t *)m; - ppp_set_auth(msg->ppp, msg->authtype, msg->user, msg->passwd); - return ERR_OK; -} - -static void pppapi_set_auth(ppp_pcb *pcb, u8_t authtype, const char *user, const char *passwd) -{ - set_auth_msg_t msg = { .ppp = pcb, .authtype = authtype, .user = user, .passwd = passwd}; - tcpip_api_call(pppapi_do_ppp_set_auth, &msg.call); -} -#endif // PPP_SUPPORT && PPP_AUTH_SUPPORT - /** * @brief lwip callback from PPP client used here to produce PPP error related events, * as well as some IP events @@ -199,15 +175,14 @@ static uint32_t pppos_low_level_output(ppp_pcb *pcb, uint8_t *data, uint32_t len return 0; } -esp_err_t esp_netif_ppp_set_auth(esp_netif_t *netif, esp_netif_auth_type_t authtype, const char *user, const char *passwd) +esp_err_t esp_netif_ppp_set_auth_internal(esp_netif_t *netif, esp_netif_auth_type_t authtype, const char *user, const char *passwd) { if (!ESP_NETIF_IS_POINT2POINT_TYPE(netif, PPP_LWIP_NETIF)) { return ESP_ERR_ESP_NETIF_INVALID_PARAMS; } #if PPP_AUTH_SUPPORT lwip_peer2peer_ctx_t *ppp_ctx = (lwip_peer2peer_ctx_t *)netif->related_data; - assert(ppp_ctx->base.netif_type == PPP_LWIP_NETIF); - pppapi_set_auth(ppp_ctx->ppp, authtype, user, passwd); + ppp_set_auth(ppp_ctx->ppp, authtype, user, passwd); return ESP_OK; #else ESP_LOGE(TAG, "%s failed: No authorisation enabled in menuconfig", __func__); @@ -235,10 +210,11 @@ netif_related_data_t * esp_netif_new_ppp(esp_netif_t *esp_netif, const esp_netif ppp_obj->base.is_point2point = true; ppp_obj->base.netif_type = PPP_LWIP_NETIF; - ppp_obj->ppp = pppapi_pppos_create(netif_impl, pppos_low_level_output, on_ppp_status_changed, esp_netif); + ppp_obj->ppp = pppos_create(netif_impl, pppos_low_level_output, on_ppp_status_changed, esp_netif); ESP_LOGD(TAG, "%s: PPP connection created: %p", __func__, ppp_obj->ppp); if (!ppp_obj->ppp) { ESP_LOGE(TAG, "%s: lwIP PPP connection cannot be created", __func__); + return NULL; } // Set the related data here, since the phase callback could be triggered before this function exits @@ -258,7 +234,7 @@ esp_err_t esp_netif_start_ppp(esp_netif_t *esp_netif) assert(ppp_ctx->base.netif_type == PPP_LWIP_NETIF); ESP_LOGD(TAG, "%s: Starting PPP connection: %p", __func__, ppp_ctx->ppp); - esp_err_t err = pppapi_connect(ppp_ctx->ppp, 0); + err_t err = ppp_connect(ppp_ctx->ppp, 0); if (err != ESP_OK) { ESP_LOGE(TAG, "%s: PPP connection cannot be started", __func__); if (ppp_ctx->ppp_error_event_enabled) { @@ -285,9 +261,9 @@ esp_err_t esp_netif_stop_ppp(netif_related_data_t *netif_related) lwip_peer2peer_ctx_t *ppp_ctx = (lwip_peer2peer_ctx_t *)netif_related; assert(ppp_ctx->base.netif_type == PPP_LWIP_NETIF); ESP_LOGD(TAG, "%s: Stopped PPP connection: %p", __func__, ppp_ctx->ppp); - err_t ret = pppapi_close(ppp_ctx->ppp, 0); + err_t ret = ppp_close(ppp_ctx->ppp, 0); if (ret != ERR_OK) { - ESP_LOGE(TAG, "pppapi_close failed with %d", ret); + ESP_LOGE(TAG, "ppp_close failed with %d", ret); return ESP_FAIL; } return ESP_OK; @@ -298,7 +274,7 @@ void esp_netif_destroy_ppp(netif_related_data_t *netif_related) lwip_peer2peer_ctx_t *ppp_ctx = (lwip_peer2peer_ctx_t *)netif_related; assert(ppp_ctx->base.netif_type == PPP_LWIP_NETIF); - pppapi_free(ppp_ctx->ppp); + ppp_free(ppp_ctx->ppp); free(netif_related); } diff --git a/components/esp_netif/lwip/esp_netif_lwip_ppp.h b/components/esp_netif/lwip/esp_netif_lwip_ppp.h index 9fb2be025d..035dc03fad 100644 --- a/components/esp_netif/lwip/esp_netif_lwip_ppp.h +++ b/components/esp_netif/lwip/esp_netif_lwip_ppp.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2019-2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2019-2023 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -9,6 +9,7 @@ /** * @brief Creates new PPP related structure + * This needs to be called withing lwIP context * * @param[in] esp_netif pointer esp-netif instance * @param[in] stack_config TCP/IP stack configuration structure @@ -21,6 +22,7 @@ netif_related_data_t * esp_netif_new_ppp(esp_netif_t *esp_netif, const esp_netif /** * @brief Creates new PPP related structure + * This needs to be called withing lwIP context * * @param[in] esp_netif pointer esp-netif instance * @@ -44,6 +46,7 @@ esp_netif_recv_ret_t esp_netif_lwip_ppp_input(void *ppp, void *buffer, size_t le /** * @brief Destroys the ppp netif object + * This needs to be called withing lwIP context * * @param[in] netif_related pointer to internal ppp context instance */ @@ -51,6 +54,7 @@ void esp_netif_destroy_ppp(netif_related_data_t *netif_related); /** * @brief Stops the PPP interface + * This needs to be called withing lwIP context * * @param[in] netif_related pointer to internal ppp context instance * @@ -61,11 +65,19 @@ esp_err_t esp_netif_stop_ppp(netif_related_data_t *netif_related); /** * @brief Sets default netif for routing priority config + * This needs to be called withing lwIP context * * @note: This function must be called from lwip thread * */ void esp_netif_ppp_set_default_netif(netif_related_data_t *netif_related); +/** + * @brief Set PPP auth internal version (TCPIP context must be locked) + * This needs to be called withing lwIP context + * + * For params/return value description, please @refitem esp_netif_ppp_set_auth() + */ +esp_err_t esp_netif_ppp_set_auth_internal(esp_netif_t *netif, esp_netif_auth_type_t authtype, const char *user, const char *passwd); #endif // _ESP_NETIF_LWIP_PPP_H_ diff --git a/components/esp_netif/private_include/esp_netif_private.h b/components/esp_netif/private_include/esp_netif_private.h index b7fcfa6ef1..f4f76815b2 100644 --- a/components/esp_netif/private_include/esp_netif_private.h +++ b/components/esp_netif/private_include/esp_netif_private.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2023 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -82,7 +82,9 @@ esp_err_t esp_netif_down(esp_netif_t *esp_netif); bool esp_netif_is_valid_static_ip(esp_netif_ip_info_t *ip_info); /** - * @brief Adds created interface to the list of netifs + * @brief Adds created interface to the list of netifs. + * This function doesn't lock the list, so you need to call esp_netif_list_lock/unlock + * manually before and after the call. * * @param[in] esp_netif Handle to esp-netif instance * @@ -90,10 +92,12 @@ bool esp_netif_is_valid_static_ip(esp_netif_ip_info_t *ip_info); * - ESP_OK -- Success * - ESP_ERR_NO_MEM -- Cannot be added due to memory allocation failure */ -esp_err_t esp_netif_add_to_list(esp_netif_t* netif); +esp_err_t esp_netif_add_to_list_unsafe(esp_netif_t* netif); /** * @brief Removes interface to be destroyed from the list of netifs + * This function doesn't lock the list, so you need to call esp_netif_list_lock/unlock + * manually before and after the call. * * @param[in] esp_netif Handle to esp-netif instance * @@ -101,7 +105,7 @@ esp_err_t esp_netif_add_to_list(esp_netif_t* netif); * - ESP_OK -- Success * - ESP_ERR_NOT_FOUND -- This netif was not found in the netif list */ -esp_err_t esp_netif_remove_from_list(esp_netif_t* netif); +esp_err_t esp_netif_remove_from_list_unsafe(esp_netif_t* netif); /** * @brief Iterates over list of interfaces without list locking. Returns first netif if NULL given as parameter @@ -165,4 +169,13 @@ esp_err_t esp_netif_add_ip6_address(esp_netif_t *esp_netif, const ip_event_add_i */ esp_err_t esp_netif_remove_ip6_address(esp_netif_t *esp_netif, const esp_ip6_addr_t *addr); +/** + * @brief Get esp_netif handle based on the if_key + * This doesn't lock the list nor TCPIP context + * + * @param if_key + * @return esp_netif handle if found, NULL otherwise + */ +esp_netif_t *esp_netif_get_handle_from_ifkey_unsafe(const char *if_key); + #endif //_ESP_NETIF_PRIVATE_H_