Bug 1174632

Summary:	shared connection losing it's address when NM service restarted
Product:	Red Hat Enterprise Linux 7	Reporter:	Vladimir Benes <vbenes>
Component:	NetworkManager	Assignee:	Jirka Klimes <jklimes>
Status:	CLOSED ERRATA	QA Contact:	Desktop QE <desktop-qa-list>
Severity:	unspecified	Docs Contact:
Priority:	unspecified
Version:	7.1	CC:	danw, dcbw, jklimes, ovasik, rkhan, thaller
Target Milestone:	rc
Target Release:	---
Hardware:	Unspecified
OS:	Unspecified
Whiteboard:
Fixed In Version:	NetworkManager-1.0.0-8.git20150107.1ea95cd3.el7	Doc Type:	Bug Fix
Doc Text:	When a connection profile with IPv4 method Shared and a static IP address was activated, NetworkManager did not respect the IP and used the default range of 10.42.0.1/24. This was fixed and NetworkManager now infers the IP range from the configured static address.	Story Points:	---
Clone Of:		Environment:
Last Closed:	2015-03-05 13:54:20 UTC	Type:	Bug
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:

Description Vladimir Benes 2014-12-16 07:56:02 UTC

Description of problem:
when shared connection with specified address range via ipv4.addresses field in nmcli exists and NM service restarts that address field is removed and thus dhcp operates with different addresses (10.42.0.1/24)

Version-Release number of selected component (if applicable):
NetworkManager-0.995.0.0-1.el7

How reproducible:
deterministic

Steps to Reproduce:
1.nmcli con add type ethernet ifname eth0 con-name dhcp
2.nmcli con mod dhcp ipv4.addresses 192.168.100.1/24 ipv4.method shared
3.nmcli con up id dhcp
4.systemctl restart NetworkManager
5.nmcli c s dhcp

Actual results:
10.42.0.1/24

Expected results:
192.168.100.1/24

Additional info:

Comment 1 Jirka Klimes 2014-12-16 12:47:44 UTC

A fix is available in NetworkManager upstream git branch jk/shared-ip-rh1174632.

Comment 2 Dan Winship 2014-12-16 13:41:55 UTC

>+is_any_ip4_address_defined (shvarFile *ifcfg, int *idx)

why do you need this rather than just assuming/requiring that it's index 0?

>+			if (!read_ip4_address (ifcfg, "GATEWAY", &gateway, error))
>+				goto done;

hm... NMSettingIP4Config::verify() doesn't forbid it, but gateway doesn't really make sense for SHARED does it? (Or do we allow having a SHARED interface also have the default route?)

Comment 3 Thomas Haller 2014-12-16 13:53:22 UTC

(In reply to Dan Winship from comment #2)
> >+is_any_ip4_address_defined (shvarFile *ifcfg, int *idx)
> 
> why do you need this rather than just assuming/requiring that it's index 0?
> 
> >+			if (!read_ip4_address (ifcfg, "GATEWAY", &gateway, error))
> >+				goto done;
> 
> hm... NMSettingIP4Config::verify() doesn't forbid it, but gateway doesn't
> really make sense for SHARED does it? (Or do we allow having a SHARED
> interface also have the default route?)

I don't know. But I agree, it it makes no sense, we should tighten verify() to reject it.




-    g_assert (connection);
-    success = nm_connection_verify (connection, &error);
-    g_assert_no_error (error);
-    g_assert (success);
+    nmtst_assert_connection_verifies_without_normalization (connection);



Rest LGTM

Comment 4 Dan Williams 2015-01-08 20:17:33 UTC

LGTM too, though I think the gateway should probably optional for the shared method since it doesn't really matter for sharing, since NAT handles routing packets instead of any default gateway.

Comment 5 Jirka Klimes 2015-01-12 15:57:54 UTC

(In reply to Dan Winship from comment #2)
> >+is_any_ip4_address_defined (shvarFile *ifcfg, int *idx)
> 
> why do you need this rather than just assuming/requiring that it's index 0?

The first address can also start at index 1 or 2.

> 
> >+			if (!read_ip4_address (ifcfg, "GATEWAY", &gateway, error))
> >+				goto done;
> 
The gateway *is* optional, because read_ip4_address() returns success if GATEWAY is missing.

(In reply to Thomas Haller from comment #3)
> (In reply to Dan Winship from comment #2)
> 
> -    g_assert (connection);
> -    success = nm_connection_verify (connection, &error);
> -    g_assert_no_error (error);
> -    g_assert (success);
> +    nmtst_assert_connection_verifies_without_normalization (connection);
> 

Fixed.

Comment 6 Jirka Klimes 2015-01-15 13:40:02 UTC

Commits in upstream master:
233498e ifcfg-rh: update test files not to use obsolete GATEWAY<n> variables
a3c52d7 man: add description of BOOTPROTO to nm-settings-ifcfg-rh manpage
c8fe3bb ifcfg-rh: read custom IP address for shared connections (rh #1174632)

Comment 10 errata-xmlrpc 2015-03-05 13:54:20 UTC

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://rhn.redhat.com/errata/RHBA-2015-0311.html