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

drivers: ieee802154: Simplify supporting custom EUI64 source. #27513

Closed
kkasperczyk-no opened this issue Aug 12, 2020 · 3 comments
Closed

drivers: ieee802154: Simplify supporting custom EUI64 source. #27513

kkasperczyk-no opened this issue Aug 12, 2020 · 3 comments
Labels
area: IEEE 802.15.4 area: Networking Enhancement Changes/Updates/Additions to existing features

Comments

@kkasperczyk-no
Copy link
Contributor

kkasperczyk-no commented Aug 12, 2020

Is your enhancement proposal related to a problem? Please describe.
Current implementation doesn't make a problem itself, but could be improved. Solution uses IEEE802154_NET_IF_NO_AUTO_START (#25629) and calls net_if_set_link_addr(iface, mac, 8, NET_LINK_IEEE802154) to set EUI64 address. First of all if user wants to pass own EUI64 address he should enable IEEE802154_NET_IF_NO_AUTO_START, which actually disables auto-start and allows to set any configuration before radio becomes operational, what is not the user purpose. User has to go around the problem and enable additional feature, which is not clearly related to achieve goal. Then net_if_set_link_addr() method should be called somewhere in the user code (e.g. in main thread) and after that interface should be bring up by net_if_up() method. It pledges user to remember about bringing up the interface and I think that is not quite consistent with assumptions of Zephyr as a system in which, thanks to the proper configuration, everything is operational and works just after reset, without any additional activities on the user site (in many cases main thread is empty and everything happens in background).

Describe the solution you'd like
Solution should be clear and simple to use for user, which may want to provide own custom method generating EUI64 address in some specific way (e.g. making some part of address random, getting it from memory chip or whatever). Moreover it should avoid situation in which something may go wrong, because user forgot to do some activities.

Describe alternatives you've considered
I propose adding general ieee802154 Kconfig option, which enables custom source of EUI64. By default it's disabled and method implemented in specific drivers is providing EUI64. When enabled, user is obliged to provide method implementation or the project compilation will fail. To make it work properly, mentioned method has to be unified for all drivers, so I proposed API for it in ieee802154_radio.h file (ieee802154_radio_get_eui64()). Each driver owner should implement this method itself, if wants to use this option. To make sure that enabling option on platform, which doesn't support it will not affect with unexpected behaviour, there was depends on phrase added (currently only nrf5 driver supports it).

What are advantages of such approach:

  • Feature is enabled by config option, which clearly describes what it is made for.
  • After enabling feature, user can't forget about implementing method, because compilation will fail without it.
  • User application main thread remains clean.

Draft PR with proposed changes link: #27512

Additional context
Inviting to discussion: @LuDuda @konradderda @nandojve @markus-becker-tridonic-com

@tbursztyka
Copy link
Collaborator

Confusing, you say:

User has to go around the problem and enable additional feature

but in your solution:

for user, which may want to provide own custom method generating EUI64 address in some specific way

So the user will anyway need to code something then, or?

It pledges user to remember about bringing up the interface and I think that is not quite consistent with assumptions of Zephyr as a system in which, thanks to the proper configuration, everything is operational and works just after reset, without any additional activities on the user site

You are wrongly assuming things here. There are use case where net ifs may not be put up by default, and will be put later on either programmatically or manually through the shell.

There is actually a (big) issue with net_if_get_link_addr/net_if_set_link_addr: it's not properly"connected to ieee802154 stack and drivers. If you net_if_set_link_addr later at runtime, the driver nor the stack will know about that change.

Anyway, have you considered using ieee802154 net_mgmt interfaces NET_REQUEST_IEEE802154_SET_EXT_ADDR/NET_REQUEST_IEEE802154_GET_EXT_ADDR ?

If the user wants to provide custom eui64 there is already ways to do that.

@kkasperczyk-no
Copy link
Contributor Author

@tbursztyka I took a look on this net_mgmt interfaces and if I understand it correctly it's a way to send network request setting interface address to another. So user is able to do this in his code at runtime, by passing desired eui64 to this request, am I right? I'm asking about this, because I had on my mind solution, in which user provides implementation of method called automatically in driver before interface initialization. I thought that such approach is safer and easier for user, because we can be sure where and when this method will be called. If you find such solution insecure and out of control (because user is providing own implementation and not using existing once), I'm ok with that, just wanted to make sure.

@ghost ghost self-assigned this Oct 1, 2023
@ghost ghost added the area: Networking label Oct 3, 2023
@ghost
Copy link

ghost commented Jul 26, 2024

@kkasperczyk-no Closing as "won't fix"/"invalid" due to the existing solution via net_mgmt(). Feel free to re-open if you don't agree.

@ghost ghost closed this as completed Jul 26, 2024
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: IEEE 802.15.4 area: Networking Enhancement Changes/Updates/Additions to existing features
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants