esp_netif/lwip: Fix core-locking config (v4.4)

* Fix thread safety issues in non-core locking
* Add option to verify thread safety issues in lwip (core-lock assertion)
* Make esp_sntp.h thread safe API
* Fix sntp example
(v4.4 - fixes minor merge issue that duplicated
DHCP_DEFINE_CUSTOM_TIMEOUTS and DHCP_REQUEST_TIMEOUT_SEQUENCE)

Closes https://github.com/espressif/esp-idf/issues/9908
Closes https://github.com/espressif/esp-idf/issues/10502
Closes https://github.com/espressif/esp-idf/issues/10466

components/esp_netif/include/esp_netif.h

@@ -906,6 +906,27 @@ void esp_netif_netstack_buf_ref(void *netstack_buf);
  */
 void esp_netif_netstack_buf_free(void *netstack_buf);
 
+/**
+ * @}
+ */
+
+/** @addtogroup ESP_NETIF_TCPIP_EXEC
+ * @{
+ */
+
+/**
+ * @brief  TCPIP thread safe callback used with esp_netif_tcpip_exec()
+ */
+typedef esp_err_t (*esp_netif_callback_fn)(void *ctx);
+
+/**
+ * @brief Utility to execute the supplied callback in TCP/IP context
+ * @param fn Pointer to the callback
+ * @param ctx Parameter to the callback
+ * @return The error code (esp_err_t) returned by the callback
+ */
+esp_err_t esp_netif_tcpip_exec(esp_netif_callback_fn fn, void *ctx);
+
 /**
  * @}
  */

components/esp_netif/lwip/esp_netif_lwip.c

@@ -90,16 +90,14 @@ typedef enum esp_netif_action {
 //
 //  Internal variables for this module
 //
-extern sys_thread_t g_lwip_task;
-
 static const char *TAG = "esp_netif_lwip";
 
-static bool tcpip_initialized = false;
 static esp_netif_t *s_last_default_esp_netif = NULL;
 
 #if !LWIP_TCPIP_CORE_LOCKING
 static sys_sem_t api_sync_sem = NULL;
 static sys_sem_t api_lock_sem = NULL;
+#endif
 
 /**
  * @brief Api callback from tcpip thread used to call esp-netif
@@ -116,10 +114,11 @@ static void esp_netif_api_cb(void *api_msg)
 
     msg->ret = msg->api_fn(msg);
     ESP_LOGD(TAG, "call api in lwip: ret=0x%x, give sem", msg->ret);
+#if !LWIP_TCPIP_CORE_LOCKING
     sys_sem_signal(&api_sync_sem);
-
-}
 #endif
+}
+
 
 
 #if LWIP_IPV6
@@ -131,24 +130,49 @@ static void netif_unset_mldv6_flag(struct netif *netif);
  * @brief Initiates a tcpip remote call if called from another task
  * or calls the function directly if executed from lwip task
  */
-static inline esp_err_t esp_netif_lwip_ipc_call(esp_netif_api_fn fn, esp_netif_t *netif, void *data)
+static inline esp_err_t esp_netif_lwip_ipc_call_msg(esp_netif_api_msg_t *msg)
+{
+    if (!sys_thread_tcpip(LWIP_CORE_LOCK_QUERY_HOLDER)) {
+        ESP_LOGD(TAG, "check: remote, if=%p fn=%p\n", msg->esp_netif, msg->api_fn);
+#if LWIP_TCPIP_CORE_LOCKING
+        tcpip_send_msg_wait_sem((tcpip_callback_fn)esp_netif_api_cb, msg, NULL);
+#else
+        sys_arch_sem_wait(&api_lock_sem, 0);
+        tcpip_send_msg_wait_sem((tcpip_callback_fn)esp_netif_api_cb, msg, &api_sync_sem);
+        sys_sem_signal(&api_lock_sem);
+#endif /* LWIP_TCPIP_CORE_LOCKING */
+        return msg->ret;
+    }
+    ESP_LOGD(TAG, "check: local, if=%p fn=%p\n",  msg->esp_netif, msg->api_fn);
+    return msg->api_fn(msg);
+}
+
+static inline esp_err_t esp_netif_lwip_ipc_call(esp_netif_api_fn fn, esp_netif_t* netif, void *data)
 {
     esp_netif_api_msg_t msg = {
             .esp_netif = netif,
             .data = data,
             .api_fn = fn
     };
-#if !LWIP_TCPIP_CORE_LOCKING
+    return esp_netif_lwip_ipc_call_msg(&msg);
-    if (g_lwip_task != xTaskGetCurrentTaskHandle()) {
+}
-        ESP_LOGD(TAG, "check: remote, if=%p fn=%p\n", netif, fn);
+
-        sys_arch_sem_wait(&api_lock_sem, 0);
+static inline esp_err_t esp_netif_lwip_ipc_call_fn(esp_netif_api_fn fn, esp_netif_callback_fn user_fn, void *ctx)
-        tcpip_send_msg_wait_sem((tcpip_callback_fn)esp_netif_api_cb, &msg, &api_sync_sem);
+{
-        sys_sem_signal(&api_lock_sem);
+    esp_netif_api_msg_t msg = {
-        return msg.ret;
+            .user_fn = user_fn,
-    }
+            .data = ctx,
-#endif /* !LWIP_TCPIP_CORE_LOCKING */
+            .api_fn = fn
-    ESP_LOGD(TAG, "check: local, if=%p fn=%p\n", netif, fn);
+    };
-    return fn(&msg);
+    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 = {
+            .api_fn = fn
+    };
+    return esp_netif_lwip_ipc_call_msg(&msg);
 }
 
 /**
@@ -308,10 +332,15 @@ void* esp_netif_get_netif_impl(esp_netif_t *esp_netif)
     return NULL;
 }
 
+static void tcpip_init_done(void *arg)
+{
+    sys_sem_t *init_sem = arg;
+    sys_sem_signal(init_sem);
+}
+
 esp_err_t esp_netif_init(void)
 {
-    if (tcpip_initialized == false) {
+    if (!sys_thread_tcpip(LWIP_CORE_IS_TCPIP_INITIALIZED)) {
-        tcpip_initialized = true;
 #if CONFIG_LWIP_HOOK_TCP_ISN_DEFAULT
         uint8_t rand_buf[16];
         /*
@@ -324,7 +353,20 @@ esp_err_t esp_netif_init(void)
         esp_fill_random(rand_buf, sizeof(rand_buf));
         lwip_init_tcp_isn(esp_log_timestamp(), rand_buf);
 #endif
-        tcpip_init(NULL, NULL);
+        sys_sem_t init_sem;
+        if (sys_sem_new(&init_sem, 0) != ERR_OK) {
+            ESP_LOGE(TAG, "esp netif cannot create tcpip_init semaphore");
+            return ESP_FAIL;
+        }
+#if LWIP_TCPIP_CORE_LOCKING
+        /* TCPIP thread is not initialized yet,
+         * pretend that the calling thread is holder
+         * to correctly set up the TCPIP task */
+        sys_thread_tcpip(LWIP_CORE_LOCK_MARK_HOLDER);
+#endif
+        tcpip_init(tcpip_init_done, &init_sem);
+        sys_sem_wait(&init_sem);
+        sys_sem_free(&init_sem);
         ESP_LOGD(TAG, "LwIP stack has been initialized");
     }
 
