luci-mod-status: use ubus for odhcpd leases #8084
Conversation
Multiple DUIDs can be defined for one static lease, and using an "option" instead of a "list" in UCI is legacy, so store the DUID as a list for correctness. Signed-off-by: David Härdeman <[email protected]>
Note that "duid" from UCI can be *either* "<DUID>" or "<DUID>%<IAID>", so update the code to handle both cases. Also, make sure the "Reserve IP" button is disabled if the "duid" is unknown (can only happen if dnsmasq is responsible for DHCPv6) and that DUIDs/IAIDs are always rendered in uppercase in the UI. Signed-off-by: David Härdeman <[email protected]>
This makes it easier to see straight away the correspondence between render() and renderLeases(). Signed-off-by: David Härdeman <[email protected]>
Instead of lease/lease6, table/table6, etc, use a consistent naming of the form "x4"/"x6". Also, fix a typo (note class "lases" for "table"). Signed-off-by: David Härdeman <[email protected]>
The "macaddr" variable is actually the list of ufpd vendor hints, so rename the variable to make this clearer. Signed-off-by: David Härdeman <[email protected]>
This is mostly in preparation for the following patches, and makes function signatures a bit simpler. Note also that there was an unused "hosts" variable in renderLeases(). Signed-off-by: David Härdeman <[email protected]>
Split the creation of the DHCPv4/DHCPv6 lease tables into separate functions in preparation for the following patches. Signed-off-by: David Härdeman <[email protected]>
Instead of going through rpcd, which digs through statefiles to find out which leases are active, communicate directly with odhcpd over ubus. This allows us to expose information which isn't otherwise available, like which interface a given lease belongs to and whether force-reconf is supported by a given client. It's also more efficient, and provides a small step towards a future where the odhcpd-statefile parsing code can be removed from rpcd. Signed-off-by: David Härdeman <[email protected]>
Similarly to the previous patch, this lets LuCI communicate directly with odhcpd to get DHCPv4 leases, which allows us to expose information like the interface for the lease, force-reconf status, and potentially more in the future. Signed-off-by: David Härdeman <[email protected]>
"Old" tables on top, "new" tables below |
|
@systemcrash I have the feeling that your javascript-fu is much better than mine, so feel free to go through this and see if it can be made more elegant... |
|
test-beef haha XD |
|
Could you add Perhaps we should distinguish the table info source - somehow hint why there are two different tables (dnsmsaq, odhcpd). Or merge the entries and use a mono table. |
| }, | ||
|
|
||
| createOdhcpdTable4(iface_leases4) { | ||
| console.log("DHCPv4 leases " + JSON.stringify(iface_leases4)); |
There was a problem hiding this comment.
not in the final product. but ok for now.
| else if (lease.valid <= 0) | ||
| exp = E('em', _('expired')); | ||
| else | ||
| exp = '%t'.format(lease.valid); |
There was a problem hiding this comment.
just use interpolated string.
There was a problem hiding this comment.
How do I do that for %t? That's a cbi.js addition to the String class, isn't it?
| else if (lease.valid <= 0) | ||
| exp = E('em', _('expired')); | ||
| else | ||
| exp = '%t'.format(lease.valid); |
|
|
||
| let host = null; | ||
| if (hint && lease.hostname && lease.hostname != hint[1] && lease.ip6addrs[0] != hint[1]) | ||
| host = '%s (%s)'.format(lease.hostname, hint[1]); |
| let host = null; | ||
|
|
||
| if (hint && lease.hostname && lease.hostname != hint[1]) | ||
| host = '%s (%s)'.format(lease.hostname, hint[1]); |
| }); | ||
|
|
||
| function DUIDtoMAC(duid) { | ||
| if (duid == null || duid == '') |
Yeah, that makes sense...
I don't think the tables can really be merged (and they might diverge even further in the future wrt the data they expose....for example, I'm planning to add a "remove lease" button to the odhcpd table)...but we could change the table titles? As in "Active DHCPv4 Leases" -> "Active DHCPv4 Leases (dnsmasq)" or "Active DHCPv4 leases (odhcpd)"? |
With a DUID including a lot of |
|
@systemcrash at least it wasn't DEAD (this time) |
|
@jow- (sorry, bit of braindump here)...since you seem to have done a lot of the heavy lifting on For context, I want to get Now, converting
So...should I just throw in the towel and focus on getting |
|
@Alphix did you want to bring this up to current or is this superseded? |
It needs further work, I think it might be material for after the next OpenWrt branch... Basically I think I need to get rpcd-mod-luci to talk to odhcpd first... Then I can think about more direct approaches after that... |
|
@Alphix What do you think about using |
Sure, see #8140
I don't have any opinion on that, but feel free to create a PR and ping @systemcrash |
|
While you are at it, a comment from my end. Currently using If we could get consinstent this, that would be great. |
5 participants
This is just a RFC for now (and it's based on top of #8083 rather than
master).Basically, it changes
luci-mod-statusso that it callsodhcpddirectly overubusto get the status of DHCPv4/DHCPv6 leases (yes, I know thatodhcpdisn't responsible for DHCPv4 in a default install).Before this change, all lease status is provided via the
rpcd-mod-luciplugin inrpcd, which will dig throughleasefilesto get the status of leases (and it'll still do that fordnsmasq-based leases). Communicating directly withodhcpdis quicker, simpler and provides access to information which wasn't exposed in the interface before (like per-lease interface information, force reconf status, etc).This isn't complete right now (for example, it'll render 4 tables, showing both what was returned from
odhcpdand what was returned fromrpcd, but that's for comparison, also needs more testing).However, I'm posting this here now so that I can get some early feedback. Ultimately I'd like to implement a similar change to
luci-mod-network/htdocs/luci-static/resources/view/network/dhcp.js, after which theodhcpd-specific code inrpcd-mod-luciwon't really be necessary anymore.It might look like a lot of duplication, but there's enough little differences between the data returned from
odhcpdandrpcd(and also the resulting HTML tables carry different information), that there's not much code that can be shared between the different table generation functions.Down the line, I guess this would also make it easier to split
40_dhcp.jsinto three separate files, one fordnsmasq, one forodhcpd-DHCPv4and one forodhcpd-DHCPv6(if that's desirable, cf. #6713).Signed-off-by: <[email protected]>row (viagit commit --signoff)<package name>: titlefirst line subject for packagesPKG_VERSIONin the Makefile