Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions Makefile.dep
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,11 @@ ifneq (,$(filter gnrc_rpl_p2p,$(USEMODULE)))
USEMODULE += gnrc_rpl
endif

ifneq (,$(filter gnrc_rpl_hop,$(USEMODULE)))
USEMODULE += gnrc_rpl
USEMODULE += gnrc_ipv6_ext
endif

ifneq (,$(filter gnrc_rpl,$(USEMODULE)))
USEMODULE += gnrc_icmpv6
USEMODULE += gnrc_ipv6_nib
Expand Down
2 changes: 1 addition & 1 deletion examples/gnrc_networking/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ USEMODULE += ps
USEMODULE += netstats_l2
USEMODULE += netstats_ipv6
USEMODULE += netstats_rpl

USEMODULE += gnrc_rpl_hop
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

please adjust the dependencies for gnrc_rpl_hop to include gnrc_ipv6_ext automagically

# Comment this out to disable code in RIOT that does safety checking
# which is not needed in a production environment but helps in the
# development process:
Expand Down
11 changes: 6 additions & 5 deletions sys/include/net/gnrc/ipv6.h
Original file line number Diff line number Diff line change
Expand Up @@ -145,13 +145,14 @@ kernel_pid_t gnrc_ipv6_init(void);
* This situation may happen when the packet has a source routing extension
* header (RFC 6554), and the packet is forwarded from an interface to another.
*
* @param[in] netif The receiving interface.
* @param[in] current A snip to process.
* @param[in] pkt A packet.
* @param[in] nh A protocol number (see @ref net_protnum) of the current snip.
* @param[in] netif The receiving interface.
* @param[in] current A snip to process.
* @param[in] pkt A packet.
* @param[in] nh A protocol number (see @ref net_protnum) of the current snip.
* @param[in] is_for_me Indicates if we are the destination of the packet.
*/
void gnrc_ipv6_demux(gnrc_netif_t *netif, gnrc_pktsnip_t *current,
gnrc_pktsnip_t *pkt, uint8_t nh);
gnrc_pktsnip_t *pkt, uint8_t nh, bool is_for_me);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I have some refactoring work regarding extension headers laying around (not even pushed yet) that would make this additional parameter most likely unnecessary. This would however throw back this PR again... So I think it makes more sense to merge this first and then clean it up with my refactoring ^^.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There is a lot of weird stuff happening with the extension headers at the moment that can be implemented much more straight forward, that is what the refactoring is for ;-)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It would be great to merge this aged PR finally.
This one (or equivalent) is required to start implementing Non-Stroing MOP for RPL.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I know :-/, but I also finally wanted to clean-up IPv6 reception. I can help you to adapt it to the refactoring!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

(Then someone else of course needs to help reviewing)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah, I'm convinced that it doesn't make sense to merge this PR now.
There's no use to have this extension header handling in, since now it would be more an obstacle then helpful.
I will wait and refactor it to the coming changes :)


/**
* @brief Get the IPv6 header from a given list of @ref gnrc_pktsnip_t
Expand Down
4 changes: 3 additions & 1 deletion sys/include/net/gnrc/ipv6/ext.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,13 @@ extern "C" {
* @param[in] current A snip to process.
* @param[in] pkt A packet.
* @param[in] nh A protocol number (see @ref net_protnum) of the current snip.
* @param[in] is_for_me Indicates if We are the destination of the packet.
*/
void gnrc_ipv6_ext_demux(gnrc_netif_t *netif,
gnrc_pktsnip_t *current,
gnrc_pktsnip_t *pkt,
uint8_t nh);
uint8_t nh,
bool is_for_me);

/**
* @brief Builds an extension header for sending.
Expand Down
105 changes: 105 additions & 0 deletions sys/include/net/gnrc/rpl/hop.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
/*
* Copyright (C) 2017 Martin Landsmann <[email protected]>
*
* This file is subject to the terms and conditions of the GNU Lesser
* General Public License v2.1. See the file LICENSE in the top level
* directory for more details.
*/

/**
* @defgroup net_gnrc_rpl_hop RPL Hop-by-Hop header extension
* @ingroup net_gnrc_rpl
* @brief Implementation of RPL Hop-by-Hop extension headers
* @see <a href="https://tools.ietf.org/html/rfc6553">
* RFC 6553
* </a>
* @{
*
* @file
* @brief Definitions for Carrying RPL Information in Data-Plane Datagrams
*
* @author Martin Landsmann <[email protected]>
*/
#ifndef NET_GNRC_RPL_HOP_H
#define NET_GNRC_RPL_HOP_H

