DHCP: No longer set interface mtu (#346)

We've been enforcing an interface MTU that is slightly larger
than the minimum for some time.
Instead, log an error than the MTU is smaller than the minimum
to send a BOOTP message.

The DHCP MTU is only used when adding routes as setting the
interface MTU can cause a PHY reset which is bad.

Fixes #345

diff --git a/src/dhcp-common.h b/src/dhcp-common.h
index 514131404004d43e4a7eaca94988f4d2a9764b57..6eaf3b3c6bf73f1849a4e8d11146284ca4501f2e 100644 (file)
--- a/src/dhcp-common.h
+++ b/src/dhcp-common.h
@@ -46,11 +46,6 @@
 #define NS_MAXLABEL MAXLABEL
 #endif
 
-/* Max MTU - defines dhcp option length */
-#define        IP_UDP_SIZE               28
-#define        MTU_MAX                 1500 - IP_UDP_SIZE
-#define        MTU_MIN                  576 + IP_UDP_SIZE
-
 #define        OT_REQUEST              (1 << 0)
 #define        OT_UINT8                (1 << 1)
 #define        OT_INT8                 (1 << 2)

diff --git a/src/dhcp.c b/src/dhcp.c
index 0267a2a9fb292d7cfbe460a0ffa7691f83f2fbab..d7a7e6f685a4492fbd1e7b861ad80abea1529588 100644 (file)
--- a/src/dhcp.c
+++ b/src/dhcp.c
@@ -97,6 +97,8 @@
 __CTASSERT(sizeof(struct ip)           == 20);
 __CTASSERT(sizeof(struct udphdr)       == 8);
 __CTASSERT(sizeof(struct bootp)                == 300);
+#define IP_UDP_SIZE    sizeof(struct ip) + sizeof(struct udphdr)
+#define BOOTP_MIN_MTU  IP_UDP_SIZE + sizeof(struct bootp)
 
 struct dhcp_op {
        uint8_t value;
@@ -676,6 +678,8 @@ dhcp_get_mtu(const struct interface *ifp)
            get_option_uint16(ifp->ctx, &mtu,
                              state->new, state->new_len, DHO_MTU) == -1)
                return 0;
+       if (mtu < IPV4_MMTU)
+               return IPV4_MMTU;
        return mtu;
 }
 
@@ -740,19 +744,24 @@ make_message(struct bootp **bootpm, const struct interface *ifp, uint8_t type)
        uint8_t *auth, auth_len;
 #endif
 
-       if ((mtu = if_getmtu(ifp)) == -1)
+       /* We could take the DHCPv6 approach and work out the
+        * message length up front rather than this big hammer approach. */
+       if ((mtu = if_getmtu(ifp)) == -1) {
                logerr("%s: if_getmtu", ifp->name);
-       else if (mtu < MTU_MIN) {
-               if (if_setmtu(ifp, MTU_MIN) == -1)
-                       logerr("%s: if_setmtu", ifp->name);
-               mtu = MTU_MIN;
+               return -1;
+       }
+       if ((size_t)mtu < BOOTP_MIN_MTU) {
+               logerr("%s: interface mtu is too small (%d<%zu)",
+                   ifp->name, mtu, BOOTP_MIN_MTU);
+               return -1;
        }
 
-       if (ifo->options & DHCPCD_BOOTP)
-               bootp = calloc(1, sizeof (*bootp));
-       else
+       if (ifo->options & DHCPCD_BOOTP) {
+               bootp = calloc(1, sizeof(*bootp));
+       } else {
                /* Make the maximal message we could send */
-               bootp = calloc(1, (size_t)(mtu - IP_UDP_SIZE));
+               bootp = calloc(1, (size_t)mtu - IP_UDP_SIZE);
+       }
 
        if (bootp == NULL)
                return -1;
@@ -797,7 +806,7 @@ make_message(struct bootp **bootpm, const struct interface *ifp, uint8_t type)
                return sizeof(*bootp);
 
        p = bootp->vend;
-       e = (uint8_t *)bootp + (mtu - IP_UDP_SIZE) - 1; /* -1 for DHO_END */
+       e = (uint8_t *)bootp + ((size_t)mtu - IP_UDP_SIZE - 1/* DHO_END */);
 
        ul = htonl(MAGIC_COOKIE);
        memcpy(p, &ul, sizeof(ul));
@@ -924,7 +933,7 @@ make_message(struct bootp **bootpm, const struct interface *ifp, uint8_t type)
                        AREA_CHECK(2);
                        *p++ = DHO_MAXMESSAGESIZE;
                        *p++ = 2;
-                       sz = htons((uint16_t)(mtu - IP_UDP_SIZE));
+                       sz = htons((uint16_t)((size_t)mtu - IP_UDP_SIZE));
                        memcpy(p, &sz, 2);
                        p += 2;
                }

