8000 Add IPv6 support and make hostname resolution more robust by gucci-on-fleek · Pull Request #11 · mstojek/nlbw2collectd · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add IPv6 support and make hostname resolution more robust #11

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 22, 2025
Merged

Add IPv6 support and make hostname resolution more robust #11

merged 1 commit into from
Jun 22, 2025

Conversation

gucci-on-fleek
Copy link
Contributor

This commit adds support for dual-stack (IPv4 and IPv6) hosts by normalizing any IP addresses to a consistent format, and then summing the statistics for each host. This commit also makes the hostname resolution more robust by using the builtin getHostHints ubus procedure.

I've rewrote lots of the original code, so please let me know if there are any changes that you want me to make.

Also, I can't find any licences in this repository, so can you please add one? I'm okay with any open source licence, so pick whichever one you prefer.

@gucci-on-fleek
Add IPv6 support and make hostname resolution more robust
1eb4511 8000 
This commit adds support for dual-stack (IPv4 and IPv6) hosts by
normalizing any IP addresses to a consistent format, and then summing
the statistics for each host. This commit also makes the hostname
resolution more robust by using the builtin `getHostHints` ubus
procedure.
@mstojek
Copy link
Owner
mstojek commented Jun 22, 2025

Thank you for your changes. I will apply them and will test it on my system. I am using only IPv4 so I can not verify IPv6 behaviour. Did you test it with IPv4/IPv6 clients? Is this working ok?

I added the GPL 3 license to this project.

This plugin has been created as replacement for Iptmon plugin which stopped working. Iptmon was using separate plugin_id for IPv4 and IPv6 data (IPv4 RRD files were stored in iptables-mangle-iptmon_rx/tx directory while IPv6 RDD files were stored in ip6tables-mangle-iptmon_rx/tx directory). As I understand your code all data (IPv6 and IPv4) will go to the same directory - we will not separate IPv6 and IPv4 data - will it count traffic properly if we will be sending traffic to device by IPv4 and IPv6 at the same time (or maybe we wont have such issue)?

@mstojek mstojek merged commit 266e910 into mstojek:main Jun 22, 2025
@mstojek
Copy link
Owner
mstojek commented Jun 22, 2025

One more question, are you able to test if we still need following workaround?

    -- collectd can only handle signed 32-bit integers, so we'll wrap any
    -- values greater than this.
    value.tx_bytes   = (value.tx_bytes   + tx_bytes  ) % 0x7fffffff
    value.rx_bytes   = (value.rx_bytes   + rx_bytes  ) % 0x7fffffff
    value.tx_packets = (value.tx_packets + tx_packets) % 0x7fffffff
    value.rx_packets = (value.rx_packets + rx_packets) % 0x7fffffff

For sure this was needed on openwrt 21.0 with really old routers, but maybe we do not need this on latest openwrt versions with modern hardware?

@gucci-on-fleek
Copy link
Contributor Author

Thank you for your changes. I will apply them and will test it on my system. I am using only IPv4 so I can not verify IPv6 behaviour. Did you test it with IPv4/IPv6 clients? Is this working ok?

Yup, all my devices have IPv6 addresses, and everything works as expected.

I added the GPL 3 license to this project.

Perfect, thanks!

Iptmon was using separate plugin_id for IPv4 and IPv6 data (IPv4 RRD files were stored in iptables-mangle-iptmon_rx/tx directory while IPv6 RDD files were stored in ip6tables-mangle-iptmon_rx/tx directory). As I understand your code all data (IPv6 and IPv4) will go to the same directory - we will not separate IPv6 and IPv4 data

Correct.

will it count traffic properly if we will be sending traffic to device by IPv4 and IPv6 at the same time (or maybe we wont have such issue)?

Yes, if a device has both IPv4 and IPv6 addresses, get_hostname should convert both addresses to the same hostname, and then this code will combine both of the nlbw readings into a single value:

nlbw2collectd/nlbw2collectd.lua

