b.a.t.m.a.n.lists.open-mesh.org archive mirror
 help / color / mirror / Atom feed
* [B.A.T.M.A.N.] [PATCHv5 0/9] DAT: Distributed ARP Table
@ 2012-02-09 23:41 Antonio Quartulli
  2012-02-09 23:41 ` [B.A.T.M.A.N.] [PATCHv5 1/9] batman-adv: implement an helper function to forge unicast packets Antonio Quartulli
                   ` (10 more replies)
  0 siblings, 11 replies; 27+ messages in thread
From: Antonio Quartulli @ 2012-02-09 23:41 UTC (permalink / raw)
  To: b.a.t.m.a.n

Hello people,

****
This is the **fifth** version of this patchset. The whole code has been reviewed
after some discussions on irc and here in the ml in the previous threads.
Several bugs have been spotted and fixed. Thanks Marek, Sven, Simon, Andrew and
all the others for the comments/feedbacks.

This patchset assumes that patch
[PATCH] batman-adv: add biggest_unsigned_int(x) macro
has already been applied
****

For details about the DAT mechanism, please refer to the preliminary
documentation that has already been put on the wiki[1]. A more exhaustive and
detailed documentation is going to come soon.



PatchsetV5 Additions:
------------------------

- Sanity check for ARP messages (messages containing multicast/loopback
  addresses are ignored)
- A new unicast packet type has been introduced (UNICAST_4ADDR), which contains
  the originator address and the payload type.
- Thanks to this new packet type, more and more debugging messages have been
  added. In this way it is easier to track the behaviour of DAT and it is possible to
  understand who and why added a given entry in the soft_iface ARP table.



^ permalink raw reply	[flat|nested] 27+ messages in thread

* [B.A.T.M.A.N.] [PATCHv5 1/9] batman-adv: implement an helper function to forge unicast packets
  2012-02-09 23:41 [B.A.T.M.A.N.] [PATCHv5 0/9] DAT: Distributed ARP Table Antonio Quartulli
@ 2012-02-09 23:41 ` Antonio Quartulli
  2012-02-09 23:41 ` [B.A.T.M.A.N.] [PATCHv5 2/9] batman-adv: add a new log level for DAT/ARP debugging Antonio Quartulli
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 27+ messages in thread
From: Antonio Quartulli @ 2012-02-09 23:41 UTC (permalink / raw)
  To: b.a.t.m.a.n

A new function named prepare_unicast_packet() has been implemented so that it can
do all the needed operations to set up a skb for unicast sending. It is general
enough to be used in every context. Helpful for later developments

Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
 unicast.c |   36 ++++++++++++++++++++++++------------
 unicast.h |    1 +
 2 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/unicast.c b/unicast.c
index 0897dfa..887342b 100644
--- a/unicast.c
+++ b/unicast.c
@@ -283,6 +283,29 @@ out:
 	return ret;
 }
 
