Skip to content

Conversation

OlegHahm
Copy link
Member

This PR introduces two features:
1.) Link layer retransmissions inside netdev2. Therefore, link layer retransmissions are disabled in the driver.
2.) netstats as a mostly generic module to collect statistics per layer. A first implementation for at86rf2xx's netdev2 driver is added.

Depends on #4645

@OlegHahm OlegHahm added Area: network Area: Networking Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR labels Feb 11, 2016
@OlegHahm
Copy link
Member Author

I know it's not a beauty, but can become quite handy if you want to measure what's really happening inside the stack.

@miri64
Copy link
Member

miri64 commented Feb 11, 2016

This seems to be at first glance more of a gnrc_netdev2 change than a netdev2 change. Please clarify in the description. Why is it called netdev_retrans and not netdev2_retrans? More thorough review will follow when dependencies got merged (or when I find the time to differentiate your code from mine ^^)

@OlegHahm
Copy link
Member Author

Well, ideally, it would go into netdev2, but I need something to queue the packets while waiting for retransmission and therefore the pktbuf is quite handy.

@miri64
Copy link
Member

miri64 commented Feb 11, 2016

Why netdev2? Isn't this a very radio-only feature?

@OlegHahm
Copy link
Member Author

Why radio-only?

@kaspar030
Copy link
Contributor

@OlegHahm Would you mind factoring out the netstats?

Regarding the retransmissions, I think a combined approach is best:

  • gnrc's powerful pktsnips show that packet queueing is best done in the network stack
  • netdev2 still needs to communicate e.g., a busy medium

I propose that netdev2 devices do, if not configured to do some kind of internal retransmissions, return a defined error when the medium is busy when sending, e.g., "-EBUSY" or "-EAGAIN" (we should use one for the device actually being busy and the other for "medium is busy".

In gnrc, we devise a simple intermediate handler that overrides _send (and possibly _get/_set), on which radio devices (gnrc_iee802154/gnrc_cc110x) can be stacked.

Why radio-only?
Maybe it's not radio-only, but for some netdev's (e.g., ethernet), it doesn't make sense to support media access control. If that handling is inside gnrc_netdev2, it will be compiled in (and executed) for all types of devices.

@OlegHahm
Copy link
Member Author

@OlegHahm Would you mind factoring out the netstats?

Yes, makes sense, will do. Although without the retransmission mechanism the stats won't be accurate.

@OlegHahm
Copy link
Member Author

The problem that I'm current facing is that I lose interrupts on the SAMR21. If I send fragmented 6LoWPAN packets and the last fragment is rather small (e.g. <= 20 bytes of payload), I will only get one TX_END IRQ.

@OlegHahm OlegHahm changed the title netdev2: link-layer retransmissions outside the transceiver driver gnrc_netdev2: link-layer retransmissions outside the transceiver driver Feb 12, 2016
@OlegHahm
Copy link
Member Author

Problem fixed. Thanks @haukepetersen for the advice.

@OlegHahm OlegHahm added the State: waiting for other PR State: The PR requires another PR to be merged first label Feb 12, 2016
@OlegHahm
Copy link
Member Author

@kaspar030, here you go: #4801

@OlegHahm
Copy link
Member Author

Surprisingly it doesn't seem to affect the performance when running pinging (tested on SAMR21-xpro with 100 pings a 1008 bytes, i.e. 10 fragments per datagram).

@miri64
Copy link
Member

miri64 commented Mar 6, 2016

Needs rebase. I did not look into the code, but if I understand the problem correctly: Can't this be solved with a tx_info struct, similar as I did with rx_info in #4648?

@OlegHahm
Copy link
Member Author

OlegHahm commented Mar 6, 2016

What can be solved?

@miri64
Copy link
Member

miri64 commented Mar 7, 2016

Getting the number of retransmissions done by the driver.

@OlegHahm
Copy link
Member Author

OlegHahm commented Mar 7, 2016

I don't understand how. The transceiver and therefore the driver doesn't know anything about the number of retransmissions.

@miri64
Copy link
Member

miri64 commented Mar 7, 2016

Huh? If not the transceiver, who else?

@OlegHahm
Copy link
Member Author

OlegHahm commented Mar 7, 2016

In this PR: gnrc_netdev2. That's basically the whole point of this PR. ;)

@miri64
Copy link
Member

miri64 commented Mar 7, 2016

Ahhhh sorry I missunderstood the PR. I thought you introduced a feature to get the number of automatic retransmissions of the device... not that you take out the retransmissions to the MAC layer... Okay, then I wait until #4646 is merged for my final review :-)

@OlegHahm
Copy link
Member Author

rebased to current master after #4646 got merged.

@miri64
Copy link
Member

miri64 commented Mar 24, 2016

Isn't netstats introduced in #4801?

@OlegHahm
Copy link
Member Author

Yes.

@miri64
Copy link
Member

miri64 commented Mar 24, 2016

So why did you rebase to master then?

@PeterKietzmann
Copy link
Member

@smlng are you aware of this PR?

@miri64
Copy link
Member

miri64 commented Oct 30, 2016

Might also be of interest for @zhuoshuguo and @daniel-k especially considering that they are working on a priority queue for MAC too.

@miri64
Copy link
Member

miri64 commented Oct 31, 2016

Postponed due to feature freeze

@miri64 miri64 modified the milestones: Release 2017.01, Release 2016.10 Oct 31, 2016
@miri64
Copy link
Member

miri64 commented Nov 7, 2016

Please see if you can unify this with #5941 and #5950 somehow (also needs rebase).

@miri64
Copy link
Member

miri64 commented Jan 10, 2017

@OlegHahm ping?

@miri64
Copy link
Member

miri64 commented Jan 10, 2017

(both PRs got merged in the meantime so rebasing is and unification with those is now required)

@OlegHahm
Copy link
Member Author

I can certainly rebase, but I'm not sure I will find the time for extensive testing.

@miri64
Copy link
Member

miri64 commented Jan 24, 2017

Ok, then let's postpone

@miri64 miri64 modified the milestones: Release 2017.04, Release 2017.01 Jan 24, 2017
@miri64
Copy link
Member

miri64 commented Mar 16, 2017

@OlegHahm What is the status of this?

@miri64
Copy link
Member

miri64 commented Jun 27, 2017

Needs rebase.

@kYc0o kYc0o mentioned this pull request Oct 30, 2017
15 tasks
@miri64
Copy link
Member

miri64 commented Nov 15, 2017

The code looks pretty stale and I think it would be better to have this done in a matter as proposed in #7736, so that other stacks can benefit from that.

@miri64 miri64 added the State: archived State: The PR has been archived for possible future re-adaptation label Nov 15, 2017
@miri64 miri64 closed this Nov 15, 2017
@benpicco benpicco mentioned this pull request Nov 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: network Area: Networking CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR State: archived State: The PR has been archived for possible future re-adaptation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants