From 524114add7004bc07c06b5518c5afb6eacbf0217 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 17 Jun 2021 15:57:41 +0200 Subject: [PATCH 1/3] dhcp: minor cleanup of DHCP plugin factory --- src/core/dhcp/nm-dhcp-listener.c | 3 ++- src/core/dhcp/nm-dhcp-manager.c | 8 ++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/core/dhcp/nm-dhcp-listener.c b/src/core/dhcp/nm-dhcp-listener.c index b8bb3c33a0..ae2c40f10f 100644 --- a/src/core/dhcp/nm-dhcp-listener.c +++ b/src/core/dhcp/nm-dhcp-listener.c @@ -26,8 +26,9 @@ /*****************************************************************************/ const NMDhcpClientFactory *const _nm_dhcp_manager_factories[6] = { + /* the order here matters, as we will try the plugins in this order to find - * the first available plugin. */ + * the first available plugin. */ #if WITH_DHCPCANON &_nm_dhcp_client_factory_dhcpcanon, diff --git a/src/core/dhcp/nm-dhcp-manager.c b/src/core/dhcp/nm-dhcp-manager.c index 44b8ede2c0..d6393eb127 100644 --- a/src/core/dhcp/nm-dhcp-manager.c +++ b/src/core/dhcp/nm-dhcp-manager.c @@ -63,9 +63,9 @@ _client_factory_find_by_name(const char *name) { int i; - g_return_val_if_fail(name, NULL); + nm_assert(name); - for (i = 0; i < G_N_ELEMENTS(_nm_dhcp_manager_factories); i++) { + for (i = 0; i < (int) G_N_ELEMENTS(_nm_dhcp_manager_factories); i++) { const NMDhcpClientFactory *f = _nm_dhcp_manager_factories[i]; if (f && nm_streq(f->name, name)) @@ -598,7 +598,7 @@ nm_dhcp_manager_init(NMDhcpManager *self) c_list_init(&priv->dhcp_client_lst_head); - for (i = 0; i < G_N_ELEMENTS(_nm_dhcp_manager_factories); i++) { + for (i = 0; i < (int) G_N_ELEMENTS(_nm_dhcp_manager_factories); i++) { const NMDhcpClientFactory *f = _nm_dhcp_manager_factories[i]; if (!f) @@ -644,7 +644,7 @@ nm_dhcp_manager_init(NMDhcpManager *self) } } if (!client_factory) { - for (i = 0; i < G_N_ELEMENTS(_nm_dhcp_manager_factories); i++) { + for (i = 0; i < (int) G_N_ELEMENTS(_nm_dhcp_manager_factories); i++) { client_factory = _client_factory_available(_nm_dhcp_manager_factories[i]); if (client_factory) break; From eb3ef97dd0894d430cae3630a46528fb61f0bef8 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 17 Jun 2021 16:06:15 +0200 Subject: [PATCH 2/3] dhcp: refactor GType handling for NMDhcpClientFactory In NetworkManager.conf, we can only configure one "[main].dhcp=" for both address families. Consequently, NMDhcpClientFactory represents also both address families. However, most plugins don't support IPv4 and IPv6 together. Thus, if a plugin does not support an address family, we fallback to the implementation of the "internal" plugin. Slightly rework the code how that is done. Instead of having a "get_type()" and "get_type_per_addr_family()" callback, have an IPv4 and IPv6 getter. --- src/core/dhcp/nm-dhcp-client.h | 4 ++-- src/core/dhcp/nm-dhcp-dhclient.c | 7 ++++--- src/core/dhcp/nm-dhcp-dhcpcanon.c | 6 +++--- src/core/dhcp/nm-dhcp-dhcpcd.c | 6 +++--- src/core/dhcp/nm-dhcp-manager.c | 34 ++++++++++++------------------- src/core/dhcp/nm-dhcp-nettools.c | 2 +- src/core/dhcp/nm-dhcp-systemd.c | 18 +++++----------- 7 files changed, 31 insertions(+), 46 deletions(-) diff --git a/src/core/dhcp/nm-dhcp-client.h b/src/core/dhcp/nm-dhcp-client.h index 3fe1b34ec9..9e3ccc8fed 100644 --- a/src/core/dhcp/nm-dhcp-client.h +++ b/src/core/dhcp/nm-dhcp-client.h @@ -217,8 +217,8 @@ gboolean nm_dhcp_client_server_id_is_rejected(NMDhcpClient *self, gconstpointer *****************************************************************************/ typedef struct { - GType (*get_type)(void); - GType (*get_type_per_addr_family)(int addr_family); + GType (*get_type_4)(void); + GType (*get_type_6)(void); const char *name; const char *(*get_path)(void); bool experimental : 1; diff --git a/src/core/dhcp/nm-dhcp-dhclient.c b/src/core/dhcp/nm-dhcp-dhclient.c index 4a11250f9b..5077407f0e 100644 --- a/src/core/dhcp/nm-dhcp-dhclient.c +++ b/src/core/dhcp/nm-dhcp-dhclient.c @@ -731,9 +731,10 @@ nm_dhcp_dhclient_class_init(NMDhcpDhclientClass *dhclient_class) } const NMDhcpClientFactory _nm_dhcp_client_factory_dhclient = { - .name = "dhclient", - .get_type = nm_dhcp_dhclient_get_type, - .get_path = nm_dhcp_dhclient_get_path, + .name = "dhclient", + .get_type_4 = nm_dhcp_dhclient_get_type, + .get_type_6 = nm_dhcp_dhclient_get_type, + .get_path = nm_dhcp_dhclient_get_path, }; #endif /* WITH_DHCLIENT */ diff --git a/src/core/dhcp/nm-dhcp-dhcpcanon.c b/src/core/dhcp/nm-dhcp-dhcpcanon.c index f3a52ea959..2be64c09f2 100644 --- a/src/core/dhcp/nm-dhcp-dhcpcanon.c +++ b/src/core/dhcp/nm-dhcp-dhcpcanon.c @@ -232,9 +232,9 @@ nm_dhcp_dhcpcanon_class_init(NMDhcpDhcpcanonClass *dhcpcanon_class) } const NMDhcpClientFactory _nm_dhcp_client_factory_dhcpcanon = { - .name = "dhcpcanon", - .get_type = nm_dhcp_dhcpcanon_get_type, - .get_path = nm_dhcp_dhcpcanon_get_path, + .name = "dhcpcanon", + .get_type_4 = nm_dhcp_dhcpcanon_get_type, + .get_path = nm_dhcp_dhcpcanon_get_path, }; #endif /* WITH_DHCPCANON */ diff --git a/src/core/dhcp/nm-dhcp-dhcpcd.c b/src/core/dhcp/nm-dhcp-dhcpcd.c index 605fb84df4..86269f9306 100644 --- a/src/core/dhcp/nm-dhcp-dhcpcd.c +++ b/src/core/dhcp/nm-dhcp-dhcpcd.c @@ -233,9 +233,9 @@ nm_dhcp_dhcpcd_class_init(NMDhcpDhcpcdClass *dhcpcd_class) } const NMDhcpClientFactory _nm_dhcp_client_factory_dhcpcd = { - .name = "dhcpcd", - .get_type = nm_dhcp_dhcpcd_get_type, - .get_path = nm_dhcp_dhcpcd_get_path, + .name = "dhcpcd", + .get_type_4 = nm_dhcp_dhcpcd_get_type, + .get_path = nm_dhcp_dhcpcd_get_path, }; #endif /* WITH_DHCPCD */ diff --git a/src/core/dhcp/nm-dhcp-manager.c b/src/core/dhcp/nm-dhcp-manager.c index d6393eb127..b264d2abeb 100644 --- a/src/core/dhcp/nm-dhcp-manager.c +++ b/src/core/dhcp/nm-dhcp-manager.c @@ -85,11 +85,10 @@ _client_factory_available(const NMDhcpClientFactory *client_factory) static GType _client_factory_get_gtype(const NMDhcpClientFactory *client_factory, int addr_family) { - GType gtype; - nm_auto_unref_gtypeclass NMDhcpClientClass *klass = NULL; + GType gtype; + GType (*get_type_fcn)(void); nm_assert(client_factory); - nm_assert_addr_family(addr_family); /* currently, the chosen DHCP plugin for IPv4 and IPv6 is configured in NetworkManager.conf * and cannot be reloaded. It would be nice to configure the plugin per address family @@ -111,29 +110,22 @@ _client_factory_get_gtype(const NMDhcpClientFactory *client_factory, int addr_fa * to those plugins. But we don't intend to do so. The internal plugin is the way forward and * not extending other plugins. */ - if (client_factory->get_type_per_addr_family) - gtype = client_factory->get_type_per_addr_family(addr_family); + if (NM_IS_IPv4(addr_family)) + get_type_fcn = client_factory->get_type_4; else - gtype = client_factory->get_type(); + get_type_fcn = client_factory->get_type_6; - if (client_factory == &_nm_dhcp_client_factory_internal) { - /* we are already using the internal plugin. Nothing to do. */ - goto out; + if (!get_type_fcn) { + /* If the factory does not support the address family, we always + * fallback to the internal. */ + if (NM_IS_IPv4(addr_family)) + get_type_fcn = _nm_dhcp_client_factory_internal.get_type_4; + else + get_type_fcn = _nm_dhcp_client_factory_internal.get_type_6; } - klass = g_type_class_ref(gtype); + gtype = get_type_fcn(); - nm_assert(NM_IS_DHCP_CLIENT_CLASS(klass)); - - if (addr_family == AF_INET6) { - if (!klass->ip6_start) - gtype = _client_factory_get_gtype(&_nm_dhcp_client_factory_internal, addr_family); - } else { - if (!klass->ip4_start) - gtype = _client_factory_get_gtype(&_nm_dhcp_client_factory_internal, addr_family); - } - -out: nm_assert(g_type_is_a(gtype, NM_TYPE_DHCP_CLIENT)); nm_assert(({ nm_auto_unref_gtypeclass NMDhcpClientClass *k = g_type_class_ref(gtype); diff --git a/src/core/dhcp/nm-dhcp-nettools.c b/src/core/dhcp/nm-dhcp-nettools.c index fe71f587a4..ddc86738b8 100644 --- a/src/core/dhcp/nm-dhcp-nettools.c +++ b/src/core/dhcp/nm-dhcp-nettools.c @@ -1248,6 +1248,6 @@ nm_dhcp_nettools_class_init(NMDhcpNettoolsClass *class) const NMDhcpClientFactory _nm_dhcp_client_factory_nettools = { .name = "nettools", - .get_type = nm_dhcp_nettools_get_type, + .get_type_4 = nm_dhcp_nettools_get_type, .experimental = TRUE, }; diff --git a/src/core/dhcp/nm-dhcp-systemd.c b/src/core/dhcp/nm-dhcp-systemd.c index c789aaee0f..4f93aef71a 100644 --- a/src/core/dhcp/nm-dhcp-systemd.c +++ b/src/core/dhcp/nm-dhcp-systemd.c @@ -1124,23 +1124,15 @@ nm_dhcp_systemd_class_init(NMDhcpSystemdClass *sdhcp_class) const NMDhcpClientFactory _nm_dhcp_client_factory_systemd = { .name = "systemd", - .get_type = nm_dhcp_systemd_get_type, + .get_type_4 = nm_dhcp_systemd_get_type, + .get_type_6 = nm_dhcp_systemd_get_type, .experimental = TRUE, }; /*****************************************************************************/ -static GType -_get_type_per_addr_family(int addr_family) -{ - nm_assert_addr_family(addr_family); - - if (addr_family == AF_INET) - return nm_dhcp_nettools_get_type(); - return nm_dhcp_systemd_get_type(); -} - const NMDhcpClientFactory _nm_dhcp_client_factory_internal = { - .name = "internal", - .get_type_per_addr_family = _get_type_per_addr_family, + .name = "internal", + .get_type_4 = nm_dhcp_nettools_get_type, + .get_type_6 = nm_dhcp_systemd_get_type, }; From c5e7e2f69403e59dbc7676ff60ecc1d36a703207 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 17 Jun 2021 16:20:17 +0200 Subject: [PATCH 3/3] dhcp/trivial: rename "NMDhcpClientFactory.experimental" to "NMDhcpClientFactory.undocumented" It's not experimental. It's not officially documented. Rename. --- src/core/dhcp/nm-dhcp-client.h | 4 +++- src/core/dhcp/nm-dhcp-manager.c | 2 +- src/core/dhcp/nm-dhcp-nettools.c | 2 +- src/core/dhcp/nm-dhcp-systemd.c | 2 +- 4 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/core/dhcp/nm-dhcp-client.h b/src/core/dhcp/nm-dhcp-client.h index 9e3ccc8fed..601225ecd6 100644 --- a/src/core/dhcp/nm-dhcp-client.h +++ b/src/core/dhcp/nm-dhcp-client.h @@ -221,7 +221,9 @@ typedef struct { GType (*get_type_6)(void); const char *name; const char *(*get_path)(void); - bool experimental : 1; + + /* whether this plugin is an undocumented, internal plugin. */ + bool undocumented : 1; } NMDhcpClientFactory; GType nm_dhcp_nettools_get_type(void); diff --git a/src/core/dhcp/nm-dhcp-manager.c b/src/core/dhcp/nm-dhcp-manager.c index b264d2abeb..c3328b010b 100644 --- a/src/core/dhcp/nm-dhcp-manager.c +++ b/src/core/dhcp/nm-dhcp-manager.c @@ -600,7 +600,7 @@ nm_dhcp_manager_init(NMDhcpManager *self) "dhcp-init: enabled DHCP client '%s'%s%s", f->name, _client_factory_available(f) ? "" : " (not available)", - f->experimental ? " (undocumented internal plugin)" : ""); + f->undocumented ? " (undocumented internal plugin)" : ""); } /* Client-specific setup */ diff --git a/src/core/dhcp/nm-dhcp-nettools.c b/src/core/dhcp/nm-dhcp-nettools.c index ddc86738b8..f755eebca3 100644 --- a/src/core/dhcp/nm-dhcp-nettools.c +++ b/src/core/dhcp/nm-dhcp-nettools.c @@ -1249,5 +1249,5 @@ nm_dhcp_nettools_class_init(NMDhcpNettoolsClass *class) const NMDhcpClientFactory _nm_dhcp_client_factory_nettools = { .name = "nettools", .get_type_4 = nm_dhcp_nettools_get_type, - .experimental = TRUE, + .undocumented = TRUE, }; diff --git a/src/core/dhcp/nm-dhcp-systemd.c b/src/core/dhcp/nm-dhcp-systemd.c index 4f93aef71a..4bb58ee354 100644 --- a/src/core/dhcp/nm-dhcp-systemd.c +++ b/src/core/dhcp/nm-dhcp-systemd.c @@ -1126,7 +1126,7 @@ const NMDhcpClientFactory _nm_dhcp_client_factory_systemd = { .name = "systemd", .get_type_4 = nm_dhcp_systemd_get_type, .get_type_6 = nm_dhcp_systemd_get_type, - .experimental = TRUE, + .undocumented = TRUE, }; /*****************************************************************************/