Skip to content

net: introduce IPv6 netstats #5178

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.

Sign up for GitHub

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

Merged
OlegHahm merged 1 commit into from
May 25, 2016
Merged

net: introduce IPv6 netstats #5178

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,10 @@ ifneq (,$(filter gnrc_netdev2,$(USEMODULE)))
USEMODULE += netopt
endif

ifneq (,$(filter netstats_%, $(USEMODULE)))
USEMODULE += netstats
endif

ifneq (,$(filter pthread,$(USEMODULE)))
USEMODULE += xtimer
USEMODULE += timex
Expand Down
2 changes: 2 additions & 0 deletions Makefile.pseudomodules
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@ PSEUDOMODULES += lwip_udp
PSEUDOMODULES += lwip_udplite
PSEUDOMODULES += netdev_default
PSEUDOMODULES += netif
PSEUDOMODULES += netstats
PSEUDOMODULES += netstats_l2
PSEUDOMODULES += netstats_ipv6
PSEUDOMODULES += newlib
PSEUDOMODULES += newlib_nano
PSEUDOMODULES += pktqueue
Expand Down
2 changes: 2 additions & 0 deletions examples/gnrc_networking/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ USEMODULE += gnrc_icmpv6_echo
USEMODULE += shell
USEMODULE += shell_commands
USEMODULE += ps
USEMODULE += netstats_l2
USEMODULE += netstats_ipv6

@OlegHahm OlegHahm Apr 15, 2016

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 so sure if we really want this - should go into a separate PR.

@cgundogan cgundogan Apr 15, 2016

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 so sure if we really want this - should go into a separate PR.

can you elaborate? I lost the context (:

@OlegHahm OlegHahm Apr 15, 2016

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.

I meant if we really want to have netstats per default in the gnrc_networking example.

@cgundogan cgundogan Apr 15, 2016

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 see. I wouldn't object, because gnrc_networking is IMO for showcasing our features, while gnrc_minimal is for showcasing our minamility.

@cgundogan cgundogan Apr 19, 2016

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.

What about compiling in netstats_ipv6 automatically if netstats and ipv6 are compiled?

@OlegHahm OlegHahm Apr 20, 2016

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.

Hm, what if I'm only interested in L2 stats?

@cgundogan cgundogan Apr 20, 2016

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.

DISABLE_MODULE=netstats_ipv6. But it was just food for thought. I am fine with both variants.


# Comment this out to disable code in RIOT that does safety checking
# which is not needed in a production environment but helps in the
Expand Down
16 changes: 16 additions & 0 deletions sys/include/net/gnrc/ipv6/netif.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include "mutex.h"
#include "net/ipv6.h"
#include "net/ipv6/addr.h"
#include "net/netstats.h"
#include "xtimer.h"

#ifdef __cplusplus
Expand Down Expand Up @@ -344,6 +345,9 @@ typedef struct {
xtimer_t rtr_adv_timer; /**< Timer for periodic router advertisements */
msg_t rtr_adv_msg; /**< msg_t for gnrc_ipv6_netif_t::rtr_adv_timer */
#endif
#ifdef MODULE_NETSTATS_IPV6
netstats_t stats; /**< transceiver's statistics */
#endif
} gnrc_ipv6_netif_t;

/**
Expand Down Expand Up @@ -587,6 +591,18 @@ static inline bool gnrc_ipv6_netif_addr_is_non_unicast(const ipv6_addr_t *addr)
*/
void gnrc_ipv6_netif_init_by_dev(void);

/**
* @brief Get sent and received statistics about IPv6 traffic on this interface.
*
* @note This function is only available if compiled with module `netstats_ipv6`.
*
* @param[in] pid The PID to the interface.
*
* @return A @ref netstats_t pointer to the statistics.
* @return NULL if no statistics are available.
*/
netstats_t *gnrc_ipv6_netif_get_stats(kernel_pid_t pid);

@cgundogan cgundogan Apr 15, 2016

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.

Wouldn't it be more suitable to put this into some unit file of the netstats module itself? Then you can also drop the @note section and it is immediately apparent, that you need said module compiled into the binary.

@OlegHahm OlegHahm Apr 15, 2016

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.

IMO that would be conceptionally unclean. Actually, I think the better idea would be drop the netstats module completely and move its "functionality" into the shell.

@OlegHahm OlegHahm Apr 15, 2016

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.

What do you think about the new solution?

@cgundogan cgundogan Apr 15, 2016

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.

...and move its "functionality" into the shell.

It depends: Can I still use netstats then if I do not compile the shell, but send the stats over e.g. UDP to a server?

@OlegHahm OlegHahm Apr 15, 2016

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.

Sure. The only thing in the netstats module was the _to_string() function that moved to the shell.


#ifdef __cplusplus
}
#endif
Expand Down
11 changes: 11 additions & 0 deletions sys/include/net/netstats.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,24 @@
* @author Oliver Hahm <oliver.hahm@inria.fr>
*/

#include <stdint.h>

#ifndef NETSTATS_H
#define NETSTATS_H

