WIP: gnrc/rpl mrhof by bergzand · Pull Request #7450 · RIOT-OS/RIOT

bergzand · 2017-08-06T22:29:30Z

This WIP is an initial work on mrhof (rfc 6719) for RPL.

Depends on #6873 (& co) for ETX statistics.

todo:

Decent link local ipv6-address to L2-address conversion
Parent selection (sec. 3.2)
Preferred parent selection (sec 3.2.2)
Correct rank computation (sec. 3.2).
~~- [ ] metric containers~~
MAX_LINK_METRIC
MAX_PATH_COST

bergzand · 2017-08-10T14:19:20Z

Now depends on #7468 for parent set calculation.

bergzand · 2017-08-10T15:53:04Z

Should be feature complete now, so testers are welcome. I'm not going to implement metric containers in this PR and only support ETX as a metric.

PR dependency graph might have gotten a bit messed up, but at least #6873 for the neighbor statistics and #7449 for a bit of additional ETX output should be enough.

bergzand · 2017-08-11T13:56:26Z

As a remark when people are going to test this. This PR relies heavily on ETX functionality of the netstats neighbor module. For this to work correctly, support from the radio is really a must. Only succes/noack reporting is not sufficient to really estimate the link quality. As far as I know only the mrf24j40 and the at86r233 support reading of transmission retries needed for accurate ETX estimation. For the mrf24j40 I have a patch ready contained in the chain of commits in this PR. I don't have access to an at86rf233, so I can't test any patch for transmission retries for that device.

So for testing, I would either advise using mrf24j40's or write a patch for the at86rf233 and use that one.
There are probably more radios that support retransmission statistics, so if #7448 is pulled, similar PR's are welcome :)

bergzand · 2017-08-18T12:00:25Z

As of now, this PR would cause gnrc_rpl to depend on the netstats_neighbor module from #6873. Would it be better to make a gnrc_rpl_mrhof submodule such that the mrhof code (and dependencies) becomes optional? I can imagine lightweight nodes not wanting the extra memory required by the netstats_neighbor module.

bergzand · 2017-08-31T09:53:07Z

Rebased and adjusted for current master

This patch adds support for per neighbor statistics such as LQI/RSSI, tx count, failure count, rx count and ETX.

bergzand · 2017-09-28T07:17:41Z

Rebased and uncrustified

cgundogan

looks fine so far, will continue reviewing and testing.

cgundogan · 2017-11-10T13:34:30Z

sys/include/net/netstats/neighbor.h

+/**
+ * @brief Store this neighbor as next in the transmission queue.
+ *
+ * Set len to zero if a nop record is needed, for example if the


len => @p len

Ack, will fix

cgundogan · 2017-11-10T13:35:57Z

sys/include/net/netstats/neighbor.h

+ * @brief Update the next recorded neighbor with the provided numbers
+ *
+ * This only increments the statistics if the length of the l2-address of the retrieved record
+ * is non-zero. see also @ref netstats_nb_record. The numbers indicate the number of transmissions


Ack, will fix

cgundogan · 2017-11-10T13:37:42Z

sys/net/gnrc/link_layer/netdev/gnrc_netdev_eth.c

            (netif_hdr->flags & GNRC_NETIF_HDR_FLAGS_MULTICAST)) {
            gnrc_netdev->dev->stats.tx_mcast_count++;
+#ifdef MODULE_NETSTATS_NEIGHBOR
+            DEBUG("l2 stats: Destination is multicast or unicast, NULL recorded");


are you missing a \n here?

cgundogan · 2017-11-10T13:38:11Z

sys/net/gnrc/link_layer/netdev/gnrc_netdev_ieee802154.c

        (GNRC_NETIF_HDR_FLAGS_BROADCAST | GNRC_NETIF_HDR_FLAGS_MULTICAST)) {
            gnrc_netdev->dev->stats.tx_mcast_count++;
+#ifdef MODULE_NETSTATS_NEIGHBOR
+            DEBUG("l2 stats: Destination is multicast or unicast, NULL recorded");


also, \n here?

cgundogan · 2017-11-10T13:39:18Z

sys/net/netstats/netstats_neighbor.c

+
+#define ENABLE_DEBUG    (0)
+#include "debug.h"
+//static char addr_str[30];


can be removed?

Whoops, yes

bergzand · 2017-11-11T10:38:54Z

@cgundogan Thanks for the (initial) review. I noticed that this PR was missing the "waiting for" tag. I've addressed your comments in #6873.

stale · 2019-08-10T05:08:07Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

bergzand force-pushed the net/rpl-mrhof branch from 27ee6a2 to 183369b Compare August 10, 2017 14:18

bergzand force-pushed the net/rpl-mrhof branch 2 times, most recently from a1f4890 to a4777df Compare August 10, 2017 15:49

bergzand mentioned this pull request Aug 22, 2017

RPL: API update suggestions #7498

Closed

bergzand force-pushed the net/rpl-mrhof branch from 1cc900c to 09f00d0 Compare August 31, 2017 09:52

miri64 requested a review from cgundogan September 19, 2017 12:11

miri64 assigned cgundogan Sep 19, 2017

miri64 added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation GNRC Area: network Area: Networking labels Sep 19, 2017

bergzand added 12 commits September 28, 2017 09:17

mrf24j40: Add tx_retries_needed netopt

917a52a

Add support for link layer neigbor statistics

27d0337

This patch adds support for per neighbor statistics such as LQI/RSSI, tx count, failure count, rx count and ETX.

net/netstats: no not whipe non-existing entry

82855ad

net/netstats Enable neighbor stats for ethernet drivers

1617314

sys/shell: Add output for neighbor stats

db04c33

net/rpl Initial rpl mrhof implementation

c184b94

Add rank calculation

f4bb880

Better parent comparison and rank calculation

fafe16e

Update number of implemented OF

c897e1f

Better checks

17a0027

Add nc lookup for ipv6-address

082a4a5

Code style

d18ca58

bergzand force-pushed the net/rpl-mrhof branch from 09f00d0 to d18ca58 Compare September 28, 2017 07:17

cgundogan requested changes Nov 10, 2017

View reviewed changes

bergzand added the State: waiting for other PR State: The PR requires another PR to be merged first label Nov 11, 2017

bergzand mentioned this pull request Nov 11, 2017

[RFC] net/netstats L1/L2 per neighbor transfer statistics #6873

Closed

4 tasks

miri64 added State: waiting for other PR and removed GNRC labels Oct 5, 2018

miri64 removed the State: waiting for other PR label Oct 12, 2018

stale bot added the State: stale State: The issue / PR has no activity for >185 days label Aug 10, 2019

stale bot closed this Sep 10, 2019

benpicco mentioned this pull request Jul 6, 2020

net/netstats: L1/L2 per neighbor statistics #14448

Merged

benpicco mentioned this pull request Jul 27, 2020

gnrc/rpl: Implement Minimum Rank with Hysteresis Objective Function #14623

Closed

Conversation

bergzand commented Aug 6, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bergzand commented Aug 10, 2017

Uh oh!

bergzand commented Aug 10, 2017

Uh oh!

bergzand commented Aug 11, 2017

Uh oh!

bergzand commented Aug 18, 2017

Uh oh!

bergzand commented Aug 31, 2017

Uh oh!

bergzand commented Sep 28, 2017

Uh oh!

cgundogan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bergzand commented Nov 11, 2017

Uh oh!

stale bot commented Aug 10, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bergzand commented Aug 6, 2017 •

edited

Loading