Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(nm): promiscuous mode configuration via snapshot #4957

Merged
merged 5 commits into from
Dec 13, 2023
Merged

feat(nm): promiscuous mode configuration via snapshot #4957

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6af488f
feat(net.configuration): support for config.promisc parameter
fdizazzo Nov 7, 2023
d834cdd
feat(nm): added support for promiscuous mode
fdizazzo Nov 7, 2023
1770bd2
test: fix broken test
fdizazzo Nov 8, 2023
f13eda9
fix(net.configuration): missing default value
fdizazzo Nov 17, 2023
f4a8d5b
Merge branch 'develop' into feat/nm_promisc
fdizazzo Dec 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion ...n/src/main/java/org/eclipse/kura/net/configuration/NetworkConfigurationPropertyNames.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,8 @@ public enum NetworkConfigurationPropertyNames {
CONFIG_VLAN_ID,
CONFIG_VLAN_INGRESS_MAP,
CONFIG_VLAN_EGRESS_MAP,
CONFIG_VLAN_FLAGS,
CONFIG_VLAN_FLAGS,
CONFIG_PROMISC,
USB_PORT,
USB_MANUFACTURER,
USB_PRODUCT,
Expand Down
9 changes: 9 additions & 0 deletions ...n/src/main/java/org/eclipse/kura/net/configuration/NetworkConfigurationServiceCommon.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,7 @@ private static void getInterfaceCommonDefinition(Tocd tocd, String ifaceName) {
addTypeDefinition(tocd, ifaceName);
addMtuDefinition(tocd, ifaceName);
addAutoconnectDefinition(tocd, ifaceName);
addPromiscDefinition(tocd, ifaceName);

addIp4InterfaceCommonDefinition(tocd, ifaceName);
addIp6InterfaceCommonDefinition(tocd, ifaceName);
Expand Down Expand Up @@ -457,6 +458,14 @@ private static void addMtuDefinition(Tocd tocd, String ifaceName) {
tad.setRequired(true);
tocd.addAD(tad);
}

private static void addPromiscDefinition(Tocd tocd, String ifaceName) {
Tad tad = buildAttributeDefinition(String.format(PREFIX + "%s.config.promisc", ifaceName),
NetworkConfigurationPropertyNames.CONFIG_PROMISC, Tscalar.INTEGER);
tad.setRequired(true);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if it is not provided? There's no default provided either so I expect bad things to happen in the old networking 🤔

Copy link
Contributor Author

@fdizazzo fdizazzo Nov 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could probably be a .setRequired(false). The idea is the Kura should do nothing if the promisc parameter is not populated. And it's actually working fine with required fields being missing 🤔

Should we actually want it as required we can use -1 as the default value, as populated by NetworkManager on newly created connections.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 @pierantoniomerlino thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, it is sufficient to add a default value (probably -1 is the right choice).

tad.setDefault("-1");

tad.setDefault("-1");
tocd.addAD(tad);
}

private static void addIp4PrefixDefinition(Tocd tocd, String ifaceName) {
tocd.addAD(buildAttributeDefinition(String.format(PREFIX + "%s.config.ip4.prefix", ifaceName),
Expand Down
2 changes: 2 additions & 0 deletions ...org/eclipse/kura/net/configuration/messages/NetworkConfigurationMessagesBundle.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,8 @@ CONFIG_VLAN_INGRESS_MAP=Ingress priority map
CONFIG_VLAN_EGRESS_MAP=Egress priority map
CONFIG_VLAN_FLAGS=Vlan flags

CONFIG_PROMISC=Enable Promiscuous Mode

FIREWALL_OPEN_PORTS=The list of firewall opened ports.
FIREWALL_PORT_FORWARDING=The list of firewall port forwarding rules.
FIREWALL_NAT=The list of firewall NAT rules.
17 changes: 13 additions & 4 deletions ....eclipse.kura.nm/src/main/java/org/eclipse/kura/nm/configuration/NMSettingsConverter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,11 +116,11 @@ public static Map<String, Map<String, Variant<?>>> buildSettings(NetworkProperti
newConnectionSettings.put("ppp", pppSettingsMap);
} else if (deviceType == NMDeviceType.NM_DEVICE_TYPE_VLAN) {
Map<String, Variant<?>> vlanSettingsMap = buildVlanSettings(properties, deviceId);
Map<String, Variant<?>> ethSettingsMap = NMSettingsConverter.buildEthernetSettings(properties, deviceId);
Map<String, Variant<?>> ethSettingsMap = NMSettingsConverter.buildEthernetSettings(properties, deviceId, nmVersion);
newConnectionSettings.put("vlan", vlanSettingsMap);
newConnectionSettings.put(NM_SETTINGS_ETHERNET, ethSettingsMap);
} else if (deviceType == NMDeviceType.NM_DEVICE_TYPE_ETHERNET) {
Map<String, Variant<?>> ethSettingsMap = NMSettingsConverter.buildEthernetSettings(properties, deviceId);
Map<String, Variant<?>> ethSettingsMap = NMSettingsConverter.buildEthernetSettings(properties, deviceId, nmVersion);
newConnectionSettings.put(NM_SETTINGS_ETHERNET, ethSettingsMap);
}

Expand Down Expand Up @@ -578,11 +578,20 @@ public static Map<String, Variant<?>> buildVlanSettings(NetworkProperties props,
return settings;
}

public static Map<String, Variant<?>> buildEthernetSettings(NetworkProperties props, String deviceId) {
public static Map<String, Variant<?>> buildEthernetSettings(NetworkProperties props, String deviceId, SemanticVersion nmVersion) {
Map<String, Variant<?>> settings = new HashMap<>();
Optional<Integer> mtu = props.getOpt(Integer.class, KURA_PROPS_IPV4_MTU, deviceId);
mtu.ifPresent(value -> settings.put("mtu", new Variant<>(new UInt32(value))));
return settings;

Optional<Integer> promisc = props.getOpt(Integer.class, "net.interface.%s.config.promisc", deviceId);
if (nmVersion.isGreaterEqualThan("1.32")) {
//ethernet.accept-all-mac-addresses only supported in NetworkManager 1.32 and above
promisc.ifPresent(value -> settings.put("accept-all-mac-addresses", new Variant<>(value)));
} else {
promisc.ifPresent(value ->
logger.warn("Ignoring parameter accept-all-mac-addresses [{}]: NetworkManager 1.32 or above is required", value));
}
return settings;
}

public static Map<String, Variant<?>> buildConnectionSettings(Optional<Connection> connection, String iface,
Expand Down
2 changes: 1 addition & 1 deletion ...in.test/src/test/java/org/eclipse/kura/net/admin/NetworkConfigurationServiceImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,7 @@ protected List<NetworkConfigurationVisitor> getVisitors() {

List<AD> ads = ocd.getAD();
assertNotNull(ads);
assertEquals(130, ads.size());
assertEquals(133, ads.size());

int adsConfigured = 0;
for (AD ad : ads) {
Expand Down
8 changes: 4 additions & 4 deletions ...c/test/java/org/eclipse/kura/net/configuration/NetworkConfigurationServiceCommonTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ private void thenComponentDefinitionHasBasicProperties() {

private void thenComponentDefinitionHasCorrectNumberOfResources() {
assertNotNull(this.ads);
assertEquals(186, this.ads.size());
assertEquals(191, this.ads.size());
}

private void thenReturnedPropertyEquals(final String key, final Object value) {
Expand Down Expand Up @@ -584,15 +584,15 @@ private void thenComponentDefinitionHasCorrectProperties() {
}

private void thenComponentDefinitionHasWifiProperties() {
assertEquals(49, this.ads.stream().filter(ad -> ad.getName().contains("wlp1s0")).count());
assertEquals(50, this.ads.stream().filter(ad -> ad.getName().contains("wlp1s0")).count());
}

private void thenComponentDefinitionHasModemProperties() {
assertEquals(38, this.ads.stream().filter(ad -> ad.getName().contains("1-4")).count());
assertEquals(39, this.ads.stream().filter(ad -> ad.getName().contains("1-4")).count());
}

private void thenComponentDefinitionHasVlanProperties() {
assertEquals(33, this.ads.stream().filter(ad -> ad.getName().contains("ens5s0")).count());
assertEquals(34, this.ads.stream().filter(ad -> ad.getName().contains("ens5s0")).count());
}

private void thenPppNumIsInteger() {
Expand Down
2 changes: 1 addition & 1 deletion ....test/src/test/java/org/eclipse/kura/nm/configuration/NMConfigurationServiceImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -610,7 +610,7 @@ private void thenComponentDefinitionHasInterfaceTypes() {
private void thenComponentDefinitionHasCorrectNumberOfResources() {
assertEquals(105, this.retrievedProperties.size());
assertNotNull(this.ads);
assertEquals(153, this.ads.size());
assertEquals(157, this.ads.size());
}

private void thenComponentDefinitionHasCorrectProperties() {
Expand Down
36 changes: 36 additions & 0 deletions ...kura.nm.test/src/test/java/org/eclipse/kura/nm/configuration/NMSettingsConverterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,30 @@ public void buildIpv6SettingsShouldNotHaveMtuWhenNotSupported() {
thenNoExceptionsHaveBeenThrown();
thenResultingMapNotContains("mtu");
}

@Test
public void buildEthernetSettingsShouldHavePromiscWhenSupported() {
givenMapWith("net.interface.eth0.config.promisc", -1);
givenNetworkPropsCreatedWithTheMap(this.internetNetworkPropertiesInstanciationMap);
givenNetworkManagerVersion("1.32");

whenBuildEthernetSettingsIsRunWith(networkProperties, "eth0", nmVersion);

thenNoExceptionsHaveBeenThrown();
thenResultingMapContains("accept-all-mac-addresses", -1);
}

@Test
public void buildEthernetSettingsShouldNotHavePromiscWhenNotSupported() {
givenMapWith("net.interface.eth0.config.promisc", 1);
givenNetworkPropsCreatedWithTheMap(this.internetNetworkPropertiesInstanciationMap);
givenNetworkManagerVersion("1.31");

whenBuildEthernetSettingsIsRunWith(networkProperties, "eth0", nmVersion);

thenNoExceptionsHaveBeenThrown();
thenResultingMapNotContains("accept-all-mac-addresses");
}

@Test
public void build8021xSettingsShouldThrowIfIsEmpty() {
Expand Down Expand Up @@ -3019,6 +3043,18 @@ private void whenBuildVlanSettingsIsRunWith(NetworkProperties props, String ifac
this.hasAGenericExecptionBeenThrown = true;
}
}

private void whenBuildEthernetSettingsIsRunWith(NetworkProperties props, String iface, SemanticVersion nmVersion) {
try {
this.resultMap = NMSettingsConverter.buildEthernetSettings(props, iface, nmVersion);
} catch (NoSuchElementException e) {
this.hasNoSuchElementExceptionBeenThrown = true;
} catch (IllegalArgumentException e) {
this.hasAnIllegalArgumentExceptionThrown = true;
} catch (Exception e) {
this.hasAGenericExecptionBeenThrown = true;
}
}

/*
* Then
Expand Down