[ovs-dev] [PATCH] xenserver: Kill bond master's dhclient when bringing up bond slave.

[Ian Campbell]

On Fri, 2010-06-25 at 21:22 +0100, Ben Pfaff wrote:
> interface-reconfigure is never explicitly called to down a bond master.
> However, when it is called to up a slave it is implicit that we are
> destroying the master.  The "bridge" version of interface-reconfigure
> always "ifdown"s the bond master in such a case, but until now the
> "vswitch" version has not done so.  Usually, it doesn't matter, because
> the bond master network device disappears when the slave is brought up,
> but one case was missed: for a bond master with an IP address obtained
> via DHCP, the dhclient process needs to be killed, and we were not doing
> it.  This commit starts doing it (by invoking ifdown on the bond master).

Looks good.

> The dhclient process that hangs around doesn't cause problems until the
> bond master is brought back up, at which point "ifup" fails because it
> refuses to start another dhclient for the same interface.
> 
> I do not know for sure whether the converse change is needed: that is,
> when a bond PIF is brought up, is interface-reconfigure expected to
> implicitly take down its slave PIFs?  But I do not think so.  This commit
> does not change that behavior.

interface-reconfigure is expected to take down slave PIFs if they are
up. The bridge bring_down_existing brings down:
      * vlan masters - (I think) not necessary with vswitch due to
        different behaviour wrt vlans and tagging ports instead of
        creating separate bridges etc.
      * bond masters - handled by this patch
      * the device itself - this brings down the bond slaves too due to
        the bridge version of bring_down_interface doing so. In vswitch
        this seems to be handled by extra_down_ports in the
        configure_datapath function.

Unless switching from a slave w/ DHCP to a bond leaves a dhclient lying
around (which I don't think it does) I think we are currently ok.

Ian.

> 
> Bug #2668.
> Bug #2734.
> Bug #2767.
> 
> Signed-off-by: Ben Pfaff <blp at nicira.com>
> ---
> (Oops, didn't go to the list the first time I sent this.)
> 
> diff --git a/xenserver/opt_xensource_libexec_InterfaceReconfigureVswitch.py b/xenserver/opt_xensource_libexec_InterfaceReconfigureVswitch.py
> index c31fa2d..b737555 100644
> --- a/xenserver/opt_xensource_libexec_InterfaceReconfigureVswitch.py
> +++ b/xenserver/opt_xensource_libexec_InterfaceReconfigureVswitch.py
> @@ -421,7 +421,18 @@ class DatapathVswitch(Datapath):
>          self._extra_ports = extra_ports
>  
>      def bring_down_existing(self):
> -        pass
> +        # interface-reconfigure is never explicitly called to down a
> +        # bond master.  However, when we are called to up a slave it
> +        # is implicit that we are destroying the master.
> +        #
> +        # This is (only) important in the case where the bond master
> +        # uses DHCP.  We need to kill the dhclient process, otherwise
> +        # bringing the bond master back up later will fail because
> +        # ifup will refuse to start a duplicate dhclient.
> +        bond_masters = pif_get_bond_masters(self._pif)
> +        for master in bond_masters:
> +            log("action_up: bring down bond master %s" % (pif_netdev_name(master)))
> +            run_command(["/sbin/ifdown", pif_bridge_name(master)])
>  
>      def configure(self):
>          # Bring up physical devices. ovs-vswitchd initially enables or