Bug 1445414 - ifcfg: try to stay compatible with pykickstart
Summary: ifcfg: try to stay compatible with pykickstart
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 7
Classification: Red Hat
Component: NetworkManager
Version: 7.4
Hardware: Unspecified
OS: Unspecified
medium
medium
Target Milestone: rc
: ---
Assignee: Thomas Haller
QA Contact: Desktop QE
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2017-04-25 15:42 UTC by Lubomir Rintel
Modified: 2017-08-01 09:27 UTC (History)
9 users (show)

Fixed In Version: NetworkManager-1.8.0-5.el7
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2017-08-01 09:27:08 UTC
Target Upstream Version:


Attachments (Terms of Use)
Proposed patch (5.89 KB, text/plain)
2017-04-26 18:48 UTC, Lubomir Rintel
no flags Details
[patch] ifcfg-rh: treat a wrongly quoted value like empty string (2.00 KB, patch)
2017-04-26 19:56 UTC, Thomas Haller
no flags Details | Diff


Links
System ID Priority Status Summary Last Updated
Red Hat Product Errata RHSA-2017:2299 normal SHIPPED_LIVE Moderate: NetworkManager and libnl3 security, bug fix and enhancement update 2017-08-01 12:40:28 UTC

Description Lubomir Rintel 2017-04-25 15:42:12 UTC
From https://access.redhat.com/solutions/2868101:

    RHEL7.2 populated /etc/sysconfig/network-scripts/ifcfg-* with PREFIX and several IPv6 variables:

Raw

TYPE="Ethernet"
BOOTPROTO="none"
IPADDR="192.0.2.2"
PREFIX="24"
GATEWAY="192.0.2.1"
DNS1="192.0.2.1"
DEFROUTE="yes"
IPV4_FAILURE_FATAL="no"
IPV6INIT="yes"
IPV6_AUTOCONF="yes"
IPV6_DEFROUTE="yes"
IPV6_PEERDNS="yes"
IPV6_PEERROUTES="yes"
IPV6_FAILURE_FATAL="no"
IPV6_ADDR_GEN_MODE="stable-privacy"
NAME="eth0"
UUID="00b141e5-cbda-4624-9cf4-ec21ebbf91fe"
DEVICE="eth0"
ONBOOT="yes"

    A kickstarted RHEL 7.3 system has a much leaner ifcfg configuration:

Raw

# Generated by parse-kickstart
UUID="8b4753fb-c562-4784-bfa7-f44dc6581e73"
DNS1="192.0.2.1"
IPADDR="102.0.2.2"
GATEWAY="192.0.2.1"
NETMASK="255.255.255.0"
BOOTPROTO="static"
DEVICE="eth0"
ONBOOT="yes"
IPV6INIT="yes"


Essentially, we need to stay compatible with the later form.

Comment 2 Lubomir Rintel 2017-04-26 18:48:59 UTC
Created attachment 1274362 [details]
Proposed patch

Comment 3 Thomas Haller 2017-04-26 19:56:20 UTC
Created attachment 1274379 [details]
[patch] ifcfg-rh: treat a wrongly quoted value like empty string

src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c:2107:7: error: variable 'tmp' is used uninitialized whenever '||' condition is true [-Werror,-Wsometimes-uninitialized]
                if (!num || g_strcmp0 (svGetValue (ifcfg, "BOOTPROTO", &tmp), "static"))





+         tmp = g_strdup_printf ("%u", prefix);
          svSetValueStr (ifcfg, prefix_key, tmp);
          g_free (tmp);

svSetValueInt64()?


+         tmp = svGetValueStr_cp (ifcfg, netmask_key);
          svUnsetValue (ifcfg, netmask_key);
