batman adv unicast fragmentation (version 3)
Hi,
thanks for the reply of the patch and all the hints. I changed some parts as you suggested and here some notes to your questions.
Quoted 5 lines
Review
Quoted 2 lines
(quoted diff) +2
-0+ if (!(my_skb_push(frag_skb, hdr_len) >= 0 && + my_skb_push(skb, hdr_len) >= 0)) {Didn't you wanted to check if one of the
my_skb_pushfailed and not if both failed?
This part check if one failed, but you are right if you think that it looks too complicated. I changed it because it is easy to misunderstand this expression.
Quoted 1 line
Can you please explain why the copyright already starts in 2007?
Copy and paste failure, changed it to 2010.
Quoted 4 lines
Review
Quoted 2 lines
(quoted diff) +2
-0+ if (atomic_read(&batman_if->frag_seqno) == FRAG_MAX_SEQ) + atomic_set(&batman_if->frag_seqno, 0);It is not a good idea to split two
atomic_operations which depends on each other.
I removed that part completely, because there is a simplier way.
And now some words to the implementation. This version of fragmentation handles only packets over a batX interface. My test setup follows:
laptop tap0 --> br0 (bat0/eth0) host1 eth1 ------> eth0 host2 -----> eth0 host3
ping -s 1472 -M do xxx
1472 bytes - data
28 bytes - icmp / ip
14 bytes - eth
Bat0 receives 1514 bytes (interface_tx, data_len), if data_len + unicast_packet is larger than the mtu of the sending interface, this packet will be fragmented.
In fragmentation 2.0 i would like to implement a solution for the problem described by simon ( i hope i understood him correctly). A node has two batman interfaces and the mtu of interface2 is smaller then interface1. In case a packet from iface1 (with the size of this mtu) travels to iface2, we currently have a problem. Version 2.0 will fix this.
regards,
andreas
PATCH 1/2 batctl: layer2 unicast packet fragmentation
This patch add options to enable/disable fragmentation
--- batctl/main.c | 41 +++++++++++++++++++++++------------------ batctl/sys.c | 7 +++++++ batctl/sys.h | 2 ++ 3 files changed, 32 insertions(+), 18 deletions(-)
batctl/main.c +23 -18
@@ -1,7 +1,7 @@/* * Copyright (C) 2007-2010 B.A.T.M.A.N. contributors: * -* Andreas Langer <[email protected]>, Marek Lindner <[email protected]> +* Andreas Langer <[email protected]>, Marek Lindner <[email protected]> * * This program is free software; you can redistribute it and/or * modify it under the terms of version 2 of the GNU General Public
@@ -41,24 +41,25 @@void print_usage (void ) {printf ("Usage: batctl [options] commands \n" );printf ("commands:\n" ); -printf (" \tinterface|if [add|del iface(s)]\tdisplay or modify the interface settings\n" ); -printf (" \toriginators|o \tdisplay the originator table\n" ); -printf (" \tinterval|it [orig_interval] \tdisplay or modify the originator interval (in ms)\n" ); -printf (" \tloglevel|ll [level] \tdisplay or modify the log level\n" ); -printf (" \tlog|l \tread the log produced by the kernel module\n" ); -printf (" \tgw_mode|gw [mode] \tdisplay or modify the gateway mode\n" ); -printf (" \tgateways|gwl \tdisplay the gateway server list\n" ); -printf (" \ttranslocal|tl \tdisplay the local translation table\n" ); -printf (" \ttransglobal|tg \tdisplay the global translation table\n" ); -printf (" \tvis_mode|vm [mode] \tdisplay or modify the status of the VIS server\n" ); -printf (" \tvis_data|vd [dot|JSON] \tdisplay the VIS data in dot or JSON format\n" ); -printf (" \taggregation|ag [0|1] \tdisplay or modify the packet aggregation setting\n" ); -printf (" \tbonding|b [0|1] \tdisplay or modify the bonding mode setting\n" ); +printf (" \tinterface|if [add|del iface(s)]\tdisplay or modify the interface settings\n" ); +printf (" \toriginators|o \tdisplay the originator table\n" ); +printf (" \tinterval|it [orig_interval] \tdisplay or modify the originator interval (in ms)\n" ); +printf (" \tloglevel|ll [level] \tdisplay or modify the log level\n" ); +printf (" \tlog|l \tread the log produced by the kernel module\n" ); +printf (" \tgw_mode|gw [mode] \tdisplay or modify the gateway mode\n" ); +printf (" \tgateways|gwl \tdisplay the gateway server list\n" ); +printf (" \ttranslocal|tl \tdisplay the local translation table\n" ); +printf (" \ttransglobal|tg \tdisplay the global translation table\n" ); +printf (" \tvis_mode|vm [mode] \tdisplay or modify the status of the VIS server\n" ); +printf (" \tvis_data|vd [dot|JSON] \tdisplay the VIS data in dot or JSON format\n" ); +printf (" \taggregation|ag [0|1] \tdisplay or modify the packet aggregation setting\n" ); +printf (" \tbonding|b [0|1] \tdisplay or modify the bonding mode setting\n" ); +printf (" \tfragmentation|f [0|1] \tdisplay or modify the fragmentation mode setting\n" );printf ("\n" ); -printf (" \tping|p <destination> \tping another batman adv host via layer 2\n" ); -printf (" \ttraceroute|tr <destination> \ttraceroute another batman adv host via layer 2\n" ); -printf (" \ttcpdump|td <interface> \ttcpdump layer 2 traffic on the given interface\n" ); -printf (" \tbisect <file1> .. <fileN>\tanalyze given log files for routing stability\n" ); +printf (" \tping|p <destination> \tping another batman adv host via layer 2\n" ); +printf (" \ttraceroute|tr <destination> \ttraceroute another batman adv host via layer 2\n" ); +printf (" \ttcpdump|td <interface> \ttcpdump layer 2 traffic on the given interface\n" ); +printf (" \tbisect <file1> .. <fileN>\tanalyze given log files for routing stability\n" );printf ("options:\n" );printf (" \t-h print this help (or 'batctl <command> -h' for the command specific help)\n" );printf (" \t-v print version\n" );
@@ -149,6 +150,10 @@ int main(int argc, char **argv) ret =handle_sys_setting (argc -1 , argv +1 ,SYS_BONDING , bonding_usage, sysfs_param_enable); + }else if ((strcmp (argv[1 ],"fragmentation" ) ==0 ) || (strcmp (argv[1 ],"f" ) ==0 )) { + + ret =handle_sys_setting (argc -1 , argv +1 ,SYS_FRAG , fragmentation_usage, sysfs_param_enable); + }else if ((strcmp (argv[1 ],"bisect" ) ==0 )) { ret =bisect (argc -1 , argv +1 );
batctl/sys.c +7 -0
@@ -252,6 +252,13 @@ void orig_interval_usage(void)printf (" \t -h print this help\n" ); } +void fragmentation_usage (void ) +{ +printf ("Usage: batctl [options] fragmentation [0|1]\n" ); +printf ("options:\n" ); +printf (" \t -h print this help\n" ); +} +int handle_sys_setting (int argc,char **argv,char *file_path,void setting_usage (void ),const char *sysfs_param[])
batctl/sys.h +2 -0
@@ -31,12 +31,14 @@#define SYS_IFACE_PATH "/sys/class/net"#define SYS_MESH_IFACE_FMT SYS_IFACE_PATH"/%s/batman_adv/mesh_iface"#define SYS_IFACE_STATUS_FMT SYS_IFACE_PATH"/%s/batman_adv/iface_status" +#define SYS_FRAG "fragmentation"extern const char *sysfs_param_enable[];extern const char *sysfs_param_server[];void aggregation_usage (void );void bonding_usage (void ); +void fragmentation_usage (void );void gw_mode_usage (void );void vis_mode_usage (void );void orig_interval_usage (void );
Signature · 1 line
1.7.0.4
PATCH 2/2 batman-adv: layer2 unicast packet fragmentation
In reply to Andreas LangerThis patch implements a simple layer2 fragmentation to allow traffic exchange over network interfaces with a MTU smaller than 1500 bytes. The fragmentation splits the big packets into two parts and marks the frames accordingly. The receiving end buffers the packets to reassemble the orignal packet before passing it to the higher layers. This feature makes it necessary to modify the batman-adv encapsulation for unicast packets by adding a sequence number, flags and the originator address. This modifcation is part of a seperate packet type for fragemented packets to keep the original overhead as low as possible. This patch enables the feature by default to ensure the data traffic can travel through the network. But it also prints a warning to notify the user about the performance implications.
Note: Fragmentation should be avoided at all costs since it has a dramatic impact on the performance, especially when it comes wifi networks. Instead of a single packet, 2 packets have to be sent! Not only valuable airtime is wasted but also packetloss decreases the throughput. A link with 50% packetloss and fragmentation enabled is pretty much unusable.
--- batman-adv/Makefile.kbuild | 2 +- batman-adv/bat_sysfs.c | 55 ++++++++++++++++ batman-adv/fragmentation.c | 152 +++++++++++++++++++++++++++++++++++++++++++ batman-adv/fragmentation.h | 34 ++++++++++ batman-adv/hard-interface.c | 34 +++++++++- batman-adv/originator.c | 10 +++ batman-adv/packet.h | 26 ++++++-- batman-adv/routing.c | 144 ++++++++++++++++++++++++++++++++--------- batman-adv/routing.h | 1 + batman-adv/soft-interface.c | 76 ++++++++++++++++++---- batman-adv/types.h | 10 +++ 11 files changed, 493 insertions(+), 51 deletions(-) create mode 100644 batman-adv/fragmentation.c create mode 100644 batman-adv/fragmentation.h
batman-adv/Makefile.kbuild +1 -1
@@ -32,4 +32,4 @@ EXTRA_CFLAGS += -DREVISION_VERSION=\"$(REVISION)\" endif obj-m += batman-adv.o -batman-adv-objs := main.o bat_debugfs.o bat_sysfs.o send.o routing.o soft-interface.o icmp_socket.o translation-table.o bitarray.o hash.o ring_buffer.o vis.o hard-interface.o aggregation.o originator.o gateway_common.o gateway_client.o $(shell [ "2" -eq "$(VERSION)" ] 2>&- && [ "6" -eq "$(PATCHLEVEL)" ] 2>&- && [ "$(SUBLEVEL)" -le "28" ] 2>&- && echo bat_printk.o) +batman-adv-objs := main.o bat_debugfs.o bat_sysfs.o send.o routing.o soft-interface.o icmp_socket.o translation-table.o bitarray.o hash.o ring_buffer.o vis.o hard-interface.o aggregation.o originator.o gateway_common.o gateway_client.o fragmentation.o $(shell [ "2" -eq "$(VERSION)" ] 2>&- && [ "6" -eq "$(PATCHLEVEL)" ] 2>&- && [ "$(SUBLEVEL)" -le "28" ] 2>&- && echo bat_printk.o)
batman-adv/bat_sysfs.c +55 -0
79 lines not on this page · raw message
batman-adv/fragmentation.c new file +152 -0
152 lines not on this page · raw message
batman-adv/fragmentation.h new file +34 -0
@@ -0,0 +1,34 @@ +/* +* Copyright (C) 2010 B.A.T.M.A.N. contributors: +* +* Andreas Langer +* +* This program is free software; you can redistribute it and/or +* modify it under the terms of version 2 of the GNU General Public +* License as published by the Free Software Foundation. +* +* This program is distributed in the hope that it will be useful, but +* WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +* General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, write to the Free Software +* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA +* 02110-1301, USA +* +*/ + +#define FRAG_TIMEOUT 60000/* purge frag list entrys after time in ms */ +#define FRAG_BUFFER_SIZE 6/* number of list elements in buffer */ +#define FRAG_MAX_SEQ 65535 + +struct sk_buff *merge_frag_packet (struct list_head *head, +struct frag_packet_list_entry *tfp, +struct sk_buff *skb); + +void create_frag_entry (struct list_head *head,struct sk_buff *skb); +void create_frag_buffer (struct list_head *head); +struct frag_packet_list_entry *search_frag_packet (struct list_head *head, +struct unicast_frag_packet *up); +void frag_list_free (struct list_head *head);
batman-adv/hard-interface.c +33 -1
@@ -166,6 +166,11 @@ int hardif_min_mtu(void)/* allow big frames if all devices are capable to do so * (have MTU > 1500 + BAT_HEADER_LEN) */ int min_mtu =ETH_DATA_LEN ; +/* FIXME: each batman_if will be attached to a softif */ +struct bat_priv *bat_priv =netdev_priv (soft_device); + +if (atomic_read (&bat_priv->frag_enabled)) + goto out;rcu_read_lock ();list_for_each_entry_rcu (batman_if, &if_list, list) {
@@ -175,7 +180,7 @@ int hardif_min_mtu(void) min_mtu); }rcu_read_unlock (); - +out:return min_mtu; }
@@ -265,8 +270,30 @@ int hardif_enable_interface(struct batman_if *batman_if)orig_hash_add_if (batman_if, bat_priv->num_ifaces);atomic_set (&batman_if->seqno,1 ); +atomic_set (&batman_if->frag_seqno,1 );bat_info (soft_device,"Adding interface: %s\n" , batman_if->dev); +if (atomic_read (&bat_priv->frag_enabled) && batman_if->net_dev->mtu < +ETH_DATA_LEN +BAT_HEADER_LEN ) +bat_info (soft_device, +"The MTU of interface %s is too small (%zi) to handle " +"the transport of batman-adv packets. Packets going " +"over this interface will be fragmented on layer2 " +"which could impact the performance. Setting the MTU " +"to %zi would solve the problem.\n" , + batman_if->dev,ETH_DATA_LEN +BAT_HEADER_LEN , +ETH_DATA_LEN +BAT_HEADER_LEN ); + +if (!atomic_read (&bat_priv->frag_enabled) && batman_if->net_dev->mtu < +ETH_DATA_LEN +BAT_HEADER_LEN ) +bat_info (soft_device, +"The MTU of interface %s is too small (%zi) to handle " +"the transport of batman-adv packets. If you experience" +" problems getting traffic through try increasing the " +"MTU to %zi.\n" , + batman_if->dev,ETH_DATA_LEN +BAT_HEADER_LEN , +ETH_DATA_LEN +BAT_HEADER_LEN ); +if (hardif_is_iface_up (batman_if))hardif_activate_interface (soft_device, bat_priv, batman_if);else
@@ -514,6 +541,11 @@ int batman_skb_recv(struct sk_buff *skb, struct net_device *dev, ret =recv_unicast_packet (skb, batman_if);break ; +/* fragmented unicast packet */ +case BAT_UNICAST_FRAG : + ret =recv_ucast_frag_packet (skb, batman_if); +break ; +/* broadcast packet */ case BAT_BCAST : ret =recv_bcast_packet (skb);
batman-adv/originator.c +10 -0
@@ -29,6 +29,7 @@#include "compat.h" #include "gateway_client.h" #include "hard-interface.h" +#include "fragmentation.h" static DECLARE_DELAYED_WORK(purge_orig_wq, purge_orig);
@@ -97,6 +98,7 @@ static void free_orig_node(void *data)kfree (neigh_node); } +frag_list_free (&orig_node->frag_list);hna_global_del_orig (orig_node,"originator timed out" );kfree (orig_node->bcast_own);
@@ -159,6 +161,10 @@ struct orig_node *get_orig_node(uint8_t *addr) size = bat_priv->num_ifaces *sizeof (uint8_t ); orig_node->bcast_own_sum =kzalloc (size,GFP_ATOMIC ); + +INIT_LIST_HEAD (&orig_node->frag_list); + orig_node->last_frag_packet =0 ; +if (!orig_node->bcast_own_sum) goto free_bcast_own;
@@ -275,6 +281,10 @@ void purge_orig(struct work_struct *work)hash_remove_bucket (orig_hash, &hashit);free_orig_node (orig_node); } + +if (time_after (jiffies, (orig_node->last_frag_packet + +msecs_to_jiffies (FRAG_TIMEOUT )))) +frag_list_free (&orig_node->frag_list); }spin_unlock_irqrestore (&orig_hash_lock, flags);
batman-adv/packet.h +20 -6
@@ -24,14 +24,15 @@#define ETH_P_BATMAN 0x4305/* unofficial/not registered Ethertype */ -#define BAT_PACKET 0x01 -#define BAT_ICMP 0x02 -#define BAT_UNICAST 0x03 -#define BAT_BCAST 0x04 -#define BAT_VIS 0x05 +#define BAT_PACKET 0x01 +#define BAT_ICMP 0x02 +#define BAT_UNICAST 0x03 +#define BAT_UNICAST_FRAG 0x04 +#define BAT_BCAST 0x05 +#define BAT_VIS 0x06/* this file is included by batctl which needs these defines */ -#define COMPAT_VERSION 10 +#define COMPAT_VERSION 12#define DIRECTLINK 0x40#define VIS_SERVER 0x20#define PRIMARIES_FIRST_HOP 0x10
@@ -47,6 +48,9 @@#define VIS_TYPE_SERVER_SYNC 0#define VIS_TYPE_CLIENT_UPDATE 1 +/* fragmentation defines */ +#define UNI_FRAG_HEAD 0x01 +struct batman_packet {uint8_t packet_type;uint8_t version;/* batman version field */
@@ -98,6 +102,16 @@ struct unicast_packet {uint8_t ttl; } __attribute__((packed)); +struct unicast_frag_packet { +uint8_t packet_type; +uint8_t version;/* batman version field */ +uint8_t dest[6 ]; +uint8_t ttl; +uint8_t flags; +uint8_t orig[6 ]; +uint16_t seqno; +} __attribute__((packed)); +struct bcast_packet {uint8_t packet_type;uint8_t version;/* batman version field */
batman-adv/routing.c +113 -31
175 lines not on this page · raw message
batman-adv/routing.h +1 -0
@@ -34,6 +34,7 @@ void update_routes(struct orig_node *orig_node, unsigned char *hna_buff, int hna_buff_len);int recv_icmp_packet (struct sk_buff *skb);int recv_unicast_packet (struct sk_buff *skb,struct batman_if *recv_if); +int recv_ucast_frag_packet (struct sk_buff *skb,struct batman_if *recv_if);int recv_bcast_packet (struct sk_buff *skb);int recv_vis_packet (struct sk_buff *skb); int recv_bat_packet(struct sk_buff *skb,
batman-adv/soft-interface.c +64 -12
112 lines not on this page · raw message
batman-adv/types.h +10 -0
@@ -41,6 +41,7 @@ struct batman_if {char addr_str[ETH_STR_LEN ];struct net_device *net_dev;atomic_t seqno; +atomic_t frag_seqno;unsigned char *packet_buff;int packet_len;struct kobject *hardif_obj;
@@ -82,6 +83,8 @@ struct orig_node {TYPE_OF_WORD bcast_bits[NUM_WORDS ];uint32_t last_bcast_seqno;struct list_head neigh_list; +struct list_head frag_list; +unsigned long last_frag_packet;struct {uint8_t candidates;struct neigh_node *selected;
@@ -118,6 +121,7 @@ struct bat_priv {struct net_device_stats stats;atomic_t aggregation_enabled;atomic_t bonding_enabled; +atomic_t frag_enabled;atomic_t vis_mode;atomic_t gw_mode;atomic_t gw_class;
@@ -192,4 +196,10 @@ struct debug_log { wait_queue_head_t queue_wait; }; +struct frag_packet_list_entry { +struct list_head list; +uint16_t seqno; +struct sk_buff *skb; +}; +#endif /* _NET_BATMAN_ADV_TYPES_H_ */
Signature · 1 line
1.7.0.4
Quoted 9 lines · Andreas Langer
Quoted 5 lines
Review
Quoted 2 lines
(quoted diff) +2
-0+ if (!(my_skb_push(frag_skb, hdr_len) >= 0 && + my_skb_push(skb, hdr_len) >= 0)) {Didn't you wanted to check if one of the
my_skb_pushfailed and not if both failed?This part check if one failed, but you are right if you think that it looks too complicated. I changed it because it is easy to misunderstand this expression.
Yes, sry. I mixed it up :)
Quoted 16 lines
And now some words to the implementation. This version of fragmentation handles only packets over a batX interface. My test setup follows:
laptop tap0 --> br0 (bat0/eth0) host1 eth1 ------> eth0 host2 -----> eth0 host3
ping -s 1472 -M do xxx
1472 bytes - data
28 bytes - icmp / ip
14 bytes - ethBat0 receives 1514 bytes (
interface_tx,data_len), ifdata_len+unicast_packetis larger than the mtu of the sending interface, this packet will be fragmented.
Quoted 5 lines
In fragmentation 2.0 i would like to implement a solution for the problem described by simon ( i hope i understood him correctly). A node has two batman interfaces and the mtu of interface2 is smaller then interface1. In case a packet from iface1 (with the size of this mtu) travels to iface2, we currently have a problem. Version 2.0 will fix this.
You described the problem correct. Just for other people: batman interfaces means not a bat0 and a bat1 interface, but two interfaces which are part of bat0.
So for example you have a wifi connection with mtu 1530 between node1 and node2 (bat0 interface has mtu 1500) and the connection between node2 and node3 is only a fast ethernet connection with mtu 1500 (no jumbo frames support).
node1 <- mtu 1530 -> node2 <- mtu 1500 -> node3
When we send data like our 1472 ping (needs mtu 1500 on bat0, and on the real interface mtu 1523) to node3, it will not be fragmented between node1 and node2 - thus it will not be fragmented between node2 and node3 (they will just drop the packet).
So it must not only be fragmented on output (data send directly through bat0), but also on forward (when bat0 received data on a participating interface and forwards it to another/same participating interface).
I see following problem:
* Data will be fragmented from node1 to node2
* fragmented packet is to big to be transported to node3
Now we would have to fragment the fragmented packet again... which is ugly. But to be honest, bat0 in a mesh which supports fragmented unicast packets is hardcoded to 1500. That means that the link between node2 and node3 must have the mtu < 775. That would be uncommon and maybe could be forbidden by batman- adv.
best regards,
Sven
Sven Eckelmann wrote:
[...]
Quoted 3 lines
Now we would have to fragment the fragmented packet again... which is ugly. But to be honest, bat0 in a mesh which supports fragmented unicast packets is hardcoded to 1500.
This was expressed wrong. The min_mtu of all participating interfaces (which is used as maximum mtu allowed by bat0) is hardcoded to 1500.
Best regards,
Sven