Re: [Dnsmasq-discuss] [RFC] Add --dhcp-reply-delay option to delay DHCP replies
On 03/23/2017 12:11 AM, Alex Xu wrote: On Mon, 20 Mar 2017 19:37:12 +0100 Floris Bos wrote: Adds option to delay replying to DHCP packets by one or more seconds. This provides a workaround for a PXE boot firmware implementation that has a bug causing it to fail if it receives a (proxy) DHCP reply instantly. On Linux it looks up the exact receive time of the UDP packet with the SIOCGSTAMP ioctl to prevent multiple delays if multiple packets come in around the same time. wouldn't it be easier to use tc tc would also be an alternative, yes. However think that's more difficult for end-users to install and add to start-up script, than an option in the dnsmasq configuration file though. And not sure if tc can be used in all circumstances. E.g. user may be running dnsmasq in a VPS with container style virtualization (Xen, OpenVZ, etc.) which limits access to kernel features like that. Yours sincerely, Floris Bos ___ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss
Re: [Dnsmasq-discuss] [RFC] Add --dhcp-reply-delay option to delay DHCP replies
Hi, On 03/22/2017 10:01 PM, Simon Kelley wrote: As a patch, it looks pretty good. The main problem I have is that the new option becomes one of the those annoying things that have to be set to make things work, but have no other value. There are already quite a few dnsmasq options which are essentially "--dont-break" and if I can avoid another, I will. You say that the PXE bug is "if it receives a (proxy) DHCP reply instantly", is this really only with proxy DHCP? It happens in all instances in which the packet that has the pxe/tftp boot information is send without any delay. In non-proxy mode it also happens when the ping check is skipped. That's believable: a PXE implementation which gets the proxy reply _before_ the standard DHCP reply might well get confused and, let's face it, it doesn't take much to confuse most PXE clients :-( If that's the case, would it be enough to delay replies by a fixed time always and only if they're proxy replies? That would have the effect of making things just work, with needing a "--dont-break" option, and wouldn't affect the normal use-case at all. It's a bit more difficult to implement, but not impossible. worst case, dhcp_reply() needs to return a flag indicating that PXE-proxy reply was made, and the call to the delay routine made based on that. In this particular case, it is likely that only one vendor is affected. I can probably activate the workaround automatically based on MAC OUI, if you prefer not to have additional options. Only other quibble is the duplication of all the poll-loop code into dhcp_delay(). Would it be neater to make one routine handle waiting for an icmp-ping reply and just waiting? That's possible. Do would make the function's signature a bit cryptic though, along the lines of: int delay_dhcp(time_t start, int sec, int fd, struct in_addr addr, int id); Yours sincerely, Floris Bos ___ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss
Re: [Dnsmasq-discuss] [RFC] Add --dhcp-reply-delay option to delay DHCP replies
On Mon, 20 Mar 2017 19:37:12 +0100 Floris Bos wrote: > Adds option to delay replying to DHCP packets by one or more seconds. > This provides a workaround for a PXE boot firmware implementation > that has a bug causing it to fail if it receives a (proxy) DHCP > reply instantly. > > On Linux it looks up the exact receive time of the UDP packet with > the SIOCGSTAMP ioctl to prevent multiple delays if multiple packets > come in around the same time. wouldn't it be easier to use tc ___ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss
Re: [Dnsmasq-discuss] [RFC] Add --dhcp-reply-delay option to delay DHCP replies
As a patch, it looks pretty good. The main problem I have is that the new option becomes one of the those annoying things that have to be set to make things work, but have no other value. There are already quite a few dnsmasq options which are essentially "--dont-break" and if I can avoid another, I will. You say that the PXE bug is "if it receives a (proxy) DHCP reply instantly", is this really only with proxy DHCP? That's believable: a PXE implementation which gets the proxy reply _before_ the standard DHCP reply might well get confused and, let's face it, it doesn't take much to confuse most PXE clients :-( If that's the case, would it be enough to delay replies by a fixed time always and only if they're proxy replies? That would have the effect of making things just work, with needing a "--dont-break" option, and wouldn't affect the normal use-case at all. It's a bit more difficult to implement, but not impossible. worst case, dhcp_reply() needs to return a flag indicating that PXE-proxy reply was made, and the call to the delay routine made based on that. Only other quibble is the duplication of all the poll-loop code into dhcp_delay(). Would it be neater to make one routine handle waiting for an icmp-ping reply and just waiting? Cheers, Simon. On 20/03/17 18:37, Floris Bos wrote: > Adds option to delay replying to DHCP packets by one or more seconds. > This provides a workaround for a PXE boot firmware implementation > that has a bug causing it to fail if it receives a (proxy) DHCP > reply instantly. > > On Linux it looks up the exact receive time of the UDP packet with > the SIOCGSTAMP ioctl to prevent multiple delays if multiple packets > come in around the same time. > --- > > Hi, > > I have a need to introduce a delay before dnsmasq replies to DHCP > packets. > > Currently have the following working code, inspired by similar > code in the icmp_ping() method. > But was wondering if this is the right way to do this, or if there is > perhaps a better method? > > And if functionality like this would be welcome for upstream > inclusion, or if the use-case is too obscure? > > Yours sincerely, > > Floris Bos > --- > man/dnsmasq.8 | 5 + > src/dhcp.c| 15 +++ > src/dnsmasq.c | 43 +++ > src/dnsmasq.h | 3 ++- > src/option.c | 8 > 5 files changed, 73 insertions(+), 1 deletion(-) > > diff --git a/man/dnsmasq.8 b/man/dnsmasq.8 > index 059eafc..5030dae 100644 > --- a/man/dnsmasq.8 > +++ b/man/dnsmasq.8 > @@ -1790,6 +1790,11 @@ a router to advertise prefixes but not a route via > itself. > .B --ra-param=low,60,1200 > The interface field may include a wildcard. > .TP > +.B --dhcp-reply-delay= > +Delays replying to DHCP packets for at least the specified number of seconds. > +This can be used as workaround for bugs in PXE boot firmware that does not > function properly when > +receiving an instant reply. > +.TP > .B --enable-tftp[=[,]] > Enable the TFTP server function. This is deliberately limited to that > needed to net-boot a client. Only reading is allowed; the tsize and > diff --git a/src/dhcp.c b/src/dhcp.c > index 08952c8..bb45ad0 100644 > --- a/src/dhcp.c > +++ b/src/dhcp.c > @@ -342,6 +342,21 @@ void dhcp_packet(time_t now, int pxe_fd) >if (iov.iov_len == 0) > return; > } > + > + if (daemon->reply_delay) > +{ > + int starttime = now; > +#ifdef HAVE_LINUX_NETWORK > + struct timeval tv; > + > + /* Use timestamp of the UDP packet received as start time for delay */ > + if (ioctl(fd, SIOCGSTAMP, &tv) == 0) > + { > + starttime = tv.tv_sec; > + } > +#endif > + delay_dhcp(starttime, daemon->reply_delay); > +} > >msg.msg_name = &dest; >msg.msg_namelen = sizeof(dest); > diff --git a/src/dnsmasq.c b/src/dnsmasq.c > index d2cc7cc..f2b0a56 100644 > --- a/src/dnsmasq.c > +++ b/src/dnsmasq.c > @@ -1858,6 +1858,49 @@ int icmp_ping(struct in_addr addr) > >return gotreply; > } > + > +void delay_dhcp(time_t start, int sec) > +{ > + /* Delay processing DHCP packets for "sec" seconds counting from "start", > + while continuing to service events on the DNS and TFTP sockets */ > + > + int rc, timeout_count; > + time_t now; > + > + for (now = dnsmasq_time(), timeout_count = 0; > + (difftime(now, start) <= (float)sec) && (timeout_count < sec * 4);) > +{ > + poll_reset(); > + set_dns_listeners(now); > + set_log_writer(); > + > +#ifdef HAVE_DHCP6 > + if (daemon->doing_ra) > + poll_listen(daemon->icmp6fd, POLLIN); > +#endif > + > + rc = do_poll(250); > + > + if (rc < 0) > + continue; > + else if (rc == 0) > + timeout_count++; > + > + now = dnsmasq_time(); > + > + check_log_writer(0); > + check_dns_listeners(now); > + > +#ifdef HAVE_DHCP6 > + if (daemon->doing_ra && poll_check(daemon->icmp6fd, POLLIN)) > + icmp6_packet(now); > +#endif > + > +#ifdef HAVE_TFTP >
[Dnsmasq-discuss] [RFC] Add --dhcp-reply-delay option to delay DHCP replies
Adds option to delay replying to DHCP packets by one or more seconds. This provides a workaround for a PXE boot firmware implementation that has a bug causing it to fail if it receives a (proxy) DHCP reply instantly. On Linux it looks up the exact receive time of the UDP packet with the SIOCGSTAMP ioctl to prevent multiple delays if multiple packets come in around the same time. --- Hi, I have a need to introduce a delay before dnsmasq replies to DHCP packets. Currently have the following working code, inspired by similar code in the icmp_ping() method. But was wondering if this is the right way to do this, or if there is perhaps a better method? And if functionality like this would be welcome for upstream inclusion, or if the use-case is too obscure? Yours sincerely, Floris Bos --- man/dnsmasq.8 | 5 + src/dhcp.c| 15 +++ src/dnsmasq.c | 43 +++ src/dnsmasq.h | 3 ++- src/option.c | 8 5 files changed, 73 insertions(+), 1 deletion(-) diff --git a/man/dnsmasq.8 b/man/dnsmasq.8 index 059eafc..5030dae 100644 --- a/man/dnsmasq.8 +++ b/man/dnsmasq.8 @@ -1790,6 +1790,11 @@ a router to advertise prefixes but not a route via itself. .B --ra-param=low,60,1200 The interface field may include a wildcard. .TP +.B --dhcp-reply-delay= +Delays replying to DHCP packets for at least the specified number of seconds. +This can be used as workaround for bugs in PXE boot firmware that does not function properly when +receiving an instant reply. +.TP .B --enable-tftp[=[,]] Enable the TFTP server function. This is deliberately limited to that needed to net-boot a client. Only reading is allowed; the tsize and diff --git a/src/dhcp.c b/src/dhcp.c index 08952c8..bb45ad0 100644 --- a/src/dhcp.c +++ b/src/dhcp.c @@ -342,6 +342,21 @@ void dhcp_packet(time_t now, int pxe_fd) if (iov.iov_len == 0) return; } + + if (daemon->reply_delay) +{ + int starttime = now; +#ifdef HAVE_LINUX_NETWORK + struct timeval tv; + + /* Use timestamp of the UDP packet received as start time for delay */ + if (ioctl(fd, SIOCGSTAMP, &tv) == 0) + { + starttime = tv.tv_sec; + } +#endif + delay_dhcp(starttime, daemon->reply_delay); +} msg.msg_name = &dest; msg.msg_namelen = sizeof(dest); diff --git a/src/dnsmasq.c b/src/dnsmasq.c index d2cc7cc..f2b0a56 100644 --- a/src/dnsmasq.c +++ b/src/dnsmasq.c @@ -1858,6 +1858,49 @@ int icmp_ping(struct in_addr addr) return gotreply; } + +void delay_dhcp(time_t start, int sec) +{ + /* Delay processing DHCP packets for "sec" seconds counting from "start", + while continuing to service events on the DNS and TFTP sockets */ + + int rc, timeout_count; + time_t now; + + for (now = dnsmasq_time(), timeout_count = 0; + (difftime(now, start) <= (float)sec) && (timeout_count < sec * 4);) +{ + poll_reset(); + set_dns_listeners(now); + set_log_writer(); + +#ifdef HAVE_DHCP6 + if (daemon->doing_ra) + poll_listen(daemon->icmp6fd, POLLIN); +#endif + + rc = do_poll(250); + + if (rc < 0) + continue; + else if (rc == 0) + timeout_count++; + + now = dnsmasq_time(); + + check_log_writer(0); + check_dns_listeners(now); + +#ifdef HAVE_DHCP6 + if (daemon->doing_ra && poll_check(daemon->icmp6fd, POLLIN)) + icmp6_packet(now); +#endif + +#ifdef HAVE_TFTP + check_tftp_listeners(now); +#endif +} +} #endif diff --git a/src/dnsmasq.h b/src/dnsmasq.h index 6b44e53..e14c8b8 100644 --- a/src/dnsmasq.h +++ b/src/dnsmasq.h @@ -960,7 +960,7 @@ extern struct daemon { int max_logs; /* queue limit */ int cachesize, ftabsize; int port, query_port, min_port, max_port; - unsigned long local_ttl, neg_ttl, max_ttl, min_cache_ttl, max_cache_ttl, auth_ttl, dhcp_ttl, use_dhcp_ttl; + unsigned long local_ttl, neg_ttl, max_ttl, min_cache_ttl, max_cache_ttl, auth_ttl, dhcp_ttl, use_dhcp_ttl, reply_delay; char *dns_client_id; struct hostsfile *addn_hosts; struct dhcp_context *dhcp, *dhcp6; @@ -1341,6 +1341,7 @@ unsigned char *extended_hwaddr(int hwtype, int hwlen, unsigned char *hwaddr, #ifdef HAVE_DHCP int make_icmp_sock(void); int icmp_ping(struct in_addr addr); +void delay_dhcp(time_t start, int sec); #endif void queue_event(int event); void send_alarm(time_t event, time_t now); diff --git a/src/option.c b/src/option.c index 31c8cb9..7f10e4a 100644 --- a/src/option.c +++ b/src/option.c @@ -159,6 +159,7 @@ struct myoption { #define LOPT_SCRIPT_ARP347 #define LOPT_DHCPTTL 348 #define LOPT_TFTP_MTU 349 +#define LOPT_REPLY_DELAY 350 #ifdef HAVE_GETOPT_LONG static const struct option opts[] = @@ -323,6 +324,7 @@ static const struct myoption opts[] = { "dns-loop-detect", 0, 0, LOPT_LOOP_DETECT }, { "script-arp", 0, 0, LOPT_SCRIPT_ARP }, { "dhcp-ttl", 1, 0 , LOPT_DHCPTTL }, +{ "dhcp-reply-delay", 1, 0, LOPT_REPLY_DE