[netcf-devel] [PATCH] Allow IPv4 static addresses simultaneous with DHCP

Laine Stump

2015-03-30 21:07:16 UTC

Because initscripts network startup ends up ignoring static IPv4
addresses when dhcp is enabled (BOOTPROTO=dhcp in the ifcfg file) and
because at least some of the frontends to Network Manager disable
configuring a static address if dhcp is enabled (and likely due to
other indications, but this all happened far in the past...) netcf has
always made DHCP and static IP addresses for IPv4 mutually exclusive
in the config - an interface either had one or it had the other.

A recent BZ filed against libvirt:

https://bugzilla.redhat.com/show_bug.cgi?id=1113983

suggested that we should also forbid simultaneous DHCP and static IPs
for IPv6, but investigation shows the following:

1) there is nothing inherent in the DHCP protocol that precludes
having both a single DHCP address and one or more static IP addresses
from each protocol (as well as an IPv6 address learned via IPv6
autoconf). Even the lead NetworkManager developer says this in an
email response to a question posed on the dnsmasq mailing list:

http://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2015q1/009346.html

2) Different networking backends currently properly support different
combinations of these address types for the two platforms (for
example, NetworkManager 0.9.10.2 on Fedora 21 will properly acquire a
DHCPv4 address to live alongside multiple static IPv4 addresses while
initscripts 9.56.7 will discard the static IPs, and initscripts will
support multiple static IPv6 addresses together with an autoconf
address, while NetworkManager apparently won't (although this could be
a configuration error on my part).

In the end, any time that all three address types for IPv6 (autoconf,
dhcp, static) or two address types for IPv4 (dhcp, static) cannot work
simultaneously on the same interface, that is a bug in the networking
backend (e.g. initscripts, NetworkManager), *not* a reason to disallow
such configurations via netcf.

For that reason, this patch opens up the RNG for the IPv4 protocol,
allowing <dhcp/> to be specified simultaneously with any number of
static <ip> elements for IPv4, just as we already did for IPv6. Any
future failures of such configurations to work properly should result
in bugs filed against the network backend in use at the time (or
possibly the dhcp server, or some other piece of infrastructure).
---
data/xml/interface.rng | 28 ++++++-------
data/xml/redhat-get.xsl | 48 +++++++++++-----------
data/xml/redhat-put.xsl | 14 +++----
tests/interface/bridge-multi-all.xml | 23 +++++++++++
.../fsroot/etc/sysconfig/network-scripts/ifcfg-br2 | 25 +++++++++++
tests/redhat/schema/bridge-multi-all.xml | 30 ++++++++++++++
tests/test-redhat.c | 3 +-
7 files changed, 123 insertions(+), 48 deletions(-)
create mode 100644 tests/interface/bridge-multi-all.xml
create mode 100644 tests/redhat/fsroot/etc/sysconfig/network-scripts/ifcfg-br2
create mode 100644 tests/redhat/schema/bridge-multi-all.xml

diff --git a/data/xml/interface.rng b/data/xml/interface.rng
index a794084..8d70e8b 100644
--- a/data/xml/interface.rng
+++ b/data/xml/interface.rng
@@ -322,24 +322,22 @@
<value>ipv4</value>
</attribute>
<interleave>
- <choice>
+ <optional>
<ref name="dhcp-element"/>
- <group>
- <oneOrMore>
- <element name="ip">
- <attribute name="address"><ref name="ipv4-addr"/></attribute>
- <optional>
- <attribute name="prefix"><ref name="ipv4-prefix"/></attribute>
- </optional>
- </element>
- </oneOrMore>
+ </optional>
+ <zeroOrMore>
+ <element name="ip">
+ <attribute name="address"><ref name="ipv4-addr"/></attribute>
<optional>
- <element name="route">
- <attribute name="gateway"><ref name="ipv4-addr"/></attribute>
- </element>
+ <attribute name="prefix"><ref name="ipv4-prefix"/></attribute>
</optional>
- </group>
- </choice>
+ </element>
+ </zeroOrMore>
+ <optional>
+ <element name="route">
+ <attribute name="gateway"><ref name="ipv4-addr"/></attribute>
+ </element>
+ </optional>
</interleave>
</element>
</define>
diff --git a/data/xml/redhat-get.xsl b/data/xml/redhat-get.xsl
index 96518ee..ec56f01 100644
--- a/data/xml/redhat-get.xsl
+++ b/data/xml/redhat-get.xsl
@@ -183,32 +183,32 @@
</xsl:template>

<xsl:template name="protocol-ipv4">
- <xsl:choose>
- <xsl:when test="dhcp">
- <node label="BOOTPROTO" value="dhcp"/>
- <xsl:if test="dhcp/@peerdns">
- <node label="PEERDNS" value="{dhcp/@peerdns}"/>
+ <xsl:if test="count(dhcp) > 0">
+ <node label="BOOTPROTO" value="dhcp"/>
+ <xsl:if test="dhcp/@peerdns">
+ <node label="PEERDNS" value="{dhcp/@peerdns}"/>
+ </xsl:if>
+ </xsl:if>
+ <xsl:if test="count(dhcp) = 0">
+ <node label="BOOTPROTO" value="none"/>
+ </xsl:if>
+ <xsl:if test="count(ip) > 0">
+ <xsl:for-each select="ip">
+ <xsl:if test="position() = 1">
+ <xsl:call-template name="ipv4-address">
+ <xsl:with-param name="index"/>
+ </xsl:call-template>
</xsl:if>
- </xsl:when>
- <xsl:when test="ip">
- <node label="BOOTPROTO" value="none"/>
- <xsl:for-each select="ip">
- <xsl:if test="position() = 1">
- <xsl:call-template name="ipv4-address">
- <xsl:with-param name="index"/>
- </xsl:call-template>
- </xsl:if>
- <xsl:if test="position() > 1 and position() < 101">
- <xsl:call-template name="ipv4-address">
- <xsl:with-param name="index" select="position() - 1"/>
- </xsl:call-template>
- </xsl:if>
- </xsl:for-each>
- <xsl:if test="route">
- <node label="GATEWAY" value="{route/@gateway}"/>
+ <xsl:if test="position() > 1 and position() < 101">
+ <xsl:call-template name="ipv4-address">
+ <xsl:with-param name="index" select="position() - 1"/>
+ </xsl:call-template>
</xsl:if>
- </xsl:when>
- </xsl:choose>
+ </xsl:for-each>
+ <xsl:if test="route">
+ <node label="GATEWAY" value="{route/@gateway}"/>
+ </xsl:if>
+ </xsl:if>
</xsl:template>

<xsl:template name="ipv4-address">
diff --git a/data/xml/redhat-put.xsl b/data/xml/redhat-put.xsl
index a89cc2d..ed56c66 100644
--- a/data/xml/redhat-put.xsl
+++ b/data/xml/redhat-put.xsl
@@ -186,16 +186,15 @@
select="count(node[substring(@label,1,6) = 'IPADDR']) > 0"/>
<xsl:variable name="uses_ipv4" select="$uses_dhcp or $uses_static"/>
<xsl:if test="$uses_ipv4">
- <protocol family="ipv4">
- <xsl:choose>
- <xsl:when test="$uses_dhcp">
+ <protocol family="ipv4">
+ <xsl:if test="$uses_dhcp">
<dhcp>
<xsl:if test="node[@label = 'PEERDNS']">
<xsl:attribute name="peerdns"><xsl:value-of select="node[@label = 'PEERDNS']/@value"></xsl:value-of></xsl:attribute>
</xsl:if>
</dhcp>
- </xsl:when>
- <xsl:when test="$uses_static">
+ </xsl:if>
+ <xsl:if test="$uses_static">

<xsl:choose>
<xsl:when test="node[@label = 'IPADDR']">
@@ -236,9 +235,8 @@
</ip>
</xsl:if>
</xsl:for-each>
- </xsl:when>
- </xsl:choose>
- </protocol>
+ </xsl:if>
+ </protocol>
</xsl:if>
</xsl:template>

