27 Nov, 2020

3 commits

  • When a packet is fragmented by batman-adv, the original batman-adv header
    is not modified. Only a new fragmentation is inserted between the original
    one and the ethernet header. The code must therefore make sure that it has
    a writable region of this size in the skbuff head.

    But it is not useful to always reallocate the skbuff by this size even when
    there would be more than enough headroom still in the skb. The reallocation
    is just to costly during in this codepath.

    Fixes: ee75ed88879a ("batman-adv: Fragment and send skbs larger than mtu")
    Signed-off-by: Sven Eckelmann
    Signed-off-by: Simon Wunderlich

    Sven Eckelmann
     
  • The batadv net_device is trying to propagate the needed_headroom and
    needed_tailroom from the lower devices. This is needed to avoid cost
    intensive reallocations using pskb_expand_head during the transmission.

    But the fragmentation code split the skb's without adding extra room at the
    end/beginning of the various fragments. This reduced the performance of
    transmissions over complex scenarios (batadv on vxlan on wireguard) because
    the lower devices had to perform the reallocations at least once.

    Fixes: ee75ed88879a ("batman-adv: Fragment and send skbs larger than mtu")
    Signed-off-by: Sven Eckelmann
    Signed-off-by: Simon Wunderlich

    Sven Eckelmann
     
  • If a batman-adv packets has to be fragmented, then the original batman-adv
    packet header is not stripped away. Instead, only a new header is added in
    front of the packet after it was split.

    This size must be considered to avoid cost intensive reallocations during
    the transmission through the various device layers.

    Fixes: 7bca68c7844b ("batman-adv: Add lower layer needed_(head|tail)room to own ones")
    Reported-by: Linus Lüssing
    Signed-off-by: Sven Eckelmann
    Signed-off-by: Simon Wunderlich

    Sven Eckelmann
     

15 Nov, 2020

1 commit

  • If THIS_MODULE is not set, the module would be removed while debugfs is
    being used.
    It eventually makes kernel panic.

    Fixes: c6c8fea29769 ("net: Add batman-adv meshing protocol")
    Signed-off-by: Taehee Yoo
    Signed-off-by: Sven Eckelmann
    Signed-off-by: Simon Wunderlich

    Taehee Yoo
     

03 Oct, 2020

1 commit


24 Sep, 2020

1 commit

  • Since now we have src in br_ip, u no longer makes sense so rename
    it to dst. No functional changes.

    v2: fix build with CONFIG_BATMAN_ADV_MCAST

    CC: Marek Lindner
    CC: Simon Wunderlich
    CC: Antonio Quartulli
    CC: Sven Eckelmann
    CC: b.a.t.m.a.n@lists.open-mesh.org
    Signed-off-by: Nikolay Aleksandrov
    Signed-off-by: David S. Miller

    Nikolay Aleksandrov
     

23 Sep, 2020

1 commit

  • Two minor conflicts:

    1) net/ipv4/route.c, adding a new local variable while
    moving another local variable and removing it's
    initial assignment.

    2) drivers/net/dsa/microchip/ksz9477.c, overlapping changes.
    One pretty prints the port mode differently, whilst another
    changes the driver to try and obtain the port mode from
    the port node rather than the switch node.

    Signed-off-by: David S. Miller

    David S. Miller
     

15 Sep, 2020

