ipvsadm: fix list_daemon to handle master/backup status in either position

PATCH In review Ryan O'Hara 2 participants 3 messages Feb 9, 2012 IPVS

Share & tools

Ryan O'Hara

author

2012-02-09 15:50 UTC #0 lore raw

PATCH 1/1 ipvsadm: fix list_daemon to handle master/backup status in either position

Attached is a patch that fixes the list_daemon function such that it does not assume that the master sync daemon status is always in the first position and master sync daemon status is always in the second position.

If libipvs uses the netlink interface to retrieve sync daemon status, the results are not guaranteed to follow this ordering. As explained in a previous email, if libipvs uses the netlink interface to retrieve sync daemon status while only a backup sync daemon is running, the backup sync daemon status will but in the first position (index 0). This differs from the getsockopt interface, which would always put master sync daemon status in first position and backup sync daemon status in the second position, even when only backup sync daemon exists. Solution is to make ipvsadm check both elements of the array for master and backup.

Ryan

ipvsadm.c +9 -6

Index: ipvsadm.c
===================================================================
@@ -1631,16 +1631,19 @@
 static void list_daemon(void)
 {
 	ipvs_daemon_t *u;
+	int i;
 
 	if (!(u = ipvs_get_daemon()))
 		exit(1);
 
-	if (u[0].state & IP_VS_STATE_MASTER)
-		printf("master sync daemon (mcast=%s, syncid=%d)\n",
-		       u[0].mcast_ifn, u[0].syncid);
-	if (u[1].state & IP_VS_STATE_BACKUP)
-		printf("backup sync daemon (mcast=%s, syncid=%d)\n",
-		       u[1].mcast_ifn, u[1].syncid);
+	for (i = 0; i < 2; i++) {
+		if (u[i].state & IP_VS_STATE_MASTER)
+			printf("master sync daemon (mcast=%s, syncid=%d)\n",
+			       u[i].mcast_ifn, u[i].syncid);
+		if (u[i].state & IP_VS_STATE_MASTER)
+			printf("master sync daemon (mcast=%s, syncid=%d)\n",
+			       u[i].mcast_ifn, u[i].syncid);
+	}
 	free(u);
 }

Alexander Holler

2012-02-09 17:42 UTC #1 lore raw

Am 09.02.2012 16:50, schrieb Ryan O'Hara:

Quoted 15 lines

Attached is a patch that fixes the list_daemon function such that it does not assume that the master sync daemon status is always in the first position and master sync daemon status is always in the second position.
If libipvs uses the netlink interface to retrieve sync daemon status, the results are not guaranteed to follow this ordering. As explained in a previous email, if libipvs uses the netlink interface to retrieve sync daemon status while only a backup sync daemon is running, the backup sync daemon status will but in the first position (index 0). This differs from the getsockopt interface, which would always put master sync daemon status in first position and backup sync daemon status in the second position, even when only backup sync daemon exists. Solution is to make ipvsadm check both elements of the array for master and backup.

Quoted 1 line

Ryan

I've fixed that through letting the netlink-api reporting the same as without netlink.

Don't know what solutions should be prefered.

Regards,

Alexander

---
 keepalived/libipvs-2.6/libipvs.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

keepalived/libipvs-2.6/libipvs.c +1 -3

index ea5e851..6bee837 100644
@@ -1003,12 +1003,9 @@ static int ipvs_daemon_parse_cb(struct nl_msg *msg, void *arg)
 	struct nlattr *attrs[IPVS_CMD_ATTR_MAX + 1];
 	struct nlattr *daemon_attrs[IPVS_DAEMON_ATTR_MAX + 1];
 	ipvs_daemon_t *u = (ipvs_daemon_t *)arg;
+	__u32 state;
 	int i = 0;
 
-	/* We may get two daemons.  If we've already got one, this is the second */
-	if (u[0].state)
-		i = 1;

Ryan O'Hara

author

2012-02-09 19:36 UTC #2 lore raw

Quoted 19 lines · Alexander Holler

Am 09.02.2012 16:50, schrieb Ryan O'Hara:
Quoted 16 lines
Attached is a patch that fixes the list_daemon function such that it does not assume that the master sync daemon status is always in the first position and master sync daemon status is always in the second position.
If libipvs uses the netlink interface to retrieve sync daemon status, the results are not guaranteed to follow this ordering. As explained in a previous email, if libipvs uses the netlink interface to retrieve sync daemon status while only a backup sync daemon is running, the backup sync daemon status will but in the first position (index 0). This differs from the getsockopt interface, which would always put master sync daemon status in first position and backup sync daemon status in the second position, even when only backup sync daemon exists. Solution is to make ipvsadm check both elements of the array for master and backup.
Quoted 1 line
Ryan

Quoted 8 lines

I've fixed that through letting the netlink-api reporting the same as without netlink.
Don't know what solutions should be prefered.
Regards,
Alexander

Thanks for the patch. After taking a close look at your patch, I believe it will fix the problem.

I'm also unsure about which is the preferred solution. I decided to fix it in ipvsadm directly because (it seemed) ipvsadm was making incorrect assumptions about the master/backup sync daemon status being in at a specific index. That said, I'm find with your patch. Keeping the strict ordering is just as reasonable.

Ryan