Commit 1c2bcc76 authored by Sven Eckelmann's avatar Sven Eckelmann Committed by Simon Wunderlich

batman-adv: Keep fragments equally sized

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: default avatarMartin Weinelt <martin@darmstadt.freifunk.net>
Fixes: ee75ed88 ("batman-adv: Fragment and send skbs larger than mtu")
Signed-off-by: default avatarSven Eckelmann <sven@narfation.org>
Acked-by: default avatarLinus Lüssing <linus.luessing@c0d3.blue>
Signed-off-by: default avatarSimon Wunderlich <sw@simonwunderlich.de>
parent 51c6b429
...@@ -404,7 +404,7 @@ bool batadv_frag_skb_fwd(struct sk_buff *skb, ...@@ -404,7 +404,7 @@ bool batadv_frag_skb_fwd(struct sk_buff *skb,
* batadv_frag_create - create a fragment from skb * batadv_frag_create - create a fragment from skb
* @skb: skb to create fragment from * @skb: skb to create fragment from
* @frag_head: header to use in new fragment * @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 * 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 * passed mtu and the old one with the rest. The new skb contains data from the
...@@ -414,11 +414,11 @@ bool batadv_frag_skb_fwd(struct sk_buff *skb, ...@@ -414,11 +414,11 @@ bool batadv_frag_skb_fwd(struct sk_buff *skb,
*/ */
static struct sk_buff *batadv_frag_create(struct sk_buff *skb, static struct sk_buff *batadv_frag_create(struct sk_buff *skb,
struct batadv_frag_packet *frag_head, struct batadv_frag_packet *frag_head,
unsigned int mtu) unsigned int fragment_size)
{ {
struct sk_buff *skb_fragment; struct sk_buff *skb_fragment;
unsigned int header_size = sizeof(*frag_head); 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); skb_fragment = netdev_alloc_skb(NULL, mtu + ETH_HLEN);
if (!skb_fragment) if (!skb_fragment)
...@@ -456,7 +456,7 @@ int batadv_frag_send_packet(struct sk_buff *skb, ...@@ -456,7 +456,7 @@ int batadv_frag_send_packet(struct sk_buff *skb,
struct sk_buff *skb_fragment; struct sk_buff *skb_fragment;
unsigned int mtu = neigh_node->if_incoming->net_dev->mtu; unsigned int mtu = neigh_node->if_incoming->net_dev->mtu;
unsigned int header_size = sizeof(frag_header); unsigned int header_size = sizeof(frag_header);
unsigned int max_fragment_size, max_packet_size; unsigned int max_fragment_size, num_fragments;
int ret; int ret;
/* To avoid merge and refragmentation at next-hops we never send /* To avoid merge and refragmentation at next-hops we never send
...@@ -464,10 +464,15 @@ int batadv_frag_send_packet(struct sk_buff *skb, ...@@ -464,10 +464,15 @@ int batadv_frag_send_packet(struct sk_buff *skb,
*/ */
mtu = min_t(unsigned int, mtu, BATADV_FRAG_MAX_FRAG_SIZE); mtu = min_t(unsigned int, mtu, BATADV_FRAG_MAX_FRAG_SIZE);
max_fragment_size = mtu - header_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 */ /* 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) {
ret = -EAGAIN; ret = -EAGAIN;
goto free_skb; goto free_skb;
} }
...@@ -507,7 +512,8 @@ int batadv_frag_send_packet(struct sk_buff *skb, ...@@ -507,7 +512,8 @@ int batadv_frag_send_packet(struct sk_buff *skb,
goto put_primary_if; goto put_primary_if;
} }
skb_fragment = batadv_frag_create(skb, &frag_header, mtu); skb_fragment = batadv_frag_create(skb, &frag_header,
max_fragment_size);
if (!skb_fragment) { if (!skb_fragment) {
ret = -ENOMEM; ret = -ENOMEM;
goto put_primary_if; goto put_primary_if;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment