From 950c71cc10924b45becf3a9f4c9fb6c3b63fda03 Mon Sep 17 00:00:00 2001 From: dylad Date: Mon, 19 Jul 2021 21:23:22 +0200 Subject: [PATCH] cpu/sam0: improve ethernet driver resilience In case of network heavy traffic on the Ethernet, interrupts fire faster than the netdev thread can process them and we run out of buffers. With this commit, we now check if we don't have buffers available, so we can flush everything and restart reception properly even if we did drop a few in the operation --- cpu/sam0_common/periph/eth.c | 60 +++++++++++++++++++++++---- cpu/sam0_common/sam0_eth/eth-netdev.c | 18 +++++--- 2 files changed, 64 insertions(+), 14 deletions(-) diff --git a/cpu/sam0_common/periph/eth.c b/cpu/sam0_common/periph/eth.c index 106908f2be..8bc848125e 100644 --- a/cpu/sam0_common/periph/eth.c +++ b/cpu/sam0_common/periph/eth.c @@ -90,6 +90,19 @@ static uint8_t rx_idx; static uint8_t rx_buf[ETH_RX_BUFFER_COUNT][ETH_RX_BUFFER_SIZE] __attribute__((aligned(GMAC_BUF_ALIGNMENT))); static uint8_t tx_buf[ETH_TX_BUFFER_COUNT][ETH_TX_BUFFER_SIZE] __attribute__((aligned(GMAC_BUF_ALIGNMENT))); +extern sam0_eth_netdev_t _sam0_eth_dev; + +/* Flush our reception buffers and reset reception internal mechanism, + this function may be call from ISR context */ +void sam0_clear_rx_buffers(void) +{ + for (int i=0; iRBQB.reg = (uint32_t) rx_desc; +} static void _init_desc_buf(void) { @@ -193,13 +206,33 @@ int sam0_eth_send(const struct iolist *iolist) return len; } -static int _try_receive(char* data, int max_len, int block) +static int _try_receive(char* data, unsigned max_len, int block) { (void)block; unsigned rxlen = 0; uint16_t idx = rx_idx; - /* Ensure we are at the beginning of the new frame */ - while (!(rx_curr->address & DESC_RX_ADDR_OWNSHP) && (rx_curr->status & DESC_RX_STATUS_STA_FRAME)) {} + uint8_t tmp = ETH_RX_BUFFER_COUNT; + + /* Check if the current rx descriptor contains the beginning of + a new frame, iterates over all our RX buffers if not. + If there is no new frame (because we may have flush our RX + buffers due to BNA interrupt), return an error to netdev so + we can move forward */ + do { + if ((rx_curr->address & DESC_RX_ADDR_OWNSHP) + && (rx_curr->status & DESC_RX_STATUS_STA_FRAME)) { + break; + } + else { + idx = (idx+1) % ETH_RX_BUFFER_COUNT; + rx_curr = &rx_desc[idx]; + tmp--; + } + } while (tmp > 0); + + if (tmp == 0) { + return -ENOBUFS; + } for (unsigned cpt=0; cpt < ETH_RX_BUFFER_COUNT; cpt++) { /* Get the length of the received frame */ @@ -210,6 +243,11 @@ static int _try_receive(char* data, int max_len, int block) if (max_len) { /* If buffer available, copy data into it */ if (data) { + /* If provided buffer is smaller than the received frame, + drop it as netdev request */ + if (rxlen + len > max_len) { + return -ENOBUFS; + } memcpy(&data[rxlen], rx_buf[idx], len); } /* Tell the GMAC IP that we don't need this frame anymore */ @@ -226,16 +264,22 @@ static int _try_receive(char* data, int max_len, int block) idx = (idx + 1) % ETH_RX_BUFFER_COUNT; rx_curr = &rx_desc[idx]; - } - /* restore the previous index if packets were not released */ - if (!max_len) { + } + /* restore the previous index if packets were not released */ + if (!max_len) { rx_curr = &rx_desc[rx_idx]; - } + } /* Point to the next buffer as GMAC IP will likely used it to store the next frame */ - else { + else { rx_idx = (idx+1) % ETH_RX_BUFFER_COUNT; rx_curr = &rx_desc[rx_idx]; + } + + /* If provided buffer is smaller than the received frame, + drop it as netdev request */ + if (data != NULL && rxlen > max_len) { + return -ENOBUFS; } return rxlen; diff --git a/cpu/sam0_common/sam0_eth/eth-netdev.c b/cpu/sam0_common/sam0_eth/eth-netdev.c index 37a20dc42f..e04616f9b0 100644 --- a/cpu/sam0_common/sam0_eth/eth-netdev.c +++ b/cpu/sam0_common/sam0_eth/eth-netdev.c @@ -40,6 +40,7 @@ extern int sam0_eth_send(const struct iolist *iolist); extern int sam0_eth_receive_blocking(char *data, unsigned max_len); extern void sam0_eth_set_mac(const eui48_t *mac); extern void sam0_eth_get_mac(char *out); +extern void sam0_clear_rx_buffers(void); /* SAM0 CPUs only have one GMAC IP, so it is safe to statically defines one in this file */ @@ -140,24 +141,29 @@ void sam0_eth_setup(netdev_t* netdev) netdev_register(netdev, NETDEV_SAM0_ETH, 0); } -/* TODO: rework the whole isr management... */ void isr_gmac(void) { uint32_t isr; - uint32_t tsr; uint32_t rsr; isr = GMAC->ISR.reg; - tsr = GMAC->TSR.reg; rsr = GMAC->RSR.reg; + (void)isr; + /* New frame received, signal it to netdev */ if (rsr & GMAC_RSR_REC) { netdev_trigger_event_isr(_sam0_eth_dev.netdev); } - - GMAC->TSR.reg = tsr; + /* Buffers Not Available, this can occur if there is a heavy traffic + on the network. In this case, disable the GMAC reception, flush + our internal buffers and re-enable the reception. This will drop + a few packets but it allows the GMAC IP to remains functional */ + if (rsr & GMAC_RSR_BNA) { + GMAC->NCR.reg &= ~GMAC_NCR_RXEN; + sam0_clear_rx_buffers(); + GMAC->NCR.reg |= GMAC_NCR_RXEN; + } GMAC->RSR.reg = rsr; - GMAC->ISR.reg = isr; cortexm_isr_end(); }