[RFC] ipvs: Cleanup sync daemon code

Previous thread: [git pull] x86 updates by Thomas Gleixner on Saturday, February 9, 2008 - 4:24 pm. (5 messages)

Next thread: [PATCH] Silent compiler warning introduced by acea6852f32b8805e166d885ed7e9f0c7cd10d41 ([BLUETOOTH]: Move children of connection device to NULL before connection down.) by S.Çağlar on Saturday, February 9, 2008 - 7:57 pm. (6 messages)
From: Sven Wegener
Date: Saturday, February 9, 2008 - 4:38 pm

Hi all,

I'd like to get your feedback on this:

- Use kthread_run instead of doing a double-fork via kernel_thread()

- Return proper error codes to user-space on failures

Currently ipvsadm --start-daemon with an invalid --mcast-interface will 
silently suceed. With these changes we get an appropriate "No such device" 
error.

- Use wait queues for both master and backup thread

Instead of doing an endless loop with sleeping for one second, we now use 
wait queues. The master sync daemon has its own wait queue and gets woken 
up when we have enough data to sent and also at a regular interval. The 
backup sync daemon sits on the wait queue of the mcast socket and gets 
woken up as soon as we have data to process.

diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
index 56f3c94..519bd96 100644
--- a/include/net/ip_vs.h
+++ b/include/net/ip_vs.h
@@ -890,6 +890,7 @@ extern char ip_vs_backup_mcast_ifn[IP_VS_IFNAME_MAXLEN];
  extern int start_sync_thread(int state, char *mcast_ifn, __u8 syncid);
  extern int stop_sync_thread(int state);
  extern void ip_vs_sync_conn(struct ip_vs_conn *cp);
+extern void ip_vs_sync_init(void);


  /*
diff --git a/net/ipv4/ipvs/ip_vs_core.c b/net/ipv4/ipvs/ip_vs_core.c
index 963981a..0ccee4b 100644
--- a/net/ipv4/ipvs/ip_vs_core.c
+++ b/net/ipv4/ipvs/ip_vs_core.c
@@ -1071,6 +1071,8 @@ static int __init ip_vs_init(void)
  {
  	int ret;

+	ip_vs_sync_init();
+
  	ret = ip_vs_control_init();
  	if (ret < 0) {
  		IP_VS_ERR("can't setup control.\n");
diff --git a/net/ipv4/ipvs/ip_vs_sync.c b/net/ipv4/ipvs/ip_vs_sync.c
index 948378d..36063d3 100644
--- a/net/ipv4/ipvs/ip_vs_sync.c
+++ b/net/ipv4/ipvs/ip_vs_sync.c
@@ -29,6 +29,9 @@
  #include <linux/in.h>
  #include <linux/igmp.h>                 /* for ip_mc_join_group */
  #include <linux/udp.h>
+#include <linux/kthread.h>
+#include <linux/wait.h>
+#include <linux/err.h>

  #include <net/ip.h>
  #include <net/sock.h>
@@ -68,7 +71,8 @@ struct ip_vs_sync_conn_options {
  ...
From: Simon Horman
Date: Saturday, February 9, 2008 - 6:27 pm

Hi Sven,

This looks good to me, assuming that its tested and works.

A few minor things:

In sb_queue_tail() master loop is woken up if
the ip_vs_sync_count reaches 10, which seems a bit arbitary.

Perhaps its just my mail reader, but the patch seemed a bit screwy when
I saved it to a file. I this fixed the problem I was seeing using s/^  / /

Unfortuantely/Fortunately I am about to leave for a few days skiing,
so if I am quiet you will know why.

Acked-by: Simon Horman <horms@verge.net.au>

-- 
Horms

--

From: Christoph Hellwig
Date: Saturday, February 9, 2008 - 9:59 pm

This is not needed anmore.  kthread_run guarantees that the newly

I don't think you need this one either.  You can use wake_up_process

Btw, it might make sense to remove sync_thread and just call the

Why can't this be initialized at compile time by:

static struct sockaddr_in mcast_addr = {
	.sin_family		= AF_INET,
	.sin_port		= htons(IP_VS_SYNC_PORT),
	.sin_addr.s_addr	= htonl(IP_VS_SYNC_GROUP),
}

(the hton* might need __constant_hton* also I'm not sure without trying)

--

From: Sven Wegener
Date: Sunday, February 10, 2008 - 4:51 am

The completion is currently used to return an error code for errors that 
happen during initialization in the threads (open socket, allocate 
memory). We could move the setup code out of the threads and have them 


When the setup code has been moved out of the threads, the code gets much 

Thanks.
--

Previous thread: [git pull] x86 updates by Thomas Gleixner on Saturday, February 9, 2008 - 4:24 pm. (5 messages)

Next thread: [PATCH] Silent compiler warning introduced by acea6852f32b8805e166d885ed7e9f0c7cd10d41 ([BLUETOOTH]: Move children of connection device to NULL before connection down.) by S.Çağlar on Saturday, February 9, 2008 - 7:57 pm. (6 messages)