GNOME Bugzilla – Bug 763937
Don't assign IP belonging to the same class from the one already active.
Last modified: 2017-01-16 09:57:29 UTC
I was trying to create a DHCP server (ethernet->ipv4->'shared to other computers' in nm-connection-editor) while I was already connected to a Wireless Hotspot. My wireless connection already has some IP from the class 10.42.0.0/24. The problem is that the second connection (The DHCP server I created) is too allocating the same network 10.42.0.0/24 with server IP as 10.42.0.1, which conflicts with the other. Network Manager should allocate a different network (like 10.42.1.1/24) in this case. Please assign to the right package if NetworkManager has nothing to do with this. -- Thank you
Try to configure one manual IP address for the shared connection, that IP address is then used to determine the range for the shared network. For example, 192.168.5.1/24 will cause the shared connection to use 192.168.5.0/24 instead. Considering the address ranges that are configured on other devices might not be a good idea. For example, if you first activate your shared connection and the conflicting 10.42.0.0/24 device afterwards, you also get a conflict. Should NM then reconfigure the shared connection? If you have such conflicts, you need some manual configuration to avoid them. That is already possible.
(In reply to Thomas Haller from comment #1) > Try to configure one manual IP address for the shared connection, that IP > address is then used to determine the range for the shared network. > I couldn't find a way to add manual IP for the shared connection in nm-connection-editior. May be I can do with 'ip address'. But I wish this to be working right out of the box. > For example, 192.168.5.1/24 will cause the shared connection to use > 192.168.5.0/24 instead. > > > > Considering the address ranges that are configured on other devices might > not be a good idea. For example, if you first activate your shared > connection and the conflicting 10.42.0.0/24 device afterwards, you also get > a conflict. Should NM then reconfigure the shared connection? If you have > such conflicts, you need some manual configuration to avoid them. That is > already possible. I think I should say it in order. 1. I connected to a wireless hotspot with my laptop. Say, I got the IP 10.42.0.23/24 for wlan0 as a DHCP client. 2. Then, on the same laptop I created a shared connection afterwards. NM assigned 10.42.0.1 to my eth0 as DHCP server, which is one the same network. As NM is already able to get the current IPs, it should have assigned eth0 an IP from a different network, say 10.42.1.1/24.
(In reply to Mohammed Sadiq from comment #2) > (In reply to Thomas Haller from comment #1) > > Try to configure one manual IP address for the shared connection, that IP > > address is then used to determine the range for the shared network. > > > > I couldn't find a way to add manual IP for the shared connection in > nm-connection-editior. Yes, that is a limitation in nm-connection-editor, which should probably be fixed. Which also means, it's non-obvious to the user that the range is configurable. Try for example: nmcli connection modify $CONNECTION_ID +ipv4.addresses 192.168.5.1/24 where you can find $CONNECTION_ID via `nmcli connection show` Afterwards, verify with `nmcli connection show $CONNECTION_ID` > May be I can do with 'ip address'. But I wish this to > be working right out of the box. No, that wouldn't work. > > For example, 192.168.5.1/24 will cause the shared connection to use > > 192.168.5.0/24 instead. > > > > > > > > Considering the address ranges that are configured on other devices might > > not be a good idea. For example, if you first activate your shared > > connection and the conflicting 10.42.0.0/24 device afterwards, you also get > > a conflict. Should NM then reconfigure the shared connection? If you have > > such conflicts, you need some manual configuration to avoid them. That is > > already possible. > > I think I should say it in order. > > 1. I connected to a wireless hotspot with my laptop. Say, I got the IP > 10.42.0.23/24 for wlan0 as a DHCP client. > 2. Then, on the same laptop I created a shared connection afterwards. NM > assigned 10.42.0.1 to my eth0 as DHCP server, which is one the same network. > > As NM is already able to get the current IPs, it should have assigned eth0 > an IP from a different network, say 10.42.1.1/24. This would not nicely work if you did 2.) before 1.). Also, the 10.42.0.23 address can show up any time after activation (say, your DHCP server is down for a few minutes). What if you get a conflicting 10.42.0.23/16 address? Should NM then fail activation of eth0 altogether? I think, NM should not try to be too smart here. If the default doesn't suit you, you already can configure it (we should improve documentation and UI support and discoverability though).
(In reply to Thomas Haller from comment #3) > (In reply to Mohammed Sadiq from comment #2) > > (In reply to Thomas Haller from comment #1) > > > Try to configure one manual IP address for the shared connection, that IP > > > address is then used to determine the range for the shared network. > > > > > > > I couldn't find a way to add manual IP for the shared connection in > > nm-connection-editior. > > Yes, that is a limitation in nm-connection-editor, which should probably be > fixed. Which also means, it's non-obvious to the user that the range is > configurable. > > > Try for example: > > nmcli connection modify $CONNECTION_ID +ipv4.addresses 192.168.5.1/24 > > where you can find $CONNECTION_ID via `nmcli connection show` > Afterwards, verify with `nmcli connection show $CONNECTION_ID` > Thanks. This will help me save my time later. > > > > May be I can do with 'ip address'. But I wish this to > > be working right out of the box. > > No, that wouldn't work. > You are right. I tried. Though I can set the IP, it gets reset to the default ones. > > > > For example, 192.168.5.1/24 will cause the shared connection to use > > > 192.168.5.0/24 instead. > > > > > > > > > > > > Considering the address ranges that are configured on other devices might > > > not be a good idea. For example, if you first activate your shared > > > connection and the conflicting 10.42.0.0/24 device afterwards, you also get > > > a conflict. Should NM then reconfigure the shared connection? If you have > > > such conflicts, you need some manual configuration to avoid them. That is > > > already possible. > > > > I think I should say it in order. > > > > 1. I connected to a wireless hotspot with my laptop. Say, I got the IP > > 10.42.0.23/24 for wlan0 as a DHCP client. > > 2. Then, on the same laptop I created a shared connection afterwards. NM > > assigned 10.42.0.1 to my eth0 as DHCP server, which is one the same network. > > > > As NM is already able to get the current IPs, it should have assigned eth0 > > an IP from a different network, say 10.42.1.1/24. > > This would not nicely work if you did 2.) before 1.). I did 1. first. And then 2. I don't actually know what's happening. > Also, the 10.42.0.23 address can show up any time after activation (say, > your DHCP server is down for a few minutes). > What if you get a conflicting 10.42.0.23/16 address? Should NM then fail > activation of eth0 altogether? > > I think, NM should not try to be too smart here. If the default doesn't suit > you, you already can configure it (we should improve documentation and UI > support and discoverability though). I was just talking about the default settings, usually in GNOME, when a hotspot is configured it sets /24 network. Anyway, I gave up that work. :-)
Created attachment 335065 [details] [review] [PATCH] editor: allow specifying IP address for shared method (In reply to Mohammed Sadiq from comment #2) > I couldn't find a way to add manual IP for the shared connection in > nm-connection-editior. May be I can do with 'ip address'. But I wish this to > be working right out of the box. Patch for nm-connection-editor to allow this.
If you add 2 IP addresses in manual mode and switch to shared, they stay there. Maybe, the "Save" button should be disabled in this case with message: "shared method can only have one address"... But then, nm-setting-ip4-config.c:verify() also allows multiple IP addresses for shared method... No strong opinion about what is correct here (or in the verify() function). Patch lgtm.
Created attachment 335157 [details] [review] [PATCH v2] editor: allow specifying IP address for shared method (In reply to Thomas Haller from comment #6) > If you add 2 IP addresses in manual mode and switch to shared, they stay > there. > > Maybe, the "Save" button should be disabled in this case with message: > "shared method can only have one address"... > > But then, nm-setting-ip4-config.c:verify() also allows multiple IP addresses > for shared method... > > No strong opinion about what is correct here (or in the verify() function). I think the best approach is to leave only the first address when switching from manual to shared (addresses are already lost when switching between methods and this shouldn't be a problem). While NMSettingIP4Config allows multiple IPs for shared method, at least the ifcfg-rh plugin can only read one, so we should enforce this constraint.
(In reply to Beniamino Galvani from comment #7) > Created attachment 335157 [details] [review] [review] > I think the best approach is to leave only the first address when switching > from manual to shared (addresses are already lost when switching between > methods and this shouldn't be a problem). IMO this "addresses are already lost when switching between methods" is inconvenient UI behavior. It would be nice, that when switching to "disabled" the addresses get seemingly removed, but when switching back, they should re-appear. Of course, your patch doesn't change that and sticks with the current behavior. So, the patch is good now, but maybe something we could improve(??). While at it, it is a constant source of confusion that the UI doesn't allow users to configure manual addresses with DHCP. Could you tackle that too? Seems like one small step. > While NMSettingIP4Config allows multiple IPs for shared method, at least the > ifcfg-rh plugin can only read one, so we should enforce this constraint. I think this is a bug. Either verify() must reject such connections, or ifcfg-rh must be able to store&reload them without missing some parts. Maybe verify() should just be tight up to disallow more then one addresses (NORMALIZABLE_ERROR). Or maybe it does make sense to configure multiple addresses on that interface, what do you think?
(In reply to Thomas Haller from comment #8) > (In reply to Beniamino Galvani from comment #7) > > Created attachment 335157 [details] [review] [review] [review] > > > I think the best approach is to leave only the first address when switching > > from manual to shared (addresses are already lost when switching between > > methods and this shouldn't be a problem). > > > IMO this "addresses are already lost when switching between methods" is > inconvenient UI behavior. It would be nice, that when switching to > "disabled" the addresses get seemingly removed, but when switching back, > they should re-appear. Of course, your patch doesn't change that and sticks > with the current behavior. So, the patch is good now, but maybe something we > could improve(??). Ok, fixed. > While at it, it is a constant source of confusion that the UI doesn't allow > users to configure manual addresses with DHCP. Could you tackle that too? > Seems like one small step. How about nm-applet branch bg/editor-ipv4-addresses-bgo763937 ? > > While NMSettingIP4Config allows multiple IPs for shared method, at least the > > ifcfg-rh plugin can only read one, so we should enforce this constraint. > > I think this is a bug. Either verify() must reject such connections, or > ifcfg-rh must be able to store&reload them without missing some parts. Maybe > verify() should just be tight up to disallow more then one addresses > (NORMALIZABLE_ERROR). Or maybe it does make sense to configure multiple > addresses on that interface, what do you think? At the moment the keyfile plugin allows writing and reading multiple addresses for the shared method, but the daemon only considers the first one. In the future we might support more than one address (only the first one will determine the DHCP pool while others will be simply added to the interface). For now I would only modify the ifcfg-rh plugin to avoid removing addresses in the reader.
(In reply to Beniamino Galvani from comment #9) > (In reply to Thomas Haller from comment #8) > > (In reply to Beniamino Galvani from comment #7) > > > Created attachment 335157 [details] [review] [review] [review] [review] > > > > > I think the best approach is to leave only the first address when switching > > > from manual to shared (addresses are already lost when switching between > > > methods and this shouldn't be a problem). > > > > > > IMO this "addresses are already lost when switching between methods" is > > inconvenient UI behavior. It would be nice, that when switching to > > "disabled" the addresses get seemingly removed, but when switching back, > > they should re-appear. Of course, your patch doesn't change that and sticks > > with the current behavior. So, the patch is good now, but maybe something we > > could improve(??). > > Ok, fixed. That's nice. Only issue, if you have 2 addresses and switch to "Shared" the 2nd address is lost. But fixing that is a bit more complicated, so it's fine as-is. the rest lgtm. > At the moment the keyfile plugin allows writing and reading multiple > addresses for the shared method, but the daemon only considers the first > one. In the future we might support more than one address (only the first > one will determine the DHCP pool while others will be simply added to the > interface). > > For now I would only modify the ifcfg-rh plugin to avoid removing addresses > in the reader. Having verify() fail with NORMALIZABLE_ERROR, would not prevent relaxing this in the future. Anyway, fixing the reader is fine too.
Created attachment 335393 [details] [review] [PATCH] libnm-core: drop extra IPs from shared connections during normalization (In reply to Thomas Haller from comment #10) > > Having verify() fail with NORMALIZABLE_ERROR, would not prevent relaxing > this in the future. Anyway, fixing the reader is fine too. I've opted to drop the extra addresses during normalization, see attached patch.
lgtm
Merged: https://git.gnome.org/browse/network-manager-applet/commit/?id=2b598b5904a7d9fc2b18aef86da278680148213c https://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=eaad7ae4312568bf7345ecdea300b98adfa44644
*** Bug 734021 has been marked as a duplicate of this bug. ***
*** Bug 777313 has been marked as a duplicate of this bug. ***