RIOT icon indicating copy to clipboard operation
RIOT copied to clipboard

sys/net/gnrc_netif: Make use of confirm send

Open maribu opened this issue 3 years ago • 3 comments

Contribution description

  • Introduce modules netdev_legacy_api and netdev_new_api to allow optimizing for the "with confirm_send()" and "without confirm_send()" variant of the netdev API
    • Each netdev driver has to depend on exactly one of those
    • If both are used (because two or more netdevs are in use and one is migrated and the other not), the API version is checked at runtime
  • With netdev_new_api translate NETDEV_EVENT_TX_COMPLETE into an sys/event event, so that this can be signaled from ISR. This safes a roundtrip to the ISR handler if the type of the event is known at ISR time (e.g. multiple signal pins or peripheral netdev)
  • If the new API is used, wait for the NETDEV_EVENT_TX_COMPLETE event before signalling upper layers completion with gnrc_tx_sync.

Testing procedure

This needs to be tested in conjunction with a driver actually being ported to the new API. However, we cannot port drivers to the new API until both GNRC and lwIP correctly operate with the new API.

https://github.com/RIOT-OS/RIOT/pull/18428 ports the STM32 Ethernet driver to the new API, so that this PR can be tested there.

Issues/PRs references

Similar to https://github.com/RIOT-OS/RIOT/pull/15821 but uses the now existing sys/event infrastructure rather than thread_flags

  • [ ] Depends on and includes: https://github.com/RIOT-OS/RIOT/pull/18426

maribu avatar May 25 '22 13:05 maribu

Does the WIP label still apply?

benpicco avatar Jul 22 '22 10:07 benpicco

This could be merged if I drop the Ethernet change, as that change will break lwIP. I am currently working on the same thing for lwIP, after which driver could safely start making use of confirm_send.

maribu avatar Jul 22 '22 10:07 maribu

I now have a branch where lwIP is working with confirm_send on the STM32 Ethernet driver.

I am still a bit worried that lwIP uses two threads that interact with the netdev. Since GNRC is using a single thread to bothc all netdev_driver_t::send() and netdev_driver_t::recv(), I have the feeling that most drivers where not written with thread safety as a requirement in mind.

maribu avatar Aug 08 '22 20:08 maribu

This needs a rebase

benpicco avatar Aug 18 '22 13:08 benpicco

Ping :)

maribu avatar Sep 14 '22 11:09 maribu

Please squash!

benpicco avatar Sep 15 '22 11:09 benpicco

All green. If anyone wants to have another look before I hit merge, it is now or never ;)

maribu avatar Sep 16 '22 13:09 maribu

Thx everyone :)

maribu avatar Sep 17 '22 18:09 maribu