Endless loop in FAULT state with two virtual_instances on same interface

Hello,

I am using keepalived-1.2.10 with lvs support disabled, so only for VRRP. I came upon the following situation:
- configure two distinct virtual instances on the same interface
- start keepalived. The two instances will transition to MASTER, as it is the only VR on the line
- unplug cable -> interface goes down -> both instances go to FAULT state
- plug-in cable -> interface goes up -> keepalived process goes into an endless loop, consuming CPU and becoming unresponsive


Due to the fact that it is quite a simple setup, I am a little surprised to encounter this, and not 100% confident that I didn't missconfigure something. The setup information is in the last part of the email; please let me know if I did something I wasn't suppose to.

I will continue with the inner details of the problem, as I found them.


From what I've seen, the endless loop is in set_vrrp_fd_bucket() ( called by new_vrrp_socket(), called by vrrp_fault()).
When the interface comes back up, the current/old fd_in and fd_out sockets of that vrrp instance are closed and the vrrp instance is removed from the fd bucket (vrrp_data->vrrp_index_fd). Then, the sockets are reopened (the file descriptors will be the same), and the instance re-added to the bucket (same fd_in -> same list in vrrp_data->vrrp_index_fd). When finally calling set_vrrp_fd_bucket(old_fd, vrrp), this does the following:

- goes to the list in the bucket of the old_fd (vrrp_data->vrrp_index_fd[old_fd%1024+1]

- iterates on the list starting with the head (the list hast two vrrp instances)
- removes the first one (vrrp1) from the list, updates its fd_in and fd_out to the newly opened ones (the same in this case), adds it to the bucket again (alloc_vrrp_fd_bucket(vrrp_ptr)) ---> ! because the fd's are the same, it will be added to the end of the same list!
- we now have vrrp2, vrrp1 in the current list
- iterates to vrrp2 and does the same thing -> vrrp1, vrrp2
- so on...

The solution that I found is to iterate the list starting form the tail. This way, you don't iterate over elements that will be added to the end of list during this process. The solution is in the git diff attached.

Please let me know what do you think. I admit that it is not tested thoroughly.

Thanks!
Catalin Gheorghe



Setup information:
0. Ubuntu 12.04, x86_64, 2 NIC -> eth0 and eth2

1. keepalived-1.2.10, clean, compiled from source


2. configured with

$ ./configure --prefix=/home/catalin/work/vrrp/keepalived/install-clean --disable-lvs --disable-lvs-syncd --enable-snmp
3. keepalived configuration file (VI_1 and VI_3 on same interface)
! Configuration File for keepalived
>
>
>vrrp_instance VI_1 {
>    state BACKUP
>    interface eth2
>    virtual_router_id 20
>    priority 100
>    advert_int 1
>    virtual_ipaddress {
>        192.168.0.200
>    }
>}
>
>
>vrrp_instance VI_2 {
>    state BACKUP
>    interface eth0
>    virtual_router_id 35
>    priority 200
>    advert_int 1
>    virtual_ipaddress {
>        10.10.10.10
>    }
>}
>
>
>vrrp_instance VI_3 {
>    state BACKUP
>    interface eth2
>    virtual_router_id 40
>    priority 200
>    advert_int 1
>    virtual_ipaddress {
>        20.20.20.20
>    }
>}
>
>
>4. keepalived cmd line arguments
>
>
>-P -D -d -n -l -f /home/catalin/work/vrrp/keepalived/install-clean/etc/keepalived/keepalived.conf.me -p /home/catalin/work/vrrp/keepalived/install-clean/sbin/keepalived.pid -r /home/catalin/work/vrrp/keepalived/install-clean/sbin/keepalived_vrrp.pid
>
>
>
>
>
>

------------------------------------------------------------------------------
Flow-based real-time traffic analytics software. Cisco certified tool.
Monitor traffic, SLAs, QoS, Medianet, WAAS etc. with NetFlow Analyzer
Customize your own dashboards, set traffic alerts and generate reports.
Network behavioral analysis & security monitoring. All-in-one tool.
http://pubads.g.doubleclick.net/gampad/clk?id=126839071&iu=/4140/ostg.clktrk

_______________________________________________
Keepalived-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/keepalived-devel

diff --git a/bin/keepalived b/bin/keepalived
index 062c99f..df9f933 100755
Binary files a/bin/keepalived and b/bin/keepalived differ
diff --git a/keepalived/vrrp/vrrp_index.c b/keepalived/vrrp/vrrp_index.c
index 049799d..629debf 100644
--- a/keepalived/vrrp/vrrp_index.c
+++ b/keepalived/vrrp/vrrp_index.c
@@ -88,11 +88,11 @@ void set_vrrp_fd_bucket(int old_fd, vrrp_t *vrrp)
 {
 	vrrp_t *vrrp_ptr;
 	element e;
-	element next;
+	element prev;
 	list l = &vrrp_data->vrrp_index_fd[old_fd%1024 + 1];
 
-	for (e = LIST_HEAD(l); e; e = next) {
-		next = e->next;
+	for (e = LIST_TAIL(l); e; e = prev) {
+		prev = e->prev;
 		vrrp_ptr =  ELEMENT_DATA(e);
 		if (vrrp_ptr->fd_in == old_fd) {
 			if (e->prev)
diff --git a/lib/list.h b/lib/list.h
index fe4b4c9..a67e102 100644
--- a/lib/list.h
+++ b/lib/list.h
@@ -45,6 +45,7 @@ struct _list {
 #define ELEMENT_NEXT(E)		((E) = (E)->next)
 #define ELEMENT_DATA(E)		((E)->data)
 #define LIST_HEAD(L)		((L)->head)
+#define LIST_TAIL(L)		((L)->tail)
 #define LIST_TAIL_DATA(L)	((L)->tail->data)
 #define LIST_ISEMPTY(L)		((L) == NULL || ((L)->head == NULL && (L)->tail == NULL))
 #define LIST_SIZE(V)		((V)->count)