From: Klemens Nanni Subject: Re: dhcpd(8): use the rdomain it executes with To: David Gwynne , tech@openbsd.org Date: Mon, 9 Jun 2025 01:02:51 +0000 07.06.2025 07:36, David Gwynne пишет: > dhcpd is weird about how it interacts with rdomains. it uses the rdomain > from the first interface given to it on the command line. > > i would argue it's more natural to use the rdomain it's executed > with, which in turn integrates it better with stuff like rcctl. it > also makes it work as expected with the "-u" flag. > > it shuffles some of the address/subnet matching to avoid reporting > overlapping address space if you have multiple interfaces with the same > networks but in separate rdomains. > > i'm not even sure we need to mention rdomains in the manpage with this > change either. I'd remove any mention of it; logs have per-interface reactions, so if you setup rdomans(4) and still run into problems, they're all you need. > > tests? ok? Works and reads OK, but the manual has an earlier untouched mention of rdomain wrt. the interface argmuments. On that: [if [... ifN]] should be just [interface ...], so feel free to add that to your diff in both manual and usage. > > at some point i should introduce dhcpd to sys/queue.h. > > Index: dhcpd.8 > =================================================================== > RCS file: /cvs/src/usr.sbin/dhcpd/dhcpd.8,v > diff -u -p -r1.36 dhcpd.8 > --- dhcpd.8 27 Jun 2024 16:39:31 -0000 1.36 > +++ dhcpd.8 7 Jun 2025 04:29:40 -0000 > @@ -132,15 +132,8 @@ on systems where > is unable to identify non-broadcast interfaces. > .Pp > .Nm > -normally runs in routing domain 0. > -In order to run in another > -.Xr rdomain 4 , > -.Nm > -needs to be started with a list of interfaces > -which share the same routing domain, > -which allows > -.Nm > -to set its own routing domain accordingly. > +will skip interfaces configured in a different routing domain to > +the one it was executed in. > .Pp > DHCP traffic always bypasses IPsec. > Otherwise there could be situations when a server has an IPsec SA for the > Index: dhcpd.c > =================================================================== > RCS file: /cvs/src/usr.sbin/dhcpd/dhcpd.c,v > diff -u -p -r1.61 dhcpd.c > --- dhcpd.c 4 Jun 2025 21:16:25 -0000 1.61 > +++ dhcpd.c 7 Jun 2025 04:29:40 -0000 > @@ -71,6 +71,7 @@ struct group root_group; > u_int16_t server_port; > u_int16_t client_port; > > +int rdomain; > struct passwd *pw; > int log_priority; > int pfpipe[2]; > @@ -87,7 +88,7 @@ char *leased_tab = NULL; > int > main(int argc, char *argv[]) > { > - int ch, cftest = 0, rdomain = -1, udpsockmode = 0; > + int ch, cftest = 0, udpsockmode = 0; > int debug = 0, verbose = 0; > char *sync_iface = NULL; > char *sync_baddr = NULL; > @@ -98,6 +99,8 @@ main(int argc, char *argv[]) > log_init(1, LOG_DAEMON); /* log to stderr until daemonized */ > log_setverbose(1); > > + rdomain = getrtable(); > + > opterr = 0; > while ((ch = getopt(argc, argv, "A:C:L:c:dfl:nu::vY:y:")) != -1) > switch (ch) { > @@ -197,11 +200,7 @@ main(int argc, char *argv[]) > > db_startup(); > if (!udpsockmode || argc > 0) > - discover_interfaces(&rdomain); > - > - if (rdomain != -1) > - if (setrtable(rdomain) == -1) > - fatal("setrtable"); > + discover_interfaces(); > > if (syncsend || syncrecv) { > if (sync_init(sync_iface, sync_baddr, sync_port) == -1) > Index: dhcpd.h > =================================================================== > RCS file: /cvs/src/usr.sbin/dhcpd/dhcpd.h,v > diff -u -p -r1.72 dhcpd.h > --- dhcpd.h 31 May 2025 07:47:45 -0000 1.72 > +++ dhcpd.h 7 Jun 2025 04:29:40 -0000 > @@ -443,7 +443,7 @@ ssize_t receive_packet(struct interface_ > extern struct interface_info *interfaces; > extern struct protocol *protocols; > extern struct dhcpd_timeout *timeouts; > -void discover_interfaces(int *); > +void discover_interfaces(void); > void dispatch(void); > int locate_network(struct packet *); > void got_one(struct protocol *); > Index: dispatch.c > =================================================================== > RCS file: /cvs/src/usr.sbin/dhcpd/dispatch.c,v > diff -u -p -r1.49 dispatch.c > --- dispatch.c 4 Jun 2025 21:16:25 -0000 1.49 > +++ dispatch.c 7 Jun 2025 04:29:40 -0000 > @@ -67,6 +67,8 @@ > #include "log.h" > #include "sync.h" > > +extern int rdomain; > + > struct interface_info *interfaces; > struct protocol *protocols; > struct dhcpd_timeout *timeouts; > @@ -81,14 +83,14 @@ int get_rdomain(char *); > subnet it's on, and add it to the list of interfaces. */ > > void > -discover_interfaces(int *rdomain) > +discover_interfaces(void) > { > struct interface_info *tmp; > struct interface_info *last, *next; > struct subnet *subnet; > struct shared_network *share; > struct sockaddr_in foo; > - int ir = 0, ird; > + int ir; > struct ifreq *tif; > struct ifaddrs *ifap, *ifa; > > @@ -99,11 +101,7 @@ discover_interfaces(int *rdomain) > * If we already have a list of interfaces, the interfaces were > * requested. > */ > - if (interfaces != NULL) > - ir = 1; > - else > - /* must specify an interface when rdomains are used */ > - *rdomain = 0; > + ir = (interfaces != NULL); > > /* Cycle through the list of interfaces looking for IP addresses. */ > for (ifa = ifap; ifa != NULL; ifa = ifa->ifa_next) { > @@ -125,15 +123,6 @@ discover_interfaces(int *rdomain) > if (tmp == NULL && ir) > continue; > > - ird = get_rdomain(ifa->ifa_name); > - if (*rdomain == -1) > - *rdomain = ird; > - else if (*rdomain != ird && ir) > - fatalx("Interface %s is not in rdomain %d", > - tmp->name, *rdomain); > - else if (*rdomain != ird && !ir) > - continue; > - > /* If there isn't already an interface by this name, > allocate one. */ > if (tmp == NULL) { > @@ -150,16 +139,19 @@ discover_interfaces(int *rdomain) > /* If we have the capability, extract link information > and record it in a linked list. */ > if (ifa->ifa_addr->sa_family == AF_LINK) { > - struct sockaddr_dl *sdl = > - ((struct sockaddr_dl *)(ifa->ifa_addr)); > + struct if_data *ifi = ifa->ifa_data; > + struct sockaddr_dl *sdl; > + > + if (rdomain != ifi->ifi_rdomain) > + continue; > + > + sdl = (struct sockaddr_dl *)ifa->ifa_addr; > tmp->index = sdl->sdl_index; > tmp->hw_address.hlen = sdl->sdl_alen; > tmp->hw_address.htype = HTYPE_ETHER; /* XXX */ > memcpy(tmp->hw_address.haddr, > LLADDR(sdl), sdl->sdl_alen); > } else if (ifa->ifa_addr->sa_family == AF_INET) { > - struct iaddr addr; > - > /* Get a pointer to the address... */ > memcpy(&foo, ifa->ifa_addr, sizeof(foo)); > > @@ -182,55 +174,27 @@ discover_interfaces(int *rdomain) > tmp->ifp = tif; > tmp->primary_address = foo.sin_addr; > } > - > - /* Grab the address... */ > - addr.len = 4; > - memcpy(addr.iabuf, &foo.sin_addr.s_addr, addr.len); > - > - /* If there's a registered subnet for this address, > - connect it together... */ > - if ((subnet = find_subnet(addr))) { > - /* If this interface has multiple aliases > - on the same subnet, ignore all but the > - first we encounter. */ > - if (!subnet->interface) { > - subnet->interface = tmp; > - subnet->interface_address = addr; > - } else if (subnet->interface != tmp) { > - log_warnx("Multiple %s %s: %s %s", > - "interfaces match the", > - "same subnet", > - subnet->interface->name, > - tmp->name); > - } > - share = subnet->shared_network; > - if (tmp->shared_network && > - tmp->shared_network != share) { > - log_warnx("Interface %s matches %s", > - tmp->name, > - "multiple shared networks"); > - } else { > - tmp->shared_network = share; > - } > - > - if (!share->interface) { > - share->interface = tmp; > - } else if (share->interface != tmp) { > - log_warnx("Multiple %s %s: %s %s", > - "interfaces match the", > - "same shared network", > - share->interface->name, > - tmp->name); > - } > - } > } > } > > /* Discard interfaces we can't listen on. */ > last = NULL; > for (tmp = interfaces; tmp; tmp = next) { > + struct iaddr addr; > + > next = tmp->next; > > + if (tmp->index == 0) { > + log_warnx("Can't listen on %s - wrong rdomain", > + tmp->name); > + /* Remove tmp from the list of interfaces. */ > + if (!last) > + interfaces = interfaces->next; > + else > + last->next = tmp->next; > + continue; > + } > + > if (!tmp->ifp) { > log_warnx("Can't listen on %s - it has no IP address.", > tmp->name); > @@ -244,6 +208,47 @@ discover_interfaces(int *rdomain) > > memcpy(&foo, &tmp->ifp->ifr_addr, sizeof tmp->ifp->ifr_addr); > > + /* Grab the address... */ > + addr.len = 4; > + memcpy(addr.iabuf, &foo.sin_addr.s_addr, addr.len); > + > + /* If there's a registered subnet for this address, > + connect it together... */ > + if ((subnet = find_subnet(addr))) { > + /* If this interface has multiple aliases > + on the same subnet, ignore all but the > + first we encounter. */ > + if (!subnet->interface) { > + subnet->interface = tmp; > + subnet->interface_address = addr; > + } else if (subnet->interface != tmp) { > + log_warnx("Multiple %s %s: %s %s", > + "interfaces match the", > + "same subnet", > + subnet->interface->name, > + tmp->name); > + } > + share = subnet->shared_network; > + if (tmp->shared_network && > + tmp->shared_network != share) { > + log_warnx("Interface %s matches %s", > + tmp->name, > + "multiple shared networks"); > + } else { > + tmp->shared_network = share; > + } > + > + if (!share->interface) { > + share->interface = tmp; > + } else if (share->interface != tmp) { > + log_warnx("Multiple %s %s: %s %s", > + "interfaces match the", > + "same shared network", > + share->interface->name, > + tmp->name); > + } > + } > + > if (!tmp->shared_network) { > log_warnx("Can't listen on %s - dhcpd.conf has no " > "subnet declaration for %s.", tmp->name, > @@ -616,22 +621,4 @@ remove_protocol(struct protocol *proto) > free(p); > } > } > -} > - > -int > -get_rdomain(char *name) > -{ > - int rv = 0, s; > - struct ifreq ifr; > - > - if ((s = socket(AF_INET, SOCK_DGRAM, 0)) == -1) > - fatal("get_rdomain socket"); > - > - memset(&ifr, 0, sizeof(ifr)); > - strlcpy(ifr.ifr_name, name, sizeof(ifr.ifr_name)); > - if (ioctl(s, SIOCGIFRDOMAIN, (caddr_t)&ifr) != -1) > - rv = ifr.ifr_rdomainid; > - > - close(s); > - return rv; > } >