Re: [Dnsmasq-discuss] [RFC] Add --dhcp-reply-delay option to delay DHCP replies

2017-03-23 Thread Floris Bos

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

2017-03-22 Thread Floris Bos

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

2017-03-22 Thread Alex Xu
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

2017-03-22 Thread Simon Kelley
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

2017-03-20 Thread Floris Bos
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