Bug #252
openTT: size check before local entry add is incorrect (not threadsafe?)
0%
Description
Just tested TT with the Emulation_Debug environment. Two nodes were enabled and I've just send 3000 packets (different mac addresses) with the attached program to the other node. Then I can see that the remote node sends TT full table requests. But the node which send the 3000 packets never sends the response. The problem seems to be that the tvlv length is 31616 bytes (tvlv_len in batadv_tt_prepare_tvlv_local_data) but this is larger than the maximum packet_size (bat_priv->packet_size_max).
If you print the size check in batadv_tt_local_add right before the if (table_size > packet_size_max) { then you can see that the transmission size jumps (each "foobar" is an add to the local table):
diff --git i/net/batman-adv/translation-table.c w/net/batman-adv/translation-table.c
index e8ec2b6c..1e37e210 100644
--- i/net/batman-adv/translation-table.c
+++ w/net/batman-adv/translation-table.c
@@ -849,6 +849,7 @@ batadv_tt_local_create(struct net_device *mesh_iface, const u8 *addr,
table_size = batadv_tt_local_table_transmit_size(bat_priv);
table_size += batadv_tt_len(1);
packet_size_max = READ_ONCE(bat_priv->packet_size_max);
+ printk("foobar %d %d\n", table_size, packet_size_max);
if (table_size > packet_size_max) {
net_ratelimited_function(batadv_info, mesh_iface,
"Local translation table size (%i) exceeds maximum packet size (%i); Ignoring new local tt entry: %pM\n",
foobar 116 22080 foobar 116 22080 foobar 116 22080 foobar 116 22080 foobar 116 22080 foobar 116 22080 foobar 116 22080 foobar 116 22080 foobar 116 22080 foobar 116 22080 foobar 116 22080 foobar 116 22080 foobar 116 22080 foobar 116 22080 foobar 116 22080 foobar 116 22080 foobar 116 22080 foobar 116 22080 foobar 116 22080 foobar 116 22080 foobar 116 22080 foobar 116 22080 foobar 116 22080 foobar 116 22080 foobar 11156 22080 foobar 11156 22080 foobar 11156 22080 foobar 11156 22080 foobar 11156 22080 foobar 11156 22080 foobar 11156 22080 foobar 11156 22080 foobar 11156 22080 foobar 11156 22080 foobar 11156 22080 foobar 11156 22080 foobar 11156 22080 .... foobar 31652 22080 foobar 31652 22080 foobar 31652 22080 foobar 31652 22080 foobar 31652 22080 foobar 31652 22080 foobar 31652 22080 foobar 31652 22080
The test was started on my node1 via
insmod /host/batman-adv/net/batman-adv/batman-adv.ko /host/batctl/batctl ra BATMAN_IV /host/batctl/batctl if add eth0 ifconfig eth0 up ifconfig bat0 up # sleep 3 /host/batctl/batctl o /host/rawsend_massive bat0 02:ba:de:af:fe:02
Files
Updated by Antonio Quartulli over 10 years ago
- Status changed from New to Feedback
- Assignee changed from Antonio Quartulli to Sven Eckelmann
Sven,
what are you actually showing in your debug output ? If you are printing table_size and packet_size_max I don't understand when you see the table_size getting larger than packet_size_max.
Another question: are you saying that the "jump" looks suspicious to you ?
Updated by Sven Eckelmann over 10 years ago
- Assignee changed from Sven Eckelmann to Antonio Quartulli
Yes, the jump looks suspicious to me.
The elements printed are table_size and packet_size_max
Updated by Antonio Quartulli over 10 years ago
In my opinion the "jump" is due to the TT commit. When a bunch of local entries are detected by a node they are marked with the N(ew) flag. Such entries are not yet part of the full table (although they are stored in the hash table) and therefore they are not sent when a TT request comes in.
As soon as a TT local commit event is triggered (usually together with the next local OGM) all the N(ew) entries are "committed" and thus the table size is increased (the N flag disappears at this point).
Still, I don't understand why the node would not reply with a TT response since the maximum size seems to always be smaller than the packet_size_max...
Updated by Sven Eckelmann over 10 years ago
No, it isn't always smaller. I just forgot to post the relevant log at the end of the run:
batadv_tt_local_add:690 31652 22080 batadv_tt_local_add:690 31652 22080 batadv_tt_local_add:690 31652 22080 batadv_tt_local_add:690 31652 22080 batadv_tt_local_add:690 31652 22080 batadv_tt_local_add:690 31652 22080 batadv_tt_local_add:690 31652 22080 batadv_tt_local_add:690 31652 22080
The local table will also not pruned (for some reason I haven't checked).
Updated by Antonio Quartulli over 10 years ago
oh ok, thanks for adding the missing part. Yes in this case it makes sense.
The timeout for a local entry is defined as
main.h:46 #define BATADV_TT_LOCAL_TIMEOUT 600000 /* in milliseconds */
Updated by Sven Eckelmann about 1 month ago
This is still a problem. Tested with three nodes and started on one node:
/host/rawsend_massive bat0 ff:ff:ff:ff:ff:ff
the other nodes started to send "Full table" requests towards the first node but never received an answer.
Updated by Sven Eckelmann about 1 month ago
The packet it wants to send out for a TT size of 26708 (table_size - 20160 is currently my packet_size_max) is 26696 bytes. I must therefore go through batadv_frag_send_packet. It calculates that it needs 22 (num_fragments) fragments but the maximum is 16 (BATADV_FRAG_MAX_FRAGMENTS).
As result, no answer is send to the requester of the full table. If it would actually be limited to 20160 (packet_size_max) - or 20148 in skb->len, it would be 16 fragments with 1260 bytes each - which would work.
Updated by Sven Eckelmann about 1 month ago
To fix it, it is necessary to have some spinlocked code (protecting some counters in bat_priv) which runs before the actual allocation of VLAN (batadv_meshif_create_vlan) or local TT entry (batadv_tt_local_create):
- before VLAN creation: directly increase the counter of reserved VLANs
- needs to be removed when allocation fails (either because of an already existing problem or because the packet size would have been exceeded)
- on local TT creation: directly increase the counter of reserved local TTs
- needs to be removed when allocation fails (either because of an already existing problem or because the packet size would have been exceeded)
After pre-reserving the VLAN/TT, it must be checked if it would be over budget or not. Not using the rcu loops and atomic reads from batadv_tt_local_table_transmit_size but using the pre-reservation numbers. If it would, the actual VLAN/TT will not be created and the pre-reserved VLAN/TT is returned.
On batadv_meshif_vlan_release and batadv_tt_local_entry_release, these counters must also be reduced again.
Updated by Sven Eckelmann about 1 month ago
- File 0001-TODO-batman-adv-tt-reject-VLAN-TT-entries-before-rea.patch 0001-TODO-batman-adv-tt-reject-VLAN-TT-entries-before-rea.patch added
Attached is a PoC implementation of the described strategy from #252#note-9. I hope it helps to get this problem solved after over 10 years
Updated by Sven Eckelmann 11 days ago
- Status changed from Feedback to In Progress
- Assignee changed from Antonio Quartulli to Simon Wunderlich
- Target version set to 2026.04