[PATCH 0/3] CLUSTERIP: Multiple CLUSTERIP patches

[PATCH 0/3] CLUSTERIP: Multiple CLUSTERIP patches

[KOVACS Krisztian]

  Hi,

  I've created three patches for CLUSTERIP. The first two are bug fixes,
while the third one tries to make CLUSTERIP more efficient by changing the
internal representation of the nodes we're responsible for.

  I'd be especially thankful for comments on the second and third patches.
The second patch changes the management of config entries significantly
and it's a very error prone area anyway. The third patch probably needs
more discussion, I seems to work, but some of this seq_file thing is still
a mistery for me... :)

--
 Regards,
  Krisztian Kovacs

[PATCH 1/3] CLUSTERIP: fix memcpy() length typo

[KOVACS Krisztian]

[-- Attachment #1: 01-config_init_memcpy_typo.patch --]
[-- Type: text/plain, Size: 873 bytes --]

CLUSTERIP: fix memcpy() length typo

Fix a trivial typo in clusterip_config_init().


Signed-off-by: KOVACS Krisztian <hidden@balabit.hu>


Index: linux-2.6.13/net/ipv4/netfilter/ipt_CLUSTERIP.c
===================================================================
--- linux-2.6.13.orig/net/ipv4/netfilter/ipt_CLUSTERIP.c	2005-08-30 13:41:22.059915425 +0200
+++ linux-2.6.13/net/ipv4/netfilter/ipt_CLUSTERIP.c	2005-08-30 14:12:40.317423733 +0200
@@ -144,7 +144,7 @@
 	memcpy(&c->clustermac, &i->clustermac, ETH_ALEN);
 	c->num_total_nodes = i->num_total_nodes;
 	c->num_local_nodes = i->num_local_nodes;
-	memcpy(&c->local_nodes, &i->local_nodes, sizeof(&c->local_nodes));
+	memcpy(&c->local_nodes, &i->local_nodes, sizeof(c->local_nodes));
 	c->hash_mode = i->hash_mode;
 	c->hash_initval = i->hash_initval;
 	atomic_set(&c->refcount, 1);

--
 Regards,
  Krisztian Kovacs
 

Re: [PATCH 1/3] CLUSTERIP: fix memcpy() length typo

[Harald Welte]

[-- Attachment #1: Type: text/plain, Size: 724 bytes --]

On Tue, Aug 30, 2005 at 02:15:37PM +0200, KOVACS Krisztian wrote:
> CLUSTERIP: fix memcpy() length typo
> 
> Fix a trivial typo in clusterip_config_init().

thanks, I wonder how often I do this.  It's one of my age-old number 1
programming errors... quite embarrassing :(

I've applied it to netfilte-2.6.14 and will submit it to DaveM soon.

-- 
- Harald Welte <laforge@netfilter.org>                 http://netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

[PATCH 2/3] CLUSTERIP: introduce reference counting for entries

[KOVACS Krisztian]

[-- Attachment #1: 02-add_entry_refcount.patch --]
[-- Type: text/plain, Size: 4657 bytes --]

CLUSTERIP: introduce reference counting for entries

The CLUSTERIP target creates a procfs entry for all different cluster
IPs.  Although more than one rules can refer to a single cluster IP (and
thus a single config structure), removal of the procfs entry is done
unconditionally in destroy(). In more complicated situations involving
deferred dereferencing of the config structure by procfs and creating a
new rule with the same cluster IP it's also possible that no entry will
be created for the new rule.

This patch fixes the problem by counting the number of entries
referencing a given config structure and moving the config list
manipulation and procfs entry deletion parts to the
clusterip_config_entry_put() function.


Signed-off-by: KOVACS Krisztian <hidden@balabit.hu>


Index: linux-2.6.13/net/ipv4/netfilter/ipt_CLUSTERIP.c
===================================================================
--- linux-2.6.13.orig/net/ipv4/netfilter/ipt_CLUSTERIP.c	2005-08-30 14:12:40.317423733 +0200
+++ linux-2.6.13/net/ipv4/netfilter/ipt_CLUSTERIP.c	2005-08-30 14:12:43.365348289 +0200
@@ -49,6 +49,7 @@
 struct clusterip_config {
 	struct list_head list;			/* list of all configs */
 	atomic_t refcount;			/* reference count */
+	atomic_t entries;			/* number of entries referencing us */
 
 	u_int32_t clusterip;			/* the IP address */
 	u_int8_t clustermac[ETH_ALEN];		/* the MAC address */
@@ -76,23 +77,44 @@
 #endif
 
 static inline void
-clusterip_config_get(struct clusterip_config *c) {
+clusterip_config_get(struct clusterip_config *c)
+{
 	atomic_inc(&c->refcount);
 }
 
 static inline void
-clusterip_config_put(struct clusterip_config *c) {
-	if (atomic_dec_and_test(&c->refcount)) {
+clusterip_config_put(struct clusterip_config *c)
+{
+	if (atomic_dec_and_test(&c->refcount))
+		kfree(c);
+}
+
+static inline void
+clusterip_config_entry_get(struct clusterip_config *c)
+{
+	atomic_inc(&c->entries);
+}
+
+static inline void
+clusterip_config_entry_put(struct clusterip_config *c)
+{
+	if (atomic_dec_and_test(&c->entries)) {
 		write_lock_bh(&clusterip_lock);
 		list_del(&c->list);
 		write_unlock_bh(&clusterip_lock);
+
 		dev_mc_delete(c->dev, c->clustermac, ETH_ALEN, 0);
 		dev_put(c->dev);
-		kfree(c);
+
+		/* In case anyone still accesses the file, the open/close
+		 * functions are also incrementing the refcount on their own,
+		 * so it's safe to remove the entry even if it's in use. */
+#ifdef CONFIG_PROC_FS
+		remove_proc_entry(c->pde->name, c->pde->parent);
+#endif
 	}
 }
 
-
 static struct clusterip_config *
 __clusterip_config_find(u_int32_t clusterip)
 {
@@ -111,7 +133,7 @@
 }
 
 static inline struct clusterip_config *
-clusterip_config_find_get(u_int32_t clusterip)
+clusterip_config_find_get(u_int32_t clusterip, int entry)
 {
 	struct clusterip_config *c;
 
@@ -122,6 +144,8 @@
 		return NULL;
 	}
 	atomic_inc(&c->refcount);
+	if (entry)
+		atomic_inc(&c->entries);
 	read_unlock_bh(&clusterip_lock);
 
 	return c;
@@ -148,6 +172,7 @@
 	c->hash_mode = i->hash_mode;
 	c->hash_initval = i->hash_initval;
 	atomic_set(&c->refcount, 1);
+	atomic_set(&c->entries, 1);
 
 #ifdef CONFIG_PROC_FS
 	/* create proc dir entry */
@@ -415,7 +440,14 @@
 
 	/* FIXME: further sanity checks */
 
-	config = clusterip_config_find_get(e->ip.dst.s_addr);
+	/* If this is not a completely new entry and already has a config
+	 * attached, simply increase the entry refcount and return */
+	if (cipinfo->config != NULL) {
+		clusterip_config_entry_get(cipinfo->config);
+		return 1;
+	}
+
+	config = clusterip_config_find_get(e->ip.dst.s_addr, 1);
 	if (!config) {
 		if (!(cipinfo->flags & CLUSTERIP_FLAG_NEW)) {
 			printk(KERN_WARNING "CLUSTERIP: no config found for %u.%u.%u.%u, need 'new'\n", NIPQUAD(e->ip.dst.s_addr));
@@ -455,13 +487,10 @@
 {
 	struct ipt_clusterip_tgt_info *cipinfo = matchinfo;
 
-	/* we first remove the proc entry and then drop the reference
-	 * count.  In case anyone still accesses the file, the open/close
-	 * functions are also incrementing the refcount on their own */
-#ifdef CONFIG_PROC_FS
-	remove_proc_entry(cipinfo->config->pde->name,
-			  cipinfo->config->pde->parent);
-#endif
+	/* if no more entries are referencing the config, remove it
+	 * from the list and destroy the proc entry */
+	clusterip_config_entry_put(cipinfo->config);
+
 	clusterip_config_put(cipinfo->config);
 }
 
@@ -533,7 +562,7 @@
 
 	/* if there is no clusterip configuration for the arp reply's 
 	 * source ip, we don't want to mangle it */
-	c = clusterip_config_find_get(payload->src_ip);
+	c = clusterip_config_find_get(payload->src_ip, 0);
 	if (!c)
 		return NF_ACCEPT;
 

--
 Regards,
  Krisztian Kovacs
 

Re: [PATCH 2/3] CLUSTERIP: introduce reference counting for entries

[Harald Welte]

[-- Attachment #1: Type: text/plain, Size: 661 bytes --]

On Tue, Aug 30, 2005 at 02:15:38PM +0200, KOVACS Krisztian wrote:
> CLUSTERIP: introduce reference counting for entries

Thanks for your patches 2 and 3.  I want to review them in more detail
and need to re-read the CLUSTERIP code first.  Expect some feedback in
the next two days.

-- 
- Harald Welte <laforge@netfilter.org>                 http://netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH 2/3] CLUSTERIP: introduce reference counting for entries

[KOVACS Krisztian]

  Hi,

On Tuesday 30 August 2005 16.07, Harald Welte wrote:
> Thanks for your patches 2 and 3.  I want to review them in more
> detail and need to re-read the CLUSTERIP code first.  Expect some
> feedback in the next two days.

  OK, thanks. BTW, I've just found a serious bug in patch 2, I knew I
was missing something... So please apply this on top of patch 2.

Index: linux-2.6.13/net/ipv4/netfilter/ipt_CLUSTERIP.c
===================================================================
--- linux-2.6.13.orig/net/ipv4/netfilter/ipt_CLUSTERIP.c	2005-08-30 
16:59:07.702210333 +0200
+++ linux-2.6.13/net/ipv4/netfilter/ipt_CLUSTERIP.c	2005-08-30 
16:50:27.431088379 +0200
@@ -444,6 +444,7 @@
 	 * attached, simply increase the entry refcount and return */
 	if (cipinfo->config != NULL) {
 		clusterip_config_entry_get(cipinfo->config);
+                clusterip_config_get(cipinfo->config);
 		return 1;
 	}

-- 
 Regards,
  Krisztian Kovacs

Re: [PATCH 2/3] CLUSTERIP: introduce reference counting for entries

[Harald Welte]

[-- Attachment #1: Type: text/plain, Size: 1013 bytes --]

On Tue, Aug 30, 2005 at 05:26:07PM +0200, KOVACS Krisztian wrote:
> 
>   Hi,
> 
> On Tuesday 30 August 2005 16.07, Harald Welte wrote:
> > Thanks for your patches 2 and 3.  I want to review them in more
> > detail and need to re-read the CLUSTERIP code first.  Expect some
> > feedback in the next two days.
> 
>   OK, thanks. BTW, I've just found a serious bug in patch 2, I knew I
> was missing something... So please apply this on top of patch 2.
 
as usual, the two days got a bit long.  But now I've reviewed the old
code and your changes.  I've applied them to my git tree with some minor
modifications/clarifications.

-- 
- Harald Welte <laforge@netfilter.org>                 http://netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH 2/3] CLUSTERIP: introduce reference counting for entries

[KOVACS Krisztian]

  Hi,

On Tuesday 13 September 2005 14.54, Harald Welte wrote:
> On Tue, Aug 30, 2005 at 05:26:07PM +0200, KOVACS Krisztian wrote:
> > On Tuesday 30 August 2005 16.07, Harald Welte wrote:
> > > Thanks for your patches 2 and 3.  I want to review them in more
> > > detail and need to re-read the CLUSTERIP code first.  Expect some
> > > feedback in the next two days.
> >
> >   OK, thanks. BTW, I've just found a serious bug in patch 2, I knew
> > I was missing something... So please apply this on top of patch 2.
>
> as usual, the two days got a bit long.  But now I've reviewed the old
> code and your changes.  I've applied them to my git tree with some
> minor modifications/clarifications.

  Thanks a lot. One more thing: what's up with the updated CLUSTERIP 
userspace?
 
http://lists.netfilter.org/pipermail/netfilter-devel/2005-August/021165.html

-- 
 Regards,
  Krisztian Kovacs

Re: [PATCH 2/3] CLUSTERIP: introduce reference counting for entries

[Harald Welte]

[-- Attachment #1: Type: text/plain, Size: 743 bytes --]

On Tue, Sep 13, 2005 at 05:16:57PM +0200, KOVACS Krisztian wrote:
> > as usual, the two days got a bit long.  But now I've reviewed the old
> > code and your changes.  I've applied them to my git tree with some
> > minor modifications/clarifications.
> 
>   Thanks a lot. One more thing: what's up with the updated CLUSTERIP 
> userspace?

submitted meanwhile.

-- 
- Harald Welte <laforge@netfilter.org>                 http://netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

[PATCH 3/3] CLUSTERIP: use a bitmap to store node responsibility data

[KOVACS Krisztian]

[-- Attachment #1: 03-responsibility_bitmap.patch --]
[-- Type: text/plain, Size: 7395 bytes --]

CLUSTERIP: use a bitmap to store node responsibility data

Instead of maintaining an array containing a list of nodes this instance
is responsible for let's use a simple bitmap. This provides the
following features:

  * clusterip_responsible() and the add_node()/delete_node() operations
    become very simple and don't need locking
  * the config structure is much smaller

In spite of the completely different internal data representation the
user-space interface remains almost unchanged; the only difference is
that the proc file does not list nodes in the order they were added.
(The target info structure remains the same.)


Signed-off-by: KOVACS Krisztian <hidden@balabit.hu>


Index: linux-2.6.13/net/ipv4/netfilter/ipt_CLUSTERIP.c
===================================================================
--- linux-2.6.13.orig/net/ipv4/netfilter/ipt_CLUSTERIP.c	2005-08-30 14:12:43.365348289 +0200
+++ linux-2.6.13/net/ipv4/netfilter/ipt_CLUSTERIP.c	2005-08-30 14:12:46.077281161 +0200
@@ -13,6 +13,7 @@
 #include <linux/config.h>
 #include <linux/proc_fs.h>
 #include <linux/jhash.h>
+#include <linux/bitops.h>
 #include <linux/skbuff.h>
 #include <linux/ip.h>
 #include <linux/tcp.h>
@@ -30,7 +31,7 @@
 #include <linux/netfilter_ipv4/ipt_CLUSTERIP.h>
 #include <linux/netfilter_ipv4/ip_conntrack.h>
 
-#define CLUSTERIP_VERSION "0.7"
+#define CLUSTERIP_VERSION "0.8"
 
 #define DEBUG_CLUSTERIP
 
@@ -55,8 +56,7 @@
 	u_int8_t clustermac[ETH_ALEN];		/* the MAC address */
 	struct net_device *dev;			/* device */
 	u_int16_t num_total_nodes;		/* total number of nodes */
-	u_int16_t num_local_nodes;		/* number of local nodes */
-	u_int16_t local_nodes[CLUSTERIP_MAX_NODES];	/* node number array */
+	unsigned long local_nodes;		/* node number array */
 
 #ifdef CONFIG_PROC_FS
 	struct proc_dir_entry *pde;		/* proc dir entry */
@@ -67,8 +67,7 @@
 
 static LIST_HEAD(clusterip_configs);
 
-/* clusterip_lock protects the clusterip_configs list _AND_ the configurable
- * data within all structurses (num_local_nodes, local_nodes[]) */
+/* clusterip_lock protects the clusterip_configs list */
 static DEFINE_RWLOCK(clusterip_lock);
 
 #ifdef CONFIG_PROC_FS
@@ -151,6 +150,17 @@
 	return c;
 }
 
+static void
+clusterip_config_init_nodelist(struct clusterip_config *c,
+			       const struct ipt_clusterip_tgt_info *i)
+{
+	int n;
+
+	for (n = 0; n < i->num_local_nodes; n++) {
+		set_bit(i->local_nodes[n] - 1, &c->local_nodes);
+	}
+}
+
 static struct clusterip_config *
 clusterip_config_init(struct ipt_clusterip_tgt_info *i, u_int32_t ip,
 			struct net_device *dev)
@@ -167,8 +177,7 @@
 	c->clusterip = ip;
 	memcpy(&c->clustermac, &i->clustermac, ETH_ALEN);
 	c->num_total_nodes = i->num_total_nodes;
-	c->num_local_nodes = i->num_local_nodes;
-	memcpy(&c->local_nodes, &i->local_nodes, sizeof(c->local_nodes));
+	clusterip_config_init_nodelist(c, i);
 	c->hash_mode = i->hash_mode;
 	c->hash_initval = i->hash_initval;
 	atomic_set(&c->refcount, 1);
@@ -196,53 +205,28 @@
 static int
 clusterip_add_node(struct clusterip_config *c, u_int16_t nodenum)
 {
-	int i;
-
-	write_lock_bh(&clusterip_lock);
 
-	if (c->num_local_nodes >= CLUSTERIP_MAX_NODES
-	    || nodenum > CLUSTERIP_MAX_NODES) {
-		write_unlock_bh(&clusterip_lock);
+	if (nodenum == 0 ||
+	    nodenum > c->num_total_nodes)
 		return 1;
-	}
-
-	/* check if we alrady have this number in our array */
-	for (i = 0; i < c->num_local_nodes; i++) {
-		if (c->local_nodes[i] == nodenum) {
-			write_unlock_bh(&clusterip_lock);
-			return 1;
-		}
-	}
 
-	c->local_nodes[c->num_local_nodes++] = nodenum;
+	/* check if we already have this number in our bitfield */
+	if (test_and_set_bit(nodenum - 1, &c->local_nodes))
+		return 1;
 
-	write_unlock_bh(&clusterip_lock);
 	return 0;
 }
 
 static int
 clusterip_del_node(struct clusterip_config *c, u_int16_t nodenum)
 {
-	int i;
-
-	write_lock_bh(&clusterip_lock);
-
-	if (c->num_local_nodes <= 1 || nodenum > CLUSTERIP_MAX_NODES) {
-		write_unlock_bh(&clusterip_lock);
+	if (nodenum == 0 ||
+	    nodenum > c->num_total_nodes)
 		return 1;
-	}
 		
-	for (i = 0; i < c->num_local_nodes; i++) {
-		if (c->local_nodes[i] == nodenum) {
-			int size = sizeof(u_int16_t)*(c->num_local_nodes-(i+1));
-			memmove(&c->local_nodes[i], &c->local_nodes[i+1], size);
-			c->num_local_nodes--;
-			write_unlock_bh(&clusterip_lock);
-			return 0;
-		}
-	}
+	if (test_and_clear_bit(nodenum - 1, &c->local_nodes))
+		return 0;
 
-	write_unlock_bh(&clusterip_lock);
 	return 1;
 }
 
@@ -310,25 +294,7 @@
 static inline int
 clusterip_responsible(struct clusterip_config *config, u_int32_t hash)
 {
-	int i;
-
-	read_lock_bh(&clusterip_lock);
-
-	if (config->num_local_nodes == 0) {
-		read_unlock_bh(&clusterip_lock);
-		return 0;
-	}
-
-	for (i = 0; i < config->num_local_nodes; i++) {
-		if (config->local_nodes[i] == hash) {
-			read_unlock_bh(&clusterip_lock);
-			return 1;
-		}
-	}
-
-	read_unlock_bh(&clusterip_lock);
-
-	return 0;
+	return test_bit(hash - 1, &config->local_nodes);
 }
 
 /*********************************************************************** 
@@ -603,56 +569,69 @@
 
 #ifdef CONFIG_PROC_FS
 
+struct clusterip_seq_position {
+	unsigned int pos;	/* position */
+	unsigned int weight;	/* number of bits set == size */
+	unsigned int bit;	/* current bit */
+	unsigned long val;	/* current value */
+};
+
 static void *clusterip_seq_start(struct seq_file *s, loff_t *pos)
 {
 	struct proc_dir_entry *pde = s->private;
 	struct clusterip_config *c = pde->data;
-	unsigned int *nodeidx;
-
-	read_lock_bh(&clusterip_lock);
-	if (*pos >= c->num_local_nodes)
+	unsigned int weight;
+	u_int32_t local_nodes;
+	struct clusterip_seq_position *idx;
+
+	/* FIXME: possible race */
+	local_nodes = c->local_nodes;
+	weight = hweight32(local_nodes);
+	if (*pos >= weight)
 		return NULL;
 
-	nodeidx = kmalloc(sizeof(unsigned int), GFP_KERNEL);
-	if (!nodeidx)
+	idx = kmalloc(sizeof(struct clusterip_seq_position), GFP_KERNEL);
+	if (!idx)
 		return ERR_PTR(-ENOMEM);
 
-	*nodeidx = *pos;
-	return nodeidx;
+	idx->pos = *pos;
+	idx->weight = weight;
+	idx->bit = ffs(local_nodes);
+	idx->val = local_nodes;
+	clear_bit(idx->bit - 1, &idx->val);
+
+	return idx;
 }
 
 static void *clusterip_seq_next(struct seq_file *s, void *v, loff_t *pos)
 {
-	struct proc_dir_entry *pde = s->private;
-	struct clusterip_config *c = pde->data;
-	unsigned int *nodeidx = (unsigned int *)v;
+	struct clusterip_seq_position *idx = (struct clusterip_seq_position *)v;
 
-	*pos = ++(*nodeidx);
-	if (*pos >= c->num_local_nodes) {
+	*pos = ++idx->pos;
+	if (*pos >= idx->weight) {
 		kfree(v);
 		return NULL;
 	}
-	return nodeidx;
+	idx->bit = ffs(idx->val);
+	clear_bit(idx->bit - 1, &idx->val);
+	return idx;
 }
 
 static void clusterip_seq_stop(struct seq_file *s, void *v)
 {
 	kfree(v);
-
-	read_unlock_bh(&clusterip_lock);
 }
 
 static int clusterip_seq_show(struct seq_file *s, void *v)
 {
-	struct proc_dir_entry *pde = s->private;
-	struct clusterip_config *c = pde->data;
-	unsigned int *nodeidx = (unsigned int *)v;
+	struct clusterip_seq_position *idx = (struct clusterip_seq_position *)v;
 
-	if (*nodeidx != 0) 
+	if (idx->pos != 0) 
 		seq_putc(s, ',');
-	seq_printf(s, "%u", c->local_nodes[*nodeidx]);
 
-	if (*nodeidx == c->num_local_nodes-1)
+	seq_printf(s, "%u", idx->bit);
+
+	if (idx->pos == idx->weight - 1)
 		seq_putc(s, '\n');
 
 	return 0;

--
 Regards,
  Krisztian Kovacs
 

Re: [PATCH 3/3] CLUSTERIP: use a bitmap to store node responsibility data

[Pablo Neira]

Hi Krisztian,

KOVACS Krisztian wrote:
> @@ -55,8 +56,7 @@
>  	u_int8_t clustermac[ETH_ALEN];		/* the MAC address */
>  	struct net_device *dev;			/* device */
>  	u_int16_t num_total_nodes;		/* total number of nodes */
> -	u_int16_t num_local_nodes;		/* number of local nodes */
> -	u_int16_t local_nodes[CLUSTERIP_MAX_NODES];	/* node number array */
> +	unsigned long local_nodes;		/* node number array */

I'm afraid that this breaks backward compatibility. Although CLUSTERIP 
is marked as experimental.

@Harald: Should we respect backward compatibility in this case?

--
Pablo

Re: [PATCH 3/3] CLUSTERIP: use a bitmap to store node responsibility data

[KOVACS Krisztian]

  Hi,

On Tuesday 30 August 2005 15.29, Pablo Neira wrote:
> KOVACS Krisztian wrote:
> > @@ -55,8 +56,7 @@
> >  	u_int8_t clustermac[ETH_ALEN];		/* the MAC address */
> >  	struct net_device *dev;			/* device */
> >  	u_int16_t num_total_nodes;		/* total number of nodes */
> > -	u_int16_t num_local_nodes;		/* number of local nodes */
> > -	u_int16_t local_nodes[CLUSTERIP_MAX_NODES];	/* node number array
> > */ +	unsigned long local_nodes;		/* node number array */
>
> I'm afraid that this breaks backward compatibility. Although
> CLUSTERIP is marked as experimental.

  IMHO this does not break compatibility, since this change only affects 
the internal clusterip_config structure, not the target info structure 
itself. (And the module does conversion in checkentry().)

-- 
 Regards,
  Krisztian Kovacs

Re: [PATCH 3/3] CLUSTERIP: use a bitmap to store node responsibility data

[Pablo Neira]

KOVACS Krisztian wrote:
> On Tuesday 30 August 2005 15.29, Pablo Neira wrote:
> 
>>KOVACS Krisztian wrote:
>>
>>>@@ -55,8 +56,7 @@
>>> 	u_int8_t clustermac[ETH_ALEN];		/* the MAC address */
>>> 	struct net_device *dev;			/* device */
>>> 	u_int16_t num_total_nodes;		/* total number of nodes */
>>>-	u_int16_t num_local_nodes;		/* number of local nodes */
>>>-	u_int16_t local_nodes[CLUSTERIP_MAX_NODES];	/* node number array
>>>*/ +	unsigned long local_nodes;		/* node number array */
>>
>>I'm afraid that this breaks backward compatibility. Although
>>CLUSTERIP is marked as experimental.
> 
> 
>   IMHO this does not break compatibility, since this change only affects 
> the internal clusterip_config structure, not the target info structure 
> itself. (And the module does conversion in checkentry().)

You're right, sorry for the noise.

--
Pablo