Merge branch 'bugfix/eth_lan8720_ci' into 'master'

esp_eth: start/stop and L2 test stability improvements

See merge request espressif/esp-idf!21898

components/esp_eth/src/esp_eth.c

@@ -50,7 +50,7 @@ typedef struct {
     bool auto_nego_en;
     eth_speed_t speed;
     eth_duplex_t duplex;
-    eth_link_t link;
+    _Atomic eth_link_t link;
     atomic_int ref_count;
     void *priv;
     _Atomic esp_eth_fsm_t fsm;
@@ -129,7 +129,7 @@ static esp_err_t eth_on_state_changed(esp_eth_mediator_t *eth, esp_eth_state_t s
     case ETH_STATE_LINK: {
         eth_link_t link = (eth_link_t)args;
         ESP_GOTO_ON_ERROR(mac->set_link(mac, link), err, TAG, "ethernet mac set link failed");
-        eth_driver->link = link;
+        atomic_store(&eth_driver->link, link);
         if (link == ETH_LINK_UP) {
             ESP_GOTO_ON_ERROR(esp_event_post(ETH_EVENT, ETHERNET_EVENT_CONNECTED, &eth_driver, sizeof(esp_eth_driver_t *), 0), err,
                               TAG, "send ETHERNET_EVENT_CONNECTED event failed");
@@ -205,7 +205,7 @@ esp_err_t esp_eth_driver_install(const esp_eth_config_t *config, esp_eth_handle_
     atomic_init(&eth_driver->fsm, ESP_ETH_FSM_STOP);
     eth_driver->mac = mac;
     eth_driver->phy = phy;
-    eth_driver->link = ETH_LINK_DOWN;
+    atomic_init(&eth_driver->link, ETH_LINK_DOWN);
     eth_driver->duplex = ETH_DUPLEX_HALF;
     eth_driver->speed = ETH_SPEED_10M;
     eth_driver->stack_input = config->stack_input;
@@ -309,7 +309,14 @@ esp_err_t esp_eth_stop(esp_eth_handle_t hdl)
     ESP_GOTO_ON_FALSE(atomic_compare_exchange_strong(&eth_driver->fsm, &expected_fsm, ESP_ETH_FSM_STOP),
                       ESP_ERR_INVALID_STATE, err, TAG, "driver not started yet");
     ESP_GOTO_ON_ERROR(esp_timer_stop(eth_driver->check_link_timer), err, TAG, "stop link timer failed");
-    ESP_GOTO_ON_ERROR(mac->stop(mac), err, TAG, "stop mac failed");
+
+    eth_link_t expected_link = ETH_LINK_UP;
+    if (atomic_compare_exchange_strong(&eth_driver->link, &expected_link, ETH_LINK_DOWN)){
+        // MAC is stopped by setting link down
+        ESP_GOTO_ON_ERROR(mac->set_link(mac, ETH_LINK_DOWN), err, TAG, "ethernet mac set link failed");
+        ESP_GOTO_ON_ERROR(esp_event_post(ETH_EVENT, ETHERNET_EVENT_DISCONNECTED, &eth_driver, sizeof(esp_eth_driver_t *), 0), err,
+                            TAG, "send ETHERNET_EVENT_DISCONNECTED event failed");
+    }
 
     ESP_GOTO_ON_ERROR(esp_event_post(ETH_EVENT, ETHERNET_EVENT_STOP, &eth_driver, sizeof(esp_eth_driver_t *), 0),
                       err, TAG, "send ETHERNET_EVENT_STOP event failed");
@@ -336,9 +343,9 @@ esp_err_t esp_eth_transmit(esp_eth_handle_t hdl, void *buf, size_t length)
     esp_err_t ret = ESP_OK;
     esp_eth_driver_t *eth_driver = (esp_eth_driver_t *)hdl;
 
-    if (atomic_load(&eth_driver->fsm) != ESP_ETH_FSM_START) {
+    if (atomic_load(&eth_driver->link) != ETH_LINK_UP) {
         ret = ESP_ERR_INVALID_STATE;
-        ESP_LOGD(TAG, "Ethernet is not started");
+        ESP_LOGD(TAG, "Ethernet link is not up, can't transmit");
         goto err;
     }
 

components/esp_eth/src/esp_eth_mac_esp.c

@@ -34,7 +34,7 @@
 static const char *TAG = "esp.emac";
 
 #define PHY_OPERATION_TIMEOUT_US (1000)
-#define MAC_STOP_TIMEOUT_US (250)
+#define MAC_STOP_TIMEOUT_US (2500) // this is absolute maximum for 10Mbps, it is 10 times faster for 100Mbps
 #define FLOW_CONTROL_LOW_WATER_MARK (CONFIG_ETH_DMA_RX_BUFFER_NUM / 3)
 #define FLOW_CONTROL_HIGH_WATER_MARK (FLOW_CONTROL_LOW_WATER_MARK * 2)
 
@@ -262,7 +262,6 @@ static esp_err_t emac_esp32_receive(esp_eth_mac_t *mac, uint8_t *buf, uint32_t *
     ESP_GOTO_ON_FALSE(buf && length, ESP_ERR_INVALID_ARG, err, TAG, "can't set buf and length to null");
     uint32_t receive_len = emac_hal_receive_frame(&emac->hal, buf, expected_len, &emac->frames_remain, &emac->free_rx_descriptor);
     /* we need to check the return value in case the buffer size is not enough */
-    ESP_LOGD(TAG, "receive len= %d", receive_len);
     ESP_GOTO_ON_FALSE(expected_len >= receive_len, ESP_ERR_INVALID_SIZE, err, TAG, "received buffer longer than expected");
     *length = receive_len;
     return ESP_OK;
@@ -370,8 +369,6 @@ static esp_err_t emac_esp32_init(esp_eth_mac_t *mac)
     ESP_GOTO_ON_FALSE(to < emac->sw_reset_timeout_ms / 10, ESP_ERR_TIMEOUT, err, TAG, "reset timeout");
     /* set smi clock */
     emac_hal_set_csr_clock_range(&emac->hal, esp_clk_apb_freq());
-    /* reset descriptor chain */
-    emac_hal_reset_desc_chain(&emac->hal);
     /* init mac registers by default */
     emac_hal_init_mac_default(&emac->hal);
     /* init dma registers with selected EMAC-DMA configuration */
@@ -406,6 +403,7 @@ static esp_err_t emac_esp32_deinit(esp_eth_mac_t *mac)
 static esp_err_t emac_esp32_start(esp_eth_mac_t *mac)
 {
     emac_esp32_t *emac = __containerof(mac, emac_esp32_t, parent);
+    /* reset descriptor chain */
     emac_hal_reset_desc_chain(&emac->hal);
     emac_hal_start(&emac->hal);
     return ESP_OK;

components/esp_eth/src/esp_eth_phy_802_3.c

@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2022 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2022-2023 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -230,12 +230,9 @@ static esp_err_t eth_phy_802_3_set_speed(esp_eth_phy_t *phy, eth_speed_t speed)
     phy_802_3_t *phy_802_3 = __containerof(phy, phy_802_3_t, parent);
     esp_eth_mediator_t *eth = phy_802_3->eth;
 
-    if (phy_802_3->link_status == ETH_LINK_UP) {
+    /* Since the link is going to be reconfigured, consider it down to be status updated once the driver re-started */
-        /* Since the link is going to be reconfigured, consider it down for a while */
+    phy_802_3->link_status = ETH_LINK_DOWN;
-        phy_802_3->link_status = ETH_LINK_DOWN;
+
-        /* Indicate to upper stream apps the link is cosidered down */
-        ESP_GOTO_ON_ERROR(eth->on_state_changed(eth, ETH_STATE_LINK, (void *)phy_802_3->link_status), err, TAG, "change link failed");
-    }
     /* Set speed */
     bmcr_reg_t bmcr;
     ESP_GOTO_ON_ERROR(eth->phy_reg_read(eth, phy_802_3->addr, ETH_PHY_BMCR_REG_ADDR, &(bmcr.val)), err, TAG, "read BMCR failed");
@@ -253,12 +250,9 @@ static esp_err_t eth_phy_802_3_set_duplex(esp_eth_phy_t *phy, eth_duplex_t duple
     phy_802_3_t *phy_802_3 = __containerof(phy, phy_802_3_t, parent);
     esp_eth_mediator_t *eth = phy_802_3->eth;
 
-    if (phy_802_3->link_status == ETH_LINK_UP) {
+    /* Since the link is going to be reconfigured, consider it down to be status updated once the driver re-started */
-        /* Since the link is going to be reconfigured, consider it down for a while */
+    phy_802_3->link_status = ETH_LINK_DOWN;
-        phy_802_3->link_status = ETH_LINK_DOWN;
+
-        /* Indicate to upper stream apps the link is cosidered down */
-        ESP_GOTO_ON_ERROR(eth->on_state_changed(eth, ETH_STATE_LINK, (void *)phy_802_3->link_status), err, TAG, "change link failed");
-    }
     /* Set duplex mode */
     bmcr_reg_t bmcr;
     ESP_GOTO_ON_ERROR(eth->phy_reg_read(eth, phy_802_3->addr, ETH_PHY_BMCR_REG_ADDR, &(bmcr.val)), err, TAG, "read BMCR failed");

components/esp_eth/src/esp_eth_phy_ksz8851snl.c

@@ -288,12 +288,9 @@ static esp_err_t phy_ksz8851_set_speed(esp_eth_phy_t *phy, eth_speed_t speed)
     phy_ksz8851snl_t *ksz8851 = __containerof(phy, phy_ksz8851snl_t, parent);
     esp_eth_mediator_t *eth   = ksz8851->eth;
 
-    if (ksz8851->link_status == ETH_LINK_UP) {
+    /* Since the link is going to be reconfigured, consider it down to be status updated once the driver re-started */
-        /* Since the link is going to be reconfigured, consider it down for a while */
+    ksz8851->link_status = ETH_LINK_DOWN;
-        ksz8851->link_status = ETH_LINK_DOWN;
+
-        /* Indicate to upper stream apps the link is cosidered down */
-        ESP_GOTO_ON_ERROR(eth->on_state_changed(eth, ETH_STATE_LINK, (void *)ksz8851->link_status), err, TAG, "change link failed");
-    }
     /* Set speed */
     uint32_t control;
     ESP_GOTO_ON_ERROR(eth->phy_reg_read(eth, ksz8851->addr, KSZ8851_P1CR, &control), err, TAG, "P1CR read failed");
@@ -315,12 +312,9 @@ static esp_err_t phy_ksz8851_set_duplex(esp_eth_phy_t *phy, eth_duplex_t duplex)
     phy_ksz8851snl_t *ksz8851 = __containerof(phy, phy_ksz8851snl_t, parent);
     esp_eth_mediator_t *eth   = ksz8851->eth;
 
-    if (ksz8851->link_status == ETH_LINK_UP) {
+    /* Since the link is going to be reconfigured, consider it down to be status updated once the driver re-started */
-        /* Since the link is going to be reconfigured, consider it down for a while */
+    ksz8851->link_status = ETH_LINK_DOWN;
-        ksz8851->link_status = ETH_LINK_DOWN;
+
-        /* Indicate to upper stream apps the link is cosidered down */
-        ESP_GOTO_ON_ERROR(eth->on_state_changed(eth, ETH_STATE_LINK, (void *)ksz8851->link_status), err, TAG, "change link failed");
-    }
     /* Set duplex mode */
     uint32_t control;
     ESP_GOTO_ON_ERROR(eth->phy_reg_read(eth, ksz8851->addr, KSZ8851_P1CR, &control), err, TAG, "P1CR read failed");

components/esp_eth/src/esp_eth_phy_w5500.c

@@ -259,12 +259,8 @@ static esp_err_t w5500_set_speed(esp_eth_phy_t *phy, eth_speed_t speed)
     phy_w5500_t *w5500 = __containerof(phy, phy_w5500_t, parent);
     esp_eth_mediator_t *eth = w5500->eth;
 
-    if (w5500->link_status == ETH_LINK_UP) {
+    /* Since the link is going to be reconfigured, consider it down to be status updated once the driver re-started */
-        /* Since the link is going to be reconfigured, consider it down for a while */
+    w5500->link_status = ETH_LINK_DOWN;
-        w5500->link_status = ETH_LINK_DOWN;
-        /* Indicate to upper stream apps the link is cosidered down */
-        ESP_GOTO_ON_ERROR(eth->on_state_changed(eth, ETH_STATE_LINK, (void *)w5500->link_status), err, TAG, "change link failed");
-    }
 
     phycfg_reg_t phycfg;
     ESP_GOTO_ON_ERROR(eth->phy_reg_read(eth, w5500->addr, W5500_REG_PHYCFGR, (uint32_t *) & (phycfg.val)), err, TAG, "read PHYCFG failed");
@@ -298,12 +294,8 @@ static esp_err_t w5500_set_duplex(esp_eth_phy_t *phy, eth_duplex_t duplex)
     phy_w5500_t *w5500 = __containerof(phy, phy_w5500_t, parent);
     esp_eth_mediator_t *eth = w5500->eth;
 
-    if (w5500->link_status == ETH_LINK_UP) {
+    /* Since the link is going to be reconfigured, consider it down to be status updated once the driver re-started */
-        /* Since the link is going to be reconfigured, consider it down for a while */
+    w5500->link_status = ETH_LINK_DOWN;
-        w5500->link_status = ETH_LINK_DOWN;
-        /* Indicate to upper stream apps the link is cosidered down */
-        ESP_GOTO_ON_ERROR(eth->on_state_changed(eth, ETH_STATE_LINK, (void *)w5500->link_status), err, TAG, "change link failed");
-    }
 
     phycfg_reg_t phycfg;
     ESP_GOTO_ON_ERROR(eth->phy_reg_read(eth, w5500->addr, W5500_REG_PHYCFGR, (uint32_t *) & (phycfg.val)), err, TAG, "read PHYCFG failed");

components/esp_eth/test_apps/main/esp_eth_test_l2.c

@@ -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
  */
@@ -9,20 +9,112 @@
 #include "freertos/task.h"
 #include "freertos/event_groups.h"
 #include "esp_eth_test_common.h"
+#include "arpa/inet.h" // for ntohs, etc.
+#include "esp_log.h"
+
+#define TEST_ETH_TYPE 0x2222
+#define TEST_CTRL_ETH_TYPE (TEST_ETH_TYPE + 1)
+
+#define WAIT_AFTER_CONN_MS 2500
+#define WAIT_AFTER_CONN_TMO_MS 20000
 
 #define ETH_BROADCAST_RECV_BIT BIT(0)
 #define ETH_MULTICAST_RECV_BIT BIT(1)
 #define ETH_UNICAST_RECV_BIT BIT(2)
+#define ETH_POKE_RESP_RECV_BIT BIT(3)
 
 #define POKE_REQ 0xFA
 #define POKE_RESP 0xFB
 #define DUMMY_TRAFFIC 0xFF
 
+typedef struct
+{
+    EventGroupHandle_t eth_event_group;
+    int unicast_rx_cnt;
+    int multicast_rx_cnt;
+    int brdcast_rx_cnt;
+
+    bool check_rx_data;
+} recv_info_t;
+
+esp_err_t l2_packet_txrx_test_cb(esp_eth_handle_t hdl, uint8_t *buffer, uint32_t length, void *priv) {
+    recv_info_t *recv_info = (recv_info_t*)priv;
+    EventGroupHandle_t eth_event_group = recv_info->eth_event_group;
+    emac_frame_t *pkt = (emac_frame_t *)buffer;
+    // check header
+    if (pkt->proto == TEST_ETH_TYPE) { // data packet
+        uint8_t local_mac_addr[ETH_ADDR_LEN];
+        esp_eth_ioctl(hdl, ETH_CMD_G_MAC_ADDR, local_mac_addr);
+        // check data content
+        if (recv_info->check_rx_data) {
+            if (length == 1024) {
+                for (int i = 0; i < (length - ETH_HEADER_LEN); ++i) {
+                    if (pkt->data[i] != (i & 0xff)) {
+                        printf("payload mismatch\n");
+                        free(buffer);
+                        return ESP_OK;
+                    }
+                }
+            }
+        }
+
+        if (memcmp(pkt->dest, "\xff\xff\xff\xff\xff\xff", ETH_ADDR_LEN) == 0) {
+            recv_info->brdcast_rx_cnt++;
+            xEventGroupSetBits(eth_event_group, ETH_BROADCAST_RECV_BIT);
+        } else if (pkt->dest[0] & 0x1) {
+            recv_info->multicast_rx_cnt++;
+            xEventGroupSetBits(eth_event_group, ETH_MULTICAST_RECV_BIT);
+        } else if (memcmp(pkt->dest, local_mac_addr, ETH_ADDR_LEN) == 0) {
+            recv_info->unicast_rx_cnt++;
+            xEventGroupSetBits(eth_event_group, ETH_UNICAST_RECV_BIT);
+        }
+    } else if (ntohs(pkt->proto) == TEST_CTRL_ETH_TYPE) { // control packet
+        if (pkt->data[0] == POKE_RESP) {
+            printf("Poke response received\n");
+            xEventGroupSetBits(eth_event_group, ETH_POKE_RESP_RECV_BIT);
+        }
+    }
+    free(buffer);
+    return ESP_OK;
+}
+
+/**
+ * @brief The function sends a "POKE" request message over the Ethernet and waits until the test script sends a reply.
+ *        Multiple "POKE" attempts are issued when timeout for the reply expires.
+ *        This function is used to drive the test flow and to ensure that data path between the test points
+ *        has been established. I.e. if DUT is connected in network with a switch, even if link is indicated up,
+ *        it may take some time the switch starts forwarding the associated port (e.g. it runs RSTP at first).
+ */
+void poke_and_wait(esp_eth_handle_t eth_handle, void *data, uint16_t size, EventGroupHandle_t eth_event_group)
+{
+    // create a control frame to control test flow between the UT and the Python test script
+    emac_frame_t *ctrl_pkt = calloc(1, 60);
+    ctrl_pkt->proto = htons(TEST_CTRL_ETH_TYPE);
+    memset(ctrl_pkt->dest, 0xff, ETH_ADDR_LEN); // broadcast addr
+    esp_eth_ioctl(eth_handle, ETH_CMD_G_MAC_ADDR, ctrl_pkt->src);
+
+    ctrl_pkt->data[0] = POKE_REQ;
+    if (data != NULL && size > 0) {
+        memcpy(&ctrl_pkt->data[1], data, size);
+    }
+
+    uint32_t tmo;
+    uint32_t i;
+    for(tmo = 0, i = 1; tmo < WAIT_AFTER_CONN_TMO_MS; tmo += WAIT_AFTER_CONN_MS, i++) {
+        printf("Poke attempt #%" PRIu32 "\n", i);
+        TEST_ESP_OK(esp_eth_transmit(eth_handle, ctrl_pkt, 60));
+        EventBits_t bits = xEventGroupWaitBits(eth_event_group, ETH_POKE_RESP_RECV_BIT,
+                               true, true, pdMS_TO_TICKS(WAIT_AFTER_CONN_MS));
+        if ((bits & ETH_POKE_RESP_RECV_BIT) == ETH_POKE_RESP_RECV_BIT) {
+            break;
+        }
+    }
+    TEST_ASSERT(tmo < WAIT_AFTER_CONN_TMO_MS);
+    free(ctrl_pkt);
+}
+
 TEST_CASE("ethernet broadcast transmit", "[ethernet_l2]")
 {
-    EventGroupHandle_t eth_event_group = xEventGroupCreate();
-    TEST_ASSERT(eth_event_group != NULL);
-
     esp_eth_mac_t *mac = mac_init(NULL, NULL);
     TEST_ASSERT_NOT_NULL(mac);
     esp_eth_phy_t *phy = phy_init(NULL);
@@ -32,19 +124,33 @@ TEST_CASE("ethernet broadcast transmit", "[ethernet_l2]")
     TEST_ESP_OK(esp_eth_driver_install(&config, &eth_handle)); // install driver
     TEST_ASSERT_NOT_NULL(eth_handle);
     extra_eth_config(eth_handle);
+
     TEST_ESP_OK(esp_event_loop_create_default());
-    TEST_ESP_OK(esp_event_handler_register(ETH_EVENT, ESP_EVENT_ANY_ID, &eth_event_handler, eth_event_group));
+    EventGroupHandle_t eth_event_state_group = xEventGroupCreate();
+    TEST_ASSERT(eth_event_state_group != NULL);
+    TEST_ESP_OK(esp_event_handler_register(ETH_EVENT, ESP_EVENT_ANY_ID, &eth_event_handler, eth_event_state_group));
+    EventGroupHandle_t eth_event_rx_group = xEventGroupCreate();
+    TEST_ASSERT(eth_event_rx_group != NULL);
+    recv_info_t recv_info = {
+        .eth_event_group = eth_event_rx_group,
+        .check_rx_data = false,
+        .unicast_rx_cnt = 0,
+        .multicast_rx_cnt = 0,
+        .brdcast_rx_cnt = 0
+    };
+
+    TEST_ESP_OK(esp_eth_update_input_path(eth_handle, l2_packet_txrx_test_cb, &recv_info));
     TEST_ESP_OK(esp_eth_start(eth_handle)); // start Ethernet driver state machine
 
     EventBits_t bits = 0;
-    bits = xEventGroupWaitBits(eth_event_group, ETH_CONNECT_BIT, true, true, pdMS_TO_TICKS(3000));
+    bits = xEventGroupWaitBits(eth_event_state_group, ETH_CONNECT_BIT, true, true, pdMS_TO_TICKS(3000));
     TEST_ASSERT((bits & ETH_CONNECT_BIT) == ETH_CONNECT_BIT);
     // if DUT is connected in network with switch: even if link is indicated up, it may take some time the switch
     // starts switching the associated port (e.g. it runs RSTP at first)
-    vTaskDelay(pdMS_TO_TICKS(1000));
+    poke_and_wait(eth_handle, NULL, 0, eth_event_rx_group);
 
     emac_frame_t *pkt = malloc(1024);
-    pkt->proto = 0x2222;
+    pkt->proto = TEST_ETH_TYPE;
     TEST_ESP_OK(esp_eth_ioctl(eth_handle, ETH_CMD_G_MAC_ADDR, pkt->src));
     memset(pkt->dest, 0xff, ETH_ADDR_LEN);     // broadcast addr
     for (int i = 0; i < (1024 - ETH_HEADER_LEN); ++i){
@@ -61,44 +167,12 @@ TEST_CASE("ethernet broadcast transmit", "[ethernet_l2]")
     phy->del(phy);
     mac->del(mac);
     extra_cleanup();
-    vEventGroupDelete(eth_event_group);
+    vEventGroupDelete(eth_event_rx_group);
+    vEventGroupDelete(eth_event_state_group);
 }
 
-static uint8_t local_mac_addr[ETH_ADDR_LEN] = {};
-
-esp_err_t l2_packet_txrx_test_cb(esp_eth_handle_t hdl, uint8_t *buffer, uint32_t length, void *priv) {
-    EventGroupHandle_t eth_event_group = (EventGroupHandle_t)priv;
-    emac_frame_t *pkt = (emac_frame_t *) buffer;
-    // check header
-    if (pkt->proto == 0x2222 && length == 1024) {
-        // check content
-        for (int i = 0; i < (length - ETH_HEADER_LEN); ++i) {
-            if (pkt->data[i] != (i & 0xff)) {
-                printf("payload mismatch\n");
-                return ESP_OK;
-            }
-        }
-        if (memcmp(pkt->dest, "\xff\xff\xff\xff\xff\xff", ETH_ADDR_LEN) == 0) {
-            printf("broadcast received...\n");
-            xEventGroupSetBits(eth_event_group, ETH_BROADCAST_RECV_BIT);
-        } else if (pkt->dest[0] & 0x1) {
-            printf("multicast received...\n");
-            xEventGroupSetBits(eth_event_group, ETH_MULTICAST_RECV_BIT);
-        } else if (memcmp(pkt->dest, local_mac_addr, ETH_ADDR_LEN) == 0) {
-            printf("unicast received...\n");
-            xEventGroupSetBits(eth_event_group, ETH_UNICAST_RECV_BIT);
-        }
-    } else {
-        printf("unexpected frame (protocol: 0x%x, length: %" PRIu32 ")\n", pkt->proto, length);
-    }
-    return ESP_OK;
-};
-
 TEST_CASE("ethernet recv_pkt", "[ethernet_l2]")
 {
-    EventGroupHandle_t eth_event_group = xEventGroupCreate();
-    TEST_ASSERT(eth_event_group != NULL);
-
     esp_eth_mac_t *mac = mac_init(NULL, NULL);
     TEST_ASSERT_NOT_NULL(mac);
     esp_eth_phy_t *phy = phy_init(NULL);
@@ -108,19 +182,41 @@ TEST_CASE("ethernet recv_pkt", "[ethernet_l2]")
     TEST_ESP_OK(esp_eth_driver_install(&config, &eth_handle)); // install driver
     TEST_ASSERT_NOT_NULL(eth_handle);
     extra_eth_config(eth_handle);
-    TEST_ESP_OK(esp_event_loop_create_default());
-    TEST_ESP_OK(esp_eth_start(eth_handle)); // start Ethernet driver state machine
 
+    TEST_ESP_OK(esp_event_loop_create_default());
+    EventGroupHandle_t eth_event_state_group = xEventGroupCreate();
+    TEST_ASSERT(eth_event_state_group != NULL);
+    TEST_ESP_OK(esp_event_handler_register(ETH_EVENT, ESP_EVENT_ANY_ID, &eth_event_handler, eth_event_state_group));
+    EventGroupHandle_t eth_event_rx_group = xEventGroupCreate();
+    TEST_ASSERT(eth_event_rx_group != NULL);
+    recv_info_t recv_info = {
+        .eth_event_group = eth_event_rx_group,
+        .check_rx_data = true,
+        .unicast_rx_cnt = 0,
+        .multicast_rx_cnt = 0,
+        .brdcast_rx_cnt = 0
+    };
+
+    uint8_t local_mac_addr[ETH_ADDR_LEN] = {};
     TEST_ESP_OK(mac->get_addr(mac, local_mac_addr));
     // test app will parse the DUT MAC from this line of log output
     printf("DUT MAC: %.2x:%.2x:%.2x:%.2x:%.2x:%.2x\n", local_mac_addr[0], local_mac_addr[1], local_mac_addr[2],
            local_mac_addr[3], local_mac_addr[4], local_mac_addr[5]);
 
-    TEST_ESP_OK(esp_eth_update_input_path(eth_handle, l2_packet_txrx_test_cb, eth_event_group));
+    TEST_ESP_OK(esp_eth_update_input_path(eth_handle, l2_packet_txrx_test_cb, &recv_info));
+    TEST_ESP_OK(esp_eth_start(eth_handle)); // start Ethernet driver state machine
 
     EventBits_t bits = 0;
-    bits = xEventGroupWaitBits(eth_event_group, ETH_BROADCAST_RECV_BIT | ETH_MULTICAST_RECV_BIT | ETH_UNICAST_RECV_BIT,
+    bits = xEventGroupWaitBits(eth_event_state_group, ETH_CONNECT_BIT, true, true, pdMS_TO_TICKS(3000));
+    TEST_ASSERT((bits & ETH_CONNECT_BIT) == ETH_CONNECT_BIT);
+    // if DUT is connected in network with switch: even if link is indicated up, it may take some time the switch
+    // starts switching the associated port (e.g. it runs RSTP at first)
+    poke_and_wait(eth_handle, NULL, 0, eth_event_rx_group);
+
+    bits = 0;
+    bits = xEventGroupWaitBits(eth_event_rx_group, ETH_BROADCAST_RECV_BIT | ETH_MULTICAST_RECV_BIT | ETH_UNICAST_RECV_BIT,
                                true, true, pdMS_TO_TICKS(5000));
+    printf("bits = 0x%" PRIu32 "\n", (uint32_t)bits & (ETH_BROADCAST_RECV_BIT | ETH_MULTICAST_RECV_BIT | ETH_UNICAST_RECV_BIT));
     TEST_ASSERT((bits & (ETH_BROADCAST_RECV_BIT | ETH_MULTICAST_RECV_BIT | ETH_UNICAST_RECV_BIT)) ==
                                  (ETH_BROADCAST_RECV_BIT | ETH_MULTICAST_RECV_BIT | ETH_UNICAST_RECV_BIT));
 
@@ -130,45 +226,13 @@ TEST_CASE("ethernet recv_pkt", "[ethernet_l2]")
     phy->del(phy);
     mac->del(mac);
     extra_cleanup();
-    vEventGroupDelete(eth_event_group);
+    vEventGroupDelete(eth_event_state_group);
+    vEventGroupDelete(eth_event_rx_group);
 }
 
-typedef struct
-{
-    SemaphoreHandle_t mutex;
-    int rx_pkt_cnt;
-} recv_info_t;
-
-static esp_err_t eth_recv_cb(esp_eth_handle_t hdl, uint8_t *buffer, uint32_t length, void *priv)
-{
-    emac_frame_t *pkt = (emac_frame_t *)buffer;
-    recv_info_t *recv_info = (recv_info_t *)priv;
-
-    if (pkt->proto == 0x2222) {
-        switch (pkt->data[0])
-        {
-        case POKE_RESP:
-            xSemaphoreGive(recv_info->mutex);
-            break;
-
-        case DUMMY_TRAFFIC:
-            (recv_info->rx_pkt_cnt)++;
-            break;
-        default:
-            break;
-        }
-    }
-    free(buffer);
-    return ESP_OK;
-}
 
 TEST_CASE("ethernet start/stop stress test under heavy traffic", "[ethernet_l2]")
 {
-    recv_info_t recv_info;
-    recv_info.mutex = xSemaphoreCreateBinary();
-    TEST_ASSERT_NOT_NULL(recv_info.mutex);
-    recv_info.rx_pkt_cnt = 0;
-
     esp_eth_mac_t *mac = mac_init(NULL, NULL);
     TEST_ASSERT_NOT_NULL(mac);
     esp_eth_phy_t *phy = phy_init(NULL);
@@ -179,28 +243,32 @@ TEST_CASE("ethernet start/stop stress test under heavy traffic", "[ethernet_l2]"
     TEST_ASSERT_NOT_NULL(eth_handle);
     extra_eth_config(eth_handle);
 
+    TEST_ESP_OK(esp_event_loop_create_default());
+    EventBits_t bits = 0;
+    EventGroupHandle_t eth_event_state_group = xEventGroupCreate();
+    TEST_ASSERT(eth_event_state_group != NULL);
+    TEST_ESP_OK(esp_event_handler_register(ETH_EVENT, ESP_EVENT_ANY_ID, &eth_event_handler, eth_event_state_group));
+    EventGroupHandle_t eth_event_rx_group = xEventGroupCreate();
+    TEST_ASSERT(eth_event_rx_group != NULL);
+    recv_info_t recv_info = {
+        .eth_event_group = eth_event_rx_group,
+        .check_rx_data = false,
+        .unicast_rx_cnt = 0,
+        .multicast_rx_cnt = 0,
+        .brdcast_rx_cnt = 0
+    };
+
+    uint8_t local_mac_addr[ETH_ADDR_LEN] = {};
     TEST_ESP_OK(mac->get_addr(mac, local_mac_addr));
     // test app will parse the DUT MAC from this line of log output
     printf("DUT MAC: %.2x:%.2x:%.2x:%.2x:%.2x:%.2x\n", local_mac_addr[0], local_mac_addr[1], local_mac_addr[2],
            local_mac_addr[3], local_mac_addr[4], local_mac_addr[5]);
 
-    TEST_ESP_OK(esp_eth_update_input_path(eth_handle, eth_recv_cb, &recv_info));
+    TEST_ESP_OK(esp_eth_update_input_path(eth_handle, l2_packet_txrx_test_cb, &recv_info));
-
-    EventBits_t bits = 0;
-    EventGroupHandle_t eth_event_group = xEventGroupCreate();
-    TEST_ASSERT(eth_event_group != NULL);
-    TEST_ESP_OK(esp_event_loop_create_default());
-    TEST_ESP_OK(esp_event_handler_register(ETH_EVENT, ESP_EVENT_ANY_ID, &eth_event_handler, eth_event_group));
-
-    // create a control frame to control test flow between the UT and the Python test script
-    emac_frame_t *ctrl_pkt = calloc(1, 60);
-    ctrl_pkt->proto = 0x2222;
-    memset(ctrl_pkt->dest, 0xff, ETH_ADDR_LEN); // broadcast addr
-    memcpy(ctrl_pkt->src, local_mac_addr, ETH_ADDR_LEN);
 
     // create dummy data packet used for traffic generation
     emac_frame_t *pkt = calloc(1, 1500);
-    pkt->proto = 0x2222;
+    pkt->proto = TEST_ETH_TYPE;
     // we don't care about dest MAC address much, however it is better to not be broadcast or multifcast to not flood
     // other network nodes
     memset(pkt->dest, 0xBA, ETH_ADDR_LEN);
@@ -208,62 +276,51 @@ TEST_CASE("ethernet start/stop stress test under heavy traffic", "[ethernet_l2]"
 
     printf("EMAC start/stop stress test under heavy Tx traffic\n");
     for (int tx_i = 0; tx_i < 10; tx_i++) {
+        printf("Tx Test iteration %d\n", tx_i);
         TEST_ESP_OK(esp_eth_start(eth_handle)); // start Ethernet driver state machine
-        bits = xEventGroupWaitBits(eth_event_group, ETH_CONNECT_BIT, true, true, pdMS_TO_TICKS(3000));
+        bits = xEventGroupWaitBits(eth_event_state_group, ETH_CONNECT_BIT, true, true, pdMS_TO_TICKS(3000));
         TEST_ASSERT((bits & ETH_CONNECT_BIT) == ETH_CONNECT_BIT);
-        // if DUT is connected in network with switch: even if link is indicated up, it may take some time the switch
-        // starts switching the associated port (e.g. it runs RSTP at first)
-        vTaskDelay(pdMS_TO_TICKS(1000));
-
         // at first, check that Tx/Rx path works as expected by poking the test script
         // this also serves as main PASS/FAIL criteria
-        ctrl_pkt->data[0] = POKE_REQ;
+        poke_and_wait(eth_handle, &tx_i, sizeof(tx_i), eth_event_rx_group);
-        ctrl_pkt->data[1] = tx_i;
-        TEST_ESP_OK(esp_eth_transmit(eth_handle, ctrl_pkt, 60));
-        TEST_ASSERT(xSemaphoreTake(recv_info.mutex, pdMS_TO_TICKS(3000)));
-        printf("Tx Test iteration %d\n", tx_i);
 
         // generate heavy Tx traffic
         printf("Note: transmit errors are expected...\n");
         for (int j = 0; j < 150; j++) {
             // return value is not checked on purpose since it is expected that it may fail time to time because
             // we may try to queue more packets than hardware is able to handle
-            pkt->data[0] = j & 0xFF; // sequence number
+            pkt->data[2] = j & 0xFF; // sequence number
             esp_eth_transmit(eth_handle, pkt, 1500);
         }
         TEST_ESP_OK(esp_eth_stop(eth_handle));
-        bits = xEventGroupWaitBits(eth_event_group, ETH_STOP_BIT, true, true, pdMS_TO_TICKS(3000));
+        bits = xEventGroupWaitBits(eth_event_state_group, ETH_STOP_BIT, true, true, pdMS_TO_TICKS(3000));
         TEST_ASSERT((bits & ETH_STOP_BIT) == ETH_STOP_BIT);
         printf("Ethernet stopped\n");
     }
 
     printf("EMAC start/stop stress test under heavy Rx traffic\n");
     for (int rx_i = 0; rx_i < 10; rx_i++) {
-        recv_info.rx_pkt_cnt = 0;
-        TEST_ESP_OK(esp_eth_start(eth_handle)); // start Ethernet driver state machine
-        bits = xEventGroupWaitBits(eth_event_group, ETH_CONNECT_BIT, true, true, pdMS_TO_TICKS(3000));
-        TEST_ASSERT((bits & ETH_CONNECT_BIT) == ETH_CONNECT_BIT);
-        // if DUT is connected in network with switch: even if link is indicated up, it may take some time the switch
-        // starts switching the associated port (e.g. it runs RSTP at first)
-        vTaskDelay(pdMS_TO_TICKS(1000));
-
-        ctrl_pkt->data[0] = POKE_REQ;
-        ctrl_pkt->data[1] = rx_i;
-        TEST_ESP_OK(esp_eth_transmit(eth_handle, ctrl_pkt, 60));
-        TEST_ASSERT(xSemaphoreTake(recv_info.mutex, pdMS_TO_TICKS(3000)));
         printf("Rx Test iteration %d\n", rx_i);
+        TEST_ESP_OK(esp_eth_start(eth_handle)); // start Ethernet driver state machine
+        bits = xEventGroupWaitBits(eth_event_state_group, ETH_CONNECT_BIT, true, true, pdMS_TO_TICKS(3000));
+        TEST_ASSERT((bits & ETH_CONNECT_BIT) == ETH_CONNECT_BIT);
+        poke_and_wait(eth_handle, &rx_i, sizeof(rx_i), eth_event_rx_group);
+
+        // wait for dummy traffic
+        recv_info.unicast_rx_cnt = 0;
+        bits = xEventGroupWaitBits(eth_event_rx_group, ETH_UNICAST_RECV_BIT, true, true, pdMS_TO_TICKS(3000));
+        TEST_ASSERT((bits & ETH_UNICAST_RECV_BIT) == ETH_UNICAST_RECV_BIT);
 
         vTaskDelay(pdMS_TO_TICKS(500));
 
         TEST_ESP_OK(esp_eth_stop(eth_handle));
-        bits = xEventGroupWaitBits(eth_event_group, ETH_STOP_BIT, true, true, pdMS_TO_TICKS(3000));
+        bits = xEventGroupWaitBits(eth_event_state_group, ETH_STOP_BIT, true, true, pdMS_TO_TICKS(3000));
         TEST_ASSERT((bits & ETH_STOP_BIT) == ETH_STOP_BIT);
-        printf("Recv packets: %d\n", recv_info.rx_pkt_cnt);
+        printf("Recv packets: %d\n", recv_info.unicast_rx_cnt);
-        TEST_ASSERT_GREATER_THAN_INT32(0, recv_info.rx_pkt_cnt);
+        TEST_ASSERT_GREATER_THAN_INT32(0, recv_info.unicast_rx_cnt);
         printf("Ethernet stopped\n");
     }
 
-    free(ctrl_pkt);
     free(pkt);
 
     TEST_ESP_OK(esp_event_handler_unregister(ETH_EVENT, ESP_EVENT_ANY_ID, eth_event_handler));
@@ -272,6 +329,6 @@ TEST_CASE("ethernet start/stop stress test under heavy traffic", "[ethernet_l2]"
     phy->del(phy);
     mac->del(mac);
     extra_cleanup();
-    vEventGroupDelete(eth_event_group);
+    vEventGroupDelete(eth_event_rx_group);
-    vSemaphoreDelete(recv_info.mutex);
+    vEventGroupDelete(eth_event_state_group);
 }

components/esp_eth/test_apps/pytest_esp_eth.py

@@ -1,11 +1,10 @@
-# SPDX-FileCopyrightText: 2022 Espressif Systems (Shanghai) CO LTD
+# SPDX-FileCopyrightText: 2022-2023 Espressif Systems (Shanghai) CO LTD
 # SPDX-License-Identifier: CC0-1.0
 
 import contextlib
 import logging
 import os
 import socket
-import time
 from multiprocessing import Pipe, Process, connection
 from typing import Iterator
 
@@ -42,8 +41,10 @@ class EthTestIntf(object):
         logging.info('Use %s for testing', self.target_if)
 
     @contextlib.contextmanager
-    def configure_eth_if(self) -> Iterator[socket.socket]:
+    def configure_eth_if(self, eth_type:int=0) -> Iterator[socket.socket]:
-        so = socket.socket(socket.AF_PACKET, socket.SOCK_RAW, socket.htons(self.eth_type))
+        if eth_type == 0:
+            eth_type = self.eth_type
+        so = socket.socket(socket.AF_PACKET, socket.SOCK_RAW, socket.htons(eth_type))
         so.bind((self.target_if, 0))
         try:
             yield so
@@ -62,15 +63,16 @@ class EthTestIntf(object):
             except Exception as e:
                 raise e
 
-    def recv_resp_poke(self, i: int) -> None:
+    def recv_resp_poke(self, i:int=0) -> None:
-        with self.configure_eth_if() as so:
+        eth_type_ctrl = self.eth_type + 1
-            so.settimeout(10)
+        with self.configure_eth_if(eth_type_ctrl) as so:
+            so.settimeout(30)
             try:
                 eth_frame = Ether(so.recv(60))
-
+                if eth_frame.load[0] == 0xfa:
-                if eth_frame.type == self.eth_type and eth_frame.load[0] == 0xfa:
                     if eth_frame.load[1] != i:
                         raise Exception('Missed Poke Packet')
+                    logging.info('Poke Packet received...')
                     eth_frame.dst = eth_frame.src
                     eth_frame.src = so.getsockname()[4]
                     eth_frame.load = bytes.fromhex('fb')    # POKE_RESP code
@@ -118,6 +120,11 @@ def ethernet_l2_test(dut: Dut) -> None:
     with target_if.configure_eth_if() as so:
         so.settimeout(30)
         dut.write('"ethernet broadcast transmit"')
+
+        # wait for POKE msg to be sure the switch already started forwarding the port's traffic
+        # (there might be slight delay due to the RSTP execution)
+        target_if.recv_resp_poke()
+
         eth_frame = Ether(so.recv(1024))
         for i in range(0, 1010):
             if eth_frame.load[i] != i & 0xff:
@@ -130,7 +137,9 @@ def ethernet_l2_test(dut: Dut) -> None:
         r'([\s\S]*)'
         r'DUT MAC: ([0-9A-Fa-f]{2}:[0-9A-Fa-f]{2}:[0-9A-Fa-f]{2}:[0-9A-Fa-f]{2}:[0-9A-Fa-f]{2}:[0-9A-Fa-f]{2})'
     )
-    time.sleep(1)
+    # wait for POKE msg to be sure the switch already started forwarding the port's traffic
+    # (there might be slight delay due to the RSTP execution)
+    target_if.recv_resp_poke()
     target_if.send_eth_packet('ff:ff:ff:ff:ff:ff')  # broadcast frame
     target_if.send_eth_packet('01:00:00:00:00:00')  # multicast frame
     target_if.send_eth_packet(res.group(2))  # unicast frame
@@ -145,19 +154,20 @@ def ethernet_l2_test(dut: Dut) -> None:
     # Start/stop under heavy Tx traffic
     for tx_i in range(10):
         target_if.recv_resp_poke(tx_i)
+        dut.expect_exact('Ethernet stopped')
 
-    # Start/stop under heavy Rx traffic
+    for rx_i in range(10):
-    pipe_rcv, pipe_send = Pipe(False)
+        target_if.recv_resp_poke(rx_i)
-    tx_proc = Process(target=target_if.traffic_gen, args=(res.group(2), pipe_rcv, ))
+        # Start/stop under heavy Rx traffic
-    tx_proc.start()
+        pipe_rcv, pipe_send = Pipe(False)
-    try:
+        tx_proc = Process(target=target_if.traffic_gen, args=(res.group(2), pipe_rcv, ))
-        for rx_i in range(10):
+        tx_proc.start()
-            target_if.recv_resp_poke(rx_i)
+        dut.expect_exact('Ethernet stopped')
-    finally:
         pipe_send.send(0)  # just send some dummy data
         tx_proc.join(5)
         if tx_proc.exitcode is None:
             tx_proc.terminate()
+
     dut.expect_unity_test_output(extra_before=res.group(1))
 
 

components/hal/emac_hal.c

@@ -23,6 +23,18 @@ typedef struct {
     uint32_t copy_len;
 }__attribute__((packed)) emac_hal_auto_buf_info_t;
 
+static esp_err_t emac_hal_flush_trans_fifo(emac_hal_context_t *hal)
+{
+    emac_ll_flush_trans_fifo_enable(hal->dma_regs, true);
+    /* no other writes to the Operation Mode register until the flush tx fifo bit is cleared */
+    for (uint32_t i = 0; i < 1000; i++) {
+        if (emac_ll_get_flush_trans_fifo(hal->dma_regs) == 0) {
+            return ESP_OK;
+        }
+    }
+    return ESP_ERR_TIMEOUT;
+}
+
 void emac_hal_iomux_init_mii(void)
 {
     /* TX_CLK to GPIO0 */
@@ -288,7 +300,7 @@ void emac_hal_init_dma_default(emac_hal_context_t *hal, emac_hal_dma_config_t *h
     /* Disable Transmit Store Forward */
     emac_ll_trans_store_forward_enable(hal->dma_regs, false);
     /* Flush Transmit FIFO */
-    emac_ll_flush_trans_fifo_enable(hal->dma_regs, true);
+    emac_hal_flush_trans_fifo(hal);
     /* Transmit Threshold Control */
     emac_ll_set_transmit_threshold(hal->dma_regs, EMAC_LL_TRANSMIT_THRESHOLD_CONTROL_64);
     /* Disable Forward Error Frame */
@@ -344,22 +356,21 @@ void emac_hal_start(emac_hal_context_t *hal)
 {
     /* Enable Ethernet MAC and DMA Interrupt */
     emac_ll_enable_corresponding_intr(hal->dma_regs, EMAC_LL_CONFIG_ENABLE_INTR_MASK);
-
+    /* Clear all pending interrupts */
-    /* Flush Transmit FIFO */
+    emac_ll_clear_all_pending_intr(hal->dma_regs);
-    emac_ll_flush_trans_fifo_enable(hal->dma_regs, true);
-
-    /* Start DMA transmission */
-    emac_ll_start_stop_dma_transmit(hal->dma_regs, true);
-    /* Start DMA reception */
-    emac_ll_start_stop_dma_receive(hal->dma_regs, true);
 
     /* Enable transmit state machine of the MAC for transmission on the MII */
     emac_ll_transmit_enable(hal->mac_regs, true);
+    /* Start DMA transmission */
+    /* Note that the EMAC Databook states the DMA could be started prior enabling
+       the MAC transmitter. However, it turned out that such order may cause the MAC
+       transmitter hangs */
+    emac_ll_start_stop_dma_transmit(hal->dma_regs, true);
+
+    /* Start DMA reception */
+    emac_ll_start_stop_dma_receive(hal->dma_regs, true);
     /* Enable receive state machine of the MAC for reception from the MII */
     emac_ll_receive_enable(hal->mac_regs, true);
-
-    /* Clear all pending interrupts */
-    emac_ll_clear_all_pending_intr(hal->dma_regs);
 }
 
 esp_err_t emac_hal_stop(emac_hal_context_t *hal)
@@ -385,6 +396,9 @@ esp_err_t emac_hal_stop(emac_hal_context_t *hal)
     /* Stop DMA reception */
     emac_ll_start_stop_dma_receive(hal->dma_regs, false);
 
+    /* Flush Transmit FIFO */
+    emac_hal_flush_trans_fifo(hal);
+
     /* Disable Ethernet MAC and DMA Interrupt */
     emac_ll_disable_all_intr(hal->dma_regs);
 

components/hal/esp32/include/hal/emac_ll.h

@@ -417,6 +417,11 @@ static inline void emac_ll_flush_trans_fifo_enable(emac_dma_dev_t *dma_regs, boo
     dma_regs->dmaoperation_mode.flush_tx_fifo = enable;
 }
 
+static inline bool emac_ll_get_flush_trans_fifo(emac_dma_dev_t *dma_regs)
+{
+    return dma_regs->dmaoperation_mode.flush_tx_fifo;
+}
+
 static inline void emac_ll_set_transmit_threshold(emac_dma_dev_t *dma_regs, uint32_t threshold)
 {
     dma_regs->dmaoperation_mode.tx_thresh_ctrl = threshold;

examples/ethernet/enc28j60/components/eth_enc28j60/esp_eth_phy_enc28j60.c

@@ -224,12 +224,8 @@ esp_err_t enc28j60_set_duplex(esp_eth_phy_t *phy, eth_duplex_t duplex)
     esp_eth_mediator_t *eth = enc28j60->eth;
     phcon1_reg_t phcon1;
 
-    if (enc28j60->link_status == ETH_LINK_UP) {
+    /* Since the link is going to be reconfigured, consider it down to be status updated once the driver re-started */
-        /* Since the link is going to be reconfigured, consider it down for a while */
+    enc28j60->link_status = ETH_LINK_DOWN;
-        enc28j60->link_status = ETH_LINK_DOWN;
-        /* Indicate to upper stream apps the link is cosidered down */
-        PHY_CHECK(eth->on_state_changed(eth, ETH_STATE_LINK, (void *)enc28j60->link_status), "change link failed", err);
-    }
 
     PHY_CHECK(eth->phy_reg_read(eth, enc28j60->addr, 0, &phcon1.val) == ESP_OK,
               "read PHCON1 failed", err);