Hi David,
the following 10 patches constitute the first batch I'd like to get the pulled into net-next-2.6/3.3. They're mostly uncritical fixes around the recently introduced tt code, some code refactoring, the kstrto update and the range check fix reported by Thomas Jarosch.
Thanks, Marek
The following changes since commit 1ea6b8f48918282bdca0b32a34095504ee65bab5:
Linux 3.2-rc1 (2011-11-07 16:16:02 -0800)
are available in the git repository at: git://git.open-mesh.org/linux-merge.git for_david
Antonio Quartulli (5): batman-adv: tt_global_del_orig() has to print the correct message batman-adv: use orig_hash_find() instead of get_orig_node() in TT code batman-adv: fixed hash functions type to uint32_t instead of int batman-adv: linearise the tt_response skb only if needed batman-adv: check for tt_reponse packet real length
Marek Lindner (1): batman-adv: refactoring gateway handling code
Simon Wunderlich (2): batman-adv: directly write tt entries without buffering batman-adv: Fix range check for expected packets
Sven Eckelmann (2): batman-adv: update internal version number batman-adv: Replace obsolete strict_strto<foo> with kstrto<foo>
net/batman-adv/bat_sysfs.c | 4 +- net/batman-adv/bitarray.c | 2 +- net/batman-adv/gateway_client.c | 153 ++++++++++++++++++++++-------------- net/batman-adv/gateway_client.h | 5 +- net/batman-adv/gateway_common.c | 4 +- net/batman-adv/hash.c | 4 +- net/batman-adv/hash.h | 13 ++-- net/batman-adv/main.h | 2 +- net/batman-adv/originator.c | 13 ++- net/batman-adv/originator.h | 2 +- net/batman-adv/routing.c | 22 ++++-- net/batman-adv/soft-interface.c | 43 +++++++--- net/batman-adv/translation-table.c | 100 ++++++----------------- net/batman-adv/vis.c | 17 +++-- 14 files changed, 202 insertions(+), 182 deletions(-)
From: Sven Eckelmann sven@narfation.org
Signed-off-by: Sven Eckelmann sven@narfation.org --- net/batman-adv/main.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/batman-adv/main.h b/net/batman-adv/main.h index 964ad4d..86354e0 100644 --- a/net/batman-adv/main.h +++ b/net/batman-adv/main.h @@ -28,7 +28,7 @@ #define DRIVER_DEVICE "batman-adv"
#ifndef SOURCE_VERSION -#define SOURCE_VERSION "2011.4.0" +#define SOURCE_VERSION "2012.0.0" #endif
/* B.A.T.M.A.N. parameters */
From: Antonio Quartulli ordex@autistici.org
When deleting the entries, tt_global_del_orig() has to print the message passed as argument instead of a static one.
Signed-off-by: Antonio Quartulli ordex@autistici.org Signed-off-by: Sven Eckelmann sven@narfation.org --- net/batman-adv/translation-table.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c index c7aafc7..1db9d96 100644 --- a/net/batman-adv/translation-table.c +++ b/net/batman-adv/translation-table.c @@ -735,9 +735,10 @@ void tt_global_del_orig(struct bat_priv *bat_priv, if (tt_global_entry->orig_node == orig_node) { bat_dbg(DBG_TT, bat_priv, "Deleting global tt entry %pM " - "(via %pM): originator time out\n", + "(via %pM): %s\n", tt_global_entry->addr, - tt_global_entry->orig_node->orig); + tt_global_entry->orig_node->orig, + message); hlist_del_rcu(node); tt_global_entry_free_ref(tt_global_entry); }
From: Sven Eckelmann sven@narfation.org
strict_strto<foo> is obsolete since v3.1-rc8-8466-g14acc55 and should be replaced with kstrto<foo>.
Signed-off-by: Sven Eckelmann sven@narfation.org --- net/batman-adv/bat_sysfs.c | 4 ++-- net/batman-adv/gateway_common.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/batman-adv/bat_sysfs.c b/net/batman-adv/bat_sysfs.c index b8a7414..c25492f 100644 --- a/net/batman-adv/bat_sysfs.c +++ b/net/batman-adv/bat_sysfs.c @@ -174,7 +174,7 @@ static int store_uint_attr(const char *buff, size_t count, unsigned long uint_val; int ret;
- ret = strict_strtoul(buff, 10, &uint_val); + ret = kstrtoul(buff, 10, &uint_val); if (ret) { bat_info(net_dev, "%s: Invalid parameter received: %s\n", @@ -239,7 +239,7 @@ static ssize_t store_vis_mode(struct kobject *kobj, struct attribute *attr, unsigned long val; int ret, vis_mode_tmp = -1;
- ret = strict_strtoul(buff, 10, &val); + ret = kstrtoul(buff, 10, &val);
if (((count == 2) && (!ret) && (val == VIS_TYPE_CLIENT_UPDATE)) || (strncmp(buff, "client", 6) == 0) || diff --git a/net/batman-adv/gateway_common.c b/net/batman-adv/gateway_common.c index 18661af..c4ac7b0 100644 --- a/net/batman-adv/gateway_common.c +++ b/net/batman-adv/gateway_common.c @@ -97,7 +97,7 @@ static bool parse_gw_bandwidth(struct net_device *net_dev, char *buff, *tmp_ptr = '\0'; }
- ret = strict_strtol(buff, 10, &ldown); + ret = kstrtol(buff, 10, &ldown); if (ret) { bat_err(net_dev, "Download speed of gateway mode invalid: %s\n", @@ -122,7 +122,7 @@ static bool parse_gw_bandwidth(struct net_device *net_dev, char *buff, *tmp_ptr = '\0'; }
- ret = strict_strtol(slash_ptr + 1, 10, &lup); + ret = kstrtol(slash_ptr + 1, 10, &lup); if (ret) { bat_err(net_dev, "Upload speed of gateway mode invalid: "
Signed-off-by: Marek Lindner lindner_marek@yahoo.de Acked-by: Antonio Quartulli ordex@autistici.org Signed-off-by: Sven Eckelmann sven@narfation.org --- net/batman-adv/gateway_client.c | 157 ++++++++++++++++++++++++--------------- net/batman-adv/gateway_client.h | 5 +- net/batman-adv/soft-interface.c | 43 +++++++---- 3 files changed, 128 insertions(+), 77 deletions(-)
diff --git a/net/batman-adv/gateway_client.c b/net/batman-adv/gateway_client.c index 619fb73..9373a14 100644 --- a/net/batman-adv/gateway_client.c +++ b/net/batman-adv/gateway_client.c @@ -25,6 +25,7 @@ #include "gateway_common.h" #include "hard-interface.h" #include "originator.h" +#include "translation-table.h" #include "routing.h" #include <linux/ip.h> #include <linux/ipv6.h> @@ -572,108 +573,142 @@ out: return ret; }
-int gw_is_target(struct bat_priv *bat_priv, struct sk_buff *skb, - struct orig_node *old_gw) +bool gw_is_dhcp_target(struct sk_buff *skb, unsigned int *header_len) { struct ethhdr *ethhdr; struct iphdr *iphdr; struct ipv6hdr *ipv6hdr; struct udphdr *udphdr; - struct gw_node *curr_gw; - struct neigh_node *neigh_curr = NULL, *neigh_old = NULL; - unsigned int header_len = 0; - int ret = 1; - - if (atomic_read(&bat_priv->gw_mode) == GW_MODE_OFF) - return 0;
/* check for ethernet header */ - if (!pskb_may_pull(skb, header_len + ETH_HLEN)) - return 0; + if (!pskb_may_pull(skb, *header_len + ETH_HLEN)) + return false; ethhdr = (struct ethhdr *)skb->data; - header_len += ETH_HLEN; + *header_len += ETH_HLEN;
/* check for initial vlan header */ if (ntohs(ethhdr->h_proto) == ETH_P_8021Q) { - if (!pskb_may_pull(skb, header_len + VLAN_HLEN)) - return 0; + if (!pskb_may_pull(skb, *header_len + VLAN_HLEN)) + return false; ethhdr = (struct ethhdr *)(skb->data + VLAN_HLEN); - header_len += VLAN_HLEN; + *header_len += VLAN_HLEN; }
/* check for ip header */ switch (ntohs(ethhdr->h_proto)) { case ETH_P_IP: - if (!pskb_may_pull(skb, header_len + sizeof(*iphdr))) - return 0; - iphdr = (struct iphdr *)(skb->data + header_len); - header_len += iphdr->ihl * 4; + if (!pskb_may_pull(skb, *header_len + sizeof(*iphdr))) + return false; + iphdr = (struct iphdr *)(skb->data + *header_len); + *header_len += iphdr->ihl * 4;
/* check for udp header */ if (iphdr->protocol != IPPROTO_UDP) - return 0; + return false;
break; case ETH_P_IPV6: - if (!pskb_may_pull(skb, header_len + sizeof(*ipv6hdr))) - return 0; - ipv6hdr = (struct ipv6hdr *)(skb->data + header_len); - header_len += sizeof(*ipv6hdr); + if (!pskb_may_pull(skb, *header_len + sizeof(*ipv6hdr))) + return false; + ipv6hdr = (struct ipv6hdr *)(skb->data + *header_len); + *header_len += sizeof(*ipv6hdr);
/* check for udp header */ if (ipv6hdr->nexthdr != IPPROTO_UDP) - return 0; + return false;
break; default: - return 0; + return false; }
- if (!pskb_may_pull(skb, header_len + sizeof(*udphdr))) - return 0; - udphdr = (struct udphdr *)(skb->data + header_len); - header_len += sizeof(*udphdr); + if (!pskb_may_pull(skb, *header_len + sizeof(*udphdr))) + return false; + udphdr = (struct udphdr *)(skb->data + *header_len); + *header_len += sizeof(*udphdr);
/* check for bootp port */ if ((ntohs(ethhdr->h_proto) == ETH_P_IP) && (ntohs(udphdr->dest) != 67)) - return 0; + return false;
if ((ntohs(ethhdr->h_proto) == ETH_P_IPV6) && (ntohs(udphdr->dest) != 547)) - return 0; - - if (atomic_read(&bat_priv->gw_mode) == GW_MODE_SERVER) - return -1; - - curr_gw = gw_get_selected_gw_node(bat_priv); - if (!curr_gw) - return 0; - - /* If old_gw != NULL then this packet is unicast. - * So, at this point we have to check the message type: if it is a - * DHCPREQUEST we have to decide whether to drop it or not */ - if (old_gw && curr_gw->orig_node != old_gw) { - if (is_type_dhcprequest(skb, header_len)) { - /* If the dhcp packet has been sent to a different gw, - * we have to evaluate whether the old gw is still - * reliable enough */ - neigh_curr = find_router(bat_priv, curr_gw->orig_node, - NULL); - neigh_old = find_router(bat_priv, old_gw, NULL); - if (!neigh_curr || !neigh_old) - goto free_neigh; - if (neigh_curr->tq_avg - neigh_old->tq_avg < - GW_THRESHOLD) - ret = -1; - } + return false; + + return true; +} + +bool gw_out_of_range(struct bat_priv *bat_priv, + struct sk_buff *skb, struct ethhdr *ethhdr) +{ + struct neigh_node *neigh_curr = NULL, *neigh_old = NULL; + struct orig_node *orig_dst_node = NULL; + struct gw_node *curr_gw = NULL; + bool ret, out_of_range = false; + unsigned int header_len = 0; + uint8_t curr_tq_avg; + + ret = gw_is_dhcp_target(skb, &header_len); + if (!ret) + goto out; + + orig_dst_node = transtable_search(bat_priv, ethhdr->h_source, + ethhdr->h_dest); + if (!orig_dst_node) + goto out; + + if (!orig_dst_node->gw_flags) + goto out; + + ret = is_type_dhcprequest(skb, header_len); + if (!ret) + goto out; + + switch (atomic_read(&bat_priv->gw_mode)) { + case GW_MODE_SERVER: + /* If we are a GW then we are our best GW. We can artificially + * set the tq towards ourself as the maximum value */ + curr_tq_avg = TQ_MAX_VALUE; + break; + case GW_MODE_CLIENT: + curr_gw = gw_get_selected_gw_node(bat_priv); + if (!curr_gw) + goto out; + + /* packet is going to our gateway */ + if (curr_gw->orig_node == orig_dst_node) + goto out; + + /* If the dhcp packet has been sent to a different gw, + * we have to evaluate whether the old gw is still + * reliable enough */ + neigh_curr = find_router(bat_priv, curr_gw->orig_node, NULL); + if (!neigh_curr) + goto out; + + curr_tq_avg = neigh_curr->tq_avg; + break; + case GW_MODE_OFF: + default: + goto out; } -free_neigh: + + neigh_old = find_router(bat_priv, orig_dst_node, NULL); + if (!!neigh_old) + goto out; + + if (curr_tq_avg - neigh_old->tq_avg > GW_THRESHOLD) + out_of_range = true; + +out: + if (orig_dst_node) + orig_node_free_ref(orig_dst_node); + if (curr_gw) + gw_node_free_ref(curr_gw); if (neigh_old) neigh_node_free_ref(neigh_old); if (neigh_curr) neigh_node_free_ref(neigh_curr); - if (curr_gw) - gw_node_free_ref(curr_gw); - return ret; + return out_of_range; } diff --git a/net/batman-adv/gateway_client.h b/net/batman-adv/gateway_client.h index b9b983c..e1edba0 100644 --- a/net/batman-adv/gateway_client.h +++ b/net/batman-adv/gateway_client.h @@ -31,7 +31,8 @@ void gw_node_update(struct bat_priv *bat_priv, void gw_node_delete(struct bat_priv *bat_priv, struct orig_node *orig_node); void gw_node_purge(struct bat_priv *bat_priv); int gw_client_seq_print_text(struct seq_file *seq, void *offset); -int gw_is_target(struct bat_priv *bat_priv, struct sk_buff *skb, - struct orig_node *old_gw); +bool gw_is_dhcp_target(struct sk_buff *skb, unsigned int *header_len); +bool gw_out_of_range(struct bat_priv *bat_priv, + struct sk_buff *skb, struct ethhdr *ethhdr);
#endif /* _NET_BATMAN_ADV_GATEWAY_CLIENT_H_ */ diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c index f9cc957..45297c8 100644 --- a/net/batman-adv/soft-interface.c +++ b/net/batman-adv/soft-interface.c @@ -563,10 +563,10 @@ static int interface_tx(struct sk_buff *skb, struct net_device *soft_iface) struct bcast_packet *bcast_packet; struct vlan_ethhdr *vhdr; struct softif_neigh *curr_softif_neigh = NULL; - struct orig_node *orig_node = NULL; + unsigned int header_len = 0; int data_len = skb->len, ret; short vid = -1; - bool do_bcast; + bool do_bcast = false;
if (atomic_read(&bat_priv->mesh_state) != MESH_ACTIVE) goto dropped; @@ -598,17 +598,28 @@ static int interface_tx(struct sk_buff *skb, struct net_device *soft_iface) /* Register the client MAC in the transtable */ tt_local_add(soft_iface, ethhdr->h_source, skb->skb_iif);
- orig_node = transtable_search(bat_priv, ethhdr->h_source, - ethhdr->h_dest); - do_bcast = is_multicast_ether_addr(ethhdr->h_dest); - if (do_bcast || (orig_node && orig_node->gw_flags)) { - ret = gw_is_target(bat_priv, skb, orig_node); + if (is_multicast_ether_addr(ethhdr->h_dest)) { + do_bcast = true;
- if (ret < 0) - goto dropped; - - if (ret) - do_bcast = false; + switch (atomic_read(&bat_priv->gw_mode)) { + case GW_MODE_SERVER: + /* gateway servers should not send dhcp + * requests into the mesh */ + ret = gw_is_dhcp_target(skb, &header_len); + if (ret) + goto dropped; + break; + case GW_MODE_CLIENT: + /* gateway clients should send dhcp requests + * via unicast to their gateway */ + ret = gw_is_dhcp_target(skb, &header_len); + if (ret) + do_bcast = false; + break; + case GW_MODE_OFF: + default: + break; + } }
/* ethernet packet should be broadcasted */ @@ -644,6 +655,12 @@ static int interface_tx(struct sk_buff *skb, struct net_device *soft_iface)
/* unicast packet */ } else { + if (atomic_read(&bat_priv->gw_mode) != GW_MODE_OFF) { + ret = gw_out_of_range(bat_priv, skb, ethhdr); + if (ret) + goto dropped; + } + ret = unicast_send_skb(skb, bat_priv); if (ret != 0) goto dropped_freed; @@ -662,8 +679,6 @@ end: softif_neigh_free_ref(curr_softif_neigh); if (primary_if) hardif_free_ref(primary_if); - if (orig_node) - orig_node_free_ref(orig_node); return NETDEV_TX_OK; }
From: Antonio Quartulli ordex@autistici.org
get_orig_node() tries to retrieve an orig_node object based on a mac address and creates it if not present. This is not the wanted behaviour in the translation table code as we don't want to create new orig_code objects but expect a NULL pointer if the object does not exist.
Reported-by: Simon Wunderlich siwu@hrz.tu-chemnitz.de Signed-off-by: Antonio Quartulli ordex@autistici.org Signed-off-by: Sven Eckelmann sven@narfation.org --- net/batman-adv/translation-table.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c index 1db9d96..7ab9d72 100644 --- a/net/batman-adv/translation-table.c +++ b/net/batman-adv/translation-table.c @@ -1188,11 +1188,11 @@ static bool send_other_tt_response(struct bat_priv *bat_priv, (tt_request->flags & TT_FULL_TABLE ? 'F' : '.'));
/* Let's get the orig node of the REAL destination */ - req_dst_orig_node = get_orig_node(bat_priv, tt_request->dst); + req_dst_orig_node = orig_hash_find(bat_priv, tt_request->dst); if (!req_dst_orig_node) goto out;
- res_dst_orig_node = get_orig_node(bat_priv, tt_request->src); + res_dst_orig_node = orig_hash_find(bat_priv, tt_request->src); if (!res_dst_orig_node) goto out;
@@ -1318,7 +1318,7 @@ static bool send_my_tt_response(struct bat_priv *bat_priv, my_ttvn = (uint8_t)atomic_read(&bat_priv->ttvn); req_ttvn = tt_request->ttvn;
- orig_node = get_orig_node(bat_priv, tt_request->src); + orig_node = orig_hash_find(bat_priv, tt_request->src); if (!orig_node) goto out;
From: Antonio Quartulli ordex@autistici.org
There are two reasons for this fix: - the result of choose_orig() and vis_choose() is an index and therefore it can't be negative. Hence it is correct to make the return type unsigned too.
- sizeof(int) may not be the same on ALL the architectures. Since we plan to use choose_orig() as DHT hash function, we need to guarantee that, given the same argument, the result is the same. Then it is correct to explicitly express the size of the return type (and the second argument). Since the expected length is currently 4, uint32_t is the most convenient choice.
Signed-off-by: Antonio Quartulli ordex@autistici.org Signed-off-by: Sven Eckelmann sven@narfation.org --- net/batman-adv/hash.c | 4 ++-- net/batman-adv/hash.h | 13 +++++++------ net/batman-adv/originator.c | 13 ++++++++----- net/batman-adv/originator.h | 2 +- net/batman-adv/routing.c | 2 +- net/batman-adv/translation-table.c | 32 ++++++++++++++++++-------------- net/batman-adv/vis.c | 17 ++++++++++------- 7 files changed, 47 insertions(+), 36 deletions(-)
diff --git a/net/batman-adv/hash.c b/net/batman-adv/hash.c index 2a17250..d1da29d 100644 --- a/net/batman-adv/hash.c +++ b/net/batman-adv/hash.c @@ -25,7 +25,7 @@ /* clears the hash */ static void hash_init(struct hashtable_t *hash) { - int i; + uint32_t i;
for (i = 0 ; i < hash->size; i++) { INIT_HLIST_HEAD(&hash->table[i]); @@ -42,7 +42,7 @@ void hash_destroy(struct hashtable_t *hash) }
/* allocates and clears the hash */ -struct hashtable_t *hash_new(int size) +struct hashtable_t *hash_new(uint32_t size) { struct hashtable_t *hash;
diff --git a/net/batman-adv/hash.h b/net/batman-adv/hash.h index d20aa71..4768717 100644 --- a/net/batman-adv/hash.h +++ b/net/batman-adv/hash.h @@ -33,17 +33,17 @@ typedef int (*hashdata_compare_cb)(const struct hlist_node *, const void *); /* the hashfunction, should return an index * based on the key in the data of the first * argument and the size the second */ -typedef int (*hashdata_choose_cb)(const void *, int); +typedef uint32_t (*hashdata_choose_cb)(const void *, uint32_t); typedef void (*hashdata_free_cb)(struct hlist_node *, void *);
struct hashtable_t { struct hlist_head *table; /* the hashtable itself with the buckets */ spinlock_t *list_locks; /* spinlock for each hash list entry */ - int size; /* size of hashtable */ + uint32_t size; /* size of hashtable */ };
/* allocates and clears the hash */ -struct hashtable_t *hash_new(int size); +struct hashtable_t *hash_new(uint32_t size);
/* free only the hashtable and the hash itself. */ void hash_destroy(struct hashtable_t *hash); @@ -57,7 +57,7 @@ static inline void hash_delete(struct hashtable_t *hash, struct hlist_head *head; struct hlist_node *node, *node_tmp; spinlock_t *list_lock; /* spinlock to protect write access */ - int i; + uint32_t i;
for (i = 0; i < hash->size; i++) { head = &hash->table[i]; @@ -93,7 +93,8 @@ static inline int hash_add(struct hashtable_t *hash, hashdata_choose_cb choose, const void *data, struct hlist_node *data_node) { - int index, ret = -1; + uint32_t index; + int ret = -1; struct hlist_head *head; struct hlist_node *node; spinlock_t *list_lock; /* spinlock to protect write access */ @@ -137,7 +138,7 @@ static inline void *hash_remove(struct hashtable_t *hash, hashdata_compare_cb compare, hashdata_choose_cb choose, void *data) { - size_t index; + uint32_t index; struct hlist_node *node; struct hlist_head *head; void *data_save = NULL; diff --git a/net/batman-adv/originator.c b/net/batman-adv/originator.c index 0e5b772..0bc2045 100644 --- a/net/batman-adv/originator.c +++ b/net/batman-adv/originator.c @@ -164,7 +164,7 @@ void originator_free(struct bat_priv *bat_priv) struct hlist_head *head; spinlock_t *list_lock; /* spinlock to protect write access */ struct orig_node *orig_node; - int i; + uint32_t i;
if (!hash) return; @@ -350,7 +350,7 @@ static void _purge_orig(struct bat_priv *bat_priv) struct hlist_head *head; spinlock_t *list_lock; /* spinlock to protect write access */ struct orig_node *orig_node; - int i; + uint32_t i;
if (!hash) return; @@ -413,7 +413,8 @@ int orig_seq_print_text(struct seq_file *seq, void *offset) int batman_count = 0; int last_seen_secs; int last_seen_msecs; - int i, ret = 0; + uint32_t i; + int ret = 0;
primary_if = primary_if_get_selected(bat_priv);
@@ -519,7 +520,8 @@ int orig_hash_add_if(struct hard_iface *hard_iface, int max_if_num) struct hlist_node *node; struct hlist_head *head; struct orig_node *orig_node; - int i, ret; + uint32_t i; + int ret;
/* resize all orig nodes because orig_node->bcast_own(_sum) depend on * if_num */ @@ -601,7 +603,8 @@ int orig_hash_del_if(struct hard_iface *hard_iface, int max_if_num) struct hlist_head *head; struct hard_iface *hard_iface_tmp; struct orig_node *orig_node; - int i, ret; + uint32_t i; + int ret;
/* resize all orig nodes because orig_node->bcast_own(_sum) depend on * if_num */ diff --git a/net/batman-adv/originator.h b/net/batman-adv/originator.h index cfc1f60..67765ff 100644 --- a/net/batman-adv/originator.h +++ b/net/batman-adv/originator.h @@ -42,7 +42,7 @@ int orig_hash_del_if(struct hard_iface *hard_iface, int max_if_num);
/* hashfunction to choose an entry in a hash table of given size */ /* hash algorithm from http://en.wikipedia.org/wiki/Hash_table */ -static inline int choose_orig(const void *data, int32_t size) +static inline uint32_t choose_orig(const void *data, uint32_t size) { const unsigned char *key = data; uint32_t hash = 0; diff --git a/net/batman-adv/routing.c b/net/batman-adv/routing.c index f961cc5..60ce407 100644 --- a/net/batman-adv/routing.c +++ b/net/batman-adv/routing.c @@ -39,7 +39,7 @@ void slide_own_bcast_window(struct hard_iface *hard_iface) struct hlist_head *head; struct orig_node *orig_node; unsigned long *word; - int i; + uint32_t i; size_t word_index;
for (i = 0; i < hash->size; i++) { diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c index 7ab9d72..5f28a7f0 100644 --- a/net/batman-adv/translation-table.c +++ b/net/batman-adv/translation-table.c @@ -67,7 +67,7 @@ static struct tt_local_entry *tt_local_hash_find(struct bat_priv *bat_priv, struct hlist_head *head; struct hlist_node *node; struct tt_local_entry *tt_local_entry, *tt_local_entry_tmp = NULL; - int index; + uint32_t index;
if (!hash) return NULL; @@ -99,7 +99,7 @@ static struct tt_global_entry *tt_global_hash_find(struct bat_priv *bat_priv, struct hlist_node *node; struct tt_global_entry *tt_global_entry; struct tt_global_entry *tt_global_entry_tmp = NULL; - int index; + uint32_t index;
if (!hash) return NULL; @@ -316,7 +316,8 @@ int tt_local_seq_print_text(struct seq_file *seq, void *offset) struct hlist_head *head; size_t buf_size, pos; char *buff; - int i, ret = 0; + uint32_t i; + int ret = 0;
primary_if = primary_if_get_selected(bat_priv); if (!primary_if) { @@ -427,7 +428,7 @@ static void tt_local_purge(struct bat_priv *bat_priv) struct hlist_node *node, *node_tmp; struct hlist_head *head; spinlock_t *list_lock; /* protects write access to the hash lists */ - int i; + uint32_t i;
for (i = 0; i < hash->size; i++) { head = &hash->table[i]; @@ -465,7 +466,7 @@ static void tt_local_table_free(struct bat_priv *bat_priv) struct tt_local_entry *tt_local_entry; struct hlist_node *node, *node_tmp; struct hlist_head *head; - int i; + uint32_t i;
if (!bat_priv->tt_local_hash) return; @@ -592,7 +593,8 @@ int tt_global_seq_print_text(struct seq_file *seq, void *offset) struct hlist_head *head; size_t buf_size, pos; char *buff; - int i, ret = 0; + uint32_t i; + int ret = 0;
primary_if = primary_if_get_selected(bat_priv); if (!primary_if) { @@ -716,7 +718,7 @@ void tt_global_del_orig(struct bat_priv *bat_priv, struct orig_node *orig_node, const char *message) { struct tt_global_entry *tt_global_entry; - int i; + uint32_t i; struct hashtable_t *hash = bat_priv->tt_global_hash; struct hlist_node *node, *safe; struct hlist_head *head; @@ -755,7 +757,7 @@ static void tt_global_roam_purge(struct bat_priv *bat_priv) struct hlist_node *node, *node_tmp; struct hlist_head *head; spinlock_t *list_lock; /* protects write access to the hash lists */ - int i; + uint32_t i;
for (i = 0; i < hash->size; i++) { head = &hash->table[i]; @@ -789,7 +791,7 @@ static void tt_global_table_free(struct bat_priv *bat_priv) struct tt_global_entry *tt_global_entry; struct hlist_node *node, *node_tmp; struct hlist_head *head; - int i; + uint32_t i;
if (!bat_priv->tt_global_hash) return; @@ -875,7 +877,8 @@ uint16_t tt_global_crc(struct bat_priv *bat_priv, struct orig_node *orig_node) struct tt_global_entry *tt_global_entry; struct hlist_node *node; struct hlist_head *head; - int i, j; + uint32_t i; + int j;
for (i = 0; i < hash->size; i++) { head = &hash->table[i]; @@ -912,7 +915,8 @@ uint16_t tt_local_crc(struct bat_priv *bat_priv) struct tt_local_entry *tt_local_entry; struct hlist_node *node; struct hlist_head *head; - int i, j; + uint32_t i; + int j;
for (i = 0; i < hash->size; i++) { head = &hash->table[i]; @@ -1049,7 +1053,7 @@ static struct sk_buff *tt_response_fill_table(uint16_t tt_len, uint8_t ttvn, struct sk_buff *skb = NULL; uint16_t tt_tot, tt_count; ssize_t tt_query_size = sizeof(struct tt_query_packet); - int i; + uint32_t i;
if (tt_query_size + tt_len > primary_if->soft_iface->mtu) { tt_len = primary_if->soft_iface->mtu - tt_query_size; @@ -1726,7 +1730,7 @@ void tt_free(struct bat_priv *bat_priv) * entry */ static void tt_local_reset_flags(struct bat_priv *bat_priv, uint16_t flags) { - int i; + uint32_t i; struct hashtable_t *hash = bat_priv->tt_local_hash; struct hlist_head *head; struct hlist_node *node; @@ -1759,7 +1763,7 @@ static void tt_local_purge_pending_clients(struct bat_priv *bat_priv) struct hlist_node *node, *node_tmp; struct hlist_head *head; spinlock_t *list_lock; /* protects write access to the hash lists */ - int i; + uint32_t i;
if (!hash) return; diff --git a/net/batman-adv/vis.c b/net/batman-adv/vis.c index f81a6b6..7445413 100644 --- a/net/batman-adv/vis.c +++ b/net/batman-adv/vis.c @@ -66,7 +66,7 @@ static int vis_info_cmp(const struct hlist_node *node, const void *data2)
/* hash function to choose an entry in a hash table of given size */ /* hash algorithm from http://en.wikipedia.org/wiki/Hash_table */ -static int vis_info_choose(const void *data, int size) +static uint32_t vis_info_choose(const void *data, uint32_t size) { const struct vis_info *vis_info = data; const struct vis_packet *packet; @@ -96,7 +96,7 @@ static struct vis_info *vis_hash_find(struct bat_priv *bat_priv, struct hlist_head *head; struct hlist_node *node; struct vis_info *vis_info, *vis_info_tmp = NULL; - int index; + uint32_t index;
if (!hash) return NULL; @@ -202,7 +202,8 @@ int vis_seq_print_text(struct seq_file *seq, void *offset) HLIST_HEAD(vis_if_list); struct if_list_entry *entry; struct hlist_node *pos, *n; - int i, j, ret = 0; + uint32_t i; + int j, ret = 0; int vis_server = atomic_read(&bat_priv->vis_mode); size_t buff_pos, buf_size; char *buff; @@ -556,7 +557,8 @@ static int find_best_vis_server(struct bat_priv *bat_priv, struct hlist_head *head; struct orig_node *orig_node; struct vis_packet *packet; - int best_tq = -1, i; + int best_tq = -1; + uint32_t i;
packet = (struct vis_packet *)info->skb_packet->data;
@@ -608,7 +610,8 @@ static int generate_vis_packet(struct bat_priv *bat_priv) struct vis_packet *packet = (struct vis_packet *)info->skb_packet->data; struct vis_info_entry *entry; struct tt_local_entry *tt_local_entry; - int best_tq = -1, i; + int best_tq = -1; + uint32_t i;
info->first_seen = jiffies; packet->vis_type = atomic_read(&bat_priv->vis_mode); @@ -696,7 +699,7 @@ unlock: * held */ static void purge_vis_packets(struct bat_priv *bat_priv) { - int i; + uint32_t i; struct hashtable_t *hash = bat_priv->vis_hash; struct hlist_node *node, *node_tmp; struct hlist_head *head; @@ -733,7 +736,7 @@ static void broadcast_vis_packet(struct bat_priv *bat_priv, struct sk_buff *skb; struct hard_iface *hard_iface; uint8_t dstaddr[ETH_ALEN]; - int i; + uint32_t i;
packet = (struct vis_packet *)info->skb_packet->data;
From: Simon Wunderlich simon.wunderlich@s2003.tu-chemnitz.de
When the translation tables (global and local) are written for debugfs, it is not neccesary to allocate a buffer, we can directly use seq_printf() to print them out.
This might actually be safer if the table changes between size calculation and traversal, and we can't estimate the required size wrong.
Signed-off-by: Simon Wunderlich siwu@hrz.tu-chemnitz.de Signed-off-by: Sven Eckelmann sven@narfation.org --- net/batman-adv/translation-table.c | 57 +---------------------------------- 1 files changed, 2 insertions(+), 55 deletions(-)
diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c index 5f28a7f0..78b9528 100644 --- a/net/batman-adv/translation-table.c +++ b/net/batman-adv/translation-table.c @@ -314,8 +314,6 @@ int tt_local_seq_print_text(struct seq_file *seq, void *offset) struct hard_iface *primary_if; struct hlist_node *node; struct hlist_head *head; - size_t buf_size, pos; - char *buff; uint32_t i; int ret = 0;
@@ -338,34 +336,13 @@ int tt_local_seq_print_text(struct seq_file *seq, void *offset) "announced via TT (TTVN: %u):\n", net_dev->name, (uint8_t)atomic_read(&bat_priv->ttvn));
- buf_size = 1; - /* Estimate length for: " * xx:xx:xx:xx:xx:xx\n" */ - for (i = 0; i < hash->size; i++) { - head = &hash->table[i]; - - rcu_read_lock(); - __hlist_for_each_rcu(node, head) - buf_size += 29; - rcu_read_unlock(); - } - - buff = kmalloc(buf_size, GFP_ATOMIC); - if (!buff) { - ret = -ENOMEM; - goto out; - } - - buff[0] = '\0'; - pos = 0; - for (i = 0; i < hash->size; i++) { head = &hash->table[i];
rcu_read_lock(); hlist_for_each_entry_rcu(tt_local_entry, node, head, hash_entry) { - pos += snprintf(buff + pos, 30, " * %pM " - "[%c%c%c%c%c]\n", + seq_printf(seq, " * %pM [%c%c%c%c%c]\n", tt_local_entry->addr, (tt_local_entry->flags & TT_CLIENT_ROAM ? 'R' : '.'), @@ -380,9 +357,6 @@ int tt_local_seq_print_text(struct seq_file *seq, void *offset) } rcu_read_unlock(); } - - seq_printf(seq, "%s", buff); - kfree(buff); out: if (primary_if) hardif_free_ref(primary_if); @@ -591,8 +565,6 @@ int tt_global_seq_print_text(struct seq_file *seq, void *offset) struct hard_iface *primary_if; struct hlist_node *node; struct hlist_head *head; - size_t buf_size, pos; - char *buff; uint32_t i; int ret = 0;
@@ -617,35 +589,13 @@ int tt_global_seq_print_text(struct seq_file *seq, void *offset) seq_printf(seq, " %-13s %s %-15s %s %s\n", "Client", "(TTVN)", "Originator", "(Curr TTVN)", "Flags");
- buf_size = 1; - /* Estimate length for: " * xx:xx:xx:xx:xx:xx (ttvn) via - * xx:xx:xx:xx:xx:xx (cur_ttvn)\n"*/ - for (i = 0; i < hash->size; i++) { - head = &hash->table[i]; - - rcu_read_lock(); - __hlist_for_each_rcu(node, head) - buf_size += 67; - rcu_read_unlock(); - } - - buff = kmalloc(buf_size, GFP_ATOMIC); - if (!buff) { - ret = -ENOMEM; - goto out; - } - - buff[0] = '\0'; - pos = 0; - for (i = 0; i < hash->size; i++) { head = &hash->table[i];
rcu_read_lock(); hlist_for_each_entry_rcu(tt_global_entry, node, head, hash_entry) { - pos += snprintf(buff + pos, 69, - " * %pM (%3u) via %pM (%3u) " + seq_printf(seq, " * %pM (%3u) via %pM (%3u) " "[%c%c%c]\n", tt_global_entry->addr, tt_global_entry->ttvn, tt_global_entry->orig_node->orig, @@ -661,9 +611,6 @@ int tt_global_seq_print_text(struct seq_file *seq, void *offset) } rcu_read_unlock(); } - - seq_printf(seq, "%s", buff); - kfree(buff); out: if (primary_if) hardif_free_ref(primary_if);
From: Antonio Quartulli ordex@autistici.org
The TT_RESPONSE skb has to be linearised only if the node plans to access the packet payload (so only if the message is directed to that node). In all the other cases the node can avoid this memory operation
Signed-off-by: Antonio Quartulli ordex@autistici.org Signed-off-by: Sven Eckelmann sven@narfation.org --- net/batman-adv/routing.c | 11 ++++++----- 1 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/net/batman-adv/routing.c b/net/batman-adv/routing.c index 60ce407..e0e7b7b 100644 --- a/net/batman-adv/routing.c +++ b/net/batman-adv/routing.c @@ -616,13 +616,14 @@ int recv_tt_query(struct sk_buff *skb, struct hard_iface *recv_if) } break; case TT_RESPONSE: - /* packet needs to be linearized to access the TT changes */ - if (skb_linearize(skb) < 0) - goto out; + if (is_my_mac(tt_query->dst)) { + /* packet needs to be linearized to access the TT + * changes */ + if (skb_linearize(skb) < 0) + goto out;
- if (is_my_mac(tt_query->dst)) handle_tt_response(bat_priv, tt_query); - else { + } else { bat_dbg(DBG_TT, bat_priv, "Routing TT_RESPONSE to %pM [%c]\n", tt_query->dst,
From: Antonio Quartulli ordex@autistici.org
Before accessing the TT_RESPONSE packet payload, the node has to ensure that the packet is long enough as it would expect to be.
Reported-by: Simon Wunderlich siwu@hrz.tu-chemnitz.de Signed-off-by: Antonio Quartulli ordex@autistici.org Signed-off-by: Sven Eckelmann sven@narfation.org --- net/batman-adv/routing.c | 9 +++++++++ 1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/net/batman-adv/routing.c b/net/batman-adv/routing.c index e0e7b7b..ef24a72 100644 --- a/net/batman-adv/routing.c +++ b/net/batman-adv/routing.c @@ -578,6 +578,7 @@ int recv_tt_query(struct sk_buff *skb, struct hard_iface *recv_if) { struct bat_priv *bat_priv = netdev_priv(recv_if->soft_iface); struct tt_query_packet *tt_query; + uint16_t tt_len; struct ethhdr *ethhdr;
/* drop packet if it has not necessary minimum size */ @@ -622,6 +623,14 @@ int recv_tt_query(struct sk_buff *skb, struct hard_iface *recv_if) if (skb_linearize(skb) < 0) goto out;
+ tt_len = tt_query->tt_data * sizeof(struct tt_change); + + /* Ensure we have all the claimed data */ + if (unlikely(skb_headlen(skb) < + sizeof(struct tt_query_packet) + + tt_len)) + goto out; + handle_tt_response(bat_priv, tt_query); } else { bat_dbg(DBG_TT, bat_priv,
From: Simon Wunderlich simon.wunderlich@s2003.tu-chemnitz.de
The check for new packets in the future used a wrong binary operator, which makes the check expression always true and accepting too many packets.
Reported-by: Thomas Jarosch thomas.jarosch@intra2net.com Signed-off-by: Simon Wunderlich siwu@hrz.tu-chemnitz.de Signed-off-by: Sven Eckelmann sven@narfation.org --- net/batman-adv/bitarray.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/batman-adv/bitarray.c b/net/batman-adv/bitarray.c index 0be9ff3..9bc63b2 100644 --- a/net/batman-adv/bitarray.c +++ b/net/batman-adv/bitarray.c @@ -155,7 +155,7 @@ int bit_get_packet(void *priv, unsigned long *seq_bits, /* sequence number is much newer, probably missed a lot of packets */
if ((seq_num_diff >= TQ_LOCAL_WINDOW_SIZE) - || (seq_num_diff < EXPECTED_SEQNO_RANGE)) { + && (seq_num_diff < EXPECTED_SEQNO_RANGE)) { bat_dbg(DBG_BATMAN, bat_priv, "We missed a lot of packets (%i) !\n", seq_num_diff - 1);
From: Marek Lindner lindner_marek@yahoo.de Date: Sat, 26 Nov 2011 22:26:42 +0800
the following 10 patches constitute the first batch I'd like to get the pulled into net-next-2.6/3.3. They're mostly uncritical fixes around the recently introduced tt code, some code refactoring, the kstrto update and the range check fix reported by Thomas Jarosch.
Pulled, thanks.
Some things to look into:
+ if (unlikely(skb_headlen(skb) < + sizeof(struct tt_query_packet) + + tt_len))
This isn't formatted correctly, all the leading edges should line up to the openning parenthesis of the unlikely:
+ if (unlikely(skb_headlen(skb) < + sizeof(struct tt_query_packet) + + tt_len))
Next, there is a lot of linearization done by the stack, but really the thing to do is to make sure that the part you want to look at is linear.
You do this using pskb_may_pull() right before you want to look at some headers. It makes sure that, for the length given, that many bytes are linear at the head of the skb.
Thanks.
Hello David,
On Sat, Nov 26, 2011 at 02:41:22 -0500, David Miller wrote: [CUT]
Some things to look into:
if (unlikely(skb_headlen(skb) <sizeof(struct tt_query_packet) +tt_len))This isn't formatted correctly, all the leading edges should line up to the openning parenthesis of the unlikely:
if (unlikely(skb_headlen(skb) <sizeof(struct tt_query_packet) +tt_len))
Thank you for reporting this issue. We have already prepared a patch which is going to be sent within the next batch.
Next, there is a lot of linearization done by the stack, but really the thing to do is to make sure that the part you want to look at is linear.
You do this using pskb_may_pull() right before you want to look at some headers. It makes sure that, for the length given, that many bytes are linear at the head of the skb.
For this issue, we had some problem to understand it.
First of all I think you are referring to patch 08/10, in which I moved a skb_linearise() operation.
To be sure it is really needed, I backtracked the code flow and noted down any eventual psbk_may_pull() (or any other linearisation operation). The result is:
=> in batman_skb_recv() - pskb_may_pull(2) => in recv_tt_query() - pskb_may_pull(sizeof(header)) - skb_linearise()
Actually it seems we haven't any useless linearisation. Would you mind explain us where you actually found the problem, please?
It might also be that I misunderstood your advice.
Thank you.
Best Regards,
From: Antonio Quartulli ordex@autistici.org Date: Fri, 2 Dec 2011 18:12:16 +0100
First of all I think you are referring to patch 08/10, in which I moved a skb_linearise() operation.
To be sure it is really needed, I backtracked the code flow and noted down any eventual psbk_may_pull() (or any other linearisation operation). The result is:
=> in batman_skb_recv()
- pskb_may_pull(2)
=> in recv_tt_query() - pskb_may_pull(sizeof(header)) - skb_linearise()
Actually it seems we haven't any useless linearisation. Would you mind explain us where you actually found the problem, please?
It might also be that I misunderstood your advice.
You only need to call pskb_may_pull() on the parts of the packet you want to access directly to parse headers etc.
If you use that interface properly, you never need to linearize, ever.
On Fri, Dec 02, 2011 at 12:57:25 -0500, David Miller wrote:
From: Antonio Quartulli ordex@autistici.org Date: Fri, 2 Dec 2011 18:12:16 +0100
First of all I think you are referring to patch 08/10, in which I moved a skb_linearise() operation.
To be sure it is really needed, I backtracked the code flow and noted down any eventual psbk_may_pull() (or any other linearisation operation). The result is:
=> in batman_skb_recv()
- pskb_may_pull(2)
=> in recv_tt_query() - pskb_may_pull(sizeof(header)) - skb_linearise()
Actually it seems we haven't any useless linearisation. Would you mind explain us where you actually found the problem, please?
It might also be that I misunderstood your advice.
You only need to call pskb_may_pull() on the parts of the packet you want to access directly to parse headers etc.
If you use that interface properly, you never need to linearize, ever.
Sorry for being too generic: we actually invoke skb_linearise() because we want to access the whole skb.
We first call pskb_may_pull() to pull the header only and then, under certain conditions, we linearise the whole skb to access it all. Should I use pskb_may_pull() even in this case?
Sorry for stealing you so much time on this issue, but I would like to fully understand it in order to avoid any further mistake.
Thank you again.
Best Regards,
From: Antonio Quartulli ordex@autistici.org Date: Sat, 3 Dec 2011 02:55:29 +0100
We first call pskb_may_pull() to pull the header only and then, under certain conditions, we linearise the whole skb to access it all. Should I use pskb_may_pull() even in this case?
Why would you need to access the whole thing?
There are only two types of possible accesses:
1) Header parsing --> use pskb_may_pull() as needed.
2) Copying the data to some other location, such as a user buffer. Use skb_copy_datagram_iovec or similar which handle fragmented SKBs just fine.
You should handle fragmented SKBs as much as possible, because linearization is expensive and often amounts to a memory allocation plus a copy if you linearize the whole thing.
On Fri, Dec 02, 2011 at 08:59:55PM -0500, David Miller wrote:
From: Antonio Quartulli ordex@autistici.org Date: Sat, 3 Dec 2011 02:55:29 +0100
We first call pskb_may_pull() to pull the header only and then, under certain conditions, we linearise the whole skb to access it all. Should I use pskb_may_pull() even in this case?
Why would you need to access the whole thing?
It contains only information for the routing/pathfinding to client nodes (nodes which do not directly participate in this mesh).
There are only two types of possible accesses:
Header parsing --> use pskb_may_pull() as needed.
Copying the data to some other location, such as a user buffer. Use skb_copy_datagram_iovec or similar which handle fragmented SKBs just fine.
I think a "copy" using skb_header_pointer/skb_copy_bits would be the right thing to do. batman-adv needs the data for kernel data structures and don't send it to a userspace program (at least not as primary functionality). I don't have benchmarks which compare both solutions (with a heavily fragmented skb) when accessing small, packed structures in a skb.
Thanks, Sven
b.a.t.m.a.n@lists.open-mesh.org
-
Antonio Quartulli -
David Miller -
Marek Lindner -
Sven Eckelmann