Hi,
I went through all changes in batman-adv since v4.9 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 (3): batman-adv: Fix own OGM check in aggregated OGMs batman-adv: mcast: fix duplicate mcast packets in BLA backbone from LAN batman-adv: mcast: fix duplicate mcast packets from BLA backbone to mesh
Sven Eckelmann (4): batman-adv: Keep fragments equally sized batman-adv: Consider fragmentation for needed_headroom batman-adv: Reserve needed_*room for fragments batman-adv: Don't always reallocate the fragmentation skb head
net/batman-adv/bat_v_ogm.c | 11 +-- net/batman-adv/bridge_loop_avoidance.c | 103 +++++++++++++++++++++---- net/batman-adv/fragmentation.c | 42 ++++++---- 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 | 5 +- 7 files changed, 172 insertions(+), 38 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.9 backported: adjust context. ] 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 fef21f75892e..b36606816bac 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 int header_size = sizeof(*frag_head); - unsigned int fragment_size = mtu - header_size; + unsigned int mtu = fragment_size + header_size;
skb_fragment = netdev_alloc_skb(NULL, mtu + ETH_HLEN); if (!skb_fragment) @@ -449,7 +449,7 @@ int batadv_frag_send_packet(struct sk_buff *skb, struct sk_buff *skb_fragment; unsigned int mtu = neigh_node->if_incoming->net_dev->mtu; unsigned int header_size = sizeof(frag_header); - unsigned int max_fragment_size, max_packet_size; + unsigned int max_fragment_size, num_fragments; int ret = -1;
/* To avoid merge and refragmentation at next-hops we never send @@ -457,10 +457,15 @@ int batadv_frag_send_packet(struct sk_buff *skb, */ mtu = min_t(unsigned int, 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) + return -EINVAL; + + 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;
bat_priv = orig_node->bat_priv; @@ -498,7 +503,8 @@ int batadv_frag_send_packet(struct sk_buff *skb, goto out; }
- 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;
This is a note to let you know that I've just added the patch titled
batman-adv: Keep fragments equally sized
to the 4.9-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summ...
The filename of the patch is: batman-adv-keep-fragments-equally-sized.patch and it can be found in the queue-4.9 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree, please let stable@vger.kernel.org know about it.
From foo@baz Tue Nov 23 01:37:10 PM CET 2021 From: Sven Eckelmann sven@narfation.org Date: Sat, 20 Nov 2021 13:39:52 +0100 Subject: batman-adv: Keep fragments equally sized To: stable@vger.kernel.org Cc: b.a.t.m.a.n@lists.open-mesh.org, "Sven Eckelmann" sven@narfation.org, "Martin Weinelt" martin@darmstadt.freifunk.net, "Linus Lüssing" linus.luessing@c0d3.blue, "Simon Wunderlich" sw@simonwunderlich.de Message-ID: 20211120123958.260826-2-sven@narfation.org
From: Sven Eckelmann sven@narfation.org
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.9 backported: adjust context. ] Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org --- net/batman-adv/fragmentation.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-)
--- a/net/batman-adv/fragmentation.c +++ b/net/batman-adv/fragmentation.c @@ -396,7 +396,7 @@ out: * 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 @@ out: */ 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 int header_size = sizeof(*frag_head); - unsigned int fragment_size = mtu - header_size; + unsigned int mtu = fragment_size + header_size;
skb_fragment = netdev_alloc_skb(NULL, mtu + ETH_HLEN); if (!skb_fragment) @@ -449,7 +449,7 @@ int batadv_frag_send_packet(struct sk_bu struct sk_buff *skb_fragment; unsigned int mtu = neigh_node->if_incoming->net_dev->mtu; unsigned int header_size = sizeof(frag_header); - unsigned int max_fragment_size, max_packet_size; + unsigned int max_fragment_size, num_fragments; int ret = -1;
/* To avoid merge and refragmentation at next-hops we never send @@ -457,10 +457,15 @@ int batadv_frag_send_packet(struct sk_bu */ mtu = min_t(unsigned int, 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) + return -EINVAL; + + 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;
bat_priv = orig_node->bat_priv; @@ -498,7 +503,8 @@ int batadv_frag_send_packet(struct sk_bu goto out; }
- 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;
Patches currently in stable-queue which might be from sven@narfation.org are
queue-4.9/batman-adv-consider-fragmentation-for-needed_headroom.patch queue-4.9/ath9k-fix-potential-interrupt-storm-on-queue-reset.patch queue-4.9/batman-adv-mcast-fix-duplicate-mcast-packets-from-bla-backbone-to-mesh.patch queue-4.9/ath10k-fix-max-antenna-gain-unit.patch queue-4.9/batman-adv-mcast-fix-duplicate-mcast-packets-in-bla-backbone-from-lan.patch queue-4.9/batman-adv-reserve-needed_-room-for-fragments.patch queue-4.9/batman-adv-fix-own-ogm-check-in-aggregated-ogms.patch queue-4.9/batman-adv-keep-fragments-equally-sized.patch queue-4.9/batman-adv-don-t-always-reallocate-the-fragmentation-skb-head.patch
From: Linus Lüssing linus.luessing@c0d3.blue
commit d8bf0c01642275c7dca1e5d02c34e4199c200b1f upstream.
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 linus.luessing@c0d3.blue Signed-off-by: Simon Wunderlich sw@simonwunderlich.de [ bp: 4.9 backported: adjust context, correct fixes line ] Signed-off-by: Sven Eckelmann sven@narfation.org --- net/batman-adv/bat_v_ogm.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/net/batman-adv/bat_v_ogm.c b/net/batman-adv/bat_v_ogm.c index d2e6885479cb..cb94ac988618 100644 --- a/net/batman-adv/bat_v_ogm.c +++ b/net/batman-adv/bat_v_ogm.c @@ -690,6 +690,12 @@ static void batadv_v_ogm_process(const struct sk_buff *skb, int ogm_offset, ntohl(ogm_packet->seqno), ogm_throughput, ogm_packet->ttl, ogm_packet->version, ntohs(ogm_packet->tvlv_len));
+ if (batadv_is_my_mac(bat_priv, ogm_packet->orig)) { + batadv_dbg(BATADV_DBG_BATMAN, bat_priv, + "Drop packet: originator packet from ourself\n"); + return; + } + /* If the troughput metric is 0, immediately drop the packet. No need to * create orig_node / neigh_node for an unusable route. */ @@ -788,11 +794,6 @@ int batadv_v_ogm_packet_recv(struct sk_buff *skb, if (batadv_is_my_mac(bat_priv, ethhdr->h_source)) return NET_RX_DROP;
- ogm_packet = (struct batadv_ogm2_packet *)skb->data; - - if (batadv_is_my_mac(bat_priv, ogm_packet->orig)) - return NET_RX_DROP; - batadv_inc_counter(bat_priv, BATADV_CNT_MGMT_RX); batadv_add_counter(bat_priv, BATADV_CNT_MGMT_RX_BYTES, skb->len + ETH_HLEN);
This is a note to let you know that I've just added the patch titled
batman-adv: Fix own OGM check in aggregated OGMs
to the 4.9-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summ...
The filename of the patch is: batman-adv-fix-own-ogm-check-in-aggregated-ogms.patch and it can be found in the queue-4.9 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree, please let stable@vger.kernel.org know about it.
From foo@baz Tue Nov 23 01:37:10 PM CET 2021 From: Sven Eckelmann sven@narfation.org Date: Sat, 20 Nov 2021 13:39:53 +0100 Subject: batman-adv: Fix own OGM check in aggregated OGMs To: stable@vger.kernel.org Cc: b.a.t.m.a.n@lists.open-mesh.org, "Linus Lüssing" linus.luessing@c0d3.blue, "Simon Wunderlich" sw@simonwunderlich.de, "Sven Eckelmann" sven@narfation.org Message-ID: 20211120123958.260826-3-sven@narfation.org
From: Linus Lüssing linus.luessing@c0d3.blue
commit d8bf0c01642275c7dca1e5d02c34e4199c200b1f upstream.
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 linus.luessing@c0d3.blue Signed-off-by: Simon Wunderlich sw@simonwunderlich.de [ bp: 4.9 backported: adjust context, correct fixes line ] Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org --- net/batman-adv/bat_v_ogm.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
--- a/net/batman-adv/bat_v_ogm.c +++ b/net/batman-adv/bat_v_ogm.c @@ -690,6 +690,12 @@ static void batadv_v_ogm_process(const s ntohl(ogm_packet->seqno), ogm_throughput, ogm_packet->ttl, ogm_packet->version, ntohs(ogm_packet->tvlv_len));
+ if (batadv_is_my_mac(bat_priv, ogm_packet->orig)) { + batadv_dbg(BATADV_DBG_BATMAN, bat_priv, + "Drop packet: originator packet from ourself\n"); + return; + } + /* If the troughput metric is 0, immediately drop the packet. No need to * create orig_node / neigh_node for an unusable route. */ @@ -788,11 +794,6 @@ int batadv_v_ogm_packet_recv(struct sk_b if (batadv_is_my_mac(bat_priv, ethhdr->h_source)) return NET_RX_DROP;
- ogm_packet = (struct batadv_ogm2_packet *)skb->data; - - if (batadv_is_my_mac(bat_priv, ogm_packet->orig)) - return NET_RX_DROP; - batadv_inc_counter(bat_priv, BATADV_CNT_MGMT_RX); batadv_add_counter(bat_priv, BATADV_CNT_MGMT_RX_BYTES, skb->len + ETH_HLEN);
Patches currently in stable-queue which might be from sven@narfation.org are
queue-4.9/batman-adv-consider-fragmentation-for-needed_headroom.patch queue-4.9/ath9k-fix-potential-interrupt-storm-on-queue-reset.patch queue-4.9/batman-adv-mcast-fix-duplicate-mcast-packets-from-bla-backbone-to-mesh.patch queue-4.9/ath10k-fix-max-antenna-gain-unit.patch queue-4.9/batman-adv-mcast-fix-duplicate-mcast-packets-in-bla-backbone-from-lan.patch queue-4.9/batman-adv-reserve-needed_-room-for-fragments.patch queue-4.9/batman-adv-fix-own-ogm-check-in-aggregated-ogms.patch queue-4.9/batman-adv-keep-fragments-equally-sized.patch queue-4.9/batman-adv-don-t-always-reallocate-the-fragmentation-skb-head.patch
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: fe2da6ff27c7 ("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.9 backport: drop usage in non-existing batadv_mcast_forw_*. ] 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 5a2aac17805b..283ac3fb9429 100644 --- a/net/batman-adv/multicast.c +++ b/net/batman-adv/multicast.c @@ -53,10 +53,12 @@ #include <net/ip.h> #include <net/ipv6.h>
+#include "bridge_loop_avoidance.h" #include "hard-interface.h" #include "hash.h" #include "log.h" #include "packet.h" +#include "send.h" #include "translation-table.h" #include "tvlv.h"
@@ -1251,6 +1253,35 @@ void batadv_mcast_free(struct batadv_priv *bat_priv) spin_unlock_bh(&bat_priv->tt.commit_lock); }
+/** + * 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 1fb00ba84907..751f54756164 100644 --- a/net/batman-adv/multicast.h +++ b/net/batman-adv/multicast.h @@ -45,6 +45,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);
int batadv_mcast_flags_seq_print_text(struct seq_file *seq, void *offset); @@ -71,6 +76,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 af0a8439cf08..4805c98588dc 100644 --- a/net/batman-adv/soft-interface.c +++ b/net/batman-adv/soft-interface.c @@ -356,9 +356,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))
This is a note to let you know that I've just added the patch titled
batman-adv: mcast: fix duplicate mcast packets in BLA backbone from LAN
to the 4.9-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summ...
The filename of the patch is: batman-adv-mcast-fix-duplicate-mcast-packets-in-bla-backbone-from-lan.patch and it can be found in the queue-4.9 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree, please let stable@vger.kernel.org know about it.
From foo@baz Tue Nov 23 01:37:10 PM CET 2021 From: Sven Eckelmann sven@narfation.org Date: Sat, 20 Nov 2021 13:39:54 +0100 Subject: batman-adv: mcast: fix duplicate mcast packets in BLA backbone from LAN To: stable@vger.kernel.org Cc: b.a.t.m.a.n@lists.open-mesh.org, "Linus Lüssing" linus.luessing@c0d3.blue, "Simon Wunderlich" sw@simonwunderlich.de, "Sven Eckelmann" sven@narfation.org Message-ID: 20211120123958.260826-4-sven@narfation.org
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: fe2da6ff27c7 ("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.9 backport: drop usage in non-existing batadv_mcast_forw_*. ] Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.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(-)
--- a/net/batman-adv/multicast.c +++ b/net/batman-adv/multicast.c @@ -53,10 +53,12 @@ #include <net/ip.h> #include <net/ipv6.h>
+#include "bridge_loop_avoidance.h" #include "hard-interface.h" #include "hash.h" #include "log.h" #include "packet.h" +#include "send.h" #include "translation-table.h" #include "tvlv.h"
@@ -1252,6 +1254,35 @@ void batadv_mcast_free(struct batadv_pri }
/** + * 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 */ --- a/net/batman-adv/multicast.h +++ b/net/batman-adv/multicast.h @@ -45,6 +45,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);
int batadv_mcast_flags_seq_print_text(struct seq_file *seq, void *offset); @@ -71,6 +76,16 @@ static inline int batadv_mcast_init(stru 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) { } --- a/net/batman-adv/soft-interface.c +++ b/net/batman-adv/soft-interface.c @@ -356,9 +356,8 @@ send: 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))
Patches currently in stable-queue which might be from sven@narfation.org are
queue-4.9/batman-adv-consider-fragmentation-for-needed_headroom.patch queue-4.9/ath9k-fix-potential-interrupt-storm-on-queue-reset.patch queue-4.9/batman-adv-mcast-fix-duplicate-mcast-packets-from-bla-backbone-to-mesh.patch queue-4.9/ath10k-fix-max-antenna-gain-unit.patch queue-4.9/batman-adv-mcast-fix-duplicate-mcast-packets-in-bla-backbone-from-lan.patch queue-4.9/batman-adv-reserve-needed_-room-for-fragments.patch queue-4.9/batman-adv-fix-own-ogm-check-in-aggregated-ogms.patch queue-4.9/batman-adv-keep-fragments-equally-sized.patch queue-4.9/batman-adv-don-t-always-reallocate-the-fragmentation-skb-head.patch
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: e5cf86d30a9b ("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.9 backported: adjust context, correct fixes line ] Signed-off-by: Sven Eckelmann sven@narfation.org --- net/batman-adv/bridge_loop_avoidance.c | 103 +++++++++++++++++++++---- 1 file changed, 87 insertions(+), 16 deletions(-)
diff --git a/net/batman-adv/bridge_loop_avoidance.c b/net/batman-adv/bridge_loop_avoidance.c index dc635bd93584..a087f559f93e 100644 --- a/net/batman-adv/bridge_loop_avoidance.c +++ b/net/batman-adv/bridge_loop_avoidance.c @@ -1593,13 +1593,16 @@ int batadv_bla_init(struct batadv_priv *bat_priv) }
/** - * batadv_bla_check_bcast_duplist - Check if a frame is in the broadcast dup. + * 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 @@ -1608,19 +1611,17 @@ int batadv_bla_init(struct batadv_priv *bat_priv) * * Return: true if a packet is in the duplicate list, false otherwise. */ -bool batadv_bla_check_bcast_duplist(struct batadv_priv *bat_priv, - struct sk_buff *skb) +static bool batadv_bla_check_duplist(struct batadv_priv *bat_priv, + struct sk_buff *skb, u8 *payload_ptr, + const u8 *orig) { - int i, curr; - __be32 crc; - struct batadv_bcast_packet *bcast_packet; struct batadv_bcast_duplist_entry *entry; bool ret = false; - - bcast_packet = (struct batadv_bcast_packet *)skb->data; + int i, curr; + __be32 crc;
/* 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);
@@ -1639,8 +1640,21 @@ bool 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 true to forbid it. @@ -1656,7 +1670,14 @@ bool 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: @@ -1665,6 +1686,48 @@ bool 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. + */ +bool 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 - Check if the originator is a gateway for * the VLAN identified by vid. @@ -1879,6 +1942,14 @@ bool 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);
This is a note to let you know that I've just added the patch titled
batman-adv: mcast: fix duplicate mcast packets from BLA backbone to mesh
to the 4.9-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summ...
The filename of the patch is: batman-adv-mcast-fix-duplicate-mcast-packets-from-bla-backbone-to-mesh.patch and it can be found in the queue-4.9 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree, please let stable@vger.kernel.org know about it.
From foo@baz Tue Nov 23 01:37:10 PM CET 2021 From: Sven Eckelmann sven@narfation.org Date: Sat, 20 Nov 2021 13:39:55 +0100 Subject: batman-adv: mcast: fix duplicate mcast packets from BLA backbone to mesh To: stable@vger.kernel.org Cc: b.a.t.m.a.n@lists.open-mesh.org, "Linus Lüssing" linus.luessing@c0d3.blue, "Simon Wunderlich" sw@simonwunderlich.de, "Sven Eckelmann" sven@narfation.org Message-ID: 20211120123958.260826-5-sven@narfation.org
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: e5cf86d30a9b ("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.9 backported: adjust context, correct fixes line ] Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org --- net/batman-adv/bridge_loop_avoidance.c | 105 +++++++++++++++++++++++++++------ 1 file changed, 88 insertions(+), 17 deletions(-)
--- a/net/batman-adv/bridge_loop_avoidance.c +++ b/net/batman-adv/bridge_loop_avoidance.c @@ -1593,13 +1593,16 @@ int batadv_bla_init(struct batadv_priv * }
/** - * batadv_bla_check_bcast_duplist - Check if a frame is in the broadcast dup. + * 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 - * - * 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. + * @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. * * 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 @@ -1608,19 +1611,17 @@ int batadv_bla_init(struct batadv_priv * * * Return: true if a packet is in the duplicate list, false otherwise. */ -bool batadv_bla_check_bcast_duplist(struct batadv_priv *bat_priv, - struct sk_buff *skb) +static bool batadv_bla_check_duplist(struct batadv_priv *bat_priv, + struct sk_buff *skb, u8 *payload_ptr, + const u8 *orig) { - int i, curr; - __be32 crc; - struct batadv_bcast_packet *bcast_packet; struct batadv_bcast_duplist_entry *entry; bool ret = false; - - bcast_packet = (struct batadv_bcast_packet *)skb->data; + int i, curr; + __be32 crc;
/* 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);
@@ -1639,8 +1640,21 @@ bool batadv_bla_check_bcast_duplist(stru 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 true to forbid it. @@ -1656,7 +1670,14 @@ bool batadv_bla_check_bcast_duplist(stru 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: @@ -1666,6 +1687,48 @@ out: }
/** + * 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. + */ +bool 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 - Check if the originator is a gateway for * the VLAN identified by vid. * @bat_priv: the bat priv with all the soft interface information @@ -1879,6 +1942,14 @@ bool batadv_bla_rx(struct batadv_priv *b 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);
Patches currently in stable-queue which might be from sven@narfation.org are
queue-4.9/batman-adv-consider-fragmentation-for-needed_headroom.patch queue-4.9/ath9k-fix-potential-interrupt-storm-on-queue-reset.patch queue-4.9/batman-adv-mcast-fix-duplicate-mcast-packets-from-bla-backbone-to-mesh.patch queue-4.9/ath10k-fix-max-antenna-gain-unit.patch queue-4.9/batman-adv-mcast-fix-duplicate-mcast-packets-in-bla-backbone-from-lan.patch queue-4.9/batman-adv-reserve-needed_-room-for-fragments.patch queue-4.9/batman-adv-fix-own-ogm-check-in-aggregated-ogms.patch queue-4.9/batman-adv-keep-fragments-equally-sized.patch queue-4.9/batman-adv-don-t-always-reallocate-the-fragmentation-skb-head.patch
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: Sven Eckelmann sven@narfation.org 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 f528761674df..4f384abb4ced 100644 --- a/net/batman-adv/hard-interface.c +++ b/net/batman-adv/hard-interface.c @@ -359,6 +359,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; }
This is a note to let you know that I've just added the patch titled
batman-adv: Consider fragmentation for needed_headroom
to the 4.9-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summ...
The filename of the patch is: batman-adv-consider-fragmentation-for-needed_headroom.patch and it can be found in the queue-4.9 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree, please let stable@vger.kernel.org know about it.
From foo@baz Tue Nov 23 01:37:10 PM CET 2021 From: Sven Eckelmann sven@narfation.org Date: Sat, 20 Nov 2021 13:39:56 +0100 Subject: batman-adv: Consider fragmentation for needed_headroom To: stable@vger.kernel.org Cc: b.a.t.m.a.n@lists.open-mesh.org, "Sven Eckelmann" sven@narfation.org, "Linus Lüssing" linus.luessing@c0d3.blue, "Simon Wunderlich" sw@simonwunderlich.de Message-ID: 20211120123958.260826-6-sven@narfation.org
From: Sven Eckelmann sven@narfation.org
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: Sven Eckelmann sven@narfation.org Signed-off-by: Simon Wunderlich sw@simonwunderlich.de Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org --- net/batman-adv/hard-interface.c | 3 +++ 1 file changed, 3 insertions(+)
--- a/net/batman-adv/hard-interface.c +++ b/net/batman-adv/hard-interface.c @@ -359,6 +359,9 @@ static void batadv_hardif_recalc_extra_s 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; }
Patches currently in stable-queue which might be from sven@narfation.org are
queue-4.9/batman-adv-consider-fragmentation-for-needed_headroom.patch queue-4.9/ath9k-fix-potential-interrupt-storm-on-queue-reset.patch queue-4.9/batman-adv-mcast-fix-duplicate-mcast-packets-from-bla-backbone-to-mesh.patch queue-4.9/ath10k-fix-max-antenna-gain-unit.patch queue-4.9/batman-adv-mcast-fix-duplicate-mcast-packets-in-bla-backbone-from-lan.patch queue-4.9/batman-adv-reserve-needed_-room-for-fragments.patch queue-4.9/batman-adv-fix-own-ogm-check-in-aggregated-ogms.patch queue-4.9/batman-adv-keep-fragments-equally-sized.patch queue-4.9/batman-adv-don-t-always-reallocate-the-fragmentation-skb-head.patch
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.9 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 b36606816bac..353772eb4f4d 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, * * Return: 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 int header_size = sizeof(*frag_head); unsigned int 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_fragment->priority = skb->priority;
/* 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 */ @@ -443,11 +447,12 @@ int 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 int mtu = neigh_node->if_incoming->net_dev->mtu; + unsigned int mtu = net_dev->mtu; unsigned int header_size = sizeof(frag_header); unsigned int max_fragment_size, num_fragments; int ret = -1; @@ -503,7 +508,7 @@ int batadv_frag_send_packet(struct sk_buff *skb, goto out; }
- 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;
This is a note to let you know that I've just added the patch titled
batman-adv: Reserve needed_*room for fragments
to the 4.9-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summ...
The filename of the patch is: batman-adv-reserve-needed_-room-for-fragments.patch and it can be found in the queue-4.9 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree, please let stable@vger.kernel.org know about it.
From foo@baz Tue Nov 23 01:37:10 PM CET 2021 From: Sven Eckelmann sven@narfation.org Date: Sat, 20 Nov 2021 13:39:57 +0100 Subject: batman-adv: Reserve needed_*room for fragments To: stable@vger.kernel.org Cc: b.a.t.m.a.n@lists.open-mesh.org, Sven Eckelmann sven@narfation.org, Simon Wunderlich sw@simonwunderlich.de Message-ID: 20211120123958.260826-7-sven@narfation.org
From: Sven Eckelmann sven@narfation.org
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.9 backported: adjust context. ] Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org --- net/batman-adv/fragmentation.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)
--- a/net/batman-adv/fragmentation.c +++ b/net/batman-adv/fragmentation.c @@ -394,6 +394,7 @@ out:
/** * 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 @@ out: * * Return: 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 int header_size = sizeof(*frag_head); unsigned int 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_fragment->priority = skb->priority;
/* 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 */ @@ -443,11 +447,12 @@ int batadv_frag_send_packet(struct sk_bu 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 int mtu = neigh_node->if_incoming->net_dev->mtu; + unsigned int mtu = net_dev->mtu; unsigned int header_size = sizeof(frag_header); unsigned int max_fragment_size, num_fragments; int ret = -1; @@ -503,7 +508,7 @@ int batadv_frag_send_packet(struct sk_bu goto out; }
- 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;
Patches currently in stable-queue which might be from sven@narfation.org are
queue-4.9/batman-adv-consider-fragmentation-for-needed_headroom.patch queue-4.9/ath9k-fix-potential-interrupt-storm-on-queue-reset.patch queue-4.9/batman-adv-mcast-fix-duplicate-mcast-packets-from-bla-backbone-to-mesh.patch queue-4.9/ath10k-fix-max-antenna-gain-unit.patch queue-4.9/batman-adv-mcast-fix-duplicate-mcast-packets-in-bla-backbone-from-lan.patch queue-4.9/batman-adv-reserve-needed_-room-for-fragments.patch queue-4.9/batman-adv-fix-own-ogm-check-in-aggregated-ogms.patch queue-4.9/batman-adv-keep-fragments-equally-sized.patch queue-4.9/batman-adv-don-t-always-reallocate-the-fragmentation-skb-head.patch
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.9 backported: adjust context. ] Signed-off-by: Sven Eckelmann sven@narfation.org --- net/batman-adv/fragmentation.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/net/batman-adv/fragmentation.c b/net/batman-adv/fragmentation.c index 353772eb4f4d..343f4fc5909d 100644 --- a/net/batman-adv/fragmentation.c +++ b/net/batman-adv/fragmentation.c @@ -528,11 +528,14 @@ int 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 + */ + ret = skb_cow_head(skb, ETH_HLEN + header_size); + if (ret < 0) goto out;
+ skb_push(skb, header_size); memcpy(skb->data, &frag_header, header_size);
/* Send the last fragment */
This is a note to let you know that I've just added the patch titled
batman-adv: Don't always reallocate the fragmentation skb head
to the 4.9-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summ...
The filename of the patch is: batman-adv-don-t-always-reallocate-the-fragmentation-skb-head.patch and it can be found in the queue-4.9 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree, please let stable@vger.kernel.org know about it.
From foo@baz Tue Nov 23 01:37:10 PM CET 2021 From: Sven Eckelmann sven@narfation.org Date: Sat, 20 Nov 2021 13:39:58 +0100 Subject: batman-adv: Don't always reallocate the fragmentation skb head To: stable@vger.kernel.org Cc: b.a.t.m.a.n@lists.open-mesh.org, Sven Eckelmann sven@narfation.org, Simon Wunderlich sw@simonwunderlich.de Message-ID: 20211120123958.260826-8-sven@narfation.org
From: Sven Eckelmann sven@narfation.org
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.9 backported: adjust context. ] Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org --- net/batman-adv/fragmentation.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
--- a/net/batman-adv/fragmentation.c +++ b/net/batman-adv/fragmentation.c @@ -528,11 +528,14 @@ int batadv_frag_send_packet(struct sk_bu 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 + */ + ret = skb_cow_head(skb, ETH_HLEN + header_size); + if (ret < 0) goto out;
+ skb_push(skb, header_size); memcpy(skb->data, &frag_header, header_size);
/* Send the last fragment */
Patches currently in stable-queue which might be from sven@narfation.org are
queue-4.9/batman-adv-consider-fragmentation-for-needed_headroom.patch queue-4.9/ath9k-fix-potential-interrupt-storm-on-queue-reset.patch queue-4.9/batman-adv-mcast-fix-duplicate-mcast-packets-from-bla-backbone-to-mesh.patch queue-4.9/ath10k-fix-max-antenna-gain-unit.patch queue-4.9/batman-adv-mcast-fix-duplicate-mcast-packets-in-bla-backbone-from-lan.patch queue-4.9/batman-adv-reserve-needed_-room-for-fragments.patch queue-4.9/batman-adv-fix-own-ogm-check-in-aggregated-ogms.patch queue-4.9/batman-adv-keep-fragments-equally-sized.patch queue-4.9/batman-adv-don-t-always-reallocate-the-fragmentation-skb-head.patch
b.a.t.m.a.n@lists.open-mesh.org
-
gregkh＠linuxfoundation.org -
Sven Eckelmann