Skip to content

gnrc_netif: add NETDEV_EVENT_PDU_CHANGED event#12835

Closed
benpicco wants to merge 1 commit intoRIOT-OS:masterfrom
benpicco:gnrc_netif-pdu_change
Closed

gnrc_netif: add NETDEV_EVENT_PDU_CHANGED event#12835
benpicco wants to merge 1 commit intoRIOT-OS:masterfrom
benpicco:gnrc_netif-pdu_change

Conversation

@benpicco
Copy link
Contributor

@benpicco benpicco commented Nov 28, 2019

Contribution description

For some devices the maximum frame size is dependant on the configuration.

This adds a NETDEV_EVENT_PDU_CHANGED event that a network driver can generate
if a change in configuration resulted in a different PDU than before.

For this the driver must implement an answer to the NETOPT_MAX_PDU_SIZE .get() request.

Testing procedure

nothing to test yet

Issues/PRs references

Will be used by #12128

For some devices the maximum frame size is dependant on the configuration.

This adds a NETDEV_EVENT_PDU_CHANGED event that a network driver can generate
if a change in configuration resulted in a different PDU than before.

For this the driver must implement the NETOPT_MAX_PDU_SIZE `.get()` request.
@benpicco benpicco added Area: network Area: Networking Type: new feature The issue requests / The PR implemements a new feature for RIOT CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Nov 28, 2019
@benpicco benpicco requested review from jia200x and miri64 November 28, 2019 14:04
@miri64
Copy link
Member

miri64 commented Nov 28, 2019

What is the flow for the decision of the PDU change? The device decides (by a packet triggering it or whatever) to change its PDU size? Or is this done via user configuration?

@benpicco
Copy link
Contributor Author

benpicco commented Nov 28, 2019

Right now it's only though (user) configuration changes. (Setting NETOPT_IEEE802154_PHY to something other than IEEE802154_PHY_OQPSK¹ or setting NETOPT_OQPSK_RATE without the IEEE802154_OQPSK_FLAG_LEGACY flag set.)

Since the device can listen to both legacy and non-legacy OQPSK frames one could also imagine a mode of operation where the TX mode is chosen based on the destination, but that would likely also just use NETOPT_OQPSK_RATE.

[1] this is driver specific. 802.15.4-2006 also knows about BPSK and ASK that will use a 127 PDU, but this particular device does only supports OQPSK in legacy mode.

@miri64
Copy link
Member

miri64 commented Nov 28, 2019

Mhhh.. In both cases this would be handled by the MAC implementation (using names for new/old MAC implementation functions): We have netif_set/gnrc_netif_set to handle the user configuration and the legacy vs non-legacy typically would happen in netif_send/netif->ops->send(). So why not keep it simple and instead of reacting to an event that is caused by the MAC layer anyway, do the configuration directly in the MAC layer? I.e. when the options are set as described (or the send decision is made), change the MTU and the information required by the network interface?

@benpicco
Copy link
Contributor Author

benpicco commented Nov 28, 2019

So why not keep it simple and instead of reacting to an event that is caused by the MAC layer anyway, do the configuration directly in the MAC layer?

Because that's not so simple. How would the MAC layer know what frame size the radio is able to send based on the setting?
The MAC layer would need to know about the driver/setting/PDU combinations or which settings might trigger a PDU change.

@miri64
Copy link
Member

miri64 commented Nov 28, 2019

How would the MAC layer know what frame size the radio is able to send based on the setting?

Because it is written in the IEEE 802.15.4 standard I hope.

@benpicco
Copy link
Contributor Author

benpicco commented Nov 28, 2019

Because it is written in the IEEE 802.15.4 standard I hope.

It is indeed.
Hm, so far I've treated legacy O-QPSK just as another rate mode of O-QPSK but this would be easier if it were a different PHY type then, so only one option can change the PDU.

Still, if you switch from, say FSK to OFDM you would have an unnecessary round-trip to the driver as opposed to just having the driver inform you if the PDU changed. (Which it would not do in this case)
And it would require additional code to work for non-802.15.4 devices and devices supporting proprietary 'jumbo' frames.

@miri64
Copy link
Member

miri64 commented Nov 28, 2019

Still, if you switch from, say FSK to OFDM you would have an unnecessary round-trip to the driver as opposed to just having the driver inform you if the PDU changed.

The current approach seems to me like an unnecessary round-trip to the MAC...

It would also work for non-802.15.4 devices and devices supporting proprietary 'jumbo' frames.

IMHO this should also be implemented in the respective MAC layers. But maybe other reviewers have differing opinions?

@jia200x
Copy link
Member

jia200x commented Nov 28, 2019

Because that's now so simple. How would the MAC layer know what frame size the radio is able to send based on the setting?
The MAC layer would need to know about the driver/setting/PDU combinations or which settings might trigger a PDU change.

There's a similar situation in LoRaWAN. The PDU is dependent of the Datarate, a combination of 2 PHY layer attributes: SF (Spreading Factor) and BW (Bandwidth).

For GNRC LoRaWAN, as pointed out by @miri64 above, this is solved in the MAC layer.

In practice the MAC layer must always maintain this information (e.g the IEEE802.15.4 is designed to run with more than one PHY layer, 2.4 GHz and SubGHz. LoRaWAN runs both the LoRa modulation and FSK). So I tend to think it will be easier in the long run to keep the information there and not in the transceiver/PHY layers. It will become more evident after the PHY/MAC rework in the scope of #12741

@benpicco benpicco added the Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR label Dec 6, 2019
@benpicco
Copy link
Contributor Author

Extending netdev_ieee802154.c to take the PHY mode into account in NETOPT_MAX_PDU_SIZE: is clean and straightforward.

However, when I add a case NETOPT_IEEE802154_PHY: to gnrc_netif_set_from_netdev() to call gnrc_netif_ipv6_init_mtu() that won't work.

gnrc_netif_ipv6_init_mtu() calls get(NETOPT_MAX_PDU_SIZE) but this is before the set(NETOPT_IEEE802154_PHY) message has reached the device, so it will always return the previous PDU.

Should the device then call gnrc_netif_ipv6_init_mtu() on it's own in the case NETOPT_IEEE802154_PHY: ?

If the call chain is gnrc -> netdev -> driver then the upper layer can't know if the lower layer operation succeeded and gnrc_netif_ipv6_init_mtu() needs to be called again.

@fjmolinas
Copy link
Contributor

Should we close this one @benpicco?

@benpicco benpicco closed this May 5, 2020
@benpicco
Copy link
Contributor Author

benpicco commented May 5, 2020

Yes this is obsolete now

@benpicco benpicco deleted the gnrc_netif-pdu_change branch May 5, 2020 07:38
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 Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR 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.

4 participants