net: l2: ieee802154: set the src addr before deciphering the frame #78525
Conversation
The call to ieee802154_decipher_data_frame() from ieee802154_recv() always failed when checking the length of the src address, as it was not yet set. This sets the src and dst address of the packet before calling the decipher function, and converts the addresses to big endian afterward. Fixes zephyrproject-rtos#78490 Signed-off-by: Lorenz Clijnen <[email protected]>
lorc-dev
force-pushed
the
issues/set-src-addr-before-decipher
branch
from
25d8f0c
to
47b28eb
Compare
There was a problem hiding this comment.
Thanks for this PR. Nothing wrong with it from a functionality pov but I'd like to propose that we also improve decoupling of abstraction layers - see the inline review comment.
I started a PR before you pushed yours. If you don't mind I'd like to propose it as an alternative solution for you to review...
Do you agree with this?
| @@ -431,19 +421,20 @@ static enum net_verdict ieee802154_recv(struct net_if *iface, struct net_pkt *pk | |||
| return NET_OK; | |||
| } | |||
|
|
|||
| set_pkt_ll_addr(net_pkt_lladdr_src(pkt), !fs->fc.pan_id_comp, fs->fc.src_addr_mode, | |||
There was a problem hiding this comment.
While this definitely works, I think we should rather fix the L2/L3 encapsulation issue that caused this bug to slip through in the first place: IMO it would be better if we removed access to net_pkt_lladdr_src(pkt) (upper layers access to the LL addr) in ieee802154_decipher_data_frame() (L2) as it doesn't make sense to let an L2 function depend on mangled upper layer fields. I'd also prefer if fields being documented as "big endian" would not contain "little endian" data even temporarily. This has bitten us so many times already.
This means that setting and mangling the address fields for the upper layer packet should continue to be done last, i.e. just before we pass the packet on to L3.
(That being said: We'll still need access to the packet in ieee802154_decipher_data_frame() for the moment being to calculate L2 header length. But that can be removed in the future as well.)
Sounds good, I will close this PR |
ieee802154_decipher_data_frame() called from ieee802154_recv() always failed when checking the length of the src address, as it was not yet set.
This sets the src and dst address of the packet before calling the decipher function, and converts the addresses to big endian afterward.
Fixes #78490