+         if (tmp) {

the "_cp" stands for copy. Using it is IMO dicouraged, because you can avoid the copy in many cases.

Also, svGetValueStr() swallows empty string (often useful where a shell script might do `test -n`). It seems in this case, the svGetValue() would be more appropriate.


  if (inet_ntop (AF_INET, &prefix, buf, INET_ADDRSTRLEN))

inet_ntop() cannot fail (unless you provide an invalid family, which you do not). Also, more often we use nm_utils_inet4_ntop().





Together with patch 0001-ifcfg-rh-treat-a-wrongly-quoted-value-like-empty-str.patch, it would be:

  if (svGetValue (ifcfg, netmask_key, &tmp) {
      char buf[INET_ADDRSTRLEN];

      g_free (tmp);
      svSetValueStr (ifcfg, netmask_key,
                     nm_utils_inet4_ntop(prefix, buf));
  }

Comment 6 Vladimir Benes 2017-05-24 13:21:14 UTC
This doesn't look good. NETMASK was chnaged to PREFIX. I think this shouldn't happen.

    [root@qe-dell-ovs5-vm-6 NetworkManager]# cat /etc/sysconfig/network-scripts/ifcfg-eth1
    # Generated by parse-kickstart
    UUID="8b4753fb-c562-4784-bfa7-f44dc6581e73"
    DNS1="192.0.2.1"
    IPADDR="102.0.2.2"
    GATEWAY="192.0.2.1"
    NETMASK="255.255.255.0"
    BOOTPROTO="static"
    DEVICE="eth1"
    ONBOOT="yes"
    IPV6INIT="yes"
    [root@qe-dell-ovs5-vm-6 NetworkManager]# nmcli connection modify "System eth1" ipv4.addresses 1.2.3.4/24 ipv4.gateway 1.2.3.1
    [root@qe-dell-ovs5-vm-6 NetworkManager]# nmcli connection show "System eth1"
    connection.id:                          System eth1
    connection.uuid:                        8b4753fb-c562-4784-bfa7-f44dc6581e73
    connection.stable-id:                   --
    connection.interface-name:              eth1
    connection.type:                        802-3-ethernet
    connection.autoconnect:                 yes
    connection.autoconnect-priority:        0
    connection.autoconnect-retries:         -1 (default)
    connection.timestamp:                   1495630993
    connection.read-only:                   no
    connection.permissions:                 --
    connection.zone:                        --
    connection.master:                      --
    connection.slave-type:                  --
    connection.autoconnect-slaves:          -1 (default)
    connection.secondaries:                 --
    connection.gateway-ping-timeout:        0
    connection.metered:                     unknown
    connection.lldp:                        -1 (default)
    802-3-ethernet.port:                    --
    802-3-ethernet.speed:                   0
    802-3-ethernet.duplex:                  --
    802-3-ethernet.auto-negotiate:          no
    802-3-ethernet.mac-address:             --
    802-3-ethernet.cloned-mac-address:      --
    802-3-ethernet.generate-mac-address-mask:--
    802-3-ethernet.mac-address-blacklist:   --
    802-3-ethernet.mtu:                     auto
    802-3-ethernet.s390-subchannels:        --
    802-3-ethernet.s390-nettype:            --
    802-3-ethernet.s390-options:            --
    802-3-ethernet.wake-on-lan:             1 (default)
    802-3-ethernet.wake-on-lan-password:    --
    ipv4.method:                            manual
    ipv4.dns:                               192.0.2.1
    ipv4.dns-search:                        --
    ipv4.dns-options:                       (default)
    ipv4.dns-priority:                      0
    ipv4.addresses:                         1.2.3.4/24
    ipv4.gateway:                           1.2.3.1
    ipv4.routes:                            { ip = 1.2.3.1/32 }
    ipv4.route-metric:                      -1
    ipv4.ignore-auto-routes:                no
    ipv4.ignore-auto-dns:                   no
    ipv4.dhcp-client-id:                    --
    ipv4.dhcp-timeout:                      0
    ipv4.dhcp-send-hostname:                yes
    ipv4.dhcp-hostname:                     --
    ipv4.dhcp-fqdn:                         --
    ipv4.never-default:                     no
    ipv4.may-fail:                          yes
    ipv4.dad-timeout:                       -1 (default)
    ipv6.method:                            auto
    ipv6.dns:                               --
    ipv6.dns-search:                        --
    ipv6.dns-options:                       (default)
    ipv6.dns-priority:                      0
    ipv6.addresses:                         --
    ipv6.gateway:                           --
    ipv6.routes:                            --
    ipv6.route-metric:                      -1
    ipv6.ignore-auto-routes:                no
    ipv6.ignore-auto-dns:                   no
    ipv6.never-default:                     no
    ipv6.may-fail:                          yes
    ipv6.ip6-privacy:                       -1 (unknown)
    ipv6.addr-gen-mode:                     eui64
    ipv6.dhcp-send-hostname:                yes
    ipv6.dhcp-hostname:                     --
    ipv6.token:                             --
    proxy.method:                           none
    proxy.browser-only:                     no
    proxy.pac-url:                          --
    proxy.pac-script:                       --
    [root@qe-dell-ovs5-vm-6 NetworkManager]# cat /etc/sysconfig/network-scripts/ifcfg-eth1
    # Generated by parse-kickstart
    UUID=8b4753fb-c562-4784-bfa7-f44dc6581e73
    DNS1=192.0.2.1
    IPADDR=1.2.3.4
    GATEWAY=1.2.3.1
    BOOTPROTO="static"
    DEVICE=eth1
    ONBOOT=yes
    IPV6INIT=yes
    TYPE=Ethernet
    PROXY_METHOD=none
    BROWSER_ONLY=no
    PREFIX=24
    DEFROUTE=yes
    IPV4_FAILURE_FATAL=no
    IPV6_AUTOCONF=yes
    IPV6_DEFROUTE=yes
    IPV6_FAILURE_FATAL=no
    NAME="System eth1"

Comment 7 Thomas Haller 2017-05-25 14:02:29 UTC
fix for master: th/ifcfg-netmask-legacy-rh1445414

(as the code changed there, the backport for nm-1-8 will need some adjustments).

Please review.

Comment 8 Beniamino Galvani 2017-05-30 09:34:17 UTC
(In reply to Thomas Haller from comment #7)
> fix for master: th/ifcfg-netmask-legacy-rh1445414

LGTM

Comment 9 Francesco Giudici 2017-05-30 10:47:31 UTC
Branch LGTM.

In the message of "ifcfg-rh: fix preserving NETMASK key in write_ip4_setting()" commit I would explicitly state that also if we preserve the legacy NETMASK key there we will add also the PREFIX key. That is not clear.

In the same commit message minor typo in the second paragraph:
"That was borken [...]"
"That was bROken [...]"

Comment 10 Thomas Haller 2017-05-30 12:29:10 UTC
fixed upstream:

master: https://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=70c0f7965d0d028364a5de05113974880e829706


the branch was partially backported to nm-1-8:
https://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=47ebca23e3403243a97c1d0936f023a7c9f1ac38


... and will be fixed in next rhel-7.4 package.

Comment 11 errata-xmlrpc 2017-08-01 09:27:08 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://access.redhat.com/errata/RHSA-2017:2299


Note You need to log in before you can comment on or make changes to this bug.