Experimental new bonding driver mode=batman
Subsystems (3)
PATCH 1/1 Experimental new bonding driver mode=batman
Hello,
I've written a new mode for the kernel bonding driver. It's similar to the round-robin mode, but it keeps statistics on TCP resends so as to favor slave devices with more bandwidth when choosing where to send packets. I've tested it on two laptops using WiFi/Ethernet and it seems to work okay, but it should be considered experimental.
Still, it should be safe to apply this patch as it doesn't do anything unless you specify "mode=batman" on the driver command line.
Signature · 2 lines
If I'm not here, I've gone out to find myself. If I get back before I return, please keep me here.
include/uapi/linux/if_bonding.h +1 -0
@@ -70,6 +70,7 @@#define BOND_MODE_8023AD 4#define BOND_MODE_TLB 5#define BOND_MODE_ALB 6/* TLB + RLB (receive load balancing) */ +#define BOND_MODE_BATMAN 7/* each slave's link has 4 states */ #define BOND_LINK_UP 0/* link is up and running */
include/net/bonding.h +70 -0
94 lines not on this page · raw message
drivers/net/bonding/bond_main.c +386 -5
477 lines not on this page · raw message
drivers/net/bonding/bond_options.c +1 -0
@@ -80,6 +80,7 @@ static const struct bond_opt_value bond_ { "802.3ad", BOND_MODE_8023AD, 0}, { "balance-tlb", BOND_MODE_TLB, 0}, { "balance-alb", BOND_MODE_ALB, 0}, + { "batman", BOND_MODE_BATMAN, 0}, { NULL, -1, 0}, };
Quoted 6 lines · Patrick Simmons
I've written a new mode for the kernel bonding driver. It's similar to the round-robin mode, but it keeps statistics on TCP resends so as to favor slave devices with more bandwidth when choosing where to send packets. I've tested it on two laptops using WiFi/Ethernet and it seems to work okay, but it should be considered experimental.
A description of how is the mode supposed to work would be definitely helpful.
Below are some comments from a quick look over the patch, I didn't think too much about the logic of the operation.
Quoted 5 lines
include/net/bonding.h +5
-0+struct batnode +{ +struct batnode * next_batnode; +char slvname[IFNAMSIZ ]; +unsigned long jiffiestamp;
include/net/bonding.h static inline int bond_get_targets_ip(__ · line 669
Quoted 10 lines
include/net/bonding.h +10
-0+u32 tcp_seq_num; +u16 port_id; +}; + +struct slave_congestion_node +{ +struct slave_congestion_node * next_slave; +char this_slave[IFNAMSIZ ]; +u32 congestion_counter; +};
Referencing the slave by its name doesn't seem very efficient, especially if you are going to do it in the xmit handler.
include/net/bonding.h extern const struct bond_parm_tbl fail_o · line 683
Quoted 3 lines
include/net/bonding.h +3
-0+#define BATTABLE_SIZE 256 +static struct batnode battable[BATTABLE_SIZE ]; +static struct slave_congestion_node * slavelist_head_ptr =NULL ;
Why one global list rather than one list per bond? And why do you need your own list in addition to already existing slave structures?
Quoted 18 lines
include/net/bonding.h +18
-0+static struct slave_congestion_node *get_slave_congestion_node (char * slvname) +{ +//printk("In get_slave_congestion_node: 0x%zx\n",the_slave); +struct slave_congestion_node * cur_slave = slavelist_head_ptr; +while (cur_slave) + { +if (strcmp (cur_slave->this_slave,slvname)==0 ) + { +//printk("Found a match: 0x%zx\n",cur_slave); +return cur_slave; + } + +if (!cur_slave->next_slave) +break ; + cur_slave = cur_slave->next_slave; + } + +//Create new slave node
include/net/bonding.h extern const struct bond_parm_tbl fail_o · line 712
Quoted 10 lines
include/net/bonding.h +10
-0+if (!cur_slave)//special case head + { + cur_slave = (struct slave_congestion_node *)(kmalloc (sizeof (struct slave_congestion_node ),GFP_ATOMIC )); + slavelist_head_ptr = cur_slave; + } +else + { + cur_slave->next_slave = (struct slave_congestion_node *)(kmalloc (sizeof (struct slave_congestion_node ),GFP_ATOMIC )); + cur_slave = cur_slave->next_slave; + }
You don't handle kmalloc() failure.
Quoted 9 lines
include/net/bonding.h +9
-0+ +//Initialize new slave node + cur_slave->next_slave =NULL ; +strlcpy (cur_slave->this_slave,slvname,IFNAMSIZ ); + cur_slave->congestion_counter =1 ; + +//printk("Created new congestion node: 0x%zx\n",cur_slave); +return cur_slave; +}
include/net/bonding.h extern const struct bond_parm_tbl fail_o · line 731
Quoted 10 lines
include/net/bonding.h +10
-0+ +static void slave_congestion_increment (char * the_slave) +{ +get_slave_congestion_node (the_slave)->congestion_counter++; +} + +static void slave_congestion_decrement (char * the_slave) +{ +get_slave_congestion_node (the_slave)->congestion_counter--; +}
Should these three really be in a header file?
Quoted 1 line
+static
DEFINE_SPINLOCK(batlock);
A lot of effort has been done recently to get rid of per-bond spinlocks. Adding a global one kind of goes against this work.
drivers/net/bonding/bond_main.c static bool bond_should_deliver_exact_ma · line 1097
Quoted 11 lines
drivers/net/bonding/bond_main.c +7
-0@@ -1082,6 +1088,69 @@ static bool bond_should_deliver_exact_mareturn false; } +static void batman_normalize_congestion (struct bonding * bond,int * congestion) +{ +static long unsigned int jiffiestamp =0 ; +long unsigned int jiffie_temp = jiffies; +struct slave * slave; +struct list_head * i; +if (jiffie_temp!=jiffiestamp && jiffie_temp%1000 ==0 )//every thousand packets, normalize congestion
Looks more like every 1000 jiffies, not packets. And what if this function doesn't get called in that one jiffy when jiffies is a multiple of 1000? (Similar problem later in bond_xmit_batman().)
drivers/net/bonding/bond_main.c static bool bond_should_deliver_exact_ma · line 1099
Quoted 1 line
drivers/net/bonding/bond_main.c +1
-0+printk (KERN_DEBUG "batman: congestion normalization has occurred.\n" );
Use pr_debug() or netdev_dbg().
Quoted 18 lines
drivers/net/bonding/bond_main.c +18
-0+static int batman_timer_invoke (void * bond_) +{ +struct bonding * bond = (struct bonding *)(bond_); +struct slave * slave; +struct list_head * i; + +while (!kthread_should_stop ()) + { +unsigned long batflags; +long unsigned int jiffiestamp = jiffies; + +msleep (10 *1000 ); +rcu_read_lock (); +spin_lock_irqsave (&batlock,batflags); + +long unsigned int jiffie_temp = jiffies; +if (!(jiffie_temp - jiffiestamp <5 *HZ )) +bond_for_each_slave_rcu (bond,slave,i)
drivers/net/bonding/bond_main.c static bool bond_should_deliver_exact_ma · line 1152
Quoted 10 lines
drivers/net/bonding/bond_main.c +10
-0+ { +struct slave_congestion_node * congest_current =get_slave_congestion_node (slave->dev->name); +if (congest_current->congestion_counter >=2 ) + congest_current->congestion_counter/=2 ; + } + +spin_unlock_irqrestore (&batlock,batflags); +rcu_read_unlock (); + } +}
This seems to only run something periodically, why do you need a (per bond) kthread instead of a simple timer?
Quoted 8 lines
drivers/net/bonding/bond_main.c +3
-1@@ -1100,7 +1169,67 @@ static rx_handler_result_t bond_handle_f slave =bond_slave_get_rcu (skb->dev); bond = slave->bond; - recv_probe =ACCESS_ONCE (bond->recv_probe); +//This seems as good a place as any to put this. +if (skb->protocol ==htons (ETH_P_IP )) + {
...
Quoted 1 line
+ }
I would suggest to handle TCP over IPv6 as well.
drivers/net/bonding/bond_main.c void bond_setup(struct net_device *bond_ · line 4430
Quoted 13 lines
drivers/net/bonding/bond_main.c +6
-0@@ -4068,6 +4419,12 @@ void bond_setup(struct net_device *bond_ bond_dev->hw_features &= ~(NETIF_F_ALL_CSUM & ~NETIF_F_HW_CSUM ); bond_dev->hw_features |=NETIF_F_GSO_ENCAP_ALL ; bond_dev->features |= bond_dev->hw_features; + +if (bond_mode ==BOND_MODE_BATMAN ) + { +printk (KERN_INFO "%s: I'm Batman.\n" ,bond_dev->name); + bond->battimer_task =kthread_run (batman_timer_invoke,bond,"%s_batman" ,bond_dev->name); + } }/* Destroy a bonding device.
You only start the kthread in bond_setup() and stop it in bond_destructor(). This doesn't handle the situation when the mode is changed after the bond is created.
One general note: the patch breaks kernel coding style in a lot of points. Running checkpatch.pl on it, I got
total: 562 errors, 466 warnings, 5 checks, 585 lines checked
The script isn't perfect but this is too much.
Michal Kubecek
Quoted 11 lines · Michal Kubecek
Quoted 6 lines · Patrick Simmons
I've written a new mode for the kernel bonding driver. It's similar to the round-robin mode, but it keeps statistics on TCP resends so as to favor slave devices with more bandwidth when choosing where to send packets. I've tested it on two laptops using WiFi/Ethernet and it seems to work okay, but it should be considered experimental.
A description of how is the mode supposed to work would be definitely helpful.
Rationale: It's helpful for cases where the slave devices have significantly different or varying bandwidth. The reason I wrote it is to bond powerline networking and wireless networking adapters into a single interface for use with connecting to a MythTV server. Neither of these systems is particularly reliable with bandwidth, but mode=batman can adaptively figure out which network has more available bandwidth at any given moment. This is better than mode=round-robin which always balances everything 50/50.
Regarding your analysis, I appreciate your comments, and I know it's rough, but I'm sorry to say I'm not really interested in doing much to improve its polish past where it is. If it fails some way when I try to deploy it, then I'll fix that, and maybe I'll play around with the balancing heuristics, but the code quality is what it is unless someone else wants to improve it. I would fix the indentation if that would make it acceptable for you to merge it, but not much more. My argument for merging it is basically "it doesn't do anything unless you pass mode=batman, so what's the harm?".
So, if you guys decide you don't want to merge it because of the global spinlock etc., that's cool and I understand, but I thought I should at least post to this list so you and any other potentially interested people know it exists. Oh, and, if you're not going to merge it, please let me know so I can know post the patch to GitHub or somewhere. And, if you could include a note in the comments at the top of bond_main.c or somewhere pointing people to the patch, I'd very much appreciate that. I don't want anyone else to have to endure hours of kernel rebuilds with KASAN enabled if they want this functionality :)
Cheers and have a good one!
--Patrick
Signature · 1 line
If I'm not here, I've gone out to find myself. If I get back before I return, please keep me here.
Quoted 13 lines · Patrick Simmons
Quoted 11 lines · Michal Kubecek
Quoted 6 lines · Patrick Simmons
I've written a new mode for the kernel bonding driver. It's similar to the round-robin mode, but it keeps statistics on TCP resends so as to favor slave devices with more bandwidth when choosing where to send packets. I've tested it on two laptops using WiFi/Ethernet and it seems to work okay, but it should be considered experimental.
A description of how is the mode supposed to work would be definitely helpful.
Quoted 8 lines
Rationale: It's helpful for cases where the slave devices have significantly different or varying bandwidth. The reason I wrote it is to bond powerline networking and wireless networking adapters into a single interface for use with connecting to a MythTV server. Neither of these systems is particularly reliable with bandwidth, but mode=batman can adaptively figure out which network has more available bandwidth at any given moment. This is better than mode=round-robin which always balances everything 50/50.
Thank you. But I rather meant some basic description of the algorithm used to achieve this goal. Both should be IMHO part of the commit message.
Quoted 11 lines
Regarding your analysis, I appreciate your comments, and I know it's rough, but I'm sorry to say I'm not really interested in doing much to improve its polish past where it is. If it fails some way when I try to deploy it, then I'll fix that, and maybe I'll play around with the balancing heuristics, but the code quality is what it is unless someone else wants to improve it. I would fix the indentation if that would make it acceptable for you to merge it, but not much more. My argument for merging it is basically "it doesn't do anything unless you pass mode=batman, so what's the harm?".
Quoted 10 lines
So, if you guys decide you don't want to merge it because of the global spinlock etc., that's cool and I understand, but I thought I should at least post to this list so you and any other potentially interested people know it exists. Oh, and, if you're not going to merge it, please let me know so I can know post the patch to GitHub or somewhere. And, if you could include a note in the comments at the top of
bond_main.cor somewhere pointing people to the patch, I'd very much appreciate that. I don't want anyone else to have to endure hours of kernel rebuilds with KASAN enabled if they want this functionality :)
Well, it's not my call, I'm not a bonding maintainer. But I believe at least some of the objections would be shared by them. Of course, it's up to you if you want to dedicate your time to improving the code to be acceptable for mainline or rather maintain it out of tree (which may end up taking even more time in the long term).
Michal Kubecek
Quoted 23 lines · Michal Kubecek
Quoted 13 lines · Patrick Simmons
Quoted 11 lines · Michal Kubecek
Quoted 6 lines · Patrick Simmons
I've written a new mode for the kernel bonding driver. It's similar to the round-robin mode, but it keeps statistics on TCP resends so as to favor slave devices with more bandwidth when choosing where to send packets. I've tested it on two laptops using WiFi/Ethernet and it seems to work okay, but it should be considered experimental.
A description of how is the mode supposed to work would be definitely helpful.
Quoted 8 lines
Rationale: It's helpful for cases where the slave devices have significantly different or varying bandwidth. The reason I wrote it is to bond powerline networking and wireless networking adapters into a single interface for use with connecting to a MythTV server. Neither of these systems is particularly reliable with bandwidth, but mode=batman can adaptively figure out which network has more available bandwidth at any given moment. This is better than mode=round-robin which always balances everything 50/50.
Quoted 3 lines
Thank you. But I rather meant some basic description of the algorithm used to achieve this goal. Both should be IMHO part of the commit message.
Agreed; the concept sounds interesting, but without a detailed
description of how it works it is difficult to evaluate its value.
Quoted 22 lines
Quoted 11 lines
Regarding your analysis, I appreciate your comments, and I know it's rough, but I'm sorry to say I'm not really interested in doing much to improve its polish past where it is. If it fails some way when I try to deploy it, then I'll fix that, and maybe I'll play around with the balancing heuristics, but the code quality is what it is unless someone else wants to improve it. I would fix the indentation if that would make it acceptable for you to merge it, but not much more. My argument for merging it is basically "it doesn't do anything unless you pass mode=batman, so what's the harm?".
Quoted 10 lines
So, if you guys decide you don't want to merge it because of the global spinlock etc., that's cool and I understand, but I thought I should at least post to this list so you and any other potentially interested people know it exists. Oh, and, if you're not going to merge it, please let me know so I can know post the patch to GitHub or somewhere. And, if you could include a note in the comments at the top of
bond_main.cor somewhere pointing people to the patch, I'd very much appreciate that. I don't want anyone else to have to endure hours of kernel rebuilds with KASAN enabled if they want this functionality :)
Quoted 5 lines
Well, it's not my call, I'm not a bonding maintainer. But I believe at least some of the objections would be shared by them. Of course, it's up to you if you want to dedicate your time to improving the code to be acceptable for mainline or rather maintain it out of tree (which may end up taking even more time in the long term).
Well, I am a bonding maintainer, and I can say that the patch in
its current state is not suitable for inclusion.
At a minimum, there are many coding style issues, commented out
debug statements, etc, along with design issues (e.g., the batman mode handling in bond_handle_frame is unconditional and takes place for all modes, not just the new batman mode).
If you (Patrick) or someone else wishes to contribute this to
mainline, I'd suggest that the first step is to read and follow the instructions in Documentation/SubmittingPatches in the kernel source code.
It is also not feasible to add pointers in the kernel source
code to out-of-tree patches; sorry.
-J
---
-Jay Vosburgh, [email protected]
Quoted 68 lines · Jay Vosburgh
Quoted 23 lines · Michal Kubecek
Quoted 13 lines · Patrick Simmons
Quoted 11 lines · Michal Kubecek
Quoted 6 lines · Patrick Simmons
I've written a new mode for the kernel bonding driver. It's similar to the round-robin mode, but it keeps statistics on TCP resends so as to favor slave devices with more bandwidth when choosing where to send packets. I've tested it on two laptops using WiFi/Ethernet and it seems to work okay, but it should be considered experimental.
A description of how is the mode supposed to work would be definitely helpful.
Quoted 8 lines
Rationale: It's helpful for cases where the slave devices have significantly different or varying bandwidth. The reason I wrote it is to bond powerline networking and wireless networking adapters into a single interface for use with connecting to a MythTV server. Neither of these systems is particularly reliable with bandwidth, but mode=batman can adaptively figure out which network has more available bandwidth at any given moment. This is better than mode=round-robin which always balances everything 50/50.
Quoted 3 lines
Thank you. But I rather meant some basic description of the algorithm used to achieve this goal. Both should be IMHO part of the commit message.
Agreed; the concept sounds interesting, but without a detailed
description of how it works it is difficult to evaluate its value.Quoted 22 lines
Quoted 11 lines
Regarding your analysis, I appreciate your comments, and I know it's rough, but I'm sorry to say I'm not really interested in doing much to improve its polish past where it is. If it fails some way when I try to deploy it, then I'll fix that, and maybe I'll play around with the balancing heuristics, but the code quality is what it is unless someone else wants to improve it. I would fix the indentation if that would make it acceptable for you to merge it, but not much more. My argument for merging it is basically "it doesn't do anything unless you pass mode=batman, so what's the harm?".
Quoted 10 lines
So, if you guys decide you don't want to merge it because of the global spinlock etc., that's cool and I understand, but I thought I should at least post to this list so you and any other potentially interested people know it exists. Oh, and, if you're not going to merge it, please let me know so I can know post the patch to GitHub or somewhere. And, if you could include a note in the comments at the top of
bond_main.cor somewhere pointing people to the patch, I'd very much appreciate that. I don't want anyone else to have to endure hours of kernel rebuilds with KASAN enabled if they want this functionality :)Quoted 5 lines
Well, it's not my call, I'm not a bonding maintainer. But I believe at least some of the objections would be shared by them. Of course, it's up to you if you want to dedicate your time to improving the code to be acceptable for mainline or rather maintain it out of tree (which may end up taking even more time in the long term).
Well, I am a bonding maintainer, and I can say that the patch in
its current state is not suitable for inclusion.At a minimum, there are many coding style issues, commented out
debug statements, etc, along with design issues (e.g., the batman mode handling inbond_handle_frameis unconditional and takes place for all modes, not just the new batman mode).
Quoted 7 lines
If you (Patrick) or someone else wishes to contribute this to
mainline, I'd suggest that the first step is to read and follow the instructions inDocumentation/SubmittingPatchesin the kernel source code.It is also not feasible to add pointers in the kernel source
code to out-of-tree patches; sorry.
Well I can now delete most of my initial response. :)
Overall I would say this is really cool functionality. Even if you do not want it merged, I think it is great that you shared it with the community this way. I got a chance to look at is a bit this morning and I agree additional explanation of the algorithm you are using would probably be nice for those checking this out for the first time.
I also think if you would be able to leverage the exiting bonding infra for using skb->queue_mapping you could probably add the same functionality (though it might be higher in the stack), but I totally understand if you want to just keep using what you are using as-is.
Quoted 8 lines · Andy Gospodarek
Well I can now delete most of my initial response. :)
Overall I would say this is really cool functionality. Even if you do not want it merged, I think it is great that you shared it with the community this way. I got a chance to look at is a bit this morning and I agree additional explanation of the algorithm you are using would probably be nice for those checking this out for the first time.
Thanks :) I explained the algorithm in my last email to the list, so I won't repeat it here.
Quoted 7 lines
I also think if you would be able to leverage the exiting bonding infra for using
skb->queue_mappingyou could probably add the same functionality (though it might be higher in the stack), but I totally understand if you want to just keep using what you are using as-is.
I didn't know about the queue_mapping thing. Thanks for the pointer.
One thing I considered doing that I didn't ultimately do was try to implement this as a TUN device. Userspace coding is always easier and, in retrospect, finding some way to manually parse IP and TCP packet headers would likely have been trying to debug kernel memory corruption bugs. But another downside would have been that I'm pretty sure all the slaves would have had to have been set to promiscuous mode to make it work.
--Patrick Simmons
Signature · 1 line
If I'm not here, I've gone out to find myself. If I get back before I return, please keep me here.
Quoted 25 lines · Jay Vosburgh
Quoted 23 lines · Michal Kubecek
Quoted 21 lines · Patrick Simmons
Quoted 11 lines · Michal Kubecek
Quoted 6 lines · Patrick Simmons
I've written a new mode for the kernel bonding driver. It's similar to the round-robin mode, but it keeps statistics on TCP resends so as to favor slave devices with more bandwidth when choosing where to send packets. I've tested it on two laptops using WiFi/Ethernet and it seems to work okay, but it should be considered experimental.
A description of how is the mode supposed to work would be definitely helpful.
Rationale: It's helpful for cases where the slave devices have significantly different or varying bandwidth. The reason I wrote it is to bond powerline networking and wireless networking adapters into a single interface for use with connecting to a MythTV server. Neither of these systems is particularly reliable with bandwidth, but mode=batman can adaptively figure out which network has more available bandwidth at any given moment. This is better than mode=round-robin which always balances everything 50/50.
Quoted 7 lines
Quoted 3 lines
Thank you. But I rather meant some basic description of the algorithm used to achieve this goal. Both should be IMHO part of the commit message.
Agreed; the concept sounds interesting, but without a detailed
description of how it works it is difficult to evaluate its value.
The basic algorithm is that each device starts out with a single "point". When a packet drop occurs and another device has been selected to retransmit the packet, that device gets another point and, if the first device has more than one point, the first device (the one that failed to successfully get the packet across) loses a point. At every packet send, all the points of all devices are added up, and each device has a (num_points)/(sum of all points) chance of being chosen to send the packet.
Every 10 seconds, all devices points are halved (unless the device only has one point). If the bonding device is approximately idle for 60 seconds, all devices' points are reset to 1 and the algorithm starts over.
Additionally, no slave ever retransmits a packet it itself sent unless it is the only active slave; but, if the slave WOULD HAVE retransmitted its own packet according to the algorithm, it doesn't lose any points and the other slave doesn't get any.
Quoted 22 lines
Quoted 22 lines
Quoted 21 lines
Regarding your analysis, I appreciate your comments, and I know it's rough, but I'm sorry to say I'm not really interested in doing much to improve its polish past where it is. If it fails some way when I try to deploy it, then I'll fix that, and maybe I'll play around with the balancing heuristics, but the code quality is what it is unless someone else wants to improve it. I would fix the indentation if that would make it acceptable for you to merge it, but not much more. My argument for merging it is basically "it doesn't do anything unless you pass mode=batman, so what's the harm?".
So, if you guys decide you don't want to merge it because of the global spinlock etc., that's cool and I understand, but I thought I should at least post to this list so you and any other potentially interested people know it exists. Oh, and, if you're not going to merge it, please let me know so I can know post the patch to GitHub or somewhere. And, if you could include a note in the comments at the top of
bond_main.cor somewhere pointing people to the patch, I'd very much appreciate that. I don't want anyone else to have to endure hours of kernel rebuilds with KASAN enabled if they want this functionality :)
Quoted 5 lines
Quoted 5 lines
Well, it's not my call, I'm not a bonding maintainer. But I believe at least some of the objections would be shared by them. Of course, it's up to you if you want to dedicate your time to improving the code to be acceptable for mainline or rather maintain it out of tree (which may end up taking even more time in the long term).
It'll probably just bit-rot :( The next time it gets looked at will probably be in 4-5 years when one of the MythTV computers dies and I have to replace it and use a newer kernel because it has new hardware that isn't supported.
But it is now and will remain indefinitely at
http://moongate.ydns.eu/bond_mode_batman.patch
for anyone who wants it.
Quoted 14 lines
Well, I am a bonding maintainer, and I can say that the patch in
its current state is not suitable for inclusion.At a minimum, there are many coding style issues, commented out
debug statements, etc, along with design issues (e.g., the batman mode handling inbond_handle_frameis unconditional and takes place for all modes, not just the new batman mode).If you (Patrick) or someone else wishes to contribute this to
mainline, I'd suggest that the first step is to read and follow the instructions inDocumentation/SubmittingPatchesin the kernel source code.
Quoted 8 lines
It is also not feasible to add pointers in the kernel source
code to out-of-tree patches; sorry.-J---
-Jay Vosburgh, [email protected]
Fair enough. I actually did read SubmittingPatches already, but thank you for pointing it out. I could easily fix the coding style issues (I imagine someone's written a "Linux kernel indentation mode" for emacs by now), and I could of course put an if statement around the stuff in bond_handle_frame (that's a bug). What I wouldn't be interested in doing would be things like getting rid of the mode-specific spinlocks, which someone mentioned earlier as a potential issue.
Basically, I'll do bug fixes etc. if that would make it suitable, but not anything like design changes that would be likely to make me have to debug the thing again. So, I guess my question is, would design changes be necessary, or would it be suitable if I fixed the indentation, cleaned up the debug comments, fixed the bug in bond_handle_frame, and did something to handle kmalloc failures?
By the way, I do appreciate the time you all have taken to look at this, and thank you for CCing me and please continue to do so because I didn't realize the list was so heavy traffic, and so I unsubscribed.
--Patrick Simmons
Signature · 1 line
If I'm not here, I've gone out to find myself. If I get back before I return, please keep me here.
Quoted 8 lines · Patrick Simmons
Hello,
I've written a new mode for the kernel bonding driver. It's similar to the round-robin mode, but it keeps statistics on TCP resends so as to favor slave devices with more bandwidth when choosing where to send packets. I've tested it on two laptops using WiFi/Ethernet and it seems to work okay, but it should be considered experimental.
Quoted 5 lines
Still, it should be safe to apply this patch as it doesn't do anything unless you specify "mode=batman" on the driver command line.
Signed-off-by Patrick Simmons <[email protected]>
It is usually a good idea to add the bonding maintainers directly, have added them. See <kernel>/MAINTAINERS for list.