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

VRRPv3 IPv6: persistent FAULT state (no IPv6 address for interface) #2275

Open
louis-6wind opened this issue Apr 3, 2023 · 8 comments
Open

Comments

@louis-6wind
Copy link
Contributor

louis-6wind commented Apr 3, 2023

Describe the bug
VRRPv3 IPv6 remains in a persistent FAULT state

To Reproduce

On a standalone VM machine

# Apply the following commands

ip -6 addr add  fd00:100::3/64 dev ens4
ip addr add 10.0.0.1/24 dev ens4

ip link set dev ens4 down
killall keepalived
sleep 2
cat>/etc/keepalived/keepalived.conf <<\EOF
global_defs
{
router_id router
dynamic_interfaces
}

vrrp_sync_group group15 {
group {
vrrp
}
}

vrrp_instance vrrp {
version 3
state BACKUP
interface ens4

use_vmac vrrp


garp_master_delay 5

virtual_router_id 15

priority 200
advert_int 1.0

virtual_ipaddress {
fd00:100::1/64
}

preempt_delay 0
}
EOF
keepalived -D
sleep 5
ip link set ens4 up
sleep 4
journalctl _PID=$(pgrep keepalived | tail -n1)

Expected behavior

Instance in MASTER state.

Keepalived version

Keepalived v2.2.7 (04/02,2023), git commit v2.2.7-148-g58be65ee

Copyright(C) 2001-2023 Alexandre Cassen, <[email protected]>

Built with kernel headers for Linux 4.15.18
Running on Linux 5.4.0-135-generic #152~18.04.2-Ubuntu SMP Tue Nov 29 08:23:49 UTC 2022
Distro: Ubuntu 18.04.1 LTS

configure options: CFLAGS=-g -O0 --prefix=/usr --sysconfdir=/etc --with-extra-cflags=-I/usr/include/libnl3 --with-extra-ldflags= --disable-lvs --with-init=systemd --host=x86_64-linux-gnu host_alias=x86_64-linux-gnu

Config options:  NFTABLES VRRP VRRP_AUTH VRRP_VMAC OLD_CHKSUM_COMPAT INIT=systemd SYSTEMD_NOTIFY

System options:  VSYSLOG MEMFD_CREATE IPV4_DEVCONF RTA_ENCAP RTA_EXPIRES RTA_NEWDST RTA_PREF FRA_SUPPRESS_PREFIXLEN FRA_SUPPRESS_IFGROUP FRA_TUN_ID RTAX_CC_ALGO RTAX_QUICKACK RTEXT_FILTER_SKIP_STATS FRA_L3MDEV FRA_UID_RANGE RTAX_FASTOPEN_NO_COOKIE RTA_VIA RTA_TTL_PROPAGATE IFA_FLAGS LWTUNNEL_ENCAP_MPLS LWTUNNEL_ENCAP_ILA IPTABLES NET_LINUX_IF_H_COLLISION VRRP_IPVLAN IFLA_LINK_NETNSID GLOB_BRACE GLOB_ALTDIRFUNC INET6_ADDR_GEN_MODE VRF SO_MARK

Distro (please complete the following information):

  • Name: Ubuntu
  • Version: 18.04-hwe
  • Architecture: x86_64

Details of any containerisation or hosted service (e.g. AWS)

NA

Configuration file:
See above
Notify and track scripts
NA

System Log entries

