Merge branch 'fix/esp_netif_lock_v5.0' into 'release/v5.0'

fix(esp_netif): Lock netif list with TCPIP context (v5.0)

See merge request espressif/esp-idf!26712
This commit is contained in:
Jiang Jiang Jian 2023-12-25 14:02:57 +08:00
commit b9ee54988f
10 changed files with 234 additions and 80 deletions

View File

@ -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
*/
@ -32,25 +32,41 @@ static SemaphoreHandle_t s_list_lock = NULL;
ESP_EVENT_DEFINE_BASE(IP_EVENT);
esp_err_t esp_netif_objects_init(void)
{
if (s_list_lock != NULL) {
// already initialized
return ESP_OK;
}
s_list_lock = xSemaphoreCreateMutex();
if (s_list_lock == NULL) {
return ESP_ERR_NO_MEM;
}
return ESP_OK;
}
void esp_netif_objects_deinit(void)
{
vSemaphoreDelete(s_list_lock);
s_list_lock = NULL;
}
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;
}
if (s_list_lock) {
xSemaphoreTake(s_list_lock, portMAX_DELAY);
} else {
ESP_LOGD(TAG, "%s list not locked (s_list_lock not initialized)", __func__);
}
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;
if (s_list_lock) {
xSemaphoreGive(s_list_lock);
} else {
ESP_LOGD(TAG, "%s list not unlocked (s_list_lock not initialized)", __func__);
}
}

View File

@ -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
*/
@ -939,12 +939,49 @@ 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()
*
* You can use esp_netif_next_unsafe() directly if all the system
* interfaces are under your control and you can safely iterate over them.
* Otherwise, iterate over interfaces using esp_netif_tcpip_exec(), or use esp_netif_find_if()
* to search in the list of netifs with defined predicate.
*
* @param[in] esp_netif Handle to esp-netif instance
*
* @return First netif from the list if supplied parameter is NULL, next one otherwise
*/
esp_netif_t *esp_netif_next(esp_netif_t *esp_netif);
/**
* @brief Iterates over list of interfaces without list locking. Returns first netif if NULL given as parameter
*
* Used for bulk search loops within TCPIP context, e.g. using esp_netif_tcpip_exec(), or if we're sure
* that the iteration is safe from our application perspective (e.g. no interface is removed between iterations)
*
* @param[in] esp_netif Handle to esp-netif instance
*
* @return First netif from the list if supplied parameter is NULL, next one otherwise
*/
esp_netif_t* esp_netif_next_unsafe(esp_netif_t* esp_netif);
/**
* @brief Predicate callback for esp_netif_find_if() used to find interface
* which meets defined criteria
*/
typedef bool (*esp_netif_find_predicate_t)(esp_netif_t *netif, void *ctx);
/**
* @brief Return a netif pointer for the first interface that meets criteria defined
* by the callback
*
* @param fn Predicate function returning true for the desired interface
* @param ctx Context pointer passed to the predicate, typically a descriptor to compare with
* @return valid netif pointer if found, NULL if not
*/
esp_netif_t *esp_netif_find_if(esp_netif_find_predicate_t fn, void *ctx);
/**
* @brief Returns number of registered esp_netif objects
*

View File

@ -187,7 +187,7 @@ esp_netif_t *esp_netif_new(const esp_netif_config_t *esp_netif_config)
}
esp_netif->ip_info_old = ip_info;
esp_netif_add_to_list(esp_netif);
esp_netif_add_to_list_unsafe(esp_netif);
// Configure the created object with provided configuration
esp_err_t ret = esp_netif_init_configuration(esp_netif, esp_netif_config);
@ -203,7 +203,7 @@ esp_netif_t *esp_netif_new(const esp_netif_config_t *esp_netif_config)
void esp_netif_destroy(esp_netif_t *esp_netif)
{
if (esp_netif) {
esp_netif_remove_from_list(esp_netif);
esp_netif_remove_from_list_unsafe(esp_netif);
free(esp_netif->ip_info);
free(esp_netif->ip_info_old);
free(esp_netif->if_key);

View File

@ -148,14 +148,6 @@ static esp_err_t set_lwip_netif_callback(struct esp_netif_api_msg_s *msg)
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;
}
static void dns_clear_servers(bool keep_fallback)
{
u8_t numdns = 0;
@ -259,6 +251,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 = {
@ -502,6 +504,10 @@ static void tcpip_init_done(void *arg)
esp_err_t esp_netif_init(void)
{
if (esp_netif_objects_init() != ESP_OK) {
ESP_LOGE(TAG, "esp_netif_objects_init() failed");
return ESP_FAIL;
}
if (!sys_thread_tcpip(LWIP_CORE_IS_TCPIP_INITIALIZED)) {
#if CONFIG_LWIP_HOOK_TCP_ISN_DEFAULT
uint8_t rand_buf[16];
@ -559,6 +565,11 @@ esp_err_t esp_netif_init(void)
esp_err_t esp_netif_deinit(void)
{
/* esp_netif_deinit() is not supported (as lwIP deinit isn't suported either)
* Once it's supported, we need to de-initialize:
* - netif objects calling esp_netif_objects_deinit()
* - other lwIP specific objects (see the comment after tcpip_initialized)
*/
if (sys_thread_tcpip(LWIP_CORE_IS_TCPIP_INITIALIZED)) {
/* deinit of LwIP not supported:
* do not deinit semaphores and states,
@ -566,6 +577,7 @@ esp_err_t esp_netif_deinit(void)
*
sys_sem_free(&api_sync_sem);
sys_sem_free(&api_lock_sem);
netif_remove_ext_callback(); (in lwip context)
*/
return ESP_ERR_NOT_SUPPORTED;
@ -736,8 +748,6 @@ esp_netif_t *esp_netif_new(const esp_netif_config_t *esp_netif_config)
esp_netif->lwip_netif = lwip_netif;
esp_netif_add_to_list(esp_netif);
#if ESP_DHCPS
// Create DHCP server structure
if (esp_netif_config->base->flags & ESP_NETIF_DHCP_SERVER) {
@ -763,9 +773,39 @@ esp_netif_t *esp_netif_new(const esp_netif_config_t *esp_netif_config)
esp_netif_lwip_ipc_no_args(set_lwip_netif_callback);
}
esp_netif_add_to_list(esp_netif);
return esp_netif;
}
typedef struct find_if_api {
esp_netif_find_predicate_t fn;
void *ctx;
} find_if_api_t;
static esp_err_t esp_netif_find_if_api(esp_netif_api_msg_t *msg)
{
find_if_api_t *find_if_api = msg->data;
esp_netif_t *esp_netif = NULL;
while ((esp_netif = esp_netif_next_unsafe(esp_netif)) != NULL) {
if (find_if_api->fn(esp_netif, find_if_api->ctx)) {
*msg->p_esp_netif = esp_netif;
return ESP_OK;
}
}
return ESP_FAIL;
}
esp_netif_t *esp_netif_find_if(esp_netif_find_predicate_t fn, void *ctx)
{
esp_netif_t *netif = NULL;
find_if_api_t find_if_api = { .fn = fn, .ctx = ctx };
if (esp_netif_lwip_ipc_call_get_netif(esp_netif_find_if_api, &netif, &find_if_api) == ESP_OK) {
return netif;
}
return NULL;
}
static void esp_netif_lwip_remove(esp_netif_t *esp_netif)
{
if (esp_netif->lwip_netif) {
@ -858,9 +898,11 @@ 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);
}
// not calling `netif_remove_ext_callback()` if number of netifs is 0
// since it's unsafe.
// It is expected to be called globally in `esp_netif_deinit()`
// once it's supported.
// }
free(esp_netif->ip_info);
free(esp_netif->ip_info_old);
free(esp_netif->if_key);

