[libvirt] [PATCH] v2:Support for adding a static route to a bridge
This patch adds support for adding a static route for a network. The "via" 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. Extensive tests are done to validate that the input definitions are correct. For example, for a static route ip definition, the address must be for a network and not a host. Additional checks are added to ensure that the specified gateway has 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. 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@czarc.net> --- docs/formatnetwork.html.in | 32 +++- docs/schemas/network.rng | 3 + src/conf/network_conf.c | 168 ++++++++++++++++++++- src/conf/network_conf.h | 2 + src/libvirt_private.syms | 1 + src/network/bridge_driver.c | 42 ++++++ src/util/virnetdev.c | 47 ++++++ src/util/virnetdev.h | 5 + .../networkxml2xmlin/dhcp6host-routed-network.xml | 4 + .../networkxml2xmlout/dhcp6host-routed-network.xml | 4 + 10 files changed, 304 insertions(+), 4 deletions(-) diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in index 4dd0415..f0cadf0 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in @@ -537,7 +537,9 @@ associated with the virtual network, and optionally enable DHCP services. These elements are only valid for isolated networks (no <code>forward</code> element specified), and for those with - a forward mode of 'route' or 'nat'. + a forward mode of 'route' or 'nat'. Another optional addressing + element <code>via</code> can be used to establish a static + route for IPv4 or IPv6 networks. </p> <pre> @@ -560,6 +562,7 @@ </dhcp> </ip> <ip family="ipv6" address="2001:db8:ca2:2::1" prefix="64" /> + <ip family="ipv6" address="2001:db9:ca1:1::" prefix="64" via="2001:db8:ca2:2::2" /> </network></pre> <dl> @@ -633,7 +636,10 @@ IPv6, multiple addresses on a single network, <code>family</code>, and <code>prefix</code> are support <span class="since">Since 0.8.7</span>. Similar to IPv4, one IPv6 address per network can also have - a <code>dhcp</code> definition. <span class="since">Since 1.0.1</span> + a <code>dhcp</code> definition. <span class="since">Since 1.0.1</span> The <code>via</code> + can be used to establish a static route for IPv4 or IPv6 networks. It is an error + to specify <code>via</code> and <code>dhcp</code> for the same IP address. + <span class="since">Since 1.0.4</span> <dl> <dt><code>tftp</code></dt> @@ -809,6 +815,28 @@ </ip> </network></pre> + <p> + Below is yet another IPv6 variation. This variation has only IPv6 + defined with DHCPv6 on the primary IPv6 network. A second IPv6 + network has a static link to a host on the first (virtual) IPv6 + network. <span class="since">Since 1.0.4</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> + <ip family="ipv6" address="2001:db8:ca2:8::" prefix="64" via="2001:db8:ca2::4" > + </ip> + </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..c1cca23 100644 --- a/docs/schemas/network.rng +++ b/docs/schemas/network.rng @@ -262,6 +262,9 @@ <attribute name="family"><ref name="addr-family"/></attribute> </optional> <optional> + <attribute name="via"><ref name="ipAddr"/></attribute> + </optional> + <optional> <element name="tftp"> <attribute name="root"><text/></attribute> </element> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index c022fe4..ea98d85 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -1149,8 +1149,10 @@ virNetworkIPDefParseXML(const char *networkName, xmlNodePtr cur, save; char *address = NULL, *netmask = NULL; - unsigned long prefix; + char *gateway = NULL; + unsigned long prefix = 0; int result = -1; + virSocketAddr testAddr; save = ctxt->node; ctxt->node = node; @@ -1162,6 +1164,7 @@ virNetworkIPDefParseXML(const char *networkName, def->prefix = 0; else def->prefix = prefix; + gateway = virXPathString("string(./@via)", ctxt); netmask = virXPathString("string(./@netmask)", ctxt); @@ -1175,7 +1178,17 @@ virNetworkIPDefParseXML(const char *networkName, } - /* validate family vs. address */ + if (gateway) { + if (virSocketAddrParse(&def->gateway, gateway, AF_UNSPEC) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Bad gateway address '%s' in definition of network '%s'"), + gateway, networkName); + goto cleanup; + } + + } + + /* validate family vs. address (and gateway address too) */ if (def->family == NULL) { if (!(VIR_SOCKET_ADDR_IS_FAMILY(&def->address, AF_INET) || VIR_SOCKET_ADDR_IS_FAMILY(&def->address, AF_UNSPEC))) { @@ -1184,6 +1197,20 @@ virNetworkIPDefParseXML(const char *networkName, address, networkName); goto cleanup; } + if (gateway && + (!(VIR_SOCKET_ADDR_IS_FAMILY(&def->gateway, AF_INET) || + VIR_SOCKET_ADDR_IS_FAMILY(&def->gateway, AF_UNSPEC)))) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("no family specified for non-IPv4 gateway address '%s' in network '%s'"), + address, networkName); + goto cleanup; + } + if (def->prefix > 0 && def->prefix > 32) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("invalid prefix=%u specified for IPv4 address '%s' in network '%s'"), + def->prefix, address, networkName); + goto cleanup; + } } else if (STREQ(def->family, "ipv4")) { if (!VIR_SOCKET_ADDR_IS_FAMILY(&def->address, AF_INET)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -1191,6 +1218,18 @@ virNetworkIPDefParseXML(const char *networkName, address, networkName); goto cleanup; } + if (gateway && (!VIR_SOCKET_ADDR_IS_FAMILY(&def->gateway, AF_INET))) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("family 'ipv4' specified for non-IPv4 gateway address '%s' in network '%s'"), + address, networkName); + goto cleanup; + } + if (def->prefix > 0 && def->prefix > 32) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("invalid prefix=%u specified for IPv4 address '%s' in network '%s'"), + def->prefix, address, networkName); + goto cleanup; + } } else if (STREQ(def->family, "ipv6")) { if (!VIR_SOCKET_ADDR_IS_FAMILY(&def->address, AF_INET6)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -1198,6 +1237,24 @@ virNetworkIPDefParseXML(const char *networkName, address, networkName); goto cleanup; } + if (def->prefix == 0 || def->prefix > 64) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("invalid prefix=%u specified for IPv6 address '%s' in network '%s'"), + def->prefix, address, networkName); + goto cleanup; + } + if (netmask) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("specifying netmask invalid for IPv6 address '%s' in network '%s'"), + address, networkName); + goto cleanup; + } + if (gateway && (!VIR_SOCKET_ADDR_IS_FAMILY(&def->gateway, AF_INET6))) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("family 'ipv6' specified for non-IPv6 gateway address '%s' in network '%s'"), + gateway, networkName); + goto cleanup; + } } else { virReportError(VIR_ERR_XML_ERROR, _("Unrecognized family '%s' in definition of network '%s'"), @@ -1241,16 +1298,55 @@ virNetworkIPDefParseXML(const char *networkName, } } + /* if static route gateway specified, make sure the address is a network address */ + if (gateway) { + if (def->prefix > 0) { + if (virSocketAddrMaskByPrefix(&def->address, def->prefix, &testAddr) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("error converting address '%s' with prefix=%u to network-address in network '%s'"), + address, def->prefix, networkName); + goto cleanup; + } + } + 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 network '%s'"), + address, netmask, networkName); + goto cleanup; + } + } + if (!virSocketAddrEqual(&def->address, &testAddr)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("address '%s' in network '%s' is not a network-address"), + address, networkName); + goto cleanup; + } + } + cur = node->children; while (cur != NULL) { if (cur->type == XML_ELEMENT_NODE && xmlStrEqual(cur->name, BAD_CAST "dhcp")) { + if (gateway) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("<dhcp> element unsupported when static route (via) previously defined in network '%s'"), + networkName); + goto cleanup; + } if (virNetworkDHCPDefParseXML(networkName, cur, def) < 0) goto cleanup; } else if (cur->type == XML_ELEMENT_NODE && xmlStrEqual(cur->name, BAD_CAST "tftp")) { char *root; + if (gateway) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("<tftp> element unsupported when static route (via) previously defined in network '%s'"), + networkName); + goto cleanup; + } + if (!VIR_SOCKET_ADDR_IS_FAMILY(&def->address, AF_INET)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Unsupported <tftp> element in an IPv6 element in network '%s'"), @@ -1274,6 +1370,7 @@ cleanup: } VIR_FREE(address); VIR_FREE(netmask); + VIR_FREE(gateway); ctxt->node = save; return result; @@ -1828,6 +1925,58 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) goto error; def->nips++; } + /* 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 on this bridge + */ + for (ii = 0; ii < nIps; ii++) { + int jj; + virSocketAddr testAddr, testGw; + bool addrMatch; + virNetworkIpDefPtr gwdef = &def->ips[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_VALID(&def2->gateway)) + continue; + 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(&gwdef->gateway, AF_UNSPEC)) && + VIR_SOCKET_ADDR_IS_FAMILY(&def2->address, AF_INET6)) + continue; + if (def2->prefix > 0) { + virSocketAddrMaskByPrefix(&def2->address, def2->prefix, &testAddr); + virSocketAddrMaskByPrefix(&gwdef->gateway, def2->prefix, &testGw); + } + if (VIR_SOCKET_ADDR_VALID(&def2->netmask)) { + virSocketAddrMask(&def2->address, &def2->netmask, &testAddr); + virSocketAddrMask(&gwdef->gateway, &def2->netmask, &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, + _("invalid static route gateway specified: '%s'"), + gw); + VIR_FREE(gw); + goto error; + } + } + } } VIR_FREE(ipNodes); @@ -2108,6 +2257,7 @@ virNetworkIpDefFormat(virBufferPtr buf, const virNetworkIpDefPtr def) { int result = -1; + bool routeFlg = false; virBufferAddLit(buf, "<ip"); @@ -2131,15 +2281,29 @@ virNetworkIpDefFormat(virBufferPtr buf, 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; + routeFlg = true; + virBufferAsprintf(buf, " via='%s'", addr); + VIR_FREE(addr); + } virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 2); if (def->tftproot) { + if (routeFlg) /* this should not occur */ + goto error; /* but in case it does, better to error out */ virBufferEscapeString(buf, "<tftp root='%s' />\n", def->tftproot); } if ((def->nranges || def->nhosts)) { int ii; + + if (routeFlg) /* this should not occur */ + goto error; /* but in case it does, better to error out */ + virBufferAddLit(buf, "<dhcp>\n"); virBufferAdjustIndent(buf, 2); diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index 6ce9a00..91aff5f 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -115,6 +115,8 @@ struct _virNetworkIpDef { char *family; /* ipv4 or ipv6 - default is ipv4 */ virSocketAddr address; /* Bridge IP address */ + virSocketAddr gateway; /* gateway IP address for ip-route */ + /* One or the other of the following two will be used for a given * IP address, but never both. The parser guarantees this. * Use virNetworkIpDefPrefix/virNetworkIpDefNetmask rather diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index ed46479..988b97a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1470,6 +1470,7 @@ virNetDevReplaceMacAddress; virNetDevReplaceNetConfig; virNetDevRestoreMacAddress; virNetDevRestoreNetConfig; +virNetDevSetGateway; virNetDevSetIPv4Address; virNetDevSetMAC; virNetDevSetMTU; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 31c8585..07058f3 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,28 @@ networkAddAddrToBridge(virNetworkObjPtr network, return 0; } +/* add an IP route to a bridge */ +static int +networkAddRouteToBridge(virNetworkObjPtr network, + virNetworkIpDefPtr ipdef) +{ + int prefix = virNetworkIpDefPrefix(ipdef); + + if (prefix < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("bridge '%s' has an invalid netmask or IP address"), + network->def->bridge); + return -1; + } + + if (virNetDevSetGateway(network->def->bridge, + &ipdef->address, + prefix, + &ipdef->gateway) < 0) + return -1; + return 0; +} + static int networkStartNetworkVirtual(struct network_driver *driver, virNetworkObjPtr network) @@ -2446,9 +2469,12 @@ networkStartNetworkVirtual(struct network_driver *driver, if (networkAddIptablesRules(driver, network) < 0) goto err1; + /* first, do all of the addresses */ for (ii = 0; (ipdef = virNetworkDefGetIpByIndex(network->def, AF_UNSPEC, ii)); ii++) { + if (VIR_SOCKET_ADDR_VALID(&ipdef->gateway)) + continue; /* this is a static route specification */ if (VIR_SOCKET_ADDR_IS_FAMILY(&ipdef->address, AF_INET)) v4present = true; if (VIR_SOCKET_ADDR_IS_FAMILY(&ipdef->address, AF_INET6)) @@ -2464,6 +2490,22 @@ networkStartNetworkVirtual(struct network_driver *driver, if (virNetDevSetOnline(network->def->bridge, 1) < 0) goto err2; + /* then do all of the static routes */ + for (ii = 0; + (ipdef = virNetworkDefGetIpByIndex(network->def, AF_UNSPEC, ii)); + 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(&ipdef->gateway)) { + if (networkAddRouteToBridge(network, ipdef) < 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 296871c..c90b3d2 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -729,6 +729,7 @@ int virNetDevGetVLanID(const char *ifname ATTRIBUTE_UNUSED, * Add an IP address to an interface. This function *does not* remove * any previously added IP addresses - that must be done separately with * brDelInetAddress. + * TODO: what is "brDelInetAddress"? * * Returns 0 in case of success or -1 in case of error. */ @@ -769,6 +770,52 @@ 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, + virSocketAddr *addr, + unsigned int prefix, + virSocketAddr *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", NULL); + virCommandAddArgFormat(cmd, "%s", gatewaystr); + virCommandAddArgList(cmd, "dev", ifname, NULL); + virCommandAddArgList(cmd, "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..8b94ea8 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, + virSocketAddr *addr, + unsigned int prefix, + virSocketAddr *gateway) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) 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..dcad62d 100644 --- a/tests/networkxml2xmlin/dhcp6host-routed-network.xml +++ b/tests/networkxml2xmlin/dhcp6host-routed-network.xml @@ -19,4 +19,8 @@ <host id='0:1:0:1:18:aa:62:fe:0:16:3e:44:55:66' name='badbob' ip='2001:db8:ac10:fd01::1:24' /> </dhcp> </ip> + <ip address="192.168.222.0" netmask="255.255.255.0" via="192.168.122.10"> + </ip> + <ip family="ipv6" address="2001:db8:ac10:fc00::" prefix="64" via="2001:db8:ac10:fd01::1:24"> + </ip> </network> diff --git a/tests/networkxml2xmlout/dhcp6host-routed-network.xml b/tests/networkxml2xmlout/dhcp6host-routed-network.xml index 7305043..880c2dd 100644 --- a/tests/networkxml2xmlout/dhcp6host-routed-network.xml +++ b/tests/networkxml2xmlout/dhcp6host-routed-network.xml @@ -21,4 +21,8 @@ <host id='0:1:0:1:18:aa:62:fe:0:16:3e:44:55:66' name='badbob' ip='2001:db8:ac10:fd01::1:24' /> </dhcp> </ip> + <ip address='192.168.222.0' netmask='255.255.255.0' via='192.168.122.10'> + </ip> + <ip family='ipv6' address='2001:db8:ac10:fc00::' prefix='64' via='2001:db8:ac10:fd01::1:24'> + </ip> </network> -- 1.8.1.4
On 03/15/2013 02:10 PM, Gene Czarcinski wrote:
This patch adds support for adding a static route for a network. The "via" 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.
Extensive tests are done to validate that the input definitions are correct. For example, for a static route ip definition, the address must be for a network and not a host. Additional checks are added to ensure that the specified gateway has 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.
The command used is of the following form:
ip route add <address>/<prefix> via <gateway> dev <virbr-bridge> \ proto static metric 1 When you examine that part of the patch in virNetworkIPdefParseXML() in network_conf.c, you will nitice that I have added a large number of tests/check that the data entered is valid. These checks and the associated error messages are intended for the situation where you are using virsh net-edit. As such, I believe these are quite good with respect to providing some guidance to the user.
Unfortuately, these same parse function is used by libvert without virsh and, when that occurs and the network definition (xml) file is found to be flawed, libvert will ignore that network definition. Better diagnostics and recovery is needed here but (currently) it is not clear how to provide it. It would be nice if there was some way to "force" virsh net-edit to edit the xml file regardless of errors which could then be corrected as if a good file was being edited and bad configuration was entered. [any suggestions welcome] Gene
On 03/15/2013 03:48 PM, Gene Czarcinski wrote:
On 03/15/2013 02:10 PM, Gene Czarcinski wrote:
This patch adds support for adding a static route for a network. The "via" 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.
Extensive tests are done to validate that the input definitions are correct. For example, for a static route ip definition, the address must be for a network and not a host. Additional checks are added to ensure that the specified gateway has 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.
The command used is of the following form:
ip route add <address>/<prefix> via <gateway> dev <virbr-bridge> \ proto static metric 1 When you examine that part of the patch in virNetworkIPdefParseXML() in network_conf.c, you will nitice that I have added a large number of tests/check that the data entered is valid. These checks and the associated error messages are intended for the situation where you are using virsh net-edit. As such, I believe these are quite good with respect to providing some guidance to the user.
Unfortuately, these same parse function is used by libvert without virsh and, when that occurs and the network definition (xml) file is found to be flawed, libvert will ignore that network definition. Better diagnostics and recovery is needed here but (currently) it is not clear how to provide it.
It would be nice if there was some way to "force" virsh net-edit to edit the xml file regardless of errors which could then be corrected as if a good file was being edited and bad configuration was entered. [any suggestions welcome]
OK, I believe I have found a way to get things correct and NOT require a libvirtd restart. It does not ijvolve code but instead uses a process of existing functions. Assume you have a network such as net3 defined (there is a net3.xml file) but the network does not show up in a net-list but you know it is there. OK, simply use: virsh net-define /etc/libvirt/qemu/networks/net3.xml If the network definition is good, it will be loaded. If it is not, there a diagnostic message will be issued pointing to the error. Use vi to correct the error and then run net-define again. Repeat until the network is defined. This was obvious after I realized what I could do but not so obvious before that. I am going to add some documentation in the docs/formatnetwork.html.in file to describe this process. Question: should some additional diagnostic messaging be issued when net-edit, net-start, net-destroy, etc. to use net-define or net-create to determine the problem with the network (xml) definition file. Gene
On 03/15/2013 03:48 PM, Gene Czarcinski wrote:
On 03/15/2013 02:10 PM, Gene Czarcinski wrote:
This patch adds support for adding a static route for a network. The "via" 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.
On 03/16/2013 09:32 AM, Gene Czarcinski wrote: ping
On 03/20/2013 10:57 AM, Gene Czarcinski wrote:
On 03/15/2013 03:48 PM, Gene Czarcinski wrote:
On 03/15/2013 02:10 PM, Gene Czarcinski wrote:
This patch adds support for adding a static route for a network. The "via" 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.
On 03/16/2013 09:32 AM, Gene Czarcinski wrote: ping
Sorry, I've had a reply "almost" done for several days, but got pulled away to a fire drill. I'll finish it up and send it as soon as I can.
On 03/20/2013 10:57 AM, Gene Czarcinski wrote:
On 03/15/2013 03:48 PM, Gene Czarcinski wrote:
On 03/15/2013 02:10 PM, Gene Czarcinski wrote:
This patch adds support for adding a static route for a network. The "via" 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.
On 03/16/2013 09:32 AM, Gene Czarcinski wrote: ping Sorry, I've had a reply "almost" done for several days, but got pulled away to a fire drill. I'll finish it up and send it as soon as I can.
On 03/20/2013 11:17 AM, Laine Stump wrote: ping Gene
On 03/15/2013 02:10 PM, Gene Czarcinski wrote:
This patch adds support for adding a static route for a network. The "via" 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.
From reading your earlier messages, my understanding is that the aim of
(First I want to make sure I correctly understand what you're wanting to do, so I'm going to try and explain it in my own words...) this patch is to automatically setup a route to a virtual network whose bridge device has no IP address assigned, and is therefore reachable only via one of the guests, is this correct? In other words, let's say that I have the following two networks defined (using IPv4 and all static IPs for brevity, but the entire discussion is equally applicable to IPv6): <network> <name>netVisibleToHost</name> <bridge name='virbr1'/> <forward mode='route'/> <ip address='192.168.200.1' prefix='24'/> </network> <network> <name>netHiddenFromHost</name> <bridge name='virbr2'/> </network> and you have a guestDirect that has two interfaces: <interface type='network'> <!-- 192.168.200.2/24 --> <source network='netVisibleToHost'/> </interface> <interface type='network'> <!-- 192.168.201.1/24 --> <source network='netHiddenFromHost'/> </interface> and another guestIndirect that has only one interface: <interface type='network'> <!-- 192.168.201.2/24 --> <source network='netHiddenFromHost'/> </interface> Additionally, the default route on guestDirect points to 192.168.200.1, and the default route on guestIndirect points to 192.168.201.1. (Presumably you don't want to simply add an IP address to netHiddenFromHost because (while it would solve your routing problems) it would violate some security policy you've built into your network topology - namely that all traffic to and from netHiddenFromHost *must* go through guestDirect.) Traffic outbound from guestIndirect would have no problem reaching its destination - it would go across netHiddenFromHost to guestDirect (192.168.201.1), which would know to forward it to the host (192.168.200.1) via netVisibleToHost, and the host presumably knows how to get to anywhere. The problem comes when trying to route the *response* destined for 192.168.201.2 (guestIndirect) - the outside world may know that those packets have to be sent to the host, but the host doesn't have a route for 192.168.201.0/24 - only guestDirect does. So, the solution is to add a route on the *host* that points traffic destined for 192.168.201.0/24 to guestDirect, a.k.a. 192.168.200.2. Since there's no place in /etc/sysconfig/network-scripts/route-* to put static routes with destinations that are only reachable through a transient interface such as the bridge devices created by libvirt, the obvious solution is to cause libvirt to add such a route, and the way you propose to do that is to add an <ip> element in the network named "netUnreachable". Am I understanding the issue so far? Assuming that I am, then as far as I can see, the correct place to configure this route isn't in the setup for netHiddenFromHost, but rather in netVisibleToHost - this is more in line with the way static routes are configured in the standard host network config (route-* files), and eliminates the problem that would occur if netHiddenFromHost was started before netVisibleToHost existed (the route would be added pointing at the wrong interface, if at all) So, what I'm proposing is that, to automatically setup the route in the above example, netHiddenFromHost would remain unchanged, and netVisibleToHost would look something like this: <network> <name>netVisibleToHost</name> <bridge name='virbr1'/> <forward mode='route'/> <ip address='192.168.200.1' prefix='24'/> <route address='192.168.201.0' prefix='24' gateway='192.168.200.2'/> </network> The route element could also have a family attribute just as <ip> does (although it's fairly simple to figure out the family just by trying to parse address and/or gateway). You might instead want to make <route> a subelement of <ip>, then validate that the gateway address is directly reachable from the given ip address (i.e. that it's on the same subnet). By putting the configuration here, you could be assured that the interface that will be used for the route will always exist at the time the route is added. Also it is conceptually more similar to the way that the routes in /etc/sysconfig/route-ethX all have gateways that are directly reachable via "ethX". *OR* (following is what I think is a bad idea, but maybe someone can tweak and salvage it :-) Here is an alternate proposal that has the advantage of tying the existence of the static route to the existence of not just the bridge, but of even the guest interface will be the gateway: instead of putting the route in the configuration of the netVisibleToHost network, put it directly in the configuration of the guest interface itself. That way when the guest is started the route will be added, and when the guest is shutdown, the route (which will anyway now be useless) will be removed. That could be added to the <interface> definition something like this: <interface type='network'> <!-- 192.168.200.2/24 --> <source network='netVisibleToHost'/> <route address='192.168.201.0' prefix='24' gateway='192.168.200.2'/> </interface> If this was done correctly, you could even hotplug a guest interface that would then be immediately used for a route, as well as updating an existing guest interface to add a route. The route added would use the information in the <route> element plus the bridge device the guest's tap device was connected to. I do have several reservations about this idea, though 1) Up until now there has been no "pollution" of the guest config with IP-related info, it has all been about hardware. (well, except for filterref parameters...) 2) Even worse than that is that the guest interface config *can't* know the IP address that the guest will use for the interface, yet we would now be hard-coding it into this new <route> element in the guest interface config. Ugh. (Now what would be *really* slick is if we could have routes in <interface> that learned their IP address from nwfilter's IP address snooping! :-) 3) If the interface happens to be connected to a bridge that has no IP address on the host (e.g. the bridge of netHiddenFromHost in the example above), the route will be a failure. 4) Likewise, if the interface is connected with macvtap, again the route would be a failure. So, in the end, the idea of adding <route> to <interface> is probably a no-go. (One upside - if the interface is a hostdev, we could simply add the route with no "dev X" appended, and it *would* work, as long as the host had an interface on the same subnet).
The command used is of the following form:
ip route add <address>/<prefix> via <gateway> dev <virbr-bridge> \ proto static metric 1
It seems to me that the "dev" given here is incorrect. If I'm not mistaken, the route you're adding with your current code will use the device name of the bridge on "netHiddenFromHost" in this command, while what you *really* want is the bridge used for "netVisibleToHost" (which isn't even known/available at the time you start netHiddenFromHost - another reason to do it in one of the ways I'm suggesting).
On 04/02/2013 03:31 PM, Laine Stump wrote:
On 03/15/2013 02:10 PM, Gene Czarcinski wrote:
This patch adds support for adding a static route for a network. The "via" 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.
(First I want to make sure I correctly understand what you're wanting to do, so I'm going to try and explain it in my own words...)
From reading your earlier messages, my understanding is that the aim of this patch is to automatically setup a route to a virtual network whose bridge device has no IP address assigned, and is therefore reachable only via one of the guests, is this correct?
In other words, let's say that I have the following two networks defined (using IPv4 and all static IPs for brevity, but the entire discussion is equally applicable to IPv6):
<network> <name>netVisibleToHost</name> <bridge name='virbr1'/> <forward mode='route'/> <ip address='192.168.200.1' prefix='24'/> </network>
<network> <name>netHiddenFromHost</name> <bridge name='virbr2'/> </network>
and you have a guestDirect that has two interfaces:
<interface type='network'> <!-- 192.168.200.2/24 --> <source network='netVisibleToHost'/> </interface> <interface type='network'> <!-- 192.168.201.1/24 --> <source network='netHiddenFromHost'/> </interface>
and another guestIndirect that has only one interface:
<interface type='network'> <!-- 192.168.201.2/24 --> <source network='netHiddenFromHost'/> </interface>
Additionally, the default route on guestDirect points to 192.168.200.1, and the default route on guestIndirect points to 192.168.201.1.
(Presumably you don't want to simply add an IP address to netHiddenFromHost because (while it would solve your routing problems) it would violate some security policy you've built into your network topology - namely that all traffic to and from netHiddenFromHost *must* go through guestDirect.)
Traffic outbound from guestIndirect would have no problem reaching its destination - it would go across netHiddenFromHost to guestDirect (192.168.201.1), which would know to forward it to the host (192.168.200.1) via netVisibleToHost, and the host presumably knows how to get to anywhere. The problem comes when trying to route the *response* destined for 192.168.201.2 (guestIndirect) - the outside world may know that those packets have to be sent to the host, but the host doesn't have a route for 192.168.201.0/24 - only guestDirect does.
So, the solution is to add a route on the *host* that points traffic destined for 192.168.201.0/24 to guestDirect, a.k.a. 192.168.200.2.
Since there's no place in /etc/sysconfig/network-scripts/route-* to put static routes with destinations that are only reachable through a transient interface such as the bridge devices created by libvirt, the obvious solution is to cause libvirt to add such a route, and the way you propose to do that is to add an <ip> element in the network named "netUnreachable".
Am I understanding the issue so far?
I believe you do understand. I have not read the rest of your message yet but I did want to bring up one point. With IPv4 I can get a lot of this functionality by the cleaver use of NAT but with IPv6 there is no NAT and everything is routed. The big issue with IPv6 (and the reason I did this) is to provide the virtualization host with information needed to route response packets back to the "hidden" guest. Without this, the can can send any packet it wants but will never get a reply. That is the basic problem being addressed.
Assuming that I am, then as far as I can see, the correct place to configure this route isn't in the setup for netHiddenFromHost, but rather in netVisibleToHost - this is more in line with the way static routes are configured in the standard host network config (route-* files), and eliminates the problem that would occur if netHiddenFromHost was started before netVisibleToHost existed (the route would be added pointing at the wrong interface, if at all)
You are correct. The "route-via" must be defined in the "visible" network definition. There is an additional constraint that the gateway address must be in the range of the visible network. IIRC, tere are a couple examples in the code I added for tests.
So, what I'm proposing is that, to automatically setup the route in the above example, netHiddenFromHost would remain unchanged, and netVisibleToHost would look something like this:
<network> <name>netVisibleToHost</name> <bridge name='virbr1'/> <forward mode='route'/> <ip address='192.168.200.1' prefix='24'/> <route address='192.168.201.0' prefix='24' gateway='192.168.200.2'/> </network>
The route element could also have a family attribute just as <ip> does (although it's fairly simple to figure out the family just by trying to parse address and/or gateway). You might instead want to make <route> a subelement of <ip>, then validate that the gateway address is directly reachable from the given ip address (i.e. that it's on the same subnet).
The <route> above is similar to what I implemented. Since I am defining a network which is "somehow" reachable via this network interface, I used the "regular" <ip> element to define the network and then added the "via=" (instead of "gateway="). One reason I chose to use "via=" rather than "gateway=" is that when you run the ip -6 route or ip -4 route you get: <ip-address> via <gateway-address> Here is a part of a definition I have: <network> <name>net7</name> ... <ip family='ipv6' address='fd00:beef:10:7::1' prefix='64'> <dhcp> <range start='fd00:beef:10:7::1:1' end='fd00:beef:10:7::1:f1' /> </dhcp> </ip> <ip family='ipv6' address='fd00:beef:12::' prefix='48' via='fd00:beef:10:7::7:2'> </ip> ... </network> and here is what you get from ip -6 route: fd00:beef:10:7::/64 dev virbr3 proto kernel metric 256 fd00:beef:12::/48 via fd00:beef:10:7::7:2 dev virbr3 proto static metric 1
By putting the configuration here, you could be assured that the interface that will be used for the route will always exist at the time the route is added. Also it is conceptually more similar to the way that the routes in /etc/sysconfig/route-ethX all have gateways that are directly reachable via "ethX".
Absolutely. "Floating" routes make no sense to me. This static route is closely coupled with the (main) address on that virtual network definition and needs to be included in the definition.
*OR* (following is what I think is a bad idea, but maybe someone can tweak and salvage it :-)
I cringe here. Since what you stated above is more or less what I implemented, I am of the opinion that this is the one that makes sense. If you use NetworkManager to define some static routes, those definitions will need to be on the which of the network where the gateway is reachable. Mmmm .. I just noticed that NetworkManager used "Gateway" and not "via" ... maybe I should change.
Here is an alternate proposal that has the advantage of tying the existence of the static route to the existence of not just the bridge, but of even the guest interface will be the gateway: instead of putting the route in the configuration of the netVisibleToHost network, put it directly in the configuration of the guest interface itself. That way when the guest is started the route will be added, and when the guest is shutdown, the route (which will anyway now be useless) will be removed. That could be added to the <interface> definition something like this:
<interface type='network'> <!-- 192.168.200.2/24 --> <source network='netVisibleToHost'/> <route address='192.168.201.0' prefix='24' gateway='192.168.200.2'/> </interface>
If this was done correctly, you could even hotplug a guest interface that would then be immediately used for a route, as well as updating an existing guest interface to add a route. The route added would use the information in the <route> element plus the bridge device the guest's tap device was connected to.
I do have several reservations about this idea, though
1) Up until now there has been no "pollution" of the guest config with IP-related info, it has all been about hardware. (well, except for filterref parameters...)
2) Even worse than that is that the guest interface config *can't* know the IP address that the guest will use for the interface, yet we would now be hard-coding it into this new <route> element in the guest interface config. Ugh. (Now what would be *really* slick is if we could have routes in <interface> that learned their IP address from nwfilter's IP address snooping! :-)
3) If the interface happens to be connected to a bridge that has no IP address on the host (e.g. the bridge of netHiddenFromHost in the example above), the route will be a failure.
4) Likewise, if the interface is connected with macvtap, again the route would be a failure.
So, in the end, the idea of adding <route> to <interface> is probably a no-go.
Yes, nogo with me also. In many respects the route definition is not much different than other network definition included in a virtual network definition except that it is only for routing.
(One upside - if the interface is a hostdev, we could simply add the route with no "dev X" appended, and it *would* work, as long as the host had an interface on the same subnet).
The command used is of the following form:
ip route add <address>/<prefix> via <gateway> dev <virbr-bridge> \ proto static metric 1
It seems to me that the "dev" given here is incorrect. If I'm not mistaken, the route you're adding with your current code will use the device name of the bridge on "netHiddenFromHost" in this command, while what you *really* want is the bridge used for "netVisibleToHost" (which isn't even known/available at the time you start netHiddenFromHost - another reason to do it in one of the ways I'm suggesting).
No, no. The <virbr-bridge> is defininiton the visible virtual network interface. The example I cited about is currently "running" with libvirt which includes the patch. Here is the result for ip -6 addr: 3: virbr3: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 inet6 fd00:beef:10:7::1/64 scope global valid_lft forever preferred_lft forever inet6 fe80::5054:ff:fef8:9ca7/64 scope link valid_lft forever preferred_lft forever So, gateway fd00:beef:10:7::7:2 is covered by network fd00:beef:10:7::/64 which is on device virbr3. The network fd00:beef:12::/48 is routed via fd00:beef:10:7::7:2 on virbr3. I do not know if you have given these patches a try but it does make more sense when you see it work. BTW, one static route for IPv4 and one static route for IPv6 is supported by my "IPv6" updates to virt-manager. I need to do a little more work to add that to the virtual-network-creation-wizard. Gene
On 04/03/2013 04:02 PM, Gene Czarcinski wrote:
On 04/02/2013 03:31 PM, Laine Stump wrote:
On 03/15/2013 02:10 PM, Gene Czarcinski wrote:
This patch adds support for adding a static route for a network. The "via" 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.
(First I want to make sure I correctly understand what you're wanting to do, so I'm going to try and explain it in my own words...)
From reading your earlier messages, my understanding is that the aim of this patch is to automatically setup a route to a virtual network whose bridge device has no IP address assigned, and is therefore reachable only via one of the guests, is this correct?
In other words, let's say that I have the following two networks defined (using IPv4 and all static IPs for brevity, but the entire discussion is equally applicable to IPv6):
<network> <name>netVisibleToHost</name> <bridge name='virbr1'/> <forward mode='route'/> <ip address='192.168.200.1' prefix='24'/> </network>
<network> <name>netHiddenFromHost</name> <bridge name='virbr2'/> </network>
and you have a guestDirect that has two interfaces:
<interface type='network'> <!-- 192.168.200.2/24 --> <source network='netVisibleToHost'/> </interface> <interface type='network'> <!-- 192.168.201.1/24 --> <source network='netHiddenFromHost'/> </interface>
and another guestIndirect that has only one interface:
<interface type='network'> <!-- 192.168.201.2/24 --> <source network='netHiddenFromHost'/> </interface>
Additionally, the default route on guestDirect points to 192.168.200.1, and the default route on guestIndirect points to 192.168.201.1.
(Presumably you don't want to simply add an IP address to netHiddenFromHost because (while it would solve your routing problems) it would violate some security policy you've built into your network topology - namely that all traffic to and from netHiddenFromHost *must* go through guestDirect.)
Traffic outbound from guestIndirect would have no problem reaching its destination - it would go across netHiddenFromHost to guestDirect (192.168.201.1), which would know to forward it to the host (192.168.200.1) via netVisibleToHost, and the host presumably knows how to get to anywhere. The problem comes when trying to route the *response* destined for 192.168.201.2 (guestIndirect) - the outside world may know that those packets have to be sent to the host, but the host doesn't have a route for 192.168.201.0/24 - only guestDirect does.
So, the solution is to add a route on the *host* that points traffic destined for 192.168.201.0/24 to guestDirect, a.k.a. 192.168.200.2.
Since there's no place in /etc/sysconfig/network-scripts/route-* to put static routes with destinations that are only reachable through a transient interface such as the bridge devices created by libvirt, the obvious solution is to cause libvirt to add such a route, and the way you propose to do that is to add an <ip> element in the network named "netUnreachable".
Am I understanding the issue so far?
I believe you do understand.
Except that it's obvious from your response that I misunderstood your patch, and thought that you were trying to make the route to an otherwise unreachable network a part of the unreachable network's config :-) (my defense is that the dual usage of the <ip> element to define a route confused me) Based on my now corrected understanding that the route is added to the config of the network which is directly connected to the gateway (rather than the network *beyond* the gateway), I have two comments/requests: 1) I think a separate <route> element is a better idea than trying to make <ip> dual purpose. Aside from confusing simple-minded people like me, when things are intertwined like that it has the potential to lead to an ambiguous situation further down the road. Also, using a separate <route> element is closer to the system config files as well as more similar to the xml used by the virInterface APIs (aka netcf). 2) Although /sbin/ip uses the term "via", I do think that "gateway" would be the preferred term, since that's been in use for many years with the (admittedly now deprecated) /sbin/route command, as well as what is used in the system ifcfg-* and route-* files. 3) I would prefer to eliminate use of /sbin/ip and do this directly via netlink/libnl. I would be willing to have this done in a separate patch that also re-wrote virNetDev(Set|Clear)IPAddress.
On 04/04/2013 12:08 PM, Laine Stump wrote:
On 04/02/2013 03:31 PM, Laine Stump wrote:
On 03/15/2013 02:10 PM, Gene Czarcinski wrote:
This patch adds support for adding a static route for a network. The "via" 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. (First I want to make sure I correctly understand what you're wanting to do, so I'm going to try and explain it in my own words...)
From reading your earlier messages, my understanding is that the aim of this patch is to automatically setup a route to a virtual network whose bridge device has no IP address assigned, and is therefore reachable only via one of the guests, is this correct?
In other words, let's say that I have the following two networks defined (using IPv4 and all static IPs for brevity, but the entire discussion is equally applicable to IPv6):
<network> <name>netVisibleToHost</name> <bridge name='virbr1'/> <forward mode='route'/> <ip address='192.168.200.1' prefix='24'/> </network>
<network> <name>netHiddenFromHost</name> <bridge name='virbr2'/> </network>
and you have a guestDirect that has two interfaces:
<interface type='network'> <!-- 192.168.200.2/24 --> <source network='netVisibleToHost'/> </interface> <interface type='network'> <!-- 192.168.201.1/24 --> <source network='netHiddenFromHost'/> </interface>
and another guestIndirect that has only one interface:
<interface type='network'> <!-- 192.168.201.2/24 --> <source network='netHiddenFromHost'/> </interface>
Additionally, the default route on guestDirect points to 192.168.200.1, and the default route on guestIndirect points to 192.168.201.1.
(Presumably you don't want to simply add an IP address to netHiddenFromHost because (while it would solve your routing problems) it would violate some security policy you've built into your network topology - namely that all traffic to and from netHiddenFromHost *must* go through guestDirect.)
Traffic outbound from guestIndirect would have no problem reaching its destination - it would go across netHiddenFromHost to guestDirect (192.168.201.1), which would know to forward it to the host (192.168.200.1) via netVisibleToHost, and the host presumably knows how to get to anywhere. The problem comes when trying to route the *response* destined for 192.168.201.2 (guestIndirect) - the outside world may know that those packets have to be sent to the host, but the host doesn't have a route for 192.168.201.0/24 - only guestDirect does.
So, the solution is to add a route on the *host* that points traffic destined for 192.168.201.0/24 to guestDirect, a.k.a. 192.168.200.2.
Since there's no place in /etc/sysconfig/network-scripts/route-* to put static routes with destinations that are only reachable through a transient interface such as the bridge devices created by libvirt, the obvious solution is to cause libvirt to add such a route, and the way you propose to do that is to add an <ip> element in the network named "netUnreachable".
Am I understanding the issue so far? I believe you do understand. OK, I am back from virt-manager. Except that it's obvious from your response that I misunderstood your
On 04/03/2013 04:02 PM, Gene Czarcinski wrote: patch, and thought that you were trying to make the route to an otherwise unreachable network a part of the unreachable network's config :-) (my defense is that the dual usage of the <ip> element to define a route confused me) You were correct to be confused. At first, I thought your suggestions for <route> were just cosmetic but then I realized that, in general, an <ip> definition resulted in a specific ".1" or ":1" device address would be assigned to an interface (bridge). The net address looked similar but there would be no device address assigned to the interface/bridge but there would be an entry for a (static) route.
While the backend will continue to be more or less the same and instantiate the static route, the "front" end will add a new element: <route>. I intend to keep "via" for specifying the gateway rather than using "gateway.". This makes it consistent with the ip-route command. So, to add a static would, the text would look like: <route address='<IPv4-network-addr>' prefix='24' via='<IPv4-gateway-addr>' /> or <route family='ipv6' address='<IPv6-net-addr>' prefix='64' via='<IPv6-gateway-addr>' /> More work: I will need to go back and "fix" the virt-manager code.
Based on my now corrected understanding that the route is added to the config of the network which is directly connected to the gateway (rather than the network *beyond* the gateway), I have two comments/requests:
1) I think a separate <route> element is a better idea than trying to make <ip> dual purpose. Aside from confusing simple-minded people like me, when things are intertwined like that it has the potential to lead to an ambiguous situation further down the road. Also, using a separate <route> element is closer to the system config files as well as more similar to the xml used by the virInterface APIs (aka netcf).
Right on ... see above.
2) Although /sbin/ip uses the term "via", I do think that "gateway" would be the preferred term, since that's been in use for many years with the (admittedly now deprecated) /sbin/route command, as well as what is used in the system ifcfg-* and route-* files.
OK, how about having it both ways. If we can have both mask and prefix, why not both via and gateway. I know gateway has some history attached to it but the new /sbin/ip uses via. I am just trying to keep a line of text being as close to not exceeding 80 characters as practical. Correct me if I am wrong but all of this is suppose to be free-form and this should be valid: <route ip='ipv6' address='fd00:dead:beef:472::1' prefix='64' gateway='fd00:dead:beef:10::2' /> Of course, when it gets written back out by code it will all be on a "single" line. How about one of you other folks chiming in on this. gateway? ... via? ... anybody (besides the two of us) care??
3) I would prefer to eliminate use of /sbin/ip and do this directly via netlink/libnl. I would be willing to have this done in a separate patch that also re-wrote virNetDev(Set|Clear)IPAddress.
This is going to take some research, etc. to do so, yes, lets do this "later" as a separate patch and use what we know works for now (kludgy as it is). At least we are now in agreement as to what the end-point is ;) Gene
On 04/09/2013 04:28 PM, Gene Czarcinski wrote:
OK, how about having it both ways. If we can have both mask and prefix, why not both via and gateway. I know gateway has some history attached to it but the new /sbin/ip uses via. I am just trying to keep a line of text being as close to not exceeding 80 characters as practical. Correct me if I am wrong but all of this is suppose to be free-form and this should be valid:
<route ip='ipv6' address='fd00:dead:beef:472::1' prefix='64' gateway='fd00:dead:beef:10::2' />
Of course, when it gets written back out by code it will all be on a "single" line.
How about one of you other folks chiming in on this. gateway? ... via? ... anybody (besides the two of us) care?? OK, unless someone can present a convincing argument, I am going with "via" and not "gateway". Thus, the general form is: <route family=... address=... prefix=... via=... /> </route>
Why "via" and not "gateway". Well, /sbin/ip uses "via" whereas /sbin/route uses "gateway". If there was a convincing argument to keep gateway instead off via, the /sbin/ip code would be different or would be changed to gateway. BTW, IMHO, netmask could disappear also and have prefix= only. Also, the current implementation enforces that the address specified with via= must be resolvable into a network-address which has been defined for the interface. That is, you cannot point via= off into some address that the virtualization host has no idea where it is. Reworked update "real soon now". Gene
On 04/10/2013 01:38 PM, Gene Czarcinski wrote:
On 04/09/2013 04:28 PM, Gene Czarcinski wrote:
OK, how about having it both ways. If we can have both mask and prefix, why not both via and gateway. I know gateway has some history attached to it but the new /sbin/ip uses via. I am just trying to keep a line of text being as close to not exceeding 80 characters as practical. Correct me if I am wrong but all of this is suppose to be free-form and this should be valid:
<route ip='ipv6' address='fd00:dead:beef:472::1' prefix='64' gateway='fd00:dead:beef:10::2' />
Of course, when it gets written back out by code it will all be on a "single" line.
How about one of you other folks chiming in on this. gateway? ... via? ... anybody (besides the two of us) care?? OK, unless someone can present a convincing argument, I am going with "via" and not "gateway". Thus, the general form is: <route family=... address=... prefix=... via=... /> </route>
Why "via" and not "gateway". Well, /sbin/ip uses "via" whereas /sbin/route uses "gateway". If there was a convincing argument to keep gateway instead off via, the /sbin/ip code would be different or would be changed to gateway. BTW, IMHO, netmask could disappear also and have prefix= only.
Nope. In libvirt *nothing* can ever disappear. We try our hardest to provide 100% backward compatibility for existing applications (and have so far been successful at it).
Also, the current implementation enforces that the address specified with via= must be resolvable into a network-address which has been defined for the interface. That is, you cannot point via= off into some address that the virtualization host has no idea where it is.
Right. It must be directly reachable by the network/interface it's added to.
On 04/09/2013 04:28 PM, Gene Czarcinski wrote:
On 04/04/2013 12:08 PM, Laine Stump wrote:
Except that it's obvious from your response that I misunderstood your patch, and thought that you were trying to make the route to an otherwise unreachable network a part of the unreachable network's config :-) (my defense is that the dual usage of the <ip> element to define a route confused me) You were correct to be confused. At first, I thought your suggestions for <route> were just cosmetic but then I realized that, in general, an <ip> definition resulted in a specific ".1" or ":1" device address would be assigned to an interface (bridge). The net address looked similar but there would be no device address assigned to the interface/bridge but there would be an entry for a (static) route.
While the backend will continue to be more or less the same and instantiate the static route, the "front" end will add a new element: <route>. I intend to keep "via" for specifying the gateway rather than using "gateway.". This makes it consistent with the ip-route command. So, to add a static would, the text would look like: <route address='<IPv4-network-addr>' prefix='24' via='<IPv4-gateway-addr>' /> or <route family='ipv6' address='<IPv6-net-addr>' prefix='64' via='<IPv6-gateway-addr>' />
More work: I will need to go back and "fix" the virt-manager code.
Based on my now corrected understanding that the route is added to the config of the network which is directly connected to the gateway (rather than the network *beyond* the gateway), I have two comments/requests:
1) I think a separate <route> element is a better idea than trying to make <ip> dual purpose. Aside from confusing simple-minded people like me, when things are intertwined like that it has the potential to lead to an ambiguous situation further down the road. Also, using a separate <route> element is closer to the system config files as well as more similar to the xml used by the virInterface APIs (aka netcf).
Right on ... see above.
2) Although /sbin/ip uses the term "via", I do think that "gateway" would be the preferred term, since that's been in use for many years with the (admittedly now deprecated) /sbin/route command, as well as what is used in the system ifcfg-* and route-* files.
OK, how about having it both ways. If we can have both mask and prefix, why not both via and gateway.
Having both mask and prefix was kind of forced by circumstance - when networks were first created, and there was only support for IPv4, the people who came up with the XML chose to use netmask; when I later added IPv6 support, netmask was inappropriate - nobody uses a netmask with IPv6, they always use prefix. Since prefix was now present, I made it also recognized for IPv4, but 1) you can only have one or the other, 2) whichever one you supply is what you get back with dumpxml. That's a different situation from simultaneously adding two attributes that have exactly the same purpose and same data type. BTW, another reason I prefer "gateway" over "via" is that the xml for virInterface (netcf) uses "gateway" for the same information in its <route> element: <interface type="ethernet" name="eth0"> <start mode="onboot"/> <protocol family="ipv4"> <ip address="192.168.21.5" prefix="24"/> <route gateway="192.168.21.1"/> </protocol> </interface>
I know gateway has some history attached to it but the new /sbin/ip uses via.
But remember that the goal isn't necessarily to make the xml be similar to any particular backend, but to make it generic enough that the backend could be replaced by something else. In this case either term would work, but I think "gateway" is more widely used and generally understandable to someone who isn't familiar with /sbin/ip. (I would still prefer if at least one other person voiced an opinion though - I don't want to seem like a "patch bully" :-)
I am just trying to keep a line of text being as close to not exceeding 80 characters as practical.
That's eventually a losing battle :-)
Correct me if I am wrong but all of this is suppose to be free-form and this should be valid:
<route ip='ipv6' address='fd00:dead:beef:472::1' prefix='64'
(I think you meant to say "family='ipv6'" here...)
gateway='fd00:dead:beef:10::2' />
Of course, when it gets written back out by code it will all be on a "single" line.
On 04/10/2013 03:09 PM, Laine Stump wrote:
On 04/09/2013 04:28 PM, Gene Czarcinski wrote:
On 04/04/2013 12:08 PM, Laine Stump wrote:
Except that it's obvious from your response that I misunderstood your patch, and thought that you were trying to make the route to an otherwise unreachable network a part of the unreachable network's config :-) (my defense is that the dual usage of the <ip> element to define a route confused me) You were correct to be confused. At first, I thought your suggestions for <route> were just cosmetic but then I realized that, in general, an <ip> definition resulted in a specific ".1" or ":1" device address would be assigned to an interface (bridge). The net address looked similar but there would be no device address assigned to the interface/bridge but there would be an entry for a (static) route.
While the backend will continue to be more or less the same and instantiate the static route, the "front" end will add a new element: <route>. I intend to keep "via" for specifying the gateway rather than using "gateway.". This makes it consistent with the ip-route command. So, to add a static would, the text would look like: <route address='<IPv4-network-addr>' prefix='24' via='<IPv4-gateway-addr>' /> or <route family='ipv6' address='<IPv6-net-addr>' prefix='64' via='<IPv6-gateway-addr>' />
Based on my now corrected understanding that the route is added to the config of the network which is directly connected to the gateway (rather than the network *beyond* the gateway), I have two comments/requests:
1) I think a separate <route> element is a better idea than trying to make <ip> dual purpose. Aside from confusing simple-minded people like me, when things are intertwined like that it has the potential to lead to an ambiguous situation further down the road. Also, using a separate <route> element is closer to the system config files as well as more similar to the xml used by the virInterface APIs (aka netcf). Right on ... see above. 2) Although /sbin/ip uses the term "via", I do think that "gateway" would be the preferred term, since that's been in use for many years with the (admittedly now deprecated) /sbin/route command, as well as what is used in the system ifcfg-* and route-* files. OK, how about having it both ways. If we can have both mask and
More work: I will need to go back and "fix" the virt-manager code. prefix, why not both via and gateway.
Having both mask and prefix was kind of forced by circumstance - when networks were first created, and there was only support for IPv4, the people who came up with the XML chose to use netmask; when I later added IPv6 support, netmask was inappropriate - nobody uses a netmask with IPv6, they always use prefix. Since prefix was now present, I made it also recognized for IPv4, but 1) you can only have one or the other, 2) whichever one you supply is what you get back with dumpxml. That's a different situation from simultaneously adding two attributes that have exactly the same purpose and same data type. Picture an IPv6 mask. Now picture having to figure out just mean it means. Ugly!
BTW, another reason I prefer "gateway" over "via" is that the xml for virInterface (netcf) uses "gateway" for the same information in its <route> element:
<interface type="ethernet" name="eth0"> <start mode="onboot"/> <protocol family="ipv4"> <ip address="192.168.21.5" prefix="24"/> <route gateway="192.168.21.1"/> </protocol> </interface>
I know gateway has some history attached to it but the new /sbin/ip uses via.
But remember that the goal isn't necessarily to make the xml be similar to any particular backend, but to make it generic enough that the backend could be replaced by something else. In this case either term would work, but I think "gateway" is more widely used and generally understandable to someone who isn't familiar with /sbin/ip.
(I would still prefer if at least one other person voiced an opinion though - I don't want to seem like a "patch bully" :-)
I am just trying to keep a line of text being as close to not exceeding 80 characters as practical.
That's eventually a losing battle :-)
Correct me if I am wrong but all of this is suppose to be free-form and this should be valid:
<route ip='ipv6' address='fd00:dead:beef:472::1' prefix='64'
(I think you meant to say "family='ipv6'" here...)
gateway='fd00:dead:beef:10::2' />
Of course, when it gets written back out by code it will all be on a "single" line.
OK, OK ... I give up. It will be "gateway" rather than "via" in the patch I submit. One reason I am giving up is that while I was creating the formatnetwork.html documentation update, I found that using "via" became a little confusing and that "gateway" made more sense. I must say that I have been impressed with the libvirt development cycle. Releases and updates come out often and they are substantial. Gene
On 03/15/2013 02:10 PM, Gene Czarcinski wrote:
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 296871c..c90b3d2 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -729,6 +729,7 @@ int virNetDevGetVLanID(const char *ifname ATTRIBUTE_UNUSED, * Add an IP address to an interface. This function *does not* remove * any previously added IP addresses - that must be done separately with * brDelInetAddress. + * TODO: what is "brDelInetAddress"?
That is a left-over comment from long ago. The function has been renamed to virNetDevClearIPv4Address. (A very useful separate patch would be one to rename virNetDev(Set|Clear)IPv4Address to virNetDev(Set|Clear)IPAddress (since they really *can* be (and are being) used to set both IPv4 and IPv6 addresses), then reimplement them using netlink/libnl calls. Likewise, it would be much nicer if virNetDevSetGateway() was implemented using netlink/libnl.
* * Returns 0 in case of success or -1 in case of error. */ @@ -769,6 +770,52 @@ cleanup: }
/** + * virNetDevSetGateway:
I'm thinking maybe this would be better named "virNetDevAddRoute", since the thing it's adding is a route (of which the gateway is one attribute).
+ * @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, + virSocketAddr *addr, + unsigned int prefix, + virSocketAddr *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", NULL); That one could have been a simple virCommandAddArg().
+ virCommandAddArgFormat(cmd, "%s", gatewaystr); + virCommandAddArgList(cmd, "dev", ifname, NULL); + virCommandAddArgList(cmd, "proto", "static", "metric", NULL); + virCommandAddArgFormat(cmd, "%u", 1); Are all of those necessary?
Partial answer to myself: "proto static" is needed because otherwise "proto boot" is assumed, and if a routing daemon is started on the host, any route added with "proto boot" will be purged, which *isn't* what we want. I'm not sure about 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 06d0650..8b94ea8 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, + virSocketAddr *addr, + unsigned int prefix, + virSocketAddr *gateway) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) 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..dcad62d 100644 --- a/tests/networkxml2xmlin/dhcp6host-routed-network.xml +++ b/tests/networkxml2xmlin/dhcp6host-routed-network.xml @@ -19,4 +19,8 @@ <host id='0:1:0:1:18:aa:62:fe:0:16:3e:44:55:66' name='badbob' ip='2001:db8:ac10:fd01::1:24' /> </dhcp> </ip> + <ip address="192.168.222.0" netmask="255.255.255.0" via="192.168.122.10"> + </ip> + <ip family="ipv6" address="2001:db8:ac10:fc00::" prefix="64" via="2001:db8:ac10:fd01::1:24"> + </ip> </network> diff --git a/tests/networkxml2xmlout/dhcp6host-routed-network.xml b/tests/networkxml2xmlout/dhcp6host-routed-network.xml index 7305043..880c2dd 100644 --- a/tests/networkxml2xmlout/dhcp6host-routed-network.xml +++ b/tests/networkxml2xmlout/dhcp6host-routed-network.xml @@ -21,4 +21,8 @@ <host id='0:1:0:1:18:aa:62:fe:0:16:3e:44:55:66' name='badbob' ip='2001:db8:ac10:fd01::1:24' /> </dhcp> </ip> + <ip address='192.168.222.0' netmask='255.255.255.0' via='192.168.122.10'> + </ip> + <ip family='ipv6' address='2001:db8:ac10:fc00::' prefix='64' via='2001:db8:ac10:fd01::1:24'> + </ip> </network>
participants (2)
-
Gene Czarcinski -
Laine Stump