Apr 03 14:00:15 ubuntu1804hwe Keepalived_vrrp[2041]: Registering Kernel netlink reflector
Apr 03 14:00:15 ubuntu1804hwe Keepalived_vrrp[2041]: Registering Kernel netlink command channel
Apr 03 14:00:15 ubuntu1804hwe Keepalived_vrrp[2041]: Sync group group15 has only 1 virtual router(s) - this probably isn't what you want
Apr 03 14:00:15 ubuntu1804hwe Keepalived_vrrp[2041]: (vrrp) the first IPv6 VIP address should be link local
Apr 03 14:00:15 ubuntu1804hwe Keepalived_vrrp[2041]: use_vmac or no_accept/strict specified, but no firewall configured - using nftables
Apr 03 14:00:15 ubuntu1804hwe Keepalived_vrrp[2041]: Assigned address 10.0.0.1 for interface ens4
Apr 03 14:00:15 ubuntu1804hwe Keepalived_vrrp[2041]: (vrrp): Success creating VMAC interface vrrp
Apr 03 14:00:15 ubuntu1804hwe Keepalived_vrrp[2041]: Assigned address fe80::dced:1ff:fe70:f533 for interface vrrp
Apr 03 14:00:15 ubuntu1804hwe Keepalived_vrrp[2041]: (vrrp): entering FAULT state (interface ens4 down)
Apr 03 14:00:15 ubuntu1804hwe Keepalived_vrrp[2041]: (vrrp): entering FAULT state (interface vrrp down)
Apr 03 14:00:15 ubuntu1804hwe Keepalived_vrrp[2041]: (vrrp) entering FAULT state (no IPv6 address for interface)
Apr 03 14:00:15 ubuntu1804hwe Keepalived_vrrp[2041]: VRRP_Group(group15): Syncing vrrp to FAULT state
Apr 03 14:00:15 ubuntu1804hwe Keepalived_vrrp[2041]: (vrrp) entering FAULT state
Apr 03 14:00:15 ubuntu1804hwe Keepalived_vrrp[2041]: Registering gratuitous NDISC shared channel
Apr 03 14:00:15 ubuntu1804hwe Keepalived_vrrp[2041]: VRRP sockpool: [ifindex( 10), family(IPv6), proto(112), fd(13,14) multicast, address(ff02::12)]
Apr 03 14:00:20 ubuntu1804hwe Keepalived_vrrp[2041]: Netlink reports ens4 up
Apr 03 14:00:20 ubuntu1804hwe Keepalived_vrrp[2041]: Netlink reports vrrp up
Apr 03 14:00:21 ubuntu1804hwe Keepalived_vrrp[2041]: Assigned address fe80::dced:1ff:fe70:f533 for interface ens4
Apr 03 14:00:21 ubuntu1804hwe Keepalived_vrrp[2041]: Assigned address fe80::dced:1ff:fe70:f533 for interface vrrp

Did keepalived coredump?
No

@louis-6wind
Copy link
Contributor Author

I have applied a debug patch and I get:

