[B.A.T.M.A.N.] [PATCH 1/3] batman-adv: atomic variable for vis-srv activation

[B.A.T.M.A.N.] [PATCH 1/3] batman-adv: atomic variable for vis-srv activation

[=?UTF-8?q?Linus=20L=C3=BCssing?=]

This fixes the bug discovered by Marek which did not allow turning on
the vis-server before an interface has been added. This is now being
done in a similar way as for (de)activating the aggregation mode with an
atomic variable.

Signed-off-by: Linus Lüssing <linus.luessing@web.de>
---
 main.c |    2 ++
 main.h |    1 +
 proc.c |   43 ++++++++++++++++++++++++-------------------
 send.c |    2 +-
 vis.c  |   41 +++--------------------------------------
 vis.h  |    2 --
 6 files changed, 31 insertions(+), 60 deletions(-)

diff --git a/main.c b/main.c
index c733504..843d552 100644
--- a/main.c
+++ b/main.c
@@ -44,6 +44,7 @@ DEFINE_SPINLOCK(forw_bcast_list_lock);
 
 atomic_t originator_interval;
 atomic_t vis_interval;
+atomic_t vis_srv_enabled;
 atomic_t aggregation_enabled;
 int16_t num_hna;
 int16_t num_ifs;
@@ -84,6 +85,7 @@ int init_module(void)
 	atomic_set(&originator_interval, 1000);
 	atomic_set(&vis_interval, 1000);/* TODO: raise this later, this is only
 					 * for debugging now. */
+	atomic_set(&vis_srv_enabled, 0);
 	atomic_set(&aggregation_enabled, 1);
 
 	/* the name should not be longer than 10 chars - see
diff --git a/main.h b/main.h
index 3dfe5fe..385818f 100644
--- a/main.h
+++ b/main.h
@@ -130,6 +130,7 @@ extern spinlock_t forw_bcast_list_lock;
 
 extern atomic_t originator_interval;
 extern atomic_t vis_interval;
+extern atomic_t vis_srv_enabled;
 extern atomic_t aggregation_enabled;
 extern int16_t num_hna;
 extern int16_t num_ifs;
diff --git a/proc.c b/proc.c
index 61e1d0d..e7b7bf3 100644
--- a/proc.c
+++ b/proc.c
@@ -325,36 +325,41 @@ static int proc_transt_global_open(struct inode *inode, struct file *file)
 static ssize_t proc_vis_srv_write(struct file *file, const char __user * buffer,
 			      size_t count, loff_t *ppos)
 {
-	char *vis_mode_string;
+	char *vis_srv_string;
 	int not_copied = 0;
+	unsigned long vis_srv_enabled_tmp;
+	int retval;
 
-	vis_mode_string = kmalloc(count, GFP_KERNEL);
+	vis_srv_string = kmalloc(count, GFP_KERNEL);
 
-	if (!vis_mode_string)
+	if (!vis_srv_string)
 		return -ENOMEM;
 
-	not_copied = copy_from_user(vis_mode_string, buffer, count);
-	vis_mode_string[count - not_copied - 1] = 0;
-
-	if ((strcmp(vis_mode_string, "client") == 0) ||
-			(strcmp(vis_mode_string, "disabled") == 0)) {
-		printk(KERN_INFO "batman-adv:Setting VIS mode to client (disabling vis server)\n");
-		vis_set_mode(VIS_TYPE_CLIENT_UPDATE);
-	} else if ((strcmp(vis_mode_string, "server") == 0) ||
-			(strcmp(vis_mode_string, "enabled") == 0)) {
-		printk(KERN_INFO "batman-adv:Setting VIS mode to server (enabling vis server)\n");
-		vis_set_mode(VIS_TYPE_SERVER_SYNC);
-	} else
+	not_copied = copy_from_user(vis_srv_string, buffer, count);
+	vis_srv_string[count - not_copied - 1] = 0;
+
+	retval = strict_strtoul(vis_srv_string, 10, &vis_srv_enabled_tmp);
+
+	/* Unknown vis mode input? */
+	if (retval == -EINVAL || vis_srv_enabled_tmp > 1) {
 		printk(KERN_ERR "batman-adv:Unknown VIS mode: %s\n",
-		       vis_mode_string);
+		       vis_srv_string);
+	}
+	else {
+		if (vis_srv_enabled_tmp == 0)
+			printk(KERN_INFO "batman-adv:Setting VIS mode to client (disabling vis server)\n");
+		else
+			printk(KERN_INFO "batman-adv:Setting VIS mode to server (enabling vis server)\n");
+		atomic_set(&vis_srv_enabled, vis_srv_enabled_tmp);
+	}
 
