fdo-mirrors/NetworkManager
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/NetworkManager/NetworkManager.git synced 2025-12-26 12:20:09 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions

ppp: fix taking control of link generated by kernel

Browse source 
NetworkManager can't control the name of the PPP interface name
created by pppd; so it has to wait for the interface to appear and
then rename it. This happens in nm_device_take_over_link() called by
nm-device-ppp.c:ppp_ifindex_set() when pppd tells NM the ifindex of
the interface that was created.

However, sometimes the initial interface name is already correct, for
example when the connection.interface-name is ppp0 and this is the
first PPP interface created.

When this happens, nm_device_update_from_platform_link() is called on
the NMDevicePPP and it sets the device ifindex. Later, when pppd
notifies NM, nm_device_take_over_link() fails because the ifindex is
already set:

 nm_device_take_over_link: assertion 'priv->ifindex <= 0' failed

Make nm_device_take_over_link() more robust to cope with this
situation.

https://bugzilla.redhat.com/show_bug.cgi?id=1849386
This commit is contained in:
Beniamino Galvani 2020-06-22 17:04:06 +02:00 • committed by Antonio Cardace
parent ce432a3abc
commit b9ce5ae9d7
No known key found for this signature in database
GPG key ID: 6BF80ABD43E377D3
3 changed files with 47 additions and 12 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
src/devices/nm-device-ppp.c
View file

@ -71,9 +71,13 @@ ppp_ifindex_set (NMPPPManager *ppp_manager,
gpointer user_data)
{
NMDevice *device = NM_DEVICE (user_data);
NMDevicePpp *self = NM_DEVICE_PPP (device);
gs_free char *old_name = NULL;
gs_free_error GError *error = NULL;
if (!nm_device_take_over_link (device, ifindex, &old_name)) {
if (!nm_device_take_over_link (device, ifindex, &old_name, &error)) {
_LOGW (LOGD_DEVICE | LOGD_PPP, "could not take control of link %d: %s",
ifindex, error->message);
nm_device_state_changed (device, NM_DEVICE_STATE_FAILED,
NM_DEVICE_STATE_REASON_IP_CONFIG_UNAVAILABLE);
return;

2
src/devices/nm-device-private.h
View file

@ -62,7 +62,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, int ifindex, char **old_name);
gboolean nm_device_take_over_link (NMDevice *self, int ifindex, char **old_name, GError **error);
gboolean nm_device_hw_addr_set (NMDevice *device,
const char *addr,

51
src/devices/nm-device.c
View file

@ -1746,25 +1746,51 @@ nm_device_get_iface (NMDevice *self)
return NM_DEVICE_GET_PRIVATE (self)->iface;
}
/**
* nm_device_take_over_link:
* @self: the #NMDevice
* @ifindex: a ifindex
* @old_name: (transfer full): on return, the name of the old link, if
* the link was renamed
* @error: location to store error, or %NULL
*
* Given an existing link, move it under the control of a device. In
* particular, the link will be renamed to match the device name. If the
* link was renamed, the old name is returned in @old_name.
*
* Returns: %TRUE if the device took control of the link, %FALSE otherwise
*/
gboolean
nm_device_take_over_link (NMDevice *self, int ifindex, char **old_name)
nm_device_take_over_link (NMDevice *self, int ifindex, char **old_name, GError **error)
{
NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self);
const NMPlatformLink *plink;
NMPlatform *platform;
gboolean up, success = TRUE;
gs_free char *name = NULL;
g_return_val_if_fail (priv->ifindex <= 0, FALSE);
nm_assert (ifindex > 0);
NM_SET_OUT (old_name, NULL);
if ( priv->ifindex > 0
&& priv->ifindex != ifindex) {
nm_utils_error_set (error, NM_UTILS_ERROR_UNKNOWN,
"the device already has ifindex %d",
priv->ifindex);
return FALSE;
}
platform = nm_device_get_platform (self);
plink = nm_platform_link_get (platform, ifindex);
if (!plink)
if (!plink) {
nm_utils_error_set (error, NM_UTILS_ERROR_UNKNOWN,
"link %d not found", ifindex);
return FALSE;
}
if (!nm_streq (plink->name, nm_device_get_iface (self))) {
gboolean up;
gboolean success;
gs_free char *name = NULL;
up = NM_FLAGS_HAS (plink->n_ifi_flags, IFF_UP);
name = g_strdup (plink->name);
@ -1775,16 +1801,21 @@ nm_device_take_over_link (NMDevice *self, int ifindex, char **old_name)
if (up)
nm_platform_link_set_up (platform, ifindex, NULL);
if (success)
NM_SET_OUT (old_name, g_steal_pointer (&name));
if (!success) {
nm_utils_error_set (error, NM_UTILS_ERROR_UNKNOWN,
"failure renaming link %d", ifindex);
return FALSE;
}
NM_SET_OUT (old_name, g_steal_pointer (&name));
}
if (success) {
if (priv->ifindex != ifindex) {
priv->ifindex = ifindex;
_notify (self, PROP_IFINDEX);
}
return success;
return TRUE;
}
int