From 95196fb7e40b12c7824e9827aadcc2ace35016a5 Mon Sep 17 00:00:00 2001 From: Erik Ekman Date: Wed, 24 Feb 2021 01:05:22 +0100 Subject: [PATCH] esp32/eth: Don't overwrite queued event with RX packet If there is an event to be handled by _esp_eth_isr(), don't overwrite it if a new packet has been received. In my testing, all SYSTEM_EVENT_ETH_CONNECTED events except the first are immediately followed by at least one SYSTEM_EVENT_ETH_RX_DONE event. This causes the SYSTEM_EVENT_ETH_CONNECTED to not get handled, and the IP stack will not be notified of the new link state. Protect the other events by dropping the packet instead. If an earlier unhandled SYSTEM_EVENT_ETH_RX_DONE event exists, overwrite it with the newer packet. I only saw this happen with lwIP and not with GNRC - I am not sure why. But it still is a race waiting to happen. The nice long term solution is probably to have a queue of unhandled events, allowing them all to be processed once there is time. --- cpu/esp32/esp-eth/esp_eth_netdev.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/cpu/esp32/esp-eth/esp_eth_netdev.c b/cpu/esp32/esp-eth/esp_eth_netdev.c index 507da80c95..f1dd3c52d7 100644 --- a/cpu/esp32/esp-eth/esp_eth_netdev.c +++ b/cpu/esp32/esp-eth/esp_eth_netdev.c @@ -100,10 +100,15 @@ static esp_err_t IRAM_ATTR _eth_input_callback(void *buffer, uint16_t len, void mutex_lock(&_esp_eth_dev.dev_lock); - memcpy(_esp_eth_dev.rx_buf, buffer, len); - _esp_eth_dev.rx_len = len; - _esp_eth_dev.event = SYSTEM_EVENT_ETH_RX_DONE; - netdev_trigger_event_isr(&_esp_eth_dev.netdev); + /* Don't overwrite other events, drop packet instead. + * Keep the latest received packet if previous has not been read. */ + if (_esp_eth_dev.event == SYSTEM_EVENT_MAX || + _esp_eth_dev.event == SYSTEM_EVENT_ETH_RX_DONE) { + memcpy(_esp_eth_dev.rx_buf, buffer, len); + _esp_eth_dev.rx_len = len; + _esp_eth_dev.event = SYSTEM_EVENT_ETH_RX_DONE; + netdev_trigger_event_isr(&_esp_eth_dev.netdev); + } mutex_unlock(&_esp_eth_dev.dev_lock);