Implement PPPoE.

git-svn-id: http://svn-archive.gnome.org/svn/NetworkManager/trunk@3418 4912f4e0-d625-0410-9fb7-b9a5a253dbdc

ChangeLog

@@ -1,3 +1,33 @@
+2008-03-10  Tambet Ingo  <tambet@gmail.com>
+
+	Implement PPPoE.
+
+	* src/ppp-manager/nm-ppp-manager.c (create_pppd_cmd_line): Use PPPoE service
+	setting. Use "nic-$eth".
+
+	* src/NetworkManagerPolicy.c (auto_activate_device): Move the check of whether
+	the device is activating here to fix a race condition.
+
+	* src/ppp-manager/nm-pppd-plugin.c (get_credentials): Implement.
+
+	* src/ppp-manager/nm-ppp-manager.c (impl_ppp_manager_need_secrets): Implement.
+	(ppp_watch_cb): Emit a signal to notify pppd is not running anymore.
+	(nm_ppp_manager_start): Take activation request instead of connection, we might
+	need it for asking secrets.
+	(nm_ppp_manager_update_secrets): Implement.
+
+	* src/nm-serial-device.c (real_act_stage2_config): Send activation request to
+	ppp manager start. It might be needed for asking secrets.
+
+	* src/nm-device-802-3-ethernet.c (real_connection_secrets_updated): Implement.
+	(ppp_state_changed): Handle pppd daemon disappearing.
+	(pppoe_stage2_config): Send activation request to ppp manager start.
+
+	* libnm-util/nm-setting-pppoe.c (nm_setting_pppoe_class_init): Fix a typo.
+
+	* introspection/nm-ppp-manager.xml: Make NeedSecrets method async, return only
+	username and password.
+	
 2008-03-10  Dan Williams  <dcbw@redhat.com>
 
 	* src/nm-device.c

introspection/nm-ppp-manager.xml

@@ -4,8 +4,9 @@
   <interface name="org.freedesktop.NetworkManager.PPP">
     <method name="NeedSecrets">
       <annotation name="org.freedesktop.DBus.GLib.CSymbol" value="impl_ppp_manager_need_secrets"/>
-      <arg name="settings" type="a{sa{sv}}" direction="in"/>
-      <arg name="setting_name" type="s" direction="out"/>
+      <annotation name="org.freedesktop.DBus.GLib.Async" value=""/>
+      <arg name="username" type="s" direction="out"/>
+      <arg name="password" type="s" direction="out"/>
     </method>
 
     <method name="SetIp4Config">

libnm-util/Makefile.am

@@ -18,6 +18,7 @@ libnm_util_include_HEADERS = 		\
 	nm-setting-connection.h		\
 	nm-setting-ip4-config.h		\
 	nm-setting-ppp.h		\
+	nm-setting-pppoe.h		\
 	nm-setting-serial.h		\
 	nm-setting-gsm.h		\
 	nm-setting-cdma.h		\
@@ -35,6 +36,7 @@ libnm_util_la_SOURCES=			\
 	nm-setting-connection.c		\
 	nm-setting-ip4-config.c		\
 	nm-setting-ppp.c		\
+	nm-setting-pppoe.c		\
 	nm-setting-serial.c		\
 	nm-setting-gsm.c		\
 	nm-setting-cdma.c		\

libnm-util/nm-connection.c

@@ -9,6 +9,7 @@
 #include "nm-setting-connection.h"
 #include "nm-setting-ip4-config.h"
 #include "nm-setting-ppp.h"
+#include "nm-setting-pppoe.h"
 #include "nm-setting-wired.h"
 #include "nm-setting-wireless.h"
 #include "nm-setting-wireless-security.h"
@@ -68,6 +69,7 @@ register_default_settings (void)
 		{ NM_SETTING_GSM_SETTING_NAME,               NM_TYPE_SETTING_GSM },
 		{ NM_SETTING_CDMA_SETTING_NAME,              NM_TYPE_SETTING_CDMA },
 		{ NM_SETTING_PPP_SETTING_NAME,               NM_TYPE_SETTING_PPP },
+		{ NM_SETTING_PPPOE_SETTING_NAME,             NM_TYPE_SETTING_PPPOE },
 		{ NM_SETTING_VPN_SETTING_NAME,               NM_TYPE_SETTING_VPN },
 		{ NM_SETTING_VPN_PROPERTIES_SETTING_NAME,    NM_TYPE_SETTING_VPN_PROPERTIES },
 		{ NULL }

libnm-util/nm-setting-pppoe.c

@@ -0,0 +1,139 @@
+/* -*- Mode: C; tab-width: 5; indent-tabs-mode: t; c-basic-offset: 5 -*- */
+
+#include "nm-setting-pppoe.h"
+
+G_DEFINE_TYPE (NMSettingPPPOE, nm_setting_pppoe, NM_TYPE_SETTING)
+
+enum {
+	PROP_0,
+	PROP_SERVICE,
+	PROP_USERNAME,
+	PROP_PASSWORD,
+
+	LAST_PROP
+};
+
+NMSetting *
+nm_setting_pppoe_new (void)
+{
+	return (NMSetting *) g_object_new (NM_TYPE_SETTING_PPPOE, NULL);
+}
+
+static gboolean
+verify (NMSetting *setting, GSList *all_settings)
+{
+	NMSettingPPPOE *self = NM_SETTING_PPPOE (setting);
+
+	if (!self->username) {
+		g_warning ("Missing username");
+		return FALSE;
+	}
+
+	return TRUE;
+}
+
+static GPtrArray *
+need_secrets (NMSetting *setting)
+{
+	NMSettingPPPOE *self = NM_SETTING_PPPOE (setting);
+	GPtrArray *secrets;
+
+	if (self->password)
+		return NULL;
+
+	secrets = g_ptr_array_sized_new (1);
+	g_ptr_array_add (secrets, NM_SETTING_PPPOE_PASSWORD);
+
+	return secrets;
+}
+
+static void
+nm_setting_pppoe_init (NMSettingPPPOE *setting)
+{
+	((NMSetting *) setting)->name = g_strdup (NM_SETTING_PPPOE_SETTING_NAME);
+}
+
+static void
+set_property (GObject *object, guint prop_id,
+		    const GValue *value, GParamSpec *pspec)
+{
+	NMSettingPPPOE *setting = NM_SETTING_PPPOE (object);
+
+	switch (prop_id) {
+	case PROP_SERVICE:
+		g_free (setting->service);
+		setting->service = g_value_dup_string (value);
+		break;
+	case PROP_USERNAME:
+		g_free (setting->username);
+		setting->username = g_value_dup_string (value);
+		break;
+	case PROP_PASSWORD:
+		g_free (setting->password);
+		setting->password = g_value_dup_string (value);
+		break;
+	default:
+		G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
+		break;
+	}
+}
+
+static void
+get_property (GObject *object, guint prop_id,
+		    GValue *value, GParamSpec *pspec)
+{
+	NMSettingPPPOE *setting = NM_SETTING_PPPOE (object);
+
+	switch (prop_id) {
+	case PROP_SERVICE:
+		g_value_set_string (value, setting->service);
+		break;
+	case PROP_USERNAME:
+		g_value_set_string (value, setting->username);
+		break;
+	case PROP_PASSWORD:
+		g_value_set_string (value, setting->password);
+		break;
+	default:
+		G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
+		break;
+	}
+}
+
+static void
+nm_setting_pppoe_class_init (NMSettingPPPOEClass *setting_class)
+{
+	GObjectClass *object_class = G_OBJECT_CLASS (setting_class);
+	NMSettingClass *parent_class = NM_SETTING_CLASS (setting_class);
+
+	/* virtual methods */
+	object_class->set_property = set_property;
+	object_class->get_property = get_property;
+	parent_class->verify       = verify;
+	parent_class->need_secrets = need_secrets;
+
+	/* Properties */
+	g_object_class_install_property
+		(object_class, PROP_SERVICE,
+		 g_param_spec_string (NM_SETTING_PPPOE_SERVICE,
+						  "Service",
+						  "Service",
+						  NULL,
+						  G_PARAM_READWRITE | NM_SETTING_PARAM_SERIALIZE));
+
+	g_object_class_install_property
+		(object_class, PROP_USERNAME,
+		 g_param_spec_string (NM_SETTING_PPPOE_USERNAME,
+						  "Username",
+						  "Username",
+						  NULL,
+						  G_PARAM_READWRITE | NM_SETTING_PARAM_SERIALIZE));
+
+	g_object_class_install_property
+		(object_class, PROP_PASSWORD,
+		 g_param_spec_string (NM_SETTING_PPPOE_PASSWORD,
+						  "Password",
+						  "Password",
+						  NULL,
+						  G_PARAM_READWRITE | NM_SETTING_PARAM_SERIALIZE | NM_SETTING_PARAM_SECRET));
+}

libnm-util/nm-setting-pppoe.h