+bool prepare_unicast_packet(struct sk_buff *skb, struct orig_node *orig_node)
+{
+	struct unicast_packet *unicast_packet;
+
+	if (my_skb_head_push(skb, sizeof(*unicast_packet)) < 0)
+		return false;
+
+	unicast_packet = (struct unicast_packet *)skb->data;
+
+	unicast_packet->header.version = COMPAT_VERSION;
+	/* batman packet type: unicast */
+	unicast_packet->header.packet_type = BAT_UNICAST;
+	/* set unicast ttl */
+	unicast_packet->header.ttl = TTL;
+	/* copy the destination for faster routing */
+	memcpy(unicast_packet->dest, orig_node->orig, ETH_ALEN);
+	/* set the destination tt version number */
+	unicast_packet->ttvn =
+		(uint8_t)atomic_read(&orig_node->last_ttvn);
+
+	return true;
+}
+
 int unicast_send_skb(struct sk_buff *skb, struct bat_priv *bat_priv)
 {
 	struct ethhdr *ethhdr = (struct ethhdr *)skb->data;
@@ -315,22 +338,11 @@ find_router:
 	if (!neigh_node)
 		goto out;
 
-	if (my_skb_head_push(skb, sizeof(*unicast_packet)) < 0)
+	if (!prepare_unicast_packet(skb, orig_node))
 		goto out;
 
 	unicast_packet = (struct unicast_packet *)skb->data;
 
-	unicast_packet->header.version = COMPAT_VERSION;
-	/* batman packet type: unicast */
-	unicast_packet->header.packet_type = BAT_UNICAST;
-	/* set unicast ttl */
-	unicast_packet->header.ttl = TTL;
-	/* copy the destination for faster routing */
-	memcpy(unicast_packet->dest, orig_node->orig, ETH_ALEN);
-	/* set the destination tt version number */
-	unicast_packet->ttvn =
-		(uint8_t)atomic_read(&orig_node->last_ttvn);
-
 	if (atomic_read(&bat_priv->fragmentation) &&
 	    data_len + sizeof(*unicast_packet) >
 				neigh_node->if_incoming->net_dev->mtu) {
diff --git a/unicast.h b/unicast.h
index a9faf6b..c51ad3d 100644
--- a/unicast.h
+++ b/unicast.h
@@ -33,6 +33,7 @@ void frag_list_free(struct list_head *head);
 int unicast_send_skb(struct sk_buff *skb, struct bat_priv *bat_priv);
 int frag_send_skb(struct sk_buff *skb, struct bat_priv *bat_priv,
 		  struct hard_iface *hard_iface, const uint8_t dstaddr[]);
+bool prepare_unicast_packet(struct sk_buff *skb, struct orig_node *orig_node);
 
 static inline int frag_can_reassemble(const struct sk_buff *skb, int mtu)
 {
-- 
1.7.3.4


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [B.A.T.M.A.N.] [PATCHv5 2/9] batman-adv: add a new log level for DAT/ARP debugging
  2012-02-09 23:41 [B.A.T.M.A.N.] [PATCHv5 0/9] DAT: Distributed ARP Table Antonio Quartulli
  2012-02-09 23:41 ` [B.A.T.M.A.N.] [PATCHv5 1/9] batman-adv: implement an helper function to forge unicast packets Antonio Quartulli
@ 2012-02-09 23:41 ` Antonio Quartulli
  2012-02-09 23:41 ` [B.A.T.M.A.N.] [PATCHv5 3/9] batman-adv: Distributed ARP Table - create DHT helper functions Antonio Quartulli
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 27+ messages in thread
From: Antonio Quartulli @ 2012-02-09 23:41 UTC (permalink / raw)
  To: b.a.t.m.a.n

A new log level has been added to concentrate messages regarding DAT: ARP
snooping, requests, response and DHT related messages.
The new log level is named DBG_ARP

Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
 README      |    3 ++-
 bat_sysfs.c |    2 +-
 main.h      |    3 ++-
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/README b/README
index 0c7a3c8..4ee2f54 100644
--- a/README
+++ b/README
@@ -201,7 +201,8 @@ abled  during run time. Following log_levels are defined:
 2 - Enable messages related to route added / changed / deleted
 4 - Enable messages related to translation table operations
 8 - Enable messages related to bridge loop avoidance
-15 - enable all messages
+16 - Enable messaged related to DAT, ARP snooping and parsing
+31 - Enable all messages
 
 The debug output can be changed at runtime  using  the  file
 /sys/class/net/bat0/mesh/log_level. e.g.
diff --git a/bat_sysfs.c b/bat_sysfs.c
index 3adb183..abf7300 100644
--- a/bat_sysfs.c
+++ b/bat_sysfs.c
@@ -401,7 +401,7 @@ BAT_ATTR_UINT(gw_sel_class, S_IRUGO | S_IWUSR, 1, TQ_MAX_VALUE,
 static BAT_ATTR(gw_bandwidth, S_IRUGO | S_IWUSR, show_gw_bwidth,
 		store_gw_bwidth);
 #ifdef CONFIG_BATMAN_ADV_DEBUG
-BAT_ATTR_UINT(log_level, S_IRUGO | S_IWUSR, 0, 15, NULL);
+BAT_ATTR_UINT(log_level, S_IRUGO | S_IWUSR, 0, 31, NULL);
 #endif
 
 static struct bat_attribute *mesh_attrs[] = {
diff --git a/main.h b/main.h
index e669eea..3060f94 100644
--- a/main.h
+++ b/main.h
@@ -126,7 +126,8 @@ enum dbg_level {
 	DBG_ROUTES = 1 << 1, /* route added / changed / deleted */
 	DBG_TT	   = 1 << 2, /* translation table operations */
 	DBG_BLA    = 1 << 3, /* bridge loop avoidance */
-	DBG_ALL    = 15
+	DBG_ARP    = 1 << 4, /* snooped arp messages / dht operations */
+	DBG_ALL    = 31
 };
 
 
-- 
1.7.3.4


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [B.A.T.M.A.N.] [PATCHv5 3/9] batman-adv: Distributed ARP Table - create DHT helper functions
  2012-02-09 23:41 [B.A.T.M.A.N.] [PATCHv5 0/9] DAT: Distributed ARP Table Antonio Quartulli
  2012-02-09 23:41 ` [B.A.T.M.A.N.] [PATCHv5 1/9] batman-adv: implement an helper function to forge unicast packets Antonio Quartulli
  2012-02-09 23:41 ` [B.A.T.M.A.N.] [PATCHv5 2/9] batman-adv: add a new log level for DAT/ARP debugging Antonio Quartulli
@ 2012-02-09 23:41 ` Antonio Quartulli
  2012-02-11 13:59   ` Marek Lindner
  2012-02-09 23:41 ` [B.A.T.M.A.N.] [PATCHv5 4/9] batman-adv: Distributed ARP Table - add ARP parsing functions Antonio Quartulli
                   ` (7 subsequent siblings)
  10 siblings, 1 reply; 27+ messages in thread
From: Antonio Quartulli @ 2012-02-09 23:41 UTC (permalink / raw)
  To: b.a.t.m.a.n

Add all the relevant functions in order to manage a Distributed Hash Table over
the B.A.T.M.A.N.-adv network. It will later be used to store several ARP entries
and implement DAT (Distributed ARP Table)

Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
 Makefile.kbuild         |    1 +
 distributed-arp-table.c |  182 +++++++++++++++++++++++++++++++++++++++++++++++
 distributed-arp-table.h |   55 ++++++++++++++
 hard-interface.c        |    5 ++
 main.h                  |    7 ++
 originator.c            |    2 +
 soft-interface.c        |    2 +
 types.h                 |    8 ++
 8 files changed, 262 insertions(+), 0 deletions(-)
 create mode 100644 distributed-arp-table.c
 create mode 100644 distributed-arp-table.h

diff --git a/Makefile.kbuild b/Makefile.kbuild
index 6d5c194..84955b3 100644
--- a/Makefile.kbuild
+++ b/Makefile.kbuild
@@ -24,6 +24,7 @@ batman-adv-y += bat_iv_ogm.o
 batman-adv-y += bat_sysfs.o
 batman-adv-y += bitarray.o
 batman-adv-$(CONFIG_BATMAN_ADV_BLA) += bridge_loop_avoidance.o
+batman-adv-y += distributed-arp-table.o
 batman-adv-y += gateway_client.o
 batman-adv-y += gateway_common.o
 batman-adv-y += hard-interface.o
diff --git a/distributed-arp-table.c b/distributed-arp-table.c
new file mode 100644
index 0000000..4c4e064
--- /dev/null
+++ b/distributed-arp-table.c
@@ -0,0 +1,182 @@
+/*
+ * Copyright (C) 2011 B.A.T.M.A.N. contributors:
+ *
+ * Antonio Quartulli
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of version 2 of the GNU General Public
+ * License as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+ * 02110-1301, USA
+ *
+ */
+
+#include <linux/if_ether.h>
+#include <linux/if_arp.h>
+
+#include "main.h"
+#include "distributed-arp-table.h"
+#include "hard-interface.h"
+#include "originator.h"
+#include "send.h"
+#include "types.h"
+#include "unicast.h"
+
+/* Given a key, selects the candidates which the DHT message has to be sent to.
+ * An originator O is selected if and only if its DHT_ID value is one of three
+ * closest values (but not greater) then the hash value of the key.
+ * ip_dst is the key.
+ *
+ * return an array of size DHT_CANDIDATES_NUM */
+static struct dht_candidate *dht_select_candidates(struct bat_priv *bat_priv,
+						   uint32_t ip_dst)
+{
+	struct hashtable_t *hash = bat_priv->orig_hash;
+	struct hlist_node *node;
+	struct hlist_head *head;
+	struct orig_node *orig_node, *max_orig_node = NULL;
+	int select, i, j;
+	dat_addr_t last_max = DAT_ADDR_MAX, max, tmp_max, ip_key;
+	struct dht_candidate *res;
+	bool chosen_me = false;
+
+	if (!hash)
+		return NULL;
+
+	res = kmalloc(DHT_CANDIDATES_NUM * sizeof(*res), GFP_ATOMIC);
+	if (!res)
+		return NULL;
+
+	ip_key = (dat_addr_t)hash_ipv4(&ip_dst, DAT_ADDR_MAX);
+
+	bat_dbg(DBG_ARP, bat_priv, "DHT_SEL_CAND key: %pI4 %u\n", &ip_dst,
+		ip_key);
+
+	for (select = 0; select < DHT_CANDIDATES_NUM; select++) {
+		max = 0;
+		max_orig_node = NULL;
+		if (!chosen_me) {
+			/* if true, wrap around the key space */
+			if (bat_priv->dht_hash > ip_key)
+				max = DAT_ADDR_MAX - bat_priv->dht_hash +
+					ip_key;
+			else
+				max = ip_key - bat_priv->dht_hash;
+			max = bat_priv->dht_hash;
+			res[select].type = DHT_CANDIDATE_ME;
+		} else
+			res[select].type = DHT_CANDIDATE_NULL;
+		/* for all origins... */
+		for (i = 0; i < hash->size; i++) {
+			head = &hash->table[i];
+
+			rcu_read_lock();
+			hlist_for_each_entry_rcu(orig_node, node, head,
+						 hash_entry) {
+				if (orig_node->dht_hash > ip_key)
+					tmp_max = DAT_ADDR_MAX -
+						orig_node->dht_hash + ip_key;
+				else
+					tmp_max = ip_key - orig_node->dht_hash;
+
+				/* this is closest! */
+				if (tmp_max <= max)
+					continue;
+
+				/* this has already been selected */
+				if (tmp_max > last_max)
+					continue;
+
+				/* In case of hash collision we can select two
+				 * nodes with the same hash, but we have ensure
+				 * they are different */
+				if (tmp_max == last_max) {
+					for (j = 0; j < select; j++)
+						if (res[j].orig_node ==
+						    orig_node)
+							break;
+					if (j < select)
+						continue;
+				}
+
+				if (!atomic_inc_not_zero(&orig_node->refcount))
+					continue;
+
+				max = tmp_max;
+				if (max_orig_node)
+					orig_node_free_ref(max_orig_node);
+				max_orig_node = orig_node;
+			}
+			rcu_read_unlock();
+		}
+		last_max = max;
+		if (max_orig_node) {
+			res[select].type = DHT_CANDIDATE_ORIG;
+			res[select].orig_node = max_orig_node;
+			bat_dbg(DBG_ARP, bat_priv, "DHT_SEL_CAND %d: %pM %u\n",
+				select, max_orig_node->orig, max);
+		}
+		if (res[select].type == DHT_CANDIDATE_ME) {
+			chosen_me = true;
+			bat_dbg(DBG_ARP, bat_priv, "DHT_SEL_CAND %d: ME %u\n",
+				select, bat_priv->dht_hash);
+		}
+
+		max_orig_node = NULL;
+	}
+
+	return res;
+}
+
+/*
+ * Sends the skb payload passed as argument to the candidates selected for
+ * 'ip'. The skb is copied by means of pskb_copy() and is sent as unicast packet
+ * to each of the selected candidate. */
+static bool dht_send_data(struct bat_priv *bat_priv, struct sk_buff *skb,
+			  uint32_t ip)
+{
+	int i;
+	bool ret = false;
+	struct neigh_node *neigh_node = NULL;
+	struct sk_buff *tmp_skb;
+	struct dht_candidate *cand = dht_select_candidates(bat_priv, ip);
+
+	if (!cand)
+		goto out;
+
+	bat_dbg(DBG_ARP, bat_priv, "DHT_SEND for %pI4\n", &ip);
+
+	for (i = 0; i < DHT_CANDIDATES_NUM; i++) {
+		if (cand[i].type == DHT_CANDIDATE_ME ||
+		    cand[i].type == DHT_CANDIDATE_NULL)
+			continue;
+
+		neigh_node = orig_node_get_router(cand[i].orig_node);
+		if (!neigh_node)
+			goto free_orig;
+
+		tmp_skb = pskb_copy(skb, GFP_ATOMIC);
+		if (prepare_unicast_packet(tmp_skb, cand[i].orig_node))
+			send_skb_packet(tmp_skb, neigh_node->if_incoming,
+					neigh_node->addr);
+		else
+			kfree_skb(tmp_skb);
+		/* set ret to true only if we send at least one request */
+		ret = true;
+		neigh_node_free_ref(neigh_node);
+free_orig:
+		orig_node_free_ref(cand[i].orig_node);
+	}
+
+out:
+	kfree(cand);
+	return ret;
+}
diff --git a/distributed-arp-table.h b/distributed-arp-table.h
new file mode 100644
index 0000000..cca5c6a
--- /dev/null
+++ b/distributed-arp-table.h
@@ -0,0 +1,55 @@
+/*
+ * Copyright (C) 2011 B.A.T.M.A.N. contributors:
+ *
+ * Antonio Quartulli
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of version 2 of the GNU General Public
+ * License as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+ * 02110-1301, USA
+ *
+ */
+
+#ifndef _NET_BATMAN_ADV_ARP_H_
+#define _NET_BATMAN_ADV_ARP_H_
+
+/*
+ * dat_addr_t is the type used for all DHT indexes. If it is changed,
+ * DAT_ADDR_MAX is changed as well.
+ *
+ * *Please be careful: dat_addr_t must be UNSIGNED*
+ */
+#define dat_addr_t uint16_t
+#define DAT_ADDR_MAX biggest_unsigned_int(dat_addr_t)
+
+/* hash function to choose an entry in a hash table of given size */
+/* hash algorithm from http://en.wikipedia.org/wiki/Hash_table */
+static inline uint32_t hash_ipv4(const void *data, uint32_t size)
+{
+	const unsigned char *key = data;
+	uint32_t hash = 0;
+	size_t i;
+
+	for (i = 0; i < 4; i++) {
+		hash += key[i];
+		hash += (hash << 10);
+		hash ^= (hash >> 6);
+	}
+
+	hash += (hash << 3);
+	hash ^= (hash >> 11);
+	hash += (hash << 15);
+
+	return hash % size;
+}
+
+#endif /* _NET_BATMAN_ADV_ARP_H_ */
diff --git a/hard-interface.c b/hard-interface.c
index 612f2c8..8cd4a3d 100644
--- a/hard-interface.c
+++ b/hard-interface.c
@@ -20,6 +20,7 @@
  */
 
 #include "main.h"
+#include "distributed-arp-table.h"
 #include "hard-interface.h"
 #include "soft-interface.h"
 #include "send.h"
@@ -118,6 +119,10 @@ static void primary_if_update_addr(struct bat_priv *bat_priv,
 	if (!primary_if)
 		goto out;
 
+	bat_priv->dht_hash = (dat_addr_t)
+				choose_orig(primary_if->net_dev->dev_addr,
+					    DAT_ADDR_MAX);
+
 	vis_packet = (struct vis_packet *)
 				bat_priv->my_vis_info->skb_packet->data;
 	memcpy(vis_packet->vis_orig, primary_if->net_dev->dev_addr, ETH_ALEN);
diff --git a/main.h b/main.h
index 3060f94..9103abd 100644
--- a/main.h
+++ b/main.h
@@ -67,6 +67,9 @@
 
 #define NUM_WORDS BITS_TO_LONGS(TQ_LOCAL_WINDOW_SIZE)
 
+/* numbers of originator to contact for any STORE/GET DHT operation */
+#define DHT_CANDIDATES_NUM 3
+
 #define LOG_BUF_LEN 8192	  /* has to be a power of 2 */
 
 #define VIS_INTERVAL 5000	/* 5 seconds */
@@ -111,6 +114,10 @@ enum uev_type {
 
 #define GW_THRESHOLD	50
 
+#define DHT_CANDIDATE_NULL 0
+#define DHT_CANDIDATE_ME   1
+#define DHT_CANDIDATE_ORIG 2
+
 /*
  * Debug Messages
  */
diff --git a/originator.c b/originator.c
index 2db4b53..3cade01 100644
--- a/originator.c
+++ b/originator.c
@@ -20,6 +20,7 @@
  */
 
 #include "main.h"
+#include "distributed-arp-table.h"
 #include "originator.h"
 #include "hash.h"
 #include "translation-table.h"
@@ -224,6 +225,7 @@ struct orig_node *get_orig_node(struct bat_priv *bat_priv, const uint8_t *addr)
 	orig_node->tt_poss_change = false;
 	orig_node->bat_priv = bat_priv;
 	memcpy(orig_node->orig, addr, ETH_ALEN);
+	orig_node->dht_hash = (dat_addr_t)choose_orig(addr, DAT_ADDR_MAX);
 	orig_node->router = NULL;
 	orig_node->tt_crc = 0;
 	atomic_set(&orig_node->last_ttvn, 0);
diff --git a/soft-interface.c b/soft-interface.c
index 8de8779..75d4a70 100644
--- a/soft-interface.c
+++ b/soft-interface.c
@@ -401,6 +401,8 @@ struct net_device *softif_create(const char *name)
 	if (ret < 0)
 		goto unreg_soft_iface;
 
+	bat_priv->dht_hash = 0;
+
 	ret = sysfs_add_meshif(soft_iface);
 	if (ret < 0)
 		goto unreg_soft_iface;
diff --git a/types.h b/types.h
index 7f7f610..d1da20e 100644
--- a/types.h
+++ b/types.h
@@ -24,6 +24,7 @@
 #ifndef _NET_BATMAN_ADV_TYPES_H_
 #define _NET_BATMAN_ADV_TYPES_H_
 
+#include "distributed-arp-table.h"
 #include "packet.h"
 #include "bitarray.h"
 
@@ -67,6 +68,7 @@ struct hard_iface {
 struct orig_node {
 	uint8_t orig[ETH_ALEN];
 	uint8_t primary_addr[ETH_ALEN];
+	dat_addr_t dht_hash;
 	struct neigh_node __rcu *router; /* rcu protected pointer */
 	unsigned long *bcast_own;
 	uint8_t *bcast_own_sum;
@@ -215,6 +217,7 @@ struct bat_priv {
 	struct gw_node __rcu *curr_gw;  /* rcu protected pointer */
 	atomic_t gw_reselect;
 	struct hard_iface __rcu *primary_if;  /* rcu protected pointer */
+	dat_addr_t dht_hash;
 	struct vis_info *my_vis_info;
 	struct bat_algo_ops *bat_algo_ops;
 };
@@ -385,4 +388,9 @@ struct bat_algo_ops {
 				struct sk_buff *skb);
 };
 
+struct dht_candidate {
+	int type;
+	struct orig_node *orig_node;
+};
+
 #endif /* _NET_BATMAN_ADV_TYPES_H_ */
-- 
1.7.3.4


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [B.A.T.M.A.N.] [PATCHv5 4/9] batman-adv: Distributed ARP Table - add ARP parsing functions
  2012-02-09 23:41 [B.A.T.M.A.N.] [PATCHv5 0/9] DAT: Distributed ARP Table Antonio Quartulli
                   ` (2 preceding siblings ...)
  2012-02-09 23:41 ` [B.A.T.M.A.N.] [PATCHv5 3/9] batman-adv: Distributed ARP Table - create DHT helper functions Antonio Quartulli
@ 2012-02-09 23:41 ` Antonio Quartulli
  2012-02-10 14:21   ` Marek Lindner
  2012-02-11 14:09   ` Marek Lindner
  2012-02-09 23:41 ` [B.A.T.M.A.N.] [PATCHv5 5/9] batman-adv: Distributed ARP Table - add snooping functions for ARP messages Antonio Quartulli
                   ` (6 subsequent siblings)
  10 siblings, 2 replies; 27+ messages in thread
From: Antonio Quartulli @ 2012-02-09 23:41 UTC (permalink / raw)
  To: b.a.t.m.a.n

ARP messages are now parsed to make it possible to trigger special actions
depending on their types (snooping).

Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
 distributed-arp-table.c |   56 +++++++++++++++++++++++++++++++++++++++++++++++
 distributed-arp-table.h |   12 ++++++++++
 2 files changed, 68 insertions(+), 0 deletions(-)

diff --git a/distributed-arp-table.c b/distributed-arp-table.c
index 4c4e064..95b9934 100644
--- a/distributed-arp-table.c
+++ b/distributed-arp-table.c
@@ -30,6 +30,22 @@
 #include "types.h"
 #include "unicast.h"
 
+static inline void bat_dbg_arp(struct bat_priv *bat_priv,
+			       struct sk_buff *skb, uint16_t type) {
+	char buf[30];
+	const char *type_str[] = { "REQUEST", "REPLY", "RREQUEST", "RREPLY",
+				      "InREQUEST", "InREPLY", "NAK" };
+
+	if (type >= 1 && type <= ARRAY_SIZE(type_str))
+		scnprintf(buf, sizeof(buf), "%s", type_str[type - 1]);
+	else
+		scnprintf(buf, sizeof(buf), "UNKNOWN (%hu)", type);
+
+	bat_dbg(DBG_ARP, bat_priv, "ARP message of type %s recognised "
+		"[src: %pM-%pI4 dst: %pM-%pI4]\n", buf, ARP_HW_SRC(skb),
+		&ARP_IP_SRC(skb), ARP_HW_DST(skb), &ARP_IP_DST(skb));
+}
+
 /* Given a key, selects the candidates which the DHT message has to be sent to.
  * An originator O is selected if and only if its DHT_ID value is one of three
  * closest values (but not greater) then the hash value of the key.
@@ -180,3 +196,43 @@ out:
 	kfree(cand);
 	return ret;
 }
+
+/* Returns arphdr->ar_op if the skb contains a valid ARP packet, otherwise
+ * returns 0 */
+static uint16_t arp_get_type(struct bat_priv *bat_priv, struct sk_buff *skb)
+{
+	struct arphdr *arphdr;
+	struct ethhdr *ethhdr;
+	uint16_t type = 0;
+
+	if (unlikely(!pskb_may_pull(skb, ETH_HLEN)))
+		goto out;
+
+	ethhdr = (struct ethhdr *)skb_mac_header(skb);
+
+	if (ethhdr->h_proto != htons(ETH_P_ARP))
+		goto out;
+
+	if (unlikely(!pskb_may_pull(skb, ETH_HLEN + arp_hdr_len(skb->dev))))
+		goto out;
+
+	arphdr = (struct arphdr *)(skb->data + sizeof(struct ethhdr));
+
+	/* Check whether the ARP packet carries a valid
+	 * IP information */
+	if (arphdr->ar_hrd != htons(ARPHRD_ETHER))
+		goto out;
+
+	if (arphdr->ar_pro != htons(ETH_P_IP))
+		goto out;
+
+	if (arphdr->ar_hln != ETH_ALEN)
+		goto out;
+
+	if (arphdr->ar_pln != 4)
+		goto out;
+
+	type = ntohs(arphdr->ar_op);
+out:
+	return type;
+}
diff --git a/distributed-arp-table.h b/distributed-arp-table.h
index cca5c6a..3e0f5c6 100644
--- a/distributed-arp-table.h
+++ b/distributed-arp-table.h
@@ -22,6 +22,12 @@
 #ifndef _NET_BATMAN_ADV_ARP_H_
 #define _NET_BATMAN_ADV_ARP_H_
 
+#include "main.h"
+
+#include <linux/if_arp.h>
+
+struct bat_priv;
+
 /*
  * dat_addr_t is the type used for all DHT indexes. If it is changed,
  * DAT_ADDR_MAX is changed as well.
@@ -31,6 +37,12 @@
 #define dat_addr_t uint16_t
 #define DAT_ADDR_MAX biggest_unsigned_int(dat_addr_t)
 
+#define ARP_HW_SRC(skb) ((uint8_t *)(skb->data) + sizeof(struct ethhdr) + \
+			sizeof(struct arphdr))
+#define ARP_IP_SRC(skb) (*(uint32_t *)(ARP_HW_SRC(skb) + ETH_ALEN))
+#define ARP_HW_DST(skb) (ARP_HW_SRC(skb) + ETH_ALEN + 4)
+#define ARP_IP_DST(skb) (*(uint32_t *)(ARP_HW_SRC(skb) + ETH_ALEN * 2 + 4))
+
 /* hash function to choose an entry in a hash table of given size */
 /* hash algorithm from http://en.wikipedia.org/wiki/Hash_table */
 static inline uint32_t hash_ipv4(const void *data, uint32_t size)
-- 
1.7.3.4


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [B.A.T.M.A.N.] [PATCHv5 5/9] batman-adv: Distributed ARP Table - add snooping functions for ARP messages
  2012-02-09 23:41 [B.A.T.M.A.N.] [PATCHv5 0/9] DAT: Distributed ARP Table Antonio Quartulli
                   ` (3 preceding siblings ...)
  2012-02-09 23:41 ` [B.A.T.M.A.N.] [PATCHv5 4/9] batman-adv: Distributed ARP Table - add ARP parsing functions Antonio Quartulli
@ 2012-02-09 23:41 ` Antonio Quartulli
  2012-02-10 14:25   ` Marek Lindner
  2012-02-09 23:41 ` [B.A.T.M.A.N.] [PATCHv5 6/9] batman-adv: Distributed ARP Table - increase default soft_iface ARP table timeout Antonio Quartulli
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 27+ messages in thread
From: Antonio Quartulli @ 2012-02-09 23:41 UTC (permalink / raw)
  To: b.a.t.m.a.n

In case of an ARP message going in or out the soft_iface, it is intercepted and
a special action is performed. In particular the DHT helper functions previously
implemented are used to store all the ARP entries belonging to the network in
order to provide a fast and unicast lookup instead of the classic broadcast flooding
mechanism.
Each node stores the entries it is responsible for (following the DHT rules) in
its soft_iface ARP table. This makes it possible to reuse the kernel data
structures and functions for ARP management.

Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
 distributed-arp-table.c |  266 ++++++++++++++++++++++++++++++++++++++++++++--
 distributed-arp-table.h |   12 ++
 main.h                  |    2 +
 send.c                  |    6 +
 soft-interface.c        |   15 +++-
 5 files changed, 288 insertions(+), 13 deletions(-)

diff --git a/distributed-arp-table.c b/distributed-arp-table.c
index 95b9934..7e9130b 100644
--- a/distributed-arp-table.c
+++ b/distributed-arp-table.c
@@ -21,6 +21,8 @@
 
 #include <linux/if_ether.h>
 #include <linux/if_arp.h>
+/* needed to use arp_tbl */
+#include <net/arp.h>
 
 #include "main.h"
 #include "distributed-arp-table.h"
@@ -28,22 +30,14 @@
 #include "originator.h"
 #include "send.h"
 #include "types.h"
+#include "translation-table.h"
 #include "unicast.h"
 
 static inline void bat_dbg_arp(struct bat_priv *bat_priv,
 			       struct sk_buff *skb, uint16_t type) {
-	char buf[30];
-	const char *type_str[] = { "REQUEST", "REPLY", "RREQUEST", "RREPLY",
-				      "InREQUEST", "InREPLY", "NAK" };
-
-	if (type >= 1 && type <= ARRAY_SIZE(type_str))
-		scnprintf(buf, sizeof(buf), "%s", type_str[type - 1]);
-	else
-		scnprintf(buf, sizeof(buf), "UNKNOWN (%hu)", type);
-
-	bat_dbg(DBG_ARP, bat_priv, "ARP message of type %s recognised "
-		"[src: %pM-%pI4 dst: %pM-%pI4]\n", buf, ARP_HW_SRC(skb),
-		&ARP_IP_SRC(skb), ARP_HW_DST(skb), &ARP_IP_DST(skb));
+	bat_dbg(DBG_ARP, bat_priv, "ARP MSG = [src: %pM-%pI4 dst: %pM-%pI4]\n",
+		ARP_HW_SRC(skb), &ARP_IP_SRC(skb), ARP_HW_DST(skb),
+		&ARP_IP_DST(skb));
 }
 
 /* Given a key, selects the candidates which the DHT message has to be sent to.
@@ -197,6 +191,31 @@ out:
 	return ret;
 }
 
+/* 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,
+			     uint8_t *hw)
+{
+	struct neighbour *n = NULL;
+	struct hard_iface *primary_if = primary_if_get_selected(bat_priv);
+	if (!primary_if)
+		goto out;
+
+	n = __neigh_lookup(&arp_tbl, &ip, primary_if->soft_iface, 1);
+	if (!n)
+		goto out;
+
+	bat_dbg(DBG_ARP, bat_priv, "Updating neighbour: %pI4 - %pM\n", &ip, hw);
+
+	neigh_update(n, hw, NUD_CONNECTED, NEIGH_UPDATE_F_OVERRIDE);
+out:
+	if (n && !IS_ERR(n))
+		neigh_release(n);
+	if (primary_if)
+		hardif_free_ref(primary_if);
+}
+
 /* Returns arphdr->ar_op if the skb contains a valid ARP packet, otherwise
  * returns 0 */
 static uint16_t arp_get_type(struct bat_priv *bat_priv, struct sk_buff *skb)
@@ -232,7 +251,230 @@ static uint16_t arp_get_type(struct bat_priv *bat_priv, struct sk_buff *skb)
 	if (arphdr->ar_pln != 4)
 		goto out;
 
+	/* Check for bad reply/request. If the ARP message is not sane, DAT
+	 * will simply ignore it */
+	if (ipv4_is_loopback(ARP_IP_SRC(skb)) ||
+	    ipv4_is_multicast(ARP_IP_SRC(skb)) ||
+	    ipv4_is_loopback(ARP_IP_DST(skb)) ||
+	    ipv4_is_multicast(ARP_IP_DST(skb)))
+		goto out;
+
 	type = ntohs(arphdr->ar_op);
 out:
 	return type;
 }
+
+/* return true if the message has been sent to the dht candidates, false
+ * otherwise. In case of true the message has to be enqueued to permit the
+ * fallback */
+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;
+	uint8_t *hw_src;
+	bool ret = false;
+	struct neighbour *n = NULL;
+	struct hard_iface *primary_if = NULL;
+	struct sk_buff *skb_new;
+
+	type = arp_get_type(bat_priv, skb);
+	/* If we get an ARP_REQUEST we have to send the unicast message to the
+	 * selected DHT candidates */
+	if (type != ARPOP_REQUEST)
+		goto out;
+
+	bat_dbg(DBG_ARP, bat_priv, "Parsing outgoing ARP REQUEST\n");
+	bat_dbg_arp(bat_priv, skb, type);
+
+	ip_src = ARP_IP_SRC(skb);
+	hw_src = ARP_HW_SRC(skb);
+	ip_dst = ARP_IP_DST(skb);
+
+	primary_if = primary_if_get_selected(bat_priv);
+	if (!primary_if)
+		goto out;
+
+	arp_neigh_update(bat_priv, ip_src, hw_src);
+
+	n = neigh_lookup(&arp_tbl, &ip_dst, primary_if->soft_iface);
+	/* check if it is a valid neigh entry */
+	if (n && (n->nud_state & NUD_CONNECTED)) {
+		skb_new = arp_create(ARPOP_REPLY, ETH_P_ARP, ip_src,
+				     primary_if->soft_iface, ip_dst, hw_src,
+				     n->ha, hw_src);
+		if (!skb_new)
+			goto out;
+
+		skb_reset_mac_header(skb_new);
+		skb_new->protocol = eth_type_trans(skb_new,
+						   primary_if->soft_iface);
+		bat_priv->stats.rx_packets++;
+		bat_priv->stats.rx_bytes += skb->len + sizeof(struct ethhdr);
+		primary_if->soft_iface->last_rx = jiffies;
+
+		netif_rx(skb_new);
+		bat_dbg(DBG_ARP, bat_priv, "ARP request replied locally\n");
+	} else
+		/* Send the request on the DHT */
+		ret = dht_send_data(bat_priv, skb, ip_dst);
+out:
+	if (n)
+		neigh_release(n);
+	if (primary_if)
+		hardif_free_ref(primary_if);
+	return ret;
+}
+
+/* This function is meant to be invoked for an ARP request which is coming into
+ * the bat0 interfaces from the mesh network. It will check for the needed data
+ * into the local table. If found, an ARP reply is sent immediately, otherwise
+ * the caller has to deliver the ARP request to the upper layer */
+bool dat_snoop_incoming_arp_request(struct bat_priv *bat_priv,
+				    struct sk_buff *skb)
+{
+	uint16_t type;
+	uint32_t ip_src, ip_dst;
+	uint8_t *hw_src;
+	struct hard_iface *primary_if = NULL;
+	struct sk_buff *skb_new;
+	struct neighbour *n = NULL;
+	bool ret = false;
+
+	type = arp_get_type(bat_priv, skb);
+	if (type != ARPOP_REQUEST)
+		goto out;
+
+	hw_src = ARP_HW_SRC(skb);
+	ip_src = ARP_IP_SRC(skb);
+	ip_dst = ARP_IP_DST(skb);
+
+	bat_dbg(DBG_ARP, bat_priv, "Parsing incoming ARP REQUEST\n");
+	bat_dbg_arp(bat_priv, skb, type);
+
+	primary_if = primary_if_get_selected(bat_priv);
+	if (!primary_if)
+		goto out;
+
+	arp_neigh_update(bat_priv, ip_src, hw_src);
+
+	n = neigh_lookup(&arp_tbl, &ip_dst, primary_if->soft_iface);
+	/* check if it is a valid neigh entry */
+	if (!n || !(n->nud_state & NUD_CONNECTED))
+		goto out;
+
+	skb_new = arp_create(ARPOP_REPLY, ETH_P_ARP, ip_src,
+			     primary_if->soft_iface, ip_dst, hw_src, n->ha,
+			     hw_src);
+
+	if (!skb_new)
+		goto out;
+
+	unicast_send_skb(skb_new, bat_priv);
+
+	ret = true;
+out:
+	if (n)
+		neigh_release(n);
+	if (primary_if)
+		hardif_free_ref(primary_if);
+	if (ret)
+		kfree_skb(skb);
+	return ret;
+}
+
+/* This function is meant to be invoked on an ARP reply packet going into the
+ * soft interface. The related neighbour entry has to be updated and the DHT has
+ * to be populated as well */
+bool dat_snoop_outgoing_arp_reply(struct bat_priv *bat_priv,
+				  struct sk_buff *skb)
+{
+	uint16_t type;
+	uint32_t ip_src, ip_dst;
+	uint8_t *hw_src, *hw_dst;
+	bool ret = false;
+
+	type = arp_get_type(bat_priv, skb);
+	if (type != ARPOP_REPLY)
+		goto out;
+
+	bat_dbg(DBG_ARP, bat_priv, "Parsing outgoing ARP REPLY\n");
+	bat_dbg_arp(bat_priv, skb, type);
+
+	hw_src = ARP_HW_SRC(skb);
+	ip_src = ARP_IP_SRC(skb);
+	hw_dst = ARP_HW_DST(skb);
+	ip_dst = ARP_IP_DST(skb);
+
+	arp_neigh_update(bat_priv, ip_src, hw_src);
+	arp_neigh_update(bat_priv, ip_dst, hw_dst);
+
+	/* Send the ARP reply to the candidates for both the IP addresses we
+	 * fetched from the ARP reply */
+	dht_send_data(bat_priv, skb, ip_src);
+	dht_send_data(bat_priv, skb, ip_dst);
+	ret = true;
+out:
+	return ret;
+}
+
+/* This function has to be invoked on an ARP reply coming into the soft
+ * interface from the mesh network. The local table has to be updated */
+bool dat_snoop_incoming_arp_reply(struct bat_priv *bat_priv,
+				  struct sk_buff *skb)
+{
+	uint16_t type;
+	uint32_t ip_src, ip_dst;
+	uint8_t *hw_src, *hw_dst;
+	bool ret = false;
+
+	type = arp_get_type(bat_priv, skb);
+	if (type != ARPOP_REPLY)
+		goto out;
+
+	bat_dbg(DBG_ARP, bat_priv, "Parsing incoming ARP REPLY\n");
+	bat_dbg_arp(bat_priv, skb, type);
+
+	hw_src = ARP_HW_SRC(skb);
+	ip_src = ARP_IP_SRC(skb);
+	hw_dst = ARP_HW_DST(skb);
+	ip_dst = ARP_IP_DST(skb);
+
+	/* Update our internal cache with both the IP addresses we fetched from
+	 * the ARP reply */
+	arp_neigh_update(bat_priv, ip_src, hw_src);
+	arp_neigh_update(bat_priv, ip_dst, hw_dst);
+
+	/* if this REPLY is directed to a client of mine, let's deliver the
+	 * packet to the interface */
+	ret = !is_my_client(bat_priv, hw_dst);
+out:
+	/* if ret == false packet has to be delivered to the interface */
+	return ret;
+}
+
+bool arp_drop_broadcast_packet(struct bat_priv *bat_priv,
+			       struct forw_packet *forw_packet)
+{
+	struct neighbour *n;
+
+	/* If this packet is an ARP_REQUEST and we already have the information
+	 * that it is going to ask, we can drop the packet */
+	if (!forw_packet->num_packets &&
+			(arp_get_type(bat_priv, forw_packet->skb) ==
+							ARPOP_REQUEST)) {
+		n = neigh_lookup(&arp_tbl, &ARP_IP_DST(forw_packet->skb),
+				 forw_packet->if_incoming->soft_iface);
+		/* check if we already know this neigh */
+		if (n && (n->nud_state & NUD_CONNECTED)) {
+			bat_dbg(DBG_ARP, bat_priv, "ARP Request for %pI4: "
+				"fallback prevented\n",
+				&ARP_IP_DST(forw_packet->skb));
+			return true;
+		}
+
+		bat_dbg(DBG_ARP, bat_priv, "ARP Request for %pI4: fallback\n",
+			&ARP_IP_DST(forw_packet->skb));
+	}
+	return false;
+}
diff --git a/distributed-arp-table.h b/distributed-arp-table.h
index 3e0f5c6..cd779f4 100644
--- a/distributed-arp-table.h
+++ b/distributed-arp-table.h
@@ -27,6 +27,7 @@
 #include <linux/if_arp.h>
 
 struct bat_priv;
+struct forw_packet;
 
 /*
  * dat_addr_t is the type used for all DHT indexes. If it is changed,
@@ -43,6 +44,17 @@ struct bat_priv;
 #define ARP_HW_DST(skb) (ARP_HW_SRC(skb) + ETH_ALEN + 4)
 #define ARP_IP_DST(skb) (*(uint32_t *)(ARP_HW_SRC(skb) + ETH_ALEN * 2 + 4))
 
+bool dat_snoop_outgoing_arp_request(struct bat_priv *bat_priv,
+				    struct sk_buff *skb);
+bool dat_snoop_incoming_arp_request(struct bat_priv *bat_priv,
+				    struct sk_buff *skb);
+bool dat_snoop_outgoing_arp_reply(struct bat_priv *bat_priv,
+				  struct sk_buff *skb);
+bool dat_snoop_incoming_arp_reply(struct bat_priv *bat_priv,
+				  struct sk_buff *skb);
+bool arp_drop_broadcast_packet(struct bat_priv *bat_priv,
+			       struct forw_packet *forw_packet);
+
 /* hash function to choose an entry in a hash table of given size */
 /* hash algorithm from http://en.wikipedia.org/wiki/Hash_table */
 static inline uint32_t hash_ipv4(const void *data, uint32_t size)
diff --git a/main.h b/main.h
index 9103abd..eed55cb 100644
--- a/main.h
+++ b/main.h
@@ -67,6 +67,8 @@
 
 #define NUM_WORDS BITS_TO_LONGS(TQ_LOCAL_WINDOW_SIZE)
 
+/* msecs after which an ARP_REQUEST is sent in broadcast as fallback */
+#define ARP_REQ_DELAY 250
 /* numbers of originator to contact for any STORE/GET DHT operation */
 #define DHT_CANDIDATES_NUM 3
 
diff --git a/send.c b/send.c
index 4137580..666a524 100644
--- a/send.c
+++ b/send.c
@@ -20,6 +20,7 @@
  */
 
 #include "main.h"
+#include "distributed-arp-table.h"
 #include "send.h"
 #include "routing.h"
 #include "translation-table.h"
@@ -29,6 +30,8 @@
 #include "gateway_common.h"
 #include "originator.h"
 
+#include <net/arp.h>
+
 static void send_outstanding_bcast_packet(struct work_struct *work);
 
 /* send out an already prepared packet to the given address via the
@@ -274,6 +277,9 @@ static void send_outstanding_bcast_packet(struct work_struct *work)
 	if (atomic_read(&bat_priv->mesh_state) == MESH_DEACTIVATING)
 		goto out;
 
+	if (arp_drop_broadcast_packet(bat_priv, forw_packet))
+		goto out;
+
 	/* rebroadcast packet */
 	rcu_read_lock();
 	list_for_each_entry_rcu(hard_iface, &hardif_list, list) {
diff --git a/soft-interface.c b/soft-interface.c
index 75d4a70..ab3fb31 100644
--- a/soft-interface.c
+++ b/soft-interface.c
@@ -22,6 +22,7 @@
 #include "main.h"
 #include "soft-interface.h"
 #include "hard-interface.h"
+#include "distributed-arp-table.h"
 #include "routing.h"
 #include "send.h"
 #include "bat_debugfs.h"
@@ -134,6 +135,7 @@ static int interface_tx(struct sk_buff *skb, struct net_device *soft_iface)
 	int data_len = skb->len, ret;
 	short vid = -1;
 	bool do_bcast = false;
+	unsigned long brd_delay = 1;
 
 	if (atomic_read(&bat_priv->mesh_state) != MESH_ACTIVE)
 		goto dropped;
@@ -194,6 +196,9 @@ static int interface_tx(struct sk_buff *skb, struct net_device *soft_iface)
 		if (!primary_if)
 			goto dropped;
 
+		if (dat_snoop_outgoing_arp_request(bat_priv, skb))
+			brd_delay = msecs_to_jiffies(ARP_REQ_DELAY);
+
 		if (my_skb_head_push(skb, sizeof(*bcast_packet)) < 0)
 			goto dropped;
 
@@ -213,7 +218,7 @@ static int interface_tx(struct sk_buff *skb, struct net_device *soft_iface)
 		bcast_packet->seqno =
 			htonl(atomic_inc_return(&bat_priv->bcast_seqno));
 
-		add_bcast_packet_to_list(bat_priv, skb, 1);
+		add_bcast_packet_to_list(bat_priv, skb, brd_delay);
 
 		/* a copy is stored in the bcast list, therefore removing
 		 * the original skb. */
@@ -227,6 +232,8 @@ static int interface_tx(struct sk_buff *skb, struct net_device *soft_iface)
 				goto dropped;
 		}
 
+		dat_snoop_outgoing_arp_reply(bat_priv, skb);
+
 		ret = unicast_send_skb(skb, bat_priv);
 		if (ret != 0)
 			goto dropped_freed;
@@ -277,6 +284,12 @@ void interface_rx(struct net_device *soft_iface,
 		goto dropped;
 	}
 
+	if (dat_snoop_incoming_arp_request(bat_priv, skb))
+		goto out;
+
+	if (dat_snoop_incoming_arp_reply(bat_priv, skb))
+		goto out;
+
 	/* skb->dev & skb->pkt_type are set here */
 	if (unlikely(!pskb_may_pull(skb, ETH_HLEN)))
 		goto dropped;
-- 
1.7.3.4


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [B.A.T.M.A.N.] [PATCHv5 6/9] batman-adv: Distributed ARP Table - increase default soft_iface ARP table timeout
  2012-02-09 23:41 [B.A.T.M.A.N.] [PATCHv5 0/9] DAT: Distributed ARP Table Antonio Quartulli
                   ` (4 preceding siblings ...)
  2012-02-09 23:41 ` [B.A.T.M.A.N.] [PATCHv5 5/9] batman-adv: Distributed ARP Table - add snooping functions for ARP messages Antonio Quartulli
@ 2012-02-09 23:41 ` Antonio Quartulli
  2012-02-09 23:41 ` [B.A.T.M.A.N.] [PATCHv5 7/9] batman-adv: Distributed ARP Table - add compile option Antonio Quartulli
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 27+ messages in thread
From: Antonio Quartulli @ 2012-02-09 23:41 UTC (permalink / raw)
  To: b.a.t.m.a.n

The default timeout value for ARP entries belonging to any soft_iface
ARP table has been incremented by a factor 4. This is necessary because the DHT
will store several network entries in the soft_iface ARP table.

Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
 distributed-arp-table.c |   19 +++++++++++++++++++
 distributed-arp-table.h |    1 +
 main.h                  |    3 +++
 soft-interface.c        |    2 ++
 4 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/distributed-arp-table.c b/distributed-arp-table.c
index 7e9130b..5c81086 100644
--- a/distributed-arp-table.c
+++ b/distributed-arp-table.c
@@ -23,6 +23,7 @@
 #include <linux/if_arp.h>
 /* needed to use arp_tbl */
 #include <net/arp.h>
+#include <linux/inetdevice.h>
 
 #include "main.h"
 #include "distributed-arp-table.h"
@@ -478,3 +479,21 @@ bool arp_drop_broadcast_packet(struct bat_priv *bat_priv,
 	}
 	return false;
 }
+
+void arp_change_timeout(struct net_device *soft_iface, const char *name)
+{
+	struct in_device *in_dev = in_dev_get(soft_iface);
+	if (!in_dev) {
+		pr_err("Unable to set ARP parameters for the batman interface "
+		       "'%s'\n", name);
+		return;
+	}
+
+	/* Introduce a delay in the ARP state-machine transactions. Entries
+	 * will be kept in the ARP table for the default time multiplied by 4 */
+	in_dev->arp_parms->base_reachable_time *= ARP_TIMEOUT_FACTOR;
+	in_dev->arp_parms->gc_staletime *= ARP_TIMEOUT_FACTOR;
+	in_dev->arp_parms->reachable_time *= ARP_TIMEOUT_FACTOR;
+
+	in_dev_put(in_dev);
+}
diff --git a/distributed-arp-table.h b/distributed-arp-table.h
index cd779f4..c9c6624 100644
--- a/distributed-arp-table.h
+++ b/distributed-arp-table.h
@@ -54,6 +54,7 @@ bool dat_snoop_incoming_arp_reply(struct bat_priv *bat_priv,
 				  struct sk_buff *skb);
 bool arp_drop_broadcast_packet(struct bat_priv *bat_priv,
 			       struct forw_packet *forw_packet);
+void arp_change_timeout(struct net_device *soft_iface, const char *name);
 
 /* hash function to choose an entry in a hash table of given size */
 /* hash algorithm from http://en.wikipedia.org/wiki/Hash_table */
diff --git a/main.h b/main.h
index eed55cb..6cbd5ce 100644
--- a/main.h
+++ b/main.h
@@ -71,6 +71,9 @@
 #define ARP_REQ_DELAY 250
 /* numbers of originator to contact for any STORE/GET DHT operation */
 #define DHT_CANDIDATES_NUM 3
+/* Factor which default ARP timeout values of the soft_iface table are
+ * multiplied by */
+#define ARP_TIMEOUT_FACTOR 4
 
 #define LOG_BUF_LEN 8192	  /* has to be a power of 2 */
 
diff --git a/soft-interface.c b/soft-interface.c
index ab3fb31..7b11aef 100644
--- a/soft-interface.c
+++ b/soft-interface.c
@@ -379,6 +379,8 @@ struct net_device *softif_create(const char *name)
 		goto free_soft_iface;
 	}
 
+	arp_change_timeout(soft_iface, name);
+
 	bat_priv = netdev_priv(soft_iface);
 
 	atomic_set(&bat_priv->aggregated_ogms, 1);
-- 
1.7.3.4


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [B.A.T.M.A.N.] [PATCHv5 7/9] batman-adv: Distributed ARP Table - add compile option
  2012-02-09 23:41 [B.A.T.M.A.N.] [PATCHv5 0/9] DAT: Distributed ARP Table Antonio Quartulli
                   ` (5 preceding siblings ...)
  2012-02-09 23:41 ` [B.A.T.M.A.N.] [PATCHv5 6/9] batman-adv: Distributed ARP Table - increase default soft_iface ARP table timeout Antonio Quartulli
@ 2012-02-09 23:41 ` Antonio Quartulli
  2012-02-10 14:32   ` Marek Lindner
  2012-02-09 23:41 ` [B.A.T.M.A.N.] [PATCHv5 8/9] batman-adv: add UNICAST_4ADDR packet type Antonio Quartulli
                   ` (3 subsequent siblings)
  10 siblings, 1 reply; 27+ messages in thread
From: Antonio Quartulli @ 2012-02-09 23:41 UTC (permalink / raw)
  To: b.a.t.m.a.n

This patch makes it possible to decide whether to include DAT within the
batman-adv binary or not.
It is extremely useful when the user wants to reduce the size of the resulting
module by cutting off any not needed feature.

Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
 Makefile                |    2 ++
 Makefile.kbuild         |    2 +-
 distributed-arp-table.c |    9 +++++++++
 distributed-arp-table.h |   16 +++++++++++++++-
 gen-compat-autoconf.sh  |    1 +
 hard-interface.c        |    2 ++
 originator.c            |    2 ++
 send.c                  |    2 --
 soft-interface.c        |    2 ++
 types.h                 |    4 ++++
 10 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index 08f8c39..a54ffbc 100644
--- a/Makefile
+++ b/Makefile
@@ -23,6 +23,8 @@
 export CONFIG_BATMAN_ADV_DEBUG=n
 # B.A.T.M.A.N. bridge loop avoidance:
 export CONFIG_BATMAN_ADV_BLA=y
+# B.A.T.M.A.N. distributed ARP table:
+export CONFIG_BATMAN_ADV_DAT=n
 
 PWD:=$(shell pwd)
 KERNELPATH ?= /lib/modules/$(shell uname -r)/build
diff --git a/Makefile.kbuild b/Makefile.kbuild
index 84955b3..ad002cd 100644
--- a/Makefile.kbuild
+++ b/Makefile.kbuild
@@ -24,7 +24,7 @@ batman-adv-y += bat_iv_ogm.o
 batman-adv-y += bat_sysfs.o
 batman-adv-y += bitarray.o
 batman-adv-$(CONFIG_BATMAN_ADV_BLA) += bridge_loop_avoidance.o
-batman-adv-y += distributed-arp-table.o
+batman-adv-$(CONFIG_BATMAN_ADV_DAT) += distributed-arp-table.o
 batman-adv-y += gateway_client.o
 batman-adv-y += gateway_common.o
 batman-adv-y += hard-interface.o
diff --git a/distributed-arp-table.c b/distributed-arp-table.c
index 5c81086..48e97e0 100644
--- a/distributed-arp-table.c
+++ b/distributed-arp-table.c
@@ -30,10 +30,13 @@
 #include "hard-interface.h"
 #include "originator.h"
 #include "send.h"
+#include "soft-interface.h"
 #include "types.h"
 #include "translation-table.h"
 #include "unicast.h"
 
+#ifdef CONFIG_BATMAN_ADV_DEBUG
+
 static inline void bat_dbg_arp(struct bat_priv *bat_priv,
 			       struct sk_buff *skb, uint16_t type) {
 	bat_dbg(DBG_ARP, bat_priv, "ARP MSG = [src: %pM-%pI4 dst: %pM-%pI4]\n",
@@ -41,6 +44,12 @@ static inline void bat_dbg_arp(struct bat_priv *bat_priv,
 		&ARP_IP_DST(skb));
 }
 
+#else
+
+#define bat_dbg_arp(...)
+
+#endif /* CONFIG_BATMAN_ADV_DEBUG */
+
 /* Given a key, selects the candidates which the DHT message has to be sent to.
  * An originator O is selected if and only if its DHT_ID value is one of three
  * closest values (but not greater) then the hash value of the key.
diff --git a/distributed-arp-table.h b/distributed-arp-table.h
index c9c6624..98fc2e1 100644
--- a/distributed-arp-table.h
+++ b/distributed-arp-table.h
@@ -22,9 +22,12 @@
 #ifndef _NET_BATMAN_ADV_ARP_H_
 #define _NET_BATMAN_ADV_ARP_H_
 
+#ifdef CONFIG_BATMAN_ADV_DAT
+
 #include "main.h"
 
 #include <linux/if_arp.h>
+#include <linux/netdevice.h>
 
 struct bat_priv;
 struct forw_packet;
@@ -39,7 +42,7 @@ struct forw_packet;
 #define DAT_ADDR_MAX biggest_unsigned_int(dat_addr_t)
 
 #define ARP_HW_SRC(skb) ((uint8_t *)(skb->data) + sizeof(struct ethhdr) + \
-			sizeof(struct arphdr))
+		sizeof(struct arphdr))
 #define ARP_IP_SRC(skb) (*(uint32_t *)(ARP_HW_SRC(skb) + ETH_ALEN))
 #define ARP_HW_DST(skb) (ARP_HW_SRC(skb) + ETH_ALEN + 4)
 #define ARP_IP_DST(skb) (*(uint32_t *)(ARP_HW_SRC(skb) + ETH_ALEN * 2 + 4))
@@ -56,6 +59,17 @@ bool arp_drop_broadcast_packet(struct bat_priv *bat_priv,
 			       struct forw_packet *forw_packet);
 void arp_change_timeout(struct net_device *soft_iface, const char *name);
 
+#else
+
+#define dat_snoop_outgoing_arp_request(...) (0)
+#define dat_snoop_incoming_arp_request(...) (0)
+#define dat_snoop_outgoing_arp_reply(...)
+#define dat_snoop_incoming_arp_reply(...) (0)
+#define arp_drop_broadcast_packet(...) (0)
+#define arp_change_timeout(...)
+
+#endif /* CONFIG_BATMAN_ADV_DAT */
+
 /* hash function to choose an entry in a hash table of given size */
 /* hash algorithm from http://en.wikipedia.org/wiki/Hash_table */
 static inline uint32_t hash_ipv4(const void *data, uint32_t size)
diff --git a/gen-compat-autoconf.sh b/gen-compat-autoconf.sh
index 7cf621b..33de95d 100755
--- a/gen-compat-autoconf.sh
+++ b/gen-compat-autoconf.sh
@@ -38,6 +38,7 @@ gen_config() {
 # write config variables
 gen_config 'CONFIG_BATMAN_ADV_DEBUG' ${CONFIG_BATMAN_ADV_DEBUG:="n"} >> "${TMP}"
 gen_config 'CONFIG_BATMAN_ADV_BLA' ${CONFIG_BATMAN_ADV_BLA:="y"} >> "${TMP}"
+gen_config 'CONFIG_BATMAN_ADV_DAT' ${CONFIG_BATMAN_ADV_DAT:="n"} >> "${TMP}"
 
 # only regenerate compat-autoconf.h when config was changed
 diff "${TMP}" "${TARGET}" > /dev/null 2>&1 || cp "${TMP}" "${TARGET}"
diff --git a/hard-interface.c b/hard-interface.c
index 8cd4a3d..9ad30b8 100644
--- a/hard-interface.c
+++ b/hard-interface.c
@@ -119,9 +119,11 @@ static void primary_if_update_addr(struct bat_priv *bat_priv,
 	if (!primary_if)
 		goto out;
 
+#ifdef CONFIG_BATMAN_ADV_DAT
 	bat_priv->dht_hash = (dat_addr_t)
 				choose_orig(primary_if->net_dev->dev_addr,
 					    DAT_ADDR_MAX);
+#endif
 
 	vis_packet = (struct vis_packet *)
 				bat_priv->my_vis_info->skb_packet->data;
diff --git a/originator.c b/originator.c
index 3cade01..6130913 100644
--- a/originator.c
+++ b/originator.c
@@ -225,7 +225,9 @@ struct orig_node *get_orig_node(struct bat_priv *bat_priv, const uint8_t *addr)
 	orig_node->tt_poss_change = false;
 	orig_node->bat_priv = bat_priv;
 	memcpy(orig_node->orig, addr, ETH_ALEN);
+#ifdef CONFIG_BATMAN_ADV_DAT
 	orig_node->dht_hash = (dat_addr_t)choose_orig(addr, DAT_ADDR_MAX);
+#endif
 	orig_node->router = NULL;
 	orig_node->tt_crc = 0;
 	atomic_set(&orig_node->last_ttvn, 0);
diff --git a/send.c b/send.c
index 666a524..bc45044 100644
--- a/send.c
+++ b/send.c
@@ -30,8 +30,6 @@
 #include "gateway_common.h"
 #include "originator.h"
 
-#include <net/arp.h>
-
 static void send_outstanding_bcast_packet(struct work_struct *work);
 
 /* send out an already prepared packet to the given address via the
diff --git a/soft-interface.c b/soft-interface.c
index 7b11aef..4426e36 100644
--- a/soft-interface.c
+++ b/soft-interface.c
@@ -416,7 +416,9 @@ struct net_device *softif_create(const char *name)
 	if (ret < 0)
 		goto unreg_soft_iface;
 
+#ifdef CONFIG_BATMAN_ADV_DAT
 	bat_priv->dht_hash = 0;
+#endif
 
 	ret = sysfs_add_meshif(soft_iface);
 	if (ret < 0)
diff --git a/types.h b/types.h
index d1da20e..647e873 100644
--- a/types.h
+++ b/types.h
@@ -68,7 +68,9 @@ struct hard_iface {
 struct orig_node {
 	uint8_t orig[ETH_ALEN];
 	uint8_t primary_addr[ETH_ALEN];
+#ifdef CONFIG_BATMAN_ADV_DAT
 	dat_addr_t dht_hash;
+#endif
 	struct neigh_node __rcu *router; /* rcu protected pointer */
 	unsigned long *bcast_own;
 	uint8_t *bcast_own_sum;
@@ -217,7 +219,9 @@ struct bat_priv {
 	struct gw_node __rcu *curr_gw;  /* rcu protected pointer */
 	atomic_t gw_reselect;
 	struct hard_iface __rcu *primary_if;  /* rcu protected pointer */
+#ifdef CONFIG_BATMAN_ADV_DAT
 	dat_addr_t dht_hash;
+#endif
 	struct vis_info *my_vis_info;
 	struct bat_algo_ops *bat_algo_ops;
 };
-- 
1.7.3.4


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [B.A.T.M.A.N.] [PATCHv5 8/9] batman-adv: add UNICAST_4ADDR packet type
  2012-02-09 23:41 [B.A.T.M.A.N.] [PATCHv5 0/9] DAT: Distributed ARP Table Antonio Quartulli
                   ` (6 preceding siblings ...)
  2012-02-09 23:41 ` [B.A.T.M.A.N.] [PATCHv5 7/9] batman-adv: Distributed ARP Table - add compile option Antonio Quartulli
@ 2012-02-09 23:41 ` Antonio Quartulli
  2012-02-10 14:42   ` Marek Lindner
  2012-02-12 14:27   ` Marek Lindner
  2012-02-09 23:41 ` [B.A.T.M.A.N.] [PATCHv5 9/9] batman-adv: Distributed ARP Table - use unicast_4addr_packet for DHT messages Antonio Quartulli
                   ` (2 subsequent siblings)
  10 siblings, 2 replies; 27+ messages in thread
From: Antonio Quartulli @ 2012-02-09 23:41 UTC (permalink / raw)
  To: b.a.t.m.a.n

The current unicast packet type does not contain the orig source address. This
patches add a new unicast packet (called UNICAST_4ADDR) which provide two new
fields: the originator source address and the subtype (the type of the data
contained in the packet payload). The former is useful to identify the node
which injected the packet into the network and the latter is useful to avoid
creating new unicast packet types in the future: a macro defining a new subtype
will be enough.

Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
 hard-interface.c |    1 +
 packet.h         |   29 ++++++++++++----
 routing.c        |   10 ++++-
 unicast.c        |   97 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 unicast.h        |    4 ++
 5 files changed, 131 insertions(+), 10 deletions(-)

diff --git a/hard-interface.c b/hard-interface.c
index 9ad30b8..31dcce0 100644
--- a/hard-interface.c
+++ b/hard-interface.c
@@ -632,6 +632,7 @@ static int batman_skb_recv(struct sk_buff *skb, struct net_device *dev,
 
 		/* unicast packet */
 	case BAT_UNICAST:
+	case BAT_UNICAST_4ADDR:
 		ret = recv_unicast_packet(skb, hard_iface);
 		break;
 
diff --git a/packet.h b/packet.h
index caa12fe..c59256b 100644
--- a/packet.h
+++ b/packet.h
@@ -25,14 +25,21 @@
 #define ETH_P_BATMAN  0x4305	/* unofficial/not registered Ethertype */
 
 enum bat_packettype {
-	BAT_OGM		 = 0x01,
-	BAT_ICMP	 = 0x02,
-	BAT_UNICAST	 = 0x03,
-	BAT_BCAST	 = 0x04,
-	BAT_VIS		 = 0x05,
-	BAT_UNICAST_FRAG = 0x06,
-	BAT_TT_QUERY	 = 0x07,
-	BAT_ROAM_ADV	 = 0x08
+	BAT_OGM		  = 0x01,
+	BAT_ICMP	  = 0x02,
+	BAT_UNICAST	  = 0x03,
+	BAT_BCAST	  = 0x04,
+	BAT_VIS		  = 0x05,
+	BAT_UNICAST_FRAG  = 0x06,
+	BAT_TT_QUERY	  = 0x07,
+	BAT_ROAM_ADV	  = 0x08,
+	BAT_UNICAST_4ADDR = 0x09
+};
+
+enum bat_subtype {
+	BAT_P_DATA       = 0x01,
+	BAT_P_DHT_GET    = 0x02,
+	BAT_P_DHT_PUT    = 0x03
 };
 
 /* this file is included by batctl which needs these defines */
@@ -158,6 +165,12 @@ struct unicast_packet {
 	uint8_t  dest[ETH_ALEN];
 } __packed;
 
+struct unicast_4addr_packet {
+	struct unicast_packet u;
+	uint8_t src[ETH_ALEN];
+	uint8_t subtype;
+} __packed;
+
 struct unicast_frag_packet {
 	struct batman_header header;
 	uint8_t  ttvn; /* destination translation table version number */
diff --git a/routing.c b/routing.c
index 7b7fcbe..e3c9e5b 100644
--- a/routing.c
+++ b/routing.c
@@ -960,14 +960,20 @@ int recv_unicast_packet(struct sk_buff *skb, struct hard_iface *recv_if)
 	struct unicast_packet *unicast_packet;
 	int hdr_size = sizeof(*unicast_packet);
 
+	unicast_packet = (struct unicast_packet *)skb->data;
+
+	/* the caller function should have already pulled 2 bytes */
+	if (unicast_packet->header.packet_type == BAT_UNICAST)
+		hdr_size = sizeof(struct unicast_packet);
+	else if (unicast_packet->header.packet_type == BAT_UNICAST_4ADDR)
+		hdr_size = sizeof(struct unicast_4addr_packet);
+
 	if (check_unicast_packet(skb, hdr_size) < 0)
 		return NET_RX_DROP;
 
 	if (!check_unicast_ttvn(bat_priv, skb))
 		return NET_RX_DROP;
 
-	unicast_packet = (struct unicast_packet *)skb->data;
-
 	/* packet for me */
 	if (is_my_mac(unicast_packet->dest)) {
 		interface_rx(recv_if->soft_iface, skb, recv_if, hdr_size);
diff --git a/unicast.c b/unicast.c
index 887342b..feaf195 100644
--- a/unicast.c
+++ b/unicast.c
@@ -306,6 +306,42 @@ bool prepare_unicast_packet(struct sk_buff *skb, struct orig_node *orig_node)
 	return true;
 }
 
+bool prepare_unicast_4addr_packet(struct bat_priv *bat_priv,
+				  struct sk_buff *skb,
+				  struct orig_node *orig_node)
+{
+	struct hard_iface *primary_if;
+	struct unicast_4addr_packet *unicast_4addr_packet;
+	bool ret = false;
+
+	primary_if = primary_if_get_selected(bat_priv);
+	if (!primary_if)
+		goto out;
+
+	if (my_skb_head_push(skb, sizeof(*unicast_4addr_packet)) < 0)
+		goto out;
+
+	unicast_4addr_packet = (struct unicast_4addr_packet *)skb->data;
+
+	unicast_4addr_packet->u.header.version = COMPAT_VERSION;
+	/* batman packet type: unicast */
+	unicast_4addr_packet->u.header.packet_type = BAT_UNICAST_4ADDR;
+	/* set unicast ttl */
+	unicast_4addr_packet->u.header.ttl = TTL;
+	/* copy the destination for faster routing */
+	memcpy(unicast_4addr_packet->u.dest, orig_node->orig, ETH_ALEN);
+	/* set the destination tt version number */
+	unicast_4addr_packet->u.ttvn =
+		(uint8_t)atomic_read(&orig_node->last_ttvn);
+	memcpy(unicast_4addr_packet->src, primary_if->net_dev->dev_addr,
+	       ETH_ALEN);
+	ret = true;
+out:
+	if (primary_if)
+		hardif_free_ref(primary_if);
+	return ret;
+}
+
 int unicast_send_skb(struct sk_buff *skb, struct bat_priv *bat_priv)
 {
 	struct ethhdr *ethhdr = (struct ethhdr *)skb->data;
@@ -366,3 +402,64 @@ out:
 		kfree_skb(skb);
 	return ret;
 }
+
+int unicast_4addr_send_skb(struct sk_buff *skb, struct bat_priv *bat_priv)
+{
+	struct ethhdr *ethhdr = (struct ethhdr *)skb->data;
+	struct unicast_4addr_packet *unicast_4addr_packet;
+	struct orig_node *orig_node;
+	struct neigh_node *neigh_node;
+	int data_len = skb->len;
+	int ret = 1;
+
+	/* get routing information */
+	if (is_multicast_ether_addr(ethhdr->h_dest)) {
+		orig_node = gw_get_selected_orig(bat_priv);
+		if (orig_node)
+			goto find_router;
+	}
+
+	/* check for tt host - increases orig_node refcount.
+	 * returns NULL in case of AP isolation */
+	orig_node = transtable_search(bat_priv, ethhdr->h_source,
+				      ethhdr->h_dest);
+
+find_router:
+	/**
+	 * find_router():
+	 *  - if orig_node is NULL it returns NULL
+	 *  - increases neigh_nodes refcount if found.
+	 */
+	neigh_node = find_router(bat_priv, orig_node, NULL);
+
+	if (!neigh_node)
+		goto out;
+
+	if (!prepare_unicast_4addr_packet(bat_priv, skb, orig_node))
+		goto out;
+
+	unicast_4addr_packet = (struct unicast_4addr_packet *)skb->data;
+
+	if (atomic_read(&bat_priv->fragmentation) &&
+	    data_len + sizeof(*unicast_4addr_packet) >
+				neigh_node->if_incoming->net_dev->mtu) {
+		/* send frag skb decreases ttl */
+		unicast_4addr_packet->u.header.ttl++;
+		ret = frag_send_skb(skb, bat_priv,
+				    neigh_node->if_incoming, neigh_node->addr);
+		goto out;
+	}
+
+	send_skb_packet(skb, neigh_node->if_incoming, neigh_node->addr);
+	ret = 0;
+	goto out;
+
+out:
+	if (neigh_node)
+		neigh_node_free_ref(neigh_node);
+	if (orig_node)
+		orig_node_free_ref(orig_node);
+	if (ret == 1)
+		kfree_skb(skb);
+	return ret;
+}
diff --git a/unicast.h b/unicast.h
index c51ad3d..1a7bf0a 100644
--- a/unicast.h
+++ b/unicast.h
@@ -31,9 +31,13 @@ int frag_reassemble_skb(struct sk_buff *skb, struct bat_priv *bat_priv,
 			struct sk_buff **new_skb);
 void frag_list_free(struct list_head *head);
 int unicast_send_skb(struct sk_buff *skb, struct bat_priv *bat_priv);
+int unicast_4addr_send_skb(struct sk_buff *skb, struct bat_priv *bat_priv);
 int frag_send_skb(struct sk_buff *skb, struct bat_priv *bat_priv,
 		  struct hard_iface *hard_iface, const uint8_t dstaddr[]);
 bool prepare_unicast_packet(struct sk_buff *skb, struct orig_node *orig_node);
+bool prepare_unicast_4addr_packet(struct bat_priv *bat_priv,
+				  struct sk_buff *skb,
+				  struct orig_node *orig_node);
 
 static inline int frag_can_reassemble(const struct sk_buff *skb, int mtu)
 {
-- 
1.7.3.4


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [B.A.T.M.A.N.] [PATCHv5 9/9] batman-adv: Distributed ARP Table - use unicast_4addr_packet for DHT messages
  2012-02-09 23:41 [B.A.T.M.A.N.] [PATCHv5 0/9] DAT: Distributed ARP Table Antonio Quartulli
                   ` (7 preceding siblings ...)
  2012-02-09 23:41 ` [B.A.T.M.A.N.] [PATCHv5 8/9] batman-adv: add UNICAST_4ADDR packet type Antonio Quartulli
@ 2012-02-09 23:41 ` Antonio Quartulli
  2012-02-11 13:44   ` Marek Lindner
  2012-02-09 23:52 ` [B.A.T.M.A.N.] [PATCHv5 0/9] DAT: Distributed ARP Table Antonio Quartulli
  2012-02-10  9:44 ` Marek Lindner
  10 siblings, 1 reply; 27+ messages in thread
From: Antonio Quartulli @ 2012-02-09 23:41 UTC (permalink / raw)
  To: b.a.t.m.a.n

In order to enable an higher verbosity level of the DAT debug messages, the
unicast_4addr_packet is now used to carry ARP packets generated by the DAT
internal mechanism. This packet type will enable batman-adv to recognise each
DAT related message and to print its source (this will help to track possibly
bogus ARP entries)

Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
 distributed-arp-table.c |  129 +++++++++++++++++++++++++++++++---------------
 distributed-arp-table.h |   16 +++---
 soft-interface.c        |   15 +++--
 3 files changed, 105 insertions(+), 55 deletions(-)

diff --git a/distributed-arp-table.c b/distributed-arp-table.c
index 48e97e0..239b5c4 100644
--- a/distributed-arp-table.c
+++ b/distributed-arp-table.c
@@ -31,6 +31,7 @@
 #include "originator.h"
 #include "send.h"
 #include "soft-interface.h"
+#include "translation-table.h"
 #include "types.h"
 #include "translation-table.h"
 #include "unicast.h"
@@ -38,10 +39,44 @@
 #ifdef CONFIG_BATMAN_ADV_DEBUG
 
 static inline void bat_dbg_arp(struct bat_priv *bat_priv,
-			       struct sk_buff *skb, uint16_t type) {
+			       struct sk_buff *skb, uint16_t type,
+			       int hdr_size) {
+	struct unicast_4addr_packet *unicast_4addr_packet;
+
 	bat_dbg(DBG_ARP, bat_priv, "ARP MSG = [src: %pM-%pI4 dst: %pM-%pI4]\n",
-		ARP_HW_SRC(skb), &ARP_IP_SRC(skb), ARP_HW_DST(skb),
-		&ARP_IP_DST(skb));
+		ARP_HW_SRC(skb, hdr_size), &ARP_IP_SRC(skb, hdr_size),
+		ARP_HW_DST(skb, hdr_size), &ARP_IP_DST(skb, hdr_size));
+
+	if (hdr_size == 0)
+		return;
+
+	/* if the AP packet is encapsulated in a batman packet, let's print some
+	 * debug messages */
+	unicast_4addr_packet = (struct unicast_4addr_packet *)skb->data;
+
+	switch (unicast_4addr_packet->u.header.packet_type) {
+	case BAT_UNICAST:
+		bat_dbg(DBG_ARP, bat_priv, "encapsulated within a UNICAST "
+			"packet\n");
+		break;
+	case BAT_UNICAST_4ADDR:
+		bat_dbg(DBG_ARP, bat_priv, "encapsulated within a "
+			"UNICAST_4ADDR packet (src: %pM)\n",
+			unicast_4addr_packet->src);
+		if (unicast_4addr_packet->subtype != BAT_P_DHT_PUT ||
+		    unicast_4addr_packet->subtype != BAT_P_DHT_GET)
+			bat_dbg(DBG_ARP, bat_priv, "It's a DAT message\n");
+		break;
+	case BAT_BCAST:
+		bat_dbg(DBG_ARP, bat_priv, "encapsulated within a BCAST packet "
+			"(src: %pM)\n",
+			((struct bcast_packet *)unicast_4addr_packet)->orig);
+		break;
+	default:
+		bat_dbg(DBG_ARP, bat_priv, "encapsulated within an unknown "
+			"packet type (0x%x)\n",
+			unicast_4addr_packet->u.header.packet_type);
+	}
 }
 
 #else
@@ -184,7 +219,8 @@ static bool dht_send_data(struct bat_priv *bat_priv, struct sk_buff *skb,
 			goto free_orig;
 
 		tmp_skb = pskb_copy(skb, GFP_ATOMIC);
-		if (prepare_unicast_packet(tmp_skb, cand[i].orig_node))
+		if (prepare_unicast_4addr_packet(bat_priv, tmp_skb,
+						 cand[i].orig_node))
 			send_skb_packet(tmp_skb, neigh_node->if_incoming,
 					neigh_node->addr);
 		else
@@ -228,24 +264,28 @@ out:
 
 /* Returns arphdr->ar_op if the skb contains a valid ARP packet, otherwise
  * returns 0 */
-static uint16_t arp_get_type(struct bat_priv *bat_priv, struct sk_buff *skb)
+static uint16_t arp_get_type(struct bat_priv *bat_priv, struct sk_buff *skb,
+			     int hdr_size)
 {
 	struct arphdr *arphdr;
 	struct ethhdr *ethhdr;
 	uint16_t type = 0;
 
-	if (unlikely(!pskb_may_pull(skb, ETH_HLEN)))
+	/* pull the ethernet header */
+	if (unlikely(!pskb_may_pull(skb, hdr_size + ETH_HLEN)))
 		goto out;
 
-	ethhdr = (struct ethhdr *)skb_mac_header(skb);
+	ethhdr = (struct ethhdr *)(skb->data + hdr_size);
 
 	if (ethhdr->h_proto != htons(ETH_P_ARP))
 		goto out;
 
-	if (unlikely(!pskb_may_pull(skb, ETH_HLEN + arp_hdr_len(skb->dev))))
+	/* pull the ARP payload */
+	if (unlikely(!pskb_may_pull(skb, hdr_size + ETH_HLEN +
+				    arp_hdr_len(skb->dev))))
 		goto out;
 
-	arphdr = (struct arphdr *)(skb->data + sizeof(struct ethhdr));
+	arphdr = (struct arphdr *)(skb->data + hdr_size + ETH_HLEN);
 
 	/* Check whether the ARP packet carries a valid
 	 * IP information */
@@ -263,10 +303,10 @@ static uint16_t arp_get_type(struct bat_priv *bat_priv, struct sk_buff *skb)
 
 	/* Check for bad reply/request. If the ARP message is not sane, DAT
 	 * will simply ignore it */
-	if (ipv4_is_loopback(ARP_IP_SRC(skb)) ||
-	    ipv4_is_multicast(ARP_IP_SRC(skb)) ||
-	    ipv4_is_loopback(ARP_IP_DST(skb)) ||
-	    ipv4_is_multicast(ARP_IP_DST(skb)))
+	if (ipv4_is_loopback(ARP_IP_SRC(skb, hdr_size)) ||
+	    ipv4_is_multicast(ARP_IP_SRC(skb, hdr_size)) ||
+	    ipv4_is_loopback(ARP_IP_DST(skb, hdr_size)) ||
+	    ipv4_is_multicast(ARP_IP_DST(skb, hdr_size)))
 		goto out;
 
 	type = ntohs(arphdr->ar_op);
@@ -288,18 +328,18 @@ bool dat_snoop_outgoing_arp_request(struct bat_priv *bat_priv,
 	struct hard_iface *primary_if = NULL;
 	struct sk_buff *skb_new;
 
-	type = arp_get_type(bat_priv, skb);
+	type = arp_get_type(bat_priv, skb, 0);
 	/* If we get an ARP_REQUEST we have to send the unicast message to the
 	 * selected DHT candidates */
 	if (type != ARPOP_REQUEST)
 		goto out;
 
 	bat_dbg(DBG_ARP, bat_priv, "Parsing outgoing ARP REQUEST\n");
-	bat_dbg_arp(bat_priv, skb, type);
+	bat_dbg_arp(bat_priv, skb, type, 0);
 
-	ip_src = ARP_IP_SRC(skb);
-	hw_src = ARP_HW_SRC(skb);
-	ip_dst = ARP_IP_DST(skb);
+	ip_src = ARP_IP_SRC(skb, 0);
+	hw_src = ARP_HW_SRC(skb, 0);
+	ip_dst = ARP_IP_DST(skb, 0);
 
 	primary_if = primary_if_get_selected(bat_priv);
 	if (!primary_if)
@@ -341,7 +381,7 @@ out:
  * into the local table. If found, an ARP reply is sent immediately, otherwise
  * the caller has to deliver the ARP request to the upper layer */
 bool dat_snoop_incoming_arp_request(struct bat_priv *bat_priv,
-				    struct sk_buff *skb)
+				    struct sk_buff *skb, int hdr_size)
 {
 	uint16_t type;
 	uint32_t ip_src, ip_dst;
@@ -351,16 +391,16 @@ bool dat_snoop_incoming_arp_request(struct bat_priv *bat_priv,
 	struct neighbour *n = NULL;
 	bool ret = false;
 
-	type = arp_get_type(bat_priv, skb);
+	type = arp_get_type(bat_priv, skb, hdr_size);
 	if (type != ARPOP_REQUEST)
 		goto out;
 
-	hw_src = ARP_HW_SRC(skb);
-	ip_src = ARP_IP_SRC(skb);
-	ip_dst = ARP_IP_DST(skb);
+	hw_src = ARP_HW_SRC(skb, hdr_size);
+	ip_src = ARP_IP_SRC(skb, hdr_size);
+	ip_dst = ARP_IP_DST(skb, hdr_size);
 
 	bat_dbg(DBG_ARP, bat_priv, "Parsing incoming ARP REQUEST\n");
-	bat_dbg_arp(bat_priv, skb, type);
+	bat_dbg_arp(bat_priv, skb, type, hdr_size);
 
 	primary_if = primary_if_get_selected(bat_priv);
 	if (!primary_if)
@@ -380,7 +420,7 @@ bool dat_snoop_incoming_arp_request(struct bat_priv *bat_priv,
 	if (!skb_new)
 		goto out;
 
-	unicast_send_skb(skb_new, bat_priv);
+	unicast_4addr_send_skb(skb_new, bat_priv);
 
 	ret = true;
 out:
@@ -404,17 +444,17 @@ bool dat_snoop_outgoing_arp_reply(struct bat_priv *bat_priv,
 	uint8_t *hw_src, *hw_dst;
 	bool ret = false;
 
-	type = arp_get_type(bat_priv, skb);
+	type = arp_get_type(bat_priv, skb, 0);
 	if (type != ARPOP_REPLY)
 		goto out;
 
 	bat_dbg(DBG_ARP, bat_priv, "Parsing outgoing ARP REPLY\n");
-	bat_dbg_arp(bat_priv, skb, type);
+	bat_dbg_arp(bat_priv, skb, type, 0);
 
-	hw_src = ARP_HW_SRC(skb);
-	ip_src = ARP_IP_SRC(skb);
-	hw_dst = ARP_HW_DST(skb);
-	ip_dst = ARP_IP_DST(skb);
+	hw_src = ARP_HW_SRC(skb, 0);
+	ip_src = ARP_IP_SRC(skb, 0);
+	hw_dst = ARP_HW_DST(skb, 0);
+	ip_dst = ARP_IP_DST(skb, 0);
 
 	arp_neigh_update(bat_priv, ip_src, hw_src);
 	arp_neigh_update(bat_priv, ip_dst, hw_dst);
@@ -431,24 +471,24 @@ out:
 /* This function has to be invoked on an ARP reply coming into the soft
  * interface from the mesh network. The local table has to be updated */
 bool dat_snoop_incoming_arp_reply(struct bat_priv *bat_priv,
-				  struct sk_buff *skb)
+				  struct sk_buff *skb, int hdr_size)
 {
 	uint16_t type;
 	uint32_t ip_src, ip_dst;
 	uint8_t *hw_src, *hw_dst;
 	bool ret = false;
 
-	type = arp_get_type(bat_priv, skb);
+	type = arp_get_type(bat_priv, skb, hdr_size);
 	if (type != ARPOP_REPLY)
 		goto out;
 
 	bat_dbg(DBG_ARP, bat_priv, "Parsing incoming ARP REPLY\n");
-	bat_dbg_arp(bat_priv, skb, type);
+	bat_dbg_arp(bat_priv, skb, type, hdr_size);
 
-	hw_src = ARP_HW_SRC(skb);
-	ip_src = ARP_IP_SRC(skb);
-	hw_dst = ARP_HW_DST(skb);
-	ip_dst = ARP_IP_DST(skb);
+	hw_src = ARP_HW_SRC(skb, hdr_size);
+	ip_src = ARP_IP_SRC(skb, hdr_size);
+	hw_dst = ARP_HW_DST(skb, hdr_size);
+	ip_dst = ARP_IP_DST(skb, hdr_size);
 
 	/* Update our internal cache with both the IP addresses we fetched from
 	 * the ARP reply */
@@ -471,20 +511,25 @@ bool arp_drop_broadcast_packet(struct bat_priv *bat_priv,
 	/* If this packet is an ARP_REQUEST and we already have the information
 	 * that it is going to ask, we can drop the packet */
 	if (!forw_packet->num_packets &&
-			(arp_get_type(bat_priv, forw_packet->skb) ==
+			(arp_get_type(bat_priv, forw_packet->skb,
+				      sizeof(struct bcast_packet)) ==
 							ARPOP_REQUEST)) {
-		n = neigh_lookup(&arp_tbl, &ARP_IP_DST(forw_packet->skb),
+		n = neigh_lookup(&arp_tbl,
+				 &ARP_IP_DST(forw_packet->skb,
+					     sizeof(struct bcast_packet)),
 				 forw_packet->if_incoming->soft_iface);
 		/* check if we already know this neigh */
 		if (n && (n->nud_state & NUD_CONNECTED)) {
 			bat_dbg(DBG_ARP, bat_priv, "ARP Request for %pI4: "
 				"fallback prevented\n",
-				&ARP_IP_DST(forw_packet->skb));
+				&ARP_IP_DST(forw_packet->skb,
+					    sizeof(struct bcast_packet)));
 			return true;
 		}
 
 		bat_dbg(DBG_ARP, bat_priv, "ARP Request for %pI4: fallback\n",
-			&ARP_IP_DST(forw_packet->skb));
+			&ARP_IP_DST(forw_packet->skb,
+				    sizeof(struct bcast_packet)));
 	}
 	return false;
 }
diff --git a/distributed-arp-table.h b/distributed-arp-table.h
index 98fc2e1..a81b021 100644
--- a/distributed-arp-table.h
+++ b/distributed-arp-table.h
@@ -41,20 +41,22 @@ struct forw_packet;
 #define dat_addr_t uint16_t
 #define DAT_ADDR_MAX biggest_unsigned_int(dat_addr_t)
 
-#define ARP_HW_SRC(skb) ((uint8_t *)(skb->data) + sizeof(struct ethhdr) + \
-		sizeof(struct arphdr))
-#define ARP_IP_SRC(skb) (*(uint32_t *)(ARP_HW_SRC(skb) + ETH_ALEN))
-#define ARP_HW_DST(skb) (ARP_HW_SRC(skb) + ETH_ALEN + 4)
-#define ARP_IP_DST(skb) (*(uint32_t *)(ARP_HW_SRC(skb) + ETH_ALEN * 2 + 4))
+#define ARP_HW_SRC(skb, hdr_size) ((uint8_t *)(skb->data + hdr_size) + \
+				sizeof(struct ethhdr) +	sizeof(struct arphdr))
+#define ARP_IP_SRC(skb, hdr_size) (*(uint32_t *)(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) + \
+				ETH_ALEN * 2 + 4))
 
 bool dat_snoop_outgoing_arp_request(struct bat_priv *bat_priv,
 				    struct sk_buff *skb);
 bool dat_snoop_incoming_arp_request(struct bat_priv *bat_priv,
-				    struct sk_buff *skb);
+				    struct sk_buff *skb, int hdr_size);
 bool dat_snoop_outgoing_arp_reply(struct bat_priv *bat_priv,
 				  struct sk_buff *skb);
 bool dat_snoop_incoming_arp_reply(struct bat_priv *bat_priv,
-				  struct sk_buff *skb);
+				  struct sk_buff *skb, int hdr_size);
 bool arp_drop_broadcast_packet(struct bat_priv *bat_priv,
 			       struct forw_packet *forw_packet);
 void arp_change_timeout(struct net_device *soft_iface, const char *name);
diff --git a/soft-interface.c b/soft-interface.c
index 4426e36..b21171b 100644
--- a/soft-interface.c
+++ b/soft-interface.c
@@ -258,6 +258,7 @@ void interface_rx(struct net_device *soft_iface,
 		  int hdr_size)
 {
 	struct bat_priv *bat_priv = netdev_priv(soft_iface);
+	struct unicast_4addr_packet *unicast_4addr_packet;
 	struct ethhdr *ethhdr;
 	struct vlan_ethhdr *vhdr;
 	short vid = -1;
@@ -266,6 +267,14 @@ void interface_rx(struct net_device *soft_iface,
 	if (!pskb_may_pull(skb, hdr_size))
 		goto dropped;
 
+	unicast_4addr_packet = (struct unicast_4addr_packet *)skb->data;
+
+	if (dat_snoop_incoming_arp_request(bat_priv, skb, hdr_size))
+		goto out;
+
+	if (dat_snoop_incoming_arp_reply(bat_priv, skb, hdr_size))
+		goto out;
+
 	skb_pull_rcsum(skb, hdr_size);
 	skb_reset_mac_header(skb);
 
@@ -284,12 +293,6 @@ void interface_rx(struct net_device *soft_iface,
 		goto dropped;
 	}
 
-	if (dat_snoop_incoming_arp_request(bat_priv, skb))
-		goto out;
-
-	if (dat_snoop_incoming_arp_reply(bat_priv, skb))
-		goto out;
-
 	/* skb->dev & skb->pkt_type are set here */
 	if (unlikely(!pskb_may_pull(skb, ETH_HLEN)))
 		goto dropped;
-- 
1.7.3.4


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCHv5 0/9] DAT: Distributed ARP Table
  2012-02-09 23:41 [B.A.T.M.A.N.] [PATCHv5 0/9] DAT: Distributed ARP Table Antonio Quartulli
                   ` (8 preceding siblings ...)
  2012-02-09 23:41 ` [B.A.T.M.A.N.] [PATCHv5 9/9] batman-adv: Distributed ARP Table - use unicast_4addr_packet for DHT messages Antonio Quartulli
@ 2012-02-09 23:52 ` Antonio Quartulli
  2012-02-10  9:44 ` Marek Lindner
  10 siblings, 0 replies; 27+ messages in thread
From: Antonio Quartulli @ 2012-02-09 23:52 UTC (permalink / raw)
  To: b.a.t.m.a.n

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

On Fri, Feb 10, 2012 at 12:41:33AM +0100, Antonio Quartulli wrote:
> Hello people,
> 
> ****
> This is the **fifth** version of this patchset. The whole code has been reviewed
> after some discussions on irc and here in the ml in the previous threads.
> Several bugs have been spotted and fixed. Thanks Marek, Sven, Simon, Andrew and
> all the others for the comments/feedbacks.
> 
> This patchset assumes that patch
> [PATCH] batman-adv: add biggest_unsigned_int(x) macro
> has already been applied
> ****
> 
> For details about the DAT mechanism, please refer to the preliminary
> documentation that has already been put on the wiki[1]. A more exhaustive and
> detailed documentation is going to come soon.
> 
> 
> 
> PatchsetV5 Additions:
> ------------------------
> 
> - Sanity check for ARP messages (messages containing multicast/loopback
>   addresses are ignored)
> - A new unicast packet type has been introduced (UNICAST_4ADDR), which contains
>   the originator address and the payload type.
> - Thanks to this new packet type, more and more debugging messages have been
>   added. In this way it is easier to track the behaviour of DAT and it is possible to
>   understand who and why added a given entry in the soft_iface ARP table.

[1] http://www.open-mesh.org/wiki/batman-adv/GSOC2011_DAT

-- 
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] 27+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCHv5 0/9] DAT: Distributed ARP Table
  2012-02-09 23:41 [B.A.T.M.A.N.] [PATCHv5 0/9] DAT: Distributed ARP Table Antonio Quartulli
                   ` (9 preceding siblings ...)
  2012-02-09 23:52 ` [B.A.T.M.A.N.] [PATCHv5 0/9] DAT: Distributed ARP Table Antonio Quartulli
@ 2012-02-10  9:44 ` Marek Lindner
  10 siblings, 0 replies; 27+ messages in thread
From: Marek Lindner @ 2012-02-10  9:44 UTC (permalink / raw)
  To: The list for a Better Approach To Mobile Ad-hoc Networking

On Friday, February 10, 2012 07:41:33 Antonio Quartulli wrote:
> ****
> This is the **fifth** version of this patchset. The whole code has been
> reviewed after some discussions on irc and here in the ml in the previous
> threads. Several bugs have been spotted and fixed. Thanks Marek, Sven,
> Simon, Andrew and all the others for the comments/feedbacks.
> 
> This patchset assumes that patch
> [PATCH] batman-adv: add biggest_unsigned_int(x) macro
> has already been applied
> ****

Please make sure your patchset is checkpatch clean.

Thanks,
Marek

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCHv5 4/9] batman-adv: Distributed ARP Table - add ARP parsing functions
  2012-02-09 23:41 ` [B.A.T.M.A.N.] [PATCHv5 4/9] batman-adv: Distributed ARP Table - add ARP parsing functions Antonio Quartulli
@ 2012-02-10 14:21   ` Marek Lindner
  2012-02-11 14:09   ` Marek Lindner
  1 sibling, 0 replies; 27+ messages in thread
From: Marek Lindner @ 2012-02-10 14:21 UTC (permalink / raw)
  To: The list for a Better Approach To Mobile Ad-hoc Networking

On Friday, February 10, 2012 07:41:37 Antonio Quartulli wrote:
> +#define ARP_HW_SRC(skb) ((uint8_t *)(skb->data) + sizeof(struct ethhdr) +
> \ +                       sizeof(struct arphdr))

You could replace all occurences of "sizeof(struct ethhdr)" with ETH_HLEN.

Cheers,
Marek

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCHv5 5/9] batman-adv: Distributed ARP Table - add snooping functions for ARP messages
  2012-02-09 23:41 ` [B.A.T.M.A.N.] [PATCHv5 5/9] batman-adv: Distributed ARP Table - add snooping functions for ARP messages Antonio Quartulli
@ 2012-02-10 14:25   ` Marek Lindner
  0 siblings, 0 replies; 27+ messages in thread
From: Marek Lindner @ 2012-02-10 14:25 UTC (permalink / raw)
  To: The list for a Better Approach To Mobile Ad-hoc Networking

On Friday, February 10, 2012 07:41:38 Antonio Quartulli wrote:
>  static inline void bat_dbg_arp(struct bat_priv *bat_priv,
>                                struct sk_buff *skb, uint16_t type) {
> -       char buf[30];
> -       const char *type_str[] = { "REQUEST", "REPLY", "RREQUEST",
> "RREPLY", -                                     "InREQUEST", "InREPLY",
> "NAK" }; -
> -       if (type >= 1 && type <= ARRAY_SIZE(type_str))
> -               scnprintf(buf, sizeof(buf), "%s", type_str[type - 1]);
> -       else
> -               scnprintf(buf, sizeof(buf), "UNKNOWN (%hu)", type);
> -
> -       bat_dbg(DBG_ARP, bat_priv, "ARP message of type %s recognised "
> -               "[src: %pM-%pI4 dst: %pM-%pI4]\n", buf, ARP_HW_SRC(skb),
> -               &ARP_IP_SRC(skb), ARP_HW_DST(skb), &ARP_IP_DST(skb));
> +       bat_dbg(DBG_ARP, bat_priv, "ARP MSG = [src: %pM-%pI4 dst:
> %pM-%pI4]\n", +               ARP_HW_SRC(skb), &ARP_IP_SRC(skb),
> ARP_HW_DST(skb), +               &ARP_IP_DST(skb));
>  }

Removing something that was just added in the previous patch is bad style and 
unlikely to be accepted.

Cheers,
Marek

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCHv5 7/9] batman-adv: Distributed ARP Table - add compile option
  2012-02-09 23:41 ` [B.A.T.M.A.N.] [PATCHv5 7/9] batman-adv: Distributed ARP Table - add compile option Antonio Quartulli
@ 2012-02-10 14:32   ` Marek Lindner
  2012-02-15 19:47     ` Antonio Quartulli
  0 siblings, 1 reply; 27+ messages in thread
From: Marek Lindner @ 2012-02-10 14:32 UTC (permalink / raw)
  To: The list for a Better Approach To Mobile Ad-hoc Networking

On Friday, February 10, 2012 07:41:40 Antonio Quartulli wrote:
> @@ -23,6 +23,8 @@
>  export CONFIG_BATMAN_ADV_DEBUG=n
>  # B.A.T.M.A.N. bridge loop avoidance:
>  export CONFIG_BATMAN_ADV_BLA=y
> +# B.A.T.M.A.N. distributed ARP table:
> +export CONFIG_BATMAN_ADV_DAT=n

Any particular reason why you wish to disable it by default ?

> --- a/distributed-arp-table.c
> +++ b/distributed-arp-table.c
> @@ -30,10 +30,13 @@
>  #include "hard-interface.h"
>  #include "originator.h"
>  #include "send.h"
> +#include "soft-interface.h"
>  #include "types.h"
>  #include "translation-table.h"
>  #include "unicast.h"

Why changing an include in this patch ?


> --- a/distributed-arp-table.h
> +++ b/distributed-arp-table.h
> @@ -22,9 +22,12 @@
>  #ifndef _NET_BATMAN_ADV_ARP_H_
>  #define _NET_BATMAN_ADV_ARP_H_
> 
> +#ifdef CONFIG_BATMAN_ADV_DAT
> +
>  #include "main.h"
> 
>  #include <linux/if_arp.h>
> +#include <linux/netdevice.h>

Another include change ?


> --- a/hard-interface.c
> +++ b/hard-interface.c
> @@ -119,9 +119,11 @@ static void primary_if_update_addr(struct bat_priv
> *bat_priv, if (!primary_if)
>  		goto out;
> 
> +#ifdef CONFIG_BATMAN_ADV_DAT
>  	bat_priv->dht_hash = (dat_addr_t)
>  				choose_orig(primary_if->net_dev->dev_addr,
>  					    DAT_ADDR_MAX);
> +#endif

A general dat_init()/dat_free() structure would be better than adding defines 
everywhere.


> --- a/send.c
> +++ b/send.c
> @@ -30,8 +30,6 @@
>  #include "gateway_common.h"
>  #include "originator.h"
> 
> -#include <net/arp.h>
> -
>  static void send_outstanding_bcast_packet(struct work_struct *work);

More changing includes ..

The README update is missing in this patch.

Cheers,
Marek

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCHv5 8/9] batman-adv: add UNICAST_4ADDR packet type
  2012-02-09 23:41 ` [B.A.T.M.A.N.] [PATCHv5 8/9] batman-adv: add UNICAST_4ADDR packet type Antonio Quartulli
