From 64f4f0ac1e58bd59b715caef27ee9157e39d2aaf Mon Sep 17 00:00:00 2001 From: David Cermak Date: Fri, 15 Jul 2022 17:32:52 +0200 Subject: [PATCH] esp_netif/lwip: Use netif-client-data to store esp_netif ptr lwip/netif struct has two places to store user's data * netif->state (1 void*) but that might be occupied in special cases * netif->client_dtat (n void*s) but that must be enabled in opts.h This commit stores esp_netif_t* primarily in state, but if any special netif is enabled in menuconfig (bridgeif, pppos), it uses netif->client_data. This commit also fixes incorrect esp_netif that is attached to IP_EVENT_GOT_IP6 event posted by pppos interfaces in: https://github.com/espressif/esp-idf/blob/c585618b97d47514b7bf5f361944a8af05348169/components/esp_netif/lwip/esp_netif_lwip_ppp.c#L114 Closes https://github.com/espressif/esp-idf/issues/9345 --- components/esp_netif/esp_netif_handlers.c | 1 - .../esp_netif/include/esp_netif_types.h | 2 - components/esp_netif/lwip/esp_netif_lwip.c | 85 +++++++++++-------- .../esp_netif/lwip/esp_netif_lwip_ppp.c | 62 +------------- components/esp_netif/lwip/netif/ethernetif.c | 3 +- components/esp_netif/lwip/netif/wlanif.c | 2 +- components/lwip/port/esp32/include/lwipopts.h | 16 +++- 7 files changed, 69 insertions(+), 102 deletions(-) diff --git a/components/esp_netif/esp_netif_handlers.c b/components/esp_netif/esp_netif_handlers.c index 838dda633e..95a046898c 100644 --- a/components/esp_netif/esp_netif_handlers.c +++ b/components/esp_netif/esp_netif_handlers.c @@ -58,7 +58,6 @@ void esp_netif_action_connected(void *esp_netif, esp_event_base_t base, int32_t if (esp_netif_is_valid_static_ip(&ip)) { ip_event_got_ip_t evt = { .esp_netif = esp_netif, - .if_index = -1, // to indicate ptr to if used .ip_changed = false, }; diff --git a/components/esp_netif/include/esp_netif_types.h b/components/esp_netif/include/esp_netif_types.h index 962fd62048..3268f4732b 100644 --- a/components/esp_netif/include/esp_netif_types.h +++ b/components/esp_netif/include/esp_netif_types.h @@ -126,7 +126,6 @@ typedef struct { * */ typedef struct { - int if_index; /*!< Interface index for which the event is received (left for legacy compilation) */ esp_netif_t *esp_netif; /*!< Pointer to corresponding esp-netif object */ esp_netif_ip_info_t ip_info; /*!< IP address, netmask, gatway IP address */ bool ip_changed; /*!< Whether the assigned IP has changed or not */ @@ -134,7 +133,6 @@ typedef struct { /** Event structure for IP_EVENT_GOT_IP6 event */ typedef struct { - int if_index; /*!< Interface index for which the event is received (left for legacy compilation) */ esp_netif_t *esp_netif; /*!< Pointer to corresponding esp-netif object */ esp_netif_ip6_info_t ip6_info; /*!< IPv6 address of the interface */ int ip_index; /*!< IPv6 address index */ diff --git a/components/esp_netif/lwip/esp_netif_lwip.c b/components/esp_netif/lwip/esp_netif_lwip.c index d702136028..049fd55e69 100644 --- a/components/esp_netif/lwip/esp_netif_lwip.c +++ b/components/esp_netif/lwip/esp_netif_lwip.c @@ -105,6 +105,10 @@ extern sys_thread_t g_lwip_task; static const char *TAG = "esp_netif_lwip"; static bool tcpip_initialized = false; + +#if LWIP_ESP_NETIF_DATA +static u8_t lwip_netif_client_id = 0xff; +#endif static esp_netif_t *s_last_default_esp_netif = NULL; static bool s_is_last_default_esp_netif_overridden = false; static netif_ext_callback_t netif_callback = { .callback_fn = NULL, .next = NULL }; @@ -346,6 +350,24 @@ esp_err_t esp_netif_set_default_netif(esp_netif_t *esp_netif) return esp_netif_update_default_netif(esp_netif, ESP_NETIF_SET_DEFAULT); } +static inline esp_netif_t* lwip_get_esp_netif(struct netif *netif) +{ +#if LWIP_ESP_NETIF_DATA + return (esp_netif_t*)netif_get_client_data(netif, lwip_netif_client_id); +#else + return (esp_netif_t*)netif->state; +#endif +} + +static inline void lwip_set_esp_netif(struct netif *netif, esp_netif_t* esp_netif) +{ +#if LWIP_ESP_NETIF_DATA + netif_set_client_data(netif, lwip_netif_client_id, esp_netif); +#else + netif->state = esp_netif; +#endif +} + #if CONFIG_ESP_NETIF_BRIDGE_EN esp_err_t esp_netif_bridge_add_port(esp_netif_t *esp_netif_br, esp_netif_t *esp_netif_port) { @@ -422,23 +444,7 @@ esp_netif_t* esp_netif_get_handle_from_netif_impl(void *dev) { // ppp_pcb ptr would never get to app code, so this function only works with vanilla lwip impl struct netif *lwip_netif = dev; -#if CONFIG_ESP_NETIF_BRIDGE_EN - // bridge lwip netif uses "state" member for something different => need to traverse all esp_netifs - if (lwip_netif->name[0] == 'b' && lwip_netif->name[1] == 'r') { - esp_netif_t* esp_netif = esp_netif_next(NULL); - do - { - if(esp_netif->lwip_netif == lwip_netif) { - return esp_netif; - } - } while ((esp_netif = esp_netif_next(esp_netif)) != NULL); - } else { - return lwip_netif->state; - } - return NULL; -#else - return lwip_netif->state; -#endif // CONFIG_ESP_NETIF_BRIDGE_EN + return lwip_get_esp_netif(lwip_netif); } void* esp_netif_get_netif_impl(esp_netif_t *esp_netif) @@ -468,6 +474,11 @@ esp_err_t esp_netif_init(void) #endif tcpip_init(NULL, NULL); ESP_LOGD(TAG, "LwIP stack has been initialized"); +#if LWIP_ESP_NETIF_DATA + if (lwip_netif_client_id == 0xFF) { + lwip_netif_client_id = netif_alloc_client_data_id(); + } +#endif } #if !LWIP_TCPIP_CORE_LOCKING @@ -657,6 +668,14 @@ esp_netif_t *esp_netif_new(const esp_netif_config_t *esp_netif_config) 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) { + lwip_netif_client_id = netif_alloc_client_data_id(); + } +#endif + struct netif * lwip_netif = calloc(1, sizeof(struct netif)); if (!lwip_netif) { ESP_LOGE(TAG, "Failed to allocate %d bytes (free heap size %d)", sizeof(struct netif), @@ -667,7 +686,6 @@ esp_netif_t *esp_netif_new(const esp_netif_config_t *esp_netif_config) return NULL; } - lwip_netif->state = esp_netif; esp_netif->lwip_netif = lwip_netif; esp_netif_add_to_list(esp_netif); @@ -691,6 +709,7 @@ esp_netif_t *esp_netif_new(const esp_netif_config_t *esp_netif_config) esp_netif_destroy(esp_netif); return NULL; } + lwip_set_esp_netif(lwip_netif, esp_netif); set_lwip_netif_callback(); @@ -761,6 +780,7 @@ static esp_err_t esp_netif_lwip_add(esp_netif_t *esp_netif) #if CONFIG_ESP_NETIF_BRIDGE_EN } #endif // CONFIG_ESP_NETIF_BRIDGE_EN + lwip_set_esp_netif(esp_netif->lwip_netif, esp_netif); return ESP_OK; } @@ -1110,13 +1130,12 @@ static esp_err_t esp_netif_start_ip_lost_timer(esp_netif_t *esp_netif); // static void esp_netif_internal_dhcpc_cb(struct netif *netif) { - if (!netif) { - ESP_LOGD(TAG, "null netif=%p", netif); + esp_netif_t *esp_netif; + ESP_LOGD(TAG, "%s lwip-netif:%p", __func__, netif); + if (netif == NULL || (esp_netif = lwip_get_esp_netif(netif)) == NULL) { + // internal pointer hasn't been configured yet (probably in the interface init_fn()) return; } - ESP_LOGD(TAG, "%s lwip-netif:%p", __func__, netif); - - esp_netif_t *esp_netif = esp_netif_get_handle_from_netif_impl(netif); esp_netif_ip_info_t *ip_info = esp_netif->ip_info; esp_netif_ip_info_t *ip_info_old = esp_netif->ip_info_old; @@ -1129,7 +1148,6 @@ static void esp_netif_internal_dhcpc_cb(struct netif *netif) !ip4_addr_cmp(ip_2_ip4(&netif->gw), (&ip_info->gw)) ) { ip_event_got_ip_t evt = { .esp_netif = esp_netif, - .if_index = -1, // invalid index, handle used .ip_changed = false, }; ip_event_t evt_id = esp_netif_get_event_id(esp_netif, ESP_NETIF_IP_EVENT_GOT_IP); @@ -1182,7 +1200,6 @@ static void esp_netif_ip_lost_timer(void *arg) if ( (!netif) || (netif && ip4_addr_cmp(ip_2_ip4(&netif->ip_addr), IP4_ADDR_ANY4))) { ip_event_got_ip_t evt = { .esp_netif = esp_netif, - .if_index = -1, }; int ret; @@ -1651,7 +1668,7 @@ static esp_err_t esp_netif_set_ip_info_api(esp_netif_api_msg_t *msg) if (!(ip4_addr_isany_val(ip_info->ip) || ip4_addr_isany_val(ip_info->netmask) || ip4_addr_isany_val(ip_info->gw))) { ip_event_t evt_id = esp_netif->get_ip_event; - ip_event_got_ip_t evt = { .esp_netif = esp_netif, .if_index = -1, .ip_changed = false}; + ip_event_got_ip_t evt = { .esp_netif = esp_netif, .ip_changed = false}; int ret; if (memcmp(ip_info, esp_netif->ip_info_old, sizeof(esp_netif_ip_info_t))) { evt.ip_changed = true; @@ -1842,20 +1859,20 @@ esp_ip6_addr_type_t esp_netif_ip6_get_addr_type(esp_ip6_addr_t* ip6_addr) } -static void esp_netif_internal_nd6_cb(struct netif *p_netif, uint8_t ip_index) +static void esp_netif_internal_nd6_cb(struct netif *netif, uint8_t ip_index) { - ESP_LOGD(TAG, "%s lwip-netif:%p", __func__, p_netif); - if (!p_netif) { - ESP_LOGD(TAG, "esp_netif_internal_nd6_cb called with null p_netif"); + esp_netif_t *esp_netif; + ESP_LOGD(TAG, "%s lwip-netif:%p", __func__, netif); + if (netif == NULL || (esp_netif = lwip_get_esp_netif(netif)) == NULL) { + // internal pointer hasn't been configured yet (probably in the interface init_fn()) return; } esp_netif_ip6_info_t ip6_info; ip6_addr_t lwip_ip6_info; - //notify event - ip_event_got_ip6_t evt = { .esp_netif = p_netif->state, .if_index = -1, .ip_index = ip_index }; + ip_event_got_ip6_t evt = { .esp_netif = esp_netif, .ip_index = ip_index }; - ip6_addr_set(&lwip_ip6_info, ip_2_ip6(&p_netif->ip6_addr[ip_index])); + ip6_addr_set(&lwip_ip6_info, ip_2_ip6(&netif->ip6_addr[ip_index])); #if LWIP_IPV6_SCOPES memcpy(&ip6_info.ip, &lwip_ip6_info, sizeof(esp_ip6_addr_t)); #else @@ -2264,7 +2281,7 @@ static esp_err_t esp_netif_add_ip6_address_api(esp_netif_api_msg_t *msg) netif_ip6_addr_set_state(msg->esp_netif->lwip_netif, index, addr->preferred ? IP6_ADDR_PREFERRED : IP6_ADDR_DEPRECATED); - ip_event_got_ip6_t evt = {.esp_netif = msg->esp_netif, .if_index = -1, .ip_index = index}; + ip_event_got_ip6_t evt = {.esp_netif = msg->esp_netif, .ip_index = index}; evt.ip6_info.ip = addr->addr; ESP_RETURN_ON_ERROR(esp_event_post(IP_EVENT, IP_EVENT_GOT_IP6, &evt, sizeof(evt), 0), TAG, "Failed to post IP_EVENT_GOT_IP6"); diff --git a/components/esp_netif/lwip/esp_netif_lwip_ppp.c b/components/esp_netif/lwip/esp_netif_lwip_ppp.c index e7860aefbb..2c6c1c1248 100644 --- a/components/esp_netif/lwip/esp_netif_lwip_ppp.c +++ b/components/esp_netif/lwip/esp_netif_lwip_ppp.c @@ -65,75 +65,17 @@ static void pppapi_set_auth(ppp_pcb *pcb, u8_t authtype, const char *user, const */ static void on_ppp_status_changed(ppp_pcb *pcb, int err_code, void *ctx) { - struct netif *pppif = ppp_netif(pcb); - const ip_addr_t *dest_ip = NULL; esp_netif_t *netif = ctx; ip_event_got_ip_t evt = { .esp_netif = netif, - .if_index = -1, }; esp_err_t err; struct lwip_peer2peer_ctx *obj = (struct lwip_peer2peer_ctx*)netif->related_data; assert(obj->base.netif_type == PPP_LWIP_NETIF); - esp_ip4_addr_t ns1; - esp_ip4_addr_t ns2; switch (err_code) { - case PPPERR_NONE: /* Connected */ + case PPPERR_NONE: ESP_LOGI(TAG, "Connected"); - if (pcb->if4_up && !ip_addr_isany(&pppif->ip_addr)) { - esp_netif_ip_info_t *ip_info = netif->ip_info; - ip4_addr_set(&ip_info->ip, ip_2_ip4(&pppif->ip_addr)); - ip4_addr_set(&ip_info->netmask, ip_2_ip4(&pppif->netmask)); - ip4_addr_set(&ip_info->gw, ip_2_ip4(&pppif->gw)); - - ip4_addr_set(&evt.ip_info.ip, ip_2_ip4(&pppif->ip_addr)); - ip4_addr_set(&evt.ip_info.gw, ip_2_ip4(&pppif->gw)); - ip4_addr_set(&evt.ip_info.netmask, ip_2_ip4(&pppif->netmask)); - - dest_ip = dns_getserver(0); - if(dest_ip != NULL){ - ip4_addr_set(&ns1, ip_2_ip4(dest_ip)); - } - dest_ip = dns_getserver(1); - if(dest_ip != NULL){ - ip4_addr_set(&ns2, ip_2_ip4(dest_ip)); - } - ESP_LOGI(TAG, "Name Server1: " IPSTR, IP2STR(&ns1)); - ESP_LOGI(TAG, "Name Server2: " IPSTR, IP2STR(&ns2)); - - - err = esp_event_post(IP_EVENT, netif->get_ip_event, &evt, sizeof(evt), 0); - if (ESP_OK != err) { - ESP_LOGE(TAG, "esp_event_post failed with code %d", err); - } - return; -#if PPP_IPV6_SUPPORT - } else if (pcb->if6_up && !ip_addr_isany(&pppif->ip6_addr[0])) { - esp_netif_ip6_info_t ip6_info; - ip6_addr_t lwip_ip6_info; - ip_event_got_ip6_t ip6_event = { .esp_netif = pppif->state, .if_index = -1 }; - - ip6_addr_set(&lwip_ip6_info, ip_2_ip6(&pppif->ip6_addr[0])); -#if LWIP_IPV6_SCOPES - memcpy(&ip6_info.ip, &lwip_ip6_info, sizeof(esp_ip6_addr_t)); -#else - memcpy(&ip6_info.ip, &lwip_ip6_info, sizeof(ip6_addr_t)); - ip6_info.ip.zone = 0; // zero out zone, as not used in lwip -#endif /* LWIP_IPV6_SCOPES */ - memcpy(&ip6_event.ip6_info, &ip6_info, sizeof(esp_netif_ip6_info_t)); - - ESP_LOGI(TAG, "Got IPv6 address " IPV6STR, IPV62STR(pppif->ip6_addr[0].u_addr.ip6)); - err = esp_event_post(IP_EVENT, IP_EVENT_GOT_IP6, &ip6_event, sizeof(ip6_event), 0); - if (ESP_OK != err) { - ESP_LOGE(TAG, "esp_event_post failed with code %d", err); - } - return; -#endif /* PPP_IPV6_SUPPORT */ - } else { - ESP_LOGE(TAG, "Unexpected connected event"); - return; - } - + break; case PPPERR_PARAM: ESP_LOGE(TAG, "Invalid parameter"); break; diff --git a/components/esp_netif/lwip/netif/ethernetif.c b/components/esp_netif/lwip/netif/ethernetif.c index c41e39e53d..c46775915d 100644 --- a/components/esp_netif/lwip/netif/ethernetif.c +++ b/components/esp_netif/lwip/netif/ethernetif.c @@ -155,8 +155,7 @@ err_t ethernetif_init(struct netif *netif) { LWIP_ASSERT("netif != NULL", (netif != NULL)); /* Have to get the esp-netif handle from netif first and then driver==ethernet handle from there */ - esp_netif_t *esp_netif = esp_netif_get_handle_from_netif_impl(netif); - + esp_netif_t *esp_netif = netif->state; /* Initialize interface hostname */ #if LWIP_NETIF_HOSTNAME #if ESP_LWIP diff --git a/components/esp_netif/lwip/netif/wlanif.c b/components/esp_netif/lwip/netif/wlanif.c index 67438e9e8f..92129ea262 100644 --- a/components/esp_netif/lwip/netif/wlanif.c +++ b/components/esp_netif/lwip/netif/wlanif.c @@ -185,7 +185,7 @@ wlanif_init(struct netif *netif) /* Initialize interface hostname */ #if ESP_LWIP - if (esp_netif_get_hostname(esp_netif_get_handle_from_netif_impl(netif), &netif->hostname) != ESP_OK) { + if (esp_netif_get_hostname(netif->state, &netif->hostname) != ESP_OK) { netif->hostname = CONFIG_LWIP_LOCAL_HOSTNAME; } #else diff --git a/components/lwip/port/esp32/include/lwipopts.h b/components/lwip/port/esp32/include/lwipopts.h index ddca55e7f3..b9561a1923 100644 --- a/components/lwip/port/esp32/include/lwipopts.h +++ b/components/lwip/port/esp32/include/lwipopts.h @@ -665,9 +665,21 @@ static inline uint32_t timeout_from_offered(uint32_t lease, uint32_t min) * LWIP_NUM_NETIF_CLIENT_DATA: Number of clients that may store * data in client_data member array of struct netif (max. 256). */ -#ifdef CONFIG_LWIP_NUM_NETIF_CLIENT_DATA -#define LWIP_NUM_NETIF_CLIENT_DATA CONFIG_LWIP_NUM_NETIF_CLIENT_DATA +#ifndef CONFIG_LWIP_NUM_NETIF_CLIENT_DATA +#define CONFIG_LWIP_NUM_NETIF_CLIENT_DATA 0 #endif +#if defined(CONFIG_ESP_NETIF_BRIDGE_EN) || defined(CONFIG_LWIP_PPP_SUPPORT) +/* + * If special lwip interfaces (like bridge, ppp) enabled + * `netif->state` is used internally and we must store esp-netif ptr + * in `netif->client_data` + */ +#define LWIP_ESP_NETIF_DATA (1) +#else +#define LWIP_ESP_NETIF_DATA (0) +#endif + +#define LWIP_NUM_NETIF_CLIENT_DATA (LWIP_ESP_NETIF_DATA + CONFIG_LWIP_NUM_NETIF_CLIENT_DATA) /** * BRIDGEIF_MAX_PORTS: this is used to create a typedef used for forwarding