Skip to contentSkip to conversation

pull request: batman-adv 2012-04-18

PATCH New Antonio Quartulli 6 participants 31 messages Apr 18, 2012 – Apr 25, 2012 10 messages not in the archive

Subsystems (3)
Networking Network drivers Batman advanced
lore
Share & tools
lore

Antonio Quartulli

author
#0 lore raw

Hello David,

this is the fixed version of my previous pull request (issued on 2012-04-17). In this patchset the issues you reported have been fixed; moreover I'm also including two new patches (1/13 and 2/13) which are respectively:

1/13) a fix for Al Viro's report about the missing htons() 2/13) a fix for a wrongly duplicated line in a comment

The following changes since commit ecffe75f934b4e3c5301fe5db278068e0efb0d6b:

hippi: fix printk format in rrunner.c (2012-04-16 23:48:38 -0400)

are available in the git repository at:

git://git.open-mesh.org/linux-merge.git tags/batman-adv-for-davem

for you to fetch changes up to 1e5cc266dbc401d11aefb966ad35e651c2f67414:

batman-adv: skip the window protection test when the originator has no neighbours (2012-04-18 09:54:02 +0200)

----------------------------------------------------------------
Included changes:
* remove duplicated line in comment
* add htons() invocation for tt_crc as suggested by Al Viro
* OriGinator Message seqno initial value is now random
* some cleanups and fixes

----------------------------------------------------------------
Antonio Quartulli (5):
batman-adv: convert the tt_crc to network order
batman-adv: remove duplicated line in comment
batman-adv: use ETH_HLEN instead of sizeof(struct ethhdr)
batman-adv: print OGM seq numbers as unsigned int
batman-adv: skip the window protection test when the originator has no neighbours

Marek Lindner (8):
batman-adv: move ogm initialization into the proper function
batman-adv: refactoring API: find generalized name for bat_ogm_init callback
batman-adv: randomize initial seqno to avoid collision
batman-adv: add iface_disable() callback to routing API
batman-adv: handle routing code initialization properly
batman-adv: refactoring API: find generalized name for bat_ogm_init_primary callback
batman-adv: rename BATMAN_OGM_LEN to BATMAN_OGM_HLEN
batman-adv: mark existing ogm variables as batman iv

 net/batman-adv/bat_iv_ogm.c            |   63 +++++++++++++++++++++-----------
 net/batman-adv/bridge_loop_avoidance.c |   11 ++----
 net/batman-adv/hard-interface.c        |   33 +++++++++--------
 net/batman-adv/icmp_socket.c           |    4 +-
 net/batman-adv/main.c                  |    5 ++-
 net/batman-adv/packet.h                |    6 +--
 net/batman-adv/routing.c               |   10 ++---
 net/batman-adv/send.c                  |   14 +++----
 net/batman-adv/soft-interface.c        |    2 +-
 net/batman-adv/translation-table.c     |    2 +-
 net/batman-adv/types.h                 |   12 +++---
 net/batman-adv/vis.c                   |    8 ++--
 12 files changed, 96 insertions(+), 74 deletions(-)
#1 lore raw

PATCH 1/13 batman-adv: convert the tt_crc to network order

Before sending out a TT_Request packet we must convert the tt_crc field value to network order (since it is 16bits long).

Reported-by Al Viro <[email protected]>
Signed-off-by Antonio Quartulli <[email protected]>
---
 net/batman-adv/translation-table.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
net/batman-adv/translation-table.c +1 -1
index e16a369..a38d315 100644
@@ -1339,7 +1339,7 @@ static int send_tt_request(struct bat_priv *bat_priv,
 	memcpy(tt_request->dst, dst_orig_node->orig, ETH_ALEN);
 	tt_request->header.ttl = TTL;
 	tt_request->ttvn = ttvn;
-	tt_request->tt_data = tt_crc;
+	tt_request->tt_data = htons(tt_crc);
 	tt_request->flags = TT_REQUEST;
 
 	if (full_table)
Signature · 1 line
1.7.9.4
#2 lore raw

PATCH 2/13 batman-adv: remove duplicated line in comment

In reply to Antonio Quartulli

Remove an accidentally added duplicated line in a function comment

Signed-off-by Antonio Quartulli <[email protected]>
---
 net/batman-adv/bridge_loop_avoidance.c |    1 -
 1 file changed, 1 deletion(-)
net/batman-adv/bridge_loop_avoidance.c +0 -1
index 1cf18ac..c8642b5 100644
@@ -735,7 +735,6 @@ static int handle_claim(struct bat_priv *bat_priv,
 
 /**
  * @bat_priv: the bat priv with all the soft interface information
- * @bat_priv: the bat priv with all the soft interface information
  * @hw_src: the Hardware source in the ARP Header
  * @hw_dst: the Hardware destination in the ARP Header
  * @ethhdr: pointer to the Ethernet header of the claim frame
Signature · 1 line
1.7.9.4
#3 lore raw

PATCH 3/13 batman-adv: move ogm initialization into the proper function

In reply to Antonio Quartulli

From: Marek Lindner <[email protected]>

Signed-off-by Marek Lindner <[email protected]>
Signed-off-by Antonio Quartulli <[email protected]>
---
 net/batman-adv/hard-interface.c |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
net/batman-adv/hard-interface.c +7 -1
index 8c4b790..f152007 100644
@@ -332,7 +332,6 @@ int hardif_enable_interface(struct hard_iface *hard_iface,
 	hard_iface->batman_adv_ptype.dev = hard_iface->net_dev;
 	dev_add_pack(&hard_iface->batman_adv_ptype);
 
-	atomic_set(&hard_iface->seqno, 1);
 	atomic_set(&hard_iface->frag_seqno, 1);
 	bat_info(hard_iface->soft_iface, "Adding interface: %s\n",
 		 hard_iface->net_dev->name);

@@ -451,6 +450,13 @@ static struct hard_iface *hardif_add_interface(struct net_device *net_dev)
 	check_known_mac_addr(hard_iface->net_dev);
 	list_add_tail_rcu(&hard_iface->list, &hardif_list);
 
+	/**
+	 * This can't be called via a bat_priv callback because
+	 * we have no bat_priv yet.
+	 */
+	atomic_set(&hard_iface->seqno, 1);
+	hard_iface->packet_buff = NULL;
+
 	return hard_iface;
 
 free_if:
Signature · 1 line
1.7.9.4
#4 lore raw

PATCH 4/13 batman-adv: refactoring API: find generalized name for bat_ogm_init callback

In reply to Antonio Quartulli

From: Marek Lindner <[email protected]>

Signed-off-by Marek Lindner <[email protected]>
Signed-off-by Antonio Quartulli <[email protected]>
---
 net/batman-adv/bat_iv_ogm.c     |    4 ++--
 net/batman-adv/hard-interface.c |    2 +-
 net/batman-adv/main.c           |    2 +-
 net/batman-adv/types.h          |    4 ++--
 4 files changed, 6 insertions(+), 6 deletions(-)
net/batman-adv/bat_iv_ogm.c +2 -2
index fab1071..117b831 100644
@@ -30,7 +30,7 @@
 #include "send.h"
 #include "bat_algo.h"
 
-static void bat_iv_ogm_init(struct hard_iface *hard_iface)
+static void bat_iv_ogm_iface_enable(struct hard_iface *hard_iface)
 {
 	struct batman_ogm_packet *batman_ogm_packet;
 

@@ -1169,7 +1169,7 @@ static void bat_iv_ogm_receive(struct hard_iface *if_incoming,
 
 static struct bat_algo_ops batman_iv __read_mostly = {
 	.name = "BATMAN IV",
-	.bat_ogm_init = bat_iv_ogm_init,
+	.bat_iface_enable = bat_iv_ogm_iface_enable,
 	.bat_ogm_init_primary = bat_iv_ogm_init_primary,
 	.bat_ogm_update_mac = bat_iv_ogm_update_mac,
 	.bat_ogm_schedule = bat_iv_ogm_schedule,
net/batman-adv/hard-interface.c +1 -1
index f152007..4d9b85d 100644
@@ -312,7 +312,7 @@ int hardif_enable_interface(struct hard_iface *hard_iface,
 	hard_iface->soft_iface = soft_iface;
 	bat_priv = netdev_priv(hard_iface->soft_iface);
 
-	bat_priv->bat_algo_ops->bat_ogm_init(hard_iface);
+	bat_priv->bat_algo_ops->bat_iface_enable(hard_iface);
 
 	if (!hard_iface->packet_buff) {
 		bat_err(hard_iface->soft_iface,
net/batman-adv/main.c +1 -1
index e67ca96..ca8f395 100644
@@ -208,7 +208,7 @@ int bat_algo_register(struct bat_algo_ops *bat_algo_ops)
 	}
 
 	/* all algorithms must implement all ops (for now) */
-	if (!bat_algo_ops->bat_ogm_init ||
+	if (!bat_algo_ops->bat_iface_enable ||
 	    !bat_algo_ops->bat_ogm_init_primary ||
 	    !bat_algo_ops->bat_ogm_update_mac ||
 	    !bat_algo_ops->bat_ogm_schedule ||
net/batman-adv/types.h +2 -2
index a5b1a63..4b92248 100644
@@ -377,8 +377,8 @@ struct recvlist_node {
 struct bat_algo_ops {
 	struct hlist_node list;
 	char *name;
-	/* init OGM when hard-interface is enabled */
-	void (*bat_ogm_init)(struct hard_iface *hard_iface);
+	/* init routing info when hard-interface is enabled */
+	void (*bat_iface_enable)(struct hard_iface *hard_iface);
 	/* init primary OGM when primary interface is selected */
 	void (*bat_ogm_init_primary)(struct hard_iface *hard_iface);
 	/* init mac addresses of the OGM belonging to this hard-interface */
Signature · 1 line
1.7.9.4
#5 lore raw

PATCH 5/13 batman-adv: randomize initial seqno to avoid collision

In reply to Antonio Quartulli

From: Marek Lindner <[email protected]>

Signed-off-by Marek Lindner <[email protected]>
Signed-off-by Antonio Quartulli <[email protected]>
---
 net/batman-adv/bat_iv_ogm.c |    5 +++++
 1 file changed, 5 insertions(+)
net/batman-adv/bat_iv_ogm.c +5 -0
index 117b831..95bfc59 100644
@@ -33,6 +33,11 @@
 static void bat_iv_ogm_iface_enable(struct hard_iface *hard_iface)
 {
 	struct batman_ogm_packet *batman_ogm_packet;
+	uint32_t random_seqno;
+
+	/* randomize initial seqno to avoid collision */
+	get_random_bytes(&random_seqno, sizeof(random_seqno));
+	atomic_set(&hard_iface->seqno, random_seqno);
 
 	hard_iface->packet_len = BATMAN_OGM_LEN;
 	hard_iface->packet_buff = kmalloc(hard_iface->packet_len, GFP_ATOMIC);
Signature · 1 line
1.7.9.4
#6 lore raw

PATCH 6/13 batman-adv: add iface_disable() callback to routing API

In reply to Antonio Quartulli

From: Marek Lindner <[email protected]>

Signed-off-by Marek Lindner <[email protected]>
Signed-off-by Antonio Quartulli <[email protected]>
---
 net/batman-adv/bat_iv_ogm.c     |    7 +++++++
 net/batman-adv/hard-interface.c |    3 +--
 net/batman-adv/main.c           |    1 +
 net/batman-adv/types.h          |    2 ++
 4 files changed, 11 insertions(+), 2 deletions(-)
net/batman-adv/bat_iv_ogm.c +7 -0
index 95bfc59..4cc66db 100644
@@ -52,6 +52,12 @@ static void bat_iv_ogm_iface_enable(struct hard_iface *hard_iface)
 	batman_ogm_packet->ttvn = 0;
 }
 
+static void bat_iv_ogm_iface_disable(struct hard_iface *hard_iface)
+{
+	kfree(hard_iface->packet_buff);
+	hard_iface->packet_buff = NULL;
+}
+
 static void bat_iv_ogm_init_primary(struct hard_iface *hard_iface)
 {
 	struct batman_ogm_packet *batman_ogm_packet;

@@ -1175,6 +1181,7 @@ static void bat_iv_ogm_receive(struct hard_iface *if_incoming,
 static struct bat_algo_ops batman_iv __read_mostly = {
 	.name = "BATMAN IV",
 	.bat_iface_enable = bat_iv_ogm_iface_enable,
+	.bat_iface_disable = bat_iv_ogm_iface_disable,
 	.bat_ogm_init_primary = bat_iv_ogm_init_primary,
 	.bat_ogm_update_mac = bat_iv_ogm_update_mac,
 	.bat_ogm_schedule = bat_iv_ogm_schedule,
net/batman-adv/hard-interface.c +1 -2
index 4d9b85d..fd9715e 100644
@@ -397,8 +397,7 @@ void hardif_disable_interface(struct hard_iface *hard_iface)
 			hardif_free_ref(new_if);
 	}
 
-	kfree(hard_iface->packet_buff);
-	hard_iface->packet_buff = NULL;
+	bat_priv->bat_algo_ops->bat_iface_disable(hard_iface);
 	hard_iface->if_status = IF_NOT_IN_USE;
 
 	/* delete all references to this hard_iface */
net/batman-adv/main.c +1 -0
index ca8f395..a47a6ce 100644
@@ -209,6 +209,7 @@ int bat_algo_register(struct bat_algo_ops *bat_algo_ops)
 
 	/* all algorithms must implement all ops (for now) */
 	if (!bat_algo_ops->bat_iface_enable ||
+	    !bat_algo_ops->bat_iface_disable ||
 	    !bat_algo_ops->bat_ogm_init_primary ||
 	    !bat_algo_ops->bat_ogm_update_mac ||
 	    !bat_algo_ops->bat_ogm_schedule ||
net/batman-adv/types.h +2 -0
index 4b92248..b034cf2 100644
@@ -379,6 +379,8 @@ struct bat_algo_ops {
 	char *name;
 	/* init routing info when hard-interface is enabled */
 	void (*bat_iface_enable)(struct hard_iface *hard_iface);
+	/* de-init routing info when hard-interface is disabled */
+	void (*bat_iface_disable)(struct hard_iface *hard_iface);
 	/* init primary OGM when primary interface is selected */
 	void (*bat_ogm_init_primary)(struct hard_iface *hard_iface);
 	/* init mac addresses of the OGM belonging to this hard-interface */
Signature · 1 line
1.7.9.4
#7 lore raw

PATCH 7/13 batman-adv: handle routing code initialization properly

In reply to Antonio Quartulli

From: Marek Lindner <[email protected]>

Signed-off-by Marek Lindner <[email protected]>
Signed-off-by Antonio Quartulli <[email protected]>
---
 net/batman-adv/bat_iv_ogm.c     |   11 ++++++++++-
 net/batman-adv/hard-interface.c |   15 ++++++---------
 net/batman-adv/types.h          |    2 +-
 3 files changed, 17 insertions(+), 11 deletions(-)
net/batman-adv/bat_iv_ogm.c +10 -1
index 4cc66db..2714670 100644
@@ -30,10 +30,11 @@
 #include "send.h"
 #include "bat_algo.h"
 
-static void bat_iv_ogm_iface_enable(struct hard_iface *hard_iface)
+static int bat_iv_ogm_iface_enable(struct hard_iface *hard_iface)
 {
 	struct batman_ogm_packet *batman_ogm_packet;
 	uint32_t random_seqno;
+	int res = -1;
 
 	/* randomize initial seqno to avoid collision */
 	get_random_bytes(&random_seqno, sizeof(random_seqno));

@@ -42,6 +43,9 @@ static void bat_iv_ogm_iface_enable(struct hard_iface *hard_iface)
 	hard_iface->packet_len = BATMAN_OGM_LEN;
 	hard_iface->packet_buff = kmalloc(hard_iface->packet_len, GFP_ATOMIC);
 
+	if (!hard_iface->packet_buff)
+		goto out;
+
 	batman_ogm_packet = (struct batman_ogm_packet *)hard_iface->packet_buff;
 	batman_ogm_packet->header.packet_type = BAT_OGM;
 	batman_ogm_packet->header.version = COMPAT_VERSION;

@@ -50,6 +54,11 @@ static void bat_iv_ogm_iface_enable(struct hard_iface *hard_iface)
 	batman_ogm_packet->tq = TQ_MAX_VALUE;
 	batman_ogm_packet->tt_num_changes = 0;
 	batman_ogm_packet->ttvn = 0;
+
+	res = 0;
+
+out:
+	return res;
 }
 
 static void bat_iv_ogm_iface_disable(struct hard_iface *hard_iface)
net/batman-adv/hard-interface.c +6 -9
index fd9715e..3b391fd 100644
@@ -304,22 +304,17 @@ int hardif_enable_interface(struct hard_iface *hard_iface,
 	if (!softif_is_valid(soft_iface)) {
 		pr_err("Can't create batman mesh interface %s: already exists as regular interface\n",
 		       soft_iface->name);
-		dev_put(soft_iface);
 		ret = -EINVAL;
-		goto err;
+		goto err_dev;
 	}
 
 	hard_iface->soft_iface = soft_iface;
 	bat_priv = netdev_priv(hard_iface->soft_iface);
 
-	bat_priv->bat_algo_ops->bat_iface_enable(hard_iface);
-
-	if (!hard_iface->packet_buff) {
-		bat_err(hard_iface->soft_iface,
-			"Can't add interface packet (%s): out of memory\n",
-			hard_iface->net_dev->name);
+	ret = bat_priv->bat_algo_ops->bat_iface_enable(hard_iface);
+	if (ret < 0) {
 		ret = -ENOMEM;
-		goto err;
+		goto err_dev;
 	}
 
 	hard_iface->if_num = bat_priv->num_ifaces;

@@ -363,6 +358,8 @@ int hardif_enable_interface(struct hard_iface *hard_iface,
 out:
 	return 0;
 
+err_dev:
+	dev_put(soft_iface);
 err:
 	hardif_free_ref(hard_iface);
 	return ret;
net/batman-adv/types.h +1 -1
index b034cf2..dd78023 100644
@@ -378,7 +378,7 @@ struct bat_algo_ops {
 	struct hlist_node list;
 	char *name;
 	/* init routing info when hard-interface is enabled */
-	void (*bat_iface_enable)(struct hard_iface *hard_iface);
+	int (*bat_iface_enable)(struct hard_iface *hard_iface);
 	/* de-init routing info when hard-interface is disabled */
 	void (*bat_iface_disable)(struct hard_iface *hard_iface);
 	/* init primary OGM when primary interface is selected */
Signature · 1 line
1.7.9.4
#8 lore raw

PATCH 8/13 batman-adv: refactoring API: find generalized name for bat_ogm_init_primary callback

In reply to Antonio Quartulli

From: Marek Lindner <[email protected]>

Signed-off-by Marek Lindner <[email protected]>
Signed-off-by Antonio Quartulli <[email protected]>
---
 net/batman-adv/bat_iv_ogm.c     |    4 ++--
 net/batman-adv/hard-interface.c |    2 +-
 net/batman-adv/main.c           |    2 +-
 net/batman-adv/types.h          |    4 ++--
 4 files changed, 6 insertions(+), 6 deletions(-)
net/batman-adv/bat_iv_ogm.c +2 -2
index 2714670..c0bdb90 100644
@@ -67,7 +67,7 @@ static void bat_iv_ogm_iface_disable(struct hard_iface *hard_iface)
 	hard_iface->packet_buff = NULL;
 }
 
-static void bat_iv_ogm_init_primary(struct hard_iface *hard_iface)
+static void bat_iv_ogm_primary_iface_set(struct hard_iface *hard_iface)
 {
 	struct batman_ogm_packet *batman_ogm_packet;
 

@@ -1191,7 +1191,7 @@ static struct bat_algo_ops batman_iv __read_mostly = {
 	.name = "BATMAN IV",
 	.bat_iface_enable = bat_iv_ogm_iface_enable,
 	.bat_iface_disable = bat_iv_ogm_iface_disable,
-	.bat_ogm_init_primary = bat_iv_ogm_init_primary,
+	.bat_primary_iface_set = bat_iv_ogm_primary_iface_set,
 	.bat_ogm_update_mac = bat_iv_ogm_update_mac,
 	.bat_ogm_schedule = bat_iv_ogm_schedule,
 	.bat_ogm_emit = bat_iv_ogm_emit,
net/batman-adv/hard-interface.c +1 -1
index 3b391fd..75a555b 100644
@@ -146,7 +146,7 @@ static void primary_if_select(struct bat_priv *bat_priv,
 	if (!new_hard_iface)
 		goto out;
 
-	bat_priv->bat_algo_ops->bat_ogm_init_primary(new_hard_iface);
+	bat_priv->bat_algo_ops->bat_primary_iface_set(new_hard_iface);
 	primary_if_update_addr(bat_priv, curr_hard_iface);
 
 out:
net/batman-adv/main.c +1 -1
index a47a6ce..7913272 100644
@@ -210,7 +210,7 @@ int bat_algo_register(struct bat_algo_ops *bat_algo_ops)
 	/* all algorithms must implement all ops (for now) */
 	if (!bat_algo_ops->bat_iface_enable ||
 	    !bat_algo_ops->bat_iface_disable ||
-	    !bat_algo_ops->bat_ogm_init_primary ||
+	    !bat_algo_ops->bat_primary_iface_set ||
 	    !bat_algo_ops->bat_ogm_update_mac ||
 	    !bat_algo_ops->bat_ogm_schedule ||
 	    !bat_algo_ops->bat_ogm_emit ||
net/batman-adv/types.h +2 -2
index dd78023..4d93aad 100644
@@ -381,8 +381,8 @@ struct bat_algo_ops {
 	int (*bat_iface_enable)(struct hard_iface *hard_iface);
 	/* de-init routing info when hard-interface is disabled */
 	void (*bat_iface_disable)(struct hard_iface *hard_iface);
-	/* init primary OGM when primary interface is selected */
-	void (*bat_ogm_init_primary)(struct hard_iface *hard_iface);
+	/* called when primary interface is selected / changed */
+	void (*bat_primary_iface_set)(struct hard_iface *hard_iface);
 	/* init mac addresses of the OGM belonging to this hard-interface */
 	void (*bat_ogm_update_mac)(struct hard_iface *hard_iface);
 	/* prepare a new outgoing OGM for the send queue */
Signature · 1 line
1.7.9.4
#9 lore raw

PATCH 9/13 batman-adv: rename BATMAN_OGM_LEN to BATMAN_OGM_HLEN

In reply to Antonio Quartulli

From: Marek Lindner <[email protected]>

Using BATMAN_OGM_LEN leaves one with the impression that this is the full packet size which is not the case. Therefore the variable is renamed.

Signed-off-by Marek Lindner <[email protected]>
Signed-off-by Antonio Quartulli <[email protected]>
---
 net/batman-adv/bat_iv_ogm.c |   12 ++++++------
 net/batman-adv/packet.h     |    2 +-
 net/batman-adv/routing.c    |    2 +-
 net/batman-adv/send.c       |   12 ++++++------
 4 files changed, 14 insertions(+), 14 deletions(-)
net/batman-adv/bat_iv_ogm.c +6 -6
index c0bdb90..a2af289 100644
@@ -40,7 +40,7 @@ static int bat_iv_ogm_iface_enable(struct hard_iface *hard_iface)
 	get_random_bytes(&random_seqno, sizeof(random_seqno));
 	atomic_set(&hard_iface->seqno, random_seqno);
 
-	hard_iface->packet_len = BATMAN_OGM_LEN;
+	hard_iface->packet_len = BATMAN_OGM_HLEN;
 	hard_iface->packet_buff = kmalloc(hard_iface->packet_len, GFP_ATOMIC);
 
 	if (!hard_iface->packet_buff)

@@ -112,7 +112,7 @@ static uint8_t hop_penalty(uint8_t tq, const struct bat_priv *bat_priv)
 static int bat_iv_ogm_aggr_packet(int buff_pos, int packet_len,
 				  int tt_num_changes)
 {
-	int next_buff_pos = buff_pos + BATMAN_OGM_LEN + tt_len(tt_num_changes);
+	int next_buff_pos = buff_pos + BATMAN_OGM_HLEN + tt_len(tt_num_changes);
 
 	return (next_buff_pos <= packet_len) &&
 		(next_buff_pos <= MAX_AGGREGATION_BYTES);

@@ -162,7 +162,7 @@ static void bat_iv_ogm_send_to_if(struct forw_packet *forw_packet,
 			batman_ogm_packet->ttvn, hard_iface->net_dev->name,
 			hard_iface->net_dev->dev_addr);
 
-		buff_pos += BATMAN_OGM_LEN +
+		buff_pos += BATMAN_OGM_HLEN +
 				tt_len(batman_ogm_packet->tt_num_changes);
 		packet_num++;
 		batman_ogm_packet = (struct batman_ogm_packet *)

@@ -540,7 +540,7 @@ static void bat_iv_ogm_forward(struct orig_node *orig_node,
 		batman_ogm_packet->flags &= ~DIRECTLINK;
 
 	bat_iv_ogm_queue_add(bat_priv, (unsigned char *)batman_ogm_packet,
-			     BATMAN_OGM_LEN + tt_len(tt_num_changes),
+			     BATMAN_OGM_HLEN + tt_len(tt_num_changes),
 			     if_incoming, 0, bat_iv_ogm_fwd_send_time());
 }
 

@@ -1173,12 +1173,12 @@ static void bat_iv_ogm_receive(struct hard_iface *if_incoming,
 		batman_ogm_packet->seqno = ntohl(batman_ogm_packet->seqno);
 		batman_ogm_packet->tt_crc = ntohs(batman_ogm_packet->tt_crc);
 
-		tt_buff = packet_buff + buff_pos + BATMAN_OGM_LEN;
+		tt_buff = packet_buff + buff_pos + BATMAN_OGM_HLEN;
 
 		bat_iv_ogm_process(ethhdr, batman_ogm_packet,
 				   tt_buff, if_incoming);
 
-		buff_pos += BATMAN_OGM_LEN +
+		buff_pos += BATMAN_OGM_HLEN +
 				tt_len(batman_ogm_packet->tt_num_changes);
 
 		batman_ogm_packet = (struct batman_ogm_packet *)
net/batman-adv/packet.h +1 -1
index 59800e8..59dec0a 100644
@@ -126,7 +126,7 @@ struct batman_ogm_packet {
 	uint16_t tt_crc;
 } __packed;
 
-#define BATMAN_OGM_LEN sizeof(struct batman_ogm_packet)
+#define BATMAN_OGM_HLEN sizeof(struct batman_ogm_packet)
 
 struct icmp_packet {
 	struct batman_header header;
net/batman-adv/routing.c +1 -1
index 78eddc9..ac13a6a 100644
@@ -254,7 +254,7 @@ int recv_bat_ogm_packet(struct sk_buff *skb, struct hard_iface *hard_iface)
 	struct ethhdr *ethhdr;
 
 	/* drop packet if it has not necessary minimum size */
-	if (unlikely(!pskb_may_pull(skb, BATMAN_OGM_LEN)))
+	if (unlikely(!pskb_may_pull(skb, BATMAN_OGM_HLEN)))
 		return NET_RX_DROP;
 
 	ethhdr = (struct ethhdr *)skb_mac_header(skb);
net/batman-adv/send.c +6 -6
index af7a674..b5f078c 100644
@@ -87,7 +87,7 @@ static void realloc_packet_buffer(struct hard_iface *hard_iface,
 	/* keep old buffer if kmalloc should fail */
 	if (new_buff) {
 		memcpy(new_buff, hard_iface->packet_buff,
-		       BATMAN_OGM_LEN);
+		       BATMAN_OGM_HLEN);
 
 		kfree(hard_iface->packet_buff);
 		hard_iface->packet_buff = new_buff;

@@ -101,13 +101,13 @@ static int prepare_packet_buffer(struct bat_priv *bat_priv,
 {
 	int new_len;
 
-	new_len = BATMAN_OGM_LEN +
+	new_len = BATMAN_OGM_HLEN +
 		  tt_len((uint8_t)atomic_read(&bat_priv->tt_local_changes));
 
 	/* if we have too many changes for one packet don't send any
 	 * and wait for the tt table request which will be fragmented */
 	if (new_len > hard_iface->soft_iface->mtu)
-		new_len = BATMAN_OGM_LEN;
+		new_len = BATMAN_OGM_HLEN;
 
 	realloc_packet_buffer(hard_iface, new_len);
 

@@ -117,14 +117,14 @@ static int prepare_packet_buffer(struct bat_priv *bat_priv,
 	atomic_set(&bat_priv->tt_ogm_append_cnt, TT_OGM_APPEND_MAX);
 
 	return tt_changes_fill_buffer(bat_priv,
-				      hard_iface->packet_buff + BATMAN_OGM_LEN,
-				      hard_iface->packet_len - BATMAN_OGM_LEN);
+				      hard_iface->packet_buff + BATMAN_OGM_HLEN,
+				      hard_iface->packet_len - BATMAN_OGM_HLEN);
 }
 
 static int reset_packet_buffer(struct bat_priv *bat_priv,
 				struct hard_iface *hard_iface)
 {
-	realloc_packet_buffer(hard_iface, BATMAN_OGM_LEN);
+	realloc_packet_buffer(hard_iface, BATMAN_OGM_HLEN);
 	return 0;
 }
Signature · 1 line
1.7.9.4
#10 lore raw

PATCH 10/13 batman-adv: mark existing ogm variables as batman iv

In reply to Antonio Quartulli

From: Marek Lindner <[email protected]>

The coming protocol changes also will have a part called "OGM". That makes it necessary to introduce a distinction in the code base.

Signed-off-by Marek Lindner <[email protected]>
Signed-off-by Antonio Quartulli <[email protected]>
---
 net/batman-adv/bat_iv_ogm.c     |    4 ++--
 net/batman-adv/hard-interface.c |    2 +-
 net/batman-adv/packet.h         |    4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)
net/batman-adv/bat_iv_ogm.c +2 -2
index a2af289..98f7182 100644
@@ -47,7 +47,7 @@ static int bat_iv_ogm_iface_enable(struct hard_iface *hard_iface)
 		goto out;
 
 	batman_ogm_packet = (struct batman_ogm_packet *)hard_iface->packet_buff;
-	batman_ogm_packet->header.packet_type = BAT_OGM;
+	batman_ogm_packet->header.packet_type = BAT_IV_OGM;
 	batman_ogm_packet->header.version = COMPAT_VERSION;
 	batman_ogm_packet->header.ttl = 2;
 	batman_ogm_packet->flags = NO_FLAGS;

@@ -934,7 +934,7 @@ static void bat_iv_ogm_process(const struct ethhdr *ethhdr,
 	 * packet in an aggregation.  Here we expect that the padding
 	 * is always zero (or not 0x01)
 	 */
-	if (batman_ogm_packet->header.packet_type != BAT_OGM)
+	if (batman_ogm_packet->header.packet_type != BAT_IV_OGM)
 		return;
 
 	/* could be changed by schedule_own_packet() */
net/batman-adv/hard-interface.c +1 -1
index 75a555b..e8c5da3 100644
@@ -604,7 +604,7 @@ static int batman_skb_recv(struct sk_buff *skb, struct net_device *dev,
 
 	switch (batman_ogm_packet->header.packet_type) {
 		/* batman originator packet */
-	case BAT_OGM:
+	case BAT_IV_OGM:
 		ret = recv_bat_ogm_packet(skb, hard_iface);
 		break;
net/batman-adv/packet.h +2 -2
index 59dec0a..f54969c 100644
@@ -25,7 +25,7 @@
 #define ETH_P_BATMAN  0x4305	/* unofficial/not registered Ethertype */
 
 enum bat_packettype {
-	BAT_OGM		 = 0x01,
+	BAT_IV_OGM	 = 0x01,
 	BAT_ICMP	 = 0x02,
 	BAT_UNICAST	 = 0x03,
 	BAT_BCAST	 = 0x04,

@@ -38,7 +38,7 @@ enum bat_packettype {
 /* this file is included by batctl which needs these defines */
 #define COMPAT_VERSION 14
 
-enum batman_flags {
+enum batman_iv_flags {
 	PRIMARIES_FIRST_HOP = 1 << 4,
 	VIS_SERVER	    = 1 << 5,
 	DIRECTLINK	    = 1 << 6
Signature · 1 line
1.7.9.4
#11 lore raw

PATCH 11/13 batman-adv: use ETH_HLEN instead of sizeof(struct ethhdr)

In reply to Antonio Quartulli

Instead of using sizeof(struct ethhdr) it is strongly recommended to use the kernel macro ETH_HLEN. This patch substitute each occurrence of the former expressione with the latter one.

Signed-off-by Antonio Quartulli <[email protected]>
---
 net/batman-adv/bat_iv_ogm.c            |    7 +++----
 net/batman-adv/bridge_loop_avoidance.c |   10 ++++------
 net/batman-adv/hard-interface.c        |    3 +--
 net/batman-adv/icmp_socket.c           |    4 ++--
 net/batman-adv/routing.c               |    8 ++++----
 net/batman-adv/send.c                  |    2 +-
 net/batman-adv/soft-interface.c        |    2 +-
 net/batman-adv/types.h                 |    2 +-
 net/batman-adv/vis.c                   |    8 ++++----
 9 files changed, 21 insertions(+), 25 deletions(-)
net/batman-adv/bat_iv_ogm.c +3 -4
index 98f7182..d7fa8af 100644
@@ -355,10 +355,9 @@ static void bat_iv_ogm_aggregate_new(const unsigned char *packet_buff,
 	if ((atomic_read(&bat_priv->aggregated_ogms)) &&
 	    (packet_len < MAX_AGGREGATION_BYTES))
 		forw_packet_aggr->skb = dev_alloc_skb(MAX_AGGREGATION_BYTES +
-						      sizeof(struct ethhdr));
+						      ETH_HLEN);
 	else
-		forw_packet_aggr->skb = dev_alloc_skb(packet_len +
-						      sizeof(struct ethhdr));
+		forw_packet_aggr->skb = dev_alloc_skb(packet_len + ETH_HLEN);
 
 	if (!forw_packet_aggr->skb) {
 		if (!own_packet)

@@ -366,7 +365,7 @@ static void bat_iv_ogm_aggregate_new(const unsigned char *packet_buff,
 		kfree(forw_packet_aggr);
 		goto out;
 	}
-	skb_reserve(forw_packet_aggr->skb, sizeof(struct ethhdr));
+	skb_reserve(forw_packet_aggr->skb, ETH_HLEN);
 
 	INIT_HLIST_NODE(&forw_packet_aggr->list);
net/batman-adv/bridge_loop_avoidance.c +4 -6
index c8642b5..ad394c6 100644
@@ -290,9 +290,7 @@ static void bla_send_claim(struct bat_priv *bat_priv, uint8_t *mac,
 		goto out;
 
 	ethhdr = (struct ethhdr *)skb->data;
-	hw_src = (uint8_t *)ethhdr +
-		 sizeof(struct ethhdr) +
-		 sizeof(struct arphdr);
+	hw_src = (uint8_t *)ethhdr + ETH_HLEN + sizeof(struct arphdr);
 
 	/* now we pretend that the client would have sent this ... */
 	switch (claimtype) {

@@ -340,7 +338,7 @@ static void bla_send_claim(struct bat_priv *bat_priv, uint8_t *mac,
 	skb_reset_mac_header(skb);
 	skb->protocol = eth_type_trans(skb, soft_iface);
 	bat_priv->stats.rx_packets++;
-	bat_priv->stats.rx_bytes += skb->len + sizeof(struct ethhdr);
+	bat_priv->stats.rx_bytes += skb->len + ETH_HLEN;
 	soft_iface->last_rx = jiffies;
 
 	netif_rx(skb);

@@ -844,7 +842,7 @@ static int bla_process_claim(struct bat_priv *bat_priv,
 		headlen = sizeof(*vhdr);
 	} else {
 		proto = ntohs(ethhdr->h_proto);
-		headlen = sizeof(*ethhdr);
+		headlen = ETH_HLEN;
 	}
 
 	if (proto != ETH_P_ARP)

@@ -1302,7 +1300,7 @@ int bla_is_backbone_gw(struct sk_buff *skb,
 		return 0;
 
 	/* first, find out the vid. */
-	if (!pskb_may_pull(skb, hdr_size + sizeof(struct ethhdr)))
+	if (!pskb_may_pull(skb, hdr_size + ETH_HLEN))
 		return 0;
 
 	ethhdr = (struct ethhdr *)(((uint8_t *)skb->data) + hdr_size);
net/batman-adv/hard-interface.c +1 -2
index e8c5da3..47c79d7 100644
@@ -574,8 +574,7 @@ static int batman_skb_recv(struct sk_buff *skb, struct net_device *dev,
 		goto err_free;
 
 	/* expect a valid ethernet header here. */
-	if (unlikely(skb->mac_len != sizeof(struct ethhdr) ||
-		     !skb_mac_header(skb)))
+	if (unlikely(skb->mac_len != ETH_HLEN || !skb_mac_header(skb)))
 		goto err_free;
 
 	if (!hard_iface->soft_iface)
net/batman-adv/icmp_socket.c +2 -2
index b87518e..2e98a57 100644
@@ -175,13 +175,13 @@ static ssize_t bat_socket_write(struct file *file, const char __user *buff,
 	if (len >= sizeof(struct icmp_packet_rr))
 		packet_len = sizeof(struct icmp_packet_rr);
 
-	skb = dev_alloc_skb(packet_len + sizeof(struct ethhdr));
+	skb = dev_alloc_skb(packet_len + ETH_HLEN);
 	if (!skb) {
 		len = -ENOMEM;
 		goto out;
 	}
 
-	skb_reserve(skb, sizeof(struct ethhdr));
+	skb_reserve(skb, ETH_HLEN);
 	icmp_packet = (struct icmp_packet_rr *)skb_put(skb, packet_len);
 
 	if (copy_from_user(icmp_packet, buff, packet_len)) {
net/batman-adv/routing.c +4 -4
index ac13a6a..ff56086 100644
@@ -313,7 +313,7 @@ static int recv_my_icmp_packet(struct bat_priv *bat_priv,
 		goto out;
 
 	/* create a copy of the skb, if needed, to modify it. */
-	if (skb_cow(skb, sizeof(struct ethhdr)) < 0)
+	if (skb_cow(skb, ETH_HLEN) < 0)
 		goto out;
 
 	icmp_packet = (struct icmp_packet_rr *)skb->data;

@@ -368,7 +368,7 @@ static int recv_icmp_ttl_exceeded(struct bat_priv *bat_priv,
 		goto out;
 
 	/* create a copy of the skb, if needed, to modify it. */
-	if (skb_cow(skb, sizeof(struct ethhdr)) < 0)
+	if (skb_cow(skb, ETH_HLEN) < 0)
 		goto out;
 
 	icmp_packet = (struct icmp_packet *)skb->data;

@@ -454,7 +454,7 @@ int recv_icmp_packet(struct sk_buff *skb, struct hard_iface *recv_if)
 		goto out;
 
 	/* create a copy of the skb, if needed, to modify it. */
-	if (skb_cow(skb, sizeof(struct ethhdr)) < 0)
+	if (skb_cow(skb, ETH_HLEN) < 0)
 		goto out;
 
 	icmp_packet = (struct icmp_packet_rr *)skb->data;

@@ -841,7 +841,7 @@ static int route_unicast_packet(struct sk_buff *skb, struct hard_iface *recv_if)
 		goto out;
 
 	/* create a copy of the skb, if needed, to modify it. */
-	if (skb_cow(skb, sizeof(struct ethhdr)) < 0)
+	if (skb_cow(skb, ETH_HLEN) < 0)
 		goto out;
 
 	unicast_packet = (struct unicast_packet *)skb->data;
net/batman-adv/send.c +1 -1
index b5f078c..7c66b61 100644
@@ -51,7 +51,7 @@ int send_skb_packet(struct sk_buff *skb, struct hard_iface *hard_iface,
 	}
 
 	/* push to the ethernet header. */
-	if (my_skb_head_push(skb, sizeof(*ethhdr)) < 0)
+	if (my_skb_head_push(skb, ETH_HLEN) < 0)
 		goto send_skb_err;
 
 	skb_reset_mac_header(skb);
net/batman-adv/soft-interface.c +1 -1
index efe0fba..6e2530b 100644
@@ -292,7 +292,7 @@ void interface_rx(struct net_device *soft_iface,
 /*	skb->ip_summed = CHECKSUM_UNNECESSARY;*/
 
 	bat_priv->stats.rx_packets++;
-	bat_priv->stats.rx_bytes += skb->len + sizeof(struct ethhdr);
+	bat_priv->stats.rx_bytes += skb->len + ETH_HLEN;
 
 	soft_iface->last_rx = jiffies;
net/batman-adv/types.h +1 -1
index 4d93aad..2f4848b 100644
@@ -27,7 +27,7 @@
 #include "packet.h"
 #include "bitarray.h"
 
-#define BAT_HEADER_LEN (sizeof(struct ethhdr) + \
+#define BAT_HEADER_LEN (ETH_HLEN + \
 	((sizeof(struct unicast_packet) > sizeof(struct bcast_packet) ? \
 	 sizeof(struct unicast_packet) : \
 	 sizeof(struct bcast_packet))))
net/batman-adv/vis.c +4 -4
index c4a5b8c..cec216f 100644
@@ -434,12 +434,12 @@ static struct vis_info *add_packet(struct bat_priv *bat_priv,
 		return NULL;
 
 	info->skb_packet = dev_alloc_skb(sizeof(*packet) + vis_info_len +
-					 sizeof(struct ethhdr));
+					 ETH_HLEN);
 	if (!info->skb_packet) {
 		kfree(info);
 		return NULL;
 	}
-	skb_reserve(info->skb_packet, sizeof(struct ethhdr));
+	skb_reserve(info->skb_packet, ETH_HLEN);
 	packet = (struct vis_packet *)skb_put(info->skb_packet, sizeof(*packet)
 					      + vis_info_len);
 

@@ -894,11 +894,11 @@ int vis_init(struct bat_priv *bat_priv)
 
 	bat_priv->my_vis_info->skb_packet = dev_alloc_skb(sizeof(*packet) +
 							  MAX_VIS_PACKET_SIZE +
-							 sizeof(struct ethhdr));
+							  ETH_HLEN);
 	if (!bat_priv->my_vis_info->skb_packet)
 		goto free_info;
 
-	skb_reserve(bat_priv->my_vis_info->skb_packet, sizeof(struct ethhdr));
+	skb_reserve(bat_priv->my_vis_info->skb_packet, ETH_HLEN);
 	packet = (struct vis_packet *)skb_put(bat_priv->my_vis_info->skb_packet,
 					      sizeof(*packet));
Signature · 1 line
1.7.9.4
#12 lore raw

PATCH 12/13 batman-adv: print OGM seq numbers as unsigned int

In reply to Antonio Quartulli

OGM sequence numbers are declared as uint32_t and so they have to printed using %u instead of %d in order to avoid wrong representations.

Signed-off-by Antonio Quartulli <[email protected]>
---
 net/batman-adv/bat_iv_ogm.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
net/batman-adv/bat_iv_ogm.c +4 -4
index d7fa8af..1014210 100644
@@ -152,7 +152,7 @@ static void bat_iv_ogm_send_to_if(struct forw_packet *forw_packet,
 							    "Sending own" :
 							    "Forwarding"));
 		bat_dbg(DBG_BATMAN, bat_priv,
-			"%s %spacket (originator %pM, seqno %d, TQ %d, TTL %d, IDF %s, ttvn %d) on interface %s [%pM]\n",
+			"%s %spacket (originator %pM, seqno %u, TQ %d, TTL %d, IDF %s, ttvn %d) on interface %s [%pM]\n",
 			fwd_str, (packet_num > 0 ? "aggregated " : ""),
 			batman_ogm_packet->orig,
 			ntohl(batman_ogm_packet->seqno),

@@ -211,7 +211,7 @@ static void bat_iv_ogm_emit(struct forw_packet *forw_packet)
 
 		/* FIXME: what about aggregated packets ? */
 		bat_dbg(DBG_BATMAN, bat_priv,
-			"%s packet (originator %pM, seqno %d, TTL %d) on interface %s [%pM]\n",
+			"%s packet (originator %pM, seqno %u, TTL %d) on interface %s [%pM]\n",
 			(forw_packet->own ? "Sending own" : "Forwarding"),
 			batman_ogm_packet->orig,
 			ntohl(batman_ogm_packet->seqno),

@@ -892,7 +892,7 @@ static int bat_iv_ogm_update_seqnos(const struct ethhdr *ethhdr,
 
 	if (need_update) {
 		bat_dbg(DBG_BATMAN, bat_priv,
-			"updating last_seqno: old %d, new %d\n",
+			"updating last_seqno: old %u, new %u\n",
 			orig_node->last_real_seqno, batman_ogm_packet->seqno);
 		orig_node->last_real_seqno = batman_ogm_packet->seqno;
 	}

@@ -945,7 +945,7 @@ static void bat_iv_ogm_process(const struct ethhdr *ethhdr,
 					   batman_ogm_packet->orig) ? 1 : 0);
 
 	bat_dbg(DBG_BATMAN, bat_priv,
-		"Received BATMAN packet via NB: %pM, IF: %s [%pM] (from OG: %pM, via prev OG: %pM, seqno %d, ttvn %u, crc %u, changes %u, td %d, TTL %d, V %d, IDF %d)\n",
+		"Received BATMAN packet via NB: %pM, IF: %s [%pM] (from OG: %pM, via prev OG: %pM, seqno %u, ttvn %u, crc %u, changes %u, td %d, TTL %d, V %d, IDF %d)\n",
 		ethhdr->h_source, if_incoming->net_dev->name,
 		if_incoming->net_dev->dev_addr, batman_ogm_packet->orig,
 		batman_ogm_packet->prev_sender, batman_ogm_packet->seqno,
Signature · 1 line
1.7.9.4
#13 lore raw

PATCH 13/13 batman-adv: skip the window protection test when the originator has no neighbours

In reply to Antonio Quartulli

When we receive an OGM from from a node for the first time, the last_real_seqno field of the orig_node structure has not been initialised yet. The value of this field is used to compute the current ogm-seqno window and therefore the protection mechanism will probably drop the packet due to an out-of-window error. To avoid this situation this patch adds a check to skip the window protection mechanism if no neighbour nodes have already been added. When the first neighbour node is added, the last_real_seqno field is initialised too.

Reported-by Marek Lindner <[email protected]>
Signed-off-by Antonio Quartulli <[email protected]>
---
 net/batman-adv/bat_iv_ogm.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
net/batman-adv/bat_iv_ogm.c +2 -1
index 1014210..8b2db2e 100644
@@ -861,7 +861,8 @@ static int bat_iv_ogm_update_seqnos(const struct ethhdr *ethhdr,
 	seq_diff = batman_ogm_packet->seqno - orig_node->last_real_seqno;
 
 	/* signalize caller that the packet is to be dropped. */
-	if (window_protected(bat_priv, seq_diff,
+	if (!hlist_empty(&orig_node->neigh_list) &&
+	    window_protected(bat_priv, seq_diff,
 			     &orig_node->batman_seqno_reset))
 		goto out;
Signature · 1 line
1.7.9.4
maintainer
#14 lore raw
In reply to Antonio Quartulli

From: Antonio Quartulli <[email protected]>
Date: Wed, 18 Apr 2012 11:59:57 +0200

Quoted 7 lines

this is the fixed version of my previous pull request (issued on 2012-04-17). In this patchset the issues you reported have been fixed; moreover I'm also including two new patches (1/13 and 2/13) which are respectively:

1/13) a fix for Al Viro's report about the missing htons() 2/13) a fix for a wrongly duplicated line in a comment

Quoted 7 lines

The following changes since commit ecffe75f934b4e3c5301fe5db278068e0efb0d6b:

hippi: fix printk format in rrunner.c (2012-04-16 23:48:38 -0400)

are available in the git repository at:

git://git.open-mesh.org/linux-merge.git tags/batman-adv-for-davem

This looks a lot better, pulled, thanks!

A message that is not in the archive (sent off-list or not yet received): 1334743210-12338-1-git-send-email-ordex-GaUfNO9RBHfsrOwW+9ziJQ@public.gmane.org

#16 lore raw
Quoted 7 lines · Antonio Quartulli

Hello David,

this is the fixed version of my previous pull request (issued on 2012-04-17). In this patchset the issues you reported have been fixed; moreover I'm also including two new patches (1/13 and 2/13) which are respectively:

1/13) a fix for Al Viro's report about the missing htons()

Speaking of endianness stuff: here's a series of 4 patches on top of your merge.git/master, hopefully getting all that stuff endian-clean, at least from the sparse POV. And AFAICS all on-the-wire stuff is correctly annotated...

A message that is not in the archive (sent off-list or not yet received): 20120418180837.GB6589-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org

#18 lore raw

PATCH 1/4 batman: don't bother flipping ->tt_data

just keep it net-endian all along

Signed-off-by Al Viro <[email protected]>
---
 net/batman-adv/packet.h            |    2 +-
 net/batman-adv/routing.c           |    6 +-----
 net/batman-adv/translation-table.c |    8 ++++----
 3 files changed, 6 insertions(+), 10 deletions(-)
net/batman-adv/packet.h +1 -1
index 307dbb3..9157f7c 100644
@@ -220,7 +220,7 @@ struct tt_query_packet {
 	 * if TT_REQUEST: crc associated with the
 	 *		  ttvn
 	 * if TT_RESPONSE: table_size */
-	uint16_t tt_data;
+	__be16   tt_data;
 } __packed;
 
 struct roam_adv_packet {
net/batman-adv/routing.c +1 -5
index 2181a91..4310cfe 100644
@@ -596,8 +596,6 @@ int recv_tt_query(struct sk_buff *skb, struct hard_iface *recv_if)
 
 	tt_query = (struct tt_query_packet *)skb->data;
 
-	tt_query->tt_data = ntohs(tt_query->tt_data);
-
 	switch (tt_query->flags & TT_QUERY_TYPE_MASK) {
 	case TT_REQUEST:
 		/* If we cannot provide an answer the tt_request is

@@ -607,7 +605,6 @@ int recv_tt_query(struct sk_buff *skb, struct hard_iface *recv_if)
 				"Routing TT_REQUEST to %pM [%c]\n",
 				tt_query->dst,
 				(tt_query->flags & TT_FULL_TABLE ? 'F' : '.'));
-			tt_query->tt_data = htons(tt_query->tt_data);
 			return route_unicast_packet(skb, recv_if);
 		}
 		break;

@@ -618,7 +615,7 @@ int recv_tt_query(struct sk_buff *skb, struct hard_iface *recv_if)
 			if (skb_linearize(skb) < 0)
 				goto out;
 
-			tt_len = tt_query->tt_data * sizeof(struct tt_change);
+			tt_len = ntohs(tt_query->tt_data) * sizeof(struct tt_change);
 
 			/* Ensure we have all the claimed data */
 			if (unlikely(skb_headlen(skb) <

@@ -631,7 +628,6 @@ int recv_tt_query(struct sk_buff *skb, struct hard_iface *recv_if)
 				"Routing TT_RESPONSE to %pM [%c]\n",
 				tt_query->dst,
 				(tt_query->flags & TT_FULL_TABLE ? 'F' : '.'));
-			tt_query->tt_data = htons(tt_query->tt_data);
 			return route_unicast_packet(skb, recv_if);
 		}
 		break;
net/batman-adv/translation-table.c +4 -4
index 88e4c8e..a9c2b59 100644
@@ -1416,7 +1416,7 @@ static bool send_other_tt_response(struct bat_priv *bat_priv,
 
 	/* I don't have the requested data */
 	if (orig_ttvn != req_ttvn ||
-	    tt_request->tt_data != req_dst_orig_node->tt_crc)
+	    tt_request->tt_data != htons(req_dst_orig_node->tt_crc))
 		goto out;
 
 	/* If the full table has been explicitly requested */

@@ -1672,7 +1672,7 @@ static void tt_fill_gtable(struct bat_priv *bat_priv,
 
 	_tt_update_changes(bat_priv, orig_node,
 			   (struct tt_change *)(tt_response + 1),
-			   tt_response->tt_data, tt_response->ttvn);
+			   ntohs(tt_response->tt_data), tt_response->ttvn);
 
 	spin_lock_bh(&orig_node->tt_buff_lock);
 	kfree(orig_node->tt_buff);

@@ -1727,7 +1727,7 @@ void handle_tt_response(struct bat_priv *bat_priv,
 
 	bat_dbg(DBG_TT, bat_priv,
 		"Received TT_RESPONSE from %pM for ttvn %d t_size: %d [%c]\n",
-		tt_response->src, tt_response->ttvn, tt_response->tt_data,
+		tt_response->src, tt_response->ttvn, ntohs(tt_response->tt_data),
 		(tt_response->flags & TT_FULL_TABLE ? 'F' : '.'));
 
 	/* we should have never asked a backbone gw */

@@ -1741,7 +1741,7 @@ void handle_tt_response(struct bat_priv *bat_priv,
 	if (tt_response->flags & TT_FULL_TABLE)
 		tt_fill_gtable(bat_priv, tt_response);
 	else
-		tt_update_changes(bat_priv, orig_node, tt_response->tt_data,
+		tt_update_changes(bat_priv, orig_node, ntohs(tt_response->tt_data),
 				  tt_response->ttvn,
 				  (struct tt_change *)(tt_response + 1));
Signature · 1 line
1.7.2.5
#19 lore raw

batman: keep batman_ogm_packet ->seqno net-endian all along

In reply to a message not in the archive

Ditto for its ->seqno...

Signed-off-by Al Viro <[email protected]>
---
 net/batman-adv/bat_iv_ogm.c |   25 ++++++++++---------------
 net/batman-adv/packet.h     |    2 +-
 2 files changed, 11 insertions(+), 16 deletions(-)
net/batman-adv/bat_iv_ogm.c +10 -15
index 5aa8861..cc6d4c3 100644
@@ -34,11 +34,11 @@ static struct neigh_node *bat_iv_ogm_neigh_new(struct hard_iface *hard_iface,
 					       const uint8_t *neigh_addr,
 					       struct orig_node *orig_node,
 					       struct orig_node *orig_neigh,
-					       uint32_t seqno)
+					       __be32 seqno)
 {
 	struct neigh_node *neigh_node;
 
-	neigh_node = neigh_node_new(hard_iface, neigh_addr, seqno);
+	neigh_node = neigh_node_new(hard_iface, neigh_addr, ntohl(seqno));
 	if (!neigh_node)
 		goto out;
 

@@ -543,8 +543,6 @@ static void bat_iv_ogm_forward(struct orig_node *orig_node,
 		"Forwarding packet: tq: %i, ttl: %i\n",
 		batman_ogm_packet->tq, batman_ogm_packet->header.ttl);
 
-	batman_ogm_packet->seqno = htonl(batman_ogm_packet->seqno);
-
 	/* switch of primaries first hop flag when forwarding */
 	batman_ogm_packet->flags &= ~PRIMARIES_FIRST_HOP;
 	if (is_single_hop_neigh)

@@ -868,13 +866,14 @@ static int bat_iv_ogm_update_seqnos(const struct ethhdr *ethhdr,
 	int32_t seq_diff;
 	int need_update = 0;
 	int set_mark, ret = -1;
+	uint32_t seqno = ntohl(batman_ogm_packet->seqno);
 
 	orig_node = get_orig_node(bat_priv, batman_ogm_packet->orig);
 	if (!orig_node)
 		return 0;
 
 	spin_lock_bh(&orig_node->ogm_cnt_lock);
-	seq_diff = batman_ogm_packet->seqno - orig_node->last_real_seqno;
+	seq_diff = seqno - orig_node->last_real_seqno;
 
 	/* signalize caller that the packet is to be dropped. */
 	if (!hlist_empty(&orig_node->neigh_list) &&

@@ -888,7 +887,7 @@ static int bat_iv_ogm_update_seqnos(const struct ethhdr *ethhdr,
 
 		is_duplicate |= bat_test_bit(tmp_neigh_node->real_bits,
 					     orig_node->last_real_seqno,
-					     batman_ogm_packet->seqno);
+					     seqno);
 
 		if (compare_eth(tmp_neigh_node->addr, ethhdr->h_source) &&
 		    (tmp_neigh_node->if_incoming == if_incoming))

@@ -910,8 +909,8 @@ static int bat_iv_ogm_update_seqnos(const struct ethhdr *ethhdr,
 	if (need_update) {
 		bat_dbg(DBG_BATMAN, bat_priv,
 			"updating last_seqno: old %u, new %u\n",
-			orig_node->last_real_seqno, batman_ogm_packet->seqno);
-		orig_node->last_real_seqno = batman_ogm_packet->seqno;
+			orig_node->last_real_seqno, seqno);
+		orig_node->last_real_seqno = seqno;
 	}
 
 	ret = is_duplicate;

@@ -967,7 +966,7 @@ static void bat_iv_ogm_process(const struct ethhdr *ethhdr,
 		"Received BATMAN packet via NB: %pM, IF: %s [%pM] (from OG: %pM, via prev OG: %pM, seqno %u, ttvn %u, crc %u, changes %u, td %d, TTL %d, V %d, IDF %d)\n",
 		ethhdr->h_source, if_incoming->net_dev->name,
 		if_incoming->net_dev->dev_addr, batman_ogm_packet->orig,
-		batman_ogm_packet->prev_sender, batman_ogm_packet->seqno,
+		batman_ogm_packet->prev_sender, ntohl(batman_ogm_packet->seqno),
 		batman_ogm_packet->ttvn, ntohs(batman_ogm_packet->tt_crc),
 		batman_ogm_packet->tt_num_changes, batman_ogm_packet->tq,
 		batman_ogm_packet->header.ttl,

@@ -1039,7 +1038,7 @@ static void bat_iv_ogm_process(const struct ethhdr *ethhdr,
 			word = &(orig_neigh_node->bcast_own[offset]);
 			bat_set_bit(word,
 				    if_incoming_seqno -
-						batman_ogm_packet->seqno - 2);
+					ntohl(batman_ogm_packet->seqno) - 2);
 			orig_neigh_node->bcast_own_sum[if_incoming->if_num] =
 				bitmap_weight(word, TQ_LOCAL_WINDOW_SIZE);
 			spin_unlock_bh(&orig_neigh_node->ogm_cnt_lock);

@@ -1132,7 +1131,7 @@ static void bat_iv_ogm_process(const struct ethhdr *ethhdr,
 	 * seqno and similar ttl as the non-duplicate */
 	if (is_bidirectional &&
 	    (!is_duplicate ||
-	     ((orig_node->last_real_seqno == batman_ogm_packet->seqno) &&
+	     ((orig_node->last_real_seqno == ntohl(batman_ogm_packet->seqno)) &&
 	      (orig_node->last_ttl - 3 <= batman_ogm_packet->header.ttl))))
 		bat_iv_ogm_orig_update(bat_priv, orig_node, ethhdr,
 				       batman_ogm_packet, if_incoming,

@@ -1210,10 +1209,6 @@ static int bat_iv_ogm_receive(struct sk_buff *skb,
 
 	/* unpack the aggregated packets and process them one by one */
 	do {
-		/* network to host order for our 32bit seqno and the
-		   orig_interval */
-		batman_ogm_packet->seqno = ntohl(batman_ogm_packet->seqno);
-
 		tt_buff = packet_buff + buff_pos + BATMAN_OGM_HLEN;
 
 		bat_iv_ogm_process(ethhdr, batman_ogm_packet,
net/batman-adv/packet.h +1 -1
index 61a7fe0..ad39938 100644
@@ -125,7 +125,7 @@ struct batman_header {
 struct batman_ogm_packet {
 	struct batman_header header;
 	uint8_t  flags;    /* 0x40: DIRECTLINK flag, 0x20 VIS_SERVER flag... */
-	uint32_t seqno;
+	__be32   seqno;
 	uint8_t  orig[ETH_ALEN];
 	uint8_t  prev_sender[ETH_ALEN];
 	uint8_t  gw_flags;  /* flags related to gateway class */
Signature · 1 line
1.7.2.5
#20 lore raw

Re: batman: keep batman_ogm_packet ->seqno net-endian all along

grr... this one would be 3/4 and the last ("trivial endianness annotations") - 4/4. My apologies...

#21 lore raw

batman: trivial endianness annotations

In reply to a message not in the archive

Missing endianness annotations - on-the-wire data and ip addresses. BTW, casting memcpy() arguments to (uint8_t *) is cargo-cult programming - they are void *, which is precisely "take any pointer to object"...

Signed-off-by Al Viro <[email protected]>
---
 net/batman-adv/bridge_loop_avoidance.c |    8 ++++----
 net/batman-adv/distributed-arp-table.c |   16 ++++++++--------
 net/batman-adv/distributed-arp-table.h |    4 ++--
 net/batman-adv/packet.h                |   12 ++++++------
 4 files changed, 20 insertions(+), 20 deletions(-)
net/batman-adv/bridge_loop_avoidance.c +4 -4
index 8bf9751..d51288b 100644
@@ -258,7 +258,7 @@ static void bla_send_claim(struct bat_priv *bat_priv, uint8_t *mac,
 	struct net_device *soft_iface;
 	uint8_t *hw_src;
 	struct bla_claim_dst local_claim_dest;
-	uint32_t zeroip = 0;
+	__be32 zeroip = 0;
 
 	primary_if = primary_if_get_selected(bat_priv);
 	if (!primary_if)

@@ -506,11 +506,11 @@ static void bla_send_announce(struct bat_priv *bat_priv,
 			      struct backbone_gw *backbone_gw)
 {
 	uint8_t mac[ETH_ALEN];
-	uint16_t crc;
+	__be16 crc;
 
 	memcpy(mac, announce_mac, 4);
 	crc = htons(backbone_gw->crc);
-	memcpy(&mac[4], (uint8_t *)&crc, 2);
+	memcpy(&mac[4], &crc, 2);
 
 	bla_send_claim(bat_priv, mac, backbone_gw->vid, CLAIM_TYPE_ANNOUNCE);
 

@@ -627,7 +627,7 @@ static int handle_announce(struct bat_priv *bat_priv,
 
 	/* handle as ANNOUNCE frame */
 	backbone_gw->lasttime = jiffies;
-	crc = ntohs(*((uint16_t *)(&an_addr[4])));
+	crc = ntohs(*((__be16 *)(&an_addr[4])));
 
 	bat_dbg(DBG_BLA, bat_priv,
 		"handle_announce(): ANNOUNCE vid %d (sent by %pM)... CRC = %04x\n",
net/batman-adv/distributed-arp-table.c +8 -8
index b43bece..d87f914 100644
@@ -192,7 +192,7 @@ static void choose_next_candidate(struct bat_priv *bat_priv,
  *
  * return an array of size DHT_CANDIDATES_NUM */
 static struct dht_candidate *dht_select_candidates(struct bat_priv *bat_priv,
-						   uint32_t ip_dst)
+						   __be32 ip_dst)
 {
 	int select;
 	dat_addr_t last_max = DAT_ADDR_MAX, ip_key;

@@ -224,7 +224,7 @@ static struct dht_candidate *dht_select_candidates(struct bat_priv *bat_priv,
  * If the packet is successfully sent to at least one candidate, then this
  * function returns true */
 static bool dht_send_data(struct bat_priv *bat_priv, struct sk_buff *skb,
-			  uint32_t ip, int packet_subtype)
+			  __be32 ip, int packet_subtype)
 {
 	int i;
 	bool ret = false;

@@ -270,7 +270,7 @@ out:
 /* Update the neighbour entry corresponding to the IP passed as parameter with
  * the hw address hw. If the neighbour entry doesn't exists, then it will be
  * created */
-static void arp_neigh_update(struct bat_priv *bat_priv, uint32_t ip,
+static void arp_neigh_update(struct bat_priv *bat_priv, __be32 ip,
 			     uint8_t *hw)
 {
 	struct neighbour *n = NULL;

@@ -299,7 +299,7 @@ static uint16_t arp_get_type(struct bat_priv *bat_priv, struct sk_buff *skb,
 {
 	struct arphdr *arphdr;
 	struct ethhdr *ethhdr;
-	uint32_t ip_src, ip_dst;
+	__be32 ip_src, ip_dst;
 	uint16_t type = 0;
 
 	/* pull the ethernet header */

@@ -352,7 +352,7 @@ bool dat_snoop_outgoing_arp_request(struct bat_priv *bat_priv,
 				    struct sk_buff *skb)
 {
 	uint16_t type = 0;
-	uint32_t ip_dst, ip_src;
+	__be32 ip_dst, ip_src;
 	uint8_t *hw_src;
 	bool ret = false;
 	struct neighbour *n = NULL;

@@ -414,7 +414,7 @@ bool dat_snoop_incoming_arp_request(struct bat_priv *bat_priv,
 				    struct sk_buff *skb, int hdr_size)
 {
 	uint16_t type;
-	uint32_t ip_src, ip_dst;
+	__be32 ip_src, ip_dst;
 	uint8_t *hw_src;
 	struct hard_iface *primary_if = NULL;
 	struct sk_buff *skb_new;

@@ -470,7 +470,7 @@ bool dat_snoop_outgoing_arp_reply(struct bat_priv *bat_priv,
 				  struct sk_buff *skb)
 {
 	uint16_t type;
-	uint32_t ip_src, ip_dst;
+	__be32 ip_src, ip_dst;
 	uint8_t *hw_src, *hw_dst;
 	bool ret = false;
 

@@ -503,7 +503,7 @@ bool dat_snoop_incoming_arp_reply(struct bat_priv *bat_priv,
 				  struct sk_buff *skb, int hdr_size)
 {
 	uint16_t type;
-	uint32_t ip_src, ip_dst;
+	__be32 ip_src, ip_dst;
 	uint8_t *hw_src, *hw_dst;
 	bool ret = false;
net/batman-adv/distributed-arp-table.h +2 -2
index fcf6c15..01d5b4f 100644
@@ -33,10 +33,10 @@
 
 #define ARP_HW_SRC(skb, hdr_size) ((uint8_t *)(skb->data + hdr_size) + \
 				   ETH_HLEN + sizeof(struct arphdr))
-#define ARP_IP_SRC(skb, hdr_size) (*(uint32_t *)(ARP_HW_SRC(skb, hdr_size) + \
+#define ARP_IP_SRC(skb, hdr_size) (*(__be32 *)(ARP_HW_SRC(skb, hdr_size) + \
 				   ETH_ALEN))
 #define ARP_HW_DST(skb, hdr_size) (ARP_HW_SRC(skb, hdr_size) + ETH_ALEN + 4)
-#define ARP_IP_DST(skb, hdr_size) (*(uint32_t *)(ARP_HW_SRC(skb, hdr_size) + \
+#define ARP_IP_DST(skb, hdr_size) (*(__be32 *)(ARP_HW_SRC(skb, hdr_size) + \
 				   ETH_ALEN * 2 + 4))
 
 bool dat_snoop_outgoing_arp_request(struct bat_priv *bat_priv,
net/batman-adv/packet.h +6 -6
index ad39938..eb4fdf6 100644
@@ -113,7 +113,7 @@ enum bla_claimframe {
 struct bla_claim_dst {
 	uint8_t magic[3];	/* FF:43:05 */
 	uint8_t type;		/* bla_claimframe */
-	uint16_t group;		/* group id */
+	__be16 group;		/* group id */
 } __packed;
 
 struct batman_header {

@@ -142,7 +142,7 @@ struct icmp_packet {
 	uint8_t  msg_type; /* see ICMP message types above */
 	uint8_t  dst[ETH_ALEN];
 	uint8_t  orig[ETH_ALEN];
-	uint16_t seqno;
+	__be16   seqno;
 	uint8_t  uid;
 	uint8_t  reserved;
 } __packed;

@@ -156,7 +156,7 @@ struct icmp_packet_rr {
 	uint8_t  msg_type; /* see ICMP message types above */
 	uint8_t  dst[ETH_ALEN];
 	uint8_t  orig[ETH_ALEN];
-	uint16_t seqno;
+	__be16   seqno;
 	uint8_t  uid;
 	uint8_t  rr_cur;
 	uint8_t  rr[BAT_RR_LEN][ETH_ALEN];

@@ -181,20 +181,20 @@ struct unicast_frag_packet {
 	uint8_t  flags;
 	uint8_t  align;
 	uint8_t  orig[ETH_ALEN];
-	uint16_t seqno;
+	__be16   seqno;
 } __packed;
 
 struct bcast_packet {
 	struct batman_header header;
 	uint8_t  reserved;
-	uint32_t seqno;
+	__be32   seqno;
 	uint8_t  orig[ETH_ALEN];
 } __packed;
 
 struct vis_packet {
 	struct batman_header header;
 	uint8_t  vis_type;	 /* which type of vis-participant sent this? */
-	uint32_t seqno;		 /* sequence number */
+	__be32   seqno;		 /* sequence number */
 	uint8_t  entries;	 /* number of entries behind this struct */
 	uint8_t  reserved;
 	uint8_t  vis_orig[ETH_ALEN];	/* originator reporting its neighbors */
Signature · 1 line
1.7.2.5
#22 lore raw

PATCH 2/4 batman: don't bother flipping ->tt_crc

In reply to Al Viro

Again, keep ->tt_crc in batman_ogv_packet net-endian

Signed-off-by Al Viro <[email protected]>
---
 net/batman-adv/bat_iv_ogm.c |    6 ++----
 net/batman-adv/packet.h     |    2 +-
 2 files changed, 3 insertions(+), 5 deletions(-)
net/batman-adv/bat_iv_ogm.c +2 -4
index ab2085c..5aa8861 100644
@@ -544,7 +544,6 @@ static void bat_iv_ogm_forward(struct orig_node *orig_node,
 		batman_ogm_packet->tq, batman_ogm_packet->header.ttl);
 
 	batman_ogm_packet->seqno = htonl(batman_ogm_packet->seqno);
-	batman_ogm_packet->tt_crc = htons(batman_ogm_packet->tt_crc);
 
 	/* switch of primaries first hop flag when forwarding */
 	batman_ogm_packet->flags &= ~PRIMARIES_FIRST_HOP;

@@ -722,7 +721,7 @@ update_tt:
 		tt_update_orig(bat_priv, orig_node, tt_buff,
 			       batman_ogm_packet->tt_num_changes,
 			       batman_ogm_packet->ttvn,
-			       batman_ogm_packet->tt_crc);
+			       ntohs(batman_ogm_packet->tt_crc));
 
 	if (orig_node->gw_flags != batman_ogm_packet->gw_flags)
 		gw_node_update(bat_priv, orig_node,

@@ -969,7 +968,7 @@ static void bat_iv_ogm_process(const struct ethhdr *ethhdr,
 		ethhdr->h_source, if_incoming->net_dev->name,
 		if_incoming->net_dev->dev_addr, batman_ogm_packet->orig,
 		batman_ogm_packet->prev_sender, batman_ogm_packet->seqno,
-		batman_ogm_packet->ttvn, batman_ogm_packet->tt_crc,
+		batman_ogm_packet->ttvn, ntohs(batman_ogm_packet->tt_crc),
 		batman_ogm_packet->tt_num_changes, batman_ogm_packet->tq,
 		batman_ogm_packet->header.ttl,
 		batman_ogm_packet->header.version, has_directlink_flag);

@@ -1214,7 +1213,6 @@ static int bat_iv_ogm_receive(struct sk_buff *skb,
 		/* network to host order for our 32bit seqno and the
 		   orig_interval */
 		batman_ogm_packet->seqno = ntohl(batman_ogm_packet->seqno);
-		batman_ogm_packet->tt_crc = ntohs(batman_ogm_packet->tt_crc);
 
 		tt_buff = packet_buff + buff_pos + BATMAN_OGM_HLEN;
net/batman-adv/packet.h +1 -1
index 9157f7c..61a7fe0 100644
@@ -132,7 +132,7 @@ struct batman_ogm_packet {
 	uint8_t  tq;
 	uint8_t  tt_num_changes;
 	uint8_t  ttvn; /* translation table version number */
-	uint16_t tt_crc;
+	__be16   tt_crc;
 } __packed;
 
 #define BATMAN_OGM_HLEN sizeof(struct batman_ogm_packet)
Signature · 1 line
1.7.2.5

A message that is not in the archive (sent off-list or not yet received): 20120418181058.GD6589-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org

author
#24 lore raw

Re: [PATCH 2/4] batman: don't bother flipping ->tt_crc

net/batman-adv/bat_iv_ogm.c update_tt: · line 724

Quoted 6 lines · Al Viro
net/batman-adv/bat_iv_ogm.c +1 -1
@@ -722,7 +721,7 @@ update_tt:
 		tt_update_orig(bat_priv, orig_node, tt_buff,
 			       batman_ogm_packet->tt_num_changes,
 			       batman_ogm_packet->ttvn,
-			       batman_ogm_packet->tt_crc);
+			       ntohs(batman_ogm_packet->tt_crc));

If we don't need to flip tt_crc, wy are you introducing ntohs here? Am I overlooking something?

net/batman-adv/bat_iv_ogm.c static void bat_iv_ogm_process(const struct ethhdr *ethhdr, · line 972

Quoted 7 lines
net/batman-adv/bat_iv_ogm.c +1 -1
@@ -969,7 +968,7 @@ static void bat_iv_ogm_process(const struct ethhdr *ethhdr,
 		ethhdr->h_source, if_incoming->net_dev->name,
 		if_incoming->net_dev->dev_addr, batman_ogm_packet->orig,
 		batman_ogm_packet->prev_sender, batman_ogm_packet->seqno,
-		batman_ogm_packet->ttvn, batman_ogm_packet->tt_crc,
+		batman_ogm_packet->ttvn, ntohs(batman_ogm_packet->tt_crc),
 		batman_ogm_packet->tt_num_changes, batman_ogm_packet->tq,

The same here..I am not understanding..

Thank you for your patches!

Cheers,

Signature · 4 lines
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

A message that is not in the archive (sent off-list or not yet received): 20120419054113.GA8658-E/[email protected]

author
#26 lore raw

Re: [PATCH 2/4] batman: don't bother flipping ->tt_crc

Quoted 10 lines · Antonio Quartulli

net/batman-adv/bat_iv_ogm.c update_tt: · line 724

Quoted 6 lines · Al Viro
net/batman-adv/bat_iv_ogm.c +1 -1
@@ -722,7 +721,7 @@ update_tt:
 		tt_update_orig(bat_priv, orig_node, tt_buff,
 			       batman_ogm_packet->tt_num_changes,
 			       batman_ogm_packet->ttvn,
-			       batman_ogm_packet->tt_crc);
+			       ntohs(batman_ogm_packet->tt_crc));

If we don't need to flip tt_crc, wy are you introducing ntohs here? Am I overlooking something?

Ok. I was missing the correct meaning of __be16. Now everything is clear.

Cheers,

Signature · 4 lines
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

A message that is not in the archive (sent off-list or not yet received): [email protected]

A message that is not in the archive (sent off-list or not yet received): [email protected]

#29 lore raw

Hello,

Quoted 12 lines · Al Viro
Quoted 5 lines · Antonio Quartulli

Hello Al,

and thank you very much for your patches.
Before committing them, do you mind if we reword the subject by substituting "batman" with "batman-adv"?

No problem...

Quoted 1 line

Moreover, patch 3/4, fixes the memcpy() casting too other than the endianess

Quoted 7 lines

4/4, actually.

Quoted 2 lines

stuff. We would prefer to have two different patches for fixing those two issues. What about splitting it and resending them?

Can do; I've just grepped for memcpy() in there, to see if there are other places like that.

Thanks!

Quoted 12 lines

Haven't found any, but
* you do an awful lot of GFP_ATOMIC allocations and those can and
do fail from time to time. What's worse, you ignore some of those failures - e.g. failing allocation in orig_hash_{add,del}_if() will be ignored by the caller. I haven't looked into that code enough to tell if it could be exploited, but I really don't like the look of it...
* orig_node_add_if() leaves junk in added array elements. You do
kmalloc() followed by memcpy(), but leave the last element uninitialized. May be safe if you assign it soon enough, but I'd suggest checking that.
* orig_node_del_if() looks odd - it removes element #hard_iface->if_num
and shifts all subsequent ones down; then it renumbers interfaces to match that. So far, so good, and there's even a plausible comment about locking:

Quoted 3 lines
        /* renumber remaining batman interfaces _inside_ of orig_hash_lock */

except that no such lock exists since commit d007260. What protects us from the obvious race in there?

Thank you very much for your analysis. I really appreciated it! I'm CCing our mailing list so that other people can comment on it.

Cheers,

Signature · 4 lines
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara
#30 lore raw

Hi,

Quoted 7 lines

Haven't found any, but

* you do an awful lot of GFP_ATOMIC allocations and those can and
do fail from time to time. What's worse, you ignore some of those failures - e.g. failing allocation in orig_hash_{add,del}_if() will be ignored by the caller. I haven't looked into that code enough to tell if it could be exploited, but I really don't like the look of it...

other GFP_* allocations can't fail ?
This whole resizing isn't escpecially beautiful and asks for some love.

Quoted 3 lines

* orig_node_add_if() leaves junk in added array elements. You do
kmalloc() followed by memcpy(), but leave the last element uninitialized. May be safe if you assign it soon enough, but I'd suggest checking that.

Replacing kmalloc() with kzalloc() should do, right ?

Quoted 7 lines

* orig_node_del_if() looks odd - it removes element #hard_iface->if_num
and shifts all subsequent ones down; then it renumbers interfaces to match that. So far, so good, and there's even a plausible comment about locking:

   /* renumber remaining batman interfaces _inside_ of orig_hash_lock */

except that no such lock exists since commit d007260. What protects us from the obvious race in there?

Thanks for catching this. I agree that this is not properly protected. All functions accessing orig_node->bcast_own(_sum) use orig_node->ogm_cnt_lock to lock each other out. Obviously we would need a global lock for the interface renumbering which will be as ugly as the current array resizing is. You don't happen to have a good example of a resizable array at hand ?

Cheers,
Marek

#31 lore raw
Quoted 5 lines · Marek Lindner

Hi,

Quoted 2 lines

Haven't found any, but

Quoted 8 lines
Quoted 5 lines

* you do an awful lot of GFP_ATOMIC allocations and those can and
do fail from time to time. What's worse, you ignore some of those failures - e.g. failing allocation in orig_hash_{add,del}_if() will be ignored by the caller. I haven't looked into that code enough to tell if it could be exploited, but I really don't like the look of it...

other GFP_* allocations can't fail ?
This whole resizing isn't escpecially beautiful and asks for some love.

Other GFP_* allocations fail only when system is in a really lousy state - killing processes, etc. GFP_ATOMIC can fail in much milder conditions; note that they can't e.g. swap a page out or write a dirty page out and free it, etc. _Any_ allocation failures need to be dealt with, of course, but with GFP_ATOMIC ones failures are just a fact of life - it's not even an emergency situation.

Quoted 5 lines
Quoted 3 lines

* orig_node_add_if() leaves junk in added array elements. You do
kmalloc() followed by memcpy(), but leave the last element uninitialized. May be safe if you assign it soon enough, but I'd suggest checking that.

Replacing kmalloc() with kzalloc() should do, right ?

*shrug*
That would do it, all right, but since you memcpy() over all but the last element, I'd suggest cleaning that last element explicitly. Hell knows - depends on how large your arrays are...

Quoted 8 lines
Quoted 7 lines

* orig_node_del_if() looks odd - it removes element #hard_iface->if_num
and shifts all subsequent ones down; then it renumbers interfaces to match that. So far, so good, and there's even a plausible comment about locking:

   /* renumber remaining batman interfaces _inside_ of orig_hash_lock */

except that no such lock exists since commit d007260. What protects us from the obvious race in there?

Quoted 5 lines

Thanks for catching this. I agree that this is not properly protected. All functions accessing orig_node->bcast_own(_sum) use orig_node->ogm_cnt_lock to lock each other out. Obviously we would need a global lock for the interface renumbering which will be as ugly as the current array resizing is. You don't happen to have a good example of a resizable array at hand ?

Depends... How large those arrays realistically get? I would probably consider allocating these guys separately and hashing them by orig_node/hwif pair, but feasibility of that depends on how many of each do you expect to see and how often do their numbers change...

#32 lore raw
Quoted 6 lines · On Monday, April 23, 2012 14:43:24 Al Viro wrote:

Other GFP_* allocations fail only when system is in a really lousy state - killing processes, etc. GFP_ATOMIC can fail in much milder conditions; note that they can't e.g. swap a page out or write a dirty page out and free it, etc. _Any_ allocation failures need to be dealt with, of course, but with GFP_ATOMIC ones failures are just a fact of life - it's not even an emergency situation.

Ok, that is what I thought.

Quoted 6 lines
Quoted 1 line

Replacing kmalloc() with kzalloc() should do, right ?

*shrug*
That would do it, all right, but since you memcpy() over all but the last element, I'd suggest cleaning that last element explicitly. Hell knows - depends on how large your arrays are...

Don't think that is worth the hassle. The index of these arrays is the number of interfaces batman is running on. In 90% of all cases it will be a single interface. Have yet to encounter a system with more than 3 interfaces.

Quoted 11 lines
Quoted 6 lines

Thanks for catching this. I agree that this is not properly protected. All functions accessing orig_node->bcast_own(_sum) use orig_node->ogm_cnt_lock to lock each other out. Obviously we would need a global lock for the interface renumbering which will be as ugly as the current array resizing is. You don't happen to have a good example of a resizable array at hand ?

Depends... How large those arrays realistically get? I would probably consider allocating these guys separately and hashing them by orig_node/hwif pair, but feasibility of that depends on how many of each do you expect to see and how often do their numbers change...

As I explained above: The index is not big and does not change often (when an interface is added or removed).
Can you explain the "hashing them by orig_node/hwif pair" part in greater detail ?

Thanks,
Marek

A message that is not in the archive (sent off-list or not yet received): [email protected]

#34 lore raw

Re: [B.A.T.M.A.N.] [PATCH 1/5] batman-adv: don't bother flipping ->tt_data

Hey,

I was about to merge the patch when I realized checkpatch is complaining. See below for details.

Review int recv_tt_query(struct sk_buff *skb, struct · line 618

Quoted 6 lines
(quoted diff) +1 -1
@@ -618,7 +615,7 @@ int recv_tt_query(struct sk_buff *skb, struct
 hard_iface *recv_if) if (skb_linearize(skb) < 0)
 				goto out;
 
-			tt_len = tt_query->tt_data * sizeof(struct tt_change);
+			tt_len = ntohs(tt_query->tt_data) * sizeof(struct tt_change);

Line is too long.

Quoted 10 lines
(quoted diff) +1 -1
@@ -1727,7 +1727,7 @@ void handle_tt_response(struct bat_priv *bat_priv,
 
 	bat_dbg(DBG_TT, bat_priv,
 		"Received TT_RESPONSE from %pM for ttvn %d t_size: %d [%c]\n",
-		tt_response->src, tt_response->ttvn, tt_response->tt_data,
+		tt_response->src, tt_response->ttvn, ntohs(tt_response->tt_data),
 		(tt_response->flags & TT_FULL_TABLE ? 'F' : '.'));
 
 	/* we should have never asked a backbone gw */

Review void handle_tt_response(struct bat_priv *bat_priv, · line 1746

Quoted 7 lines
(quoted diff) +1 -1
@@ -1741,7 +1741,7 @@ void handle_tt_response(struct bat_priv *bat_priv,
 	if (tt_response->flags & TT_FULL_TABLE)
 		tt_fill_gtable(bat_priv, tt_response);
 	else
-		tt_update_changes(bat_priv, orig_node, tt_response->tt_data,
+		tt_update_changes(bat_priv, orig_node, ntohs(tt_response->tt_data),
 				  tt_response->ttvn,
 				  (struct tt_change *)(tt_response + 1));

Both changes are too long as well.

Regards,
Marek

A message that is not in the archive (sent off-list or not yet received): [email protected]

Applied
#36 lore raw

Re: [B.A.T.M.A.N.] [PATCH 5/5] batman-adv: get rid of pointless cast in memcpy()

Quoted 4 lines · On Sunday, April 22, 2012 14:50:29 Al Viro wrote:

memcpy() arguments are void *, precisely to avoid that kind of pointless casts.

Signed-off-by Al Viro <[email protected]>

Applied in revision 815cfb5.

Thanks,
Marek

A message that is not in the archive (sent off-list or not yet received): [email protected]

Applied
#38 lore raw

Re: [B.A.T.M.A.N.] [PATCH 4/5] batman-adv: trivial endianness annotations

Quoted 7 lines · On Sunday, April 22, 2012 14:47:50 Al Viro wrote:
Signed-off-by Al Viro <[email protected]>
---
 net/batman-adv/bridge_loop_avoidance.c |    6 +++---
 net/batman-adv/distributed-arp-table.c |   16 ++++++++--------
 net/batman-adv/distributed-arp-table.h |    4 ++--
 net/batman-adv/packet.h                |   12 ++++++------
 4 files changed, 19 insertions(+), 19 deletions(-)

Applied in revision 276f053.

Thanks,
Marek

A message that is not in the archive (sent off-list or not yet received): [email protected]

Applied
#40 lore raw

Re: [B.A.T.M.A.N.] [PATCH 3/5] batman-adv: keep batman_ogm_packet ->seqno net-endian all along

Quoted 5 lines · On Sunday, April 22, 2012 14:46:29 Al Viro wrote:
Signed-off-by Al Viro <[email protected]>
---
 net/batman-adv/bat_iv_ogm.c |   25 ++++++++++---------------
 net/batman-adv/packet.h     |    2 +-
 2 files changed, 11 insertions(+), 16 deletions(-)

Applied in revision e62e464.

Thanks,
Marek