View File

@ -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
*/
@ -22,9 +22,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;

View File

@ -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
*/
@ -103,23 +103,12 @@ esp_err_t esp_netif_add_to_list(esp_netif_t* netif);
*/
esp_err_t esp_netif_remove_from_list(esp_netif_t* netif);
/**
* @brief Iterates over list of interfaces without list locking. Returns first netif if NULL given as parameter
*
* Used for bulk search loops to avoid locking and unlocking every iteration. esp_netif_list_lock and esp_netif_list_unlock
* must be used to guard the search loop
*
* @param[in] esp_netif Handle to esp-netif instance
*
* @return First netif from the list if supplied parameter is NULL, next one otherwise
*/
esp_netif_t* esp_netif_next_unsafe(esp_netif_t* netif);
/**
* @brief Locking network interface list. Use only in connection with esp_netif_next_unsafe
*
* @return ESP_OK on success, specific mutex error if failed to lock
*/
esp_err_t esp_netif_list_lock(void);
/**
@ -165,4 +154,17 @@ 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 Initialize netif objects for handling lists of interfaces one esp_netif level
*
* @return esp_err_t ESP_OK on success
*/
esp_err_t esp_netif_objects_init(void);
/**
* @brief Deinitialize netif objects
*
*/
void esp_netif_objects_deinit(void);
#endif //_ESP_NETIF_PRIVATE_H_

View File

@ -77,6 +77,44 @@ TEST(esp_netif, create_delete_multiple_netifs)
}
static bool desc_matches_with(esp_netif_t *netif, void *ctx)
{
return strcmp(ctx, esp_netif_get_desc(netif)) == 0;
}
TEST(esp_netif, find_netifs)
{
// Create some interfaces
const char* if_keys[] = { "if1", "if2", "if3", "if4", "if5"};
const int nr_of_netifs = sizeof(if_keys)/sizeof(char*);
esp_netif_t *netifs[nr_of_netifs];
for (int i=0; i<nr_of_netifs; ++i) {
// Create all interfaces, the same string is used as a key and as description
esp_netif_inherent_config_t base_netif_config = { .if_key = if_keys[i], .if_desc = if_keys[i] };
esp_netif_config_t cfg = { .base = &base_netif_config, .stack = ESP_NETIF_NETSTACK_DEFAULT_WIFI_STA };
netifs[i] = esp_netif_new(&cfg);
TEST_ASSERT_NOT_NULL(netifs[i]);
}
// not found
esp_netif_t *found_netif = esp_netif_find_if(desc_matches_with, "not_present");
TEST_ASSERT_EQUAL(found_netif, NULL);
// should find the same netif, as returned from esp_netif_get_handle_from_ifkey(), as the key is the same as description
for (int i=0; i<nr_of_netifs; ++i) {
found_netif = esp_netif_find_if(desc_matches_with, (void*)if_keys[i]);
TEST_ASSERT_EQUAL(found_netif, esp_netif_get_handle_from_ifkey(if_keys[i]));
}
// destroy one by one and check it's cannot be find per its description
for (int i=0; i<nr_of_netifs; ++i) {
esp_netif_destroy(netifs[i]);
found_netif = esp_netif_find_if(desc_matches_with, (void*)if_keys[i]);
TEST_ASSERT_EQUAL(found_netif, NULL);
}
}
TEST(esp_netif, dhcp_client_state_transitions_wifi_sta)
{
// init default wifi netif
@ -273,6 +311,7 @@ TEST_GROUP_RUNNER(esp_netif)
RUN_TEST_CASE(esp_netif, init_and_destroy)
RUN_TEST_CASE(esp_netif, get_from_if_key)
RUN_TEST_CASE(esp_netif, create_delete_multiple_netifs)
RUN_TEST_CASE(esp_netif, find_netifs)
RUN_TEST_CASE(esp_netif, dhcp_client_state_transitions_wifi_sta)
RUN_TEST_CASE(esp_netif, dhcp_server_state_transitions_wifi_ap)
RUN_TEST_CASE(esp_netif, dhcp_server_state_transitions_mesh)

View File

@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2021-2022 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2021-2023 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
@ -333,6 +333,12 @@ static int l2tap_close(int fd)
return 0;
}
// used to find a netif with the attached driver matching the argument
static bool netif_driver_matches(esp_netif_t *netif, void* driver)
{
return esp_netif_get_io_driver(netif) == driver;
}
static int l2tap_ioctl(int fd, int cmd, va_list args)
{
esp_netif_t *esp_netif;
@ -383,11 +389,8 @@ static int l2tap_ioctl(int fd, int cmd, va_list args)
case L2TAP_G_INTF_DEVICE: ;
const char **str_p = va_arg(args, const char **);
*str_p = NULL;
esp_netif = NULL;
while ((esp_netif = esp_netif_next(esp_netif)) != NULL) {
if (s_l2tap_sockets[fd].driver_handle == esp_netif_get_io_driver(esp_netif)) {
*str_p = esp_netif_get_ifkey(esp_netif);
}
if ((esp_netif = esp_netif_find_if(netif_driver_matches, s_l2tap_sockets[fd].driver_handle)) != NULL) {
*str_p = esp_netif_get_ifkey(esp_netif);
}
break;
case L2TAP_S_DEVICE_DRV_HNDL: ;

View File

@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2022 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2022-2023 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Unlicense OR CC0-1.0
*/
@ -43,26 +43,25 @@ bool example_is_our_netif(const char *prefix, esp_netif_t *netif)
return strncmp(prefix, esp_netif_get_desc(netif), strlen(prefix) - 1) == 0;
}
esp_netif_t *get_example_netif_from_desc(const char *desc)
static bool netif_desc_matches_with(esp_netif_t *netif, void *ctx)
{
esp_netif_t *netif = NULL;
while ((netif = esp_netif_next(netif)) != NULL) {
if (strcmp(esp_netif_get_desc(netif), desc) == 0) {
return netif;
}
}
return netif;
return strcmp(ctx, esp_netif_get_desc(netif)) == 0;
}
void example_print_all_netif_ips(const char *prefix)
esp_netif_t *get_example_netif_from_desc(const char *desc)
{
return esp_netif_find_if(netif_desc_matches_with, (void*)desc);
}
static esp_err_t print_all_ips_tcpip(void* ctx)
{
const char *prefix = ctx;
// iterate over active interfaces, and print out IPs of "our" netifs
esp_netif_t *netif = NULL;
esp_netif_ip_info_t ip;
for (int i = 0; i < esp_netif_get_nr_of_ifs(); ++i) {
netif = esp_netif_next(netif);
while ((netif = esp_netif_next_unsafe(netif)) != NULL) {
if (example_is_our_netif(prefix, netif)) {
ESP_LOGI(TAG, "Connected to %s", esp_netif_get_desc(netif));
esp_netif_ip_info_t ip;
ESP_ERROR_CHECK(esp_netif_get_ip_info(netif, &ip));
ESP_LOGI(TAG, "- IPv4 address: " IPSTR ",", IP2STR(&ip.ip));
@ -76,6 +75,13 @@ void example_print_all_netif_ips(const char *prefix)
#endif
}
}
return ESP_OK;
}
void example_print_all_netif_ips(const char *prefix)
{
// Print all IPs in TCPIP context to avoid potential races of removing/adding netifs when iterating over the list
esp_netif_tcpip_exec(print_all_ips_tcpip, (void*) prefix);
}

View File

@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2022 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2022-2023 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Unlicense OR CC0-1.0
*/
@ -215,34 +215,41 @@ static void send_ping(char *src_addr_str, char *dst_addr_str, char *interface)
free(msghdr.msg_control);
}
/**
* @brief API struct to pass interface name and source addresses in TCPIP context
*
* @param[out] interface Name of the interface.
* @param[out] src_addr_str Global/Unique local IPv6 address of the interface
*/
typedef struct src_iface_api {
char *interface;
char *src_addr_str;
} src_iface_api_t;
/**
* @brief Goes over each interface and searches for one Global/Unique local IPv6 address.
* Returns the interface name and IPv6 address of the interface in case of success.
*
* @param[out] interface Name of the interface.
* @param[out] src_addr_str Global/Unique local IPv6 address of the interface
* @return
* >0 : Successfully found an interface with Global/Unique local IPv6 address.
* -1 : Unable to to find a valid interface with Global/Unique local IPv6 address.
* ESP_OK : Successfully found an interface with Global/Unique local IPv6 address.
* ESP_FAIL : Unable to to find a valid interface with Global/Unique local IPv6 address.
*/
bool get_src_iface(char *interface, char *src_addr_str)
static esp_err_t get_src_iface(void* ctx)
{
src_iface_api_t *api = ctx;
esp_netif_t *netif = NULL;
int ip6_addrs_count = 0;
esp_ip6_addr_t ip6[LWIP_IPV6_NUM_ADDRESSES];
esp_err_t ret = ESP_FAIL;
// Get interface details and own global ipv6 address
for (int i = 0; i < esp_netif_get_nr_of_ifs(); ++i) {
netif = esp_netif_next(netif);
ret = esp_netif_get_netif_impl_name(netif, interface);
while ((netif = esp_netif_next_unsafe(netif)) != NULL) {
esp_err_t ret = esp_netif_get_netif_impl_name(netif, api->interface);
if ((ESP_FAIL == ret) || (NULL == netif)) {
ESP_LOGE(TAG, "No interface available");
return false;
return ESP_FAIL;
}
ESP_LOGI(TAG, "Interface: %s", interface);
ESP_LOGI(TAG, "Interface: %s", api->interface);
ip6_addrs_count = esp_netif_get_all_ip6(netif, ip6);
for (int j = 0; j < ip6_addrs_count; ++j) {
@ -252,13 +259,13 @@ bool get_src_iface(char *interface, char *src_addr_str)
if ((ESP_IP6_ADDR_IS_GLOBAL == ipv6_type) ||
(ESP_IP6_ADDR_IS_UNIQUE_LOCAL == ipv6_type)) {
// Break as we have the source address
sprintf(src_addr_str, IPV6STR, IPV62STR(ip6[j]));
return true;
sprintf(api->src_addr_str, IPV6STR, IPV62STR(ip6[j]));
return ESP_OK;
}
}
}
return false;
return ESP_FAIL;
}
@ -268,7 +275,8 @@ static void ping6_test_task(void *pvParameters)
char interface[10];
char dst_addr_str[] = CONFIG_EXAMPLE_DST_IPV6_ADDR;
if (true == get_src_iface(interface, src_addr_str)) {
src_iface_api_t api = { .interface = interface, .src_addr_str = src_addr_str };
if (esp_netif_tcpip_exec(get_src_iface, &api) == ESP_OK) {
ESP_LOGI(TAG, "Source address: %s", src_addr_str);
ESP_LOGI(TAG, "Destination address: %s", dst_addr_str);
ESP_LOGI(TAG, "Interface name: %s", interface);