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

@@ -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 <string.h>
 
@@ -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;
 }

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

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,22 +1132,22 @@ 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)
 {
+    return esp_netif_lwip_ipc_call(esp_netif_start_api, esp_netif, NULL);
+}
+
+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
-        // No need to start PPP interface in lwip thread
-        esp_err_t ret = esp_netif_start_ppp(esp_netif);
+        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_STARTED);
+            esp_netif_update_default_netif(esp_netif, ESP_NETIF_STOPPED);
         }
         return ret;
 #endif
     }
-    return esp_netif_lwip_ipc_call(esp_netif_start_api, esp_netif, NULL);
-}
-
-static esp_err_t esp_netif_stop_api(esp_netif_api_msg_t *msg)
-{
-    esp_netif_t *esp_netif = msg->esp_netif;
 
     struct netif *lwip_netif = esp_netif->lwip_netif;
     if (lwip_netif == NULL) {
@@ -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);
-    esp_netif_list_unlock();
+        int ret = ip_napt_enable_netif(esp_netif->lwip_netif, 1);
 
-    if (ret == 0) {
-        return ESP_FAIL;
-    }
+        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 */
+        return ESP_OK;
+    }
 }
+#endif // !IP_NAPT
 
-esp_err_t esp_netif_napt_disable(esp_netif_t *esp_netif)
+esp_err_t esp_netif_napt_enable(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;
-    }
+    return esp_netif_lwip_ipc_call(esp_netif_napt_control_api, esp_netif, (void*)true /* Enable */);
+#endif /* IP_NAPT */
+}
 
-    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();
+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
+    return esp_netif_lwip_ipc_call(esp_netif_napt_control_api, esp_netif, (void*)false /* Disable */);
 #endif /* IP_NAPT */
 }
 

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;
 

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);
 }
 

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_

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_