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_netif: allow for wait of minimum time between sends #11837

Merged
merged 1 commit into from
Aug 9, 2019

Conversation

miri64
Copy link
Member

@miri64 miri64 commented Jul 12, 2019

Contribution description

This was cherry-picked out of #11068 (which became a huge mess during my experiment phase in preparation for the 6LoWPAN fragment forwarding paper.

This adds a minimum sleep between every send, to allow for slowing down the send operations of a device.

To be perfectly honest, I don't know if and how useful this could be in production operation or beyond experimentation in general, but it helped me to confirm some things during my experimentation. However, I kept it optional and marked it as experimental so I think it does not harm to merge.

Testing procedure

First compile and flash gnrc_networking normally with GNRC_NETIF_MIN_WAIT_AFTER_SEND_US=0. Ping another node with the same configuration. There should be no significant changes to to master.

Now compile with a higher delay e.g. GNRC_NETIF_MIN_WAIT_AFTER_SEND_US=5000. The RTT when pinging should now significantly increase (maybe even timeout), if you have access to a sniffer: the time different between two transmissions (ignoring L2 retransmissions) should now be at minimum at the configured time interval.

Issues/PRs references

Cherry-picked from #11068, but this PR has no dependencies.

@miri64 miri64 added Area: network Area: Networking Type: new feature The issue requests / The PR implemements a new feature for RIOT labels Jul 12, 2019
@miri64 miri64 requested review from smlng and PeterKietzmann July 12, 2019 15:04
Copy link
Contributor

@fjmolinas fjmolinas left a comment

Choose a reason for hiding this comment

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

Tested using a samr21-xpro and pba-d-01-kw2x. I only have a minor comment on the doc, if you agree please squash directly.

  • GNRC_NETIF_MIN_WAIT_AFTER_SEND_US=5000
> ping6 fe80::d1c1:6d4e:ab6a:1336 
2019-08-09 10:07:59,059 - INFO #  ping6 fe80::d1c1:6d4e:ab6a:1336
2019-08-09 10:08:02,058 - INFO # 
2019-08-09 10:08:02,062 - INFO # --- fe80::d1c1:6d4e:ab6a:1336 PING statistics ---
2019-08-09 10:08:02,067 - INFO # 3 packets transmitted, 0 packets received, 100% packet loss
  • GNRC_NETIF_MIN_WAIT_AFTER_SEND_US=500
ping6 fe80::d1c1:6d4e:ab6a:1336 
2019-08-09 10:08:58,750 - INFO # ping6 fe80::d1c1:6d4e:ab6a:1336
2019-08-09 10:08:58,766 - INFO # 12 bytes from fe80::d1c1:6d4e:ab6a:1336: icmp_seq=0 ttl=64 rssi=-57 dBm time=8.689 ms
2019-08-09 10:08:59,765 - INFO # 12 bytes from fe80::d1c1:6d4e:ab6a:1336: icmp_seq=1 ttl=64 rssi=-63 dBm time=8.995 ms
2019-08-09 10:09:00,764 - INFO # 12 bytes from fe80::d1c1:6d4e:ab6a:1336: icmp_seq=2 ttl=64 rssi=-64 dBm time=8.371 ms
2019-08-09 10:09:00,764 - INFO # 
2019-08-09 10:09:00,768 - INFO # --- fe80::d1c1:6d4e:ab6a:1336 PING statistics ---
2019-08-09 10:09:00,774 - INFO # 3 packets transmitted, 3 packets received, 0% packet loss
2019-08-09 10:09:00,778 - INFO # round-trip min/avg/max = 8.371/8.685/8.995 ms

  • GNRC_NETIF_MIN_WAIT_AFTER_SEND_US=0
2019-08-09 10:06:41,414 - INFO #  ping6 fe80::d1c1:6d4e:ab6a:1336
2019-08-09 10:06:41,429 - INFO # 12 bytes from fe80::d1c1:6d4e:ab6a:1336: id=83 seq=1 hop limit=64 time = 7.684 ms
2019-08-09 10:06:42,443 - INFO # 12 bytes from fe80::d1c1:6d4e:ab6a:1336: id=83 seq=2 hop limit=64 time = 7.692 ms
2019-08-09 10:06:43,457 - INFO # 12 bytes from fe80::d1c1:6d4e:ab6a:1336: id=83 seq=3 hop limit=64 time = 8.321 ms

@fjmolinas fjmolinas added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines labels Aug 9, 2019
Copy link
Contributor

@fjmolinas fjmolinas left a comment

Choose a reason for hiding this comment

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

ACK, please squash.

@miri64 miri64 force-pushed the gnrc_netif/enh/inter-tx-wait branch from 6b617ee to 488c47c Compare August 9, 2019 08:21
@miri64
Copy link
Member Author

miri64 commented Aug 9, 2019

ACK, please squash.

Done

@fjmolinas fjmolinas added the Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines label Aug 9, 2019
@fjmolinas
Copy link
Contributor

All green! GO!

@fjmolinas fjmolinas merged commit fde5037 into RIOT-OS:master Aug 9, 2019
@miri64 miri64 deleted the gnrc_netif/enh/inter-tx-wait branch August 9, 2019 08:56
@kb2ma kb2ma added this to the Release 2019.10 milestone Sep 16, 2019
miri64 added a commit to 5G-I3/RIOT-public that referenced this pull request Sep 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: network Area: Networking CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants