All of lore.kernel.org
 help / color / mirror / Atom feed
* [B.A.T.M.A.N.] BLAII + gw_mode, DHCP sometimes gets dropped
@ 2012-07-03 20:07 Guido Iribarren
  2012-07-04  9:12 ` Simon Wunderlich
  0 siblings, 1 reply; 16+ messages in thread
From: Guido Iribarren @ 2012-07-03 20:07 UTC (permalink / raw)
  To: The list for a Better Approach To Mobile Ad-hoc Networking

Hello there again,
I have observed a problem since updating to 2012.2 and enabled BLAII

I'm compiling logs to understand what's happening, but as always,
reading logs only gets me more lost :(
So here i am again begging for help

the setup is the same I described in yesterday's attachment, but
what's not pictured is an ethernet cable between colmena-casa and
f8d11504758.
f8d11504758 is the only router that connects to the internet (through
WAN cable), and it's also the only one that has dnsmasq running and
gw_mode=server.
All the other nodes have gw_mode=client

All of the nodes have bridge_loop_avoidance=1
(even though there are no other utp connections, so it could in fact
be enabled only on colmena-casa and f8d11504758)

with this setup, dhcp requests from the mesh sometimes get "lost",
either they don't reach f8d11504758 or the reply doesn't get out

this didn't happen with batman 2012.1 , setup as indicated by the BLAI
wiki page (batctl if add br-lan)
furthermore, with batman 2012.2 , BLAII activated, but gw_mode=off in
all nodes, DHCP also works fine.

So, a few questions arise:
is it a problem to activate bridge_loop_avoidance=1 in all nodes,
regardless of the fact that they "need" it or not? (that is, it is
activated on nodes that don't have any ethernet cables connected and
couldn't possibly create a bridge loop)

would it make a difference, if I add br-lan to bat0 (batctl if add
br-lan) the way I used to do with batman 2012.1 ?

any other thoughts or ideas?

thanks as always!

Gui

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

* Re: [B.A.T.M.A.N.] BLAII + gw_mode, DHCP sometimes gets dropped
  2012-07-03 20:07 [B.A.T.M.A.N.] BLAII + gw_mode, DHCP sometimes gets dropped Guido Iribarren
@ 2012-07-04  9:12 ` Simon Wunderlich
  2012-07-04 12:43   ` Guido Iribarren
  0 siblings, 1 reply; 16+ messages in thread
From: Simon Wunderlich @ 2012-07-04  9:12 UTC (permalink / raw)
  To: The list for a Better Approach To Mobile Ad-hoc Networking

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

Hello Guido,

On Tue, Jul 03, 2012 at 05:07:17PM -0300, Guido Iribarren wrote:
> Hello there again,
> I have observed a problem since updating to 2012.2 and enabled BLAII
> 
> I'm compiling logs to understand what's happening, but as always,
> reading logs only gets me more lost :(
> So here i am again begging for help

There are some debug levels for BLA as well, and you can now get the
claimlist with batctl (which is basically the list of clients a gateway
feels responsible for) - this may help for debugging. But first,
we should clarify some more details for your setup.

> 
> the setup is the same I described in yesterday's attachment, but
> what's not pictured is an ethernet cable between colmena-casa and
> f8d11504758.
> f8d11504758 is the only router that connects to the internet (through
> WAN cable), and it's also the only one that has dnsmasq running and
> gw_mode=server.
> All the other nodes have gw_mode=client
> 
> All of the nodes have bridge_loop_avoidance=1
> (even though there are no other utp connections, so it could in fact
> be enabled only on colmena-casa and f8d11504758)
> 
> with this setup, dhcp requests from the mesh sometimes get "lost",
> either they don't reach f8d11504758 or the reply doesn't get out

Questions:
 * which node runs the DHCP server? colmena-casa, f8d11504758 or something else?
 * at which point is DHCP getting lost? is the DISCOVER/REQUEST from the client
   getting lost, or the reply from the server?
 * Can you specify "sometimes" a little bit more? What are the circumstances, how
   often does it happen?

> 
> this didn't happen with batman 2012.1 , setup as indicated by the BLAI
> wiki page (batctl if add br-lan)
> furthermore, with batman 2012.2 , BLAII activated, but gw_mode=off in
> all nodes, DHCP also works fine.

Mhm, that's rather strange ... we had a similar problem when ap isolation
was activated. Do you have this feature turned on?

So DHCP is only having problems when gw-mode is turned on colmena-casa
and f8d11504758?

> 
> So, a few questions arise:
> is it a problem to activate bridge_loop_avoidance=1 in all nodes,
> regardless of the fact that they "need" it or not? (that is, it is
> activated on nodes that don't have any ethernet cables connected and
> couldn't possibly create a bridge loop)

No, that it is not a problem - you can activate it everywhere. It will
just send some additional control packets on bat0, but won't do anything
as long as it does not detect other gateways.

> 
> would it make a difference, if I add br-lan to bat0 (batctl if add
> br-lan) the way I used to do with batman 2012.1 ?

That won't help, because the design of BLA changed and the old BLA has been
removed. Please keep the bridge out of bat0. :)

Cheers,
	Simon

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

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

* Re: [B.A.T.M.A.N.] BLAII + gw_mode, DHCP sometimes gets dropped
  2012-07-04  9:12 ` Simon Wunderlich
@ 2012-07-04 12:43   ` Guido Iribarren
  2012-07-04 13:12     ` Guido Iribarren
                       ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Guido Iribarren @ 2012-07-04 12:43 UTC (permalink / raw)
  To: The list for a Better Approach To Mobile Ad-hoc Networking