@ 2012-02-10 14:42   ` Marek Lindner
  2012-02-12 14:27   ` Marek Lindner
  1 sibling, 0 replies; 27+ messages in thread
From: Marek Lindner @ 2012-02-10 14:42 UTC (permalink / raw)
  To: The list for a Better Approach To Mobile Ad-hoc Networking

On Friday, February 10, 2012 07:41:41 Antonio Quartulli wrote:
> --- a/routing.c
> +++ b/routing.c
> @@ -960,14 +960,20 @@ int recv_unicast_packet(struct sk_buff *skb, struct
> hard_iface *recv_if) struct unicast_packet *unicast_packet;
>  	int hdr_size = sizeof(*unicast_packet);
> 
> +	unicast_packet = (struct unicast_packet *)skb->data;
> +
> +	/* the caller function should have already pulled 2 bytes */
> +	if (unicast_packet->header.packet_type == BAT_UNICAST)
> +		hdr_size = sizeof(struct unicast_packet);
> +	else if (unicast_packet->header.packet_type == BAT_UNICAST_4ADDR)
> +		hdr_size = sizeof(struct unicast_4addr_packet);
> +

The first if statement should not be necessary given the default value of 
hdr_size. We also should stick to the general principle of using 
*variable_name instead of sizeof(). That was suggested in some of the kernel 
coding guidelines.


