Skip to content

Conversation

maribu
Copy link
Member

@maribu maribu commented Sep 15, 2025

Contribution description

This changes the behavior of the netdev implementation to match what async drivers do. The current implementation is synchronous, but that can be fixed once an async UART API is available.

In any case, we rather should not complicate testing of the network stack by having network devices come in different flavors.

Testing procedure

No regressions in DOSE

Issues/PRs references

None

This changes the behavior of the netdev implementation to match what
async drivers do. The current implementation is synchronous, but that
can be fixed once an async UART API is available.

In any case, we rather should not complicate testing of the network stack
by having network devices come in different flavors.
@maribu maribu requested a review from benpicco September 15, 2025 08:43
@maribu maribu added the Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation label Sep 15, 2025
@github-actions github-actions bot added the Area: drivers Area: Device drivers label Sep 15, 2025
@maribu
Copy link
Member Author

maribu commented Sep 15, 2025

@elenaf9 FYI

@benpicco
Copy link
Contributor

Will this really simplify things? We still need the

    if (gnrc_netif_netdev_legacy_api(netif) || (res != 0)) {
        _tx_done(netif, pkt, tx_sync, res, push_back);
    }

check regardless - even if we get rid of gnrc_netif_netdev_legacy_api() we would still have

    int res = netif->ops->send(netif, pkt);
    if (res != 0) {
        _tx_done(netif, pkt, tx_sync, res, push_back);
    }

so we get the behavior that blocking netdevs can return the number of sent bytes directly for free.

@maribu
Copy link
Member Author

maribu commented Sep 29, 2025

I really would like to get rid of any driver details leaking into the network stack. Right now we have to test every network stack change with at least three drivers (legacy, new async, new sync) to be sure we don't introduce any regressions. I really would like to get back where a CI test on native would catch any network stack breakage.

But maybe the better approach here would be to just add an async UART API and turn DOSE/SLIP/ETHOS to a truly async driver, not one that fakes being async.

A transmission of a large frame via UART can take ages. E.g. sending 256 B with 10 bits per byte (1 start bit, 8 data bits, 0 parity bits, 1 stop bit) would mean blocking the thread for 22 ms at 115,200 Baud or 267 ms at 9,600 Baud. If we do not want to be force to spawn one thread for each and every netdev, those latency are pretty prohibitive.

@benpicco
Copy link
Contributor

Isn't netdev_legacy_api essentially the same as the new API in blocking mode? So it would be easier getting rid of that than to clutter the code of the synchronous netdevs (which I assume will always exist).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: drivers Area: Device drivers Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants