Hi,
I went through all changes in batman-adv since v4.4 with a Fixes: line and checked whether they were backported to the LTS kernels. The ones which weren't ported and applied to this branch are now part of this patch series.
There are also following three patches included:
* batman-adv: Consider fragmentation for needed_headroom * batman-adv: Reserve needed_*room for fragments * batman-adv: Don't always reallocate the fragmentation skb head
which could in some circumstances cause packet loss but which were created to fix high CPU load/low throughput problems. But I've added them here anyway because the corresponding VXLAN patches were also added to stable. And some stable kernels also got these fixes a while back.
Kind regards, Sven
Linus Lüssing (4): batman-adv: Fix multicast TT issues with bogus ROAM flags batman-adv: mcast: fix duplicate mcast packets in BLA backbone from LAN batman-adv: mcast: fix duplicate mcast packets in BLA backbone from mesh batman-adv: mcast: fix duplicate mcast packets from BLA backbone to mesh
Sven Eckelmann (6): batman-adv: Keep fragments equally sized batman-adv: Prevent duplicated softif_vlan entry batman-adv: Consider fragmentation for needed_headroom batman-adv: Reserve needed_*room for fragments batman-adv: Don't always reallocate the fragmentation skb head batman-adv: Avoid WARN_ON timing related checks
Taehee Yoo (1): batman-adv: set .owner to THIS_MODULE
net/batman-adv/bat_iv_ogm.c | 4 +- net/batman-adv/bridge_loop_avoidance.c | 133 ++++++++++++++++++++----- net/batman-adv/bridge_loop_avoidance.h | 4 +- net/batman-adv/debugfs.c | 1 + net/batman-adv/fragmentation.c | 41 +++++--- net/batman-adv/hard-interface.c | 3 + net/batman-adv/multicast.c | 31 ++++++ net/batman-adv/multicast.h | 15 +++ net/batman-adv/soft-interface.c | 31 +++--- net/batman-adv/translation-table.c | 6 +- 10 files changed, 215 insertions(+), 54 deletions(-)
commit 1c2bcc766be44467809f1798cd4ceacafe20a852 upstream.
The batman-adv fragmentation packets have the design problem that they cannot be refragmented and cannot handle padding by the underlying link. The latter often leads to problems when networks are incorrectly configured and don't use a common MTU.
The sender could for example fragment a 1271 byte frame (plus external ethernet header (14) and batadv unicast header (10)) to fit in a 1280 bytes large MTU of the underlying link (max. 1294 byte frames). This would create a 1294 bytes large frame (fragment 2) and a 55 bytes large frame (fragment 1). The extra 54 bytes are the fragment header (20) added to each fragment and the external ethernet header (14) for the second fragment.
Let us assume that the next hop is then not able to transport 1294 bytes to its next hop. The 1294 byte large frame will be dropped but the 55 bytes large fragment will still be forwarded to its destination.
Or let us assume that the underlying hardware requires that each frame has a minimum size (e.g. 60 bytes). Then it will pad the 55 bytes frame to 60 bytes. The receiver of the 60 bytes frame will no longer be able to correctly assemble the two frames together because it is not aware that 5 bytes of the 60 bytes frame are padding and don't belong to the reassembled frame.
This can partly be avoided by splitting frames more equally. In this example, the 675 and 674 bytes large fragment frames could both potentially reach its destination without being too large or too small.
Reported-by: Martin Weinelt martin@darmstadt.freifunk.net Fixes: ee75ed88879a ("batman-adv: Fragment and send skbs larger than mtu") Signed-off-by: Sven Eckelmann sven@narfation.org Acked-by: Linus Lüssing linus.luessing@c0d3.blue Signed-off-by: Simon Wunderlich sw@simonwunderlich.de [ bp: 4.4 backported: adjust context, switch back to old return type + labels ] Signed-off-by: Sven Eckelmann sven@narfation.org --- net/batman-adv/fragmentation.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/net/batman-adv/fragmentation.c b/net/batman-adv/fragmentation.c index 9751b207b01f..3aceac21b283 100644 --- a/net/batman-adv/fragmentation.c +++ b/net/batman-adv/fragmentation.c @@ -396,7 +396,7 @@ bool batadv_frag_skb_fwd(struct sk_buff *skb, * batadv_frag_create - create a fragment from skb * @skb: skb to create fragment from * @frag_head: header to use in new fragment - * @mtu: size of new fragment + * @fragment_size: size of new fragment * * Split the passed skb into two fragments: A new one with size matching the * passed mtu and the old one with the rest. The new skb contains data from the @@ -406,11 +406,11 @@ bool batadv_frag_skb_fwd(struct sk_buff *skb, */ static struct sk_buff *batadv_frag_create(struct sk_buff *skb, struct batadv_frag_packet *frag_head, - unsigned int mtu) + unsigned int fragment_size) { struct sk_buff *skb_fragment; unsigned header_size = sizeof(*frag_head); - unsigned fragment_size = mtu - header_size; + unsigned mtu = fragment_size + header_size;
skb_fragment = netdev_alloc_skb(NULL, mtu + ETH_HLEN); if (!skb_fragment) @@ -448,7 +448,7 @@ bool batadv_frag_send_packet(struct sk_buff *skb, struct sk_buff *skb_fragment; unsigned mtu = neigh_node->if_incoming->net_dev->mtu; unsigned header_size = sizeof(frag_header); - unsigned max_fragment_size, max_packet_size; + unsigned max_fragment_size, num_fragments; bool ret = false;
/* To avoid merge and refragmentation at next-hops we never send @@ -456,10 +456,15 @@ bool batadv_frag_send_packet(struct sk_buff *skb, */ mtu = min_t(unsigned, mtu, BATADV_FRAG_MAX_FRAG_SIZE); max_fragment_size = mtu - header_size; - max_packet_size = max_fragment_size * BATADV_FRAG_MAX_FRAGMENTS; + + if (skb->len == 0 || max_fragment_size == 0) + goto out_err; + + num_fragments = (skb->len - 1) / max_fragment_size + 1; + max_fragment_size = (skb->len - 1) / num_fragments + 1;
/* Don't even try to fragment, if we need more than 16 fragments */ - if (skb->len > max_packet_size) + if (num_fragments > BATADV_FRAG_MAX_FRAGMENTS) goto out_err;
bat_priv = orig_node->bat_priv; @@ -484,7 +489,8 @@ bool batadv_frag_send_packet(struct sk_buff *skb, if (frag_header.no == BATADV_FRAG_MAX_FRAGMENTS - 1) goto out_err;
- skb_fragment = batadv_frag_create(skb, &frag_header, mtu); + skb_fragment = batadv_frag_create(skb, &frag_header, + max_fragment_size); if (!skb_fragment) goto out_err;
From: Linus Lüssing linus.luessing@c0d3.blue
commit a44ebeff6bbd6ef50db41b4195fca87b21aefd20 upstream.
When a (broken) node wrongly sends multicast TT entries with a ROAM flag then this causes any receiving node to drop all entries for the same multicast MAC address announced by other nodes, leading to packet loss.
Fix this DoS vector by only storing TT sync flags. For multicast TT non-sync'ing flag bits like ROAM are unused so far anyway.
Fixes: 1d8ab8d3c176 ("batman-adv: Modified forwarding behaviour for multicast packets") Reported-by: Leonardo Mörlein me@irrelefant.net Signed-off-by: Linus Lüssing linus.luessing@c0d3.blue Signed-off-by: Simon Wunderlich sw@simonwunderlich.de [ bp: 4.4 backported: adjust context, use old style to access flags ] Signed-off-by: Sven Eckelmann sven@narfation.org --- net/batman-adv/translation-table.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c index 5f976485e8c6..208cf66868e9 100644 --- a/net/batman-adv/translation-table.c +++ b/net/batman-adv/translation-table.c @@ -1426,7 +1426,8 @@ static bool batadv_tt_global_add(struct batadv_priv *bat_priv, ether_addr_copy(common->addr, tt_addr); common->vid = vid;
- common->flags = flags & (~BATADV_TT_SYNC_MASK); + if (!is_multicast_ether_addr(common->addr)) + common->flags = flags & (~BATADV_TT_SYNC_MASK);
tt_global_entry->roam_at = 0; /* node must store current time in case of roaming. This is @@ -1489,7 +1490,8 @@ static bool batadv_tt_global_add(struct batadv_priv *bat_priv, * TT_CLIENT_WIFI, therefore they have to be copied in the * client entry */ - tt_global_entry->common.flags |= flags & (~BATADV_TT_SYNC_MASK); + if (!is_multicast_ether_addr(common->addr)) + tt_global_entry->common.flags |= flags & (~BATADV_TT_SYNC_MASK);
/* If there is the BATADV_TT_CLIENT_ROAM flag set, there is only * one originator left in the list and we previously received a
commit 94cb82f594ed86be303398d6dfc7640a6f1d45d4 upstream.
The function batadv_softif_vlan_get is responsible for adding new softif_vlan to the softif_vlan_list. It first checks whether the entry already is in the list or not. If it is, then the creation of a new entry is aborted.
But the lock for the list is only held when the list is really modified. This could lead to duplicated entries because another context could create an entry with the same key between the check and the list manipulation.
The check and the manipulation of the list must therefore be in the same locked code section.
Fixes: 5d2c05b21337 ("batman-adv: add per VLAN interface attribute framework") Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Simon Wunderlich sw@simonwunderlich.de [ bp: 4.4 backport: switch back to atomic_t based reference counting. ] Signed-off-by: Sven Eckelmann sven@narfation.org --- net/batman-adv/soft-interface.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c index ff693887ea82..f1e2e7e33500 100644 --- a/net/batman-adv/soft-interface.c +++ b/net/batman-adv/soft-interface.c @@ -539,15 +539,20 @@ int batadv_softif_create_vlan(struct batadv_priv *bat_priv, unsigned short vid) struct batadv_softif_vlan *vlan; int err;
+ spin_lock_bh(&bat_priv->softif_vlan_list_lock); + vlan = batadv_softif_vlan_get(bat_priv, vid); if (vlan) { batadv_softif_vlan_free_ref(vlan); + spin_unlock_bh(&bat_priv->softif_vlan_list_lock); return -EEXIST; }
vlan = kzalloc(sizeof(*vlan), GFP_ATOMIC); - if (!vlan) + if (!vlan) { + spin_unlock_bh(&bat_priv->softif_vlan_list_lock); return -ENOMEM; + }
vlan->bat_priv = bat_priv; vlan->vid = vid; @@ -555,16 +560,19 @@ int batadv_softif_create_vlan(struct batadv_priv *bat_priv, unsigned short vid)
atomic_set(&vlan->ap_isolation, 0);
+ hlist_add_head_rcu(&vlan->list, &bat_priv->softif_vlan_list); + spin_unlock_bh(&bat_priv->softif_vlan_list_lock); + + /* batadv_sysfs_add_vlan cannot be in the spinlock section due to the + * sleeping behavior of the sysfs functions and the fs_reclaim lock + */ err = batadv_sysfs_add_vlan(bat_priv->soft_iface, vlan); if (err) { - kfree(vlan); + /* ref for the list */ + batadv_softif_vlan_free_ref(vlan); return err; }
- spin_lock_bh(&bat_priv->softif_vlan_list_lock); - hlist_add_head_rcu(&vlan->list, &bat_priv->softif_vlan_list); - spin_unlock_bh(&bat_priv->softif_vlan_list_lock); - /* add a new TT local entry. This one will be marked with the NOPURGE * flag */
From: Linus Lüssing linus.luessing@c0d3.blue
commit 3236d215ad38a3f5372e65cd1e0a52cf93d3c6a2 upstream.
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: 1d8ab8d3c176 ("batman-adv: Modified forwarding behaviour for multicast packets") Signed-off-by: Linus Lüssing linus.luessing@c0d3.blue Signed-off-by: Simon Wunderlich sw@simonwunderlich.de [ bp: 4.4 backport: drop usage in non-existing batadv_mcast_forw_*, correct fixes line ] Signed-off-by: Sven Eckelmann sven@narfation.org --- net/batman-adv/multicast.c | 31 +++++++++++++++++++++++++++++++ net/batman-adv/multicast.h | 15 +++++++++++++++ net/batman-adv/soft-interface.c | 5 ++--- 3 files changed, 48 insertions(+), 3 deletions(-)
diff --git a/net/batman-adv/multicast.c b/net/batman-adv/multicast.c index 8aa2d65df86f..44965f71ad73 100644 --- a/net/batman-adv/multicast.c +++ b/net/batman-adv/multicast.c @@ -44,7 +44,9 @@ #include <net/addrconf.h> #include <net/ipv6.h>
+#include "bridge_loop_avoidance.h" #include "packet.h" +#include "send.h" #include "translation-table.h"
/** @@ -805,6 +807,35 @@ void batadv_mcast_free(struct batadv_priv *bat_priv) batadv_mcast_mla_tt_retract(bat_priv, NULL); }
+/** + * batadv_mcast_forw_send_orig() - send a multicast packet to an originator + * @bat_priv: the bat priv with all the soft interface information + * @skb: the multicast packet to send + * @vid: the vlan identifier + * @orig_node: the originator to send the packet to + * + * Return: NET_XMIT_DROP in case of error or NET_XMIT_SUCCESS otherwise. + */ +int batadv_mcast_forw_send_orig(struct batadv_priv *bat_priv, + struct sk_buff *skb, + unsigned short vid, + struct batadv_orig_node *orig_node) +{ + /* Avoid sending multicast-in-unicast packets to other BLA + * gateways - they already got the frame from the LAN side + * we share with them. + * TODO: Refactor to take BLA into account earlier, to avoid + * reducing the mcast_fanout count. + */ + if (batadv_bla_is_backbone_gw_orig(bat_priv, orig_node->orig, vid)) { + dev_kfree_skb(skb); + return NET_XMIT_SUCCESS; + } + + return batadv_send_skb_unicast(bat_priv, skb, BATADV_UNICAST, 0, + orig_node, vid); +} + /** * batadv_mcast_purge_orig - reset originator global mcast state modifications * @orig: the originator which is going to get purged diff --git a/net/batman-adv/multicast.h b/net/batman-adv/multicast.h index 8f3cb04b9f13..dd83ef07e2f2 100644 --- a/net/batman-adv/multicast.h +++ b/net/batman-adv/multicast.h @@ -44,6 +44,11 @@ enum batadv_forw_mode batadv_mcast_forw_mode(struct batadv_priv *bat_priv, struct sk_buff *skb, struct batadv_orig_node **mcast_single_orig);
+int batadv_mcast_forw_send_orig(struct batadv_priv *bat_priv, + struct sk_buff *skb, + unsigned short vid, + struct batadv_orig_node *orig_node); + void batadv_mcast_init(struct batadv_priv *bat_priv);
void batadv_mcast_free(struct batadv_priv *bat_priv); @@ -68,6 +73,16 @@ static inline int batadv_mcast_init(struct batadv_priv *bat_priv) return 0; }
+static inline int +batadv_mcast_forw_send_orig(struct batadv_priv *bat_priv, + struct sk_buff *skb, + unsigned short vid, + struct batadv_orig_node *orig_node) +{ + kfree_skb(skb); + return NET_XMIT_DROP; +} + static inline void batadv_mcast_free(struct batadv_priv *bat_priv) { } diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c index f1e2e7e33500..64b46e9e365b 100644 --- a/net/batman-adv/soft-interface.c +++ b/net/batman-adv/soft-interface.c @@ -353,9 +353,8 @@ static int batadv_interface_tx(struct sk_buff *skb, goto dropped; ret = batadv_send_skb_via_gw(bat_priv, skb, vid); } else if (mcast_single_orig) { - ret = batadv_send_skb_unicast(bat_priv, skb, - BATADV_UNICAST, 0, - mcast_single_orig, vid); + ret = batadv_mcast_forw_send_orig(bat_priv, skb, vid, + mcast_single_orig); } else { if (batadv_dat_snoop_outgoing_arp_request(bat_priv, skb))
From: Linus Lüssing linus.luessing@c0d3.blue
commit 74c09b7275126da1b642b90c9cdc3ae8b729ad4b upstream
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: 2d3f6ccc4ea5 ("batman-adv: check incoming packet type for bla") Signed-off-by: Linus Lüssing linus.luessing@c0d3.blue Acked-by: Simon Wunderlich sw@simonwunderlich.de [ bp: 4.4 backported: adjust context, correct fixes line ] Signed-off-by: Sven Eckelmann sven@narfation.org --- net/batman-adv/bridge_loop_avoidance.c | 34 +++++++++++++++++++------- net/batman-adv/bridge_loop_avoidance.h | 4 +-- net/batman-adv/soft-interface.c | 6 ++--- 3 files changed, 30 insertions(+), 14 deletions(-)
diff --git a/net/batman-adv/bridge_loop_avoidance.c b/net/batman-adv/bridge_loop_avoidance.c index 1267cbb1a329..c8fbcaed5844 100644 --- a/net/batman-adv/bridge_loop_avoidance.c +++ b/net/batman-adv/bridge_loop_avoidance.c @@ -1538,7 +1538,7 @@ void batadv_bla_free(struct batadv_priv *bat_priv) * @bat_priv: the bat priv with all the soft interface information * @skb: the frame to be checked * @vid: the VLAN ID of the frame - * @is_bcast: the packet came in a broadcast packet type. + * @packet_type: the batman packet type this frame came in * * bla_rx avoidance checks if: * * we have to race for a claim @@ -1549,7 +1549,7 @@ void batadv_bla_free(struct batadv_priv *bat_priv) * process the skb. */ int batadv_bla_rx(struct batadv_priv *bat_priv, struct sk_buff *skb, - unsigned short vid, bool is_bcast) + unsigned short vid, int packet_type) { struct batadv_bla_backbone_gw *backbone_gw; struct ethhdr *ethhdr; @@ -1568,9 +1568,24 @@ int batadv_bla_rx(struct batadv_priv *bat_priv, struct sk_buff *skb, goto allow;
if (unlikely(atomic_read(&bat_priv->bla.num_requests))) - /* don't allow broadcasts while requests are in flight */ - if (is_multicast_ether_addr(ethhdr->h_dest) && is_bcast) - goto handled; + /* don't allow multicast packets while requests are in flight */ + if (is_multicast_ether_addr(ethhdr->h_dest)) + /* Both broadcast flooding or multicast-via-unicasts + * delivery might send to multiple backbone gateways + * sharing the same LAN and therefore need to coordinate + * which backbone gateway forwards into the LAN, + * by claiming the payload source address. + * + * Broadcast flooding and multicast-via-unicasts + * delivery use the following two batman packet types. + * Note: explicitly exclude BATADV_UNICAST_4ADDR, + * as the DHCP gateway feature will send explicitly + * to only one BLA gateway, so the claiming process + * should be avoided there. + */ + if (packet_type == BATADV_BCAST || + packet_type == BATADV_UNICAST) + goto handled;
ether_addr_copy(search_claim.addr, ethhdr->h_source); search_claim.vid = vid; @@ -1598,13 +1613,14 @@ int batadv_bla_rx(struct batadv_priv *bat_priv, struct sk_buff *skb, goto allow; }
- /* if it is a broadcast ... */ - if (is_multicast_ether_addr(ethhdr->h_dest) && is_bcast) { + /* if it is a multicast ... */ + if (is_multicast_ether_addr(ethhdr->h_dest) && + (packet_type == BATADV_BCAST || packet_type == BATADV_UNICAST)) { /* ... drop it. the responsible gateway is in charge. * - * We need to check is_bcast because with the gateway + * We need to check packet type because with the gateway * feature, broadcasts (like DHCP requests) may be sent - * using a unicast packet type. + * using a unicast 4 address packet type. See comment above. */ goto handled; } else { diff --git a/net/batman-adv/bridge_loop_avoidance.h b/net/batman-adv/bridge_loop_avoidance.h index 025152b34282..d1553c46df8c 100644 --- a/net/batman-adv/bridge_loop_avoidance.h +++ b/net/batman-adv/bridge_loop_avoidance.h @@ -27,7 +27,7 @@ struct sk_buff;
#ifdef CONFIG_BATMAN_ADV_BLA int batadv_bla_rx(struct batadv_priv *bat_priv, struct sk_buff *skb, - unsigned short vid, bool is_bcast); + unsigned short vid, int packet_type); int batadv_bla_tx(struct batadv_priv *bat_priv, struct sk_buff *skb, unsigned short vid); int batadv_bla_is_backbone_gw(struct sk_buff *skb, @@ -50,7 +50,7 @@ void batadv_bla_free(struct batadv_priv *bat_priv);
static inline int batadv_bla_rx(struct batadv_priv *bat_priv, struct sk_buff *skb, unsigned short vid, - bool is_bcast) + int packet_type) { return 0; } diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c index 64b46e9e365b..5105e860d3aa 100644 --- a/net/batman-adv/soft-interface.c +++ b/net/batman-adv/soft-interface.c @@ -393,10 +393,10 @@ void batadv_interface_rx(struct net_device *soft_iface, struct vlan_ethhdr *vhdr; struct ethhdr *ethhdr; unsigned short vid; - bool is_bcast; + int packet_type;
batadv_bcast_packet = (struct batadv_bcast_packet *)skb->data; - is_bcast = (batadv_bcast_packet->packet_type == BATADV_BCAST); + packet_type = batadv_bcast_packet->packet_type;
/* check if enough space is available for pulling, and pull */ if (!pskb_may_pull(skb, hdr_size)) @@ -444,7 +444,7 @@ void batadv_interface_rx(struct net_device *soft_iface, /* Let the bridge loop avoidance check the packet. If will * not handle it, we can safely push it up. */ - if (batadv_bla_rx(bat_priv, skb, vid, is_bcast)) + if (batadv_bla_rx(bat_priv, skb, vid, packet_type)) goto out;
if (orig_node)
From: Linus Lüssing linus.luessing@c0d3.blue
commit 2369e827046920ef0599e6a36b975ac5c0a359c2 upstream.
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: fe2da6ff27c7 ("batman-adv: add broadcast duplicate check") Signed-off-by: Linus Lüssing linus.luessing@c0d3.blue Signed-off-by: Simon Wunderlich sw@simonwunderlich.de [ bp: 4.4 backported: adjust context, correct fixes line, switch back to int return type ] Signed-off-by: Sven Eckelmann sven@narfation.org --- net/batman-adv/bridge_loop_avoidance.c | 99 ++++++++++++++++++++++---- 1 file changed, 85 insertions(+), 14 deletions(-)
diff --git a/net/batman-adv/bridge_loop_avoidance.c b/net/batman-adv/bridge_loop_avoidance.c index c8fbcaed5844..e97b1e000670 100644 --- a/net/batman-adv/bridge_loop_avoidance.c +++ b/net/batman-adv/bridge_loop_avoidance.c @@ -1366,31 +1366,32 @@ int batadv_bla_init(struct batadv_priv *bat_priv) }
/** - * batadv_bla_check_bcast_duplist + * batadv_bla_check_duplist() - Check if a frame is in the broadcast dup. * @bat_priv: the bat priv with all the soft interface information - * @skb: contains the bcast_packet to be checked + * @skb: contains the multicast packet to be checked + * @payload_ptr: pointer to position inside the head buffer of the skb + * marking the start of the data to be CRC'ed + * @orig: originator mac address, NULL if unknown * - * check if it is on our broadcast list. Another gateway might - * have sent the same packet because it is connected to the same backbone, - * so we have to remove this duplicate. + * Check if it is on our broadcast list. Another gateway might have sent the + * same packet because it is connected to the same backbone, so we have to + * remove this duplicate. * * This is performed by checking the CRC, which will tell us * with a good chance that it is the same packet. If it is furthermore * sent by another host, drop it. We allow equal packets from * the same host however as this might be intended. */ -int batadv_bla_check_bcast_duplist(struct batadv_priv *bat_priv, - struct sk_buff *skb) +static int batadv_bla_check_duplist(struct batadv_priv *bat_priv, + struct sk_buff *skb, u8 *payload_ptr, + const u8 *orig) { int i, curr, ret = 0; __be32 crc; - struct batadv_bcast_packet *bcast_packet; struct batadv_bcast_duplist_entry *entry;
- bcast_packet = (struct batadv_bcast_packet *)skb->data; - /* calculate the crc ... */ - crc = batadv_skb_crc32(skb, (u8 *)(bcast_packet + 1)); + crc = batadv_skb_crc32(skb, payload_ptr);
spin_lock_bh(&bat_priv->bla.bcast_duplist_lock);
@@ -1409,8 +1410,21 @@ int batadv_bla_check_bcast_duplist(struct batadv_priv *bat_priv, if (entry->crc != crc) continue;
- if (batadv_compare_eth(entry->orig, bcast_packet->orig)) - continue; + /* are the originators both known and not anonymous? */ + if (orig && !is_zero_ether_addr(orig) && + !is_zero_ether_addr(entry->orig)) { + /* If known, check if the new frame came from + * the same originator: + * We are safe to take identical frames from the + * same orig, if known, as multiplications in + * the mesh are detected via the (orig, seqno) pair. + * So we can be a bit more liberal here and allow + * identical frames from the same orig which the source + * host might have sent multiple times on purpose. + */ + if (batadv_compare_eth(entry->orig, orig)) + continue; + }
/* this entry seems to match: same crc, not too old, * and from another gw. therefore return 1 to forbid it. @@ -1426,7 +1440,14 @@ int batadv_bla_check_bcast_duplist(struct batadv_priv *bat_priv, entry = &bat_priv->bla.bcast_duplist[curr]; entry->crc = crc; entry->entrytime = jiffies; - ether_addr_copy(entry->orig, bcast_packet->orig); + + /* known originator */ + if (orig) + ether_addr_copy(entry->orig, orig); + /* anonymous originator */ + else + eth_zero_addr(entry->orig); + bat_priv->bla.bcast_duplist_curr = curr;
out: @@ -1435,6 +1456,48 @@ int batadv_bla_check_bcast_duplist(struct batadv_priv *bat_priv, return ret; }
+/** + * batadv_bla_check_ucast_duplist() - Check if a frame is in the broadcast dup. + * @bat_priv: the bat priv with all the soft interface information + * @skb: contains the multicast packet to be checked, decapsulated from a + * unicast_packet + * + * Check if it is on our broadcast list. Another gateway might have sent the + * same packet because it is connected to the same backbone, so we have to + * remove this duplicate. + * + * Return: true if a packet is in the duplicate list, false otherwise. + */ +static bool batadv_bla_check_ucast_duplist(struct batadv_priv *bat_priv, + struct sk_buff *skb) +{ + return batadv_bla_check_duplist(bat_priv, skb, (u8 *)skb->data, NULL); +} + +/** + * batadv_bla_check_bcast_duplist() - Check if a frame is in the broadcast dup. + * @bat_priv: the bat priv with all the soft interface information + * @skb: contains the bcast_packet to be checked + * + * Check if it is on our broadcast list. Another gateway might have sent the + * same packet because it is connected to the same backbone, so we have to + * remove this duplicate. + * + * Return: true if a packet is in the duplicate list, false otherwise. + */ +int batadv_bla_check_bcast_duplist(struct batadv_priv *bat_priv, + struct sk_buff *skb) +{ + struct batadv_bcast_packet *bcast_packet; + u8 *payload_ptr; + + bcast_packet = (struct batadv_bcast_packet *)skb->data; + payload_ptr = (u8 *)(bcast_packet + 1); + + return batadv_bla_check_duplist(bat_priv, skb, payload_ptr, + bcast_packet->orig); +} + /** * batadv_bla_is_backbone_gw_orig * @bat_priv: the bat priv with all the soft interface information @@ -1587,6 +1650,14 @@ int batadv_bla_rx(struct batadv_priv *bat_priv, struct sk_buff *skb, packet_type == BATADV_UNICAST) goto handled;
+ /* potential duplicates from foreign BLA backbone gateways via + * multicast-in-unicast packets + */ + if (is_multicast_ether_addr(ethhdr->h_dest) && + packet_type == BATADV_UNICAST && + batadv_bla_check_ucast_duplist(bat_priv, skb)) + goto handled; + ether_addr_copy(search_claim.addr, ethhdr->h_source); search_claim.vid = vid; claim = batadv_claim_hash_find(bat_priv, &search_claim);
From: Taehee Yoo ap420073@gmail.com
commit 14a2e551faea53d45bc11629a9dac88f88950ca7 upstream.
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 ap420073@gmail.com Signed-off-by: Simon Wunderlich sw@simonwunderlich.de [ bp: 4.4 backported: switch to old filename. ] Signed-off-by: Sven Eckelmann sven@narfation.org --- net/batman-adv/debugfs.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/net/batman-adv/debugfs.c b/net/batman-adv/debugfs.c index b2ef03a3a2d4..b905763dc2e7 100644 --- a/net/batman-adv/debugfs.c +++ b/net/batman-adv/debugfs.c @@ -214,6 +214,7 @@ static const struct file_operations batadv_log_fops = { .read = batadv_log_read, .poll = batadv_log_poll, .llseek = no_llseek, + .owner = THIS_MODULE, };
static int batadv_debug_log_setup(struct batadv_priv *bat_priv)
commit 4ca23e2c2074465bff55ea14221175fecdf63c5f upstream.
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 linus.luessing@c0d3.blue Signed-off-by: Simon Wunderlich sw@simonwunderlich.de Signed-off-by: Sven Eckelmann sven@narfation.org --- net/batman-adv/hard-interface.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c index c59bbc327763..0bd7c9e6c9a0 100644 --- a/net/batman-adv/hard-interface.c +++ b/net/batman-adv/hard-interface.c @@ -316,6 +316,9 @@ static void batadv_hardif_recalc_extra_skbroom(struct net_device *soft_iface) needed_headroom = lower_headroom + (lower_header_len - ETH_HLEN); needed_headroom += batadv_max_header_len();
+ /* fragmentation headers don't strip the unicast/... header */ + needed_headroom += sizeof(struct batadv_frag_packet); + soft_iface->needed_headroom = needed_headroom; soft_iface->needed_tailroom = lower_tailroom; }
commit c5cbfc87558168ef4c3c27ce36eba6b83391db19 upstream.
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 sven@narfation.org Signed-off-by: Simon Wunderlich sw@simonwunderlich.de [ bp: 4.4 backported: adjust context. ] Signed-off-by: Sven Eckelmann sven@narfation.org --- net/batman-adv/fragmentation.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/net/batman-adv/fragmentation.c b/net/batman-adv/fragmentation.c index 3aceac21b283..07dd799e0d56 100644 --- a/net/batman-adv/fragmentation.c +++ b/net/batman-adv/fragmentation.c @@ -394,6 +394,7 @@ bool batadv_frag_skb_fwd(struct sk_buff *skb,
/** * batadv_frag_create - create a fragment from skb + * @net_dev: outgoing device for fragment * @skb: skb to create fragment from * @frag_head: header to use in new fragment * @fragment_size: size of new fragment @@ -404,22 +405,25 @@ bool batadv_frag_skb_fwd(struct sk_buff *skb, * * Returns the new fragment, NULL on error. */ -static struct sk_buff *batadv_frag_create(struct sk_buff *skb, +static struct sk_buff *batadv_frag_create(struct net_device *net_dev, + struct sk_buff *skb, struct batadv_frag_packet *frag_head, unsigned int fragment_size) { + unsigned int ll_reserved = LL_RESERVED_SPACE(net_dev); + unsigned int tailroom = net_dev->needed_tailroom; struct sk_buff *skb_fragment; unsigned header_size = sizeof(*frag_head); unsigned mtu = fragment_size + header_size;
- skb_fragment = netdev_alloc_skb(NULL, mtu + ETH_HLEN); + skb_fragment = dev_alloc_skb(ll_reserved + mtu + tailroom); if (!skb_fragment) goto err;
skb->priority = TC_PRIO_CONTROL;
/* Eat the last mtu-bytes of the skb */ - skb_reserve(skb_fragment, header_size + ETH_HLEN); + skb_reserve(skb_fragment, ll_reserved + header_size); skb_split(skb, skb_fragment, skb->len - fragment_size);
/* Add the header */ @@ -442,11 +446,12 @@ bool batadv_frag_send_packet(struct sk_buff *skb, struct batadv_orig_node *orig_node, struct batadv_neigh_node *neigh_node) { + struct net_device *net_dev = neigh_node->if_incoming->net_dev; struct batadv_priv *bat_priv; struct batadv_hard_iface *primary_if = NULL; struct batadv_frag_packet frag_header; struct sk_buff *skb_fragment; - unsigned mtu = neigh_node->if_incoming->net_dev->mtu; + unsigned mtu = net_dev->mtu; unsigned header_size = sizeof(frag_header); unsigned max_fragment_size, num_fragments; bool ret = false; @@ -489,7 +494,7 @@ bool batadv_frag_send_packet(struct sk_buff *skb, if (frag_header.no == BATADV_FRAG_MAX_FRAGMENTS - 1) goto out_err;
- skb_fragment = batadv_frag_create(skb, &frag_header, + skb_fragment = batadv_frag_create(net_dev, skb, &frag_header, max_fragment_size); if (!skb_fragment) goto out_err;
commit 992b03b88e36254e26e9a4977ab948683e21bd9f upstream.
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 sven@narfation.org Signed-off-by: Simon Wunderlich sw@simonwunderlich.de [ bp: 4.4 backported: adjust context, switch back to old return type + labels ] Signed-off-by: Sven Eckelmann sven@narfation.org --- net/batman-adv/fragmentation.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/net/batman-adv/fragmentation.c b/net/batman-adv/fragmentation.c index 07dd799e0d56..371f50804fc2 100644 --- a/net/batman-adv/fragmentation.c +++ b/net/batman-adv/fragmentation.c @@ -507,11 +507,13 @@ bool batadv_frag_send_packet(struct sk_buff *skb, frag_header.no++; }
- /* Make room for the fragment header. */ - if (batadv_skb_head_push(skb, header_size) < 0 || - pskb_expand_head(skb, header_size + ETH_HLEN, 0, GFP_ATOMIC) < 0) + /* make sure that there is at least enough head for the fragmentation + * and ethernet headers + */ + if (skb_cow_head(skb, ETH_HLEN + header_size) < 0) goto out_err;
+ skb_push(skb, header_size); memcpy(skb->data, &frag_header, header_size);
/* Send the last fragment */
commit 9f460ae31c4435fd022c443a6029352217a16ac1 upstream.
The soft/batadv interface for a queued OGM can be changed during the time the OGM was queued for transmission and when the OGM is actually transmitted by the worker.
But WARN_ON must be used to denote kernel bugs and not to print simple warnings. A warning can simply be printed using pr_warn.
Reported-by: Tetsuo Handa penguin-kernel@i-love.sakura.ne.jp Reported-by: syzbot+c0b807de416427ff3dd1@syzkaller.appspotmail.com Fixes: ef0a937f7a14 ("batman-adv: consider outgoing interface in OGM sending") Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Simon Wunderlich sw@simonwunderlich.de [ bp: 4.4 backported: adjust context. ] Signed-off-by: Sven Eckelmann sven@narfation.org --- net/batman-adv/bat_iv_ogm.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c index caea5bb38d4b..6f8d2fe114f6 100644 --- a/net/batman-adv/bat_iv_ogm.c +++ b/net/batman-adv/bat_iv_ogm.c @@ -526,8 +526,10 @@ static void batadv_iv_ogm_emit(struct batadv_forw_packet *forw_packet) if (WARN_ON(!forw_packet->if_outgoing)) goto out;
- if (WARN_ON(forw_packet->if_outgoing->soft_iface != soft_iface)) + if (forw_packet->if_outgoing->soft_iface != soft_iface) { + pr_warn("%s: soft interface switch for queued OGM\n", __func__); goto out; + }
if (forw_packet->if_incoming->if_status != BATADV_IF_ACTIVE) goto out;
On Sat, Nov 20, 2021 at 01:39:28PM +0100, Sven Eckelmann wrote:
Hi,
I went through all changes in batman-adv since v4.4 with a Fixes: line and checked whether they were backported to the LTS kernels. The ones which weren't ported and applied to this branch are now part of this patch series.
There are also following three patches included:
- batman-adv: Consider fragmentation for needed_headroom
- batman-adv: Reserve needed_*room for fragments
- batman-adv: Don't always reallocate the fragmentation skb head
which could in some circumstances cause packet loss but which were created to fix high CPU load/low throughput problems. But I've added them here anyway because the corresponding VXLAN patches were also added to stable. And some stable kernels also got these fixes a while back.
All patches and series now queued up, thanks!
greg k-h
linux-stable-mirror@lists.linaro.org
-
Greg KH -
Sven Eckelmann