> +bool prepare_unicast_4addr_packet(struct bat_priv *bat_priv,
> +				  struct sk_buff *skb,
> +				  struct orig_node *orig_node)
> +{
> +	struct hard_iface *primary_if;
> +	struct unicast_4addr_packet *unicast_4addr_packet;
> +	bool ret = false;
> +
> +	primary_if = primary_if_get_selected(bat_priv);
> +	if (!primary_if)
> +		goto out;
> +
> +	if (my_skb_head_push(skb, sizeof(*unicast_4addr_packet)) < 0)
> +		goto out;
> +
> +	unicast_4addr_packet = (struct unicast_4addr_packet *)skb->data;
> +
> +	unicast_4addr_packet->u.header.version = COMPAT_VERSION;
> +	/* batman packet type: unicast */
> +	unicast_4addr_packet->u.header.packet_type = BAT_UNICAST_4ADDR;
> +	/* set unicast ttl */
> +	unicast_4addr_packet->u.header.ttl = TTL;
> +	/* copy the destination for faster routing */
> +	memcpy(unicast_4addr_packet->u.dest, orig_node->orig, ETH_ALEN);
> +	/* set the destination tt version number */
> +	unicast_4addr_packet->u.ttvn =
> +		(uint8_t)atomic_read(&orig_node->last_ttvn);
> +	memcpy(unicast_4addr_packet->src, primary_if->net_dev->dev_addr,
> +	       ETH_ALEN);
> +	ret = true;
> +out:
> +	if (primary_if)
> +		hardif_free_ref(primary_if);
> +	return ret;
> +}