@@ -0,0 +1,41 @@
+/* -*- Mode: C; tab-width: 5; indent-tabs-mode: t; c-basic-offset: 5 -*- */
+
+#ifndef NM_SETTING_PPPOE_H
+#define NM_SETTING_PPPOE_H
+
+#include <nm-setting.h>
+
+G_BEGIN_DECLS
+
+#define NM_TYPE_SETTING_PPPOE            (nm_setting_pppoe_get_type ())
+#define NM_SETTING_PPPOE(obj)            (G_TYPE_CHECK_INSTANCE_CAST ((obj), NM_TYPE_SETTING_PPPOE, NMSettingPPPOE))
+#define NM_SETTING_PPPOE_CLASS(klass)    (G_TYPE_CHECK_CLASS_CAST ((klass), NM_TYPE_SETTING_PPPOE, NMSettingPPPOEClass))
+#define NM_IS_SETTING_PPPOE(obj)         (G_TYPE_CHECK_INSTANCE_TYPE ((obj), NM_TYPE_SETTING_PPPOE))
+#define NM_IS_SETTING_PPPOE_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((obj), NM_TYPE_SETTING_PPPOE))
+#define NM_SETTING_PPPOE_GET_CLASS(obj)  (G_TYPE_INSTANCE_GET_CLASS ((obj), NM_TYPE_SETTING_PPPOE, NMSettingPPPOEClass))
+
+#define NM_SETTING_PPPOE_SETTING_NAME "pppoe"
+
+#define NM_SETTING_PPPOE_SERVICE  "service"
+#define NM_SETTING_PPPOE_USERNAME "username"
+#define NM_SETTING_PPPOE_PASSWORD "password"
+
+typedef struct {
+	NMSetting parent;
+
+	char *service;
+	char *username;
+	char *password;
+} NMSettingPPPOE;
+
+typedef struct {
+	NMSettingClass parent;
+} NMSettingPPPOEClass;
+
+GType nm_setting_pppoe_get_type (void);
+
+NMSetting *nm_setting_pppoe_new (void);
+
+G_END_DECLS
+
+#endif /* NM_SETTING_PPPOE_H */

src/NetworkManagerPolicy.c

@@ -163,6 +163,14 @@ auto_activate_device (gpointer user_data)
 	GSList *connections, *iter;
 
 	g_assert (data);
+
+	// FIXME: if a device is already activating (or activated) with a connection
+	// but another connection now overrides the current one for that device,
+	// deactivate the device and activate the new connection instead of just
+	// bailing if the device is already active
+	if (nm_device_get_act_request (data->device))
+		return FALSE;
+
 	policy = data->policy;
 
 	/* System connections first, then user connections */
@@ -238,13 +246,6 @@ schedule_activate_check (NMPolicy *policy, NMDevice *device)
 	if (!nm_device_can_activate (device, wireless_enabled))
 		return;
 