Apr 03 14:05:18 ubuntu1804hwe Keepalived_vrrp[2391]: Registering Kernel netlink reflector
Apr 03 14:05:18 ubuntu1804hwe Keepalived_vrrp[2391]: Registering Kernel netlink command channel
Apr 03 14:05:18 ubuntu1804hwe Keepalived_vrrp[2391]: Sync group group15 has only 1 virtual router(s) - this probably isn't what you want
Apr 03 14:05:18 ubuntu1804hwe Keepalived_vrrp[2391]: (vrrp) the first IPv6 VIP address should be link local
Apr 03 14:05:18 ubuntu1804hwe Keepalived_vrrp[2391]: use_vmac or no_accept/strict specified, but no firewall configured - using nftables
Apr 03 14:05:18 ubuntu1804hwe Keepalived_vrrp[2391]: Assigned address 10.0.0.1 for interface ens4
Apr 03 14:05:18 ubuntu1804hwe Keepalived_vrrp[2391]: (vrrp): Success creating VMAC interface vrrp
Apr 03 14:05:18 ubuntu1804hwe Keepalived_vrrp[2391]: Link-local from make_link_local_address
Apr 03 14:05:18 ubuntu1804hwe Keepalived_vrrp[2391]: Adding link local address fe80::dced:1ff:fe70:f533 for vrrp, interface 11
Apr 03 14:05:18 ubuntu1804hwe Keepalived_vrrp[2391]: Assigned address fe80::dced:1ff:fe70:f533 for interface vrrp
Apr 03 14:05:18 ubuntu1804hwe Keepalived_vrrp[2391]: (vrrp) num_script_if_fault 0
Apr 03 14:05:18 ubuntu1804hwe Keepalived_vrrp[2391]: (vrrp): entering FAULT state (interface ens4 down)
Apr 03 14:05:18 ubuntu1804hwe Keepalived_vrrp[2391]: (vrrp) num_script_if_fault 1
Apr 03 14:05:18 ubuntu1804hwe Keepalived_vrrp[2391]: (vrrp): entering FAULT state (interface vrrp down)
Apr 03 14:05:18 ubuntu1804hwe Keepalived_vrrp[2391]: (vrrp) num_script_if_fault 2
Apr 03 14:05:18 ubuntu1804hwe Keepalived_vrrp[2391]: (vrrp) entering FAULT state (no IPv6 address for interface)
Apr 03 14:05:18 ubuntu1804hwe Keepalived_vrrp[2391]: (vrrp) num_script_if_fault 3
Apr 03 14:05:18 ubuntu1804hwe Keepalived_vrrp[2391]: VRRP_Group(group15): Syncing vrrp to FAULT state
Apr 03 14:05:18 ubuntu1804hwe Keepalived_vrrp[2391]: (vrrp) entering FAULT state
Apr 03 14:05:18 ubuntu1804hwe Keepalived_vrrp[2391]: Registering gratuitous NDISC shared channel
Apr 03 14:05:18 ubuntu1804hwe Keepalived_vrrp[2391]: VRRP sockpool: [ifindex( 11), family(IPv6), proto(112), fd(13,14) multicast, address(ff02::12)]
Apr 03 14:05:23 ubuntu1804hwe Keepalived_vrrp[2391]: Netlink reports ens4 up
Apr 03 14:05:23 ubuntu1804hwe Keepalived_vrrp[2391]: (vrrp) num_script_if_fault 2
Apr 03 14:05:23 ubuntu1804hwe Keepalived_vrrp[2391]: Netlink reports vrrp up
Apr 03 14:05:23 ubuntu1804hwe Keepalived_vrrp[2391]: (vrrp) num_script_if_fault 1
Apr 03 14:05:24 ubuntu1804hwe Keepalived_vrrp[2391]: Assigned address fe80::dced:1ff:fe70:f533 for interface ens4
Apr 03 14:05:24 ubuntu1804hwe Keepalived_vrrp[2391]: Replacing link local address fe80::dced:1ff:fe70:f533 by fe80::dced:1ff:fe70:f533 for vrrp, interface 11
Apr 03 14:05:24 ubuntu1804hwe Keepalived_vrrp[2391]: Assigned address fe80::dced:1ff:fe70:f533 for interface vrrp

Debug patch:

diff --git a/keepalived/vrrp/vrrp.c b/keepalived/vrrp/vrrp.c
index 9448b38a..30cb6951 100644
--- a/keepalived/vrrp/vrrp.c
+++ b/keepalived/vrrp/vrrp.c
@@ -2346,8 +2346,11 @@ del_vrrp_from_interface(vrrp_t *vrrp, interface_t *ifp)
 
 	list_for_each_entry_safe(top, top_tmp, &ifp->tracking_vrrp, e_list) {
 		if (top->obj.vrrp == vrrp && top->type == TRACK_VRRP_DYNAMIC) {
-			if (!IF_ISUP(ifp) && !__test_bit(VRRP_FLAG_DONT_TRACK_PRIMARY, &vrrp->flags))
+			if (!IF_ISUP(ifp) && !__test_bit(VRRP_FLAG_DONT_TRACK_PRIMARY, &vrrp->flags)) {
 				vrrp->num_script_if_fault--;
+				log_message(LOG_INFO, "(%s) num_script_if_fault %u",
+						vrrp->iname, vrrp->num_script_if_fault);
+			}
 			free_tracking_obj(top);
 			break;
 		}
@@ -4677,8 +4680,11 @@ vrrp_complete_init(void)
 	 * may include link down/link up, and these will alter num_script_if_fault
 	 * but that is initialised in initialise_tracking_priorities() called below.
 	 * We therefore need to clear num_script_if_fault here. */
-	list_for_each_entry(vrrp, &vrrp_data->vrrp, e_list)
+	list_for_each_entry(vrrp, &vrrp_data->vrrp, e_list) {
 		vrrp->num_script_if_fault = vrrp->num_config_faults;
+		log_message(LOG_INFO, "(%s) num_script_if_fault %u",
+				vrrp->iname, vrrp->num_script_if_fault);
+	}
 
 	/* Remove any VIPs from the list of default addresses for interfaces */
 	if (!reload)
diff --git a/keepalived/vrrp/vrrp_if.c b/keepalived/vrrp/vrrp_if.c
index 18f69828..3e580d27 100644
--- a/keepalived/vrrp/vrrp_if.c
+++ b/keepalived/vrrp/vrrp_if.c
@@ -1452,6 +1452,8 @@ cleanup_lost_interface(interface_t *ifp)
 		    __test_bit(VRRP_FLAG_DUPLICATE_VRID_FAULT, &vrrp->flags)) {
 			__clear_bit(VRRP_FLAG_DUPLICATE_VRID_FAULT, &vrrp->flags);
 			vrrp->num_script_if_fault--;
+			log_message(LOG_INFO, "(%s) num_script_if_fault %u",
+					vrrp->iname, vrrp->num_script_if_fault);
 		}
 #endif
 