This function looks like code duplication we coudl avoid. 
prepare_unicast_packet() does the same thing except for 2-3 fields ..


> +
> +int unicast_4addr_send_skb(struct sk_buff *skb, struct bat_priv *bat_priv)
> +{
> +	struct ethhdr *ethhdr = (struct ethhdr *)skb->data;
> +	struct unicast_4addr_packet *unicast_4addr_packet;
> +	struct orig_node *orig_node;
> +	struct neigh_node *neigh_node;
> +	int data_len = skb->len;
> +	int ret = 1;
> +
> +	/* get routing information */
> +	if (is_multicast_ether_addr(ethhdr->h_dest)) {
> +		orig_node = gw_get_selected_orig(bat_priv);
> +		if (orig_node)
> +			goto find_router;
> +	}
> +
> +	/* check for tt host - increases orig_node refcount.
> +	 * returns NULL in case of AP isolation */
> +	orig_node = transtable_search(bat_priv, ethhdr->h_source,
> +				      ethhdr->h_dest);
> +
> +find_router:
> +	/**
> +	 * find_router():
> +	 *  - if orig_node is NULL it returns NULL
> +	 *  - increases neigh_nodes refcount if found.
> +	 */
> +	neigh_node = find_router(bat_priv, orig_node, NULL);
> +
> +	if (!neigh_node)
> +		goto out;
> +
> +	if (!prepare_unicast_4addr_packet(bat_priv, skb, orig_node))
> +		goto out;
> +
> +	unicast_4addr_packet = (struct unicast_4addr_packet *)skb->data;
> +
> +	if (atomic_read(&bat_priv->fragmentation) &&
> +	    data_len + sizeof(*unicast_4addr_packet) >
> +				neigh_node->if_incoming->net_dev->mtu) {
> +		/* send frag skb decreases ttl */
> +		unicast_4addr_packet->u.header.ttl++;
> +		ret = frag_send_skb(skb, bat_priv,
> +				    neigh_node->if_incoming, neigh_node->addr);
> +		goto out;
> +	}
> +
> +	send_skb_packet(skb, neigh_node->if_incoming, neigh_node->addr);
> +	ret = 0;
> +	goto out;
> +
> +out:
> +	if (neigh_node)
> +		neigh_node_free_ref(neigh_node);
> +	if (orig_node)
> +		orig_node_free_ref(orig_node);
> +	if (ret == 1)
> +		kfree_skb(skb);
> +	return ret;
> +}

Same here. Isn't it possible to let function handle the difference in packet 
type without having the same function twice ? 

Regards,
Marek

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCHv5 9/9] batman-adv: Distributed ARP Table - use unicast_4addr_packet for DHT messages
  2012-02-09 23:41 ` [B.A.T.M.A.N.] [PATCHv5 9/9] batman-adv: Distributed ARP Table - use unicast_4addr_packet for DHT messages Antonio Quartulli
@ 2012-02-11 13:44   ` Marek Lindner
  2012-02-11 13:57     ` Antonio Quartulli
  0 siblings, 1 reply; 27+ messages in thread
From: Marek Lindner @ 2012-02-11 13:44 UTC (permalink / raw)
  To: The list for a Better Approach To Mobile Ad-hoc Networking

On Friday, February 10, 2012 07:41:42 Antonio Quartulli wrote:
> In order to enable an higher verbosity level of the DAT debug messages, the
> unicast_4addr_packet is now used to carry ARP packets generated by the DAT
> internal mechanism. This packet type will enable batman-adv to recognise
> each DAT related message and to print its source (this will help to track
> possibly bogus ARP entries)
> 
> Signed-off-by: Antonio Quartulli <ordex@autistici.org>
> ---
>  distributed-arp-table.c |  129
> +++++++++++++++++++++++++++++++--------------- distributed-arp-table.h |  
> 16 +++---
>  soft-interface.c        |   15 +++--
>  3 files changed, 105 insertions(+), 55 deletions(-)
> 
> diff --git a/distributed-arp-table.c b/distributed-arp-table.c
> index 48e97e0..239b5c4 100644
> --- a/distributed-arp-table.c
> +++ b/distributed-arp-table.c
> @@ -31,6 +31,7 @@
>  #include "originator.h"
>  #include "send.h"
>  #include "soft-interface.h"
> +#include "translation-table.h"
>  #include "types.h"
>  #include "translation-table.h"
>  #include "unicast.h"
> @@ -38,10 +39,44 @@
>  #ifdef CONFIG_BATMAN_ADV_DEBUG
> 
>  static inline void bat_dbg_arp(struct bat_priv *bat_priv,
> -			       struct sk_buff *skb, uint16_t type) {
> +			       struct sk_buff *skb, uint16_t type,
> +			       int hdr_size) {
> +	struct unicast_4addr_packet *unicast_4addr_packet;
> +
>  	bat_dbg(DBG_ARP, bat_priv, "ARP MSG = [src: %pM-%pI4 dst: %pM-%pI4]\n",
> -		ARP_HW_SRC(skb), &ARP_IP_SRC(skb), ARP_HW_DST(skb),
> -		&ARP_IP_DST(skb));
> +		ARP_HW_SRC(skb, hdr_size), &ARP_IP_SRC(skb, hdr_size),
> +		ARP_HW_DST(skb, hdr_size), &ARP_IP_DST(skb, hdr_size));
> +
> +	if (hdr_size == 0)
> +		return;
> +
> +	/* if the AP packet is encapsulated in a batman packet, let's print some
> +	 * debug messages */
> +	unicast_4addr_packet = (struct unicast_4addr_packet *)skb->data;
> +
> +	switch (unicast_4addr_packet->u.header.packet_type) {
> +	case BAT_UNICAST:
> +		bat_dbg(DBG_ARP, bat_priv, "encapsulated within a UNICAST "
> +			"packet\n");
> +		break;
> +	case BAT_UNICAST_4ADDR:
> +		bat_dbg(DBG_ARP, bat_priv, "encapsulated within a "
> +			"UNICAST_4ADDR packet (src: %pM)\n",
> +			unicast_4addr_packet->src);
> +		if (unicast_4addr_packet->subtype != BAT_P_DHT_PUT ||
> +		    unicast_4addr_packet->subtype != BAT_P_DHT_GET)
> +			bat_dbg(DBG_ARP, bat_priv, "It's a DAT message\n");

Isn't this check going to fail as soon as a new subtype was added ?


> +	unicast_4addr_packet = (struct unicast_4addr_packet *)skb->data;
> +
> +	if (dat_snoop_incoming_arp_request(bat_priv, skb, hdr_size))
> +		goto out;
> +
> +	if (dat_snoop_incoming_arp_reply(bat_priv, skb, hdr_size))
> +		goto out;
> +

The unicast_4addr_packet variable isn't used anywhere ...

Most of this patch simply adds a hdr_size variable to the functions added by 
earlier patches. Why not adding the unicast_4addr type at the beginning of the 
patch series ? You would not need this patch.

Cheers,
Marek

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCHv5 9/9] batman-adv: Distributed ARP Table - use unicast_4addr_packet for DHT messages
  2012-02-11 13:44   ` Marek Lindner
