LVS
lvs-users
Google
 
Web LinuxVirtualServer.org

Re: [PATCH] IPVS syncd zombies fixe

To: Alexandre Cassen <Alexandre.Cassen@xxxxxxxxxx>
Subject: Re: [PATCH] IPVS syncd zombies fixe
Cc: lvs-users@xxxxxxxxxxxxxxxxxxxxxx
Cc: netdev@xxxxxxxxxxx
Cc: keepalived-devel@xxxxxxxxxxxxxxxxxxxxx
From: Wensong Zhang <wensong@xxxxxxxxxxxx>
Date: Thu, 19 Aug 2004 00:45:24 +0800 (CST)

Hi Alexandre,

Thanks for locating the problem.

I don't remember the exact reason that I removed the waitpid call in ipvs
syncd in Feburary and considered it is superfluous, maybe because the
sys_wait4 is not exported on some architecture. I'll use your program to 
do a test soon, and see if there is a better solution.

Thanks,

Wensong


On Tue, 17 Aug 2004, Alexandre Cassen wrote:

> Hello Wensong,
> 
> I received since last few months lot of bug report from Keepalived users 
> using VRRP with ipvs syncd option. Finally today, I found time to look at 
> this issue and have a patch for syncd code.
> 
> While starting a kernel syncd thread, we must reap the fork_sync_thread 
> since this will produce zombies while returning to main calling point. 
> Since syncd parent is the init process, we don't need to reap it during the 
> syncd shutdown but we must reap it at starting time.
> 
> You will find bellow a patch to fixe this and a quick trash C code to argue 
> and highlight the need. The patch is against 2.6.7, same patch must be 
> applied to 2.4 branch.
> 
> Best regards,
> Alexandre
> 
> 
> 
> --[ Patch: Begin Snip ]--
> 
> diff -urN linux-2.6.7.orig/net/ipv4/ipvs/ip_vs_sync.c 
> linux-2.6.7/net/ipv4/ipvs/ip_vs_sync.c
> --- linux-2.6.7.orig/net/ipv4/ipvs/ip_vs_sync.c 2004-06-16 
> 07:18:38.000000000 +0200
> +++ linux-2.6.7/net/ipv4/ipvs/ip_vs_sync.c      2004-08-17 
> 18:30:36.143940704 +0200
> @@ -18,9 +18,11 @@
>    *                                     messages filtering.
>    */
> 
> +#define __KERNEL_SYSCALLS__            /*  for waitpid */
> +
>   #include <linux/module.h>
>   #include <linux/slab.h>
> -#include <linux/net.h>
> +#include <linux/unistd.h>
>   #include <linux/completion.h>
> 
>   #include <linux/skbuff.h>
> @@ -612,6 +614,7 @@
>          return len;
>   }
> 
> +static int errno;
> 
>   static DECLARE_WAIT_QUEUE_HEAD(sync_wait);
>   static pid_t sync_master_pid = 0;
> @@ -840,6 +843,7 @@
>   int start_sync_thread(int state, char *mcast_ifn, __u8 syncid)
>   {
>          DECLARE_COMPLETION(startup);
> +       int waitpid_result;
>          pid_t pid;
> 
>          if ((state == IP_VS_STATE_MASTER && sync_master_pid) ||
> @@ -868,6 +872,15 @@
>                  goto repeat;
>          }
> 
> +       /*
> +        * We must reap the fork_sync_thread here. Really needed to not
> +        * produce zombies while returning.
> +        */
> +       if ((waitpid_result = waitpid(pid, NULL, __WCLONE)) != pid) {
> +               IP_VS_ERR("%s: waitpid(%d,...) failed, errno %d\n",
> +                         __FUNCTION__, pid, -waitpid_result);
> +       }
> +
>          wait_for_completion(&startup);
> 
>          return 0;
> 
> --[ Patch: End Snip ]--
> 
> 
> 
> --[ Quick Code: Begin Snip ]--
> 
> /*
>   * Soft:        Simple IPVS Syncd probing tool.
>   *
>   * Compilation: gcc -I/usr/src/linux/include main.c -o syncd_zombies
>   *
>   * Version:     $Id: main.c,v 0.0.1 2004/08/17 21:35:41 acassen Exp $
>   *
>   * Authors:     Alexandre Cassen, <acassen@xxxxxxxxxxxx>
>   *
>   *              This program is distributed in the hope that it will be 
> useful,
>   *              but WITHOUT ANY WARRANTY; without even the implied warranty 
> of
>   *              MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
>   *              See the GNU General Public License for more details.
>   *
>   *              This program is free software; you can redistribute it and/or
>   *              modify it under the terms of the GNU General Public License
>   *              as published by the Free Software Foundation; either version
>   *              2 of the License, or (at your option) any later version.
>   *
>   * Copyright (C) 2001-2004 Alexandre Cassen, <acassen@xxxxxxxxxxxx>
>   */
> 
> #include <stdio.h>
> #include <unistd.h>
> #include <stdlib.h>
> #include <string.h>
> #include <sys/wait.h>
> #include <sys/socket.h>
> #include <netinet/in.h>
> #include <net/ip_vs.h>
> 
> static struct ip_vs_daemon_user *daemonrule;
> static int sockfd = -1;
> static void *ipvs_func = NULL;
> struct ip_vs_getinfo ipvs_info;
> typedef struct ip_vs_daemon_user ipvs_daemon_t;
> 
> int
> ipvs_init(void)
> {
>          socklen_t len;
> 
>          len = sizeof (ipvs_info);
>          if ((sockfd = socket(AF_INET, SOCK_RAW, IPPROTO_RAW)) == -1)
>                  return -1;
> 
>          if (getsockopt(sockfd, IPPROTO_IP, IP_VS_SO_GET_INFO,
>                         (char *) &ipvs_info, &len))
>                  return -1;
> 
>          return 0;
> }
> 
> int
> ipvs_start_daemon(ipvs_daemon_t * dm)
> {
>          ipvs_func = ipvs_start_daemon;
>          return setsockopt(sockfd, IPPROTO_IP, IP_VS_SO_SET_STARTDAEMON,
>                            (char *) dm, sizeof (*dm));
> }
> 
> extern int
> ipvs_stop_daemon(ipvs_daemon_t * dm)
> {
>          ipvs_func = ipvs_stop_daemon;
>          return setsockopt(sockfd, IPPROTO_IP, IP_VS_SO_SET_STOPDAEMON,
>                            (char *) dm, sizeof (*dm));
> }
> 
> static int
> modprobe_ipvs(void)
> {
>          char *argv[] = { "/sbin/modprobe", "-s", "-k", "--", "ip_vs", NULL };
>          int child;
>          int status;
>          int rc;
> 
>          if (!(child = fork())) {
>                  execv(argv[0], argv);
>                  exit(1);
>          }
> 
>          rc = waitpid(child, &status, 0);
>          if (!WIFEXITED(status) || WEXITSTATUS(status))
>                  return 1;
>          return 0;
> }
> 
> static void
> ipvs_talk(int cmd)
> {
>          int result = -1;
> 
>          switch (cmd) {
>          case IP_VS_SO_SET_STARTDAEMON:
>                  result = ipvs_start_daemon(daemonrule);
>                  break;
>          case IP_VS_SO_SET_STOPDAEMON:
>                  result = ipvs_stop_daemon(daemonrule);
>                  break;
>          }
> }
> 
> static int
> ipvs_start(void)
> {
>          if (ipvs_init()) {
>                  if (modprobe_ipvs() || ipvs_init()) {
>                          printf("IPVS: Can't initialize IPVS !!!\n");
>                          return -1;
>                  }
>          }
>          return 0;
> }
> 
> int
> ipvs_syncd_cmd(int cmd, char *ifname, int state)
> {
>          memset(daemonrule, 0, sizeof (struct ip_vs_daemon_user));
>          daemonrule->state = state;
>          strncpy(daemonrule->mcast_ifn, ifname, IP_VS_IFNAME_MAXLEN);
>          ipvs_talk(cmd);
>          return 0;
> }
> 
> int
> main(int argc, char **argv)
> {
>          daemonrule = (struct ip_vs_daemon_user *) malloc(sizeof (struct 
> ip_vs_daemon_user));
> 
>          ipvs_start();
>          ipvs_syncd_cmd(IP_VS_SO_SET_STARTDAEMON, "eth0", IP_VS_STATE_MASTER);
>          ipvs_syncd_cmd(IP_VS_SO_SET_STOPDAEMON, "eth0", IP_VS_STATE_MASTER);
>          ipvs_syncd_cmd(IP_VS_SO_SET_STARTDAEMON, "eth0", IP_VS_STATE_BACKUP);
>          ipvs_syncd_cmd(IP_VS_SO_SET_STOPDAEMON, "eth0", IP_VS_STATE_BACKUP);
> 
>          free(daemonrule);
> 
>          for (;;) { /* Dummy loop */ }
>          exit(0);
> }
> 
> --[ Quick Code: End Snip ]--
> 
> 

<Prev in Thread] Current Thread [Next in Thread>