Misleading API in netdev driver

[maribu]

Problem Description

The int (*recv )(netdev_t *dev, void *buf, size_t len, void *info) member in netdev_driver_t (see Doxygen) is a textbook example of a misleading API:

1. The function does three completely different things depending on arguments:
   • Receive the message and return its size, if buf != NULL
   • Returns the message size of the incoming message if (buf == NULL) && (len == 0)
   • Drops the incoming message if (buf == NULL) && (len != 0)
2. The function name only reflects one of the three cases
3. One of the three cases is a corner case (the drop packet case only occurs under high load), so a bug is going to be unnoticed for quite some time

Symptopms

This misleading API already lead to a bug #9784. I predict similar bugs will show up in the future, if the API is not changed. Update: Many similar bugs were found, see #9832

Suggestions to Address the Problem

1. Split the function into three functions (e.g. recv(), get_size() and drop()). This would be the cleanest API, but the ROM size of the implementation is likely to increase, as all three will share some common code. (I assume the compiler will inline the common code when implemented in separate functions, so even when no duplicate C code is present, the ROM size will likely grow.)
   • Pros:
     • Cleanest API: Reviewers and implementers will no longer easily forget about the size/drop features
     • Checking for missing size/drop implementations possible by an assert
     • No increase in runtime overhead or RAM (barely measurable speedup expected by not longer needing conditional jumps)
     • Biggest increase in code readability (control flow simplified, functions get shorter, function names match their intention, header parsing will be moved to short static functions)
   • Cons:
     • Biggest ROM increase compared to other approaches expected
     • Biggest increase in lines of code
2. Rename the function, e.g. to recv_or_size_or_drop(). While this name is very clumsy, at least it is very obvious that this functions has to implement three different things.
   • Pro:
     • No increase in RAM/ROM usage and runtime overhead
   • Cons:
     • Only cosmetic, does not really address the problem
     • Unclean API
3. Add an additional parameter (e.g. an enum) to specify which of the three things the function should do. This would be much more obvious to the programmer, even though a bit wasteful
   • Pros:
     • No increase in ROM usage (and likely in RAM usage, if increased stack usage does not result in higher stack sizes being used)
     • Compiler helps implementer if both size and drop is forgotten (unused argument)
   • Cons:
     • Unclean API
     • Slight increase in runtime (barely measurable)
4. Like 1., but only split off drop and keep size in recv
   • Pros:
     • Only change that part of the API that was affected by bugs so far
     • Less ROM overhead and lines of code than 1.
   • Cons:
     • Still only slightly less unclean API
     • Compared to 1. most of the ROM / lines of code increase is already been paid, so why not pay the rest and get a clean API
5. Add static inline functions to increase readability: In the upper layers add wrappers to call the size and drop feature od the recv function more explicitly; in the lower layers to test for the drop/size/recv mode more readable
   • Pros:
     • No increase in RAM/ROM usage and runtime overhead
     • Correct recv implementations and upper layer code gets more readible
   • Cons:
     • Unclean API
     • The problem is (mostly) about incorrect lower layer implementations, this change does not help much here

[jnohlgard]

I agree that this is a problem, it is also easy for the maintainers to overlook missing functionality when reviewing a new network driver because dropping the packet is a corner case and is not used very often in many simple lab tests. I don't know which solution is the best though. It would take some effort to refactor all drivers to an API change, so it's best if we discuss the potential solutions some more before beginning to change the code.

[maribu]

Also, there may be more than the three options that came to my mind. Maybe in the course of a discussion a better solution will be proposed than the three suggestions.

The suggestion 2 would be relatively cheap in terms of work required. I personally favor suggestion 1, as it is the most obvious API. This would make it easier for newbies to understand and for reviews to review. But that one would require the most work :-(

[jnohlgard]

My personal preference is splitting the API into three functions. The drivers can still use the same implementation behind the scenes, but it will be more clear when one function is missing.

[jia200x]

My personal preference is splitting the API into three functions. The drivers can still use the same implementation behind the scenes, but it will be more clear when one function is missing.

I also think so! It improves code readability

[maribu]

To push the discussion further I opened a PR implementing three separate functions approach. By adding a two glue functions driver implementing the old API are "converted" to the new one. This allows to gradually port the drivers to the new API.

[haukepetersen]

I agree that the design of the recv function is not the most beautiful API we have in RIOT. But as netdev is one of our core interfaces and being very low level, efficiency is extremely important here, and that is what was driving the 'overloading' of this function in the first place. IMO it is not an option here, to spend the extra amount of memory for addtional function pointers, just because it is slightly nicer to read!

I think option 5 would be quite a nice addition (at is completely transparent both to usage and resource usage). Tackling the issue of driver not fully implementing these three behaviors, adding explicit checks for this on the netdev checklist as given in https://github.com/RIOT-OS/RIOT/wiki/Guide:-Writing-a-device-driver-in-RIOT should help.

[maribu]

@haukepeterson: I think you got some facts wrong.

I assume by efficiency you mean how much cpu time a driver spents on handling recv/drop/size features of the netdev_driver_t interface. There is practically no difference between all approaches. (Theoretically splitting the recv function implementing the recv, drop and size feature as separate functions is faster by dropping the if-statement and the additional parameter of approach 3 will be slower. But practically it will be challenging to just measure the difference.) So efficiency as criteria is not helpful here, as there is practically no difference.

Regarding memory (= RAM) usage: Again, there is no difference. (Theoretically approach 3. needs 1-4 bytes more stack which could lead to the stack size needing to be increased, but practically this won't be the case.) So again, a good criteria, but not helpful here.

The really relevant tradeoff here is ROM size vs. (un-)clean API (and the security and maintainability implications of an (un-)clean API).

the extra amount of memory for addtional function pointers,

The majority of the ROM increase is not due to the function pointers, but the duplicated machine code (as the compiler inlines static helper functions used in all three cases). For instance, the ROM increase is 146 Bytes in case of the enc28j60 driver (see discussion in #9832 for details).

But as netdev is one of our core interfaces and being very low level [...]

This argument is often brought up, as its logic flaw is easily made. If internal code is hard to maintain/read/debug/review, all code using it directly or indirectly is affected by all the easily avoided bugs introduced this way. Also, fewer eyes reviews internal, low-level code, so that corner case bugs easily go unnoticed for quite some time. And from many, many hours of debugging I know for sure that internal low level functions that are in use for ages are the very last place to look at when debugging, so avoiding bugs there is much more worth than in more exposed positions.

just because it is slightly nicer to read!

You missed the point. The recv API is a textbook example of an misleading API. As result, a lot of bugs (missing drop implementation and missing size implementation without appropriate documentation/asserts) were introduced in RIOT (see all the FIXME comments in #9832). The missing drop implementation is not only easily overlooked by the person implementing a new driver, but also by the one reviewing it.

And to put this very clearly: The missing drop implementation is trivially and remotely exploitable denial of service vulnerability. (E.g. see the "ping of death" @gschorcht describes here #9806.) Similar bugs in Linux would get CVE numbers and the whole insane security circus (a catchy name, a logo, a web page and maybe even a franchise shop). My point is: Those bugs are extremely relevant for any serious use of RIOT and preventing that this type of bug reappears in any future netdev_driver implementation has a huge value.

A clean API is a huge step to make this kind of vulnerability less likely to pop up again in the future and I personally would not hastitate to "pay" 146 Bytes of ROM size for that.

Also, let me proof the following sentence wrong:

In case of the netdev_driver_t API, compact ROM size is generally more relevant than a clean API

We could easily save more ROM size replacing all function pointers in netdev_driver_t by a single handle_everything () function pointer. The additional required if-statements would consume about as much ROM as the removed function pointers safe, but the common locking/unlocking code would safe a few bytes of ROM for each function absorbed by handle_everything (). (And there might be more common code to eliminate as well.) Thus, this handle_everything ()-API would be more ROM efficient than current API. Yet, it is easy to see that this handle_everything ()-API would result in a unmaintainable mess. This reductio ad absurdo proves, that generally preferring ROM size over a clean API is wrong. (Of course, any concrete argument for this specific ROM vs readability tradeoff is not proved wrong.)

[maribu]

@OlegHahm, @miri64, @rousselk, @haukepetersen, @LudwigKnuepfer, @bergzand, @thomaseichinger, @smlng, @kaspar030, @PeterKietzmann, @mehlis, @BytesGalore, @kYc0o: You are listed as contributors to the cc2420 netdev_driver, which contains a critical DoS vulnerability which can easily be triggered by bringing RIOT in a low-memory state:

• The upper layer wants to get the size of the incoming packet. Instead of calling a size()-function, misleadingly the recv() function is called with buf == NULL and with len == 0. This is implemented correctly by the driver
• The upper layer tries to allocate a buffer big enough for that packet, but fails
• The upper layer wants the driver to drop the incoming packet. Instead of calling a drop()-function, misleadingly the recv() function is called with buf == NULL and with len > 0. This is not implemented, instead the size() facility of recv() is performed again. As a result, RIOT loops endlessly trying to drop that packet --> nothing else gets executed anymore and the board is to be powered off and on again.

Roughly every second RIOT netdev_driver has the same security hole. I reported this issue 6 weeks ago. I think its about time to attend to it and finally fix it: The very least that should be done is fixing the bugs in all affected drivers. Better would be to fix the underlying problem: The misleading netdev_driver API should be fixed by introducing a separate size() and drop() function, so that this bug gets not re-introduced with every new netdev_driver implemenation added to RIOT.

[maribu]

Also: In the decision is made to fix the API by splitting recv() into recv(), size() and drop(), then I'm very happy update my PR to include ports of every netdev_driver. (An then the compatibility layer could be removed from the PR as well.)

[miri64]

Also: In the decision is made to fix the API by splitting recv() into recv(), size() and drop(), then I'm very happy update my PR to include ports of every netdev_driver. (An then the compatibility layer could be removed from the PR as well.)

Maybe to settle this argument once and for all, first provide a port to the API to one device (e.g. cc2420) so we can compare the actual code size. I agree that we should fix this issue one way or the other ASAP. I also noticed while reviewing #9942 that not only the current API is misleading, its documentation is lacking to outright wrong in its current form. I'm preparing a PR for that at the moment.

[maribu]

Maybe to settle this argument once and for all, first provide a port to the API to one device (e.g. cc2420) so we can compare the actual code size.

Already did that on the 25th of August for the enc28j60, which also hat the same bug: #9832 (comment) ;-)

[miri64]

Ok, sorry did not know about that :-/

[maribu]

Closed in favor of fixing the drivers against current interface #10410. Maybe paying more attention and providing better documentation will solve this issue and comes for zero ROM cost compared to API changes.

If more attention and better documentation does not stop the same issue from being re-introduced, an API change can still be discussed. If the bug does not re-appear, we didn't waste ROM size on an API change that turned out to be unnecessary.

[jia200x]

I want to re-open this issue.
While working in the IEEE802.15.4 radio HAL I have found some stuff that might be useful. I'm pretty sure this also applies to other LL (Ethernet, LoRa, etc)

Considering that:

• Most simple applications only require a static receive function: that is, the recv function is called with a buffer and the pkt is drop if the len is bigger than the buffer size.
• Some stacks require to know the packet length in order to ensure zero copy (GNRC, LWIP)
• In all radios except mrf24j40 it's possible to get the packet length from an indirect method (register access, SRAM, etc).
• The overloaded recv logic is not for free. The complex logic also adds some ROM consumption compared to a static receive function (e.g fb_open() -> if(buf != NULL) { fb_read() } -> fb_close())
• It's more beneficial to read the packet length indirectly instead of opening and releasing the framebuffer. Doing so, we drop all kind of frame buffer protections (e.g at86rf2xx requires to go to PLL_ON to avoid overwriting the FB with incoming packets)

So, I think a variant of proposal 1. is feasible with:

• A mandatory static recv function: it tries to write the packet to a buffer. If buf len < pkt_size, drop.
• An (optional) len function that return the length of the packet. This function can be disabled in compile-time for applications with static input buffers.

I wrote this for the cc2420 and at86rf2xx and adapted gnrc_netif_ieee802154:

Patch

diff --git a/drivers/at86rf2xx/at86rf2xx_netdev.c b/drivers/at86rf2xx/at86rf2xx_netdev.c
index 6daa510ebc..2c11b358b9 100644
--- a/drivers/at86rf2xx/at86rf2xx_netdev.c
+++ b/drivers/at86rf2xx/at86rf2xx_netdev.c
@@ -47,12 +47,14 @@ static int _send(netdev_t *netdev, const iolist_t *iolist);
 static int _recv(netdev_t *netdev, void *buf, size_t len, void *info);
 static int _init(netdev_t *netdev);
 static void _isr(netdev_t *netdev);
+static int _len(netdev_t *netdev);
 static int _get(netdev_t *netdev, netopt_t opt, void *val, size_t max_len);
 static int _set(netdev_t *netdev, netopt_t opt, const void *val, size_t len);
 
 const netdev_driver_t at86rf2xx_driver = {
     .send = _send,
     .recv = _recv,
+    .len = _len,
     .init = _init,
     .isr = _isr,
     .get = _get,
@@ -138,19 +140,38 @@ static int _send(netdev_t *netdev, const iolist_t *iolist)
     return (int)len;
 }
 
-static int _recv(netdev_t *netdev, void *buf, size_t len, void *info)
+static int _len(netdev_t *netdev)
 {
     at86rf2xx_t *dev = (at86rf2xx_t *)netdev;
     uint8_t phr;
-    size_t pkt_len;
 
+#if defined(MODULE_AT86RFA1) || defined(MODULE_AT86RFR2)
+    phr = TST_RX_LENGTH;
+#else
     /* frame buffer protection will be unlocked as soon as at86rf2xx_fb_stop() is called,
      * Set receiver to PLL_ON state to be able to free the SPI bus and avoid losing data. */
     at86rf2xx_set_state(dev, AT86RF2XX_STATE_PLL_ON);
 
     /* start frame buffer access */
     at86rf2xx_fb_start(dev);
+    at86rf2xx_fb_read(dev, &phr, 1);
+    at86rf2xx_fb_stop(dev);
+#endif
+    return (phr & 0x7f) - 2;
+}
 
+static int _recv(netdev_t *netdev, void *buf, size_t len, void *info)
+{
+    at86rf2xx_t *dev = (at86rf2xx_t *)netdev;
+    uint8_t phr;
+    size_t pkt_len;
+    int res = -1;
+
+    /* start frame buffer access */
+    at86rf2xx_fb_start(dev);
+    if (buf == NULL) {
+        goto drop;
+    }
     /* get the size of the received packet */
 #if defined(MODULE_AT86RFA1) || defined(MODULE_AT86RFR2)
     phr = TST_RX_LENGTH;
@@ -161,81 +182,63 @@ static int _recv(netdev_t *netdev, void *buf, size_t len, void *info)
     /* ignore MSB (refer p.80) and subtract length of FCS field */
     pkt_len = (phr & 0x7f) - 2;
 
-    /* return length when buf == NULL */
-    if (buf == NULL) {
-        /* release SPI bus */
-        at86rf2xx_fb_stop(dev);
-
-        /* drop packet, continue receiving */
-        if (len > 0) {
-            /* set device back in operation state which was used before last transmission.
-             * This state is saved in at86rf2xx.c/at86rf2xx_tx_prepare() e.g RX_AACK_ON */
-            at86rf2xx_set_state(dev, dev->idle_state);
-        }
-
-        return pkt_len;
-    }
-
     /* not enough space in buf */
-    if (pkt_len > len) {
-        at86rf2xx_fb_stop(dev);
-        /* set device back in operation state which was used before last transmission.
-         * This state is saved in at86rf2xx.c/at86rf2xx_tx_prepare() e.g RX_AACK_ON */
-        at86rf2xx_set_state(dev, dev->idle_state);
-        return -ENOBUFS;
-    }
-    /* copy payload */
-    at86rf2xx_fb_read(dev, (uint8_t *)buf, pkt_len);
-
-    /* Ignore FCS but advance fb read - we must give a temporary buffer here,
-     * as we are not allowed to issue SPI transfers without any buffer */
-    uint8_t tmp[2];
-    at86rf2xx_fb_read(dev, tmp, 2);
-    (void)tmp;
-
-    /* AT86RF212B RSSI_BASE_VAL + 1.03 * ED, base varies for diff. modulation and datarates
-     * AT86RF232  RSSI_BASE_VAL + ED, base -91dBm
-     * AT86RF233  RSSI_BASE_VAL + ED, base -94dBm
-     * AT86RF231  RSSI_BASE_VAL + ED, base -91dBm
-     * AT86RFA1   RSSI_BASE_VAL + ED, base -90dBm
-     * AT86RFR2   RSSI_BASE_VAL + ED, base -90dBm
-     *
-     * AT86RF231 MAN. p.92, 8.4.3 Data Interpretation
-     * AT86RF232 MAN. p.91, 8.4.3 Data Interpretation
-     * AT86RF233 MAN. p.102, 8.5.3 Data Interpretation
-     *
-     * for performance reasons we ignore the 1.03 scale factor on the 212B,
-     * which causes a slight error in the values, but the accuracy of the ED
-     * value is specified as +/- 5 dB, so it should not matter very much in real
-     * life.
-     */
-    if (info != NULL) {
-        uint8_t ed = 0;
-        netdev_ieee802154_rx_info_t *radio_info = info;
-        at86rf2xx_fb_read(dev, &(radio_info->lqi), 1);
+    if (pkt_len <= len) {
+        /* copy payload */
+        at86rf2xx_fb_read(dev, (uint8_t *)buf, pkt_len);
+
+        /* Ignore FCS but advance fb read - we must give a temporary buffer here,
+         * as we are not allowed to issue SPI transfers without any buffer */
+        uint8_t tmp[2];
+        at86rf2xx_fb_read(dev, tmp, 2);
+        (void)tmp;
+
+        /* AT86RF212B RSSI_BASE_VAL + 1.03 * ED, base varies for diff. modulation and datarates
+         * AT86RF232  RSSI_BASE_VAL + ED, base -91dBm
+         * AT86RF233  RSSI_BASE_VAL + ED, base -94dBm
+         * AT86RF231  RSSI_BASE_VAL + ED, base -91dBm
+         * AT86RFA1   RSSI_BASE_VAL + ED, base -90dBm
+         * AT86RFR2   RSSI_BASE_VAL + ED, base -90dBm
+         *
+         * AT86RF231 MAN. p.92, 8.4.3 Data Interpretation
+         * AT86RF232 MAN. p.91, 8.4.3 Data Interpretation
+         * AT86RF233 MAN. p.102, 8.5.3 Data Interpretation
+         *
+         * for performance reasons we ignore the 1.03 scale factor on the 212B,
+         * which causes a slight error in the values, but the accuracy of the ED
+         * value is specified as +/- 5 dB, so it should not matter very much in real
+         * life.
+         */
+        if (info != NULL) {
+            uint8_t ed = 0;
+            netdev_ieee802154_rx_info_t *radio_info = info;
+            at86rf2xx_fb_read(dev, &(radio_info->lqi), 1);
 
 #if defined(MODULE_AT86RF231) || defined(MODULE_AT86RFA1) || defined(MODULE_AT86RFR2)
-        /* AT86RF231 does not provide ED at the end of the frame buffer, read
-         * from separate register instead */
-        at86rf2xx_fb_stop(dev);
-        ed = at86rf2xx_reg_read(dev, AT86RF2XX_REG__PHY_ED_LEVEL);
+            /* AT86RF231 does not provide ED at the end of the frame buffer, read
+             * from separate register instead */
+            at86rf2xx_fb_stop(dev);
+            ed = at86rf2xx_reg_read(dev, AT86RF2XX_REG__PHY_ED_LEVEL);
 #else
-        at86rf2xx_fb_read(dev, &ed, 1);
-        at86rf2xx_fb_stop(dev);
+            at86rf2xx_fb_read(dev, &ed, 1);
+            at86rf2xx_fb_stop(dev);
 #endif
-        radio_info->rssi = RSSI_BASE_VAL + ed;
-        DEBUG("[at86rf2xx] LQI:%d high is good, RSSI:%d high is either good or"
-              "too much interference.\n", radio_info->lqi, radio_info->rssi);
-    }
-    else {
-        at86rf2xx_fb_stop(dev);
+            radio_info->rssi = RSSI_BASE_VAL + ed;
+            DEBUG("[at86rf2xx] LQI:%d high is good, RSSI:%d high is either good or"
+                  "too much interference.\n", radio_info->lqi, radio_info->rssi);
+            res = pkt_len;
+            goto out;
+        }
     }
 
+drop:
+    at86rf2xx_fb_stop(dev);
     /* set device back in operation state which was used before last transmission.
      * This state is saved in at86rf2xx.c/at86rf2xx_tx_prepare() e.g RX_AACK_ON */
     at86rf2xx_set_state(dev, dev->idle_state);
 
-    return pkt_len;
+out:
+    return res;
 }
 
 static int _set_state(at86rf2xx_t *dev, netopt_state_t state)
diff --git a/drivers/cc2420/cc2420.c b/drivers/cc2420/cc2420.c
index b8e8c2a0b2..8041529025 100644
--- a/drivers/cc2420/cc2420.c
+++ b/drivers/cc2420/cc2420.c
@@ -173,29 +173,18 @@ int cc2420_rx(cc2420_t *dev, uint8_t *buf, size_t max_len, void *info)
 {
     (void)info;
 
-    uint8_t len;
+    uint8_t len = -1;
     uint8_t crc_corr;
 
-    /* without a provided buffer, only readout the length and return it */
-    if (buf == NULL) {
-        /* get the packet length (without dropping it) (first byte in RX FIFO) */
-        cc2420_ram_read(dev, CC2420_RAM_RXFIFO, &len, 1);
-        len -= 2;   /* subtract RSSI and FCF */
-        DEBUG("cc2420: recv: packet of length %i in RX FIFO\n", (int)len);
-        if (max_len != 0) {
-            DEBUG("cc2420: recv: Dropping frame as requested\n");
-            _flush_rx_fifo(dev);
-        }
-    }
-    else {
+    if (buf != NULL)
         /* read length byte */
         cc2420_fifo_read(dev, &len, 1);
         len -= 2;   /* subtract RSSI and FCF */
 
         if (len > max_len) {
             DEBUG("cc2420: recv: Supplied buffer to small\n");
-            _flush_rx_fifo(dev);
-            return -ENOBUFS;
+            len = -ENOBUFS;
+            goto out;
         }
 
         /* read fifo contents */
@@ -218,10 +207,9 @@ int cc2420_rx(cc2420_t *dev, uint8_t *buf, size_t max_len, void *info)
             radio_info->rssi = CC2420_RSSI_OFFSET + rssi;
             radio_info->lqi = crc_corr & CC2420_CRCCOR_COR_MASK;
         }
-
-        /* finally flush the FIFO */
-        _flush_rx_fifo(dev);
-    }
+out:
+    /* finally flush the FIFO */
+    _flush_rx_fifo(dev);
 
     return (int)len;
 }
diff --git a/drivers/cc2420/cc2420_netdev.c b/drivers/cc2420/cc2420_netdev.c
index 10b4eddf1a..ff93043c83 100644
--- a/drivers/cc2420/cc2420_netdev.c
+++ b/drivers/cc2420/cc2420_netdev.c
@@ -42,12 +42,14 @@ static int _send(netdev_t *netdev, const iolist_t *iolist);
 static int _recv(netdev_t *netdev, void *buf, size_t len, void *info);
 static int _init(netdev_t *netdev);
 static void _isr(netdev_t *netdev);
+static int _len(netdev_t *netdev);
 static int _get(netdev_t *netdev, netopt_t opt, void *val, size_t max_len);
 static int _set(netdev_t *netdev, netopt_t opt, const void *val, size_t len);
 
 const netdev_driver_t cc2420_driver = {
     .send = _send,
     .recv = _recv,
+    .len = _len,
     .init = _init,
     .isr = _isr,
     .get = _get,
@@ -149,6 +151,14 @@ static int _send(netdev_t *netdev, const iolist_t *iolist)
     return (int)cc2420_send(dev, iolist);
 }
 
+static int _len(netdev_t *netdev)
+{
+    cc2420_t *dev = (cc2420_t *)netdev;
+    uint8_t len;
+    cc2420_ram_read(dev, CC2420_RAM_RXFIFO, &len, 1);
+    return len - 2;
+}
+
 static int _recv(netdev_t *netdev, void *buf, size_t len, void *info)
 {
     cc2420_t *dev = (cc2420_t *)netdev;
diff --git a/drivers/include/net/netdev.h b/drivers/include/net/netdev.h
index 72094e86eb..f335104284 100644
--- a/drivers/include/net/netdev.h
+++ b/drivers/include/net/netdev.h
@@ -352,6 +352,7 @@ typedef struct netdev_driver {
      * @return packet size (or upper bound estimation) if buf == NULL
      */
     int (*recv)(netdev_t *dev, void *buf, size_t len, void *info);
+    int (*len)(netdev_t *dev);
 
     /**
      * @brief the driver's initialization function
diff --git a/sys/net/gnrc/netif/ieee802154/gnrc_netif_ieee802154.c b/sys/net/gnrc/netif/ieee802154/gnrc_netif_ieee802154.c
index ad15ec080a..be0261ca9b 100644
--- a/sys/net/gnrc/netif/ieee802154/gnrc_netif_ieee802154.c
+++ b/sys/net/gnrc/netif/ieee802154/gnrc_netif_ieee802154.c
@@ -98,7 +98,7 @@ static gnrc_pktsnip_t *_recv(gnrc_netif_t *netif)
     netdev_t *dev = netif->dev;
     netdev_ieee802154_rx_info_t rx_info;
     gnrc_pktsnip_t *pkt = NULL;
-    int bytes_expected = dev->driver->recv(dev, NULL, 0, NULL);
+    int bytes_expected = dev->driver->len(dev);
 
     if (bytes_expected >= (int)IEEE802154_MIN_FRAME_LEN) {
         int nread;

I didn't introduce any functions (e.g SRAM access) to have a fair comparison. These are the results:

samr21-xpro (at86rf2xx): Default example on master

   text	  data	   bss	   dec	   hex	filename
  41008	   500	  6064	 47572	  b9d4	/data/riotbuild/riotbase/examples/default/bin/samr21-xpro/default.elf

samr21-xpro (at86rf2xx): Default example with patch

Building application "default" for "samr21-xpro" with MCU "samd21".

   text	  data	   bss	   dec	   hex	filename
  41004	   500	  6064	 47568	  b9d0	/data/riotbuild/riotbase/examples/default/bin/samr21-xpro/default.elf

z1 (cc2420): Default example on master

Building application "default" for "z1" with MCU "msp430fxyz".

   text	  data	   bss	   dec	   hex	filename
  34808	    20	  2804	 37632	  9300	/data/riotbuild/riotbase/examples/default/bin/z1/default.elf

z1 (cc2420): Default example with patch

Building application "default" for "z1" with MCU "msp430fxyz".

   text	  data	   bss	   dec	   hex	filename
  34790	    20	  2804	 37614	  92ee	/data/riotbuild/riotbase/examples/default/bin/z1/default.elf

Haven't tried on AVR yet, but this shows that the overloaded solution is not necessarily better in terms of ROM consumption and the API is very unclean. Also there are some optimization that could be done with a static recv function that are hard to write now

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.