diff mbox

batman-adv: Adding netfilter-bridge hooks

Message ID 1274232349-10414-1-git-send-email-linus.luessing@web.de
State Accepted, archived
Headers show

Commit Message

Linus Lüssing May 19, 2010, 1:25 a.m. UTC
batman-adv is receiving and sending the packets of its own ether type
on a very early/low level. Therefore we need to add explicit hooks to
give netfilter/ebtables a chance to filter them.

Signed-off-by: Linus Lüssing <linus.luessing@web.de>
Reported-by: Antonio Quartulli <ordex@ritirata.org>
---
 batman-adv-kernelland/hard-interface.c |   17 +++++++++++++++--
 batman-adv-kernelland/send.c           |    8 ++++++--
 2 files changed, 21 insertions(+), 4 deletions(-)

Comments

Antonio Quartulli May 21, 2010, 8:21 a.m. UTC | #1
Hi all,

On Wed, May 19, 2010 at 03:25:49AM +0200, Linus Lüssing wrote:
> batman-adv is receiving and sending the packets of its own ether type
> on a very early/low level. Therefore we need to add explicit hooks to
> give netfilter/ebtables a chance to filter them.
> 
> Signed-off-by: Linus Lüssing <linus.luessing@web.de>
> Reported-by: Antonio Quartulli <ordex@ritirata.org>
> ---
>  batman-adv-kernelland/hard-interface.c |   17 +++++++++++++++--
>  batman-adv-kernelland/send.c           |    8 ++++++--
>  2 files changed, 21 insertions(+), 4 deletions(-)
> 
> diff --git a/batman-adv-kernelland/hard-interface.c b/batman-adv-kernelland/hard-interface.c
> index cc7fbae..6a64930 100644
> --- a/batman-adv-kernelland/hard-interface.c
> +++ b/batman-adv-kernelland/hard-interface.c
> @@ -28,9 +28,11 @@
>  #include "bat_sysfs.h"
>  #include "originator.h"
>  #include "hash.h"
> -#include "compat.h"
>  
>  #include <linux/if_arp.h>
> +#include <linux/netfilter_bridge.h>
> +
> +#include "compat.h"
>  
>  #define MIN(x, y) ((x) < (y) ? (x) : (y))
>  
> @@ -433,6 +435,11 @@ out:
>  	return NOTIFY_DONE;
>  }
>  
> +int batman_skb_recv_finish(struct sk_buff *skb)
> +{
> +	return NF_ACCEPT;
> +}
> +
>  /* receive a packet with the batman ethertype coming on a hard
>   * interface */
>  int batman_skb_recv(struct sk_buff *skb, struct net_device *dev,
> @@ -452,6 +459,13 @@ int batman_skb_recv(struct sk_buff *skb, struct net_device *dev,
>  	if (atomic_read(&module_state) != MODULE_ACTIVE)
>  		goto err_free;
>  
> +	/* if netfilter/ebtables wants to block incoming batman
> +	 * packets then give them a chance to do so here */
> +	ret = NF_HOOK(PF_BRIDGE, NF_BR_LOCAL_IN, skb, dev, NULL,
> +		      batman_skb_recv_finish);
> +	if (ret != 1)
> +		goto err_out;
> +
>  	/* packet should hold at least type and version */
>  	if (unlikely(skb_headlen(skb) < 2))
>  		goto err_free;
> @@ -531,7 +545,6 @@ err_out:
>  	return NET_RX_DROP;
>  }
>  
> -
>  struct notifier_block hard_if_notifier = {
>  	.notifier_call = hard_if_event,
>  };
> diff --git a/batman-adv-kernelland/send.c b/batman-adv-kernelland/send.c
> index 99d11fe..b0d3627 100644
> --- a/batman-adv-kernelland/send.c
> +++ b/batman-adv-kernelland/send.c
> @@ -29,6 +29,7 @@
>  #include "vis.h"
>  #include "aggregation.h"
>  #include "gateway_common.h"
> +#include <linux/netfilter_bridge.h>
>  
>  #include "compat.h"
>  
> @@ -93,9 +94,12 @@ int send_skb_packet(struct sk_buff *skb,
>  
>  	/* dev_queue_xmit() returns a negative result on error.	 However on
>  	 * congestion and traffic shaping, it drops and returns NET_XMIT_DROP
> -	 * (which is > 0). This will not be treated as an error. */
> +	 * (which is > 0). This will not be treated as an error.
> +	 * Also, if netfilter/ebtables wants to block outgoing batman
> +	 * packets then giving them a chance to do so here */
>  
> -	return dev_queue_xmit(skb);
> +	return NF_HOOK(PF_BRIDGE, NF_BR_LOCAL_OUT, skb, NULL, skb->dev,
> +		       dev_queue_xmit);
>  send_skb_err:
>  	kfree_skb(skb);
>  	return NET_XMIT_DROP;
> -- 
> 1.5.6.5

I gave a try to this patch, but I see something strange.
After enabling a simple ebtables rule:
ebtables -A INPUT -s MAC -j DROP
and
ebtables -A FORWARD -s MAC -j DROP (to be sure..)

I saw that batman ping was timing out, while the "originator list" (shown
with batctl o) is still filled with the other node entry...

I did something wrong?

Regards
Linus Lüssing May 21, 2010, 10:17 a.m. UTC | #2
>
>I gave a try to this patch, but I see something strange.
>After enabling a simple ebtables rule:
>ebtables -A INPUT -s MAC -j DROP
>and
>ebtables -A FORWARD -s MAC -j DROP (to be sure..)
>
>I saw that batman ping was timing out, while the "originator list" (shown
>with batctl o) is still filled with the other node entry...

Hi Antonio,

thanks for trying the patch! In my case, it worked, I tried it with
ebtables -I INPUT -s MAC -j DROP or
ebtables -I INPUT -p 0x4305 -j DROP
(and the same for -I OUTPUT)

batctl td reported, that it's not receiving any batman packets anymore and also
the originator table was empty after a couple of minutes.

Hmm, and also that batping is timing out for you seems to indicate that it should work
on your side. Could you check with batctl td too? How long have you been waiting for
the originator table to clear? (dead nodes are not being cleared immediately in batman,
as they don't harm the routing decisions and we still need the last measurements in case they
might get alive again)

Cheers, Linus
Antonio Quartulli May 21, 2010, 6:45 p.m. UTC | #3
On ven, mag 21, 2010 at 12:17:28 +0200, Linus Lüssing wrote:
> >
> >I gave a try to this patch, but I see something strange.
> >After enabling a simple ebtables rule:
> >ebtables -A INPUT -s MAC -j DROP
> >and
> >ebtables -A FORWARD -s MAC -j DROP (to be sure..)
> >
> >I saw that batman ping was timing out, while the "originator list" (shown
> >with batctl o) is still filled with the other node entry...
> 
> Hi Antonio,
> 
> thanks for trying the patch! In my case, it worked, I tried it with
> ebtables -I INPUT -s MAC -j DROP or
> ebtables -I INPUT -p 0x4305 -j DROP
> (and the same for -I OUTPUT)
> 
> batctl td reported, that it's not receiving any batman packets anymore and also
> the originator table was empty after a couple of minutes.
> 
> Hmm, and also that batping is timing out for you seems to indicate that it should work
> on your side. Could you check with batctl td too? How long have you been waiting for
> the originator table to clear? (dead nodes are not being cleared immediately in batman,
> as they don't harm the routing decisions and we still need the last measurements in case they
> might get alive again)
> 
> Cheers, Linus

Hi Linus,
	I thought that the dead node should be pushed away after a
while, not 3 minutes. So that was my mistake...indeed everything was working correctly!

I tried it again five minutes ago, and everything went as I expected!

Thanks very much!

Regards

P.S. batctl td was not so usefull since I have to point it to wlan0..and
obviously I can see all the packets there.
Marek Lindner May 22, 2010, 10:51 a.m. UTC | #4
Hey,

>  I thought that the dead node should be pushed away after a
> while, not 3 minutes. So that was my mistake...indeed everything was
> working correctly!
> 
> I tried it again five minutes ago, and everything went as I expected!

I just pushed the patch (revision 1678). Thanks for bringing it up and thanks 
to Linus for fixing it.

Cheers,
Marek
Linus Lüssing May 25, 2010, 11:56 p.m. UTC | #5
>Hi Linus,
>	I thought that the dead node should be pushed away after a
>while, not 3 minutes. So that was my mistake...indeed everything was working correctly!
>
>I tried it again five minutes ago, and everything went as I expected!
>
>Thanks very much!

Great! You're welcome :). Feel free to share any interesting tests and results with this 
manual, explicit filtering capabilities.

>P.S. batctl td was not so usefull since I have to point it to wlan0..and
>obviously I can see all the packets there.
Usually I my self am using "batctl td" in conjunction with grep. For instance I'm having an interface which is nearly only
having batman-adv traffic. I'm also usuallly not deactivating IPv6 so I get some annoying extra packets in batctl td -
but I can easily get rid of this with " batctl td | grep -v "Warning" ". '-v' is pretty handy to throw out undesired lines from
the output.

Cheers, Linus

>
>-- 
>Antonio Quartulli
>
>Ognuno di noi, da solo, non vale nulla 
>Ernesto "Che" Guevara
diff mbox

Patch

diff --git a/batman-adv-kernelland/hard-interface.c b/batman-adv-kernelland/hard-interface.c
index cc7fbae..6a64930 100644
--- a/batman-adv-kernelland/hard-interface.c
+++ b/batman-adv-kernelland/hard-interface.c
@@ -28,9 +28,11 @@ 
 #include "bat_sysfs.h"
 #include "originator.h"
 #include "hash.h"
-#include "compat.h"
 
 #include <linux/if_arp.h>
+#include <linux/netfilter_bridge.h>
+
+#include "compat.h"
 
 #define MIN(x, y) ((x) < (y) ? (x) : (y))
 
@@ -433,6 +435,11 @@  out:
 	return NOTIFY_DONE;
 }
 
+int batman_skb_recv_finish(struct sk_buff *skb)
+{
+	return NF_ACCEPT;
+}
+
 /* receive a packet with the batman ethertype coming on a hard
  * interface */
 int batman_skb_recv(struct sk_buff *skb, struct net_device *dev,
@@ -452,6 +459,13 @@  int batman_skb_recv(struct sk_buff *skb, struct net_device *dev,
 	if (atomic_read(&module_state) != MODULE_ACTIVE)
 		goto err_free;
 
+	/* if netfilter/ebtables wants to block incoming batman
+	 * packets then give them a chance to do so here */
+	ret = NF_HOOK(PF_BRIDGE, NF_BR_LOCAL_IN, skb, dev, NULL,
+		      batman_skb_recv_finish);
+	if (ret != 1)
+		goto err_out;
+
 	/* packet should hold at least type and version */
 	if (unlikely(skb_headlen(skb) < 2))
 		goto err_free;
@@ -531,7 +545,6 @@  err_out:
 	return NET_RX_DROP;
 }
 
-
 struct notifier_block hard_if_notifier = {
 	.notifier_call = hard_if_event,
 };
diff --git a/batman-adv-kernelland/send.c b/batman-adv-kernelland/send.c
index 99d11fe..b0d3627 100644
--- a/batman-adv-kernelland/send.c
+++ b/batman-adv-kernelland/send.c
@@ -29,6 +29,7 @@ 
 #include "vis.h"
 #include "aggregation.h"
 #include "gateway_common.h"
+#include <linux/netfilter_bridge.h>
 
 #include "compat.h"
 
@@ -93,9 +94,12 @@  int send_skb_packet(struct sk_buff *skb,
 
 	/* dev_queue_xmit() returns a negative result on error.	 However on
 	 * congestion and traffic shaping, it drops and returns NET_XMIT_DROP
-	 * (which is > 0). This will not be treated as an error. */
+	 * (which is > 0). This will not be treated as an error.
+	 * Also, if netfilter/ebtables wants to block outgoing batman
+	 * packets then giving them a chance to do so here */
 
-	return dev_queue_xmit(skb);
+	return NF_HOOK(PF_BRIDGE, NF_BR_LOCAL_OUT, skb, NULL, skb->dev,
+		       dev_queue_xmit);
 send_skb_err:
 	kfree_skb(skb);
 	return NET_XMIT_DROP;