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

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?  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?
> > +
> >   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
>