On Wed, Jul 4, 2012 at 6:12 AM, Simon Wunderlich
<simon.wunderlich@s2003.tu-chemnitz.de> wrote:
> Hello Guido,
>
> On Tue, Jul 03, 2012 at 05:07:17PM -0300, Guido Iribarren wrote:
>> Hello there again,
>> I have observed a problem since updating to 2012.2 and enabled BLAII
>>
>> I'm compiling logs to understand what's happening, but as always,
>> reading logs only gets me more lost :(
>> So here i am again begging for help
>
> There are some debug levels for BLA as well, and you can now get the
> claimlist with batctl (which is basically the list of clients a gateway
> feels responsible for) - this may help for debugging. But first,
> we should clarify some more details for your setup.
Yes, I've seen the cl command, but didn't completely understand how to
interpret it. For example, right now I see the clients claimed in the
cl of mesh nodes, and even the same client claimed in different nodes.

( when I say mesh nodes, and in the rest of the email, i'm referring
to http://www.open-mesh.org/wiki/batman-adv/Bridge-loop-avoidance-II#Definitions
)

i.e. sample mesh-nodes:
root@charly:~# batctl cl
Claims announced for the mesh bat0 (orig            charly, group id 6412)
   Client               VID      Originator        [o] (CRC )
 * 00:25:d3:f5:93:76 on    -1 by            charly [x] (77f9)
 * f8d1113b6e66_eth0 on    -1 by            charly [x] (77f9)

root@hquilla:~# batctl cl
Claims announced for the mesh bat0 (orig           hquilla, group id 82cb)
   Client               VID      Originator        [o] (CRC )
 * 00:25:d3:f5:93:76 on    -1 by           hquilla [x] (c72e)
 * 00:24:81:4b:ea:6d on    -1 by           hquilla [x] (c72e)

maybe that's fine because they have different group ids? (??)
is there any documentation on the cl output?
as far i could interpret, CRC "identifies" a particular version of a table,
[o] = [x] means "this is claimed by myself"
group id identifies different backbones (like in this case:)
http://www.open-mesh.org/wiki/batman-adv/Bridge-loop-avoidance-Testcases#Two-LANs-connected-by-one-mesh

and VID, is always set to -1 :P
oh, maaaaybe it's vlan id (?) since i'm not using VLANs

>>
>> the setup is the same I described in yesterday's attachment, but
>> what's not pictured is an ethernet cable between colmena-casa and
>> f8d11504758.
>> f8d11504758 is the only router that connects to the internet (through
>> WAN cable), and it's also the only one that has dnsmasq running and
>> gw_mode=server.
>> All the other nodes have gw_mode=client
>>
>> All of the nodes have bridge_loop_avoidance=1
>> (even though there are no other utp connections, so it could in fact
>> be enabled only on colmena-casa and f8d11504758)
>>
>> with this setup, dhcp requests from the mesh sometimes get "lost",
>> either they don't reach f8d11504758 or the reply doesn't get out
>
> Questions:
>  * which node runs the DHCP server? colmena-casa, f8d11504758 or something else?
Only the node f8d11504758 runs a DHCP server (dnsmasq) on its interface br-lan
no other dhcp server is running on the network

>  * at which point is DHCP getting lost? is the DISCOVER/REQUEST from the client
>    getting lost, or the reply from the server?

Well, I just managed to get a clarifying tcpdump!
hquilla sent a select (REQUEST) that reached the wlan0-2 (mesh)
interface of f8d11504758 and it was silently dropped (didn't appear on
a batctl td of bat0)
this repeated several times, until a lucky REQUEST managed to pass
through, was sniffed at bat0, and got a reply from dnsmasq

I couldn't see any difference between the unlucky and lucky REQUESTs
or DISCOVERs,
but running a "batctl cl -w1" did the trick:
when the client is currently claimed by f8d11504758 as in
 *      hquilla_eth0 on    -1 by      f8d11504758 [x] (d38b)

both the REQUESTs and DISCOVERs reach dnsmasq fine

but if the client is currently claimed by colmena-casa as in
 *      hquilla_eth0 on    -1 by      colmena-casa [ ] (3d7f)
these discover/requests get dropped by batman when they arrive through wlan0-2

>  * Can you specify "sometimes" a little bit more? What are the circumstances, how
>    often does it happen?
Well, most of the time :) dhcp clients keep trying and eventually they
get the lease, but in unlucky times that might even take hours :(
at any point in time, there are "lucky" clients who can get a lease
and renew it without problem, and other "unlucky" that can't get a
reply at all.

from what i've just seen at the "batctl cl", this luck is related to
being claimed by the "right" backbone node.

>
>>
>> this didn't happen with batman 2012.1 , setup as indicated by the BLAI
>> wiki page (batctl if add br-lan)
>> furthermore, with batman 2012.2 , BLAII activated, but gw_mode=off in
>> all nodes, DHCP also works fine.
>
> Mhm, that's rather strange ... we had a similar problem when ap isolation
> was activated. Do you have this feature turned on?
Nope

>
> So DHCP is only having problems when gw-mode is turned on colmena-casa
> and f8d11504758?

gw-mode is activated in all mesh nodes, not only in colmena-casa and
f8d11504758
it's set to client on every node except f8d11504758, which has gw_mode=server

As far as i can recall, disabling gw_mode=client in every mesh node,
solved the problem.
But now that i found out about this "batctl td" thing, i'm in doubt
about the validity of the previous statement :(
i should check again and report.

>>
>> So, a few questions arise:
>> is it a problem to activate bridge_loop_avoidance=1 in all nodes,
>> regardless of the fact that they "need" it or not? (that is, it is
>> activated on nodes that don't have any ethernet cables connected and
>> couldn't possibly create a bridge loop)
>
> No, that it is not a problem - you can activate it everywhere. It will
> just send some additional control packets on bat0, but won't do anything
> as long as it does not detect other gateways.
>
>>
>> would it make a difference, if I add br-lan to bat0 (batctl if add
>> br-lan) the way I used to do with batman 2012.1 ?
>
> That won't help, because the design of BLA changed and the old BLA has been
> removed. Please keep the bridge out of bat0. :)
Ok, thanks a lot for the clarifications!

Have a great day,

Gui

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

* Re: [B.A.T.M.A.N.] BLAII + gw_mode, DHCP sometimes gets dropped
  2012-07-04 12:43   ` Guido Iribarren
@ 2012-07-04 13:12     ` Guido Iribarren
  2012-07-04 14:05       ` Simon Wunderlich
  2012-07-04 13:49     ` [B.A.T.M.A.N.] BLAII + gw_mode, DHCP sometimes gets dropped Simon Wunderlich
  2012-07-04 13:59     ` [B.A.T.M.A.N.] [PATCH] batman-adv: check incoming packet type for bla Simon Wunderlich
  2 siblings, 1 reply; 16+ messages in thread
From: Guido Iribarren @ 2012-07-04 13:12 UTC (permalink / raw)
  To: The list for a Better Approach To Mobile Ad-hoc Networking

On Wed, Jul 4, 2012 at 9:43 AM, Guido Iribarren
<guidoiribarren@buenosaireslibre.org> wrote:
> On Wed, Jul 4, 2012 at 6:12 AM, Simon Wunderlich
> <simon.wunderlich@s2003.tu-chemnitz.de> wrote:
>> Hello Guido,
>>

> gw-mode is activated in all mesh nodes, not only in colmena-casa and
> f8d11504758
> it's set to client on every node except f8d11504758, which has gw_mode=server
>
> As far as i can recall, disabling gw_mode=client in every mesh node,
> solved the problem.
> But now that i found out about this "batctl td" thing, i'm in doubt
> about the validity of the previous statement :(
> i should check again and report.

I can now confirm this.

if i set gw_mode=off in a particular mesh node, DHCP requests
originating from that node work correctly, even when the mesh node
client is claimed by the "wrong" backbone node.

in this scenario:
 *         ruth_eth0 on    -1 by      colmena-casa [ ] (7a27)
 *      hquilla_eth0 on    -1 by      colmena-casa [ ] (7a27)

root@ruth:~# batctl gw off
root@ruth:~# /sbin/udhcpc -t 0 -i br-lan:ipv4 -f -R
udhcpc (v1.19.4) started
Sending discover...
Sending select for 10.254.0.131...
Lease of 10.254.0.131 obtained, lease time 600

root@hquilla:~# batctl gw client
root@hquilla:~# /sbin/udhcpc -t 0 -i br-lan:ipv4 -f -R
udhcpc (v1.19.4) started
Sending discover...
Sending discover...
Sending discover...^C

when hquilla sends the request, this is received at f8d11504758
interface wlan0-2
BAT colmena-casa > f8d11504758: UCAST.......
and gets dropped (doesn't reach dnsmasq)

when ruth (gw_mode=off) sends the request, this is received at
f8d11504758 interface wlan0-2
BAT colmena: BCAST, orig ruth, ......
(then rebroadcasted several times)
and it reaches dnsmasq properly.


If you prefer the raw "batctl td" for some eye-injuring reason, here you go:
http://pastebin.com/HNBg01hR

hope that helps!

Gui

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

* Re: [B.A.T.M.A.N.] BLAII + gw_mode, DHCP sometimes gets dropped
  2012-07-04 12:43   ` Guido Iribarren
  2012-07-04 13:12     ` Guido Iribarren
@ 2012-07-04 13:49     ` Simon Wunderlich
  2012-07-04 13:59     ` [B.A.T.M.A.N.] [PATCH] batman-adv: check incoming packet type for bla Simon Wunderlich
  2 siblings, 0 replies; 16+ messages in thread
From: Simon Wunderlich @ 2012-07-04 13:49 UTC (permalink / raw)
  To: The list for a Better Approach To Mobile Ad-hoc Networking

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

On Wed, Jul 04, 2012 at 09:43:13AM -0300, Guido Iribarren wrote:
> On Wed, Jul 4, 2012 at 6:12 AM, Simon Wunderlich
> <simon.wunderlich@s2003.tu-chemnitz.de> wrote:
> > Hello Guido,
> >
> > On Tue, Jul 03, 2012 at 05:07:17PM -0300, Guido Iribarren wrote:
> >> Hello there again,
> >> I have observed a problem since updating to 2012.2 and enabled BLAII
> >>
> >> I'm compiling logs to understand what's happening, but as always,
> >> reading logs only gets me more lost :(
> >> So here i am again begging for help
> >
> > There are some debug levels for BLA as well, and you can now get the
> > claimlist with batctl (which is basically the list of clients a gateway
> > feels responsible for) - this may help for debugging. But first,
> > we should clarify some more details for your setup.
> Yes, I've seen the cl command, but didn't completely understand how to
> interpret it. For example, right now I see the clients claimed in the
> cl of mesh nodes, and even the same client claimed in different nodes.
> 

> ( when I say mesh nodes, and in the rest of the email, i'm referring
> to http://www.open-mesh.org/wiki/batman-adv/Bridge-loop-avoidance-II#Definitions
> )
> 
> i.e. sample mesh-nodes:
> root@charly:~# batctl cl
> Claims announced for the mesh bat0 (orig            charly, group id 6412)
>    Client               VID      Originator        [o] (CRC )
>  * 00:25:d3:f5:93:76 on    -1 by            charly [x] (77f9)
>  * f8d1113b6e66_eth0 on    -1 by            charly [x] (77f9)
> 
> root@hquilla:~# batctl cl
> Claims announced for the mesh bat0 (orig           hquilla, group id 82cb)
>    Client               VID      Originator        [o] (CRC )
>  * 00:25:d3:f5:93:76 on    -1 by           hquilla [x] (c72e)
>  * 00:24:81:4b:ea:6d on    -1 by           hquilla [x] (c72e)
> 
> maybe that's fine because they have different group ids? (??)

Yup. They are not interconnected via Ethernet, so they are in different
backbones and have a different group id.

If there are other gatways on the backbone with claims, you should see them
in the claim list as well.

> is there any documentation on the cl output?

Yup, you can find it here:

http://www.open-mesh.org/wiki/batman-adv/Understand-your-batman-adv-network

> as far i could interpret, CRC "identifies" a particular version of a table,

yup.

> [o] = [x] means "this is claimed by myself"

yup.

> group id identifies different backbones (like in this case:)
> http://www.open-mesh.org/wiki/batman-adv/Bridge-loop-avoidance-Testcases#Two-LANs-connected-by-one-mesh
> 

yup.

> and VID, is always set to -1 :P
> oh, maaaaybe it's vlan id (?) since i'm not using VLANs

it means you have no VLAN here.

> 
> >>
> >> the setup is the same I described in yesterday's attachment, but
> >> what's not pictured is an ethernet cable between colmena-casa and
> >> f8d11504758.
> >> f8d11504758 is the only router that connects to the internet (through
> >> WAN cable), and it's also the only one that has dnsmasq running and
> >> gw_mode=server.
> >> All the other nodes have gw_mode=client
> >>
> >> All of the nodes have bridge_loop_avoidance=1
> >> (even though there are no other utp connections, so it could in fact
> >> be enabled only on colmena-casa and f8d11504758)
> >>
> >> with this setup, dhcp requests from the mesh sometimes get "lost",
> >> either they don't reach f8d11504758 or the reply doesn't get out
> >
> > Questions:
> >  * which node runs the DHCP server? colmena-casa, f8d11504758 or something else?
> Only the node f8d11504758 runs a DHCP server (dnsmasq) on its interface br-lan
> no other dhcp server is running on the network
> 

OK.

> >  * at which point is DHCP getting lost? is the DISCOVER/REQUEST from the client
> >    getting lost, or the reply from the server?
> 
> Well, I just managed to get a clarifying tcpdump!
> hquilla sent a select (REQUEST) that reached the wlan0-2 (mesh)
> interface of f8d11504758 and it was silently dropped (didn't appear on
> a batctl td of bat0)
> this repeated several times, until a lucky REQUEST managed to pass
> through, was sniffed at bat0, and got a reply from dnsmasq
> 
> I couldn't see any difference between the unlucky and lucky REQUESTs
> or DISCOVERs,
> but running a "batctl cl -w1" did the trick:
> when the client is currently claimed by f8d11504758 as in
>  *      hquilla_eth0 on    -1 by      f8d11504758 [x] (d38b)
> 
> both the REQUESTs and DISCOVERs reach dnsmasq fine
> 
> but if the client is currently claimed by colmena-casa as in
>  *      hquilla_eth0 on    -1 by      colmena-casa [ ] (3d7f)
> these discover/requests get dropped by batman when they arrive through wlan0-2
> 

Ah, this helps. If the client is claimed by colmena-case, the request
should go from hquilla to colmena-casa via mesh, and from colmena-casa to
f8d11504758 via LAN. 

I guess the problem is the interaction between BLA and the gateway feature. The DHCP
request is sent via unicast to f8d11504758, but the destination address is still
broadcast. The bla implementation in f8d11504758 will then think that colmena-casa 
also has received the broadcast (but it didn't), and therefore drop it.

I'll send a patch for that soon ...

> >
> > So DHCP is only having problems when gw-mode is turned on colmena-casa
> > and f8d11504758?
> 
> gw-mode is activated in all mesh nodes, not only in colmena-casa and
> f8d11504758
> it's set to client on every node except f8d11504758, which has gw_mode=server
> 
> As far as i can recall, disabling gw_mode=client in every mesh node,
> solved the problem.
> But now that i found out about this "batctl td" thing, i'm in doubt
> about the validity of the previous statement :(
> i should check again and report.
> 

That would at least match to the hypothesis. :)

[removed the rest of the text. will send the patch soon ...]

Cheers,
	Simon

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

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

* [B.A.T.M.A.N.] [PATCH] batman-adv: check incoming packet type for bla
  2012-07-04 12:43   ` Guido Iribarren
  2012-07-04 13:12     ` Guido Iribarren
  2012-07-04 13:49     ` [B.A.T.M.A.N.] BLAII + gw_mode, DHCP sometimes gets dropped Simon Wunderlich
@ 2012-07-04 13:59     ` Simon Wunderlich
  2012-07-04 18:40       ` [B.A.T.M.A.N.] [PATCH-v2] " Simon Wunderlich
  2 siblings, 1 reply; 16+ messages in thread
From: Simon Wunderlich @ 2012-07-04 13:59 UTC (permalink / raw)
  To: b.a.t.m.a.n; +Cc: Simon Wunderlich

If the gateway functionality is used, some broadcast packets (DHCP
requests) may be transmitted as unicast packets. As the bridge loop
avoidance code now only considers the payload Ethernet destination,
it may drop the DHCP request for clients which are claimed by other
backbone gateways, because it falsely infers from the broadcast address
that the right backbone gateway should havehandled the broadcast.

Fix this by checking and delegating the batman-adv packet type used
for transmission.

Reported-by: Guido Iribarren <guidoiribarren@buenosaireslibre.org>
Signed-off-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
---
 bridge_loop_avoidance.c |    6 ++++--
 bridge_loop_avoidance.h |    5 +++--
 soft-interface.c        |    6 +++++-
 3 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/bridge_loop_avoidance.c b/bridge_loop_avoidance.c
index 6452e2f..8478083 100644
--- a/bridge_loop_avoidance.c
+++ b/bridge_loop_avoidance.c
@@ -1368,6 +1368,7 @@ void batadv_bla_free(struct batadv_priv *bat_priv)
 /* @bat_priv: the bat priv with all the soft interface information
  * @skb: the frame to be checked
  * @vid: the VLAN ID of the frame
+ * @is_bcast: the packet came in a broadcast packet type
  *
  * bla_rx avoidance checks if:
  *  * we have to race for a claim
@@ -1377,7 +1378,8 @@ void batadv_bla_free(struct batadv_priv *bat_priv)
  * returns 1, otherwise it returns 0 and the caller shall further
  * process the skb.
  */
-int batadv_bla_rx(struct batadv_priv *bat_priv, struct sk_buff *skb, short vid)
+int batadv_bla_rx(struct batadv_priv *bat_priv, struct sk_buff *skb, short vid,
+		  bool is_bcast)
 {
 	struct ethhdr *ethhdr;
 	struct batadv_claim search_claim, *claim = NULL;
@@ -1422,7 +1424,7 @@ int batadv_bla_rx(struct batadv_priv *bat_priv, struct sk_buff *skb, short vid)
 	}
 
 	/* if it is a broadcast ... */
-	if (is_multicast_ether_addr(ethhdr->h_dest)) {
+	if (is_multicast_ether_addr(ethhdr->h_dest) && is_bcast) {
 		/* ... drop it. the responsible gateway is in charge. */
 		goto handled;
 	} else {
diff --git a/bridge_loop_avoidance.h b/bridge_loop_avoidance.h
index 35d39e3..789cb73 100644
--- a/bridge_loop_avoidance.h
+++ b/bridge_loop_avoidance.h
@@ -21,7 +21,8 @@
 #define _NET_BATMAN_ADV_BLA_H_
 
 #ifdef CONFIG_BATMAN_ADV_BLA
-int batadv_bla_rx(struct batadv_priv *bat_priv, struct sk_buff *skb, short vid);
+int batadv_bla_rx(struct batadv_priv *bat_priv, struct sk_buff *skb, short vid,
+		  bool is_bcast);
 int batadv_bla_tx(struct batadv_priv *bat_priv, struct sk_buff *skb, short vid);
 int batadv_bla_is_backbone_gw(struct sk_buff *skb,
 			      struct batadv_orig_node *orig_node, int hdr_size);
@@ -42,7 +43,7 @@ void batadv_bla_free(struct batadv_priv *bat_priv);
 #else /* ifdef CONFIG_BATMAN_ADV_BLA */
 
 static inline int batadv_bla_rx(struct batadv_priv *bat_priv,
-				struct sk_buff *skb, short vid)
+				struct sk_buff *skb, short vid, bool is_bcast)
 {
 	return 0;
 }
diff --git a/soft-interface.c b/soft-interface.c
index 96736ea..be39f89 100644
--- a/soft-interface.c
+++ b/soft-interface.c
@@ -274,9 +274,13 @@ void batadv_interface_rx(struct net_device *soft_iface,
 	struct batadv_priv *bat_priv = netdev_priv(soft_iface);
 	struct ethhdr *ethhdr;
 	struct vlan_ethhdr *vhdr;
+	struct batadv_header *batadv_header = (struct batadv_header *)skb->data;
 	short vid __maybe_unused = -1;
+	bool is_bcast;
 	__be16 ethertype = __constant_htons(BATADV_ETH_P_BATMAN);
 
+	is_bcast = !!(batadv_header->packet_type == BATADV_BCAST);
+
 	/* check if enough space is available for pulling, and pull */
 	if (!pskb_may_pull(skb, hdr_size))
 		goto dropped;
@@ -323,7 +327,7 @@ void batadv_interface_rx(struct net_device *soft_iface,
 	/* Let the bridge loop avoidance check the packet. If will
 	 * not handle it, we can safely push it up.
 	 */
-	if (batadv_bla_rx(bat_priv, skb, vid))
+	if (batadv_bla_rx(bat_priv, skb, vid, is_bcast))
 		goto out;
 
 	netif_rx(skb);
-- 
1.7.10


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

* Re: [B.A.T.M.A.N.] BLAII + gw_mode, DHCP sometimes gets dropped
  2012-07-04 13:12     ` Guido Iribarren
@ 2012-07-04 14:05       ` Simon Wunderlich
  2012-07-04 18:12         ` Guido Iribarren
  0 siblings, 1 reply; 16+ messages in thread
From: Simon Wunderlich @ 2012-07-04 14:05 UTC (permalink / raw)
  To: The list for a Better Approach To Mobile Ad-hoc Networking

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

Hey Guido,

thanks again for the good debugging, that really helped
to understand the problem :)

I've sent a patch, is it possible for you to apply it
and retry? it should be enough to update the two 
gateway nodes.

Cheers
	Simon

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

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

* Re: [B.A.T.M.A.N.] BLAII + gw_mode, DHCP sometimes gets dropped
  2012-07-04 14:05       ` Simon Wunderlich
@ 2012-07-04 18:12         ` Guido Iribarren
  2012-07-04 18:30           ` Simon Wunderlich
                             ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Guido Iribarren @ 2012-07-04 18:12 UTC (permalink / raw)
  To: The list for a Better Approach To Mobile Ad-hoc Networking

On Wed, Jul 4, 2012 at 11:05 AM, Simon Wunderlich
<simon.wunderlich@s2003.tu-chemnitz.de> wrote:
> Hey Guido,
>
> thanks again for the good debugging, that really helped
> to understand the problem :)
Thanks free software! :D

> I've sent a patch, is it possible for you to apply it
> and retry? it should be enough to update the two
> gateway nodes.
Definitely!
here's the output, tho:

Applying ./patches/9999-batman-adv-check-incoming-packet-type-for-bla.patch
using plaintext:
patching file bridge_loop_avoidance.c
Hunk #1 succeeded at 1351 with fuzz 1 (offset -17 lines).
Hunk #2 FAILED at 1378.
Hunk #3 FAILED at 1423.
2 out of 3 hunks FAILED -- saving rejects to file bridge_loop_avoidance.c.rej
patching file bridge_loop_avoidance.h
Hunk #1 FAILED at 21.
Hunk #2 FAILED at 42.
2 out of 2 hunks FAILED -- saving rejects to file bridge_loop_avoidance.h.rej
patching file soft-interface.c
Hunk #1 FAILED at 274.
Hunk #2 FAILED at 323.
2 out of 2 hunks FAILED -- saving rejects to file soft-interface.c.rej
Patch failed!  Please fix
./patches/9999-batman-adv-check-incoming-packet-type-for-bla.patch!

i'm trying to apply against
openwrt@torvic:~/trunk/feeds/packages/net/batman-adv$ svn info
Path: .
URL: svn://svn.openwrt.org/openwrt/packages/net/batman-adv
Repository Root: svn://svn.openwrt.org/openwrt
Repository UUID: 3c298f89-4303-0410-b956-a3cf2f4a3e73
Revision: 32587
Node Kind: directory
Schedule: normal
Last Changed Author: marek
Last Changed Rev: 32579
Last Changed Date: 2012-07-02 13:20:20 -0300 (Mon, 02 Jul 2012)


Visually inspecting the rejects, seems my source code still doesn't
have the batadv_ prefix in functions names.
i.e.:

int bla_rx(struct bat_priv *bat_priv, struct sk_buff *skb, short vid)

I've seen many patches flying around dealing with the batadv_ prefix
for dave, but didn't pay much attention to them: which of those should
i grab?

Or, Simon, could you kindly send another patch that i can apply
cleanly to r32587 of
 svn://svn.openwrt.org/openwrt/packages/net/batman-adv

in any case, i see the changes are simple, so i *could* try a manual
apply of the patch....... but last time i tried (with other patch), it
got overwritten by openwrt build system.

Thanks a lot for the support!

Gui



>
> Cheers
>         Simon
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
>
> iEYEARECAAYFAk/0TYwACgkQrzg/fFk7axbznwCg5oyyF4UaWJBV8GXztFSGG8sa
> vnAAoJYcgNuCcdA5fZKrIvaUE888FzSI
> =zd3Q
> -----END PGP SIGNATURE-----
>

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

* Re: [B.A.T.M.A.N.] BLAII + gw_mode, DHCP sometimes gets dropped
  2012-07-04 18:12         ` Guido Iribarren
@ 2012-07-04 18:30           ` Simon Wunderlich
  2012-07-04 18:46             ` Guido Iribarren
  2012-07-04 18:33           ` [B.A.T.M.A.N.] [PATCH-maint] batman-adv: check incoming packet type for bla Simon Wunderlich
  2012-07-04 18:38           ` [B.A.T.M.A.N.] [PATCH-maint-v2] " Simon Wunderlich
  2 siblings, 1 reply; 16+ messages in thread
From: Simon Wunderlich @ 2012-07-04 18:30 UTC (permalink / raw)
  To: The list for a Better Approach To Mobile Ad-hoc Networking

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

Sorry, my fault. Will send a patch based on maint in a minute.

On Wed, Jul 04, 2012 at 03:12:59PM -0300, Guido Iribarren wrote:
> On Wed, Jul 4, 2012 at 11:05 AM, Simon Wunderlich
> <simon.wunderlich@s2003.tu-chemnitz.de> wrote:
> > Hey Guido,
> >
> > thanks again for the good debugging, that really helped
> > to understand the problem :)
> Thanks free software! :D
> 
> > I've sent a patch, is it possible for you to apply it
> > and retry? it should be enough to update the two
> > gateway nodes.
> Definitely!
> here's the output, tho:
> 
> Applying ./patches/9999-batman-adv-check-incoming-packet-type-for-bla.patch
> using plaintext:
> patching file bridge_loop_avoidance.c
> Hunk #1 succeeded at 1351 with fuzz 1 (offset -17 lines).
> Hunk #2 FAILED at 1378.
> Hunk #3 FAILED at 1423.
> 2 out of 3 hunks FAILED -- saving rejects to file bridge_loop_avoidance.c.rej
> patching file bridge_loop_avoidance.h
> Hunk #1 FAILED at 21.
> Hunk #2 FAILED at 42.
> 2 out of 2 hunks FAILED -- saving rejects to file bridge_loop_avoidance.h.rej
> patching file soft-interface.c
> Hunk #1 FAILED at 274.
> Hunk #2 FAILED at 323.
> 2 out of 2 hunks FAILED -- saving rejects to file soft-interface.c.rej
> Patch failed!  Please fix
> ./patches/9999-batman-adv-check-incoming-packet-type-for-bla.patch!
> 
> i'm trying to apply against
> openwrt@torvic:~/trunk/feeds/packages/net/batman-adv$ svn info
> Path: .
> URL: svn://svn.openwrt.org/openwrt/packages/net/batman-adv
> Repository Root: svn://svn.openwrt.org/openwrt
> Repository UUID: 3c298f89-4303-0410-b956-a3cf2f4a3e73
> Revision: 32587
> Node Kind: directory
> Schedule: normal
> Last Changed Author: marek
> Last Changed Rev: 32579
> Last Changed Date: 2012-07-02 13:20:20 -0300 (Mon, 02 Jul 2012)
> 
> 
> Visually inspecting the rejects, seems my source code still doesn't
> have the batadv_ prefix in functions names.
> i.e.:
> 
> int bla_rx(struct bat_priv *bat_priv, struct sk_buff *skb, short vid)
> 
> I've seen many patches flying around dealing with the batadv_ prefix
> for dave, but didn't pay much attention to them: which of those should
> i grab?
> 
> Or, Simon, could you kindly send another patch that i can apply
> cleanly to r32587 of
>  svn://svn.openwrt.org/openwrt/packages/net/batman-adv
> 
> in any case, i see the changes are simple, so i *could* try a manual
> apply of the patch....... but last time i tried (with other patch), it
> got overwritten by openwrt build system.
> 
> Thanks a lot for the support!
> 
> Gui
> 
> 
> 
> >
> > Cheers
> >         Simon
> >
> > -----BEGIN PGP SIGNATURE-----
> > Version: GnuPG v1.4.10 (GNU/Linux)
> >
> > iEYEARECAAYFAk/0TYwACgkQrzg/fFk7axbznwCg5oyyF4UaWJBV8GXztFSGG8sa
> > vnAAoJYcgNuCcdA5fZKrIvaUE888FzSI
> > =zd3Q
> > -----END PGP SIGNATURE-----
> >
> 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

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

* [B.A.T.M.A.N.] [PATCH-maint] batman-adv: check incoming packet type for bla
  2012-07-04 18:12         ` Guido Iribarren
  2012-07-04 18:30           ` Simon Wunderlich
@ 2012-07-04 18:33           ` Simon Wunderlich
  2012-07-04 18:38           ` [B.A.T.M.A.N.] [PATCH-maint-v2] " Simon Wunderlich
  2 siblings, 0 replies; 16+ messages in thread
From: Simon Wunderlich @ 2012-07-04 18:33 UTC (permalink / raw)
  To: b.a.t.m.a.n; +Cc: Simon Wunderlich

If the gateway functionality is used, some broadcast packets (DHCP
requests) may be transmitted as unicast packets. As the bridge loop
avoidance code now only considers the payload Ethernet destination,
it may drop the DHCP request for clients which are claimed by other
backbone gateways, because it falsely infers from the broadcast address
that the right backbone gateway should havehandled the broadcast.

Fix this by checking and delegating the batman-adv packet type used
for transmission.

Reported-by: Guido Iribarren <guidoiribarren@buenosaireslibre.org>
Signed-off-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
---
PATCHv2 adds a comment why we are doing this, and adds the is_bcast
check to the other is_multicast_ether_addr() check in this function.

PATCH-maint is the backport for maint
---
 bridge_loop_avoidance.c |   15 +++++++++++----
 bridge_loop_avoidance.h |    5 +++--
 soft-interface.c        |    6 +++++-
 3 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/bridge_loop_avoidance.c b/bridge_loop_avoidance.c
index 8bf9751..2b82a0f 100644
--- a/bridge_loop_avoidance.c
+++ b/bridge_loop_avoidance.c
@@ -1351,6 +1351,7 @@ void bla_free(struct bat_priv *bat_priv)
  * @bat_priv: the bat priv with all the soft interface information
  * @skb: the frame to be checked
  * @vid: the VLAN ID of the frame
+ * @is_bcast: the packet came in a broadcast packet type.
  *
  * bla_rx avoidance checks if:
  *  * we have to race for a claim
@@ -1361,7 +1362,8 @@ void bla_free(struct bat_priv *bat_priv)
  * process the skb.
  *
  */
-int bla_rx(struct bat_priv *bat_priv, struct sk_buff *skb, short vid)
+int bla_rx(struct bat_priv *bat_priv, struct sk_buff *skb, short vid,
+	   bool is_bcast)
 {
 	struct ethhdr *ethhdr;
 	struct claim search_claim, *claim = NULL;
@@ -1380,7 +1382,7 @@ int bla_rx(struct bat_priv *bat_priv, struct sk_buff *skb, short vid)
 
 	if (unlikely(atomic_read(&bat_priv->bla_num_requests)))
 		/* don't allow broadcasts while requests are in flight */
-		if (is_multicast_ether_addr(ethhdr->h_dest))
+		if (is_multicast_ether_addr(ethhdr->h_dest) && is_bcast)
 			goto handled;
 
 	memcpy(search_claim.addr, ethhdr->h_source, ETH_ALEN);
@@ -1406,8 +1408,13 @@ int bla_rx(struct bat_priv *bat_priv, struct sk_buff *skb, short vid)
 	}
 
 	/* if it is a broadcast ... */
-	if (is_multicast_ether_addr(ethhdr->h_dest)) {
-		/* ... drop it. the responsible gateway is in charge. */
+	if (is_multicast_ether_addr(ethhdr->h_dest) && is_bcast) {
+		/* ... drop it. the responsible gateway is in charge.
+		 *
+		 * We need to check is_bcast because with the gateway
+		 * feature, broadcasts (like DHCP requests) may be sent
+		 * using a unicast packet type. 
+		 */
 		goto handled;
 	} else {
 		/* seems the client considers us as its best gateway.
diff --git a/bridge_loop_avoidance.h b/bridge_loop_avoidance.h
index e39f93a..dc5227b 100644
--- a/bridge_loop_avoidance.h
+++ b/bridge_loop_avoidance.h
@@ -23,7 +23,8 @@
 #define _NET_BATMAN_ADV_BLA_H_
 
 #ifdef CONFIG_BATMAN_ADV_BLA
-int bla_rx(struct bat_priv *bat_priv, struct sk_buff *skb, short vid);
+int bla_rx(struct bat_priv *bat_priv, struct sk_buff *skb, short vid,
+	   bool is_bcast);
 int bla_tx(struct bat_priv *bat_priv, struct sk_buff *skb, short vid);
 int bla_is_backbone_gw(struct sk_buff *skb,
 		       struct orig_node *orig_node, int hdr_size);
@@ -41,7 +42,7 @@ void bla_free(struct bat_priv *bat_priv);
 #else /* ifdef CONFIG_BATMAN_ADV_BLA */
 
 static inline int bla_rx(struct bat_priv *bat_priv, struct sk_buff *skb,
-			 short vid)
+			 short vid, bool is_bcast)
 {
 	return 0;
 }
diff --git a/soft-interface.c b/soft-interface.c
index 6e2530b..92a4070 100644
--- a/soft-interface.c
+++ b/soft-interface.c
@@ -256,7 +256,11 @@ void interface_rx(struct net_device *soft_iface,
 	struct bat_priv *bat_priv = netdev_priv(soft_iface);
 	struct ethhdr *ethhdr;
 	struct vlan_ethhdr *vhdr;
+	struct batman_header *batadv_header = (struct batman_header *)skb->data;
 	short vid __maybe_unused = -1;
+	bool is_bcast;
+
+	is_bcast = !!(batadv_header->packet_type == BAT_BCAST);
 
 	/* check if enough space is available for pulling, and pull */
 	if (!pskb_may_pull(skb, hdr_size))
@@ -302,7 +306,7 @@ void interface_rx(struct net_device *soft_iface,
 	/* Let the bridge loop avoidance check the packet. If will
 	 * not handle it, we can safely push it up.
 	 */
-	if (bla_rx(bat_priv, skb, vid))
+	if (bla_rx(bat_priv, skb, vid, is_bcast))
 		goto out;
 
 	netif_rx(skb);
-- 
1.7.10


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

* [B.A.T.M.A.N.] [PATCH-maint-v2] batman-adv: check incoming packet type for bla
  2012-07-04 18:12         ` Guido Iribarren
  2012-07-04 18:30           ` Simon Wunderlich
  2012-07-04 18:33           ` [B.A.T.M.A.N.] [PATCH-maint] batman-adv: check incoming packet type for bla Simon Wunderlich
@ 2012-07-04 18:38           ` Simon Wunderlich
  2012-07-05 22:04             ` Marek Lindner
  2 siblings, 1 reply; 16+ messages in thread
From: Simon Wunderlich @ 2012-07-04 18:38 UTC (permalink / raw)
  To: b.a.t.m.a.n; +Cc: Simon Wunderlich

If the gateway functionality is used, some broadcast packets (DHCP
requests) may be transmitted as unicast packets. As the bridge loop
avoidance code now only considers the payload Ethernet destination,
it may drop the DHCP request for clients which are claimed by other
backbone gateways, because it falsely infers from the broadcast address
that the right backbone gateway should havehandled the broadcast.

Fix this by checking and delegating the batman-adv packet type used
for transmission.

Reported-by: Guido Iribarren <guidoiribarren@buenosaireslibre.org>
Signed-off-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
---
PATCHv2 adds a comment why we are doing this, and adds the is_bcast
check to the other is_multicast_ether_addr() check in this function.

PATCH-maint is the backport for maint

PATCH-maint-v2 is even checkpatch clean, and remove obsolete !!
---
 bridge_loop_avoidance.c |   15 +++++++++++----
 bridge_loop_avoidance.h |    5 +++--
 soft-interface.c        |    6 +++++-
 3 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/bridge_loop_avoidance.c b/bridge_loop_avoidance.c
index 8bf9751..c5863f4 100644
--- a/bridge_loop_avoidance.c
+++ b/bridge_loop_avoidance.c
@@ -1351,6 +1351,7 @@ void bla_free(struct bat_priv *bat_priv)
  * @bat_priv: the bat priv with all the soft interface information
  * @skb: the frame to be checked
  * @vid: the VLAN ID of the frame
+ * @is_bcast: the packet came in a broadcast packet type.
  *
  * bla_rx avoidance checks if:
  *  * we have to race for a claim
@@ -1361,7 +1362,8 @@ void bla_free(struct bat_priv *bat_priv)
  * process the skb.
  *
  */
-int bla_rx(struct bat_priv *bat_priv, struct sk_buff *skb, short vid)
+int bla_rx(struct bat_priv *bat_priv, struct sk_buff *skb, short vid,
+	   bool is_bcast)
 {
 	struct ethhdr *ethhdr;
 	struct claim search_claim, *claim = NULL;
@@ -1380,7 +1382,7 @@ int bla_rx(struct bat_priv *bat_priv, struct sk_buff *skb, short vid)
 
 	if (unlikely(atomic_read(&bat_priv->bla_num_requests)))
 		/* don't allow broadcasts while requests are in flight */
-		if (is_multicast_ether_addr(ethhdr->h_dest))
+		if (is_multicast_ether_addr(ethhdr->h_dest) && is_bcast)
 			goto handled;
 
 	memcpy(search_claim.addr, ethhdr->h_source, ETH_ALEN);
@@ -1406,8 +1408,13 @@ int bla_rx(struct bat_priv *bat_priv, struct sk_buff *skb, short vid)
 	}
 
 	/* if it is a broadcast ... */
-	if (is_multicast_ether_addr(ethhdr->h_dest)) {
-		/* ... drop it. the responsible gateway is in charge. */
+	if (is_multicast_ether_addr(ethhdr->h_dest) && is_bcast) {
+		/* ... drop it. the responsible gateway is in charge.
+		 *
+		 * We need to check is_bcast because with the gateway
+		 * feature, broadcasts (like DHCP requests) may be sent
+		 * using a unicast packet type.
+		 */
 		goto handled;
 	} else {
 		/* seems the client considers us as its best gateway.
diff --git a/bridge_loop_avoidance.h b/bridge_loop_avoidance.h
index e39f93a..dc5227b 100644
--- a/bridge_loop_avoidance.h
+++ b/bridge_loop_avoidance.h
@@ -23,7 +23,8 @@
 #define _NET_BATMAN_ADV_BLA_H_
 
 #ifdef CONFIG_BATMAN_ADV_BLA
-int bla_rx(struct bat_priv *bat_priv, struct sk_buff *skb, short vid);
+int bla_rx(struct bat_priv *bat_priv, struct sk_buff *skb, short vid,
+	   bool is_bcast);
 int bla_tx(struct bat_priv *bat_priv, struct sk_buff *skb, short vid);
 int bla_is_backbone_gw(struct sk_buff *skb,
 		       struct orig_node *orig_node, int hdr_size);
@@ -41,7 +42,7 @@ void bla_free(struct bat_priv *bat_priv);
 #else /* ifdef CONFIG_BATMAN_ADV_BLA */
 
 static inline int bla_rx(struct bat_priv *bat_priv, struct sk_buff *skb,
-			 short vid)
+			 short vid, bool is_bcast)
 {
 	return 0;
 }
diff --git a/soft-interface.c b/soft-interface.c
index 6e2530b..a0ec0e4 100644
--- a/soft-interface.c
+++ b/soft-interface.c
@@ -256,7 +256,11 @@ void interface_rx(struct net_device *soft_iface,
 	struct bat_priv *bat_priv = netdev_priv(soft_iface);
 	struct ethhdr *ethhdr;
 	struct vlan_ethhdr *vhdr;
+	struct batman_header *batadv_header = (struct batman_header *)skb->data;
 	short vid __maybe_unused = -1;
+	bool is_bcast;
+
+	is_bcast = (batadv_header->packet_type == BAT_BCAST);
 
 	/* check if enough space is available for pulling, and pull */
 	if (!pskb_may_pull(skb, hdr_size))
@@ -302,7 +306,7 @@ void interface_rx(struct net_device *soft_iface,
 	/* Let the bridge loop avoidance check the packet. If will
 	 * not handle it, we can safely push it up.
 	 */
-	if (bla_rx(bat_priv, skb, vid))
+	if (bla_rx(bat_priv, skb, vid, is_bcast))
 		goto out;
 
 	netif_rx(skb);
-- 
1.7.10


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

* [B.A.T.M.A.N.] [PATCH-v2] batman-adv: check incoming packet type for bla
  2012-07-04 13:59     ` [B.A.T.M.A.N.] [PATCH] batman-adv: check incoming packet type for bla Simon Wunderlich
@ 2012-07-04 18:40       ` Simon Wunderlich
  0 siblings, 0 replies; 16+ messages in thread
From: Simon Wunderlich @ 2012-07-04 18:40 UTC (permalink / raw)
  To: b.a.t.m.a.n; +Cc: Simon Wunderlich

If the gateway functionality is used, some broadcast packets (DHCP
requests) may be transmitted as unicast packets. As the bridge loop
avoidance code now only considers the payload Ethernet destination,
it may drop the DHCP request for clients which are claimed by other
backbone gateways, because it falsely infers from the broadcast address
that the right backbone gateway should havehandled the broadcast.

Fix this by checking and delegating the batman-adv packet type used
for transmission.

Reported-by: Guido Iribarren <guidoiribarren@buenosaireslibre.org>
Signed-off-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
---
PATCHv2 adds a comment why we are doing this, and adds the is_bcast
check to the other is_multicast_ether_addr() check in this function,
and remove obsolete !!.
---
 bridge_loop_avoidance.c |   15 +++++++++++----
 bridge_loop_avoidance.h |    5 +++--
 soft-interface.c        |    6 +++++-
 3 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/bridge_loop_avoidance.c b/bridge_loop_avoidance.c
index 6452e2f..1af7af5 100644
--- a/bridge_loop_avoidance.c
+++ b/bridge_loop_avoidance.c
@@ -1368,6 +1368,7 @@ void batadv_bla_free(struct batadv_priv *bat_priv)
 /* @bat_priv: the bat priv with all the soft interface information
  * @skb: the frame to be checked
  * @vid: the VLAN ID of the frame
+ * @is_bcast: the packet came in a broadcast packet type.
  *
  * bla_rx avoidance checks if:
  *  * we have to race for a claim
@@ -1377,7 +1378,8 @@ void batadv_bla_free(struct batadv_priv *bat_priv)
  * returns 1, otherwise it returns 0 and the caller shall further
  * process the skb.
  */
-int batadv_bla_rx(struct batadv_priv *bat_priv, struct sk_buff *skb, short vid)
+int batadv_bla_rx(struct batadv_priv *bat_priv, struct sk_buff *skb, short vid,
+		  bool is_bcast)
 {
 	struct ethhdr *ethhdr;
 	struct batadv_claim search_claim, *claim = NULL;
@@ -1396,7 +1398,7 @@ int batadv_bla_rx(struct batadv_priv *bat_priv, struct sk_buff *skb, short vid)
 
 	if (unlikely(atomic_read(&bat_priv->bla_num_requests)))
 		/* don't allow broadcasts while requests are in flight */
-		if (is_multicast_ether_addr(ethhdr->h_dest))
+		if (is_multicast_ether_addr(ethhdr->h_dest) && is_bcast)
 			goto handled;
 
 	memcpy(search_claim.addr, ethhdr->h_source, ETH_ALEN);
@@ -1422,8 +1424,13 @@ int batadv_bla_rx(struct batadv_priv *bat_priv, struct sk_buff *skb, short vid)
 	}
 
 	/* if it is a broadcast ... */
-	if (is_multicast_ether_addr(ethhdr->h_dest)) {
-		/* ... drop it. the responsible gateway is in charge. */
+	if (is_multicast_ether_addr(ethhdr->h_dest) && is_bcast) {
+		/* ... drop it. the responsible gateway is in charge.
+		 *
+		 * We need to check is_bcast because with the gateway
+		 * feature, broadcasts (like DHCP requests) may be sent
+		 * using a unicast packet type.
+		 */
 		goto handled;
 	} else {
 		/* seems the client considers us as its best gateway.
diff --git a/bridge_loop_avoidance.h b/bridge_loop_avoidance.h
index 35d39e3..789cb73 100644
--- a/bridge_loop_avoidance.h
+++ b/bridge_loop_avoidance.h
@@ -21,7 +21,8 @@
 #define _NET_BATMAN_ADV_BLA_H_
 
 #ifdef CONFIG_BATMAN_ADV_BLA
-int batadv_bla_rx(struct batadv_priv *bat_priv, struct sk_buff *skb, short vid);
+int batadv_bla_rx(struct batadv_priv *bat_priv, struct sk_buff *skb, short vid,
+		  bool is_bcast);
 int batadv_bla_tx(struct batadv_priv *bat_priv, struct sk_buff *skb, short vid);
 int batadv_bla_is_backbone_gw(struct sk_buff *skb,
 			      struct batadv_orig_node *orig_node, int hdr_size);
@@ -42,7 +43,7 @@ void batadv_bla_free(struct batadv_priv *bat_priv);
 #else /* ifdef CONFIG_BATMAN_ADV_BLA */
 
 static inline int batadv_bla_rx(struct batadv_priv *bat_priv,
-				struct sk_buff *skb, short vid)
+				struct sk_buff *skb, short vid, bool is_bcast)
 {
 	return 0;
 }
diff --git a/soft-interface.c b/soft-interface.c
index 96736ea..ae7d23e 100644
--- a/soft-interface.c
+++ b/soft-interface.c
@@ -274,9 +274,13 @@ void batadv_interface_rx(struct net_device *soft_iface,
 	struct batadv_priv *bat_priv = netdev_priv(soft_iface);
 	struct ethhdr *ethhdr;
 	struct vlan_ethhdr *vhdr;
+	struct batadv_header *batadv_header = (struct batadv_header *)skb->data;
 	short vid __maybe_unused = -1;
+	bool is_bcast;
 	__be16 ethertype = __constant_htons(BATADV_ETH_P_BATMAN);
 
+	is_bcast = (batadv_header->packet_type == BATADV_BCAST);
+
 	/* check if enough space is available for pulling, and pull */
 	if (!pskb_may_pull(skb, hdr_size))
 		goto dropped;
@@ -323,7 +327,7 @@ void batadv_interface_rx(struct net_device *soft_iface,
 	/* Let the bridge loop avoidance check the packet. If will
 	 * not handle it, we can safely push it up.
 	 */
-	if (batadv_bla_rx(bat_priv, skb, vid))
+	if (batadv_bla_rx(bat_priv, skb, vid, is_bcast))
 		goto out;
 
 	netif_rx(skb);
-- 
1.7.10


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

* Re: [B.A.T.M.A.N.] BLAII + gw_mode, DHCP sometimes gets dropped
  2012-07-04 18:30           ` Simon Wunderlich
@ 2012-07-04 18:46             ` Guido Iribarren
  2012-07-05 19:51               ` Guido Iribarren
  0 siblings, 1 reply; 16+ messages in thread
From: Guido Iribarren @ 2012-07-04 18:46 UTC (permalink / raw)
  To: The list for a Better Approach To Mobile Ad-hoc Networking

On Wed, Jul 4, 2012 at 3:30 PM, Simon Wunderlich
<simon.wunderlich@s2003.tu-chemnitz.de> wrote:
> Sorry, my fault. Will send a patch based on maint in a minute.
>

Applying ./patches/9999-batman-adv-check-incoming-packet-type-for-bla.patch
using plaintext:
patching file bridge_loop_avoidance.c
patching file bridge_loop_avoidance.h
patching file soft-interface.c

Now i'm having fun, heh
Thanks!

Gui

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

* Re: [B.A.T.M.A.N.] BLAII + gw_mode, DHCP sometimes gets dropped
  2012-07-04 18:46             ` Guido Iribarren
@ 2012-07-05 19:51               ` Guido Iribarren
  2012-07-06  8:24                 ` Simon Wunderlich
  0 siblings, 1 reply; 16+ messages in thread
From: Guido Iribarren @ 2012-07-05 19:51 UTC (permalink / raw)
  To: The list for a Better Approach To Mobile Ad-hoc Networking

Your patch works flawlessly Simon,
I sysupgraded f8d11504758 with the patched batman, and DHCP requests
are back to normal when using gw_mode=client

I patched colmena-casa first, but it didn't make a difference; which
was expected, since colmena-casa is just a backbone gateway, and it
doesn't have the actual DHCP server running, as such, it's only job
was to forward unicast requests to f8d11504758, which was already
doing fine.
The problem was that f8d11504758 was dropping the packets, and with
your patch it doesn't anymore.
Thanks a lot!


On Wed, Jul 4, 2012 at 3:46 PM, Guido Iribarren
<guidoiribarren@buenosaireslibre.org> wrote:
> On Wed, Jul 4, 2012 at 3:30 PM, Simon Wunderlich
> <simon.wunderlich@s2003.tu-chemnitz.de> wrote:
>> Sorry, my fault. Will send a patch based on maint in a minute.
>>
>
> Applying ./patches/9999-batman-adv-check-incoming-packet-type-for-bla.patch
> using plaintext:
> patching file bridge_loop_avoidance.c
> patching file bridge_loop_avoidance.h
> patching file soft-interface.c
>
> Now i'm having fun, heh
> Thanks!
>
> Gui

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

* Re: [B.A.T.M.A.N.] [PATCH-maint-v2] batman-adv: check incoming packet type for bla
  2012-07-04 18:38           ` [B.A.T.M.A.N.] [PATCH-maint-v2] " Simon Wunderlich
@ 2012-07-05 22:04             ` Marek Lindner
  0 siblings, 0 replies; 16+ messages in thread
From: Marek Lindner @ 2012-07-05 22:04 UTC (permalink / raw)
  To: The list for a Better Approach To Mobile Ad-hoc Networking

On Wednesday, July 04, 2012 20:38:19 Simon Wunderlich wrote:
> If the gateway functionality is used, some broadcast packets (DHCP
> requests) may be transmitted as unicast packets. As the bridge loop
> avoidance code now only considers the payload Ethernet destination,
> it may drop the DHCP request for clients which are claimed by other
> backbone gateways, because it falsely infers from the broadcast address
> that the right backbone gateway should havehandled the broadcast.
> 
> Fix this by checking and delegating the batman-adv packet type used
> for transmission.
> 
> Reported-by: Guido Iribarren <guidoiribarren@buenosaireslibre.org>
> Signed-off-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>

Applied in revision e324701.

Thanks,
Marek

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

* Re: [B.A.T.M.A.N.] BLAII + gw_mode, DHCP sometimes gets dropped
  2012-07-05 19:51               ` Guido Iribarren
@ 2012-07-06  8:24                 ` Simon Wunderlich
  0 siblings, 0 replies; 16+ messages in thread
From: Simon Wunderlich @ 2012-07-06  8:24 UTC (permalink / raw)
  To: The list for a Better Approach To Mobile Ad-hoc Networking

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

Hello Guido,

thank you very much for testing, it's nice to hear that it works! :)

Cheers,
	Simon
On Thu, Jul 05, 2012 at 04:51:57PM -0300, Guido Iribarren wrote:
> Your patch works flawlessly Simon,
> I sysupgraded f8d11504758 with the patched batman, and DHCP requests
> are back to normal when using gw_mode=client
> 
> I patched colmena-casa first, but it didn't make a difference; which
> was expected, since colmena-casa is just a backbone gateway, and it
> doesn't have the actual DHCP server running, as such, it's only job
> was to forward unicast requests to f8d11504758, which was already
> doing fine.
> The problem was that f8d11504758 was dropping the packets, and with
> your patch it doesn't anymore.
> Thanks a lot!
> 
> 
> On Wed, Jul 4, 2012 at 3:46 PM, Guido Iribarren
> <guidoiribarren@buenosaireslibre.org> wrote:
> > On Wed, Jul 4, 2012 at 3:30 PM, Simon Wunderlich
> > <simon.wunderlich@s2003.tu-chemnitz.de> wrote:
> >> Sorry, my fault. Will send a patch based on maint in a minute.
> >>
> >
> > Applying ./patches/9999-batman-adv-check-incoming-packet-type-for-bla.patch
> > using plaintext:
> > patching file bridge_loop_avoidance.c
> > patching file bridge_loop_avoidance.h
> > patching file soft-interface.c
> >
> > Now i'm having fun, heh
> > Thanks!
> >
> > Gui
> 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

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

end of thread, other threads:[~2012-07-06  8:24 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-03 20:07 [B.A.T.M.A.N.] BLAII + gw_mode, DHCP sometimes gets dropped Guido Iribarren
2012-07-04  9:12 ` Simon Wunderlich
2012-07-04 12:43   ` Guido Iribarren
2012-07-04 13:12     ` Guido Iribarren
2012-07-04 14:05       ` Simon Wunderlich
2012-07-04 18:12         ` Guido Iribarren
2012-07-04 18:30           ` Simon Wunderlich
2012-07-04 18:46             ` Guido Iribarren
2012-07-05 19:51               ` Guido Iribarren
2012-07-06  8:24                 ` Simon Wunderlich
2012-07-04 18:33           ` [B.A.T.M.A.N.] [PATCH-maint] batman-adv: check incoming packet type for bla Simon Wunderlich
2012-07-04 18:38           ` [B.A.T.M.A.N.] [PATCH-maint-v2] " Simon Wunderlich
2012-07-05 22:04             ` Marek Lindner
2012-07-04 13:49     ` [B.A.T.M.A.N.] BLAII + gw_mode, DHCP sometimes gets dropped Simon Wunderlich
2012-07-04 13:59     ` [B.A.T.M.A.N.] [PATCH] batman-adv: check incoming packet type for bla Simon Wunderlich
2012-07-04 18:40       ` [B.A.T.M.A.N.] [PATCH-v2] " Simon Wunderlich

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.