[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 06/10] network: Bridge - Add support for a DHCP Relay Agent



On 02/27/2013 09:57 PM, TJ wrote:
> From: TJ <linux iam tj>
>
> A DHCP relay daemon will be started that will forward all DHCP/BOOTP
> requests on the bridge network via the first declared forward
> interface. 

Okay, I think we've got our first candidate for something we might want
to configure. Picking the forward interface would probably be correct
95% of the time, but it seems reasonable someone might want to forward
it somewhere else. Also there's the fact that most people don't define
any forward interface for their networks, just leaving it up to the
host's IP routing to determine the appropriate egress for every packet.
(I don't really like what's done with the forward interface anyway - it
doesn't do anything about routing to force traffic egress via that
particular interface, just rejects any traffic that would have gone out
a different interface).


> The relay is broadcast rather than routed so no IP address
> is needed on the bridge.
>
> The XML <relay> element's "relay" property of the active DHCP stanza
> defaults to 'no'. Set it to 'yes' to enable the relay:
>
> <ip ...>
>  <dhcp relay='yes'/>
> </ip>
>
> The relay will not be started if the "enable" property is 'no':
>
> <ip ...>
>  <dhcp enable='no' relay='yes'/>

I don't see the utility of that - it's the same as simply omitting the
<dhcp> element altogether.

