diff mbox series

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

Message ID CADXxVS+UQfrbWFfsDnW1QvkpvmEnkxMdv+OPBy=XpPXEFdcU8Q@mail.gmail.com (mailing list archive)
State New
Headers show
Series [RFC] netconfig: Global option to send hostname with dhcp request for all networks | expand

Checks

Context Check Description
tedd_an/pre-ci_am fail error: patch fragment without header at line 4: @@ -391,13 +393,20 @@ send_hostname: hint: Use 'git am --show-current-patch' to see the failed patch

Commit Message

Bryce Johnson Feb. 27, 2025, 9:42 p.m. UTC 
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.

---
                        "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");
    }

+   goto mdns;
+
+
 mdns:
    /* If the networks has this set take that over the global */
    if (l_settings_has_key(active_settings, "Network", "MulticastDNS")) {

Comments

James Prestwood Feb. 28, 2025, 1:58 p.m. UTC | #1 
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.
> +
>   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.
>      }
>
> +   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
Bryce Johnson Feb. 28, 2025, 3:21 p.m. UTC | #2 
On Fri, Feb 28, 2025 at 6:58 AM James Prestwood <prestwoj@gmail.com> 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
>
James Prestwood Feb. 28, 2025, 3:43 p.m. UTC | #3 
Hi Bryce,

On 2/28/25 7:21 AM, Bryce Johnson wrote:
> On Fri, Feb 28, 2025 at 6:58 AM James Prestwood <prestwoj@gmail.com> 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
>>
diff mbox series

Patch

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.
+
 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",