3:02 a.m.
The first patch is a trivial fix to the network schema
since the name in dhcp-host is really optional.
The second patch adds the capability to use the client id
to improve the reliability of having a system use a fixed
IPv6 address.
Gene Czarcinski (2):
Trivial fix: in dhcp-host the name is optional
use client id for IPv6 DHCP host definition
docs/formatnetwork.html.in | 18 ++++++-
docs/schemas/basictypes.rng | 59 ++++++++++++++++++++++
docs/schemas/network.rng | 7 ++-
src/conf/network_conf.c | 29 +++++++++--
src/conf/network_conf.h | 1 +
src/network/bridge_driver.c | 3 +-
src/util/virdnsmasq.c | 20 ++++++--
src/util/virdnsmasq.h | 1 +
tests/networkxml2confdata/dhcp6-nat-network.xml | 5 +-
tests/networkxml2confdata/dhcp6-network.xml | 5 +-
.../dhcp6host-routed-network.xml | 5 +-
11 files changed, 138 insertions(+), 15 deletions(-)
--
1.8.1.2
3:02 a.m.
New subject: [libvirt] [PATCH 1/2] Trivial fix: in dhcp-host the name is optional
Although in IPv4 one must pick either mac or name, either
can be omitted. Similarly, for IPv6, the name
can be optionally omitted.
Signed-off-by: Gene Czarcinski <gene(a)czarc.net>
---
docs/schemas/network.rng | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng
index 09d7c73..a479453 100644
--- a/docs/schemas/network.rng
+++ b/docs/schemas/network.rng
@@ -281,7 +281,9 @@
<optional>
<attribute name="mac"><ref
name="uniMacAddr"/></attribute>
</optional>
- <attribute
name="name"><text/></attribute>
+ <optional>
+ <attribute
name="name"><text/></attribute>
+ </optional>
<attribute name="ip"><ref
name="ipAddr"/></attribute>
</element>
</zeroOrMore>
--
1.8.1.2
11:32 p.m.
New subject: [libvirt] [PATCH 1/2] Trivial fix: in dhcp-host the name is optional
On 02/15/2013 02:02 PM, Gene Czarcinski wrote:
Although in IPv4 one must pick either mac or name, either
can be omitted. Similarly, for IPv6, the name
can be optionally omitted.
Signed-off-by: Gene Czarcinski <gene(a)czarc.net>
---
docs/schemas/network.rng | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng
index 09d7c73..a479453 100644
--- a/docs/schemas/network.rng
+++ b/docs/schemas/network.rng
@@ -281,7 +281,9 @@
<optional>
<attribute name="mac"><ref
name="uniMacAddr"/></attribute>
</optional>
- <attribute
name="name"><text/></attribute>
+ <optional>
+ <attribute
name="name"><text/></attribute>
+ </optional>
<attribute name="ip"><ref
name="ipAddr"/></attribute>
</element>
</zeroOrMore>
ACK.
5:39 a.m.
New subject: [libvirt] [PATCH 1/2] Trivial fix: in dhcp-host the name is optional
On 02/15/2013 12:02 PM, Gene Czarcinski wrote:
Although in IPv4 one must pick either mac or name, either
can be omitted. Similarly, for IPv6, the name
can be optionally omitted.
Signed-off-by: Gene Czarcinski <gene(a)czarc.net>
---
docs/schemas/network.rng | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng
index 09d7c73..a479453 100644
--- a/docs/schemas/network.rng
+++ b/docs/schemas/network.rng
@@ -281,7 +281,9 @@
<optional>
<attribute name="mac"><ref
name="uniMacAddr"/></attribute>
</optional>
- <attribute
name="name"><text/></attribute>
+ <optional>
+ <attribute
name="name"><text/></attribute>
+ </optional>
Hmm. This says that I can omit both name and mac, and still validate.
Better would be this RelaxNG construct, which says that I must supply at
least one, but can also supply both; the difference is that failure to
supply either will cause a (desired) validation failure.
<choice>
<group>
<attribute name="mac"><ref
name="uniMacAddr"/></attribute>
<optional>
<attribute name="name"><text/></attribute>
</optional>
</group>
<attribute name="name"><text/></attribute>
</choice>
I know Laine already ack'd but since we haven't pushed yet, I'm
wondering if it is worth tightening the grammar.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
2:39 p.m.
New subject: [libvirt] [PATCH 1/2] Trivial fix: in dhcp-host the name is optional
On 02/19/2013 04:39 PM, Eric Blake wrote:
On 02/15/2013 12:02 PM, Gene Czarcinski wrote:
> Although in IPv4 one must pick either mac or name, either
> can be omitted. Similarly, for IPv6, the name
> can be optionally omitted.
> Signed-off-by: Gene Czarcinski <gene(a)czarc.net>
> ---
> docs/schemas/network.rng | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng
> index 09d7c73..a479453 100644
> --- a/docs/schemas/network.rng
> +++ b/docs/schemas/network.rng
> @@ -281,7 +281,9 @@
> <optional>
> <attribute name="mac"><ref
name="uniMacAddr"/></attribute>
> </optional>
> - <attribute
name="name"><text/></attribute>
> + <optional>
> + <attribute
name="name"><text/></attribute>
> + </optional>
Hmm. This says that I can omit both name and mac, and still validate.
Better would be this RelaxNG construct, which says that I must supply at
least one, but can also supply both; the difference is that failure to
supply either will cause a (desired) validation failure.
<choice>
<group>
<attribute name="mac"><ref
name="uniMacAddr"/></attribute>
<optional>
<attribute name="name"><text/></attribute>
</optional>
</group>
<attribute name="name"><text/></attribute>
</choice>
I know Laine already ack'd but since we haven't pushed yet, I'm
wondering if it is worth tightening the grammar.
Looks good to me. I'm pushing it with that change.
10:01 p.m.
New subject: [libvirt] [PATCH 1/2] Trivial fix: in dhcp-host the name is optional
On 02/19/2013 04:39 PM, Eric Blake wrote:
On 02/15/2013 12:02 PM, Gene Czarcinski wrote:
> Although in IPv4 one must pick either mac or name, either
> can be omitted. Similarly, for IPv6, the name
> can be optionally omitted.
> Signed-off-by: Gene Czarcinski <gene(a)czarc.net>
> ---
> docs/schemas/network.rng | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng
> index 09d7c73..a479453 100644
> --- a/docs/schemas/network.rng
> +++ b/docs/schemas/network.rng
> @@ -281,7 +281,9 @@
> <optional>
> <attribute name="mac"><ref
name="uniMacAddr"/></attribute>
> </optional>
> - <attribute
name="name"><text/></attribute>
> + <optional>
> + <attribute
name="name"><text/></attribute>
> + </optional>
Hmm. This says that I can omit both name and mac, and still validate.
Better would be this RelaxNG construct, which says that I must supply at
least one, but can also supply both; the difference is that failure to
supply either will cause a (desired) validation failure.
<choice>
<group>
<attribute name="mac"><ref
name="uniMacAddr"/></attribute>
<optional>
<attribute name="name"><text/></attribute>
</optional>
</group>
<attribute name="name"><text/></attribute>
</choice>
I know Laine already ack'd but since we haven't pushed yet, I'm
wondering if it is worth tightening the grammar.
First of all, I like your suggestion since it is closer to reality.
However, when I posed the question here, this was my answer:
On 02/18/2013 09:12 AM, Daniel P. Berrange wrote:
The schemas are intended to provide a simple "syntax check"
for XML
documents that an app has created. eg detect typos in element/attribute
names, or use of elements which don't use or incorrect nesting of
elements. Even for syntax checks though, the schemas a not going to
provide 100% coverage - todo so would make the schemas far more complicated
and it is just not worth the effort. The XML parser is the ultimate
place where 100% complete syntax checks are performed.
What they generally do not do, is provide any kind of "semantic check"
about whether the configuration actually makes sense. For example, the
schema won't tell you that requesting virtio disks with a vmware guest
is semantically invalid. This is left to the virt driver.
So, my interpretation is
that the schema should not call false negatives
but getting all the positives correct is not necessary.
This whole thing with dhcp-host is further complicated by IPv6. That is,
for IPv4 you need MAC whereas for IPv6 you need (client) ID but not
MAC. Supposedly, for IPv4 you could specify client-id but I have not
experimented with that.
In both IPv4 and IPv6, my experiments with dnsmasq say that specifying a
name is not sufficient to have an IP address assigned whereas for IPv4
specifying a MAC and for IPv6 specifying an ID is sufficient.
Gene
3:02 a.m.
New subject: [libvirt] [PATCH 2/2] use client id for IPv6 DHCP host definition
Originally, only a host name was used to associate a
DHCPv6 request with a specific IPv6 address. Further testing
demonstrates that this is an unreliable method and, instead,
a client-id or DUID needs to be used. According to DHCPv6
standards, this id can be a duid-LLT, duid-LL, or duid-UUID
even though dnsmasq will accept almost any text string.
Although validity checking of a specified string makes sure it is
hexadecimal notation with bytes separated by colons, there is no
rigorous check to make sure it meets the standard.
Documentation and schemas have been updated.
Signed-off-by: Gene Czarcinski <gene(a)czarc.net>
---
docs/formatnetwork.html.in | 18 ++++++-
docs/schemas/basictypes.rng | 59 ++++++++++++++++++++++
docs/schemas/network.rng | 3 ++
src/conf/network_conf.c | 29 +++++++++--
src/conf/network_conf.h | 1 +
src/network/bridge_driver.c | 3 +-
src/util/virdnsmasq.c | 20 ++++++--
src/util/virdnsmasq.h | 1 +
tests/networkxml2confdata/dhcp6-nat-network.xml | 5 +-
tests/networkxml2confdata/dhcp6-network.xml | 5 +-
.../dhcp6host-routed-network.xml | 5 +-
11 files changed, 135 insertions(+), 14 deletions(-)
diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in
index 7b42529..94391d7 100644
--- a/docs/formatnetwork.html.in
+++ b/docs/formatnetwork.html.in
@@ -749,6 +749,11 @@
<p>
Below is another IPv6 varition. Instead of a dhcp range being
specified, this example has a couple of IPv6 host definitions.
+ Note that most of the dhcp host definitions use an "id" (client
+ id or DUID) since this has proven to be a more reliable way
+ of specifying the interface and its association with an IPv6
+ address. The first is a DUID-LLT, the second a DUID-LL, and
+ the third a DUID-UUID. <span class="since">Since
1.0.3</span>
</p>
<pre>
@@ -764,7 +769,9 @@
<ip family="ipv6" address="2001:db8:ca2:2::1"
prefix="64" >
<dhcp>
<host name="paul" ip="2001:db8:ca2:2:3::1"
/>
- <host name="bob" ip="2001:db8:ca2:2:3::2"
/>
+ <host id="0:1:0:1:18:aa:62:fe:0:16:3e:44:55:66"
ip="2001:db8:ca2:2:3::2" />
+ <host id="0:3:0:1:0:16:3e:11:22:33" name="ralph"
ip="2001:db8:ca2:2:3::3" />
+ <host
id="0:4:7e:7d:f0:7d:a8:bc:c5:d2:13:32:11:ed:16:ea:84:63" name="badbob"
ip="2001:db8:ca2:2:3::4" />
</dhcp>
</ip>
</network></pre>
@@ -795,6 +802,11 @@
<p>
This variation of an isolated network defines only IPv6.
+ Note that most of the dhcp host definitions use an "id" (client
+ id or DUID) since this has proven to be a more reliable way
+ of specifying the interface and its association with an IPv6
+ address. The first is a DUID-LLT, the second a DUID-LL, and
+ the third a DUID-UUID. <span class="since">Since
1.0.3</span>
</p>
<pre>
@@ -804,7 +816,9 @@
<ip family="ipv6" address="2001:db8:ca2:6::1"
prefix="64" >
<dhcp>
<host name="peter" ip="2001:db8:ca2:6:6::1"
/>
- <host name="dariusz" ip="2001:db8:ca2:6:6::2"
/>
+ <host id="0:1:0:1:18:aa:62:fe:0:16:3e:44:55:66"
ip="2001:db8:ca2:6:6::2" />
+ <host id="0:3:0:1:0:16:3e:11:22:33" name="dariusz"
ip="2001:db8:ca2:6:6::3" />
+ <host
id="0:4:7e:7d:f0:7d:a8:bc:c5:d2:13:32:11:ed:16:ea:84:63" name="anita"
ip="2001:db8:ca2:6:6::4" />
</dhcp>
</ip>
</network></pre>
diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng
index 2d2f442..5e2a333 100644
--- a/docs/schemas/basictypes.rng
+++ b/docs/schemas/basictypes.rng
@@ -101,6 +101,65 @@
</data>
</define>
+ <!--====================================================================-->
+ <!--The duid is a unique identifier used in DHCPv6 to identity an -->
+ <!--interface on a device (system). The duid is often used by servers -->
+ <!--such as dnsmasq to assign a specific IP address (and optionally a -->
+ <!--name to an interface. The applicable standards are RFC3315 and -->
+ <!--RFC6355. These standards actualy require the duid to be fixed for -->
+ <!--the hardward device and applicable to all network interfaces on -->
+ <!--that device. It is not clear that any software currently enforces -->
+ <!--this requirement although it could be implemented manually. -->
+ <!--====================================================================-->
+ <!--There are currently four types of duids defined: -->
+ <!-- type 1, duid-LLT, link-layer (MAC) plus 32 bit time when the -->
+ <!-- duid-LLT was created in seconds from January 1, 2000 -->
+ <!-- type 2, duid-EN, 32 bit "enterprise number" followed by a
-->
+ <!-- variable length unique identifier. -->
+ <!-- type 3, duid-LL, link-layer (MAC) -->
+ <!-- type 4, duid-UUID, a 128 bit UUID (16 bytes) -->
+ <!--RFC3315 states that the maximum length of a duid is 128 bytes plus -->
+ <!--the 16 bit type field. Often, the machine type is "1" which is the
-->
+ <!--number assigned to ethernet. -->
+
+ <define name="duidLLT">
+ <data type="string">
+ <!-- 0======| type======| 0======| machine type======|
time================| link-layer============| -->
+ <param
name="pattern">[0]{1,2}:[0]{0,1}[1]:[0]{1,2}:[0]{0,1}[a-fA-F1-9](:[a-fA-F0-9]{1,2}){4}(:[a-fA-F0-9]{1,2}){6,8}</param>
+ </data>
+ </define>
+
+ <define name="duidEN">
+ <data type="string">
+ <!-- 0======| type======| Enterprise number===| unique id
==============| -->
+ <param
name="pattern">[0]{1,2}:[0]{0,1}[2](:[a-fA-F0-9]{1,2}){4}(:[a-fA-F0-9]{1,2}){1,124}</param>
+ </data>
+ </define>
+
+ <define name="duidLL">
+ <data type="string">
+ <!-- 0======| type======| 0======| machine type======|
link-layer============| -->
+ <param
name="pattern">[0]{1,2}:[0]{0,1}[3]:[0]{1,2}:[0]{0,1}[a-fA-F1-9](:[a-fA-F0-9]{1,2}){6,8}</param>
+ </data>
+ </define>
+
+ <define name="duidUUID">
+ <data type="string">
+ <!-- 0======| type======| UUID=================| -->
+ <param
name="pattern">[0]{1,2}:[0]{0,1}[4](:[a-fA-F0-9]{1,2}){16}</param>
+ </data>
+ </define>
+
+ <define name="DUID">
+ <choice>
+ <ref name="duidLLT"/>
+ <ref name="duidEN"/>
+ <ref name="duidLL"/>
+ <ref name="duidUUID"/>
+ </choice>
+ </define>
+ <!--======================================================================-->
+
<!-- An ipv4 "dotted quad" address -->
<define name="ipv4Addr">
<data type="string">
diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng
index a479453..98be9d6 100644
--- a/docs/schemas/network.rng
+++ b/docs/schemas/network.rng
@@ -279,6 +279,9 @@
<zeroOrMore>
<element name="host">
<optional>
+ <attribute name="id"><ref
name="DUID"/></attribute>
+ </optional>
+ <optional>
<attribute name="mac"><ref
name="uniMacAddr"/></attribute>
</optional>
<optional>
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index c93916d..7848a52 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -118,6 +118,7 @@ static void
virNetworkDHCPHostDefClear(virNetworkDHCPHostDefPtr def)
{
VIR_FREE(def->mac);
+ VIR_FREE(def->id);
VIR_FREE(def->name);
}
@@ -678,7 +679,7 @@ virNetworkDHCPHostDefParseXML(const char *networkName,
virNetworkDHCPHostDefPtr host,
bool partialOkay)
{
- char *mac = NULL, *name = NULL, *ip = NULL;
+ char *mac = NULL, *name = NULL, *ip = NULL, *id = NULL;
virMacAddr addr;
virSocketAddr inaddr;
int ret = -1;
@@ -707,10 +708,25 @@ virNetworkDHCPHostDefParseXML(const char *networkName,
}
}
+ id = virXMLPropString(node, "id");
+ if (id) {
+ char *cp = id;
+
+ while (*cp) {
+ if ((*cp != ':') && (!c_isxdigit(*cp))) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("Cannot use id '%s' in network
'%s'"),
+ id, networkName);
+ goto cleanup;
+ }
+ cp++;
+ }
+ }
+
name = virXMLPropString(node, "name");
if (name && (!c_isalpha(name[0]))) {
virReportError(VIR_ERR_XML_ERROR,
- _("Cannot use name address '%s' in network
'%s'"),
+ _("Cannot use host name '%s' in network
'%s'"),
name, networkName);
goto cleanup;
}
@@ -738,10 +754,10 @@ virNetworkDHCPHostDefParseXML(const char *networkName,
* address or name (IPv4)
*/
if (VIR_SOCKET_ADDR_IS_FAMILY(&def->address, AF_INET6)) {
- if (!name) {
+ if (!(id || name)) {
virReportError(VIR_ERR_XML_ERROR,
_("Static host definition in IPv6 network '%s'
"
- "must have name attribute"),
+ "must have id or name attribute"),
networkName);
goto cleanup;
}
@@ -763,6 +779,8 @@ virNetworkDHCPHostDefParseXML(const char *networkName,
host->mac = mac;
mac = NULL;
+ host->id = id;
+ id = NULL;
host->name = name;
name = NULL;
if (ip)
@@ -771,6 +789,7 @@ virNetworkDHCPHostDefParseXML(const char *networkName,
cleanup:
VIR_FREE(mac);
+ VIR_FREE(id);
VIR_FREE(name);
VIR_FREE(ip);
return ret;
@@ -2020,6 +2039,8 @@ virNetworkIpDefFormat(virBufferPtr buf,
virBufferAddLit(buf, "<host ");
if (def->hosts[ii].mac)
virBufferAsprintf(buf, "mac='%s' ",
def->hosts[ii].mac);
+ if (def->hosts[ii].id)
+ virBufferAsprintf(buf, "id='%s' ",
def->hosts[ii].id);
if (def->hosts[ii].name)
virBufferAsprintf(buf, "name='%s' ",
def->hosts[ii].name);
if (VIR_SOCKET_ADDR_VALID(&def->hosts[ii].ip)) {
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index 4c634ed..48d8718 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -73,6 +73,7 @@ typedef struct _virNetworkDHCPHostDef virNetworkDHCPHostDef;
typedef virNetworkDHCPHostDef *virNetworkDHCPHostDefPtr;
struct _virNetworkDHCPHostDef {
char *mac;
+ char *id;
char *name;
virSocketAddr ip;
};
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 21255f0..6c41f55 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -599,7 +599,8 @@ networkBuildDnsmasqDhcpHostsList(dnsmasqContext *dctx,
for (i = 0; i < ipdef->nhosts; i++) {
virNetworkDHCPHostDefPtr host = &(ipdef->hosts[i]);
if (VIR_SOCKET_ADDR_VALID(&host->ip))
- if (dnsmasqAddDhcpHost(dctx, host->mac, &host->ip, host->name,
ipv6) < 0)
+ if (dnsmasqAddDhcpHost(dctx, host->mac, &host->ip,
+ host->name, host->id, ipv6) < 0)
return -1;
}
diff --git a/src/util/virdnsmasq.c b/src/util/virdnsmasq.c
index 6637a89..98ad5cd 100644
--- a/src/util/virdnsmasq.c
+++ b/src/util/virdnsmasq.c
@@ -303,6 +303,7 @@ hostsfileAdd(dnsmasqHostsfile *hostsfile,
const char *mac,
virSocketAddr *ip,
const char *name,
+ const char *id,
bool ipv6)
{
char *ipstr = NULL;
@@ -314,9 +315,19 @@ hostsfileAdd(dnsmasqHostsfile *hostsfile,
/* the first test determines if it is a dhcpv6 host */
if (ipv6) {
- if (virAsprintf(&hostsfile->hosts[hostsfile->nhosts].host,
"%s,[%s]",
- name, ipstr) < 0)
- goto alloc_error;
+ if (name && id) {
+ if (virAsprintf(&hostsfile->hosts[hostsfile->nhosts].host,
"id:%s,%s,[%s]",
+ id, name, ipstr) < 0)
+ goto alloc_error;
+ } else if (name && !id) {
+ if (virAsprintf(&hostsfile->hosts[hostsfile->nhosts].host,
"%s,[%s]",
+ name, ipstr) < 0)
+ goto alloc_error;
+ } else if (!name && id) {
+ if (virAsprintf(&hostsfile->hosts[hostsfile->nhosts].host,
"id:%s,[%s]",
+ id, ipstr) < 0)
+ goto alloc_error;
+ }
}
else if (name && mac) {
if (virAsprintf(&hostsfile->hosts[hostsfile->nhosts].host,
"%s,%s,%s",
@@ -511,9 +522,10 @@ dnsmasqAddDhcpHost(dnsmasqContext *ctx,
const char *mac,
virSocketAddr *ip,
const char *name,
+ const char *id,
bool ipv6)
{
- return hostsfileAdd(ctx->hostsfile, mac, ip, name, ipv6);
+ return hostsfileAdd(ctx->hostsfile, mac, ip, name, id, ipv6);
}
/*
diff --git a/src/util/virdnsmasq.h b/src/util/virdnsmasq.h
index a97d3e6..ed560da 100644
--- a/src/util/virdnsmasq.h
+++ b/src/util/virdnsmasq.h
@@ -86,6 +86,7 @@ int dnsmasqAddDhcpHost(dnsmasqContext *ctx,
const char *mac,
virSocketAddr *ip,
const char *name,
+ const char *id,
bool ipv6);
int dnsmasqAddHost(dnsmasqContext *ctx,
virSocketAddr *ip,
diff --git a/tests/networkxml2confdata/dhcp6-nat-network.xml
b/tests/networkxml2confdata/dhcp6-nat-network.xml
index 72103f7..4259173 100644
--- a/tests/networkxml2confdata/dhcp6-nat-network.xml
+++ b/tests/networkxml2confdata/dhcp6-nat-network.xml
@@ -15,8 +15,11 @@
<ip family='ipv6' address='2001:db8:ac10:fd01::1'
prefix='64'>
<dhcp>
<range start='2001:db8:ac10:fd01::1:10'
end='2001:db8:ac10:fd01::1:ff' />
- <host name='ralph' ip='2001:db8:ac10:fd01::1:20' />
+ <host id='0:4:7e:7d:f0:7d:a8:bc:c5:d2:13:32:11:ed:16:ea:84:63'
ip='2001:db8:ac10:fd01::1:20' />
<host name='paul' ip='2001:db8:ac10:fd01::1:21' />
+ <host id='0:3:0:1:0:16:3e:11:22:33' name='peter.xyz'
ip='2001:db8:ac10:fd01::1:22' />
+ <host id='0:3:0:1:0:16:3e:44:55:33'
ip='2001:db8:ac10:fd01::1:23' />
+ <host id='0:1:0:1:18:aa:62:fe:0:16:3e:44:55:66' name='badbob'
ip='2001:db8:ac10:fd01::1:24' />
</dhcp>
</ip>
<ip family='ipv4' address='10.24.10.1'>
diff --git a/tests/networkxml2confdata/dhcp6-network.xml
b/tests/networkxml2confdata/dhcp6-network.xml
index 311013a..776737e 100644
--- a/tests/networkxml2confdata/dhcp6-network.xml
+++ b/tests/networkxml2confdata/dhcp6-network.xml
@@ -7,8 +7,11 @@
<ip family='ipv6' address='2001:db8:ac10:fd01::1'
prefix='64'>
<dhcp>
<range start='2001:db8:ac10:fd01::1:10'
end='2001:db8:ac10:fd01::1:ff' />
- <host name='ralph' ip='2001:db8:ac10:fd01::1:20' />
+ <host id='0:4:7e:7d:f0:7d:a8:bc:c5:d2:13:32:11:ed:16:ea:84:63'
ip='2001:db8:ac10:fd01::1:20' />
<host name='paul' ip='2001:db8:ac10:fd01::1:21' />
+ <host id='0:3:0:1:0:16:3e:11:22:33' name='peter.xyz'
ip='2001:db8:ac10:fd01::1:22' />
+ <host id='0:3:0:1:0:16:3e:44:55:33'
ip='2001:db8:ac10:fd01::1:23' />
+ <host id='0:1:0:1:18:aa:62:fe:0:16:3e:44:55:66' name='badbob'
ip='2001:db8:ac10:fd01::1:24' />
</dhcp>
</ip>
</network>
diff --git a/tests/networkxml2confdata/dhcp6host-routed-network.xml
b/tests/networkxml2confdata/dhcp6host-routed-network.xml
index 38d9ebf..2693d87 100644
--- a/tests/networkxml2confdata/dhcp6host-routed-network.xml
+++ b/tests/networkxml2confdata/dhcp6host-routed-network.xml
@@ -12,8 +12,11 @@
</ip>
<ip family='ipv6' address='2001:db8:ac10:fd01::1'
prefix='64'>
<dhcp>
- <host name='ralph' ip='2001:db8:ac10:fd01::1:20' />
+ <host id='0:4:7e:7d:f0:7d:a8:bc:c5:d2:13:32:11:ed:16:ea:84:63'
ip='2001:db8:ac10:fd01::1:20' />
<host name='paul' ip='2001:db8:ac10:fd01::1:21' />
+ <host id='0:3:0:1:0:16:3e:11:22:33' name='peter.xyz'
ip='2001:db8:ac10:fd01::1:22' />
+ <host id='0:3:0:1:0:16:3e:44:55:33'
ip='2001:db8:ac10:fd01::1:23' />
+ <host id='0:1:0:1:18:aa:62:fe:0:16:3e:44:55:66' name='badbob'
ip='2001:db8:ac10:fd01::1:24' />
</dhcp>
</ip>
</network>
--
1.8.1.2
10:38 a.m.
New subject: [libvirt] [PATCH 2/2] use client id for IPv6 DHCP host definition
On 02/15/2013 02:02 PM, Gene Czarcinski wrote:
Originally, only a host name was used to associate a
DHCPv6 request with a specific IPv6 address. Further testing
demonstrates that this is an unreliable method and, instead,
a client-id or DUID needs to be used. According to DHCPv6
standards, this id can be a duid-LLT, duid-LL, or duid-UUID
even though dnsmasq will accept almost any text string.
Although validity checking of a specified string makes sure it is
hexadecimal notation with bytes separated by colons, there is no
rigorous check to make sure it meets the standard.
Documentation and schemas have been updated.
Signed-off-by: Gene Czarcinski <gene(a)czarc.net>
---
docs/formatnetwork.html.in | 18 ++++++-
docs/schemas/basictypes.rng | 59 ++++++++++++++++++++++
docs/schemas/network.rng | 3 ++
src/conf/network_conf.c | 29 +++++++++--
src/conf/network_conf.h | 1 +
src/network/bridge_driver.c | 3 +-
src/util/virdnsmasq.c | 20 ++++++--
src/util/virdnsmasq.h | 1 +
tests/networkxml2confdata/dhcp6-nat-network.xml | 5 +-
tests/networkxml2confdata/dhcp6-network.xml | 5 +-
.../dhcp6host-routed-network.xml | 5 +-
11 files changed, 135 insertions(+), 14 deletions(-)
diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in
index 7b42529..94391d7 100644
--- a/docs/formatnetwork.html.in
+++ b/docs/formatnetwork.html.in
@@ -749,6 +749,11 @@
<p>
Below is another IPv6 varition. Instead of a dhcp range being
specified, this example has a couple of IPv6 host definitions.
+ Note that most of the dhcp host definitions use an "id" (client
+ id or DUID) since this has proven to be a more reliable way
+ of specifying the interface and its association with an IPv6
+ address. The first is a DUID-LLT, the second a DUID-LL, and
+ the third a DUID-UUID. <span class="since">Since
1.0.3</span>
</p>
<pre>
@@ -764,7 +769,9 @@
<ip family="ipv6" address="2001:db8:ca2:2::1"
prefix="64" >
<dhcp>
<host name="paul" ip="2001:db8:ca2:2:3::1"
/>
- <host name="bob" ip="2001:db8:ca2:2:3::2"
/>
+ <host id="0:1:0:1:18:aa:62:fe:0:16:3e:44:55:66"
ip="2001:db8:ca2:2:3::2" />
+ <host id="0:3:0:1:0:16:3e:11:22:33" name="ralph"
ip="2001:db8:ca2:2:3::3" />
+ <host
id="0:4:7e:7d:f0:7d:a8:bc:c5:d2:13:32:11:ed:16:ea:84:63" name="badbob"
ip="2001:db8:ca2:2:3::4" />
</dhcp>
</ip>
</network></pre>
@@ -795,6 +802,11 @@
<p>
This variation of an isolated network defines only IPv6.
+ Note that most of the dhcp host definitions use an "id" (client
+ id or DUID) since this has proven to be a more reliable way
+ of specifying the interface and its association with an IPv6
+ address. The first is a DUID-LLT, the second a DUID-LL, and
+ the third a DUID-UUID. <span class="since">Since
1.0.3</span>
</p>
<pre>
@@ -804,7 +816,9 @@
<ip family="ipv6" address="2001:db8:ca2:6::1"
prefix="64" >
<dhcp>
<host name="peter" ip="2001:db8:ca2:6:6::1"
/>
- <host name="dariusz" ip="2001:db8:ca2:6:6::2"
/>
+ <host id="0:1:0:1:18:aa:62:fe:0:16:3e:44:55:66"
ip="2001:db8:ca2:6:6::2" />
+ <host id="0:3:0:1:0:16:3e:11:22:33"
name="dariusz" ip="2001:db8:ca2:6:6::3" />
+ <host
id="0:4:7e:7d:f0:7d:a8:bc:c5:d2:13:32:11:ed:16:ea:84:63" name="anita"
ip="2001:db8:ca2:6:6::4" />
</dhcp>
</ip>
</network></pre>
diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng
index 2d2f442..5e2a333 100644
--- a/docs/schemas/basictypes.rng
+++ b/docs/schemas/basictypes.rng
@@ -101,6 +101,65 @@
</data>
</define>
+ <!--====================================================================-->
+ <!--The duid is a unique identifier used in DHCPv6 to identity an -->
+ <!--interface on a device (system). The duid is often used by servers -->
+ <!--such as dnsmasq to assign a specific IP address (and optionally a -->
+ <!--name to an interface. The applicable standards are RFC3315 and -->
+ <!--RFC6355. These standards actualy require the duid to be fixed for -->
+ <!--the hardward device and applicable to all network interfaces on -->
+ <!--that device. It is not clear that any software currently enforces -->
+ <!--this requirement although it could be implemented manually. -->
+ <!--====================================================================-->
+ <!--There are currently four types of duids defined: -->
+ <!-- type 1, duid-LLT, link-layer (MAC) plus 32 bit time when the -->
+ <!-- duid-LLT was created in seconds from January 1, 2000 -->
+ <!-- type 2, duid-EN, 32 bit "enterprise number" followed by a
-->
+ <!-- variable length unique identifier. -->
+ <!-- type 3, duid-LL, link-layer (MAC) -->
+ <!-- type 4, duid-UUID, a 128 bit UUID (16 bytes) -->
+ <!--RFC3315 states that the maximum length of a duid is 128 bytes plus -->
+ <!--the 16 bit type field. Often, the machine type is "1" which is the
-->
+ <!--number assigned to ethernet. -->
+
+ <define name="duidLLT">
+ <data type="string">
+ <!-- 0======| type======| 0======| machine type======|
time================| link-layer============| -->
+ <param
name="pattern">[0]{1,2}:[0]{0,1}[1]:[0]{1,2}:[0]{0,1}[a-fA-F1-9](:[a-fA-F0-9]{1,2}){4}(:[a-fA-F0-9]{1,2}){6,8}</param>
+ </data>
+ </define>
+
+ <define name="duidEN">
+ <data type="string">
+ <!-- 0======| type======| Enterprise number===| unique id
==============| -->
+ <param
name="pattern">[0]{1,2}:[0]{0,1}[2](:[a-fA-F0-9]{1,2}){4}(:[a-fA-F0-9]{1,2}){1,124}</param>
+ </data>
+ </define>
+
+ <define name="duidLL">
+ <data type="string">
+ <!-- 0======| type======| 0======| machine type======|
link-layer============| -->
+ <param
name="pattern">[0]{1,2}:[0]{0,1}[3]:[0]{1,2}:[0]{0,1}[a-fA-F1-9](:[a-fA-F0-9]{1,2}){6,8}</param>
+ </data>
+ </define>
+
+ <define name="duidUUID">
+ <data type="string">
+ <!-- 0======| type======| UUID=================| -->
+ <param
name="pattern">[0]{1,2}:[0]{0,1}[4](:[a-fA-F0-9]{1,2}){16}</param>
+ </data>
+ </define>
+
+ <define name="DUID">
+ <choice>
+ <ref name="duidLLT"/>
+ <ref name="duidEN"/>
+ <ref name="duidLL"/>
+ <ref name="duidUUID"/>
+ </choice>
+ </define>
+ <!--======================================================================-->
+
<!-- An ipv4 "dotted quad" address -->
<define name="ipv4Addr">
<data type="string">
diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng
index a479453..98be9d6 100644
--- a/docs/schemas/network.rng
+++ b/docs/schemas/network.rng
@@ -279,6 +279,9 @@
<zeroOrMore>
<element name="host">
<optional>
+ <attribute name="id"><ref
name="DUID"/></attribute>
+ </optional>
+ <optional>
<attribute name="mac"><ref
name="uniMacAddr"/></attribute>
</optional>
<optional>
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index c93916d..7848a52 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -118,6 +118,7 @@ static void
virNetworkDHCPHostDefClear(virNetworkDHCPHostDefPtr def)
{
VIR_FREE(def->mac);
+ VIR_FREE(def->id);
VIR_FREE(def->name);
}
@@ -678,7 +679,7 @@ virNetworkDHCPHostDefParseXML(const char *networkName,
virNetworkDHCPHostDefPtr host,
bool partialOkay)
{
- char *mac = NULL, *name = NULL, *ip = NULL;
+ char *mac = NULL, *name = NULL, *ip = NULL, *id = NULL;
virMacAddr addr;
virSocketAddr inaddr;
int ret = -1;
@@ -707,10 +708,25 @@ virNetworkDHCPHostDefParseXML(const char *networkName,
}
}
+ id = virXMLPropString(node, "id");
+ if (id) {
+ char *cp = id;
+
+ while (*cp) {
+ if ((*cp != ':') && (!c_isxdigit(*cp))) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("Cannot use id '%s' in network
'%s'"),
+ id, networkName);
+ goto cleanup;
+ }
+ cp++;
Instead of a loop, you could just do something like
char *cp = id + strcspn(id, "0123456789abcdefABCDEF:");
if (*cp) {
virReportError(VIR_ERR_XML_ERROR,
_("Invalid character '%c' in id '%s' of
network
'%s'"),
*cp, id, networkName);
}
+ }
+ }
+
name = virXMLPropString(node, "name");
if (name && (!c_isalpha(name[0]))) {
Hmm. This isn't a change of this patch, but that check could do with a
bit more validation in a separate patch. While the first character must
be alpha, all characters after that must be alphanumeric, "-", or
".".
virReportError(VIR_ERR_XML_ERROR,
- _("Cannot use name address '%s' in network
'%s'"),
+ _("Cannot use host name '%s' in network
'%s'"),
name, networkName);
goto cleanup;
}
@@ -738,10 +754,10 @@ virNetworkDHCPHostDefParseXML(const char *networkName,
* address or name (IPv4)
*/
if (VIR_SOCKET_ADDR_IS_FAMILY(&def->address, AF_INET6)) {
- if (!name) {
+ if (!(id || name)) {
virReportError(VIR_ERR_XML_ERROR,
_("Static host definition in IPv6 network '%s'
"
- "must have name attribute"),
+ "must have id or name attribute"),
networkName);
goto cleanup;
}
@@ -763,6 +779,8 @@ virNetworkDHCPHostDefParseXML(const char *networkName,
host->mac = mac;
mac = NULL;
+ host->id = id;
+ id = NULL;
host->name = name;
name = NULL;
if (ip)
@@ -771,6 +789,7 @@ virNetworkDHCPHostDefParseXML(const char *networkName,
cleanup:
VIR_FREE(mac);
+ VIR_FREE(id);
VIR_FREE(name);
VIR_FREE(ip);
return ret;
@@ -2020,6 +2039,8 @@ virNetworkIpDefFormat(virBufferPtr buf,
virBufferAddLit(buf, "<host ");
if (def->hosts[ii].mac)
virBufferAsprintf(buf, "mac='%s' ",
def->hosts[ii].mac);
+ if (def->hosts[ii].id)
+ virBufferAsprintf(buf, "id='%s' ",
def->hosts[ii].id);
if (def->hosts[ii].name)
virBufferAsprintf(buf, "name='%s' ",
def->hosts[ii].name);
if (VIR_SOCKET_ADDR_VALID(&def->hosts[ii].ip)) {
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index 4c634ed..48d8718 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -73,6 +73,7 @@ typedef struct _virNetworkDHCPHostDef virNetworkDHCPHostDef;
typedef virNetworkDHCPHostDef *virNetworkDHCPHostDefPtr;
struct _virNetworkDHCPHostDef {
char *mac;
+ char *id;
char *name;
virSocketAddr ip;
};
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 21255f0..6c41f55 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -599,7 +599,8 @@ networkBuildDnsmasqDhcpHostsList(dnsmasqContext *dctx,
for (i = 0; i < ipdef->nhosts; i++) {
virNetworkDHCPHostDefPtr host = &(ipdef->hosts[i]);
if (VIR_SOCKET_ADDR_VALID(&host->ip))
- if (dnsmasqAddDhcpHost(dctx, host->mac, &host->ip, host->name,
ipv6) < 0)
+ if (dnsmasqAddDhcpHost(dctx, host->mac, &host->ip,
+ host->name, host->id, ipv6) < 0)
return -1;
}
diff --git a/src/util/virdnsmasq.c b/src/util/virdnsmasq.c
index 6637a89..98ad5cd 100644
--- a/src/util/virdnsmasq.c
+++ b/src/util/virdnsmasq.c
@@ -303,6 +303,7 @@ hostsfileAdd(dnsmasqHostsfile *hostsfile,
const char *mac,
virSocketAddr *ip,
const char *name,
+ const char *id,
bool ipv6)
{
char *ipstr = NULL;
@@ -314,9 +315,19 @@ hostsfileAdd(dnsmasqHostsfile *hostsfile,
/* the first test determines if it is a dhcpv6 host */
if (ipv6) {
- if (virAsprintf(&hostsfile->hosts[hostsfile->nhosts].host,
"%s,[%s]",
- name, ipstr) < 0)
- goto alloc_error;
+ if (name && id) {
+ if (virAsprintf(&hostsfile->hosts[hostsfile->nhosts].host,
"id:%s,%s,[%s]",
+ id, name, ipstr) < 0)
+ goto alloc_error;
+ } else if (name && !id) {
+ if (virAsprintf(&hostsfile->hosts[hostsfile->nhosts].host,
"%s,[%s]",
+ name, ipstr) < 0)
+ goto alloc_error;
+ } else if (!name && id) {
+ if (virAsprintf(&hostsfile->hosts[hostsfile->nhosts].host,
"id:%s,[%s]",
+ id, ipstr) < 0)
+ goto alloc_error;
+ }
Heh. There might be a more compact way to express all those options, but
it would probably just make it confusing to follow :-)
}
else if (name && mac) {
Although it's not really related, this might be a good chance to move
this else up to the same line as the preceding {.
if
(virAsprintf(&hostsfile->hosts[hostsfile->nhosts].host, "%s,%s,%s",
@@ -511,9 +522,10 @@ dnsmasqAddDhcpHost(dnsmasqContext *ctx,
const char *mac,
virSocketAddr *ip,
const char *name,
+ const char *id,
bool ipv6)
{
- return hostsfileAdd(ctx->hostsfile, mac, ip, name, ipv6);
+ return hostsfileAdd(ctx->hostsfile, mac, ip, name, id, ipv6);
}
/*
diff --git a/src/util/virdnsmasq.h b/src/util/virdnsmasq.h
index a97d3e6..ed560da 100644
--- a/src/util/virdnsmasq.h
+++ b/src/util/virdnsmasq.h
@@ -86,6 +86,7 @@ int dnsmasqAddDhcpHost(dnsmasqContext *ctx,
const char *mac,
virSocketAddr *ip,
const char *name,
+ const char *id,
bool ipv6);
int dnsmasqAddHost(dnsmasqContext *ctx,
virSocketAddr *ip,
diff --git a/tests/networkxml2confdata/dhcp6-nat-network.xml
b/tests/networkxml2confdata/dhcp6-nat-network.xml
index 72103f7..4259173 100644
--- a/tests/networkxml2confdata/dhcp6-nat-network.xml
+++ b/tests/networkxml2confdata/dhcp6-nat-network.xml
@@ -15,8 +15,11 @@
<ip family='ipv6' address='2001:db8:ac10:fd01::1'
prefix='64'>
<dhcp>
<range start='2001:db8:ac10:fd01::1:10'
end='2001:db8:ac10:fd01::1:ff' />
- <host name='ralph' ip='2001:db8:ac10:fd01::1:20' />
+ <host id='0:4:7e:7d:f0:7d:a8:bc:c5:d2:13:32:11:ed:16:ea:84:63'
ip='2001:db8:ac10:fd01::1:20' />
<host name='paul' ip='2001:db8:ac10:fd01::1:21' />
+ <host id='0:3:0:1:0:16:3e:11:22:33' name='peter.xyz'
ip='2001:db8:ac10:fd01::1:22' />
+ <host id='0:3:0:1:0:16:3e:44:55:33'
ip='2001:db8:ac10:fd01::1:23' />
+ <host id='0:1:0:1:18:aa:62:fe:0:16:3e:44:55:66' name='badbob'
ip='2001:db8:ac10:fd01::1:24' />
</dhcp>
</ip>
<ip family='ipv4' address='10.24.10.1'>
diff --git a/tests/networkxml2confdata/dhcp6-network.xml
b/tests/networkxml2confdata/dhcp6-network.xml
index 311013a..776737e 100644
--- a/tests/networkxml2confdata/dhcp6-network.xml
+++ b/tests/networkxml2confdata/dhcp6-network.xml
@@ -7,8 +7,11 @@
<ip family='ipv6' address='2001:db8:ac10:fd01::1'
prefix='64'>
<dhcp>
<range start='2001:db8:ac10:fd01::1:10'
end='2001:db8:ac10:fd01::1:ff' />
- <host name='ralph' ip='2001:db8:ac10:fd01::1:20' />
+ <host id='0:4:7e:7d:f0:7d:a8:bc:c5:d2:13:32:11:ed:16:ea:84:63'
ip='2001:db8:ac10:fd01::1:20' />
<host name='paul' ip='2001:db8:ac10:fd01::1:21' />
+ <host id='0:3:0:1:0:16:3e:11:22:33' name='peter.xyz'
ip='2001:db8:ac10:fd01::1:22' />
+ <host id='0:3:0:1:0:16:3e:44:55:33'
ip='2001:db8:ac10:fd01::1:23' />
+ <host id='0:1:0:1:18:aa:62:fe:0:16:3e:44:55:66' name='badbob'
ip='2001:db8:ac10:fd01::1:24' />
</dhcp>
</ip>
</network>
diff --git a/tests/networkxml2confdata/dhcp6host-routed-network.xml
b/tests/networkxml2confdata/dhcp6host-routed-network.xml
index 38d9ebf..2693d87 100644
--- a/tests/networkxml2confdata/dhcp6host-routed-network.xml
+++ b/tests/networkxml2confdata/dhcp6host-routed-network.xml
@@ -12,8 +12,11 @@
</ip>
<ip family='ipv6' address='2001:db8:ac10:fd01::1'
prefix='64'>
<dhcp>
- <host name='ralph' ip='2001:db8:ac10:fd01::1:20' />
+ <host id='0:4:7e:7d:f0:7d:a8:bc:c5:d2:13:32:11:ed:16:ea:84:63'
ip='2001:db8:ac10:fd01::1:20' />
<host name='paul' ip='2001:db8:ac10:fd01::1:21' />
+ <host id='0:3:0:1:0:16:3e:11:22:33' name='peter.xyz'
ip='2001:db8:ac10:fd01::1:22' />
+ <host id='0:3:0:1:0:16:3e:44:55:33'
ip='2001:db8:ac10:fd01::1:23' />
+ <host id='0:1:0:1:18:aa:62:fe:0:16:3e:44:55:66' name='badbob'
ip='2001:db8:ac10:fd01::1:24' />
</dhcp>
</ip>
</network>
Other than the suggestion to use strcspn() instead of a while loop, this
all looks fine to me. ACK with that change made. If you (or someone
else) wants to ACK the short interdiff I've attached that makes that
change, I'll push it.
5:44 a.m.
New subject: [libvirt] [PATCH 2/2] use client id for IPv6 DHCP host definition
On 02/18/2013 07:38 PM, Laine Stump wrote:
On 02/15/2013 02:02 PM, Gene Czarcinski wrote:
> Originally, only a host name was used to associate a
> DHCPv6 request with a specific IPv6 address. Further testing
> demonstrates that this is an unreliable method and, instead,
> a client-id or DUID needs to be used. According to DHCPv6
> standards, this id can be a duid-LLT, duid-LL, or duid-UUID
> even though dnsmasq will accept almost any text string.
>
Other than the suggestion to use strcspn() instead of a while loop, this
all looks fine to me. ACK with that change made. If you (or someone
else) wants to ACK the short interdiff I've attached that makes that
change, I'll push it.
>From 0b2f2f0794e931af901bc31e4fe6eadc799bee33 Mon Sep 17 00:00:00 2001
From: Laine Stump <laine(a)laine.org>
Date: Mon, 18 Feb 2013 21:33:49 -0500
Subject: [PATCH] Changes to squash into "use client id for IPv6 DHCP host
definition"
---
src/conf/network_conf.c | 15 +++++----------
src/util/virdnsmasq.c | 3 +--
2 files changed, 6 insertions(+), 12 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 8657284..12bf4d7 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -710,16 +710,11 @@ virNetworkDHCPHostDefParseXML(const char *networkName,
id = virXMLPropString(node, "id");
if (id) {
- char *cp = id;
-
- while (*cp) {
- if ((*cp != ':') && (!c_isxdigit(*cp))) {
- virReportError(VIR_ERR_XML_ERROR,
- _("Cannot use id '%s' in network
'%s'"),
- id, networkName);
- goto cleanup;
- }
- cp++;
+ char *cp = id + strcspn(id, "0123456789abcdefABCDEF:");
+ if (*cp) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("Invalid character '%c' in id '%s' of
network '%s'"),
+ *cp, id, networkName);
That's not quite right. You want to use strspn(), not strcspn().
+++ b/src/util/virdnsmasq.c
@@ -328,8 +328,7 @@ hostsfileAdd(dnsmasqHostsfile *hostsfile,
id, ipstr) < 0)
goto alloc_error;
}
- }
- else if (name && mac) {
+ } else if (name && mac) {
This hunk is good.
ACK to your interdiff once you use the correct function.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
3:52 p.m.
New subject: [libvirt] [PATCH 2/2] use client id for IPv6 DHCP host definition
On 02/19/2013 04:44 PM, Eric Blake wrote:
On 02/18/2013 07:38 PM, Laine Stump wrote:
> On 02/15/2013 02:02 PM, Gene Czarcinski wrote:
>> Originally, only a host name was used to associate a
>> DHCPv6 request with a specific IPv6 address. Further testing
>> demonstrates that this is an unreliable method and, instead,
>> a client-id or DUID needs to be used. According to DHCPv6
>> standards, this id can be a duid-LLT, duid-LL, or duid-UUID
>> even though dnsmasq will accept almost any text string.
>>
> Other than the suggestion to use strcspn() instead of a while loop, this
> all looks fine to me. ACK with that change made. If you (or someone
> else) wants to ACK the short interdiff I've attached that makes that
> change, I'll push it.
>
> >From 0b2f2f0794e931af901bc31e4fe6eadc799bee33 Mon Sep 17 00:00:00 2001
> From: Laine Stump <laine(a)laine.org>
> Date: Mon, 18 Feb 2013 21:33:49 -0500
> Subject: [PATCH] Changes to squash into "use client id for IPv6 DHCP host
> definition"
>
> ---
> src/conf/network_conf.c | 15 +++++----------
> src/util/virdnsmasq.c | 3 +--
> 2 files changed, 6 insertions(+), 12 deletions(-)
>
> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
> index 8657284..12bf4d7 100644
> --- a/src/conf/network_conf.c
> +++ b/src/conf/network_conf.c
> @@ -710,16 +710,11 @@ virNetworkDHCPHostDefParseXML(const char *networkName,
>
> id = virXMLPropString(node, "id");
> if (id) {
> - char *cp = id;
> -
> - while (*cp) {
> - if ((*cp != ':') && (!c_isxdigit(*cp))) {
> - virReportError(VIR_ERR_XML_ERROR,
> - _("Cannot use id '%s' in network
'%s'"),
> - id, networkName);
> - goto cleanup;
> - }
> - cp++;
> + char *cp = id + strcspn(id, "0123456789abcdefABCDEF:");
> + if (*cp) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("Invalid character '%c' in id '%s'
of network '%s'"),
> + *cp, id, networkName);
That's not quite right. You want to use strspn(), not strcspn().
Argh! I would have caught that if I'd paid attention to make check.
In the meantime, I noticed that there weren't any networkxml2xml tests
for dhcp6 host records, so I added one before pushing.
> +++ b/src/util/virdnsmasq.c
> @@ -328,8 +328,7 @@ hostsfileAdd(dnsmasqHostsfile *hostsfile,
> id, ipstr) < 0)
> goto alloc_error;
> }
> - }
> - else if (name && mac) {
> + } else if (name && mac) {
This hunk is good.
ACK to your interdiff once you use the correct function.
I've now pushed both the patches in this series.
Thanks Gene! (and thanks for Eric for catching my mistake!)
3:12 a.m.
On 02/15/2013 02:02 PM, Gene Czarcinski wrote:
The first patch is a trivial fix to the network schema
since the name in dhcp-host is really optional.
The second patch adds the capability to use the client id
to improve the reliability of having a system use a fixed
IPv6 address.
Gene Czarcinski (2):
Trivial fix: in dhcp-host the name is optional
use client id for IPv6 DHCP host definition
docs/formatnetwork.html.in | 18 ++++++-
docs/schemas/basictypes.rng | 59 ++++++++++++++++++++++
docs/schemas/network.rng | 7 ++-
src/conf/network_conf.c | 29 +++++++++--
src/conf/network_conf.h | 1 +
src/network/bridge_driver.c | 3 +-
src/util/virdnsmasq.c | 20 ++++++--
src/util/virdnsmasq.h | 1 +
tests/networkxml2confdata/dhcp6-nat-network.xml | 5 +-
tests/networkxml2confdata/dhcp6-network.xml | 5 +-
.../dhcp6host-routed-network.xml | 5 +-
11 files changed, 138 insertions(+), 15 deletions(-)
When I originally submitted the patches for DHCPv6 I thought that using
the host name for specifying hosts with fixed IPv6 addresses would
work. Lots of testing proved that to be a foolish assumption. The only
thing that works is to specify the ID (Client ID). This has been
testing with DUID-LLT, DUID-LL, and DUID-UUID.
One of the most difficult parts in this was getting the patterns in the
schemas correct for the various types of DUID. I think I have tested
them sufficiently but ??? One of the more difficult things was to
handle bytes where the leading zero is suppressed.
Gene
3:14 a.m.
On 02/15/2013 02:12 PM, Gene Czarcinski wrote:
On 02/15/2013 02:02 PM, Gene Czarcinski wrote:
> The first patch is a trivial fix to the network schema
> since the name in dhcp-host is really optional.
>
> The second patch adds the capability to use the client id
> to improve the reliability of having a system use a fixed
> IPv6 address.
>
> Gene Czarcinski (2):
> Trivial fix: in dhcp-host the name is optional
> use client id for IPv6 DHCP host definition
>
> docs/formatnetwork.html.in | 18 ++++++-
> docs/schemas/basictypes.rng | 59
> ++++++++++++++++++++++
> docs/schemas/network.rng | 7 ++-
> src/conf/network_conf.c | 29 +++++++++--
> src/conf/network_conf.h | 1 +
> src/network/bridge_driver.c | 3 +-
> src/util/virdnsmasq.c | 20 ++++++--
> src/util/virdnsmasq.h | 1 +
> tests/networkxml2confdata/dhcp6-nat-network.xml | 5 +-
> tests/networkxml2confdata/dhcp6-network.xml | 5 +-
> .../dhcp6host-routed-network.xml | 5 +-
> 11 files changed, 138 insertions(+), 15 deletions(-)
>
When I originally submitted the patches for DHCPv6 I thought that
using the host name for specifying hosts with fixed IPv6 addresses
would work. Lots of testing proved that to be a foolish assumption.
The only thing that works is to specify the ID (Client ID). This has
been testing with DUID-LLT, DUID-LL, and DUID-UUID.
One of the most difficult parts in this was getting the patterns in
the schemas correct for the various types of DUID. I think I have
tested them sufficiently but ??? One of the more difficult things was
to handle bytes where the leading zero is suppressed.
One other little thing.
These patches with built on v1.0.2. I have
rebased to the current top level with no problems (at least when I tried).
Gene
4265
days inactive
4275
days old
11 comments
3 participants
participants (3)
-
Eric Blake -
Gene Czarcinski -
Laine Stump