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;
};
/**
signature.asc
Description: This is a digitally signed message part