-	// FIXME: if a device is already activating (or activated) with a connection
-	// but another connection now overrides the current one for that device,
-	// deactivate the device and activate the new connection instead of just
-	// bailing if the device is already active
-	if (nm_device_get_act_request (device))
-		return;
-
 	for (iter = policy->pending_activation_checks; iter; iter = g_slist_next (iter)) {
 		/* Only one pending activation check at a time */
 		if (((ActivateData *) iter->data)->device == device)

src/nm-device-802-3-ethernet.c

@@ -39,6 +39,8 @@
 #include "NetworkManagerSystem.h"
 #include "nm-setting-connection.h"
 #include "nm-setting-wired.h"
+#include "nm-setting-pppoe.h"
+#include "ppp-manager/nm-ppp-manager.h"
 #include "nm-utils.h"
 
 #include "nm-device-802-3-ethernet-glue.h"
@@ -58,6 +60,10 @@ typedef struct {
 
 	NMSupplicantInterface *sup_iface;
 	gulong			iface_state_id; 
+
+	/* PPPoE */
+	NMPPPManager *ppp_manager;
+	NMIP4Config  *pending_ip4_config;
 } NMDevice8023EthernetPrivate;
 
 enum {
@@ -395,40 +401,191 @@ real_get_best_auto_connection (NMDevice *dev,
 	return NULL;
 }
 
-static NMActStageReturn
-real_act_stage4_get_ip4_config (NMDevice *dev,
-                                NMIP4Config **config)
+static void
+real_connection_secrets_updated (NMDevice *dev,
+						   NMConnection *connection,
+						   const char *setting_name)
 {
-	NMDevice8023Ethernet *self = NM_DEVICE_802_3_ETHERNET (dev);
-	NMActStageReturn ret = NM_ACT_STAGE_RETURN_FAILURE;
-	NMDevice8023EthernetClass *klass;
-	NMDeviceClass *parent_class;
+	NMDevice8023EthernetPrivate *priv = NM_DEVICE_802_3_ETHERNET_GET_PRIVATE (dev);
+
+	if (priv->ppp_manager) {
+		/* PPPoE */
+		nm_ppp_manager_update_secrets (priv->ppp_manager, nm_device_get_iface (dev), connection);
+	}
+}
+
+
+/*****************************************************************************/
+/* PPPoE */
+
+static void
+ppp_state_changed (NMPPPManager *ppp_manager, NMPPPStatus status, gpointer user_data)
+{
+	NMDevice *device = NM_DEVICE (user_data);
+
+	switch (status) {
+	case NM_PPP_STATUS_NETWORK:
+		nm_device_state_changed (device, NM_DEVICE_STATE_IP_CONFIG);
+		break;
+	case NM_PPP_STATUS_DISCONNECT:
+		nm_device_state_changed (device, NM_DEVICE_STATE_FAILED);
+		break;
+	case NM_PPP_STATUS_DEAD:
+		if (nm_device_get_state (device) == NM_DEVICE_STATE_ACTIVATED)
+			nm_device_interface_deactivate (NM_DEVICE_INTERFACE (device));
+		else
+			nm_device_state_changed (device, NM_DEVICE_STATE_FAILED);
+		break;
+	default:
+		break;
+	}
+}
+
+static void
+ppp_ip4_config (NMPPPManager *ppp_manager,
+			 const char *iface,
+			 NMIP4Config *config,
+			 gpointer user_data)
+{
+	NMDevice *device = NM_DEVICE (user_data);
+
+	nm_device_set_ip_iface (device, iface);
+	NM_DEVICE_802_3_ETHERNET_GET_PRIVATE (device)->pending_ip4_config = g_object_ref (config);
+	nm_device_activate_schedule_stage4_ip_config_get (device);
+}
+
+static NMActStageReturn
+pppoe_stage2_config (NMDevice8023Ethernet *self)
+{
+	NMDevice8023EthernetPrivate *priv = NM_DEVICE_802_3_ETHERNET_GET_PRIVATE (self);
+	NMActRequest *req;
+	GError *err = NULL;
+	NMActStageReturn ret;
+
+	req = nm_device_get_act_request (NM_DEVICE (self));
+	g_assert (req);
+
+	priv->ppp_manager = nm_ppp_manager_new ();
+	if (nm_ppp_manager_start (priv->ppp_manager,
+						 nm_device_get_iface (NM_DEVICE (self)),
+						 req,
+						 &err)) {
+		g_signal_connect (priv->ppp_manager, "state-changed",
+					   G_CALLBACK (ppp_state_changed),
+					   self);
+		g_signal_connect (priv->ppp_manager, "ip4-config",
+					   G_CALLBACK (ppp_ip4_config),
+					   self);
+		ret = NM_ACT_STAGE_RETURN_POSTPONE;
+	} else {
+		nm_warning ("%s", err->message);
+		g_error_free (err);
+
+		g_object_unref (priv->ppp_manager);
+		priv->ppp_manager = NULL;
+
+		ret = NM_ACT_STAGE_RETURN_FAILURE;
+	}
+
+	return ret;
+}
+
+/* FIXME: Move it to nm-device.c and then get rid of all foo_device_get_setting() all around.
+   It's here now to keep the patch short. */
+static NMSetting *
+device_get_setting (NMDevice *device, GType setting_type)
+{
+	NMActRequest *req;
+	NMSetting *setting = NULL;
+
+	req = nm_device_get_act_request (device);
+	if (req) {
+		NMConnection *connection;
+
+		connection = nm_act_request_get_connection (req);
+		if (connection)
+			setting = nm_connection_get_setting (connection, setting_type);
+	}
+
+	return setting;
+}
+
+static NMActStageReturn
+real_act_stage2_config (NMDevice *device)
+{
+	NMSettingConnection *s_connection;
+	NMActStageReturn ret;
+
+	s_connection = NM_SETTING_CONNECTION (device_get_setting (device, NM_TYPE_SETTING_CONNECTION));
+	g_assert (s_connection);
+
+	if (!strcmp (s_connection->type, NM_SETTING_WIRED_SETTING_NAME))
+		ret = NM_ACT_STAGE_RETURN_SUCCESS;
+	else if (!strcmp (s_connection->type, NM_SETTING_PPPOE_SETTING_NAME))
+		ret = pppoe_stage2_config (NM_DEVICE_802_3_ETHERNET (device));
+	else {
+		nm_warning ("Invalid connection type '%s' for ethernet device", s_connection->type);
+		ret = NM_ACT_STAGE_RETURN_FAILURE;
+	}
+
+	return ret;
+}
+
+static NMActStageReturn
+real_act_stage4_get_ip4_config (NMDevice *device, NMIP4Config **config)
+{
+	NMDevice8023Ethernet *self = NM_DEVICE_802_3_ETHERNET (device);
+	NMDevice8023EthernetPrivate *priv = NM_DEVICE_802_3_ETHERNET_GET_PRIVATE (self);
+	NMActStageReturn ret;
 
 	g_return_val_if_fail (config != NULL, NM_ACT_STAGE_RETURN_FAILURE);
 	g_return_val_if_fail (*config == NULL, NM_ACT_STAGE_RETURN_FAILURE);
 
-	/* Chain up to parent */
-	klass = NM_DEVICE_802_3_ETHERNET_GET_CLASS (self);
-	parent_class = NM_DEVICE_CLASS (g_type_class_peek_parent (klass));
-	ret = parent_class->act_stage4_get_ip4_config (dev, config);
+	if (!priv->ppp_manager) {
+		/* Regular ethernet connection. */
 
-	if ((ret == NM_ACT_STAGE_RETURN_SUCCESS) && *config) {
-		NMConnection *connection;
-		NMSettingWired *s_wired;
+		/* Chain up to parent */
+		ret = NM_DEVICE_CLASS (nm_device_802_3_ethernet_parent_class)->act_stage4_get_ip4_config (device, config);
 
-		connection = nm_act_request_get_connection (nm_device_get_act_request (dev));
-		g_assert (connection);
-		s_wired = NM_SETTING_WIRED (nm_connection_get_setting (connection, NM_TYPE_SETTING_WIRED));
-		g_assert (s_wired);
+		if ((ret == NM_ACT_STAGE_RETURN_SUCCESS)) {
+			NMConnection *connection;
+			NMSettingWired *s_wired;
 
-		/* MTU override */
-		if (s_wired->mtu)
-			nm_ip4_config_set_mtu (*config, s_wired->mtu);
+			connection = nm_act_request_get_connection (nm_device_get_act_request (device));
+			g_assert (connection);
+			s_wired = NM_SETTING_WIRED (nm_connection_get_setting (connection, NM_TYPE_SETTING_WIRED));
+			g_assert (s_wired);
+
+			/* MTU override */
+			if (s_wired->mtu)
+				nm_ip4_config_set_mtu (*config, s_wired->mtu);
+		}
+	} else {
+		/* PPPoE */
+		*config = priv->pending_ip4_config;
+		priv->pending_ip4_config = NULL;
+		ret = NM_ACT_STAGE_RETURN_SUCCESS;
 	}
 
 	return ret;
 }
 
+static void
+real_deactivate_quickly (NMDevice *device)
+{
+	NMDevice8023EthernetPrivate *priv = NM_DEVICE_802_3_ETHERNET_GET_PRIVATE (device);
+
+	if (priv->pending_ip4_config) {
+		g_object_unref (priv->pending_ip4_config);
+		priv->pending_ip4_config = NULL;
+	}
+
+	if (priv->ppp_manager) {
+		g_object_unref (priv->ppp_manager);
+		priv->ppp_manager = NULL;
+	}
+}
+
 static void
 nm_device_802_3_ethernet_dispose (GObject *object)
 {
@@ -514,7 +671,11 @@ nm_device_802_3_ethernet_class_init (NMDevice8023EthernetClass *klass)
 	parent_class->set_hw_address = real_set_hw_address;
 	parent_class->get_best_auto_connection = real_get_best_auto_connection;
 	parent_class->can_activate = real_can_activate;
+	parent_class->connection_secrets_updated = real_connection_secrets_updated;
+
+	parent_class->act_stage2_config = real_act_stage2_config;
 	parent_class->act_stage4_get_ip4_config = real_act_stage4_get_ip4_config;
+	parent_class->deactivate_quickly = real_deactivate_quickly;
 
 	/* properties */
 	g_object_class_install_property

src/nm-serial-device.c

@@ -877,20 +877,17 @@ real_act_stage2_config (NMDevice *device)
 {
 	NMSerialDevicePrivate *priv = NM_SERIAL_DEVICE_GET_PRIVATE (device);
 	NMActRequest *req;
-	NMConnection *connection;
 	GError *err = NULL;
 	NMActStageReturn ret;
 
 	req = nm_device_get_act_request (device);
 	g_assert (req);
-	connection = nm_act_request_get_connection (req);
-	g_assert (connection);
 
 	priv->ppp_manager = nm_ppp_manager_new ();
 
 	if (nm_ppp_manager_start (priv->ppp_manager,
 						 nm_device_get_iface (device),
-						 connection,
+						 req,
 						 &err)) {
 		g_signal_connect (priv->ppp_manager, "state-changed",
 					   G_CALLBACK (ppp_state_changed),

src/ppp-manager/nm-ppp-manager.c

@@ -7,15 +7,17 @@
 #include <unistd.h>
 
 #include "nm-ppp-manager.h"
+#include "nm-setting-connection.h"
 #include "nm-setting-ppp.h"
+#include "nm-setting-pppoe.h"
 #include "nm-dbus-manager.h"
 #include "nm-utils.h"
 #include "nm-marshal.h"
 
-static gboolean impl_ppp_manager_need_secrets (NMPPPManager *manager,
-									  GHashTable *connection,
-									  char **service_name,
-									  GError **err);
+static void impl_ppp_manager_need_secrets (NMPPPManager *manager,
+								   char **username,
+								   char **password,
+								   DBusGMethodInvocation *context);
 
 static gboolean impl_ppp_manager_set_state (NMPPPManager *manager,
 								    guint32 state,
@@ -29,11 +31,15 @@ static gboolean impl_ppp_manager_set_ip4_config (NMPPPManager *manager,
 
 #define NM_PPPD_PLUGIN PLUGINDIR "/nm-pppd-plugin.so"
 #define NM_PPP_WAIT_PPPD 10000 /* 10 seconds */
+#define PPP_MANAGER_SECRET_TRIES "ppp-manager-secret-tries"
 
 typedef struct {
 	GPid pid;
 	NMDBusManager *dbus_manager;
 
+	NMActRequest *act_req;
+	DBusGMethodInvocation *pending_secrets_context;
+
 	guint32 ppp_watch_id;
 	guint32 ppp_timeout_handler;
 } NMPPPManagerPrivate;
@@ -51,6 +57,10 @@ enum {
 
 static guint signals[LAST_SIGNAL] = { 0 };
 
+typedef enum {
+	NM_PPP_MANAGER_ERROR_UNKOWN
+} NMPPPManagerError;
+
 GQuark
 nm_ppp_manager_error_quark (void)
 {
@@ -110,10 +120,12 @@ constructor (GType type,
 static void
 finalize (GObject *object)
 {
-	//NMPPPManagerPrivate *priv = NM_PPP_MANAGER_GET_PRIVATE (object);
+	NMPPPManagerPrivate *priv = NM_PPP_MANAGER_GET_PRIVATE (object);
 
 	nm_ppp_manager_stop (NM_PPP_MANAGER (object));
-	//g_object_unref (priv->dbus_manager);
+
+	g_object_unref (priv->act_req);
+	g_object_unref (priv->dbus_manager);
 
 	G_OBJECT_CLASS (nm_ppp_manager_parent_class)->finalize (object);
 }
@@ -173,15 +185,38 @@ remove_timeout_handler (NMPPPManager *manager)
 	}
 }
 
-static gboolean
+static void
 impl_ppp_manager_need_secrets (NMPPPManager *manager,
-						 GHashTable *connection,
-						 char **service_name,
-						 GError **err)
+						 char **username,
+						 char **password,
+						 DBusGMethodInvocation *context)
 {
+	NMPPPManagerPrivate *priv = NM_PPP_MANAGER_GET_PRIVATE (manager);
+	NMConnection *connection;
+	const char *setting_name;
+
 	remove_timeout_handler (manager);
-	
-	return TRUE;
+
+	connection = nm_act_request_get_connection (priv->act_req);
+
+	nm_connection_clear_secrets (connection);
+	setting_name = nm_connection_need_secrets (connection, NULL);
+	if (setting_name) {
+		guint32 tries;
+
+		tries = GPOINTER_TO_UINT (g_object_get_data (G_OBJECT (connection), PPP_MANAGER_SECRET_TRIES));
+		nm_act_request_request_connection_secrets (priv->act_req, setting_name, tries == 0 ? TRUE : FALSE);
+		g_object_set_data (G_OBJECT (connection), PPP_MANAGER_SECRET_TRIES, GUINT_TO_POINTER (++tries));
+		priv->pending_secrets_context = context;
+	} else {
+		GError *err = NULL;
+
+		g_set_error (&err, NM_PPP_MANAGER_ERROR, NM_PPP_MANAGER_ERROR_UNKOWN,
+				   "Cleared secrets, but setting didn't need any secrets.");
+
+		nm_warning ("%s", err->message);
+		dbus_g_method_return_error (context, err);
+	}
 }
 
 static gboolean impl_ppp_manager_set_state (NMPPPManager *manager,
@@ -239,9 +274,6 @@ impl_ppp_manager_set_ip4_config (NMPPPManager *manager,
 			nm_ip4_config_add_nameserver (config, g_array_index (dns, guint, i));
 	}
 
-	/* FIXME: The plugin helpfully sends WINS servers as well
-	   and we're insensitive clods and ignore them. */
-
 	val = (GValue *) g_hash_table_lookup (config_hash, NM_PPP_IP4_CONFIG_INTERFACE);
 	if (val)
 		iface = g_value_get_string (val);
@@ -414,7 +446,8 @@ ppp_exit_code (guint pppd_exit_status)
 static void
 ppp_watch_cb (GPid pid, gint status, gpointer user_data)
 {
-	NMPPPManagerPrivate *priv = NM_PPP_MANAGER_GET_PRIVATE (user_data);
+	NMPPPManager *manager = NM_PPP_MANAGER (user_data);
+	NMPPPManagerPrivate *priv = NM_PPP_MANAGER_GET_PRIVATE (manager);
 	guint err;
 
 	if (WIFEXITED (status)) {
@@ -432,6 +465,8 @@ ppp_watch_cb (GPid pid, gint status, gpointer user_data)
 	waitpid (pid, NULL, WNOHANG);
 
 	priv->pid = 0;
+
+	g_signal_emit (manager, signals[STATE_CHANGED], 0, NM_PPP_STATUS_DEAD);
 }
 
 static gboolean
@@ -446,7 +481,10 @@ pppd_timed_out (gpointer data)
 }
 
 static NMCmdLine *
-create_pppd_cmd_line (NMSettingPPP *setting, const char *device, GError **err)
+create_pppd_cmd_line (NMSettingPPP *setting, 
+				  NMSettingPPPOE *pppoe,
+				  const char *device,
+				  GError **err)
 {
 	const char *ppp_binary;
 	NMCmdLine *cmd;
@@ -464,7 +502,27 @@ create_pppd_cmd_line (NMSettingPPP *setting, const char *device, GError **err)
 
 	nm_cmd_line_add_string (cmd, "nodetach");
 	nm_cmd_line_add_string (cmd, "lock");
-	nm_cmd_line_add_string (cmd, device);
+
+	if (pppoe) {
+		char *dev_str;
+
+		nm_cmd_line_add_string (cmd, "plugin");
+		nm_cmd_line_add_string (cmd, "rp-pppoe.so");
+
+		dev_str = g_strdup_printf ("nic-%s", device);
+		nm_cmd_line_add_string (cmd, dev_str);
+		g_free (dev_str);
+
+		if (pppoe->service) {
+			nm_cmd_line_add_string (cmd, "rp_pppoe_service");
+			nm_cmd_line_add_string (cmd, pppoe->service);
+		}
+
+		nm_cmd_line_add_string (cmd, "user");
+		nm_cmd_line_add_string (cmd, pppoe->username);
+	} else {
+		nm_cmd_line_add_string (cmd, device);
+	}
 
 	if (setting->baud)
 		nm_cmd_line_add_int (cmd, setting->baud);
@@ -528,25 +586,64 @@ pppd_child_setup (gpointer user_data G_GNUC_UNUSED)
 	setpgid (pid, pid);
 }
 
+static void
+pppoe_fill_defaults (NMSettingPPP *setting)
+{
+	if (!setting->mtu)
+		setting->mtu = 1492;
+
+	if (!setting->mru)
+		setting->mru = 1492;
+
+	if (!setting->lcp_echo_interval)
+		setting->lcp_echo_interval = 20;
+
+	if (!setting->lcp_echo_failure)
+		setting->lcp_echo_failure = 3;
+
+	setting->noauth = TRUE;
+	setting->usepeerdns = TRUE;
+	setting->nodeflate = TRUE;
+
+	/* FIXME: These commented settings should be set as well, update NMSettingPPP first. */
+#if 0
+	setting->noipdefault = TRUE;
+	setting->default_asyncmap = TRUE;
+	setting->defaultroute = TRUE;
+	setting->hide_password = TRUE;
+	setting->noaccomp = TRUE;
+	setting->nopcomp = TRUE;
+	setting->novj = TRUE;
+	setting->novjccomp = TRUE;
+#endif
+}
+
 gboolean
 nm_ppp_manager_start (NMPPPManager *manager,
 				  const char *device,
-				  NMConnection *connection,
+				  NMActRequest *req,
 				  GError **err)
 {
 	NMPPPManagerPrivate *priv;
+	NMConnection *connection;
 	NMSettingPPP *ppp_setting;
+	NMSettingPPPOE *pppoe_setting;
 	NMCmdLine *ppp_cmd;
 	char *cmd_str;
 	GSource *ppp_watch;
 
 	g_return_val_if_fail (NM_IS_PPP_MANAGER (manager), FALSE);
 	g_return_val_if_fail (device != NULL, FALSE);
-	g_return_val_if_fail (NM_IS_CONNECTION (connection), FALSE);
+	g_return_val_if_fail (NM_IS_ACT_REQUEST (req), FALSE);
 
+	connection = nm_act_request_get_connection (req);
 	ppp_setting = NM_SETTING_PPP (nm_connection_get_setting (connection, NM_TYPE_SETTING_PPP));
+	pppoe_setting = (NMSettingPPPOE *) nm_connection_get_setting (connection, NM_TYPE_SETTING_PPPOE);
 
-	ppp_cmd = create_pppd_cmd_line (ppp_setting, device, err);
+	if (pppoe_setting)
+		pppoe_fill_defaults (ppp_setting);
+
+	ppp_cmd = create_pppd_cmd_line (ppp_setting, pppoe_setting, device, err);
 	if (!ppp_cmd)
 		return FALSE;
 
@@ -577,6 +674,7 @@ nm_ppp_manager_start (NMPPPManager *manager,
 	g_source_unref (ppp_watch);
 
 	priv->ppp_timeout_handler = g_timeout_add (NM_PPP_WAIT_PPPD, pppd_timed_out, manager);
+	priv->act_req = g_object_ref (req);
 
  out:
 	if (ppp_cmd)
@@ -585,6 +683,45 @@ nm_ppp_manager_start (NMPPPManager *manager,
 	return priv->pid > 0;
 }
 
+void
+nm_ppp_manager_update_secrets (NMPPPManager *manager,
+						 const char *device,
+						 NMConnection *connection)
+{
+	NMPPPManagerPrivate *priv = NM_PPP_MANAGER_GET_PRIVATE (manager);
+	NMSettingConnection *s_connection;
+	NMSettingPPPOE *pppoe_setting;
+
+	g_return_if_fail (NM_IS_PPP_MANAGER (manager));
+	g_return_if_fail (device != NULL);
+	g_return_if_fail (NM_IS_CONNECTION (connection));
+	g_return_if_fail (priv->pending_secrets_context != NULL);
+
+	s_connection = NM_SETTING_CONNECTION (nm_connection_get_setting (connection, NM_TYPE_SETTING_CONNECTION));
+	g_assert (s_connection);
+
+	if (strcmp (s_connection->type, NM_SETTING_PPPOE_SETTING_NAME))
+		/* Not for us */
+		return;
+
+	/* This is sort of a hack but...
+	   pppd plugin only ever needs username and password.
+	   Passing the full connection there would mean some bloat:
+	   the plugin would need to link against libnm-util just to parse this.
+	   So instead, let's just send what it needs */
+
+	pppoe_setting = NM_SETTING_PPPOE (nm_connection_get_setting (connection, NM_TYPE_SETTING_PPPOE));
+	g_assert (pppoe_setting);
+
+	nm_info ("Sending secrets: %s %s", pppoe_setting->username, pppoe_setting->password);
+
+	/* FIXME: Do we have to strdup the values here? */
+	dbus_g_method_return (priv->pending_secrets_context, 
+					  g_strdup (pppoe_setting->username),
+					  g_strdup (pppoe_setting->password));
+	priv->pending_secrets_context = NULL;
+}
+
 void
 nm_ppp_manager_stop (NMPPPManager *manager)
 {

src/ppp-manager/nm-ppp-manager.h

@@ -7,6 +7,7 @@
 #include <glib-object.h>
 
 #include "nm-ppp-status.h"
+#include "nm-activation-request.h"
 #include "nm-connection.h"
 #include "nm-ip4-config.h"
 #include "nm-pppd-plugin.h"
@@ -36,13 +37,19 @@ NMPPPManager *nm_ppp_manager_new (void);
 
 gboolean nm_ppp_manager_start (NMPPPManager *manager,
 						 const char *device,
-						 NMConnection *connection,
+						 NMActRequest *req,
 						 GError **err);
 
+void     nm_ppp_manager_update_secrets (NMPPPManager *manager,
+								const char *device,
+								NMConnection *connection);
+
 void     nm_ppp_manager_stop  (NMPPPManager *manager);
 
 
 #define NM_PPP_MANAGER_ERROR nm_ppp_manager_error_quark()
+#define NM_TYPE_PPP_MANAGER_ERROR (nm_ppp_manager_error_get_type ()) 
+
 GQuark nm_ppp_manager_error_quark (void);
 
 #endif /* NM_PPP_MANAGER_H */

src/ppp-manager/nm-pppd-plugin.c

@@ -1,5 +1,6 @@
 /* -*- Mode: C; tab-width: 5; indent-tabs-mode: t; c-basic-offset: 5 -*- */
 
+#include <string.h>
 #include <pppd/pppd.h>
 #include <pppd/fsm.h>
 #include <pppd/ipcp.h>
@@ -26,6 +27,8 @@ nm_phasechange (void *data, int arg)
 	NMPPPStatus ppp_status = NM_PPP_STATUS_UNKNOWN;
 	char *ppp_phase;
 
+	g_return_if_fail (DBUS_IS_G_PROXY (proxy));
+
 	switch (arg) {
 	case PHASE_DEAD:
 		ppp_status = NM_PPP_STATUS_DEAD;
@@ -135,6 +138,8 @@ nm_ip_up (void *data, int arg)
 	GArray *array;
 	GValue *val;
 
+	g_return_if_fail (DBUS_IS_G_PROXY (proxy));
+
 	if (!opts.ouraddr) {
 		g_warning ("Didn't receive an internal IP from pppd");
 		return;
@@ -199,10 +204,59 @@ nm_ip_up (void *data, int arg)
 	g_hash_table_destroy (hash);
 }
 
+static int
+get_credentials (char *username, char *password)
+{
+	char *my_username;
+	char *my_password;
+	size_t len;
+	GError *err = NULL;
+
+	g_return_val_if_fail (DBUS_IS_G_PROXY (proxy), -1);
+
+	my_username = my_password = NULL;
+	dbus_g_proxy_call (proxy, "NeedSecrets", &err,
+				    G_TYPE_INVALID,
+				    G_TYPE_STRING, &my_username,
+				    G_TYPE_STRING, &my_password,
+				    G_TYPE_INVALID);
+
+	if (err) {
+		g_warning ("Could not get secrets: %s", err->message);
+		g_error_free (err);
+		return -1;
+	}
+
+	if (my_username) {
+		len = strlen (my_username) + 1;
+		len = len < MAXNAMELEN ? len : MAXNAMELEN;
+
+		strncpy (username, my_username, len);
+		username[len - 1] = '\0';
+
+		g_free (my_username);
+	}
+
+	if (my_password) {
+		len = strlen (my_password) + 1;
+		len = len < MAXSECRETLEN ? len : MAXSECRETLEN;
+
+		strncpy (password, my_password, len);
+		password[len - 1] = '\0';
+
+		g_free (my_password);
+	}
+
+	return 0;
+}
+
 static void
 nm_exit_notify (void *data, int arg)
 {
-	g_object_unref (data);
+	g_return_if_fail (DBUS_IS_G_PROXY (proxy));
+
+	g_object_unref (proxy);
+	proxy = NULL;
 }
 
 int
@@ -227,6 +281,9 @@ plugin_init (void)
 
 	dbus_g_connection_unref (bus);
 
+	chap_passwd_hook = get_credentials;
+	pap_passwd_hook = get_credentials;
+
 	add_notifier (&phasechange, nm_phasechange, NULL);
 	add_notifier (&ip_up_notifier, nm_ip_up, NULL);
 	add_notifier (&exitnotify, nm_exit_notify, proxy);