[B.A.T.M.A.N.] [PATCH] batman-adv: Do not add multicast MAC addresses to translation table
The current translation table mechanism is not suitable for multicast addresses and we are currently flooding such frames anyway. Therefore this patch prevents multicast MAC addresses being added to the translation table. Signed-off-by: Linus Lüssing linus.luess...@web.de --- soft-interface.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/soft-interface.c b/soft-interface.c index 2d1f895..9955319 100644 --- a/soft-interface.c +++ b/soft-interface.c @@ -180,7 +180,8 @@ static int batadv_interface_tx(struct sk_buff *skb, goto dropped; /* Register the client MAC in the transtable */ - batadv_tt_local_add(soft_iface, ethhdr-h_source, skb-skb_iif); + if (!is_multicast_ether_addr(ethhdr-h_source)) + batadv_tt_local_add(soft_iface, ethhdr-h_source, skb-skb_iif); /* don't accept stp packets. STP does not help in meshes. * better use the bridge loop avoidance ... -- 1.7.10.4
Re: [B.A.T.M.A.N.] [PATCH] batman-adv: Fix broadcast packet CRC calculation
On Wednesday 17 October 2012 13:38:30 Linus Lüssing wrote: So far the crc16 checksum for a batman-adv broadcast data packet, received on a batman-adv hard interface, was calculated over zero bytes of its content leading to many incoming broadcast data packets wrongly being dropped. This patch fixes this issue by calculating the crc16 over the actual, complete broadcast payload. The issue is a regrission introduced by e5cf86d30a9b32feccb6b866a3f23b0e0483a3b0. Signed-off-by: Linus Lüssing linus.luess...@web.de --- It led to about 60-80% broadcast packet loss to a direct neighbor with a ping6 with a 1s interval in our scenario, but about no packet loss with an interval smaller than 0.2s. Also see: https://projects.universe-factory.net/issues/65 (German) bridge_loop_avoidance.c |8 routing.c |8 +++- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/bridge_loop_avoidance.c b/bridge_loop_avoidance.c index 6705d35..e7b5777 100644 --- a/bridge_loop_avoidance.c +++ b/bridge_loop_avoidance.c @@ -1205,8 +1205,8 @@ int batadv_bla_init(struct batadv_priv *bat_priv) /** * batadv_bla_check_bcast_duplist * @bat_priv: the bat priv with all the soft interface information - * @bcast_packet: originator mac address - * @hdr_size: maximum length of the frame + * @bcast_packet: encapsulated broadcast frame plus batman header + * @bcast_packet_len: length of encapsulated broadcast frame plus batman header * * check if it is on our broadcast list. Another gateway might * have sent the same packet because it is connected to the same backbone, @@ -1219,14 +1219,14 @@ int batadv_bla_init(struct batadv_priv *bat_priv) */ int batadv_bla_check_bcast_duplist(struct batadv_priv *bat_priv, struct batadv_bcast_packet *bcast_packet, -int hdr_size) +int bcast_packet_len) { int i, length, curr; uint8_t *content; uint16_t crc; struct batadv_bcast_duplist_entry *entry; - length = hdr_size - sizeof(*bcast_packet); + length = bcast_packet_len - sizeof(*bcast_packet); content = (uint8_t *)bcast_packet; content += sizeof(*bcast_packet); diff --git a/routing.c b/routing.c index bc2b88b..f861b7c 100644 --- a/routing.c +++ b/routing.c @@ -1136,8 +1136,14 @@ int batadv_recv_bcast_packet(struct sk_buff *skb, spin_unlock_bh(orig_node-bcast_seqno_lock); + /* keep skb linear for crc calculation */ + if (skb_linearize(skb) 0) + goto out; Please don't ues linearize here. Try to fix the crc calculation using skb_walk_frags (see for example net/netfilter/nf_nat_proto_sctp.c ). And Simon owes you a beer for this debugging ;) Kind regards, Sven signature.asc Description: This is a digitally signed message part.
Re: [B.A.T.M.A.N.] [PATCH] batman-adv: Do not add multicast MAC addresses to translation table
On Wed, Oct 17, 2012 at 03:07:35PM +0200, Linus Lüssing wrote: The current translation table mechanism is not suitable for multicast addresses and we are currently flooding such frames anyway. Therefore this patch prevents multicast MAC addresses being added to the translation table. Signed-off-by: Linus Lüssing linus.luess...@web.de --- soft-interface.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/soft-interface.c b/soft-interface.c index 2d1f895..9955319 100644 --- a/soft-interface.c +++ b/soft-interface.c @@ -180,7 +180,8 @@ static int batadv_interface_tx(struct sk_buff *skb, goto dropped; /* Register the client MAC in the transtable */ - batadv_tt_local_add(soft_iface, ethhdr-h_source, skb-skb_iif); + if (!is_multicast_ether_addr(ethhdr-h_source)) + batadv_tt_local_add(soft_iface, ethhdr-h_source, skb-skb_iif); How can the source address be multicast? Usually multicast addresses are found in the destination field... Is there any scenario where this is possible? I am wondering whether we should directly drop packet having mcast address as source.. Cheers, -- Antonio Quartulli ..each of us alone is worth nothing.. Ernesto Che Guevara pgpBZEfTYvk04.pgp Description: PGP signature
Re: [B.A.T.M.A.N.] [PATCH] batman-adv: Fix broadcast packet CRC calculation
On Wednesday 17 October 2012 13:38:30 Linus Lüssing wrote: So far the crc16 checksum for a batman-adv broadcast data packet, received on a batman-adv hard interface, was calculated over zero bytes of its content leading to many incoming broadcast data packets wrongly being dropped. This patch fixes this issue by calculating the crc16 over the actual, complete broadcast payload. The issue is a regrission introduced by e5cf86d30a9b32feccb6b866a3f23b0e0483a3b0. Signed-off-by: Linus Lüssing linus.luess...@web.de --- It led to about 60-80% broadcast packet loss to a direct neighbor with a ping6 with a 1s interval in our scenario, but about no packet loss with an interval smaller than 0.2s. Also see: https://projects.universe-factory.net/issues/65 (German) bridge_loop_avoidance.c |8 routing.c |8 +++- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/bridge_loop_avoidance.c b/bridge_loop_avoidance.c index 6705d35..e7b5777 100644 --- a/bridge_loop_avoidance.c +++ b/bridge_loop_avoidance.c @@ -1205,8 +1205,8 @@ int batadv_bla_init(struct batadv_priv *bat_priv) /** * batadv_bla_check_bcast_duplist * @bat_priv: the bat priv with all the soft interface information - * @bcast_packet: originator mac address - * @hdr_size: maximum length of the frame + * @bcast_packet: encapsulated broadcast frame plus batman header + * @bcast_packet_len: length of encapsulated broadcast frame plus batman header * * check if it is on our broadcast list. Another gateway might * have sent the same packet because it is connected to the same backbone, @@ -1219,14 +1219,14 @@ int batadv_bla_init(struct batadv_priv *bat_priv) */ int batadv_bla_check_bcast_duplist(struct batadv_priv *bat_priv, struct batadv_bcast_packet *bcast_packet, -int hdr_size) +int bcast_packet_len) { int i, length, curr; uint8_t *content; uint16_t crc; struct batadv_bcast_duplist_entry *entry; - length = hdr_size - sizeof(*bcast_packet); + length = bcast_packet_len - sizeof(*bcast_packet); content = (uint8_t *)bcast_packet; content += sizeof(*bcast_packet); diff --git a/routing.c b/routing.c index bc2b88b..f861b7c 100644 --- a/routing.c +++ b/routing.c @@ -1136,8 +1136,14 @@ int batadv_recv_bcast_packet(struct sk_buff *skb, spin_unlock_bh(orig_node-bcast_seqno_lock); + /* keep skb linear for crc calculation */ + if (skb_linearize(skb) 0) + goto out; + + bcast_packet = (struct batadv_bcast_packet *)skb-data; + /* check whether this has been sent by another originator before */ - if (batadv_bla_check_bcast_duplist(bat_priv, bcast_packet, hdr_size)) + if (batadv_bla_check_bcast_duplist(bat_priv, bcast_packet, skb-len)) goto out; /* rebroadcast packet */ Btw. why should renaming the hdr_size to bcast_packet_len help to get a different value than 0? Kind regards, Sven signature.asc Description: This is a digitally signed message part.
Re: [B.A.T.M.A.N.] [PATCH] batman-adv: Fix broadcast packet CRC calculation
On Wednesday 17 October 2012 15:50:50 Sven Eckelmann wrote: On Wednesday 17 October 2012 13:38:30 Linus Lüssing wrote: So far the crc16 checksum for a batman-adv broadcast data packet, received on a batman-adv hard interface, was calculated over zero bytes of its content leading to many incoming broadcast data packets wrongly being dropped. This patch fixes this issue by calculating the crc16 over the actual, complete broadcast payload. The issue is a regrission introduced by e5cf86d30a9b32feccb6b866a3f23b0e0483a3b0. Signed-off-by: Linus Lüssing linus.luess...@web.de --- It led to about 60-80% broadcast packet loss to a direct neighbor with a ping6 with a 1s interval in our scenario, but about no packet loss with an interval smaller than 0.2s. Also see: https://projects.universe-factory.net/issues/65 (German) bridge_loop_avoidance.c |8 routing.c |8 +++- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/bridge_loop_avoidance.c b/bridge_loop_avoidance.c index 6705d35..e7b5777 100644 --- a/bridge_loop_avoidance.c +++ b/bridge_loop_avoidance.c @@ -1205,8 +1205,8 @@ int batadv_bla_init(struct batadv_priv *bat_priv) /** * batadv_bla_check_bcast_duplist * @bat_priv: the bat priv with all the soft interface information - * @bcast_packet: originator mac address - * @hdr_size: maximum length of the frame + * @bcast_packet: encapsulated broadcast frame plus batman header + * @bcast_packet_len: length of encapsulated broadcast frame plus batman header * * check if it is on our broadcast list. Another gateway might * have sent the same packet because it is connected to the same backbone, @@ -1219,14 +1219,14 @@ int batadv_bla_init(struct batadv_priv *bat_priv) */ int batadv_bla_check_bcast_duplist(struct batadv_priv *bat_priv, struct batadv_bcast_packet *bcast_packet, - int hdr_size) + int bcast_packet_len) { int i, length, curr; uint8_t *content; uint16_t crc; struct batadv_bcast_duplist_entry *entry; - length = hdr_size - sizeof(*bcast_packet); + length = bcast_packet_len - sizeof(*bcast_packet); content = (uint8_t *)bcast_packet; content += sizeof(*bcast_packet); diff --git a/routing.c b/routing.c index bc2b88b..f861b7c 100644 --- a/routing.c +++ b/routing.c @@ -1136,8 +1136,14 @@ int batadv_recv_bcast_packet(struct sk_buff *skb, spin_unlock_bh(orig_node-bcast_seqno_lock); + /* keep skb linear for crc calculation */ + if (skb_linearize(skb) 0) + goto out; + + bcast_packet = (struct batadv_bcast_packet *)skb-data; + /* check whether this has been sent by another originator before */ - if (batadv_bla_check_bcast_duplist(bat_priv, bcast_packet, hdr_size)) + if (batadv_bla_check_bcast_duplist(bat_priv, bcast_packet, skb-len)) goto out; /* rebroadcast packet */ Btw. why should renaming the hdr_size to bcast_packet_len help to get a different value than 0? Ok, Marek just noticed the skb-len :D Kind regards, Sven signature.asc Description: This is a digitally signed message part.
Re: [B.A.T.M.A.N.] [PATCHv2 2/2] batman-adv: Fix potential broadcast BLA-duplicate-check race condition
Looks good, although I don't think this will happen very often - it is only called when receiving packets, and AFAIK there can only be multiple threads when using multiple interfaces. Anyway, doing the spinlock shouldn't hurt and might save us some pain when this function is used more often, or with multiple interfaces, or cases I don't see now. :) Acked-by: Simon Wunderlich s...@hrz.tu-chemnitz.de On Wed, Oct 17, 2012 at 02:53:05PM +0200, Linus Lüssing wrote: Threads in the bottom half of batadv_bla_check_bcast_duplist() might otherwise for instance overwrite variables which other threads might be using/reading at the same time in the top half, potentially leading to messing up the bcast_duplist, possibly resulting in false bridge loop avoidance duplicate check decisions. Signed-off-by: Linus Lüssing linus.luess...@web.de --- Note: I didn't observe such issues yet, probably because of the usually quite low broadcast data throughput - and on high broadcast throughput, then packet loss is usually caused by the limited capacity for broadcast packets on 802.11 media anyway. bridge_loop_avoidance.c | 16 +++- main.c |1 + types.h |2 ++ 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/bridge_loop_avoidance.c b/bridge_loop_avoidance.c index b828875..c6c1c59 100644 --- a/bridge_loop_avoidance.c +++ b/bridge_loop_avoidance.c @@ -1263,7 +1263,7 @@ int batadv_bla_check_bcast_duplist(struct batadv_priv *bat_priv, struct batadv_bcast_packet *bcast_packet, int bcast_packet_len) { - int i, length, curr; + int i, length, curr, ret = 0; uint8_t *content; uint16_t crc; struct batadv_bcast_duplist_entry *entry; @@ -1275,6 +1275,8 @@ int batadv_bla_check_bcast_duplist(struct batadv_priv *bat_priv, /* calculate the crc ... */ crc = crc16(0, content, length); + spin_lock_bh(bat_priv-bla.bcast_duplist_lock); + for (i = 0; i BATADV_DUPLIST_SIZE; i++) { curr = (bat_priv-bla.bcast_duplist_curr + i); curr %= BATADV_DUPLIST_SIZE; @@ -1296,9 +1298,11 @@ int batadv_bla_check_bcast_duplist(struct batadv_priv *bat_priv, /* this entry seems to match: same crc, not too old, * and from another gw. therefore return 1 to forbid it. */ - return 1; + ret = 1; + goto out; } - /* not found, add a new entry (overwrite the oldest entry) */ + /* not found, add a new entry (overwrite the oldest entry) + * and allow it, its the first occurence. */ curr = (bat_priv-bla.bcast_duplist_curr + BATADV_DUPLIST_SIZE - 1); curr %= BATADV_DUPLIST_SIZE; entry = bat_priv-bla.bcast_duplist[curr]; @@ -1307,8 +1311,10 @@ int batadv_bla_check_bcast_duplist(struct batadv_priv *bat_priv, memcpy(entry-orig, bcast_packet-orig, ETH_ALEN); bat_priv-bla.bcast_duplist_curr = curr; - /* allow it, its the first occurence. */ - return 0; +out: + spin_unlock_bh(bat_priv-bla.bcast_duplist_lock); + + return ret; } diff --git a/main.c b/main.c index 70797de..f54efe9 100644 --- a/main.c +++ b/main.c @@ -102,6 +102,7 @@ int batadv_mesh_init(struct net_device *soft_iface) spin_lock_init(bat_priv-gw.list_lock); spin_lock_init(bat_priv-vis.hash_lock); spin_lock_init(bat_priv-vis.list_lock); + spin_lock_init(bat_priv-bla.bcast_duplist_lock); INIT_HLIST_HEAD(bat_priv-forw_bat_list); INIT_HLIST_HEAD(bat_priv-forw_bcast_list); diff --git a/types.h b/types.h index 8a5d84c..7b3d0d7 100644 --- a/types.h +++ b/types.h @@ -228,6 +228,8 @@ struct batadv_priv_bla { struct batadv_hashtable *backbone_hash; struct batadv_bcast_duplist_entry bcast_duplist[BATADV_DUPLIST_SIZE]; int bcast_duplist_curr; + /* protects bcast_duplist and bcast_duplist_curr */ + spinlock_t bcast_duplist_lock; struct batadv_bla_claim_dst claim_dest; struct delayed_work work; }; -- 1.7.10.4 signature.asc Description: Digital signature
Re: [B.A.T.M.A.N.] [PATCHv2 1/2] batman-adv: Fix broadcast packet CRC calculation
Good catch! When can I buy you the beer? :D Acked-by: Simon Wunderlich s...@hrz.tu-chemnitz.de On Wed, Oct 17, 2012 at 02:53:04PM +0200, Linus Lüssing wrote: So far the crc16 checksum for a batman-adv broadcast data packet, received on a batman-adv hard interface, was calculated over zero bytes of its content leading to many incoming broadcast data packets wrongly being dropped. This patch fixes this issue by calculating the crc16 over the actual, complete broadcast payload. The issue is a regression introduced by batman-adv: add broadcast duplicate check. Signed-off-by: Linus Lüssing linus.luess...@web.de --- It led to about 60-80% broadcast packet loss to a direct neighbor with a ping6 with a 1s interval in our scenario, but about no packet loss with an interval smaller than 0.2s. Also see: https://projects.universe-factory.net/issues/65 (German) v2: ~ fixed typo (regrission vs. regression) ~ commit name instead of commit hash in the commit message bridge_loop_avoidance.c |8 routing.c |8 +++- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/bridge_loop_avoidance.c b/bridge_loop_avoidance.c index a617f2c..b828875 100644 --- a/bridge_loop_avoidance.c +++ b/bridge_loop_avoidance.c @@ -1247,8 +1247,8 @@ int batadv_bla_init(struct batadv_priv *bat_priv) /** * batadv_bla_check_bcast_duplist * @bat_priv: the bat priv with all the soft interface information - * @bcast_packet: originator mac address - * @hdr_size: maximum length of the frame + * @bcast_packet: encapsulated broadcast frame plus batman header + * @bcast_packet_len: length of encapsulated broadcast frame plus batman header * * check if it is on our broadcast list. Another gateway might * have sent the same packet because it is connected to the same backbone, @@ -1261,14 +1261,14 @@ int batadv_bla_init(struct batadv_priv *bat_priv) */ int batadv_bla_check_bcast_duplist(struct batadv_priv *bat_priv, struct batadv_bcast_packet *bcast_packet, -int hdr_size) +int bcast_packet_len) { int i, length, curr; uint8_t *content; uint16_t crc; struct batadv_bcast_duplist_entry *entry; - length = hdr_size - sizeof(*bcast_packet); + length = bcast_packet_len - sizeof(*bcast_packet); content = (uint8_t *)bcast_packet; content += sizeof(*bcast_packet); diff --git a/routing.c b/routing.c index 6b2104d..5da62df 100644 --- a/routing.c +++ b/routing.c @@ -1181,8 +1181,14 @@ int batadv_recv_bcast_packet(struct sk_buff *skb, spin_unlock_bh(orig_node-bcast_seqno_lock); + /* keep skb linear for crc calculation */ + if (skb_linearize(skb) 0) + goto out; + + bcast_packet = (struct batadv_bcast_packet *)skb-data; + /* check whether this has been sent by another originator before */ - if (batadv_bla_check_bcast_duplist(bat_priv, bcast_packet, hdr_size)) + if (batadv_bla_check_bcast_duplist(bat_priv, bcast_packet, skb-len)) goto out; /* rebroadcast packet */ -- 1.7.10.4 signature.asc Description: Digital signature
[B.A.T.M.A.N.] [RFC] batman-adv: Add function to calculate crc32c for the skb payload
Signed-off-by: Sven Eckelmann s...@narfation.org --- main.c | 34 ++ main.h |1 + 2 files changed, 35 insertions(+) diff --git a/main.c b/main.c index 70797de..122c675 100644 --- a/main.c +++ b/main.c @@ -33,6 +33,7 @@ #include vis.h #include hash.h #include bat_algo.h +#include linux/crc32c.h /* List manipulations on hardif_list have to be rtnl_lock()'ed, @@ -432,6 +433,39 @@ int batadv_compat_seq_print_text(struct seq_file *seq, void *offset) return 0; } +/** + * batadv_crc32 - calculate CRC32 of the whole packet and skip bytes in header + * @skb: skb pointing to fragmented socket buffers + * @payload_ptr: Pointer to position inside the head buffer of the skb + * marking the start of the data to be CRC'ed + * + * payload_ptr must always point to the point in the skb head buffer and not to + * a fragment. + */ +__be32 batadv_crc32(const struct sk_buff *skb, u8 *payload_ptr) +{ + u32 crc = 0; + struct sk_buff *iter; + size_t skip_len, read_len; + const skb_frag_t *f; + int i; + + skip_len = payload_ptr - skb-data; + read_len = skb_headlen(skb) - skip_len; + if (read_len) + crc = crc32c(crc, payload_ptr, read_len); + + for (i = 0; i skb_shinfo(skb)-nr_frags; i++) { + f = skb_shinfo(skb)-frags[i]; + crc = crc32c(crc, skb_frag_address(f), skb_frag_size(f)); + } + + skb_walk_frags(skb, iter) + crc = crc32c(crc, iter-data, skb_headlen(iter)); + + return htonl(crc); +} + static int batadv_param_set_ra(const char *val, const struct kernel_param *kp) { struct batadv_algo_ops *bat_algo_ops; diff --git a/main.h b/main.h index 2dfcf8c..bb0a710 100644 --- a/main.h +++ b/main.h @@ -177,6 +177,7 @@ int batadv_algo_register(struct batadv_algo_ops *bat_algo_ops); int batadv_algo_select(struct batadv_priv *bat_priv, char *name); int batadv_algo_seq_print_text(struct seq_file *seq, void *offset); int batadv_compat_seq_print_text(struct seq_file *seq, void *offset); +__be32 batadv_crc32(const struct sk_buff *skb, u8 *payload_ptr); /** * enum batadv_dbg_level - available log levels -- 1.7.10.4
Re: [B.A.T.M.A.N.] [RFC] batman-adv: Add function to calculate crc32c for the skb payload
On 2012-10-17 17:59, Sven Eckelmann wrote: +__be32 batadv_crc32(const struct sk_buff *skb, u8 *payload_ptr) +{ + u32 crc = 0; + struct sk_buff *iter; + size_t skip_len, read_len; + const skb_frag_t *f; + int i; + + skip_len = payload_ptr - skb-data; + read_len = skb_headlen(skb) - skip_len; + if (read_len) + crc = crc32c(crc, payload_ptr, read_len); + + for (i = 0; i skb_shinfo(skb)-nr_frags; i++) { + f = skb_shinfo(skb)-frags[i]; + crc = crc32c(crc, skb_frag_address(f), skb_frag_size(f)); + } + + skb_walk_frags(skb, iter) + crc = crc32c(crc, iter-data, skb_headlen(iter)); + + return htonl(crc); +} I tested the crc'ing of non-linear skb buffers and can confirm that it works as expected, so: Tested-by: Martin Hundebøll mar...@hundeboll.net ... at least for batadv_crc32() :) -- Kind Regards Martin Hundebøll Frederiks Allé 99, 1.th 8000 Aarhus C Denmark +45 61 65 54 61 mar...@hundeboll.net
[B.A.T.M.A.N.] [RFCv2] batman-adv: Add function to calculate crc32c for the skb payload
Signed-off-by: Sven Eckelmann s...@narfation.org --- v2: - Added compat code for kernel like 2.6.39 - Use kmap_atomic to get the page compat.h | 20 main.c | 38 ++ main.h |1 + 3 files changed, 59 insertions(+) diff --git a/compat.h b/compat.h index 0caf43b..4e059e8 100644 --- a/compat.h +++ b/compat.h @@ -146,6 +146,26 @@ static inline void skb_reset_mac_len(struct sk_buff *skb) #endif /* KERNEL_VERSION(3, 0, 0) */ +#if LINUX_VERSION_CODE KERNEL_VERSION(3, 2, 0) + +static inline struct page *skb_frag_page(const skb_frag_t *frag) +{ + return frag-page; +} + +static inline void *skb_frag_address(const skb_frag_t *frag) +{ + return page_address(skb_frag_page(frag)) + frag-page_offset; +} + +static inline unsigned int skb_frag_size(const skb_frag_t *frag) +{ + return frag-size; +} + +#endif /* KERNEL_VERSION(3, 2, 0) */ + + #if LINUX_VERSION_CODE KERNEL_VERSION(3, 4, 0) static inline void eth_hw_addr_random(struct net_device *dev) diff --git a/main.c b/main.c index 70797de..009c9b0 100644 --- a/main.c +++ b/main.c @@ -33,6 +33,8 @@ #include vis.h #include hash.h #include bat_algo.h +#include linux/crc32c.h +#include linux/highmem.h /* List manipulations on hardif_list have to be rtnl_lock()'ed, @@ -432,6 +434,42 @@ int batadv_compat_seq_print_text(struct seq_file *seq, void *offset) return 0; } +/** + * batadv_crc32 - calculate CRC32 of the whole packet and skip bytes in header + * @skb: skb pointing to fragmented socket buffers + * @payload_ptr: Pointer to position inside the head buffer of the skb + * marking the start of the data to be CRC'ed + * + * payload_ptr must always point to the point in the skb head buffer and not to + * a fragment. + */ +__be32 batadv_crc32(const struct sk_buff *skb, u8 *payload_ptr) +{ + u32 crc = 0; + struct sk_buff *iter; + size_t skip_len, read_len; + const skb_frag_t *f; + u8 *vaddr; + int i; + + skip_len = payload_ptr - skb-data; + read_len = skb_headlen(skb) - skip_len; + if (read_len) + crc = crc32c(crc, payload_ptr, read_len); + + for (i = 0; i skb_shinfo(skb)-nr_frags; i++) { + f = skb_shinfo(skb)-frags[i]; + vaddr = kmap_atomic(skb_frag_page(f)); + crc = crc32c(crc, vaddr + f-page_offset, skb_frag_size(f)); + kunmap_atomic(vaddr); + } + + skb_walk_frags(skb, iter) + crc = crc32c(crc, iter-data, skb_headlen(iter)); + + return htonl(crc); +} + static int batadv_param_set_ra(const char *val, const struct kernel_param *kp) { struct batadv_algo_ops *bat_algo_ops; diff --git a/main.h b/main.h index 2dfcf8c..bb0a710 100644 --- a/main.h +++ b/main.h @@ -177,6 +177,7 @@ int batadv_algo_register(struct batadv_algo_ops *bat_algo_ops); int batadv_algo_select(struct batadv_priv *bat_priv, char *name); int batadv_algo_seq_print_text(struct seq_file *seq, void *offset); int batadv_compat_seq_print_text(struct seq_file *seq, void *offset); +__be32 batadv_crc32(const struct sk_buff *skb, u8 *payload_ptr); /** * enum batadv_dbg_level - available log levels -- 1.7.10.4
Re: [B.A.T.M.A.N.] [PATCHv2 1/2] batman-adv: Fix broadcast packet CRC calculation
On Wednesday, October 17, 2012 22:17:33 Simon Wunderlich wrote: Good catch! When can I buy you the beer? :D Acked-by: Simon Wunderlich s...@hrz.tu-chemnitz.de Applied in revision 2cebfac. Thanks, Marek
Re: [B.A.T.M.A.N.] [PATCHv2 2/2] batman-adv: Fix potential broadcast BLA-duplicate-check race condition
On Wednesday, October 17, 2012 22:17:05 Simon Wunderlich wrote: Looks good, although I don't think this will happen very often - it is only called when receiving packets, and AFAIK there can only be multiple threads when using multiple interfaces. Anyway, doing the spinlock shouldn't hurt and might save us some pain when this function is used more often, or with multiple interfaces, or cases I don't see now. :) Acked-by: Simon Wunderlich s...@hrz.tu-chemnitz.de Applied in revision 4c1721b. Thanks, Marek
Re: [B.A.T.M.A.N.] [PATCH] batman-adv: Do not add multicast MAC addresses to translation table
Gesendet: Mittwoch, 17. Oktober 2012 um 15:36 Uhr Von: Antonio Quartulli or...@autistici.org An: The list for a Better Approach To Mobile Ad-hoc Networking b.a.t.m.a.n@lists.open-mesh.org Betreff: Re: [B.A.T.M.A.N.] [PATCH] batman-adv: Do not add multicast MAC addresses to translation table On Wed, Oct 17, 2012 at 03:07:35PM +0200, Linus Lüssing wrote: The current translation table mechanism is not suitable for multicast addresses and we are currently flooding such frames anyway. Therefore this patch prevents multicast MAC addresses being added to the translation table. Signed-off-by: Linus Lüssing linus.luess...@web.de --- soft-interface.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/soft-interface.c b/soft-interface.c index 2d1f895..9955319 100644 --- a/soft-interface.c +++ b/soft-interface.c @@ -180,7 +180,8 @@ static int batadv_interface_tx(struct sk_buff *skb, goto dropped; /* Register the client MAC in the transtable */ - batadv_tt_local_add(soft_iface, ethhdr-h_source, skb-skb_iif); + if (!is_multicast_ether_addr(ethhdr-h_source)) + batadv_tt_local_add(soft_iface, ethhdr-h_source, skb-skb_iif); How can the source address be multicast? Usually multicast addresses are found in the destination field... Is there any scenario where this is possible? I am wondering whether we should directly drop packet having mcast address as source.. Hm, no, I do not have a specific scenario in mind. I only accidentally noticed such an FF:FF:FF:FF:FF:FF mac address in our vis graph here when I made a typo with the tool 'mausezahn'. I don't know of any RFC or so which would generally forbid using a multicast source MAC for every ether type (although for sane IPv4/v6 stacks I haven't seen something like this before either and yes, I bet it's not allowed there). The Linux bridge I had on top of bat0 was happily forwarding such frames into batman-adv. So I thought doing it like the bridge code - forwarding but with no learning - might be the right way? Cheers Cheers, -- Antonio Quartulli ..each of us alone is worth nothing.. Ernesto Che Guevara
Re: [B.A.T.M.A.N.] [PATCH] batman-adv: Do not add multicast MAC addresses to translation table
On Wed, Oct 17, 2012 at 08:43:05PM +0200, Linus Lüssing wrote: Gesendet: Mittwoch, 17. Oktober 2012 um 15:36 Uhr Von: Antonio Quartulli or...@autistici.org An: The list for a Better Approach To Mobile Ad-hoc Networking b.a.t.m.a.n@lists.open-mesh.org Betreff: Re: [B.A.T.M.A.N.] [PATCH] batman-adv: Do not add multicast MAC addresses to translation table On Wed, Oct 17, 2012 at 03:07:35PM +0200, Linus Lüssing wrote: The current translation table mechanism is not suitable for multicast addresses and we are currently flooding such frames anyway. Therefore this patch prevents multicast MAC addresses being added to the translation table. Signed-off-by: Linus Lüssing linus.luess...@web.de --- soft-interface.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/soft-interface.c b/soft-interface.c index 2d1f895..9955319 100644 --- a/soft-interface.c +++ b/soft-interface.c @@ -180,7 +180,8 @@ static int batadv_interface_tx(struct sk_buff *skb, goto dropped; /* Register the client MAC in the transtable */ - batadv_tt_local_add(soft_iface, ethhdr-h_source, skb-skb_iif); + if (!is_multicast_ether_addr(ethhdr-h_source)) + batadv_tt_local_add(soft_iface, ethhdr-h_source, skb-skb_iif); How can the source address be multicast? Usually multicast addresses are found in the destination field... Is there any scenario where this is possible? I am wondering whether we should directly drop packet having mcast address as source.. Hm, no, I do not have a specific scenario in mind. I only accidentally noticed such an FF:FF:FF:FF:FF:FF mac address in our vis graph here when I made a typo with the tool 'mausezahn'. I don't know of any RFC or so which would generally forbid using a multicast source MAC for every ether type (although for sane IPv4/v6 stacks I haven't seen something like this before either and yes, I bet it's not allowed there). The Linux bridge I had on top of bat0 was happily forwarding such frames into batman-adv. So I thought doing it like the bridge code - forwarding but with no learning - might be the right way? I think you are right. We should not care about the payload logic. If that is not allowed for the upper layer, then that layer will drop the packet once received. So we should keep forwarding such packets and yes, we should prevent learning from them. Therefore: Acked-by: Antonio Quartulli or...@autistici.org Thanks a lot Linüs. Cheers, -- Antonio Quartulli ..each of us alone is worth nothing.. Ernesto Che Guevara pgpoMPyeB2rxS.pgp Description: PGP signature
[B.A.T.M.A.N.] [RFCv3] batman-adv: Add function to calculate crc32c for the skb payload
Signed-off-by: Sven Eckelmann s...@narfation.org --- v3: Now with support for kernels till 2.6.29 compat.h | 34 ++ main.c | 38 ++ main.h |1 + 3 files changed, 73 insertions(+) diff --git a/compat.h b/compat.h index 0caf43b..83befd1 100644 --- a/compat.h +++ b/compat.h @@ -37,6 +37,9 @@ #if LINUX_VERSION_CODE KERNEL_VERSION(2, 6, 31) +#define skb_walk_frags(skb, iter) \ + for (iter = skb_shinfo(skb)-frag_list; iter; iter = iter-next) + #define __compat__module_param_call(p1, p2, p3, p4, p5, p6, p7) \ __module_param_call(p1, p2, p3, p4, p5, p7) @@ -78,6 +81,17 @@ #endif /* KERNEL_VERSION(2, 6, 35) */ +#if LINUX_VERSION_CODE KERNEL_VERSION(2, 6, 37) + +#define kmap_atomic(p) kmap_atomic(p, KM_SKB_DATA_SOFTIRQ) + +#ifdef kunmap_atomic +#undef kunmap_atomic +#endif +#define kunmap_atomic(x, arg...)do { pagefault_enable(); } while (0) + +#endif /* KERNEL_VERSION(2, 6, 37) */ + #if LINUX_VERSION_CODE KERNEL_VERSION(2, 6, 36) @@ -146,6 +160,26 @@ static inline void skb_reset_mac_len(struct sk_buff *skb) #endif /* KERNEL_VERSION(3, 0, 0) */ +#if LINUX_VERSION_CODE KERNEL_VERSION(3, 2, 0) + +static inline struct page *skb_frag_page(const skb_frag_t *frag) +{ + return frag-page; +} + +static inline void *skb_frag_address(const skb_frag_t *frag) +{ + return page_address(skb_frag_page(frag)) + frag-page_offset; +} + +static inline unsigned int skb_frag_size(const skb_frag_t *frag) +{ + return frag-size; +} + +#endif /* KERNEL_VERSION(3, 2, 0) */ + + #if LINUX_VERSION_CODE KERNEL_VERSION(3, 4, 0) static inline void eth_hw_addr_random(struct net_device *dev) diff --git a/main.c b/main.c index 70797de..df5f7d6 100644 --- a/main.c +++ b/main.c @@ -17,6 +17,8 @@ * 02110-1301, USA */ +#include linux/crc32c.h +#include linux/highmem.h #include main.h #include sysfs.h #include debugfs.h @@ -432,6 +434,42 @@ int batadv_compat_seq_print_text(struct seq_file *seq, void *offset) return 0; } +/** + * batadv_crc32 - calculate CRC32 of the whole packet and skip bytes in header + * @skb: skb pointing to fragmented socket buffers + * @payload_ptr: Pointer to position inside the head buffer of the skb + * marking the start of the data to be CRC'ed + * + * payload_ptr must always point to the point in the skb head buffer and not to + * a fragment. + */ +__be32 batadv_crc32(const struct sk_buff *skb, u8 *payload_ptr) +{ + u32 crc = 0; + struct sk_buff *iter; + size_t skip_len, read_len; + const skb_frag_t *f; + u8 *vaddr; + int i; + + skip_len = payload_ptr - skb-data; + read_len = skb_headlen(skb) - skip_len; + if (read_len) + crc = crc32c(crc, payload_ptr, read_len); + + for (i = 0; i skb_shinfo(skb)-nr_frags; i++) { + f = skb_shinfo(skb)-frags[i]; + vaddr = kmap_atomic(skb_frag_page(f)); + crc = crc32c(crc, vaddr + f-page_offset, skb_frag_size(f)); + kunmap_atomic(vaddr); + } + + skb_walk_frags(skb, iter) + crc = crc32c(crc, iter-data, skb_headlen(iter)); + + return htonl(crc); +} + static int batadv_param_set_ra(const char *val, const struct kernel_param *kp) { struct batadv_algo_ops *bat_algo_ops; diff --git a/main.h b/main.h index 2dfcf8c..bb0a710 100644 --- a/main.h +++ b/main.h @@ -177,6 +177,7 @@ int batadv_algo_register(struct batadv_algo_ops *bat_algo_ops); int batadv_algo_select(struct batadv_priv *bat_priv, char *name); int batadv_algo_seq_print_text(struct seq_file *seq, void *offset); int batadv_compat_seq_print_text(struct seq_file *seq, void *offset); +__be32 batadv_crc32(const struct sk_buff *skb, u8 *payload_ptr); /** * enum batadv_dbg_level - available log levels -- 1.7.10.4
[B.A.T.M.A.N.] Web-based Visualisation Tool of Choice for batman-adv?
Hi all, Is there an obvious choice for web-based visualisation of batman-adv networks? We currently use a custom product (SPUD - http://dev.villagetelco.org/trac/browser/spud/trunk/INSTALL) built specifically for Village Telco but would prefer to join our efforts to a broader community-based effort. I am aware of Nodewatcher http://nodewatcher.readthedocs.org/en/latest/installation.html and Nodeshot http://wiki.ninux.org/InstallNodeshot but both of these take a little adapting to batman-adv. Is there anything out there that has been developed specifically for batman-adv? Thanks... Steve -- Steve Song http://villagetelco.org http://manypossibilities.net
Re: [B.A.T.M.A.N.] Web-based Visualisation Tool of Choice for batman-adv?
On Wed, Oct 17, 2012 at 04:44:51PM -0300, Steve Song wrote: Hi all, Is there an obvious choice for web-based visualisation of batman-adv networks? We currently use a custom product (SPUD - http://dev.villagetelco.org/trac/browser/spud/trunk/INSTALL) built specifically for Village Telco but would prefer to join our efforts to a broader community-based effort. I am aware of Nodewatcher http://nodewatcher.readthedocs.org/en/latest/installation.html and Nodeshot http://wiki.ninux.org/InstallNodeshot but both of these take a little adapting to batman-adv. Is there anything out there that has been developed specifically for batman-adv? I know about people in Ninux that wrote a script taking the output of batctl vd dot and converting it in a format understandable by nodeshot. In ninux they have a mesh network running batman-adv and it is entirely shown on the map server ;-) Maybe you can try to ask them via the mailing list? Bye! -- Antonio Quartulli ..each of us alone is worth nothing.. Ernesto Che Guevara pgpFJpWSxns86.pgp Description: PGP signature
[B.A.T.M.A.N.] [PATCH 2/4] alfred: Fix parsing of short program parameter -v and -V
Signed-off-by: Sven Eckelmann s...@narfation.org --- main.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/main.c b/main.c index 0315fe3..96e401a 100644 --- a/main.c +++ b/main.c @@ -78,7 +78,7 @@ struct globals *alfred_init(int argc, char *argv[]) globals-best_server = NULL; globals-clientmode_version = 0; - while ((opt = getopt_long(argc, argv, ms:r:hi:, long_options, + while ((opt = getopt_long(argc, argv, ms:r:hi:vV:, long_options, opt_ind)) != -1) { switch (opt) { case 'r': -- 1.7.10.4
[B.A.T.M.A.N.] [PATCH 3/4] alfred: Test for failed global memory allocation
Signed-off-by: Sven Eckelmann s...@narfation.org --- main.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/main.c b/main.c index 96e401a..ccb72cd 100644 --- a/main.c +++ b/main.c @@ -67,11 +67,11 @@ struct globals *alfred_init(int argc, char *argv[]) }; globals = malloc(sizeof(*globals)); - memset(globals, 0, sizeof(*globals)); - if (!globals) return NULL; + memset(globals, 0, sizeof(*globals)); + globals-opmode = OPMODE_SLAVE; globals-clientmode = CLIENT_NONE; globals-interface = NULL; -- 1.7.10.4
[B.A.T.M.A.N.] [PATCH 4/4] alfred: Fix test for failed sendbuf allocation
Signed-off-by: Sven Eckelmann s...@narfation.org --- recv.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/recv.c b/recv.c index dce7e50..8ddf7ab 100644 --- a/recv.c +++ b/recv.c @@ -217,7 +217,7 @@ int send_alfred_packet(struct globals *globals, uint8_t *dest, void *buf, char *sendbuf; sendbuf = malloc(sizeof(*ethhdr) + length); - if (!buf) + if (!sendbuf) return -1; ethhdr = (struct ethhdr *)sendbuf; -- 1.7.10.4
Re: [B.A.T.M.A.N.] [RFCv3] batman-adv: Add function to calculate crc32c for the skb payload
On Thursday, October 18, 2012 03:10:39 Sven Eckelmann wrote: Signed-off-by: Sven Eckelmann s...@narfation.org --- v3: Now with support for kernels till 2.6.29 compat.h | 34 ++ main.c | 38 ++ main.h |1 + 3 files changed, 73 insertions(+) Applied in revision a7fe307. Thanks, Marek
[B.A.T.M.A.N.] [PATCH] batman-adv: Fix broadcast duplist for fragmentation
If the skb is fragmented, the checksum must be computed on the individual fragments, just using skb-data may fail on fragmented data. Instead of doing linearizing the packet, use the new batadv_crc32 to do that more efficiently- it should not hurt replacing the old crc16 by the new crc32. Reported-by: Sven Eckelmann s...@narfation.org Signed-off-by: Simon Wunderlich s...@hrz.tu-chemnitz.de --- bridge_loop_avoidance.c | 18 +++--- bridge_loop_avoidance.h |3 +-- routing.c | 10 +++--- types.h |2 +- 4 files changed, 12 insertions(+), 21 deletions(-) diff --git a/bridge_loop_avoidance.c b/bridge_loop_avoidance.c index c6c1c59..b766441 100644 --- a/bridge_loop_avoidance.c +++ b/bridge_loop_avoidance.c @@ -1247,8 +1247,7 @@ int batadv_bla_init(struct batadv_priv *bat_priv) /** * batadv_bla_check_bcast_duplist * @bat_priv: the bat priv with all the soft interface information - * @bcast_packet: encapsulated broadcast frame plus batman header - * @bcast_packet_len: length of encapsulated broadcast frame plus batman header + * @skb: contains the bcast_packet to be checked * * check if it is on our broadcast list. Another gateway might * have sent the same packet because it is connected to the same backbone, @@ -1260,20 +1259,17 @@ int batadv_bla_init(struct batadv_priv *bat_priv) * the same host however as this might be intended. */ int batadv_bla_check_bcast_duplist(struct batadv_priv *bat_priv, - struct batadv_bcast_packet *bcast_packet, - int bcast_packet_len) + struct sk_buff *skb) { - int i, length, curr, ret = 0; - uint8_t *content; - uint16_t crc; + int i, curr, ret = 0; + uint32_t crc; + struct batadv_bcast_packet *bcast_packet; struct batadv_bcast_duplist_entry *entry; - length = bcast_packet_len - sizeof(*bcast_packet); - content = (uint8_t *)bcast_packet; - content += sizeof(*bcast_packet); + bcast_packet = (struct batadv_bcast_packet *)skb-data; /* calculate the crc ... */ - crc = crc16(0, content, length); + crc = batadv_skb_crc32(skb, (u8 *)(bcast_packet + 1)); spin_lock_bh(bat_priv-bla.bcast_duplist_lock); diff --git a/bridge_loop_avoidance.h b/bridge_loop_avoidance.h index 789cb73..f1bc9cd 100644 --- a/bridge_loop_avoidance.h +++ b/bridge_loop_avoidance.h @@ -31,8 +31,7 @@ int batadv_bla_backbone_table_seq_print_text(struct seq_file *seq, void *offset); int batadv_bla_is_backbone_gw_orig(struct batadv_priv *bat_priv, uint8_t *orig); int batadv_bla_check_bcast_duplist(struct batadv_priv *bat_priv, - struct batadv_bcast_packet *bcast_packet, - int hdr_size); + struct sk_buff *skb); void batadv_bla_update_orig_address(struct batadv_priv *bat_priv, struct batadv_hard_iface *primary_if, struct batadv_hard_iface *oldif); diff --git a/routing.c b/routing.c index 5da62df..5da7724 100644 --- a/routing.c +++ b/routing.c @@ -1181,16 +1181,12 @@ int batadv_recv_bcast_packet(struct sk_buff *skb, spin_unlock_bh(orig_node-bcast_seqno_lock); - /* keep skb linear for crc calculation */ - if (skb_linearize(skb) 0) - goto out; - - bcast_packet = (struct batadv_bcast_packet *)skb-data; - /* check whether this has been sent by another originator before */ - if (batadv_bla_check_bcast_duplist(bat_priv, bcast_packet, skb-len)) + if (batadv_bla_check_bcast_duplist(bat_priv, skb)) goto out; + bcast_packet = (struct batadv_bcast_packet *)skb-data; + /* rebroadcast packet */ batadv_add_bcast_packet_to_list(bat_priv, skb, 1); diff --git a/types.h b/types.h index 7b3d0d7..354b699 100644 --- a/types.h +++ b/types.h @@ -156,7 +156,7 @@ struct batadv_neigh_node { #ifdef CONFIG_BATMAN_ADV_BLA struct batadv_bcast_duplist_entry { uint8_t orig[ETH_ALEN]; - uint16_t crc; + uint32_t crc; unsigned long entrytime; }; #endif -- 1.7.10
Re: [B.A.T.M.A.N.] [PATCH 2/4] alfred: Fix parsing of short program parameter -v and -V
Applied in revision 3c2de69. Thanks, Simon On Wed, Oct 17, 2012 at 09:55:56PM +0200, Sven Eckelmann wrote: Signed-off-by: Sven Eckelmann s...@narfation.org --- main.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/main.c b/main.c index 0315fe3..96e401a 100644 --- a/main.c +++ b/main.c @@ -78,7 +78,7 @@ struct globals *alfred_init(int argc, char *argv[]) globals-best_server = NULL; globals-clientmode_version = 0; - while ((opt = getopt_long(argc, argv, ms:r:hi:, long_options, + while ((opt = getopt_long(argc, argv, ms:r:hi:vV:, long_options, opt_ind)) != -1) { switch (opt) { case 'r': -- 1.7.10.4 signature.asc Description: Digital signature