core: rework IP configuration in NetworkManager using layer 3 configuration

Completely rework IP configuration in the daemon. Use NML3Cfg as layer 3
manager for the IP configuration of an interface. Use NML3ConfigData as
pieces of configuration that the various components collect and
configure. NMDevice is managing most of the IP configuration at a higher
level, that is, it starts DHCP and other IP methods. Rework the state
handling there.

This is a huge rework of how NetworkManager daemon handles IP
configuration. Some fallout is to be expected.

It appears the patch deletes many lines of code. That is not accurate, because
you also have to count the files `src/core/nm-l3*`, which were unused previously.

Co-authored-by: Beniamino Galvani <bgalvani@redhat.com>

1 files changed, 130 insertions, 122 deletions

diff --git a/src/core/ppp/nm-ppp-manager.c b/src/core/ppp/nm-ppp-manager.c
index e6790f2260..c347978ff0 100644
--- a/src/core/ppp/nm-ppp-manager.c
+++ b/src/core/ppp/nm-ppp-manager.c
@@ -32,8 +32,7 @@
 #include "libnm-platform/nm-platform-utils.h"
 #include "libnm-core-intern/nm-core-internal.h"
 #include "nm-act-request.h"
-#include "nm-ip4-config.h"
-#include "nm-ip6-config.h"
+#include "nm-l3-config-data.h"
 #include "nm-dbus-object.h"
 
 #include "nm-pppd-plugin.h"
