* [B.A.T.M.A.N.] pull request: batman-adv 2012-04-18
@ 2012-04-18 9:59 Antonio Quartulli
2012-04-18 9:59 ` [B.A.T.M.A.N.] [PATCH 01/13] batman-adv: convert the tt_crc to network order Antonio Quartulli
` (14 more replies)
0 siblings, 15 replies; 31+ messages in thread
From: Antonio Quartulli @ 2012-04-18 9:59 UTC (permalink / raw)
To: davem; +Cc: netdev, b.a.t.m.a.n
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(-)
^ permalink raw reply [flat|nested] 31+ messages in thread
* [B.A.T.M.A.N.] [PATCH 01/13] batman-adv: convert the tt_crc to network order
2012-04-18 9:59 [B.A.T.M.A.N.] pull request: batman-adv 2012-04-18 Antonio Quartulli
@ 2012-04-18 9:59 ` Antonio Quartulli
2012-04-18 9:59 ` [B.A.T.M.A.N.] [PATCH 02/13] batman-adv: remove duplicated line in comment Antonio Quartulli
` (13 subsequent siblings)
14 siblings, 0 replies; 31+ messages in thread
From: Antonio Quartulli @ 2012-04-18 9:59 UTC (permalink / raw)
To: davem; +Cc: netdev, b.a.t.m.a.n
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 <viro@ZenIV.linux.org.uk>
Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
net/batman-adv/translation-table.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index e16a369..a38d315 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -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)
--
1.7.9.4
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [B.A.T.M.A.N.] [PATCH 02/13] batman-adv: remove duplicated line in comment
2012-04-18 9:59 [B.A.T.M.A.N.] pull request: batman-adv 2012-04-18 Antonio Quartulli
2012-04-18 9:59 ` [B.A.T.M.A.N.] [PATCH 01/13] batman-adv: convert the tt_crc to network order Antonio Quartulli
@ 2012-04-18 9:59 ` Antonio Quartulli
2012-04-18 10:00 ` [B.A.T.M.A.N.] [PATCH 03/13] batman-adv: move ogm initialization into the proper function Antonio Quartulli
` (12 subsequent siblings)
14 siblings, 0 replies; 31+ messages in thread
From: Antonio Quartulli @ 2012-04-18 9:59 UTC (permalink / raw)
To: davem; +Cc: netdev, b.a.t.m.a.n
Remove an accidentally added duplicated line in a function comment
Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
net/batman-adv/bridge_loop_avoidance.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/net/batman-adv/bridge_loop_avoidance.c b/net/batman-adv/bridge_loop_avoidance.c
index 1cf18ac..c8642b5 100644
--- a/net/batman-adv/bridge_loop_avoidance.c
+++ b/net/batman-adv/bridge_loop_avoidance.c
@@ -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
--
1.7.9.4
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [B.A.T.M.A.N.] [PATCH 03/13] batman-adv: move ogm initialization into the proper function
2012-04-18 9:59 [B.A.T.M.A.N.] pull request: batman-adv 2012-04-18 Antonio Quartulli
2012-04-18 9:59 ` [B.A.T.M.A.N.] [PATCH 01/13] batman-adv: convert the tt_crc to network order Antonio Quartulli
2012-04-18 9:59 ` [B.A.T.M.A.N.] [PATCH 02/13] batman-adv: remove duplicated line in comment Antonio Quartulli
@ 2012-04-18 10:00 ` Antonio Quartulli
2012-04-18 10:00 ` [B.A.T.M.A.N.] [PATCH 04/13] batman-adv: refactoring API: find generalized name for bat_ogm_init callback Antonio Quartulli
` (11 subsequent siblings)
14 siblings, 0 replies; 31+ messages in thread
From: Antonio Quartulli @ 2012-04-18 10:00 UTC (permalink / raw)
To: davem; +Cc: netdev, b.a.t.m.a.n, Marek Lindner
From: Marek Lindner <lindner_marek@yahoo.de>
Signed-off-by: Marek Lindner <lindner_marek@yahoo.de>
Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
net/batman-adv/hard-interface.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c
index 8c4b790..f152007 100644
--- a/net/batman-adv/hard-interface.c
+++ b/net/batman-adv/hard-interface.c
@@ -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:
--
1.7.9.4
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [B.A.T.M.A.N.] [PATCH 04/13] batman-adv: refactoring API: find generalized name for bat_ogm_init callback
2012-04-18 9:59 [B.A.T.M.A.N.] pull request: batman-adv 2012-04-18 Antonio Quartulli
` (2 preceding siblings ...)
2012-04-18 10:00 ` [B.A.T.M.A.N.] [PATCH 03/13] batman-adv: move ogm initialization into the proper function Antonio Quartulli
@ 2012-04-18 10:00 ` Antonio Quartulli
2012-04-18 10:00 ` [B.A.T.M.A.N.] [PATCH 05/13] batman-adv: randomize initial seqno to avoid collision Antonio Quartulli
` (10 subsequent siblings)
14 siblings, 0 replies; 31+ messages in thread
From: Antonio Quartulli @ 2012-04-18 10:00 UTC (permalink / raw)
To: davem; +Cc: netdev, b.a.t.m.a.n, Marek Lindner
From: Marek Lindner <lindner_marek@yahoo.de>
Signed-off-by: Marek Lindner <lindner_marek@yahoo.de>
Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
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(-)
diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c
index fab1071..117b831 100644
--- a/net/batman-adv/bat_iv_ogm.c
+++ b/net/batman-adv/bat_iv_ogm.c
@@ -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,
diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c
index f152007..4d9b85d 100644
--- a/net/batman-adv/hard-interface.c
+++ b/net/batman-adv/hard-interface.c
@@ -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,
diff --git a/net/batman-adv/main.c b/net/batman-adv/main.c
index e67ca96..ca8f395 100644
--- a/net/batman-adv/main.c
+++ b/net/batman-adv/main.c
@@ -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 ||
diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
index a5b1a63..4b92248 100644
--- a/net/batman-adv/types.h
+++ b/net/batman-adv/types.h
@@ -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 */
--
1.7.9.4
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [B.A.T.M.A.N.] [PATCH 05/13] batman-adv: randomize initial seqno to avoid collision
2012-04-18 9:59 [B.A.T.M.A.N.] pull request: batman-adv 2012-04-18 Antonio Quartulli
` (3 preceding siblings ...)
2012-04-18 10:00 ` [B.A.T.M.A.N.] [PATCH 04/13] batman-adv: refactoring API: find generalized name for bat_ogm_init callback Antonio Quartulli
@ 2012-04-18 10:00 ` Antonio Quartulli
2012-04-18 10:00 ` [B.A.T.M.A.N.] [PATCH 06/13] batman-adv: add iface_disable() callback to routing API Antonio Quartulli
` (9 subsequent siblings)
14 siblings, 0 replies; 31+ messages in thread
From: Antonio Quartulli @ 2012-04-18 10:00 UTC (permalink / raw)
To: davem; +Cc: netdev, b.a.t.m.a.n, Marek Lindner
From: Marek Lindner <lindner_marek@yahoo.de>
Signed-off-by: Marek Lindner <lindner_marek@yahoo.de>
Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
net/batman-adv/bat_iv_ogm.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c
index 117b831..95bfc59 100644
--- a/net/batman-adv/bat_iv_ogm.c
+++ b/net/batman-adv/bat_iv_ogm.c
@@ -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);
--
1.7.9.4
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [B.A.T.M.A.N.] [PATCH 06/13] batman-adv: add iface_disable() callback to routing API
2012-04-18 9:59 [B.A.T.M.A.N.] pull request: batman-adv 2012-04-18 Antonio Quartulli
` (4 preceding siblings ...)
2012-04-18 10:00 ` [B.A.T.M.A.N.] [PATCH 05/13] batman-adv: randomize initial seqno to avoid collision Antonio Quartulli
@ 2012-04-18 10:00 ` Antonio Quartulli
2012-04-18 10:00 ` [B.A.T.M.A.N.] [PATCH 07/13] batman-adv: handle routing code initialization properly Antonio Quartulli
` (8 subsequent siblings)
14 siblings, 0 replies; 31+ messages in thread
From: Antonio Quartulli @ 2012-04-18 10:00 UTC (permalink / raw)
To: davem; +Cc: netdev, b.a.t.m.a.n, Marek Lindner
From: Marek Lindner <lindner_marek@yahoo.de>
Signed-off-by: Marek Lindner <lindner_marek@yahoo.de>
Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
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(-)
diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c
index 95bfc59..4cc66db 100644
--- a/net/batman-adv/bat_iv_ogm.c
+++ b/net/batman-adv/bat_iv_ogm.c
@@ -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,
diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c
index 4d9b85d..fd9715e 100644
--- a/net/batman-adv/hard-interface.c
+++ b/net/batman-adv/hard-interface.c
@@ -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 */
diff --git a/net/batman-adv/main.c b/net/batman-adv/main.c
index ca8f395..a47a6ce 100644
--- a/net/batman-adv/main.c
+++ b/net/batman-adv/main.c
@@ -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 ||
diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
index 4b92248..b034cf2 100644
--- a/net/batman-adv/types.h
+++ b/net/batman-adv/types.h
@@ -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 */
--
1.7.9.4
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [B.A.T.M.A.N.] [PATCH 07/13] batman-adv: handle routing code initialization properly
2012-04-18 9:59 [B.A.T.M.A.N.] pull request: batman-adv 2012-04-18 Antonio Quartulli
` (5 preceding siblings ...)
2012-04-18 10:00 ` [B.A.T.M.A.N.] [PATCH 06/13] batman-adv: add iface_disable() callback to routing API Antonio Quartulli
@ 2012-04-18 10:00 ` Antonio Quartulli
2012-04-18 10:00 ` [B.A.T.M.A.N.] [PATCH 08/13] batman-adv: refactoring API: find generalized name for bat_ogm_init_primary callback Antonio Quartulli
` (7 subsequent siblings)
14 siblings, 0 replies; 31+ messages in thread
From: Antonio Quartulli @ 2012-04-18 10:00 UTC (permalink / raw)
To: davem; +Cc: netdev, b.a.t.m.a.n, Marek Lindner
From: Marek Lindner <lindner_marek@yahoo.de>
Signed-off-by: Marek Lindner <lindner_marek@yahoo.de>
Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
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(-)
diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c
index 4cc66db..2714670 100644
--- a/net/batman-adv/bat_iv_ogm.c
+++ b/net/batman-adv/bat_iv_ogm.c
@@ -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)
diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c
index fd9715e..3b391fd 100644
--- a/net/batman-adv/hard-interface.c
+++ b/net/batman-adv/hard-interface.c
@@ -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;
diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
index b034cf2..dd78023 100644
--- a/net/batman-adv/types.h
+++ b/net/batman-adv/types.h
@@ -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 */
--
1.7.9.4
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [B.A.T.M.A.N.] [PATCH 08/13] batman-adv: refactoring API: find generalized name for bat_ogm_init_primary callback
2012-04-18 9:59 [B.A.T.M.A.N.] pull request: batman-adv 2012-04-18 Antonio Quartulli
` (6 preceding siblings ...)
2012-04-18 10:00 ` [B.A.T.M.A.N.] [PATCH 07/13] batman-adv: handle routing code initialization properly Antonio Quartulli
@ 2012-04-18 10:00 ` Antonio Quartulli
2012-04-18 10:00 ` [B.A.T.M.A.N.] [PATCH 09/13] batman-adv: rename BATMAN_OGM_LEN to BATMAN_OGM_HLEN Antonio Quartulli
` (6 subsequent siblings)
14 siblings, 0 replies; 31+ messages in thread
From: Antonio Quartulli @ 2012-04-18 10:00 UTC (permalink / raw)
To: davem; +Cc: netdev, b.a.t.m.a.n, Marek Lindner
From: Marek Lindner <lindner_marek@yahoo.de>
Signed-off-by: Marek Lindner <lindner_marek@yahoo.de>
Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
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(-)
diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c
index 2714670..c0bdb90 100644
--- a/net/batman-adv/bat_iv_ogm.c
+++ b/net/batman-adv/bat_iv_ogm.c
@@ -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,
diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c
index 3b391fd..75a555b 100644
--- a/net/batman-adv/hard-interface.c
+++ b/net/batman-adv/hard-interface.c
@@ -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:
diff --git a/net/batman-adv/main.c b/net/batman-adv/main.c
index a47a6ce..7913272 100644
--- a/net/batman-adv/main.c
+++ b/net/batman-adv/main.c
@@ -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 ||
diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
index dd78023..4d93aad 100644
--- a/net/batman-adv/types.h
+++ b/net/batman-adv/types.h
@@ -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 */
--
1.7.9.4
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [B.A.T.M.A.N.] [PATCH 09/13] batman-adv: rename BATMAN_OGM_LEN to BATMAN_OGM_HLEN
2012-04-18 9:59 [B.A.T.M.A.N.] pull request: batman-adv 2012-04-18 Antonio Quartulli
` (7 preceding siblings ...)
2012-04-18 10:00 ` [B.A.T.M.A.N.] [PATCH 08/13] batman-adv: refactoring API: find generalized name for bat_ogm_init_primary callback Antonio Quartulli
@ 2012-04-18 10:00 ` Antonio Quartulli
2012-04-18 10:00 ` [B.A.T.M.A.N.] [PATCH 10/13] batman-adv: mark existing ogm variables as batman iv Antonio Quartulli
` (5 subsequent siblings)
14 siblings, 0 replies; 31+ messages in thread
From: Antonio Quartulli @ 2012-04-18 10:00 UTC (permalink / raw)
To: davem; +Cc: netdev, b.a.t.m.a.n, Marek Lindner
From: Marek Lindner <lindner_marek@yahoo.de>
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 <lindner_marek@yahoo.de>
Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
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(-)
diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c
index c0bdb90..a2af289 100644
--- a/net/batman-adv/bat_iv_ogm.c
+++ b/net/batman-adv/bat_iv_ogm.c
@@ -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 *)
diff --git a/net/batman-adv/packet.h b/net/batman-adv/packet.h
index 59800e8..59dec0a 100644
--- a/net/batman-adv/packet.h
+++ b/net/batman-adv/packet.h
@@ -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;
diff --git a/net/batman-adv/routing.c b/net/batman-adv/routing.c
index 78eddc9..ac13a6a 100644
--- a/net/batman-adv/routing.c
+++ b/net/batman-adv/routing.c
@@ -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);
diff --git a/net/batman-adv/send.c b/net/batman-adv/send.c
index af7a674..b5f078c 100644
--- a/net/batman-adv/send.c
+++ b/net/batman-adv/send.c
@@ -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;
}
--
1.7.9.4
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [B.A.T.M.A.N.] [PATCH 10/13] batman-adv: mark existing ogm variables as batman iv
2012-04-18 9:59 [B.A.T.M.A.N.] pull request: batman-adv 2012-04-18 Antonio Quartulli
` (8 preceding siblings ...)
2012-04-18 10:00 ` [B.A.T.M.A.N.] [PATCH 09/13] batman-adv: rename BATMAN_OGM_LEN to BATMAN_OGM_HLEN Antonio Quartulli
@ 2012-04-18 10:00 ` Antonio Quartulli
2012-04-18 10:00 ` [B.A.T.M.A.N.] [PATCH 11/13] batman-adv: use ETH_HLEN instead of sizeof(struct ethhdr) Antonio Quartulli
` (4 subsequent siblings)
14 siblings, 0 replies; 31+ messages in thread
From: Antonio Quartulli @ 2012-04-18 10:00 UTC (permalink / raw)
To: davem; +Cc: netdev, b.a.t.m.a.n, Marek Lindner
From: Marek Lindner <lindner_marek@yahoo.de>
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 <lindner_marek@yahoo.de>
Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
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(-)
diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c
index a2af289..98f7182 100644
--- a/net/batman-adv/bat_iv_ogm.c
+++ b/net/batman-adv/bat_iv_ogm.c
@@ -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() */
diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c
index 75a555b..e8c5da3 100644
--- a/net/batman-adv/hard-interface.c
+++ b/net/batman-adv/hard-interface.c
@@ -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;
diff --git a/net/batman-adv/packet.h b/net/batman-adv/packet.h
index 59dec0a..f54969c 100644
--- a/net/batman-adv/packet.h
+++ b/net/batman-adv/packet.h
@@ -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
--
1.7.9.4
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [B.A.T.M.A.N.] [PATCH 11/13] batman-adv: use ETH_HLEN instead of sizeof(struct ethhdr)
2012-04-18 9:59 [B.A.T.M.A.N.] pull request: batman-adv 2012-04-18 Antonio Quartulli
` (9 preceding siblings ...)
2012-04-18 10:00 ` [B.A.T.M.A.N.] [PATCH 10/13] batman-adv: mark existing ogm variables as batman iv Antonio Quartulli
@ 2012-04-18 10:00 ` Antonio Quartulli
2012-04-18 10:00 ` [B.A.T.M.A.N.] [PATCH 12/13] batman-adv: print OGM seq numbers as unsigned int Antonio Quartulli
` (3 subsequent siblings)
14 siblings, 0 replies; 31+ messages in thread
From: Antonio Quartulli @ 2012-04-18 10:00 UTC (permalink / raw)
To: davem; +Cc: netdev, b.a.t.m.a.n
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 <ordex@autistici.org>
---
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(-)
diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c
index 98f7182..d7fa8af 100644
--- a/net/batman-adv/bat_iv_ogm.c
+++ b/net/batman-adv/bat_iv_ogm.c
@@ -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);
diff --git a/net/batman-adv/bridge_loop_avoidance.c b/net/batman-adv/bridge_loop_avoidance.c
index c8642b5..ad394c6 100644
--- a/net/batman-adv/bridge_loop_avoidance.c
+++ b/net/batman-adv/bridge_loop_avoidance.c
@@ -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);
diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c
index e8c5da3..47c79d7 100644
--- a/net/batman-adv/hard-interface.c
+++ b/net/batman-adv/hard-interface.c
@@ -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)
diff --git a/net/batman-adv/icmp_socket.c b/net/batman-adv/icmp_socket.c
index b87518e..2e98a57 100644
--- a/net/batman-adv/icmp_socket.c
+++ b/net/batman-adv/icmp_socket.c
@@ -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)) {
diff --git a/net/batman-adv/routing.c b/net/batman-adv/routing.c
index ac13a6a..ff56086 100644
--- a/net/batman-adv/routing.c
+++ b/net/batman-adv/routing.c
@@ -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;
diff --git a/net/batman-adv/send.c b/net/batman-adv/send.c
index b5f078c..7c66b61 100644
--- a/net/batman-adv/send.c
+++ b/net/batman-adv/send.c
@@ -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);
diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c
index efe0fba..6e2530b 100644
--- a/net/batman-adv/soft-interface.c
+++ b/net/batman-adv/soft-interface.c
@@ -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;
diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
index 4d93aad..2f4848b 100644
--- a/net/batman-adv/types.h
+++ b/net/batman-adv/types.h
@@ -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))))
diff --git a/net/batman-adv/vis.c b/net/batman-adv/vis.c
index c4a5b8c..cec216f 100644
--- a/net/batman-adv/vis.c
+++ b/net/batman-adv/vis.c
@@ -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));
--
1.7.9.4
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [B.A.T.M.A.N.] [PATCH 12/13] batman-adv: print OGM seq numbers as unsigned int
2012-04-18 9:59 [B.A.T.M.A.N.] pull request: batman-adv 2012-04-18 Antonio Quartulli
` (10 preceding siblings ...)
2012-04-18 10:00 ` [B.A.T.M.A.N.] [PATCH 11/13] batman-adv: use ETH_HLEN instead of sizeof(struct ethhdr) Antonio Quartulli
@ 2012-04-18 10:00 ` Antonio Quartulli
2012-04-18 10:00 ` [B.A.T.M.A.N.] [PATCH 13/13] batman-adv: skip the window protection test when the originator has no neighbours Antonio Quartulli
` (2 subsequent siblings)
14 siblings, 0 replies; 31+ messages in thread
From: Antonio Quartulli @ 2012-04-18 10:00 UTC (permalink / raw)
To: davem; +Cc: netdev, b.a.t.m.a.n
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 <ordex@autistici.org>
---
net/batman-adv/bat_iv_ogm.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c
index d7fa8af..1014210 100644
--- a/net/batman-adv/bat_iv_ogm.c
+++ b/net/batman-adv/bat_iv_ogm.c
@@ -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,
--
1.7.9.4
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [B.A.T.M.A.N.] [PATCH 13/13] batman-adv: skip the window protection test when the originator has no neighbours
2012-04-18 9:59 [B.A.T.M.A.N.] pull request: batman-adv 2012-04-18 Antonio Quartulli
` (11 preceding siblings ...)
2012-04-18 10:00 ` [B.A.T.M.A.N.] [PATCH 12/13] batman-adv: print OGM seq numbers as unsigned int Antonio Quartulli
@ 2012-04-18 10:00 ` Antonio Quartulli
2012-04-18 17:22 ` [B.A.T.M.A.N.] pull request: batman-adv 2012-04-18 David Miller
2012-04-18 18:08 ` Al Viro
14 siblings, 0 replies; 31+ messages in thread
From: Antonio Quartulli @ 2012-04-18 10:00 UTC (permalink / raw)
To: davem; +Cc: netdev, b.a.t.m.a.n
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 <lindner_marek@yahoo.de>
Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
net/batman-adv/bat_iv_ogm.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c
index 1014210..8b2db2e 100644
--- a/net/batman-adv/bat_iv_ogm.c
+++ b/net/batman-adv/bat_iv_ogm.c
@@ -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;
--
1.7.9.4
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [B.A.T.M.A.N.] pull request: batman-adv 2012-04-18
2012-04-18 9:59 [B.A.T.M.A.N.] pull request: batman-adv 2012-04-18 Antonio Quartulli
` (12 preceding siblings ...)
2012-04-18 10:00 ` [B.A.T.M.A.N.] [PATCH 13/13] batman-adv: skip the window protection test when the originator has no neighbours Antonio Quartulli
@ 2012-04-18 17:22 ` David Miller
2012-04-18 18:08 ` Al Viro
14 siblings, 0 replies; 31+ messages in thread
From: David Miller @ 2012-04-18 17:22 UTC (permalink / raw)
To: ordex; +Cc: netdev, b.a.t.m.a.n
From: Antonio Quartulli <ordex@autistici.org>
Date: Wed, 18 Apr 2012 11:59:57 +0200
> 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
This looks a lot better, pulled, thanks!
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [B.A.T.M.A.N.] pull request: batman-adv 2012-04-18
2012-04-18 9:59 [B.A.T.M.A.N.] pull request: batman-adv 2012-04-18 Antonio Quartulli
` (13 preceding siblings ...)
2012-04-18 17:22 ` [B.A.T.M.A.N.] pull request: batman-adv 2012-04-18 David Miller
@ 2012-04-18 18:08 ` Al Viro
2012-04-18 18:09 ` [B.A.T.M.A.N.] [PATCH 1/4] batman: don't bother flipping ->tt_data Al Viro
` (4 more replies)
14 siblings, 5 replies; 31+ messages in thread
From: Al Viro @ 2012-04-18 18:08 UTC (permalink / raw)
To: Antonio Quartulli; +Cc: netdev, b.a.t.m.a.n, davem
On Wed, Apr 18, 2012 at 11:59:57AM +0200, Antonio Quartulli wrote:
> 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...
^ permalink raw reply [flat|nested] 31+ messages in thread
* [B.A.T.M.A.N.] [PATCH 1/4] batman: don't bother flipping ->tt_data
2012-04-18 18:08 ` Al Viro
@ 2012-04-18 18:09 ` Al Viro
2012-04-18 18:10 ` [B.A.T.M.A.N.] [PATCH 2/4] batman: don't bother flipping ->tt_crc Al Viro
` (3 subsequent siblings)
4 siblings, 0 replies; 31+ messages in thread
From: Al Viro @ 2012-04-18 18:09 UTC (permalink / raw)
To: Antonio Quartulli; +Cc: netdev, b.a.t.m.a.n, davem
just keep it net-endian all along
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
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(-)
diff --git a/net/batman-adv/packet.h b/net/batman-adv/packet.h
index 307dbb3..9157f7c 100644
--- a/net/batman-adv/packet.h
+++ b/net/batman-adv/packet.h
@@ -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 {
diff --git a/net/batman-adv/routing.c b/net/batman-adv/routing.c
index 2181a91..4310cfe 100644
--- a/net/batman-adv/routing.c
+++ b/net/batman-adv/routing.c
@@ -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;
diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index 88e4c8e..a9c2b59 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -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));
--
1.7.2.5
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [B.A.T.M.A.N.] [PATCH 2/4] batman: don't bother flipping ->tt_crc
2012-04-18 18:08 ` Al Viro
2012-04-18 18:09 ` [B.A.T.M.A.N.] [PATCH 1/4] batman: don't bother flipping ->tt_data Al Viro
@ 2012-04-18 18:10 ` Al Viro
2012-04-19 5:41 ` Antonio Quartulli
2012-04-18 18:11 ` [B.A.T.M.A.N.] batman: keep batman_ogm_packet ->seqno net-endian all along Al Viro
` (2 subsequent siblings)
4 siblings, 1 reply; 31+ messages in thread
From: Al Viro @ 2012-04-18 18:10 UTC (permalink / raw)
To: Antonio Quartulli; +Cc: netdev, b.a.t.m.a.n, davem
Again, keep ->tt_crc in batman_ogv_packet net-endian
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
net/batman-adv/bat_iv_ogm.c | 6 ++----
net/batman-adv/packet.h | 2 +-
2 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c
index ab2085c..5aa8861 100644
--- a/net/batman-adv/bat_iv_ogm.c
+++ b/net/batman-adv/bat_iv_ogm.c
@@ -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;
diff --git a/net/batman-adv/packet.h b/net/batman-adv/packet.h
index 9157f7c..61a7fe0 100644
--- a/net/batman-adv/packet.h
+++ b/net/batman-adv/packet.h
@@ -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)
--
1.7.2.5
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [B.A.T.M.A.N.] batman: keep batman_ogm_packet ->seqno net-endian all along
2012-04-18 18:08 ` Al Viro
2012-04-18 18:09 ` [B.A.T.M.A.N.] [PATCH 1/4] batman: don't bother flipping ->tt_data Al Viro
2012-04-18 18:10 ` [B.A.T.M.A.N.] [PATCH 2/4] batman: don't bother flipping ->tt_crc Al Viro
@ 2012-04-18 18:11 ` Al Viro
2012-04-18 18:15 ` Al Viro
2012-04-18 18:14 ` [B.A.T.M.A.N.] batman: trivial endianness annotations Al Viro
[not found] ` <20120419061026.GC8658@ritirata.org>
4 siblings, 1 reply; 31+ messages in thread
From: Al Viro @ 2012-04-18 18:11 UTC (permalink / raw)
To: Antonio Quartulli; +Cc: netdev, b.a.t.m.a.n, davem
Ditto for its ->seqno...
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
net/batman-adv/bat_iv_ogm.c | 25 ++++++++++---------------
net/batman-adv/packet.h | 2 +-
2 files changed, 11 insertions(+), 16 deletions(-)
diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c
index 5aa8861..cc6d4c3 100644
--- a/net/batman-adv/bat_iv_ogm.c
+++ b/net/batman-adv/bat_iv_ogm.c
@@ -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,
diff --git a/net/batman-adv/packet.h b/net/batman-adv/packet.h
index 61a7fe0..ad39938 100644
--- a/net/batman-adv/packet.h
+++ b/net/batman-adv/packet.h
@@ -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 */
--
1.7.2.5
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [B.A.T.M.A.N.] batman: trivial endianness annotations
2012-04-18 18:08 ` Al Viro
` (2 preceding siblings ...)
2012-04-18 18:11 ` [B.A.T.M.A.N.] batman: keep batman_ogm_packet ->seqno net-endian all along Al Viro
@ 2012-04-18 18:14 ` Al Viro
[not found] ` <20120419061026.GC8658@ritirata.org>
4 siblings, 0 replies; 31+ messages in thread
From: Al Viro @ 2012-04-18 18:14 UTC (permalink / raw)
To: Antonio Quartulli; +Cc: netdev, b.a.t.m.a.n, davem
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 <viro@zeniv.linux.org.uk>
---
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(-)
diff --git a/net/batman-adv/bridge_loop_avoidance.c b/net/batman-adv/bridge_loop_avoidance.c
index 8bf9751..d51288b 100644
--- a/net/batman-adv/bridge_loop_avoidance.c
+++ b/net/batman-adv/bridge_loop_avoidance.c
@@ -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",
diff --git a/net/batman-adv/distributed-arp-table.c b/net/batman-adv/distributed-arp-table.c
index b43bece..d87f914 100644
--- a/net/batman-adv/distributed-arp-table.c
+++ b/net/batman-adv/distributed-arp-table.c
@@ -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;
diff --git a/net/batman-adv/distributed-arp-table.h b/net/batman-adv/distributed-arp-table.h
index fcf6c15..01d5b4f 100644
--- a/net/batman-adv/distributed-arp-table.h
+++ b/net/batman-adv/distributed-arp-table.h
@@ -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,
diff --git a/net/batman-adv/packet.h b/net/batman-adv/packet.h
index ad39938..eb4fdf6 100644
--- a/net/batman-adv/packet.h
+++ b/net/batman-adv/packet.h
@@ -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 */
--
1.7.2.5
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [B.A.T.M.A.N.] batman: keep batman_ogm_packet ->seqno net-endian all along
2012-04-18 18:11 ` [B.A.T.M.A.N.] batman: keep batman_ogm_packet ->seqno net-endian all along Al Viro
@ 2012-04-18 18:15 ` Al Viro
0 siblings, 0 replies; 31+ messages in thread
From: Al Viro @ 2012-04-18 18:15 UTC (permalink / raw)
To: Antonio Quartulli; +Cc: netdev, b.a.t.m.a.n, davem
grr... this one would be 3/4 and the last ("trivial endianness annotations") -
4/4. My apologies...
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCH 2/4] batman: don't bother flipping ->tt_crc
2012-04-18 18:10 ` [B.A.T.M.A.N.] [PATCH 2/4] batman: don't bother flipping ->tt_crc Al Viro
@ 2012-04-19 5:41 ` Antonio Quartulli
2012-04-19 5:49 ` Antonio Quartulli
0 siblings, 1 reply; 31+ messages in thread
From: Antonio Quartulli @ 2012-04-19 5:41 UTC (permalink / raw)
To: Al Viro; +Cc: netdev, b.a.t.m.a.n, davem
[-- Attachment #1: Type: text/plain, Size: 1048 bytes --]
On Wed, Apr 18, 2012 at 07:10:58 +0100, Al Viro wrote:
> @@ -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?
> @@ -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,
--
Antonio Quartulli
..each of us alone is worth nothing..
Ernesto "Che" Guevara
[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCH 2/4] batman: don't bother flipping ->tt_crc
2012-04-19 5:41 ` Antonio Quartulli
@ 2012-04-19 5:49 ` Antonio Quartulli
0 siblings, 0 replies; 31+ messages in thread
From: Antonio Quartulli @ 2012-04-19 5:49 UTC (permalink / raw)
To: Al Viro; +Cc: netdev, b.a.t.m.a.n, davem
[-- Attachment #1: Type: text/plain, Size: 688 bytes --]
On Thu, Apr 19, 2012 at 07:41:14AM +0200, Antonio Quartulli wrote:
> On Wed, Apr 18, 2012 at 07:10:58 +0100, Al Viro wrote:
> > @@ -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,
--
Antonio Quartulli
..each of us alone is worth nothing..
Ernesto "Che" Guevara
[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [B.A.T.M.A.N.] pull request: batman-adv 2012-04-18
[not found] ` <20120419134854.GA6871@ZenIV.linux.org.uk>
@ 2012-04-19 14:09 ` Antonio Quartulli
2012-04-23 5:18 ` Marek Lindner
0 siblings, 1 reply; 31+ messages in thread
From: Antonio Quartulli @ 2012-04-19 14:09 UTC (permalink / raw)
To: Al Viro; +Cc: b.a.t.m.a.n, Marek Lindner
[-- Attachment #1: Type: text/plain, Size: 1982 bytes --]
Hello,
On Thu, Apr 19, 2012 at 02:48:54 +0100, Al Viro wrote:
> On Thu, Apr 19, 2012 at 08:10:27AM +0200, Antonio Quartulli wrote:
>
> > 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...
>
> > Moreover, patch 3/4, fixes the memcpy() casting too other than the endianess
>
> 4/4, actually.
>
> > 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!
> 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:
> /* 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,
--
Antonio Quartulli
..each of us alone is worth nothing..
Ernesto "Che" Guevara
[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [B.A.T.M.A.N.] pull request: batman-adv 2012-04-18
2012-04-19 14:09 ` [B.A.T.M.A.N.] pull request: batman-adv 2012-04-18 Antonio Quartulli
@ 2012-04-23 5:18 ` Marek Lindner
2012-04-23 6:43 ` Al Viro
0 siblings, 1 reply; 31+ messages in thread
From: Marek Lindner @ 2012-04-23 5:18 UTC (permalink / raw)
To: b.a.t.m.a.n; +Cc: Al Viro
Hi,
> 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.
> * 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 ?
> * 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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [B.A.T.M.A.N.] pull request: batman-adv 2012-04-18
2012-04-23 5:18 ` Marek Lindner
@ 2012-04-23 6:43 ` Al Viro
2012-04-23 7:17 ` Marek Lindner
0 siblings, 1 reply; 31+ messages in thread
From: Al Viro @ 2012-04-23 6:43 UTC (permalink / raw)
To: Marek Lindner; +Cc: b.a.t.m.a.n
On Mon, Apr 23, 2012 at 01:18:25PM +0800, Marek Lindner wrote:
>
> Hi,
>
> > 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.
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.
> > * 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...
> > * 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 ?
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...
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [B.A.T.M.A.N.] pull request: batman-adv 2012-04-18
2012-04-23 6:43 ` Al Viro
@ 2012-04-23 7:17 ` Marek Lindner
0 siblings, 0 replies; 31+ messages in thread
From: Marek Lindner @ 2012-04-23 7:17 UTC (permalink / raw)
To: b.a.t.m.a.n; +Cc: Al Viro
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.
> > 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.
> > 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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCH 1/5] batman-adv: don't bother flipping ->tt_data
[not found] ` <20120422064426.GU6871@ZenIV.linux.org.uk>
@ 2012-04-25 12:11 ` Marek Lindner
0 siblings, 0 replies; 31+ messages in thread
From: Marek Lindner @ 2012-04-25 12:11 UTC (permalink / raw)
To: Al Viro; +Cc: The list for a Better Approach To Mobile Ad-hoc Networking
Hey,
I was about to merge the patch when I realized checkpatch is complaining. See
below for details.
> @@ -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.
> @@ -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));
Both changes are too long as well.
Regards,
Marek
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCH 5/5] batman-adv: get rid of pointless cast in memcpy()
[not found] ` <20120422065029.GY6871@ZenIV.linux.org.uk>
@ 2012-04-25 12:14 ` Marek Lindner
0 siblings, 0 replies; 31+ messages in thread
From: Marek Lindner @ 2012-04-25 12:14 UTC (permalink / raw)
To: Al Viro; +Cc: The list for a Better Approach To Mobile Ad-hoc Networking
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 <viro@zeniv.linux.org.uk>
Applied in revision 815cfb5.
Thanks,
Marek
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCH 4/5] batman-adv: trivial endianness annotations
[not found] ` <20120422064750.GX6871@ZenIV.linux.org.uk>
@ 2012-04-25 12:18 ` Marek Lindner
0 siblings, 0 replies; 31+ messages in thread
From: Marek Lindner @ 2012-04-25 12:18 UTC (permalink / raw)
To: Al Viro; +Cc: The list for a Better Approach To Mobile Ad-hoc Networking
On Sunday, April 22, 2012 14:47:50 Al Viro wrote:
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
> 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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCH 3/5] batman-adv: keep batman_ogm_packet ->seqno net-endian all along
[not found] ` <20120422064629.GW6871@ZenIV.linux.org.uk>
@ 2012-04-25 12:25 ` Marek Lindner
0 siblings, 0 replies; 31+ messages in thread
From: Marek Lindner @ 2012-04-25 12:25 UTC (permalink / raw)
To: Al Viro; +Cc: The list for a Better Approach To Mobile Ad-hoc Networking
On Sunday, April 22, 2012 14:46:29 Al Viro wrote:
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
> 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
^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2012-04-25 12:25 UTC | newest]
Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-18 9:59 [B.A.T.M.A.N.] pull request: batman-adv 2012-04-18 Antonio Quartulli
2012-04-18 9:59 ` [B.A.T.M.A.N.] [PATCH 01/13] batman-adv: convert the tt_crc to network order Antonio Quartulli
2012-04-18 9:59 ` [B.A.T.M.A.N.] [PATCH 02/13] batman-adv: remove duplicated line in comment Antonio Quartulli
2012-04-18 10:00 ` [B.A.T.M.A.N.] [PATCH 03/13] batman-adv: move ogm initialization into the proper function Antonio Quartulli
2012-04-18 10:00 ` [B.A.T.M.A.N.] [PATCH 04/13] batman-adv: refactoring API: find generalized name for bat_ogm_init callback Antonio Quartulli
2012-04-18 10:00 ` [B.A.T.M.A.N.] [PATCH 05/13] batman-adv: randomize initial seqno to avoid collision Antonio Quartulli
2012-04-18 10:00 ` [B.A.T.M.A.N.] [PATCH 06/13] batman-adv: add iface_disable() callback to routing API Antonio Quartulli
2012-04-18 10:00 ` [B.A.T.M.A.N.] [PATCH 07/13] batman-adv: handle routing code initialization properly Antonio Quartulli
2012-04-18 10:00 ` [B.A.T.M.A.N.] [PATCH 08/13] batman-adv: refactoring API: find generalized name for bat_ogm_init_primary callback Antonio Quartulli
2012-04-18 10:00 ` [B.A.T.M.A.N.] [PATCH 09/13] batman-adv: rename BATMAN_OGM_LEN to BATMAN_OGM_HLEN Antonio Quartulli
2012-04-18 10:00 ` [B.A.T.M.A.N.] [PATCH 10/13] batman-adv: mark existing ogm variables as batman iv Antonio Quartulli
2012-04-18 10:00 ` [B.A.T.M.A.N.] [PATCH 11/13] batman-adv: use ETH_HLEN instead of sizeof(struct ethhdr) Antonio Quartulli
2012-04-18 10:00 ` [B.A.T.M.A.N.] [PATCH 12/13] batman-adv: print OGM seq numbers as unsigned int Antonio Quartulli
2012-04-18 10:00 ` [B.A.T.M.A.N.] [PATCH 13/13] batman-adv: skip the window protection test when the originator has no neighbours Antonio Quartulli
2012-04-18 17:22 ` [B.A.T.M.A.N.] pull request: batman-adv 2012-04-18 David Miller
2012-04-18 18:08 ` Al Viro
2012-04-18 18:09 ` [B.A.T.M.A.N.] [PATCH 1/4] batman: don't bother flipping ->tt_data Al Viro
2012-04-18 18:10 ` [B.A.T.M.A.N.] [PATCH 2/4] batman: don't bother flipping ->tt_crc Al Viro
2012-04-19 5:41 ` Antonio Quartulli
2012-04-19 5:49 ` Antonio Quartulli
2012-04-18 18:11 ` [B.A.T.M.A.N.] batman: keep batman_ogm_packet ->seqno net-endian all along Al Viro
2012-04-18 18:15 ` Al Viro
2012-04-18 18:14 ` [B.A.T.M.A.N.] batman: trivial endianness annotations Al Viro
[not found] ` <20120419061026.GC8658@ritirata.org>
[not found] ` <20120419134854.GA6871@ZenIV.linux.org.uk>
2012-04-19 14:09 ` [B.A.T.M.A.N.] pull request: batman-adv 2012-04-18 Antonio Quartulli
2012-04-23 5:18 ` Marek Lindner
2012-04-23 6:43 ` Al Viro
2012-04-23 7:17 ` Marek Lindner
[not found] ` <20120422064426.GU6871@ZenIV.linux.org.uk>
2012-04-25 12:11 ` [B.A.T.M.A.N.] [PATCH 1/5] batman-adv: don't bother flipping ->tt_data Marek Lindner
[not found] ` <20120422065029.GY6871@ZenIV.linux.org.uk>
2012-04-25 12:14 ` [B.A.T.M.A.N.] [PATCH 5/5] batman-adv: get rid of pointless cast in memcpy() Marek Lindner
[not found] ` <20120422064750.GX6871@ZenIV.linux.org.uk>
2012-04-25 12:18 ` [B.A.T.M.A.N.] [PATCH 4/5] batman-adv: trivial endianness annotations Marek Lindner
[not found] ` <20120422064629.GW6871@ZenIV.linux.org.uk>
2012-04-25 12:25 ` [B.A.T.M.A.N.] [PATCH 3/5] batman-adv: keep batman_ogm_packet ->seqno net-endian all along Marek Lindner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).