Open-Mesh » batman-adv

Got a report for @Antonio Quartulli 's commit 9323158ef9f4 ("batman-adv: OGMv2 - implement originators logic")

batadv_v_ogm_route_update() retrieves a (neighbor) router from orig_node's ifinfo_list. All items in this list have as backpointer (orig_node_id) in the router object the orig_node which owns this list. An extra check between the orig_node and the router->orig_node_id can newer return an "not equal" result for B.A.T.M.A.N. V.

The batadv_v_ogm_route_update() is supposed to drop OGM2 packets which were received from a neighbor that we do not have a route towards, unless the packet was sent by the originator itself (single hop). This mirrors the equivalent check in B.A.T.M.A.N. IV (batadv_iv_ogm_process_per_outif()):

    /* drop packet if sender is not a direct neighbor and if we
     * don't route towards it
     */
    if (!is_single_hop_neigh && !orig_neigh_router) {

The OGMv2 variant however compares the originator of the currently selected router against the originator the OGM belongs to:

    router = batadv_orig_router_get(orig_node, if_outgoing);
    if (router && router->orig_node != orig_node && !orig_neigh_router) {

router was just looked up via orig_node's own ifinfo, so it is always a neighbor entry of orig_node itself and router->orig_node == orig_node holds by construction. The condition can never be true and the check is dead code: OGM2s rebroadcast by unknown senders are still accepted for route updates and considered for forwarding instead of being dropped like in B.A.T.M.A.N. IV.

---

The report and the patch has to be reviewed and (most likely) adjusted.

At the moment, it looks to me like the check is useless and could be dropped:

diff --git a/net/batman-adv/bat_v_ogm.c b/net/batman-adv/bat_v_ogm.c
index 70846c997410d200408b58d82325380dbb559fd0..7c57167b2b3c5fdb089d1bd7705f1d666fecd0aa 100644
--- a/net/batman-adv/bat_v_ogm.c
+++ b/net/batman-adv/bat_v_ogm.c
@@ -705,7 +705,6 @@ static bool batadv_v_ogm_route_update(struct batadv_priv *bat_priv,
                       struct batadv_hard_iface *if_incoming,
                       struct batadv_hard_iface *if_outgoing)
 {
-    struct batadv_neigh_node *orig_neigh_router = NULL;
     struct batadv_neigh_ifinfo *router_ifinfo = NULL;
     struct batadv_neigh_ifinfo *neigh_ifinfo = NULL;
     struct batadv_orig_node *orig_neigh_node;
@@ -721,18 +720,10 @@ static bool batadv_v_ogm_route_update(struct batadv_priv *bat_priv,
     if (!orig_neigh_node)
         goto out;

-    orig_neigh_router = batadv_orig_router_get(orig_neigh_node,
-                           if_outgoing);
-
     /* drop packet if sender is not a direct neighbor and if we
      * don't route towards it
      */
     router = batadv_orig_router_get(orig_node, if_outgoing);
-    if (router && ACCESS_PRIVATE(router, orig_node_id) != orig_node && !orig_neigh_router) {
-        batadv_dbg(BATADV_DBG_BATMAN, bat_priv,
-               "Drop packet: OGM via unknown neighbor!\n");
-        goto out;
-    }

     /* Mark the OGM to be considered for forwarding, and update routes
      * if needed.
@@ -774,7 +765,6 @@ static bool batadv_v_ogm_route_update(struct batadv_priv *bat_priv,
     batadv_update_route(bat_priv, orig_node, if_outgoing, neigh_node);
 out:
     batadv_neigh_node_put(router);
-    batadv_neigh_node_put(orig_neigh_router);
     batadv_orig_node_put(orig_neigh_node);
     batadv_neigh_ifinfo_put(router_ifinfo);
     batadv_neigh_ifinfo_put(neigh_ifinfo);

But this should be checked by someone with more expertise in B.A.T.M.A.N. V