@ 2012-02-11 13:57     ` Antonio Quartulli
  0 siblings, 0 replies; 27+ messages in thread
From: Antonio Quartulli @ 2012-02-11 13:57 UTC (permalink / raw)
  To: The list for a Better Approach To Mobile Ad-hoc Networking

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

On Sat, Feb 11, 2012 at 09:44:23PM +0800, Marek Lindner wrote:
> On Friday, February 10, 2012 07:41:42 Antonio Quartulli wrote:
> > In order to enable an higher verbosity level of the DAT debug messages, the
> > unicast_4addr_packet is now used to carry ARP packets generated by the DAT
> > internal mechanism. This packet type will enable batman-adv to recognise
> > each DAT related message and to print its source (this will help to track
> > possibly bogus ARP entries)
> > 
> > Signed-off-by: Antonio Quartulli <ordex@autistici.org>
> > ---
> >  distributed-arp-table.c |  129
> > +++++++++++++++++++++++++++++++--------------- distributed-arp-table.h |  
> > 16 +++---
> >  soft-interface.c        |   15 +++--
> >  3 files changed, 105 insertions(+), 55 deletions(-)
> > 
> > diff --git a/distributed-arp-table.c b/distributed-arp-table.c
> > index 48e97e0..239b5c4 100644
> > --- a/distributed-arp-table.c
> > +++ b/distributed-arp-table.c
> > @@ -31,6 +31,7 @@
> >  #include "originator.h"
> >  #include "send.h"
> >  #include "soft-interface.h"
> > +#include "translation-table.h"
> >  #include "types.h"
> >  #include "translation-table.h"
> >  #include "unicast.h"
> > @@ -38,10 +39,44 @@
> >  #ifdef CONFIG_BATMAN_ADV_DEBUG
> > 
> >  static inline void bat_dbg_arp(struct bat_priv *bat_priv,
> > -			       struct sk_buff *skb, uint16_t type) {
> > +			       struct sk_buff *skb, uint16_t type,
> > +			       int hdr_size) {
> > +	struct unicast_4addr_packet *unicast_4addr_packet;
> > +
> >  	bat_dbg(DBG_ARP, bat_priv, "ARP MSG = [src: %pM-%pI4 dst: %pM-%pI4]\n",
> > -		ARP_HW_SRC(skb), &ARP_IP_SRC(skb), ARP_HW_DST(skb),
> > -		&ARP_IP_DST(skb));
> > +		ARP_HW_SRC(skb, hdr_size), &ARP_IP_SRC(skb, hdr_size),
> > +		ARP_HW_DST(skb, hdr_size), &ARP_IP_DST(skb, hdr_size));
> > +
> > +	if (hdr_size == 0)
> > +		return;
> > +
> > +	/* if the AP packet is encapsulated in a batman packet, let's print some
> > +	 * debug messages */
> > +	unicast_4addr_packet = (struct unicast_4addr_packet *)skb->data;
> > +
> > +	switch (unicast_4addr_packet->u.header.packet_type) {
> > +	case BAT_UNICAST:
> > +		bat_dbg(DBG_ARP, bat_priv, "encapsulated within a UNICAST "
> > +			"packet\n");
> > +		break;
> > +	case BAT_UNICAST_4ADDR:
> > +		bat_dbg(DBG_ARP, bat_priv, "encapsulated within a "
> > +			"UNICAST_4ADDR packet (src: %pM)\n",
> > +			unicast_4addr_packet->src);
> > +		if (unicast_4addr_packet->subtype != BAT_P_DHT_PUT ||
> > +		    unicast_4addr_packet->subtype != BAT_P_DHT_GET)
> > +			bat_dbg(DBG_ARP, bat_priv, "It's a DAT message\n");
> 
> Isn't this check going to fail as soon as a new subtype was added ?
> 
> 
> > +	unicast_4addr_packet = (struct unicast_4addr_packet *)skb->data;
> > +
> > +	if (dat_snoop_incoming_arp_request(bat_priv, skb, hdr_size))
> > +		goto out;
> > +
> > +	if (dat_snoop_incoming_arp_reply(bat_priv, skb, hdr_size))
> > +		goto out;
> > +
> 
> The unicast_4addr_packet variable isn't used anywhere ...
> 
> Most of this patch simply adds a hdr_size variable to the functions added by 
> earlier patches. Why not adding the unicast_4addr type at the beginning of the 
> patch series ? You would not need this patch.

Thank you for your feedback.
Yes, you are right. Actually I didn't care about the "weight" of th patch: I
simply added the new type at the end and then applied the needed changes. But I
agree with you: moving the new type patch at the beginning is a good idea.

Cheers,

> 
> Cheers,
> Marek

-- 
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] 27+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCHv5 3/9] batman-adv: Distributed ARP Table - create DHT helper functions
  2012-02-09 23:41 ` [B.A.T.M.A.N.] [PATCHv5 3/9] batman-adv: Distributed ARP Table - create DHT helper functions Antonio Quartulli
