Skip to content

Commit

Permalink
Merge branch 'bugfix/spi_eth_recv_alloc_v5.1' into 'release/v5.1'
Browse files Browse the repository at this point in the history 
fix(esp_eth): improved SPI Ethernet _alloc_recv_buf error handling (v5.1)

See merge request espressif/esp-idf!29082
  • Loading branch information
@jack0c
jack0c committed Feb 28, 2024
2 parents c404e95 + 41b67e1 commit ab894a1
Show file tree
Hide file tree
Showing 7 changed files with 369 additions and 107 deletions.
45 changes: 25 additions & 20 deletions components/esp_eth/src/esp_eth_mac_dm9051.c
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2019-2023 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2019-2024 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
Expand Down Expand Up @@ -817,6 +817,7 @@ static void emac_dm9051_task(void *arg)
{
emac_dm9051_t *emac = (emac_dm9051_t *)arg;
uint8_t status = 0;
esp_err_t ret;
while (1) {
// check if the task receives any notification
if (ulTaskNotifyTake(pdTRUE, pdMS_TO_TICKS(1000)) == 0 && // if no notification ...
Expand All @@ -832,31 +833,35 @@ static void emac_dm9051_task(void *arg)
/* define max expected frame len */
uint32_t frame_len = ETH_MAX_PACKET_SIZE;
uint8_t *buffer;
dm9051_alloc_recv_buf(emac, &buffer, &frame_len);
/* we have memory to receive the frame of maximal size previously defined */
if (buffer != NULL) {
uint32_t buf_len = DM9051_ETH_MAC_RX_BUF_SIZE_AUTO;
if (emac->parent.receive(&emac->parent, buffer, &buf_len) == ESP_OK) {
if (buf_len == 0) {
if ((ret = dm9051_alloc_recv_buf(emac, &buffer, &frame_len)) == ESP_OK) {
if (buffer != NULL) {
/* we have memory to receive the frame of maximal size previously defined */
uint32_t buf_len = DM9051_ETH_MAC_RX_BUF_SIZE_AUTO;
if (emac->parent.receive(&emac->parent, buffer, &buf_len) == ESP_OK) {
if (buf_len == 0) {
dm9051_flush_recv_frame(emac);
free(buffer);
} else if (frame_len > buf_len) {
ESP_LOGE(TAG, "received frame was truncated");
free(buffer);
} else {
ESP_LOGD(TAG, "receive len=%u", buf_len);
/* pass the buffer to stack (e.g. TCP/IP layer) */
emac->eth->stack_input(emac->eth, buffer, buf_len);
}
} else {
ESP_LOGE(TAG, "frame read from module failed");
dm9051_flush_recv_frame(emac);
free(buffer);
} else if (frame_len > buf_len) {
ESP_LOGE(TAG, "received frame was truncated");
free(buffer);
} else {
ESP_LOGD(TAG, "receive len=%u", buf_len);
/* pass the buffer to stack (e.g. TCP/IP layer) */
emac->eth->stack_input(emac->eth, buffer, buf_len);
}
} else {
ESP_LOGE(TAG, "frame read from module failed");
dm9051_flush_recv_frame(emac);
free(buffer);
} else if (frame_len) {
ESP_LOGE(TAG, "invalid combination of frame_len(%u) and buffer pointer(%p)", frame_len, buffer);
}
/* if allocation failed and there is a waiting frame */
} else if (frame_len) {
} else if (ret == ESP_ERR_NO_MEM) {
ESP_LOGE(TAG, "no mem for receive buffer");
dm9051_flush_recv_frame(emac);
} else {
ESP_LOGE(TAG, "unexpected error 0x%x", ret);
}
} while (emac->packets_remain);
}
Expand Down
45 changes: 25 additions & 20 deletions components/esp_eth/src/esp_eth_mac_ksz8851snl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
*
* SPDX-License-Identifier: MIT
*
* SPDX-FileContributor: 2021-2023 Espressif Systems (Shanghai) CO LTD
* SPDX-FileContributor: 2021-2024 Espressif Systems (Shanghai) CO LTD
*/

#include <string.h>
Expand Down Expand Up @@ -689,6 +689,7 @@ static esp_err_t emac_ksz8851_set_peer_pause_ability(esp_eth_mac_t *mac, uint32_
static void emac_ksz8851snl_task(void *arg)
{
emac_ksz8851snl_t *emac = (emac_ksz8851snl_t *)arg;
esp_err_t ret;
while (1) {
ulTaskNotifyTake(pdTRUE, portMAX_DELAY);

Expand Down Expand Up @@ -742,31 +743,35 @@ static void emac_ksz8851snl_task(void *arg)
/* define max expected frame len */
uint32_t frame_len = ETH_MAX_PACKET_SIZE;
uint8_t *buffer;
emac_ksz8851_alloc_recv_buf(emac, &buffer, &frame_len);
/* we have memory to receive the frame of maximal size previously defined */
if (buffer != NULL) {
uint32_t buf_len = KSZ8851_ETH_MAC_RX_BUF_SIZE_AUTO;
if (emac->parent.receive(&emac->parent, buffer, &buf_len) == ESP_OK) {
if (buf_len == 0) {
if ((ret = emac_ksz8851_alloc_recv_buf(emac, &buffer, &frame_len)) == ESP_OK) {
if (buffer != NULL) {
/* we have memory to receive the frame of maximal size previously defined */
uint32_t buf_len = KSZ8851_ETH_MAC_RX_BUF_SIZE_AUTO;
if (emac->parent.receive(&emac->parent, buffer, &buf_len) == ESP_OK) {
if (buf_len == 0) {
emac_ksz8851_flush_recv_queue(emac);
free(buffer);
} else if (frame_len > buf_len) {
ESP_LOGE(TAG, "received frame was truncated");
free(buffer);
} else {
ESP_LOGD(TAG, "receive len=%u", buf_len);
/* pass the buffer to stack (e.g. TCP/IP layer) */
emac->eth->stack_input(emac->eth, buffer, buf_len);
}
} else {
ESP_LOGE(TAG, "frame read from module failed");
emac_ksz8851_flush_recv_queue(emac);
free(buffer);
} else if (frame_len > buf_len) {
ESP_LOGE(TAG, "received frame was truncated");
free(buffer);
} else {
ESP_LOGD(TAG, "receive len=%u", buf_len);
/* pass the buffer to stack (e.g. TCP/IP layer) */
emac->eth->stack_input(emac->eth, buffer, buf_len);
}
} else {
ESP_LOGE(TAG, "frame read from module failed");
emac_ksz8851_flush_recv_queue(emac);
free(buffer);
} else if (frame_len) {
ESP_LOGE(TAG, "invalid combination of frame_len(%u) and buffer pointer(%p)", frame_len, buffer);
}
/* if allocation failed and there is a waiting frame */
} else if (frame_len) {
} else if (ret == ESP_ERR_NO_MEM) {
ESP_LOGE(TAG, "no mem for receive buffer");
emac_ksz8851_flush_recv_queue(emac);
} else {
ESP_LOGE(TAG, "unexpected error 0x%x", ret);
}
}
ksz8851_write_reg(emac, KSZ8851_IER, ier);
Expand Down
50 changes: 28 additions & 22 deletions components/esp_eth/src/esp_eth_mac_w5500.c
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2020-2023 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2020-2024 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
Expand Down Expand Up @@ -677,7 +677,7 @@ static esp_err_t emac_w5500_receive(esp_eth_mac_t *mac, uint8_t *buf, uint32_t *
remain_bytes -= rx_len + 2;
emac->packets_remain = remain_bytes > 0;

*length = rx_len;
*length = copy_len;
return ret;
err:
*length = 0;
Expand All @@ -700,12 +700,13 @@ static esp_err_t emac_w5500_flush_recv_frame(emac_w5500_t *emac)
// read head first
ESP_GOTO_ON_ERROR(w5500_read_buffer(emac, &rx_len, sizeof(rx_len), offset), err, TAG, "read frame header failed");
// update read pointer
offset = rx_len;
rx_len = __builtin_bswap16(rx_len);
offset += rx_len;
offset = __builtin_bswap16(offset);
ESP_GOTO_ON_ERROR(w5500_write(emac, W5500_REG_SOCK_RX_RD(0), &offset, sizeof(offset)), err, TAG, "write RX RD failed");
/* issue RECV command */
ESP_GOTO_ON_ERROR(w5500_send_command(emac, W5500_SCR_RECV, 100), err, TAG, "issue RECV command failed");
// check if there're more data need to process
rx_len = __builtin_bswap16(rx_len);
remain_bytes -= rx_len;
emac->packets_remain = remain_bytes > 0;
}
Expand All @@ -731,6 +732,7 @@ static void emac_w5500_task(void *arg)
uint8_t *buffer = NULL;
uint32_t frame_len = 0;
uint32_t buf_len = 0;
esp_err_t ret;
while (1) {
/* check if the task receives any notification */
if (ulTaskNotifyTake(pdTRUE, pdMS_TO_TICKS(1000)) == 0 && // if no notification ...
Expand All @@ -747,29 +749,33 @@ static void emac_w5500_task(void *arg)
do {
/* define max expected frame len */
frame_len = ETH_MAX_PACKET_SIZE;
emac_w5500_alloc_recv_buf(emac, &buffer, &frame_len);
/* we have memory to receive the frame of maximal size previously defined */
if (buffer != NULL) {
buf_len = W5500_ETH_MAC_RX_BUF_SIZE_AUTO;
if (emac->parent.receive(&emac->parent, buffer, &buf_len) == ESP_OK) {
if (buf_len == 0) {
free(buffer);
} else if (frame_len > buf_len) {
ESP_LOGE(TAG, "received frame was truncated");
free(buffer);
if ((ret = emac_w5500_alloc_recv_buf(emac, &buffer, &frame_len)) == ESP_OK) {
if (buffer != NULL) {
/* we have memory to receive the frame of maximal size previously defined */
buf_len = W5500_ETH_MAC_RX_BUF_SIZE_AUTO;
if (emac->parent.receive(&emac->parent, buffer, &buf_len) == ESP_OK) {
if (buf_len == 0) {
free(buffer);
} else if (frame_len > buf_len) {
ESP_LOGE(TAG, "received frame was truncated");
free(buffer);
} else {
ESP_LOGD(TAG, "receive len=%u", buf_len);
/* pass the buffer to stack (e.g. TCP/IP layer) */
emac->eth->stack_input(emac->eth, buffer, buf_len);
}
} else {
ESP_LOGD(TAG, "receive len=%u", buf_len);
/* pass the buffer to stack (e.g. TCP/IP layer) */
emac->eth->stack_input(emac->eth, buffer, buf_len);
ESP_LOGE(TAG, "frame read from module failed");
free(buffer);
}
} else {
ESP_LOGE(TAG, "frame read from module failed");
free(buffer);
} else if (frame_len) {
ESP_LOGE(TAG, "invalid combination of frame_len(%u) and buffer pointer(%p)", frame_len, buffer);
}
/* if allocation failed and there is a waiting frame */
} else if (frame_len) {
} else if (ret == ESP_ERR_NO_MEM) {
ESP_LOGE(TAG, "no mem for receive buffer");
emac_w5500_flush_recv_frame(emac);
} else {
ESP_LOGE(TAG, "unexpected error 0x%x", ret);
}
} while (emac->packets_remain);
}
Expand Down
14 changes: 13 additions & 1 deletion components/esp_eth/test_apps/main/esp_eth_test_common.c
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2022-2023 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2022-2024 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Unlicense OR CC0-1.0
*/
Expand Down Expand Up @@ -96,23 +96,31 @@ esp_eth_phy_t *phy_init(eth_phy_config_t *phy_config)
phy_config->phy_addr = ESP_ETH_PHY_ADDR_AUTO;
#if CONFIG_TARGET_ETH_PHY_DEVICE_IP101
phy = esp_eth_phy_new_ip101(phy_config);
ESP_LOGI(TAG, "DUT PHY: IP101");
#elif CONFIG_TARGET_ETH_PHY_DEVICE_LAN8720
phy = esp_eth_phy_new_lan87xx(phy_config);
ESP_LOGI(TAG, "DUT PHY: LAN8720");
#elif CONFIG_TARGET_ETH_PHY_DEVICE_KSZ8041
phy = esp_eth_phy_new_ksz80xx(phy_config);
ESP_LOGI(TAG, "DUT PHY: KSZ8041");
#elif CONFIG_TARGET_ETH_PHY_DEVICE_RTL8201
phy = esp_eth_phy_new_rtl8201(phy_config);
ESP_LOGI(TAG, "DUT PHY: RTL8201");
#elif CONFIG_TARGET_ETH_PHY_DEVICE_DP83848
phy = esp_eth_phy_new_dp83848(phy_config);
ESP_LOGI(TAG, "DUT PHY: DP83848");
#endif // CONFIG_TARGET_ETH_PHY_DEVICE_IP101
#elif CONFIG_TARGET_USE_SPI_ETHERNET
phy_config->reset_gpio_num = -1;
#if CONFIG_TARGET_ETH_PHY_DEVICE_W5500
phy = esp_eth_phy_new_w5500(phy_config);
ESP_LOGI(TAG, "DUT PHY: W5500");
#elif CONFIG_TARGET_ETH_PHY_DEVICE_KSZ8851SNL
phy = esp_eth_phy_new_ksz8851snl(phy_config);
ESP_LOGI(TAG, "DUT PHY: KSZ8851SNL");
#elif CONFIG_TARGET_ETH_PHY_DEVICE_DM9051
phy = esp_eth_phy_new_dm9051(phy_config);
ESP_LOGI(TAG, "DUT PHY: DM9051");
#endif // CONFIG_TARGET_ETH_PHY_DEVICE_W5500
#endif // CONFIG_TARGET_USE_INTERNAL_ETHERNET
return phy;
Expand Down Expand Up @@ -145,14 +153,18 @@ void eth_event_handler(void *arg, esp_event_base_t event_base,
EventGroupHandle_t eth_event_group = (EventGroupHandle_t)arg;
switch (event_id) {
case ETHERNET_EVENT_CONNECTED:
ESP_LOGI(TAG, "Ethernet Link Up");
xEventGroupSetBits(eth_event_group, ETH_CONNECT_BIT);
break;
case ETHERNET_EVENT_DISCONNECTED:
ESP_LOGI(TAG, "Ethernet Link Down");
break;
case ETHERNET_EVENT_START:
ESP_LOGI(TAG, "Ethernet Started");
xEventGroupSetBits(eth_event_group, ETH_START_BIT);
break;
case ETHERNET_EVENT_STOP:
ESP_LOGI(TAG, "Ethernet Stopped");
xEventGroupSetBits(eth_event_group, ETH_STOP_BIT);
break;
default:
Expand Down
34 changes: 31 additions & 3 deletions components/esp_eth/test_apps/main/esp_eth_test_hal.c
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2022 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2022-2024 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Unlicense OR CC0-1.0
*/
Expand All @@ -14,6 +14,10 @@
#define ETHERTYPE_TX_MULTI_2 0x2223 // frame transmitted via emac_hal_transmit_multiple_buf_frame (2 buffers)
#define ETHERTYPE_TX_MULTI_3 0x2224 // frame transmitted via emac_hal_transmit_multiple_buf_frame (3 buffers)

#define MINIMUM_TEST_FRAME_SIZE 64

#define MAX(a, b) ((a) > (b) ? (a) : (b))

static const char *TAG = "esp32_eth_test_hal";

typedef struct
Expand Down Expand Up @@ -123,8 +127,32 @@ TEST_CASE("hal receive/transmit", "[emac_hal]")
test_pkt->data[i] = i & 0xFF;
}

// verify that HAL driver correctly processes frame from EMAC descriptors
uint16_t transmit_size = CONFIG_ETH_DMA_BUFFER_SIZE;
uint16_t transmit_size;

ESP_LOGI(TAG, "Verify DMA descriptors are returned back to owner");
// find if Rx or Tx buffer number is bigger and work with bigger number
uint32_t config_eth_dma_max_buffer_num = MAX(CONFIG_ETH_DMA_RX_BUFFER_NUM, CONFIG_ETH_DMA_TX_BUFFER_NUM);
// start with short frames since EMAC Rx FIFO may be different of size for different chips => it may help with following fail isolation
for (int32_t i = 0; i < config_eth_dma_max_buffer_num*2; i++) {
transmit_size = MINIMUM_TEST_FRAME_SIZE;
ESP_LOGI(TAG, "transmit frame size: %" PRIu16 ", i = %" PRIi32, transmit_size, i);
recv_info.expected_size = transmit_size;
TEST_ESP_OK(esp_eth_transmit(eth_handle, test_pkt, transmit_size));
TEST_ASSERT(xSemaphoreTake(recv_info.mutex, pdMS_TO_TICKS(500)));
}

ESP_LOGI(TAG, "Verify that we are able to transmit/receive all frame sizes");
// iteration over different sizes may help with fail isolation
for (int i = 1; (MINIMUM_TEST_FRAME_SIZE *i) < ETH_MAX_PAYLOAD_LEN; i++) {
transmit_size = MINIMUM_TEST_FRAME_SIZE * i;
ESP_LOGI(TAG, "transmit frame size: %" PRIu16, transmit_size);
recv_info.expected_size = transmit_size;
TEST_ESP_OK(esp_eth_transmit(eth_handle, test_pkt, transmit_size));
TEST_ASSERT(xSemaphoreTake(recv_info.mutex, pdMS_TO_TICKS(500)));
}

ESP_LOGI(TAG, "Verify that DMA driver correctly processes frame from EMAC descriptors at boundary conditions");
transmit_size = CONFIG_ETH_DMA_BUFFER_SIZE;
ESP_LOGI(TAG, "transmit frame size: %" PRIu16, transmit_size);
recv_info.expected_size = transmit_size;
TEST_ESP_OK(esp_eth_transmit(eth_handle, test_pkt, transmit_size));
Expand Down
Loading

0 comments on commit ab894a1

Please sign in to comment.