From 5fccb73f864d7c9bc3440f73353b482666c22826 Mon Sep 17 00:00:00 2001 From: morris Date: Tue, 6 Nov 2018 19:10:01 +0800 Subject: [PATCH] ethernetif: fix potential memory leak 1. If L2_TO_L3_RX_BUF_MODE is not selected, we must assign l2_owner explictly before we call pbuf_free. 2. free intr resource in esp_eth_deinit Closes https://github.com/espressif/esp-idf/issues/2670 --- components/ethernet/emac_main.c | 4 +- components/ethernet/test/test_emac_deinit.c | 53 +++++++++++++++++-- components/lwip/port/esp32/netif/ethernetif.c | 1 - 3 files changed, 53 insertions(+), 5 deletions(-) diff --git a/components/ethernet/emac_main.c b/components/ethernet/emac_main.c index a22827c5c..de6db0f9f 100644 --- a/components/ethernet/emac_main.c +++ b/components/ethernet/emac_main.c @@ -73,6 +73,7 @@ static uint8_t emac_sig_cnt[EMAC_SIG_MAX] = {0}; static TimerHandle_t emac_timer = NULL; static SemaphoreHandle_t emac_rx_xMutex = NULL; static SemaphoreHandle_t emac_tx_xMutex = NULL; +static intr_handle_t eth_intr_handle = NULL; static const char *TAG = "emac"; static bool pause_send = false; #ifdef CONFIG_PM_ENABLE @@ -1168,7 +1169,7 @@ esp_err_t esp_eth_init_internal(eth_config_t *config) EMAC_TASK_PRIORITY, &emac_task_hdl); - esp_intr_alloc(ETS_ETH_MAC_INTR_SOURCE, 0, emac_process_intr, NULL, NULL); + esp_intr_alloc(ETS_ETH_MAC_INTR_SOURCE, 0, emac_process_intr, NULL, ð_intr_handle); emac_config.emac_status = EMAC_RUNTIME_INIT; @@ -1211,6 +1212,7 @@ esp_err_t esp_eth_deinit(void) free(emac_dma_tx_buf[i]); emac_dma_tx_buf[i] = NULL; } + esp_intr_free(eth_intr_handle); ret = ESP_OK; _exit: return ret; diff --git a/components/ethernet/test/test_emac_deinit.c b/components/ethernet/test/test_emac_deinit.c index 370f7975b..3b540728a 100644 --- a/components/ethernet/test/test_emac_deinit.c +++ b/components/ethernet/test/test_emac_deinit.c @@ -7,17 +7,19 @@ * @author morris * @date 2018-08-24 */ +#include #include -#include "unity.h" + #include "freertos/FreeRTOS.h" #include "freertos/task.h" - +#include "freertos/event_groups.h" #include "esp_system.h" #include "esp_err.h" #include "esp_event_loop.h" #include "esp_event.h" #include "esp_log.h" #include "esp_eth.h" +#include "unity.h" #include "rom/gpio.h" @@ -37,6 +39,9 @@ static const char *TAG = "eth_test_deinit"; #define CONFIG_PHY_ADDRESS 31 #define CONFIG_PHY_CLOCK_MODE 0 +static EventGroupHandle_t eth_event_group = NULL; +static const int GOTIP_BIT = BIT0; + static void phy_device_power_enable_via_gpio(bool enable) { if (!enable) { @@ -67,6 +72,47 @@ static void eth_gpio_config_rmii(void) phy_rmii_smi_configure_pins(PIN_SMI_MDC, PIN_SMI_MDIO); } +static esp_err_t eth_event_handler(void *ctx, system_event_t *event) +{ + tcpip_adapter_ip_info_t ip; + + switch (event->event_id) { + case SYSTEM_EVENT_ETH_CONNECTED: + ESP_LOGI(TAG, "Ethernet Link Up"); + break; + case SYSTEM_EVENT_ETH_DISCONNECTED: + ESP_LOGI(TAG, "Ethernet Link Down"); + break; + case SYSTEM_EVENT_ETH_START: + ESP_LOGI(TAG, "Ethernet Started"); + break; + case SYSTEM_EVENT_ETH_GOT_IP: + memset(&ip, 0, sizeof(tcpip_adapter_ip_info_t)); + ESP_ERROR_CHECK(tcpip_adapter_get_ip_info(ESP_IF_ETH, &ip)); + ESP_LOGI(TAG, "Ethernet Got IP Addr"); + ESP_LOGI(TAG, "~~~~~~~~~~~"); + ESP_LOGI(TAG, "ETHIP:" IPSTR, IP2STR(&ip.ip)); + ESP_LOGI(TAG, "ETHMASK:" IPSTR, IP2STR(&ip.netmask)); + ESP_LOGI(TAG, "ETHGW:" IPSTR, IP2STR(&ip.gw)); + ESP_LOGI(TAG, "~~~~~~~~~~~"); + xEventGroupSetBits(eth_event_group, GOTIP_BIT); + break; + case SYSTEM_EVENT_ETH_STOP: + ESP_LOGI(TAG, "Ethernet Stopped"); + break; + default: + break; + } + return ESP_OK; +} + +TEST_CASE("start event loop", "[ethernet][ignore]") +{ + eth_event_group = xEventGroupCreate(); + tcpip_adapter_init(); + ESP_ERROR_CHECK(esp_event_loop_init(eth_event_handler, NULL)); +} + TEST_CASE("test emac deinit", "[ethernet][ignore]") { eth_config_t config = DEFAULT_ETHERNET_PHY_CONFIG; @@ -79,7 +125,8 @@ TEST_CASE("test emac deinit", "[ethernet][ignore]") ESP_ERROR_CHECK(esp_eth_init(&config)); ESP_ERROR_CHECK(esp_eth_enable()); - vTaskDelay(2000 / portTICK_RATE_MS); + xEventGroupWaitBits(eth_event_group, GOTIP_BIT, true, true, portMAX_DELAY); + vTaskDelay(15000 / portTICK_RATE_MS); ESP_ERROR_CHECK(esp_eth_disable()); ESP_ERROR_CHECK(esp_eth_deinit()); diff --git a/components/lwip/port/esp32/netif/ethernetif.c b/components/lwip/port/esp32/netif/ethernetif.c index e17dd1bc4..48693a934 100644 --- a/components/lwip/port/esp32/netif/ethernetif.c +++ b/components/lwip/port/esp32/netif/ethernetif.c @@ -196,7 +196,6 @@ if (netif->input(p, netif) != ERR_OK) { /* full packet send to tcpip_thread to process */ if (netif->input(p, netif) != ERR_OK) { LWIP_DEBUGF(NETIF_DEBUG, ("ethernetif_input: IP input error\n")); - p->l2_owner = NULL; pbuf_free(p); } #endif