@ 2012-02-11 13:59   ` Marek Lindner
  2012-02-13 20:33     ` Antonio Quartulli
  0 siblings, 1 reply; 27+ messages in thread
From: Marek Lindner @ 2012-02-11 13:59 UTC (permalink / raw)
  To: The list for a Better Approach To Mobile Ad-hoc Networking

On Friday, February 10, 2012 07:41:36 Antonio Quartulli wrote:
> +       for (select = 0; select < DHT_CANDIDATES_NUM; select++) {
> +               max = 0;
> +               max_orig_node = NULL;
> +               if (!chosen_me) {
> +                       /* if true, wrap around the key space */
> +                       if (bat_priv->dht_hash > ip_key)
> +                               max = DAT_ADDR_MAX - bat_priv->dht_hash +
> +                                       ip_key;
> +                       else
> +                               max = ip_key - bat_priv->dht_hash;
> +                       max = bat_priv->dht_hash;

Somehow this does not make sense to me. Why do we calculate a magic value for 
"max" if we set it to bat_priv->dht_hash afterwards ?
Moreover, the name dht_hash is a bit confusing. At first I thought it is a 
pointer to a hash (read: orig_hash/tt_local_hash/tt_global_hash/etc). Perhaps 
the comment should explain what and why the magic is supposed to achieve 
instead of stating what the c code is obviously doing ?

Cheers,
Marek

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCHv5 4/9] batman-adv: Distributed ARP Table - add ARP parsing functions
  2012-02-09 23:41 ` [B.A.T.M.A.N.] [PATCHv5 4/9] batman-adv: Distributed ARP Table - add ARP parsing functions Antonio Quartulli
  2012-02-10 14:21   ` Marek Lindner
@ 2012-02-11 14:09   ` Marek Lindner
  2012-02-14 10:43     ` Antonio Quartulli
  1 sibling, 1 reply; 27+ messages in thread
From: Marek Lindner @ 2012-02-11 14:09 UTC (permalink / raw)
  To: The list for a Better Approach To Mobile Ad-hoc Networking

On Friday, February 10, 2012 07:41:37 Antonio Quartulli wrote:
> +static inline void bat_dbg_arp(struct bat_priv *bat_priv,
> +                              struct sk_buff *skb, uint16_t type) {
> +       char buf[30];
> +       const char *type_str[] = { "REQUEST", "REPLY", "RREQUEST",
> "RREPLY", +                                     "InREQUEST", "InREPLY",
> "NAK" }; +
> +       if (type >= 1 && type <= ARRAY_SIZE(type_str))
> +               scnprintf(buf, sizeof(buf), "%s", type_str[type - 1]);
> +       else
> +               scnprintf(buf, sizeof(buf), "UNKNOWN (%hu)", type);
> +
> +       bat_dbg(DBG_ARP, bat_priv, "ARP message of type %s recognised "
> +               "[src: %pM-%pI4 dst: %pM-%pI4]\n", buf, ARP_HW_SRC(skb),
> +               &ARP_IP_SRC(skb), ARP_HW_DST(skb), &ARP_IP_DST(skb));
> +}

Every time this fucntion is called an additional bat_dgb() call is made 
directly before that (as far as I can tell). Wouldn't it make sense to include 
this extra message as parameter to bat_dbg_arp() ?

Regards,
Marek

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCHv5 8/9] batman-adv: add UNICAST_4ADDR packet type
  2012-02-09 23:41 ` [B.A.T.M.A.N.] [PATCHv5 8/9] batman-adv: add UNICAST_4ADDR packet type Antonio Quartulli
  2012-02-10 14:42   ` Marek Lindner
@ 2012-02-12 14:27   ` Marek Lindner
  1 sibling, 0 replies; 27+ messages in thread
From: Marek Lindner @ 2012-02-12 14:27 UTC (permalink / raw)
  To: The list for a Better Approach To Mobile Ad-hoc Networking

On Friday, February 10, 2012 07:41:41 Antonio Quartulli wrote:
> +enum bat_subtype {
> +       BAT_P_DATA       = 0x01,
> +       BAT_P_DHT_GET    = 0x02,
> +       BAT_P_DHT_PUT    = 0x03
>  };

One more before I shut up and wait for the next patchset.  ;-)

BAT_P_DHT_GET / BAT_P_DHT_PUT isn't used anywhere.

Regards,
Marek

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCHv5 3/9] batman-adv: Distributed ARP Table - create DHT helper functions
  2012-02-11 13:59   ` Marek Lindner
@ 2012-02-13 20:33     ` Antonio Quartulli
  2012-02-14  6:07       ` Marek Lindner
  0 siblings, 1 reply; 27+ messages in thread
From: Antonio Quartulli @ 2012-02-13 20:33 UTC (permalink / raw)
  To: The list for a Better Approach To Mobile Ad-hoc Networking

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

On Sat, Feb 11, 2012 at 09:59:57PM +0800, Marek Lindner wrote:
> On Friday, February 10, 2012 07:41:36 Antonio Quartulli wrote:
> > +       for (select = 0; select < DHT_CANDIDATES_NUM; select++) {
> > +               max = 0;
> > +               max_orig_node = NULL;
> > +               if (!chosen_me) {
> > +                       /* if true, wrap around the key space */
> > +                       if (bat_priv->dht_hash > ip_key)
> > +                               max = DAT_ADDR_MAX - bat_priv->dht_hash +
> > +                                       ip_key;
> > +                       else
> > +                               max = ip_key - bat_priv->dht_hash;
> > +                       max = bat_priv->dht_hash;
> 
> Somehow this does not make sense to me. Why do we calculate a magic value for 
> "max" if we set it to bat_priv->dht_hash afterwards ?

Thank you, actually I didn't spot it in the tests because of the limited number
of nodes involved. I thought they were enough.

> Moreover, the name dht_hash is a bit confusing. At first I thought it is a 
> pointer to a hash (read: orig_hash/tt_local_hash/tt_global_hash/etc).

ok, I'll rename it to dht_id (or dht_addr ?)

> Perhaps 
> the comment should explain what and why the magic is supposed to achieve 
> instead of stating what the c code is obviously doing ?
> 

I agree :-)


Thanks!


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] 27+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCHv5 3/9] batman-adv: Distributed ARP Table - create DHT helper functions
  2012-02-13 20:33     ` Antonio Quartulli
@ 2012-02-14  6:07       ` Marek Lindner
  2012-02-14  7:47         ` Antonio Quartulli
  0 siblings, 1 reply; 27+ messages in thread
From: Marek Lindner @ 2012-02-14  6:07 UTC (permalink / raw)
  To: The list for a Better Approach To Mobile Ad-hoc Networking

On Tuesday, February 14, 2012 04:33:52 Antonio Quartulli wrote:
> > Moreover, the name dht_hash is a bit confusing. At first I thought it is
> > a pointer to a hash (read: orig_hash/tt_local_hash/tt_global_hash/etc).
> 
> ok, I'll rename it to dht_id (or dht_addr ?)

It is difficult for me to suggest a better name because I don't understand 
what this variable is used for. 

Regards,
Marek

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCHv5 3/9] batman-adv: Distributed ARP Table - create DHT helper functions
  2012-02-14  6:07       ` Marek Lindner
@ 2012-02-14  7:47         ` Antonio Quartulli
  0 siblings, 0 replies; 27+ messages in thread
From: Antonio Quartulli @ 2012-02-14  7:47 UTC (permalink / raw)
  To: The list for a Better Approach To Mobile Ad-hoc Networking

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

On Tue, Feb 14, 2012 at 02:07:57PM +0800, Marek Lindner wrote:
> On Tuesday, February 14, 2012 04:33:52 Antonio Quartulli wrote:
> > > Moreover, the name dht_hash is a bit confusing. At first I thought it is
> > > a pointer to a hash (read: orig_hash/tt_local_hash/tt_global_hash/etc).
> > 
> > ok, I'll rename it to dht_id (or dht_addr ?)
> 
> It is difficult for me to suggest a better name because I don't understand 
> what this variable is used for. 

Ops sorry! Actually this variable represent the address of the node (me or the
others, depending on the struct it belongs to - bat_priv or orig_node) in the
dht space. In the dht space every object has an id, either mesh_node and stored
keys.

I'd say that dht_addr better represent this concept.

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] 27+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCHv5 4/9] batman-adv: Distributed ARP Table - add ARP parsing functions
  2012-02-11 14:09   ` Marek Lindner
@ 2012-02-14 10:43     ` Antonio Quartulli
  0 siblings, 0 replies; 27+ messages in thread
From: Antonio Quartulli @ 2012-02-14 10:43 UTC (permalink / raw)
  To: The list for a Better Approach To Mobile Ad-hoc Networking

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

On Sat, Feb 11, 2012 at 10:09:30PM +0800, Marek Lindner wrote:
> On Friday, February 10, 2012 07:41:37 Antonio Quartulli wrote:
> > +static inline void bat_dbg_arp(struct bat_priv *bat_priv,
> > +                              struct sk_buff *skb, uint16_t type) {
> > +       char buf[30];
> > +       const char *type_str[] = { "REQUEST", "REPLY", "RREQUEST",
> > "RREPLY", +                                     "InREQUEST", "InREPLY",
> > "NAK" }; +
> > +       if (type >= 1 && type <= ARRAY_SIZE(type_str))
> > +               scnprintf(buf, sizeof(buf), "%s", type_str[type - 1]);
> > +       else
> > +               scnprintf(buf, sizeof(buf), "UNKNOWN (%hu)", type);
> > +
> > +       bat_dbg(DBG_ARP, bat_priv, "ARP message of type %s recognised "
> > +               "[src: %pM-%pI4 dst: %pM-%pI4]\n", buf, ARP_HW_SRC(skb),
> > +               &ARP_IP_SRC(skb), ARP_HW_DST(skb), &ARP_IP_DST(skb));
> > +}
> 
> Every time this fucntion is called an additional bat_dgb() call is made 
> directly before that (as far as I can tell). Wouldn't it make sense to include 
> this extra message as parameter to bat_dbg_arp() ?

I'd say it's a good idea. Despite of the current usage, I thought this function
to be responsible to print the content of the message only. The fact that a
bat_dbg() invocation always precedes its is just a coincident.

But adding a another argument (which can eventually be NULL) makes much sense :)
Thanks!

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] 27+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCHv5 7/9] batman-adv: Distributed ARP Table - add compile option
  2012-02-10 14:32   ` Marek Lindner
@ 2012-02-15 19:47     ` Antonio Quartulli
  2012-02-16  6:35       ` Marek Lindner
  0 siblings, 1 reply; 27+ messages in thread
From: Antonio Quartulli @ 2012-02-15 19:47 UTC (permalink / raw)
  To: The list for a Better Approach To Mobile Ad-hoc Networking

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

On Fri, Feb 10, 2012 at 10:32:00PM +0800, Marek Lindner wrote:
> On Friday, February 10, 2012 07:41:40 Antonio Quartulli wrote:
> > @@ -23,6 +23,8 @@
> >  export CONFIG_BATMAN_ADV_DEBUG=n
> >  # B.A.T.M.A.N. bridge loop avoidance:
> >  export CONFIG_BATMAN_ADV_BLA=y
> > +# B.A.T.M.A.N. distributed ARP table:
> > +export CONFIG_BATMAN_ADV_DAT=n
> 
> Any particular reason why you wish to disable it by default ?

It's a new (read experimental) feature. I don't want people that just upgrade
their batman-adv package to get this new code without having been asked :-)

This is the only reason for being off by default. But if you think we should
push people to test it....then ok :) I can switch it to true!

> > --- a/hard-interface.c
> > +++ b/hard-interface.c
> > @@ -119,9 +119,11 @@ static void primary_if_update_addr(struct bat_priv
> > *bat_priv, if (!primary_if)
> >  		goto out;
> > 
> > +#ifdef CONFIG_BATMAN_ADV_DAT
> >  	bat_priv->dht_hash = (dat_addr_t)
> >  				choose_orig(primary_if->net_dev->dev_addr,
> >  					    DAT_ADDR_MAX);
> > +#endif
> 
> A general dat_init()/dat_free() structure would be better than adding defines 
> everywhere.

ok, so that we can limit defines to dat.c/h and to structure definitions. nice
suggestion :-)

> The README update is missing in this patch.

Good point. Thank you!

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] 27+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCHv5 7/9] batman-adv: Distributed ARP Table - add compile option
  2012-02-15 19:47     ` Antonio Quartulli
@ 2012-02-16  6:35       ` Marek Lindner
  0 siblings, 0 replies; 27+ messages in thread
From: Marek Lindner @ 2012-02-16  6:35 UTC (permalink / raw)
  To: The list for a Better Approach To Mobile Ad-hoc Networking

On Thursday, February 16, 2012 03:47:08 Antonio Quartulli wrote:
> On Fri, Feb 10, 2012 at 10:32:00PM +0800, Marek Lindner wrote:
> > On Friday, February 10, 2012 07:41:40 Antonio Quartulli wrote:
> > > @@ -23,6 +23,8 @@
> > >
> > >  export CONFIG_BATMAN_ADV_DEBUG=n
> > >  # B.A.T.M.A.N. bridge loop avoidance:
> > >  export CONFIG_BATMAN_ADV_BLA=y
> > >
> > > +# B.A.T.M.A.N. distributed ARP table:
> > > +export CONFIG_BATMAN_ADV_DAT=n
> >
> > 
> >
> > Any particular reason why you wish to disable it by default ?
> 
> It's a new (read experimental) feature. I don't want people that just
> upgrade their batman-adv package to get this new code without having been
> asked :-)
> 
> This is the only reason for being off by default. But if you think we
> should push people to test it....then ok :) I can switch it to true!

It is a feature most people will benefit from. Of course it has to work! :)
IMHO the compile time option is for those that want a "slim" batman-adv.

Regards,
Marek

^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2012-02-16  6:35 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-02-09 23:41 [B.A.T.M.A.N.] [PATCHv5 0/9] DAT: Distributed ARP Table Antonio Quartulli
2012-02-09 23:41 ` [B.A.T.M.A.N.] [PATCHv5 1/9] batman-adv: implement an helper function to forge unicast packets Antonio Quartulli
2012-02-09 23:41 ` [B.A.T.M.A.N.] [PATCHv5 2/9] batman-adv: add a new log level for DAT/ARP debugging Antonio Quartulli
2012-02-09 23:41 ` [B.A.T.M.A.N.] [PATCHv5 3/9] batman-adv: Distributed ARP Table - create DHT helper functions Antonio Quartulli
2012-02-11 13:59   ` Marek Lindner
2012-02-13 20:33     ` Antonio Quartulli
2012-02-14  6:07       ` Marek Lindner
2012-02-14  7:47         ` Antonio Quartulli
2012-02-09 23:41 ` [B.A.T.M.A.N.] [PATCHv5 4/9] batman-adv: Distributed ARP Table - add ARP parsing functions Antonio Quartulli
2012-02-10 14:21   ` Marek Lindner
2012-02-11 14:09   ` Marek Lindner
2012-02-14 10:43     ` Antonio Quartulli
2012-02-09 23:41 ` [B.A.T.M.A.N.] [PATCHv5 5/9] batman-adv: Distributed ARP Table - add snooping functions for ARP messages Antonio Quartulli
2012-02-10 14:25   ` Marek Lindner
2012-02-09 23:41 ` [B.A.T.M.A.N.] [PATCHv5 6/9] batman-adv: Distributed ARP Table - increase default soft_iface ARP table timeout Antonio Quartulli
2012-02-09 23:41 ` [B.A.T.M.A.N.] [PATCHv5 7/9] batman-adv: Distributed ARP Table - add compile option Antonio Quartulli
2012-02-10 14:32   ` Marek Lindner
2012-02-15 19:47     ` Antonio Quartulli
2012-02-16  6:35       ` Marek Lindner
2012-02-09 23:41 ` [B.A.T.M.A.N.] [PATCHv5 8/9] batman-adv: add UNICAST_4ADDR packet type Antonio Quartulli
2012-02-10 14:42   ` Marek Lindner
2012-02-12 14:27   ` Marek Lindner
2012-02-09 23:41 ` [B.A.T.M.A.N.] [PATCHv5 9/9] batman-adv: Distributed ARP Table - use unicast_4addr_packet for DHT messages Antonio Quartulli
2012-02-11 13:44   ` Marek Lindner
2012-02-11 13:57     ` Antonio Quartulli
2012-02-09 23:52 ` [B.A.T.M.A.N.] [PATCHv5 0/9] DAT: Distributed ARP Table Antonio Quartulli
2012-02-10  9:44 ` 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).