[Dnsmasq-discuss] rfc3527 support

[Simon Kelley]

Takayuki Kaiso wrote:
> Hi, Simon
> 
>  Ok,  I did the same test using dnsmasq-2.58test6.  this version
> improves a bit , but not good enough.
> 
>  I will show my test result with dnsmasq-2.58test6  and want you to look
> at your codes to push me ahead.   thanks.
> 
> (1)  dnsmasq-2.58test6 improved only for Server Identifier field
> problem, but siaddr is still left as 0.

That's great. Your Ipod Touch should be getting a DHCP lease now, since
the siaddr is only used for netbooting: A DHCPOFFER with a correct
Server Identifier should be enough to let the DHCP process complete. Are
you trying to netboot the Ipod? If so you can set the siaddr directly
using the --dhcp-boot option (it's server_address in the manpage, the
third option) the idea of setting siaddr to context->local is to provide
a default if this is not set explicitly. The default is just the IP
address of the server which is most likely to be routable from the host.
> 
>       Regarding Server Identifier field, I saw the following codes (line
> 714) in dnsmasq-2.58test6 contributed this improvement.
> 
>     
> --------------------------------------------------------------------------------------------------------------
> 
>     /* If configured, we can override the server-id to be the address of
> the relay,
>      so that all traffic goes via the relay and can pick up agent-id
> info. This can be
>      configured for all relays, or by address. */
> 
>     if (daemon->override && mess->giaddr.s_addr != 0 && override.s_addr
> == 0)
>     {
>       if (!daemon->override_relays)
>       override = mess->giaddr;
>       else
>       {
>           struct addr_list *l;
>           for (l = daemon->override_relays; l; l = l->next)
>              if (l->addr.s_addr == mess->giaddr.s_addr)
>           break;
>           if (l)
>               override = mess->giaddr;
>       }
>     }
>     

That code is only active if the --dhcp-proxy configuration option is
set, so it's probably not valid here.

> -----------------------------------------------------------------------------------------------------------------
> 
> 
> (2) To investigate the reason that "siaddr" is left 0 field,
>       I put the three my_syslog lines shown below.   (around line 990 in
> rfc2131.c)
> 
>       
> ------------------------------------------------------------------------------
> 
>       my_syslog(MS_DHCP | LOG_INFO, _("@@ set OPTION_SERVER_IDENTIFIER
> in dhcp_reply() @@  context->local=%s  \n"), inet_ntoa(context->local)) ;
>       my_syslog(MS_DHCP | LOG_INFO, _("@@ set OPTION_SERVER_IDENTIFIER
> in dhcp_reply() @@  context->router=%s  \n"), inet_ntoa(context->router)) ;
>       my_syslog(MS_DHCP | LOG_INFO, _("@@ set OPTION_SERVER_IDENTIFIER
> in dhcp_reply() @@  server_id=%s  \n"), inet_ntoa(server_id(context,
> override, fallback)) ) ;
> 
>       option_put(mess, end, OPTION_SERVER_IDENTIFIER, INADDRSZ,
> ntohl(server_id(context, override, fallback).s_addr));
>        
> -----------------------------------------------------------------------------
> 
> 
>       then, got the following result.
> 
>       mess->siaddr is actually filled in by just copying context->local
> (as you see, in do_options() )
>       and the last two my_syslog output below tell us the reason why
> mess->siaddr is left as 0.
>       this is just because context->local is 0.
> 
>      Mar  4 21:50:14 RMRZOO-81 daemon.info dnsmasq-dhcp[31469]: @@ set
> OPTION_SERVER_IDENTIFIER in dhcp_reply() @@  context->local=0.0.0.0
>      Mar  4 21:50:14 RMRZOO-81 daemon.info dnsmasq-dhcp[31469]: @@ set
> OPTION_SERVER_IDENTIFIER in dhcp_reply() @@  context->router=0.0.0.0
>      Mar  4 21:50:14 RMRZOO-81 daemon.info dnsmasq-dhcp[31469]: @@ set
> OPTION_SERVER_IDENTIFIER in dhcp_reply() @@  server_id=10.0.1.81
> 
>      Mar  4 21:50:14 RMRZOO-81 daemon.info dnsmasq-dhcp[31469]: @@
> do_options() @@  context->local=0.0.0.0
>      Mar  4 21:50:14 RMRZOO-81 daemon.info dnsmasq-dhcp[31469]: @@
> do_options() @@  context->router=0.0.0.0
> 
> 
> (3)  How to fix the siaddr problem described in (2),
> 
>     In my mind, there are two candidate place to put the fix,
>     one place is complete_context() in dhcp.c  and the another place
>     is  around line 265 in rfc2131.c where context_new is built and
>     replaces context with it.
> 
>     You suggested me that the latter is cleaner and I see your point.
>     If we do not need to maintain the target context earlier in dhcp.c,
>     I completely agree with you.
> 
>     then,
>     to implement the fix in rfc2131.c, I think it is better to pass
> "iface_addr" from dhcp.c when
>     calling dhcp_reply() in order to avoid executing ioctl() to get the
> local interface address again
>     in rfc2131.c
> 

Agree, in fact the change from 2.57 to 2.58 does exactly that, the value
of iface_addr appears in dhcp_reply in the argument "fallback".

>     what is your opinion ?

Add fallback to the arguments list of do_options() (and all the places
it gets called from) and then change the siaddr code in do_options to be

if (context && context->local.s_addr != 0)
	mess->siaddr = context->local;
else
	mess->siaddr = fallback;



Cheers,

Simon.