> </ip>
>
> Signed-off-by: TJ <linux iam tj>
> ---
>  src/network/bridge_driver.c | 146 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 146 insertions(+)
>
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index 8410c93..c02d3de 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -587,6 +587,145 @@ cleanup:
>       * which is later saved into a file
>       */
>  
> +static virNetworkIpDefPtr
> +networkGetActiveDhcp(virNetworkObjPtr network)
> +{
> +    virNetworkIpDefPtr dhcp = NULL;
> +
> +    if (network->def && network->def->ipv4_dhcp)
> +        dhcp = network->def->ipv4_dhcp;
> +
> +    if (!dhcp &&
> +        network->def && network->def->ipv6_dhcp)
> +        dhcp = network->def->ipv6_dhcp;
> +
> +    return dhcp;
> +}
> +
> +static int
> +networkBuildDhcpRelayArgv(virNetworkObjPtr network,
> +                        const char *pidfile,
> +                        virCommandPtr cmd)
> +{
> +    int ret = -1;
> +
> +    /* PID file */
> +    virCommandAddArgList(cmd, "-r", pidfile, NULL);
> +
> +    /* Listen for DHCP requests on the bridge interface */
> +    virCommandAddArgList(cmd, "-i", network->def->bridge, NULL);
> +
> +    /* Use the first forwarding device to broadcast to the upstream DHCP server */
> +    if (network->def->forward.nifs > 0) {
> +        virCommandAddArgList(cmd, "-b", network->def->forward.ifs[0].device.dev, NULL);
> +	ret = 0;
> +    } else
> +	virReportSystemError(VIR_ERR_INVALID_INTERFACE,
> +	    _("DHCP relay requires at least one network %s\n"),
> +              "<forward ... dev='eth?'/> or <interface dev='eth?'/>");
> +
> +    return ret;
> +}
> +
> +static int
> +networkBuildDhcpRelayCommandLine(virNetworkObjPtr network, virCommandPtr *cmdout,
> +                                  char *pidfile)
> +{
> +    virCommandPtr cmd = NULL;
> +    int ret = -1;
> +
> +    cmd = virCommandNew(DHCPRELAY);
> +    if (networkBuildDhcpRelayArgv(network, pidfile, cmd) < 0) {
> +        goto cleanup;
> +    }
> +
> +    if (cmdout)
> +        *cmdout = cmd;
> +    ret = 0;
> +cleanup:
> +    if (ret < 0)
> +        virCommandFree(cmd);
> +    return ret;
> +}
> +
> +static int
> +networkStartDhcpRelayDaemon(struct network_driver *driver ATTRIBUTE_UNUSED,
> +                             virNetworkObjPtr network)
> +{
> +    virCommandPtr cmd = NULL;
> +    virNetworkIpDefPtr ipdef = NULL;
> +    char *pidfile = NULL;
> +    char *tmp = NULL;
> +    int pid_len;
> +    int ret = 0;
> +    const char *dhcprelay = "dhcprelay_";
> +
> +    ipdef = networkGetActiveDhcp(network);
> +    /* Prepare for DHCP relay agent */
> +    if (ipdef && ipdef->dhcp_enabled && ipdef->dhcp_relay) {
> +	ret = -1;
> +
> +        if (virFileMakePath(NETWORK_PID_DIR) < 0) {
> +            virReportSystemError(errno,
> +                                 _("cannot create directory %s"),
> +                                 NETWORK_PID_DIR);
> +            goto cleanup;
> +        }
> +
> +        pid_len = strlen(dhcprelay) + strlen(network->def->name);
> +        if ( VIR_ALLOC_N(tmp, pid_len+1) >= 0) {
> +	    tmp = strcpy(tmp, dhcprelay);
> +	    tmp = strncat(tmp, network->def->name, pid_len);
> +	    if (!(pidfile = virPidFileBuildPath(NETWORK_PID_DIR, tmp))) {
> +	        virReportOOMError();
> +	        goto cleanup;
> +	    }
> +        } else {
> +	    virReportOOMError();
> +	    goto cleanup;
> +	}
> +
> +        ret = networkBuildDhcpRelayCommandLine(network, &cmd, pidfile);
> +        if (ret < 0)
> +	    goto cleanup;
> +
> +        ret = virCommandRun(cmd, NULL);
> +        if (ret < 0)
> +	    goto cleanup;
> +	
> +        ret = virPidFileRead(NETWORK_PID_DIR, pidfile, &network->dhcprelayPid);
> +        if (ret < 0)
> +	    virReportSystemError(errno, _("%s is not running"), DHCPRELAY);
> +
> +cleanup:
> +        VIR_FREE(tmp);
> +        VIR_FREE(pidfile);
> +        virCommandFree(cmd);
> +    }
> +    return ret;
> +}
> +
> +static int
> +networkRestartDhcpRelayDaemon(struct network_driver *driver,
> +                              virNetworkObjPtr network)
> +{
> +    /* if there is a running DHCP relay agent, kill it */
> +    if (network->dhcprelayPid > 0) {
> +        networkKillDaemon(network->dhcprelayPid, DHCPRELAY,
> +                          network->def->name);
> +        network->dhcprelayPid = -1;
> +    }
> +    /* now start the daemon if it should be started */
> +    return networkStartDhcpRelayDaemon(driver, network);
> +}
> +
> +static int
> +networkRefreshDhcpRelayDaemon(struct network_driver *driver,
> +                              virNetworkObjPtr network)
> +{
> +    return networkRestartDhcpRelayDaemon(driver, network);
> +}
> +
>  static int
>  networkBuildDnsmasqDhcpHostsList(dnsmasqContext *dctx,
>                                   virNetworkIpDefPtr ipdef)
> @@ -1496,6 +1635,7 @@ networkRefreshDaemons(struct network_driver *driver)
>               * dnsmasq and/or radvd, or restart them if they've
>               * disappeared.
>               */
> +            networkRefreshDhcpRelayDaemon(driver, network);
>              networkRefreshDhcpDaemon(driver, network);
>              networkRefreshRadvd(driver, network);
>          }
> @@ -2462,6 +2602,10 @@ networkStartNetworkVirtual(struct network_driver *driver,
>          networkStartDhcpDaemon(driver, network) < 0)
>          goto err3;
>  
> +    /* start DHCP relay-agent (doesn't need IP address(es) to function) */
> +    if (networkStartDhcpRelayDaemon(driver, network) < 0)
> +	goto err3;
> +
>      /* start radvd if there are any ipv6 addresses */
>      if (v6present && networkStartRadvd(driver, network) < 0)
>          goto err4;
> @@ -3276,6 +3420,8 @@ networkUpdate(virNetworkPtr net,
>               */
>              if (networkRestartDhcpDaemon(driver, network) < 0)
>                  goto cleanup;
> +            if (networkRestartDhcpRelayDaemon(driver, network) < 0)
> +                goto cleanup;
>  
>          } else if (section == VIR_NETWORK_SECTION_IP_DHCP_HOST) {
>              /* if we previously weren't listening for dhcp and now we


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]