Skip to content

netdev: Add netopt to retrieve netdev_ieee802154_t#9335

Closed
bergzand wants to merge 4 commits intoRIOT-OS:masterfrom
bergzand:pr/netdev/ieee802154/get_netdev_ptr
Closed

netdev: Add netopt to retrieve netdev_ieee802154_t#9335
bergzand wants to merge 4 commits intoRIOT-OS:masterfrom
bergzand:pr/netdev/ieee802154/get_netdev_ptr

Conversation

@bergzand
Copy link
Copy Markdown
Member

Contribution description

This PR adds a get operation to retrieve the netdev_ieee802154_t from a radio. This because the struct contains essential information required for constructing an ieee802154 frame.

Issues/PRs references

required to save a new struct member in #9329

bergzand added 3 commits June 12, 2018 15:54
To prevent issues with gnrc_netif_802154, this getter is added to
retrieve the netdev_ieee802154_t struct. This struct is required for
retrieving the panid and incrementing sequence number with packet
transmission and more.
@bergzand bergzand added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: network Area: Networking labels Jun 13, 2018
@miri64
Copy link
Copy Markdown
Member

miri64 commented Jun 13, 2018

I think I have a problem understanding the problem here... Why can't you just use netif->dev? The current approach seems somewhat dirty.

@jnohlgard
Copy link
Copy Markdown
Member

The problem comes from a design flaw IMO, placing the link layer frame state variables inside the device driver instead of inside the gnrc_netif layer.
An idea for a solution:
move the netif_ieee802154 state from inside the netdev_ieee802154_t struct to a stack allocated variable inside the gnrc_netif thread.
A way to do this would be to let the gnrc_netif_create function take an extra argument "state_len" which would be used inside the _gnrc_netif_thread function to create a VLA (variable length array) on the thread stack. The state is typically quite small (proto, seq, pan) so it should not be a problem with stack space. The gnrc_netif_ieee802154_create function would then pass sizeof(netif_ieee802154_state) to gnrc_netif_create. This way we can have other kinds of framers which still can operate inside the gnrc space (with pktsnip instead of iolist) but using different kinds of state vectors instead of having a crapload of #ifdef inside the header files like it is now.
I can create a proof of concept later today or tomorrow if you want

@jnohlgard
Copy link
Copy Markdown
Member

It would also remove the need for device drivers to specify the protocol via the netdev_ieee802154_t::proto member (e.g. https://github.com/RIOT-OS/RIOT/blob/master/drivers/at86rf2xx/at86rf2xx.c#L91-L96). The device driver IMO should not have to care about what protocol is running in a higher layer

@bergzand
Copy link
Copy Markdown
Member Author

I had a discussion about this yesterday with @miri64. Our idea was to have a generic netdev_ieee802154_mac_t. this would be filled by gnrc_netif_ieee802154 and netdev_ieee802154.c would get a send() function which reformats the header into a proper (compressed) ieee802154 mac header. gnrc_netif_ieee802154 then doesn't have to provide the variables it doesn't know about (pan_id, sequence number, etc). The netdev layer would provide these when reformatting the header.

@bergzand
Copy link
Copy Markdown
Member Author

The device driver IMO should not have to care about what protocol is running in a higher layer

Agreed :)

@bergzand
Copy link
Copy Markdown
Member Author

Closing this one as the approach is not the way to go:tm: here.

@bergzand bergzand closed this Jun 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: network Area: Networking Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants