Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gnrc_sixlowpan_iphc: add support for IPv6 extension header compression #12879

Merged
merged 5 commits into from
Jan 31, 2020

Conversation

miri64
Copy link
Member

@miri64 miri64 commented Dec 5, 2019

Contribution description

Up until now there was still a part of RFC 6282 missing: IPv6 extension header compression. This PR completes IPHC according to RFC 6282 by adding both compression and decompression for extension headers and encapsulated IPv6 headers (which is part of IPv6 extension header compression in RFC 6282).

Testing procedure

Up until now i only confirmed that the current functionality is not regressed by using tests/gnrc_udp on two iotlab-m3s and sending:

  • unfragmented packets without NHC:
    ping6 <link-local addres of 2nd node>
    
  • fragmented packets without NHC:
    ping6 -s 160 <link-local addres of 2nd node>
    
  • unfragmented packets with NHC
    // start server on 2nd node with `udp server start 61616`
    udp send <link-local addres of 2nd node> 61616 8
    
  • fragmented packets with NHC
    // start server on 2nd node with `udp server start 61616`
    udp send <link-local addres of 2nd node> 61616 160
    

Additionally I tested the use case that caused me implementing this: sending IPv6 fragmented packets compressed:

ping6 -s 1500 <link-local addres of 2nd node>
// start server on 2nd node with `udp server start 61616`
udp send <link-local addres of 2nd node> 61616 1500

And confirmed with Wireshark, that the packets are sensible (as far as I can judge).

I plan to provide a dedicated test application utilizing socket_zep and scapy to have more coverage, but that I will do if I find the time for that (this is why this PR is still WIP). (no NHC support for scapy yet)

Issues/PRs references

None technically, however I cherry-picked be7b6d3 out of #12629 to avoid conflicts between this PR and the clean-ups in #12629.

@miri64 miri64 added Area: network Area: Networking State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet Type: new feature The issue requests / The PR implemements a new feature for RIOT labels Dec 5, 2019
@miri64 miri64 requested a review from cgundogan December 5, 2019 14:07
@miri64
Copy link
Member Author

miri64 commented Dec 5, 2019

And confirmed with Wireshark, that the packets are sensible (as far as I can judge).

Wireshark does not work well with IPHC and IPv6 fragmentation apparently (and I had to handle that corner case in a very hacky way as well): As with UDP NHC the length field is elided (with encapsulated IPv6 headers as well), we have no way within NHC to reconstruct it if it is in a fragmented IPv6 packet (as it is inferred from the IPv6 packet's length minus the preceding extension headers, which is however with IPv6 fragmentation just the length of the first fragment). We only can update the length field after we reassembled the fragmented IPv6 packet so within the IPv6 extension header module, which is what I did.

@miri64 miri64 force-pushed the gnrc_sixlowpan_iphc/feat/nhc-ext branch from 22a4045 to 9a27cc8 Compare December 10, 2019 14:54
@miri64
Copy link
Member Author

miri64 commented Dec 10, 2019

Rebased to current master as the merging #12629 caused some conflicts after all :-/

@miri64 miri64 removed the State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet label Dec 17, 2019
@miri64
Copy link
Member Author

miri64 commented Dec 17, 2019

Sadly, scapy has no NHC support yet either, so I updated the OP and removed the WIP label.

@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Dec 20, 2019
@miri64
Copy link
Member Author

miri64 commented Jan 21, 2020

Fixed issues pointed out by Murdock.

@benpicco
Copy link
Contributor

Code looks good. I get this is only for fragmented next headers right? So no change in L2-PDU is to be expected.

A node samr21-xpro node with the code from this PR is still able to send and receive fragmented IPv6 packets to a node that doesn't have this patch.

Feel free to squash the fix.

@miri64
Copy link
Member Author

miri64 commented Jan 30, 2020

Code looks good. I get this is only for fragmented next headers right? So no change in L2-PDU is to be expected.

No for fragment next header. So when the IPv6 packet is fragmented via the fragment header.

@miri64
Copy link
Member Author

miri64 commented Jan 30, 2020

(I'm using this code already relatively successfully on in my experiments for a few days btw)

@miri64 miri64 force-pushed the gnrc_sixlowpan_iphc/feat/nhc-ext branch from d34fecd to 0300fb4 Compare January 30, 2020 17:00
@miri64
Copy link
Member Author

miri64 commented Jan 30, 2020

Squashed

Copy link
Contributor

@benpicco benpicco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good, no regressions when sending fragmented packets to old RIOT nodes or Linux nodes.

@miri64
Copy link
Member Author

miri64 commented Jan 30, 2020

Code looks good, no regressions when sending fragmented packets to old RIOT nodes or Linux nodes.

Please clarify what you mean by "fragmented" packets in a 6Lo+IPv6 fragmentation context. Did you send only 6LoWPAN-fragmented packets (packet size <=1280) or also IPv6-fragmented packets (>1280; which in turn are 6LoWPAN-fragmented packets)

@benpicco
Copy link
Contributor

Did you send only 6LoWPAN-fragmented packets (packet size <=1280) or also IPv6-fragmented packets (>1280; which in turn are 6LoWPAN-fragmented packets)

I wasn't even aware I could send packets > 1280 bytes. I only tried 6LoWPAN fragmented packets.

@miri64
Copy link
Member Author

miri64 commented Jan 31, 2020

Did you send only 6LoWPAN-fragmented packets (packet size <=1280) or also IPv6-fragmented packets (>1280; which in turn are 6LoWPAN-fragmented packets)

I wasn't even aware I could send packets > 1280 bytes. I only tried 6LoWPAN fragmented packets.

You helped make it possible (#11623, #11596) ;-). This PR adds compression for the IPv6 extension headers (including the fragment header), so using IPv6 fragmentation for testing is important, as lined out in the testing procedures for this PR.

@miri64
Copy link
Member Author

miri64 commented Jan 31, 2020

However, since I use this for days now in my setup

(I'm using this code already relatively successfully on in my experiments for a few days btw)

I think we can merge anyway, what do you think @benpicco?

@benpicco benpicco merged commit c7b0483 into RIOT-OS:master Jan 31, 2020
@miri64 miri64 deleted the gnrc_sixlowpan_iphc/feat/nhc-ext branch January 31, 2020 12:03
@miri64 miri64 restored the gnrc_sixlowpan_iphc/feat/nhc-ext branch January 31, 2020 12:03
@miri64 miri64 deleted the gnrc_sixlowpan_iphc/feat/nhc-ext branch January 31, 2020 12:03
@miri64 miri64 restored the gnrc_sixlowpan_iphc/feat/nhc-ext branch January 31, 2020 12:03
@miri64 miri64 deleted the gnrc_sixlowpan_iphc/feat/nhc-ext branch February 4, 2020 13:56
@leandrolanzieri leandrolanzieri added this to the Release 2020.04 milestone Feb 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: network Area: Networking CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants