Re: [PATCH 4/5] vlan: create NMDeviceEthernet for vlan device

[Dan Williams <dcbw redhat com>]

On Mon, 2011-11-21 at 08:20 -0500, Weiping Pan wrote:
> A vlan device is like a virtual ethernet device.
> So we reuse NMDeviceEthernet insead of creating another NMDeviceVlan,
> and we add some special handling to detect vlan connections.
> 
> V3:
> 1 call nm_system_add_vlan_device() to create vlan device in kernel
> 
> V2:
> 1 delete NMDeviceVlan, just use NMDeviceEthernet
> 
> Signed-off-by: Weiping Pan <wpan redhat com>
> ---
>  src/nm-device-ethernet.c   |   34 +++++++++++++++++++++++++++++++---
>  src/nm-device-ethernet.h   |    1 +
>  src/nm-manager.c           |   11 +++++++++++
>  src/nm-udev-manager.c      |    4 +++-
>  src/settings/nm-settings.c |    9 +++++++++
>  5 files changed, 55 insertions(+), 4 deletions(-)
> 
> diff --git a/src/nm-device-ethernet.c b/src/nm-device-ethernet.c
> index b64a1da..0dd3992 100644
> --- a/src/nm-device-ethernet.c
> +++ b/src/nm-device-ethernet.c
> @@ -56,6 +56,7 @@
>  #include "nm-setting-8021x.h"
>  #include "nm-setting-pppoe.h"
>  #include "nm-setting-bond.h"
> +#include "nm-setting-vlan.h"
>  #include "ppp-manager/nm-ppp-manager.h"
>  #include "nm-logging.h"
>  #include "nm-properties-changed-signal.h"
> @@ -616,6 +617,22 @@ nm_device_bond_connection_matches (NMDevice *device, NMConnection *connection)
>  	return FALSE;
>  }
>  
> +gboolean
> +nm_device_vlan_connection_matches (NMDevice *device, NMConnection *connection)
> +{
> +	NMSettingVlan *s_vlan;
> +	const char *devname;
> +
> +	devname = nm_device_get_iface (device);
> +	g_assert(devname);
> +
> +	s_vlan = nm_connection_get_setting_vlan (connection);
> +	if (s_vlan && !strcmp (devname, nm_setting_vlan_get_interface_name (s_vlan)))

Can use g_strcmp0() here and save some code.  (it's NULL safe).

Also for this patch, some of Thomas' bond cleanups touch this code and
make things easier for both bond and vlan, so maybe I should apply those
first and then you can rebase on top of them?

Dan

> +		return TRUE;
> +
> +	return FALSE;
> +}
> +
>  /* Returns speed in Mb/s */
>  static guint32
>  nm_device_ethernet_get_speed (NMDeviceEthernet *self)
> @@ -915,6 +932,13 @@ real_get_best_auto_connection (NMDevice *dev,
>  			continue;
>  		}
>  	
> +		if (!strcmp (connection_type, NM_SETTING_VLAN_SETTING_NAME)) {
> +			if (nm_device_vlan_connection_matches (dev, connection))
> +				return connection;
> +
> +			continue;
> +		}
> +
>  		if (!strcmp (connection_type, NM_SETTING_PPPOE_SETTING_NAME))
>  			is_pppoe = TRUE;
>  
> @@ -1650,7 +1674,7 @@ real_check_connection_compatible (NMDevice *device,
>  	NMSettingConnection *s_con;
>  	NMSettingWired *s_wired;
>  	const char *connection_type;
> -	gboolean is_pppoe = FALSE, is_bond = FALSE;
> +	gboolean is_pppoe = FALSE, is_bond = FALSE, is_vlan = FALSE;
>  	const GByteArray *mac;
>  	gboolean try_mac = TRUE;
>  	const GSList *mac_blacklist, *mac_blacklist_iter;
> @@ -1661,10 +1685,11 @@ real_check_connection_compatible (NMDevice *device,
>  	connection_type = nm_setting_connection_get_connection_type (s_con);
>  	if (   strcmp (connection_type, NM_SETTING_WIRED_SETTING_NAME)
>  	    && strcmp (connection_type, NM_SETTING_BOND_SETTING_NAME)
> +	    && strcmp (connection_type, NM_SETTING_VLAN_SETTING_NAME)
>  	    && strcmp (connection_type, NM_SETTING_PPPOE_SETTING_NAME)) {
>  		g_set_error (error,
>  		             NM_ETHERNET_ERROR, NM_ETHERNET_ERROR_CONNECTION_NOT_WIRED,
> -		             "The connection was not a wired, bond, or PPPoE connection.");
> +		             "The connection was not a wired, bond, vlan, or PPPoE connection.");
>  		return FALSE;
>  	}
>  
> @@ -1674,9 +1699,12 @@ real_check_connection_compatible (NMDevice *device,
>  	if (!strcmp (connection_type, NM_SETTING_BOND_SETTING_NAME))
>  		is_bond = TRUE;
>  
> +	if (!strcmp (connection_type, NM_SETTING_VLAN_SETTING_NAME))
> +		is_vlan = TRUE;
> +
>  	s_wired = (NMSettingWired *) nm_connection_get_setting (connection, NM_TYPE_SETTING_WIRED);
>  	/* Wired setting is optional for PPPoE */
> -	if (!is_pppoe && !s_wired && !is_bond) {
> +	if (!is_pppoe && !s_wired && !is_bond && !is_vlan) {
>  		g_set_error (error,
>  		             NM_ETHERNET_ERROR, NM_ETHERNET_ERROR_CONNECTION_INVALID,
>  		             "The connection was not a valid wired connection.");
> diff --git a/src/nm-device-ethernet.h b/src/nm-device-ethernet.h
> index c7adbf8..9fe6765 100644
> --- a/src/nm-device-ethernet.h
> +++ b/src/nm-device-ethernet.h
> @@ -64,6 +64,7 @@ void nm_device_ethernet_get_address (NMDeviceEthernet *dev,
>                                       struct ether_addr *addr);
>  
>  gboolean nm_device_bond_connection_matches (NMDevice *device, NMConnection *connection);
> +gboolean nm_device_vlan_connection_matches (NMDevice *device, NMConnection *connection);
>  
>  G_END_DECLS
>  
> diff --git a/src/nm-manager.c b/src/nm-manager.c
> index 7205c7a..56cd7d7 100644
> --- a/src/nm-manager.c
> +++ b/src/nm-manager.c
> @@ -947,6 +947,8 @@ connection_needs_virtual_device (NMConnection *connection)
>  {
>  	if (nm_connection_is_type (connection, NM_SETTING_BOND_SETTING_NAME))
>  		return TRUE;
> +	if (nm_connection_is_type (connection, NM_SETTING_VLAN_SETTING_NAME))
> +		return TRUE;
>  
>  	return FALSE;
>  }
> @@ -963,6 +965,15 @@ system_update_virtual_device (NMConnection *connection)
>  		return nm_system_add_bonding_master (s_bond);
>  	}
>  
> +	if (nm_connection_is_type (connection, NM_SETTING_VLAN_SETTING_NAME)) {
> +		NMSettingVlan *s_vlan;
> +
> +		s_vlan = nm_connection_get_setting_vlan (connection);
> +		g_assert (s_vlan);
> +
> +		return nm_system_add_vlan_device(s_vlan);
> +	}
> +
>  	return TRUE;
>  }
>  
> diff --git a/src/nm-udev-manager.c b/src/nm-udev-manager.c
> index ede39bb..c66bb9a 100644
> --- a/src/nm-udev-manager.c
> +++ b/src/nm-udev-manager.c
> @@ -437,11 +437,13 @@ device_creator (NMUdevManager *manager,
>  		if (type) {
>  			if (g_strcmp0 (type, "bond") == 0)
>  				driver = "bonding";
> +			else if (g_strcmp0 (type, "vlan") == 0)
> +				driver = "8021q";
>  			g_free (type);
>  		} else if (g_str_has_prefix (ifname, "easytether")) {
>  			driver = "easytether";
>  		}
> -		
> +
>  		if (!driver) {
>  			nm_log_warn (LOGD_HW, "%s: couldn't determine device driver; ignoring...", path);
>  			goto out;
> diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c
> index 7cf930a..9aa131e 100644
> --- a/src/settings/nm-settings.c
> +++ b/src/settings/nm-settings.c
> @@ -53,6 +53,7 @@
>  #include <nm-setting-wireless.h>
>  #include <nm-setting-wireless-security.h>
>  #include <nm-setting-bond.h>
> +#include <nm-setting-vlan.h>
>  
>  #include "../nm-device-ethernet.h"
>  #include "nm-dbus-glib-types.h"
> @@ -1203,6 +1204,14 @@ have_connection_for_device (NMSettings *self, GByteArray *mac, NMDevice *device)
>  				continue;
>  		}
>  
> +		if (!strcmp (ctype, NM_SETTING_VLAN_SETTING_NAME)) {
> +			if (nm_device_vlan_connection_matches (device, connection)) {
> +				ret = TRUE;
> +				break;
> +			} else
> +				continue;
> +		}
> +
>  		if (   strcmp (ctype, NM_SETTING_WIRED_SETTING_NAME)
>  		    && strcmp (ctype, NM_SETTING_PPPOE_SETTING_NAME))
>  			continue;