gnrc_ipv6_nib: fix preparation of delayed NA #10985
Conversation
miri64
added
Type: bug
miri64
added
the
Process: needs backport
Currently the constructed NA for a delayed NA case is neither used nor released nor does it get an IPv6 header to be used properly. This fixes that case.
miri64
force-pushed
the
gnrc_ipv6_nib/fix/delayed-na
branch
from
0b3bbdd to
aa953a4
Compare
|
Rebased to current master to ease testing. |
|
@aabadie do you still have leaks with this PR? |
|
If yes, please provide a dump of the packet buffer... I can't get any leaks anymore. |
|
@miri64, tested several times and no more leaks! Good job! Codewise I can't tell if the changes are good or not. Maybe @cgundogan can tell ? |
| if ((payload = _check_release_pkt(pkt, payload)) == NULL) { | ||
| return; | ||
| } | ||
| ((ipv6_hdr_t *)payload->data)->hl = 255; |
There was a problem hiding this comment.
why 255 and not the configured default for the interface?
There was a problem hiding this comment.
Because NAs must have their hop-limit set to 255 to ensure they are link-local. NAs with any other hop-limit value are to be ignored silently.
There was a problem hiding this comment.
Maybe this value could be provided as a define with some documentation ?
There was a problem hiding this comment.
Sure, but since I use this magic number also in gnrc_ndp_*_send() I'd rather do this in a follow-up, so I don't need to touch these functions in a PR that needs to be backported.
There was a problem hiding this comment.
Sure, but since I use this magic number also in
gnrc_ndp_*_send()I'd rather do this in a follow-up, so I don't need to touch these functions in a PR that needs to be backported.
👍
cgundogan
added
Reviewed: 1-fundamentals
|
Backport provided in #11009 |
|
I think that concludes at least my part of release critical bugs that needed backporting. Let me know, if you need anything else for RC3. |
Contribution description
Currently the constructed NA for a delayed NA case is neither used nor released nor does it get an IPv6 header to be used properly. This fixes that case.
Testing procedure
Since this is currently an edge-case we shouldn't really enter ATM, I fear you have to read and understand the code ... But the test case proposed by @aabadie proposed in RIOT-OS/Release-Specs#98 (comment) might be one to test if together with #10978 this reduces (or even eliminates) the leaks in the packet buffer.
Issues/PRs references
I'm still not sure this fixes all leaks (this code shouldn't even be entered since we do not use anycasts addresses at least in the test case these leaks were found), but definitely closes more leaks together with #10978.