From 2507046fea3165ee050e22af28266a685689c841 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Mon, 8 Jan 2018 16:31:56 +0100 Subject: [PATCH 1/4] device: add nm_device_set_ip_ifindex() --- src/devices/nm-device-private.h | 1 + src/devices/nm-device.c | 54 +++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/src/devices/nm-device-private.h b/src/devices/nm-device-private.h index f1486c5490..be1e099335 100644 --- a/src/devices/nm-device-private.h +++ b/src/devices/nm-device-private.h @@ -45,6 +45,7 @@ enum NMActStageReturn { NMSettings *nm_device_get_settings (NMDevice *self); +gboolean nm_device_set_ip_ifindex (NMDevice *self, int ifindex); gboolean nm_device_set_ip_iface (NMDevice *self, const char *iface); void nm_device_activate_schedule_stage3_ip_config_start (NMDevice *device); diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index c4569ef613..9fe4a5e54f 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -1281,6 +1281,60 @@ nm_device_get_ip_ifindex (const NMDevice *self) return priv->ip_iface ? priv->ip_ifindex : priv->ifindex; } +gboolean +nm_device_set_ip_ifindex (NMDevice *self, int ifindex) +{ + NMDevicePrivate *priv; + NMPlatform *platform; + const char *name = NULL; + + g_return_val_if_fail (NM_IS_DEVICE (self), FALSE); + + priv = NM_DEVICE_GET_PRIVATE (self); + platform = nm_device_get_platform (self); + + if (ifindex > 0) { + const NMPlatformLink *plink; + + plink = nm_platform_link_get (platform, ifindex); + if (!plink) { + nm_platform_process_events (platform); + plink = nm_platform_link_get (NM_PLATFORM_GET, ifindex); + } + if (!plink) { + _LOGW (LOGD_DEVICE, "ip-ifindex: ifindex %d not found", ifindex); + return FALSE; + } + name = plink->name; + } + + if (priv->ip_ifindex == ifindex) + return FALSE; + + _LOGD (LOGD_DEVICE, "ip-ifindex: update ifindex to %d", ifindex); + priv->ip_ifindex = ifindex; + if (!nm_streq0 (priv->ip_iface, name)) { + _LOGD (LOGD_DEVICE, "ip-ifindex: update ip-iface to %s%s%s", + NM_PRINT_FMT_QUOTED (name, "\"", name, "\"", "NULL")); + priv->ip_iface = g_strdup (name); + _notify (self, PROP_IP_IFACE); + } + + if (priv->ip_ifindex > 0) { + if (nm_platform_check_kernel_support (nm_device_get_platform (self), + NM_PLATFORM_KERNEL_SUPPORT_USER_IPV6LL)) + nm_platform_link_set_user_ipv6ll_enabled (nm_device_get_platform (self), priv->ip_ifindex, TRUE); + + if (!nm_platform_link_is_up (nm_device_get_platform (self), priv->ip_ifindex)) + nm_platform_link_set_up (nm_device_get_platform (self), priv->ip_ifindex, NULL); + } + + /* We don't care about any saved values from the old iface */ + g_hash_table_remove_all (priv->ip6_saved_properties); + + return TRUE; +} + /** * nm_device_set_ip_iface: * @self: the #NMDevice From dd98ada33f33820e0d0874d9aa97e0c2bfc7cdd0 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Mon, 8 Jan 2018 16:45:43 +0100 Subject: [PATCH 2/4] ppp: introduce SetIfindex pppd plugin D-Bus method If IPV6CP terminates before IPCP, pppd enters the RUNNING phase and we start IP configuration without having an IP interface set, which triggers assertions. Instead, add a SetIfindex() D-Bus method that gets called by the plugin when pppd becomes RUNNING. The method sets the IP ifindex of the device and starts IP configuration. https://bugzilla.redhat.com/show_bug.cgi?id=1515829 --- .../org.freedesktop.NetworkManager.PPP.xml | 5 + src/devices/adsl/nm-device-adsl.c | 20 ++- src/devices/nm-device-ethernet.c | 20 ++- src/devices/nm-device-ppp.c | 47 +++---- src/devices/nm-device-private.h | 3 +- src/devices/nm-device.c | 22 +-- src/devices/wwan/nm-modem.c | 24 ++-- src/ppp/nm-ppp-manager.c | 127 ++++++++++++------ src/ppp/nm-ppp-manager.h | 1 + src/ppp/nm-pppd-plugin.c | 16 +++ 10 files changed, 191 insertions(+), 94 deletions(-) diff --git a/introspection/org.freedesktop.NetworkManager.PPP.xml b/introspection/org.freedesktop.NetworkManager.PPP.xml index 401ee9492b..d840c786bf 100644 --- a/introspection/org.freedesktop.NetworkManager.PPP.xml +++ b/introspection/org.freedesktop.NetworkManager.PPP.xml @@ -25,5 +25,10 @@ + + + + + diff --git a/src/devices/adsl/nm-device-adsl.c b/src/devices/adsl/nm-device-adsl.c index e9bd41ae00..2512c3187e 100644 --- a/src/devices/adsl/nm-device-adsl.c +++ b/src/devices/adsl/nm-device-adsl.c @@ -430,9 +430,23 @@ ppp_state_changed (NMPPPManager *ppp_manager, NMPPPStatus status, gpointer user_ } } +static void +ppp_ifindex_set (NMPPPManager *ppp_manager, + int ifindex, + const char *iface, + gpointer user_data) +{ + NMDevice *device = NM_DEVICE (user_data); + + if (!nm_device_set_ip_ifindex (device, ifindex)) { + nm_device_state_changed (device, + NM_DEVICE_STATE_FAILED, + NM_DEVICE_STATE_REASON_IP_CONFIG_UNAVAILABLE); + } +} + static void ppp_ip4_config (NMPPPManager *ppp_manager, - const char *iface, NMIP4Config *config, gpointer user_data) { @@ -440,7 +454,6 @@ ppp_ip4_config (NMPPPManager *ppp_manager, /* Ignore PPP IP4 events that come in after initial configuration */ if (nm_device_activate_ip4_state_in_conf (device)) { - nm_device_set_ip_iface (device, iface); nm_device_activate_schedule_ip4_config_result (device, config); } } @@ -499,6 +512,9 @@ act_stage3_ip4_config_start (NMDevice *device, g_signal_connect (priv->ppp_manager, NM_PPP_MANAGER_SIGNAL_STATE_CHANGED, G_CALLBACK (ppp_state_changed), self); + g_signal_connect (priv->ppp_manager, NM_PPP_MANAGER_SIGNAL_IFINDEX_SET, + G_CALLBACK (ppp_ifindex_set), + self); g_signal_connect (priv->ppp_manager, NM_PPP_MANAGER_SIGNAL_IP4_CONFIG, G_CALLBACK (ppp_ip4_config), self); diff --git a/src/devices/nm-device-ethernet.c b/src/devices/nm-device-ethernet.c index d11f9a11c2..54c8176f67 100644 --- a/src/devices/nm-device-ethernet.c +++ b/src/devices/nm-device-ethernet.c @@ -952,9 +952,23 @@ ppp_state_changed (NMPPPManager *ppp_manager, NMPPPStatus status, gpointer user_ } } +static void +ppp_ifindex_set (NMPPPManager *ppp_manager, + int ifindex, + const char *iface, + gpointer user_data) +{ + NMDevice *device = NM_DEVICE (user_data); + + if (!nm_device_set_ip_ifindex (device, ifindex)) { + nm_device_state_changed (device, + NM_DEVICE_STATE_FAILED, + NM_DEVICE_STATE_REASON_IP_CONFIG_UNAVAILABLE); + } +} + static void ppp_ip4_config (NMPPPManager *ppp_manager, - const char *iface, NMIP4Config *config, gpointer user_data) { @@ -962,7 +976,6 @@ ppp_ip4_config (NMPPPManager *ppp_manager, /* Ignore PPP IP4 events that come in after initial configuration */ if (nm_device_activate_ip4_state_in_conf (device)) { - nm_device_set_ip_iface (device, iface); nm_device_activate_schedule_ip4_config_result (device, config); } } @@ -1009,6 +1022,9 @@ pppoe_stage3_ip4_config_start (NMDeviceEthernet *self, NMDeviceStateReason *out_ g_signal_connect (priv->ppp_manager, NM_PPP_MANAGER_SIGNAL_STATE_CHANGED, G_CALLBACK (ppp_state_changed), self); + g_signal_connect (priv->ppp_manager, NM_PPP_MANAGER_SIGNAL_IFINDEX_SET, + G_CALLBACK (ppp_ifindex_set), + self); g_signal_connect (priv->ppp_manager, NM_PPP_MANAGER_SIGNAL_IP4_CONFIG, G_CALLBACK (ppp_ip4_config), self); diff --git a/src/devices/nm-device-ppp.c b/src/devices/nm-device-ppp.c index 8b3968d513..8d60e3b6b6 100644 --- a/src/devices/nm-device-ppp.c +++ b/src/devices/nm-device-ppp.c @@ -36,7 +36,6 @@ _LOG_DECLARE_SELF(NMDevicePpp); typedef struct _NMDevicePppPrivate { NMPPPManager *ppp_manager; NMIP4Config *pending_ip4_config; - char *pending_ifname; } NMDevicePppPrivate; struct _NMDevicePpp { @@ -88,37 +87,45 @@ ppp_state_changed (NMPPPManager *ppp_manager, NMPPPStatus status, gpointer user_ case NM_PPP_STATUS_DEAD: nm_device_state_changed (device, NM_DEVICE_STATE_FAILED, NM_DEVICE_STATE_REASON_PPP_FAILED); break; - case NM_PPP_STATUS_RUNNING: - nm_device_activate_schedule_stage3_ip_config_start (device); - break; default: break; } } +static void +ppp_ifindex_set (NMPPPManager *ppp_manager, + int ifindex, + const char *iface, + gpointer user_data) +{ + NMDevice *device = NM_DEVICE (user_data); + gs_free char *old_name = NULL; + + if (!nm_device_take_over_link (device, ifindex, &old_name)) { + nm_device_state_changed (device, NM_DEVICE_STATE_FAILED, + NM_DEVICE_STATE_REASON_IP_CONFIG_UNAVAILABLE); + return; + } + + if (old_name) + nm_manager_remove_device (nm_manager_get (), old_name, NM_DEVICE_TYPE_PPP); + + nm_device_activate_schedule_stage3_ip_config_start (device); +} + static void ppp_ip4_config (NMPPPManager *ppp_manager, - const char *iface, NMIP4Config *config, gpointer user_data) { NMDevice *device = NM_DEVICE (user_data); NMDevicePpp *self = NM_DEVICE_PPP (device); NMDevicePppPrivate *priv = NM_DEVICE_PPP_GET_PRIVATE (self); - gboolean renamed; _LOGT (LOGD_DEVICE | LOGD_PPP, "received IPv4 config from pppd"); if (nm_device_get_state (device) == NM_DEVICE_STATE_IP_CONFIG) { if (nm_device_activate_ip4_state_in_conf (device)) { - if (!nm_device_take_over_link (device, iface, &renamed)) { - nm_device_state_changed (device, NM_DEVICE_STATE_FAILED, - NM_DEVICE_STATE_REASON_IP_CONFIG_UNAVAILABLE); - return; - } - if (renamed) - nm_manager_remove_device (nm_manager_get (), iface, NM_DEVICE_TYPE_PPP); - nm_device_activate_schedule_ip4_config_result (device, config); return; } @@ -126,8 +133,6 @@ ppp_ip4_config (NMPPPManager *ppp_manager, if (priv->pending_ip4_config) g_object_unref (priv->pending_ip4_config); priv->pending_ip4_config = g_object_ref (config); - g_free (priv->pending_ifname); - priv->pending_ifname = g_strdup (iface); } } @@ -147,7 +152,6 @@ act_stage2_config (NMDevice *device, NMDeviceStateReason *out_failure_reason) g_return_val_if_fail (s_pppoe, NM_ACT_STAGE_RETURN_FAILURE); g_clear_object (&priv->pending_ip4_config); - nm_clear_g_free (&priv->pending_ifname); priv->ppp_manager = nm_ppp_manager_create (nm_setting_pppoe_get_parent (s_pppoe), &error); @@ -175,6 +179,9 @@ act_stage2_config (NMDevice *device, NMDeviceStateReason *out_failure_reason) g_signal_connect (priv->ppp_manager, NM_PPP_MANAGER_SIGNAL_STATE_CHANGED, G_CALLBACK (ppp_state_changed), self); + g_signal_connect (priv->ppp_manager, NM_PPP_MANAGER_SIGNAL_IFINDEX_SET, + G_CALLBACK (ppp_ifindex_set), + self); g_signal_connect (priv->ppp_manager, NM_PPP_MANAGER_SIGNAL_IP4_CONFIG, G_CALLBACK (ppp_ip4_config), self); @@ -189,13 +196,8 @@ act_stage3_ip4_config_start (NMDevice *device, { NMDevicePpp *self = NM_DEVICE_PPP (device); NMDevicePppPrivate *priv = NM_DEVICE_PPP_GET_PRIVATE (self); - gboolean renamed; if (priv->pending_ip4_config) { - if (!nm_device_take_over_link (device, priv->pending_ifname, &renamed)) - return NM_ACT_STAGE_RETURN_FAILURE; - if (renamed) - nm_manager_remove_device (nm_manager_get (), priv->pending_ifname, NM_DEVICE_TYPE_PPP); if (out_config) *out_config = g_steal_pointer (&priv->pending_ip4_config); else @@ -256,7 +258,6 @@ dispose (GObject *object) NMDevicePppPrivate *priv = NM_DEVICE_PPP_GET_PRIVATE (self); g_clear_object (&priv->pending_ip4_config); - nm_clear_g_free (&priv->pending_ifname); G_OBJECT_CLASS (nm_device_ppp_parent_class)->dispose (object); } diff --git a/src/devices/nm-device-private.h b/src/devices/nm-device-private.h index be1e099335..4aa7d4364c 100644 --- a/src/devices/nm-device-private.h +++ b/src/devices/nm-device-private.h @@ -46,6 +46,7 @@ enum NMActStageReturn { NMSettings *nm_device_get_settings (NMDevice *self); gboolean nm_device_set_ip_ifindex (NMDevice *self, int ifindex); + gboolean nm_device_set_ip_iface (NMDevice *self, const char *iface); void nm_device_activate_schedule_stage3_ip_config_start (NMDevice *device); @@ -58,7 +59,7 @@ gboolean nm_device_bring_up (NMDevice *self, gboolean wait, gboolean *no_firmwar void nm_device_take_down (NMDevice *self, gboolean block); -gboolean nm_device_take_over_link (NMDevice *self, const char *ifname, gboolean *renamed); +gboolean nm_device_take_over_link (NMDevice *self, int ifindex, char **old_name); gboolean nm_device_hw_addr_set (NMDevice *device, const char *addr, diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 9fe4a5e54f..59b5312a11 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -1179,28 +1179,26 @@ nm_device_get_iface (NMDevice *self) } gboolean -nm_device_take_over_link (NMDevice *self, const char *ifname, gboolean *renamed) +nm_device_take_over_link (NMDevice *self, int ifindex, char **old_name) { NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); const NMPlatformLink *plink; NMPlatform *platform; gboolean up, success = TRUE; - int ifindex; + gs_free char *name = NULL; g_return_val_if_fail (priv->ifindex <= 0, FALSE); - g_return_val_if_fail (ifname, FALSE); - NM_SET_OUT (renamed, FALSE); + NM_SET_OUT (old_name, NULL); platform = nm_device_get_platform (self); - plink = nm_platform_link_get_by_ifname (platform, ifname); + plink = nm_platform_link_get (platform, ifindex); if (!plink) return FALSE; - ifindex = plink->ifindex; - - if (!nm_streq (ifname, nm_device_get_iface (self))) { + if (!nm_streq (plink->name, nm_device_get_iface (self))) { up = NM_FLAGS_HAS (plink->n_ifi_flags, IFF_UP); + name = g_strdup (plink->name); /* Rename the link to the device ifname */ if (up) @@ -1209,7 +1207,8 @@ nm_device_take_over_link (NMDevice *self, const char *ifname, gboolean *renamed) if (up) nm_platform_link_set_up (platform, ifindex, NULL); - NM_SET_OUT (renamed, success); + if (success) + NM_SET_OUT (old_name, g_steal_pointer (&name)); } if (success) { @@ -1306,10 +1305,11 @@ nm_device_set_ip_ifindex (NMDevice *self, int ifindex) return FALSE; } name = plink->name; - } + } else + g_return_val_if_fail (ifindex == 0, FALSE); if (priv->ip_ifindex == ifindex) - return FALSE; + return TRUE; _LOGD (LOGD_DEVICE, "ip-ifindex: update ifindex to %d", ifindex); priv->ip_ifindex = ifindex; diff --git a/src/devices/wwan/nm-modem.c b/src/devices/wwan/nm-modem.c index 010a2b604f..10baa424af 100644 --- a/src/devices/wwan/nm-modem.c +++ b/src/devices/wwan/nm-modem.c @@ -450,20 +450,26 @@ ppp_state_changed (NMPPPManager *ppp_manager, NMPPPStatus status, gpointer user_ } static void -set_data_port (NMModem *self, const char *new_data_port) +ppp_ifindex_set (NMPPPManager *ppp_manager, + int ifindex, + const char *iface, + gpointer user_data) { + NMModem *self = NM_MODEM (user_data); NMModemPrivate *priv = NM_MODEM_GET_PRIVATE (self); - if (g_strcmp0 (priv->data_port, new_data_port) != 0) { + /* Notify about the new data port to use. + * + * @iface might be %NULL. */ + if (g_strcmp0 (priv->data_port, iface) != 0) { g_free (priv->data_port); - priv->data_port = g_strdup (new_data_port); + priv->data_port = g_strdup (iface); _notify (self, PROP_DATA_PORT); } } static void ppp_ip4_config (NMPPPManager *ppp_manager, - const char *iface, NMIP4Config *config, gpointer user_data) { @@ -475,9 +481,6 @@ ppp_ip4_config (NMPPPManager *ppp_manager, guint32 good_dns2 = htonl (0x04020202); /* GTE nameserver */ gboolean dns_workaround = FALSE; - /* Notify about the new data port to use */ - set_data_port (self, iface); - /* Work around a PPP bug (#1732) which causes many mobile broadband * providers to return 10.11.12.13 and 10.11.12.14 for the DNS servers. * Apparently fixed in ppp-2.4.5 but we've had some reports that this is @@ -519,16 +522,12 @@ ppp_ip4_config (NMPPPManager *ppp_manager, static void ppp_ip6_config (NMPPPManager *ppp_manager, - const char *iface, const NMUtilsIPv6IfaceId *iid, NMIP6Config *config, gpointer user_data) { NMModem *self = NM_MODEM (user_data); - /* Notify about the new data port to use */ - set_data_port (self, iface); - NM_MODEM_GET_PRIVATE (self)->iid = *iid; nm_modem_emit_ip6_config_result (self, config, NULL); @@ -639,6 +638,9 @@ ppp_stage3_ip_config_start (NMModem *self, g_signal_connect (priv->ppp_manager, NM_PPP_MANAGER_SIGNAL_STATE_CHANGED, G_CALLBACK (ppp_state_changed), self); + g_signal_connect (priv->ppp_manager, NM_PPP_MANAGER_SIGNAL_IFINDEX_SET, + G_CALLBACK (ppp_ifindex_set), + self); g_signal_connect (priv->ppp_manager, NM_PPP_MANAGER_SIGNAL_IP4_CONFIG, G_CALLBACK (ppp_ip4_config), self); diff --git a/src/ppp/nm-ppp-manager.c b/src/ppp/nm-ppp-manager.c index 7dd6a9b95c..d1e12643ef 100644 --- a/src/ppp/nm-ppp-manager.c +++ b/src/ppp/nm-ppp-manager.c @@ -76,6 +76,7 @@ GType nm_ppp_manager_get_type (void); enum { STATE_CHANGED, + IFINDEX_SET, IP4_CONFIG, IP6_CONFIG, STATS, @@ -93,6 +94,8 @@ typedef struct { GPid pid; char *parent_iface; + char *ip_iface; + int ifindex; NMActRequest *act_req; GDBusMethodInvocation *pending_secrets_context; @@ -103,7 +106,6 @@ typedef struct { guint ppp_timeout_handler; /* Monitoring */ - char *ip_iface; int monitor_fd; guint monitor_id; @@ -174,24 +176,28 @@ monitor_cb (gpointer user_data) { NMPPPManager *manager = NM_PPP_MANAGER (user_data); NMPPPManagerPrivate *priv = NM_PPP_MANAGER_GET_PRIVATE (manager); - struct ifreq req; - struct ppp_stats stats; + const char *ifname; - memset (&req, 0, sizeof (req)); - memset (&stats, 0, sizeof (stats)); - req.ifr_data = (caddr_t) &stats; + ifname = nm_platform_link_get_name (NM_PLATFORM_GET, priv->ifindex); - strncpy (req.ifr_name, priv->ip_iface, sizeof (req.ifr_name)); - if (ioctl (priv->monitor_fd, SIOCGPPPSTATS, &req) < 0) { - if (errno != ENODEV) - _LOGW ("could not read ppp stats: %s", strerror (errno)); - } else { - g_signal_emit (manager, signals[STATS], 0, - (guint) stats.p.ppp_ibytes, - (guint) stats.p.ppp_obytes); + if (ifname) { + struct ppp_stats stats = { }; + struct ifreq req = { + .ifr_data = (caddr_t) &stats, + }; + + nm_utils_ifname_cpy (req.ifr_name, ifname); + if (ioctl (priv->monitor_fd, SIOCGPPPSTATS, &req) < 0) { + if (errno != ENODEV) + _LOGW ("could not read ppp stats: %s", strerror (errno)); + } else { + g_signal_emit (manager, signals[STATS], 0, + (guint) stats.p.ppp_ibytes, + (guint) stats.p.ppp_obytes); + } } - return TRUE; + return G_SOURCE_CONTINUE; } static void @@ -399,23 +405,54 @@ impl_ppp_manager_set_state (NMPPPManager *manager, g_dbus_method_invocation_return_value (context, NULL); } +static void +impl_ppp_manager_set_ifindex (NMPPPManager *manager, + GDBusMethodInvocation *context, + gint32 ifindex) +{ + NMPPPManagerPrivate *priv = NM_PPP_MANAGER_GET_PRIVATE (manager); + const NMPlatformLink *plink = NULL; + nm_auto_nmpobj const NMPObject *obj_keep_alive = NULL; + + _LOGD ("set-ifindex %d", (int) ifindex); + + if (priv->ifindex >= 0) { + _LOGW ("can't change the ifindex from %d to %d", priv->ifindex, (int) ifindex); + return; + } + + if (ifindex > 0) { + plink = nm_platform_link_get (NM_PLATFORM_GET, ifindex); + if (!plink) { + nm_platform_process_events (NM_PLATFORM_GET); + plink = nm_platform_link_get (NM_PLATFORM_GET, ifindex); + } + } + + if (!plink) { + _LOGW ("unknown interface with ifindex %d", ifindex); + ifindex = 0; + } + + priv->ifindex = ifindex; + + obj_keep_alive = nmp_object_ref (NMP_OBJECT_UP_CAST (plink)); + + g_signal_emit (manager, signals[IFINDEX_SET], 0, ifindex, plink->name); + g_dbus_method_invocation_return_value (context, NULL); +} + static gboolean set_ip_config_common (NMPPPManager *self, GVariant *config_dict, - const char *iface_prop, guint32 *out_mtu) { NMPPPManagerPrivate *priv = NM_PPP_MANAGER_GET_PRIVATE (self); NMConnection *applied_connection; NMSettingPpp *s_ppp; - const char *iface; - if (!g_variant_lookup (config_dict, iface_prop, "&s", &iface)) { - _LOGE ("no interface received!"); + if (priv->ifindex <= 0) return FALSE; - } - if (priv->ip_iface == NULL) - priv->ip_iface = g_strdup (iface); /* Got successful IP config; obviously the secrets worked */ applied_connection = nm_act_request_get_applied_connection (priv->act_req); @@ -441,20 +478,15 @@ impl_ppp_manager_set_ip4_config (NMPPPManager *manager, NMPlatformIP4Address address; guint32 u32, mtu; GVariantIter *iter; - int ifindex; _LOGI ("(IPv4 Config Get) reply received."); nm_clear_g_source (&priv->ppp_timeout_handler); - if (!set_ip_config_common (manager, config_dict, NM_PPP_IP4_CONFIG_INTERFACE, &mtu)) + if (!set_ip_config_common (manager, config_dict, &mtu)) goto out; - ifindex = nm_platform_link_get_ifindex (NM_PLATFORM_GET, priv->ip_iface); - if (ifindex <= 0) - goto out; - - config = nm_ip4_config_new (nm_platform_get_multi_idx (NM_PLATFORM_GET), ifindex); + config = nm_ip4_config_new (nm_platform_get_multi_idx (NM_PLATFORM_GET), priv->ifindex); if (mtu) nm_ip4_config_set_mtu (config, mtu, NM_IP_CONFIG_SOURCE_PPP); @@ -467,7 +499,7 @@ impl_ppp_manager_set_ip4_config (NMPPPManager *manager, if (g_variant_lookup (config_dict, NM_PPP_IP4_CONFIG_GATEWAY, "u", &u32)) { const NMPlatformIP4Route r = { - .ifindex = ifindex, + .ifindex = priv->ifindex, .rt_source = NM_IP_CONFIG_SOURCE_PPP, .gateway = u32, .table_coerced = nm_platform_route_table_coerce (priv->ip4_route_table), @@ -503,7 +535,7 @@ impl_ppp_manager_set_ip4_config (NMPPPManager *manager, } /* Push the IP4 config up to the device */ - g_signal_emit (manager, signals[IP4_CONFIG], 0, priv->ip_iface, config); + g_signal_emit (manager, signals[IP4_CONFIG], 0, config); out: g_dbus_method_invocation_return_value (context, NULL); @@ -549,27 +581,22 @@ impl_ppp_manager_set_ip6_config (NMPPPManager *manager, struct in6_addr a; NMUtilsIPv6IfaceId iid = NM_UTILS_IPV6_IFACE_ID_INIT; gboolean has_peer = FALSE; - int ifindex; _LOGI ("(IPv6 Config Get) reply received."); nm_clear_g_source (&priv->ppp_timeout_handler); - if (!set_ip_config_common (manager, config_dict, NM_PPP_IP6_CONFIG_INTERFACE, NULL)) + if (!set_ip_config_common (manager, config_dict, NULL)) goto out; - ifindex = nm_platform_link_get_ifindex (NM_PLATFORM_GET, priv->ip_iface); - if (ifindex <= 0) - goto out; - - config = nm_ip6_config_new (nm_platform_get_multi_idx (NM_PLATFORM_GET), ifindex); + config = nm_ip6_config_new (nm_platform_get_multi_idx (NM_PLATFORM_GET), priv->ifindex); memset (&addr, 0, sizeof (addr)); addr.plen = 64; if (iid_value_to_ll6_addr (config_dict, NM_PPP_IP6_CONFIG_PEER_IID, &a, NULL)) { const NMPlatformIP6Route r = { - .ifindex = ifindex, + .ifindex = priv->ifindex, .rt_source = NM_IP_CONFIG_SOURCE_PPP, .gateway = a, .table_coerced = nm_platform_route_table_coerce (priv->ip6_route_table), @@ -587,7 +614,7 @@ impl_ppp_manager_set_ip6_config (NMPPPManager *manager, nm_ip6_config_add_address (config, &addr); /* Push the IPv6 config and interface identifier up to the device */ - g_signal_emit (manager, signals[IP6_CONFIG], 0, priv->ip_iface, &iid, config); + g_signal_emit (manager, signals[IP6_CONFIG], 0, &iid, config); } else _LOGE ("invalid IPv6 address received!"); @@ -1244,6 +1271,7 @@ nm_ppp_manager_init (NMPPPManager *manager) { NMPPPManagerPrivate *priv = NM_PPP_MANAGER_GET_PRIVATE (manager); + priv->ifindex = -1; priv->monitor_fd = -1; priv->ip4_route_table = RT_TABLE_MAIN; priv->ip4_route_metric = 460; @@ -1284,7 +1312,6 @@ finalize (GObject *object) { NMPPPManagerPrivate *priv = NM_PPP_MANAGER_GET_PRIVATE ((NMPPPManager *) object); - g_free (priv->ip_iface); g_free (priv->parent_iface); G_OBJECT_CLASS (nm_ppp_manager_parent_class)->finalize (object); @@ -1320,14 +1347,23 @@ nm_ppp_manager_class_init (NMPPPManagerClass *manager_class) G_TYPE_NONE, 1, G_TYPE_UINT); + signals[IFINDEX_SET] = + g_signal_new (NM_PPP_MANAGER_SIGNAL_IFINDEX_SET, + G_OBJECT_CLASS_TYPE (object_class), + G_SIGNAL_RUN_FIRST, + 0, + NULL, NULL, 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, 2, - G_TYPE_STRING, + G_TYPE_NONE, 1, G_TYPE_OBJECT); signals[IP6_CONFIG] = @@ -1336,7 +1372,9 @@ nm_ppp_manager_class_init (NMPPPManagerClass *manager_class) G_SIGNAL_RUN_FIRST, 0, NULL, NULL, NULL, - G_TYPE_NONE, 3, G_TYPE_STRING, G_TYPE_POINTER, G_TYPE_OBJECT); + G_TYPE_NONE, 2, + G_TYPE_POINTER, + G_TYPE_OBJECT); signals[STATS] = g_signal_new (NM_PPP_MANAGER_SIGNAL_STATS, @@ -1354,6 +1392,7 @@ nm_ppp_manager_class_init (NMPPPManagerClass *manager_class) "SetIp4Config", impl_ppp_manager_set_ip4_config, "SetIp6Config", impl_ppp_manager_set_ip6_config, "SetState", impl_ppp_manager_set_state, + "SetIfindex", impl_ppp_manager_set_ifindex, NULL); } diff --git a/src/ppp/nm-ppp-manager.h b/src/ppp/nm-ppp-manager.h index 35fb1b6091..5457726e96 100644 --- a/src/ppp/nm-ppp-manager.h +++ b/src/ppp/nm-ppp-manager.h @@ -25,6 +25,7 @@ #define NM_PPP_MANAGER_PARENT_IFACE "parent-iface" #define NM_PPP_MANAGER_SIGNAL_STATE_CHANGED "state-changed" +#define NM_PPP_MANAGER_SIGNAL_IFINDEX_SET "ifindex-set" #define NM_PPP_MANAGER_SIGNAL_IP4_CONFIG "ip4-config" #define NM_PPP_MANAGER_SIGNAL_IP6_CONFIG "ip6-config" #define NM_PPP_MANAGER_SIGNAL_STATS "stats" diff --git a/src/ppp/nm-pppd-plugin.c b/src/ppp/nm-pppd-plugin.c index 9f8451d87a..8c9646a46c 100644 --- a/src/ppp/nm-pppd-plugin.c +++ b/src/ppp/nm-pppd-plugin.c @@ -28,6 +28,7 @@ #include #include #include +#include #include #include @@ -128,6 +129,15 @@ nm_phasechange (void *data, int arg) NULL, NULL, NULL); } + + if (ppp_status == PHASE_RUNNING) { + g_dbus_proxy_call (proxy, + "SetIfindex", + g_variant_new ("(i)", if_nametoindex (ifname)), + G_DBUS_CALL_FLAGS_NONE, -1, + NULL, + NULL, NULL); + } } static void @@ -150,6 +160,9 @@ nm_ip_up (void *data, int arg) g_variant_builder_init (&builder, G_VARIANT_TYPE_VARDICT); + /* Keep sending the interface name to be backwards compatible + * with older versions of NM during a package upgrade, where + * NM is not restarted and the pppd plugin was not loaded. */ g_variant_builder_add (&builder, "{sv}", NM_PPP_IP4_CONFIG_INTERFACE, g_variant_new_string (ifname)); @@ -244,6 +257,9 @@ nm_ip6_up (void *data, int arg) g_message ("nm-ppp-plugin: (%s): ip6-up event", __func__); g_variant_builder_init (&builder, G_VARIANT_TYPE_VARDICT); + /* Keep sending the interface name to be backwards compatible + * with older versions of NM during a package upgrade, where + * NM is not restarted and the pppd plugin was not loaded. */ g_variant_builder_add (&builder, "{sv}", NM_PPP_IP6_CONFIG_INTERFACE, g_variant_new_string (ifname)); From 41ef5853e687891438de584d5f44eb7fbaa9ac8a Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Mon, 8 Jan 2018 16:55:07 +0100 Subject: [PATCH 3/4] ppp/trivial: rename field --- src/devices/nm-device-ppp.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/devices/nm-device-ppp.c b/src/devices/nm-device-ppp.c index 8d60e3b6b6..23ee357018 100644 --- a/src/devices/nm-device-ppp.c +++ b/src/devices/nm-device-ppp.c @@ -35,7 +35,7 @@ _LOG_DECLARE_SELF(NMDevicePpp); typedef struct _NMDevicePppPrivate { NMPPPManager *ppp_manager; - NMIP4Config *pending_ip4_config; + NMIP4Config *ip4_config; } NMDevicePppPrivate; struct _NMDevicePpp { @@ -130,9 +130,9 @@ ppp_ip4_config (NMPPPManager *ppp_manager, return; } } else { - if (priv->pending_ip4_config) - g_object_unref (priv->pending_ip4_config); - priv->pending_ip4_config = g_object_ref (config); + if (priv->ip4_config) + g_object_unref (priv->ip4_config); + priv->ip4_config = g_object_ref (config); } } @@ -151,7 +151,7 @@ act_stage2_config (NMDevice *device, NMDeviceStateReason *out_failure_reason) s_pppoe = (NMSettingPppoe *) nm_device_get_applied_setting ((NMDevice *) self, NM_TYPE_SETTING_PPPOE); g_return_val_if_fail (s_pppoe, NM_ACT_STAGE_RETURN_FAILURE); - g_clear_object (&priv->pending_ip4_config); + g_clear_object (&priv->ip4_config); priv->ppp_manager = nm_ppp_manager_create (nm_setting_pppoe_get_parent (s_pppoe), &error); @@ -197,11 +197,11 @@ act_stage3_ip4_config_start (NMDevice *device, NMDevicePpp *self = NM_DEVICE_PPP (device); NMDevicePppPrivate *priv = NM_DEVICE_PPP_GET_PRIVATE (self); - if (priv->pending_ip4_config) { + if (priv->ip4_config) { if (out_config) - *out_config = g_steal_pointer (&priv->pending_ip4_config); + *out_config = g_steal_pointer (&priv->ip4_config); else - g_clear_object (&priv->pending_ip4_config); + g_clear_object (&priv->ip4_config); return NM_ACT_STAGE_RETURN_SUCCESS; } @@ -257,7 +257,7 @@ dispose (GObject *object) NMDevicePpp *self = NM_DEVICE_PPP (object); NMDevicePppPrivate *priv = NM_DEVICE_PPP_GET_PRIVATE (self); - g_clear_object (&priv->pending_ip4_config); + g_clear_object (&priv->ip4_config); G_OBJECT_CLASS (nm_device_ppp_parent_class)->dispose (object); } From 398f9105b49e1c44389462533a53b5a11cc94eca Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Mon, 8 Jan 2018 17:03:53 +0100 Subject: [PATCH 4/4] ppp: update interface name in the plugin after NM changes it When NM knows of the ifindex/name of the new PPP interface (through the SetIfindex() call), it renames it. This can race with the pppd daemon, which issues ioctl() using the interface name cached in the global 'ifname' variable: ... NetworkManager[27213]: [1515427406.0036] ppp-manager: set-ifindex 71 pppd[27801]: sent [CCP ConfRej id=0x1 ] NetworkManager[27213]: [1515427406.0036] platform: link: setting 'ppp5' (71) name dsl-ppp pppd[27801]: sent [IPCP ConfAck id=0x2 ] pppd[27801]: ioctl(SIOCSIFADDR): No such device (line 2473) pppd[27801]: Interface configuration failed pppd[27801]: Couldn't get PPP statistics: No such device ... Fortunately the variable is exposed to plugins and so we can turn the SetIfindex() D-Bus call into a synchronous one and then update the value of the 'ifname' global variable with the new interface name assigned by NM. --- src/ppp/nm-pppd-plugin.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/src/ppp/nm-pppd-plugin.c b/src/ppp/nm-pppd-plugin.c index 8c9646a46c..c5496e6622 100644 --- a/src/ppp/nm-pppd-plugin.c +++ b/src/ppp/nm-pppd-plugin.c @@ -53,7 +53,9 @@ static void nm_phasechange (void *data, int arg) { NMPPPStatus ppp_status = NM_PPP_STATUS_UNKNOWN; + char new_name[IF_NAMESIZE]; char *ppp_phase; + int index; g_return_if_fail (G_IS_DBUS_PROXY (proxy)); @@ -131,12 +133,22 @@ nm_phasechange (void *data, int arg) } if (ppp_status == PHASE_RUNNING) { - g_dbus_proxy_call (proxy, - "SetIfindex", - g_variant_new ("(i)", if_nametoindex (ifname)), - G_DBUS_CALL_FLAGS_NONE, -1, - NULL, - NULL, NULL); + index = if_nametoindex (ifname); + /* Make a sync call to ensure that when the call + * terminates the interface already has its final + * name. */ + g_dbus_proxy_call_sync (proxy, + "SetIfindex", + g_variant_new ("(i)", index), + G_DBUS_CALL_FLAGS_NONE, + 25000, + NULL, NULL); + /* Update the name in pppd if NM changed it */ + if ( if_indextoname (index, new_name) + && !nm_streq0 (ifname, new_name)) { + g_message ("nm-ppp-plugin: interface name changed from '%s' to '%s'", ifname, new_name); + strncpy (ifname, new_name, IF_NAMESIZE); + } } }