Re: [B.A.T.M.A.N.] [PATCH 5/5] batman-adv: avoid temporary routing loops by being strict on forwarded OGMs

Daniele Furlan
2012-03-12T07:24:36Z

Hi,
I agree with this patch, actually it should solve the loop problem as it is very
similar to the proposed modification in my thesis.

In this way I think it is not necessary a global window to average TQ values
but only the latest value can be sufficient. Doing this can be re-implemented
a mechanism to improve OGM reliability sending old TQ ant TTL (and also
sequence number) of the current best router if for example 2 OGM are missing.

Regards.

2012/3/11 Simon Wunderlich <simon.wunderlich@s2003.tu-chemnitz.de>:
> Hey there,
>
> I've tested this patch on my old ring setup which used to create temporary
> routing loops, and this patch improved the situation.
>
> Feel free to add:
>
> Tested-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
>
> On Sun, Mar 11, 2012 at 06:17:53AM +0800, Marek Lindner wrote:
>> batman-adv would forward OGMs from non-besthops while replacing the the TQ
>> and TTL values with the values from the best hop. In certain corner cases
>> this leads to a temporary routing loop.
>> This patch changes this behavior: Only packets from best next hops are
>> forwarded - TQ and TTL values won't be replaced anymore. However, the protocol
>> needs to rebroadcast OGMs from single hop neighbors regardless of whether or
>> not they are the best hop. To handle this case a new flag is introduced to
>> alert neighboring nodes about the forwarded OGM that is not from my best
>> next hop. It is to be discarded by all nodes except for the one originating
>> the OGM.
>>
>> Signed-off-by: Marek Lindner <lindner_marek@yahoo.de>
>> ---
>>  bat_iv_ogm.c |   61 ++++++++++++++++++++++++++++++---------------------------
>>  packet.h     |    1 +
>>  2 files changed, 33 insertions(+), 29 deletions(-)
>>
>> diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c
>> index 1e92e38..85617c4 100644
>> --- a/bat_iv_ogm.c
>> +++ b/bat_iv_ogm.c
>> @@ -507,11 +507,10 @@ static void bat_iv_ogm_forward(struct orig_node *orig_node,
>>                              const struct ethhdr *ethhdr,
>>                              struct batman_ogm_packet *batman_ogm_packet,
>>                              bool is_single_hop_neigh,
>> +                            bool is_from_best_next_hop,
>>                              struct hard_iface *if_incoming)
>>  {
>>       struct bat_priv *bat_priv = netdev_priv(if_incoming->soft_iface);
>> -     struct neigh_node *router;
>> -     uint8_t in_tq, in_ttl, tq_avg = 0;
>>       uint8_t tt_num_changes;
>>
>>       if (batman_ogm_packet->header.ttl <= 1) {
>> @@ -519,41 +518,31 @@ static void bat_iv_ogm_forward(struct orig_node *orig_node,
>>               return;
>>       }
>>
>> -     router = orig_node_get_router(orig_node);
>> +     if (!is_from_best_next_hop) {
>> +             /**
>> +             * Mark the forwarded packet when it is not coming from our best
>> +             * next hop. We still need to forward the packet for our neighbor
>> +             * link quality detection to work in case the packet originated
>> +             * from a single hop neighbor. Otherwise we can simply drop the
>> +             * ogm.
>> +             */
>> +             if (is_single_hop_neigh)
>> +                     batman_ogm_packet->flags |= NOT_BEST_NEXT_HOP;
>> +             else
>> +                     return;
>> +     }
>>
>> -     in_tq = batman_ogm_packet->tq;
>> -     in_ttl = batman_ogm_packet->header.ttl;
>>       tt_num_changes = batman_ogm_packet->tt_num_changes;
>>
>>       batman_ogm_packet->header.ttl--;
>>       memcpy(batman_ogm_packet->prev_sender, ethhdr->h_source, ETH_ALEN);
>>
>> -     /* rebroadcast tq of our best ranking neighbor to ensure the rebroadcast
>> -      * of our best tq value */
>> -     if (router && router->tq_avg != 0) {
>> -
>> -             /* rebroadcast ogm of best ranking neighbor as is */
>> -             if (!compare_eth(router->addr, ethhdr->h_source)) {
>> -                     batman_ogm_packet->tq = router->tq_avg;
>> -
>> -                     if (router->last_ttl)
>> -                             batman_ogm_packet->header.ttl =
>> -                                     router->last_ttl - 1;
>> -             }
>> -
>> -             tq_avg = router->tq_avg;
>> -     }
>> -
>> -     if (router)
>> -             neigh_node_free_ref(router);
>> -
>>       /* apply hop penalty */
>>       batman_ogm_packet->tq = hop_penalty(batman_ogm_packet->tq, bat_priv);
>>
>>       bat_dbg(DBG_BATMAN, bat_priv,
>> -             "Forwarding packet: tq_orig: %i, tq_avg: %i, tq_forw: %i, ttl_orig: %i, ttl_forw: %i\n",
>> -             in_tq, tq_avg, batman_ogm_packet->tq, in_ttl - 1,
>> -             batman_ogm_packet->header.ttl);
>> +             "Forwarding packet: tq: %i, ttl: %i\n",
>> +             batman_ogm_packet->tq, batman_ogm_packet->header.ttl);
>>
>>       batman_ogm_packet->seqno = htonl(batman_ogm_packet->seqno);
>>       batman_ogm_packet->tt_crc = htons(batman_ogm_packet->tt_crc);
>> @@ -949,6 +938,7 @@ static void bat_iv_ogm_process(const struct ethhdr *ethhdr,
>>       int is_my_addr = 0, is_my_orig = 0, is_my_oldorig = 0;
>>       int is_broadcast = 0, is_bidirectional;
>>       bool is_single_hop_neigh = false;
>> +     bool is_from_best_next_hop = false;
>>       int is_duplicate;
>>       uint32_t if_incoming_seqno;
>>
>> @@ -1070,6 +1060,13 @@ static void bat_iv_ogm_process(const struct ethhdr *ethhdr,
>>               return;
>>       }
>>
>> +     if (batman_ogm_packet->flags & NOT_BEST_NEXT_HOP) {
>> +             bat_dbg(DBG_BATMAN, bat_priv,
>> +                     "Drop packet: ignoring all packets not forwarded from "
>> +                     "the best next hop (sender: %pM)\n", ethhdr->h_source);
>> +             return;
>> +     }
>> +
>>       orig_node = get_orig_node(bat_priv, batman_ogm_packet->orig);
>>       if (!orig_node)
>>               return;
>> @@ -1094,6 +1091,10 @@ static void bat_iv_ogm_process(const struct ethhdr *ethhdr,
>>       if (router)
>>               router_router = orig_node_get_router(router->orig_node);
>>
>> +     if ((router && router->tq_avg != 0) &&
>> +         (compare_eth(router->addr, ethhdr->h_source)))
>> +             is_from_best_next_hop = true;
>> +
>>       /* avoid temporary routing loops */
>>       if (router && router_router &&
>>           (compare_eth(router->addr, batman_ogm_packet->prev_sender)) &&
>> @@ -1144,7 +1145,8 @@ static void bat_iv_ogm_process(const struct ethhdr *ethhdr,
>>
>>               /* mark direct link on incoming interface */
>>               bat_iv_ogm_forward(orig_node, ethhdr, batman_ogm_packet,
>> -                                is_single_hop_neigh, if_incoming);
>> +                                is_single_hop_neigh, is_from_best_next_hop,
>> +                                if_incoming);
>>
>>               bat_dbg(DBG_BATMAN, bat_priv,
>>                       "Forwarding packet: rebroadcast neighbor packet with direct link flag\n");
>> @@ -1167,7 +1169,8 @@ static void bat_iv_ogm_process(const struct ethhdr *ethhdr,
>>       bat_dbg(DBG_BATMAN, bat_priv,
>>               "Forwarding packet: rebroadcast originator packet\n");
>>       bat_iv_ogm_forward(orig_node, ethhdr, batman_ogm_packet,
>> -                        is_single_hop_neigh, if_incoming);
>> +                        is_single_hop_neigh, is_from_best_next_hop,
>> +                        if_incoming);
>>
>>  out_neigh:
>>       if ((orig_neigh_node) && (!is_single_hop_neigh))
>> diff --git a/packet.h b/packet.h
>> index 02b0c87..3c4c533 100644
>> --- a/packet.h
>> +++ b/packet.h
>> @@ -47,6 +47,7 @@ enum bat_subtype {
>>  #define COMPAT_VERSION 14
>>
>>  enum batman_iv_flags {
>> +     NOT_BEST_NEXT_HOP   = 1 << 3,
>>       PRIMARIES_FIRST_HOP = 1 << 4,
>>       VIS_SERVER          = 1 << 5,
>>       DIRECTLINK          = 1 << 6
>> --
>> 1.7.9.1
>>
>>
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
>
> iEYEARECAAYFAk9dHi8ACgkQrzg/fFk7axbJ+QCg5TaIOetOvSoTjr5vHCHr4k7M
> /6YAoKC76tkASLbl9TuYFKqLTIHXRK2s
> =JNhg
> -----END PGP SIGNATURE-----
>

--
Daniele Furlan