From patchwork Mon Dec 1 18:01:37 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Philipp Psurek X-Patchwork-Id: 4257 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=74.125.82.44; helo=mail-wg0-f44.google.com; envelope-from=philipp.psurek@gmail.com; receiver=b.a.t.m.a.n@lists.open-mesh.org Received: from mail-wg0-f44.google.com (mail-wg0-f44.google.com [74.125.82.44]) by open-mesh.org (Postfix) with ESMTPS id 611276014C3 for ; Mon, 1 Dec 2014 19:01:51 +0100 (CET) Received: by mail-wg0-f44.google.com with SMTP id b13so14910980wgh.3 for ; Mon, 01 Dec 2014 10:01:51 -0800 (PST) X-Received: by 10.180.208.112 with SMTP id md16mr83820207wic.37.1417456909550; Mon, 01 Dec 2014 10:01:49 -0800 (PST) Received: from heizung.hausnetz (e181169242.adsl.alicedsl.de. [85.181.169.242]) by mx.google.com with ESMTPSA id wx3sm28556799wjc.19.2014.12.01.10.01.48 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Mon, 01 Dec 2014 10:01:48 -0800 (PST) Message-ID: <1417456897.2788.2.camel@gmail.com> From: Philipp Psurek To: batman-ml Date: Mon, 01 Dec 2014 19:01:37 +0100 In-Reply-To: <1417388720-5019-1-git-send-email-sven@narfation.org> References: <1417388720-5019-1-git-send-email-sven@narfation.org> X-Mailer: Evolution 3.12.4 Mime-Version: 1.0 Cc: martin@hundeboll.net, sven@narfation.org Subject: Re: [B.A.T.M.A.N.] [RFC] batman-adv: Calculate extra tail size based on queued fragments X-BeenThere: b.a.t.m.a.n@lists.open-mesh.org X-Mailman-Version: 2.1.15 Precedence: list Reply-To: The list for a Better Approach To Mobile Ad-hoc Networking List-Id: The list for a Better Approach To Mobile Ad-hoc Networking List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 01 Dec 2014 18:01:51 -0000 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; }; /**