Re: [B.A.T.M.A.N.] [RFCv2] batman-adv: Add blocking of one hop OGM messages

[Simon Wunderlich]

Hey Martin,

On Fri, Feb 10, 2012 at 03:53:11PM +0100, Martin Hundebøll wrote:
  13 files changed, 491 insertions(+), 0 deletions(-)

please excuse my superficial review, but do we really need to bloat batman-adv
by 491 more lines to drop these OGMs? I'm afraid the filter will evolve
more and more, and eventually the research/debugging code in batman-adv
is bigger than the routing code. ;)

(sorry for ranting ;] )

Have you considered using and/or extending the former ebtables patch
from Linus[1]? We removed [2] it after some discussions [3], but maybe
its worth fixing these problems if its just about dropping some neighbors
packets. The patch has some problems, but it was really short and I guess
we wouldn't mind accepting the performance overhead by ebtables for
research/debugging purposes. Plus, we could make this a compile-feature too.

Cheers,
Simon

[1] 
http://ftp.uni-kl.de/pub/linux/kernel/people/gregkh/staging/2.6/2.6.35/0120-Staging-batman-adv-Adding-netfilter-bridge-hooks.patch
[2] https://lists.open-mesh.org/pipermail/b.a.t.m.a.n/2010-September/003305.html
[3] http://lkml.indiana.edu/hypermail/linux/kernel/1008.1/02232.html


signature.asc
Description: Digital signature

Re: [B.A.T.M.A.N.] [PATCHv5 7/9] batman-adv: Distributed ARP Table - add compile option

[Antonio Quartulli]

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 itthen 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


pgpWgALM0vvkG.pgp
Description: PGP signature

[B.A.T.M.A.N.] [PATCH] batman-adv: use ETH_HLEN instead of sizeof(struct ethhdr)

[Antonio Quartulli]

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

Signed-off-by: Antonio Quartulli or...@autistici.org
---
 bat_iv_ogm.c|7 +++
 bridge_loop_avoidance.c |8 +++-
 hard-interface.c|3 +--
 icmp_socket.c   |4 ++--
 routing.c   |8 
 soft-interface.c|2 +-
 types.h |2 +-
 vis.c   |8 
 8 files changed, 19 insertions(+), 23 deletions(-)

diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c
index caa67e4..42c121d 100644
--- a/bat_iv_ogm.c
+++ b/bat_iv_ogm.c
@@ -358,10 +358,9 @@ static void bat_iv_ogm_aggregate_new(const unsigned char 
*packet_buff,
if ((atomic_read(bat_priv-aggregated_ogms)) 
(packet_len  MAX_AGGREGATION_BYTES))
forw_packet_aggr-skb = dev_alloc_skb(MAX_AGGREGATION_BYTES +
- sizeof(struct ethhdr));
+ ETH_HLEN);
else
-   forw_packet_aggr-skb = dev_alloc_skb(packet_len +
- sizeof(struct ethhdr));
+   forw_packet_aggr-skb = dev_alloc_skb(packet_len + ETH_HLEN);
 
if (!forw_packet_aggr-skb) {
if (!own_packet)
@@ -369,7 +368,7 @@ static void bat_iv_ogm_aggregate_new(const unsigned char 
*packet_buff,
kfree(forw_packet_aggr);
goto out;
}
-   skb_reserve(forw_packet_aggr-skb, sizeof(struct ethhdr));
+   skb_reserve(forw_packet_aggr-skb, ETH_HLEN);
 
INIT_HLIST_NODE(forw_packet_aggr-list);
 
diff --git a/bridge_loop_avoidance.c b/bridge_loop_avoidance.c
index 99b0a8f..af93e5c 100644
--- a/bridge_loop_avoidance.c
+++ b/bridge_loop_avoidance.c
@@ -292,9 +292,7 @@ static void bla_send_claim(struct bat_priv *bat_priv, 
uint8_t *mac,
goto out;
 
ethhdr = (struct ethhdr *)skb-data;
-   hw_src = (uint8_t *) ethhdr +
-sizeof(struct ethhdr) +
-sizeof(struct arphdr);
+   hw_src = (uint8_t *) ethhdr + ETH_HLEN + sizeof(struct arphdr);
 
/* now we pretend that the client would have sent this ... */
switch (claimtype) {
@@ -346,7 +344,7 @@ static void bla_send_claim(struct bat_priv *bat_priv, 
uint8_t *mac,
skb_reset_mac_header(skb);
skb-protocol = eth_type_trans(skb, soft_iface);
bat_priv-stats.rx_packets++;
-   bat_priv-stats.rx_bytes += skb-len + sizeof(struct ethhdr);
+   bat_priv-stats.rx_bytes += skb-len + ETH_HLEN;
soft_iface-last_rx = jiffies;
 
netif_rx(skb);
@@ -1317,7 +1315,7 @@ int bla_is_backbone_gw(struct sk_buff *skb,
return 0;
 
/* first, find out the vid. */
-   if (!pskb_may_pull(skb, hdr_size + sizeof(struct ethhdr)))
+   if (!pskb_may_pull(skb, hdr_size + ETH_HLEN))
return 0;
 
ethhdr = (struct ethhdr *) (((uint8_t *)skb-data) + hdr_size);
diff --git a/hard-interface.c b/hard-interface.c
index 30b1c07..dfa948b 100644
--- a/hard-interface.c
+++ b/hard-interface.c
@@ -587,8 +587,7 @@ static int batman_skb_recv(struct sk_buff *skb, struct 
net_device *dev,
goto err_free;
 
/* expect a valid ethernet header here. */
-   if (unlikely(skb-mac_len != sizeof(struct ethhdr)
-   || !skb_mac_header(skb)))
+   if (unlikely(skb-mac_len != ETH_HLEN || !skb_mac_header(skb)))
goto err_free;
 
if (!hard_iface-soft_iface)
diff --git a/icmp_socket.c b/icmp_socket.c
index 9b755f9..3128920 100644
--- a/icmp_socket.c
+++ b/icmp_socket.c
@@ -177,13 +177,13 @@ static ssize_t bat_socket_write(struct file *file, const 
char __user *buff,
if (len = sizeof(struct icmp_packet_rr))
packet_len = sizeof(struct icmp_packet_rr);
 
-   skb = dev_alloc_skb(packet_len + sizeof(struct ethhdr));
+   skb = dev_alloc_skb(packet_len + ETH_HLEN);
if (!skb) {
len = -ENOMEM;
goto out;
}
 
-   skb_reserve(skb, sizeof(struct ethhdr));
+   skb_reserve(skb, ETH_HLEN);
icmp_packet = (struct icmp_packet_rr *)skb_put(skb, packet_len);
 
if (copy_from_user(icmp_packet, buff, packet_len)) {
diff --git a/routing.c b/routing.c
index 92fe20b..a055386 100644
--- a/routing.c
+++ b/routing.c
@@ -313,7 +313,7 @@ static int recv_my_icmp_packet(struct bat_priv *bat_priv,
goto out;
 
/* create a copy of the skb, if needed, to modify it. */
-   if (skb_cow(skb, sizeof(struct ethhdr))  0)
+   if (skb_cow(skb, ETH_HLEN)  0)
goto out;
 
icmp_packet = (struct icmp_packet_rr *)skb-data;
@@ -369,7 +369,7 @@ static int recv_icmp_ttl_exceeded(struct bat_priv *bat_priv,
goto out;