Skip to contentSkip to conversation

bonding: do't permit slaves to change their mtu ndependently

PATCH Superseded Ding Tianhong 4 participants 4 messages Jan 24, 2014

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 bonding: do't permit slaves to change their mtu ndependently

I have come to a conclusion by testing every mode for mtu changing:

1). If the slaves support changing mtu and no need to restart the device,
just like virtual nic, the master will not lost any packages for every mode.

2). If the slaves support changing mtu and need to restart the device,
just like Intel 82599, the AB, 802.3, ALB and TLB mode may lost packages, but other modes could work well.

The reason is that when the slave's mtu has been changed, the slave will restart, if the slave is current active slave, the master may set the slave to backup state and reselect a new slave, after the reselect processing, the master could work again, but if in load-balance mode, the master could select another active slave to send and recv packages.

So the best way to fix the problem is don't permit slave to change their mtu independently.

Cc Jay Vosburgh <[email protected]>
Cc Veaceslav Falico <[email protected]>
Cc Andy Gospodarek <[email protected]>
---
 drivers/net/bonding/bond_main.c | 18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)
drivers/net/bonding/bond_main.c +5 -13
index 2ca949f..e127031c 100644
@@ -2808,20 +2808,12 @@ static int bond_slave_netdev_event(unsigned long event,
 		 * ... Or is it this?
 		 */
 		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.
+	case NETDEV_PRECHANGEMTU:
+		/* The master and slaves should have the
+		 * same mtu, so don't permit slaves to
+		 * change their mtu independently.
 		 */
-		break;
+		return NOTIFY_BAD;
 	case NETDEV_CHANGENAME:
 		/* we don't care if we don't have primary set */
 		if (!USES_PRIMARY(bond->params.mode) ||
Signature · 1 line
1.8.0
maintainer
#1 lore raw

Diag, your commit messages and subject lines chronically have many typos.

Please take the time out necessary to correct such errors before sending your patches.

This subject line shows a complete lack of care.

Subject: Re: [PATCH net-next] bonding: do't permit slaves to change their mtu ndependently

Acked-by
#2 lore raw
Quoted 2 lines · Fri, Jan 24, 2014 at 10:55:38AM CET, [email protected] wrote:

Diag, your commit messages and subject lines chronically have

^^^^

Also typo :)

Quoted 8 lines

many typos.

Please take the time out necessary to correct such errors before sending your patches.

This subject line shows a complete lack of care.

Subject: Re: [PATCH net-next] bonding: do't permit slaves to change their mtu ndependently

This patch (leaving the typos out of the equation) looks correct to me.

Quoted 4 lines

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

#3 lore raw
In reply to David Miller
Quoted 9 lines · David Miller

Diag, your commit messages and subject lines chronically have many typos.

Please take the time out necessary to correct such errors before sending your patches.

This subject line shows a complete lack of care.

I am very sorry, I was not in my office, so I could not resend now by office email, sorry for that.

Ding

Quoted 7 lines

Subject: Re: [PATCH net-next] bonding: do't permit slaves to change their mtu ndependently --
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
.