| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20100520053403.GA2867@psychotron.redhat.com>
Date: Thu, 20 May 2010 07:34:04 +0200
From: Jiri Pirko <jpirko@...hat.com>
To: Jay Vosburgh <fubar@...ibm.com>
Cc: netdev@...r.kernel.org, davem@...emloft.net,
bonding-devel@...ts.sourceforge.net, monis@...taire.com
Subject: Re: [PATCH net-next-2.6] bonding: move slave MTU handling from sysfs
V2
Thu, May 20, 2010 at 02:07:41AM CEST, fubar@...ibm.com wrote:
>Jiri Pirko <jpirko@...hat.com> wrote:
>
>>V1->V2: corrected res/ret use
>>
>>For some reason, MTU handling (storing, and restoring) is taking place in
>>bond_sysfs. The correct place for this code is in bond_enslave, bond_release.
>>So move it there.
>
> In principle this looks ok, as do the other patches, but none of
>them apply to net-next-2.6 for me except for the "optimize
>tlb_get_least_loaded_slave" patch. It looks like you left out a patch,
>see below.
>
>>Signed-off-by: Jiri Pirko <jpirko@...hat.com>
>>---
>> drivers/net/bonding/bond_main.c | 15 ++++++++++++++-
>> drivers/net/bonding/bond_sysfs.c | 22 ++--------------------
>> 2 files changed, 16 insertions(+), 21 deletions(-)
>>
>>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>>index 5e12462..2c3f9db 100644
>>--- a/drivers/net/bonding/bond_main.c
>>+++ b/drivers/net/bonding/bond_main.c
>>@@ -1533,6 +1533,14 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
>> */
>> new_slave->original_flags = slave_dev->flags;
>>
>>+ /* Save slave's original mtu and then set it to match the bond */
>>+ new_slave->original_mtu = slave_dev->mtu;
>>+ res = dev_set_mtu(slave_dev, bond->dev->mtu);
>>+ if (res) {
>>+ pr_debug("Error %d calling dev_set_mtu\n", res);
>>+ goto err_free;
>>+ }
>>+
>> /*
>> * Save slave's original ("permanent") mac address for modes
>> * that need it, and for restoring it upon release, and then
>>@@ -1550,7 +1558,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
>> res = dev_set_mac_address(slave_dev, &addr);
>> if (res) {
>> pr_debug("Error %d calling set_mac_address\n", res);
>>- goto err_free;
>>+ goto err_restore_mtu;
>> }
>> }
>>
>>@@ -1785,6 +1793,9 @@ err_restore_mac:
>> dev_set_mac_address(slave_dev, &addr);
>> }
>>
>>+err_restore_mtu:
>>+ dev_set_mtu(slave_dev, new_slave->original_mtu);
>>+
>> err_free:
>> kfree(new_slave);
>>
>>@@ -1969,6 +1980,8 @@ int bond_release(struct net_device *bond_dev, struct net_device *slave_dev)
>> dev_set_mac_address(slave_dev, &addr);
>> }
>>
>>+ dev_set_mtu(slave_dev, slave->original_mtu);
>>+
>> slave_dev->priv_flags &= ~(IFF_MASTER_8023AD | IFF_MASTER_ALB |
>> IFF_SLAVE_INACTIVE | IFF_BONDING |
>> IFF_SLAVE_NEEDARP);
>>diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
>>index 392e291..29a7a8a 100644
>>--- a/drivers/net/bonding/bond_sysfs.c
>>+++ b/drivers/net/bonding/bond_sysfs.c
>>@@ -220,7 +220,6 @@ static ssize_t bonding_store_slaves(struct device *d,
>> char command[IFNAMSIZ + 1] = { 0, };
>> char *ifname;
>> int i, res, ret = count;
>>- u32 original_mtu;
>> struct slave *slave;
>> struct net_device *dev = NULL;
>> struct bonding *bond = to_bond(d);
>
> This chunk doesn't apply to net-next-2.6 because your context
>doesn't match; it looks like you've removed the variable "found" in your
>"before" source. On closer inspection, "found" isn't actually used
>meaningfully, so I'm guessing you removed it in a prior patch but didn't
>submit that patch.
>
> If that's the case, could you repost the whole series, with
>sequence numbers?
I don't think that's necessary for now. The patch removing found was posted as a
first one:
http://patchwork.ozlabs.org/patch/52795/
I tried it several times. Patches are cleanly applicable in order I posted it.
Jirka
>
> -J
>
>>@@ -281,18 +280,7 @@ static ssize_t bonding_store_slaves(struct device *d,
>> memcpy(bond->dev->dev_addr, dev->dev_addr,
>> dev->addr_len);
>>
>>- /* Set the slave's MTU to match the bond */
>>- original_mtu = dev->mtu;
>>- res = dev_set_mtu(dev, bond->dev->mtu);
>>- if (res) {
>>- ret = res;
>>- goto out;
>>- }
>>-
>> res = bond_enslave(bond->dev, dev);
>>- bond_for_each_slave(bond, slave, i)
>>- if (strnicmp(slave->dev->name, ifname, IFNAMSIZ) == 0)
>>- slave->original_mtu = original_mtu;
>> if (res)
>> ret = res;
>>
>>@@ -301,23 +289,17 @@ static ssize_t bonding_store_slaves(struct device *d,
>>
>> if (command[0] == '-') {
>> dev = NULL;
>>- original_mtu = 0;
>> bond_for_each_slave(bond, slave, i)
>> if (strnicmp(slave->dev->name, ifname, IFNAMSIZ) == 0) {
>> dev = slave->dev;
>>- original_mtu = slave->original_mtu;
>> break;
>> }
>> if (dev) {
>> pr_info("%s: Removing slave %s\n",
>> bond->dev->name, dev->name);
>>- res = bond_release(bond->dev, dev);
>>- if (res) {
>>+ res = bond_release(bond->dev, dev);
>>+ if (res)
>> ret = res;
>>- goto out;
>>- }
>>- /* set the slave MTU to the default */
>>- dev_set_mtu(dev, original_mtu);
>> } else {
>> pr_err("unable to remove non-existent slave %s for bond %s.\n",
>> ifname, bond->dev->name);
>>--
>>1.6.6.1
>
>---
> -Jay Vosburgh, IBM Linux Technology Center, fubar@...ibm.com
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists