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_icmpv6_error: initial import #3184

Merged
merged 1 commit into from
Jan 26, 2016
Merged
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
4 changes: 4 additions & 0 deletions Makefile.dep
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,10 @@ ifneq (,$(filter gnrc_icmpv6_echo,$(USEMODULE)))
USEMODULE += gnrc_icmpv6
endif

ifneq (,$(filter gnrc_icmpv6_error,$(USEMODULE)))
USEMODULE += gnrc_icmpv6
endif

ifneq (,$(filter gnrc_icmpv6,$(USEMODULE)))
USEMODULE += inet_csum
USEMODULE += gnrc_ipv6
Expand Down
54 changes: 52 additions & 2 deletions sys/include/net/gnrc/icmpv6/error.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,66 @@
* @brief ICMPv6 error message definitions
*
* @author Martine Lenders <[email protected]>
*
* @todo implement build and handle functions
*/
#ifndef GNRC_ICMPV6_ERROR_H_
#define GNRC_ICMPV6_ERROR_H_

#include <stdint.h>

#include "net/icmpv6.h"
#include "net/ipv6/hdr.h"
#include "net/gnrc/pkt.h"

#ifdef __cplusplus
extern "C" {
#endif

/**
* @brief Builds an ICMPv6 destination unreachable message for sending.
*
* @param[in] code The code for the message @see net/icmpv6.h.
* @param[in] orig_pkt The invoking packet.
*
* @return The destination unreachable message on success.
* @return NULL, on failure.
*/
gnrc_pktsnip_t *gnrc_icmpv6_error_dst_unr_build(uint8_t code, gnrc_pktsnip_t *orig_pkt);

/**
* @brief Builds an ICMPv6 packet too big message for sending.
*
* @param[in] mtu The maximum transission unit of the next-hop link.
* @param[in] orig_pkt The invoking packet.
*
* @return The packet too big message on success.
* @return NULL, on failure.
*/
gnrc_pktsnip_t *gnrc_icmpv6_error_pkt_too_big_build(uint32_t mtu, gnrc_pktsnip_t *orig_pkt);

/**
* @brief Builds an ICMPv6 time exceeded message for sending.
*
* @param[in] code The code for the message @see net/icmpv6.h.
* @param[in] orig_pkt The invoking packet.
*
* @return The time exceeded message on success.
* @return NULL, on failure.
*/
gnrc_pktsnip_t *gnrc_icmpv6_error_time_exc_build(uint8_t code, gnrc_pktsnip_t *orig_pkt);

/**
* @brief Builds an ICMPv6 parameter problem message for sending.
*
* @param[in] code The code for the message @see net/icmpv6.h.
* @param[in] ptr Pointer to the errorneous octet in @p orig_pkt.
* @param[in] orig_pkt The invoking packet.
*
* @return The parameter problem message on success.
* @return NULL, on failure.
*/
gnrc_pktsnip_t *gnrc_icmpv6_error_param_prob_build(uint8_t code, void *ptr,
gnrc_pktsnip_t *orig_pkt);

#ifdef __cplusplus
}
#endif
Expand Down
2 changes: 1 addition & 1 deletion sys/include/net/icmpv6.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ extern "C" {
* IANA, ICMPv6 "type" Numbers
* </a>
*/
#define ICMPV6_DEST_UNR (1) /**< Destination unreachable message */
#define ICMPV6_DST_UNR (1) /**< Destination unreachable message */
#define ICMPV6_PKT_TOO_BIG (2) /**< Packet Too Big message */
#define ICMPV6_TIME_EXC (3) /**< Time Exceeded message */
#define ICMPV6_PARAM_PROB (4) /**< Parameter Problem message */
Expand Down
3 changes: 3 additions & 0 deletions sys/net/gnrc/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ endif
ifneq (,$(filter gnrc_icmpv6_echo,$(USEMODULE)))
DIRS += network_layer/icmpv6/echo
endif
ifneq (,$(filter gnrc_icmpv6_error,$(USEMODULE)))
DIRS += network_layer/icmpv6/error
endif
ifneq (,$(filter gnrc_ipv6,$(USEMODULE)))
DIRS += network_layer/ipv6
endif
Expand Down
3 changes: 3 additions & 0 deletions sys/net/gnrc/network_layer/icmpv6/error/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
MODULE = gnrc_icmpv6_error

include $(RIOTBASE)/Makefile.base
122 changes: 122 additions & 0 deletions sys/net/gnrc/network_layer/icmpv6/error/gnrc_icmpv6_error.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
/*
* Copyright (C) 2015 Martine Lenders <[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.
*/

/**
* @{
*
* @file
*/

#include "net/gnrc/pktbuf.h"

#include "net/ipv6.h"
#include "net/gnrc/ipv6/netif.h"
#include "net/gnrc/icmpv6/error.h"
#include "net/gnrc/icmpv6.h"

/* all error messages are basically the same size and format */
#define ICMPV6_ERROR_SZ (sizeof(icmpv6_error_dst_unr_t))
#define ICMPV6_ERROR_SET_VALUE(data, value) \
((icmpv6_error_pkt_too_big_t *)(data))->mtu = byteorder_htonl(value)

/* TODO: generalize and centralize (see https://github.com/RIOT-OS/RIOT/pull/3184) */
#define MIN(a, b) ((a) < (b)) ? (a) : (b)

static inline size_t _fit(gnrc_pktsnip_t *pkt)
{
/* TODO: replace IPV6_MIN_MTU with known path MTU? */
return MIN((gnrc_pkt_len(pkt) + ICMPV6_ERROR_SZ), IPV6_MIN_MTU);
}

/* Build a generic error message */
static gnrc_pktsnip_t *_icmpv6_error_build(uint8_t type, uint8_t code,
gnrc_pktsnip_t *orig_pkt, uint32_t value)
{
gnrc_pktsnip_t *pkt = gnrc_icmpv6_build(NULL, type, code, _fit(orig_pkt));
Copy link
Member

Choose a reason for hiding this comment

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

Why can one not call this function with orig_pkt instead of NULL directly and save the while loop?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because orig_pkt must be copied into the packet in case it + the ICMPv6 error header + the IPv6 header does not fit the MTU.

Copy link
Member Author

Choose a reason for hiding this comment

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

Also, orig_pkt might still be in use by someone else.

Copy link
Member Author

Choose a reason for hiding this comment

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

(which could be circumvented by gnrc_pktbuf_hold() on orig_pkt, but regardless, my first point is still valid)

Copy link
Member

Choose a reason for hiding this comment

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

Interesting. Is this designed to work like this (copy as much of the packet into the error messages as possible)?

Copy link
Member Author

Choose a reason for hiding this comment

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

What is designed to work like what?

Copy link
Member

Choose a reason for hiding this comment

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

ICMPv6 error messages.

Copy link
Member Author

Choose a reason for hiding this comment

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


/* copy as much of the originating packet into error message as fits the message's size */
if (pkt != NULL) {
size_t offset = ICMPV6_ERROR_SZ;
uint8_t *data = pkt->data;
ICMPV6_ERROR_SET_VALUE(data, value);
while ((orig_pkt != NULL) && (offset < pkt->size)) {
memcpy(data + offset, orig_pkt->data,
MIN(pkt->size - offset, orig_pkt->size));
offset += MIN(pkt->size - offset, orig_pkt->size);
orig_pkt = orig_pkt->next;
}
}

return pkt;
}

gnrc_pktsnip_t *gnrc_icmpv6_error_dst_unr_build(uint8_t code, gnrc_pktsnip_t *orig_pkt)
{
return _icmpv6_error_build(ICMPV6_DST_UNR, code, orig_pkt, 0);
}

gnrc_pktsnip_t *gnrc_icmpv6_error_pkt_too_big_build(uint32_t mtu, gnrc_pktsnip_t *orig_pkt)
{
return _icmpv6_error_build(ICMPV6_PKT_TOO_BIG, 0, orig_pkt, mtu);
}

gnrc_pktsnip_t *gnrc_icmpv6_error_time_exc_build(uint8_t code, gnrc_pktsnip_t *orig_pkt)
{
return _icmpv6_error_build(ICMPV6_TIME_EXC, code, orig_pkt, 0);
}
Copy link
Member

Choose a reason for hiding this comment

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

What's the purpose of these wrapper functions? Don't seem to shorten anything.

Copy link
Member Author

Choose a reason for hiding this comment

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

They are the API functions.

Copy link
Member Author

Choose a reason for hiding this comment

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

All other ICMPv6 messages have their own build functions, too. So it would be inconsistent to have only two (these three in one and the param_prob one) for the errors. Especially since the 4th parameter of _icmpv6_error_build() is for the most cases just used to set the reserved field.

Copy link
Member

Choose a reason for hiding this comment

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

All other ICMPv6 messages have their own build functions, too

With #3693 not any more. ;)

I think providing _icmpv6_error_build() as an API function is sufficient.

Copy link
Member Author

Choose a reason for hiding this comment

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

The case of gnrc_icmpv6_echo_build() is a totally different one as this one. A generic gnrc_icmpv6_error_build() would expose reserved fields (which have to be 0) to the user.

Copy link
Member Author

Choose a reason for hiding this comment

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

Also how would you understandably document the fourth parameter?

Sets the x-th offseted word in the packet. Needs to be 0 for ICMPV6_DST_UNR and ICMPV6_TIME_EXC and the MTU of the interface for ICMPV6_PKT_TOO_BIG.

?

Copy link
Member

Choose a reason for hiding this comment

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

Which "user" needs to send ICMPv6 echo packets?

Copy link
Member Author

Choose a reason for hiding this comment

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

Me e.g. (and these are echo packages). Let's discuss this on Monday f2f.

Copy link
Member

Choose a reason for hiding this comment

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

any result of this discussion?

Copy link
Member Author

Choose a reason for hiding this comment

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

#3693 was merged, but I still argue that it doesn't make sense here. The different message types have different default values (some need code to be 0, some need the "value" field [which is not existent in any of the packet types] to be 0 [when it is the unused-field]) and param_prob even expects a pointer inside orig_pkt making it the odd one out and an API that would be like @OlegHahm proposed inconsistent.


static inline bool _in_range(uint8_t *ptr, uint8_t *start, size_t sz)
{
return (ptr >= start) && (ptr < (start + sz));
}

gnrc_pktsnip_t *gnrc_icmpv6_error_param_prob_build(uint8_t code, void *ptr,
gnrc_pktsnip_t *orig_pkt)
{
gnrc_pktsnip_t *pkt = gnrc_icmpv6_build(NULL, ICMPV6_PARAM_PROB, code,
_fit(orig_pkt));

/* copy as much of the originating packet into error message and
* determine relative *ptr* offset */
if (pkt != NULL) {
size_t offset = sizeof(icmpv6_error_param_prob_t);
uint8_t *data = pkt->data;
uint32_t ptr_offset = 0U;
bool found_offset = false;

while (orig_pkt != NULL) {
/* copy as long as it fits into packet */
if (offset < pkt->size) {
memcpy(data + offset, orig_pkt->data,
MIN(pkt->size - offset, orig_pkt->size));
offset += MIN(pkt->size - offset, orig_pkt->size);
}

if (_in_range(ptr, orig_pkt->data, orig_pkt->size)) {
ptr_offset += (uint32_t)(((uint8_t *)ptr) - ((uint8_t *)orig_pkt->data));
found_offset = true;
}
else if (!found_offset) {
ptr_offset += (uint32_t)orig_pkt->size;
}

orig_pkt = orig_pkt->next;

if ((offset < pkt->size) && found_offset) {
break;
}
}

/* set "pointer" field to relative pointer offset */
((icmpv6_error_param_prob_t *)data)->ptr = byteorder_htonl(ptr_offset);
}

return pkt;
}

/** @} */