shared-state-publish_dnsmasq_leases recognize IPv6 when IPv6 leases are present

[ilario]

Fix #969
I still have to test the modified code.
@pony1k can you test this?

[codecov-commenter]

Codecov Report

Merging #975 (ee65122) into master (8de4eb1) will increase coverage by 0.15%.
The diff coverage is n/a.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##           master     #975      +/-   ##
==========================================
+ Coverage   77.71%   77.87%   +0.15%     
==========================================
  Files          52       52              
  Lines        4362     4388      +26     
==========================================
+ Hits         3390     3417      +27     
+ Misses        972      971       -1     

Impacted Files	Coverage Δ
...i-unstuck-wa/files/usr/lib/lua/wifi_unstuck_wa.lua	100.00% <0.00%> (ø)
...kages/lime-system/files/usr/lib/lua/lime/utils.lua	85.79% <0.00%> (+0.29%)	⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[ilario]

Just realized that the if statements can be rearranged in a more useful way, will improve in a few days.

[ilario]

Now the if sentences are more useful.
@pony1k can you test if this works?
Thanks!!!

[pony1k]

The dhcp.leases file contains lines with hostname *, i.e.

1675107518 12:34:65:3d:ce:2b 10.51.147.128 * 01:12:34:65:3d:ce:2b
1675106752 ab:cd:15:8a:62:93 10.51.147.146 * 01:ab:cd:15:8a:62:93

It is correct that we don't want to add * into the hostTable. But I think we still want to add them into the leasesTable, which will not happen with this arrangement of if-statements.

[ilario]

Ah ok, it makes sense, thanks! I am going to correct this today.

[ilario]

Just updated, please check it.
Thanks!

[G10h4ck]

looks ok, please test it both with anygw and without, if it works well we can merge it

[pony1k]

I have tested it on different routers with and without anygw. It works fine except one thing: As stated in the man-page, when dnsmasq calls the script with add for an ipv6-lease, it will pass the duid of the host instead of the mac-address. The duid then appears in the /tmp/dhcp.hosts_remote-file in other nodes, in the place where the mac-address would be. The man-page of dnsmasq states that in this case, the duid should be prefixed with id:. I.e.
id:00:01:00:01:29:9d:9c:12:00:4c:e0:ab:cd:ef,[fd43:1508:3393:fe00::d312]
instead of
00:01:00:01:29:9d:9c:12:00:4c:e0:ab:cd:ef,[fd43:1508:3393:fe00::d312]

[ilario]

@pony1k with the current code of this PR, that should not appear at all: a line like

1674755842 1276192316 fd70:d3e4:df63:184d::ccbf katze 00:04:eb:cf:8f:90:ce:2c:e3:20:12:55:c6:fe:9b:c7:16:ee

does not get added to leasesTable which is then used for generating /tmp/dhcp.hosts_remote as it will match the not tonumber(second field of the string that usually is the MAC).

But I think I got what you mean: that also this line should be added but using the last field prefixed with id:.
I can add that over the next days :)
Thanks!

[pony1k]

does not get added to leasesTable which is then used for generating /tmp/dhcp.hosts_remote as it will match the not tonumber(second field of the string that usually is the MAC).

It does not get added to leasesTable, but it still ends up in the dnsmasq-leases.json. Look in the add_lease-function. This function gets called when the script is run with add as first argument.

But I think I got what you mean: that also this line should be added but using the last field prefixed with id:. I can add that over the next days :)

I'm not sure if it should. Maybe it's fine to not share ipv6-leases, since collisions are very unlikely. I haven't reflected on this.

[ilario]

does not get added to leasesTable which is then used for generating /tmp/dhcp.hosts_remote as it will match the not tonumber(second field of the string that usually is the MAC).

It does not get added to leasesTable, but it still ends up in the dnsmasq-leases.json. Look in the add_lease-function. This function gets called when the script is run with add as first argument.

Ooh, ok!
I will modify that also there then!

But I think I got what you mean: that also this line should be added but using the last field prefixed with id:. I can add that over the next days :)

I'm not sure if it should. Maybe it's fine to not share ipv6-leases, since collisions are very unlikely. I haven't reflected on this.

Meh, it shouldn't harm neither.

[G10h4ck]

I'm not sure if it should. Maybe it's fine to not share ipv6-leases, since collisions are very unlikely. I haven't reflected on this.

Yeah unless a very conflict prone assignation algorithm is used (never saw anything like that, plus it make no sense at all in a mesh network), no need to share IPv6 leases as the collision is so unlikely that also the RFC standards take that for granted. Most probably we didn't share those also with this motivation. As it is not needed, avoid sharing them is better, so the global state is lighter, less RAM is used, less CPU is used, less traffic is generated, everything works better.

[ilario]

Ok, I added the filter also to the add_lease function, for leaving out the IPv6.
@pony1k @G10h4ck do you think is ok now?

[G10h4ck]

Looks good to me, @pony1k can you test this and report back so we can merge it before re-targeting?

[pony1k]

I have tested it. It works fine. Thank you!

[ilario]

Thank you both!