From b9d85bb783582af13dc870f38a8a459d54d3bbea Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 6 Mar 2014 21:11:47 +0100 Subject: [PATCH 1/4] platform: fix link_type_from_udev() to use ifname from libnl When an interface gets renamed, we first receive a libnl update with the changed interface name. This results in the following chain of calls: - event_notification() - announce_object() - link_init() - link_extract_type() - link_type_from_udev() Then link_type_from_udev() looks up the name in the udev data (getting the previous name, because we did not yet recieve the udev notification) and passes the name to wifi_utils_is_wifi(), which eventually calls nm_platform_link_get_ifindex() -- doing a lookup by the old name. Fix this, by passing the ifname from libnl to link_type_from_udev(). Also, change hack_empty_master_iff_lower_up() because it is called from event_notification(), at a moment when the link cache possibly does not yet know the ifindex -- so that the call chain to link_extract_type(), link_type_from_udev(), wifi_utils_is_wifi() again might lead to lookup for something that does not yet exist. Note, that in this case the name would not yet exist, because we did not yet put the libnl object into the link cache. Signed-off-by: Thomas Haller --- src/platform/nm-linux-platform.c | 40 ++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index ef519c8415..a289994988 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -554,11 +554,13 @@ type_to_string (NMLinkType type) } G_STMT_END static NMLinkType -link_type_from_udev (NMPlatform *platform, int ifindex, int arptype, const char **out_name) +link_type_from_udev (NMPlatform *platform, int ifindex, const char *ifname, int arptype, const char **out_name) { NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform); GUdevDevice *udev_device; - const char *prop, *name, *sysfs_path; + const char *prop, *sysfs_path; + + g_assert (ifname); udev_device = g_hash_table_lookup (priv->udev_devices, GINT_TO_POINTER (ifindex)); if (!udev_device) @@ -569,9 +571,8 @@ link_type_from_udev (NMPlatform *platform, int ifindex, int arptype, const char return_type (NM_LINK_TYPE_OLPC_MESH, "olpc-mesh"); prop = g_udev_device_get_property (udev_device, "DEVTYPE"); - name = g_udev_device_get_name (udev_device); sysfs_path = g_udev_device_get_sysfs_path (udev_device); - if (g_strcmp0 (prop, "wlan") == 0 || wifi_utils_is_wifi (name, sysfs_path)) + if (g_strcmp0 (prop, "wlan") == 0 || wifi_utils_is_wifi (ifname, sysfs_path)) return_type (NM_LINK_TYPE_WIFI, "wifi"); else if (g_strcmp0 (prop, "wwan") == 0) return_type (NM_LINK_TYPE_WWAN_ETHERNET, "wwan"); @@ -664,26 +665,31 @@ link_extract_type (NMPlatform *platform, struct rtnl_link *rtnllink, const char if (!type) { int arptype = rtnl_link_get_arptype (rtnllink); const char *driver; + const char *ifname; + if (arptype == ARPHRD_LOOPBACK) return_type (NM_LINK_TYPE_LOOPBACK, "loopback"); else if (arptype == ARPHRD_INFINIBAND) return_type (NM_LINK_TYPE_INFINIBAND, "infiniband"); - else if (arptype == 256) { + + ifname = rtnl_link_get_name (rtnllink); + if (arptype == 256) { /* Some s390 CTC-type devices report 256 for the encapsulation type * for some reason, but we need to call them Ethernet. FIXME: use * something other than interface name to detect CTC here. */ - if (g_str_has_prefix (rtnl_link_get_name (rtnllink), "ctc")) + if (g_str_has_prefix (ifname, "ctc")) return_type (NM_LINK_TYPE_ETHERNET, "ethernet"); } - driver = ethtool_get_driver (rtnl_link_get_name (rtnllink)); + driver = ethtool_get_driver (ifname); if (!g_strcmp0 (driver, "openvswitch")) return_type (NM_LINK_TYPE_OPENVSWITCH, "openvswitch"); return link_type_from_udev (platform, rtnl_link_get_ifindex (rtnllink), + ifname, arptype, out_name); } else if (!strcmp (type, "dummy")) @@ -827,6 +833,7 @@ hack_empty_master_iff_lower_up (NMPlatform *platform, struct nl_object *object) struct rtnl_link *rtnllink; int ifindex; struct nl_object *slave; + const char *type; if (!object) return; @@ -837,18 +844,15 @@ hack_empty_master_iff_lower_up (NMPlatform *platform, struct nl_object *object) ifindex = rtnl_link_get_ifindex (rtnllink); - switch (link_extract_type (platform, rtnllink, NULL)) { - case NM_LINK_TYPE_BRIDGE: - case NM_LINK_TYPE_BOND: - for (slave = nl_cache_get_first (priv->link_cache); slave; slave = nl_cache_get_next (slave)) { - struct rtnl_link *rtnlslave = (struct rtnl_link *) slave; - if (rtnl_link_get_master (rtnlslave) == ifindex - && rtnl_link_get_flags (rtnlslave) & IFF_LOWER_UP) - return; - } - break; - default: + type = rtnl_link_get_type (rtnllink); + if (!type || (strcmp (type, "bridge") != 0 && strcmp (type, "bond") != 0)) return; + + for (slave = nl_cache_get_first (priv->link_cache); slave; slave = nl_cache_get_next (slave)) { + struct rtnl_link *rtnlslave = (struct rtnl_link *) slave; + if (rtnl_link_get_master (rtnlslave) == ifindex + && rtnl_link_get_flags (rtnlslave) & IFF_LOWER_UP) + return; } rtnl_link_unset_flags (rtnllink, IFF_LOWER_UP); From 3d396d6f72db7c15e92e4f21c70c2d708c0676fa Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 7 Mar 2014 13:30:30 +0100 Subject: [PATCH 2/4] platform: react on udev signal "move" Signed-off-by: Thomas Haller --- src/platform/nm-linux-platform.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index a289994988..f62108e209 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -3036,6 +3036,7 @@ udev_device_added (NMPlatform *platform, auto_nl_object struct rtnl_link *rtnllink = NULL; const char *ifname; int ifindex; + gboolean is_changed; ifname = g_udev_device_get_name (udev_device); if (!ifname) { @@ -3055,6 +3056,7 @@ udev_device_added (NMPlatform *platform, return; } + is_changed = g_hash_table_lookup_extended (priv->udev_devices, GINT_TO_POINTER (ifindex), NULL, NULL); g_hash_table_insert (priv->udev_devices, GINT_TO_POINTER (ifindex), g_object_ref (udev_device)); @@ -3065,7 +3067,7 @@ udev_device_added (NMPlatform *platform, return; } - announce_object (platform, (struct nl_object *) rtnllink, ADDED, NM_PLATFORM_REASON_EXTERNAL); + announce_object (platform, (struct nl_object *) rtnllink, is_changed ? CHANGED : ADDED, NM_PLATFORM_REASON_EXTERNAL); } static void @@ -3128,7 +3130,7 @@ handle_udev_event (GUdevClient *client, action, subsys, g_udev_device_get_name (udev_device), ifindex ? ifindex : "unknown", seqnum); - if (!strcmp (action, "add")) + if (!strcmp (action, "add") || !strcmp (action, "move")) udev_device_added (platform, udev_device); if (!strcmp (action, "remove")) udev_device_removed (platform, udev_device); From b8654a1b3572201bc95740c7e75fdee11a0ff7b3 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 7 Mar 2014 12:30:45 +0100 Subject: [PATCH 3/4] dhcp: downgrade warning about unhandled DHCP event in case of terminated client When we kill a client, we usually get a DHCP event afterwards that cannot be associated with the client anymore (because we forgot about its PID). Do not log a warning in that case, but only a debug message. Signed-off-by: Thomas Haller --- src/dhcp-manager/nm-dhcp-manager.c | 31 +++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/src/dhcp-manager/nm-dhcp-manager.c b/src/dhcp-manager/nm-dhcp-manager.c index c0e4a56d32..3ac220d6a4 100644 --- a/src/dhcp-manager/nm-dhcp-manager.c +++ b/src/dhcp-manager/nm-dhcp-manager.c @@ -43,6 +43,7 @@ #include "nm-config.h" #include "nm-dbus-glib-types.h" #include "nm-glib-compat.h" +#include "NetworkManagerUtils.h" GQuark nm_dhcp_manager_error_quark (void) @@ -198,41 +199,41 @@ nm_dhcp_manager_handle_event (DBusGProxy *proxy, char *iface = NULL; char *pid_str = NULL; char *reason = NULL; - unsigned long temp; + long pid; iface = get_option (options, "interface"); if (iface == NULL) { - nm_log_warn (LOGD_DHCP, "DHCP event didn't have associated interface."); + nm_log_warn (LOGD_DHCP, "DHCP event: didn't have associated interface."); goto out; } pid_str = get_option (options, "pid"); - if (pid_str == NULL) { - nm_log_warn (LOGD_DHCP, "DHCP event didn't have associated PID."); + pid = nm_utils_ascii_str_to_int64 (pid_str, 10, 0, LONG_MAX, -1); + if (pid == -1 || pid != (GPid)pid) { + nm_log_warn (LOGD_DHCP, "DHCP event: couldn't convert PID '%s' to an integer", pid_str ? pid_str : "(null)"); goto out; } - temp = strtoul (pid_str, NULL, 10); - if ((temp == ULONG_MAX) && (errno == ERANGE)) { - nm_log_warn (LOGD_DHCP, "couldn't convert PID"); - goto out; - } - - client = get_client_for_pid (manager, (GPid) temp); + reason = get_option (options, "reason"); + client = get_client_for_pid (manager, (GPid) pid); if (client == NULL) { - nm_log_warn (LOGD_DHCP, "(pid %ld) unhandled DHCP event for interface %s", temp, iface); + if (reason && g_ascii_strcasecmp (reason, "RELEASE") == 0) { + /* This happens regularly, when the dhcp client gets killed and we receive its last message. + * Don't log a warning in this case. */ + nm_log_dbg (LOGD_DHCP, "(pid %ld) unhandled RELEASE DHCP event for interface %s", pid, iface); + } else + nm_log_warn (LOGD_DHCP, "(pid %ld) unhandled DHCP event for interface %s", pid, iface); goto out; } if (strcmp (iface, nm_dhcp_client_get_iface (client))) { nm_log_warn (LOGD_DHCP, "(pid %ld) received DHCP event from unexpected interface '%s' (expected '%s')", - temp, iface, nm_dhcp_client_get_iface (client)); + pid, iface, nm_dhcp_client_get_iface (client)); goto out; } - reason = get_option (options, "reason"); if (reason == NULL) { - nm_log_warn (LOGD_DHCP, "(pid %ld) DHCP event didn't have a reason", temp); + nm_log_warn (LOGD_DHCP, "(pid %ld) DHCP event didn't have a reason", pid); goto out; } From 1f383bc53fc5d8d3a34e51f1023b29e7436d83c0 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 28 Jan 2014 21:25:34 +0100 Subject: [PATCH 4/4] core: support renaming of NMDevice (changes of ifname) https://bugzilla.gnome.org/show_bug.cgi?id=726177 https://bugzilla.redhat.com/show_bug.cgi?id=907836 Signed-off-by: Thomas Haller --- src/devices/nm-device-private.h | 1 + src/devices/nm-device.c | 128 +++++++++++++++++++++++++++----- 2 files changed, 110 insertions(+), 19 deletions(-) diff --git a/src/devices/nm-device-private.h b/src/devices/nm-device-private.h index e5c9942b36..8dae02e0bd 100644 --- a/src/devices/nm-device-private.h +++ b/src/devices/nm-device-private.h @@ -79,6 +79,7 @@ void nm_device_set_dhcp_timeout (NMDevice *device, guint32 timeout); void nm_device_set_dhcp_anycast_address (NMDevice *device, guint8 *addr); gboolean nm_device_dhcp4_renew (NMDevice *device, gboolean release); +gboolean nm_device_dhcp6_renew (NMDevice *device, gboolean release); void nm_device_recheck_available_connections (NMDevice *device); diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index fb37c968d6..20970d122a 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -339,6 +339,7 @@ static gboolean spec_match_list (NMDevice *device, const GSList *specs); static void _clear_available_connections (NMDevice *device, gboolean do_signal); static void dhcp4_cleanup (NMDevice *self, gboolean stop, gboolean release); +static void dhcp6_cleanup (NMDevice *self, gboolean stop, gboolean release); static const char *reason_to_string (NMDeviceStateReason reason); @@ -1121,32 +1122,96 @@ link_changed_cb (NMPlatform *platform, int ifindex, NMPlatformLink *info, NMPlat { NMDeviceClass *klass = NM_DEVICE_GET_CLASS (device); NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (device); + int my_ifindex = nm_device_get_ifindex (device); + gboolean change_ip_ifname = FALSE; /* Ignore other devices. */ - if (ifindex != nm_device_get_ifindex (device)) - return; + if (ifindex == my_ifindex) { - /* We don't filter by 'reason' because we are interested in *all* link - * changes. For example a call to nm_platform_link_set_up() may result - * in an internal carrier change (i.e. we ask the kernel to set IFF_UP - * and it results in also setting IFF_LOWER_UP. - */ + /* We don't filter by 'reason' because we are interested in *all* link + * changes. For example a call to nm_platform_link_set_up() may result + * in an internal carrier change (i.e. we ask the kernel to set IFF_UP + * and it results in also setting IFF_LOWER_UP. + */ - if (info->udi && g_strcmp0 (info->udi, priv->udi)) { - /* Update UDI to what udev gives us */ - g_free (priv->udi); - priv->udi = g_strdup (info->udi); - g_object_notify (G_OBJECT (device), NM_DEVICE_UDI); + if (info->udi && g_strcmp0 (info->udi, priv->udi)) { + /* Update UDI to what udev gives us */ + g_free (priv->udi); + priv->udi = g_strdup (info->udi); + g_object_notify (G_OBJECT (device), NM_DEVICE_UDI); + } + + /* Update MTU if it has changed. */ + if (priv->mtu != info->mtu) { + priv->mtu = info->mtu; + g_object_notify (G_OBJECT (device), NM_DEVICE_MTU); + } + + if (info->name[0] && strcmp (priv->iface, info->name) != 0) { + nm_log_info (LOGD_DEVICE, "(%s): interface index %d renamed iface from '%s' to '%s'", + priv->iface, my_ifindex, priv->iface, info->name); + g_free (priv->iface); + priv->iface = g_strdup (info->name); + + change_ip_ifname = !priv->ip_iface; + if (change_ip_ifname) + g_hash_table_remove_all (priv->ip6_saved_properties); + + g_object_notify (G_OBJECT (device), NM_DEVICE_IFACE); + if (change_ip_ifname) + g_object_notify (G_OBJECT (device), NM_DEVICE_IP_IFACE); + + /* Re-match available connections against the new interface name */ + nm_device_recheck_available_connections (device); + + /* Let any connections that use the new interface name have a chance + * to auto-activate on the device. + */ + nm_device_emit_recheck_auto_activate (device); + } + + if (klass->link_changed) + klass->link_changed (device, info); + + } else if (priv->ip_iface && ifindex == nm_device_get_ip_ifindex (device)) { + if (info->name[0] && strcmp (priv->ip_iface, info->name)) { + nm_log_info (LOGD_DEVICE, "(%s): interface index %d renamed ip_iface (%d) from '%s' to '%s'", + priv->iface, my_ifindex, nm_device_get_ip_ifindex (device), + priv->ip_iface, info->name); + g_free (priv->ip_iface); + priv->ip_iface = g_strdup (info->name); + + g_hash_table_remove_all (priv->ip6_saved_properties); + + g_object_notify (G_OBJECT (device), NM_DEVICE_IP_IFACE); + change_ip_ifname = TRUE; + } } - /* Update MTU if it has changed. */ - if (priv->mtu != info->mtu) { - priv->mtu = info->mtu; - g_object_notify (G_OBJECT (device), NM_DEVICE_MTU); + if (change_ip_ifname) { + if (priv->dhcp4_client) { + if (!nm_device_dhcp4_renew (device, FALSE)) { + nm_device_state_changed (device, + NM_DEVICE_STATE_FAILED, + NM_DEVICE_STATE_REASON_DHCP_FAILED); + return; + } + } + if (priv->dhcp6_client) { + if (!nm_device_dhcp6_renew (device, FALSE)) { + nm_device_state_changed (device, + NM_DEVICE_STATE_FAILED, + NM_DEVICE_STATE_REASON_DHCP_FAILED); + return; + } + } + if (priv->rdisc) { + /* FIXME: todo */ + } + if (priv->dnsmasq_manager) { + /* FIXME: todo */ + } } - - if (klass->link_changed) - klass->link_changed (device, info); } static void @@ -3167,6 +3232,31 @@ dhcp6_start (NMDevice *self, return ret; } +gboolean +nm_device_dhcp6_renew (NMDevice *self, gboolean release) +{ + NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); + NMActStageReturn ret; + NMDeviceStateReason reason; + NMConnection *connection; + + g_return_val_if_fail (priv->dhcp6_client != NULL, FALSE); + + nm_log_info (LOGD_DHCP6, "(%s): DHCPv6 lease renewal requested", + nm_device_get_iface (self)); + + /* Terminate old DHCP instance and release the old lease */ + dhcp6_cleanup (self, TRUE, release); + + connection = nm_device_get_connection (self); + g_assert (connection); + + /* Start DHCP again on the interface */ + ret = dhcp6_start (self, connection, priv->dhcp6_mode, &reason); + + return (ret != NM_ACT_STAGE_RETURN_FAILURE); +} + /******************************************/ static gboolean