Parcourir la source

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
David Cermak il y a 3 ans
Parent
commit
64f4f0ac1e

+ 0 - 1
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,
             };
 

+ 0 - 2
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 */

+ 51 - 34
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");

+ 2 - 60
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;

+ 1 - 2
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

+ 1 - 1
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

+ 14 - 2
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