From 3069dce934d36aac0c7d43c09e51feaf5bb0b96f Mon Sep 17 00:00:00 2001 From: Ido Rosen Date: Fri, 23 Apr 2021 05:34:34 -0400 Subject: [PATCH 1/3] Linux: use broadcast flag with ipvlan interfaces by default. Linux ipvlan interfaces share a MAC address with their siblings and parent physical interface. Before they are assigned an IP address, these virtual interfaces do not receive DHCP OFFER unicast messages because the ipvlan driver does not know to pass them to the virtual interface yet by IP. This chicken-and-egg problem is resolved with two changes: In this patch, we set the broadcast flag for an interface if it belongs to the ipvlan driver, as detected via SIOCETHTOOL ETHTOOL_GDRVINFO. (closes #32) A forthcoming patch will automatically modify the DHCP IAID for ipvlan interfaces so that they do not conflict with the parent (lower/physical) interface IAID. For now, dhcpcd will display a warning log message when conflicting IAID (same MAC address) interfaces are active. (A minor grammar correction is included free of charge.) --- src/common.h | 4 ++++ src/dhcpcd.8.in | 2 +- src/dhcpcd.c | 2 +- src/if-linux.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 50 insertions(+), 2 deletions(-) diff --git a/src/common.h b/src/common.h index ff8f3f8b9..387401649 100644 --- a/src/common.h +++ b/src/common.h @@ -63,6 +63,10 @@ #define ROUNDUP4(a) (1 + (((a) - 1) | 3)) #define ROUNDUP8(a) (1 + (((a) - 1) | 7)) +#ifndef FIELD_SIZEOF +#define FIELD_SIZEOF(t, f) sizeof(((t *)0)->f) +#endif + /* Some systems don't define timespec macros */ #ifndef timespecclear #define timespecclear(tsp) (tsp)->tv_sec = (time_t)((tsp)->tv_nsec = 0L) diff --git a/src/dhcpcd.8.in b/src/dhcpcd.8.in index c307f69f0..81e19e6af 100644 --- a/src/dhcpcd.8.in +++ b/src/dhcpcd.8.in @@ -651,7 +651,7 @@ of a randomly generated number. .It Fl J , Fl Fl broadcast Instructs the DHCP server to broadcast replies back to the client. Normally this is only set for non-Ethernet interfaces, -such as FireWire and InfiniBand. +such as FireWire and InfiniBand, and on Linux-based systems for ipvlan as well. In most instances, .Nm will set this automatically. diff --git a/src/dhcpcd.c b/src/dhcpcd.c index 149a55c82..1c6a65692 100644 --- a/src/dhcpcd.c +++ b/src/dhcpcd.c @@ -470,7 +470,7 @@ configure_interface1(struct interface *ifp) if (!(ifo->options & DHCPCD_IAID)) { /* - * An IAID is for identifying a unqiue interface within + * An IAID is for identifying an unique interface within * the client. It is 4 bytes long. Working out a default * value is problematic. * diff --git a/src/if-linux.c b/src/if-linux.c index b642ab306..3c53a99ab 100644 --- a/src/if-linux.c +++ b/src/if-linux.c @@ -42,6 +42,7 @@ #include #include #include +#include /* for ipvlan detection */ #include #include @@ -309,12 +310,55 @@ if_init(struct interface *ifp) return if_writepathuint(ifp->ctx, path, 1) == -1 ? -1 : 0; } +/* Returns number of bytes written to driver, else 0 (error or indeterminate). */ +static size_t +if_get_driver(struct interface *ifp, char *driver, const size_t driverlen) +{ + struct ethtool_drvinfo drvinfo = { .cmd = ETHTOOL_GDRVINFO }; + struct ifreq ifr = { .ifr_data = (void *)&drvinfo }; + + strlcpy(ifr.ifr_name, ifp->name, sizeof(ifr.ifr_name)); + if (ioctl(ifp->ctx->pf_inet_fd, SIOCETHTOOL, &ifr) != 0) { + logerr("%s: SIOCETHTOOL ifname=%s", __func__, ifp->name); + return 0; /* 0 means error or indeterminate driver name */ + } + return strlcpy(driver, drvinfo.driver, driverlen); +} + +static bool +if_cmp_driver(struct interface *ifp, const char *driver) +{ + char ifdriver[FIELD_SIZEOF(struct ethtool_drvinfo, driver)]; + size_t n = if_get_driver(ifp, ifdriver, sizeof(ifdriver)); + + if (n == 0) { + logerr("%s: if_get_driver ifname=%s", __func__, ifp->name); + return false; + } + if (strncmp(ifdriver, driver, n) == 0) + return true; + return false; +} + +static bool +if_ipvlan(struct interface *ifp) +{ + if (if_cmp_driver(ifp, "ipvlan")) + return true; + return false; +} + int if_conf(struct interface *ifp) { char path[sizeof(SYS_LAYER2) + IF_NAMESIZE]; int n; + /* Set broadcast flag for ipvlan interfaces. + * XXX: move this out to dhcpcd if needed on other platforms. */ + if (if_ipvlan(ifp)) + ifp->options->options |= DHCPCD_BROADCAST; + /* Some qeth setups require the use of the broadcast flag. */ snprintf(path, sizeof(path), SYS_LAYER2, ifp->name); n = check_proc_int(ifp->ctx, path); From a90cbd6ec3620d6c621734f7d11fc76b65862835 Mon Sep 17 00:00:00 2001 From: Ido Rosen Date: Sun, 25 Apr 2021 06:32:13 -0400 Subject: [PATCH 2/3] Linux: change return value of if_get_driver to ssize_t. Differentiate between ioctl error and zero-length driver name in if_get_driver. --- src/if-linux.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/src/if-linux.c b/src/if-linux.c index 3c53a99ab..9d6f5a92b 100644 --- a/src/if-linux.c +++ b/src/if-linux.c @@ -310,8 +310,11 @@ if_init(struct interface *ifp) return if_writepathuint(ifp->ctx, path, 1) == -1 ? -1 : 0; } -/* Returns number of bytes written to driver, else 0 (error or indeterminate). */ -static size_t +/* Return values: + * -1 = ioctl error + * 0 = no driver name or driver name indeterminate + * >0 = length of driver name written to provided buffer. */ +static ssize_t if_get_driver(struct interface *ifp, char *driver, const size_t driverlen) { struct ethtool_drvinfo drvinfo = { .cmd = ETHTOOL_GDRVINFO }; @@ -320,23 +323,25 @@ if_get_driver(struct interface *ifp, char *driver, const size_t driverlen) strlcpy(ifr.ifr_name, ifp->name, sizeof(ifr.ifr_name)); if (ioctl(ifp->ctx->pf_inet_fd, SIOCETHTOOL, &ifr) != 0) { logerr("%s: SIOCETHTOOL ifname=%s", __func__, ifp->name); - return 0; /* 0 means error or indeterminate driver name */ + return -1; /* -1 means ioctl error */ } - return strlcpy(driver, drvinfo.driver, driverlen); + return (ssize_t)strlcpy(driver, drvinfo.driver, driverlen); } static bool if_cmp_driver(struct interface *ifp, const char *driver) { char ifdriver[FIELD_SIZEOF(struct ethtool_drvinfo, driver)]; - size_t n = if_get_driver(ifp, ifdriver, sizeof(ifdriver)); + ssize_t n = if_get_driver(ifp, ifdriver, sizeof(ifdriver)); - if (n == 0) { + if (n == -1) { logerr("%s: if_get_driver ifname=%s", __func__, ifp->name); - return false; + } else if (n == 0) { + logerr("%s: driver name empty ifname=%s", __func__, ifp->name); + } else if (n > 0) { + if (strncmp(ifdriver, driver, n) == 0) + return true; } - if (strncmp(ifdriver, driver, n) == 0) - return true; return false; } From 4b97e2da9e76c2efdc45e08b07f534681dc4d195 Mon Sep 17 00:00:00 2001 From: Ido Rosen Date: Sun, 25 Apr 2021 06:37:23 -0400 Subject: [PATCH 3/3] Linux: fix sign-conversion compiler warning. --- src/if-linux.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/if-linux.c b/src/if-linux.c index 9d6f5a92b..19798cbd6 100644 --- a/src/if-linux.c +++ b/src/if-linux.c @@ -338,10 +338,9 @@ if_cmp_driver(struct interface *ifp, const char *driver) logerr("%s: if_get_driver ifname=%s", __func__, ifp->name); } else if (n == 0) { logerr("%s: driver name empty ifname=%s", __func__, ifp->name); - } else if (n > 0) { - if (strncmp(ifdriver, driver, n) == 0) - return true; } + if ((n >= 0) && (strncmp(ifdriver, driver, (size_t)n) == 0)) + return true; return false; }