Patchwork [net] bonding: reset the slave's mtu when its be changed

login
register
mail settings
Submitter dingtianhong
Date Jan. 10, 2014, 11:32 a.m.
Message ID <52CFDA63.8070601@huawei.com>
Download mbox | patch
Permalink /patch/309256/
State Changes Requested
Delegated to: David Miller
Headers show

Comments

dingtianhong - Jan. 10, 2014, 11:32 a.m.
All slave should have the same mtu with mastet's, and the bond do it when
enslave the slave, but the user could change the slave's mtu, it will cause
the master and slave have different mtu, althrough in AB mode, it does not
matter if the slave is not the current slave, but in other mode, it is incorrect,
so reset the slave's mtu like the master set.

Cc: Jay Vosburgh <fubar@us.ibm.com>
Cc: Veaceslav Falico <vfalico@redhat.com>
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
 drivers/net/bonding/bond_main.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

--
1.8.0


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - Jan. 10, 2014, 6:33 p.m.
From: Ding Tianhong <dingtianhong@huawei.com>
Date: Fri, 10 Jan 2014 19:32:51 +0800

> All slave should have the same mtu with mastet's, and the bond do it when
                                          ^^^^^^

Typo, you mean "master"
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sergei Shtylyov - Jan. 10, 2014, 7:41 p.m.
Hello.

On 01/10/2014 02:32 PM, Ding Tianhong wrote:

> All slave should have the same mtu with mastet's, and the bond do it when

    Only "master", already noted by Dave.

> enslave the slave, but the user could change the slave's mtu, it will cause
> the master and slave have different mtu, althrough in AB mode, it does not

    Only "although".

> matter if the slave is not the current slave, but in other mode, it is incorrect,
> so reset the slave's mtu like the master set.

> Cc: Jay Vosburgh <fubar@us.ibm.com>
> Cc: Veaceslav Falico <vfalico@redhat.com>
> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
> ---
>   drivers/net/bonding/bond_main.c | 21 ++++++++++-----------
>   1 file changed, 10 insertions(+), 11 deletions(-)

> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 398e299..e7b5bcf 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -2882,18 +2882,17 @@ static int bond_slave_netdev_event(unsigned long event,
>   		 */
>   		break;
>   	case NETDEV_CHANGEMTU:
> -		/*
> -		 * TODO: Should slaves be allowed to
> -		 * independently alter their MTU?  For
> -		 * an active-backup bond, slaves need
> -		 * not be the same type of device, so
> -		 * MTUs may vary.  For other modes,
> -		 * slaves arguably should have the
> -		 * same MTUs. To do this, we'd need to
> -		 * take over the slave's change_mtu
> -		 * function for the duration of their
> -		 * servitude.
> +		/* All slave should have the same mtu
> +		 * as master.
>   		 */
> +		if (slave->dev->mtu != bond->dev->mtu) {
> +			int res;

    Please insert empty line after declaration.

> +			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 for slave %s\n",
> +					 res, slave->dev->name);

    {} wouldn't hurt around multi-line *if* arm.

> +		}
>   		break;
>   	case NETDEV_CHANGENAME:
>   		/*

WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 398e299..e7b5bcf 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2882,18 +2882,17 @@  static int bond_slave_netdev_event(unsigned long event,
 		 */
 		break;
 	case NETDEV_CHANGEMTU:
-		/*
-		 * TODO: Should slaves be allowed to
-		 * independently alter their MTU?  For
-		 * an active-backup bond, slaves need
-		 * not be the same type of device, so
-		 * MTUs may vary.  For other modes,
-		 * slaves arguably should have the
-		 * same MTUs. To do this, we'd need to
-		 * take over the slave's change_mtu
-		 * function for the duration of their
-		 * servitude.
+		/* All slave should have the same mtu
+		 * as master.
 		 */
+		if (slave->dev->mtu != bond->dev->mtu) {
+			int res;
+			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 for slave %s\n",
+					 res, slave->dev->name);
+		}
 		break;
 	case NETDEV_CHANGENAME:
 		/*