net: l2: ieee802154 improved association request and response procedure (incl. test coverage) #60120
Conversation
Minor simplification in the definition of IEEE 802.15.4 Kconfig packet fragmentation configuration. Signed-off-by: Florian Grandel <fgrandel@code-for-humans.de>
The IEEE 802.15.4-2020 standard introduces an association type field to support fast association, see sections 6.4.3 and 7.5.2. We do not yet implement fast association but we introduce the flag to make this obvious. Signed-off-by: Florian Grandel <fgrandel@code-for-humans.de>
The IEEE 802.15.4 stack defines radio API helpers that provide simplified and encapsulated access to radio API features. These helpers were missing the `_radio_` infix. This infix is introduced to clearly distinguish between MAC and PHY concerns. While PHY features may be shared between L2 implementations (including the functions concerned here), this is not true for MAC features. Signed-off-by: Florian Grandel <fgrandel@code-for-humans.de>
Calculates macResponseWaitTime and applies it to the association process. As the timing calculation re-uses symbol period calculations and other PHY timing constants previously introduced, these are now shared as utility functions. Signed-off-by: Florian Grandel <fgrandel@code-for-humans.de>
Input validation of some of the IEEE 802.15.4 net_mgmt commands was incomplete and/or inconsistent. This change introduces a consistent approach to input validation that is easier to follow. Signed-off-by: Florian Grandel <fgrandel@code-for-humans.de>
zephyrbot
requested review from
canisLupus1313,
cfriedt,
jciupis,
mariuszpos,
pdgendt and
ssharks
|
UPDATE: Fixed - was actually just a platform specific bug. |
Fixes an endianness bug in PAN ID assignment. Signed-off-by: Florian Grandel <fgrandel@code-for-humans.de>
Fixes an off-by-one bug in the parsing routine of the coordinator address when associating via shell command. Signed-off-by: Florian Grandel <fgrandel@code-for-humans.de>
ghost
force-pushed
the
feat/ieee802154-improved-assoc
branch
from
3724f46
to
158e292
Compare
|
Just curious if you've tested on multiple hw platforms. |
No - I'm testing this on CC13xx chips only. But most of the code I'm touching in the native stack was so badly broken before anyway that I'm quite sure that no one used that so far. We'll test quite intensively on various hardware once we have the TSCH stack running. We'll also do compat tests with Contiki then - one of the maintainers, Atis Elst promised to help us with that. |
ghost
force-pushed
the
feat/ieee802154-improved-assoc
branch
3 times, most recently
from
f63bade
to
9d5d972
Compare
@cfriedt Yes, this is true. As a first step I'm documenting the correspondence of standard primitives and native code features wherever possible, this is ongoing work. The long term goal is: #58438 (comment) (which includes your proposal but is more general). |
ghost
force-pushed
the
feat/ieee802154-improved-assoc
branch
from
9d5d972
to
48a7874
Compare
|
UPDATE: Just wrote tests for setting the external address and disassociation via shell (not part of this PR to not overload it but which led to minor bug fixes in the disassociation code), confirming that both work. The tests will be provided in a separate PR once this one is merged. |
ghost
force-pushed
the
feat/ieee802154-improved-assoc
branch
from
48a7874
to
5d27ac9
Compare
ghost
force-pushed
the
feat/ieee802154-improved-assoc
branch
from
5d27ac9
to
2f27a46
Compare
This change introduces test coverage for association request and response. Based on this coverage, several closely related issues were found in the association process which cannot be split into separate changes without breaking the build. Most notably did the associate and disassociate net_mgmt commands send already encoded IEEE 802.15.4 MPDUs to L3 rather than L2. L3 treated them as payload and made L2 wrap them with another LL header/footer which produced invalid packets. The tests also enforce better aligment of the association process with the IEEE 802.15.4-2020 standard: * Association requests now ask for ACK as required by the standard. The fake driver was enhanced to produce ACK packages when requested. * macPanId and macCoordinator* MAC PIB attributes are set in the right order for improved filtering of association responses. * The coordinator may decide not to assign a short address to the end device even when associated. This is now supported. * The coordinator may or may not use a short address. Coordinators choosing not to support short addresses are now supported. * Updating the association will now remove any previously added short address from the hardware filter. * The short address may no longer be changed by the user while associated to a PAN. Only the coordinator is allowed to allocate short addresses. * Validation of outgoing and incoming association request/response packets is improved. All changes are documented by pointers into the spec. Signed-off-by: Florian Grandel <fgrandel@code-for-humans.de>
Improves the documentation of `struct ieee802154_context` members. Signed-off-by: Florian Grandel <fgrandel@code-for-humans.de>
Removes redundant ACK state from `struct ieee802154_context` and simplifies the ACK procedure. Signed-off-by: Florian Grandel <fgrandel@code-for-humans.de>
ghost
force-pushed
the
feat/ieee802154-improved-assoc
branch
from
1e1c681
to
94659d4
Compare
This is the next change set in the #50336 series.
First a few minor changes are introduced that probably do not justify their own PR and should be easy to review although they are somewhat unrelated. Please let me know if you want to see them in a separate PR.
The main focus of this PR is on the association request and response procedure:
It turned out that the associate and disassociate commands probably never really worked as they had some fundamental flaws (see the commit comments). I tried hard to make several changes to make the fixes easier to review but I didn't succeed w/o breaking the build. If you have an idea how to break this up let me know. But then probably it is not so important even if there are still some bugs in there as it cannot be more broken than before. ;-)
Disassociation does not yet have test coverage - but then again - it cannot be more broken now than it already was. And the PR is already large enough. Test coverage for disassociation will follow in a later PR.
Finally I tried to simplify the locking scheme of the ACK procedure. To be honest, I'm not even sure that ACK properly worked before - although the tests seemed to indicate that it did. I tested this simplified procedure on hardware but it would be great if you had a closer look whether this kind of locking really worked and maybe also why it worked before (or not)...
Anyway let me know if you have any ideas how I can make this easier to review.