gnrc_sixlowpan: Refactoring #8511

miri64 · 2018-02-02T15:03:43Z

Motivation

GNRC's 6LoWPAN has some quality defects in its design, that need to be addressed before @cgundogan can start porting ICNLoWPAN to RIOT. Also I have some issues with certain points in the code that in hindsight I would have done differently.

A model for the 6LoWPAN adaption layer

In a try to solve both problems in my motivation in one go I came up with a model for the 6LoWPAN adaption layer that drafts the different components of 6LoWPAN (fragmentation, IPHC, etc.) as layered sub-modules (in the following referred to as 6Lo modules) with a common API. In reviewing all the RFCs currently available on the adaption layer (RFC4944, RFC6282, RFC7400, RFC8025, RFC8138 (and also some drafts)) I identified 8 6Lo modules (which in turn of course can have their own sub-module). In the following the cursive ones are already implemented in RIOT and need to be refactored to this module:

Mesh routing
Fragmentation
Uncompressed IPv6
IPHC (This includes implementation for NHC and GHC (since it plugs into IPHC as an NHC format))
Paging
6LoRH
ICNLoWPAN
GNRC dispatcher: this just delegates the finally processed datagram to the next layer in the network stack

I omitted HC1 since I believe there is no benefit in ever porting it, but if someone would plan to it would also be its own 6Lo layer.

Every 6Lo modules will provide two functions with the following prototype:

void send(gnrc_pktsnip_t *pkt, void *ctx, uint8_t page);
void recv(gnrc_pktsnip_t *pkt, void *ctx, uint8_t page);

For the more visual inclined, here is a UML diagram I threw together:

The context is there so e.g. the compression models can be handed the reassembly buffer so they can de-compress the packet directly in it instead of awkwardly returning some values by having 3 (!) return values to the compression decode function as it is currently the case (i.e. so we don't have to back-trace into a previous 6Lo layer, once the task of the next layer layer did its job).

How to proceed

I think moving over to this module shouldn't be too intrusive as it is pretty simple and most components are already in a shape that is quite similar. I think @cgundogan can divide most of the refactoring tasks among ourselves.

Provide GNRC dispatcher (@miri64, gnrc_sixlowpan: Introduce 6Lo GNRC dispatch sub-layer #8512)
Refactor IPHC / NHC (@miri64, gnrc_sixlowpan_iphc: refactor reception for #8511 #9484 and gnrc_sixlowpan_iphc: refactor sending for #8511 #9485)
Refactor fragmentation (@miri64, gnrc_sixlowpan_frag: adapt for #8511 #9372)
Provide paging module (@cgundogan ?)
Refactor libICNL (@cgundogan, re-use parts of gnrc: generalize gnrc_sixlowpan to enable resuability #8218?)
Provide GHC module (@A-Paul)

I'm going to open a project to track our process on that more dynamically.

This abstracts the sending and receiving of 6Lo packets to the new 6Lo sub-layer model introduced in RIOT-OS#8511 and exemplifies it as well.

miri64 · 2018-03-06T16:35:21Z

While working on untangling IPHC and fragmentation, I might have found a way to have it work without context at least for this use-case.

miri64 · 2018-06-14T14:56:55Z

While working on untangling IPHC and fragmentation, I might have found a way to have it work without context at least for this use-case.

Actually, that isn't the case. What I have to hand down for encoding though, is the datagram-size. I'll ponder about this some more and then adapt this issue, if need be.

This exposes the parts of the reassembly buffer to be usable as context as proposed in RIOT-OS#8511. I only exposed *parts of* for two reasons: 1. I don't need to expose further types (like `rbuf_int_t`), that are not of interest outside of fragmentation. 2. This allows for an easy future extension for the virtual reassembly buffer as proposed in [[1]]. This makes this change a little bit more involved, because instead of just renaming the type, I also need to add the usage of the `super` member, but I think in the end this little preparation work will be beneficial in the future. [1]: https://tools.ietf.org/html/draft-watteyne-6lo-minimal-fragment-01#section-3

miri64 · 2018-06-14T15:41:07Z

In the mean time: the reassembly buffer definitely needs to be exposed (see #9353)

This refactors the `gnrc_sixlowpan_frag` module for the API proposed in RIOT-OS#8511. The `ctx` for `gnrc_sixlowpan_frag_send()` is required to be a `gnrc_sixlowpan_msg_frag_t` object, so IPHC can later on use it to provide the *original* datagram size (otherwise, we would need to adapt the API just for that, which seems to me as convoluted as this proposal). I also provide an expose function with a future possibility to provide more than just one `gnrc_sixlowpan_msg_frag_t` object later on (plus having cleaner module separation in general).

This exposes the parts of the reassembly buffer to be usable as context as proposed in RIOT-OS#8511. I only exposed *parts of* for two reasons: 1. I don't need to expose further types (like `rbuf_int_t`), that are not of interest outside of fragmentation. 2. This allows for an easy future extension for the virtual reassembly buffer as proposed in [[1]]. This makes this change a little bit more involved, because instead of just renaming the type, I also need to add the usage of the `super` member, but I think in the end this little preparation work will be beneficial in the future. [1]: https://tools.ietf.org/html/draft-watteyne-6lo-minimal-fragment-01#section-3

This refactors the `gnrc_sixlowpan_frag` module for the API proposed in RIOT-OS#8511. The `ctx` for `gnrc_sixlowpan_frag_send()` is required to be a `gnrc_sixlowpan_msg_frag_t` object, so IPHC can later on use it to provide the *original* datagram size (otherwise, we would need to adapt the API just for that, which seems to me as convoluted as this proposal). I also provide an expose function with a future possibility to provide more than just one `gnrc_sixlowpan_msg_frag_t` object later on (plus having cleaner module separation in general).

gnrc_sixlowpan_frag: adapt for #8511

This refactors reception/decoding part of `gnrc_sixlowpan_iphc` to the more layered approach modeled in RIOT-OS#8511. Since the reception part is already complicated enough I decided to divide send and receive up into separate changes it into its own change.

This refactors sending/encoding part of `gnrc_sixlowpan_iphc` to the more layered approach modeled in RIOT-OS#8511. Since the reception part is already was pretty complicated to refactor, I decided to divide send and receive up into separate changes.

gnrc_sixlowpan_iphc: refactor sending for #8511

This refactors reception/decoding part of `gnrc_sixlowpan_iphc` to the more layered approach modeled in RIOT-OS#8511. Since the reception part is already complicated enough I decided to divide send and receive up into separate changes.

gnrc_sixlowpan_iphc: refactor reception for #8511

miri64 · 2018-07-26T11:02:36Z

With #9484 merged now this means the basic refactoring work is finished. @cgundogan are you planning to provide a paging module? @A-Paul did you follow the progress of this PR for your work on GHC?