Hi, thank you for the patch and the detailed explanation.
I knew that Android is having similar problems under vmd, maybe that's also because of busybox' udhcpc. I have to clarify that vmd does not implement "DHCP" but "BOOTP". I picked BOOTP because it was simpler to implement and totally sufficient for vmd's use case: we don't need lease times, stateful configuration, or any of the fancy DHCP options. Less code and complexity. My assumption was that DHCP is a superset of BOOTP; most DHCP clients support BOOTP responses (with vendor extensions). udhcpc is the first DHCP-only client that I've seen. But now I stumbled over RFC 1534 where it says: "3. DHCP clients and BOOTP servers A DHCP client MAY use a reply from a BOOTP server if the configuration returned from the BOOTP server is acceptable to the DHCP client. A DHCP client MUST assume that an IP address returned in a message from a BOOTP server has an infinite lease. A DHCP client SHOULD choose to use a reply from a DHCP server in preference to a reply from a BOOTP server." So udhcpc is stupid but it is actually not wrong. So I'm wondering if your diff is the right approach: should we add the minimal DHCP-in-BOOTP fields as a workaround for udhcpc or should we rather change it to be some kind of minimal RFC-compliant DHCP? Reyk > On 08.09.2017, at 06:42, Anthony Coulter <[email protected]> wrote: > > The DHCP client available in the Alpine Linux installer (udhcpc, part > of BusyBox) does not accept responses that do not include the DHCP > message type option. Worse, it expects the message type to be > DHCPOFFER in some circumstances and DHCPREQUEST in others. The DHCP > server in vmd omits this option entirely, which makes it impossible > to install Alpine Linux in a virtual machine configured with "-L". > > The simplest fix would be to use "resp.options[6] == DHCPOFFER" instead > of is_discover (see the patch below) because in practice the DHCP > message type will be the first option present after the magic cookie. > This was the first thing I tried, and it worked. But it's incorrect. > > RFC 1534 says that requests with no message type can be treated as > BOOTP and not DHCP messages. It also says that we can send DHCP options > to BOOTP messages if we so desire, so it doesn't really matter whether > we initialize is_discover to zero or one. > > Note that udhcpc also complains about two more options (server ID and > lease time) that are missing from the response message. I didn't do > anything about this because udhcpc uses sensible defaults. > > I've tested this change with udhcpc (in a virtual Alpine Linux system) > and dhclient (in a virtual OpenBSD system) and it works for both. I > have not tried anything else. > > Regards, > Anthony Coulter > > Index: dhcp.c > =================================================================== > RCS file: /cvs/src/usr.sbin/vmd/dhcp.c,v > retrieving revision 1.3 > diff -u -p -u -p -r1.3 dhcp.c > --- dhcp.c 24 Apr 2017 07:14:27 -0000 1.3 > +++ dhcp.c 8 Sep 2017 04:12:10 -0000 > @@ -44,6 +44,7 @@ dhcp_request(struct vionet_dev *dev, cha > struct dhcp_packet req, resp; > struct in_addr in, mask; > size_t resplen, o; > + int is_discover = 1; > > if (buflen < (ssize_t)(BOOTP_MIN_LEN + sizeof(struct ether_header))) > return (-1); > @@ -76,6 +77,15 @@ dhcp_request(struct vionet_dev *dev, cha > if (req.ciaddr.s_addr != 0 || req.file[0] != '\0' || req.hops != 0) > return (-1); > > + for (o = DHCP_OPTIONS_COOKIE_LEN; > + o + offsetof(struct dhcp_packet, options) < buflen && > + req.options[o] != DHO_END; > + o += req.options[o+1] + 2) > + if (req.options[o] == DHO_DHCP_MESSAGE_TYPE) { > + is_discover = (req.options[o+2] == DHCPDISCOVER); > + break; > + } > + > memset(&resp, 0, sizeof(resp)); > resp.op = BOOTREPLY; > resp.htype = req.htype; > @@ -123,6 +133,10 @@ dhcp_request(struct vionet_dev *dev, cha > memcpy(&resp.options, > DHCP_OPTIONS_COOKIE, DHCP_OPTIONS_COOKIE_LEN); > o+= DHCP_OPTIONS_COOKIE_LEN; > + > + resp.options[o++] = DHO_DHCP_MESSAGE_TYPE; > + resp.options[o++] = 1; > + resp.options[o++] = is_discover ? DHCPOFFER : DHCPACK; > > resp.options[o++] = DHO_SUBNET_MASK; > resp.options[o++] = sizeof(mask); >