@@ -1642,6 +1644,8 @@ update_added_interface(interface_t *ifp)
 					__set_bit(VRRP_FLAG_DUPLICATE_VRID_FAULT, &vrrp->flags);
 					log_message(LOG_INFO, "VRID conflict between %s and %s IPv%d vrid %d",
 							vrrp->iname, vrrp1->iname, vrrp->family == AF_INET ? 4 : 6, vrrp->vrid);
+					log_message(LOG_INFO, "(%s) num_script_if_fault %u",
+							vrrp->iname, vrrp->num_script_if_fault);
 					break;
 				}
 			}
@@ -1654,6 +1658,8 @@ update_added_interface(interface_t *ifp)
 					log_message(LOG_INFO, "(%s) interface %s is down",
 							vrrp->iname, vrrp->configured_ifp->base_ifp->ifname);
 					vrrp->num_script_if_fault++;
+					log_message(LOG_INFO, "(%s) num_script_if_fault %u",
+							vrrp->iname, vrrp->num_script_if_fault);
 				}
 			}
 
diff --git a/keepalived/vrrp/vrrp_scheduler.c b/keepalived/vrrp/vrrp_scheduler.c
index 56dd4113..f6187933 100644
--- a/keepalived/vrrp/vrrp_scheduler.c
+++ b/keepalived/vrrp/vrrp_scheduler.c
@@ -671,6 +671,8 @@ try_up_instance(vrrp_t *vrrp, bool leaving_init)
 			return;
 	}
 	else if (--vrrp->num_script_if_fault || vrrp->num_script_init) {
+		log_message(LOG_INFO, "(%s) num_script_if_fault %u",
+				vrrp->iname, vrrp->num_script_if_fault);
 		if (!vrrp->num_script_if_fault) {
 			if (vrrp->sync) {
 				vrrp->sync->num_member_fault--;
@@ -680,6 +682,9 @@ try_up_instance(vrrp_t *vrrp, bool leaving_init)
 		}
 
 		return;
+	} else {
+		log_message(LOG_INFO, "(%s) num_script_if_fault %u",
+				vrrp->iname, vrrp->num_script_if_fault);
 	}
 
 	if (vrrp->wantstate == VRRP_STATE_MAST && vrrp->base_priority == VRRP_PRIO_OWNER) {
diff --git a/keepalived/vrrp/vrrp_track.c b/keepalived/vrrp/vrrp_track.c
index e6253b4a..c3afffc1 100644
--- a/keepalived/vrrp/vrrp_track.c
+++ b/keepalived/vrrp/vrrp_track.c
@@ -551,6 +551,8 @@ down_instance(vrrp_t *vrrp)
 
 		if (vrrp->sync && vrrp->sync->num_member_fault++ == 0)
 			vrrp_sync_fault(vrrp);
+		log_message(LOG_INFO, "(%s) num_script_if_fault %u",
+				vrrp->iname, vrrp->num_script_if_fault);
 	}
 }
 
@@ -669,6 +671,8 @@ initialise_track_script_state(tracked_sc_t *tsc, vrrp_t *vrrp)
 			vrrp->num_script_if_fault++;
 			log_message(LOG_INFO, "(%s): entering FAULT state due to script %s", vrrp->iname, tsc->scr->sname);
 			vrrp->state = VRRP_STATE_FAULT;