diff --git a/tests/interface/bridge-multi-all.xml b/tests/interface/bridge-multi-all.xml
new file mode 100644
index 0000000..0cdf484
--- /dev/null
+++ b/tests/interface/bridge-multi-all.xml
@@ -0,0 +1,23 @@
+<interface type="bridge" name="br2">
+ <start mode="onboot"/>
+ <protocol family="ipv4">
+ <dhcp peerdns="yes"/>
+ <ip address="10.0.0.1" prefix="24"/>
+ <ip address="192.168.125.4" prefix="24"/>
+ <ip address="192.168.125.5" prefix="31"/>
+ <ip address="192.168.125.6"/>
+ <ip address="192.168.125.7"/>
+ <ip address="192.168.125.8"/>
+ <ip address="192.168.125.9"/>
+ <ip address="192.168.125.10"/>
+ <ip address="172.16.20.32" prefix="12"/>
+ </protocol>
+ <protocol family="ipv6">
+ <autoconf/>
+ <dhcp/>
+ <ip address="3ffe:ffff:0:5::1" prefix="128"/>
+ <ip address="3ffe:ffff:0:5::3" prefix="128"/>
+ <ip address="3ffe:ffff:0:5::5" prefix="128"/>
+ </protocol>
+ <bridge stp="on" delay="0"/>
+</interface>
diff --git a/tests/redhat/fsroot/etc/sysconfig/network-scripts/ifcfg-br2 b/tests/redhat/fsroot/etc/sysconfig/network-scripts/ifcfg-br2
new file mode 100644
index 0000000..3d45d9e
--- /dev/null
+++ b/tests/redhat/fsroot/etc/sysconfig/network-scripts/ifcfg-br2
@@ -0,0 +1,25 @@
+DEVICE="br2"
+ONBOOT='yes'
+TYPE="Bridge"
+BOOTPROTO="dhcp"
+PEERDNS="yes"
+IPADDR="10.0.0.1"
+NETMASK=255.255.255.0
+IPADDR1="192.168.125.4"
+NETMASK1="255.255.255.0"
+IPADDR2="192.168.125.5"
+NETMASK2="255.255.255.254"
+IPADDR3="192.168.125.6"
+IPADDR4="192.168.125.7"
+IPADDR5="192.168.125.8"
+IPADDR6="192.168.125.9"
+IPADDR7="192.168.125.10"
+IPADDR8="172.16.20.32"
+NETMASK8="255.240.0.0"
+IPV6INIT="yes"
+IPV6_AUTOCONF="yes"
+DHCPV6C="yes"
+IPV6ADDR="3ffe:ffff:0:5::1/128"
+IPV6ADDR_SECONDARIES="3ffe:ffff:0:5::3/128 3ffe:ffff:0:5::5/128"
+STP=on
+DELAY=0
diff --git a/tests/redhat/schema/bridge-multi-all.xml b/tests/redhat/schema/bridge-multi-all.xml
new file mode 100644
index 0000000..4b67a5c
--- /dev/null
+++ b/tests/redhat/schema/bridge-multi-all.xml
@@ -0,0 +1,30 @@
+<?xml version="1.0"?>
+<forest>
+ <tree path="/files/etc/sysconfig/network-scripts/ifcfg-br2">
+ <node label="DEVICE" value="br2"/>
+ <node label="ONBOOT" value="yes"/>
+ <node label="TYPE" value="Bridge"/>
+ <node label="BOOTPROTO" value="dhcp"/>
+ <node label="PEERDNS" value="yes"/>
+ <node label="IPADDR" value="10.0.0.1"/>
+ <node label="NETMASK" value="255.255.255.0"/>
+ <node label="IPADDR1" value="192.168.125.4"/>
+ <node label="NETMASK1" value="255.255.255.0"/>
+ <node label="IPADDR2" value="192.168.125.5"/>
+ <node label="NETMASK2" value="255.255.255.254"/>
+ <node label="IPADDR3" value="192.168.125.6"/>
+ <node label="IPADDR4" value="192.168.125.7"/>
+ <node label="IPADDR5" value="192.168.125.8"/>
+ <node label="IPADDR6" value="192.168.125.9"/>
+ <node label="IPADDR7" value="192.168.125.10"/>
+ <node label="IPADDR8" value="172.16.20.32"/>
+ <node label="NETMASK8" value="255.240.0.0"/>
+ <node label="IPV6INIT" value="yes"/>
+ <node label="IPV6_AUTOCONF" value="yes"/>
+ <node label="DHCPV6C" value="yes"/>
+ <node label="IPV6ADDR" value="3ffe:ffff:0:5::1/128"/>
+ <node label="IPV6ADDR_SECONDARIES" value="3ffe:ffff:0:5::3/128 3ffe:ffff:0:5::5/128"/>
+ <node label="STP" value="on"/>
+ <node label="DELAY" value="0"/>
+ </tree>
+</forest>
diff --git a/tests/test-redhat.c b/tests/test-redhat.c
index a08bc23..404f375 100644
--- a/tests/test-redhat.c
+++ b/tests/test-redhat.c
@@ -43,7 +43,7 @@ static void testListInterfaces(CuTest *tc) {
int nint;
char **names;
static const char *const exp_names[] =
- { "br0", "br1", "bond0", "lo", "eth3", "eth4" };
+ { "br0", "br1", "br2", "bond0", "lo", "eth3", "eth4" };
static const int exp_nint = ARRAY_CARDINALITY(exp_names);

nint = ncf_num_of_interfaces(ncf, NETCF_IFACE_ACTIVE|NETCF_IFACE_INACTIVE);
@@ -181,6 +181,7 @@ static void testTransforms(CuTest *tc) {
assert_transforms(tc, "bridge-empty");
assert_transforms(tc, "bridge-bond");
assert_transforms(tc, "bridge-multi");
+ assert_transforms(tc, "bridge-multi-all");
assert_transforms(tc, "ethernet-static");
assert_transforms(tc, "ethernet-static-no-prefix");
assert_transforms(tc, "ethernet-dhcp");

--
2.1.0