TheThingsNetwork / lorawan-stack

The Things Stack, an Open Source LoRaWAN Network Server
https://www.thethingsindustries.com/stack/
Apache License 2.0
978 stars 307 forks source link

Issue DevEUI from range with maximum per application #3750

Closed johanstokking closed 3 years ago

johanstokking commented 3 years ago

Summary

Issue DevEUIs from a configurable range, with a configurable maximum per application.

Why do we need this?

For people running The Things Stack and own a IEEE MAC block to issue DevEUIs for devices that still need to be programmed. This is also useful for The Things Network community network, and reducing the chance of registering a device with a real DevEUI.

What is already there? What do you see now?

The requirement to enter a DevEUI.

What is missing? What do you want to see?

A button to issue a new DevEUI.

How do you propose to implement this?

Discussion point is: do we want this?

My take is yes, to reduce pollution. We may consider this for AppEUI/JoinEUI too, like we did previously with V2.

How do you propose to test this?

Unit and integration testing

Can you do this yourself and submit a Pull Request?

Will test

pgalic96 commented 3 years ago

I have more time from 1st of April, so wondering if this is where I should start with the stack issues. cc @johanstokking @htdvisser to me this seems like a very useful feature, but I would like to avoid the situation that happened with the entity rights transfer feature so we should discuss if we really need this. Also seems its something community is interested in as well.

htdvisser commented 3 years ago

Just found an older issue about the same with additional details: https://github.com/TheThingsIndustries/lorawan-stack/issues/2324

nejraselimovic commented 3 years ago

@pgalic96 anything else you need to discuss about here? We agreed that we do need this indeed.

pgalic96 commented 3 years ago

Should we allow only configuration of one large address block from which all EUI’s (Join, Gtw, Dev) could be issued, or force configuration of an address block per each EUI (Join,Gtw,Dev)? The second might be better from performance pov, as issuing app eui (and incrementing counters) wouldn’t block issuing dev eui and so on. cc @johanstokking

johanstokking commented 3 years ago

The scope of this issue is DevEUI; don't bother with JoinEUI and Gateway EUI.

For the JoinEUI: people can (and should) use all-zeros if they don't have any.

For the gateway EUI: this isn't really a problem as gateways have MAC addresses from which EUIs can be derived.

pgalic96 commented 3 years ago

@johanstokking

As far as I can see we have two options:

  1. Introduce the RPC where a user can request the DevEUI for an application: in CLI it would be a command like applications test-app issue-dev-eui which increments the counters for the application dev eui's issued, and returns a DevEUI.
    • pros: it's a separate RPC, making it completely independent from device creation. It would also look nice in the console as I imagine button Request DevEUI which would then populate the DevEUI field. Users could also continue using the convention of having their device id match the DevEUI.
    • cons: No guarantees this DevEUI would actually be registered. We could add some garbage control (expiry after certain time if the DevEUI is not registered for device) but considering the current simple behavior for issuing DevEUI where the address to be issued gets incremented in the block, we would need a more sophisticated way of keeping track which DevEUI's are available.

If we don't care about DevEUI's issued and not being used, we could still go with this, but I don't think this helps in reducing pollution in the DevEUI address space.

  1. Having the DevEUI issuer as an internal function, not being exposed by RPC. We can have a flag in CLI device create command (--request-dev-eui-from-address-block or smth like that) which would request DevEUI internally.
    • pros: The whole process would be atomic, which would guarantee the issued DevEUI is assigned to the device.
    • cons: It would probably not be possible to assign the DevEUI to the device-id, except if we want to control that as well with some cli flags and additional functionality but that just adds complexity.

We could also have a combination of these two approaches in the CLI, where firstly the RPC for issuing DevEUI is called and then the end device create RPC with the retrieved DevEUI, but again, things could go wrong in time between these two RPC's which would end up in DevEUI not being used.

I suggest the second approach, but let me know your opinion on this.

johanstokking commented 3 years ago

Having the DevEUI issuer as an internal function, not being exposed by RPC. We can have a flag in CLI device create command (--request-dev-eui-from-address-block or smth like that) which would request DevEUI internally.

The CLI can't do anything that isn't API exposed.

We could also have a combination of these two approaches in the CLI, where firstly the RPC for issuing DevEUI is called and then the end device create RPC with the retrieved DevEUI, but again, things could go wrong in time between these two RPC's which would end up in DevEUI not being used.

I like this approach best. I'm not too afraid of things going wrong between those rpcs, but mostly that the second rpc fails, because IS, NS, AS or JS rejected the device create because of a validation error. In that case we will end up with DevEUIs not being actually used.

We could do a best effort approach to yielding the issued DevEUI back to the pool, but since the "next DevEUI" is a global counter, this can also race between different clients. So it's not deterministic. Plus this is only one of the cases that a DevEUI wouldn't be used; deleting or never activating the device is another.