+			log_message(LOG_INFO, "(%s) num_script_if_fault %u",
+					vrrp->iname, vrrp->num_script_if_fault);
 		}
 		return;
 	}
@@ -705,11 +709,15 @@ initialise_track_bfd_state(tracked_bfd_t *tbfd, vrrp_t *vrrp)
 				vrrp->total_priority += tbfd->weight * multiplier;
 			else if (!tbfd->weight) {
 				vrrp->num_script_if_fault++;
+				log_message(LOG_INFO, "(%s) num_script_if_fault %u",
+						vrrp->iname, vrrp->num_script_if_fault);
 				vrrp->state = VRRP_STATE_FAULT;
 			}
 		}
 	} else if (tbfd->bfd->bfd_up == tbfd->weight_reverse) {
 		vrrp->num_script_if_fault++;
+		log_message(LOG_INFO, "(%s) num_script_if_fault %u",
+				vrrp->iname, vrrp->num_script_if_fault);
 		vrrp->state = VRRP_STATE_FAULT;
 	}
 }
@@ -736,6 +744,8 @@ initialise_interface_tracking_priorities(void)
 					log_message(LOG_INFO, "(%s): entering FAULT state (interface %s down)", vrrp->iname, ifp->ifname);
 					vrrp->state = VRRP_STATE_FAULT;
 					vrrp->num_script_if_fault++;
+					log_message(LOG_INFO, "(%s) num_script_if_fault %u",
+							vrrp->iname, vrrp->num_script_if_fault);
 				}
 			} else if (IF_FLAGS_UP(ifp)) {
 				if (top->weight > 0)
@@ -766,6 +776,8 @@ initialise_vrrp_file_tracking_priorities(void)
 				log_message(LOG_INFO, "(%s): entering FAULT state (tracked file %s has status %i)", vrrp->iname, tfile->fname, status);
 				vrrp->state = VRRP_STATE_FAULT;
 				vrrp->num_script_if_fault++;
+				log_message(LOG_INFO, "(%s) num_script_if_fault %u",
+						vrrp->iname, vrrp->num_script_if_fault);
 			}
 			else
 				vrrp->total_priority += (status > 253 ? 253 : status);
@@ -796,6 +808,8 @@ initialise_process_tracking_priorities(void)
 							    , vrrp->iname, tprocess->pname);
 					vrrp->state = VRRP_STATE_FAULT;
 					vrrp->num_script_if_fault++;
+					log_message(LOG_INFO, "(%s) num_script_if_fault %u",
+							vrrp->iname, vrrp->num_script_if_fault);
 				}
 			}
 			else if (tprocess->have_quorum) {
@@ -827,6 +841,8 @@ initialise_vrrp_tracking_priorities(vrrp_t *vrrp)
 				    , vrrp->iname, vrrp->family == AF_INET ? 4 : 6);
 		vrrp->state = VRRP_STATE_FAULT;
 		vrrp->num_script_if_fault++;
+		log_message(LOG_INFO, "(%s) num_script_if_fault %u",
+				vrrp->iname, vrrp->num_script_if_fault);
 	}
 
 	/* Initialise the vrrp instance's tracked scripts */
diff --git a/keepalived/vrrp/vrrp_vmac.c b/keepalived/vrrp/vrrp_vmac.c
index 86f53bf5..1c03a953 100644
--- a/keepalived/vrrp/vrrp_vmac.c
+++ b/keepalived/vrrp/vrrp_vmac.c
@@ -123,6 +123,7 @@ del_link_local_address(interface_t *ifp)
 static bool
 change_link_local_address(interface_t *ifp, struct in6_addr *old_addr, struct in6_addr *new_addr)
 {
+	char old_addr_str[INET6_ADDRSTRLEN], new_addr_str[INET6_ADDRSTRLEN];
 	ip_address_t ipaddress;
 
 	memset(&ipaddress, 0, sizeof(ipaddress));
@@ -135,6 +136,11 @@ change_link_local_address(interface_t *ifp, struct in6_addr *old_addr, struct in
 	ipaddress.ifa.ifa_prefixlen = 64;
 	ipaddress.ifa.ifa_index = ifp->ifindex;
 
+	inet_ntop(AF_INET6, old_addr, old_addr_str, sizeof(old_addr_str));
+	inet_ntop(AF_INET6, new_addr, new_addr_str, sizeof(new_addr_str));
+	log_message(LOG_INFO, "Replacing link local address %s by %s for %s, interface %u",
+			old_addr_str, new_addr_str, ifp->ifname, ifp->ifindex);
+
 	if (netlink_ipaddress(&ipaddress, IPADDRESS_DEL) != 1)
 		log_message(LOG_INFO, "Deleting link-local address from vmac failed");
 	else
@@ -239,14 +245,24 @@ set_link_local_address(const vrrp_t *vrrp)
 	 * MAC address.
 	 * This is so that VRRP advertisements will be sent from a non-VIP address, but
 	 * using the VRRP MAC address */
+	char addr_str[INET6_ADDRSTRLEN];
 	struct in6_addr addr;
 
-	if (vrrp->saddr.ss_family == AF_INET6)
+	if (vrrp->saddr.ss_family == AF_INET6) {
 		addr = PTR_CAST_CONST(struct sockaddr_in6, &vrrp->saddr)->sin6_addr;
-	else if (!IN6_IS_ADDR_UNSPECIFIED(&vrrp->configured_ifp->sin6_addr))
+		log_message(LOG_INFO, "Link-local from vrrp->saddr->sin6_addr");
+	} else if (!IN6_IS_ADDR_UNSPECIFIED(&vrrp->configured_ifp->sin6_addr)) {
 		addr = vrrp->configured_ifp->sin6_addr;
-	else
+		log_message(LOG_INFO, "Link-local from vrrp->configured_ifp->sin6_addr");
+	}
+	else {
 		make_link_local_address(&addr, vrrp->configured_ifp->base_ifp->hw_addr);
+		log_message(LOG_INFO, "Link-local from make_link_local_address");
+	}
+
+	inet_ntop(AF_INET6, &addr, addr_str, sizeof(addr_str));
+	log_message(LOG_INFO, "Adding link local address %s for %s, interface %u",
+			addr_str, vrrp->iname, vrrp->ifp->ifindex);
 
 	return add_link_local_address(vrrp->ifp, &addr);
 }

@louis-6wind
Copy link
Contributor Author

This is a regression from 4c7a94a

@louis-6wind
Copy link
Contributor Author

Fix for the issue #2277

@pqarmitage
Copy link
Collaborator

I am currently investigating this. Unfortunately the patch in #2277 is not correct. The root cause of the problem is that the vrrp instance has not been added to the list of track interfaces of the VMAC interface prior to the address being added. There is a further problem that the address is not copied from the interface to the VRRP instance if the VRRP instance is in fault state.

I have the basis of a patch to resolve this, but am currently checking that it doesn't have any untoward side effects.

@louis-6wind
Copy link
Contributor Author

louis-6wind commented Apr 13, 2023

@pqarmitage. Thank you for your help. Could you share with me the patch you are testing so that I can help in testing?

I think that the num_script_if_fault counter is not very convenient. We don't easily know what has incremented and decremented the counter. And the counter could go back to zero for a reason in case of a bug. What do you think of replacing it with bit flags ? For example:

uint16_t flags_script_if_fault;

#define SCRIPT_IF_FAULT_INTERFACE_DOWN 0x1
#define SCRIPT_IF_FAULT_BASE_INTERFACE_DOWN 0x2
#define SCRIPT_IF_FAULT_BASE_DUPLICATE_VRID 0x4
(...)

When a fault is found, the bit is set to 1 and reset when it recovers.

@pqarmitage
Copy link
Collaborator

The patch at the moment is:

diff --git a/keepalived/core/keepalived_netlink.c b/keepalived/core/keepalived_netlink.c
index 4b9058ca..fa38df50 100644
--- a/keepalived/core/keepalived_netlink.c
+++ b/keepalived/core/keepalived_netlink.c
@@ -1006,7 +1006,6 @@ netlink_if_address_filter(__attribute__((unused)) struct sockaddr_nl *snl, struc
                                                            vrrp->family == AF_INET ? VRRP_CONFIGURED_IFP(vrrp) :
 #endif
                                                            vrrp->ifp) &&
-                                                   vrrp->num_script_if_fault &&
                                                    vrrp->family == ifa->ifa_family &&
                                                    vrrp->saddr.ss_family == AF_UNSPEC &&
                                                    (!__test_bit(VRRP_FLAG_SADDR_FROM_CONFIG, &vrrp->flags) || is_tracking_saddr)) {
@@ -1015,7 +1014,8 @@ netlink_if_address_filter(__attribute__((unused)) struct sockaddr_nl *snl, struc
                                                                inet_ip4tosockaddr(addr.in, &vrrp->saddr);
                                                        else
                                                                inet_ip6tosockaddr(addr.in6, &vrrp->saddr);
-                                                       try_up_instance(vrrp, false);
+                                                       if (vrrp->num_script_if_fault)
+                                                               try_up_instance(vrrp, false);
                                                }
 #ifdef _HAVE_VRRP_VMAC_
                                                /* If IPv6 link local and vmac doesn't have an address or we have
diff --git a/keepalived/vrrp/vrrp.c b/keepalived/vrrp/vrrp.c
index 9448b38a..1660b841 100644
--- a/keepalived/vrrp/vrrp.c
+++ b/keepalived/vrrp/vrrp.c
@@ -3604,6 +3604,18 @@ vrrp_complete_instance(vrrp_t * vrrp)
                __clear_bit(VRRP_VMAC_XMITBASE_BIT, &vrrp->flags);
        }
 
+
+       /* If the VRRP instance does not have an address, but the interface does, then
+        * copy address from interface to VRRP instance. */
+log_message(LOG_INFO, "%s: vrrp->saddr.ss_family %d, vrrp->family %d if IPv4 %x IPv6 %x", vrrp->iname, vrrp->saddr.ss_family, vrrp->family, ifp->sin_addr.s_addr, ifp->sin6_addr.s6_addr32[0]);
+       if (vrrp->saddr.ss_family == AF_UNSPEC) {
+               if (vrrp->family == AF_INET) {
+                       if (ifp->sin_addr.s_addr != INADDR_ANY)
+                               inet_ip4tosockaddr(&ifp->sin_addr, &vrrp->saddr);
+               } else if (!IN6_IS_ADDR_UNSPECIFIED(&ifp->sin6_addr))
+                       inet_ip6tosockaddr(&ifp->sin6_addr, &vrrp->saddr);
+       }
+
        if (__test_bit(VRRP_VMAC_BIT, &vrrp->flags)
 #ifdef _HAVE_VRRP_IPVLAN_
            || __test_bit(VRRP_IPVLAN_BIT, &vrrp->flags)
@@ -3667,7 +3679,8 @@ vrrp_complete_instance(vrrp_t * vrrp)
                }
 
                /* Add this instance to the vmac interface */
-               add_vrrp_to_interface(vrrp, vrrp->ifp, __test_bit(VRRP_FLAG_DONT_TRACK_PRIMARY, &vrrp->flags) ? VRRP_NOT_TRACK_IF : 0, false, true, TRACK_VRRP);
+log_message(LOG_INFO, "Not adding %s instance to if %s", vrrp->iname, vrrp->ifp->ifname);
+//             add_vrrp_to_interface(vrrp, vrrp->ifp, __test_bit(VRRP_FLAG_DONT_TRACK_PRIMARY, &vrrp->flags) ? VRRP_NOT_TRACK_IF : 0, false, true, TRACK_VRRP);
        }
 #endif
 
diff --git a/keepalived/vrrp/vrrp_vmac.c b/keepalived/vrrp/vrrp_vmac.c
index 56421389..e4278f59 100644
--- a/keepalived/vrrp/vrrp_vmac.c
+++ b/keepalived/vrrp/vrrp_vmac.c
@@ -89,7 +89,7 @@ add_link_local_address(interface_t *ifp, struct in6_addr* sin6_addr)
        }
 
        /* Save the new address */
-       ifp->sin6_addr = ipaddress.u.sin6_addr;
+//     ifp->sin6_addr = ipaddress.u.sin6_addr;
 
        return true;
 }
@@ -115,7 +115,7 @@ del_link_local_address(interface_t *ifp)
                return false;
        }
 
-       CLEAR_IP6_ADDR(&ifp->sin6_addr);
+//     CLEAR_IP6_ADDR(&ifp->sin6_addr);
 
        return true;
 }
@@ -141,13 +141,13 @@ change_link_local_address(interface_t *ifp, struct in6_addr *old_addr, struct in
 
        if (netlink_ipaddress(&ipaddress, IPADDRESS_DEL) != 1)
                log_message(LOG_INFO, "Deleting link-local address from vmac failed");
-       else
-               CLEAR_IP6_ADDR(&ifp->sin6_addr);
+//     else
+//             CLEAR_IP6_ADDR(&ifp->sin6_addr);
 
        ipaddress.u.sin6_addr = *new_addr;
        if (netlink_ipaddress(&ipaddress, IPADDRESS_ADD) != 1) {
                log_message(LOG_INFO, "Adding link-local address to vmac failed");
-               CLEAR_IP6_ADDR(&ifp->sin6_addr);
+//             CLEAR_IP6_ADDR(&ifp->sin6_addr);
 
                return false;
        }
@@ -167,7 +167,7 @@ replace_link_local_address(interface_t *ifp)
                return false;
 
        /* Save the new address */
-       ifp->sin6_addr = ipaddress_new;
+//     ifp->sin6_addr = ipaddress_new;
 
        return true;
 }
@@ -399,6 +399,9 @@ netlink_link_add_vmac(vrrp_t *vrrp, const interface_t *old_interface)
                        ifp->base_ifp = ifp;
                }
 
+log_message(LOG_INFO, "Adding %s instance to if %s", vrrp->iname, vrrp->ifp->ifname);
+add_vrrp_to_interface(vrrp, ifp, __test_bit(VRRP_FLAG_DONT_TRACK_PRIMARY, &vrrp->flags) ? VRRP_NOT_TRACK_IF : 0, false, true, TRACK_VRRP);
+
                /* If we do anything that might cause the interface state to change, we must
                 * read the reflected netlink messages to ensure that the link status doesn't
                 * get updated by out of date queued messages */

With regard to your suggestion:

What do you think of replacing it with bit flags ? For example:

uint16_t flags_script_if_fault;

#define SCRIPT_IF_FAULT_INTERFACE_DOWN 0x1
#define SCRIPT_IF_FAULT_BASE_INTERFACE_DOWN 0x2
#define SCRIPT_IF_FAULT_BASE_DUPLICATE_VRID 0x4
(...)

I am happy with the idea of changing the above to flags, but a counter is still needed for script faults, since there could be multiple scripts that are in fault state.

@louis-6wind
Copy link
Contributor Author

It seems there is also CLEAR_IP6_ADDR(&ifp->sin6_addr); to remove in add_link_local_address()

It works on my side (I did not do some extensive tests)

@pqarmitage
Copy link
Collaborator

There is an update to the patch above required:

diff --git a/keepalived/vrrp/vrrp.c b/keepalived/vrrp/vrrp.c
index 8ced28f9..c85d8190 100644
--- a/keepalived/vrrp/vrrp.c
+++ b/keepalived/vrrp/vrrp.c
@@ -3609,7 +3609,7 @@ vrrp_complete_instance(vrrp_t * vrrp)
        if (!ifp)
                ifp = vrrp->ifp;
 log_message(LOG_INFO, "%s: vrrp->saddr.ss_family %d, vrrp->family %d if IPv4 %x IPv6 %x", vrrp->iname, vrrp->saddr.ss_family, vrrp->family, ifp->sin_addr.s_addr, ifp->sin6_addr.s6_addr32[0]);
-       if (vrrp->saddr.ss_family == AF_UNSPEC) {
+       if (vrrp->saddr.ss_family == AF_UNSPEC && ifp) {
                if (vrrp->family == AF_INET) {
                        if (ifp->sin_addr.s_addr != INADDR_ANY)
                                inet_ip4tosockaddr(&ifp->sin_addr, &vrrp->saddr);

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