* [PATCH net-2.6] bonding: Bonding driver does not consider the gso_max_size setting of slaves.
@ 2012-11-21 11:18 sarveshwar.bandi
2012-11-21 11:56 ` Neil Horman
2012-11-21 14:43 ` Eric Dumazet
0 siblings, 2 replies; 6+ messages in thread
From: sarveshwar.bandi @ 2012-11-21 11:18 UTC (permalink / raw)
To: davem; +Cc: netdev, Sarveshwar Bandi
From: Sarveshwar Bandi <sarveshwar.bandi@emulex.com>
Patch sets the lowest non-zero gso_max_size value of the slaves during enslave
and detach.
Signed-off-by: Sarveshwar Bandi <sarveshwar.bandi@emulex.com>
---
drivers/net/bonding/bond_main.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index b2530b0..5f19d16 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1198,6 +1198,31 @@ static void bond_attach_slave(struct bonding *bond, struct slave *new_slave)
bond->slave_cnt++;
}
+static void bond_set_gso_max_size(struct bonding *bond)
+{
+ struct slave *slave;
+ struct net_device *bond_dev = bond->dev;
+ unsigned int gso_max_size = 0;
+ bool reset_gso_size = true;
+ int i;
+
+ bond_for_each_slave(bond, slave, i) {
+ if (!slave->dev->gso_max_size)
+ continue;
+
+ reset_gso_size = false;
+
+ if (!gso_max_size ||
+ slave->dev->gso_max_size < gso_max_size)
+ gso_max_size = slave->dev->gso_max_size;
+ }
+
+ if (gso_max_size && gso_max_size < bond_dev->gso_max_size)
+ netif_set_gso_max_size(bond_dev, gso_max_size);
+ else if (reset_gso_size)
+ netif_set_gso_max_size(bond_dev, 0);
+}
+
/*
* This function detaches the slave from the list.
* WARNING: no check is made to verify if the slave effectively
@@ -1403,6 +1428,8 @@ done:
flags = bond_dev->priv_flags & ~IFF_XMIT_DST_RELEASE;
bond_dev->priv_flags = flags | dst_release_flag;
+ bond_set_gso_max_size(bond);
+
read_unlock(&bond->lock);
netdev_change_features(bond_dev);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net-2.6] bonding: Bonding driver does not consider the gso_max_size setting of slaves.
2012-11-21 11:18 [PATCH net-2.6] bonding: Bonding driver does not consider the gso_max_size setting of slaves sarveshwar.bandi
@ 2012-11-21 11:56 ` Neil Horman
2012-11-21 12:03 ` Bandi,Sarveshwar
2012-11-21 14:44 ` Eric Dumazet
2012-11-21 14:43 ` Eric Dumazet
1 sibling, 2 replies; 6+ messages in thread
From: Neil Horman @ 2012-11-21 11:56 UTC (permalink / raw)
To: sarveshwar.bandi; +Cc: davem, netdev
On Wed, Nov 21, 2012 at 04:48:56PM +0530, sarveshwar.bandi@emulex.com wrote:
> From: Sarveshwar Bandi <sarveshwar.bandi@emulex.com>
>
> Patch sets the lowest non-zero gso_max_size value of the slaves during enslave
> and detach.
>
> Signed-off-by: Sarveshwar Bandi <sarveshwar.bandi@emulex.com>
> ---
> drivers/net/bonding/bond_main.c | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index b2530b0..5f19d16 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -1198,6 +1198,31 @@ static void bond_attach_slave(struct bonding *bond, struct slave *new_slave)
> bond->slave_cnt++;
> }
>
> +static void bond_set_gso_max_size(struct bonding *bond)
> +{
> + struct slave *slave;
> + struct net_device *bond_dev = bond->dev;
> + unsigned int gso_max_size = 0;
> + bool reset_gso_size = true;
> + int i;
> +
> + bond_for_each_slave(bond, slave, i) {
> + if (!slave->dev->gso_max_size)
> + continue;
> +
> + reset_gso_size = false;
> +
> + if (!gso_max_size ||
> + slave->dev->gso_max_size < gso_max_size)
> + gso_max_size = slave->dev->gso_max_size;
> + }
> +
> + if (gso_max_size && gso_max_size < bond_dev->gso_max_size)
> + netif_set_gso_max_size(bond_dev, gso_max_size);
> + else if (reset_gso_size)
> + netif_set_gso_max_size(bond_dev, 0);
> +}
> +
This seems a bit overly complex. It doesn't seem like you need the
reset_go_size bool in here at all. Just initalize gso_max_size to GSO_MAX_SIZE
and reduce it every time you find a smaller gso value on a slave. Then you can
unilaterally set the bond devices gso_max_size without having to check any bools
or do any comparisons.
Regards
Neil
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH net-2.6] bonding: Bonding driver does not consider the gso_max_size setting of slaves.
2012-11-21 11:56 ` Neil Horman
@ 2012-11-21 12:03 ` Bandi,Sarveshwar
2012-11-21 14:44 ` Eric Dumazet
1 sibling, 0 replies; 6+ messages in thread
From: Bandi,Sarveshwar @ 2012-11-21 12:03 UTC (permalink / raw)
To: Neil Horman; +Cc: davem, netdev
Understood. Will respin the patch.
-----Original Message-----
From: Neil Horman [mailto:nhorman@tuxdriver.com]
Sent: Wednesday, November 21, 2012 5:26 PM
To: Bandi,Sarveshwar
Cc: davem@davemloft.net; netdev@vger.kernel.org
Subject: Re: [PATCH net-2.6] bonding: Bonding driver does not consider the gso_max_size setting of slaves.
On Wed, Nov 21, 2012 at 04:48:56PM +0530, sarveshwar.bandi@emulex.com wrote:
> From: Sarveshwar Bandi <sarveshwar.bandi@emulex.com>
>
> Patch sets the lowest non-zero gso_max_size value of the slaves during
> enslave and detach.
>
> Signed-off-by: Sarveshwar Bandi <sarveshwar.bandi@emulex.com>
> ---
> drivers/net/bonding/bond_main.c | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/drivers/net/bonding/bond_main.c
> b/drivers/net/bonding/bond_main.c index b2530b0..5f19d16 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -1198,6 +1198,31 @@ static void bond_attach_slave(struct bonding *bond, struct slave *new_slave)
> bond->slave_cnt++;
> }
>
> +static void bond_set_gso_max_size(struct bonding *bond) {
> + struct slave *slave;
> + struct net_device *bond_dev = bond->dev;
> + unsigned int gso_max_size = 0;
> + bool reset_gso_size = true;
> + int i;
> +
> + bond_for_each_slave(bond, slave, i) {
> + if (!slave->dev->gso_max_size)
> + continue;
> +
> + reset_gso_size = false;
> +
> + if (!gso_max_size ||
> + slave->dev->gso_max_size < gso_max_size)
> + gso_max_size = slave->dev->gso_max_size;
> + }
> +
> + if (gso_max_size && gso_max_size < bond_dev->gso_max_size)
> + netif_set_gso_max_size(bond_dev, gso_max_size);
> + else if (reset_gso_size)
> + netif_set_gso_max_size(bond_dev, 0); }
> +
This seems a bit overly complex. It doesn't seem like you need the reset_go_size bool in here at all. Just initalize gso_max_size to GSO_MAX_SIZE and reduce it every time you find a smaller gso value on a slave. Then you can unilaterally set the bond devices gso_max_size without having to check any bools or do any comparisons.
Regards
Neil
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-2.6] bonding: Bonding driver does not consider the gso_max_size setting of slaves.
2012-11-21 11:18 [PATCH net-2.6] bonding: Bonding driver does not consider the gso_max_size setting of slaves sarveshwar.bandi
2012-11-21 11:56 ` Neil Horman
@ 2012-11-21 14:43 ` Eric Dumazet
2012-11-21 14:44 ` Bandi,Sarveshwar
1 sibling, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2012-11-21 14:43 UTC (permalink / raw)
To: sarveshwar.bandi; +Cc: davem, netdev
On Wed, 2012-11-21 at 16:48 +0530, sarveshwar.bandi@emulex.com wrote:
> From: Sarveshwar Bandi <sarveshwar.bandi@emulex.com>
>
> Patch sets the lowest non-zero gso_max_size value of the slaves during enslave
> and detach.
>
> Signed-off-by: Sarveshwar Bandi <sarveshwar.bandi@emulex.com>
> ---
> drivers/net/bonding/bond_main.c | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index b2530b0..5f19d16 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -1198,6 +1198,31 @@ static void bond_attach_slave(struct bonding *bond, struct slave *new_slave)
> bond->slave_cnt++;
> }
>
> +static void bond_set_gso_max_size(struct bonding *bond)
> +{
> + struct slave *slave;
> + struct net_device *bond_dev = bond->dev;
> + unsigned int gso_max_size = 0;
> + bool reset_gso_size = true;
> + int i;
> +
> + bond_for_each_slave(bond, slave, i) {
> + if (!slave->dev->gso_max_size)
> + continue;
> +
> + reset_gso_size = false;
> +
> + if (!gso_max_size ||
> + slave->dev->gso_max_size < gso_max_size)
> + gso_max_size = slave->dev->gso_max_size;
> + }
> +
> + if (gso_max_size && gso_max_size < bond_dev->gso_max_size)
> + netif_set_gso_max_size(bond_dev, gso_max_size);
> + else if (reset_gso_size)
> + netif_set_gso_max_size(bond_dev, 0);
> +}
> +
This seems a bit complex, and I have no idea why you call
netif_set_gso_max_size(bond_dev, 0);
Default gso_max_size is GSO_MAX_SIZE, not 0
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH net-2.6] bonding: Bonding driver does not consider the gso_max_size setting of slaves.
2012-11-21 14:43 ` Eric Dumazet
@ 2012-11-21 14:44 ` Bandi,Sarveshwar
0 siblings, 0 replies; 6+ messages in thread
From: Bandi,Sarveshwar @ 2012-11-21 14:44 UTC (permalink / raw)
To: Eric Dumazet; +Cc: davem, netdev
Eric,
I realized the good up and I have reposted this patch. Please review the latest.
Thanks,
Sarvesh
-----Original Message-----
From: Eric Dumazet [mailto:eric.dumazet@gmail.com]
Sent: Wednesday, November 21, 2012 8:14 PM
To: Bandi,Sarveshwar
Cc: davem@davemloft.net; netdev@vger.kernel.org
Subject: Re: [PATCH net-2.6] bonding: Bonding driver does not consider the gso_max_size setting of slaves.
On Wed, 2012-11-21 at 16:48 +0530, sarveshwar.bandi@emulex.com wrote:
> From: Sarveshwar Bandi <sarveshwar.bandi@emulex.com>
>
> Patch sets the lowest non-zero gso_max_size value of the slaves during
> enslave and detach.
>
> Signed-off-by: Sarveshwar Bandi <sarveshwar.bandi@emulex.com>
> ---
> drivers/net/bonding/bond_main.c | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/drivers/net/bonding/bond_main.c
> b/drivers/net/bonding/bond_main.c index b2530b0..5f19d16 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -1198,6 +1198,31 @@ static void bond_attach_slave(struct bonding *bond, struct slave *new_slave)
> bond->slave_cnt++;
> }
>
> +static void bond_set_gso_max_size(struct bonding *bond) {
> + struct slave *slave;
> + struct net_device *bond_dev = bond->dev;
> + unsigned int gso_max_size = 0;
> + bool reset_gso_size = true;
> + int i;
> +
> + bond_for_each_slave(bond, slave, i) {
> + if (!slave->dev->gso_max_size)
> + continue;
> +
> + reset_gso_size = false;
> +
> + if (!gso_max_size ||
> + slave->dev->gso_max_size < gso_max_size)
> + gso_max_size = slave->dev->gso_max_size;
> + }
> +
> + if (gso_max_size && gso_max_size < bond_dev->gso_max_size)
> + netif_set_gso_max_size(bond_dev, gso_max_size);
> + else if (reset_gso_size)
> + netif_set_gso_max_size(bond_dev, 0); }
> +
This seems a bit complex, and I have no idea why you call netif_set_gso_max_size(bond_dev, 0);
Default gso_max_size is GSO_MAX_SIZE, not 0
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-2.6] bonding: Bonding driver does not consider the gso_max_size setting of slaves.
2012-11-21 11:56 ` Neil Horman
2012-11-21 12:03 ` Bandi,Sarveshwar
@ 2012-11-21 14:44 ` Eric Dumazet
1 sibling, 0 replies; 6+ messages in thread
From: Eric Dumazet @ 2012-11-21 14:44 UTC (permalink / raw)
To: Neil Horman; +Cc: sarveshwar.bandi, davem, netdev
On Wed, 2012-11-21 at 06:56 -0500, Neil Horman wrote:
> This seems a bit overly complex. It doesn't seem like you need the
> reset_go_size bool in here at all. Just initalize gso_max_size to GSO_MAX_SIZE
> and reduce it every time you find a smaller gso value on a slave. Then you can
> unilaterally set the bond devices gso_max_size without having to check any bools
> or do any comparisons.
Oh well, I should have read your answer ;)
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-11-21 14:44 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-11-21 11:18 [PATCH net-2.6] bonding: Bonding driver does not consider the gso_max_size setting of slaves sarveshwar.bandi
2012-11-21 11:56 ` Neil Horman
2012-11-21 12:03 ` Bandi,Sarveshwar
2012-11-21 14:44 ` Eric Dumazet
2012-11-21 14:43 ` Eric Dumazet
2012-11-21 14:44 ` Bandi,Sarveshwar
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.