4 commits

  • Scenario:
    * Multicast frame send from BLA backbone gateways (multiple nodes
    with their bat0 bridged together, with BLA enabled) sharing the same
    LAN to nodes in the mesh

    Issue:
    * Nodes receive the frame multiple times on bat0 from the mesh,
    once from each foreign BLA backbone gateway which shares the same LAN
    with another

    For multicast frames via batman-adv broadcast packets coming from the
    same BLA backbone but from different backbone gateways duplicates are
    currently detected via a CRC history of previously received packets.

    However this CRC so far was not performed for multicast frames received
    via batman-adv unicast packets. Fixing this by appyling the same check
    for such packets, too.

    Room for improvements in the future: Ideally we would introduce the
    possibility to not only claim a client, but a complete originator, too.
    This would allow us to only send a multicast-in-unicast packet from a BLA
    backbone gateway claiming the node and by that avoid potential redundant
    transmissions in the first place.

    Fixes: 279e89b2281a ("batman-adv: add broadcast duplicate check")
    Signed-off-by: Linus Lüssing
    Signed-off-by: Sven Eckelmann
    Signed-off-by: Simon Wunderlich

    Linus Lüssing
     
  • Scenario:
    * Multicast frame send from mesh to a BLA backbone (multiple nodes
    with their bat0 bridged together, with BLA enabled)

    Issue:
    * BLA backbone nodes receive the frame multiple times on bat0,
    once from mesh->bat0 and once from each backbone_gw from LAN

    For unicast, a node will send only to the best backbone gateway
    according to the TQ. However for multicast we currently cannot determine
    if multiple destination nodes share the same backbone if they don't share
    the same backbone with us. So we need to keep sending the unicasts to
    all backbone gateways and let the backbone gateways decide which one
    will forward the frame. We can use the CLAIM mechanism to make this
    decision.

    One catch: The batman-adv gateway feature for DHCP packets potentially
    sends multicast packets in the same batman-adv unicast header as the
    multicast optimizations code. And we are not allowed to drop those even
    if we did not claim the source address of the sender, as for such
    packets there is only this one multicast-in-unicast packet.

    How can we distinguish the two cases?

    The gateway feature uses a batman-adv unicast 4 address header. While
    the multicast-to-unicasts feature uses a simple, 3 address batman-adv
    unicast header. So let's use this to distinguish.

    Fixes: fe2da6ff27c7 ("batman-adv: check incoming packet type for bla")
    Signed-off-by: Linus Lüssing
    Signed-off-by: Sven Eckelmann
    Signed-off-by: Simon Wunderlich

    Linus Lüssing
     
  • Scenario:
    * Multicast frame send from a BLA backbone (multiple nodes with
    their bat0 bridged together, with BLA enabled)

    Issue:
    * BLA backbone nodes receive the frame multiple times on bat0

    For multicast frames received via batman-adv broadcast packets the
    originator of the broadcast packet is checked before decapsulating and
    forwarding the frame to bat0 (batadv_bla_is_backbone_gw()->
    batadv_recv_bcast_packet()). If it came from a node which shares the
    same BLA backbone with us then it is not forwarded to bat0 to avoid a
    loop.

    When sending a multicast frame in a non-4-address batman-adv unicast
    packet we are currently missing this check - and cannot do so because
    the batman-adv unicast packet has no originator address field.

    However, we can simply fix this on the sender side by only sending the
    multicast frame via unicasts to interested nodes which do not share the
    same BLA backbone with us. This also nicely avoids some unnecessary
    transmissions on mesh side.

    Note that no infinite loop was observed, probably because of dropping
    via batadv_interface_tx()->batadv_bla_tx(). However the duplicates still
    utterly confuse switches/bridges, ICMPv6 duplicate address detection and
    neighbor discovery and therefore leads to long delays before being able
    to establish TCP connections, for instance. And it also leads to the Linux
    bridge printing messages like:
    "br-lan: received packet on eth1 with own address as source address ..."

    Fixes: 2d3f6ccc4ea5 ("batman-adv: Modified forwarding behaviour for multicast packets")
    Signed-off-by: Linus Lüssing
    Signed-off-by: Sven Eckelmann
    Signed-off-by: Simon Wunderlich

    Linus Lüssing
     
  • The fix for receiving (internally generated) bla packets outside the
    interrupt context introduced the usage of in_interrupt(). But this
    functionality is only defined in linux/preempt.h which was not included
    with the same patch.

    Fixes: 279e89b2281a ("batman-adv: bla: use netif_rx_ni when not in interrupt context")
    Signed-off-by: Sven Eckelmann
    Signed-off-by: Simon Wunderlich

    Sven Eckelmann
     

05 Sep, 2020

2 commits

  • The unicast packet rerouting code makes several assumptions. For
    instance it assumes that there is always exactly one destination in the
    TT. This breaks for multicast frames in a unicast packets in several ways:

    For one thing if there is actually no TT entry and the destination node
    was selected due to the multicast tvlv flags it announced. Then an
    intermediate node will wrongly drop the packet.

    For another thing if there is a TT entry but the TTVN of this entry is
    newer than the originally addressed destination node: Then the
    intermediate node will wrongly redirect the packet, leading to
    duplicated multicast packets at a multicast listener and missing
    packets at other multicast listeners or multicast routers.

    Fixing this by not applying the unicast packet rerouting to batman-adv
    unicast packets with a multicast payload. We are not able to detect a
    roaming multicast listener at the moment and will just continue to send
    the multicast frame to both the new and old destination for a while in
    case of such a roaming multicast listener.

    Fixes: a73105b8d4c7 ("batman-adv: improved client announcement mechanism")
    Signed-off-by: Linus Lüssing
    Signed-off-by: Sven Eckelmann
    Signed-off-by: Simon Wunderlich

    Linus Lüssing
     
  • We got slightly different patches removing a double word
    in a comment in net/ipv4/raw.c - picked the version from net.

    Simple conflict in drivers/net/ethernet/ibm/ibmvnic.c. Use cached
    values instead of VNIC login response buffer (following what
    commit 507ebe6444a4 ("ibmvnic: Fix use-after-free of VNIC login
    response buffer") did).

    Signed-off-by: Jakub Kicinski

    Jakub Kicinski
     

27 Aug, 2020

1 commit

  • It seems that due to a copy & paste error the void pointer
    in batadv_choose_backbone_gw() is cast to the wrong type.

    Fixing this by using "struct batadv_bla_backbone_gw" instead of "struct
    batadv_bla_claim" which better matches the caller's side.

    For now it seems that we were lucky because the two structs both have
    their orig/vid and addr/vid in the beginning. However I stumbled over
    this issue when I was trying to add some debug variables in front of
    "orig" in batadv_backbone_gw, which caused hash lookups to fail.

    Fixes: 07568d0369f9 ("batman-adv: don't rely on positions in struct for hashing")
    Signed-off-by: Linus Lüssing
    Signed-off-by: Sven Eckelmann

    Linus Lüssing
     

19 Aug, 2020

8 commits

  • batadv_bla_send_claim() gets called from worker thread context through
    batadv_bla_periodic_work(), thus netif_rx_ni needs to be used in that
    case. This fixes "NOHZ: local_softirq_pending 08" log messages seen
    when batman-adv is enabled.

    Fixes: 23721387c409 ("batman-adv: add basic bridge loop avoidance code")
    Signed-off-by: Jussi Kivilinna
    Signed-off-by: Sven Eckelmann
    Signed-off-by: Simon Wunderlich

    Jussi Kivilinna
     
  • The own OGM check is currently misplaced and can lead to the following
    issues:

    For one thing we might receive an aggregated OGM from a neighbor node
    which has our own OGM in the first place. We would then not only skip
    our own OGM but erroneously also any other, following OGM in the
    aggregate.

    For another, we might receive an OGM aggregate which has our own OGM in
    a place other then the first one. Then we would wrongly not skip this
    OGM, leading to populating the orginator and gateway table with ourself.

    Fixes: 9323158ef9f4 ("batman-adv: OGMv2 - implement originators logic")
    Signed-off-by: Linus Lüssing
    Signed-off-by: Sven Eckelmann
    Signed-off-by: Simon Wunderlich

    Linus Lüssing
     
  • The gateway client code can try to optimize the delivery of DHCP packets to
    avoid broadcasting them through the whole mesh. But also transmissions to
    the client can be optimized by looking up the destination via the chaddr of
    the DHCP packet.

    But the chaddr is currently only done when chaddr is fully inside the
    non-paged area of the skbuff. Otherwise it will not be initialized and the
    unoptimized path should have been taken.

    But the implementation didn't handle this correctly. It didn't retrieve the
    correct chaddr but still tried to perform the TT lookup with this
    uninitialized memory.

    Reported-by: syzbot+ab16e463b903f5a37036@syzkaller.appspotmail.com
    Fixes: 6c413b1c22a2 ("batman-adv: send every DHCP packet as bat-unicast")
    Signed-off-by: Sven Eckelmann
    Acked-by: Antonio Quartulli
    Signed-off-by: Simon Wunderlich

    Sven Eckelmann
     
  • The commit c0842fbc1b18 ("random32: move the pseudo-random 32-bit
    definitions to prandom.h") introduced a new header for the pseudo random
    functions from (previously) linux/random.h. One future goal of the
    prandom.h change is to make code to switch just the new header file and to
    avoid the implicit include. This would allow the removal of the implicit
    include from random.h

    Signed-off-by: Sven Eckelmann

    Sven Eckelmann
     
  • checkpatch found various instances of "Possible repeated word" in various
    comments.

    Signed-off-by: Sven Eckelmann
    Signed-off-by: Simon Wunderlich

    Sven Eckelmann
     
  • Delete the doubled word "time" in a comment.
    Delete the doubled word "address" in a comment.

    Signed-off-by: Randy Dunlap
    Signed-off-by: Sven Eckelmann
    Signed-off-by: Simon Wunderlich

    Randy Dunlap
     
  • The function batadv_hardif_remove_interfaces was meant to remove all
    interfaces which are currently in the list of known (compatible) hardifs
    during module unload. But the function unregister_netdevice_notifier is
    called in batadv_exit before batadv_hardif_remove_interfaces. This will
    trigger NETDEV_UNREGISTER events for all available interfaces and in this
    process remove all interfaces from batadv_hardif_list. And
    batadv_hardif_remove_interfaces only operated on this (empty) list.

    Signed-off-by: Sven Eckelmann
    Signed-off-by: Simon Wunderlich

    Sven Eckelmann
     
  • Signed-off-by: Simon Wunderlich

    Simon Wunderlich
     

26 Jun, 2020

3 commits


10 Jun, 2020

1 commit

  • The dynamic key update for addr_list_lock still causes troubles,
    for example the following race condition still exists:

    CPU 0: CPU 1:
    (RCU read lock) (RTNL lock)
    dev_mc_seq_show() netdev_update_lockdep_key()
    -> lockdep_unregister_key()
    -> netif_addr_lock_bh()

    because lockdep doesn't provide an API to update it atomically.
    Therefore, we have to move it back to static keys and use subclass
    for nest locking like before.

    In commit 1a33e10e4a95 ("net: partially revert dynamic lockdep key
    changes"), I already reverted most parts of commit ab92d68fc22f
    ("net: core: add generic lockdep keys").

    This patch reverts the rest and also part of commit f3b0a18bb6cb
    ("net: remove unnecessary variables and callback"). After this
    patch, addr_list_lock changes back to using static keys and
    subclasses to satisfy lockdep. Thanks to dev->lower_level, we do
    not have to change back to ->ndo_get_lock_subclass().

    And hopefully this reduces some syzbot lockdep noises too.

    Reported-by: syzbot+f3a0e80c34b3fc28ac5e@syzkaller.appspotmail.com
    Cc: Taehee Yoo
    Cc: Dmitry Vyukov
    Signed-off-by: Cong Wang
    Signed-off-by: David S. Miller

    Cong Wang
     

27 May, 2020

1 commit


26 May, 2020

1 commit

  • The commit 8c46fcd78308 ("batman-adv: disable ethtool link speed detection
    when auto negotiation off") disabled the usage of ethtool's link_ksetting
    when auto negotation was enabled due to invalid values when used with
    tun/tap virtual net_devices. According to the patch, automatic measurements
    should be used for these kind of interfaces.

    But there are major flaws with this argumentation:

    * automatic measurements are not implemented
    * auto negotiation has nothing to do with the validity of the retrieved
    values

    The first point has to be fixed by a longer patch series. The "validity"
    part of the second point must be addressed in the same patch series by
    dropping the usage of ethtool's link_ksetting (thus always doing automatic
    measurements over ethernet).

    Drop the patch again to have more default values for various net_device
    types/configurations. The user can still overwrite them using the
    batadv_hardif's BATADV_ATTR_THROUGHPUT_OVERRIDE.

    Reported-by: Matthias Schiffer
    Signed-off-by: Sven Eckelmann
    Signed-off-by: Simon Wunderlich

    Sven Eckelmann
     

22 May, 2020

2 commits

  • In commit a63fc6b75cca ("rcu: Upgrade rcu_swap_protected() to
    rcu_replace_pointer()") a new helper macro named rcu_replace_pointer() was
    introduced to simplify code requiring to switch an rcu pointer to a new
    value while extracting the old one.

    Use rcu_replace_pointer() where appropriate to make code slimer.

    Signed-off-by: Antonio Quartulli
    Signed-off-by: Sven Eckelmann
    Signed-off-by: Simon Wunderlich

    Antonio Quartulli
     
  • The commit 1a33e10e4a95 ("net: partially revert dynamic lockdep key
    changes") reverts the commit ab92d68fc22f ("net: core: add generic lockdep
    keys"). But it forgot to also revert the commit 5759af0682b3 ("batman-adv:
    Drop lockdep.h include for soft-interface.c") which depends on the latter.

    Signed-off-by: Sven Eckelmann
    Signed-off-by: Simon Wunderlich

    Sven Eckelmann
     

21 May, 2020

1 commit


07 May, 2020

1 commit


05 May, 2020

1 commit

  • This patch reverts the folowing commits:

    commit 064ff66e2bef84f1153087612032b5b9eab005bd
    "bonding: add missing netdev_update_lockdep_key()"

    commit 53d374979ef147ab51f5d632dfe20b14aebeccd0
    "net: avoid updating qdisc_xmit_lock_key in netdev_update_lockdep_key()"

    commit 1f26c0d3d24125992ab0026b0dab16c08df947c7
    "net: fix kernel-doc warning in "

    commit ab92d68fc22f9afab480153bd82a20f6e2533769
    "net: core: add generic lockdep keys"

    but keeps the addr_list_lock_key because we still lock
    addr_list_lock nestedly on stack devices, unlikely xmit_lock
    this is safe because we don't take addr_list_lock on any fast
    path.

    Reported-and-tested-by: syzbot+aaa6fa4949cc5d9b7b25@syzkaller.appspotmail.com
    Cc: Dmitry Vyukov
    Cc: Taehee Yoo
    Signed-off-by: Cong Wang
    Acked-by: Taehee Yoo
    Signed-off-by: David S. Miller

    Cong Wang
     

24 Apr, 2020

1 commit


21 Apr, 2020

6 commits

  • batadv_v_ogm_process() invokes batadv_hardif_neigh_get(), which returns
    a reference of the neighbor object to "hardif_neigh" with increased
    refcount.

    When batadv_v_ogm_process() returns, "hardif_neigh" becomes invalid, so
    the refcount should be decreased to keep refcount balanced.

    The reference counting issue happens in one exception handling paths of
    batadv_v_ogm_process(). When batadv_v_ogm_orig_get() fails to get the
    orig node and returns NULL, the refcnt increased by
    batadv_hardif_neigh_get() is not decreased, causing a refcnt leak.

    Fix this issue by jumping to "out" label when batadv_v_ogm_orig_get()
    fails to get the orig node.

    Fixes: 9323158ef9f4 ("batman-adv: OGMv2 - implement originators logic")
    Signed-off-by: Xiyu Yang
    Signed-off-by: Xin Tan
    Signed-off-by: Sven Eckelmann
    Signed-off-by: Simon Wunderlich

    Xiyu Yang
     
  • batadv_show_throughput_override() invokes batadv_hardif_get_by_netdev(),
    which gets a batadv_hard_iface object from net_dev with increased refcnt
    and its reference is assigned to a local pointer 'hard_iface'.

    When batadv_store_throughput_override() returns, "hard_iface" becomes
    invalid, so the refcount should be decreased to keep refcount balanced.

    The issue happens in one error path of
    batadv_store_throughput_override(). When batadv_parse_throughput()
    returns NULL, the refcnt increased by batadv_hardif_get_by_netdev() is
    not decreased, causing a refcnt leak.

    Fix this issue by jumping to "out" label when batadv_parse_throughput()
    returns NULL.

    Fixes: 0b5ecc6811bd ("batman-adv: add throughput override attribute to hard_ifaces")
    Signed-off-by: Xiyu Yang
    Signed-off-by: Xin Tan
    Signed-off-by: Sven Eckelmann
    Signed-off-by: Simon Wunderlich

    Xiyu Yang
     
  • batadv_show_throughput_override() invokes batadv_hardif_get_by_netdev(),
    which gets a batadv_hard_iface object from net_dev with increased refcnt
    and its reference is assigned to a local pointer 'hard_iface'.

    When batadv_show_throughput_override() returns, "hard_iface" becomes
    invalid, so the refcount should be decreased to keep refcount balanced.

    The issue happens in the normal path of
    batadv_show_throughput_override(), which forgets to decrease the refcnt
    increased by batadv_hardif_get_by_netdev() before the function returns,
    causing a refcnt leak.

    Fix this issue by calling batadv_hardif_put() before the
    batadv_show_throughput_override() returns in the normal path.

    Fixes: 0b5ecc6811bd ("batman-adv: add throughput override attribute to hard_ifaces")
    Signed-off-by: Xiyu Yang
    Signed-off-by: Xin Tan
    Signed-off-by: Sven Eckelmann
    Signed-off-by: Simon Wunderlich

    Xiyu Yang
     
  • and change to pseudorandom numbers, as this is a traffic dithering
    operation that doesn't need crypto-grade.

    The previous code operated in 4 steps:

    1. Generate a random byte 0
    Signed-off-by: Sven Eckelmann
    Signed-off-by: Simon Wunderlich

    George Spelvin
     
  • The kernel provides a function to create random values from 0 - (max-1)
    since commit f337db64af05 ("random32: add prandom_u32_max and convert open
    coded users"). Simply use this function to replace code sections which use
    prandom_u32 and a handcrafted method to map it to the correct range.

    Signed-off-by: Sven Eckelmann

    Sven Eckelmann
     
  • The commit 04ae87a52074 ("ftrace: Rework event_create_dir()") restructured
    various macros in the ftrace framework. These changes also had the nice
    side effect that the linux/types.h include is no longer necessary to define
    some of the types used by these macros.

    Signed-off-by: Sven Eckelmann
    Signed-off-by: Simon Wunderlich

    Sven Eckelmann