* Re: [PATCH] src: configure systemd-resolved's MulticastDNS= setting
2021-02-10 23:43 [PATCH] src: configure systemd-resolved's MulticastDNS= setting Daniel Lin
@ 2021-02-11 15:59 ` Denis Kenzior
2021-02-11 18:04 ` Daniel Lin
2021-02-11 18:11 ` [PATCH v2] " Daniel Lin
2021-02-11 20:54 ` [PATCH v3] " Daniel Lin
2 siblings, 1 reply; 8+ messages in thread
From: Denis Kenzior @ 2021-02-11 15:59 UTC (permalink / raw)
To: iwd
[-- Attachment #1: Type: text/plain, Size: 8293 bytes --]
Hi Daniel,
On 2/10/21 5:43 PM, Daniel Lin wrote:
> When using iwd.conf:[General].EnableNetworkConfiguration=true, it is not
> possible to configure systemd.network:[Network].MulticastDNS= as
> systemd-networkd considers the link to be unmanaged. This patch allows
> iwd to configure that setting on systemd-resolved directly.
> ---
> src/iwd.config.rst | 7 ++++
> src/netconfig.c | 6 ++++
> src/resolve.c | 83 ++++++++++++++++++++++++++++++++--------------
> src/resolve.h | 1 +
> 4 files changed, 73 insertions(+), 24 deletions(-)
>
> diff --git a/src/iwd.config.rst b/src/iwd.config.rst
> index 0064dfd2..73d41cc0 100644
> --- a/src/iwd.config.rst
> +++ b/src/iwd.config.rst
> @@ -212,6 +212,13 @@ The group ``[Network]`` contains network configuration related settings.
>
> If not specified, ``300`` is used as default.
>
> + * - MulticastDNS
> + - Values: true, false, resolve
Is there a default? If so, it should be in bold. What happens if this setting
is omitted?
> +
> + Configures multicast DNS. See ``man 5 systemd.network`` for details.
> +
> + Only applies when ``NameResolvingService=systemd``.
> +
> Blacklist
> ---------
>
> diff --git a/src/netconfig.c b/src/netconfig.c
> index cf092b78..749ba0ad 100644
> --- a/src/netconfig.c
> +++ b/src/netconfig.c
> @@ -963,6 +963,8 @@ bool netconfig_configure(struct netconfig *netconfig,
> const uint8_t *mac_address,
> netconfig_notify_func_t notify, void *user_data)
> {
> + char *mdns;
> +
> netconfig->dns4_overrides = l_settings_get_string_list(active_settings,
> "IPv4", "DNS", ' ');
>
> @@ -1006,6 +1008,10 @@ bool netconfig_configure(struct netconfig *netconfig,
>
> netconfig_ipv6_select_and_install(netconfig);
>
> + mdns = l_settings_get_string(active_settings, "Network", "MulticastDNS");
This can return NULL if no MulticastDNS setting is found. What happens then?
> + resolve_set_mdns(netconfig->resolve, mdns);
> + l_free(mdns);
> +
> return true;
> }
>
> diff --git a/src/resolve.c b/src/resolve.c
> index d3b483a8..993bbf34 100644
> --- a/src/resolve.c
> +++ b/src/resolve.c
> @@ -40,6 +40,7 @@
> struct resolve_ops {
> void (*set_dns)(struct resolve *resolve, char **dns_list);
> void (*set_domains)(struct resolve *resolve, char **domain_list);
> + void (*set_mdns)(struct resolve *resolve, char *mdns);
should probably be const char *mdns
> void (*revert)(struct resolve *resolve);
> void (*destroy)(struct resolve *resolve);
> };
> @@ -78,6 +79,17 @@ void resolve_set_domains(struct resolve *resolve, char **domain_list)
> resolve->ops->set_domains(resolve, domain_list);
> }
>
> +void resolve_set_mdns(struct resolve *resolve, char *mdns)
> +{
> + if (!mdns)
> + return;
> +
> + if (!resolve->ops->set_mdns)
> + return;
> +
> + resolve->ops->set_mdns(resolve, mdns);
> +}
> +
> void resolve_revert(struct resolve *resolve)
> {
> if (!resolve->ops->revert)
> @@ -112,9 +124,10 @@ struct systemd {
> struct resolve super;
> };
>
> -static void systemd_link_dns_reply(struct l_dbus_message *message,
> +static void systemd_link_generic_reply(struct l_dbus_message *message,
> void *user_data)
> {
> + const char *type = user_data;
> const char *name;
> const char *text;
>
> @@ -123,8 +136,8 @@ static void systemd_link_dns_reply(struct l_dbus_message *message,
>
> l_dbus_message_get_error(message, &name, &text);
>
> - l_error("resolve-systemd: Failed to modify the DNS entries. %s: %s",
> - name, text);
> + l_error("resolve-systemd: Failed to modify the %s entries. %s: %s",
> + type, name, text);
> }
>
> static bool systemd_builder_add_dns(struct l_dbus_message_builder *builder,
> @@ -205,23 +218,8 @@ static void resolve_systemd_set_dns(struct resolve *resolve, char **dns_list)
> l_dbus_message_builder_finalize(builder);
> l_dbus_message_builder_destroy(builder);
>
> - l_dbus_send_with_reply(dbus_get_bus(), message, systemd_link_dns_reply,
> - NULL, NULL);
> -}
> -
> -static void systemd_set_link_domains_reply(struct l_dbus_message *message,
> - void *user_data)
> -{
> - const char *name;
> - const char *text;
> -
> - if (!l_dbus_message_is_error(message))
> - return;
> -
> - l_dbus_message_get_error(message, &name, &text);
> -
> - l_error("resolve-systemd: Failed to modify the domain entries. %s: %s",
> - name, text);
> + l_dbus_send_with_reply(dbus_get_bus(), message, systemd_link_generic_reply,
> + "DNS", NULL);
> }
>
> static void resolve_systemd_set_domains(struct resolve *resolve,
> @@ -266,8 +264,43 @@ static void resolve_systemd_set_domains(struct resolve *resolve,
> l_dbus_message_builder_finalize(builder);
> l_dbus_message_builder_destroy(builder);
>
> - l_dbus_send_with_reply(dbus_get_bus(), message,
> - systemd_set_link_domains_reply, NULL, NULL);
> + l_dbus_send_with_reply(dbus_get_bus(), message, systemd_link_generic_reply,
> + "domains", NULL);
> +}
> +
> +static void resolve_systemd_set_mdns(struct resolve *resolve, char *mdns)
> +{
> + struct l_dbus_message_builder *builder;
> + struct l_dbus_message *message;
> +
> + l_debug("ifindex: %u", resolve->ifindex);
> +
> + if (L_WARN_ON(!systemd_state.is_ready))
> + return;
> +
> + message = l_dbus_message_new_method_call(dbus_get_bus(),
> + SYSTEMD_RESOLVED_SERVICE,
> + SYSTEMD_RESOLVED_MANAGER_PATH,
> + SYSTEMD_RESOLVED_MANAGER_INTERFACE,
> + "SetLinkMulticastDNS");
> +
> + if (!message)
> + return;
> +
> + builder = l_dbus_message_builder_new(message);
> + if (!builder) {
> + l_dbus_message_unref(message);
> + return;
> + }
> +
> + l_dbus_message_builder_append_basic(builder, 'i', &resolve->ifindex);
> + l_dbus_message_builder_append_basic(builder, 's', mdns);
> +
> + l_dbus_message_builder_finalize(builder);
> + l_dbus_message_builder_destroy(builder);
Using a builder for something this simple is probably overkill. You can replace
the last ~12 lines by something like:
l_dbus_message_set_arguments(message, "is", resolve->ifindex, mdns);
> +
> + l_dbus_send_with_reply(dbus_get_bus(), message, systemd_link_generic_reply,
> + "MulticastDNS", NULL);
> }
>
> static void resolve_systemd_revert(struct resolve *resolve)
> @@ -288,8 +321,8 @@ static void resolve_systemd_revert(struct resolve *resolve)
> return;
>
> l_dbus_message_set_arguments(message, "i", resolve->ifindex);
> - l_dbus_send_with_reply(dbus_get_bus(), message, systemd_link_dns_reply,
> - NULL, NULL);
> + l_dbus_send_with_reply(dbus_get_bus(), message, systemd_link_generic_reply,
> + "DNS", NULL);
> }
>
> static void resolve_systemd_destroy(struct resolve *resolve)
> @@ -302,6 +335,7 @@ static void resolve_systemd_destroy(struct resolve *resolve)
> static const struct resolve_ops systemd_ops = {
> .set_dns = resolve_systemd_set_dns,
> .set_domains = resolve_systemd_set_domains,
> + .set_mdns = resolve_systemd_set_mdns,
> .revert = resolve_systemd_revert,
> .destroy = resolve_systemd_destroy,
> };
> @@ -482,6 +516,7 @@ static void resolve_resolvconf_destroy(struct resolve *resolve)
> static struct resolve_ops resolvconf_ops = {
> .set_dns = resolve_resolvconf_set_dns,
> .set_domains = resolve_resolvconf_set_domains,
> + .set_mdns = NULL,
This should already be NULL, so not needed and can be dropped
> .revert = resolve_resolvconf_revert,
> .destroy = resolve_resolvconf_destroy,
> };
> diff --git a/src/resolve.h b/src/resolve.h
> index 64ed15f3..09cd9ac2 100644
> --- a/src/resolve.h
> +++ b/src/resolve.h
> @@ -23,5 +23,6 @@
> struct resolve *resolve_new(uint32_t ifindex);
> void resolve_set_dns(struct resolve *resolve, char **dns_list);
> void resolve_set_domains(struct resolve *resolve, char **domain_list);
> +void resolve_set_mdns(struct resolve *resolve, char *mdns);
> void resolve_revert(struct resolve *resolve);
> void resolve_free(struct resolve *resolve);
>
Regards,
-Denis
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] src: configure systemd-resolved's MulticastDNS= setting
2021-02-11 15:59 ` Denis Kenzior
@ 2021-02-11 18:04 ` Daniel Lin
2021-02-11 20:45 ` Denis Kenzior
0 siblings, 1 reply; 8+ messages in thread
From: Daniel Lin @ 2021-02-11 18:04 UTC (permalink / raw)
To: iwd
[-- Attachment #1: Type: text/plain, Size: 9537 bytes --]
On Thu, 2021-02-11 at 09:59 -0600, Denis Kenzior wrote:
> Hi Daniel,
>
> On 2/10/21 5:43 PM, Daniel Lin wrote:
> > When using iwd.conf:[General].EnableNetworkConfiguration=true, it
> > is not
> > possible to configure systemd.network:[Network].MulticastDNS= as
> > systemd-networkd considers the link to be unmanaged. This patch
> > allows
> > iwd to configure that setting on systemd-resolved directly.
> > ---
> > src/iwd.config.rst | 7 ++++
> > src/netconfig.c | 6 ++++
> > src/resolve.c | 83 ++++++++++++++++++++++++++++++++---------
> > -----
> > src/resolve.h | 1 +
> > 4 files changed, 73 insertions(+), 24 deletions(-)
> >
> > diff --git a/src/iwd.config.rst b/src/iwd.config.rst
> > index 0064dfd2..73d41cc0 100644
> > --- a/src/iwd.config.rst
> > +++ b/src/iwd.config.rst
> > @@ -212,6 +212,13 @@ The group ``[Network]`` contains network
> > configuration related settings.
> >
> > If not specified, ``300`` is used as default.
> >
> > + * - MulticastDNS
> > + - Values: true, false, resolve
>
> Is there a default? If so, it should be in bold. What happens if
> this setting
> is omitted?
If the setting is omitted, we don't touch the default value. Which
appears to be false, but somebody could patch systemd-resolved to
change that, so I wasn't sure if that should be documented here. I'll
add that though.
>
> > +
> > + Configures multicast DNS. See ``man 5 systemd.network`` for
> > details.
> > +
> > + Only applies when ``NameResolvingService=systemd``.
> > +
> > Blacklist
> > ---------
> >
> > diff --git a/src/netconfig.c b/src/netconfig.c
> > index cf092b78..749ba0ad 100644
> > --- a/src/netconfig.c
> > +++ b/src/netconfig.c
> > @@ -963,6 +963,8 @@ bool netconfig_configure(struct netconfig
> > *netconfig,
> > const uint8_t *mac_address,
> > netconfig_notify_func_t notify, void
> > *user_data)
> > {
> > + char *mdns;
> > +
> > netconfig->dns4_overrides =
> > l_settings_get_string_list(active_settings,
> > "IPv4", "DNS",
> > ' ');
> >
> > @@ -1006,6 +1008,10 @@ bool netconfig_configure(struct netconfig
> > *netconfig,
> >
> > netconfig_ipv6_select_and_install(netconfig);
> >
> > + mdns = l_settings_get_string(active_settings, "Network",
> > "MulticastDNS");
>
> This can return NULL if no MulticastDNS setting is found. What
> happens then?
Nothing happens, there's a null check in resolve_set_mdns().
>
> > + resolve_set_mdns(netconfig->resolve, mdns);
> > + l_free(mdns);
> > +
> > return true;
> > }
> >
> > diff --git a/src/resolve.c b/src/resolve.c
> > index d3b483a8..993bbf34 100644
> > --- a/src/resolve.c
> > +++ b/src/resolve.c
> > @@ -40,6 +40,7 @@
> > struct resolve_ops {
> > void (*set_dns)(struct resolve *resolve, char **dns_list);
> > void (*set_domains)(struct resolve *resolve, char
> > **domain_list);
> > + void (*set_mdns)(struct resolve *resolve, char *mdns);
>
> should probably be const char *mdns
OK. Shouldn't the nearby `char **` parameters be `const char *const *`
too, then?
>
> > void (*revert)(struct resolve *resolve);
> > void (*destroy)(struct resolve *resolve);
> > };
> > @@ -78,6 +79,17 @@ void resolve_set_domains(struct resolve
> > *resolve, char **domain_list)
> > resolve->ops->set_domains(resolve, domain_list);
> > }
> >
> > +void resolve_set_mdns(struct resolve *resolve, char *mdns)
> > +{
> > + if (!mdns)
> > + return;
> > +
> > + if (!resolve->ops->set_mdns)
> > + return;
> > +
> > + resolve->ops->set_mdns(resolve, mdns);
> > +}
> > +
> > void resolve_revert(struct resolve *resolve)
> > {
> > if (!resolve->ops->revert)
> > @@ -112,9 +124,10 @@ struct systemd {
> > struct resolve super;
> > };
> >
> > -static void systemd_link_dns_reply(struct l_dbus_message *message,
> > +static void systemd_link_generic_reply(struct l_dbus_message
> > *message,
> > void
> > *user_data)
> > {
> > + const char *type = user_data;
> > const char *name;
> > const char *text;
> >
> > @@ -123,8 +136,8 @@ static void systemd_link_dns_reply(struct
> > l_dbus_message *message,
> >
> > l_dbus_message_get_error(message, &name, &text);
> >
> > - l_error("resolve-systemd: Failed to modify the DNS entries. %s:
> > %s",
> > - name,
> > text);
> > + l_error("resolve-systemd: Failed to modify the %s entries. %s:
> > %s",
> > + type,
> > name, text);
> > }
> >
> > static bool systemd_builder_add_dns(struct l_dbus_message_builder
> > *builder,
> > @@ -205,23 +218,8 @@ static void resolve_systemd_set_dns(struct
> > resolve *resolve, char **dns_list)
> > l_dbus_message_builder_finalize(builder);
> > l_dbus_message_builder_destroy(builder);
> >
> > - l_dbus_send_with_reply(dbus_get_bus(), message,
> > systemd_link_dns_reply,
> > - NULL,
> > NULL);
> > -}
> > -
> > -static void systemd_set_link_domains_reply(struct l_dbus_message
> > *message,
> > - void
> > *user_data)
> > -{
> > - const char *name;
> > - const char *text;
> > -
> > - if (!l_dbus_message_is_error(message))
> > - return;
> > -
> > - l_dbus_message_get_error(message, &name, &text);
> > -
> > - l_error("resolve-systemd: Failed to modify the domain entries.
> > %s: %s",
> > - name,
> > text);
> > + l_dbus_send_with_reply(dbus_get_bus(), message,
> > systemd_link_generic_reply,
> > + "DNS",
> > NULL);
> > }
> >
> > static void resolve_systemd_set_domains(struct resolve *resolve,
> > @@ -266,8 +264,43 @@ static void resolve_systemd_set_domains(struct
> > resolve *resolve,
> > l_dbus_message_builder_finalize(builder);
> > l_dbus_message_builder_destroy(builder);
> >
> > - l_dbus_send_with_reply(dbus_get_bus(), message,
> > - systemd_set_link_domains_reply, NULL,
> > NULL);
> > + l_dbus_send_with_reply(dbus_get_bus(), message,
> > systemd_link_generic_reply,
> > + "domains", NULL);
> > +}
> > +
> > +static void resolve_systemd_set_mdns(struct resolve *resolve, char
> > *mdns)
> > +{
> > + struct l_dbus_message_builder *builder;
> > + struct l_dbus_message *message;
> > +
> > + l_debug("ifindex: %u", resolve->ifindex);
> > +
> > + if (L_WARN_ON(!systemd_state.is_ready))
> > + return;
> > +
> > + message = l_dbus_message_new_method_call(dbus_get_bus(),
> > + SYSTEMD_RESOLVED_SERVICE,
> > + SYSTEMD_RESOLVED_MANAGER_PATH,
> > + SYSTEMD_RESOLVED_MANAGER_INTERF
> > ACE,
> > + "SetLinkMulticastDNS");
> > +
> > + if (!message)
> > + return;
> > +
> > + builder = l_dbus_message_builder_new(message);
> > + if (!builder) {
> > + l_dbus_message_unref(message);
> > + return;
> > + }
> > +
> > + l_dbus_message_builder_append_basic(builder, 'i', &resolve-
> > >ifindex);
> > + l_dbus_message_builder_append_basic(builder, 's', mdns);
> > +
> > + l_dbus_message_builder_finalize(builder);
> > + l_dbus_message_builder_destroy(builder);
>
> Using a builder for something this simple is probably overkill. You
> can replace
> the last ~12 lines by something like:
>
> l_dbus_message_set_arguments(message, "is", resolve->ifindex, mdns);
OK.
>
> > +
> > + l_dbus_send_with_reply(dbus_get_bus(), message,
> > systemd_link_generic_reply,
> > + "Multic
> > astDNS", NULL);
> > }
> >
> > static void resolve_systemd_revert(struct resolve *resolve)
> > @@ -288,8 +321,8 @@ static void resolve_systemd_revert(struct
> > resolve *resolve)
> > return;
> >
> > l_dbus_message_set_arguments(message, "i", resolve->ifindex);
> > - l_dbus_send_with_reply(dbus_get_bus(), message,
> > systemd_link_dns_reply,
> > - NULL,
> > NULL);
> > + l_dbus_send_with_reply(dbus_get_bus(), message,
> > systemd_link_generic_reply,
> > + "DNS",
> > NULL);
> > }
> >
> > static void resolve_systemd_destroy(struct resolve *resolve)
> > @@ -302,6 +335,7 @@ static void resolve_systemd_destroy(struct
> > resolve *resolve)
> > static const struct resolve_ops systemd_ops = {
> > .set_dns = resolve_systemd_set_dns,
> > .set_domains = resolve_systemd_set_domains,
> > + .set_mdns = resolve_systemd_set_mdns,
> > .revert = resolve_systemd_revert,
> > .destroy = resolve_systemd_destroy,
> > };
> > @@ -482,6 +516,7 @@ static void resolve_resolvconf_destroy(struct
> > resolve *resolve)
> > static struct resolve_ops resolvconf_ops = {
> > .set_dns = resolve_resolvconf_set_dns,
> > .set_domains = resolve_resolvconf_set_domains,
> > + .set_mdns = NULL,
>
> This should already be NULL, so not needed and can be dropped
OK.
>
> > .revert = resolve_resolvconf_revert,
> > .destroy = resolve_resolvconf_destroy,
> > };
> > diff --git a/src/resolve.h b/src/resolve.h
> > index 64ed15f3..09cd9ac2 100644
> > --- a/src/resolve.h
> > +++ b/src/resolve.h
> > @@ -23,5 +23,6 @@
> > struct resolve *resolve_new(uint32_t ifindex);
> > void resolve_set_dns(struct resolve *resolve, char **dns_list);
> > void resolve_set_domains(struct resolve *resolve, char
> > **domain_list);
> > +void resolve_set_mdns(struct resolve *resolve, char *mdns);
> > void resolve_revert(struct resolve *resolve);
> > void resolve_free(struct resolve *resolve);
> >
Thanks for the review, I will send an updated patch.
>
> Regards,
> -Denis
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] src: configure systemd-resolved's MulticastDNS= setting
2021-02-11 18:04 ` Daniel Lin
@ 2021-02-11 20:45 ` Denis Kenzior
0 siblings, 0 replies; 8+ messages in thread
From: Denis Kenzior @ 2021-02-11 20:45 UTC (permalink / raw)
To: iwd
[-- Attachment #1: Type: text/plain, Size: 1455 bytes --]
Hi Daniel,
>> Is there a default? If so, it should be in bold. What happens if
>> this setting
>> is omitted?
>
> If the setting is omitted, we don't touch the default value. Which
Ok, so there's no default that iwd should pick.
> appears to be false, but somebody could patch systemd-resolved to
> change that, so I wasn't sure if that should be documented here. I'll
> add that though.
So instead we may want to mention that if this setting is omitted, the
systemd-networkd default is used.
>>> + mdns = l_settings_get_string(active_settings, "Network",
>>> "MulticastDNS");
>>
>> This can return NULL if no MulticastDNS setting is found. What
>> happens then?
>
> Nothing happens, there's a null check in resolve_set_mdns().
>
k, makes sense. I somehow missed that originally.
>>> @@ -40,6 +40,7 @@
>>> struct resolve_ops {
>>> void (*set_dns)(struct resolve *resolve, char **dns_list);
>>> void (*set_domains)(struct resolve *resolve, char
>>> **domain_list);
>>> + void (*set_mdns)(struct resolve *resolve, char *mdns);
>>
>> should probably be const char *mdns
>
> OK. Shouldn't the nearby `char **` parameters be `const char *const *`
> too, then?
>
Possibly. I think we resisted using the const variation since not many old C
hacks like to use such syntax. So we kept using 'char **'.
For passing simple C-strings we like to be const-correct though.
Regards,
-Denis
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2] src: configure systemd-resolved's MulticastDNS= setting
2021-02-10 23:43 [PATCH] src: configure systemd-resolved's MulticastDNS= setting Daniel Lin
2021-02-11 15:59 ` Denis Kenzior
@ 2021-02-11 18:11 ` Daniel Lin
2021-02-11 20:48 ` Denis Kenzior
2021-02-11 20:54 ` [PATCH v3] " Daniel Lin
2 siblings, 1 reply; 8+ messages in thread
From: Daniel Lin @ 2021-02-11 18:11 UTC (permalink / raw)
To: iwd
[-- Attachment #1: Type: text/plain, Size: 6601 bytes --]
When using iwd.conf:[General].EnableNetworkConfiguration=true, it is not
possible to configure systemd.network:[Network].MulticastDNS= as
systemd-networkd considers the link to be unmanaged. This patch allows
iwd to configure that setting on systemd-resolved directly.
---
src/iwd.config.rst | 7 +++++
src/netconfig.c | 6 ++++
src/resolve.c | 70 ++++++++++++++++++++++++++++++----------------
src/resolve.h | 1 +
4 files changed, 60 insertions(+), 24 deletions(-)
diff --git a/src/iwd.config.rst b/src/iwd.config.rst
index 0064dfd2..d430909a 100644
--- a/src/iwd.config.rst
+++ b/src/iwd.config.rst
@@ -212,6 +212,13 @@ The group ``[Network]`` contains network configuration related settings.
If not specified, ``300`` is used as default.
+ * - MulticastDNS
+ - Values: true, **false**, resolve
+
+ Configures multicast DNS. See ``man 5 systemd.network`` for details.
+
+ Only applies when ``NameResolvingService=systemd``.
+
Blacklist
---------
diff --git a/src/netconfig.c b/src/netconfig.c
index cf092b78..749ba0ad 100644
--- a/src/netconfig.c
+++ b/src/netconfig.c
@@ -963,6 +963,8 @@ bool netconfig_configure(struct netconfig *netconfig,
const uint8_t *mac_address,
netconfig_notify_func_t notify, void *user_data)
{
+ char *mdns;
+
netconfig->dns4_overrides = l_settings_get_string_list(active_settings,
"IPv4", "DNS", ' ');
@@ -1006,6 +1008,10 @@ bool netconfig_configure(struct netconfig *netconfig,
netconfig_ipv6_select_and_install(netconfig);
+ mdns = l_settings_get_string(active_settings, "Network", "MulticastDNS");
+ resolve_set_mdns(netconfig->resolve, mdns);
+ l_free(mdns);
+
return true;
}
diff --git a/src/resolve.c b/src/resolve.c
index d3b483a8..5b20b8c9 100644
--- a/src/resolve.c
+++ b/src/resolve.c
@@ -40,6 +40,7 @@
struct resolve_ops {
void (*set_dns)(struct resolve *resolve, char **dns_list);
void (*set_domains)(struct resolve *resolve, char **domain_list);
+ void (*set_mdns)(struct resolve *resolve, const char *mdns);
void (*revert)(struct resolve *resolve);
void (*destroy)(struct resolve *resolve);
};
@@ -78,6 +79,17 @@ void resolve_set_domains(struct resolve *resolve, char **domain_list)
resolve->ops->set_domains(resolve, domain_list);
}
+void resolve_set_mdns(struct resolve *resolve, const char *mdns)
+{
+ if (!mdns)
+ return;
+
+ if (!resolve->ops->set_mdns)
+ return;
+
+ resolve->ops->set_mdns(resolve, mdns);
+}
+
void resolve_revert(struct resolve *resolve)
{
if (!resolve->ops->revert)
@@ -112,9 +124,10 @@ struct systemd {
struct resolve super;
};
-static void systemd_link_dns_reply(struct l_dbus_message *message,
+static void systemd_link_generic_reply(struct l_dbus_message *message,
void *user_data)
{
+ const char *type = user_data;
const char *name;
const char *text;
@@ -123,8 +136,8 @@ static void systemd_link_dns_reply(struct l_dbus_message *message,
l_dbus_message_get_error(message, &name, &text);
- l_error("resolve-systemd: Failed to modify the DNS entries. %s: %s",
- name, text);
+ l_error("resolve-systemd: Failed to modify the %s entries. %s: %s",
+ type, name, text);
}
static bool systemd_builder_add_dns(struct l_dbus_message_builder *builder,
@@ -205,23 +218,8 @@ static void resolve_systemd_set_dns(struct resolve *resolve, char **dns_list)
l_dbus_message_builder_finalize(builder);
l_dbus_message_builder_destroy(builder);
- l_dbus_send_with_reply(dbus_get_bus(), message, systemd_link_dns_reply,
- NULL, NULL);
-}
-
-static void systemd_set_link_domains_reply(struct l_dbus_message *message,
- void *user_data)
-{
- const char *name;
- const char *text;
-
- if (!l_dbus_message_is_error(message))
- return;
-
- l_dbus_message_get_error(message, &name, &text);
-
- l_error("resolve-systemd: Failed to modify the domain entries. %s: %s",
- name, text);
+ l_dbus_send_with_reply(dbus_get_bus(), message, systemd_link_generic_reply,
+ "DNS", NULL);
}
static void resolve_systemd_set_domains(struct resolve *resolve,
@@ -266,8 +264,31 @@ static void resolve_systemd_set_domains(struct resolve *resolve,
l_dbus_message_builder_finalize(builder);
l_dbus_message_builder_destroy(builder);
- l_dbus_send_with_reply(dbus_get_bus(), message,
- systemd_set_link_domains_reply, NULL, NULL);
+ l_dbus_send_with_reply(dbus_get_bus(), message, systemd_link_generic_reply,
+ "domains", NULL);
+}
+
+static void resolve_systemd_set_mdns(struct resolve *resolve, const char *mdns)
+{
+ struct l_dbus_message *message;
+
+ l_debug("ifindex: %u", resolve->ifindex);
+
+ if (L_WARN_ON(!systemd_state.is_ready))
+ return;
+
+ message = l_dbus_message_new_method_call(dbus_get_bus(),
+ SYSTEMD_RESOLVED_SERVICE,
+ SYSTEMD_RESOLVED_MANAGER_PATH,
+ SYSTEMD_RESOLVED_MANAGER_INTERFACE,
+ "SetLinkMulticastDNS");
+
+ if (!message)
+ return;
+
+ l_dbus_message_set_arguments(message, "is", resolve->ifindex, mdns);
+ l_dbus_send_with_reply(dbus_get_bus(), message, systemd_link_generic_reply,
+ "MulticastDNS", NULL);
}
static void resolve_systemd_revert(struct resolve *resolve)
@@ -288,8 +309,8 @@ static void resolve_systemd_revert(struct resolve *resolve)
return;
l_dbus_message_set_arguments(message, "i", resolve->ifindex);
- l_dbus_send_with_reply(dbus_get_bus(), message, systemd_link_dns_reply,
- NULL, NULL);
+ l_dbus_send_with_reply(dbus_get_bus(), message, systemd_link_generic_reply,
+ "DNS", NULL);
}
static void resolve_systemd_destroy(struct resolve *resolve)
@@ -302,6 +323,7 @@ static void resolve_systemd_destroy(struct resolve *resolve)
static const struct resolve_ops systemd_ops = {
.set_dns = resolve_systemd_set_dns,
.set_domains = resolve_systemd_set_domains,
+ .set_mdns = resolve_systemd_set_mdns,
.revert = resolve_systemd_revert,
.destroy = resolve_systemd_destroy,
};
diff --git a/src/resolve.h b/src/resolve.h
index 64ed15f3..2301a288 100644
--- a/src/resolve.h
+++ b/src/resolve.h
@@ -23,5 +23,6 @@
struct resolve *resolve_new(uint32_t ifindex);
void resolve_set_dns(struct resolve *resolve, char **dns_list);
void resolve_set_domains(struct resolve *resolve, char **domain_list);
+void resolve_set_mdns(struct resolve *resolve, const char *mdns);
void resolve_revert(struct resolve *resolve);
void resolve_free(struct resolve *resolve);
--
2.30.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v2] src: configure systemd-resolved's MulticastDNS= setting
2021-02-11 18:11 ` [PATCH v2] " Daniel Lin
@ 2021-02-11 20:48 ` Denis Kenzior
0 siblings, 0 replies; 8+ messages in thread
From: Denis Kenzior @ 2021-02-11 20:48 UTC (permalink / raw)
To: iwd
[-- Attachment #1: Type: text/plain, Size: 1318 bytes --]
On 2/11/21 12:11 PM, Daniel Lin wrote:
> When using iwd.conf:[General].EnableNetworkConfiguration=true, it is not
> possible to configure systemd.network:[Network].MulticastDNS= as
> systemd-networkd considers the link to be unmanaged. This patch allows
> iwd to configure that setting on systemd-resolved directly.
> ---
> src/iwd.config.rst | 7 +++++
> src/netconfig.c | 6 ++++
> src/resolve.c | 70 ++++++++++++++++++++++++++++++----------------
> src/resolve.h | 1 +
> 4 files changed, 60 insertions(+), 24 deletions(-)
>
LGTM, only nitpick is..
> diff --git a/src/iwd.config.rst b/src/iwd.config.rst
> index 0064dfd2..d430909a 100644
> --- a/src/iwd.config.rst
> +++ b/src/iwd.config.rst
> @@ -212,6 +212,13 @@ The group ``[Network]`` contains network configuration related settings.
>
> If not specified, ``300`` is used as default.
>
> + * - MulticastDNS
> + - Values: true, **false**, resolve
As discussed in the v1 reply, we should not indicate a default and mention that
this setting isn't set at all if omitted.
> +
> + Configures multicast DNS. See ``man 5 systemd.network`` for details.
> +
> + Only applies when ``NameResolvingService=systemd``.
> +
> Blacklist
> ---------
>
Regards,
-Denis
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3] src: configure systemd-resolved's MulticastDNS= setting
2021-02-10 23:43 [PATCH] src: configure systemd-resolved's MulticastDNS= setting Daniel Lin
2021-02-11 15:59 ` Denis Kenzior
2021-02-11 18:11 ` [PATCH v2] " Daniel Lin
@ 2021-02-11 20:54 ` Daniel Lin
2021-02-11 21:08 ` Denis Kenzior
2 siblings, 1 reply; 8+ messages in thread
From: Daniel Lin @ 2021-02-11 20:54 UTC (permalink / raw)
To: iwd
[-- Attachment #1: Type: text/plain, Size: 6708 bytes --]
When using iwd.conf:[General].EnableNetworkConfiguration=true, it is not
possible to configure systemd.network:[Network].MulticastDNS= as
systemd-networkd considers the link to be unmanaged. This patch allows
iwd to configure that setting on systemd-resolved directly.
---
src/iwd.config.rst | 9 ++++++
src/netconfig.c | 6 ++++
src/resolve.c | 70 ++++++++++++++++++++++++++++++----------------
src/resolve.h | 1 +
4 files changed, 62 insertions(+), 24 deletions(-)
diff --git a/src/iwd.config.rst b/src/iwd.config.rst
index 0064dfd2..b3c9491e 100644
--- a/src/iwd.config.rst
+++ b/src/iwd.config.rst
@@ -212,6 +212,15 @@ The group ``[Network]`` contains network configuration related settings.
If not specified, ``300`` is used as default.
+ * - MulticastDNS
+ - Values: true, false, resolve
+
+ Configures multicast DNS on each interface. If not specified,
+ systemd-resolved's default value will remain untouched.
+ See ``man 5 systemd.network`` for details.
+
+ Only applies when ``NameResolvingService=systemd``.
+
Blacklist
---------
diff --git a/src/netconfig.c b/src/netconfig.c
index cf092b78..749ba0ad 100644
--- a/src/netconfig.c
+++ b/src/netconfig.c
@@ -963,6 +963,8 @@ bool netconfig_configure(struct netconfig *netconfig,
const uint8_t *mac_address,
netconfig_notify_func_t notify, void *user_data)
{
+ char *mdns;
+
netconfig->dns4_overrides = l_settings_get_string_list(active_settings,
"IPv4", "DNS", ' ');
@@ -1006,6 +1008,10 @@ bool netconfig_configure(struct netconfig *netconfig,
netconfig_ipv6_select_and_install(netconfig);
+ mdns = l_settings_get_string(active_settings, "Network", "MulticastDNS");
+ resolve_set_mdns(netconfig->resolve, mdns);
+ l_free(mdns);
+
return true;
}
diff --git a/src/resolve.c b/src/resolve.c
index d3b483a8..5b20b8c9 100644
--- a/src/resolve.c
+++ b/src/resolve.c
@@ -40,6 +40,7 @@
struct resolve_ops {
void (*set_dns)(struct resolve *resolve, char **dns_list);
void (*set_domains)(struct resolve *resolve, char **domain_list);
+ void (*set_mdns)(struct resolve *resolve, const char *mdns);
void (*revert)(struct resolve *resolve);
void (*destroy)(struct resolve *resolve);
};
@@ -78,6 +79,17 @@ void resolve_set_domains(struct resolve *resolve, char **domain_list)
resolve->ops->set_domains(resolve, domain_list);
}
+void resolve_set_mdns(struct resolve *resolve, const char *mdns)
+{
+ if (!mdns)
+ return;
+
+ if (!resolve->ops->set_mdns)
+ return;
+
+ resolve->ops->set_mdns(resolve, mdns);
+}
+
void resolve_revert(struct resolve *resolve)
{
if (!resolve->ops->revert)
@@ -112,9 +124,10 @@ struct systemd {
struct resolve super;
};
-static void systemd_link_dns_reply(struct l_dbus_message *message,
+static void systemd_link_generic_reply(struct l_dbus_message *message,
void *user_data)
{
+ const char *type = user_data;
const char *name;
const char *text;
@@ -123,8 +136,8 @@ static void systemd_link_dns_reply(struct l_dbus_message *message,
l_dbus_message_get_error(message, &name, &text);
- l_error("resolve-systemd: Failed to modify the DNS entries. %s: %s",
- name, text);
+ l_error("resolve-systemd: Failed to modify the %s entries. %s: %s",
+ type, name, text);
}
static bool systemd_builder_add_dns(struct l_dbus_message_builder *builder,
@@ -205,23 +218,8 @@ static void resolve_systemd_set_dns(struct resolve *resolve, char **dns_list)
l_dbus_message_builder_finalize(builder);
l_dbus_message_builder_destroy(builder);
- l_dbus_send_with_reply(dbus_get_bus(), message, systemd_link_dns_reply,
- NULL, NULL);
-}
-
-static void systemd_set_link_domains_reply(struct l_dbus_message *message,
- void *user_data)
-{
- const char *name;
- const char *text;
-
- if (!l_dbus_message_is_error(message))
- return;
-
- l_dbus_message_get_error(message, &name, &text);
-
- l_error("resolve-systemd: Failed to modify the domain entries. %s: %s",
- name, text);
+ l_dbus_send_with_reply(dbus_get_bus(), message, systemd_link_generic_reply,
+ "DNS", NULL);
}
static void resolve_systemd_set_domains(struct resolve *resolve,
@@ -266,8 +264,31 @@ static void resolve_systemd_set_domains(struct resolve *resolve,
l_dbus_message_builder_finalize(builder);
l_dbus_message_builder_destroy(builder);
- l_dbus_send_with_reply(dbus_get_bus(), message,
- systemd_set_link_domains_reply, NULL, NULL);
+ l_dbus_send_with_reply(dbus_get_bus(), message, systemd_link_generic_reply,
+ "domains", NULL);
+}
+
+static void resolve_systemd_set_mdns(struct resolve *resolve, const char *mdns)
+{
+ struct l_dbus_message *message;
+
+ l_debug("ifindex: %u", resolve->ifindex);
+
+ if (L_WARN_ON(!systemd_state.is_ready))
+ return;
+
+ message = l_dbus_message_new_method_call(dbus_get_bus(),
+ SYSTEMD_RESOLVED_SERVICE,
+ SYSTEMD_RESOLVED_MANAGER_PATH,
+ SYSTEMD_RESOLVED_MANAGER_INTERFACE,
+ "SetLinkMulticastDNS");
+
+ if (!message)
+ return;
+
+ l_dbus_message_set_arguments(message, "is", resolve->ifindex, mdns);
+ l_dbus_send_with_reply(dbus_get_bus(), message, systemd_link_generic_reply,
+ "MulticastDNS", NULL);
}
static void resolve_systemd_revert(struct resolve *resolve)
@@ -288,8 +309,8 @@ static void resolve_systemd_revert(struct resolve *resolve)
return;
l_dbus_message_set_arguments(message, "i", resolve->ifindex);
- l_dbus_send_with_reply(dbus_get_bus(), message, systemd_link_dns_reply,
- NULL, NULL);
+ l_dbus_send_with_reply(dbus_get_bus(), message, systemd_link_generic_reply,
+ "DNS", NULL);
}
static void resolve_systemd_destroy(struct resolve *resolve)
@@ -302,6 +323,7 @@ static void resolve_systemd_destroy(struct resolve *resolve)
static const struct resolve_ops systemd_ops = {
.set_dns = resolve_systemd_set_dns,
.set_domains = resolve_systemd_set_domains,
+ .set_mdns = resolve_systemd_set_mdns,
.revert = resolve_systemd_revert,
.destroy = resolve_systemd_destroy,
};
diff --git a/src/resolve.h b/src/resolve.h
index 64ed15f3..2301a288 100644
--- a/src/resolve.h
+++ b/src/resolve.h
@@ -23,5 +23,6 @@
struct resolve *resolve_new(uint32_t ifindex);
void resolve_set_dns(struct resolve *resolve, char **dns_list);
void resolve_set_domains(struct resolve *resolve, char **domain_list);
+void resolve_set_mdns(struct resolve *resolve, const char *mdns);
void resolve_revert(struct resolve *resolve);
void resolve_free(struct resolve *resolve);
--
2.30.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v3] src: configure systemd-resolved's MulticastDNS= setting
2021-02-11 20:54 ` [PATCH v3] " Daniel Lin
@ 2021-02-11 21:08 ` Denis Kenzior
0 siblings, 0 replies; 8+ messages in thread
From: Denis Kenzior @ 2021-02-11 21:08 UTC (permalink / raw)
To: iwd
[-- Attachment #1: Type: text/plain, Size: 696 bytes --]
Hi Daniel,
On 2/11/21 2:54 PM, Daniel Lin wrote:
> When using iwd.conf:[General].EnableNetworkConfiguration=true, it is not
> possible to configure systemd.network:[Network].MulticastDNS= as
> systemd-networkd considers the link to be unmanaged. This patch allows
> iwd to configure that setting on systemd-resolved directly.
> ---
> src/iwd.config.rst | 9 ++++++
> src/netconfig.c | 6 ++++
> src/resolve.c | 70 ++++++++++++++++++++++++++++++----------------
> src/resolve.h | 1 +
> 4 files changed, 62 insertions(+), 24 deletions(-)
>
I tweaked the line breaks in the patch slightly to fit on 80 char columns.
Applied, thanks.
Regards,
-Denis
^ permalink raw reply [flat|nested] 8+ messages in thread