Re: [RFC] netconfig: Global option to send hostname with dhcp request for all networks

Hi Bryce,

On 2/28/25 7:21 AM, Bryce Johnson wrote:
> On Fri, Feb 28, 2025 at 6:58 AM James Prestwood <[email protected]> wrote:
>> Hi Bryce,
>>
>> On 2/27/25 1:42 PM, Bryce Johnson wrote:
>>> Add global option to send hostname with dhcp always with
>>> [IPv4].SendHostnameAlways so the first DHCP request will include the
>>> hostname on first connection and don't need to modify the .psk files
>>> to add SendHostname after they are created.
>>>
>>> ---
>>> diff --git a/src/iwd.config.rst b/src/iwd.config.rst
>>> index 4ba7b4e7..4f473e07 100644
>>> --- a/src/iwd.config.rst
>>> +++ b/src/iwd.config.rst
>>> @@ -429,6 +429,11 @@ The group ``[IPv4]`` contains settings related to
>>> IPv4 network configuration.
>>>           will limit the number of access points that can be running
>>>           simultaneously on different interfaces.
>>>
>>> +   * - SendHostnameAlways
>>> +     - Values: true, **false**
>>> +
>>> +       Always send the hostname in the DHCP request for all networks.
>> NIT: You could probably just shorten this to "SendHostname". I
>> personally don't mind if settings have the same name between main.conf
>> and network profiles.
> Thanks for the review. I'll push up a new patch.  Looks like I
> probably should have done it in 2 patches, one for the doc and one for
> the netconfig?
Yes, Denis prefers splitting up patches per-file (when possible).
>    Is there an order of preference if in the main.conf
> SendHostname is true, but in the psk it is false?  I assume the psk
> one should take preference?
Yeah I would say if its explicitly set false in the PSK file, that 
should be the priority. Otherwise use the global setting.
>>> +
>>>    DriverQuirks
>>>    ------------
>>>
>>> diff --git a/src/netconfig.c b/src/netconfig.c
>>> index cd19217c..5959a5ac 100644
>>> --- a/src/netconfig.c
>>> +++ b/src/netconfig.c
>>> @@ -375,7 +375,9 @@ gateway:
>>>       }
>>>
>>>    send_hostname:
>>> -   if (l_settings_has_key(active_settings, "IPv4", "SendHostname") &&
>>> +   l_settings_get_bool(iwd_get_config(), "IPv4",
>>> "SendHostnameAlways", &send_hostname);
>>> +
>>> +   if (!send_hostname && l_settings_has_key(active_settings, "IPv4",
>>> "SendHostname") &&
>>>               !l_settings_get_bool(active_settings, "IPv4",
>>>                           "SendHostname",
>>>                           &send_hostname)) {
>>> @@ -391,13 +393,20 @@ send_hostname:
>>>           goto mdns;
>>>       }
>>>
>>> -   if (send_hostname &&
>>> -           !l_netconfig_set_hostname(netconfig->nc, hostname)) {
>>> -       l_error("netconfig: l_netconfig_set_hostname() failed");
>>> -       success = false;
>>> -       goto mdns;
>>> +   if (send_hostname) {
>>> +       if (!l_netconfig_set_hostname(netconfig->nc, hostname)) {
>>> +           l_error("netconfig: l_netconfig_set_hostname() failed");
>>> +           success = false;
>>> +       } else {
>>> +           l_debug("netconfig: Hostname '%s' will be sent in DHCP
>>> requests", hostname);
>>> +       }
>>> +   } else {
>>> +       l_debug("netconfig: No hostname will be sent in DHCP requests");
>> It probably seemed consistent, but l_error() actually doesn't include
>> the file/function which is why those all have "netconfig:" in them. For
>> debug prints we actually don't need to prefix with "netconfig:" since it
>> will always include the file and function name.
> Will fix this as well.
>
>>>       }
>>>
>>> +   goto mdns;
>>> +
>>> +
>>>    mdns:
>>>       /* If the networks has this set take that over the global */
>>>       if (l_settings_has_key(active_settings, "Network", "MulticastDNS")) {
>>>
>> Thanks,
>>
>> James
>>