Skip to contentSkip to conversation

bonding: don't permit slaves to change their mtu

PATCH In review Ding Tianhong 2 participants 3 messages Jan 14, 2014

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

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

The commit 2315dc91a5059d7da9a8b9b9daf78d695c11383e (net: make dev_set_mtu() honor notification return code) will deal with the return value for NETDEV_CHANGEMTU notification, and the slaves should not change their mtu, so add return value to prevent doing it.

---
 drivers/net/bonding/bond_main.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)
drivers/net/bonding/bond_main.c +4 -12
index e06c445..af4e678 100644
@@ -2846,19 +2846,11 @@ 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.
+		/* The master and slaves should have the
+		 * the same mtu, so do't permit slaves
+		 * to change their mtu independently.
 		 */
-		break;
+		return NOTIFY_BAD;
 	case NETDEV_CHANGENAME:
 		/*
 		 * TODO: handle changing the primary's name
Signature · 1 line
1.8.0
#1 lore raw
Quoted 5 lines · Ding Tianhong

The commit 2315dc91a5059d7da9a8b9b9daf78d695c11383e (net: make dev_set_mtu() honor notification return code) will deal with the return value for NETDEV_CHANGEMTU notification, and the slaves should not change their mtu, so add return value to prevent doing it.

In another email you said you've tested the mtu changes and some of the bonds have packet loss when mtu is changed, and some of them don't.

Maybe it'd be good to understand which modes can tolerate the mtu change (if it can be tolerated at all/if it should really matter) and allow it for specific bond modes only/for any bond modes?

Quoted 2 lines

Don't add my name unless I specifically ask you to, please.

Thank you.

Quoted 37 lines

---
drivers/net/bonding/bond_main.c | 16 ++++------------

1 file changed, 4 insertions(+), 12 deletions(-)
drivers/net/bonding/bond_main.c +4 -12
index e06c445..af4e678 100644
@@ -2846,19 +2846,11 @@ 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.
+		/* The master and slaves should have the
+		 * the same mtu, so do't permit slaves
+		 * to change their mtu independently.
 		 */
-		break;
+		return NOTIFY_BAD;
 	case NETDEV_CHANGENAME:
 		/*
 		 * TODO: handle changing the primary's name
Signature · 1 line
1.8.0
#2 lore raw
Quoted 7 lines · Veaceslav Falico
Quoted 5 lines · Ding Tianhong

The commit 2315dc91a5059d7da9a8b9b9daf78d695c11383e (net: make dev_set_mtu() honor notification return code) will deal with the return value for NETDEV_CHANGEMTU notification, and the slaves should not change their mtu, so add return value to prevent doing it.

Quoted 7 lines

In another email you said you've tested the mtu changes and some of the bonds have packet loss when mtu is changed, and some of them don't.

Maybe it'd be good to understand which modes can tolerate the mtu change (if it can be tolerated at all/if it should really matter) and allow it for specific bond modes only/for any bond modes?

Ok, need more analysis.

Quoted 7 lines
Quoted 2 lines

Don't add my name unless I specifically ask you to, please.

Thank you.

Ok

Quoted 43 lines
Quoted 37 lines

---
drivers/net/bonding/bond_main.c | 16 ++++------------

1 file changed, 4 insertions(+), 12 deletions(-)
drivers/net/bonding/bond_main.c +4 -12
index e06c445..af4e678 100644
@@ -2846,19 +2846,11 @@ 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.
+        /* The master and slaves should have the
+         * the same mtu, so do't permit slaves
+         * to change their mtu independently.
         */
-        break;
+        return NOTIFY_BAD;
    case NETDEV_CHANGENAME:
        /*
         * TODO: handle changing the primary's name
Signature · 1 line
1.8.0
Signature · 3 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