#ifdef __cplusplus
extern "C" {
#endif

/**
* @name @ref net_netstats module names
* @{
*/
#define NETSTATS_LAYER2 (0x01)
#define NETSTATS_IPV6 (0x02)
#define NETSTATS_ALL (0xFF)
/** @} */

/**
* @brief Global statistics struct
*/
Expand Down
18 changes: 18 additions & 0 deletions sys/net/gnrc/network_layer/ipv6/gnrc_ipv6.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,11 @@ static void _send_to_iface(kernel_pid_t iface, gnrc_pktsnip_t *pkt)
gnrc_pktbuf_release(pkt);
return;
}
#ifdef MODULE_NETSTATS_IPV6
if_entry->stats.tx_success++;
if_entry->stats.tx_bytes += gnrc_pkt_len(pkt->next);

@cgundogan cgundogan Apr 19, 2016

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 sure if it's available here, but why not use the length field of the ipv6 header? Isn't that more accurate than gnrc_pkt_len(pkt->next)?

@miri64 miri64 Apr 20, 2016

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.

IPv6 header length + ipv6->len; that would be. pkt->next may contain the netif header that isn't really data that is send over the interface.

@OlegHahm OlegHahm Apr 20, 2016

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.

I agree that IPv6 header length + ipv6->len sounds like a good idea, but in which case can pkt->next contain a netif header and which type would pkt be in this case?

@OlegHahm OlegHahm Apr 20, 2016

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.

Hm, looking at the code again, I wonder if the current solution is not more reasonable: we add the amount of data that is actually passed to the lower layer here.

@cgundogan cgundogan Apr 20, 2016

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.

But this amount of data can also include padding, if I am not mistaken? AFAIK, the pkts in the pktbuf need to be size aligned(?)

@OlegHahm OlegHahm Apr 20, 2016

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.

Hm, who does actually remove the padding before sending?

@cgundogan cgundogan Apr 20, 2016

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.

Hm, no idea. In gnrc_pktbuf_static.c:423 the size for a pktnsip gets aligned, but I have no idea where the magic happens that gnrc_pkt_len reports the correct size. Maybe in gnrc_pktbuf_static.c:449?

#endif

#ifdef MODULE_GNRC_SIXLOWPAN
if (if_entry->flags & GNRC_IPV6_NETIF_FLAGS_SIXLOWPAN) {
DEBUG("ipv6: send to 6LoWPAN instead\n");
Expand Down Expand Up @@ -419,6 +424,9 @@ static void _send_unicast(kernel_pid_t iface, uint8_t *dst_l2addr,

DEBUG("ipv6: send unicast over interface %" PRIkernel_pid "\n", iface);
/* and send to interface */
#ifdef MODULE_NETSTATS_IPV6
gnrc_ipv6_netif_get_stats(iface)->tx_unicast_count++;

@cgundogan cgundogan Apr 15, 2016

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.

What about keeping the netstats struct cached in a (static) global (to this unit file only) variable, assigned only once in gnrc_ipv6_init? You can then reuse that at several places in this file, instead of calling gnrc_ipv6_netif_get_stats each time.

@cgundogan cgundogan Apr 15, 2016

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 see, the iface parameter breaks this idea. But I still think this should be somehow cached .. most of the time we only have one interface.

@OlegHahm OlegHahm Apr 15, 2016

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.

And in this case the call should just de-reference a pointer.

@cgundogan cgundogan Apr 15, 2016

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.

assuming the call gets inlined.

@OlegHahm OlegHahm Apr 15, 2016

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.

Yes, we have to trust the compiler here a little bit. I'm not objecting "caching" this value, I just don't know how without sacrificing RAM.

#endif
_send_to_iface(iface, pkt);
}

Expand Down Expand Up @@ -486,6 +494,9 @@ static inline void _send_multicast_over_iface(kernel_pid_t iface, gnrc_pktsnip_t
DEBUG("ipv6: send multicast over interface %" PRIkernel_pid "\n", iface);
/* mark as multicast */
((gnrc_netif_hdr_t *)pkt->data)->flags |= GNRC_NETIF_HDR_FLAGS_MULTICAST;
#ifdef MODULE_NETSTATS_IPV6
gnrc_ipv6_netif_get_stats(iface)->tx_mcast_count++;
#endif
/* and send to interface */
_send_to_iface(iface, pkt);
}
Expand Down Expand Up @@ -788,6 +799,13 @@ static void _receive(gnrc_pktsnip_t *pkt)

if (netif != NULL) {
iface = ((gnrc_netif_hdr_t *)netif->data)->if_pid;

#ifdef MODULE_NETSTATS_IPV6
assert(iface);
netstats_t *stats = gnrc_ipv6_netif_get_stats(iface);
stats->rx_count++;
stats->rx_bytes += (gnrc_pkt_len(pkt) - netif->size);
#endif
}

first_ext = pkt;
Expand Down
8 changes: 8 additions & 0 deletions sys/net/gnrc/network_layer/ipv6/netif/gnrc_ipv6_netif.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -897,6 +897,14 @@ void gnrc_ipv6_netif_init_by_dev(void)
}
}

#ifdef MODULE_NETSTATS_IPV6
netstats_t *gnrc_ipv6_netif_get_stats(kernel_pid_t pid)
{
gnrc_ipv6_netif_t *iface = gnrc_ipv6_netif_get(pid);
return &(iface->stats);
}
#endif

/**
* @}
*/
83 changes: 71 additions & 12 deletions sys/shell/commands/sc_netif.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,30 +73,59 @@ static bool _is_iface(kernel_pid_t dev)
return false;
}

#ifdef MODULE_NETSTATS_L2
static int _netif_stats(kernel_pid_t dev, bool reset)
#if defined(MODULE_NETSTATS)
const char *_netstats_module_to_str(uint8_t module)
{
switch (module) {
case NETSTATS_LAYER2:
return "Layer 2";
case NETSTATS_IPV6:
return "IPv6";
case NETSTATS_ALL:
return "all";
default:
return "Unknown";
}
}

static int _netif_stats(kernel_pid_t dev, unsigned module, bool reset)
{
netstats_t *stats;
int res = -ENOTSUP;
res = gnrc_netapi_get(dev, NETOPT_STATS, 0, &stats, sizeof(&stats));

if (module == NETSTATS_LAYER2) {
res = gnrc_netapi_get(dev, NETOPT_STATS, 0, &stats, sizeof(&stats));
}
#ifdef MODULE_NETSTATS_IPV6
else if (module == NETSTATS_IPV6) {
stats = gnrc_ipv6_netif_get_stats(dev);
if (stats != NULL) {
res = 1;
}
}
#endif

if (res < 0) {
puts(" Protocol or device doesn't provide statistics.");
}
else if (reset) {
memset(stats, 0, sizeof(netstats_t));
puts("Reset statistics!");
printf("Reset statistics for module %s!\n", _netstats_module_to_str(module));
}
else {
printf(" RX packets %u bytes %u\n"
" TX packets %u (Multicast: %u) bytes %u\n"
" TX succeeded %u errors %u\n",
printf(" Statistics for %s\n"
" RX packets %u bytes %u\n"
" TX packets %u (Multicast: %u) bytes %u\n"
" TX succeeded %u errors %u\n",
_netstats_module_to_str(module),
(unsigned) stats->rx_count,
(unsigned) stats->rx_bytes,
(unsigned) (stats->tx_unicast_count + stats->tx_mcast_count),
(unsigned) stats->tx_mcast_count,
(unsigned) stats->tx_bytes,
(unsigned) stats->tx_success,
(unsigned) stats->tx_failed);
res = 0;
}
return res;
}
Expand Down Expand Up @@ -155,7 +184,8 @@ static void _del_usage(char *cmd_name)

static void _stats_usage(char *cmd_name)
{
printf("usage: %s <if_id> stats [reset]\n", cmd_name);
printf("usage: %s <if_id> stats [l2|ipv6] [reset]\n", cmd_name);
puts(" reset can be only used if the module is specified.");
}

static void _print_netopt(netopt_t opt)
Expand Down Expand Up @@ -460,7 +490,10 @@ static void _netif_list(kernel_pid_t dev)

#ifdef MODULE_NETSTATS_L2
puts("");
_netif_stats(dev, false);
_netif_stats(dev, NETSTATS_LAYER2, false);
#endif
#ifdef MODULE_NETSTATS_IPV6
_netif_stats(dev, NETSTATS_IPV6, false);
#endif
puts("");
}
Expand Down Expand Up @@ -1106,13 +1139,39 @@ int _netif_config(int argc, char **argv)

return _netif_mtu((kernel_pid_t)dev, argv[3]);
}
#ifdef MODULE_NETSTATS_L2
#ifdef MODULE_NETSTATS
else if (strcmp(argv[2], "stats") == 0) {
uint8_t module;
bool reset = false;
if ((argc > 3) && (strncmp(argv[3], "reset", 5) == 0)) {

/* check for requested module */
if ((argc == 3) || (strcmp(argv[3], "all") == 0)) {
module = NETSTATS_ALL;
}
else if (strcmp(argv[3], "l2") == 0) {
module = NETSTATS_LAYER2;
}
else if (strcmp(argv[3], "ipv6") == 0) {
module = NETSTATS_IPV6;
}
else {
printf("Module %s doesn't exist or does not provide statistics.\n", argv[3]);

return 0;
}

/* check if reset flag was given */
if ((argc > 4) && (strncmp(argv[4], "reset", 5) == 0)) {
reset = true;
}
return _netif_stats((kernel_pid_t)dev, reset);
if (module & NETSTATS_LAYER2) {
_netif_stats((kernel_pid_t) dev, NETSTATS_LAYER2, reset);
}
if (module & NETSTATS_IPV6) {
_netif_stats((kernel_pid_t) dev, NETSTATS_IPV6, reset);
}

return 1;
}
#endif
#ifdef MODULE_GNRC_IPV6_NETIF
Expand Down