@@ -46,6 +45,26 @@ static NM_CACHED_QUARK_FCN("ppp-manager-secret-tries", ppp_manager_secret_tries_
 
 /*****************************************************************************/
 
+/* FIXME(l3cfg:ppp): I think NMPPPManager's API should be improved to be easier
+ * usable (by the higher layers). That means to make the class more complex, to
+ * provide a simpler API.
+ *
+ * For example:
+ *
+ * - NM_PPP_MANAGER_SIGNAL_STATE_CHANGED just gets re-emitted when we receive
+ *   the D-Bus call from the plugin. The emitted state is like NM_PPP_STATUS_SERIALCONN,
+ *   but none of the users cares about this (what would it mean anyway)? The
+ *   class should itself consume the state, and emit something more consumable
+ *   (like: interface is ready (with ifindex), IPvX configuration done (with l3cd).
+ *
+ * - currently signals can be emitted in any order, and it's not clear which
+ *   signals we can expect. For example, when we activate a device, we may want to wait
+ *   for IPv4 and IPv6 configuration, but it's not clear whether this configuration
+ *   is still to be received or whether we can stop waiting.
+ **/
+
+/*****************************************************************************/
+
 #define NM_TYPE_PPP_MANAGER (nm_ppp_manager_get_type())
 #define NM_PPP_MANAGER(obj) (G_TYPE_CHECK_INSTANCE_CAST((obj), NM_TYPE_PPP_MANAGER, NMPPPManager))
 #define NM_PPP_MANAGER_CLASS(klass) \
@@ -62,8 +81,7 @@ GType nm_ppp_manager_get_type(void);
 enum {
     STATE_CHANGED,
     IFINDEX_SET,
-    IP4_CONFIG,
-    IP6_CONFIG,
+    NEW_CONFIG,
     STATS,
 
     LAST_SIGNAL
@@ -91,11 +109,6 @@ typedef struct {
     /* Monitoring */
     int   monitor_fd;
     guint monitor_id;
-
-    guint32 ip4_route_table;
-    guint32 ip4_route_metric;
-    guint32 ip6_route_table;
-    guint32 ip6_route_metric;
 } NMPPPManagerPrivate;
 
 struct _NMPPPManager {
@@ -131,30 +144,19 @@ static void _ppp_manager_stop_cancel(NMPPPManagerStopHandle *handle);
 /*****************************************************************************/
 
 static void
-_ppp_manager_set_route_parameters(NMPPPManager *self,
-                                  guint32       ip4_route_table,
-                                  guint32       ip4_route_metric,
-                                  guint32       ip6_route_table,
-                                  guint32       ip6_route_metric)
+_emit_signal_new_config(NMPPPManager *            self,
+                        int                       addr_family,
+                        const NML3ConfigData *    l3cd,
+                        const NMUtilsIPv6IfaceId *iid)
 {
-    NMPPPManagerPrivate *priv;
+    nm_assert(NM_IS_PPP_MANAGER(self));
+    nm_assert_addr_family(addr_family);
+    nm_assert(NM_IS_L3_CONFIG_DATA(l3cd));
+    nm_assert((!!iid) == (addr_family == AF_INET6));
 
-    g_return_if_fail(NM_IS_PPP_MANAGER(self));
+    nm_l3_config_data_seal(l3cd);
 
-    priv = NM_PPP_MANAGER_GET_PRIVATE(self);
-    if (priv->ip4_route_table != ip4_route_table || priv->ip4_route_metric != ip4_route_metric
-        || priv->ip6_route_table != ip6_route_table || priv->ip6_route_metric != ip6_route_metric) {
-        priv->ip4_route_table  = ip4_route_table;
-        priv->ip4_route_metric = ip4_route_metric;
-        priv->ip6_route_table  = ip6_route_table;
-        priv->ip6_route_metric = ip6_route_metric;
-
-        _LOGT("route-parameters: table-v4: %u, metric-v4: %u, table-v6: %u, metric-v6: %u",
-              priv->ip4_route_table,
-              priv->ip4_route_metric,
-              priv->ip6_route_table,
-              priv->ip6_route_metric);
-    }
+    g_signal_emit(self, signals[NEW_CONFIG], 0, addr_family, l3cd, iid);
 }
 
 /*****************************************************************************/
@@ -419,10 +421,16 @@ impl_ppp_manager_set_state(NMDBusObject *                     obj,
                            GVariant *                         parameters)
 {
     NMPPPManager *self = NM_PPP_MANAGER(obj);
-    guint32       state;
+    guint32       ppp_state;
+
+    g_variant_get(parameters, "(u)", &ppp_state);
+
+    if (ppp_state >= NM_PPP_STATUS_INTERN_DEAD) {
+        /* we don't expect an intern state to be reported by the plugin. */
+        ppp_state = NM_PPP_STATUS_UNKNOWN;
+    }
 
-    g_variant_get(parameters, "(u)", &state);
-    g_signal_emit(self, signals[STATE_CHANGED], 0, (guint) state);
+    g_signal_emit(self, signals[STATE_CHANGED], 0, (guint) ppp_state);
     g_dbus_method_invocation_return_value(invocation, NULL);
 }
 
@@ -435,9 +443,10 @@ impl_ppp_manager_set_ifindex(NMDBusObject *                     obj,
                              GDBusMethodInvocation *            invocation,
                              GVariant *                         parameters)
 {
-    NMPPPManager *        self                     = NM_PPP_MANAGER(obj);
-    NMPPPManagerPrivate * priv                     = NM_PPP_MANAGER_GET_PRIVATE(self);
-    const NMPlatformLink *plink                    = NULL;
+    NMPPPManager *  self                           = NM_PPP_MANAGER(obj);
+    gs_unref_object NMPPPManager *self_keep_alive  = NULL;
+    NMPPPManagerPrivate *         priv             = NM_PPP_MANAGER_GET_PRIVATE(self);
+    const NMPlatformLink *        plink            = NULL;
     nm_auto_nmpobj const NMPObject *obj_keep_alive = NULL;
     gint32                          ifindex;
 
@@ -456,6 +465,10 @@ impl_ppp_manager_set_ifindex(NMDBusObject *                     obj,
     if (ifindex > 0) {
         plink = nm_platform_link_get(NM_PLATFORM_GET, ifindex);
         if (!plink) {
+            /* processing events has side-effects. We need to keep self alive
+             * during that.*/
+            self_keep_alive = g_object_ref(self);
+
             nm_platform_process_events(NM_PLATFORM_GET);
             plink = nm_platform_link_get(NM_PLATFORM_GET, ifindex);
         }
@@ -510,12 +523,12 @@ impl_ppp_manager_set_ip4_config(NMDBusObject *                     obj,
                                 GDBusMethodInvocation *            invocation,
                                 GVariant *                         parameters)
 {
-    NMPPPManager *       self           = NM_PPP_MANAGER(obj);
-    NMPPPManagerPrivate *priv           = NM_PPP_MANAGER_GET_PRIVATE(self);
-    gs_unref_object NMIP4Config *config = NULL;
-    NMPlatformIP4Address         address;
-    guint32                      u32, mtu;
-    GVariantIter *               iter;
+    NMPPPManager *          self                 = NM_PPP_MANAGER(obj);
+    NMPPPManagerPrivate *   priv                 = NM_PPP_MANAGER_GET_PRIVATE(self);
+    nm_auto_unref_l3cd_init NML3ConfigData *l3cd = NULL;
+    NMPlatformIP4Address                    address;
+    guint32                                 u32, mtu;
+    GVariantIter *                          iter;
     gs_unref_variant GVariant *config_dict = NULL;
 
     _LOGI("(IPv4 Config Get) reply received.");
@@ -527,37 +540,41 @@ impl_ppp_manager_set_ip4_config(NMDBusObject *                     obj,
     if (!set_ip_config_common(self, config_dict, &mtu))
         goto out;
 
-    config = nm_ip4_config_new(nm_platform_get_multi_idx(NM_PLATFORM_GET), priv->ifindex);
+    l3cd = nm_l3_config_data_new(nm_platform_get_multi_idx(NM_PLATFORM_GET),
+                                 priv->ifindex,
+                                 NM_IP_CONFIG_SOURCE_PPP);
 
-    if (mtu)
-        nm_ip4_config_set_mtu(config, mtu, NM_IP_CONFIG_SOURCE_PPP);
+    nm_l3_config_data_set_mtu(l3cd, mtu);
 
-    memset(&address, 0, sizeof(address));
-    address.plen = 32;
+    address = (NMPlatformIP4Address){
+        .plen = 32,
+    };
 
     if (g_variant_lookup(config_dict, NM_PPP_IP4_CONFIG_ADDRESS, "u", &u32))
         address.address = u32;
 
+    if (g_variant_lookup(config_dict, NM_PPP_IP4_CONFIG_PREFIX, "u", &u32))
+        address.plen = u32;
+
     if (g_variant_lookup(config_dict, NM_PPP_IP4_CONFIG_GATEWAY, "u", &u32)) {
         const NMPlatformIP4Route r = {
             .ifindex       = priv->ifindex,
             .rt_source     = NM_IP_CONFIG_SOURCE_PPP,
             .gateway       = u32,
-            .table_coerced = nm_platform_route_table_coerce(priv->ip4_route_table),
-            .metric        = priv->ip4_route_metric,
+            .table_any     = TRUE,
+            .table_coerced = 0,
+            .metric_any    = TRUE,
+            .metric        = 0,
         };
 
-        nm_ip4_config_add_route(config, &r, NULL);
+        nm_l3_config_data_add_route_4(l3cd, &r);
         address.peer_address = u32;
     } else
         address.peer_address = address.address;
 
-    if (g_variant_lookup(config_dict, NM_PPP_IP4_CONFIG_PREFIX, "u", &u32))
-        address.plen = u32;
-
-    if (address.address && address.plen && address.plen <= 32) {
+    if (address.address && address.plen > 0 && address.plen <= 32) {
         address.addr_source = NM_IP_CONFIG_SOURCE_PPP;
-        nm_ip4_config_add_address(config, &address);
+        nm_l3_config_data_add_address_4(l3cd, &address);
     } else {
         _LOGE("invalid IPv4 address received!");
         goto out;
@@ -565,18 +582,17 @@ impl_ppp_manager_set_ip4_config(NMDBusObject *                     obj,
 
     if (g_variant_lookup(config_dict, NM_PPP_IP4_CONFIG_DNS, "au", &iter)) {
         while (g_variant_iter_next(iter, "u", &u32))
-            nm_ip4_config_add_nameserver(config, u32);
+            nm_l3_config_data_add_nameserver(l3cd, AF_INET, &u32);
         g_variant_iter_free(iter);
     }
 
     if (g_variant_lookup(config_dict, NM_PPP_IP4_CONFIG_WINS, "au", &iter)) {
         while (g_variant_iter_next(iter, "u", &u32))
-            nm_ip4_config_add_wins(config, u32);
+            nm_l3_config_data_add_wins(l3cd, u32);
         g_variant_iter_free(iter);
     }
 
-    /* Push the IP4 config up to the device */
-    g_signal_emit(self, signals[IP4_CONFIG], 0, config);
+    _emit_signal_new_config(self, AF_INET, l3cd, NULL);
 
 out:
     g_dbus_method_invocation_return_value(invocation, NULL);
@@ -620,14 +636,14 @@ impl_ppp_manager_set_ip6_config(NMDBusObject *                     obj,
                                 GDBusMethodInvocation *            invocation,
                                 GVariant *                         parameters)
 {
-    NMPPPManager *       self           = NM_PPP_MANAGER(obj);
-    NMPPPManagerPrivate *priv           = NM_PPP_MANAGER_GET_PRIVATE(self);
-    gs_unref_object NMIP6Config *config = NULL;
-    NMPlatformIP6Address         addr;
-    struct in6_addr              a;
-    NMUtilsIPv6IfaceId           iid       = NM_UTILS_IPV6_IFACE_ID_INIT;
-    gboolean                     has_peer  = FALSE;
-    gs_unref_variant GVariant *config_dict = NULL;
+    NMPPPManager *          self                 = NM_PPP_MANAGER(obj);
+    NMPPPManagerPrivate *   priv                 = NM_PPP_MANAGER_GET_PRIVATE(self);
+    nm_auto_unref_l3cd_init NML3ConfigData *l3cd = NULL;
+    NMPlatformIP6Address                    address;
+    struct in6_addr                         a;
+    NMUtilsIPv6IfaceId                      iid      = NM_UTILS_IPV6_IFACE_ID_INIT;
+    gboolean                                has_peer = FALSE;
+    gs_unref_variant GVariant *config_dict           = NULL;
 
     _LOGI("(IPv6 Config Get) reply received.");
 
@@ -638,32 +654,37 @@ impl_ppp_manager_set_ip6_config(NMDBusObject *                     obj,
     if (!set_ip_config_common(self, config_dict, NULL))
         goto out;
 
-    config = nm_ip6_config_new(nm_platform_get_multi_idx(NM_PLATFORM_GET), priv->ifindex);
+    l3cd = nm_l3_config_data_new(nm_platform_get_multi_idx(NM_PLATFORM_GET),
+                                 priv->ifindex,
+                                 NM_IP_CONFIG_SOURCE_PPP);
 
-    memset(&addr, 0, sizeof(addr));
-    addr.plen = 64;
+    address = (NMPlatformIP6Address){
+        .plen        = 64,
+        .addr_source = NM_IP_CONFIG_SOURCE_PPP,
+    };
 
     if (iid_value_to_ll6_addr(config_dict, NM_PPP_IP6_CONFIG_PEER_IID, &a, NULL)) {
         const NMPlatformIP6Route r = {
             .ifindex       = priv->ifindex,
             .rt_source     = NM_IP_CONFIG_SOURCE_PPP,
             .gateway       = a,
-            .table_coerced = nm_platform_route_table_coerce(priv->ip6_route_table),
-            .metric        = priv->ip6_route_metric,
+            .table_any     = TRUE,
+            .table_coerced = 0,
+            .metric_any    = TRUE,
+            .metric        = 0,
         };
 
-        nm_ip6_config_add_route(config, &r, NULL);
-        addr.peer_address = a;
-        has_peer          = TRUE;
+        nm_l3_config_data_add_route_6(l3cd, &r);
+        address.peer_address = a;
+        has_peer             = TRUE;
     }
 
-    if (iid_value_to_ll6_addr(config_dict, NM_PPP_IP6_CONFIG_OUR_IID, &addr.address, &iid)) {
+    if (iid_value_to_ll6_addr(config_dict, NM_PPP_IP6_CONFIG_OUR_IID, &address.address, &iid)) {
         if (!has_peer)
-            addr.peer_address = addr.address;
-        nm_ip6_config_add_address(config, &addr);
+            address.peer_address = address.address;
+        nm_l3_config_data_add_address_6(l3cd, &address);
 
-        /* Push the IPv6 config and interface identifier up to the device */
-        g_signal_emit(self, signals[IP6_CONFIG], 0, &iid, config);
+        _emit_signal_new_config(self, AF_INET6, l3cd, &iid);
     } else
         _LOGE("invalid IPv6 address received!");
 
@@ -726,7 +747,7 @@ ppp_watch_cb(GPid pid, int status, gpointer user_data)
     priv->pid          = 0;
     priv->ppp_watch_id = 0;
     _ppp_cleanup(self);
-    g_signal_emit(self, signals[STATE_CHANGED], 0, (guint) NM_PPP_STATUS_DEAD);
+    g_signal_emit(self, signals[STATE_CHANGED], 0, (guint) NM_PPP_STATUS_INTERN_DEAD);
 }
 
 static gboolean
@@ -734,10 +755,16 @@ pppd_timed_out(gpointer data)
 {
     NMPPPManager *self = NM_PPP_MANAGER(data);
 
+    /* FIXME(l3cfg): we should not use NMPPPManager directly, instead use
+     * NMPppMgr. That one already schedules a (better) timer. We don't need
+     * a timeout here anymore.
+     *
+     * At this moment, NMPPPManager is still used by NMModem. Once that changes,
+     * this timer needs to go. */
     _LOGW("pppd timed out or didn't initialize our dbus module");
     _ppp_manager_stop(self, NULL, NULL, NULL);
 
-    g_signal_emit(self, signals[STATE_CHANGED], 0, (guint) NM_PPP_STATUS_DEAD);
+    g_signal_emit(self, signals[STATE_CHANGED], 0, (guint) NM_PPP_STATUS_INTERN_DEAD);
 
     return FALSE;
 }
@@ -975,9 +1002,8 @@ _ppp_manager_start(NMPPPManager *self,
     gs_unref_ptrarray GPtrArray *ppp_cmd = NULL;
     gs_free char *               cmd_str = NULL;
     struct stat                  st;
-    const char *                 ip6_method, *ip4_method;
-    gboolean                     ip6_enabled = FALSE;
-    gboolean                     ip4_enabled = FALSE;
+    gboolean                     ip6_enabled;
+    gboolean                     ip4_enabled;
 
     g_return_val_if_fail(NM_IS_PPP_MANAGER(self), FALSE);
     g_return_val_if_fail(NM_IS_ACT_REQUEST(req), FALSE);
@@ -1022,11 +1048,7 @@ _ppp_manager_start(NMPPPManager *self,
 
     adsl_setting = (NMSettingAdsl *) nm_connection_get_setting(connection, NM_TYPE_SETTING_ADSL);
 
-    /* Figure out what address methods should be enabled */
-    ip4_method  = nm_utils_get_ip_config_method(connection, AF_INET);
-    ip4_enabled = nm_streq(ip4_method, NM_SETTING_IP4_CONFIG_METHOD_AUTO);
-    ip6_method  = nm_utils_get_ip_config_method(connection, AF_INET6);
-    ip6_enabled = nm_streq(ip6_method, NM_SETTING_IP6_CONFIG_METHOD_AUTO);
+    nm_utils_ppp_ip_methods_enabled(connection, &ip4_enabled, &ip6_enabled);
 
     ppp_cmd = create_pppd_cmd_line(self,
                                    s_ppp,
@@ -1059,9 +1081,10 @@ _ppp_manager_start(NMPPPManager *self,
 
     _LOGI("pppd started with pid %lld", (long long) priv->pid);
 
-    priv->ppp_watch_id        = g_child_watch_add(priv->pid, (GChildWatchFunc) ppp_watch_cb, self);
-    priv->ppp_timeout_handler = g_timeout_add_seconds(timeout_secs, pppd_timed_out, self);
-    priv->act_req             = g_object_ref(req);
+    priv->ppp_watch_id = g_child_watch_add(priv->pid, (GChildWatchFunc) ppp_watch_cb, self);
+    if (timeout_secs > 0)
+        priv->ppp_timeout_handler = g_timeout_add_seconds(timeout_secs, pppd_timed_out, self);
+    priv->act_req = g_object_ref(req);
 
     return TRUE;
 fail:
@@ -1290,12 +1313,8 @@ nm_ppp_manager_init(NMPPPManager *self)
 {
     NMPPPManagerPrivate *priv = NM_PPP_MANAGER_GET_PRIVATE(self);
 
-    priv->ifindex          = -1;
-    priv->monitor_fd       = -1;
-    priv->ip4_route_table  = RT_TABLE_MAIN;
-    priv->ip4_route_metric = 460;
-    priv->ip6_route_table  = RT_TABLE_MAIN;
-    priv->ip6_route_metric = 460;
+    priv->ifindex    = -1;
+    priv->monitor_fd = -1;
 }
 
 static NMPPPManager *
@@ -1400,7 +1419,7 @@ nm_ppp_manager_class_init(NMPPPManagerClass *manager_class)
                                           NULL,
                                           G_TYPE_NONE,
                                           1,
-                                          G_TYPE_UINT);
+                                          G_TYPE_UINT /* ppp_state */);
 
     signals[IFINDEX_SET] = g_signal_new(NM_PPP_MANAGER_SIGNAL_IFINDEX_SET,
                                         G_OBJECT_CLASS_TYPE(object_class),
@@ -1411,21 +1430,10 @@ nm_ppp_manager_class_init(NMPPPManagerClass *manager_class)
                                         NULL,
                                         G_TYPE_NONE,
                                         2,
-                                        G_TYPE_INT,
-                                        G_TYPE_STRING);
-
-    signals[IP4_CONFIG] = g_signal_new(NM_PPP_MANAGER_SIGNAL_IP4_CONFIG,
-                                       G_OBJECT_CLASS_TYPE(object_class),
-                                       G_SIGNAL_RUN_FIRST,
-                                       0,
-                                       NULL,
-                                       NULL,
-                                       NULL,
-                                       G_TYPE_NONE,
-                                       1,
-                                       G_TYPE_OBJECT);
+                                        G_TYPE_INT /* ifindex */,
+                                        G_TYPE_STRING /* ifname */);
 
-    signals[IP6_CONFIG] = g_signal_new(NM_PPP_MANAGER_SIGNAL_IP6_CONFIG,
+    signals[NEW_CONFIG] = g_signal_new(NM_PPP_MANAGER_SIGNAL_NEW_CONFIG,
                                        G_OBJECT_CLASS_TYPE(object_class),
                                        G_SIGNAL_RUN_FIRST,
                                        0,
@@ -1433,9 +1441,10 @@ nm_ppp_manager_class_init(NMPPPManagerClass *manager_class)
                                        NULL,
                                        NULL,
                                        G_TYPE_NONE,
-                                       2,
-                                       G_TYPE_POINTER,
-                                       G_TYPE_OBJECT);
+                                       3,
+                                       G_TYPE_INT,      /* addr_family */
+                                       G_TYPE_POINTER,  /* (const NML3ConfigData *) */
+                                       G_TYPE_POINTER); /* (const NMUtilsIPv6IfaceId *) */
 
     signals[STATS] = g_signal_new(NM_PPP_MANAGER_SIGNAL_STATS,
                                   G_OBJECT_CLASS_TYPE(object_class),
@@ -1451,9 +1460,8 @@ nm_ppp_manager_class_init(NMPPPManagerClass *manager_class)
 }
 
 const NMPPPOps ppp_ops = {
-    .create               = _ppp_manager_new,
-    .set_route_parameters = _ppp_manager_set_route_parameters,
-    .start                = _ppp_manager_start,
-    .stop                 = _ppp_manager_stop,
-    .stop_cancel          = _ppp_manager_stop_cancel,
+    .create      = _ppp_manager_new,
+    .start       = _ppp_manager_start,
+    .stop        = _ppp_manager_stop,
+    .stop_cancel = _ppp_manager_stop_cancel,
 };