gnrc_ipv6: fix UDP issue when a packet passes through another node

[mattiantonini]

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.

[miri64]

Where is this set now?

[miri64]

@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.

[Yonezawa-T2]

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.

[jia200x]

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

[miri64]

@mattiantonini please always prefix commit titles with the module's name that is concerned: gnrc_ipv6: ...

[miri64]

You can do that with

git commit --amend
git push -f

in your branch. The rest happens automatically.

[mattiantonini]

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 :)

[miri64]

I've changed the commit title, but for the wild whitespace I've to do a new commit, right?

Yep, changes to the patch are made as fix up commits (e. g. with git commit --fixup=HEAD) and are only squashed (using git rebase -i --autosquash origin/master) when the review is finished.

I'm sorry for my ignorance :)

No problem, we've all been there ;)

[kYc0o]

So is this PR solving the problem?

[jia200x]

If I'm not wrong:

This file in current master branch assigns the pkt type to GNRC_NETTYPE_UNDEF (line 855):
pkt->type = GNRC_NETTYPE_UNDEF; /* snip is no longer IPv6 */

and then call the gnrc_pkt_len_upto (line 889):

gnrc_pkt_len_upto(pkt, GNRC_NETTYPE_IPV6) - sizeof(ipv6_hdr_t)

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 gnrc_pkt_len_upto(pkt, GNRC_NETTYPE_IPV6) - sizeof(ipv6_hdr_t)) will be the length of the payload.

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?

[Yonezawa-T2]

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.

[kYc0o]

Ok let's discuss this for the next release.

[kYc0o]

Why is it closed? The problem is gone?

[mattiantonini]

@kYc0o I opened a new PR with an update version

[kYc0o]

Great! Thanks! :)