Re: vmd: add minimal DHCP support
On Sun, Nov 05, 2017 at 09:34:53PM +0100, Reyk Floeter wrote: > On Thu, Nov 02, 2017 at 10:26:55PM +0100, Reyk Floeter wrote: > > Hi, > > > > the problem got reported a few times and a similar diff was floating > > around: vmd's "local interface" implements a simple auto-magic BOOTP > > server, but udhcpc doesn't support BOOTP, which is the predecessor and > > a valid subset of DHCP. udhcpc is used by busybox and many embedded > > Linux distributions, maybe even by Android. > > > > The following diff adds minimal DHCP support which works with udhcpc. > > > > Thanks Carlos and Mike, I just committed the change. > > Here is a practical example of the actual difference: > > 1. Before the change in simple stateless BOOTP mode: > > ``` > # dhclient vio0 > DHCPDISCOVER on vio0 - interval 1 > BOOTREPLY from 100.64.1.2 (fe:e1:bb:d1:a9:65) > bound to 100.64.1.3 -- renewal in 8000 seconds. > > # cat /var/db/dhclient.leases.vio0 > lease { > bootp; > interface "vio0"; > fixed-address 100.64.1.3; > next-server 100.64.1.2; > option subnet-mask 255.255.255.254; > option routers 100.64.1.2; > option domain-name-servers 100.64.1.2; > option dhcp-lease-time 12000; > option dhcp-renewal-time 8000; > option dhcp-rebinding-time 1; > renew 0 2017/11/05 22:03:14 UTC; > rebind 0 2017/11/05 22:36:34 UTC; > expire 0 2017/11/05 23:09:54 UTC; > } > ``` > > 2. After the change in minimal DHCP mode: > > ``` > # dhclient vio0 > DHCPDISCOVER on vio0 - interval 1 > DHCPOFFER from 100.64.1.2 (fe:e1:bb:d1:09:1f) > DHCPREQUEST on vio0 to 255.255.255.255 > DHCPACK from 100.64.1.2 (fe:e1:bb:d1:09:1f) > bound to 100.64.1.3 -- renewal in 2147483647 seconds. > > # cat /var/db/dhclient.leases.vio0 > lease { > interface "vio0"; > fixed-address 100.64.1.3; > next-server 100.64.1.2; > option subnet-mask 255.255.255.254; > option routers 100.64.1.2; > option domain-name-servers 100.64.1.2,100.64.1.2; > option dhcp-lease-time 4294967295; > option dhcp-message-type 5; > option dhcp-server-identifier 100.64.1.2; > renew 5 2085/11/23 23:02:27 UTC; > rebind 6 2136/12/08 07:28:03 UTC; > expire 4 2153/12/13 02:16:35 UTC; > } > ``` > > As the lease never changes, I'm setting the lease time to 0x, > as RFC 2132 section 9.2 defines it as "The time is in units of > seconds, and is specified as a 32-bit unsigned integer". This doesn't > seem to cause any problems on dhclient or udhpc but it would be > interesting to find out if there is any other DHCP implementation that > handles it as "int" ... > > Reyk > Thanks reyk for taking the time here to fix this. -ml
Re: vmd: add minimal DHCP support
On Thu, Nov 02, 2017 at 10:26:55PM +0100, Reyk Floeter wrote: > Hi, > > the problem got reported a few times and a similar diff was floating > around: vmd's "local interface" implements a simple auto-magic BOOTP > server, but udhcpc doesn't support BOOTP, which is the predecessor and > a valid subset of DHCP. udhcpc is used by busybox and many embedded > Linux distributions, maybe even by Android. > > The following diff adds minimal DHCP support which works with udhcpc. > Thanks Carlos and Mike, I just committed the change. Here is a practical example of the actual difference: 1. Before the change in simple stateless BOOTP mode: ``` # dhclient vio0 DHCPDISCOVER on vio0 - interval 1 BOOTREPLY from 100.64.1.2 (fe:e1:bb:d1:a9:65) bound to 100.64.1.3 -- renewal in 8000 seconds. # cat /var/db/dhclient.leases.vio0 lease { bootp; interface "vio0"; fixed-address 100.64.1.3; next-server 100.64.1.2; option subnet-mask 255.255.255.254; option routers 100.64.1.2; option domain-name-servers 100.64.1.2; option dhcp-lease-time 12000; option dhcp-renewal-time 8000; option dhcp-rebinding-time 1; renew 0 2017/11/05 22:03:14 UTC; rebind 0 2017/11/05 22:36:34 UTC; expire 0 2017/11/05 23:09:54 UTC; } ``` 2. After the change in minimal DHCP mode: ``` # dhclient vio0 DHCPDISCOVER on vio0 - interval 1 DHCPOFFER from 100.64.1.2 (fe:e1:bb:d1:09:1f) DHCPREQUEST on vio0 to 255.255.255.255 DHCPACK from 100.64.1.2 (fe:e1:bb:d1:09:1f) bound to 100.64.1.3 -- renewal in 2147483647 seconds. # cat /var/db/dhclient.leases.vio0 lease { interface "vio0"; fixed-address 100.64.1.3; next-server 100.64.1.2; option subnet-mask 255.255.255.254; option routers 100.64.1.2; option domain-name-servers 100.64.1.2,100.64.1.2; option dhcp-lease-time 4294967295; option dhcp-message-type 5; option dhcp-server-identifier 100.64.1.2; renew 5 2085/11/23 23:02:27 UTC; rebind 6 2136/12/08 07:28:03 UTC; expire 4 2153/12/13 02:16:35 UTC; } ``` As the lease never changes, I'm setting the lease time to 0x, as RFC 2132 section 9.2 defines it as "The time is in units of seconds, and is specified as a 32-bit unsigned integer". This doesn't seem to cause any problems on dhclient or udhpc but it would be interesting to find out if there is any other DHCP implementation that handles it as "int" ... Reyk
Re: vmd: add minimal DHCP support
On Thu, Nov 02, 2017 at 10:26:55PM +0100, Reyk Floeter wrote: > Hi, > > the problem got reported a few times and a similar diff was floating > around: vmd's "local interface" implements a simple auto-magic BOOTP > server, but udhcpc doesn't support BOOTP, which is the predecessor and > a valid subset of DHCP. udhcpc is used by busybox and many embedded > Linux distributions, maybe even by Android. > > The following diff adds minimal DHCP support which works with udhcpc. > > OK? > > Reyk > no objections here, thanks. -ml > Index: usr.sbin/vmd/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 > --- usr.sbin/vmd/dhcp.c 24 Apr 2017 07:14:27 - 1.3 > +++ usr.sbin/vmd/dhcp.c 2 Nov 2017 21:15:03 - > @@ -22,6 +22,7 @@ > #include > #include > #include > +#include > > #include > #include > @@ -38,12 +39,13 @@ extern struct vmd *env; > ssize_t > dhcp_request(struct vionet_dev *dev, char *buf, size_t buflen, char **obuf) > { > - unsigned char *respbuf = NULL; > + unsigned char *respbuf = NULL, *op, *oe, dhcptype = 0; > ssize_t offset, respbuflen = 0; > struct packet_ctxpc; > struct dhcp_packet req, resp; > - struct in_addr in, mask; > + struct in_addr server_addr, mask, client_addr, requested_addr; > size_t resplen, o; > + uint32_t ltime; > > if (buflen < (ssize_t)(BOOTP_MIN_LEN + sizeof(struct ether_header))) > return (-1); > @@ -76,24 +78,54 @@ dhcp_request(struct vionet_dev *dev, cha > if (req.ciaddr.s_addr != 0 || req.file[0] != '\0' || req.hops != 0) > return (-1); > > + /* Get a few DHCP options (best effort as we fall back to BOOTP) */ > + if (memcmp(, > + DHCP_OPTIONS_COOKIE, DHCP_OPTIONS_COOKIE_LEN) == 0) { > + memset(_addr, 0, sizeof(requested_addr)); > + op = req.options + DHCP_OPTIONS_COOKIE_LEN; > + oe = req.options + sizeof(req.options); > + while (*op != DHO_END && op < oe) { > + if (op[0] == DHO_PAD) { > + op++; > + continue; > + } > + if (op + 1 + op[1] >= oe) > + break; > + if (op[0] == DHO_DHCP_MESSAGE_TYPE && > + op[1] == 1) > + dhcptype = op[2]; > + else if (op[0] == DHO_DHCP_REQUESTED_ADDRESS && > + op[1] == sizeof(requested_addr)) > + memcpy(_addr, [2], > + sizeof(requested_addr)); > + op += 2 + op[1]; > + } > + } > + > memset(, 0, sizeof(resp)); > resp.op = BOOTREPLY; > resp.htype = req.htype; > resp.hlen = req.hlen; > resp.xid = req.xid; > > - if ((in.s_addr = vm_priv_addr(>vmd_cfg.cfg_localprefix, > + if ((client_addr.s_addr = > + vm_priv_addr(>vmd_cfg.cfg_localprefix, > dev->vm_vmid, dev->idx, 1)) == 0) > return (-1); > - memcpy(, , sizeof(in)); > - memcpy((_dst)->sin_addr, , sizeof(in)); > + memcpy(, _addr, > + sizeof(client_addr)); > + memcpy((_dst)->sin_addr, _addr, > + sizeof(client_addr)); > ss2sin(_dst)->sin_port = htons(CLIENT_PORT); > > - if ((in.s_addr = vm_priv_addr(>vmd_cfg.cfg_localprefix, > + if ((server_addr.s_addr = > + vm_priv_addr(>vmd_cfg.cfg_localprefix, > dev->vm_vmid, dev->idx, 0)) == 0) > return (-1); > - memcpy(, , sizeof(in)); > - memcpy((_src)->sin_addr, , sizeof(in)); > + memcpy(, _addr, > + sizeof(server_addr)); > + memcpy((_src)->sin_addr, _addr, > + sizeof(server_addr)); > ss2sin(_src)->sin_port = htons(SERVER_PORT); > > /* Packet is already allocated */ > @@ -124,6 +156,44 @@ dhcp_request(struct vionet_dev *dev, cha > DHCP_OPTIONS_COOKIE, DHCP_OPTIONS_COOKIE_LEN); > o+= DHCP_OPTIONS_COOKIE_LEN; > > + /* Did we receive a DHCP request or was it just BOOTP? */ > + if (dhcptype) { > + /* > + * There is no need for a real state machine as we always > + * answer with the same client IP and options for the VM. > + */ > + if (dhcptype == DHCPDISCOVER) > + dhcptype = DHCPOFFER; > + else if (dhcptype == DHCPREQUEST && > + (requested_addr.s_addr == 0 || > + client_addr.s_addr == requested_addr.s_addr)) > + dhcptype = DHCPACK; > + else > + dhcptype = DHCPNAK; > + > + resp.options[o++] =
Re: vmd: add minimal DHCP support
On 11/02/17 14:26, Reyk Floeter wrote: Hi, the problem got reported a few times and a similar diff was floating around: vmd's "local interface" implements a simple auto-magic BOOTP server, but udhcpc doesn't support BOOTP, which is the predecessor and a valid subset of DHCP. udhcpc is used by busybox and many embedded Linux distributions, maybe even by Android. The following diff adds minimal DHCP support which works with udhcpc. OK? Reyk Nice patch. +1 +--+ Carlos Index: usr.sbin/vmd/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 --- usr.sbin/vmd/dhcp.c 24 Apr 2017 07:14:27 - 1.3 +++ usr.sbin/vmd/dhcp.c 2 Nov 2017 21:15:03 - @@ -22,6 +22,7 @@ #include #include #include +#include #include #include @@ -38,12 +39,13 @@ extern struct vmd *env; ssize_t dhcp_request(struct vionet_dev *dev, char *buf, size_t buflen, char **obuf) { - unsigned char *respbuf = NULL; + unsigned char *respbuf = NULL, *op, *oe, dhcptype = 0; ssize_t offset, respbuflen = 0; struct packet_ctxpc; struct dhcp_packet req, resp; - struct in_addr in, mask; + struct in_addr server_addr, mask, client_addr, requested_addr; size_t resplen, o; + uint32_t ltime; if (buflen < (ssize_t)(BOOTP_MIN_LEN + sizeof(struct ether_header))) return (-1); @@ -76,24 +78,54 @@ dhcp_request(struct vionet_dev *dev, cha if (req.ciaddr.s_addr != 0 || req.file[0] != '\0' || req.hops != 0) return (-1); + /* Get a few DHCP options (best effort as we fall back to BOOTP) */ + if (memcmp(, + DHCP_OPTIONS_COOKIE, DHCP_OPTIONS_COOKIE_LEN) == 0) { + memset(_addr, 0, sizeof(requested_addr)); + op = req.options + DHCP_OPTIONS_COOKIE_LEN; + oe = req.options + sizeof(req.options); + while (*op != DHO_END && op < oe) { + if (op[0] == DHO_PAD) { + op++; + continue; + } + if (op + 1 + op[1] >= oe) + break; + if (op[0] == DHO_DHCP_MESSAGE_TYPE && + op[1] == 1) + dhcptype = op[2]; + else if (op[0] == DHO_DHCP_REQUESTED_ADDRESS && + op[1] == sizeof(requested_addr)) + memcpy(_addr, [2], + sizeof(requested_addr)); + op += 2 + op[1]; + } + } + memset(, 0, sizeof(resp)); resp.op = BOOTREPLY; resp.htype = req.htype; resp.hlen = req.hlen; resp.xid = req.xid; - if ((in.s_addr = vm_priv_addr(>vmd_cfg.cfg_localprefix, + if ((client_addr.s_addr = + vm_priv_addr(>vmd_cfg.cfg_localprefix, dev->vm_vmid, dev->idx, 1)) == 0) return (-1); - memcpy(, , sizeof(in)); - memcpy((_dst)->sin_addr, , sizeof(in)); + memcpy(, _addr, + sizeof(client_addr)); + memcpy((_dst)->sin_addr, _addr, + sizeof(client_addr)); ss2sin(_dst)->sin_port = htons(CLIENT_PORT); - if ((in.s_addr = vm_priv_addr(>vmd_cfg.cfg_localprefix, + if ((server_addr.s_addr = + vm_priv_addr(>vmd_cfg.cfg_localprefix, dev->vm_vmid, dev->idx, 0)) == 0) return (-1); - memcpy(, , sizeof(in)); - memcpy((_src)->sin_addr, , sizeof(in)); + memcpy(, _addr, + sizeof(server_addr)); + memcpy((_src)->sin_addr, _addr, + sizeof(server_addr)); ss2sin(_src)->sin_port = htons(SERVER_PORT); /* Packet is already allocated */ @@ -124,6 +156,44 @@ dhcp_request(struct vionet_dev *dev, cha DHCP_OPTIONS_COOKIE, DHCP_OPTIONS_COOKIE_LEN); o+= DHCP_OPTIONS_COOKIE_LEN; + /* Did we receive a DHCP request or was it just BOOTP? */ + if (dhcptype) { + /* +* There is no need for a real state machine as we always +* answer with the same client IP and options for the VM. +*/ + if (dhcptype == DHCPDISCOVER) + dhcptype = DHCPOFFER; + else if (dhcptype == DHCPREQUEST && + (requested_addr.s_addr == 0 || + client_addr.s_addr == requested_addr.s_addr)) + dhcptype = DHCPACK; + else + dhcptype = DHCPNAK; + + resp.options[o++] = DHO_DHCP_MESSAGE_TYPE; + resp.options[o++] = sizeof(dhcptype); + memcpy([o], , sizeof(dhcptype)); +
vmd: add minimal DHCP support
Hi, the problem got reported a few times and a similar diff was floating around: vmd's "local interface" implements a simple auto-magic BOOTP server, but udhcpc doesn't support BOOTP, which is the predecessor and a valid subset of DHCP. udhcpc is used by busybox and many embedded Linux distributions, maybe even by Android. The following diff adds minimal DHCP support which works with udhcpc. OK? Reyk Index: usr.sbin/vmd/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 --- usr.sbin/vmd/dhcp.c 24 Apr 2017 07:14:27 - 1.3 +++ usr.sbin/vmd/dhcp.c 2 Nov 2017 21:15:03 - @@ -22,6 +22,7 @@ #include #include #include +#include #include #include @@ -38,12 +39,13 @@ extern struct vmd *env; ssize_t dhcp_request(struct vionet_dev *dev, char *buf, size_t buflen, char **obuf) { - unsigned char *respbuf = NULL; + unsigned char *respbuf = NULL, *op, *oe, dhcptype = 0; ssize_t offset, respbuflen = 0; struct packet_ctxpc; struct dhcp_packet req, resp; - struct in_addr in, mask; + struct in_addr server_addr, mask, client_addr, requested_addr; size_t resplen, o; + uint32_t ltime; if (buflen < (ssize_t)(BOOTP_MIN_LEN + sizeof(struct ether_header))) return (-1); @@ -76,24 +78,54 @@ dhcp_request(struct vionet_dev *dev, cha if (req.ciaddr.s_addr != 0 || req.file[0] != '\0' || req.hops != 0) return (-1); + /* Get a few DHCP options (best effort as we fall back to BOOTP) */ + if (memcmp(, + DHCP_OPTIONS_COOKIE, DHCP_OPTIONS_COOKIE_LEN) == 0) { + memset(_addr, 0, sizeof(requested_addr)); + op = req.options + DHCP_OPTIONS_COOKIE_LEN; + oe = req.options + sizeof(req.options); + while (*op != DHO_END && op < oe) { + if (op[0] == DHO_PAD) { + op++; + continue; + } + if (op + 1 + op[1] >= oe) + break; + if (op[0] == DHO_DHCP_MESSAGE_TYPE && + op[1] == 1) + dhcptype = op[2]; + else if (op[0] == DHO_DHCP_REQUESTED_ADDRESS && + op[1] == sizeof(requested_addr)) + memcpy(_addr, [2], + sizeof(requested_addr)); + op += 2 + op[1]; + } + } + memset(, 0, sizeof(resp)); resp.op = BOOTREPLY; resp.htype = req.htype; resp.hlen = req.hlen; resp.xid = req.xid; - if ((in.s_addr = vm_priv_addr(>vmd_cfg.cfg_localprefix, + if ((client_addr.s_addr = + vm_priv_addr(>vmd_cfg.cfg_localprefix, dev->vm_vmid, dev->idx, 1)) == 0) return (-1); - memcpy(, , sizeof(in)); - memcpy((_dst)->sin_addr, , sizeof(in)); + memcpy(, _addr, + sizeof(client_addr)); + memcpy((_dst)->sin_addr, _addr, + sizeof(client_addr)); ss2sin(_dst)->sin_port = htons(CLIENT_PORT); - if ((in.s_addr = vm_priv_addr(>vmd_cfg.cfg_localprefix, + if ((server_addr.s_addr = + vm_priv_addr(>vmd_cfg.cfg_localprefix, dev->vm_vmid, dev->idx, 0)) == 0) return (-1); - memcpy(, , sizeof(in)); - memcpy((_src)->sin_addr, , sizeof(in)); + memcpy(, _addr, + sizeof(server_addr)); + memcpy((_src)->sin_addr, _addr, + sizeof(server_addr)); ss2sin(_src)->sin_port = htons(SERVER_PORT); /* Packet is already allocated */ @@ -124,6 +156,44 @@ dhcp_request(struct vionet_dev *dev, cha DHCP_OPTIONS_COOKIE, DHCP_OPTIONS_COOKIE_LEN); o+= DHCP_OPTIONS_COOKIE_LEN; + /* Did we receive a DHCP request or was it just BOOTP? */ + if (dhcptype) { + /* +* There is no need for a real state machine as we always +* answer with the same client IP and options for the VM. +*/ + if (dhcptype == DHCPDISCOVER) + dhcptype = DHCPOFFER; + else if (dhcptype == DHCPREQUEST && + (requested_addr.s_addr == 0 || + client_addr.s_addr == requested_addr.s_addr)) + dhcptype = DHCPACK; + else + dhcptype = DHCPNAK; + + resp.options[o++] = DHO_DHCP_MESSAGE_TYPE; + resp.options[o++] = sizeof(dhcptype); + memcpy([o], , sizeof(dhcptype)); + o += sizeof(dhcptype); + + /* Our lease