[PATCH net-next-2.6 1/2] mq: support for bonding

[PATCH net-next-2.6 1/2] mq: support for bonding

["Oleg A. Arkhangelsky"]

Make bonding driver multiqueue aware.

Signed-off-by: Oleg A. Arkhangelsky <sysoleg@yandex.ru> 

---

 drivers/net/bonding/bond_main.c |    5 +++--
 drivers/net/bonding/bonding.h   |    1 +
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 430c022..ea4ff33 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -4928,8 +4928,9 @@ int bond_create(struct net *net, const char *name)
 
 	rtnl_lock();
 
-	bond_dev = alloc_netdev(sizeof(struct bonding), name ? name : "",
-				bond_setup);
+	bond_dev = alloc_netdev_mq(sizeof(struct bonding), name ? name : "",
+				bond_setup,
+				min_t(u32, BOND_MAX_TX_QUEUES, num_online_cpus()));
 	if (!bond_dev) {
 		pr_err("%s: eek! can't alloc netdev!\n", name);
 		res = -ENOMEM;
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 257a7a4..4a6cfb4 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -29,6 +29,7 @@
 #define DRV_DESCRIPTION	"Ethernet Channel Bonding Driver"
 
 #define BOND_MAX_ARP_TARGETS	16
+#define BOND_MAX_TX_QUEUES	8 
 
 #define IS_UP(dev)					   \
 	      ((((dev)->flags & IFF_UP) == IFF_UP)	&& \

---

-- 
wbr, Oleg.

Re: [PATCH net-next-2.6 1/2] mq: support for bonding

[Eric Dumazet]

Le samedi 27 février 2010 à 16:27 +0300, "Oleg A. Arkhangelsky" a
écrit :
> 
> Make bonding driver multiqueue aware.
> 

I wish it could be true :)

> Signed-off-by: Oleg A. Arkhangelsky <sysoleg@yandex.ru> 
> 
> ---
> 
>  drivers/net/bonding/bond_main.c |    5 +++--
>  drivers/net/bonding/bonding.h   |    1 +
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 430c022..ea4ff33 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -4928,8 +4928,9 @@ int bond_create(struct net *net, const char *name)
>  
>  	rtnl_lock();
>  
> -	bond_dev = alloc_netdev(sizeof(struct bonding), name ? name : "",
> -				bond_setup);
> +	bond_dev = alloc_netdev_mq(sizeof(struct bonding), name ? name : "",
> +				bond_setup,
> +				min_t(u32, BOND_MAX_TX_QUEUES, num_online_cpus()));
>  	if (!bond_dev) {
>  		pr_err("%s: eek! can't alloc netdev!\n", name);
>  		res = -ENOMEM;
> diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
> index 257a7a4..4a6cfb4 100644
> --- a/drivers/net/bonding/bonding.h
> +++ b/drivers/net/bonding/bonding.h
> @@ -29,6 +29,7 @@
>  #define DRV_DESCRIPTION	"Ethernet Channel Bonding Driver"
>  
>  #define BOND_MAX_ARP_TARGETS	16
> +#define BOND_MAX_TX_QUEUES	8 
>  
>  #define IS_UP(dev)					   \
>  	      ((((dev)->flags & IFF_UP) == IFF_UP)	&& \
> 
> ---
> 

Hi Oleg

1) Could you explain why this arbitrary value (8) was chosen ?

My dev machine has 16 cpus, and my network card has 16 queues per NIC

2) This multi queue support is not a real one, since bond driver is
currently using a per device central lock (bond->lock and
bond->curr_slave_lock).
Every xmit has to get this lock(s) and performance is not optimal.

Re: Re: [PATCH net-next-2.6 1/2] mq: support for bonding

["Oleg A. Arkhangelsky"]

Hi Eric,

27.02.10, 17:29, "Eric Dumazet" <eric.dumazet@gmail.com>:

>  My dev machine has 16 cpus, and my network card has 16 queues per NIC

We should borrow this number from real device when enslaving it, picking
maximum value from among slaves. But the main problem is that we don't
known anything about slaves in bond_create() and there is no way to change
number of tx queues later. Maybe we could solve this by adding new module
parameter for bonding (num_tx_queues)?

>  Every xmit has to get this lock(s) and performance is not optimal.

You're right. I don't notice it. I see two solutions:

1) Convert all rw_locks to RCU mechanism
2) Use plain array instead of linked list to store list of slaves. In this case we
don't need to lock when doing bond_for_each_slave().

.. Or am I wrong? What do you think?

-- 
wbr, Oleg.

Re: Re: [PATCH net-next-2.6 1/2] mq: support for bonding

[Eric Dumazet]

Le dimanche 28 février 2010 à 11:25 +0300, "Oleg A. Arkhangelsky" a
écrit :
> Hi Eric,
> 
> 27.02.10, 17:29, "Eric Dumazet" <eric.dumazet@gmail.com>:
> 
> >  My dev machine has 16 cpus, and my network card has 16 queues per NIC
> 
> We should borrow this number from real device when enslaving it, picking
> maximum value from among slaves. But the main problem is that we don't
> known anything about slaves in bond_create() and there is no way to change
> number of tx queues later. Maybe we could solve this by adding new module
> parameter for bonding (num_tx_queues)?
> 

It would be pretty hard to dynamically adjust number of txqueues
dynamically. Only current choice would be a sysfs parameter, to size
subsequent bond_create() queues, and also visible as a module parameter
so that implicit bond devices have the right number of queues.



> >  Every xmit has to get this lock(s) and performance is not optimal.
> 
> You're right. I don't notice it. I see two solutions:
> 
> 1) Convert all rw_locks to RCU mechanism
> 2) Use plain array instead of linked list to store list of slaves. In this case we
> don't need to lock when doing bond_for_each_slave().
> 

Best thing would be RCU of course, at least for the active/backup mode.