@@ -350,12 +392,11 @@ esp_err_t esp_netif_init(void)
 
 esp_err_t esp_netif_deinit(void)
 {
-    if (tcpip_initialized == true) {
+    if (sys_thread_tcpip(LWIP_CORE_IS_TCPIP_INITIALIZED)) {
         /* deinit of LwIP not supported:
          * do not deinit semaphores and states,
          * so init could be called multiple times
          *
-        tcpip_initialized = false;
         sys_sem_free(&api_sync_sem);
         sys_sem_free(&api_lock_sem);
          */
@@ -461,6 +502,16 @@ static esp_err_t esp_netif_init_configuration(esp_netif_t *esp_netif, const esp_
     return ESP_OK;
 }
 
+static esp_err_t tcpip_exec_api(esp_netif_api_msg_t *msg)
+{
+    return msg->user_fn(msg->data);
+}
+
+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)
 {
     // mandatory configuration must be provided when creating esp_netif object
@@ -585,6 +636,12 @@ 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)
+{
+    esp_netif_lwip_remove(msg->esp_netif);
+    return ESP_OK;
+}
+
 void esp_netif_destroy(esp_netif_t *esp_netif)
 {
     if (esp_netif) {
@@ -593,7 +650,7 @@ void esp_netif_destroy(esp_netif_t *esp_netif)
         free(esp_netif->ip_info_old);
         free(esp_netif->if_key);
         free(esp_netif->if_desc);
-        esp_netif_lwip_remove(esp_netif);
+        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);
@@ -641,6 +698,15 @@ static esp_err_t esp_netif_reset_ip_info(esp_netif_t *esp_netif)
     return ESP_OK;
 }
 
+esp_err_t esp_netif_set_mac_api(esp_netif_api_msg_t *msg)
+{
+    uint8_t *mac = msg->data;
+    esp_netif_t* esp_netif = msg->esp_netif;
+    memcpy(esp_netif->mac, mac, NETIF_MAX_HWADDR_LEN);
+    memcpy(esp_netif->lwip_netif->hwaddr, mac, NETIF_MAX_HWADDR_LEN);
+    return ESP_OK;
+}
+
 esp_err_t esp_netif_set_mac(esp_netif_t *esp_netif, uint8_t mac[])
 {
     if (esp_netif == NULL || esp_netif->lwip_netif == NULL) {
@@ -649,9 +715,7 @@ esp_err_t esp_netif_set_mac(esp_netif_t *esp_netif, uint8_t mac[])
     if (_IS_NETIF_ANY_POINT2POINT_TYPE(esp_netif)) {
         return ESP_ERR_NOT_SUPPORTED;
     }
-    memcpy(esp_netif->mac, mac, NETIF_MAX_HWADDR_LEN);
+    return esp_netif_lwip_ipc_call(esp_netif_set_mac_api, esp_netif, mac);
-    memcpy(esp_netif->lwip_netif->hwaddr, mac, NETIF_MAX_HWADDR_LEN);
-    return ESP_OK;
 }
 
 esp_err_t esp_netif_get_mac(esp_netif_t *esp_netif, uint8_t mac[])
@@ -1795,70 +1859,79 @@ int32_t esp_netif_get_event_id(esp_netif_t *esp_netif, esp_netif_ip_event_type_t
     }
 }
 
+struct dhcp_params {
+    esp_netif_dhcp_option_mode_t op;
+    esp_netif_dhcp_option_id_t id;
+    void *val;
+    uint32_t len;
+};
+
 #if ESP_DHCPS
-esp_err_t esp_netif_dhcps_option(esp_netif_t *esp_netif, esp_netif_dhcp_option_mode_t opt_op, esp_netif_dhcp_option_id_t opt_id, void *opt_val,
+esp_err_t esp_netif_dhcps_option_api(esp_netif_api_msg_t *msg)
-                                 uint32_t opt_len)
 {
-    void *opt_info = dhcps_option_info(opt_id, opt_len);
+    esp_netif_t *esp_netif = msg->esp_netif;
+    struct dhcp_params *opt = msg->data;
+    void *opt_info = dhcps_option_info(opt->id, opt->len);
+
     if (esp_netif == NULL) {
         return ESP_ERR_ESP_NETIF_IF_NOT_READY;
     }
 
     esp_netif_dhcp_status_t dhcps_status = esp_netif->dhcps_status;
-    if (opt_info == NULL || opt_val == NULL) {
+    if (opt_info == NULL || opt->val == NULL) {
         return ESP_ERR_ESP_NETIF_INVALID_PARAMS;
     }
 
-    if (opt_op == ESP_NETIF_OP_GET) {
+    if (opt->op == ESP_NETIF_OP_GET) {
         if (dhcps_status == ESP_NETIF_DHCP_STOPPED) {
             return ESP_ERR_ESP_NETIF_DHCP_ALREADY_STOPPED;
         }
 
-        switch (opt_id) {
+        switch (opt->id) {
             case IP_ADDRESS_LEASE_TIME: {
-                *(uint32_t *)opt_val = *(uint32_t *)opt_info;
+                *(uint32_t *)opt->val = *(uint32_t *)opt_info;
                 break;
             }
             case ESP_NETIF_SUBNET_MASK:
             case REQUESTED_IP_ADDRESS: {
-                memcpy(opt_val, opt_info, opt_len);
+                memcpy(opt->val, opt_info, opt->len);
                 break;
             }
             case ROUTER_SOLICITATION_ADDRESS: {
                 if ((*(uint8_t *)opt_info) & OFFER_ROUTER) {
-                    *(uint8_t *)opt_val = 1;
+                    *(uint8_t *)opt->val = 1;
                 } else {
-                    *(uint8_t *)opt_val = 0;
+                    *(uint8_t *)opt->val = 0;
                 }
                 break;
             }
             case DOMAIN_NAME_SERVER: {
                 if ((*(uint8_t *)opt_info) & OFFER_DNS) {
-                    *(uint8_t *)opt_val = 1;
+                    *(uint8_t *)opt->val = 1;
                 } else {
-                    *(uint8_t *)opt_val = 0;
+                    *(uint8_t *)opt->val = 0;
                 }
                 break;
             }
             default:
                 break;
         }
-    } else if (opt_op == ESP_NETIF_OP_SET) {
+    } else if (opt->op == ESP_NETIF_OP_SET) {
         if (dhcps_status == ESP_NETIF_DHCP_STARTED) {
             return ESP_ERR_ESP_NETIF_DHCP_ALREADY_STARTED;
         }
 
-        switch (opt_id) {
+        switch (opt->id) {
             case IP_ADDRESS_LEASE_TIME: {
-                if (*(uint32_t *)opt_val != 0) {
+                if (*(uint32_t *)opt->val != 0) {
-                    *(uint32_t *)opt_info = *(uint32_t *)opt_val;
+                    *(uint32_t *)opt_info = *(uint32_t *)opt->val;
                 } else {
                     *(uint32_t *)opt_info = DHCPS_LEASE_TIME_DEF;
                 }
                 break;
             }
             case ESP_NETIF_SUBNET_MASK: {
-                memcpy(opt_info, opt_val, opt_len);
+                memcpy(opt_info, opt->val, opt->len);
                 break;
             }
             case REQUESTED_IP_ADDRESS: {
@@ -1866,7 +1939,7 @@ esp_err_t esp_netif_dhcps_option(esp_netif_t *esp_netif, esp_netif_dhcp_option_m
                 uint32_t softap_ip = 0;
                 uint32_t start_ip = 0;
                 uint32_t end_ip = 0;
-                dhcps_lease_t *poll = opt_val;
+                dhcps_lease_t *poll = opt->val;
 
                 if (poll->enable) {
                     memset(&info, 0x00, sizeof(esp_netif_ip_info_t));
@@ -1893,11 +1966,11 @@ esp_err_t esp_netif_dhcps_option(esp_netif_t *esp_netif, esp_netif_dhcp_option_m
                     }
                 }
 
-                memcpy(opt_info, opt_val, opt_len);
+                memcpy(opt_info, opt->val, opt->len);
                 break;
             }
             case ROUTER_SOLICITATION_ADDRESS: {
-                if (*(uint8_t *)opt_val) {
+                if (*(uint8_t *)opt->val) {
                     *(uint8_t *)opt_info |= OFFER_ROUTER;
                 } else {
                     *(uint8_t *)opt_info &= ((~OFFER_ROUTER) & 0xFF);
@@ -1905,7 +1978,7 @@ esp_err_t esp_netif_dhcps_option(esp_netif_t *esp_netif, esp_netif_dhcp_option_m
                 break;
             }
             case DOMAIN_NAME_SERVER: {
-                if (*(uint8_t *)opt_val) {
+                if (*(uint8_t *)opt->val) {
                     *(uint8_t *)opt_info |= OFFER_DNS;
                 } else {
                     *(uint8_t *)opt_info &= ((~OFFER_DNS) & 0xFF);
@@ -1916,63 +1989,82 @@ esp_err_t esp_netif_dhcps_option(esp_netif_t *esp_netif, esp_netif_dhcp_option_m
             default:
                 break;
         }
-        dhcps_set_option_info(opt_id, opt_info, opt_len);
+        dhcps_set_option_info(opt->id, opt_info, opt->len);
     } else {
         return ESP_ERR_ESP_NETIF_INVALID_PARAMS;
     }
 
     return ESP_OK;
 }
+
+esp_err_t esp_netif_dhcps_option(esp_netif_t *esp_netif, esp_netif_dhcp_option_mode_t opt_op, esp_netif_dhcp_option_id_t opt_id, void *opt_val,
+                                 uint32_t opt_len)
+{
+    if (esp_netif == NULL) {
+        return ESP_ERR_ESP_NETIF_IF_NOT_READY;
+    }
+    struct dhcp_params opts = { .op = opt_op, .id = opt_id, .len = opt_len, .val = opt_val };
+    return esp_netif_lwip_ipc_call(esp_netif_dhcps_option_api, esp_netif, &opts);
+}
 #endif
 
+esp_err_t esp_netif_dhcpc_option_api(esp_netif_api_msg_t *msg)
+{
+    esp_netif_t *esp_netif = msg->esp_netif;
+    struct dhcp_params *opt = msg->data;
+
+    struct dhcp *dhcp = netif_dhcp_data(esp_netif->lwip_netif);
+    if (dhcp == NULL || opt->val == NULL) {
+        return ESP_ERR_ESP_NETIF_INVALID_PARAMS;
+    }
+    if (opt->op == ESP_NETIF_OP_GET) {
+        if (esp_netif->dhcpc_status == ESP_NETIF_DHCP_STOPPED) {
+            return ESP_ERR_ESP_NETIF_DHCP_ALREADY_STOPPED;
+        }
+        switch (opt->id) {
+            case ESP_NETIF_IP_REQUEST_RETRY_TIME:
+                if (opt->len == sizeof(dhcp->tries)) {
+                    *(uint8_t *)opt->val = dhcp->tries;
+                }
+                break;
+#if ESP_DHCP && !ESP_DHCP_DISABLE_VENDOR_CLASS_IDENTIFIER
+            case ESP_NETIF_VENDOR_SPECIFIC_INFO:
+                return dhcp_get_vendor_specific_information(opt->len, opt->val);
+#endif
+            default:
+                return ESP_ERR_ESP_NETIF_INVALID_PARAMS;
+        }
+    } else if (opt->op == ESP_NETIF_OP_SET) {
+        if (esp_netif->dhcpc_status == ESP_NETIF_DHCP_STARTED) {
+            return ESP_ERR_ESP_NETIF_DHCP_ALREADY_STARTED;
+        }
+        switch (opt->id) {
+            case ESP_NETIF_IP_REQUEST_RETRY_TIME:
+                if (opt->len == sizeof(dhcp->tries)) {
+                    dhcp->tries = *(uint8_t *)opt->val;
+                }
+                break;
+#if ESP_DHCP && !ESP_DHCP_DISABLE_VENDOR_CLASS_IDENTIFIER
+            case ESP_NETIF_VENDOR_CLASS_IDENTIFIER:
+                return dhcp_set_vendor_class_identifier(opt->len, opt->val);
+#endif
+            default:
+                return ESP_ERR_ESP_NETIF_INVALID_PARAMS;
+        }
+    } else {
+        return ESP_ERR_ESP_NETIF_INVALID_PARAMS;
+    }
+    return ESP_OK;
+}
+
 esp_err_t esp_netif_dhcpc_option(esp_netif_t *esp_netif, esp_netif_dhcp_option_mode_t opt_op, esp_netif_dhcp_option_id_t opt_id, void *opt_val,
                                  uint32_t opt_len)
 {
     if (esp_netif == NULL || esp_netif->lwip_netif == NULL) {
         return ESP_ERR_ESP_NETIF_IF_NOT_READY;
     }
-    struct dhcp *dhcp = netif_dhcp_data(esp_netif->lwip_netif);
+    struct dhcp_params opts = { .op = opt_op, .id = opt_id, .len = opt_len, .val = opt_val };
-     if (dhcp == NULL || opt_val == NULL) {
+    return esp_netif_lwip_ipc_call(esp_netif_dhcpc_option_api, esp_netif, &opts);
-        return ESP_ERR_ESP_NETIF_INVALID_PARAMS;
-    }
-    if (opt_op == ESP_NETIF_OP_GET) {
-        if (esp_netif->dhcpc_status == ESP_NETIF_DHCP_STOPPED) {
-            return ESP_ERR_ESP_NETIF_DHCP_ALREADY_STOPPED;
-        }
-        switch (opt_id) {
-            case ESP_NETIF_IP_REQUEST_RETRY_TIME:
-                if (opt_len == sizeof(dhcp->tries)) {
-                    *(uint8_t *)opt_val = dhcp->tries;
-                }
-                break;
-#if ESP_DHCP && !ESP_DHCP_DISABLE_VENDOR_CLASS_IDENTIFIER
-            case ESP_NETIF_VENDOR_SPECIFIC_INFO:
-                return dhcp_get_vendor_specific_information(opt_len, opt_val);
-#endif
-            default:
-                return ESP_ERR_ESP_NETIF_INVALID_PARAMS;
-        }
-    } else if (opt_op == ESP_NETIF_OP_SET) {
-        if (esp_netif->dhcpc_status == ESP_NETIF_DHCP_STARTED) {
-            return ESP_ERR_ESP_NETIF_DHCP_ALREADY_STARTED;
-        }
-        switch (opt_id) {
-            case ESP_NETIF_IP_REQUEST_RETRY_TIME:
-                if (opt_len == sizeof(dhcp->tries)) {
-                    dhcp->tries = *(uint8_t *)opt_val;
-                }
-                break;
-#if ESP_DHCP && !ESP_DHCP_DISABLE_VENDOR_CLASS_IDENTIFIER
-            case ESP_NETIF_VENDOR_CLASS_IDENTIFIER:
-                return dhcp_set_vendor_class_identifier(opt_len, opt_val);
-#endif
-            default:
-                return ESP_ERR_ESP_NETIF_INVALID_PARAMS;
-        }
-    } else {
-        return ESP_ERR_ESP_NETIF_INVALID_PARAMS;
-    }
-    return ESP_OK;
 }
 
 int esp_netif_get_netif_impl_index(esp_netif_t *esp_netif)
@@ -1983,6 +2075,13 @@ int esp_netif_get_netif_impl_index(esp_netif_t *esp_netif)
     return netif_get_index(esp_netif->lwip_netif);
 }
 
+esp_err_t esp_netif_get_netif_impl_name_api(esp_netif_api_msg_t *msg)
+{
+    struct netif* netif = msg->esp_netif->lwip_netif;
+    netif_index_to_name(netif_get_index(netif), msg->data);
+    return ESP_OK;
+}
+
 esp_err_t esp_netif_get_netif_impl_name(esp_netif_t *esp_netif, char* name)
 {
     ESP_LOGD(TAG, "%s esp_netif:%p", __func__, esp_netif);
@@ -1990,8 +2089,7 @@ esp_err_t esp_netif_get_netif_impl_name(esp_netif_t *esp_netif, char* name)
     if (esp_netif == NULL || esp_netif->lwip_netif == NULL) {
         return ESP_ERR_ESP_NETIF_INVALID_PARAMS;
     }
-    netif_index_to_name(netif_get_index(esp_netif->lwip_netif), name);
+    return esp_netif_lwip_ipc_call(esp_netif_get_netif_impl_name_api, esp_netif, name);
-    return ESP_OK;
 }
 
 #if CONFIG_LWIP_IPV6

components/esp_netif/lwip/esp_netif_lwip_internal.h

@@ -53,7 +53,10 @@ typedef struct esp_netif_api_msg_s {
     int type;  /**< The first field MUST be int */
     int ret;
     esp_netif_api_fn api_fn;
-    esp_netif_t *esp_netif;
+    union {
+        esp_netif_t *esp_netif;
+        esp_netif_callback_fn user_fn;
+    };
     void    *data;
 } esp_netif_api_msg_t;
 

components/lwip/Kconfig

@@ -19,12 +19,21 @@ menu "LWIP"
         bool "Enable tcpip core locking"
         default n
         help
-            If Enable tcpip　core locking,Creates a global mutex that is held
+            If Enable tcpip core locking,Creates a global mutex that is held
             during TCPIP thread operations.Can be locked by client code to perform
             lwIP operations without changing into TCPIP thread using callbacks.
             See LOCK_TCPIP_CORE() and UNLOCK_TCPIP_CORE().
 
-            If disable tcpip　core locking,TCP IP will perform tasks through context switching．
+            If disable tcpip core locking,TCP IP will perform tasks through context switching
+
+    config LWIP_CHECK_THREAD_SAFETY
+        bool "Checks that lwip API runs in expected context"
+        default n
+        help
+            Enable to check that the project does not violate lwip thread safety.
+            If enabled, all lwip functions that require thread awareness run an assertion
+            to verify that the TCP/IP core functionality is either locked or accessed
+            from the correct thread.
 
     config LWIP_DNS_SUPPORT_MDNS_QUERIES
         bool "Enable mDNS queries in resolving host name"

components/lwip/apps/sntp/sntp.c

@@ -1,27 +1,22 @@
-// Copyright 2015-2019 Espressif Systems (Shanghai) PTE LTD
+/*
-//
+ * SPDX-FileCopyrightText: 2015-2023 Espressif Systems (Shanghai) CO LTD
-// Licensed under the Apache License, Version 2.0 (the "License");
+ *
-// you may not use this file except in compliance with the License.
+ * SPDX-License-Identifier: Apache-2.0
-// You may obtain a copy of the License at
+ */
-
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
 
 #include <stdlib.h>
 #include <time.h>
 #include <unistd.h>
 #include <sys/time.h>
 #include "esp_log.h"
-#include "sntp.h"
+#include "esp_sntp.h"
-#include "lwip/apps/sntp.h"
+#include "lwip/tcpip.h"
 
 static const char *TAG = "sntp";
 
+ESP_STATIC_ASSERT(SNTP_OPMODE_POLL == ESP_SNTP_OPMODE_POLL, "SNTP mode in lwip doesn't match the IDF enum. Please make sure lwIP version is correct");
+ESP_STATIC_ASSERT(SNTP_OPMODE_LISTENONLY == ESP_SNTP_OPMODE_LISTENONLY, "SNTP mode in lwip doesn't match the IDF enum. Please make sure lwIP version is correct");
+
 static volatile sntp_sync_mode_t sntp_sync_mode = SNTP_SYNC_MODE_IMMED;
 static volatile sntp_sync_status_t sntp_sync_status = SNTP_SYNC_STATUS_RESET;
 static sntp_sync_time_cb_t time_sync_notification_cb = NULL;
@@ -108,11 +103,17 @@ uint32_t sntp_get_sync_interval(void)
     return s_sync_interval;
 }
 
+static void sntp_do_restart(void *ctx)
+{
+    (void)ctx;
+    sntp_stop();
+    sntp_init();
+}
+
 bool sntp_restart(void)
 {
     if (sntp_enabled()) {
-        sntp_stop();
+        tcpip_callback(sntp_do_restart, NULL);
-        sntp_init();
         return true;
     }
     return false;
@@ -132,3 +133,98 @@ void sntp_get_system_time(uint32_t *sec, uint32_t *us)
     *(us) = tv.tv_usec;
     sntp_set_sync_status(SNTP_SYNC_STATUS_RESET);
 }
+
+static void do_setoperatingmode(void *ctx)
+{
+    esp_sntp_operatingmode_t operating_mode = (esp_sntp_operatingmode_t)ctx;
+    sntp_setoperatingmode(operating_mode);
+}
+
+void esp_sntp_setoperatingmode(esp_sntp_operatingmode_t operating_mode)
+{
+    tcpip_callback(do_setoperatingmode, (void*)operating_mode);
+}
+
+static void do_init(void *ctx)
+{
+    sntp_init();
+}
+
+void esp_sntp_init(void)
+{
+    tcpip_callback(do_init, NULL);
+}
+
+static void do_stop(void *ctx)
+{
+    sntp_stop();
+}
+
+void esp_sntp_stop(void)
+{
+    tcpip_callback(do_stop, NULL);
+}
+
+struct do_setserver {
+    u8_t idx;
+    const ip_addr_t *addr;
+};
+
+static void do_setserver(void *ctx)
+{
+    struct do_setserver *params = ctx;
+    sntp_setserver(params->idx, params->addr);
+}
+
+void esp_sntp_setserver(u8_t idx, const ip_addr_t *addr)
+{
+    struct do_setserver params = {
+            .idx = idx,
+            .addr = addr
+    };
+    tcpip_callback(do_setserver, &params);
+}
+
+struct do_setservername {
+    u8_t idx;
+    const char *server;
+};
+
+static void do_setservername(void *ctx)
+{
+    struct do_setservername *params = ctx;
+    sntp_setservername(params->idx, params->server);
+}
+
+void esp_sntp_setservername(u8_t idx, const char *server)
+{
+    struct do_setservername params = {
+            .idx = idx,
+            .server = server
+    };
+    tcpip_callback(do_setservername, &params);
+}
+
+const char *esp_sntp_getservername(u8_t idx)
+{
+    return sntp_getservername(idx);
+}
+
+const ip_addr_t* esp_sntp_getserver(u8_t idx)
+{
+    return sntp_getserver(idx);
+}
+
+#if LWIP_DHCP_GET_NTP_SRV
+static void do_servermode_dhcp(void* ctx)
+{
+    u8_t servermode = (bool)ctx ? 1 : 0;
+    sntp_servermode_dhcp(servermode);
+}
+
+void esp_sntp_servermode_dhcp(bool enable)
+{
+    tcpip_callback(do_servermode_dhcp, (void*)enable);
+}
+
+#endif /* LWIP_DHCP_GET_NTP_SRV */

components/lwip/include/apps/esp_sntp.h

@@ -18,7 +18,6 @@
 #include "lwip/err.h"
 #include "lwip/apps/sntp.h"
 
-
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -46,6 +45,17 @@ extern "C" {
  *    to wait for the next sync cycle.
  */
 
+/// Aliases for esp_sntp prefixed API (inherently thread safe)
+#define esp_sntp_sync_time sntp_sync_time
+#define esp_sntp_set_sync_mode sntp_set_sync_mode
+#define esp_sntp_get_sync_mode sntp_get_sync_mode
+#define esp_sntp_get_sync_status sntp_get_sync_status
+#define esp_sntp_set_sync_status sntp_set_sync_status
+#define esp_sntp_set_time_sync_notification_cb sntp_set_time_sync_notification_cb
+#define esp_sntp_set_sync_interval sntp_set_sync_interval
+#define esp_sntp_get_sync_interval sntp_get_sync_interval
+#define esp_sntp_restart sntp_restart
+
 /// SNTP time update mode
 typedef enum {
     SNTP_SYNC_MODE_IMMED,   /*!< Update system time immediately when receiving a response from the SNTP server. */
@@ -59,6 +69,12 @@ typedef enum {
     SNTP_SYNC_STATUS_IN_PROGRESS,   // Smooth time sync in progress.
 } sntp_sync_status_t;
 
+/// SNTP operating modes per lwip SNTP module
+typedef enum {
+    ESP_SNTP_OPMODE_POLL,
+    ESP_SNTP_OPMODE_LISTENONLY,
+} esp_sntp_operatingmode_t;
+
 /**
  * @brief SNTP callback function for notifying about time sync event
  *
@@ -151,6 +167,60 @@ uint32_t sntp_get_sync_interval(void);
  */
 bool sntp_restart(void);
 
+/**
+ * @brief Sets SNTP operating mode. The mode has to be set before init.
+ *
+ * @param operating_mode Desired operating mode
+ */
+void esp_sntp_setoperatingmode(esp_sntp_operatingmode_t operating_mode);
+
+/**
+ * @brief Init and start SNTP service
+ */
+void esp_sntp_init(void);
+
+/**
+ * @brief Stops SNTP service
+ */
+void esp_sntp_stop(void);
+
+/**
+ * @brief Sets SNTP server address
+ *
+ * @param idx Index of the server
+ * @param addr IP address of the server
+ */
+void esp_sntp_setserver(u8_t idx, const ip_addr_t *addr);
+
+/**
+ * @brief Sets SNTP hostname
+ * @param idx Index of the server
+ * @param server Name of the server
+ */
+void esp_sntp_setservername(u8_t idx, const char *server);
+
+/**
+ * @brief Gets SNTP server name
+ * @param idx Index of the server
+ * @return Name of the server
+ */
+const char *esp_sntp_getservername(u8_t idx);
+
+/**
+ * @brief Get SNTP server IP
+ * @param idx Index of the server
+ * @return IP address of the server
+ */
+const ip_addr_t* esp_sntp_getserver(u8_t idx);
+
+#if LWIP_DHCP_GET_NTP_SRV
+/**
+ * @brief Enable acquiring SNTP server from DHCP
+ * @param enable True for enabling SNTP from DHCP
+ */
+void esp_sntp_servermode_dhcp(bool enable);
+#endif /* LWIP_DHCP_GET_NTP_SRV */
+
 #ifdef __cplusplus
 }
 #endif

components/lwip/include/apps/sntp/sntp.h

@@ -1,27 +1,16 @@
-// Copyright 2020 Espressif Systems (Shanghai) PTE LTD
+/*
-//
+ * SPDX-FileCopyrightText: 2020-2022 Espressif Systems (Shanghai) CO LTD
-// Licensed under the Apache License, Version 2.0 (the "License");
+ *
-// you may not use this file except in compliance with the License.
+ * SPDX-License-Identifier: Apache-2.0
-// You may obtain a copy of the License at
+ */
 
-//     http://www.apache.org/licenses/LICENSE-2.0
+#pragma once
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-#ifndef __SNTP_H__
-#define __SNTP_H__
 
+#warning "sntp.h in IDF's lwip port folder is deprecated. Please include esp_sntp.h"
 /*
  * This header is provided only for compatibility reasons for existing
  * applications which directly include "sntp.h" from the IDF default paths.
- * and will be removed in IDF v5.0.
+ * and will be removed in IDF v6.0.
  * It is recommended to use "esp_sntp.h" from IDF's lwip port folder
  */
-
 #include "esp_sntp.h"
-
-#endif // __SNTP_H__

components/lwip/port/esp32/freertos/sys_arch.c

@@ -560,3 +560,36 @@ sys_delay_ms(uint32_t ms)
 {
   vTaskDelay(ms / portTICK_PERIOD_MS);
 }
+
+bool
+sys_thread_tcpip(sys_thread_core_lock_t type)
+{
+    static sys_thread_t lwip_task = NULL;
+#if LWIP_TCPIP_CORE_LOCKING
+    static sys_thread_t core_lock_holder = NULL;
+#endif
+    switch (type) {
+        default:
+            return false;
+        case LWIP_CORE_IS_TCPIP_INITIALIZED:
+            return lwip_task != NULL;
+        case LWIP_CORE_MARK_TCPIP_TASK:
+            LWIP_ASSERT("LWIP_CORE_MARK_TCPIP_TASK: lwip_task == NULL", (lwip_task == NULL));
+            lwip_task = (sys_thread_t) xTaskGetCurrentTaskHandle();
+            return true;
+#if LWIP_TCPIP_CORE_LOCKING
+        case LWIP_CORE_LOCK_QUERY_HOLDER:
+            return lwip_task ? core_lock_holder == (sys_thread_t) xTaskGetCurrentTaskHandle() : true;
+        case LWIP_CORE_LOCK_MARK_HOLDER:
+            core_lock_holder = (sys_thread_t) xTaskGetCurrentTaskHandle();
+            return true;
+        case LWIP_CORE_LOCK_UNMARK_HOLDER:
+            core_lock_holder = NULL;
+            return true;
+#else
+        case LWIP_CORE_LOCK_QUERY_HOLDER:
+            return lwip_task == NULL || lwip_task == (sys_thread_t) xTaskGetCurrentTaskHandle();
+#endif /* LWIP_TCPIP_CORE_LOCKING */
+    }
+    return true;
+}

components/lwip/port/esp32/include/arch/sys_arch.h

@@ -97,6 +97,17 @@ sys_sem_t* sys_thread_sem_init(void);
 void sys_thread_sem_deinit(void);
 sys_sem_t* sys_thread_sem_get(void);
 
+typedef enum {
+    LWIP_CORE_LOCK_QUERY_HOLDER,
+    LWIP_CORE_LOCK_MARK_HOLDER,
+    LWIP_CORE_LOCK_UNMARK_HOLDER,
+    LWIP_CORE_MARK_TCPIP_TASK,
+    LWIP_CORE_IS_TCPIP_INITIALIZED,
+} sys_thread_core_lock_t;
+
+bool
+sys_thread_tcpip(sys_thread_core_lock_t type);
+
 #ifdef __cplusplus
 }
 #endif

components/lwip/port/esp32/include/lwip_default_hooks.h

@@ -18,6 +18,7 @@
 #include "lwip/arch.h"
 #include "lwip/err.h"
 
+
 #ifdef ESP_IDF_LWIP_HOOK_FILENAME
 #include ESP_IDF_LWIP_HOOK_FILENAME
 #endif

components/lwip/port/esp32/include/lwipopts.h

@@ -33,6 +33,31 @@ extern "C"
    ---------- Platform specific locking ----------
    -----------------------------------------------
 */
+/**
+ * LWIP_TCPIP_CORE_LOCKING
+ * Creates a global mutex that is held during TCPIP thread operations.
+ * Can be locked by client code to perform lwIP operations without changing
+ * into TCPIP thread using callbacks. See LOCK_TCPIP_CORE() and
+ * UNLOCK_TCPIP_CORE().
+ * Your system should provide mutexes supporting priority inversion to use this.
+ */
+#ifdef CONFIG_LWIP_TCPIP_CORE_LOCKING
+#define LWIP_TCPIP_CORE_LOCKING         1
+#define LOCK_TCPIP_CORE()     do { sys_mutex_lock(&lock_tcpip_core); sys_thread_tcpip(LWIP_CORE_LOCK_MARK_HOLDER); } while(0)
+#define UNLOCK_TCPIP_CORE()   do { sys_thread_tcpip(LWIP_CORE_LOCK_UNMARK_HOLDER); sys_mutex_unlock(&lock_tcpip_core);  } while(0)
+#ifdef CONFIG_LWIP_CHECK_THREAD_SAFETY
+#define LWIP_ASSERT_CORE_LOCKED() do { LWIP_ASSERT("Required to lock TCPIP core functionality!", sys_thread_tcpip(LWIP_CORE_LOCK_QUERY_HOLDER)); } while(0)
+#endif /* CONFIG_LWIP_CHECK_THREAD_SAFETY */
+
+#else
+#define LWIP_TCPIP_CORE_LOCKING         0
+#ifdef CONFIG_LWIP_CHECK_THREAD_SAFETY
+#define LWIP_ASSERT_CORE_LOCKED()     do { LWIP_ASSERT("Required to run in TCPIP context!", sys_thread_tcpip(LWIP_CORE_LOCK_QUERY_HOLDER)); } while(0)
+#endif /* CONFIG_LWIP_CHECK_THREAD_SAFETY */
+#endif /* CONFIG_LWIP_TCPIP_CORE_LOCKING */
+
+#define LWIP_MARK_TCPIP_THREAD() sys_thread_tcpip(LWIP_CORE_MARK_TCPIP_TASK)
+
 /**
  * SYS_LIGHTWEIGHT_PROT==1: if you want inter-task protection for certain
  * critical regions during buffer allocation, deallocation and memory
@@ -286,14 +311,6 @@ static inline uint32_t timeout_from_offered(uint32_t lease, uint32_t min)
  */
 #define ESP_DHCP_DISABLE_VENDOR_CLASS_IDENTIFIER       CONFIG_LWIP_DHCP_DISABLE_VENDOR_CLASS_ID
 
-#define DHCP_DEFINE_CUSTOM_TIMEOUTS     1
-/* Since for embedded devices it's not that hard to miss a discover packet, so lower
- * the discover retry backoff time from (2,4,8,16,32,60,60)s to (500m,1,2,4,8,15,15)s.
- */
- #define DHCP_REQUEST_TIMEOUT_SEQUENCE(state, tries)   (state == DHCP_STATE_REQUESTING ? \
-                                                       (uint16_t)(1 * 1000) : \
-                                                       (uint16_t)(((tries) < 6 ? 1 << (tries) : 60) * 250))
-
 /*
    ------------------------------------
    ---------- AUTOIP options ----------
@@ -623,11 +640,30 @@ static inline uint32_t timeout_from_offered(uint32_t lease, uint32_t min)
    ---------- Sequential layer options ----------
    ----------------------------------------------
 */
-/**
+
- * LWIP_TCPIP_CORE_LOCKING: (EXPERIMENTAL!)
+#define LWIP_NETCONN                    1
- * Don't use it if you're not an active lwIP project member
+
+/** LWIP_NETCONN_SEM_PER_THREAD==1: Use one (thread-local) semaphore per
+ * thread calling socket/netconn functions instead of allocating one
+ * semaphore per netconn (and per select etc.)
+ * ATTENTION: a thread-local semaphore for API calls is needed:
+ * - LWIP_NETCONN_THREAD_SEM_GET() returning a sys_sem_t*
+ * - LWIP_NETCONN_THREAD_SEM_ALLOC() creating the semaphore
+ * - LWIP_NETCONN_THREAD_SEM_FREE() freeing the semaphore
+ * The latter 2 can be invoked up by calling netconn_thread_init()/netconn_thread_cleanup().
+ * Ports may call these for threads created with sys_thread_new().
  */
-#define LWIP_TCPIP_CORE_LOCKING         CONFIG_LWIP_TCPIP_CORE_LOCKING
+#define LWIP_NETCONN_SEM_PER_THREAD     1
+
+/** LWIP_NETCONN_FULLDUPLEX==1: Enable code that allows reading from one thread,
+ * writing from a 2nd thread and closing from a 3rd thread at the same time.
+ * ATTENTION: This is currently really alpha! Some requirements:
+ * - LWIP_NETCONN_SEM_PER_THREAD==1 is required to use one socket/netconn from
+ *   multiple threads at once
+ * - sys_mbox_free() has to unblock receive tasks waiting on recvmbox/acceptmbox
+ *   and prevent a task pending on this during/after deletion
+ */
+#define LWIP_NETCONN_FULLDUPLEX         1
 
 /*
    ------------------------------------
@@ -1114,11 +1150,6 @@ static inline uint32_t timeout_from_offered(uint32_t lease, uint32_t min)
 #define CHECKSUM_CHECK_ICMP             CONFIG_LWIP_CHECKSUM_CHECK_ICMP
 
 #define LWIP_NETCONN_FULLDUPLEX         1
-#if LWIP_TCPIP_CORE_LOCKING
-#define LWIP_NETCONN_SEM_PER_THREAD     0
-#else
-#define LWIP_NETCONN_SEM_PER_THREAD     1
-#endif /* LWIP_TCPIP_CORE_LOCKING */
 
 #define LWIP_DHCP_MAX_NTP_SERVERS       CONFIG_LWIP_DHCP_MAX_NTP_SERVERS
 #define LWIP_TIMEVAL_PRIVATE            0

components/lwip/test_afl_host/sdkconfig.defaults

@@ -1,2 +1,4 @@
+CONFIG_LWIP_TCPIP_CORE_LOCKING=n
+CONFIG_LWIP_CHECK_THREAD_SAFETY=n
 CONFIG_LWIP_DNS_SUPPORT_MDNS_QUERIES=n
 CONFIG_LWIP_TIMERS_ONDEMAND=n

docs/en/api-guides/lwip.rst

@@ -14,6 +14,11 @@ ESP-IDF supports the following lwIP TCP/IP stack functions:
 Adapted APIs
 ^^^^^^^^^^^^
 
+    .. warning::
+
+        When using any lwIP API (other than `BSD Sockets API`_), please make sure that it is thread safe. To check if a given API call is safe, enable :ref:`CONFIG_LWIP_CHECK_THREAD_SAFETY` and run the application. This way lwIP asserts the TCP/IP core functionality to be correctly accessed; the execution aborts if it is not locked properly or accessed from the correct task (`lwIP FreeRTOS Task`_).
+        The general recommendation is to use :doc:`/api-reference/network/esp_netif` component to interact with lwIP.
+
 Some common lwIP "app" APIs are supported indirectly by ESP-IDF:
 
 - DHCP Server & Client are supported indirectly via the :doc:`/api-reference/network/esp_netif` functionality

docs/en/api-reference/system/system_time.rst

@@ -110,9 +110,9 @@ To start synchronization via SNTP, just call the following three functions.
 
 .. code-block:: c
 
-    sntp_setoperatingmode(SNTP_OPMODE_POLL);
+    esp_sntp_setoperatingmode(ESP_SNTP_OPMODE_POLL);
-    sntp_setservername(0, "pool.ntp.org");
+    esp_sntp_setservername(0, "pool.ntp.org");
-    sntp_init();
+    esp_sntp_init();
 
 An application with this initialization code will periodically synchronize the time. The time synchronization period is determined by :envvar:`CONFIG_LWIP_SNTP_UPDATE_DELAY` (default value is one hour). To modify the variable, set :ref:`CONFIG_LWIP_SNTP_UPDATE_DELAY` in project configuration.
 

examples/protocols/http2_request/main/http2_request_example_main.c

@@ -17,7 +17,7 @@
 #include "freertos/task.h"
 #include "esp_wifi.h"
 #include "esp_event.h"
-#include "lwip/apps/sntp.h"
+#include "esp_sntp.h"
 #include "esp_system.h"
 #include "nvs_flash.h"
 #include "protocol_examples_common.h"
@@ -89,8 +89,8 @@ static void set_time(void)
     settimeofday(&tv, &tz);
 
     /* Start SNTP service */
-    sntp_setoperatingmode(SNTP_OPMODE_POLL);
+    esp_sntp_setoperatingmode(ESP_SNTP_OPMODE_POLL);
-    sntp_init();
+    esp_sntp_init();
 }
 
 static void http2_task(void *args)

examples/protocols/sntp/main/sntp_example_main.c

@@ -126,8 +126,8 @@ static void obtain_time(void)
      * NTP server address could be aquired via DHCP,
      * see LWIP_DHCP_GET_NTP_SRV menuconfig option
      */
-#ifdef LWIP_DHCP_GET_NTP_SRV
+#if LWIP_DHCP_GET_NTP_SRV
-    sntp_servermode_dhcp(1);
+    esp_sntp_servermode_dhcp(1);      // accept NTP offers from DHCP server, if any
 #endif
 
     /* This helper function configures Wi-Fi or Ethernet, as selected in menuconfig.
@@ -156,11 +156,11 @@ static void obtain_time(void)
 static void initialize_sntp(void)
 {
     ESP_LOGI(TAG, "Initializing SNTP");
-    sntp_setoperatingmode(SNTP_OPMODE_POLL);
+    esp_sntp_setoperatingmode(ESP_SNTP_OPMODE_POLL);
-    sntp_setservername(0, "pool.ntp.org");
+    esp_sntp_setservername(0, "pool.ntp.org");
     sntp_set_time_sync_notification_cb(time_sync_notification_cb);
 #ifdef CONFIG_SNTP_TIME_SYNC_METHOD_SMOOTH
     sntp_set_sync_mode(SNTP_SYNC_MODE_SMOOTH);
 #endif
-    sntp_init();
+    esp_sntp_init();
 }