drivers/at86rf215: make use of packet queue if available

[benpicco]

Contribution description

Instead of making the driver block until the hardware finishes TXing, we can now simply queue the packet and rely on it getting re-send when the hardware is ready.

This allows to remove an ugly hack where we call the ISR in a loop to achieve blocking send while still servicing radio events.

Testing procedure

Use the netif_pktq module and send fragmented packets.

master

2020-09-04 18:39:16,406 #  ping6 2001:db8::204:2519:1801:c8c5 -s 512 -c 10
2020-09-04 18:39:16,533 # 520 bytes from 2001:db8::204:2519:1801:c8c5: icmp_seq=0 ttl=64 rssi=-54 dBm time=113.710 ms
2020-09-04 18:39:17,525 # 520 bytes from 2001:db8::204:2519:1801:c8c5: icmp_seq=1 ttl=64 rssi=-57 dBm time=109.456 ms
2020-09-04 18:39:18,534 # 520 bytes from 2001:db8::204:2519:1801:c8c5: icmp_seq=2 ttl=64 rssi=-56 dBm time=120.383 ms
2020-09-04 18:39:19,540 # 520 bytes from 2001:db8::204:2519:1801:c8c5: icmp_seq=3 ttl=64 rssi=-62 dBm time=118.382 ms
2020-09-04 18:39:20,533 # 520 bytes from 2001:db8::204:2519:1801:c8c5: icmp_seq=4 ttl=64 rssi=-62 dBm time=110.219 ms
2020-09-04 18:39:22,533 # 520 bytes from 2001:db8::204:2519:1801:c8c5: icmp_seq=6 ttl=64 rssi=-65 dBm time=112.776 ms
2020-09-04 18:39:23,539 # 520 bytes from 2001:db8::204:2519:1801:c8c5: icmp_seq=7 ttl=64 rssi=-61 dBm time=113.988 ms
2020-09-04 18:39:24,548 # 520 bytes from 2001:db8::204:2519:1801:c8c5: icmp_seq=8 ttl=64 rssi=-61 dBm time=123.613 ms
2020-09-04 18:39:26,401 # 
2020-09-04 18:39:26,420 # --- 2001:db8::204:2519:1801:c8c5 PING statistics ---
2020-09-04 18:39:26,425 # 10 packets transmitted, 8 packets received, 20% packet loss
2020-09-04 18:39:26,428 # round-trip min/avg/max = 109.456/115.315/123.613 ms

netif_pktq

2020-09-04 18:38:11,040 #  ping6 2001:db8::204:2519:1801:c8c5 -s 512 -c 10
2020-09-04 18:38:11,133 # 520 bytes from 2001:db8::204:2519:1801:c8c5: icmp_seq=0 ttl=64 rssi=-57 dBm time=77.002 ms
2020-09-04 18:38:12,128 # 520 bytes from 2001:db8::204:2519:1801:c8c5: icmp_seq=1 ttl=64 rssi=-55 dBm time=73.160 ms
2020-09-04 18:38:13,121 # 520 bytes from 2001:db8::204:2519:1801:c8c5: icmp_seq=2 ttl=64 rssi=-56 dBm time=75.721 ms
2020-09-04 18:38:14,129 # 520 bytes from 2001:db8::204:2519:1801:c8c5: icmp_seq=3 ttl=64 rssi=-56 dBm time=74.772 ms
2020-09-04 18:38:15,120 # 520 bytes from 2001:db8::204:2519:1801:c8c5: icmp_seq=4 ttl=64 rssi=-55 dBm time=73.810 ms
2020-09-04 18:38:16,127 # 520 bytes from 2001:db8::204:2519:1801:c8c5: icmp_seq=5 ttl=64 rssi=-56 dBm time=72.549 ms
2020-09-04 18:38:17,134 # 520 bytes from 2001:db8::204:2519:1801:c8c5: icmp_seq=6 ttl=64 rssi=-56 dBm time=77.309 ms
2020-09-04 18:38:18,130 # 520 bytes from 2001:db8::204:2519:1801:c8c5: icmp_seq=7 ttl=64 rssi=-57 dBm time=83.665 ms
2020-09-04 18:38:19,133 # 520 bytes from 2001:db8::204:2519:1801:c8c5: icmp_seq=8 ttl=64 rssi=-58 dBm time=75.729 ms
2020-09-04 18:38:20,128 # 520 bytes from 2001:db8::204:2519:1801:c8c5: icmp_seq=9 ttl=64 rssi=-59 dBm time=76.030 ms
2020-09-04 18:38:20,129 # 
2020-09-04 18:38:20,131 # --- 2001:db8::204:2519:1801:c8c5 PING statistics ---
2020-09-04 18:38:20,142 # 10 packets transmitted, 10 packets received, 0% packet loss
2020-09-04 18:38:20,147 # round-trip min/avg/max = 72.549/75.974/83.665 ms

Issues/PRs references

uses feature from #11263

[miri64]

Not sure how this would be expressed in the build system: but shouldn't netif_pktq rather be a feature that the driver may request? If a stack does not support this for whatever reason you would still be able to use it without netif_pktq and the user wouldn't be bothered with such a low-level decision.

[benpicco]

Are you suggesting something like FEATURE_OPTIONAL?
IIRC modules can't provide features with the current build system.

Or should I just add USEMODULE += netif_pktq to the driver's Makefile.dep and drop the blocking code entirely?

[miri64]

I meant the first, but did not know, that it is not easily realizable, currently. I would not enforce the packet queue, as one might want to optimize it out.

[fjmolinas]

I meant the first, but did not know, that it is not easily realizable, currently. I would not enforce the packet queue, as one might want to optimize it out.

For now FEATURES_OPTIONAL does not apply to this use case. The only thing that kind of resembles this is DEFAULT_MODULE but that has its own issues, would only work if gnrc_netif_pktq has no dependencies of its own.

AFAIK this seems like the right way for the use to include netif_pktq if solicited, its an opt-in policy, the only opt-out option for modules is DEFAULT_MODULE.

[fjmolinas]

@miri64 I think the way it is modeled currently is OK considered our current buildystem, should we go ahead with this one? Are there any other showstoppers?

[benpicco]

So many options which one should I take? 😄

[miri64]

I would prefer the last one :-).

[benpicco]

Pulling in at86rf215_blocking_send when gnrc_netif_pktq is not selected sounds like the best approach.

[benpicco]

The only problem I see is that now drivers can't depend on having a netif_pktq available and still have to implement a fall-back blocking send :\

[miri64]

That will be addressed by @jia200x's new API though, right?

[fjmolinas]

Is everything good on your side here @miri64 ?

[maribu]

Looks good to me. To me it sounded like @miri64 is fine with the PR. I will test and (depending on the test result) ACK, but I would like to wait for @miri64 to confirm her comments are addressed before the merge button is hit.

[maribu]

Works as advertised. On the nucleo-f767zi I only get a small reduction of the RTT (124.234/124.345/124.441 ms vs 124.675/124.735/124.811 ms for ping6 -s 1024). I blindly assume this is due to the CPU not being a bottleneck on the fast nucleo-f767zi anyway.

[maribu]

ACK

[miri64]

Murdock is very unhappy :-/

[benpicco]