On 05/07/2013 01:42 PM, Gene Czarcinski wrote:
> network: static route support for <network>
>
> This update includes Laine Stump's comments/suggestions. Once
> again he has improved my over-engineered solutions. My original
> patch and his patch have been squashed/merged with this being the
> result. That plus a little simplified code added to
> bridge_driver.c to handle the zero address situations.
>
> This code does not restrict any combination of
> address/netmask/prefix for a static route specification. The
> metric attribute has been added so that existing route specifications
> can be overridden.
>
> Documentation has been updated to reflect a v1.0.6 target.
>
> 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
> "address/(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.
>
> The command used is of the following form:
>
> ip route add <address>/<prefix> via <gateway> dev <virbr-bridge> \
> proto static metric 1
This should say "metric <metric>".
>
> For family='ipv4' address='0.0.0.0' netmask='0.0.0.0' or prefix='0' is
> supported.
>
> For family='ipv6' address='::' prefix=0' is supported.
>
> Anytime an attempt is made to define a static route which duplicates
> an existing static route (for example, address=::, prefix=0, metric=1),
> the following error message will be sent to syslog:
> RTNETLINK answers: File exists
>
> This can be overridden by increasing the value of metric. Caution should
> be used when doing this ... especially for a default route.
>
> Note: The use of the command-line interface should be replaced by
> direct use of libnl so that error conditions can be handled better. But,
> that is being left as an exersize for another day.
exercise
> .
> Signed-off-by: Gene Czarcinski <gene@xxxxxxxxx>
> ---
> docs/formatnetwork.html.in | 77 +++++
> docs/schemas/network.rng | 22 ++
> src/conf/network_conf.c | 340 ++++++++++++++++++++-
> src/conf/network_conf.h | 22 ++
> src/libvirt_private.syms | 1 +
> src/network/bridge_driver.c | 64 ++++
> src/util/virnetdev.c | 46 +++
> src/util/virnetdev.h | 7 +
> .../networkxml2xmlin/dhcp6host-routed-network.xml | 2 +
> .../networkxml2xmlout/dhcp6host-routed-network.xml | 2 +
> 10 files changed, 582 insertions(+), 1 deletion(-)
>
> diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in
> index d72bd0a..4c7e74f 100644
> --- a/docs/formatnetwork.html.in
> +++ b/docs/formatnetwork.html.in
> @@ -546,6 +546,56 @@
> starting.
> </p>
>
> + <h5><a name="elementsStaticroute">Static Routes</a></h5>
> + <p>
> + 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 so that such
which are not directly reachable from the virtualization host, but *are*
reachable from a guest domain that is itself reachable from the host.
> + networks can be reachable from the virtualization
> + host <span class="since">Since 1.0.6</span>.
> + </p>
> +
> + <p>
> + As shown in <a href="formatnetwork.html#examplesNoGateway">this example</a>,
> + it is possible to define a virtual bridge interface with no
s/virtual bridge/virtual network/
> + IPv4 or IPv6 networks. Such interfaces are useful in supporting
s/networks/addresses/
s/interfaces/networks/
> + 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.
which are only reachable via a guest.
> A guest
> + with connectivity to the guest-only network and another network
"with connectivity *both* to .... and ...."
> + that is directly reachable from the host can act as a gateway between the
> + networks. A static route added to the "visible" network definition
"host-visible"
> + 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. Note
> + that the third static route specification includes the
> + <code>metric</code> attribute specification with a value of 2.
> + This indicates that this particular definition is overriding another
> + route definition with the same address and prefix but with a lower
> + value for the metric.
Actually it's the opposite - the lower the metric, the more preferred it
is. You can think of the metric as the "cost" of travelling in this
direction.
If someone wanted to have a route that automatically overrode an
existing route when the libvirt network was started, they would need to
setup their system config to give the "permanent" route a higher metric.
> + </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 family="ipv6" address="2001:db9:4:1::" prefix="64" gateway="2001:db8:ca2:2::3" metric='2'>
> + </route>
> + ...
> + </pre>
> +
> <h3><a name="elementsAddress">Addressing</a></h3>
>
> <p>
> @@ -577,6 +627,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>
> @@ -826,6 +877,32 @@
> </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
s/visable/visible/
> + the bridge interface but will have a static route defined for this
> + network via the specified gateway. Note that the gateway address
> + 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>
s/1.0.5/1.0.6/
> + </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 493edae..ded8580 100644
> --- a/docs/schemas/network.rng
> +++ b/docs/schemas/network.rng
> @@ -316,6 +316,28 @@
> </optional>
> </element>
> </zeroOrMore>
> + <!-- <route> element -->
> + <zeroOrMore>
> + <!-- The (static) route element specifies a network address and gateway
> + address to access that network. Both the network address and
> + the gateway address must be specified. -->
> + <element name="route">
> + <optional>
> + <attribute name="family"><ref name="addr-family"/></attribute>
> + </optional>
> + <attribute name="address"><ref name="ipAddr"/></attribute>
> + <optional>
> + <choice>
> + <attribute name="netmask"><ref name="ipv4Addr"/></attribute>
> + <attribute name="prefix"><ref name="ipPrefix"/></attribute>
> + </choice>
> + </optional>
> + <attribute name="gateway"><ref name="ipAddr"/></attribute>
> + <optional>
> + <attribute name="metric"><ref name="unsignedInt"/></attribute>
> + </optional>
> + </element>
> + </zeroOrMore>
> </interleave>
> </element>
> </define>
> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
> index 8abfa53..12ac879 100644
> --- a/src/conf/network_conf.c
> +++ b/src/conf/network_conf.c
> @@ -143,6 +143,12 @@ virNetworkIpDefClear(virNetworkIpDefPtr def)
> }
>
> static void
> +virNetworkRouteDefClear(virNetworkRouteDefPtr def)
> +{
> + VIR_FREE(def->family);
> +}
> +
> +static void
> virNetworkDNSTxtDefClear(virNetworkDNSTxtDefPtr def)
> {
> VIR_FREE(def->name);
> @@ -221,6 +227,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]);
> }
> @@ -1270,6 +1281,219 @@ 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, metric = 0;
> + int result = -1;
> + int prefixRc, metricRc;
> + 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 of network '%s'"),
> + networkName);
> + goto cleanup;
> + }
> + def->has_prefix = (prefixRc == 0);
> + def->prefix = prefix;
> + metricRc = virXPathULong("string(./@metric)", ctxt, &metric);
> + if (metricRc == -2) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("Invalid metric specified "
> + "in route definition of network '%s'"),
> + networkName);
> + goto cleanup;
> + }
> + def->has_metric = (metricRc == 0);
> + def->metric = metric;
> +
> + /* Note: both network and gateway addresses must be specified */
> +
> + if (!address) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("Missing required address attribute "
> + "in route definition of network '%s'"),
> + networkName);
> + goto cleanup;
> + }
> +
> + if (!gateway) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("Missing required gateway attribute "
> + "in route definition of network '%s'"),
> + networkName);
> + goto cleanup;
> + }
> +
> + if (virSocketAddrParse(&def->address, address, AF_UNSPEC) < 0) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("Bad network address '%s' "
> + "in route definition of network '%s'"),
> + address, networkName);
> + goto cleanup;
> + }
> +
> + if (virSocketAddrParse(&def->gateway, gateway, AF_UNSPEC) < 0) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("Bad gateway address '%s' "
> + "in route definition of network '%s'"),
> + gateway, 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,
This should be VIR_ERR_XML_ERROR. CONFIG_UNSUPPORTED is reserved for
cases where a configuration is theoretically okay, but this particular
driver doesn't support it. Same comment applies for all the other uses
of VIR_ERR_CONFIG_UNSUPPORTED in this patch.
> + _("%s family specified for non-IPv4 address '%s' "
> + "in route definition of network '%s'"),
> + def->family == NULL? "no" : "ipv4",
> + address, networkName);
> + goto cleanup;
> + }
> + if (!VIR_SOCKET_ADDR_IS_FAMILY(&def->gateway, AF_INET)) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("%s family specified for non-IPv4 gateway '%s' "
> + "in route definition of network '%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 of network '%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 (def->has_prefix) {
> + /* 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;
> + }
> + }
> + if (def->prefix > 32) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("Invalid IPv4 prefix %u for '%s' specified "
> + "in route definition of network '%s'"),
> + def->prefix,
> + def->family == NULL? "no" : "ipv4", networkName);
This should really just say "Invalid prefix %u specified in route
definition of network '%s'".
> + 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 of network '%s'"),
> + address, networkName);
> + goto cleanup;
> + }
> + if (netmask) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("specifying netmask invalid for IPv6 address '%s' "
> + "in route definition of network '%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 of network '%s'"),
> + gateway, networkName);
> + goto cleanup;
> + }
> + if (def->prefix > 128) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("Invalid IPv6 prefix %u for '%s' specified "
> + "in route definition of network '%s'"),
> + def->prefix, def->family, networkName);
This one should also be simplified.
> + goto cleanup;
> + }
> + } else {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("Unrecognized family '%s' "
> + "in route definition of network'%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,
This is an INTERNAL error - we've already validated address and netmask,
and assured that both of them are IPv4, so there shouldn't be any reason
for this to fail.
> + _("error converting address '%s' with netmask '%s' "
> + "to network-address "
> + "in route definition of network '%s'"),
> + address, netmask, networkName);
> + goto cleanup;
> + }
> + } else {
> + if (virSocketAddrMaskByPrefix(&def->address,
> + def->prefix, &testAddr) < 0) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
This is also INTERNAL.
> + _("error converting address '%s' with prefix %u "
> + "to network-address "
> + "in route definition of network '%s'"),
> + address, def->prefix, networkName);
> + goto cleanup;
> + }
> + }
> + if (!virSocketAddrEqual(&def->address, &testAddr)) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("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)
> @@ -1684,8 +1908,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;
> @@ -1839,6 +2064,68 @@ 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 verified/assumed valid;
> + * the question being asked and answered here is 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];
> + addrMatch = false;
> + for (jj = 0; jj < nIps; jj++) {
> + virNetworkIpDefPtr def2 = &def->ips[jj];
> + 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;
> + }
> + }
> + }
> +
Hmm. routeNodes is freed in the error case, but not in the normal case.
Following the pattern of the rest of this function, you would need a
VIR_FREE(routeNodes) right here.
> forwardNode = virXPathNode("./forward", ctxt);
> if (forwardNode &&
> virNetworkForwardDefParseXML(def->name, forwardNode, ctxt, &def->forward) < 0) {
> @@ -1911,6 +2198,7 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
> return def;
>
> error:
> + VIR_FREE(routeNodes);
> VIR_FREE(stp);
> virNetworkDefFree(def);
> VIR_FREE(ipNodes);
> @@ -2136,6 +2424,51 @@ 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->has_prefix) {
> + 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);
> + }
> + if (def->has_metric && def->metric > 0) {
> + virBufferAsprintf(buf," metric='%u'", def->metric);
> + }
Ah yes - since a metric of 0 isn't allowed (it's reserved for direct
routes), we need to check for that in the parser.
> + virBufferAddLit(buf, "/>\n");
> +
> + result = 0;
> +error:
> + return result;
> +}
> +
> +static int
> virPortGroupDefFormat(virBufferPtr buf,
> const virPortGroupDefPtr def)
> {
> @@ -2347,6 +2680,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 e187f05..43f80d4 100644
> --- a/src/conf/network_conf.h
> +++ b/src/conf/network_conf.h
> @@ -149,6 +149,25 @@ 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.
> + */
> + virSocketAddr netmask; /* ipv4 - either netmask or prefix specified */
> + unsigned int prefix; /* ipv6 - only prefix allowed */
> + bool has_prefix; /* prefix= was specified */
> + unsigned int metric; /* value for metric (defaults to 1) */
> + bool has_metric; /* metric= was specified */
It's good that you duplicated the has_* idea for this.
> + virSocketAddr gateway; /* gateway IP address for ip-route */
> + };
> +
> typedef struct _virNetworkForwardIfDef virNetworkForwardIfDef;
> typedef virNetworkForwardIfDef *virNetworkForwardIfDefPtr;
> struct _virNetworkForwardIfDef {
> @@ -224,6 +243,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 d4cb4a3..dfa4779 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1463,6 +1463,7 @@ virMacAddrSetRaw;
>
>
> # util/virnetdev.h
> +virNetDevAddRoute;
> virNetDevClearIPv4Address;
> virNetDevExists;
> virNetDevGetIndex;
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index 9c5a8ae..4392e20 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -2391,6 +2391,7 @@ out:
> return ret;
> }
>
> +/* add an IP address to a bridge */
> static int
> networkAddAddrToBridge(virNetworkObjPtr network,
> virNetworkIpDefPtr ipdef)
> @@ -2411,6 +2412,55 @@ networkAddAddrToBridge(virNetworkObjPtr network,
> return 0;
> }
>
> +/* add an IP (static) route to a bridge */
> +static int
> +networkAddRouteToBridge(virNetworkObjPtr network,
> + virNetworkRouteDefPtr routedef)
> +{
> + int prefix = 0;
> + unsigned int metric;
> + virSocketAddrPtr addr = &routedef->address;
> + virSocketAddrPtr mask = &routedef->netmask;
> + virSocketAddr zero;
> +
> + /* this creates an all-0 address of the appropriate family */
> + ignore_value(virSocketAddrParse(&zero,
> + (VIR_SOCKET_ADDR_IS_FAMILY(addr,AF_INET)
> + ? "0.0.0.0" : "::"),
> + VIR_SOCKET_ADDR_FAMILY(addr)));
> +
> + if (virSocketAddrEqual(addr, &zero)) {
> + if (routedef->has_prefix && routedef->prefix == 0)
> + prefix = 0;
> + else if ((VIR_SOCKET_ADDR_IS_FAMILY(mask, AF_INET) &&
> + virSocketAddrEqual(mask, &zero)))
> + prefix = 0;
> + else
> + prefix = virSocketAddrGetIpPrefix(addr, mask, routedef->prefix);
> + } else {
> + prefix = virSocketAddrGetIpPrefix(addr, mask, routedef->prefix);
> + }
Much simpler! :-)
> +
> + if (prefix < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("network '%s' has an invalid netmask "
> + "or IP address in route definition"),
> + network->def->name);
> + return -1;
> + }
> +
> + if (routedef->has_metric && routedef->metric > 0)
> + metric = routedef->metric;
> + else
> + metric = 1;
> +
> + if (virNetDevAddRoute(network->def->bridge, &routedef->address,
> + prefix, &routedef->gateway, metric) < 0) {
> + return -1;
> + }
> + return 0;
> +}
> +
> static int
> networkStartNetworkVirtual(struct network_driver *driver,
> virNetworkObjPtr network)
> @@ -2419,6 +2469,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;
>
> @@ -2495,6 +2546,19 @@ networkStartNetworkVirtual(struct network_driver *driver,
> if (virNetDevSetOnline(network->def->bridge, 1) < 0)
> goto err2;
>
> + 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 d987b8e..01aaff1 100644
> --- a/src/util/virnetdev.c
> +++ b/src/util/virnetdev.c
> @@ -783,6 +783,52 @@ cleanup:
> }
>
> /**
> + * virNetDevSetGateway:
Oops. Forgot to change the name.
> + * @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
> +virNetDevAddRoute(const char *ifname,
> + virSocketAddrPtr addr,
> + unsigned int prefix,
> + virSocketAddrPtr gateway,
> + unsigned int metric)
> +{
> + 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", metric);
> +
> + 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 551ea22..0b394ad 100644
> --- a/src/util/virnetdev.h
> +++ b/src/util/virnetdev.h
> @@ -42,6 +42,13 @@ int virNetDevSetIPv4Address(const char *ifname,
> virSocketAddr *addr,
> unsigned int prefix)
> ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
> +int virNetDevAddRoute(const char *ifname,
> + virSocketAddrPtr addr,
> + unsigned int prefix,
> + virSocketAddrPtr gateway,
> + unsigned int metric)
> + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4)
> + ATTRIBUTE_NONNULL(5) 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..40f6dfe 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 1d3035b..fc8666b 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>
ACK with the above nits fixed. I have fixed them all (see attached
interdiff) and pushed.
Thanks for the contribution!
>From b26c5abbed4756b1c89f749a2844e15202968189 Mon Sep 17 00:00:00 2001
From: Laine Stump <laine@xxxxxxxxx>
Date: Mon, 13 May 2013 15:26:43 -0400
Subject: [PATCH] squash into network route patches
---
docs/formatnetwork.html.in | 61 +++++++++++++++++++++------------------
src/conf/network_conf.c | 71 ++++++++++++++++++++++++++++------------------
src/util/virnetdev.c | 2 +-
3 files changed, 79 insertions(+), 55 deletions(-)
diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in
index 4c7e74f..a1198ce 100644
--- a/docs/formatnetwork.html.in
+++ b/docs/formatnetwork.html.in
@@ -548,25 +548,25 @@
<h5><a name="elementsStaticroute">Static Routes</a></h5>
<p>
- 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 so that such
- networks can be reachable from the virtualization
- host <span class="since">Since 1.0.6</span>.
+ Static route definitions are used to provide routing information
+ to the virtualization host for networks which are not directly
+ reachable from the virtualization host, but *are* reachable from
+ a guest domain that is itself reachable from the
+ host <span class="since">since 1.0.6</span>.
</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.
+ As shown in <a href="formatnetwork.html#examplesNoGateway">this
+ example</a>, it is possible to define a virtual network
+ interface with no IPv4 or IPv6 addresses. Such networks are
+ useful to provide host connectivity to networks which are only
+ reachable via a guest. A guest with connectivity both to the
+ guest-only network and to another network that is directly
+ reachable from the host can act as a gateway between the
+ networks. A static route added to the "host-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>
@@ -576,9 +576,15 @@
<code>gateway</code> gateway address specifications. Note
that the third static route specification includes the
<code>metric</code> attribute specification with a value of 2.
- This indicates that this particular definition is overriding another
- route definition with the same address and prefix but with a lower
- value for the metric.
+ This particular route would *not* be preferred if there was
+ another existing rout on the system with the same address and
+ prefix but with a lower value for the metric. If there is a
+ route in the host system configuration that should be overriden
+ by a route in a virtual network whenever the virtual network is
+ running, the configuration for the system-defined route should
+ be modified to have a higher metric, and the route on the
+ virtual network given a lower metric (for example, the default
+ metric of "1").
</p>
<pre>
@@ -878,14 +884,15 @@
</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 will have a static route defined for this
- network via the specified gateway. Note that the gateway address
- 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>
+ 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
+ directly visible on the bridge interface but there will be a
+ static route defined for this network via the specified
+ gateway. Note that the gateway address 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.6</span>
</p>
<pre>
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index b996fbe..eb10c0a 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -1325,7 +1325,16 @@ virNetworkRouteDefParseXML(const char *networkName,
networkName);
goto cleanup;
}
- def->has_metric = (metricRc == 0);
+ if (metricRc == 0) {
+ def->has_metric = true;
+ if (metric == 0) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("Invalid metric value, must be > 0 "
+ "in route definition of network '%s'"),
+ networkName);
+ goto cleanup;
+ }
+ }
def->metric = metric;
/* Note: both network and gateway addresses must be specified */
@@ -1366,18 +1375,22 @@ virNetworkRouteDefParseXML(const char *networkName,
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' "
+ virReportError(VIR_ERR_XML_ERROR,
+ def->family == NULL ?
+ _("No family specified for non-IPv4 address '%s' "
+ "in route definition of network '%s'") :
+ _("IPv4 family specified for non-IPv4 address '%s' "
"in route definition of network '%s'"),
- def->family == NULL? "no" : "ipv4",
address, networkName);
goto cleanup;
}
if (!VIR_SOCKET_ADDR_IS_FAMILY(&def->gateway, AF_INET)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("%s family specified for non-IPv4 gateway '%s' "
+ virReportError(VIR_ERR_XML_ERROR,
+ def->family == NULL ?
+ _("No family specified for non-IPv4 gateway '%s' "
+ "in route definition of network '%s'") :
+ _("IPv4 family specified for non-IPv4 gateway '%s' "
"in route definition of network '%s'"),
- def->family == NULL? "no" : "ipv4",
address, networkName);
goto cleanup;
}
@@ -1390,56 +1403,57 @@ virNetworkRouteDefParseXML(const char *networkName,
goto cleanup;
}
if (!VIR_SOCKET_ADDR_IS_FAMILY(&def->netmask, AF_INET)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("network '%s' has invalid netmask '%s' "
+ virReportError(VIR_ERR_XML_ERROR,
+ _("Network '%s' has invalid netmask '%s' "
"for address '%s' (both must be IPv4)"),
networkName, netmask, address);
goto cleanup;
}
if (def->has_prefix) {
/* can't have both netmask and prefix at the same time */
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("route definition '%s' cannot have both "
+ virReportError(VIR_ERR_XML_ERROR,
+ _("Route definition '%s' cannot have both "
"a prefix and a netmask"),
networkName);
goto cleanup;
}
}
if (def->prefix > 32) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("Invalid IPv4 prefix %u for '%s' specified "
- "in route definition of network '%s'"),
- def->prefix,
- def->family == NULL? "no" : "ipv4", networkName);
+ virReportError(VIR_ERR_XML_ERROR,
+ _("Invalid prefix %u specified "
+ "in route definition of network '%s', "
+ "must be 0 - 32"),
+ def->prefix, networkName);
goto cleanup;
}
} else if (STREQ(def->family, "ipv6")) {
if (!VIR_SOCKET_ADDR_IS_FAMILY(&def->address, AF_INET6)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ virReportError(VIR_ERR_XML_ERROR,
_("ipv6 family specified for non-IPv6 address '%s' "
"in route definition of network '%s'"),
address, networkName);
goto cleanup;
}
if (netmask) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("specifying netmask invalid for IPv6 address '%s' "
+ virReportError(VIR_ERR_XML_ERROR,
+ _("Specifying netmask invalid for IPv6 address '%s' "
"in route definition of network '%s'"),
address, networkName);
goto cleanup;
}
if (!VIR_SOCKET_ADDR_IS_FAMILY(&def->gateway, AF_INET6)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ virReportError(VIR_ERR_XML_ERROR,
_("ipv6 specified for non-IPv6 gateway address '%s' "
"in route definition of network '%s'"),
gateway, networkName);
goto cleanup;
}
if (def->prefix > 128) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("Invalid IPv6 prefix %u for '%s' specified "
- "in route definition of network '%s'"),
- def->prefix, def->family, networkName);
+ virReportError(VIR_ERR_XML_ERROR,
+ _("Invalid prefix %u specified "
+ "in route definition of network '%s', "
+ "must be 0 - 128"),
+ def->prefix, networkName);
goto cleanup;
}
} else {
@@ -1453,7 +1467,7 @@ virNetworkRouteDefParseXML(const char *networkName,
/* make sure the address is a network address */
if (netmask) {
if (virSocketAddrMask(&def->address, &def->netmask, &testAddr) < 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ virReportError(VIR_ERR_INTERNAL_ERROR,
_("error converting address '%s' with netmask '%s' "
"to network-address "
"in route definition of network '%s'"),
@@ -1463,7 +1477,7 @@ virNetworkRouteDefParseXML(const char *networkName,
} else {
if (virSocketAddrMaskByPrefix(&def->address,
def->prefix, &testAddr) < 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ virReportError(VIR_ERR_INTERNAL_ERROR,
_("error converting address '%s' with prefix %u "
"to network-address "
"in route definition of network '%s'"),
@@ -1472,7 +1486,7 @@ virNetworkRouteDefParseXML(const char *networkName,
}
}
if (!virSocketAddrEqual(&def->address, &testAddr)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ virReportError(VIR_ERR_XML_ERROR,
_("address '%s' in route definition of network '%s' "
"is not a network address"),
address, networkName);
@@ -2125,6 +2139,7 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
}
}
}
+ VIR_FREE(routeNodes);
forwardNode = virXPathNode("./forward", ctxt);
if (forwardNode &&
@@ -2436,6 +2451,7 @@ virNetworkRouteDefFormat(virBufferPtr buf,
}
if (VIR_SOCKET_ADDR_VALID(&def->address)) {
char *addr = virSocketAddrFormat(&def->address);
+
if (!addr)
goto error;
virBufferAsprintf(buf, " address='%s'", addr);
@@ -2443,6 +2459,7 @@ virNetworkRouteDefFormat(virBufferPtr buf,
}
if (VIR_SOCKET_ADDR_VALID(&def->netmask)) {
char *addr = virSocketAddrFormat(&def->netmask);
+
if (!addr)
goto error;
virBufferAsprintf(buf, " netmask='%s'", addr);
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 0f390f4..cee4001 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -822,7 +822,7 @@ cleanup:
}
/**
- * virNetDevSetGateway:
+ * virNetDevAddRoute:
* @ifname: the interface name
* @addr: the IP network address (IPv4 or IPv6)
* @prefix: number of 1 bits in the netmask
--
1.7.11.7
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list