-	kfree(vis_mode_string);
+	kfree(vis_srv_string);
 	return count;
 }
 
 static int proc_vis_srv_read(struct seq_file *seq, void *offset)
 {
-	int vis_server = is_vis_server();
+	int vis_server = atomic_read(&vis_srv_enabled);
 
 	seq_printf(seq, "[%c] client mode (server disabled) \n",
 			(!vis_server) ? 'x' : ' ');
@@ -380,7 +385,7 @@ static int proc_vis_data_read(struct seq_file *seq, void *offset)
 	unsigned long flags;
 
 	rcu_read_lock();
-	if (list_empty(&if_list) || (!is_vis_server())) {
+	if (list_empty(&if_list) || (!atomic_read(&vis_srv_enabled))) {
 		rcu_read_unlock();
 		goto end;
 	}
diff --git a/send.c b/send.c
index fd48f3f..92d14a6 100644
--- a/send.c
+++ b/send.c
@@ -279,7 +279,7 @@ void schedule_own_packet(struct batman_if *batman_if)
 	/* change sequence number to network order */
 	batman_packet->seqno = htons((uint16_t)atomic_read(&batman_if->seqno));
 
-	if (is_vis_server())
+	if (atomic_read(&vis_srv_enabled))
 		batman_packet->flags = VIS_SERVER;
 	else
 		batman_packet->flags = 0;
diff --git a/vis.c b/vis.c
index fa8afdb..e7c14b5 100644
--- a/vis.c
+++ b/vis.c
@@ -49,41 +49,6 @@ static void free_info(void *data)
 	kfree(info);
 }
 
-/* set the mode of the visualization to client or server */
-void vis_set_mode(int mode)
-{
-	unsigned long flags;
-	spin_lock_irqsave(&vis_hash_lock, flags);
-
-	if (my_vis_info != NULL)
-		my_vis_info->packet.vis_type = mode;
-
-	spin_unlock_irqrestore(&vis_hash_lock, flags);
-}
-
-/* is_vis_server(), locked outside */
-static int is_vis_server_locked(void)
-{
-	if (my_vis_info != NULL)
-		if (my_vis_info->packet.vis_type == VIS_TYPE_SERVER_SYNC)
-			return 1;
-
-	return 0;
-}
-
-/* get the current set mode */
-int is_vis_server(void)
-{
-	int ret = 0;
-	unsigned long flags;
-
-	spin_lock_irqsave(&vis_hash_lock, flags);
-	ret = is_vis_server_locked();
-	spin_unlock_irqrestore(&vis_hash_lock, flags);
-
-	return ret;
-}
-
 /* Compare two vis packets, used by the hashing algorithm */
 static int vis_info_cmp(void *data1, void *data2)
 {
@@ -280,7 +245,7 @@ void receive_server_sync_packet(struct vis_packet *vis_packet, int vis_info_len)
 
 	/* only if we are server ourselves and packet is newer than the one in
 	 * hash.*/
-	if (is_vis_server_locked() && is_new) {
+	if (atomic_read(&vis_srv_enabled) && is_new) {
 		memcpy(info->packet.target_orig, broadcastAddr, ETH_ALEN);
 		if (list_empty(&info->send_list))
 			list_add_tail(&info->send_list, &send_list);
@@ -309,7 +274,7 @@ void receive_client_update_packet(struct vis_packet *vis_packet,
 
 
 	/* send only if we're the target server or ... */
-	if (is_vis_server_locked() &&
+	if (atomic_read(&vis_srv_enabled) &&
 	    is_my_mac(info->packet.target_orig) &&
 	    is_new) {
 		info->packet.vis_type = VIS_TYPE_SERVER_SYNC;	/* upgrade! */
@@ -380,7 +345,7 @@ static int generate_vis_packet(void)
 	info->packet.seqno++;
 	info->packet.entries = 0;
 
-	if (!is_vis_server_locked()) {
+	if (!atomic_read(&vis_srv_enabled)) {
 		best_tq = find_best_vis_server(info);
 		if (best_tq < 0) {
 			spin_unlock_irqrestore(&orig_hash_lock, flags);
diff --git a/vis.h b/vis.h
index 2e24258..0cdafde 100644
--- a/vis.h
+++ b/vis.h
@@ -48,8 +48,6 @@ struct recvlist_node {
 extern struct hashtable_t *vis_hash;
 extern spinlock_t vis_hash_lock;
 
-void vis_set_mode(int mode);
-int is_vis_server(void);
 void proc_vis_read_entry(struct seq_file *seq,
 				struct vis_info_entry *entry,
 				struct hlist_head *if_list,
-- 
1.6.5.7

Re: [B.A.T.M.A.N.] [PATCH 1/3] batman-adv: atomic variable for vis-srv activation

[Marek Lindner]

Hi,

> This fixes the bug discovered by Marek which did not allow turning on
> the vis-server before an interface has been added. This is now being
> done in a similar way as for (de)activating the aggregation mode with an
> atomic variable.

great that you work on this!  :)

I'm no expert on the vis code but here my 2 cents:
* You hard-coded integer values instead of using the existing defines. Any 
reason for that ?
* my_vis_info->packet.vis_type never gets updated ?
* This patch removes functionality from the /proc parsing function although 
this is not related to the patch-topic. 
* checkpatch reports 8 errors & 8 warnings

Regards,
Marek

Re: [B.A.T.M.A.N.] [PATCH 1/3] batman-adv: atomic variable for vis-srv activation

[Linus Lüssing]

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

On Sun, Jan 03, 2010 at 03:26:02AM +0800, Marek Lindner wrote:
> 
> Hi,
> 
> > This fixes the bug discovered by Marek which did not allow turning on
> > the vis-server before an interface has been added. This is now being
> > done in a similar way as for (de)activating the aggregation mode with an
> > atomic variable.
> 
> great that you work on this!  :)
> 
> I'm no expert on the vis code but here my 2 cents:
> * You hard-coded integer values instead of using the existing defines. Any 
> reason for that ?
Hmm, so far we are having too modes only, vis server being enabled
or disabled. But in those packet functions we are talking about
types (two ones only so far again) instead. In the second one I found it
ok to use the defines, but for the proc.c switching, I found a
simple 0 or 1 more intuitive, as you already know that this
function is for enabling/disabling the vis server and you don't
have to wonder about a certain vis-types field in the vis-packet
format. Hmm, I'm also wondering, are there any plans for
introducing other vis_types? (What would you think about changing
the 8 bits vis_type field to flags instead?)
> * my_vis_info->packet.vis_type never gets updated ?
Damn it, you are right :). I assume, that it'd be ok to remove the default
value being set in vis_init and explicitly setting this field
every time a new vis packet gets generated in generate_vis_packet()
(instead of updating this variable from proc.c too)?
> * This patch removes functionality from the /proc parsing function although 
> this is not related to the patch-topic.
Yes, sorry, you're right I'd removed the
server/client/enabled/disabled things and added just 0/1 as input
values for proc to make the syntax similar to the aggregation
stuff and simple in general for the kernel module. I'm going to put
that in a seperate patch.
> * checkpatch reports 8 errors & 8 warnings
Ehm, just got 1 error at the else part... (sorry, never used
checkpatch before, hope that I'm not handling it wrong in any
way...)
> 
> Regards,
> Marek
> _______________________________________________
> B.A.T.M.A.N mailing list
> B.A.T.M.A.N@lists.open-mesh.net
> https://lists.open-mesh.net/mm/listinfo/b.a.t.m.a.n
> 

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

Re: [B.A.T.M.A.N.] [PATCH 1/3] batman-adv: atomic variable for vis-srv activation

[Andrew Lunn]

> > * checkpatch reports 8 errors & 8 warnings
> Ehm, just got 1 error at the else part... (sorry, never used
> checkpatch before, hope that I'm not handling it wrong in any
> way...)

The number of errors will vary depending on which version of
checkpatch you use. I tend to use the one from the current -rc kernel,
or when sending patches to GregKH, checkpatch from linux-next.

I suggest you don't use anything older than 2.6.32.

  Andrew

Re: [B.A.T.M.A.N.] [PATCH 1/3] batman-adv: atomic variable for vis-srv activation

[Marek Lindner]

On Tuesday 05 January 2010 12:20:58 Linus Lüssing wrote:
> Hmm, so far we are having too modes only, vis server being enabled
> or disabled. But in those packet functions we are talking about
> types (two ones only so far again) instead. In the second one I found it
> ok to use the defines, but for the proc.c switching, I found a
> simple 0 or 1 more intuitive, as you already know that this
> function is for enabling/disabling the vis server and you don't
> have to wonder about a certain vis-types field in the vis-packet
> format. Hmm, I'm also wondering, are there any plans for
> introducing other vis_types? (What would you think about changing
> the 8 bits vis_type field to flags instead?)

I think using defines instead of hard-coded values always is the better choice. 
Even if it is more intuitive for you now you never know when this code is 
going to be extended. In addition, you still want to understand the code in 6 
months and/or make it easier of others to understand.

Whether it makes sense to rename more variables / defines is another discussion 
and belongs into another patch. :)


> Damn it, you are right :). I assume, that it'd be ok to remove the default
> value being set in vis_init and explicitly setting this field
> every time a new vis packet gets generated in generate_vis_packet()
> (instead of updating this variable from proc.c too)?

I think a default value is good to have.  ;-)
generate_vis_packet() looks like a good place to update the packet's variable.


> Yes, sorry, you're right I'd removed the
> server/client/enabled/disabled things and added just 0/1 as input
> values for proc to make the syntax similar to the aggregation
> stuff and simple in general for the kernel module. I'm going to put
> that in a seperate patch.

Great.

Regards,
Marek

[B.A.T.M.A.N.] [PATCH] batman-adv: atomic variable for vis-srv activation

[Linus Lüssing]

This fixes the bug discovered by Marek Lindner which did not allow
turning on the vis-server before an interface has been added. With this
patch we are using a global atomic variable for activating and
deactiating the vis-server-mode instead, which can be used before
inserting an interface.

Signed-off-by: Linus Lüssing <linus.luessing@web.de>
---
 batman-adv-kernelland/main.c |    2 +
 batman-adv-kernelland/main.h |    1 +
 batman-adv-kernelland/proc.c |   13 ++++++-----
 batman-adv-kernelland/send.c |    3 +-
 batman-adv-kernelland/vis.c  |   45 +++++------------------------------------
 batman-adv-kernelland/vis.h  |    2 -
 6 files changed, 18 insertions(+), 48 deletions(-)

diff --git a/batman-adv-kernelland/main.c b/batman-adv-kernelland/main.c
index a64f070..002e6ea 100644
--- a/batman-adv-kernelland/main.c
+++ b/batman-adv-kernelland/main.c
@@ -46,6 +46,7 @@ DEFINE_SPINLOCK(forw_bcast_list_lock);
 
 atomic_t originator_interval;
 atomic_t vis_interval;
+atomic_t vis_mode;
 atomic_t aggregation_enabled;
 int16_t num_hna;
 int16_t num_ifs;
@@ -86,6 +87,7 @@ int init_module(void)
 	atomic_set(&originator_interval, 1000);
 	atomic_set(&vis_interval, 1000);/* TODO: raise this later, this is only
 					 * for debugging now. */
+	atomic_set(&vis_mode, VIS_TYPE_CLIENT_UPDATE);
 	atomic_set(&aggregation_enabled, 1);
 	atomic_set(&gw_mode, GW_MODE_OFF);
 	atomic_set(&gw_srv_class, 0);
diff --git a/batman-adv-kernelland/main.h b/batman-adv-kernelland/main.h
index 3dfe5fe..5daa9a4 100644
--- a/batman-adv-kernelland/main.h
+++ b/batman-adv-kernelland/main.h
@@ -130,6 +130,7 @@ extern spinlock_t forw_bcast_list_lock;
 
 extern atomic_t originator_interval;
 extern atomic_t vis_interval;
+extern atomic_t vis_mode;
 extern atomic_t aggregation_enabled;
 extern int16_t num_hna;
 extern int16_t num_ifs;
diff --git a/batman-adv-kernelland/proc.c b/batman-adv-kernelland/proc.c
index 747ed5f..c41dc19 100644
--- a/batman-adv-kernelland/proc.c
+++ b/batman-adv-kernelland/proc.c
@@ -342,11 +342,11 @@ static ssize_t proc_vis_srv_write(struct file *file, const char __user * buffer,
 	if ((strcmp(vis_mode_string, "client") == 0) ||
 			(strcmp(vis_mode_string, "disabled") == 0)) {
 		printk(KERN_INFO "batman-adv:Setting VIS mode to client (disabling vis server)\n");
-		vis_set_mode(VIS_TYPE_CLIENT_UPDATE);
+		atomic_set(&vis_mode, VIS_TYPE_CLIENT_UPDATE);
 	} else if ((strcmp(vis_mode_string, "server") == 0) ||
 			(strcmp(vis_mode_string, "enabled") == 0)) {
 		printk(KERN_INFO "batman-adv:Setting VIS mode to server (enabling vis server)\n");
-		vis_set_mode(VIS_TYPE_SERVER_SYNC);
+		atomic_set(&vis_mode, VIS_TYPE_SERVER_SYNC);
 	} else
 		printk(KERN_ERR "batman-adv:Unknown VIS mode: %s\n",
 		       vis_mode_string);
@@ -357,12 +357,12 @@ static ssize_t proc_vis_srv_write(struct file *file, const char __user * buffer,
 
 static int proc_vis_srv_read(struct seq_file *seq, void *offset)
 {
-	int vis_server = is_vis_server();
+	int vis_server = atomic_read(&vis_mode);
 
 	seq_printf(seq, "[%c] client mode (server disabled) \n",
-			(!vis_server) ? 'x' : ' ');
+			(vis_server == VIS_TYPE_CLIENT_UPDATE) ? 'x' : ' ');
 	seq_printf(seq, "[%c] server mode (server enabled) \n",
-			(vis_server) ? 'x' : ' ');
+			(vis_server == VIS_TYPE_SERVER_SYNC) ? 'x' : ' ');
 
 	return 0;
 }
@@ -381,9 +381,10 @@ static int proc_vis_data_read(struct seq_file *seq, void *offset)
 	int i;
 	char tmp_addr_str[ETH_STR_LEN];
 	unsigned long flags;
+	int vis_server = atomic_read(&vis_mode);
 
 	rcu_read_lock();
-	if (list_empty(&if_list) || (!is_vis_server())) {
+	if (list_empty(&if_list) || (vis_server == VIS_TYPE_CLIENT_UPDATE)) {
 		rcu_read_unlock();
 		goto end;
 	}
diff --git a/batman-adv-kernelland/send.c b/batman-adv-kernelland/send.c
index a40e8b8..dc9b217 100644
--- a/batman-adv-kernelland/send.c
+++ b/batman-adv-kernelland/send.c
@@ -251,6 +251,7 @@ void schedule_own_packet(struct batman_if *batman_if)
 {
 	unsigned long send_time;
 	struct batman_packet *batman_packet;
+	int vis_server = atomic_read(&vis_mode);
 
 	/**
 	 * the interface gets activated here to avoid race conditions between
@@ -275,7 +276,7 @@ void schedule_own_packet(struct batman_if *batman_if)
 	/* change sequence number to network order */
 	batman_packet->seqno = htons((uint16_t)atomic_read(&batman_if->seqno));
 
-	if (is_vis_server())
+	if (vis_server == VIS_TYPE_SERVER_SYNC)
 		batman_packet->flags = VIS_SERVER;
 	else
 		batman_packet->flags = 0;
diff --git a/batman-adv-kernelland/vis.c b/batman-adv-kernelland/vis.c
index fa8afdb..b118d1e 100644
--- a/batman-adv-kernelland/vis.c
+++ b/batman-adv-kernelland/vis.c
@@ -49,41 +49,6 @@ static void free_info(void *data)
 	kfree(info);
 }
 
-/* set the mode of the visualization to client or server */
-void vis_set_mode(int mode)
-{
-	unsigned long flags;
-	spin_lock_irqsave(&vis_hash_lock, flags);
-
-	if (my_vis_info != NULL)
-		my_vis_info->packet.vis_type = mode;
-
-	spin_unlock_irqrestore(&vis_hash_lock, flags);
-}
-
-/* is_vis_server(), locked outside */
-static int is_vis_server_locked(void)
-{
-	if (my_vis_info != NULL)
-		if (my_vis_info->packet.vis_type == VIS_TYPE_SERVER_SYNC)
-			return 1;
-
-	return 0;
-}
-
-/* get the current set mode */
-int is_vis_server(void)
-{
-	int ret = 0;
-	unsigned long flags;
-
-	spin_lock_irqsave(&vis_hash_lock, flags);
-	ret = is_vis_server_locked();
-	spin_unlock_irqrestore(&vis_hash_lock, flags);
-
-	return ret;
-}
-
 /* Compare two vis packets, used by the hashing algorithm */
 static int vis_info_cmp(void *data1, void *data2)
 {
@@ -272,6 +237,7 @@ void receive_server_sync_packet(struct vis_packet *vis_packet, int vis_info_len)
 	struct vis_info *info;
 	int is_new;
 	unsigned long flags;
+	int vis_server = atomic_read(&vis_mode);
 
 	spin_lock_irqsave(&vis_hash_lock, flags);
 	info = add_packet(vis_packet, vis_info_len, &is_new);
@@ -280,7 +246,7 @@ void receive_server_sync_packet(struct vis_packet *vis_packet, int vis_info_len)
 
 	/* only if we are server ourselves and packet is newer than the one in
 	 * hash.*/
-	if (is_vis_server_locked() && is_new) {
+	if (vis_server == VIS_TYPE_SERVER_SYNC && is_new) {
 		memcpy(info->packet.target_orig, broadcastAddr, ETH_ALEN);
 		if (list_empty(&info->send_list))
 			list_add_tail(&info->send_list, &send_list);
@@ -296,6 +262,7 @@ void receive_client_update_packet(struct vis_packet *vis_packet,
 	struct vis_info *info;
 	int is_new;
 	unsigned long flags;
+	int vis_server = atomic_read(&vis_mode);
 
 	/* clients shall not broadcast. */
 	if (is_bcast(vis_packet->target_orig))
@@ -309,7 +276,7 @@ void receive_client_update_packet(struct vis_packet *vis_packet,
 
 
 	/* send only if we're the target server or ... */
-	if (is_vis_server_locked() &&
+	if (vis_server == VIS_TYPE_SERVER_SYNC  &&
 	    is_my_mac(info->packet.target_orig) &&
 	    is_new) {
 		info->packet.vis_type = VIS_TYPE_SERVER_SYNC;	/* upgrade! */
@@ -373,6 +340,7 @@ static int generate_vis_packet(void)
 	unsigned long flags;
 
 	info->first_seen = jiffies;
+	info->packet.vis_type = atomic_read(&vis_mode);
 
 	spin_lock_irqsave(&orig_hash_lock, flags);
 	memcpy(info->packet.target_orig, broadcastAddr, ETH_ALEN);
@@ -380,7 +348,7 @@ static int generate_vis_packet(void)
 	info->packet.seqno++;
 	info->packet.entries = 0;
 
-	if (!is_vis_server_locked()) {
+	if (info->packet.vis_type == VIS_TYPE_CLIENT_UPDATE) {
 		best_tq = find_best_vis_server(info);
 		if (best_tq < 0) {
 			spin_unlock_irqrestore(&orig_hash_lock, flags);
@@ -578,7 +546,6 @@ int vis_init(void)
 	INIT_LIST_HEAD(&my_vis_info->send_list);
 	my_vis_info->packet.version = COMPAT_VERSION;
 	my_vis_info->packet.packet_type = BAT_VIS;
-	my_vis_info->packet.vis_type = VIS_TYPE_CLIENT_UPDATE;
 	my_vis_info->packet.ttl = TTL;
 	my_vis_info->packet.seqno = 0;
 	my_vis_info->packet.entries = 0;
diff --git a/batman-adv-kernelland/vis.h b/batman-adv-kernelland/vis.h
index 2e24258..0cdafde 100644
--- a/batman-adv-kernelland/vis.h
+++ b/batman-adv-kernelland/vis.h
@@ -48,8 +48,6 @@ struct recvlist_node {
 extern struct hashtable_t *vis_hash;
 extern spinlock_t vis_hash_lock;
 
-void vis_set_mode(int mode);
-int is_vis_server(void);
 void proc_vis_read_entry(struct seq_file *seq,
 				struct vis_info_entry *entry,
 				struct hlist_head *if_list,
-- 
1.6.6

Re: [B.A.T.M.A.N.] [PATCH] batman-adv: atomic variable for vis-srv activation

[Linus Lüssing]

I now decided to use a more general variable name and as Marek
suggested the checks against the given define-values. This should
keep the structure of several "vis-modes" in general, as it had
been done before by Simon, instead of just "on" and "off".

The checkpatch-script from the batman-adv linux git branch showed
me 0 errors/warnings for this one.

- So far, just had the time to test this patch with my laptop and
  home-server only. But switching as well as receiving/displaying
  the vis-stuff worked fine for those (didn't have a closer look
  with tcpdump/wireshark for this vis-type part though).

Cheers, Linus

Re: [B.A.T.M.A.N.] [PATCH] batman-adv: atomic variable for vis-srv activation

[Sven Eckelmann]

[-- Attachment #1: Type: Text/Plain, Size: 2650 bytes --]

Marek Lindner wrote:
> On Monday 11 January 2010 12:57:56 you wrote:
> > This fixes the bug discovered by Marek Lindner which did not allow
> > turning on the vis-server before an interface has been added. With this
> > patch we are using a global atomic variable for activating and
> > deactiating the vis-server-mode instead, which can be used before
> > inserting an interface.
> 
> Off-list:
> Wollte mir grad deinen Patch reinziehen, aber trotz "git send-email" ist
>  auch diese Mail is base64 encoded ?? Im Header sehe ich jedenfalls
>  "X-Mailer: git- send-email 1.6.6". Evtl. hängt das mit charset (utf8)
>  zusammen ?

Aaaalso. Ich habe ersteinmal keine Ahnung woher es kommt. git-format-patch
produziert sowas jedenfalls nicht. git-send-email eigentlich auch nicht (der 
bleibt immer bei "Content-Type: text/plain; ...". Wenn ich jetzt im Git 
Quellcode suche, dann finde ich auch nur funktionen um base64 in Mails zu 
dekodieren.

> @Sven: Hast du eine Idee, woran das liegen könnte ?

Vielleicht macht der Mailman sowas oder irgendwas anderes auf open-mesh.org? 
Ich habe jetzt einfach mal an meine Adresse auf open-mesh.net die Mail 
geschickt. Die sieht aber vollkommen in Ordnung aus.

Wenn ich in die mbox von mailman schauen 
/var/lib/mailman/archives/private/b.a.t.m.a.n.mbox/b.a.t.m.a.n.mbox , dann 
sieht die Mail so aus wie ich sie von git-send-email erwarten wuerde. Kann es 
also sein, dass es beim wieder raussenden passiert? Also nicht das raussenden 
ueber sendmail, sondern das Vorbereiten von mailman.

NEWS fuer 2.1.8 (15-Apr-2006)

{{{
     - Updated email library to 2.5.7 which will encode payload into qp/base64
       upon setting.  This enabled backing out the scrubber related patches
       including 'X-Mailman-Scrubbed' header in 2.1.7.
}}}

Wieso es das macht ist vielleicht(tm) das "Content-Transfer-Encoding: 8bit" in 
der Mail von Linus + Decorating. Wenn wir das rausnehmen, sollte es eigentlich 
gehen.

Siehe dazu auch Mailman/Handlers/Decorate.py  process(..)

Ich habe ersteinmal in 
https://lists.open-mesh.org/mm/admin/b.a.t.m.a.n/?VARHELP=nondigest/msg_footer
den Footer
{{{
_______________________________________________
%(real_name)s mailing list
%(real_name)s@%(host_name)s
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

}}}
entfernt. Vielleicht kann Linus nocheinmal exakt genauso seinen Patch an die 
Mailingliste senden.

....das waere jedenfalls bis jetzt meine Vermutung. Leider weiss ich gerade 
nicht wie ich das am einfachsten Testen koennte (bis auf das Linus den Patch 
einfach nochmal sendet).

Gruesse,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [B.A.T.M.A.N.] [PATCH] batman-adv: atomic variable for vis-srv activation

[Sven Eckelmann]

[-- Attachment #1: Type: Text/Plain, Size: 184 bytes --]

Sorry for the german mail. This was intended to be off the list. I must 
accidentally brought that mail back to the mailinglist while playing with 
mailman.

Best regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [B.A.T.M.A.N.] [PATCH] batman-adv: atomic variable for vis-srv activation

[Andrew Lunn]

On Fri, Jan 15, 2010 at 07:34:38PM +0100, Sven Eckelmann wrote:
> Marek Lindner wrote:
> > On Monday 11 January 2010 12:57:56 you wrote:
> > > This fixes the bug discovered by Marek Lindner which did not allow
> > > turning on the vis-server before an interface has been added. With this
> > > patch we are using a global atomic variable for activating and
> > > deactiating the vis-server-mode instead, which can be used before
> > > inserting an interface.
> > 
> > Off-list:
> > Wollte mir grad deinen Patch reinziehen, aber trotz "git send-email" ist
> >  auch diese Mail is base64 encoded ?? Im Header sehe ich jedenfalls
> >  "X-Mailer: git- send-email 1.6.6". Evtl. h?ngt das mit charset (utf8)
> >  zusammen ?
> 
> Aaaalso. Ich habe ersteinmal keine Ahnung woher es kommt. git-format-patch
> produziert sowas jedenfalls nicht. git-send-email eigentlich auch nicht (der 
> bleibt immer bei "Content-Type: text/plain; ...". Wenn ich jetzt im Git 
> Quellcode suche, dann finde ich auch nur funktionen um base64 in Mails zu 
> dekodieren.

All the patches i've generated so far from git have been plain old
ASCII, even though some contain extended characters, because of
umlauts in names. So i agree with Sven, it is more likely to be a
problem with the mail system somewhere, not git.

	Andrew

[B.A.T.M.A.N.] [PATCH] batman-adv: atomic variable for vis-srv activation

[Linus Lüssing]

This fixes the bug discovered by Marek Lindner which did not allow
turning on the vis-server before an interface has been added. With this
patch we are using a global atomic variable for activating and
deactiating the vis-server-mode instead, which can be used before
inserting an interface.

Signed-off-by: Linus Lüssing <linus.luessing@web.de>
---
 batman-adv-kernelland/main.c |    2 +
 batman-adv-kernelland/main.h |    1 +
 batman-adv-kernelland/proc.c |   13 ++++++-----
 batman-adv-kernelland/send.c |    3 +-
 batman-adv-kernelland/vis.c  |   45 +++++------------------------------------
 batman-adv-kernelland/vis.h  |    2 -
 6 files changed, 18 insertions(+), 48 deletions(-)

diff --git a/batman-adv-kernelland/main.c b/batman-adv-kernelland/main.c
index a64f070..002e6ea 100644
--- a/batman-adv-kernelland/main.c
+++ b/batman-adv-kernelland/main.c
@@ -46,6 +46,7 @@ DEFINE_SPINLOCK(forw_bcast_list_lock);
 
 atomic_t originator_interval;
 atomic_t vis_interval;
+atomic_t vis_mode;
 atomic_t aggregation_enabled;
 int16_t num_hna;
 int16_t num_ifs;
@@ -86,6 +87,7 @@ int init_module(void)
 	atomic_set(&originator_interval, 1000);
 	atomic_set(&vis_interval, 1000);/* TODO: raise this later, this is only
 					 * for debugging now. */
+	atomic_set(&vis_mode, VIS_TYPE_CLIENT_UPDATE);
 	atomic_set(&aggregation_enabled, 1);
 	atomic_set(&gw_mode, GW_MODE_OFF);
 	atomic_set(&gw_srv_class, 0);
diff --git a/batman-adv-kernelland/main.h b/batman-adv-kernelland/main.h
index 3dfe5fe..5daa9a4 100644
--- a/batman-adv-kernelland/main.h
+++ b/batman-adv-kernelland/main.h
@@ -130,6 +130,7 @@ extern spinlock_t forw_bcast_list_lock;
 
 extern atomic_t originator_interval;
 extern atomic_t vis_interval;
+extern atomic_t vis_mode;
 extern atomic_t aggregation_enabled;
 extern int16_t num_hna;
 extern int16_t num_ifs;
diff --git a/batman-adv-kernelland/proc.c b/batman-adv-kernelland/proc.c
index 747ed5f..c41dc19 100644
--- a/batman-adv-kernelland/proc.c
+++ b/batman-adv-kernelland/proc.c
@@ -342,11 +342,11 @@ static ssize_t proc_vis_srv_write(struct file *file, const char __user * buffer,
 	if ((strcmp(vis_mode_string, "client") == 0) ||
 			(strcmp(vis_mode_string, "disabled") == 0)) {
 		printk(KERN_INFO "batman-adv:Setting VIS mode to client (disabling vis server)\n");
-		vis_set_mode(VIS_TYPE_CLIENT_UPDATE);
+		atomic_set(&vis_mode, VIS_TYPE_CLIENT_UPDATE);
 	} else if ((strcmp(vis_mode_string, "server") == 0) ||
 			(strcmp(vis_mode_string, "enabled") == 0)) {
 		printk(KERN_INFO "batman-adv:Setting VIS mode to server (enabling vis server)\n");
-		vis_set_mode(VIS_TYPE_SERVER_SYNC);
+		atomic_set(&vis_mode, VIS_TYPE_SERVER_SYNC);
 	} else
 		printk(KERN_ERR "batman-adv:Unknown VIS mode: %s\n",
 		       vis_mode_string);
@@ -357,12 +357,12 @@ static ssize_t proc_vis_srv_write(struct file *file, const char __user * buffer,
 
 static int proc_vis_srv_read(struct seq_file *seq, void *offset)
 {
-	int vis_server = is_vis_server();
+	int vis_server = atomic_read(&vis_mode);
 
 	seq_printf(seq, "[%c] client mode (server disabled) \n",
-			(!vis_server) ? 'x' : ' ');
+			(vis_server == VIS_TYPE_CLIENT_UPDATE) ? 'x' : ' ');
 	seq_printf(seq, "[%c] server mode (server enabled) \n",
-			(vis_server) ? 'x' : ' ');
+			(vis_server == VIS_TYPE_SERVER_SYNC) ? 'x' : ' ');
 
 	return 0;
 }
@@ -381,9 +381,10 @@ static int proc_vis_data_read(struct seq_file *seq, void *offset)
 	int i;
 	char tmp_addr_str[ETH_STR_LEN];
 	unsigned long flags;
+	int vis_server = atomic_read(&vis_mode);
 
 	rcu_read_lock();
-	if (list_empty(&if_list) || (!is_vis_server())) {
+	if (list_empty(&if_list) || (vis_server == VIS_TYPE_CLIENT_UPDATE)) {
 		rcu_read_unlock();
 		goto end;
 	}
diff --git a/batman-adv-kernelland/send.c b/batman-adv-kernelland/send.c
index a40e8b8..dc9b217 100644
--- a/batman-adv-kernelland/send.c
+++ b/batman-adv-kernelland/send.c
@@ -251,6 +251,7 @@ void schedule_own_packet(struct batman_if *batman_if)
 {
 	unsigned long send_time;
 	struct batman_packet *batman_packet;
+	int vis_server = atomic_read(&vis_mode);
 
 	/**
 	 * the interface gets activated here to avoid race conditions between
@@ -275,7 +276,7 @@ void schedule_own_packet(struct batman_if *batman_if)
 	/* change sequence number to network order */
 	batman_packet->seqno = htons((uint16_t)atomic_read(&batman_if->seqno));
 
-	if (is_vis_server())
+	if (vis_server == VIS_TYPE_SERVER_SYNC)
 		batman_packet->flags = VIS_SERVER;
 	else
 		batman_packet->flags = 0;
diff --git a/batman-adv-kernelland/vis.c b/batman-adv-kernelland/vis.c
index fa8afdb..b118d1e 100644
--- a/batman-adv-kernelland/vis.c
+++ b/batman-adv-kernelland/vis.c
@@ -49,41 +49,6 @@ static void free_info(void *data)
 	kfree(info);
 }
 
-/* set the mode of the visualization to client or server */
-void vis_set_mode(int mode)
-{
-	unsigned long flags;
-	spin_lock_irqsave(&vis_hash_lock, flags);
-
-	if (my_vis_info != NULL)
-		my_vis_info->packet.vis_type = mode;
-
-	spin_unlock_irqrestore(&vis_hash_lock, flags);
-}
-
-/* is_vis_server(), locked outside */
-static int is_vis_server_locked(void)
-{
-	if (my_vis_info != NULL)
-		if (my_vis_info->packet.vis_type == VIS_TYPE_SERVER_SYNC)
-			return 1;
-
-	return 0;
-}
-
-/* get the current set mode */
-int is_vis_server(void)
-{
-	int ret = 0;
-	unsigned long flags;
-
-	spin_lock_irqsave(&vis_hash_lock, flags);
-	ret = is_vis_server_locked();
-	spin_unlock_irqrestore(&vis_hash_lock, flags);
-
-	return ret;
-}
-
 /* Compare two vis packets, used by the hashing algorithm */
 static int vis_info_cmp(void *data1, void *data2)
 {
@@ -272,6 +237,7 @@ void receive_server_sync_packet(struct vis_packet *vis_packet, int vis_info_len)
 	struct vis_info *info;
 	int is_new;
 	unsigned long flags;
+	int vis_server = atomic_read(&vis_mode);
 
 	spin_lock_irqsave(&vis_hash_lock, flags);
 	info = add_packet(vis_packet, vis_info_len, &is_new);
@@ -280,7 +246,7 @@ void receive_server_sync_packet(struct vis_packet *vis_packet, int vis_info_len)
 
 	/* only if we are server ourselves and packet is newer than the one in
 	 * hash.*/
-	if (is_vis_server_locked() && is_new) {
+	if (vis_server == VIS_TYPE_SERVER_SYNC && is_new) {
 		memcpy(info->packet.target_orig, broadcastAddr, ETH_ALEN);
 		if (list_empty(&info->send_list))
 			list_add_tail(&info->send_list, &send_list);
@@ -296,6 +262,7 @@ void receive_client_update_packet(struct vis_packet *vis_packet,
 	struct vis_info *info;
 	int is_new;
 	unsigned long flags;
+	int vis_server = atomic_read(&vis_mode);
 
 	/* clients shall not broadcast. */
 	if (is_bcast(vis_packet->target_orig))
@@ -309,7 +276,7 @@ void receive_client_update_packet(struct vis_packet *vis_packet,
 
 
 	/* send only if we're the target server or ... */
-	if (is_vis_server_locked() &&
+	if (vis_server == VIS_TYPE_SERVER_SYNC  &&
 	    is_my_mac(info->packet.target_orig) &&
 	    is_new) {
 		info->packet.vis_type = VIS_TYPE_SERVER_SYNC;	/* upgrade! */
@@ -373,6 +340,7 @@ static int generate_vis_packet(void)
 	unsigned long flags;
 
 	info->first_seen = jiffies;
+	info->packet.vis_type = atomic_read(&vis_mode);
 
 	spin_lock_irqsave(&orig_hash_lock, flags);
 	memcpy(info->packet.target_orig, broadcastAddr, ETH_ALEN);
@@ -380,7 +348,7 @@ static int generate_vis_packet(void)
 	info->packet.seqno++;
 	info->packet.entries = 0;
 
-	if (!is_vis_server_locked()) {
+	if (info->packet.vis_type == VIS_TYPE_CLIENT_UPDATE) {
 		best_tq = find_best_vis_server(info);
 		if (best_tq < 0) {
 			spin_unlock_irqrestore(&orig_hash_lock, flags);
@@ -578,7 +546,6 @@ int vis_init(void)
 	INIT_LIST_HEAD(&my_vis_info->send_list);
 	my_vis_info->packet.version = COMPAT_VERSION;
 	my_vis_info->packet.packet_type = BAT_VIS;
-	my_vis_info->packet.vis_type = VIS_TYPE_CLIENT_UPDATE;
 	my_vis_info->packet.ttl = TTL;
 	my_vis_info->packet.seqno = 0;
 	my_vis_info->packet.entries = 0;
diff --git a/batman-adv-kernelland/vis.h b/batman-adv-kernelland/vis.h
index 2e24258..0cdafde 100644
--- a/batman-adv-kernelland/vis.h
+++ b/batman-adv-kernelland/vis.h
@@ -48,8 +48,6 @@ struct recvlist_node {
 extern struct hashtable_t *vis_hash;
 extern spinlock_t vis_hash_lock;
 
-void vis_set_mode(int mode);
-int is_vis_server(void);
 void proc_vis_read_entry(struct seq_file *seq,
 				struct vis_info_entry *entry,
 				struct hlist_head *if_list,
-- 
1.6.6

Re: [B.A.T.M.A.N.] [PATCH] batman-adv: atomic variable for vis-srv activation

[Marek Lindner]

On Tuesday 19 January 2010 04:42:29 Linus Lüssing wrote:
> This fixes the bug discovered by Marek Lindner which did not allow
> turning on the vis-server before an interface has been added. With this
> patch we are using a global atomic variable for activating and
> deactiating the vis-server-mode instead, which can be used before
> inserting an interface.

Thanks! Applied in revision 1557.

Regards,
Marek