Hi Martin, hi Sven, hi all
Thank you very much for analysing, describing and hacking on our issue. I’m sorry to ruin your other thread Sven ;-)
You are asking about our set-up in other threads. To save you from reading the commands I posted for Martin, let me make a quick draw:
This bug posted here: _________ User | NAT | |------------| on ===================> | VM | <=================> | Mesh-Nodes |<--AP |_______| |____________| MTU ipip-Tunnel to our VM NAT bat15 over fastd-Tunnel ^ 1500 which is connected to 1:n to meshing nodes | NC-disabled our ISP | bat15-Link to MTU 1400 is set to default fastd MTU 1426 | other nodes reproduce the bug of | wrong MTU 1528 the other VMs | [from bat14] no batman involved in |(will be changed this tunnel | to 1560 with | next firmware)
On the other VMs, I can't analyse the bug, there is a GRE-tunnel to ISPs routers with fixed MTU of 1400. Over these tunnels there are no batman packages sent through. Only IPv4 and IPv6. The VM do the NAT to a public IP.
I’m sorry making you such a mess with the MTU. Nowadays many communitys do an outsourcing of the gateways to the internet. They provide more throughput and a coherent batman cloud if there are white spaces without nodes on the map.
I’d like to inform you that I implement the patch posted with this mail 18 h ago. It is a mix of the patch Martin gave me earlier and your 1st patch from “Calculate extra tail size based on queued fragments”. There was no crash, but this means nothing. Now I see, there are many, many patches which solves the bug with different approaches. Please tell me exactly which one I should test because I don't speak any C.
Best regards and happy hacking
Philipp
________________________ Freifunk Rheinland e. V. – Funkzelle Wuppertal –
# batctl -v batctl 2014.3.0 [batman-adv: 2014.3.0-44-g650251a-dirty]
diff --git a/fragmentation.c b/fragmentation.c index 362e91a..743d0d3 100644 --- a/fragmentation.c +++ b/fragmentation.c @@ -217,6 +217,18 @@ err: return ret; }
+static inline void batadv_frag_dbg_entry(struct batadv_frag_list_entry *entry) +{ + struct sk_buff *skb = entry->skb; + struct batadv_frag_packet *packet; + + packet = (struct batadv_frag_packet *)skb->data; + + printk(KERN_DEBUG " skb->len: %u, skb->tailroom: %u, pkt->pkt_type: %hhu, pkt->version: %hhu, pkt->no: %hhu, pkt->seqno: %hu, pkt->total_size: %hu\n", + skb->len, skb_tailroom(skb), packet->packet_type, packet->version, + packet->no, ntohs(packet->seqno), ntohs(packet->total_size)); +} + /** * batadv_frag_merge_packets - merge a chain of fragments * @chain: head of chain with fragments @@ -228,18 +240,14 @@ err: * Returns the merged skb or NULL on error. */ static struct sk_buff * -batadv_frag_merge_packets(struct hlist_head *chain, struct sk_buff *skb) +batadv_frag_merge_packets(struct hlist_head *chain) { struct batadv_frag_packet *packet; - struct batadv_frag_list_entry *entry; + struct batadv_frag_list_entry *entry, dbg_entry; + struct batadv_frag_table_entry *table_entry; struct sk_buff *skb_out = NULL; - int size, hdr_size = sizeof(struct batadv_frag_packet); - - /* Make sure incoming skb has non-bogus data. */ - packet = (struct batadv_frag_packet *)skb->data; - size = ntohs(packet->total_size); - if (size > batadv_frag_size_limit()) - goto free; + int size, hdr_size = sizeof(struct batadv_frag_packet), i = 0; + int extra_tail;
/* Remove first entry, as this is the destination for the rest of the * fragments. @@ -247,13 +255,26 @@ batadv_frag_merge_packets(struct hlist_head *chain, struct sk_buff *skb) entry = hlist_entry(chain->first, struct batadv_frag_list_entry, list); hlist_del(&entry->list); skb_out = entry->skb; + memcpy(&dbg_entry, entry, sizeof(dbg_entry)); kfree(entry);
+// if (size < skb->len) +// goto debug; +// +// if (size < skb_out->len) +// goto debug; +// + packet = (struct batadv_frag_packet *)skb_out->data; + size = ntohs(packet->total_size); + /* Make room for the rest of the fragments. */ - if (pskb_expand_head(skb_out, 0, size - skb->len, GFP_ATOMIC) < 0) { - kfree_skb(skb_out); - skb_out = NULL; - goto free; + if (size > skb_out->len) { + extra_tail = size - skb_out->len; + if (pskb_expand_head(skb_out, 0, extra_tail, GFP_ATOMIC) < 0) { + kfree_skb(skb_out); + skb_out = NULL; + goto free; + } }
/* Move the existing MAC header to just before the payload. (Override @@ -268,6 +289,11 @@ batadv_frag_merge_packets(struct hlist_head *chain, struct sk_buff *skb) /* Copy the payload of the each fragment into the last skb */ hlist_for_each_entry(entry, chain, list) { size = entry->skb->len - hdr_size; + i++; + + if (skb_tailroom(skb_out) < size) + goto debug; + memcpy(skb_put(skb_out, size), entry->skb->data + hdr_size, size); } @@ -276,6 +302,19 @@ free: /* Locking is not needed, because 'chain' is not part of any orig. */ batadv_frag_clear_chain(chain); return skb_out; + +debug: + table_entry = container_of(chain, struct batadv_frag_table_entry, head); + printk(KERN_DEBUG "batadv_frag_merge_packets: i: %i, size: %i, entry->seqno: %hu, entry->size: %hu, entry->total_size: %hu\n", + i, size, table_entry->seqno, table_entry->size, + table_entry->total_size); + batadv_frag_dbg_entry(&dbg_entry); + + hlist_for_each_entry(entry, chain, list) + batadv_frag_dbg_entry(entry); + + batadv_frag_clear_chain(chain); + return NULL; }
/** @@ -304,7 +343,7 @@ bool batadv_frag_skb_buffer(struct sk_buff **skb, if (hlist_empty(&head)) goto out;
- skb_out = batadv_frag_merge_packets(&head, *skb); + skb_out = batadv_frag_merge_packets(&head); if (!skb_out) goto out_err;
diff --git a/types.h b/types.h index 462a70c..c4d7d24 100644 --- a/types.h +++ b/types.h @@ -132,6 +132,7 @@ struct batadv_orig_ifinfo { * @timestamp: time (jiffie) of last received fragment * @seqno: sequence number of the fragments in the list * @size: accumulated size of packets in list + * @total_size: expected size of the assembled packet */ struct batadv_frag_table_entry { struct hlist_head head; @@ -139,6 +140,7 @@ struct batadv_frag_table_entry { unsigned long timestamp; uint16_t seqno; uint16_t size; + uint16_t total_size; };
/**