#include "net/ipv6/hdr.h"
#include "net/ipv6/addr.h"

#ifdef __cplusplus
extern "C" {
#endif

/**
* @brief Type of the Hop-by-Hop Option.
*
* @see <a href="https://tools.ietf.org/html/rfc6553#section-6">
* RFC6553, section 6
* </a>
*/
#define GNRC_RPL_HOP_OPT_TYPE (0x63)

/**
* @name Return types used in processing a hop-by-hop option
*
* @{
*/
#define HOP_OPT_SUCCESS (0)
#define HOP_OPT_ERR_HEADER_LENGTH (-1)
#define HOP_OPT_ERR_INCONSISTENCY (-2)
#define HOP_OPT_ERR_FLAG_R_SET (-3)
#define HOP_OPT_ERR_FLAG_F_SET (-4)
#define HOP_OPT_ERR_NOT_FOR_ME (-5)
/** @} */

/**
* @name Option flags
* @see <a href="https://tools.ietf.org/html/rfc6553#section-3">
* RFC6553, section 3
* </a>
* @{
*/
#define GNRC_RPL_HOP_OPT_FLAG_O (1 << 0)
#define GNRC_RPL_HOP_OPT_FLAG_R (1 << 1)
#define GNRC_RPL_HOP_OPT_FLAG_F (1 << 2)
/** @} */

/**
* @brief The RPL Hop-by-Hop header extension.
*
* @see <a href="https://tools.ietf.org/html/rfc6553#section-3">
* RFC 6553
* </a>
*/
typedef struct __attribute__((packed)) {
uint8_t nh; /**< the option type of the next extension */
uint8_t hbh_len; /**< the hbh length without the first octet */
uint8_t type; /**< option identifier (0x63) */
uint8_t len; /**< length in 8 octets without first octet */
uint8_t ORF_flags; /**< O|R|F flags followed by 5 unused bits */
uint8_t instance_id; /**< id of the instance */
uint16_t sender_rank; /**< rank of the sender */
} gnrc_rpl_hop_opt_t;


/**
* @brief parse the given hop-by-hop option, check for inconsistencies,
* adjust the option for further processing and return the result.
*
* @param[in,out] hop pointer to the hop-by-hop header option
*
* @returns HOP_OPT_SUCCESS on success
* HOP_OPT_ERR_HEADER_LENGTH if there was a header length mismatch
* HOP_OPT_ERR_INCONSISTENCY if the F flag was already set
* HOP_OPT_ERR_FLAG_R_SET if we set the R flag
* HOP_OPT_ERR_FLAG_F_SET if we set the F flag
* HOP_OPT_ERR_NOT_FOR_ME if the content is not for us
*/
int gnrc_rpl_hop_opt_process(gnrc_rpl_hop_opt_t *hop);

#ifdef __cplusplus
}
#endif

#endif /* NET_GNRC_RPL_HOP_H */
/** @} */
15 changes: 15 additions & 0 deletions sys/include/net/ipv6/ext.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
#include <stdint.h>

#include "net/ipv6/ext/rh.h"
#include "net/gnrc/netif.h"
#include "net/gnrc/pkt.h"

