Bluetooth: Remove rx < tx prio check

[alwa-nordic]

The comment states the reason for this requirement is to detect violations by a remote peer. The ATT version of this was removed in ea04fd9. TLDR: It's neither possible for, nor the job of the host to police the remote device.

We remove this requirement to be more flexible about the number of priority levels in the system, and to avoid the temptation of using priorities as a synchronization mechanism.

[Thalley]

It's neither possible for, nor the job of the host to police the remote device.

If a remote device sends 2 ATT requests without waiting for a response today, what does the Zephyr host do?

If the RX thread gets a higher priority than the TX thread, will it handle it the same way?

Just want to make sure that this doesn't allow for remote devices to DOS us if the thread priorities are "incorrect"

[jori-nordic]

Then we should have a test for the DoS instead :)
We do have a test for the pipelining that you describe: https://github.com/jori-nordic/zephyr/blob/remove-ipsp/tests/bsim/bluetooth/host/att/pipeline/dut/src/main.c#L322

[Thalley]

Then we should have a test for the DoS instead :) We do have a test for the pipelining that you describe: https://github.com/jori-nordic/zephyr/blob/remove-ipsp/tests/bsim/bluetooth/host/att/pipeline/dut/src/main.c#L322

Yeah, it should be trivial to run (or even make a CI config) for that test where the RX thread has a higher priority than the TX thread, to see if we can actually remove this requirement without potentionally breaking stuff

[alwa-nordic]

If a remote device sends 2 ATT requests without waiting for a response today, what does the Zephyr host do?

The current implementation will just-work, handing the pipelined the requests.

If the RX thread gets a higher priority than the TX thread, will it handle it the same way?

Yes. But the TX of the first response may be delayed and sent with the second.

Just want to make sure that this doesn't allow for remote devices to DOS us if the thread priorities are "incorrect"

This is a valid concern. The DOS in this case means the host will use more att_pool buffers or other tx resources than one would expect, possibly starving other tx users. But this DOS is already possible in a myriad of other ways. E.g. I think the remote device can send a bunch of L2CAP control messages and NAK the responses on air. We might need to implement per-acl buffer pools to remedy this.

[Thalley]

As long as receiving multiple ATT requests from a malfunctioning/malicious remote device device doesn't when RX has higher prio than TX doesn't break things, I'm OK with removing this

It would be nice to have a least some testing where CONFIG_BT_RX_PRIO > CONFIG_BT_HCI_TX_PRIO to ensure that Zephyr works without this restricton

[jori-nordic]

@alwa-nordic can you add a prj.conf for the pipeline test with the priorities inverted? can be in a future PR