From 37154d4c08b57c2b5410138a4c516ff5ecf93e2b Mon Sep 17 00:00:00 2001 From: morris Date: Wed, 27 Nov 2019 21:38:27 +0800 Subject: [PATCH] ethernet:add reference counter --- components/esp_eth/CMakeLists.txt | 3 +++ components/esp_eth/component.mk | 3 +++ components/esp_eth/include/esp_eth.h | 29 +++++++++++++++++++++++++ components/esp_eth/src/esp_eth.c | 30 ++++++++++++++++++++++++++ components/esp_eth/test/test_emac.c | 32 +++++++++++++++++++++++----- 5 files changed, 92 insertions(+), 5 deletions(-) diff --git a/components/esp_eth/CMakeLists.txt b/components/esp_eth/CMakeLists.txt index ed9845218..147caf988 100644 --- a/components/esp_eth/CMakeLists.txt +++ b/components/esp_eth/CMakeLists.txt @@ -17,3 +17,6 @@ idf_component_register(SRCS "${esp_eth_srcs}" INCLUDE_DIRS "include" REQUIRES "esp_event" PRIV_REQUIRES "tcpip_adapter" "driver" "log") + +# uses C11 atomic feature +set_source_files_properties(src/esp_eth.c PROPERTIES COMPILE_FLAGS -std=gnu11) diff --git a/components/esp_eth/component.mk b/components/esp_eth/component.mk index a9c95f19a..c9991de51 100644 --- a/components/esp_eth/component.mk +++ b/components/esp_eth/component.mk @@ -11,3 +11,6 @@ endif ifndef CONFIG_ETH_SPI_ETHERNET_DM9051 COMPONENT_OBJEXCLUDE += src/esp_eth_mac_dm9051.o src/esp_eth_phy_dm9051.o endif + +# uses C11 atomic feature +src/esp_eth.o: CFLAGS += -std=gnu11 diff --git a/components/esp_eth/include/esp_eth.h b/components/esp_eth/include/esp_eth.h index cf4b57050..6f4cb0fad 100644 --- a/components/esp_eth/include/esp_eth.h +++ b/components/esp_eth/include/esp_eth.h @@ -117,12 +117,16 @@ esp_err_t esp_eth_driver_install(const esp_eth_config_t *config, esp_eth_handle_ /** * @brief Uninstall Ethernet driver +* @note It's not recommended to uninstall Ethernet driver unless it won't get used any more in application code. +* To uninstall Ethernet driver, you have to make sure, all references to the driver are released. +* Ethernet driver can only be uninstalled successfully when reference counter equals to one. * * @param[in] hdl: handle of Ethernet driver * * @return * - ESP_OK: uninstall esp_eth driver successfully * - ESP_ERR_INVALID_ARG: uninstall esp_eth driver failed because of some invalid argument +* - ESP_ERR_INVALID_STATE: uninstall esp_eth driver failed because it has more than one reference * - ESP_FAIL: uninstall esp_eth driver failed because some other error occurred */ esp_err_t esp_eth_driver_uninstall(esp_eth_handle_t hdl); @@ -169,6 +173,31 @@ esp_err_t esp_eth_receive(esp_eth_handle_t hdl, uint8_t *buf, uint32_t *length); */ esp_err_t esp_eth_ioctl(esp_eth_handle_t hdl, esp_eth_io_cmd_t cmd, void *data); +/** +* @brief Increase Ethernet driver reference +* @note Ethernet driver handle can be obtained by os timer, netif, etc. +* It's dangerous when thread A is using Ethernet but thread B uninstall the driver. +* Using reference counter can prevent such risk, but care should be taken, when you obtain Ethernet driver, +* this API must be invoked so that the driver won't be uninstalled during your using time. +* +* +* @param[in] hdl: handle of Ethernet driver +* @return +* - ESP_OK: increase reference successfully +* - ESP_ERR_INVALID_ARG: increase reference failed because of some invalid argument +*/ +esp_err_t esp_eth_increase_reference(esp_eth_handle_t hdl); + +/** +* @brief Decrease Ethernet driver reference +* +* @param[in] hdl: handle of Ethernet driver +* @return +* - ESP_OK: increase reference successfully +* - ESP_ERR_INVALID_ARG: increase reference failed because of some invalid argument +*/ +esp_err_t esp_eth_decrease_reference(esp_eth_handle_t hdl); + #ifdef __cplusplus } #endif diff --git a/components/esp_eth/src/esp_eth.c b/components/esp_eth/src/esp_eth.c index ea813687d..9444f6b32 100644 --- a/components/esp_eth/src/esp_eth.c +++ b/components/esp_eth/src/esp_eth.c @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. #include +#include #include "esp_log.h" #include "esp_eth.h" #include "esp_event.h" @@ -51,6 +52,7 @@ typedef struct { eth_speed_t speed; eth_duplex_t duplex; eth_link_t link; + atomic_int ref_count; esp_err_t (*stack_input)(esp_eth_handle_t eth_handle, uint8_t *buffer, uint32_t length); esp_err_t (*on_lowlevel_init_done)(esp_eth_handle_t eth_handle); esp_err_t (*on_lowlevel_deinit_done)(esp_eth_handle_t eth_handle); @@ -145,7 +147,9 @@ static void eth_check_link_timer_cb(TimerHandle_t xTimer) { esp_eth_driver_t *eth_driver = (esp_eth_driver_t *)pvTimerGetTimerID(xTimer); esp_eth_phy_t *phy = eth_driver->phy; + esp_eth_increase_reference(eth_driver); phy->get_link(phy); + esp_eth_decrease_reference(eth_driver); } ////////////////////////////////User face APIs//////////////////////////////////////////////// @@ -164,6 +168,7 @@ esp_err_t esp_eth_driver_install(const esp_eth_config_t *config, esp_eth_handle_ ETH_CHECK(mac && phy, "can't set eth->mac or eth->phy to null", err, ESP_ERR_INVALID_ARG); esp_eth_driver_t *eth_driver = calloc(1, sizeof(esp_eth_driver_t)); ETH_CHECK(eth_driver, "request memory for eth_driver failed", err, ESP_ERR_NO_MEM); + atomic_init(ð_driver->ref_count, 1); eth_driver->mac = mac; eth_driver->phy = phy; eth_driver->link = ETH_LINK_DOWN; @@ -210,6 +215,9 @@ esp_err_t esp_eth_driver_uninstall(esp_eth_handle_t hdl) esp_err_t ret = ESP_OK; esp_eth_driver_t *eth_driver = (esp_eth_driver_t *)hdl; ETH_CHECK(eth_driver, "ethernet driver handle can't be null", err, ESP_ERR_INVALID_ARG); + // don't uninstall driver unless there's only one reference + ETH_CHECK(atomic_load(ð_driver->ref_count) == 1, + "more than one reference to ethernet driver", err, ESP_ERR_INVALID_STATE); esp_eth_mac_t *mac = eth_driver->mac; esp_eth_phy_t *phy = eth_driver->phy; ETH_CHECK(xTimerDelete(eth_driver->check_link_timer, 0) == pdPASS, "delete eth_link_timer failed", err, ESP_FAIL); @@ -284,3 +292,25 @@ esp_err_t esp_eth_ioctl(esp_eth_handle_t hdl, esp_eth_io_cmd_t cmd, void *data) err: return ret; } + +esp_err_t esp_eth_increase_reference(esp_eth_handle_t hdl) +{ + esp_err_t ret = ESP_OK; + esp_eth_driver_t *eth_driver = (esp_eth_driver_t *)hdl; + ETH_CHECK(eth_driver, "ethernet driver handle can't be null", err, ESP_ERR_INVALID_ARG); + atomic_fetch_add(ð_driver->ref_count, 1); + return ESP_OK; +err: + return ret; +} + +esp_err_t esp_eth_decrease_reference(esp_eth_handle_t hdl) +{ + esp_err_t ret = ESP_OK; + esp_eth_driver_t *eth_driver = (esp_eth_driver_t *)hdl; + ETH_CHECK(eth_driver, "ethernet driver handle can't be null", err, ESP_ERR_INVALID_ARG); + atomic_fetch_sub(ð_driver->ref_count, 1); + return ESP_OK; +err: + return ret; +} diff --git a/components/esp_eth/test/test_emac.c b/components/esp_eth/test/test_emac.c index 78995d8c5..3a036518f 100644 --- a/components/esp_eth/test/test_emac.c +++ b/components/esp_eth/test/test_emac.c @@ -67,6 +67,24 @@ static void got_ip_event_handler(void *arg, esp_event_base_t event_base, xEventGroupSetBits(eth_event_group, ETH_GOT_IP_BIT); } + +static esp_err_t test_uninstall_driver(esp_eth_handle_t eth_hdl, uint32_t ms_to_wait) +{ + int i = 0; + ms_to_wait += 100; + for (i = 0; i < ms_to_wait / 100; i++) { + vTaskDelay(pdMS_TO_TICKS(100)); + if (esp_eth_driver_uninstall(eth_hdl) == ESP_OK) { + break; + } + } + if (i < ms_to_wait / 10) { + return ESP_OK; + } else { + return ESP_FAIL; + } +} + TEST_CASE("esp32 ethernet io test", "[ethernet][test_env=UT_T2_Ethernet]") { TEST_ESP_OK(esp_event_loop_create_default()); @@ -90,7 +108,8 @@ TEST_CASE("esp32 ethernet io test", "[ethernet][test_env=UT_T2_Ethernet]") TEST_ESP_OK(esp_eth_ioctl(eth_handle, ETH_CMD_G_PHY_ADDR, &phy_addr)); ESP_LOGI(TAG, "Ethernet PHY Address: %d", phy_addr); TEST_ASSERT(phy_addr >= 0 && phy_addr <= 31); - TEST_ESP_OK(esp_eth_driver_uninstall(eth_handle)); + /* driver should be uninstalled within 2 seconds */ + TEST_ESP_OK(test_uninstall_driver(eth_handle, 2000)); TEST_ESP_OK(phy->del(phy)); TEST_ESP_OK(mac->del(mac)); TEST_ESP_OK(esp_event_loop_delete_default()); @@ -116,7 +135,8 @@ TEST_CASE("esp32 ethernet event test", "[ethernet][test_env=UT_T2_Ethernet]") /* wait for connection establish */ bits = xEventGroupWaitBits(eth_event_group, ETH_CONNECT_BIT, true, true, pdMS_TO_TICKS(ETH_CONNECT_TIMEOUT_MS)); TEST_ASSERT((bits & ETH_CONNECT_BIT) == ETH_CONNECT_BIT); - TEST_ESP_OK(esp_eth_driver_uninstall(eth_handle)); + /* driver should be uninstalled within 2 seconds */ + TEST_ESP_OK(test_uninstall_driver(eth_handle, 2000)); /* wait for connection stop */ bits = xEventGroupWaitBits(eth_event_group, ETH_STOP_BIT, true, true, pdMS_TO_TICKS(ETH_STOP_TIMEOUT_MS)); TEST_ASSERT((bits & ETH_STOP_BIT) == ETH_STOP_BIT); @@ -149,7 +169,8 @@ TEST_CASE("esp32 ethernet dhcp test", "[ethernet][test_env=UT_T2_Ethernet]") /* wait for IP lease */ bits = xEventGroupWaitBits(eth_event_group, ETH_GOT_IP_BIT, true, true, pdMS_TO_TICKS(ETH_GET_IP_TIMEOUT_MS)); TEST_ASSERT((bits & ETH_GOT_IP_BIT) == ETH_GOT_IP_BIT); - TEST_ESP_OK(esp_eth_driver_uninstall(eth_handle)); + /* driver should be uninstalled within 2 seconds */ + TEST_ESP_OK(test_uninstall_driver(eth_handle, 2000)); /* wait for connection stop */ bits = xEventGroupWaitBits(eth_event_group, ETH_STOP_BIT, true, true, pdMS_TO_TICKS(ETH_STOP_TIMEOUT_MS)); TEST_ASSERT((bits & ETH_STOP_BIT) == ETH_STOP_BIT); @@ -186,7 +207,7 @@ TEST_CASE("dm9051 io test", "[ethernet][ignore]") }; TEST_ESP_OK(spi_bus_add_device(HSPI_HOST, &devcfg, &spi_handle)); gpio_install_isr_service(0); - tcpip_adapter_init(); + test_case_uses_tcpip(); TEST_ESP_OK(esp_event_loop_create_default()); TEST_ESP_OK(tcpip_adapter_set_default_eth_handlers()); TEST_ESP_OK(esp_event_handler_register(ETH_EVENT, ESP_EVENT_ANY_ID, ð_event_handler, NULL)); @@ -200,7 +221,8 @@ TEST_CASE("dm9051 io test", "[ethernet][ignore]") esp_eth_handle_t eth_handle = NULL; TEST_ESP_OK(esp_eth_driver_install(&config, ð_handle)); vTaskDelay(pdMS_TO_TICKS(portMAX_DELAY)); - TEST_ESP_OK(esp_eth_driver_uninstall(eth_handle)); + /* driver should be uninstalled within 2 seconds */ + TEST_ESP_OK(test_uninstall_driver(eth_handle, 2000)); TEST_ESP_OK(phy->del(phy)); TEST_ESP_OK(mac->del(mac)); TEST_ESP_OK(esp_event_loop_delete_default());