Skip to contentSkip to conversation

Re-enable IP when MTU > 68

PATCH In review Breno Leitao 3 participants 6 messages Aug 29, 2008 – Sep 3, 2008

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

PATCH 1/1 Re-enable IP when MTU > 68

Actually if you run "ifconfig ethX mtu 56; ifconfig ethX mtu 1500", then your interface will get corrupted. On an attempt to assign an IP to the interface, it'll result in:
SIOCSIFADDR: No buffer space available
This happens because when decreasing the MTU under 68, it'll destroy the in_dev reference forever, even after getting MTU higher than 68 bytes.

This patch just checks if the in_dev is NULL on a NETDEV_CHANGEMTU event and if MTU is bigger than 68, then re-enable in_dev

---

net/ipv4/devinet.c +4 -0
index 91d3d96..758b4f6 100644
@@ -1048,6 +1048,10 @@ static int inetdev_event(struct notifier_block *this, unsigned long event,
 				IN_DEV_CONF_SET(in_dev, NOXFRM, 1);
 				IN_DEV_CONF_SET(in_dev, NOPOLICY, 1);
 			}
+		} else if (event == NETDEV_CHANGEMTU) {
+			/* Re-enabling IP */
+			if (dev->mtu > 68)
+				in_dev = inetdev_init(dev);
 		}
 		goto out;
 	}
#1 lore raw

Breno Leitao wrote:
[...]

net/ipv4/devinet.c static int inetdev_event(struct notifier_block *this, unsigned long event, · line 1053

Quoted 11 lines
net/ipv4/devinet.c +3 -0
index 91d3d96..758b4f6 100644
@@ -1048,6 +1048,10 @@ static int inetdev_event(struct notifier_block *this, unsigned long event,
 				IN_DEV_CONF_SET(in_dev, NOXFRM, 1);
 				IN_DEV_CONF_SET(in_dev, NOPOLICY, 1);
 			}
+		} else if (event == NETDEV_CHANGEMTU) {
+			/* Re-enabling IP */
+			if (dev->mtu > 68)

[...]

Should be >= to match the other tests. And it would really be cleaner to use something like

	static inline bool inetdev_valid_mtu(unsigned mtu)
	{
		return mtu >= 68;
	}

rather than repeating the magic number.

Ben.

Signature · 3 lines
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
#2 lore raw

Ben,

Quoted 7 lines · Ben Hutchings

Should be >= to match the other tests. And it would really be cleaner to use something like

	static inline bool inetdev_valid_mtu(unsigned mtu)
	{
		return mtu >= 68;
	}

Where is the best place to insert this function ?

maintainer
#3 lore raw

From: Breno Leitao <[email protected]>
Date: Fri, 29 Aug 2008 12:23:03 -0300

Quoted 11 lines

Ben,

Quoted 7 lines · Ben Hutchings

Should be >= to match the other tests. And it would really be cleaner to use something like

	static inline bool inetdev_valid_mtu(unsigned mtu)
	{
		return mtu >= 68;
	}

Where is the best place to insert this function ?

Right above the two places it would be called :)

#4 lore raw

Re-enable IP when the MTU gets back to a valid size.

This patch just checks if the in_dev is NULL on a NETDEV_CHANGEMTU event and if MTU is valid (bigger than 68), then re-enable in_dev.

Also a function that checks valid MTU size was created.

---

net/ipv4/devinet.c +12 -3
index 91d3d96..27193dd 100644
@@ -1029,6 +1029,11 @@ skip:
 	}
 }
 
+static inline bool inetdev_valid_mtu(unsigned mtu)
+{
+	return mtu >= 68;
+}
+
 /* Called only under RTNL semaphore */
 
 static int inetdev_event(struct notifier_block *this, unsigned long event,
@@ -1048,6 +1053,10 @@ static int inetdev_event(struct notifier_block *this, unsigned long event,
 				IN_DEV_CONF_SET(in_dev, NOXFRM, 1);
 				IN_DEV_CONF_SET(in_dev, NOPOLICY, 1);
 			}
+		} else if (event == NETDEV_CHANGEMTU) {
+			/* Re-enabling IP */
+			if (inetdev_valid_mtu(dev->mtu))
+				in_dev = inetdev_init(dev);
 		}
 		goto out;
 	}
@@ -1058,7 +1067,7 @@ static int inetdev_event(struct notifier_block *this, unsigned long event,
 		dev->ip_ptr = NULL;
 		break;
 	case NETDEV_UP:
-		if (dev->mtu < 68)
+		if (!inetdev_valid_mtu(dev->mtu))
 			break;
 		if (dev->flags & IFF_LOOPBACK) {
 			struct in_ifaddr *ifa;
@@ -1080,9 +1089,9 @@ static int inetdev_event(struct notifier_block *this, unsigned long event,
 		ip_mc_down(in_dev);
 		break;
 	case NETDEV_CHANGEMTU:
-		if (dev->mtu >= 68)
+		if (inetdev_valid_mtu(dev->mtu))
 			break;
-		/* MTU falled under 68, disable IP */
+		/* disable IP when MTU is not enough */
 	case NETDEV_UNREGISTER:
 		inetdev_destroy(in_dev);
 		break;
maintainer
#5 lore raw

From: Breno Leitao <[email protected]>
Date: Sat, 30 Aug 2008 14:57:18 -0300

Quoted 8 lines

Re-enable IP when the MTU gets back to a valid size.

This patch just checks if the in_dev is NULL on a NETDEV_CHANGEMTU event and if MTU is valid (bigger than 68), then re-enable in_dev.

Also a function that checks valid MTU size was created.

Looks good, patch applied, thanks.