A single EUI64 from a MAC-S block costs USD 0.0000029 so let's not to over-engineer avoiding DevEUI pollution. Making sure that we don't run out of DevEUIs too soon will be controlled by the maximum of DevEUIs per application.

So my proposal:

  1. A global counter in IS
    • configuration for the first (and last) DevEUI to issue
    • configuration for the maximum number of DevEUIs per application
  2. A counter per application
  3. An rpc for issuing a new DevEUI for an application. Needs DEVICES_WRITE access. This first increments the counter per application if it didn't reach the limit. Then it bumps the global counter and returns the DevEUI
  4. Client support
    • CLI gets a flag --issue-dev-eui that's an alternative to passing --dev-eui. When this flag is passed, the CLI first requests the DevEUI from IS before proceeding. If the device ID isn't given, it can automatically be generated in the form eui-XXXXXXXXXXXXXXXX (either the one passed explicitly with --dev-eui or --issue-dev-eui)
    • Console gets a button in the DevEUI input box to issue a new DevEUI
pgalic96 commented 3 years ago

The CLI can't do anything that isn't API exposed.

I meant this internal function would be invoked as part of device creation procedure.

I like this approach best. I'm not too afraid of things going wrong between those rpcs, but mostly that the second rpc fails, because IS, NS, AS or JS rejected the device create because of a validation error. In that case we will end up with DevEUIs not being actually used.

Yeah then this approach makes the most sense. Didn't take into account that there are numerous ways to release the DevEUI address without being able to re-use it again.

nejraselimovic commented 3 years ago

@pgalic96 I'm closing this as I see #4175 should've closed it. Please reopen and move to next milestone if this is not the case.

pgalic96 commented 3 years ago

@nejraselimovic that was a wrongly open PR, #4180 is the correct one, and there you can see Johan's comment:

https://github.com/TheThingsNetwork/lorawan-stack/pull/4180#issuecomment-844952662

kschiffer commented 3 years ago

@pgalic96 So are you planning to pick up the Console part of this?

pgalic96 commented 3 years ago

@kschiffer We should probably discuss first how this would look like in the console. We already discussed it a bit, so there are a few ways we can implement this functionality:

  1. Add a button Request DevEUI address that would populate the DevEUI field. This doesn't guarantee this DevEUI will be used once it is requested but currently this behavior is possible via CLI. This would also allow user to choose whether they want to name their device-id using this DevEUI or not.
  2. Add a Request DevEUI checkbox in the device creation form, which would correspond to the behavior we have for the end device creation cli command (you can specify a flag which would create a device with issued DevEUI). In this case if user prefers the device id to be based on its DevEUI we can also allow that using a checkbox.

Regarding whether I pick up this or not, I will be able to pick this up in two weeks. This is something that is rising in priority so if you think it's simple enough and can be done quickly then you can pick it up, otherwise I can start on this when my priorities clear up.

kschiffer commented 3 years ago

Your first solution is the way to go as far as I'm concerned, for the reasons that you mention. There should be a button next to the field saying Generate. I know that that caption is not 100% specific, but I think it's best to avoid different terminology to not confuse users with the exact technicalities. The tooltip of the field needs to be updated then to give some information about the Generate button.

johanstokking commented 3 years ago

I also think that option 1 is the best solution. I do have two additions:

  1. Make it really clear that when the user clicks Generate, it counts towards their maximum of DevEUIs per application. Should the user confirm? Or can we make this clear in the UI?
  2. Once a DevEUI has been generated, disable the possibility to generate another one in the same screen. This avoids users clicking the button 20 times, exhausting their DevEUIs rapidly
kschiffer commented 3 years ago

Should the user confirm? Or can we make this clear in the UI?

Is it possible to retrieve or calculate the amount of DevEUIs left? Otherwise we can display an orange warning icon next to the button that triggers a tooltip explaining this.

Once a DevEUI has been generated, disable the possibility to generate another one in the same screen. This avoids users clicking the button 20 times, exhausting their DevEUIs rapidly

Yeah, we can just gray out the button once it was clicked.

pgalic96 commented 3 years ago

Is it possible to retrieve or calculate the amount of DevEUIs left? Otherwise we can display an orange warning icon next to the button that triggers a tooltip explaining this.

Yes, this information is part of the application db model (application.dev_eui_counter).

kschiffer commented 3 years ago

Okay then let's just display (1/100 available DevEUIs used) next to the button.

htdvisser commented 3 years ago

This did not make it for the v3.13.3 deadline, so pushing to v3.14.0

johanstokking commented 3 years ago

@pgalic96 are we gonna make this in v3.14.0?

pgalic96 commented 3 years ago

I don't think so, I added the functionality but there are a few things like error handling missing. Also I am waiting for https://github.com/TheThingsNetwork/lorawan-stack/pull/4291 to be rebased and merged to add it to the manual device creation form as well. Will discuss with @kschiffer if it can be squeezed in.