[PATCH v4 0/1] Improve validation of --mtu option

[PATCH v4 0/1] Improve validation of --mtu option

[David Gibson]

Here are some of the more straightforward parts of my patches making
various MTU related fixes.  These ones improve how we validate the
--mtu option.

Changes since v3:
 * None, just verified rebase and reposted.
Changes since v2:
 * Don't disable a protocol because the MTU is too small, just warn.
   It's only an advertised MTU, not an enforced MTU

David Gibson (1):
  conf: Be more precise about minimum MTUs

 conf.c | 18 +++++++++++++++---
 ip.h   |  7 +++++++
 util.h |  6 ------
 3 files changed, 22 insertions(+), 9 deletions(-)

-- 
2.48.1

[PATCH v4 1/1] conf: Be more precise about minimum MTUs

[David Gibson]

Currently we reject the -m option if given a value less than ETH_MIN_MTU
(68).  That define is derived from the kernel, but its name is misleading:
it doesn't really have anything to do with Ethernet per se, but is rather
the minimum payload any L2 link must be able to handle in order to carry
IPv4.  For IPv6, it's not sufficient: that requires an MTU of at least
1280.

Newer kernels have better named constants IPV4_MIN_MTU and IPv6_MIN_MTU.
Copy and use those constants instead, along with some more specific error
messages.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 conf.c | 18 +++++++++++++++---
 ip.h   |  7 +++++++
 util.h |  6 ------
 3 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/conf.c b/conf.c
index c5ee07b0..065e7201 100644
--- a/conf.c
+++ b/conf.c
@@ -1663,9 +1663,9 @@ void conf(struct ctx *c, int argc, char **argv)
 			if (errno || *e)
 				die("Invalid MTU: %s", optarg);
 
-			if (mtu && (mtu < ETH_MIN_MTU || mtu > ETH_MAX_MTU)) {
-				die("MTU %lu out of range (%u..%u)", mtu,
-				    ETH_MIN_MTU, ETH_MAX_MTU);
+			if (mtu > ETH_MAX_MTU) {
+				die("MTU %lu too large (max %u)",
+				    mtu, ETH_MAX_MTU);
 			}
 
 			c->mtu = mtu;
@@ -1842,9 +1842,21 @@ void conf(struct ctx *c, int argc, char **argv)
 		c->ifi4 = conf_ip4(ifi4, &c->ip4);
 	if (!v4_only)
 		c->ifi6 = conf_ip6(ifi6, &c->ip6);
+
+	if (c->ifi4 && c->mtu < IPV4_MIN_MTU) {
+		warn("MTU %"PRIu16" is too small for IPv4 (minimum %u)",
+		     c->mtu, IPV4_MIN_MTU);
+	}
+	if (c->ifi6 && c->mtu < IPV6_MIN_MTU) {
+		warn("MTU %"PRIu16" is too small for IPv6 (minimum %u)",
+			     c->mtu, IPV6_MIN_MTU);
+	}
+
 	if ((*c->ip4.ifname_out && !c->ifi4) ||
 	    (*c->ip6.ifname_out && !c->ifi6))
 		die("External interface not usable");
+
+
 	if (!c->ifi4 && !c->ifi6) {
 		info("No external interface as template, switch to local mode");
 
diff --git a/ip.h b/ip.h
index 858cc899..d9099bda 100644
--- a/ip.h
+++ b/ip.h
@@ -105,4 +105,11 @@ static const struct in6_addr in6addr_ll_all_nodes = {
 /* IPv4 Limited Broadcast (RFC 919, Section 7), 255.255.255.255 */
 static const struct in_addr in4addr_broadcast = { 0xffffffff };
 
+#ifndef IPV4_MIN_MTU
+#define IPV4_MIN_MTU		68
+#endif
+#ifndef IPV6_MIN_MTU
+#define IPV6_MIN_MTU		1280
+#endif
+
 #endif /* IP_H */
diff --git a/util.h b/util.h
index 50e96d32..0f70f4d4 100644
--- a/util.h
+++ b/util.h
@@ -34,15 +34,9 @@
 #ifndef ETH_MAX_MTU
 #define ETH_MAX_MTU			USHRT_MAX
 #endif
-#ifndef ETH_MIN_MTU
-#define ETH_MIN_MTU			68
-#endif
 #ifndef IP_MAX_MTU
 #define IP_MAX_MTU			USHRT_MAX
 #endif
-#ifndef IPV6_MIN_MTU
-#define IPV6_MIN_MTU			1280
-#endif
 
 #ifndef MIN
 #define MIN(x, y)		(((x) < (y)) ? (x) : (y))
-- 
@@ -34,15 +34,9 @@
 #ifndef ETH_MAX_MTU
 #define ETH_MAX_MTU			USHRT_MAX
 #endif
-#ifndef ETH_MIN_MTU
-#define ETH_MIN_MTU			68
-#endif
 #ifndef IP_MAX_MTU
 #define IP_MAX_MTU			USHRT_MAX
 #endif
-#ifndef IPV6_MIN_MTU
-#define IPV6_MIN_MTU			1280
-#endif
 
 #ifndef MIN
 #define MIN(x, y)		(((x) < (y)) ? (x) : (y))
-- 
2.48.1

Re: [PATCH v4 0/1] Improve validation of --mtu option

[Stefano Brivio]

On Wed,  5 Mar 2025 17:20:02 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> Here are some of the more straightforward parts of my patches making
> various MTU related fixes.  These ones improve how we validate the
> --mtu option.
> 
> Changes since v3:
>  * None, just verified rebase and reposted.
> Changes since v2:
>  * Don't disable a protocol because the MTU is too small, just warn.
>    It's only an advertised MTU, not an enforced MTU
> 
> David Gibson (1):
>   conf: Be more precise about minimum MTUs

Applied.

-- 
Stefano