Re: [PATCH 8/8] bonding: Move bonding master state to UNAVAILABLE regardless of carrier state

[Dan Williams <dcbw redhat com>]

On Mon, 2011-11-21 at 17:50 +0100, Thomas Graf wrote:
> The carrier of a bonding device is the sum of the carrier state of
> all its slaves. The carrier is always off if no slaves have been
> attached to the master yet.
> 
> Therefore the carrier state is of no interest when making a bonding
> connection available but we still want to use carrier detection to
> move the connection out of ACTIVATED if the carrier of all slaves
> are off.
> 
> A neat solution is to always put the bonding master directly into
> DISCONNECTED whenever its state is changed to UNAVAILABLE.
> 
> This will make a bonding master available for activation immediately
> and move it to DISCONNECTED whenever all slaves have been without
> a carrier for 4 seconds.
> 
> In the future, the move from UNAVAILABLE to DISCONNECTED may be made
> dependant on the availability of at least one configured slave.
> 
> Signed-off-by: Thomas Graf <tgraf redhat com>
> ---
>  src/nm-device-ethernet.c |   26 ++++++++++++++++++++++++++
>  1 files changed, 26 insertions(+), 0 deletions(-)
> 
> diff --git a/src/nm-device-ethernet.c b/src/nm-device-ethernet.c
> index 88d12b3..b36f348 100644
> --- a/src/nm-device-ethernet.c
> +++ b/src/nm-device-ethernet.c
> @@ -82,6 +82,12 @@ typedef enum
>  #define NM_ETHERNET_ERROR (nm_ethernet_error_quark ())
>  #define NM_TYPE_ETHERNET_ERROR (nm_ethernet_error_get_type ()) 
>  
> +typedef enum
> +{
> +	NM_ETHERNET_TYPE_UNSPEC = 0,
> +	NM_ETHERNET_TYPE_BOND,
> +} NMEthernetType;
> +
>  typedef struct Supplicant {
>  	NMSupplicantManager *mgr;
>  	NMSupplicantInterface *iface;
> @@ -120,6 +126,8 @@ typedef struct {
>  	/* PPPoE */
>  	NMPPPManager *ppp_manager;
>  	NMIP4Config  *pending_ip4_config;
> +
> +	NMEthernetType      type;
>  } NMDeviceEthernetPrivate;
>  
>  enum {
> @@ -414,6 +422,11 @@ constructor (GType type,
>  	self = NM_DEVICE (object);
>  	priv = NM_DEVICE_ETHERNET_GET_PRIVATE (self);
>  
> +	if (!strcmp(nm_device_get_driver (self), "bonding"))
> +		priv->type = NM_ETHERNET_TYPE_BOND;
> +	else
> +		priv->type = NM_ETHERNET_TYPE_UNSPEC;

That part is somewhat icky; we could create a new private GObject
property (ie, NM_PROPERTY_PARAM_NO_EXPORT) for the device type and pass
that in when the device is created.  No strong feelings on this now
though, we could leave that for a later cleanup as long as we put a
FIXME comment there.

Dan

>  	nm_log_dbg (LOGD_HW | LOGD_ETHER, "(%s): kernel ifindex %d",
>  	            nm_device_get_iface (NM_DEVICE (self)),
>  	            nm_device_get_ifindex (NM_DEVICE (self)));
> @@ -498,6 +511,8 @@ device_state_changed (NMDevice *device,
>                        NMDeviceStateReason reason,
>                        gpointer user_data)
>  {
> +	NMDeviceEthernet *self = NM_DEVICE_ETHERNET (device);
> +	NMDeviceEthernetPrivate *priv = NM_DEVICE_ETHERNET_GET_PRIVATE (self);
>  
>  	switch (new_state) {
>  	case NM_DEVICE_STATE_ACTIVATED:
> @@ -505,6 +520,17 @@ device_state_changed (NMDevice *device,
>  	case NM_DEVICE_STATE_DISCONNECTED:
>  		clear_secrets_tries (device);
>  		break;
> +	
> +	case NM_DEVICE_STATE_UNAVAILABLE:
> +		switch (priv->type) {
> +		case NM_ETHERNET_TYPE_BOND:
> +			/* Use NM_DEVICE_STATE_REASON_CARRIER to make sure num retries is reset */
> +			nm_device_state_changed (device, NM_DEVICE_STATE_DISCONNECTED, NM_DEVICE_STATE_REASON_CARRIER);
> +			break;
> +
> +		default:
> +			break;
> +		}
>  	default:
>  		break;
>  	}