Rework NIC / Network Reporting (#75)

* Rework NIC / Network Reporting

* Switch to monotonic timer for calculating time deltas between reports. Relying on `DateTime` to calculate time deltas is dangerous as it would be susceptible to phenomena such as a user changing their time or leap seconds. The Stopwatch class allows us to get a monotonic timer if it exists.
* Interface statistics reset on wake. Previously this would cause nonsensical negative values to be reported. The fix is to reset the internal state when the interface statistics have been reset.
* Previously, interface hardware already seen when scanned would be re-registered causing zeros to be reported, so the fix is to keep track of what NICs have already been registered
* Corollary, old interfaces were not closed when removed.
* Document the code and constants used (eg: "800" is not indicative of converting bytes to bits and a ratio to a percentage). Hopefully it is much clearer.

Thanks to @nickbabcock
This commit is contained in:
Nick Babcock
2019-01-13 12:40:03 +01:00
committed by Phyxion
parent 2bc12a363f
commit 963dd27247
2 changed files with 78 additions and 40 deletions
+44 -31
View File
@@ -9,6 +9,7 @@
*/
using System;
using System.Diagnostics;
using System.Globalization;
using System.Net.NetworkInformation;
@@ -21,14 +22,14 @@ namespace OpenHardwareMonitor.Hardware.Nic
private Sensor uploadSpeed;
private Sensor downloadSpeed;
private Sensor networkUtilization;
private DateTime latesTime;
private long lastTick;
private long bytesUploaded;
private long bytesDownloaded;
private readonly NetworkInterface networkInterface;
public Nic(NetworkInterface networkInterface, ISettings settings, int index)
: base(networkInterface.Name, new Identifier("NIC",index.ToString(CultureInfo.InvariantCulture)), settings)
public Nic(NetworkInterface networkInterface, ISettings settings)
: base(networkInterface.Name, new Identifier("nic", networkInterface.Id), settings)
{
this.networkInterface = networkInterface;
dataUploaded = new Sensor("Data Uploaded", 2, SensorType.Data, this, settings);
@@ -43,7 +44,7 @@ namespace OpenHardwareMonitor.Hardware.Nic
ActivateSensor(networkUtilization);
bytesUploaded = NetworkInterface.GetIPStatistics().BytesSent;
bytesDownloaded = NetworkInterface.GetIPStatistics().BytesReceived;
latesTime = DateTime.Now;
lastTick = Stopwatch.GetTimestamp();
}
public override HardwareType HardwareType
@@ -64,37 +65,49 @@ namespace OpenHardwareMonitor.Hardware.Nic
public override void Update()
{
DateTime newTime = DateTime.Now;
float dt = (float)(newTime - latesTime).TotalSeconds;
latesTime = newTime;
IPv4InterfaceStatistics interfaceStats = NetworkInterface.GetIPv4Statistics();
long newTick = Stopwatch.GetTimestamp();
double dt = new TimeSpan(newTick - lastTick).TotalSeconds;
IPv4InterfaceStatistics interfaceStats = NetworkInterface.GetIPv4Statistics();
// Report out the number of GB (2^30 Bytes) that this interface has up/downloaded. Note
// that these values can reset back at zero (eg: after waking from sleep).
dataUploaded.Value = (float)(interfaceStats.BytesSent / (double)0x40000000);
dataDownloaded.Value = (float)(interfaceStats.BytesReceived / (double)0x40000000);
// Detect a reset in interface stats if the new total is less than what was previously
// seen. While setting the previous values to zero doesn't encapsulate the value the
// instant before the reset, it is the best approximation we have.
if (interfaceStats.BytesSent < bytesUploaded || interfaceStats.BytesReceived < bytesDownloaded)
{
bytesUploaded = 0;
bytesDownloaded = 0;
}
long dBytesUploaded = interfaceStats.BytesSent - bytesUploaded;
long dBytesDownloaded = interfaceStats.BytesReceived - bytesDownloaded;
uploadSpeed.Value = (float)dBytesUploaded / dt;
downloadSpeed.Value = (float)dBytesDownloaded / dt;
networkUtilization.Value = Clamp((float)Math.Max(dBytesUploaded, dBytesDownloaded) * 800 / NetworkInterface.Speed / dt, 0, 100);
// Upload and download speeds are reported as the number of bytes transfered over the
// time difference since the last report. In this way, the values represent the average
// number of bytes up/downloaded in a second.
uploadSpeed.Value = (float)(dBytesUploaded / dt);
downloadSpeed.Value = (float)(dBytesDownloaded / dt);
// Network speed is in bits per second, so when calculating the load on the NIC we first
// grab the total number of bits up/downloaded
long dbits = (dBytesUploaded + dBytesDownloaded) * 8;
// Converts the ratio of total bits transferred over time over theoretical max bits
// transfer rate into a percentage load
double load = (dbits / dt / NetworkInterface.Speed) * 100;
// Finally clamp the value between 0% and 100% to avoid reporting nonsensical numbers
networkUtilization.Value = (float)Math.Min(Math.Max(load, 0), 100);
// Store the recorded values and time, so they can be used in the next update
bytesUploaded = interfaceStats.BytesSent;
bytesDownloaded = interfaceStats.BytesReceived;
dataUploaded.Value = ((float)bytesUploaded / 1073741824);
dataDownloaded.Value = ((float)bytesDownloaded / 1073741824);
}
/// <summary>
/// Clamps the specified value.
/// </summary>
/// <param name="value">The value.</param>
/// <param name="min">The minimum.</param>
/// <param name="max">The maximum.</param>
/// <returns><see cref="System.Single"/>.</returns>
private static float Clamp(float value, float min, float max)
{
if (value < min)
return min;
if (value > max)
return max;
return value;
lastTick = newTick;
}
}
}
+34 -9
View File
@@ -9,6 +9,8 @@ namespace OpenHardwareMonitor.Hardware.Nic
{
private readonly ISettings _settings;
private List<Nic> _hardware = new List<Nic>();
private readonly object _scanLock = new object();
private readonly Dictionary<string, Nic> _nics = new Dictionary<string, Nic>();
public NicGroup(ISettings settings)
{
@@ -20,10 +22,38 @@ namespace OpenHardwareMonitor.Hardware.Nic
private void ScanNics(ISettings settings)
{
NetworkInterfaces = NetworkInterface.GetAllNetworkInterfaces();
_hardware = NetworkInterfaces.Where(DesiredNetworkType)
.Select((x, i) => new Nic(x, settings, i))
.ToList();
// If no network is marked up (excluding loopback and tunnel) then don't scan
// for interfaces.
if (!NetworkInterface.GetIsNetworkAvailable())
{
return;
}
// When multiple events fire concurrently, we don't want threads interferring
// with others as they manipulate non-thread safe state.
lock (_scanLock)
{
var networkInterfaces = NetworkInterface.GetAllNetworkInterfaces()
.Where(DesiredNetworkType)
.OrderBy(x => x.Name);
var scanned = networkInterfaces.ToDictionary(x => x.Id, x => x);
var newNics = scanned.Where(x => !_nics.ContainsKey(x.Key));
var removedNics = _nics.Where(x => !scanned.ContainsKey(x.Key)).ToList();
foreach (var nic in removedNics)
{
nic.Value.Close();
_nics.Remove(nic.Key);
}
foreach (var nic in newNics)
{
_nics.Add(nic.Key, new Nic(nic.Value, settings));
}
_hardware = _nics.Values.OrderBy(x => x.Name).ToList();
}
}
private void NetworkChange_NetworkAddressChanged(object sender, System.EventArgs e)
@@ -46,9 +76,6 @@ namespace OpenHardwareMonitor.Hardware.Nic
public string GetReport()
{
if (NetworkInterfaces == null)
return null;
var report = new StringBuilder();
foreach (Nic hw in _hardware)
@@ -75,8 +102,6 @@ namespace OpenHardwareMonitor.Hardware.Nic
}
}
public NetworkInterface[] NetworkInterfaces { get; set; }
public void Close()
{
NetworkChange.NetworkAddressChanged -= NetworkChange_NetworkAddressChanged;