Lines 96 to 104 in 266e910

-- collectd only accepts a single value for each
-- plugin_instance/type_instance, but with IPv4 and IPv6, a single host
-- can have multiple IPs, so we'll need to sum them here.
local value = values[client] or {
tx_bytes = 0,
tx_packets = 0,
rx_bytes = 0,
rx_packets = 0
}

One more question, are you able to test if we still need following workaround?

    -- collectd can only handle signed 32-bit integers, so we'll wrap any
    -- values greater than this.
    value.tx_bytes   = (value.tx_bytes   + tx_bytes  ) % 0x7fffffff
    value.rx_bytes   = (value.rx_bytes   + rx_bytes  ) % 0x7fffffff
    value.tx_packets = (value.tx_packets + tx_packets) % 0x7fffffff
    value.rx_packets = (value.rx_packets + rx_packets) % 0x7fffffff

For sure this was needed on openwrt 21.0 with really old routers, but maybe we do not need this on latest openwrt versions with modern hardware?

I just tried it now without that, and it didn't work properly—as soon as a host exceeded 2^31-1, the value got stuck at 0 forever. I've submitted a patch upstream, but 8000 there hasn't been a release since 2020, so I have no idea if/when that will get merged.

@mstojek
Copy link
Owner
mstojek commented Jun 23, 2025

Thank you for your responses. I also applied your code to my Openwrt system and it works stable and without issues.

I just tried it now without that, and it didn't work properly—as soon as a host exceeded 2^31-1, the value got stuck at 0 forever. collectd/collectd#4369, but there hasn't been a release since 2020, so I have no idea if/when that will get merged.

Thanks for opening issue against collectd. Perhaps one day they will include those changes.
Another option would be to submit this patch to Openwrt - they already are patching collectd so probably they can also add your patch.

gucci-on-fleek added a commit to gucci-on-fleek/openwrt-packages that referenced this pull request Jun 24, 2025
@gucci-on-fleek
collectd: support large integers with 32-bit Lua
80fb0a2 
Currently, if you set a value larger than 2^31-1 from Lua on a 32-bit
platform, collectd will truncate the value to 0, as this is what
`lua_tointeger` returns on Lua 5.1 with a 32-bit `lua_Integer`. This
commit modifies the `collectd-mod-lua` plugin to use `lua_tonumber` to
first get the value as a float, and then cast it to the appropriate (>32
bit) integer type, ensuring that large values are not truncated.

See also:
- collectd/collectd#4369
- mstojek/nlbw2collectd#11

Signed-off-by: Max Chernoff <git@maxchernoff.ca>
gucci-on-fleek added a commit to gucci-on-fleek/openwrt-packages that referenced this pull request Jun 24, 2025
@gucci-on-fleek
collectd: support large integers with 32-bit Lua
ae0c091 
Currently, if you set a value larger than 2^31-1 from Lua on a 32-bit
platform, collectd will truncate the value to 0, as this is what
`lua_tointeger` returns on Lua 5.1 with a 32-bit `lua_Integer`. This
commit modifies the `collectd-mod-lua` plugin to use `lua_tonumber` to
first get the value as a float, and then cast it to the appropriate (>32
bit) integer type, ensuring that large values are not truncated.

See also:
- collectd/collectd#4369
- mstojek/nlbw2collectd#11

Signed-off-by: Max Chernoff <git@maxchernoff.ca>
@gucci-on-fleek gucci-on-fleek mentioned this pull request Jun 24, 2025
Open
4 tasks
@gucci-on-fleek
Copy link
Contributor Author

@mstojek

Another option would be to submit this patch to Openwrt - they already are patching collectd so probably they can also add your patch.

Good idea, I've submitted it in openwrt/packages#26824.

@mstojek
Copy link
Owner
mstojek commented Jun 24, 2025

Great! I hope that they will apply your patch to the Openwrt!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@gucci-on-fleek @mstojek
0