Skip to content

Commit 8926c80

Browse files
pierantoniomerlinopierantoniomerlino
authored
feat(nm): Made network configuration timeout configurable (#5858)
* Added timeout property in system service Signed-off-by: pierantoniomerlino <[email protected]> * Added timeout property in nm Signed-off-by: pierantoniomerlino <[email protected]> * Added async methods for applying network config Signed-off-by: pierantoniomerlino <[email protected]> * Fixed copyright Signed-off-by: pierantoniomerlino <[email protected]> * Fixed copyright again Signed-off-by: pierantoniomerlino <[email protected]> * Fixed typo Signed-off-by: pierantoniomerlino <[email protected]> * Attempt to fix reset issue Signed-off-by: pierantoniomerlino <[email protected]> * Several fixes Signed-off-by: pierantoniomerlino <[email protected]> * Fixed copyright and log Signed-off-by: pierantoniomerlino <[email protected]> * Added test for code coverage Signed-off-by: pierantoniomerlino <[email protected]> --------- Signed-off-by: pierantoniomerlino <[email protected]>
1 parent a775840 commit 8926c80

8 files changed

Lines changed: 154 additions & 63 deletions

File tree

‎kura/org.eclipse.kura.api/src/main/java/org/eclipse/kura/system/SystemService.java‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
/*******************************************************************************
2-
* Copyright (c) 2011, 2024 Eurotech and/or its affiliates and others
3-
*
2+
* Copyright (c) 2011, 2025 Eurotech and/or its affiliates and others
3+
*
44
* This program and the accompanying materials are made
55
* available under the terms of the Eclipse Public License 2.0
66
* which is available at https://www.eclipse.org/legal/epl-2.0/
7-
*
7+
*
88
* SPDX-License-Identifier: EPL-2.0
9-
*
9+
*
1010
* Contributors:
1111
* Eurotech
1212
******************************************************************************/
@@ -141,6 +141,11 @@ public interface SystemService {
141141
*/
142142
public static final String KEY_WPA3_WIFI_SECURITY_ENABLE = "kura.wpa3.wifi.security.enable";
143143

144+
/**
145+
* @since 3.0
146+
*/
147+
public static final String KEY_NETWORK_CONFIGURATION_TIMEOUT = "kura.network.configuration.timeout";
148+
144149
/**
145150
* @deprecated
146151
*/
@@ -633,4 +638,12 @@ public interface SystemService {
633638
*/
634639
public boolean isWPA3WifiSecurityEnabled();
635640

641+
/**
642+
* Returns the timeout in seconds used for applying a network configuration.
643+
*
644+
* @since 3.0
645+
* @return the timeout value in seconds
646+
*/
647+
public int getNetworkConfigurationTimeout();
648+
636649
}

‎kura/org.eclipse.kura.core.system/src/main/java/org/eclipse/kura/core/system/SystemServiceImpl.java‎

Lines changed: 22 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -936,30 +936,12 @@ public String getKuraSnapshotsDirectory() {
936936

937937
@Override
938938
public int getKuraSnapshotsCount() {
939-
int iMaxCount = 10;
940-
final Optional<String> maxCount = getProperty(KEY_KURA_SNAPSHOTS_COUNT);
941-
if (maxCount.isPresent() && maxCount.get().trim().length() > 0) {
942-
try {
943-
iMaxCount = Integer.parseInt(maxCount.get());
944-
} catch (NumberFormatException nfe) {
945-
logger.error("Error - Invalid kura.snapshots.count setting. Using default.", nfe);
946-
}
947-
}
948-
return iMaxCount;
939+
return getIntegerPropertyValue(KEY_KURA_SNAPSHOTS_COUNT, 10);
949940
}
950941

951942
@Override
952943
public int getKuraWifiTopChannel() {
953-
final Optional<String> topWifiChannel = getProperty(KEY_KURA_WIFI_TOP_CHANNEL);
954-
if (topWifiChannel.isPresent() && topWifiChannel.get().trim().length() > 0) {
955-
return Integer.parseInt(topWifiChannel.get());
956-
}
957-
958-
if (logger.isDebugEnabled()) {
959-
logger.debug("The last wifi channel is not defined for this system - setting fake value.");
960-
}
961-
962-
return Integer.MAX_VALUE;
944+
return getIntegerPropertyValue(KEY_KURA_WIFI_TOP_CHANNEL, Integer.MAX_VALUE);
963945
}
964946

965947
@Override
@@ -974,23 +956,12 @@ public String getKuraWebEnabled() {
974956

975957
@Override
976958
public int getFileCommandZipMaxUploadSize() {
977-
final Optional<String> commandMaxUpload = getProperty(KEY_FILE_COMMAND_ZIP_MAX_SIZE);
978-
if (commandMaxUpload.isPresent() && commandMaxUpload.get().trim().length() > 0) {
979-
return Integer.parseInt(commandMaxUpload.get());
980-
}
981-
logger.warn("Maximum command line upload size not available. Set default to 100 MB");
982-
return 100;
959+
return getIntegerPropertyValue(KEY_FILE_COMMAND_ZIP_MAX_SIZE, 100);
983960
}
984961

985962
@Override
986963
public int getFileCommandZipMaxUploadNumber() {
987-
final Optional<String> commandMaxFilesUpload = getProperty(KEY_FILE_COMMAND_ZIP_MAX_NUMBER);
988-
if (commandMaxFilesUpload.isPresent() && commandMaxFilesUpload.get().trim().length() > 0) {
989-
return Integer.parseInt(commandMaxFilesUpload.get());
990-
}
991-
logger.warn(
992-
"Missing the parameter that specifies the maximum number of files uploadable using the command servlet. Set default to 1024 files");
993-
return 1024;
964+
return getIntegerPropertyValue(KEY_FILE_COMMAND_ZIP_MAX_NUMBER, 1024);
994965
}
995966

996967
@Override
@@ -1542,4 +1513,22 @@ public boolean isWPA3WifiSecurityEnabled() {
15421513
return false;
15431514
}
15441515

1516+
@Override
1517+
public int getNetworkConfigurationTimeout() {
1518+
return getIntegerPropertyValue(KEY_NETWORK_CONFIGURATION_TIMEOUT, 30);
1519+
}
1520+
1521+
private int getIntegerPropertyValue(String propertyName, int defaultValue) {
1522+
final Optional<String> propertyValue = getProperty(propertyName);
1523+
if (propertyValue.isPresent() && !propertyValue.get().trim().isEmpty()) {
1524+
try {
1525+
return Integer.parseInt(propertyValue.get());
1526+
} catch (NumberFormatException e) {
1527+
logger.error("Cannot parse integer value for property {}: {}. Set it to default {}.", propertyName,
1528+
propertyValue.get(), defaultValue, e);
1529+
}
1530+
}
1531+
return defaultValue;
1532+
}
1533+
15451534
}

‎kura/org.eclipse.kura.nm/src/main/java/org/eclipse/kura/nm/NMDbusConnector.java‎

Lines changed: 46 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,11 @@
2424
import java.util.Objects;
2525
import java.util.Optional;
2626
import java.util.Set;
27+
import java.util.concurrent.CompletableFuture;
28+
import java.util.concurrent.ExecutorService;
29+
import java.util.concurrent.Executors;
2730
import java.util.concurrent.TimeoutException;
31+
import java.util.concurrent.atomic.AtomicBoolean;
2832

2933
import org.eclipse.kura.KuraException;
3034
import org.eclipse.kura.executor.CommandExecutorService;
@@ -106,8 +110,11 @@ public class NMDbusConnector {
106110
private NMConfigurationEnforcementHandler configurationEnforcementHandler = null;
107111
private NMDeviceAddedHandler deviceAddedHandler = null;
108112

109-
private boolean configurationEnforcementHandlerIsArmed = false;
113+
private AtomicBoolean configurationEnforcementHandlerIsArmed = new AtomicBoolean(false);
110114
private ModemTaskManager modemTaskManager;
115+
private int timeout = 30;
116+
private final ExecutorService executorService = Executors.newSingleThreadExecutor();
117+
private CompletableFuture<Void> configurationTask;
111118

112119
private NMDbusConnector(DBusConnection dbusConnection) throws DBusException {
113120
this.dbusConnection = Objects.requireNonNull(dbusConnection);
@@ -135,11 +142,12 @@ public DBusConnection getDbusConnection() {
135142

136143
public void setSystemService(SystemService systemService) {
137144
this.optionalSystemService = Optional.of(systemService);
145+
this.timeout = systemService.getNetworkConfigurationTimeout();
138146
}
139147

140148
protected boolean configurationEnforcementIsActive() {
141149
return Objects.nonNull(this.configurationEnforcementHandler) && Objects.nonNull(this.deviceAddedHandler)
142-
&& this.configurationEnforcementHandlerIsArmed;
150+
&& this.configurationEnforcementHandlerIsArmed.get();
143151
}
144152

145153
protected boolean modemTaskHandlerIsPresent(String deviceId) {
@@ -160,7 +168,7 @@ public void checkVersion() {
160168
logger.debug("NM Version: {}", nmVersion);
161169
}
162170

163-
public synchronized List<String> getInterfaceIds() throws DBusException {
171+
public List<String> getInterfaceIds() throws DBusException {
164172
List<Device> availableDevices = this.networkManager.getAllDevices();
165173

166174
List<String> supportedDeviceNames = new ArrayList<>();
@@ -175,7 +183,7 @@ public synchronized List<String> getInterfaceIds() throws DBusException {
175183
return supportedDeviceNames;
176184
}
177185

178-
public synchronized String getInterfaceName(String interfaceId) throws DBusException {
186+
public String getInterfaceName(String interfaceId) throws DBusException {
179187
Optional<Device> device = getNetworkManagerDeviceByInterfaceId(interfaceId);
180188
if (device.isPresent()) {
181189
NMDeviceType deviceType = this.networkManager.getDeviceType(device.get().getObjectPath());
@@ -221,7 +229,7 @@ public String getInterfaceIdByDBusPath(String dbusPath) throws DBusException {
221229
}
222230
}
223231

224-
public synchronized NetworkInterfaceStatus getInterfaceStatus(String interfaceId, boolean recompute,
232+
public NetworkInterfaceStatus getInterfaceStatus(String interfaceId, boolean recompute,
225233
CommandExecutorService commandExecutorService) throws DBusException, KuraException {
226234
NetworkInterfaceStatus networkInterfaceStatus = null;
227235

@@ -358,7 +366,20 @@ ip4configProperties, ip6configProperties, new AccessPointsProperties(activeAcces
358366
return networkInterfaceStatus;
359367
}
360368

369+
private void runAsync(Runnable task) {
370+
cancelConfigurationTask();
371+
this.configurationTask = CompletableFuture.runAsync(task, this.executorService);
372+
}
373+
374+
private void cancelConfigurationTask() {
375+
if (this.configurationTask != null && !this.configurationTask.isDone()) {
376+
logger.warn("A previous configuration task is still running. Aborting current configuration task.");
377+
this.configurationTask.cancel(true);
378+
}
379+
}
380+
361381
public synchronized void apply(Map<String, Object> networkConfiguration) throws DBusException {
382+
logger.debug("Apply networkConfiguration");
362383
try {
363384
configurationEnforcementDisable();
364385
// Disable ModemTaskHandler since it is supposed to be a new configuration
@@ -371,10 +392,12 @@ public synchronized void apply(Map<String, Object> networkConfiguration) throws
371392
}
372393

373394
public synchronized void apply() throws DBusException {
395+
logger.debug("Apply cached networkConfiguration");
374396
if (Objects.isNull(this.cachedConfiguration)) {
375397
logger.warn("No cached network configuration found.");
376398
return;
377399
}
400+
378401
try {
379402
configurationEnforcementDisable();
380403
doApply(this.cachedConfiguration);
@@ -384,13 +407,15 @@ public synchronized void apply() throws DBusException {
384407
}
385408

386409
public synchronized void apply(String deviceId) throws DBusException {
410+
logger.debug("Apply cached networkConfiguration for device {}", deviceId);
387411
if (Objects.isNull(deviceId) || deviceId.isEmpty()) {
388412
throw new IllegalArgumentException("DeviceId cannot be null or empty.");
389413
}
390414
if (Objects.isNull(this.cachedConfiguration)) {
391415
logger.warn("No cached network configuration found.");
392416
return;
393417
}
418+
394419
try {
395420
configurationEnforcementDisable();
396421
doApply(deviceId, this.cachedConfiguration);
@@ -399,6 +424,17 @@ public synchronized void apply(String deviceId) throws DBusException {
399424
}
400425
}
401426

427+
public synchronized void asyncApply(Map<String, Object> networkConfiguration) {
428+
logger.debug("Apply networkConfiguration with asynchronous task");
429+
runAsync(() -> {
430+
try {
431+
apply(networkConfiguration);
432+
} catch (DBusException e) {
433+
logger.error("Couldn't apply network configuration settings due to: ", e);
434+
}
435+
});
436+
}
437+
402438
private synchronized void doApply(Map<String, Object> networkConfiguration) throws DBusException {
403439
logger.info("Applying configuration using NetworkManager Dbus connector");
404440
NetworkProperties properties = new NetworkProperties(networkConfiguration);
@@ -552,7 +588,7 @@ private void enableInterface(String deviceId, NetworkProperties properties, Devi
552588
connection, deviceId, interfaceName, deviceType, this.networkManager.getVersion());
553589

554590
DeviceStateLock dsLock = new DeviceStateLock(this.dbusConnection, device.getObjectPath(),
555-
NMDeviceState.NM_DEVICE_STATE_CONFIG);
591+
NMDeviceState.NM_DEVICE_STATE_CONFIG, this.timeout);
556592

557593
if (connection.isPresent()) {
558594
connection.get().Update(newConnectionSettings);
@@ -605,7 +641,7 @@ private void createVirtualInterface(String deviceId, NetworkProperties propertie
605641
this.networkManager.setDeviceManaged(createdDevice, true);
606642
}
607643
DeviceStateLock dsLock = new DeviceStateLock(this.dbusConnection, createdDevice.getObjectPath(),
608-
NMDeviceState.NM_DEVICE_STATE_ACTIVATED);
644+
NMDeviceState.NM_DEVICE_STATE_ACTIVATED, this.timeout);
609645
this.networkManager.activateConnection(createdConnection, createdDevice);
610646
dsLock.waitForSignal();
611647
} catch (DBusExecutionException | DBusException | TimeoutException e) {
@@ -653,7 +689,7 @@ private void disable(Optional<Device> optDevice, String deviceId) throws DBusExc
653689
NMDeviceState deviceState = this.networkManager.getDeviceState(device);
654690
if (Boolean.TRUE.equals(NMDeviceState.isConnected(deviceState))) {
655691
DeviceStateLock dsLock = new DeviceStateLock(this.dbusConnection, device.getObjectPath(),
656-
NMDeviceState.NM_DEVICE_STATE_DISCONNECTED);
692+
NMDeviceState.NM_DEVICE_STATE_DISCONNECTED, this.timeout);
657693
device.Disconnect();
658694
dsLock.waitForSignal();
659695
}
@@ -674,10 +710,9 @@ private void configurationEnforcementEnable() throws DBusException {
674710
this.configurationEnforcementHandler = new NMConfigurationEnforcementHandler(this);
675711
this.deviceAddedHandler = new NMDeviceAddedHandler(this);
676712
}
677-
678713
this.dbusConnection.addSigHandler(Device.StateChanged.class, this.configurationEnforcementHandler);
679714
this.dbusConnection.addSigHandler(NetworkManager.DeviceAdded.class, this.deviceAddedHandler);
680-
this.configurationEnforcementHandlerIsArmed = true;
715+
this.configurationEnforcementHandlerIsArmed.set(true);
681716
logger.debug("Network configuration enforcement set to {} (Expected: true)",
682717
this.configurationEnforcementHandlerIsArmed);
683718
}
@@ -687,7 +722,7 @@ private void configurationEnforcementDisable() throws DBusException {
687722
this.dbusConnection.removeSigHandler(Device.StateChanged.class, this.configurationEnforcementHandler);
688723
this.dbusConnection.removeSigHandler(NetworkManager.DeviceAdded.class, this.deviceAddedHandler);
689724
}
690-
this.configurationEnforcementHandlerIsArmed = false;
725+
this.configurationEnforcementHandlerIsArmed.set(false);
691726
logger.debug("Network configuration enforcement set to {} (Expected: false)",
692727
this.configurationEnforcementHandlerIsArmed);
693728
}

‎kura/org.eclipse.kura.nm/src/main/java/org/eclipse/kura/nm/configuration/NMConfigurationServiceImpl.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*******************************************************************************
2-
* Copyright (c) 2023, 2024 Eurotech and/or its affiliates and others
2+
* Copyright (c) 2023, 2025 Eurotech and/or its affiliates and others
33
*
44
* This program and the accompanying materials are made
55
* available under the terms of the Eclipse Public License 2.0
@@ -426,8 +426,8 @@ private void writeNetworkConfigurationSettings(Map<String, Object> networkProper
426426
}
427427

428428
try {
429-
this.nmDbusConnector.apply(networkProperties);
430-
} catch (DBusExecutionException | DBusException e) {
429+
this.nmDbusConnector.asyncApply(networkProperties);
430+
} catch (DBusExecutionException e) {
431431
logger.error("Couldn't apply network configuration settings due to: ", e);
432432
}
433433
}

‎kura/org.eclipse.kura.nm/src/main/java/org/eclipse/kura/nm/signal/handlers/DeviceStateLock.java‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,21 +30,23 @@ public class DeviceStateLock {
3030
private final CountDownLatch latch = new CountDownLatch(1);
3131
private final NMDeviceStateChangeHandler stateHandler;
3232
private final DBusConnection dbusConnection;
33+
private final int timeout;
3334

34-
public DeviceStateLock(DBusConnection dbusConnection, String dbusPath, NMDeviceState expectedNmDeviceState)
35-
throws DBusException {
35+
public DeviceStateLock(DBusConnection dbusConnection, String dbusPath, NMDeviceState expectedNmDeviceState,
36+
int timeout) throws DBusException {
3637
if (Objects.isNull(dbusPath) || dbusPath.isEmpty() || dbusPath.equals("/")) {
3738
throw new IllegalArgumentException(String.format("Illegal DBus path for DeviceStateLock \"%s\"", dbusPath));
3839
}
3940
this.dbusConnection = Objects.requireNonNull(dbusConnection);
4041
this.stateHandler = new NMDeviceStateChangeHandler(this.latch, dbusPath, expectedNmDeviceState);
42+
this.timeout = timeout;
4143

4244
this.dbusConnection.addSigHandler(Device.StateChanged.class, this.stateHandler);
4345
}
4446

4547
public void waitForSignal() throws DBusException {
4648
try {
47-
boolean countdownCompleted = this.latch.await(5, TimeUnit.SECONDS);
49+
boolean countdownCompleted = this.latch.await(this.timeout, TimeUnit.SECONDS);
4850
if (!countdownCompleted) {
4951
logger.warn("Timeout elapsed. Exiting anyway");
5052
}

‎kura/org.eclipse.kura.nm/src/main/java/org/eclipse/kura/nm/signal/handlers/NMConfigurationEnforcementHandler.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*******************************************************************************
2-
* Copyright (c) 2023 Eurotech and/or its affiliates and others
2+
* Copyright (c) 2023, 2025 Eurotech and/or its affiliates and others
33
*
44
* This program and the accompanying materials are made
55
* available under the terms of the Eclipse Public License 2.0
@@ -39,8 +39,8 @@ public void handle(Device.StateChanged s) {
3939
NMDeviceState newState = NMDeviceState.fromUInt32(s.getNewState());
4040
NMDeviceStateReason reason = NMDeviceStateReason.fromUInt32(s.getReason());
4141

42-
logger.debug("Device state change detected: {} -> {} (reason: {}), for device {}", oldState, newState, reason,
43-
s.getPath());
42+
logger.debug("Device state change detected by configuration enforcer: {} -> {} (reason: {}), for device {}",
43+
oldState, newState, reason, s.getPath());
4444

4545
boolean deviceDisconnectedBecauseOfConfigurationEvent = oldState != NMDeviceState.NM_DEVICE_STATE_FAILED
4646
&& oldState != NMDeviceState.NM_DEVICE_STATE_UNAVAILABLE

‎kura/test/org.eclipse.kura.core.system.test/src/main/java/org/eclipse/kura/core/system/test/SystemServiceTest.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,4 +319,10 @@ public void shouldGetDefaultWPA3WifiSecuritySupportProperty() {
319319
assertFalse(systemService.isWPA3WifiSecurityEnabled());
320320
}
321321

322+
@TestTarget(targetPlatforms = { TestTarget.PLATFORM_ALL })
323+
@Test
324+
public void shouldGetDefaultNetworkConfigurationTimeoutProperty() {
325+
assertEquals(30, systemService.getNetworkConfigurationTimeout());
326+
}
327+
322328
}

0 commit comments

Comments
 (0)