[Dnsmasq-discuss] rfc3527 support
Takayuki Kaiso
tkaiso2 at thinktube.com
Sat May 7 05:50:30 BST 2011
Hi Simon
Thank you so much for the comment, your comments are so helpful.
(1) thanks to new "fallback" parameter, it made my update easy.
now dnsmasq-2.58test6 fills in Server Identifier correctly as I reported in my last mail.
(2) regarding siaddr, I will take the following approach by you.
> 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;
BTW, my iPod is not netbooting, and so I might need to confirm if siaddr is
really necessary, but it seems the iOS seems so picky for me and
want to fill this value.
(3) regarding the "fallback" solution, let me ask one question.
below shows the snippet from do_options() in rfc2131.c, and I guess
you add these codes for default scenario when those are not
configured explicitly. (with the help of complete_context() in dhcp.c )
I see this improved the robustness of the logic and I think this is helpful for users.
But "fallback" solution introduced in dnsmasq-2.58test6 does not
seem to pay attention to this default handling so much. Am I wrong ?
then
to complete the "fallback" solution, it is better for us to
pass other value, "relay address", in addition to "iface_addr"(=fallback), isn't it ?
For "netmask" and "broadcast address" shown below, we need to go back to complete_context()
or execute equivalent codes here if we really want to make it. I am not sure if this will pay or not.
( this was why I updated the codes around complete_context() in dhcp.c at the beginning )
-----------------------------------------------------------------------------------
/* replies to DHCPINFORM may not have a valid context */
if (context)
{
if (!option_find2(netid, config_opts, OPTION_NETMASK))
option_put(mess, end, OPTION_NETMASK, INADDRSZ, ntohl(context->netmask.s_addr));
/* May not have a "guessed" broadcast address if we got no packets via a relay
from this net yet (ie just unicast renewals after a restart */
if (context->broadcast.s_addr && !option_find2(netid, config_opts, OPTION_BROADCAST))
option_put(mess, end, OPTION_BROADCAST, INADDRSZ, ntohl(context->broadcast.s_addr));
/* Same comments as broadcast apply, and also may not be able to get a sensible
default when using subnet select. User must configure by steam in that case. */
if (context->router.s_addr && in_list(req_options, OPTION_ROUTER) &&
!option_find2(netid, config_opts, OPTION_ROUTER))
option_put(mess, end, OPTION_ROUTER, INADDRSZ, ntohl(context->router.s_addr));
if (in_list(req_options, OPTION_DNSSERVER) &&
!option_find2(netid, config_opts, OPTION_DNSSERVER))
option_put(mess, end, OPTION_DNSSERVER, INADDRSZ, ntohl(context->local.s_addr));
}
-------------------------------------------------------------------------------------------
or can we say "do not count on the default value processing and should
all parameters explicitly when people take exceptional configuration like rfc3527 case." ?
For me at this moment, I will try configure all of these parameters explicitly.
Thank you, Simon
Takayuki Kaiso
> 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.
>
More information about the Dnsmasq-discuss
mailing list