diff --git a/src/if-options.c b/src/if-options.c
index ee8644d20c132c3414a7b089d6166e7234328b1a..4cdc294b838b1d5a5533b688a1c48fa0bd57d159 100644 (file)
--- a/src/if-options.c
+++ b/src/if-options.c
@@ -1218,7 +1218,7 @@ parse_option(struct dhcpcd_ctx *ctx, const char *ifname, struct if_options *ifo,
                        if (p == NULL)
                                break;
                        ifo->mtu = (unsigned int)strtou(p, NULL, 0,
-                           MTU_MIN, MTU_MAX, &e);
+                           IPV4_MMTU, UINT_MAX, &e);
                        if (e) {
                                logerrx("invalid MTU %s", p);
                                return -1;

diff --git a/src/if.c b/src/if.c
index 3ed9f6b38efb43026de3c78e01cb7f47244efa87..5da7d6d7090e46acc721cb3472059c70c176fa47 100644 (file)
--- a/src/if.c
+++ b/src/if.c
@@ -852,27 +852,18 @@ if_loopback(struct dhcpcd_ctx *ctx)
 }
 
 int
-if_domtu(const struct interface *ifp, short int mtu)
+if_getmtu(const struct interface *ifp)
 {
-       int r;
-       struct ifreq ifr;
-
 #ifdef __sun
-       if (mtu == 0)
-               return if_mtu_os(ifp);
-#endif
+       return if_mtu_os(ifp);
+#else
+       struct ifreq ifr = { .ifr_mtu = 0 };
 
-       memset(&ifr, 0, sizeof(ifr));
        strlcpy(ifr.ifr_name, ifp->name, sizeof(ifr.ifr_name));
-       ifr.ifr_mtu = mtu;
-       if (mtu != 0)
-               r = if_ioctl(ifp->ctx, SIOCSIFMTU, &ifr, sizeof(ifr));
-       else
-               r = pioctl(ifp->ctx, SIOCGIFMTU, &ifr, sizeof(ifr));
-
-       if (r == -1)
+       if (pioctl(ifp->ctx, SIOCGIFMTU, &ifr, sizeof(ifr)) == -1)
                return -1;
        return ifr.ifr_mtu;
+#endif
 }
 
 #ifdef ALIAS_ADDR

diff --git a/src/if.h b/src/if.h
index df5f921e63987562c2f3ba0268940d8a2ac21dab..5b7848f0b4ceb77fed5c611710af707db505fd49 100644 (file)
--- a/src/if.h
+++ b/src/if.h
@@ -178,9 +178,7 @@ struct interface *if_find(struct if_head *, const char *);
 struct interface *if_findindex(struct if_head *, unsigned int);
 struct interface *if_loopback(struct dhcpcd_ctx *);
 void if_free(struct interface *);
-int if_domtu(const struct interface *, short int);
-#define if_getmtu(ifp) if_domtu((ifp), 0)
-#define if_setmtu(ifp, mtu) if_domtu((ifp), (mtu))
+int if_getmtu(const struct interface *);
 int if_carrier(struct interface *, const void *);
 bool if_roaming(struct interface *);
 

diff --git a/src/ipv4.h b/src/ipv4.h
index 4f64534a41fe4e9ff79e8d679a5a8c4eb3036cb0..2d8de7d083061a5a74acbb1ba31101ab1d722f96 100644 (file)
--- a/src/ipv4.h
+++ b/src/ipv4.h
@@ -58,6 +58,10 @@
     (((uint32_t)(A) & 0x000000ff) << 24))
 #endif /* BYTE_ORDER */
 
+#ifndef IPV4_MMTU
+#define IPV4_MMTU 68
+#endif
+
 #ifdef __sun
    /* Solaris lacks these defines.
     * While it supports DaD, to seems to only expose IFF_DUPLICATE