gnrc_ipv6: fix UDP issue when a packet passes through another node#5596
gnrc_ipv6: fix UDP issue when a packet passes through another node#5596mattiantonini wants to merge 1 commit intoRIOT-OS:masterfrom
Conversation
| ipv6 = gnrc_pktbuf_mark(pkt, sizeof(ipv6_hdr_t), GNRC_NETTYPE_IPV6); | ||
|
|
||
| first_ext = pkt; | ||
| pkt->type = GNRC_NETTYPE_UNDEF; /* snip is no longer IPv6 */ |
There was a problem hiding this comment.
@Yonezawa-T2 this line is your's. I'm not sure why it is here, but I'm also not sure how it is related to the rest of the fix.
There was a problem hiding this comment.
The 6LoWPAN module creates packet like this:
then the IPv6 module marks IPv6 header:
but pkt no longer contains IPv6 header, so that we have to change its type to GNRC_NETTYPE_UNDEF.
There was a problem hiding this comment.
I checked the implementation of gnrc_pkt_len_upto (here, line 147).
The sum is made before the if(pkt->type == type), so what I see is if type is GNRC_NETTYPE_UNDEF, it will count the payload plus ipv6 hdr.
So, the pkt->type = GNRC_NETTYPE_UNDEF line should be after the call to gnrc_pkt_len_upto
|
@mattiantonini please always prefix commit titles with the module's name that is concerned: |
|
You can do that with git commit --amend
git push -fin your branch. The rest happens automatically. |
be77ac1 to
6c09442
Compare
|
I've changed the commit title, but for the wild whitespace I've to do a new commit, right? I'm sorry for my ignorance :) |
Yep, changes to the patch are made as fix up commits (e. g. with
No problem, we've all been there ;) |
removed wild whitespace
d16e89c to
afe48d3
Compare
|
So is this PR solving the problem? |
|
If I'm not wrong: This file in current master branch assigns the pkt type to GNRC_NETTYPE_UNDEF (line 855): and then call the gnrc_pkt_len_upto (line 889):
So, basically the gnrc_pkt_len_upto will count the current packet (with GNRC_NETTYPE_UNDEF) and the next packet (GNRC_NETTYPE_IPV6). This is ipv6_hdr + ipv6_payload. So, the final result of What I see if removing the GNRC_NETTYPE_UNDEF set, and removing the "- sizeof(ipv6_hdr_t)", the result should be the same. Only the first packet will be count if its type is GNRC_NETTYPE_IPv6. Am I missing something? What do you think? |
|
The 6LoWPAN module creates packet like this:
while other modules creates packet like this:
This is correct because marking a packet as a IPv6 packet is responsibility of the IPv6 module in general. Only the 6LoWPAN module is special. So this PR will not work in non-6LoWPAN setup. |
|
Ok let's discuss this for the next release. |
|
Why is it closed? The problem is gone? |
|
@kYc0o I opened a new PR with an update version |
|
Great! Thanks! :) |
The issue was related to the marking of non-valid IPv6 pck and to payload length. The IPv6 Payload length contains just the length of the payload (eg. UDP header+UDP Payload) without the (fixed) ipv6 header length.