#ifdef __cplusplus
extern "C" {
Expand All @@ -31,6 +33,19 @@ extern "C" {
#define IPV6_EXT_LEN_UNIT (8U) /**< Unit in byte for the extension header's
* length field */

/** @brief Message type to reply IPv6 that a next header has been processed */
#define GNRC_IPV6_EXT_HANDLE_NEXT_HDR (0x0399)

/**
* @brief Message content container to process extension headers sequentially.
*/
typedef struct {
gnrc_netif_t *netif; /**< The interface we received the packet */
gnrc_pktsnip_t *current; /**< The snip of the IP6 header */
gnrc_pktsnip_t *next_hdr; /**< The next header to be processed */
uint8_t nh_type; /**< The type of the next header */
} gnrc_ipv6_ext_hdr_handle_t;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this however by its very name should go into GNRC not net/ipv6/ext.h (which is outside the GNRC realm).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

(not just the name, also the fact that you are using GNRC-specific types here)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Don't know, its purpose is only to enable the sequential iteration of extension headers for IPv6 packets.
Although the structure uses gnrc-types I would expect to find such helper in this extension specific headerfile.
Probably I can just remove the prefix gnrc_ from the typename to avoid confusion.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@BytesGalore before you do anything, have a look at my refactoring work first of the past few weeks. It also should very much simplify the handling of the hop-by-hop-header, as it is now treated separately (which we can do, because they always MUST come first according to RFC8200; see #10244). You should now be able to just hook into the according switch-branch with gnrc_ipv6_ext_demux() for that handling.


/**
* @brief IPv6 extension headers.
*
Expand Down
12 changes: 12 additions & 0 deletions sys/include/net/netopt.h
Original file line number Diff line number Diff line change
Expand Up @@ -553,6 +553,18 @@ typedef enum {
* incoming frames until unset.
*/
NETOPT_PHY_BUSY,
/**
* @brief Type for sequiential processing of IPv6 extension headers
*
* On GET it requests a thread to provide an extension header
* and prepends it to the given payload of the given (IPv6) packet.
* On SET it always returns -ENOTSUP.
*
* When the header is done the thread replies to the call.
* The result is the pktsnip_t with the extension header
* The result is NULL if no extension header is passed.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You hand over a pointer (not a pointer of a pointer) below, so this should be impossible. Why not use the numeric return-value of NETAPI_GET (which is returned by gnrc_netapi_get())?, e.g. sizeof(gnrc_pktsnip_t) on success, -ENOMEM (or whatever is applicable) on error.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

not a pointer of a pointer

true, it would require some wrapping.
You mean exploiting the int return type for pointer passing, why not.

*/
NETOPT_IPV6_EXT_HDR,

/* add more options if needed */

Expand Down
3 changes: 3 additions & 0 deletions sys/net/gnrc/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,9 @@ endif
ifneq (,$(filter gnrc_rpl_p2p,$(USEMODULE)))
DIRS += routing/rpl/p2p
endif
ifneq (,$(filter gnrc_rpl_hop,$(USEMODULE)))
DIRS += routing/rpl/hop
endif
ifneq (,$(filter gnrc_sixlowpan,$(USEMODULE)))
DIRS += network_layer/sixlowpan
endif
Expand Down
27 changes: 24 additions & 3 deletions sys/net/gnrc/network_layer/ipv6/ext/gnrc_ipv6_ext.c
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,8 @@ static inline bool _has_valid_size(gnrc_pktsnip_t *pkt, uint8_t nh)
void gnrc_ipv6_ext_demux(gnrc_netif_t *netif,
gnrc_pktsnip_t *current,
gnrc_pktsnip_t *pkt,
uint8_t nh)
uint8_t nh,
bool is_for_me)
{
ipv6_ext_t *ext;

Expand All @@ -195,7 +196,7 @@ void gnrc_ipv6_ext_demux(gnrc_netif_t *netif,
return;
}

gnrc_ipv6_demux(netif, current, pkt, nh); /* demultiplex next header */
gnrc_ipv6_demux(netif, current, pkt, nh, is_for_me); /* demultiplex next header */

return;

Expand All @@ -212,6 +213,26 @@ void gnrc_ipv6_ext_demux(gnrc_netif_t *netif,
#endif /* MODULE_GNRC_IPV6_EXT_RH */

case PROTNUM_IPV6_EXT_HOPOPT:
/* if current != pkt, size is already checked */
if (current == pkt && !_has_valid_size(pkt, nh)) {
DEBUG("ipv6_ext: invalid size\n");
gnrc_pktbuf_release(pkt);
return;
}

nh = ext->nh;
DEBUG("ipv6_ext: next header = %" PRIu8 "\n", nh);

if ((current = _mark_extension_header(current, &pkt)) == NULL) {
return;
}

gnrc_pktbuf_hold(pkt, 1); /* don't release on next dispatch */
if (gnrc_netapi_dispatch_receive(GNRC_NETTYPE_IPV6, nh, pkt) == 0) {
gnrc_pktbuf_release(pkt);
}

break;
case PROTNUM_IPV6_EXT_DST:
case PROTNUM_IPV6_EXT_FRAG:
case PROTNUM_IPV6_EXT_AH:
Expand Down Expand Up @@ -241,7 +262,7 @@ void gnrc_ipv6_ext_demux(gnrc_netif_t *netif,
break;

default:
gnrc_ipv6_demux(netif, current, pkt, nh); /* demultiplex next header */
gnrc_ipv6_demux(netif, current, pkt, nh, is_for_me); /* demultiplex next header */
return;
}
}
Expand Down
Loading