platform/ethtool,mii: retry ioctl when interface name was renamed for ehttool/mii

ethtool/mii API is based on the ifname. As an interface can be renamed,
this API is inherently racy. We would prefer to use the ifindex instead.
The ifindex of a device cannot change (altough it can repeat, which opens a
different race *sigh*).

Anyway, we were already trying to minimize the race be resolving the
name from ifindex immediately before the call to ethtool/mii.

Do better than that. Now resolve the name before and after the call. If
the name changed in the meantime, we have an indication that a race
might have happend (but we cannot be sure).

Note that this can not catch every possible kind of rename race. If you are very
unlucky a swapping of names cannot be detected.

For getters this is relatively straight forward. Just retry when we
have an indication to fall victim to a race (up to a few times). Yes, we
still cannot be 100% sure, but this should be very reliable in practice.

For setters (that modify the device) we also retry. We do so under the
assumption that setting the same options multiple times has no bad effect.
Note that for setters the race of swapping interface names is particularly
bad. If we hit a very unlucky race condition, we might set the setting on
the wrong interface and there is nothing we can do about it. The retry only
ensures that eventually we will set it on the right interface.

Note that this involves one more if_indextoname() call for each operation (in
the common case when there is no renaming race). In cases where we make
multiple ioctl calls, we cache and reuse the information though. So, for such
calls the overhead is even smaller.
This commit is contained in:
Thomas Haller
2019-05-02 17:13:51 +02:00
parent 945620624a
commit 856322562e
2 changed files with 317 additions and 174 deletions

View File

@@ -6514,7 +6514,8 @@ link_supports_carrier_detect (NMPlatform *platform, int ifindex)
* us whether the device actually supports carrier detection in the first
* place. We assume any device that does implements one of these two APIs.
*/
return nmp_utils_ethtool_supports_carrier_detect (ifindex) || nmp_utils_mii_supports_carrier_detect (ifindex);
return nmp_utils_ethtool_supports_carrier_detect (ifindex)
|| nmp_utils_mii_supports_carrier_detect (ifindex);
}
static gboolean