2:45 p.m.
This update includes two patch files:
1. The first file adds virSocketAddrGetIpPrefix() to
determine the prefix for a network. This function
is used by the static route code and has also been
used to update (replace the code in)
virNetworkIpDefPrefix() in src/conf/network_conf.c
2. The second and major update adds functionality to
implement static route for both IPv4 and IPv6. Having
static route provides the routing information needed
to forward packets not directly reachable from a
host.
As far as I can determine, this update has adopted all
suggestions made against the previous version (except
that I am still using CommandRun ... that will be
addressed in a future update).
I also plan to take a look at <ip> processing such as
handling ULong return codes. I also raise the question
as to whether address= should be manditory rather than
(currently) optional ... I am not sure what <ip> means
with address= being optional.
Gene Czarcinski (2):
create virSocketAddrGetIpPrefix utility function
Support for static routes on a virtual bridge
docs/formatnetwork.html.in | 80 +++++
docs/schemas/network.rng | 22 ++
src/conf/network_conf.c | 364 +++++++++++++++++++--
src/conf/network_conf.h | 20 ++
src/libvirt_private.syms | 2 +
src/network/bridge_driver.c | 41 +++
src/util/virnetdev.c | 44 +++
src/util/virnetdev.h | 5 +
src/util/virsocketaddr.c | 43 +++
src/util/virsocketaddr.h | 3 +
.../networkxml2xmlin/dhcp6host-routed-network.xml | 2 +
.../networkxml2xmlout/dhcp6host-routed-network.xml | 2 +
12 files changed, 600 insertions(+), 28 deletions(-)
--
1.8.1.4
2:45 p.m.
New subject: [libvirt] [PATCH-v4 1/2] create virSocketAddrGetIpPrefix utility function
Create the utility function virSocketAddrGetIpPrefix() to
determine the prefix for this network. The code in this
function was adapted from virNetworkIpDefPrefix().
Update virNetworkIpDefPrefix() in src/conf/network_conf.c
to use the new utility function.
.
Signed-off-by: Gene Czarcinski <gene(a)czarc.net>
---
src/conf/network_conf.c | 30 +++---------------------------
src/libvirt_private.syms | 1 +
src/util/virsocketaddr.c | 43 +++++++++++++++++++++++++++++++++++++++++++
src/util/virsocketaddr.h | 3 +++
4 files changed, 50 insertions(+), 27 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index c5535e6..1726066 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -582,33 +582,9 @@ virNetworkDefGetIpByIndex(const virNetworkDefPtr def,
*/
int virNetworkIpDefPrefix(const virNetworkIpDefPtr def)
{
- if (def->prefix > 0) {
- return def->prefix;
- } else if (VIR_SOCKET_ADDR_VALID(&def->netmask)) {
- return virSocketAddrGetNumNetmaskBits(&def->netmask);
- } else if (VIR_SOCKET_ADDR_IS_FAMILY(&def->address, AF_INET)) {
- /* Return the natural prefix for the network's ip address.
- * On Linux we could use the IN_CLASSx() macros, but those
- * aren't guaranteed on all platforms, so we just deal with
- * the bits ourselves.
- */
- unsigned char octet
- = ntohl(def->address.data.inet4.sin_addr.s_addr) >> 24;
- if ((octet & 0x80) == 0) {
- /* Class A network */
- return 8;
- } else if ((octet & 0xC0) == 0x80) {
- /* Class B network */
- return 16;
- } else if ((octet & 0xE0) == 0xC0) {
- /* Class C network */
- return 24;
- }
- return -1;
- } else if (VIR_SOCKET_ADDR_IS_FAMILY(&def->address, AF_INET6)) {
- return 64;
- }
- return -1;
+ return virSocketAddrGetIpPrefix(&def->address,
+ &def->netmask,
+ def->prefix);
}
/* Fill in a virSocketAddr with the proper netmask for this
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 96eea0a..ade4525 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1690,6 +1690,7 @@ virSocketAddrCheckNetmask;
virSocketAddrEqual;
virSocketAddrFormat;
virSocketAddrFormatFull;
+virSocketAddrGetIpPrefix;
virSocketAddrGetPort;
virSocketAddrGetRange;
virSocketAddrIsNetmask;
diff --git a/src/util/virsocketaddr.c b/src/util/virsocketaddr.c
index 3dfa3fb..673b026 100644
--- a/src/util/virsocketaddr.c
+++ b/src/util/virsocketaddr.c
@@ -754,4 +754,47 @@ virSocketAddrPrefixToNetmask(unsigned int prefix,
error:
return result;
+ }
+
+/**
+ * virSocketAddrGetIpPrefix:
+ * @address: network address
+ * @netmask: netmask for this network
+ * @prefix: prefix if specified instead of netmask
+ *
+ * Returns prefix value on success or -1 on error.
+ */
+
+int
+virSocketAddrGetIpPrefix(const virSocketAddrPtr address,
+ const virSocketAddrPtr netmask,
+ int prefix)
+{
+ if (prefix > 0) {
+ return prefix;
+ } else if (VIR_SOCKET_ADDR_VALID(netmask)) {
+ return virSocketAddrGetNumNetmaskBits(netmask);
+ } else if (VIR_SOCKET_ADDR_IS_FAMILY(address, AF_INET)) {
+ /* Return the natural prefix for the network's ip address.
+ * On Linux we could use the IN_CLASSx() macros, but those
+ * aren't guaranteed on all platforms, so we just deal with
+ * the bits ourselves.
+ */
+ unsigned char octet
+ = ntohl(address->data.inet4.sin_addr.s_addr) >> 24;
+ if ((octet & 0x80) == 0) {
+ /* Class A network */
+ return 8;
+ } else if ((octet & 0xC0) == 0x80) {
+ /* Class B network */
+ return 16;
+ } else if ((octet & 0xE0) == 0xC0) {
+ /* Class C network */
+ return 24;
+ }
+ return -1;
+ } else if (VIR_SOCKET_ADDR_IS_FAMILY(address, AF_INET6)) {
+ return 64;
+ }
+ return -1;
}
diff --git a/src/util/virsocketaddr.h b/src/util/virsocketaddr.h
index 8993f7b..1c9e54a 100644
--- a/src/util/virsocketaddr.h
+++ b/src/util/virsocketaddr.h
@@ -116,6 +116,9 @@ int virSocketAddrGetNumNetmaskBits(const virSocketAddrPtr netmask);
int virSocketAddrPrefixToNetmask(unsigned int prefix,
virSocketAddrPtr netmask,
int family);
+int virSocketAddrGetIpPrefix(const virSocketAddrPtr address,
+ const virSocketAddrPtr netmask,
+ int prefix);
bool virSocketAddrEqual(const virSocketAddrPtr s1,
const virSocketAddrPtr s2);
bool virSocketAddrIsPrivate(const virSocketAddrPtr addr);
--
1.8.1.4
9:20 a.m.
New subject: [libvirt] [PATCH-v4 1/2] create virSocketAddrGetIpPrefix utility function
On 04/20/2013 03:45 PM, Gene Czarcinski wrote:
Create the utility function virSocketAddrGetIpPrefix() to
determine the prefix for this network. The code in this
function was adapted from virNetworkIpDefPrefix().
Update virNetworkIpDefPrefix() in src/conf/network_conf.c
to use the new utility function.
.
Signed-off-by: Gene Czarcinski <gene(a)czarc.net>
---
ACK. I'll push it after I've reviewed 2/2.
10:48 a.m.
New subject: [libvirt] [PATCH-v4 1/2] create virSocketAddrGetIpPrefix utility function
On 04/22/2013 10:20 AM, Laine Stump wrote:
On 04/20/2013 03:45 PM, Gene Czarcinski wrote:
> Create the utility function virSocketAddrGetIpPrefix() to
> determine the prefix for this network. The code in this
> function was adapted from virNetworkIpDefPrefix().
>
> Update virNetworkIpDefPrefix() in src/conf/network_conf.c
> to use the new utility function.
> .
> Signed-off-by: Gene Czarcinski <gene(a)czarc.net>
> ---
ACK. I'll push it after I've reviewed 2/2.
After going through patch 2/2, I realized that maintaining the "return
-1 when no prefix or netmask or address is specified" rule in this new
function will prevent someone from declaring a default route on a network:
<route gateway='1.2.3.4'/>
because this will return -1. I checked all the callers of the original
function, and they all are already guaranteed to have a valid address,
so we can safely change the default case of this function to return 0.
I'm going to make that change and push this patch so that you don't have
to resubmit it with the new spin of 2/2 that I'm requesting.
2:45 p.m.
New subject: [libvirt] [PATCH-v4 2/2] Support for static routes on a virtual bridge
This patch adds support for adding a static route for
a network. The "gateway" sub-element specifies
the gateway's IP address. Both IPv4 and IPv6
static routes are supported although it is
expected that this functionality will have
more use with IPv6.
This updates add the <route> element to define a
static route. The gateway= subelement species the
gateway address which is to receive the packets
forwarded to the network specified on the
<route> definition.
Tests are done to validate that the input definitions are
correct. For example, for a static route ip definition,
the address must be a network address and not a host address.
Additional checks are added to ensure that the specified gateway
is directly reachable via a network defined on this bridge.
Whan a static route is added to a bridge, there is a slight
possibility that the gateway address will be incorrect. If
this is handled as an error, that bridge becomes unusable and
can only be recovered by rebooting. If the error is
ignored, then that network can be destroyed and the network
definition file edited to correct the problem. Unfortunately,
the error message only appears in syslog. However, with
the checks performed when the network definition file is parsed,
it is unlikely that this condition will ever occur.
Handling of errors in this manner is consistent with other uses
of the CommandRun interface.
The command used is of the following form:
ip route add <address>/<prefix> via <gateway> dev <virbr-bridge>
\
proto static metric 1
.
Signed-off-by: Gene Czarcinski <gene(a)czarc.net>
---
docs/formatnetwork.html.in | 80 +++++
docs/schemas/network.rng | 22 ++
src/conf/network_conf.c | 334 ++++++++++++++++++++-
src/conf/network_conf.h | 20 ++
src/libvirt_private.syms | 1 +
src/network/bridge_driver.c | 41 +++
src/util/virnetdev.c | 44 +++
src/util/virnetdev.h | 5 +
.../networkxml2xmlin/dhcp6host-routed-network.xml | 2 +
.../networkxml2xmlout/dhcp6host-routed-network.xml | 2 +
10 files changed, 550 insertions(+), 1 deletion(-)
diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in
index 4dd0415..cbf4a5c 100644
--- a/docs/formatnetwork.html.in
+++ b/docs/formatnetwork.html.in
@@ -529,6 +529,60 @@
starting.
</p>
+ <h5><a name="elementsStaticroute">Static
Routes</a></h5>
+ <p>
+ It has been possible to define static routes on a virtual bridge
+ device <span class="since">Since 1.0.5</span>. Static route
+ definitions are used to provide routing information to the
+ virtualization host for networks which are not defined as a
+ network on one of the virtual bridges on the virtualization host.
+ </p>
+
+ <p>
+ As shown in <a href="formatnetwork.html#examplesNoGateway">this
example</a>,
+ it is possible to define a virtual bridge interface with no
+ IPv4 or IPv6 networks. Such interfaces are useful in supporting
+ networks which have no visibility or direct connectivity with the
+ virtualization host, but can be used to support
+ virtual networks which only have guest connectivity. A guest
+ with connectivity to the guest-only network and another network
+ that is directly reachable from the host can act as a gateway between the
+ networks. A static route added to the "visible" network definition
+ provides the routing information so that IP packets can be sent
+ from the virtualization host to guests on the hidden network.
+ </p>
+
+ <p>
+ Here is a fragment of a definition which shows the static
+ route specification as well as the IPv4 and IPv6 definitions
+ for network addresses which are referred to in the
+ <code>gateway</code> gateway address specifications.
+ </p>
+
+ <pre>
+ ...
+ <ip address="192.168.122.1"
netmask="255.255.255.0">
+ <dhcp>
+ <range start="192.168.122.128"
end="192.168.122.254" />
+ </dhcp>
+ </ip>
+ <route address="192.168.222.0" prefix="24"
gateway="192.168.122.2" />
+ <ip family="ipv6" address="2001:db8:ca2:2::1"
prefix="64" />
+ <route family="ipv6" address="2001:db8:ca2:3::"
prefix="64" gateway="2001:db8:ca2:2::2">
+ </route>
+ ...
+ </pre>
+
+ <p>
+ Notice that the <code>route</code> elements look very similar to
+ the <code>ip</code> address specification. Although parts are the
same,
+ the meaning and effects of the two elements are very different. The
+ <code>ip</code> address specification results in an address being
defined
+ on the virtual bridge whereas the <code>route</code> definition is
used
+ to create the static route for the specified network via the gateway address
+ on the virtual bridge device.
+ </p>
+
<h3><a name="elementsAddress">Addressing</a></h3>
<p>
@@ -560,6 +614,7 @@
</dhcp>
</ip>
<ip family="ipv6" address="2001:db8:ca2:2::1"
prefix="64" />
+ <route family="ipv6" address="2001:db9:ca1:1::"
prefix="64" gateway="2001:db8:ca2:2::2" />
</network></pre>
<dl>
@@ -809,6 +864,31 @@
</ip>
</network></pre>
+ <p>
+ Below is yet another IPv6 variation. This variation has only IPv6
+ defined with DHCPv6 on the primary IPv6 network. A static link
+ if defined for a second IPv6 network which will not be visable on
+ the bridge interface but rill have a static route defined for this
+ network via the specified gateway. Note that the gateway address
+ must be valid for the networks supported on the bridge interface.
+ <span class="since">Since 1.0.5</span>
+ </p>
+
+ <pre>
+ <network>
+ <name>net7</name>
+ <bridge name="virbr7" />
+ <forward mode="route"/>
+ <ip family="ipv6" address="2001:db8:ca2:7::1"
prefix="64" >
+ <dhcp>
+ <range start="2001:db8:ca2:7::100"
end="2001:db8:ca2::1ff" />
+ <host
id="0:4:7e:7d:f0:7d:a8:bc:c5:d2:13:32:11:ed:16:ea:84:63" name="lucas"
ip="2001:db8:ca2:2:3::4" />
+ </dhcp>
+ </ip>
+ <route family="ipv6" address="2001:db8:ca2:8::"
prefix="64" gateway="2001:db8:ca2:7::4" >
+ </route>
+ </network></pre>
+
<h3><a name="examplesPrivate">Isolated network
config</a></h3>
<p>
diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng
index 6c3fae2..5bcba32 100644
--- a/docs/schemas/network.rng
+++ b/docs/schemas/network.rng
@@ -305,6 +305,28 @@
</optional>
</element>
</zeroOrMore>
+ <!-- <route> element -->
+ <zeroOrMore>
+ <!-- The (static) route element specifies a network address and gateway
+ address to access that network. -->
+ <element name="route">
+ <optional>
+ <attribute name="family"><ref
name="addr-family"/></attribute>
+ </optional>
+ <optional>
+ <attribute name="address"><ref
name="ipAddr"/></attribute>
+ </optional>
+ <optional>
+ <choice>
+ <attribute name="netmask"><ref
name="ipv4Addr"/></attribute>
+ <attribute name="prefix"><ref
name="ipPrefix"/></attribute>
+ </choice>
+ </optional>
+ <optional>
+ <attribute name="gateway"><ref
name="ipAddr"/></attribute>
+ </optional>
+ </element>
+ </zeroOrMore>
</interleave>
</element>
</define>
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 1726066..2b56ca7 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -137,6 +137,12 @@ virNetworkIpDefClear(virNetworkIpDefPtr def)
}
static void
+virNetworkRouteDefClear(virNetworkRouteDefPtr def)
+{
+ VIR_FREE(def->family);
+}
+
+static void
virNetworkDNSTxtDefClear(virNetworkDNSTxtDefPtr def)
{
VIR_FREE(def->name);
@@ -215,6 +221,11 @@ virNetworkDefFree(virNetworkDefPtr def)
}
VIR_FREE(def->ips);
+ for (ii = 0 ; ii < def->nroutes && def->routes ; ii++) {
+ virNetworkRouteDefClear(&def->routes[ii]);
+ }
+ VIR_FREE(def->routes);
+
for (ii = 0; ii < def->nPortGroups && def->portGroups; ii++) {
virPortGroupDefClear(&def->portGroups[ii]);
}
@@ -1256,6 +1267,209 @@ cleanup:
}
static int
+virNetworkRouteDefParseXML(const char *networkName,
+ xmlNodePtr node,
+ xmlXPathContextPtr ctxt,
+ virNetworkRouteDefPtr def)
+{
+ /*
+ * virNetworkRouteDef object is already allocated as part
+ * of an array. On failure clear: it out, but don't free it.
+ */
+
+ xmlNodePtr save;
+ char *address = NULL, *netmask = NULL;
+ char *gateway = NULL;
+ unsigned long prefix = 0;
+ int result = -1;
+ int prefixRc;
+ virSocketAddr testAddr;
+
+ save = ctxt->node;
+ ctxt->node = node;
+
+ /* grab raw data from XML */
+ def->family = virXPathString("string(./@family)", ctxt);
+
+ address = virXPathString("string(./@address)", ctxt);
+
+ netmask = virXPathString("string(./@netmask)", ctxt);
+
+ gateway = virXPathString("string(./@gateway)", ctxt);
+
+ prefixRc = virXPathULong("string(./@prefix)", ctxt, &prefix);
+ if (prefixRc == -2) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("Invalid prefix specified in route definition
'%s'"),
+ networkName);
+ goto cleanup;
+ }
+
+ /* Note: both network and gateway addresses must be specified */
+
+ if (address) {
+ if (virSocketAddrParse(&def->address, address, AF_UNSPEC) < 0) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("Bad network address '%s' in route definition
'%s'"),
+ address, networkName);
+ goto cleanup;
+ }
+
+ } else {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("A network address must be specified in route definition
'%s'"),
+ networkName);
+ goto cleanup;
+ }
+
+ if (gateway) {
+ if (virSocketAddrParse(&def->gateway, gateway, AF_UNSPEC) < 0) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("Bad gateway address '%s' in route definition
'%s'"),
+ gateway, networkName);
+ goto cleanup;
+ }
+
+ } else {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("A gateway address must be specified in route definition
'%s'"),
+ networkName);
+ goto cleanup;
+ }
+
+ /* validate network address, etc. for each family */
+ if ((def->family == NULL) || (STREQ(def->family, "ipv4"))) {
+ if (!(VIR_SOCKET_ADDR_IS_FAMILY(&def->address, AF_INET) ||
+ VIR_SOCKET_ADDR_IS_FAMILY(&def->address, AF_UNSPEC))) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("%s family specified for non-IPv4 address '%s'
in route definition '%s'"),
+ def->family == NULL? "no" : "ipv4",
address, networkName);
+ goto cleanup;
+ }
+ if (!(VIR_SOCKET_ADDR_IS_FAMILY(&def->gateway, AF_INET) ||
+ VIR_SOCKET_ADDR_IS_FAMILY(&def->gateway, AF_UNSPEC))) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("%s family specified for non-IPv4 gateway address
'%s' in route definition '%s'"),
+ def->family == NULL? "no" : "ipv4",
address, networkName);
+ goto cleanup;
+ }
+ if (netmask) {
+ if (virSocketAddrParse(&def->netmask, netmask, AF_UNSPEC) < 0) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("Bad netmask address '%s' in route
definition '%s'"),
+ netmask, networkName);
+ goto cleanup;
+ }
+
+ if (!VIR_SOCKET_ADDR_IS_FAMILY(&def->netmask, AF_INET)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("network '%s' has invalid netmask
'%s' for address '%s' (both must be IPv4)"),
+ networkName, netmask, address);
+ goto cleanup;
+ }
+ if (!(prefixRc < 0)) {
+ /* can't have both netmask and prefix at the same time */
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("route definition '%s' cannot have both a
prefix and a netmask"),
+ networkName);
+ goto cleanup;
+ }
+ }
+ else {
+ if (prefixRc < 0 || prefix == 0) {
+ int num = virSocketAddrGetIpPrefix(&def->address,
&def->netmask, prefix);
+ if (num < 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Invalid IPv4 prefix specified in route
definition '%s'"),
+ networkName);
+ goto cleanup;
+ }
+ else
+ def->prefix = num;
+ }
+ else
+ def->prefix = prefix;
+ }
+ if (prefix > 32 ) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Invalid IPv4 prefix '%lu' for '%s'
specified in route definition '%s'"),
+ prefix, def->family == NULL? "no" :
"ipv4", networkName);
+ goto cleanup;
+ }
+ } else if (STREQ(def->family, "ipv6")) {
+ if (!VIR_SOCKET_ADDR_IS_FAMILY(&def->address, AF_INET6)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("ipv6 family specified for non-IPv6 address
'%s' in route definition '%s'"),
+ address, networkName);
+ goto cleanup;
+ }
+ if (netmask) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("specifying netmask invalid for IPv6 address
'%s' in route definition '%s'"),
+ address, networkName);
+ goto cleanup;
+ }
+ if (!VIR_SOCKET_ADDR_IS_FAMILY(&def->gateway, AF_INET6)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("ipv6 specified for non-IPv6 gateway address
'%s' in route definition '%s'"),
+ gateway, networkName);
+ goto cleanup;
+ }
+ if (prefix > 128 ) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Invalid IPv6 prefix '%lu' for '%s'
specified in route definition '%s'"),
+ prefix, def->family, networkName);
+ goto cleanup;
+ }
+ if (prefixRc < 0 || prefix == 0)
+ def->prefix = 64; /* IPv6 default */
+ else
+ def->prefix = prefix;
+ } else {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("Unrecognized family '%s' in definition of route
definition '%s'"),
+ def->family, networkName);
+ goto cleanup;
+ }
+
+ /* make sure the address is a network address */
+ if (netmask) {
+ if (virSocketAddrMask(&def->address, &def->netmask, &testAddr)
< 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("error converting address '%s' with netmask
'%s' to network-address in route definition '%s'"),
+ address, netmask, networkName);
+ goto cleanup;
+ }
+ } else {
+ if (virSocketAddrMaskByPrefix(&def->address, def->prefix,
&testAddr) < 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("error converting address '%s' with prefix=%u
to network-address in route definition '%s'"),
+ address, def->prefix, networkName);
+ goto cleanup;
+ }
+ }
+ if (!virSocketAddrEqual(&def->address, &testAddr)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("address '%s' is not a network-address in route
definition '%s'"),
+ address, networkName);
+ goto cleanup;
+ }
+
+ result = 0;
+
+cleanup:
+ if (result < 0) {
+ virNetworkRouteDefClear(def);
+ }
+ VIR_FREE(address);
+ VIR_FREE(netmask);
+ VIR_FREE(gateway);
+
+ ctxt->node = save;
+ return result;
+}
+
+static int
virNetworkPortGroupParseXML(virPortGroupDefPtr def,
xmlNodePtr node,
xmlXPathContextPtr ctxt)
@@ -1652,8 +1866,9 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
char *tmp;
char *stp = NULL;
xmlNodePtr *ipNodes = NULL;
+ xmlNodePtr *routeNodes = NULL;
xmlNodePtr *portGroupNodes = NULL;
- int nIps, nPortGroups;
+ int nIps, nPortGroups, nRoutes;
xmlNodePtr dnsNode = NULL;
xmlNodePtr virtPortNode = NULL;
xmlNodePtr forwardNode = NULL;
@@ -1807,6 +2022,76 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
}
VIR_FREE(ipNodes);
+ nRoutes = virXPathNodeSet("./route", ctxt, &routeNodes);
+ if (nRoutes < 0)
+ goto error;
+
+ if (nRoutes > 0) {
+ int ii;
+
+ /* allocate array to hold all the route definitions */
+ if (VIR_ALLOC_N(def->routes, nRoutes) < 0) {
+ virReportOOMError();
+ goto error;
+ }
+ /* parse each definition */
+ for (ii = 0; ii < nRoutes; ii++) {
+ int ret = virNetworkRouteDefParseXML(def->name, routeNodes[ii],
+ ctxt, &def->routes[ii]);
+ if (ret < 0)
+ goto error;
+ def->nroutes++;
+ }
+
+ /* now validate the correctness of any static route gateways specified
+ *
+ * note: the parameters within each definition are varified/assumed valid;
+ * the question being asked and answered here is if the specified gateway
+ * address has a network definition (reachable) on this bridge.
+ */
+ nRoutes = def->nroutes;
+ nIps = def->nips;
+ for (ii = 0; ii < nRoutes; ii++) {
+ int jj;
+ virSocketAddr testAddr, testGw;
+ bool addrMatch;
+ virNetworkRouteDefPtr gwdef = &def->routes[ii];
+ if (VIR_SOCKET_ADDR_VALID(&gwdef->gateway)) {
+ addrMatch = false;
+ for (jj = 0; jj < nIps; jj++) {
+ virNetworkIpDefPtr def2 = &def->ips[jj];
+ if ((VIR_SOCKET_ADDR_IS_FAMILY(&gwdef->gateway, AF_INET6))
&&
+ (!VIR_SOCKET_ADDR_IS_FAMILY(&def2->address, AF_INET6)))
+ continue;
+ if ((VIR_SOCKET_ADDR_IS_FAMILY(&def2->address, AF_INET6))
&&
+ (!VIR_SOCKET_ADDR_IS_FAMILY(&gwdef->gateway, AF_INET6)))
+ continue;
+ if ((VIR_SOCKET_ADDR_IS_FAMILY(&gwdef->gateway, AF_INET))
&&
+ VIR_SOCKET_ADDR_IS_FAMILY(&def2->address, AF_INET6))
+ continue;
+ int prefix = virNetworkIpDefPrefix(def2);
+ virSocketAddrMaskByPrefix(&def2->address, prefix,
&testAddr);
+ virSocketAddrMaskByPrefix(&gwdef->gateway, prefix,
&testGw);
+ if (VIR_SOCKET_ADDR_VALID(&testAddr) &&
+ VIR_SOCKET_ADDR_VALID(&testGw) &&
+ virSocketAddrEqual(&testAddr, &testGw)) {
+ addrMatch = true;
+ break;
+ }
+ }
+ if (!addrMatch) {
+ char *gw = virSocketAddrFormat(&gwdef->gateway);
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unreachable static route gateway '%s'
specified for network '%s'"),
+ gw, def->name);
+ VIR_FREE(gw);
+ goto error;
+ }
+ }
+ }
+ }
+ VIR_FREE(routeNodes);
+
forwardNode = virXPathNode("./forward", ctxt);
if (forwardNode &&
virNetworkForwardDefParseXML(def->name, forwardNode, ctxt,
&def->forward) < 0) {
@@ -2179,6 +2464,48 @@ error:
}
static int
+virNetworkRouteDefFormat(virBufferPtr buf,
+ const virNetworkRouteDefPtr def)
+{
+ int result = -1;
+
+ virBufferAddLit(buf, "<route");
+
+ if (def->family) {
+ virBufferAsprintf(buf, " family='%s'", def->family);
+ }
+ if (VIR_SOCKET_ADDR_VALID(&def->address)) {
+ char *addr = virSocketAddrFormat(&def->address);
+ if (!addr)
+ goto error;
+ virBufferAsprintf(buf, " address='%s'", addr);
+ VIR_FREE(addr);
+ }
+ if (VIR_SOCKET_ADDR_VALID(&def->netmask)) {
+ char *addr = virSocketAddrFormat(&def->netmask);
+ if (!addr)
+ goto error;
+ virBufferAsprintf(buf, " netmask='%s'", addr);
+ VIR_FREE(addr);
+ }
+ if (def->prefix > 0) {
+ virBufferAsprintf(buf," prefix='%u'", def->prefix);
+ }
+ if (VIR_SOCKET_ADDR_VALID(&def->gateway)) {
+ char *addr = virSocketAddrFormat(&def->gateway);
+ if (!addr)
+ goto error;
+ virBufferAsprintf(buf, " gateway='%s'", addr);
+ VIR_FREE(addr);
+ }
+ virBufferAddLit(buf, " />\n");
+
+ result = 0;
+error:
+ return result;
+}
+
+static int
virPortGroupDefFormat(virBufferPtr buf,
const virPortGroupDefPtr def)
{
@@ -2376,6 +2703,11 @@ virNetworkDefFormatInternal(virBufferPtr buf,
goto error;
}
+ for (ii = 0; ii < def->nroutes; ii++) {
+ if (virNetworkRouteDefFormat(buf, &def->routes[ii]) < 0)
+ goto error;
+ }
+
if (virNetDevVPortProfileFormat(def->virtPortProfile, buf) < 0)
goto error;
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index 6ce9a00..37e7858 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -135,6 +135,23 @@ struct _virNetworkIpDef {
virSocketAddr bootserver;
};
+typedef struct _virNetworkRouteDef virNetworkRouteDef;
+typedef virNetworkRouteDef *virNetworkRouteDefPtr;
+struct _virNetworkRouteDef {
+ char *family; /* ipv4 or ipv6 - default is ipv4 */
+ virSocketAddr address; /* Routed Network IP address */
+
+ /* One or the other of the following two will be used for a given
+ * Network address, but never both. The parser guarantees this.
+ * The virSocketAddrGetIpPrefix() can be used to get a
+ * valid prefix.
+ */
+ unsigned int prefix; /* ipv6 - only prefix allowed */
+ virSocketAddr netmask; /* ipv4 - either netmask or prefix specified */
+
+ virSocketAddr gateway; /* gateway IP address for ip-route */
+ };
+
typedef struct _virNetworkForwardIfDef virNetworkForwardIfDef;
typedef virNetworkForwardIfDef *virNetworkForwardIfDefPtr;
struct _virNetworkForwardIfDef {
@@ -209,6 +226,9 @@ struct _virNetworkDef {
size_t nips;
virNetworkIpDefPtr ips; /* ptr to array of IP addresses on this network */
+ size_t nroutes;
+ virNetworkRouteDefPtr routes; /* ptr to array of static routes on this interface */
+
virNetworkDNSDef dns; /* dns related configuration */
virNetDevVPortProfilePtr virtPortProfile;
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index ade4525..987fe64 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1466,6 +1466,7 @@ virNetDevReplaceMacAddress;
virNetDevReplaceNetConfig;
virNetDevRestoreMacAddress;
virNetDevRestoreNetConfig;
+virNetDevSetGateway;
virNetDevSetIPv4Address;
virNetDevSetMAC;
virNetDevSetMTU;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index e8b314a..39f97d1 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2360,6 +2360,7 @@ out:
return ret;
}
+/* add an IP address to a bridge */
static int
networkAddAddrToBridge(virNetworkObjPtr network,
virNetworkIpDefPtr ipdef)
@@ -2380,6 +2381,30 @@ networkAddAddrToBridge(virNetworkObjPtr network,
return 0;
}
+/* add an IP (static) route to a bridge */
+static int
+networkAddRouteToBridge(virNetworkObjPtr network,
+ virNetworkRouteDefPtr routedef)
+{
+ int prefix = virSocketAddrGetIpPrefix(&routedef->address,
+ &routedef->netmask,
+ routedef->prefix);
+
+ if (prefix < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("bridge '%s' has an invalid netmask or IP address
for route definition"),
+ network->def->bridge);
+ return -1;
+ }
+
+ if (virNetDevSetGateway(network->def->bridge,
+ &routedef->address,
+ prefix,
+ &routedef->gateway) < 0)
+ return -1;
+ return 0;
+}
+
static int
networkStartNetworkVirtual(struct network_driver *driver,
virNetworkObjPtr network)
@@ -2388,6 +2413,7 @@ networkStartNetworkVirtual(struct network_driver *driver,
bool v4present = false, v6present = false;
virErrorPtr save_err = NULL;
virNetworkIpDefPtr ipdef;
+ virNetworkRouteDefPtr routedef;
char *macTapIfName = NULL;
int tapfd = -1;
@@ -2446,6 +2472,7 @@ networkStartNetworkVirtual(struct network_driver *driver,
if (networkAddIptablesRules(driver, network) < 0)
goto err1;
+ /* first, do all of the real addresses */
for (ii = 0;
(ipdef = virNetworkDefGetIpByIndex(network->def, AF_UNSPEC, ii));
ii++) {
@@ -2464,6 +2491,20 @@ networkStartNetworkVirtual(struct network_driver *driver,
if (virNetDevSetOnline(network->def->bridge, 1) < 0)
goto err2;
+ /* now, do all of the static routes */
+ for (ii = 0; ii < network->def->nroutes; ii++) {
+ routedef = &network->def->routes[ii];
+ /* Add the IP route to the bridge */
+ /* ignore errors, error msg will be generated */
+ /* but libvirt will not know and net-destroy will work. */
+ if (VIR_SOCKET_ADDR_VALID(&routedef->gateway)) {
+ if (networkAddRouteToBridge(network, routedef) < 0) {
+ /* an error occurred adding the static route */
+ continue; /* for now, do nothing */
+ }
+ }
+ }
+
/* If forward.type != NONE, turn on global IP forwarding */
if (network->def->forward.type != VIR_NETWORK_FORWARD_NONE &&
networkEnableIpForwarding(v4present, v6present) < 0) {
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 00e0f94..e4b5358 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -769,6 +769,50 @@ cleanup:
}
/**
+ * virNetDevSetGateway:
+ * @ifname: the interface name
+ * @addr: the IP network address (IPv4 or IPv6)
+ * @prefix: number of 1 bits in the netmask
+ * @gateway: via address for route (same as @addr)
+ *
+ * Add a route for a network IP address to an interface. This function
+ * *does not* remove any previously added IP static routes.
+ *
+ * Returns 0 in case of success or -1 in case of error.
+ */
+
+int virNetDevSetGateway(const char *ifname,
+ virSocketAddrPtr addr,
+ unsigned int prefix,
+ virSocketAddrPtr gateway)
+{
+ virCommandPtr cmd = NULL;
+ char *addrstr = NULL, *gatewaystr = NULL;
+ int ret = -1;
+
+ if (!(addrstr = virSocketAddrFormat(addr)))
+ goto cleanup;
+ if (!(gatewaystr = virSocketAddrFormat(gateway)))
+ goto cleanup;
+ cmd = virCommandNew(IP_PATH);
+ virCommandAddArgList(cmd, "route", "add", NULL);
+ virCommandAddArgFormat(cmd, "%s/%u", addrstr, prefix);
+ virCommandAddArgList(cmd, "via", gatewaystr, "dev", ifname,
+ "proto", "static", "metric",
NULL);
+ virCommandAddArgFormat(cmd, "%u", 1);
+
+ if (virCommandRun(cmd, NULL) < 0)
+ goto cleanup;
+
+ ret = 0;
+cleanup:
+ VIR_FREE(addrstr);
+ VIR_FREE(gatewaystr);
+ virCommandFree(cmd);
+ return ret;
+}
+
+/**
* virNetDevClearIPv4Address:
* @ifname: the interface name
* @addr: the IP address (IPv4 or IPv6)
diff --git a/src/util/virnetdev.h b/src/util/virnetdev.h
index 06d0650..07e2e63 100644
--- a/src/util/virnetdev.h
+++ b/src/util/virnetdev.h
@@ -42,6 +42,11 @@ int virNetDevSetIPv4Address(const char *ifname,
virSocketAddr *addr,
unsigned int prefix)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
+int virNetDevSetGateway(const char *ifname,
+ virSocketAddrPtr addr,
+ unsigned int prefix,
+ virSocketAddrPtr gateway)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4)
ATTRIBUTE_RETURN_CHECK;
int virNetDevClearIPv4Address(const char *ifname,
virSocketAddr *addr,
unsigned int prefix)
diff --git a/tests/networkxml2xmlin/dhcp6host-routed-network.xml
b/tests/networkxml2xmlin/dhcp6host-routed-network.xml
index 2693d87..991ce19 100644
--- a/tests/networkxml2xmlin/dhcp6host-routed-network.xml
+++ b/tests/networkxml2xmlin/dhcp6host-routed-network.xml
@@ -19,4 +19,6 @@
<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>
+ <route address="192.168.222.0" netmask="255.255.255.0"
gateway="192.168.122.10" />
+ <route family="ipv6" address="2001:db8:ac10:fc00::"
prefix="64" gateway="2001:db8:ac10:fd01::1:24" />
</network>
diff --git a/tests/networkxml2xmlout/dhcp6host-routed-network.xml
b/tests/networkxml2xmlout/dhcp6host-routed-network.xml
index 7305043..90a0e0a 100644
--- a/tests/networkxml2xmlout/dhcp6host-routed-network.xml
+++ b/tests/networkxml2xmlout/dhcp6host-routed-network.xml
@@ -21,4 +21,6 @@
<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>
+ <route address='192.168.222.0' netmask='255.255.255.0'
gateway='192.168.122.10' />
+ <route family='ipv6' address='2001:db8:ac10:fc00::'
prefix='64' gateway='2001:db8:ac10:fd01::1:24' />
</network>
--
1.8.1.4
10:59 a.m.
New subject: [libvirt] [PATCH-v4 2/2] Support for static routes on a virtual bridge
(I had thought there would be just a few tweaks to the documentation
that I could squash in and push, but as I reviewed I found more issues,
and don't have time to fix them myself. It is *very* close though, and
the freeze for 1.0.5 is coming up (within a couple days, I think?), so
if you can make the changes below, maybe we can get this into 1.0.5.)
On 04/20/2013 03:45 PM, Gene Czarcinski wrote:
Support for static routes on a virtual bridge
This patch adds support for adding a static route for
a network. The "gateway" sub-element specifies
the gateway's IP address. Both IPv4 and IPv6
static routes are supported although it is
expected that this functionality will have
more use with IPv6.
This updates add the <route> element to define a
static route. The gateway= subelement species the
gateway address which is to receive the packets
forwarded to the network specified on the
<route> definition.
The first two paragraphs are nearly equivalent, and their first
sentences are also redundant with the first line of the comment. I've
merged them into a single, more succinct introduction.
(btw, note that "gateway" is an attribute, not a subelement):
network: static route support for <network>
This patch adds the <route> subelement of <network> to define a static
route. the address and prefix (or netmask) attribute identify the
destination network, and the gateway attribute specifies the next hop
address (which must be directly reachable from the containing
<network>) which is to receive the packets destined for
"adress/(prefix|netmask)".
Tests are done to validate that the input definitions are
correct. For example, for a static route ip definition,
the address must be a network address and not a host address.
Additional checks are added to ensure that the specified gateway
is directly reachable via a network defined on this bridge.
Whan a static route is added to a bridge, there is a slight
possibility that the gateway address will be incorrect. If
this is handled as an error, that bridge becomes unusable and
can only be recovered by rebooting. If the error is
ignored, then that network can be destroyed and the network
definition file edited to correct the problem.
This explanation is fine for this patch, but I'd still (separately) like
a more detailed description of what makes the network unusable. Are we
missing something when we tear down the network if we fail at this
point? If so, we should fix our error exit code to do the right thing. I
think that can be done in a separate patch though,.
Unfortunately,
the error message only appears in syslog. However, with
the checks performed when the network definition file is parsed,
it is unlikely that this condition will ever occur.
Handling of errors in this manner is consistent with other uses
of the CommandRun interface.
The command used is of the following form:
ip route add <address>/<prefix> via <gateway> dev <virbr-bridge>
\
proto static metric 1
.
Signed-off-by: Gene Czarcinski <gene(a)czarc.net>
---
docs/formatnetwork.html.in | 80 +++++
docs/schemas/network.rng | 22 ++
src/conf/network_conf.c | 334 ++++++++++++++++++++-
src/conf/network_conf.h | 20 ++
src/libvirt_private.syms | 1 +
src/network/bridge_driver.c | 41 +++
src/util/virnetdev.c | 44 +++
src/util/virnetdev.h | 5 +
.../networkxml2xmlin/dhcp6host-routed-network.xml | 2 +
.../networkxml2xmlout/dhcp6host-routed-network.xml | 2 +
10 files changed, 550 insertions(+), 1 deletion(-)
diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in
index 4dd0415..cbf4a5c 100644
--- a/docs/formatnetwork.html.in
+++ b/docs/formatnetwork.html.in
@@ -529,6 +529,60 @@
starting.
</p>
+ <h5><a name="elementsStaticroute">Static
Routes</a></h5>
+ <p>
+ It has been possible to define static routes on a virtual bridge
+ device <span class="since">Since 1.0.5</span>. Static route
The documentation is meant to be a "snapshot of current behavior", not a
historical narrative (with annotations indicating when functionality was
introduced). Because of that, I think that "it has been possible to
define..." style isn't appropriate. We can just start of with "Static
route ....." and toss in a <since> at the end of the paragraph. That
will be more consistent with the rest of the document.
+ definitions are used to provide routing information to the
+ virtualization host for networks which are not defined as a
+ network on one of the virtual bridges on the virtualization host.
+ </p>
+
+ <p>
+ As shown in <a href="formatnetwork.html#examplesNoGateway">this
example</a>,
+ it is possible to define a virtual bridge interface with no
+ IPv4 or IPv6 networks. Such interfaces are useful in supporting
+ networks which have no visibility or direct connectivity with the
+ virtualization host, but can be used to support
+ virtual networks which only have guest connectivity. A guest
+ with connectivity to the guest-only network and another network
+ that is directly reachable from the host can act as a gateway between the
+ networks. A static route added to the "visible" network definition
+ provides the routing information so that IP packets can be sent
+ from the virtualization host to guests on the hidden network.
+ </p>
(This is all very good/useful information, but someday soon we need to
move it out of this "reference" type file into a "admin guide" type
file.)
+
+ <p>
+ Here is a fragment of a definition which shows the static
+ route specification as well as the IPv4 and IPv6 definitions
+ for network addresses which are referred to in the
+ <code>gateway</code> gateway address specifications.
+ </p>
+
+ <pre>
+ ...
+ <ip address="192.168.122.1"
netmask="255.255.255.0">
+ <dhcp>
+ <range start="192.168.122.128"
end="192.168.122.254" />
+ </dhcp>
+ </ip>
+ <route address="192.168.222.0" prefix="24"
gateway="192.168.122.2" />
+ <ip family="ipv6" address="2001:db8:ca2:2::1"
prefix="64" />
+ <route family="ipv6" address="2001:db8:ca2:3::"
prefix="64" gateway="2001:db8:ca2:2::2">
+ </route>
+ ...
+ </pre>
+
+ <p>
+ Notice that the <code>route</code> elements look very similar to
+ the <code>ip</code> address specification. Although parts are the
same,
+ the meaning and effects of the two elements are very different. The
+ <code>ip</code> address specification results in an address being
defined
+ on the virtual bridge whereas the <code>route</code> definition is
used
+ to create the static route for the specified network via the gateway address
+ on the virtual bridge device.
+ </p>
This paragraph is unnecessary, and I think would just serve to confuse,
so it should be removed.
+
<h3><a name="elementsAddress">Addressing</a></h3>
<p>
@@ -560,6 +614,7 @@
</dhcp>
</ip>
<ip family="ipv6" address="2001:db8:ca2:2::1"
prefix="64" />
+ <route family="ipv6" address="2001:db9:ca1:1::"
prefix="64" gateway="2001:db8:ca2:2::2" />
</network></pre>
<dl>
@@ -809,6 +864,31 @@
</ip>
</network></pre>
+ <p>
+ Below is yet another IPv6 variation. This variation has only IPv6
+ defined with DHCPv6 on the primary IPv6 network. A static link
+ if defined for a second IPv6 network which will not be visable on
+ the bridge interface but rill have a static route defined for this
s/rill/will/
+ network via the specified gateway. Note that the gateway
address
+ must be valid for the networks supported on the bridge interface.
"must be directly reachable via (on the same subnet as) one of the <ip>
addresses defined for this <network>"
+ <span class="since">Since 1.0.5</span>
+ </p>
+
+ <pre>
+ <network>
+ <name>net7</name>
+ <bridge name="virbr7" />
+ <forward mode="route"/>
+ <ip family="ipv6" address="2001:db8:ca2:7::1"
prefix="64" >
+ <dhcp>
+ <range start="2001:db8:ca2:7::100"
end="2001:db8:ca2::1ff" />
+ <host
id="0:4:7e:7d:f0:7d:a8:bc:c5:d2:13:32:11:ed:16:ea:84:63" name="lucas"
ip="2001:db8:ca2:2:3::4" />
+ </dhcp>
+ </ip>
+ <route family="ipv6" address="2001:db8:ca2:8::"
prefix="64" gateway="2001:db8:ca2:7::4" >
+ </route>
+ </network></pre>
+
<h3><a name="examplesPrivate">Isolated network
config</a></h3>
<p>
diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng
index 6c3fae2..5bcba32 100644
--- a/docs/schemas/network.rng
+++ b/docs/schemas/network.rng
@@ -305,6 +305,28 @@
</optional>
</element>
</zeroOrMore>
+ <!-- <route> element -->
+ <zeroOrMore>
+ <!-- The (static) route element specifies a network address and gateway
+ address to access that network. -->
+ <element name="route">
+ <optional>
+ <attribute name="family"><ref
name="addr-family"/></attribute>
+ </optional>
+ <optional>
+ <attribute name="address"><ref
name="ipAddr"/></attribute>
+ </optional>
+ <optional>
+ <choice>
+ <attribute name="netmask"><ref
name="ipv4Addr"/></attribute>
+ <attribute name="prefix"><ref
name="ipPrefix"/></attribute>
+ </choice>
+ </optional>
+ <optional>
+ <attribute name="gateway"><ref
name="ipAddr"/></attribute>
+ </optional>
<gateway> is mandatory. It doesn't make sense to have a route without a
gateway (w/o address or prefix is fine though - that makes it a default
route).
+ </element>
+ </zeroOrMore>
</interleave>
</element>
</define>
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 1726066..2b56ca7 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -137,6 +137,12 @@ virNetworkIpDefClear(virNetworkIpDefPtr def)
}
static void
+virNetworkRouteDefClear(virNetworkRouteDefPtr def)
+{
+ VIR_FREE(def->family);
+}
+
+static void
virNetworkDNSTxtDefClear(virNetworkDNSTxtDefPtr def)
{
VIR_FREE(def->name);
@@ -215,6 +221,11 @@ virNetworkDefFree(virNetworkDefPtr def)
}
VIR_FREE(def->ips);
+ for (ii = 0 ; ii < def->nroutes && def->routes ; ii++) {
+ virNetworkRouteDefClear(&def->routes[ii]);
+ }
+ VIR_FREE(def->routes);
+
for (ii = 0; ii < def->nPortGroups && def->portGroups; ii++) {
virPortGroupDefClear(&def->portGroups[ii]);
}
@@ -1256,6 +1267,209 @@ cleanup:
}
static int
+virNetworkRouteDefParseXML(const char *networkName,
+ xmlNodePtr node,
+ xmlXPathContextPtr ctxt,
+ virNetworkRouteDefPtr def)
+{
+ /*
+ * virNetworkRouteDef object is already allocated as part
+ * of an array. On failure clear: it out, but don't free it.
+ */
+
+ xmlNodePtr save;
+ char *address = NULL, *netmask = NULL;
+ char *gateway = NULL;
+ unsigned long prefix = 0;
+ int result = -1;
+ int prefixRc;
+ virSocketAddr testAddr;
+
+ save = ctxt->node;
+ ctxt->node = node;
+
+ /* grab raw data from XML */
+ def->family = virXPathString("string(./@family)", ctxt);
+
+ address = virXPathString("string(./@address)", ctxt);
+
+ netmask = virXPathString("string(./@netmask)", ctxt);
+
+ gateway = virXPathString("string(./@gateway)", ctxt);
+
+ prefixRc = virXPathULong("string(./@prefix)", ctxt, &prefix);
+ if (prefixRc == -2) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("Invalid prefix specified in route definition
'%s'"),
+ networkName);
+ goto cleanup;
+ }
+
+ /* Note: both network and gateway addresses must be specified */
address should be optional unless prefix or netmask is non-0, although
I've now noticed that won't be handled properly due to
virSocketAddrGetIpPrefix returning -1 when there is no address or prefix
or netmask (I'm fixing that before I push that patch, so you can just
toss your 1/2 patch, rebase, and assume it's fixed).
+
+ if (address) {
+ if (virSocketAddrParse(&def->address, address, AF_UNSPEC) < 0) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("Bad network address '%s' in route definition
'%s'"),
+ address, networkName);
+ goto cleanup;
+ }
+
+ } else {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("A network address must be specified in route definition
'%s'"),
+ networkName);
+ goto cleanup;
+ }
+
+ if (gateway) {
+ if (virSocketAddrParse(&def->gateway, gateway, AF_UNSPEC) < 0) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("Bad gateway address '%s' in route definition
'%s'"),
+ gateway, networkName);
+ goto cleanup;
+ }
+
+ } else {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("A gateway address must be specified in route definition
'%s'"),
+ networkName);
+ goto cleanup;
+ }
How about structuring it like this:
if (!gateway) {
"Missing error"
goto cleanup;
}
if (virSocketAddrPars(...) < 0) {
"bad gateway error"
goto cleanup;
}
+
+ /* validate network address, etc. for each family */
+ if ((def->family == NULL) || (STREQ(def->family, "ipv4"))) {
+ if (!(VIR_SOCKET_ADDR_IS_FAMILY(&def->address, AF_INET) ||
+ VIR_SOCKET_ADDR_IS_FAMILY(&def->address, AF_UNSPEC))) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("%s family specified for non-IPv4 address '%s'
in route definition '%s'"),
+ def->family == NULL? "no" : "ipv4",
address, networkName);
+ goto cleanup;
+ }
+ if (!(VIR_SOCKET_ADDR_IS_FAMILY(&def->gateway, AF_INET) ||
+ VIR_SOCKET_ADDR_IS_FAMILY(&def->gateway, AF_UNSPEC))) {
gateway is required, so AF_UNSPEC is also unacceptable.
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("%s family specified for non-IPv4 gateway address
'%s' in route definition '%s'"),
+ def->family == NULL? "no" : "ipv4",
address, networkName);
+ goto cleanup;
+ }
+ if (netmask) {
+ if (virSocketAddrParse(&def->netmask, netmask, AF_UNSPEC) < 0) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("Bad netmask address '%s' in route
definition '%s'"),
+ netmask, networkName);
+ goto cleanup;
+ }
I think you're going to end up needing the parsed netmask sooner (when
you're trying to decide whether or not address is required), so you
should probably do this parsing right after you get the netmask string
from the xml.
+
+ if (!VIR_SOCKET_ADDR_IS_FAMILY(&def->netmask, AF_INET)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("network '%s' has invalid netmask
'%s' for address '%s' (both must be IPv4)"),
+ networkName, netmask, address);
+ goto cleanup;
+ }
+ if (!(prefixRc < 0)) {
+ /* can't have both netmask and prefix at the same time */
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("route definition '%s' cannot have both a
prefix and a netmask"),
+ networkName);
+ goto cleanup;
+ }
+ }
+ else {
} else {
+ if (prefixRc < 0 || prefix == 0) {
+ int num = virSocketAddrGetIpPrefix(&def->address,
&def->netmask, prefix);
+ if (num < 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Invalid IPv4 prefix specified in route
definition '%s'"),
+ networkName);
+ goto cleanup;
+ }
+ else
+ def->prefix = num;
You should only set def->prefix if the definition contains "prefix", not
if you're deriving it from netmask, or getting the natural netmask based
on network class. It will be useful to know the result of
virSocketAddrGetIpPrefix during the parse, but only in order to decide
whether or not "address" is optional; don't save it in the def.
+ }
+ else
+ def->prefix = prefix;
+ }
+ if (prefix > 32 ) {
Extra space after "32" (make syntax-check catches this)
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Invalid IPv4 prefix '%lu' for '%s'
specified in route definition '%s'"),
+ prefix, def->family == NULL? "no" :
"ipv4", networkName);
+ goto cleanup;
+ }
+ } else if (STREQ(def->family, "ipv6")) {
+ if (!VIR_SOCKET_ADDR_IS_FAMILY(&def->address, AF_INET6)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("ipv6 family specified for non-IPv6 address
'%s' in route definition '%s'"),
+ address, networkName);
+ goto cleanup;
+ }
address should be optional if prefix is 0.
+ if (netmask) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("specifying netmask invalid for IPv6 address
'%s' in route definition '%s'"),
+ address, networkName);
+ goto cleanup;
+ }
+ if (!VIR_SOCKET_ADDR_IS_FAMILY(&def->gateway, AF_INET6)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("ipv6 specified for non-IPv6 gateway address
'%s' in route definition '%s'"),
+ gateway, networkName);
+ goto cleanup;
+ }
+ if (prefix > 128 ) {
Another extra space.
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Invalid IPv6 prefix '%lu' for '%s'
specified in route definition '%s'"),
+ prefix, def->family, networkName);
+ goto cleanup;
+ }
+ if (prefixRc < 0 || prefix == 0)
+ def->prefix = 64; /* IPv6 default */
+ else
+ def->prefix = prefix;
Again, you should only set def->prefix if the incoming xml has a prefix
attribute; don't automatically set it (only do that when you're creating
the commandline to add the route)
+ } else {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("Unrecognized family '%s' in definition of route
definition '%s'"),
+ def->family, networkName);
+ goto cleanup;
+ }
+
+ /* make sure the address is a network address */
+ if (netmask) {
+ if (virSocketAddrMask(&def->address, &def->netmask, &testAddr)
< 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("error converting address '%s' with netmask
'%s' to network-address in route definition '%s'"),
+ address, netmask, networkName);
+ goto cleanup;
+ }
+ } else {
+ if (virSocketAddrMaskByPrefix(&def->address, def->prefix,
&testAddr) < 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("error converting address '%s' with prefix=%u
to network-address in route definition '%s'"),
+ address, def->prefix, networkName);
+ goto cleanup;
+ }
+ }
+ if (!virSocketAddrEqual(&def->address, &testAddr)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("address '%s' is not a network-address in route
definition '%s'"),
"address '%s' in <route> definition of network '%s' is
not a
network address"
+ address, networkName);
+ goto cleanup;
+ }
+
+ result = 0;
+
+cleanup:
+ if (result < 0) {
+ virNetworkRouteDefClear(def);
+ }
+ VIR_FREE(address);
+ VIR_FREE(netmask);
+ VIR_FREE(gateway);
+
+ ctxt->node = save;
+ return result;
+}
+
+static int
virNetworkPortGroupParseXML(virPortGroupDefPtr def,
xmlNodePtr node,
xmlXPathContextPtr ctxt)
@@ -1652,8 +1866,9 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
char *tmp;
char *stp = NULL;
xmlNodePtr *ipNodes = NULL;
+ xmlNodePtr *routeNodes = NULL;
xmlNodePtr *portGroupNodes = NULL;
- int nIps, nPortGroups;
+ int nIps, nPortGroups, nRoutes;
xmlNodePtr dnsNode = NULL;
xmlNodePtr virtPortNode = NULL;
xmlNodePtr forwardNode = NULL;
@@ -1807,6 +2022,76 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
}
VIR_FREE(ipNodes);
+ nRoutes = virXPathNodeSet("./route", ctxt, &routeNodes);
+ if (nRoutes < 0)
+ goto error;
+
+ if (nRoutes > 0) {
+ int ii;
+
+ /* allocate array to hold all the route definitions */
+ if (VIR_ALLOC_N(def->routes, nRoutes) < 0) {
+ virReportOOMError();
+ goto error;
+ }
+ /* parse each definition */
+ for (ii = 0; ii < nRoutes; ii++) {
+ int ret = virNetworkRouteDefParseXML(def->name, routeNodes[ii],
+ ctxt, &def->routes[ii]);
+ if (ret < 0)
+ goto error;
+ def->nroutes++;
+ }
+
+ /* now validate the correctness of any static route gateways specified
+ *
+ * note: the parameters within each definition are varified/assumed valid;
+ * the question being asked and answered here is if the specified gateway
+ * address has a network definition (reachable) on this bridge.
"if the specified gateway is directly reachable from this bridge"
+ */
+ nRoutes = def->nroutes;
+ nIps = def->nips;
+ for (ii = 0; ii < nRoutes; ii++) {
+ int jj;
+ virSocketAddr testAddr, testGw;
+ bool addrMatch;
+ virNetworkRouteDefPtr gwdef = &def->routes[ii];
+ if (VIR_SOCKET_ADDR_VALID(&gwdef->gateway)) {
+ addrMatch = false;
+ for (jj = 0; jj < nIps; jj++) {
+ virNetworkIpDefPtr def2 = &def->ips[jj];
+ if ((VIR_SOCKET_ADDR_IS_FAMILY(&gwdef->gateway, AF_INET6))
&&
+ (!VIR_SOCKET_ADDR_IS_FAMILY(&def2->address, AF_INET6)))
+ continue;
+ if ((VIR_SOCKET_ADDR_IS_FAMILY(&def2->address, AF_INET6))
&&
+ (!VIR_SOCKET_ADDR_IS_FAMILY(&gwdef->gateway, AF_INET6)))
+ continue;
+ if ((VIR_SOCKET_ADDR_IS_FAMILY(&gwdef->gateway, AF_INET))
&&
+ VIR_SOCKET_ADDR_IS_FAMILY(&def2->address, AF_INET6))
+ continue;
Don't you just mean:
if (VIR_SOCKET_ADDR_FAMILY(&gwdef->gateway)
!= VIR_SOCKET_ADDR_FAMILY(&def2->address)) {
continue;
}
+ int prefix = virNetworkIpDefPrefix(def2);
+ virSocketAddrMaskByPrefix(&def2->address, prefix,
&testAddr);
+ virSocketAddrMaskByPrefix(&gwdef->gateway, prefix,
&testGw);
+ if (VIR_SOCKET_ADDR_VALID(&testAddr) &&
+ VIR_SOCKET_ADDR_VALID(&testGw) &&
+ virSocketAddrEqual(&testAddr, &testGw)) {
+ addrMatch = true;
+ break;
+ }
+ }
+ if (!addrMatch) {
+ char *gw = virSocketAddrFormat(&gwdef->gateway);
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unreachable static route gateway '%s'
specified for network '%s'"),
+ gw, def->name);
+ VIR_FREE(gw);
+ goto error;
+ }
+ }
+ }
+ }
+ VIR_FREE(routeNodes);
This VIR_FREE() needs to go down to the end of the function so that it's
always called, even on error returns.
+
forwardNode = virXPathNode("./forward", ctxt);
if (forwardNode &&
virNetworkForwardDefParseXML(def->name, forwardNode, ctxt,
&def->forward) < 0) {
@@ -2179,6 +2464,48 @@ error:
}
static int
+virNetworkRouteDefFormat(virBufferPtr buf,
+ const virNetworkRouteDefPtr def)
+{
+ int result = -1;
+
+ virBufferAddLit(buf, "<route");
+
+ if (def->family) {
+ virBufferAsprintf(buf, " family='%s'", def->family);
+ }
+ if (VIR_SOCKET_ADDR_VALID(&def->address)) {
+ char *addr = virSocketAddrFormat(&def->address);
+ if (!addr)
+ goto error;
+ virBufferAsprintf(buf, " address='%s'", addr);
+ VIR_FREE(addr);
+ }
+ if (VIR_SOCKET_ADDR_VALID(&def->netmask)) {
+ char *addr = virSocketAddrFormat(&def->netmask);
+ if (!addr)
+ goto error;
+ virBufferAsprintf(buf, " netmask='%s'", addr);
+ VIR_FREE(addr);
+ }
+ if (def->prefix > 0) {
+ virBufferAsprintf(buf," prefix='%u'", def->prefix);
+ }
+ if (VIR_SOCKET_ADDR_VALID(&def->gateway)) {
+ char *addr = virSocketAddrFormat(&def->gateway);
+ if (!addr)
+ goto error;
+ virBufferAsprintf(buf, " gateway='%s'", addr);
+ VIR_FREE(addr);
+ }
+ virBufferAddLit(buf, " />\n");
Remove the extra space before the "/" (don't forget to update the test
cases accordingly)
+
+ result = 0;
+error:
+ return result;
+}
+
+static int
virPortGroupDefFormat(virBufferPtr buf,
const virPortGroupDefPtr def)
{
@@ -2376,6 +2703,11 @@ virNetworkDefFormatInternal(virBufferPtr buf,
goto error;
}
+ for (ii = 0; ii < def->nroutes; ii++) {
+ if (virNetworkRouteDefFormat(buf, &def->routes[ii]) < 0)
+ goto error;
+ }
+
if (virNetDevVPortProfileFormat(def->virtPortProfile, buf) < 0)
goto error;
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index 6ce9a00..37e7858 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -135,6 +135,23 @@ struct _virNetworkIpDef {
virSocketAddr bootserver;
};
+typedef struct _virNetworkRouteDef virNetworkRouteDef;
+typedef virNetworkRouteDef *virNetworkRouteDefPtr;
+struct _virNetworkRouteDef {
+ char *family; /* ipv4 or ipv6 - default is ipv4 */
+ virSocketAddr address; /* Routed Network IP address */
+
+ /* One or the other of the following two will be used for a given
+ * Network address, but never both. The parser guarantees this.
+ * The virSocketAddrGetIpPrefix() can be used to get a
+ * valid prefix.
+ */
+ unsigned int prefix; /* ipv6 - only prefix allowed */
+ virSocketAddr netmask; /* ipv4 - either netmask or prefix specified */
+
+ virSocketAddr gateway; /* gateway IP address for ip-route */
+ };
+
typedef struct _virNetworkForwardIfDef virNetworkForwardIfDef;
typedef virNetworkForwardIfDef *virNetworkForwardIfDefPtr;
struct _virNetworkForwardIfDef {
@@ -209,6 +226,9 @@ struct _virNetworkDef {
size_t nips;
virNetworkIpDefPtr ips; /* ptr to array of IP addresses on this network */
+ size_t nroutes;
+ virNetworkRouteDefPtr routes; /* ptr to array of static routes on this interface */
+
virNetworkDNSDef dns; /* dns related configuration */
virNetDevVPortProfilePtr virtPortProfile;
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index ade4525..987fe64 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1466,6 +1466,7 @@ virNetDevReplaceMacAddress;
virNetDevReplaceNetConfig;
virNetDevRestoreMacAddress;
virNetDevRestoreNetConfig;
+virNetDevSetGateway;
virNetDevSetIPv4Address;
virNetDevSetMAC;
virNetDevSetMTU;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index e8b314a..39f97d1 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2360,6 +2360,7 @@ out:
return ret;
}
+/* add an IP address to a bridge */
static int
networkAddAddrToBridge(virNetworkObjPtr network,
virNetworkIpDefPtr ipdef)
@@ -2380,6 +2381,30 @@ networkAddAddrToBridge(virNetworkObjPtr network,
return 0;
}
+/* add an IP (static) route to a bridge */
+static int
+networkAddRouteToBridge(virNetworkObjPtr network,
+ virNetworkRouteDefPtr routedef)
+{
+ int prefix = virSocketAddrGetIpPrefix(&routedef->address,
+ &routedef->netmask,
+ routedef->prefix);
+
+ if (prefix < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("bridge '%s' has an invalid netmask or IP address
for route definition"),
+ network->def->bridge);
+ return -1;
+ }
+
+ if (virNetDevSetGateway(network->def->bridge,
+ &routedef->address,
+ prefix,
+ &routedef->gateway) < 0)
+ return -1;
+ return 0;
+}
+
static int
networkStartNetworkVirtual(struct network_driver *driver,
virNetworkObjPtr network)
@@ -2388,6 +2413,7 @@ networkStartNetworkVirtual(struct network_driver *driver,
bool v4present = false, v6present = false;
virErrorPtr save_err = NULL;
virNetworkIpDefPtr ipdef;
+ virNetworkRouteDefPtr routedef;
char *macTapIfName = NULL;
int tapfd = -1;
@@ -2446,6 +2472,7 @@ networkStartNetworkVirtual(struct network_driver *driver,
if (networkAddIptablesRules(driver, network) < 0)
goto err1;
+ /* first, do all of the real addresses */
That comment just confuses - what is "real address"? (this is probably
an artifact of earlier versions of the patch where you re-used <ip> for
routes).
for (ii = 0;
(ipdef = virNetworkDefGetIpByIndex(network->def, AF_UNSPEC, ii));
ii++) {
@@ -2464,6 +2491,20 @@ networkStartNetworkVirtual(struct network_driver *driver,
if (virNetDevSetOnline(network->def->bridge, 1) < 0)
goto err2;
+ /* now, do all of the static routes */
Now that routes have their own list, that comment is redundant as well
(the code self-documents)
+ for (ii = 0; ii < network->def->nroutes; ii++) {
+ routedef = &network->def->routes[ii];
+ /* Add the IP route to the bridge */
+ /* ignore errors, error msg will be generated */
+ /* but libvirt will not know and net-destroy will work. */
+ if (VIR_SOCKET_ADDR_VALID(&routedef->gateway)) {
+ if (networkAddRouteToBridge(network, routedef) < 0) {
+ /* an error occurred adding the static route */
+ continue; /* for now, do nothing */
+ }
+ }
+ }
+
/* If forward.type != NONE, turn on global IP forwarding */
if (network->def->forward.type != VIR_NETWORK_FORWARD_NONE &&
networkEnableIpForwarding(v4present, v6present) < 0) {
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 00e0f94..e4b5358 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -769,6 +769,50 @@ cleanup:
}
/**
+ * virNetDevSetGateway:
+ * @ifname: the interface name
+ * @addr: the IP network address (IPv4 or IPv6)
+ * @prefix: number of 1 bits in the netmask
+ * @gateway: via address for route (same as @addr)
+ *
+ * Add a route for a network IP address to an interface. This function
+ * *does not* remove any previously added IP static routes.
+ *
+ * Returns 0 in case of success or -1 in case of error.
+ */
+
+int virNetDevSetGateway(const char *ifname,
+ virSocketAddrPtr addr,
+ unsigned int prefix,
+ virSocketAddrPtr gateway)
+{
+ virCommandPtr cmd = NULL;
+ char *addrstr = NULL, *gatewaystr = NULL;
+ int ret = -1;
+
+ if (!(addrstr = virSocketAddrFormat(addr)))
+ goto cleanup;
+ if (!(gatewaystr = virSocketAddrFormat(gateway)))
+ goto cleanup;
+ cmd = virCommandNew(IP_PATH);
+ virCommandAddArgList(cmd, "route", "add", NULL);
+ virCommandAddArgFormat(cmd, "%s/%u", addrstr, prefix);
+ virCommandAddArgList(cmd, "via", gatewaystr, "dev", ifname,
+ "proto", "static", "metric",
NULL);
+ virCommandAddArgFormat(cmd, "%u", 1);
+
+ if (virCommandRun(cmd, NULL) < 0)
+ goto cleanup;
+
+ ret = 0;
+cleanup:
+ VIR_FREE(addrstr);
+ VIR_FREE(gatewaystr);
+ virCommandFree(cmd);
+ return ret;
+}
+
+/**
* virNetDevClearIPv4Address:
* @ifname: the interface name
* @addr: the IP address (IPv4 or IPv6)
diff --git a/src/util/virnetdev.h b/src/util/virnetdev.h
index 06d0650..07e2e63 100644
--- a/src/util/virnetdev.h
+++ b/src/util/virnetdev.h
@@ -42,6 +42,11 @@ int virNetDevSetIPv4Address(const char *ifname,
virSocketAddr *addr,
unsigned int prefix)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
+int virNetDevSetGateway(const char *ifname,
+ virSocketAddrPtr addr,
+ unsigned int prefix,
+ virSocketAddrPtr gateway)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4)
ATTRIBUTE_RETURN_CHECK;
int virNetDevClearIPv4Address(const char *ifname,
virSocketAddr *addr,
unsigned int prefix)
diff --git a/tests/networkxml2xmlin/dhcp6host-routed-network.xml
b/tests/networkxml2xmlin/dhcp6host-routed-network.xml
index 2693d87..991ce19 100644
--- a/tests/networkxml2xmlin/dhcp6host-routed-network.xml
+++ b/tests/networkxml2xmlin/dhcp6host-routed-network.xml
@@ -19,4 +19,6 @@
<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>
+ <route address="192.168.222.0" netmask="255.255.255.0"
gateway="192.168.122.10" />
+ <route family="ipv6" address="2001:db8:ac10:fc00::"
prefix="64" gateway="2001:db8:ac10:fd01::1:24" />
</network>
diff --git a/tests/networkxml2xmlout/dhcp6host-routed-network.xml
b/tests/networkxml2xmlout/dhcp6host-routed-network.xml
index 7305043..90a0e0a 100644
--- a/tests/networkxml2xmlout/dhcp6host-routed-network.xml
+++ b/tests/networkxml2xmlout/dhcp6host-routed-network.xml
@@ -21,4 +21,6 @@
<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>
+ <route address='192.168.222.0' netmask='255.255.255.0'
gateway='192.168.122.10' />
+ <route family='ipv6' address='2001:db8:ac10:fc00::'
prefix='64' gateway='2001:db8:ac10:fd01::1:24' />
</network>
This is *very* close!
2:13 p.m.
New subject: [libvirt] [PATCH-v4 2/2] Support for static routes on a virtual bridge
On 04/22/2013 11:59 AM, Laine Stump wrote:
address should be optional unless prefix or netmask is non-0,
although
I've now noticed that won't be handled properly due to
virSocketAddrGetIpPrefix returning -1 when there is no address or prefix
or netmask (I'm fixing that before I push that patch, so you can just
toss your 1/2 patch, rebase, and assume it's fixed).
I have most of the stuff
reworked except for the address, gateway,
netmask, and prefix code. Getting all of those balanced so they work
correctly is a bit tricky..
1. For <route>, I am requiring that both address= and gateway= be
specified with address='0.0.0.0' and address='::' being valid
addresses. For IPv4, netmask='0.0.0.0' works correctly but prefix=0
does not.
For IPv4, address='0.0.0.0' results in a default route. I am not sure
what all these extra default routes are going to do to things but lets
not get in the way of the experimenter.
For IPv6, this address='::', prefix='0' is a slightly different matter
as default routes are usually handled differently. I am going to go
ahead and implement it but I am not sure it is a good idea. "Normally,"
if you do not specify a prefix for IPv6, the default is 64. But if you
do specify one, then it will be used.
It is getting real close and it should be ready "real soon now" ;))
Gene
3:13 p.m.
New subject: [libvirt] [PATCH-v4 2/2] Support for static routes on a virtual bridge
On 04/25/2013 03:13 PM, Gene Czarcinski wrote:
On 04/22/2013 11:59 AM, Laine Stump wrote:
> address should be optional unless prefix or netmask is non-0, although
> I've now noticed that won't be handled properly due to
> virSocketAddrGetIpPrefix returning -1 when there is no address or prefix
> or netmask (I'm fixing that before I push that patch, so you can just
> toss your 1/2 patch, rebase, and assume it's fixed).
I have most of the stuff reworked except for the address, gateway,
netmask, and prefix code. Getting all of those balanced so they work
correctly is a bit tricky..
1. For <route>, I am requiring that both address= and gateway= be
specified with address='0.0.0.0' and address='::' being valid
addresses. For IPv4, netmask='0.0.0.0' works correctly but prefix=0
does not.
For IPv4, address='0.0.0.0' results in a default route. I am not sure
what all these extra default routes are going to do to things but lets
not get in the way of the experimenter.
For IPv6, this address='::', prefix='0' is a slightly different matter
as default routes are usually handled differently. I am going to go
ahead and implement it but I am not sure it is a good idea.
"Normally," if you do not specify a prefix for IPv6, the default is
64. But if you do specify one, then it will be used.
It is getting real close and it should be ready "real soon now" ;))
AARRRRGH!!!!
With IPv4 using address='0.0.0.0' and netmask='0.0.0.0' things work just
fine but with prefix not so much. The problem is that with prefix=0, it
is not in the xml which then results it it defaulting at a later time.
This is an extreme corner case. Usually a zero prefix is just ignored.
Gene
3:34 p.m.
New subject: [libvirt] [PATCH-v4 2/2] Support for static routes on a virtual bridge
On 04/25/2013 02:13 PM, Gene Czarcinski wrote:
> It is getting real close and it should be ready "real soon
now" ;))
>
AARRRRGH!!!!
With IPv4 using address='0.0.0.0' and netmask='0.0.0.0' things work just
fine but with prefix not so much. The problem is that with prefix=0, it
is not in the xml which then results it it defaulting at a later time.
Is it getting defaulted back to 0? In the past, if we have XML elements
that can validly be 0, but where 0 is not the default, it works to add a
witness field (has_prefix); if has_prefix is true, then use prefix
instead of defaulting to a non-zero prefix that we normally use. Yeah,
it's a bit of a pain, but it's not the first time.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
3:37 p.m.
New subject: [libvirt] [PATCH-v4 2/2] Support for static routes on a virtual bridge
On 04/25/2013 04:13 PM, Gene Czarcinski wrote:
On 04/25/2013 03:13 PM, Gene Czarcinski wrote:
> On 04/22/2013 11:59 AM, Laine Stump wrote:
>> address should be optional unless prefix or netmask is non-0, although
>> I've now noticed that won't be handled properly due to
>> virSocketAddrGetIpPrefix returning -1 when there is no address or
>> prefix
>> or netmask (I'm fixing that before I push that patch, so you can just
>> toss your 1/2 patch, rebase, and assume it's fixed).
> I have most of the stuff reworked except for the address, gateway,
> netmask, and prefix code. Getting all of those balanced so they work
> correctly is a bit tricky..
>
> 1. For <route>, I am requiring that both address= and gateway= be
> specified with address='0.0.0.0' and address='::' being valid
> addresses. For IPv4, netmask='0.0.0.0' works correctly but prefix=0
> does not.
>
> For IPv4, address='0.0.0.0' results in a default route. I am not
> sure what all these extra default routes are going to do to things
> but lets not get in the way of the experimenter.
>
> For IPv6, this address='::', prefix='0' is a slightly different
> matter as default routes are usually handled differently. I am going
> to go ahead and implement it but I am not sure it is a good idea.
> "Normally," if you do not specify a prefix for IPv6, the default is
> 64. But if you do specify one, then it will be used.
>
> It is getting real close and it should be ready "real soon now" ;))
>
AARRRRGH!!!!
With IPv4 using address='0.0.0.0' and netmask='0.0.0.0' things work
just fine but with prefix not so much. The problem is that with
prefix=0, it is not in the xml which then results it it defaulting at
a later time. This is an extreme corner case. Usually a zero prefix
is just ignored.
Defaulting to what? I thought that when I pushed the utility function
for that, I modified it to return a prefix of 0 if the address was
0.0.0.0 and neither netmask nor prefix was set.
I guess it might be problematic if address was *not* 0 and you wanted an
explicit 0 prefix, but I don't think that would ever be useful.
If you really want prefix to show up in the xml if someone explicitly
puts "prefix='0'" in there, you can add a "bool
prefix_specified;" to
the object, and set that when you see a prefix, even if it's 0. Then in
the formatter you'll know that you should write out the value of prefix,
even if it's 0.
There are a few examples of doing this in either the network or domain
xml parser/formatter - just search for occurrences of the word
"_specified" in src/conf/*.[ch] and you'll find them.
8:16 a.m.
New subject: [libvirt] [PATCH-v4 2/2] Support for static routes on a virtual bridge
On 04/25/2013 04:37 PM, Laine Stump wrote:
On 04/25/2013 04:13 PM, Gene Czarcinski wrote:
> On 04/25/2013 03:13 PM, Gene Czarcinski wrote:
>> On 04/22/2013 11:59 AM, Laine Stump wrote:
>>> address should be optional unless prefix or netmask is non-0, although
>>> I've now noticed that won't be handled properly due to
>>> virSocketAddrGetIpPrefix returning -1 when there is no address or
>>> prefix
>>> or netmask (I'm fixing that before I push that patch, so you can just
>>> toss your 1/2 patch, rebase, and assume it's fixed).
>> I have most of the stuff reworked except for the address, gateway,
>> netmask, and prefix code. Getting all of those balanced so they work
>> correctly is a bit tricky..
>>
>> 1. For <route>, I am requiring that both address= and gateway= be
>> specified with address='0.0.0.0' and address='::' being valid
>> addresses. For IPv4, netmask='0.0.0.0' works correctly but prefix=0
>> does not.
>>
>> For IPv4, address='0.0.0.0' results in a default route. I am not
>> sure what all these extra default routes are going to do to things
>> but lets not get in the way of the experimenter.
>>
>> For IPv6, this address='::', prefix='0' is a slightly different
>> matter as default routes are usually handled differently. I am going
>> to go ahead and implement it but I am not sure it is a good idea.
>> "Normally," if you do not specify a prefix for IPv6, the default is
>> 64. But if you do specify one, then it will be used.
>>
>> It is getting real close and it should be ready "real soon now" ;))
>>
> AARRRRGH!!!!
>
> With IPv4 using address='0.0.0.0' and netmask='0.0.0.0' things work
> just fine but with prefix not so much. The problem is that with
> prefix=0, it is not in the xml which then results it it defaulting at
> a later time. This is an extreme corner case. Usually a zero prefix
> is just ignored.
Defaulting to what? I thought that when I pushed the utility function
for that, I modified it to return a prefix of 0 if the address was
0.0.0.0 and neither netmask nor prefix was set.
I guess it might be problematic if address was *not* 0 and you wanted an
explicit 0 prefix, but I don't think that would ever be useful.
If you really want prefix to show up in the xml if someone explicitly
puts "prefix='0'" in there, you can add a "bool
prefix_specified;" to
the object, and set that when you see a prefix, even if it's 0. Then in
the formatter you'll know that you should write out the value of prefix,
even if it's 0.
There are a few examples of doing this in either the network or domain
xml parser/formatter - just search for occurrences of the word
"_specified" in src/conf/*.[ch] and you'll find them.
OK, I have been trying all kinds of things in network_conf.c (which I
now need to go back an clean up/simplify) because it is not the
problem. The real problem is in virSocketAddrGetIpPrefix(). For IPv4,
a valid address is '0.0.0.0' and for IPv6 a similar address is '::' and
both are valid addresses. For IPv4, if a netmask of '0.0.0.0' is
specified, things work but a prefix=0 is ignored/overridden. For IPv6,
the prefix=0 is also overriden with the default of 64.
To get this to work, I have to pass INVALID IPv4 and IPv6 addresses or
not use the routine if the zero addresses are specified. If the
addresses are zero, then I believe we can correctly assume that a prefix
of zero is also correct. This code needs to be in bridge_driver.c.
Gene
2:54 p.m.
New subject: [libvirt] [PATCH-v4 2/2] Support for static routes on a virtual bridge
On 04/26/2013 09:16 AM, Gene Czarcinski wrote:
On 04/25/2013 04:37 PM, Laine Stump wrote:
> On 04/25/2013 04:13 PM, Gene Czarcinski wrote:
>> On 04/25/2013 03:13 PM, Gene Czarcinski wrote:
>>> On 04/22/2013 11:59 AM, Laine Stump wrote:
>>>> address should be optional unless prefix or netmask is non-0,
>>>> although
>>>> I've now noticed that won't be handled properly due to
>>>> virSocketAddrGetIpPrefix returning -1 when there is no address or
>>>> prefix
>>>> or netmask (I'm fixing that before I push that patch, so you can
just
>>>> toss your 1/2 patch, rebase, and assume it's fixed).
>>> I have most of the stuff reworked except for the address, gateway,
>>> netmask, and prefix code. Getting all of those balanced so they work
>>> correctly is a bit tricky..
>>>
>>> 1. For <route>, I am requiring that both address= and gateway= be
>>> specified with address='0.0.0.0' and address='::' being
valid
>>> addresses. For IPv4, netmask='0.0.0.0' works correctly but prefix=0
>>> does not.
>>>
>>> For IPv4, address='0.0.0.0' results in a default route. I am not
>>> sure what all these extra default routes are going to do to things
>>> but lets not get in the way of the experimenter.
>>>
>>> For IPv6, this address='::', prefix='0' is a slightly
different
>>> matter as default routes are usually handled differently. I am going
>>> to go ahead and implement it but I am not sure it is a good idea.
>>> "Normally," if you do not specify a prefix for IPv6, the default
is
>>> 64. But if you do specify one, then it will be used.
>>>
>>> It is getting real close and it should be ready "real soon now"
;))
>>>
>> AARRRRGH!!!!
>>
>> With IPv4 using address='0.0.0.0' and netmask='0.0.0.0' things
work
>> just fine but with prefix not so much. The problem is that with
>> prefix=0, it is not in the xml which then results it it defaulting at
>> a later time. This is an extreme corner case. Usually a zero prefix
>> is just ignored.
> Defaulting to what? I thought that when I pushed the utility function
> for that, I modified it to return a prefix of 0 if the address was
> 0.0.0.0 and neither netmask nor prefix was set.
>
> I guess it might be problematic if address was *not* 0 and you wanted an
> explicit 0 prefix, but I don't think that would ever be useful.
>
> If you really want prefix to show up in the xml if someone explicitly
> puts "prefix='0'" in there, you can add a "bool
prefix_specified;" to
> the object, and set that when you see a prefix, even if it's 0. Then in
> the formatter you'll know that you should write out the value of prefix,
> even if it's 0.
>
> There are a few examples of doing this in either the network or domain
> xml parser/formatter - just search for occurrences of the word
> "_specified" in src/conf/*.[ch] and you'll find them.
>
>
OK, I have been trying all kinds of things in network_conf.c (which I
now need to go back an clean up/simplify) because it is not the
problem. The real problem is in virSocketAddrGetIpPrefix(). For
IPv4, a valid address is '0.0.0.0' and for IPv6 a similar address is
'::' and both are valid addresses. For IPv4, if a netmask of
'0.0.0.0' is specified, things work but a prefix=0 is
ignored/overridden. For IPv6, the prefix=0 is also overriden with the
default of 64.
To get this to work, I have to pass INVALID IPv4 and IPv6 addresses or
not use the routine if the zero addresses are specified. If the
addresses are zero, then I believe we can correctly assume that a
prefix of zero is also correct. This code needs to be in
bridge_driver.c.
OK, here is the "latest".
For IPv4. all this stuff with prefix=0 or netmask=0.0.0.0 and
address=0.0.0.0 works.
For IPv6, anything involving prefix=0 does not appear to work. In fact,
it gets an error in syslog (you know, one of those conditions I
mentioned that are best ignored so that the bridge interface does not
get totally hosed). Here is the message that appears in syslog:
---------------------------------
Apr 26 15:09:57 falcon libvirtd: 18332: error : virCommandWait:2315 :
internal error Child process (/usr/sbin/ip route add ::/0 via
fd00:beef:10:93::2:5 dev virbr15 proto static metric 1) unexpected exit
status 2: RTNETLINK answers: File exists
--------------------------------
Other IPv6 static routes seem to work just fine.
I was able to replicate the error by having a network definition with
two identical ipv6 static route definitions. However, the first IPv6
route showed up in "ip -6 route".
Anyway, at this point I believe I have two choices:
1. proceed and ignore the IPv6 ::/0 error (IMHO: bad idea).
2. For ::/0, do not do the static route but instead issue an error
message that states this is not supported (my preferred solution).
Comments? Suggestion?? Direction??
Gene
4228
days inactive
4234
days old
11 comments
3 participants
participants (3)
-
Eric Blake -
Gene Czarcinski -
Laine Stump