ethernet: optimise tx and rx

components/esp_eth/src/esp_eth.c

@@ -299,6 +299,8 @@ esp_err_t esp_eth_transmit(esp_eth_handle_t hdl, void *buf, uint32_t length)
 {
     esp_err_t ret = ESP_OK;
     esp_eth_driver_t *eth_driver = (esp_eth_driver_t *)hdl;
+    ETH_CHECK(buf, "can't set buf to null", err, ESP_ERR_INVALID_ARG);
+    ETH_CHECK(length, "buf length can't be zero", err, ESP_ERR_INVALID_ARG);
     ETH_CHECK(eth_driver, "ethernet driver handle can't be null", err, ESP_ERR_INVALID_ARG);
     esp_eth_mac_t *mac = eth_driver->mac;
     return mac->transmit(mac, buf, length);
@@ -310,6 +312,8 @@ esp_err_t esp_eth_receive(esp_eth_handle_t hdl, uint8_t *buf, uint32_t *length)
 {
     esp_err_t ret = ESP_OK;
     esp_eth_driver_t *eth_driver = (esp_eth_driver_t *)hdl;
+    ETH_CHECK(buf && length, "can't set buf and length to null", err, ESP_ERR_INVALID_ARG);
+    ETH_CHECK(*length > 60, "length can't be less than 60", err, ESP_ERR_INVALID_ARG);
     ETH_CHECK(eth_driver, "ethernet driver handle can't be null", err, ESP_ERR_INVALID_ARG);
     esp_eth_mac_t *mac = eth_driver->mac;
     return mac->receive(mac, buf, length);

components/esp_eth/src/esp_eth_mac_dm9051.c

@@ -397,8 +397,10 @@ static void emac_dm9051_task(void *arg)
         if (status & ISR_PR) {
             do {
                 length = ETH_MAX_PACKET_SIZE;
-                buffer = (uint8_t *)heap_caps_malloc(length, MALLOC_CAP_DMA);
-                if (emac->parent.receive(&emac->parent, buffer, &length) == ESP_OK) {
+                buffer = heap_caps_malloc(length, MALLOC_CAP_DMA);
+                if (!buffer) {
+                    ESP_LOGE(TAG, "no mem for receive buffer");
+                } else if (emac->parent.receive(&emac->parent, buffer, &length) == ESP_OK) {
                     /* pass the buffer to stack (e.g. TCP/IP layer) */
                     if (length) {
                         emac->eth->stack_input(emac->eth, buffer, length);
@@ -597,8 +599,6 @@ static esp_err_t emac_dm9051_transmit(esp_eth_mac_t *mac, uint8_t *buf, uint32_t
 {
     esp_err_t ret = ESP_OK;
     emac_dm9051_t *emac = __containerof(mac, emac_dm9051_t, parent);
-    MAC_CHECK(buf, "can't set buf to null", err, ESP_ERR_INVALID_ARG);
-    MAC_CHECK(length, "buf length can't be zero", err, ESP_ERR_INVALID_ARG);
     /* Check if last transmit complete */
     uint8_t tcr = 0;
     MAC_CHECK(dm9051_register_read(emac, DM9051_TCR, &tcr) == ESP_OK, "read TCR failed", err, ESP_FAIL);
@@ -620,7 +620,6 @@ static esp_err_t emac_dm9051_receive(esp_eth_mac_t *mac, uint8_t *buf, uint32_t
 {
     esp_err_t ret = ESP_OK;
     emac_dm9051_t *emac = __containerof(mac, emac_dm9051_t, parent);
-    MAC_CHECK(buf && length, "can't set buf and length to null", err, ESP_ERR_INVALID_ARG);
     uint8_t rxbyte = 0;
     uint16_t rx_len = 0;
     __attribute__((aligned(4))) dm9051_rx_header_t header; // SPI driver needs the rx buffer 4 byte align

components/esp_eth/src/esp_eth_mac_esp32.c

@@ -214,12 +214,8 @@ static esp_err_t emac_esp32_transmit(esp_eth_mac_t *mac, uint8_t *buf, uint32_t
 {
     esp_err_t ret = ESP_OK;
     emac_esp32_t *emac = __containerof(mac, emac_esp32_t, parent);
-    MAC_CHECK(buf, "can't set buf to null", err, ESP_ERR_INVALID_ARG);
-    MAC_CHECK(length, "buf length can't be zero", err, ESP_ERR_INVALID_ARG);
-    /* Check if the descriptor is owned by the Ethernet DMA (when 1) or CPU (when 0) */
-    MAC_CHECK(emac_hal_get_tx_desc_owner(&emac->hal) == EMAC_DMADESC_OWNER_CPU,
-              "CPU doesn't own the Tx Descriptor", err, ESP_ERR_INVALID_STATE);
-    emac_hal_transmit_frame(&emac->hal, buf, length);
+    uint32_t sent_len = emac_hal_transmit_frame(&emac->hal, buf, length);
+    MAC_CHECK(sent_len == length, "insufficient TX buffer size", err, ESP_ERR_INVALID_SIZE);
     return ESP_OK;
 err:
     return ret;
@@ -228,20 +224,17 @@ err:
 static esp_err_t emac_esp32_receive(esp_eth_mac_t *mac, uint8_t *buf, uint32_t *length)
 {
     esp_err_t ret = ESP_OK;
+    uint32_t expected_len = *length;
     emac_esp32_t *emac = __containerof(mac, emac_esp32_t, parent);
     MAC_CHECK(buf && length, "can't set buf and length to null", err, ESP_ERR_INVALID_ARG);
-    uint32_t receive_len = emac_hal_receive_frame(&emac->hal, buf, *length, &emac->frames_remain);
+    uint32_t receive_len = emac_hal_receive_frame(&emac->hal, buf, expected_len, &emac->frames_remain);
     /* we need to check the return value in case the buffer size is not enough */
-    if (*length < receive_len) {
-        ESP_LOGE(TAG, "buffer size too small");
-        /* tell upper layer the size we need */
-        *length = receive_len;
-        ret = ESP_ERR_INVALID_SIZE;
-        goto err;
-    }
+    ESP_LOGD(TAG, "receive len= %d", receive_len);
+    MAC_CHECK(expected_len >= receive_len, "received buffer longer than expected", err, ESP_ERR_INVALID_SIZE);
     *length = receive_len;
     return ESP_OK;
 err:
+    *length = expected_len;
     return ret;
 }
 
@@ -255,8 +248,10 @@ static void emac_esp32_rx_task(void *arg)
         ulTaskNotifyTake(pdFALSE, portMAX_DELAY);
         do {
             length = ETH_MAX_PACKET_SIZE;
-            buffer = (uint8_t *)malloc(length);
-            if (emac_esp32_receive(&emac->parent, buffer, &length) == ESP_OK) {
+            buffer = malloc(length);
+            if (!buffer) {
+                ESP_LOGE(TAG, "no mem for receive buffer");
+            } else if (emac_esp32_receive(&emac->parent, buffer, &length) == ESP_OK) {
                 /* pass the buffer to stack (e.g. TCP/IP layer) */
                 if (length) {
                     emac->eth->stack_input(emac->eth, buffer, length);
@@ -291,9 +286,9 @@ static esp_err_t emac_esp32_init(esp_eth_mac_t *mac)
     esp_eth_mediator_t *eth = emac->eth;
     /* enable peripheral clock */
     periph_module_enable(PERIPH_EMAC_MODULE);
-    /* enable clock, config gpio, etc */
+    /* init clock, config gpio, etc */
     emac_hal_lowlevel_init(&emac->hal);
-    /* init gpio used by gpio */
+    /* init gpio used by smi interface */
     emac_esp32_init_smi_gpio(emac);
     MAC_CHECK(eth->on_state_changed(eth, ETH_STATE_LLINIT, NULL) == ESP_OK, "lowlevel init failed", err, ESP_FAIL);
     /* software reset */

components/esp_eth/src/esp_eth_mac_openeth.c

@@ -66,7 +66,7 @@ static esp_err_t emac_opencores_receive(esp_eth_mac_t *mac, uint8_t *buf, uint32
 
 static IRAM_ATTR void emac_opencores_isr_handler(void *args)
 {
-    emac_opencores_t *emac = (emac_opencores_t*) args;
+    emac_opencores_t *emac = (emac_opencores_t *) args;
     BaseType_t high_task_wakeup;
 
     uint32_t status = REG_READ(OPENETH_INT_SOURCE_REG);
@@ -94,10 +94,12 @@ static void emac_opencores_rx_task(void *arg)
     uint32_t length = 0;
     while (1) {
         if (ulTaskNotifyTake(pdFALSE, portMAX_DELAY)) {
-            while(true) {
-                buffer = (uint8_t *)malloc(ETH_MAX_PACKET_SIZE);
+            while (true) {
                 length = ETH_MAX_PACKET_SIZE;
-                if (emac_opencores_receive(&emac->parent, buffer, &length) == ESP_OK) {
+                buffer = malloc(length);
+                if (!buffer) {
+                    ESP_LOGE(TAG, "no mem for receive buffer");
+                } else if (emac_opencores_receive(&emac->parent, buffer, &length) == ESP_OK) {
                     // pass the buffer to the upper layer
                     if (length) {
                         emac->eth->stack_input(emac->eth, buffer, length);
@@ -232,8 +234,6 @@ static esp_err_t emac_opencores_transmit(esp_eth_mac_t *mac, uint8_t *buf, uint3
 {
     esp_err_t ret = ESP_OK;
     emac_opencores_t *emac = __containerof(mac, emac_opencores_t, parent);
-    MAC_CHECK(buf, "can't set buf to null", err, ESP_ERR_INVALID_ARG);
-    MAC_CHECK(length, "buf length can't be zero", err, ESP_ERR_INVALID_ARG);
     MAC_CHECK(length < DMA_BUF_SIZE * TX_BUF_COUNT, "insufficient TX buffer size", err, ESP_ERR_INVALID_SIZE);
 
     uint32_t bytes_remaining = length;
@@ -243,7 +243,7 @@ static esp_err_t emac_opencores_transmit(esp_eth_mac_t *mac, uint8_t *buf, uint3
     while (bytes_remaining > 0) {
         uint32_t will_write = MIN(bytes_remaining, DMA_BUF_SIZE);
         memcpy(emac->tx_buf[emac->cur_tx_desc], buf, will_write);
-        openeth_tx_desc_t* desc_ptr = openeth_tx_desc(emac->cur_tx_desc);
+        openeth_tx_desc_t *desc_ptr = openeth_tx_desc(emac->cur_tx_desc);
         openeth_tx_desc_t desc_val = *desc_ptr;
         desc_val.wr = (emac->cur_tx_desc == TX_BUF_COUNT - 1);
         desc_val.len = will_write;
@@ -265,7 +265,6 @@ static esp_err_t emac_opencores_receive(esp_eth_mac_t *mac, uint8_t *buf, uint32
 {
     esp_err_t ret = ESP_OK;
     emac_opencores_t *emac = __containerof(mac, emac_opencores_t, parent);
-    MAC_CHECK(buf && length, "can't set buf and length to null", err, ESP_ERR_INVALID_ARG);
 
     openeth_rx_desc_t *desc_ptr = openeth_rx_desc(emac->cur_rx_desc);
     openeth_rx_desc_t desc_val = *desc_ptr;
@@ -294,7 +293,7 @@ static esp_err_t emac_opencores_init(esp_eth_mac_t *mac)
     esp_eth_mediator_t *eth = emac->eth;
     MAC_CHECK(eth->on_state_changed(eth, ETH_STATE_LLINIT, NULL) == ESP_OK, "lowlevel init failed", err, ESP_FAIL);
     MAC_CHECK(esp_read_mac(emac->addr, ESP_MAC_ETH) == ESP_OK, "fetch ethernet mac address failed", err, ESP_FAIL);
-    
+
     // Sanity check
     if (REG_READ(OPENETH_MODER_REG) != OPENETH_MODER_DEFAULT) {
         ESP_LOGE(TAG, "CONFIG_ETH_USE_OPENETH should only be used when running in QEMU.");
@@ -378,7 +377,7 @@ esp_eth_mac_t *esp_eth_mac_new_openeth(const eth_mac_config_t *config)
     emac->parent.set_promiscuous = emac_opencores_set_promiscuous;
     emac->parent.transmit = emac_opencores_transmit;
     emac->parent.receive = emac_opencores_receive;
-    
+
     // Initialize the interrupt
     MAC_CHECK(esp_intr_alloc(OPENETH_INTR_SOURCE, ESP_INTR_FLAG_IRAM, emac_opencores_isr_handler,
                              emac, &(emac->intr_hdl)) == ESP_OK,

components/soc/esp32/emac_hal.c

@@ -438,11 +438,12 @@ uint32_t emac_hal_get_tx_desc_owner(emac_hal_context_t *hal)
     return hal->tx_desc->TDES0.Own;
 }
 
-void emac_hal_transmit_frame(emac_hal_context_t *hal, uint8_t *buf, uint32_t length)
+uint32_t emac_hal_transmit_frame(emac_hal_context_t *hal, uint8_t *buf, uint32_t length)
 {
     /* Get the number of Tx buffers to use for the frame */
     uint32_t bufcount = 0;
     uint32_t lastlen = length;
+    uint32_t sentout = 0;
     while (lastlen > CONFIG_ETH_DMA_BUFFER_SIZE) {
         lastlen -= CONFIG_ETH_DMA_BUFFER_SIZE;
         bufcount++;
@@ -452,6 +453,10 @@ void emac_hal_transmit_frame(emac_hal_context_t *hal, uint8_t *buf, uint32_t len
     }
     /* A frame is transmitted in multiple descriptor */
     for (uint32_t i = 0; i < bufcount; i++) {
+        /* Check if the descriptor is owned by the Ethernet DMA (when 1) or CPU (when 0) */
+        if (hal->tx_desc->TDES0.Own != EMAC_DMADESC_OWNER_CPU) {
+            goto err;
+        }
         /* Clear FIRST and LAST segment bits */
         hal->tx_desc->TDES0.FirstSegment = 0;
         hal->tx_desc->TDES0.LastSegment = 0;
@@ -468,18 +473,22 @@ void emac_hal_transmit_frame(emac_hal_context_t *hal, uint8_t *buf, uint32_t len
             hal->tx_desc->TDES1.TransmitBuffer1Size = lastlen;
             /* copy data from uplayer stack buffer */
             memcpy((void *)(hal->tx_desc->Buffer1Addr), buf + i * CONFIG_ETH_DMA_BUFFER_SIZE, lastlen);
+            sentout += lastlen;
         } else {
             /* Program size */
             hal->tx_desc->TDES1.TransmitBuffer1Size = CONFIG_ETH_DMA_BUFFER_SIZE;
             /* copy data from uplayer stack buffer */
             memcpy((void *)(hal->tx_desc->Buffer1Addr), buf + i * CONFIG_ETH_DMA_BUFFER_SIZE, CONFIG_ETH_DMA_BUFFER_SIZE);
+            sentout += CONFIG_ETH_DMA_BUFFER_SIZE;
         }
         /* Set Own bit of the Tx descriptor Status: gives the buffer back to ETHERNET DMA */
         hal->tx_desc->TDES0.Own = EMAC_DMADESC_OWNER_DMA;
         /* Point to next descriptor */
         hal->tx_desc = (eth_dma_tx_descriptor_t *)(hal->tx_desc->Buffer2NextDescAddr);
     }
+err:
     hal->dma_regs->dmatxpolldemand = 0;
+    return sentout;
 }
 
 uint32_t emac_hal_receive_frame(emac_hal_context_t *hal, uint8_t *buf, uint32_t size, uint32_t *frames_remain)
@@ -488,7 +497,9 @@ uint32_t emac_hal_receive_frame(emac_hal_context_t *hal, uint8_t *buf, uint32_t
     eth_dma_rx_descriptor_t *first_desc = NULL;
     uint32_t iter = 0;
     uint32_t seg_count = 0;
-    uint32_t len = 0;
+    uint32_t ret_len = 0;
+    uint32_t copy_len = 0;
+    uint32_t write_len = 0;
     uint32_t frame_count = 0;
 
     first_desc = hal->rx_desc;
@@ -500,13 +511,9 @@ uint32_t emac_hal_receive_frame(emac_hal_context_t *hal, uint8_t *buf, uint32_t
         /* Last segment in frame */
         if (desc_iter->RDES0.LastDescriptor) {
             /* Get the Frame Length of the received packet: substruct 4 bytes of the CRC */
-            len = desc_iter->RDES0.FrameLength - ETH_CRC_LENGTH;
-            /* check if the buffer can store the whole frame */
-            if (len > size) {
-                /* return the real size that we want */
-                /* user need to compare the return value to the size they prepared when this function returned */
-                return len;
-            }
+            ret_len = desc_iter->RDES0.FrameLength - ETH_CRC_LENGTH;
+            /* packets larger than expected will be truncated */
+            copy_len = ret_len > size ? size : ret_len;
             /* update unhandled frame count */
             frame_count++;
         }
@@ -530,15 +537,16 @@ uint32_t emac_hal_receive_frame(emac_hal_context_t *hal, uint8_t *buf, uint32_t
         }
         desc_iter = first_desc;
         for (iter = 0; iter < seg_count - 1; iter++) {
+            write_len = copy_len < CONFIG_ETH_DMA_BUFFER_SIZE ? copy_len : CONFIG_ETH_DMA_BUFFER_SIZE;
             /* copy data to buffer */
-            memcpy(buf + iter * CONFIG_ETH_DMA_BUFFER_SIZE,
-                   (void *)(desc_iter->Buffer1Addr), CONFIG_ETH_DMA_BUFFER_SIZE);
+            memcpy(buf, (void *)(desc_iter->Buffer1Addr), write_len);
+            buf += write_len;
+            copy_len -= write_len;
             /* Set Own bit in Rx descriptors: gives the buffers back to DMA */
             desc_iter->RDES0.Own = EMAC_DMADESC_OWNER_DMA;
             desc_iter = (eth_dma_rx_descriptor_t *)(desc_iter->Buffer2NextDescAddr);
         }
-        memcpy(buf + iter * CONFIG_ETH_DMA_BUFFER_SIZE,
-               (void *)(desc_iter->Buffer1Addr), len % CONFIG_ETH_DMA_BUFFER_SIZE);
+        memcpy(buf, (void *)(desc_iter->Buffer1Addr), copy_len);
         desc_iter->RDES0.Own = EMAC_DMADESC_OWNER_DMA;
         /* update rxdesc */
         hal->rx_desc = (eth_dma_rx_descriptor_t *)(desc_iter->Buffer2NextDescAddr);
@@ -547,7 +555,7 @@ uint32_t emac_hal_receive_frame(emac_hal_context_t *hal, uint8_t *buf, uint32_t
         frame_count--;
     }
     *frames_remain = frame_count;
-    return len;
+    return ret_len;
 }
 
 IRAM_ATTR void emac_hal_isr(void *arg)

components/soc/esp32/include/hal/emac.h

@@ -242,6 +242,8 @@ typedef struct {
 #define EMAC_DMATXDESC_CHECKSUM_TCPUDPICMPSEGMENT 2 /*!< TCP/UDP/ICMP Checksum Insertion calculated over segment only */
 #define EMAC_DMATXDESC_CHECKSUM_TCPUDPICMPFULL 3    /*!< TCP/UDP/ICMP Checksum Insertion fully calculated */
 
+_Static_assert(sizeof(eth_dma_tx_descriptor_t) == 32, "eth_dma_tx_descriptor_t should occupy 32 bytes in memory");
+
 /**
 * @brief Ethernet DMA RX Descriptor
 *
@@ -328,6 +330,8 @@ typedef struct {
 #define EMAC_DMADESC_OWNER_CPU  (0)
 #define EMAC_DMADESC_OWNER_DMA  (1)
 
+_Static_assert(sizeof(eth_dma_rx_descriptor_t) == 32, "eth_dma_rx_descriptor_t should occupy 32 bytes in memory");
+
 typedef struct {
     emac_mac_dev_t *mac_regs;
     emac_dma_dev_t *dma_regs;
@@ -378,7 +382,7 @@ void emac_hal_stop(emac_hal_context_t *hal);
 
 uint32_t emac_hal_get_tx_desc_owner(emac_hal_context_t *hal);
 
-void emac_hal_transmit_frame(emac_hal_context_t *hal, uint8_t *buf, uint32_t length);
+uint32_t emac_hal_transmit_frame(emac_hal_context_t *hal, uint8_t *buf, uint32_t length);
 
 uint32_t emac_hal_receive_frame(emac_hal_context_t *hal, uint8_t *buf, uint32_t size, uint32_t *frames_remain);