Actions
Feature #419
openBLA: redundant and superficial GW check
Start date:
09/14/2020
Due date:
% Done:
0%
Estimated time:
Description
The source address check in batadv_recv_unicast_packet() here is both superficial and redundant:
989 /* packet for me */
990 if (batadv_is_my_mac(bat_priv, unicast_packet->dest)) {
991 /* If this is a unicast packet from another backgone gw,
992 * drop it.
993 */
994 orig_addr_gw = eth_hdr(skb)->h_source;
995 orig_node_gw = batadv_orig_hash_find(bat_priv, orig_addr_gw);
996 if (orig_node_gw) {
997 is_gw = batadv_bla_is_backbone_gw(skb, orig_node_gw,
998 hdr_size);
999 batadv_orig_node_put(orig_node_gw);
1000 if (is_gw) {
1001 batadv_dbg(BATADV_DBG_BLA, bat_priv,
1002 "%s(): Dropped unicast pkt received from another backbone gw %pM.\n",
1003 __func__, orig_addr_gw);
1004 goto free_skb;
1005 }
1006 }
1007
Redundant, because the sender is already supposed to perform this check, so no need to do it again on reception.
Superficial, because it only works if:- The BLA backbone gateway we share a LAN with is a direct neighbor of us.
- The BLA backbone gateway we share a LAN with transmits the packet via its primary interface to us.
In all other cases, like received via multiple hops or via a secondary interface from the other BLA gateway does not work.
Suggestion:- Either remove this check.
- Or turn the according batadv_dbg() into a pr_warn_ratelimited() to help in spotting potential bugs
(This check initially made it hard to reproduce the issue this patch is supposed to fix: https://patchwork.open-mesh.org/project/b.a.t.m.a.n./patch/20200914012136.5278-2-linus.luessing@c0d3.blue/. Initially it was easy to reproduce in a physical setup but then difficult to reproduce in a virtual one, because they had different configurations regarding primary vs. secondary interfaces.)
No data to display
Actions