Re: [PATCH 4/6] VLAN: add NMDeviceVLAN



On 11/16/2011 01:54 PM, Dan Williams wrote:
On Fri, 2011-10-21 at 09:52 +0800, Weiping Pan wrote:
NMDeviceVLAN represents a VLAN device in NetworkManager.
When a VLAN device is created in kernel, NetworkManager will receive the event
and create a corresponding NMDeviceVLAN.
Like the setting, let's use NMDeviceVlan here for consistency.

Ok.

      
Signed-off-by: Weiping Pan <wpan redhat com>
---
 include/NetworkManager.h |    1 +
 src/Makefile.am          |    2 +
 src/nm-device-vlan.c     |  330 ++++++++++++++++++++++++++++++++++++++++++++++
 src/nm-device-vlan.h     |   59 ++++++++
 4 files changed, 392 insertions(+), 0 deletions(-)
 create mode 100644 src/nm-device-vlan.c
 create mode 100644 src/nm-device-vlan.h

diff --git a/include/NetworkManager.h b/include/NetworkManager.h
index 3522dd2..87d7d7f 100644
--- a/include/NetworkManager.h
+++ b/include/NetworkManager.h
@@ -114,6 +114,7 @@ typedef enum {
 	NM_DEVICE_TYPE_OLPC_MESH = 6,
 	NM_DEVICE_TYPE_WIMAX     = 7,
 	NM_DEVICE_TYPE_MODEM     = 8,
+	NM_DEVICE_TYPE_VLAN      = 9,
 } NMDeviceType;
 
 /**
diff --git a/src/Makefile.am b/src/Makefile.am
index cbcfdc6..e086024 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -126,6 +126,8 @@ NetworkManager_SOURCES = \
 		nm-device-bt.h \
 		nm-device-modem.h \
 		nm-device-modem.c \
+		nm-device-vlan.h \
+		nm-device-vlan.c \
 		nm-wifi-ap.c \
 		nm-wifi-ap.h \
 		nm-wifi-ap-utils.c \
diff --git a/src/nm-device-vlan.c b/src/nm-device-vlan.c
new file mode 100644
index 0000000..23abdbc
--- /dev/null
+++ b/src/nm-device-vlan.c
@@ -0,0 +1,330 @@
+/* -*- Mode: C; tab-width: 4; indent-tabs-mode: t; c-basic-offset: 4 -*- */
+/* NetworkManager -- Network link manager
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Copyright (C) 2011 Red Hat, Inc.
+ */
+
+#include <string.h>
+#include <glib.h>
+
+#include "nm-system.h"
+#include "nm-device-vlan.h"
+#include "nm-device-interface.h"
+#include "nm-device-private.h"
+#include "nm-properties-changed-signal.h"
+#include "nm-marshal.h"
+#include "nm-logging.h"
+
+static void device_interface_init (NMDeviceInterface *iface_class);
+
+G_DEFINE_TYPE_EXTENDED (NMDeviceVLAN, nm_device_vlan, NM_TYPE_DEVICE, 0,
+                        G_IMPLEMENT_INTERFACE (NM_TYPE_DEVICE_INTERFACE, device_interface_init))
+
+#define NM_DEVICE_VLAN_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE ((o), NM_TYPE_DEVICE_VLAN, NMDeviceVLANPrivate))
+
+typedef struct {
+	char *interface_name;
+	guint32 vlan_id;
+	guint32 vlan_flags;
+	char *vlan_priority_ingress_map;
+	char *vlan_priority_egress_map;
+} NMDeviceVLANPrivate;
+
+enum {
+	PROP_0,
+	PROP_INTERFACE_NAME,
+	PROP_VLAN_ID,
+	PROP_VLAN_FLAGS,
+	PROP_VLAN_PRIORITY_INGRESS_MAP,
+	PROP_VLAN_PRIORITY_EGRESS_MAP,
+	LAST_PROP
+};
So for properties on the device (which get exported over D-Bus) it's
really only useful to have properties that clients are going to be
interested in; are all these properties interesting ones, or are they
mainly implementation details?  Clients can get most of these properties
from the NMSettingVlan itself since they aren't going to change while
the interface is activated.  Especially INTERFACE_NAME; that's already
handled by the parent class' 'interface' property.  Basically, if a
property isn't going to change while the interface is activated/up, it
probably shouldn't be a property of the device, but clients can get it
via the connection data itself.  We can always add device properties
later if we find clients want them for some reason, but we can't really
take them away.

When a vlan device is up, the client can only change
+	PROP_VLAN_FLAGS,
+	PROP_VLAN_PRIORITY_INGRESS_MAP,
+	PROP_VLAN_PRIORITY_EGRESS_MAP,

I test them with command vconfig.

I thought the client could change the name of vlan device,
either through "ip link set p2p4 name pwp", or  rtnl_link_set_name(),
but both methods can work when the device is down.
So, I can remove PROP_INTERFACE_NAME and PROP_VLAN_ID here.

      
+static guint32
+real_get_generic_capabilities (NMDevice *device)
+{
+	return NM_DEVICE_CAP_NM_SUPPORTED;
+}
+
+static NMConnection *
+real_get_best_auto_connection (NMDevice *device,
+							   GSList *connections,
+							   char **specific_object)
+{
+	GSList *iter;
+
+	for (iter = connections; iter; iter = g_slist_next (iter)) {
+		NMConnection *connection = NM_CONNECTION (iter->data);
+		NMSettingConnection *s_con;
+		NMSettingVLAN *s_vlan;
+		const char *connection_type;
+
+		s_con = NM_SETTING_CONNECTION (nm_connection_get_setting (connection, NM_TYPE_SETTING_CONNECTION));
+		g_assert (s_con);
+
+		connection_type = nm_setting_connection_get_connection_type (s_con);
+		if (!strcmp (connection_type, NM_SETTING_VLAN_SETTING_NAME)) {
+			s_vlan = (NMSettingVLAN *) nm_connection_get_setting (connection, NM_TYPE_SETTING_VLAN);
+			if (s_vlan) {
+				int vlan_id = 0;
+				const char *interface_name = NULL;
+				vlan_id = nm_setting_vlan_get_vlan_id(s_vlan);
+				interface_name = nm_setting_vlan_get_interface_name(s_vlan);
+				if (!strcmp(interface_name, nm_device_get_iface(device))) {
+					return connection;
+				}
+			}
+		}
+	}
+
+	return NULL;
+}
+
+static gboolean
+real_check_connection_compatible (NMDevice *device,
+                                  NMConnection *connection,
+                                  GError **error)
+{
+	NMSettingConnection *s_con;
+	NMSettingVLAN *s_vlan;
+	const char *connection_type;
+
+	s_con = NM_SETTING_CONNECTION (nm_connection_get_setting (connection, NM_TYPE_SETTING_CONNECTION));
+	g_assert (s_con);
+
+	connection_type = nm_setting_connection_get_connection_type (s_con);
+
+	if (!strcmp (connection_type, NM_SETTING_VLAN_SETTING_NAME)) {
+		s_vlan = (NMSettingVLAN *) nm_connection_get_setting (connection, NM_TYPE_SETTING_VLAN);
+		if (s_vlan) {
+			return TRUE;
+		}
+	}
Should probably check the interface name here too, since we shouldn't be
treating VLAN configs with different interface names as compatible with
this VLAN interface.

Sorry, I don't know what is your idea.
To be compatible with initscripts, the name of vlan device should be something like vlan43,
but I think the user can use whatever name he likes.

      
+
+	return FALSE;
+}
+
+static gboolean
+real_is_up (NMDevice *device)
+{
+	return nm_system_iface_is_up (nm_device_get_ip_ifindex (device));
+}
+
+static gboolean
+real_bring_up (NMDevice *device)
+{
+	return TRUE;
+}
+
+static void
+real_take_down (NMDevice *device)
+{
+}
+
+static gboolean
+real_hw_is_up (NMDevice *device)
+{
+	return nm_system_iface_is_up (nm_device_get_ip_ifindex (device));
+}
+
+static gboolean
+real_hw_bring_up (NMDevice *dev, gboolean *no_firmware)
+{
+	return nm_system_iface_set_up (nm_device_get_ip_ifindex (dev), TRUE, no_firmware);
+}
+
+static void
+real_hw_take_down (NMDevice *dev)
+{
+	nm_system_iface_set_up (nm_device_get_ip_ifindex (dev), FALSE, NULL);
+}
+
+static void
+device_state_changed (NMDevice *device,
+                      NMDeviceState new_state,
+                      NMDeviceState old_state,
+                      NMDeviceStateReason reason,
+                      gpointer user_data)
+{
+	nm_device_state_changed (device, new_state, reason);
+}
Shouldn't need this function; this is just used by the device internally
to respond to state changes, it doesn't need to re-emit them.  It's a
bit of GObject trivia which I'm happy to share if you're interested.

Ok.

      
+
+NMDevice *
+nm_device_vlan_new (NMSettingVLAN *setting)
+{
+	return (NMDevice *) g_object_new (NM_TYPE_DEVICE_VLAN,
+	                                  NM_DEVICE_INTERFACE_UDI, nm_setting_vlan_get_interface_name(setting),
+	                                  NM_DEVICE_INTERFACE_IFACE, nm_setting_vlan_get_interface_name(setting),
+	                                  NM_DEVICE_INTERFACE_DRIVER, "8021q",
+	                                  NM_DEVICE_INTERFACE_TYPE_DESC, "VLAN",
+	                                  NM_DEVICE_INTERFACE_DEVICE_TYPE, NM_DEVICE_TYPE_VLAN,
+	                                  NULL);
+}
+
+static void
+device_interface_init (NMDeviceInterface *iface_class)
+{
+}
+
+static void
+nm_device_vlan_init (NMDeviceVLAN *self)
+{
+	NMDeviceVLANPrivate *priv = NM_DEVICE_VLAN_GET_PRIVATE (self);
+	priv->vlan_priority_ingress_map = NULL;
+	priv->vlan_priority_egress_map = NULL;
+	g_signal_connect (self, "state-changed", G_CALLBACK (device_state_changed), self);
SHouldn't need to connect to the signal unless we're actually handling
state changes, which it doesn't appear that the class is.

OK.

      
+}
+
+static void
+finalize (GObject *object)
+{
+	NMDeviceVLANPrivate *priv = NM_DEVICE_VLAN_GET_PRIVATE (object);
+
+	g_free(priv->interface_name);
+	g_free(priv->vlan_priority_ingress_map);
+	g_free(priv->vlan_priority_egress_map);
+
+	G_OBJECT_CLASS (nm_device_vlan_parent_class)->finalize (object);
+}
+
+static void
+set_property (GObject *object, guint prop_id,
+			  const GValue *value, GParamSpec *pspec)
+{
+	NMDeviceVLAN *vlan = NM_DEVICE_VLAN (object);
+	NMDeviceVLANPrivate *priv = NM_DEVICE_VLAN_GET_PRIVATE (vlan);
+
+	switch (prop_id) {
+	case PROP_INTERFACE_NAME:
+		g_free(priv->interface_name);
+		priv->interface_name = g_value_dup_string (value);
+		break;
+	case PROP_VLAN_ID:
+		priv->vlan_id = g_value_get_uint(value);
+		break;
+	case PROP_VLAN_FLAGS:
+		priv->vlan_flags |= g_value_get_uint(value);
Should just assign here; don't need to | the value.

Ok.

      
+		break;
+	case PROP_VLAN_PRIORITY_INGRESS_MAP:
+		g_free(priv->vlan_priority_ingress_map);
+		priv->vlan_priority_ingress_map = g_value_dup_string(value);
+		break;
+	case PROP_VLAN_PRIORITY_EGRESS_MAP:
+		g_free(priv->vlan_priority_egress_map);
+		priv->vlan_priority_egress_map = g_value_dup_string(value);
+		break;
+	default:
+		break;
+	}
+}
+
+static void
+get_property (GObject *object, guint prop_id,
+			  GValue *value, GParamSpec *pspec)
+{
+	NMDeviceVLAN *vlan= NM_DEVICE_VLAN (object);
+	NMDeviceVLANPrivate *priv = NM_DEVICE_VLAN_GET_PRIVATE (vlan);
+
+	switch (prop_id) {
+	case PROP_INTERFACE_NAME:
+		g_value_set_string(value, priv->interface_name);
+		break;
+	case PROP_VLAN_ID:
+		g_value_set_uint(value, priv->vlan_id);
+		break;
+	case PROP_VLAN_FLAGS:
+		g_value_set_uint(value, priv->vlan_flags);
+		break;
+	case PROP_VLAN_PRIORITY_INGRESS_MAP:
+		g_value_set_string(value, priv->vlan_priority_ingress_map);
+		break;
+	case PROP_VLAN_PRIORITY_EGRESS_MAP:
+		g_value_set_string(value, priv->vlan_priority_egress_map);
+		break;
+	default:
+		break;
+	}
+}
+
+static void
+nm_device_vlan_class_init (NMDeviceVLANClass *class)
+{
+	GObjectClass *object_class = G_OBJECT_CLASS (class);
+	NMDeviceClass *device_class = NM_DEVICE_CLASS (class);
+
+	g_type_class_add_private (object_class, sizeof (NMDeviceVLANPrivate));
+
+	/* Virtual methods */
+	object_class->finalize = finalize;
+	object_class->get_property = get_property;
+	object_class->set_property = set_property;
+
+	device_class->get_generic_capabilities = real_get_generic_capabilities;
+	device_class->check_connection_compatible = real_check_connection_compatible;
+	device_class->hw_is_up = real_hw_is_up;
+	device_class->hw_bring_up = real_hw_bring_up;
+	device_class->hw_take_down = real_hw_take_down;
+	device_class->is_up = real_is_up;
+	device_class->bring_up = real_bring_up;
+	device_class->take_down = real_take_down;
+	device_class->get_best_auto_connection = real_get_best_auto_connection;
+
+	/* Properties */
+	g_object_class_install_property
+		(object_class, PROP_INTERFACE_NAME,
+		g_param_spec_string(	NM_DEVICE_VLAN_INTERFACE_NAME,
+					"InterfaceName",
+					NULL,
+					NULL,
+					G_PARAM_READWRITE | G_PARAM_CONSTRUCT ));
+
+	g_object_class_install_property
+		(object_class, PROP_VLAN_ID,
+		 g_param_spec_uint (NM_DEVICE_VLAN_VLAN_ID,
+						"vlan id",
+						NULL,
+						0,
+						G_MAXUINT32,
+						0,
+						G_PARAM_READWRITE | G_PARAM_CONSTRUCT ));
+
+	g_object_class_install_property
+		(object_class, PROP_VLAN_FLAGS,
+		 g_param_spec_uint (NM_DEVICE_VLAN_VLAN_FLAGS,
+						"vlan flags",
+						NULL,
+						0,
+						G_MAXUINT32,
+						0,
+						G_PARAM_READWRITE | G_PARAM_CONSTRUCT ));
+
+	g_object_class_install_property
+		(object_class, PROP_VLAN_PRIORITY_INGRESS_MAP,
+		g_param_spec_string(	NM_DEVICE_VLAN_VLAN_INGRESS_PRIORITY_MAP,
+					"vlan ingress priority map",
+					NULL,
+					NULL,
+					G_PARAM_READWRITE | G_PARAM_CONSTRUCT ));
+
+	g_object_class_install_property
+		(object_class, PROP_VLAN_PRIORITY_EGRESS_MAP,
+		g_param_spec_string(	NM_DEVICE_VLAN_VLAN_EGRESS_PRIORITY_MAP,
+					"vlan egress priority map",
+					NULL,
+					NULL,
+					G_PARAM_READWRITE | G_PARAM_CONSTRUCT));
+}
diff --git a/src/nm-device-vlan.h b/src/nm-device-vlan.h
new file mode 100644
index 0000000..33b88ae
--- /dev/null
+++ b/src/nm-device-vlan.h
@@ -0,0 +1,59 @@
+/* -*- Mode: C; tab-width: 4; indent-tabs-mode: t; c-basic-offset: 4 -*- */
+/* NetworkManager -- Network link manager
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Copyright (C) 2011 Red Hat, Inc.
+ */
+
+#ifndef NM_DEVICE_VLAN_H
+#define NM_DEVICE_VLAN_H
+
+#include <glib.h>
+#include <glib-object.h>
+
+#include "nm-device.h"
+#include "nm-setting-vlan.h"
+
+#define NM_TYPE_DEVICE_VLAN            (nm_device_vlan_get_type ())
+#define NM_DEVICE_VLAN(obj)            (G_TYPE_CHECK_INSTANCE_CAST ((obj), NM_TYPE_DEVICE_VLAN, NMDeviceVLAN))
+#define NM_DEVICE_VLAN_CLASS(klass)    (G_TYPE_CHECK_CLASS_CAST ((klass), NM_TYPE_DEVICE_VLAN, NMDeviceVLANClass))
+#define NM_IS_DEVICE_VLAN(obj)         (G_TYPE_CHECK_INSTANCE_TYPE ((obj), NM_TYPE_DEVICE_VLAN))
+#define NM_IS_DEVICE_VLAN_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((obj), NM_TYPE_DEVICE_VLAN))
+#define NM_DEVICE_VLAN_GET_CLASS(obj)  (G_TYPE_INSTANCE_GET_CLASS ((obj), NM_TYPE_DEVICE_VLAN, NMDeviceVLANClass))
+
+#define NM_DEVICE_VLAN_INTERFACE_NAME 			"interface-name"
+#define NM_DEVICE_VLAN_VLAN_SLAVE 			"vlan-slave"
+#define NM_DEVICE_VLAN_VLAN_ID				"vlan-id"
+#define NM_DEVICE_VLAN_VLAN_FLAGS 			"vlan-flags"
+#define NM_DEVICE_VLAN_VLAN_NAME_TYPE 			"vlan-name-type"
+#define NM_DEVICE_VLAN_VLAN_INGRESS_PRIORITY_MAP 	"vlan-priority-ingress-map"
+#define NM_DEVICE_VLAN_VLAN_EGRESS_PRIORITY_MAP 	"vlan-priority-egress-map"
Liek NMSettingVlan, these properties are already namespaced, so we can
remove the VLAN_/vlan- duplication in the names.  But depending on
discussion above we may not need most of them...

Thanks,
Dan

Ok, thanks
Weiping Pan

      
+typedef struct {
+	NMDevice parent;
+} NMDeviceVLAN;
+
+typedef struct {
+	NMDeviceClass parent;
+
+	void (*properties_changed) (NMDeviceVLAN *self, GHashTable *properties);
+} NMDeviceVLANClass;
+
+GType nm_device_vlan_get_type (void);
+
+NMDevice *nm_device_vlan_new (NMSettingVLAN *setting);
+
+#endif /* NM_DEVICE_VLAN_H */




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