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

Feature request: luci-app-keepalived: add interface_up_down_delays option #7475

Open
grifos opened this issue Dec 16, 2024 · 5 comments
Open

Comments

@grifos
Copy link

grifos commented Dec 16, 2024

What would you like to see in luci?

The maintainer and keepalived expert is Florian @feckert

Would it be possible to add the keepalived interface_up_down_delays option to luci-app-keepalived?

Please note: I realize that to implement this there may also have to be changes to the keepalived package itself, for the UCI code (the actual functionality should already be present in keepalived). Please just let me know if I should also open a feature request in packages.

This keepalived feature allows for the configuration of VRRP master/backup switchover delays to interfaces up or down events.

It was mainly intended to prevent instability due to flapping links, but another major use case is to prevent downtime and unpredictable behavior when using STP, which is my use case.

In my tests, with STP enabled, a WAN side keepalived instance will switchover to primary as soon as a tracked LAN interface comes back up, however traffic will not pass due to STP delays. Also, the other VRRP node does not see the main node's VRRP messages and it remains stuck in master mode, even after STP has enabled the link, and only a keepalived restart fixes the problem.

The keepalived.conf syntax is as follows:

 interface_up_down_delays {
                ifname down_delay [up_delay]
                ifname2 down_delay [up_delay]
                ...
            }

More info can be found here: https://www.keepalived.org/manpage.html

Sadly I'm not a programmer, else I'd have been more than happy to submit a PR.

Thank you for all the work you do on OpenWrt. As someone wrote on Phoronix, you guys are doing God's work ;)

@feckert
Copy link
Member

feckert commented Dec 16, 2024

Since I don't have a setup, can you please check if the patch init script creates a valid configuration if you make the following changes to /etc/init.d/keepalived and /etc/config/keepalived.

Changes to /etc/init.d/keepalived:

diff --git a/net/keepalived/files/keepalived.init b/net/keepalived/files/keepalived.init
index cbbff4941..e9d5a4b5e 100644
--- a/net/keepalived/files/keepalived.init
+++ b/net/keepalived/files/keepalived.init
@@ -287,6 +287,25 @@ static_routes() {
        done
 }

+interface_up_down_delay() {
+       local device
+       local down_delay
+       local up_delay
+       local line
+
+       config_get device "$1" device
+       config_get down_delay "$1" down_delay
+       config_get up_delay "$1" up_delay
+
+       [ -z "$device" ] && return
+       [ -z "$down_delay" ] && return
+
+       line="${device} ${down_delay}"
+       [ -z "$up_delay" ] || line="${line} ${up_delay}"
+
+       printf '%b%s\n' "$INDENT_1" "$line" >> "$KEEPALIVED_CONF"
+}
+
 # Count 'vrrp_instance' with the given name ; called by vrrp_instance_check()
 vrrp_instance_name_count() {
        local name
@@ -609,6 +628,10 @@ process_config() {
        config_foreach_wrapper static_ipaddress
        config_section_close

+       config_section_open "interface_up_down_delay"
+       config_foreach_wrapper interface_up_down_delay
+       config_section_close
+
        config_section_open "static_routes"
        config_foreach_wrapper static_routes
        config_section_close

Add additional interface_up_down_delay section to to /etc/config/keepalived:

config interface_up_down_delay
    option device <device>
    option down_delay <number in seconds>
    option up_delay <number in seconds>

@grifos
Copy link
Author

grifos commented Dec 16, 2024

Hi Florian, thanks for the very fast reply!

Yes, it creates a valid keepalived.conf configuration, here's an excerpt:

	smtp_server 127.0.0.1
	router_id r2
}

static_ipaddress {
}

interface_up_down_delay {
	br-lan 10 30
}

static_routes {
}

vrrp_instance wan {
	authentication {

@grifos
Copy link
Author

grifos commented Dec 23, 2024

Correction to the above:

The correct syntax for the function is interface_up_down_delays (with the trailing "s"), like on the keepalived documentation.

@feckert
Copy link
Member

feckert commented Jan 2, 2025

@grifos I'll update the code when I get back to the office.

@grifos
Copy link
Author

grifos commented Jan 2, 2025

Thank you Florian.

One thing to note: the configured delay times have to be less than 2 x the VRRP advert interval, see the below logs. I think it's worth mentioning that on the Luci field.

Thu Jan 2 17:16:19 2025 daemon.info Keepalived_vrrp[29416]: wan: interface wan debounce timer(s) not less that 2 * advert_int - resetting

Thu Jan 2 17:16:19 2025 daemon.info Keepalived_vrrp[29416]: lan: interface br-lan.101 debounce timer(s) not less that 2 * advert_int - resetting

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants