This patch adds interface alternating to the new bonding feature.
Instead of only sending in a round robin fashion on the usable interfaces, we can also attempt to use a different interface for sending than for receiving (interface alternating). This should reduce problems of the half-duplex nature of WiFi Hardware and thus increase performance. The bonding modes are now enhanced from two modes (disabled/enabled) to the following 4 modes:
* 0 - bonding off * 1 - round robin (as before) * 2 - interface alternating * 3 - smart bonding (round robin + interface alternating)
This is an experimental patch and targeted for upcoming experiments at Wireless Battle Mesh v3 in Bracciano/Italy.
Feedback, comments and reviews appreciated!
Signed-off-by: Simon Wunderlich siwu@hrz.tu-chemnitz.de --- Index: a/batman-adv-kernelland/types.h =================================================================== --- a/batman-adv-kernelland/types.h (revision 1679) +++ a/batman-adv-kernelland/types.h (working copy) @@ -119,7 +119,7 @@ struct bat_priv { struct net_device_stats stats; atomic_t aggregation_enabled; - atomic_t bonding_enabled; + atomic_t bonding_mode; atomic_t vis_mode; atomic_t gw_mode; atomic_t gw_class; Index: a/batman-adv-kernelland/soft-interface.c =================================================================== --- a/batman-adv-kernelland/soft-interface.c (revision 1679) +++ a/batman-adv-kernelland/soft-interface.c (working copy) @@ -250,7 +250,7 @@ if (!orig_node) orig_node = transtable_search(ethhdr->h_dest);
- router = find_router(orig_node); + router = find_router(orig_node, NULL);
if (!router) goto unlock; Index: a/batman-adv-kernelland/hard-interface.c =================================================================== --- a/batman-adv-kernelland/hard-interface.c (revision 1679) +++ a/batman-adv-kernelland/hard-interface.c (working copy) @@ -514,7 +514,7 @@
/* unicast packet */ case BAT_UNICAST: - ret = recv_unicast_packet(skb); + ret = recv_unicast_packet(skb, batman_if); break;
/* broadcast packet */ Index: a/batman-adv-kernelland/routing.c =================================================================== --- a/batman-adv-kernelland/routing.c (revision 1679) +++ a/batman-adv-kernelland/routing.c (working copy) @@ -417,7 +417,7 @@
{ /* don't care if bonding is not enabled */ - if (!atomic_read(&bat_priv->bonding_enabled)) { + if (!atomic_read(&bat_priv->bonding_mode)) { orig_node->bond.candidates = 0; return; } @@ -429,7 +429,7 @@ return; }
-/* mark possible bonding candidates in the neighbor list */ +/* mark possible bond.candidates in the neighbor list */ void update_bonding_candidates(struct bat_priv *bat_priv, struct orig_node *orig_node) { @@ -440,7 +440,7 @@ struct neigh_node *first_candidate, *last_candidate;
/* don't care if bonding is not enabled */ - if (!atomic_read(&bat_priv->bonding_enabled)) { + if (!atomic_read(&bat_priv->bonding_mode)) { orig_node->bond.candidates = 0; return; } @@ -453,7 +453,7 @@
best_tq = orig_node->router->tq_avg;
- /* update bonding candidates */ + /* update bond.candidates */
candidates = 0;
@@ -1007,14 +1007,16 @@
/* find a suitable router for this originator, and use * bonding if possible. */ -struct neigh_node *find_router(struct orig_node *orig_node) +struct neigh_node *find_router(struct orig_node *orig_node, + struct batman_if *recv_if) { /* FIXME: each orig_node->batman_if will be attached to a softif */ struct bat_priv *bat_priv = netdev_priv(soft_device); struct orig_node *primary_orig_node; struct orig_node *router_orig; - struct neigh_node *router; + struct neigh_node *router, *first_candidate; static uint8_t zero_mac[ETH_ALEN] = {0, 0, 0, 0, 0, 0}; + int bonding_mode;
if (!orig_node) return NULL; @@ -1022,8 +1024,9 @@ if (!orig_node->router) return NULL;
- /* don't care if bonding is not enabled */ - if (!atomic_read(&bat_priv->bonding_enabled)) + /* just return default router if bonding is not enabled */ + bonding_mode = atomic_read(&bat_priv->bonding_mode); + if (!bonding_mode) return orig_node->router;
router_orig = orig_node->router->orig_node; @@ -1052,19 +1055,50 @@ if (primary_orig_node->bond.candidates < 2) return orig_node->router;
- router = primary_orig_node->bond.selected; + switch (bonding_mode) { + case BONDING_ROUNDROBIN: + router = primary_orig_node->bond.selected;
- /* sanity check - this should never happen. */ - if (!router) - return orig_node->router; + /* sanity check - this should never happen. */ + if (!router) + return orig_node->router;
- /* select the next bonding partner ... */ - primary_orig_node->bond.selected = router->next_bond_candidate; + /* select the next bonding partner ... */ + primary_orig_node->bond.selected = router->next_bond_candidate; + break; + case BONDING_ALTERNATE: + /* in alternate mode, the first node should + * always choose the default router. */ + if (recv_if == NULL) + return orig_node->router;
+ /* all nodes between should choose a candidate which + * is is not on the interface where the packet came + * in. There might be more than one alternative + * interface, and we send the packet in a round robin + * fashion on these interfaces in this case. */ + case BONDING_SMART: + first_candidate = primary_orig_node->bond.selected; + router = first_candidate; + + do { + /* recv_if == NULL on the first node. */ + if (router->if_incoming != recv_if) + break; + router = router->next_bond_candidate; + } while (router != first_candidate); + + /* select the next to have some diversity here. */ + primary_orig_node->bond.selected = router->next_bond_candidate; + break; + default: + router = orig_node->router; + } + return router; }
-int recv_unicast_packet(struct sk_buff *skb) +int recv_unicast_packet(struct sk_buff *skb, struct batman_if *recv_if) { struct unicast_packet *unicast_packet; struct orig_node *orig_node; @@ -1116,7 +1150,7 @@ orig_node = ((struct orig_node *) hash_find(orig_hash, unicast_packet->dest));
- router = find_router(orig_node); + router = find_router(orig_node, recv_if);
if (!router) { spin_unlock_irqrestore(&orig_hash_lock, flags); Index: a/batman-adv-kernelland/main.h =================================================================== --- a/batman-adv-kernelland/main.h (revision 1679) +++ a/batman-adv-kernelland/main.h (working copy) @@ -63,6 +63,10 @@ * to be considered as bonding candidates */
#define BONDING_TQ_THRESHOLD 50 +#define BONDING_NONE 0 +#define BONDING_ROUNDROBIN 1 +#define BONDING_ALTERNATE 2 +#define BONDING_SMART 3
#define MAX_AGGREGATION_BYTES 512 /* should not be bigger than 512 bytes or * change the size of Index: a/batman-adv-kernelland/routing.h =================================================================== --- a/batman-adv-kernelland/routing.h (revision 1679) +++ a/batman-adv-kernelland/routing.h (working copy) @@ -32,11 +32,12 @@ struct neigh_node *neigh_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); +int recv_unicast_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, struct batman_if *batman_if); -struct neigh_node *find_router(struct orig_node *orig_node); +struct neigh_node *find_router(struct orig_node *orig_node, + struct batman_if *recv_if); void update_bonding_candidates(struct bat_priv *bat_priv, struct orig_node *orig_node); Index: a/batman-adv-kernelland/bat_sysfs.c =================================================================== --- a/batman-adv-kernelland/bat_sysfs.c (revision 1679) +++ a/batman-adv-kernelland/bat_sysfs.c (working copy) @@ -92,10 +92,9 @@ { struct device *dev = to_dev(kobj->parent); struct bat_priv *bat_priv = netdev_priv(to_net_dev(dev)); - int bond_status = atomic_read(&bat_priv->bonding_enabled); + int bonding_mode = atomic_read(&bat_priv->bonding_mode);
- return sprintf(buff, "%s\n", - bond_status == 0 ? "disabled" : "enabled"); + return sprintf(buff, "%d\n", bonding_mode); }
static ssize_t store_bond(struct kobject *kobj, struct attribute *attr, @@ -104,17 +103,25 @@ struct device *dev = to_dev(kobj->parent); struct net_device *net_dev = to_net_dev(dev); struct bat_priv *bat_priv = netdev_priv(net_dev); - int bonding_enabled_tmp = -1; + int bonding_mode_tmp = -1;
- if (((count == 2) && (buff[0] == '1')) || - (strncmp(buff, "enable", 6) == 0)) - bonding_enabled_tmp = 1; + if (count == 2) + switch (buff[0]) { + case '0': + bonding_mode_tmp = 0; + break; + case '1': + bonding_mode_tmp = BONDING_ROUNDROBIN; + break; + case '2': + bonding_mode_tmp = BONDING_ALTERNATE; + break; + case '3': + bonding_mode_tmp = BONDING_SMART; + break; + }
- if (((count == 2) && (buff[0] == '0')) || - (strncmp(buff, "disable", 7) == 0)) - bonding_enabled_tmp = 0; - - if (bonding_enabled_tmp < 0) { + if (bonding_mode_tmp < 0) { if (buff[count - 1] == '\n') buff[count - 1] = '\0';
@@ -123,16 +130,14 @@ return -EINVAL; }
- if (atomic_read(&bat_priv->bonding_enabled) == bonding_enabled_tmp) + if (atomic_read(&bat_priv->bonding_mode) == bonding_mode_tmp) return count;
- printk(KERN_INFO "batman-adv:Changing bonding from: %s to: %s on mesh: %s\n", - atomic_read(&bat_priv->bonding_enabled) == 1 ? - "enabled" : "disabled", - bonding_enabled_tmp == 1 ? "enabled" : "disabled", + printk(KERN_INFO "batman-adv:Changing bonding from: %d to: %d on mesh: %s\n", + atomic_read(&bat_priv->bonding_mode), bonding_mode_tmp, net_dev->name);
- atomic_set(&bat_priv->bonding_enabled, (unsigned)bonding_enabled_tmp); + atomic_set(&bat_priv->bonding_mode, (unsigned)bonding_mode_tmp); return count; }
@@ -303,7 +308,7 @@ /* FIXME: should be done in the general mesh setup routine as soon as we have it */ atomic_set(&bat_priv->aggregation_enabled, 1); - atomic_set(&bat_priv->bonding_enabled, 0); + atomic_set(&bat_priv->bonding_mode, 0); atomic_set(&bat_priv->vis_mode, VIS_TYPE_CLIENT_UPDATE); atomic_set(&bat_priv->gw_mode, GW_MODE_OFF); atomic_set(&bat_priv->gw_class, 0);
Only some non-technical comments about the patch for now:
I thought we decided that the "Staging: " part in the title will be appended when the stuff will be send to Greg and should be omitted inside our source trees. Also your patch wouldn't apply in linux-next because the bonding stuff is in master and not in maint (only maint will be send to the kernel folks).
This is an experimental patch and targeted for upcoming experiments at Wireless Battle Mesh v3 in Bracciano/Italy.
Feedback, comments and reviews appreciated!
That part is quite interesting for the mailinglist but should not be part of the commit message, or am I wrong? In that situation such information should be appended after the "---". git-am or similar tools should automatically ignore that parts, but the receiver can still read that comment.
Index: a/batman-adv-kernelland/main.h
--- a/batman-adv-kernelland/main.h (revision 1679) +++ a/batman-adv-kernelland/main.h (working copy) @@ -63,6 +63,10 @@
- to be considered as bonding candidates */
#define BONDING_TQ_THRESHOLD 50 +#define BONDING_NONE 0 +#define BONDING_ROUNDROBIN 1
You've wrote something like "space""tab""1" here. Can you please remove the space before the tab.
Have you already painted some nice graphics or wrote some text to explain the modes further (yes, code is the best documentation, but a bad specification)?
The rest looks like it should be tested in italy :)
Best regards, Sven
Hey,
i've checked in a changed and fixed up version of this patch in r1686.
This version has the interface alternating enabled by default, so we don't need to change the "bonding_enabled" to "bonding_mode" anymore. We have decided here in Bracciano to have alternating as default as it appears to be beneficial most of the times, but we couldn't really think of a scenario where this might be a problem.
I've also added some documentation at http://www.open-mesh.net/wiki/bonding-alternating and i'm looking for comments and proofreading and comments. ;)
I've also followed your other comments (hopefully correctly), so thank you very much for the review.
best regards Simon
On Wed, May 26, 2010 at 12:39:22PM +0200, Sven Eckelmann wrote:
Only some non-technical comments about the patch for now:
I thought we decided that the "Staging: " part in the title will be appended when the stuff will be send to Greg and should be omitted inside our source trees. Also your patch wouldn't apply in linux-next because the bonding stuff is in master and not in maint (only maint will be send to the kernel folks).
This is an experimental patch and targeted for upcoming experiments at Wireless Battle Mesh v3 in Bracciano/Italy.
Feedback, comments and reviews appreciated!
That part is quite interesting for the mailinglist but should not be part of the commit message, or am I wrong? In that situation such information should be appended after the "---". git-am or similar tools should automatically ignore that parts, but the receiver can still read that comment.
Index: a/batman-adv-kernelland/main.h
--- a/batman-adv-kernelland/main.h (revision 1679) +++ a/batman-adv-kernelland/main.h (working copy) @@ -63,6 +63,10 @@
- to be considered as bonding candidates */
#define BONDING_TQ_THRESHOLD 50 +#define BONDING_NONE 0 +#define BONDING_ROUNDROBIN 1
You've wrote something like "space""tab""1" here. Can you please remove the space before the tab.
Have you already painted some nice graphics or wrote some text to explain the modes further (yes, code is the best documentation, but a bad specification)?
The rest looks like it should be tested in italy :)
Best regards, Sven
Simon Wunderlich wrote:
I've also followed your other comments (hopefully correctly), so thank you very much for the review.
[...]
This is an experimental patch and targeted for upcoming experiments at Wireless Battle Mesh v3 in Bracciano/Italy.
Feedback, comments and reviews appreciated!
That part is quite interesting for the mailinglist but should not be part of the commit message, or am I wrong? In that situation such information should be appended after the "---". git-am or similar tools should automatically ignore that parts, but the receiver can still read that comment.
That "---" part was actually meant for the mail and not for the svn commit messages. It will not be stripped from commit messages by git-svn - so it remains in the git master branch. I have removed it manually from my rebase branch - please tell me if that wasn't what you wanted and I will re-add it.
I noticed the "The patch also includes a bug fix...". What exactly do you mean here? I would guess that it is only this part:
/* we only care if the other candidate is even* considered as candidate. */if (tmp_neigh_node2->next_bond_candidate == NULL)continue;
But nice to see that the testing resulted in an even more compact patch which is better readable. :)
Best regards, Sven
Hey Sven,
On Fri, Jun 04, 2010 at 10:56:50PM +0200, Sven Eckelmann wrote:
Simon Wunderlich wrote:
I've also followed your other comments (hopefully correctly), so thank you very much for the review.
[...]
This is an experimental patch and targeted for upcoming experiments at Wireless Battle Mesh v3 in Bracciano/Italy.
Feedback, comments and reviews appreciated!
That part is quite interesting for the mailinglist but should not be part of the commit message, or am I wrong? In that situation such information should be appended after the "---". git-am or similar tools should automatically ignore that parts, but the receiver can still read that comment.
That "---" part was actually meant for the mail and not for the svn commit messages. It will not be stripped from commit messages by git-svn - so it remains in the git master branch. I have removed it manually from my rebase branch - please tell me if that wasn't what you wanted and I will re-add it.
Oh, okay. We don't actually need it in the commit message, this was more an internal notice, so it is fine to keep it out.
I noticed the "The patch also includes a bug fix...". What exactly do you mean here? I would guess that it is only this part:
/* we only care if the other candidate is even* considered as candidate. */if (tmp_neigh_node2->next_bond_candidate == NULL)continue;
Yep, that is the bug fix. ;)
But nice to see that the testing resulted in an even more compact patch which is better readable. :)
thank you. I'm also happier with this version. ;)
best regards, Simon
b.a.t.m.a.n@lists.open-mesh.org
-
Simon Wunderlich -
Sven Eckelmann