[B.A.T.M.A.N.] [PATCH] batman-adv: Mark as received also old rebroadcast

[B.A.T.M.A.N.] [PATCH] batman-adv: Mark as received also old rebroadcast

[Daniele Furlan]

This patch avoid local TQ drop in case of heavy load. In this case,
rebroadcast can be delayed more than orig_interval causing the receiver
to incorrectly drop old packets without marking them as received.

Signed-off-by: Daniele Furlan <daniele.furlan@gmail.com>
---
 routing.c |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/routing.c b/routing.c
index c620d4f..5e99188 100644
--- a/routing.c
+++ b/routing.c
@@ -720,17 +720,15 @@ void receive_bat_packet(const struct ethhdr *ethhdr,
 
 		/* neighbor has to indicate direct link and it has to
 		 * come via the corresponding interface */
-		/* if received seqno equals last send seqno save new
-		 * seqno for bidirectional check */
+		/* save packet seqno for bidirectional check */
 		if (has_directlink_flag &&
 		    compare_eth(if_incoming->net_dev->dev_addr,
-				batman_packet->orig) &&
-		    (batman_packet->seqno - if_incoming_seqno + 2 == 0)) {
+				batman_packet->orig)) {
 			offset = if_incoming->if_num * NUM_WORDS;
 
 			spin_lock_bh(&orig_neigh_node->ogm_cnt_lock);
 			word = &(orig_neigh_node->bcast_own[offset]);
-			bit_mark(word, 0);
+			bit_mark(word, if_incoming_seqno - batman_packet->seqno - 2);
 			orig_neigh_node->bcast_own_sum[if_incoming->if_num] =
 				bit_packet_count(word);
 			spin_unlock_bh(&orig_neigh_node->ogm_cnt_lock);
-- 
1.7.1

Re: [B.A.T.M.A.N.] [PATCH] batman-adv: Mark as received also old rebroadcast

[Marek Lindner]

On Tuesday, June 07, 2011 12:45:55 AM Daniele Furlan wrote:
> This patch avoid local TQ drop in case of heavy load. In this case,
> rebroadcast can be delayed more than orig_interval causing the receiver
> to incorrectly drop old packets without marking them as received.

Do mind you explaining in more detail what led you to make this change and in 
which scenarios it improves the situation (please give a concrete example) ?

Thanks,
Marek

Re: [B.A.T.M.A.N.] [PATCH] batman-adv: Mark as received also old rebroadcast

[Daniele Furlan]

2011/6/7 Marek Lindner <lindner_marek@yahoo.de>:
> On Tuesday, June 07, 2011 12:45:55 AM Daniele Furlan wrote:
>> This patch avoid local TQ drop in case of heavy load. In this case,
>> rebroadcast can be delayed more than orig_interval causing the receiver
>> to incorrectly drop old packets without marking them as received.
>
> Do mind you explaining in more detail what led you to make this change and in
> which scenarios it improves the situation (please give a concrete example) ?
>

When a node is saturated, obviously tends to delay packet send. This
delay is applied
also to batman management frame.
It frequently happens that ogm rebroadcast of neighbour are delayed
more than orig_interval
causing the receiver to discard old packets. This is reflect to the local TQ
that decrease quickly.

I verified this behaviour with wireshark that highlight that in many
case batman management
frame are sent with a great delay.

Running some test with netperf (but also using ping -f the result did
not change), it happens
that TQ towards all node in the network drops to values near 0. This
beahaviuor lead to "random" route
change that are not correlated with the quality of the link.
I verified that this patch solve this problem.

I hope I was clear.

> Thanks,
> Marek
>



-- 
Daniele Furlan

Re: [B.A.T.M.A.N.] [PATCH] batman-adv: Mark as received also old rebroadcast

[Marek Lindner]

On Tuesday, June 07, 2011 10:23:58 AM Daniele Furlan wrote:
> Running some test with netperf (but also using ping -f the result did
> not change), it happens that TQ towards all node in the network drops to
> values near 0. This beahaviuor lead to "random" route change that are not
> correlated with the quality of the link. I verified that this patch solve
> this problem.

Thanks for the explanations. I hope you don't mind that I add these 
information to your commit message before submiting it ?

Regards,
Marek

Re: [B.A.T.M.A.N.] [PATCH] batman-adv: Mark as received also old rebroadcast

[Daniele Furlan]

2011/6/8 Marek Lindner <lindner_marek@yahoo.de>:
> On Tuesday, June 07, 2011 10:23:58 AM Daniele Furlan wrote:
>> Running some test with netperf (but also using ping -f the result did
>> not change), it happens that TQ towards all node in the network drops to
>> values near 0. This beahaviuor lead to "random" route change that are not
>> correlated with the quality of the link. I verified that this patch solve
>> this problem.
>
> Thanks for the explanations. I hope you don't mind that I add these
> information to your commit message before submiting it ?

Ok no problem, let's add it to the commit message.

Thank'you!

>
> Regards,
> Marek
>

-- 
Daniele Furlan

Re: [B.A.T.M.A.N.] [PATCH] batman-adv: Mark as received also old rebroadcast

[Marek Lindner]

On Tuesday, June 07, 2011 12:45:55 AM Daniele Furlan wrote:
> This patch avoid local TQ drop in case of heavy load. In this case,
> rebroadcast can be delayed more than orig_interval causing the receiver
> to incorrectly drop old packets without marking them as received.

I applied the patch (in revision 9e80af2) with the following commit message:

accept delayed rebroadcasts to avoid bogus routing under heavy load
    
When a link is saturated (re)broadcasts of OGMs are delayed. Under heavy load
this delay may exceed the orig interval which leads to OGMs being dropped (the
code would only accept an OGM rebroadcast if it arrived before the next OGM 
was broadcasted). With this patch batman-adv will also accept delayed OGMs in 
order to avoid a bogus influence on the routing metric.

Thanks,
Marek