Re: dhcp client usage

[Per Hallsmark <per hallsmark t2data se>]

Hmmm... looking further, it seems that the change iface -> ip_iface has 
already begun in some part of the code.
Attach is a patch that does the trick for dhcp as well.
Just to be safe, I added in the PROP_IFACE setting that it copies the 
value to both iface and ip_iface. This makes sure that device that 
doesn't set ip_iface will work anyway (making the change sort of 
transparant :-))


Per Hallsmark wrote:
> Hello,
>
> I'm adding functionality to NM in order to support a broadband module.
>
> The hw module has a cdc-acm interface which I send AT command stuff to 
> and
> in the end, a cdc-ether interface is created. So far so god, I use the 
> nm_device_set_ip_iface()
> in order to specify where the IP traffic should go (at least this is 
> my assumption :-) )
> so iface is ttyACM0 and ip_iface is mb0 in my case. Next step is to do 
> a DHCP
> request session on the ip_iface in order to fetch ip, gw and dns's.
> Unfortunally though in nm-device.c, the routines that handles dhcp 
> uses iface
> and not ip_iface so here it fails for the moment.
>
> If I let my cdc-acm interface (which is a bit similar to 
> nm-hso-gsm-device.c|h,
> but yet not) return ok for ip config, I get the nice antenna icon and 
> interface is
> believed to be up.
> Thereafter I can manually start dhclient on the ethernet interface and 
> all is ok.
>
> But howto make NM do this for me? Shouldn't really the dhcp routines 
> in nm-device
> use the ip_iface instead of iface? (guess this will break stuff a bit 
> until it is fixed
> by having all interfaces specify ip_iface even though it could be same 
> as iface in some places)
>
> Please enlighten me a bit in the NM architecture for the dhcp stuff 
> here :-)
>
> Thanks and best regards,
> Per
>
> _______________________________________________
> NetworkManager-list mailing list
> NetworkManager-list gnome org
> http://mail.gnome.org/mailman/listinfo/networkmanager-list

Index: nm-device.c
===================================================================
--- nm-device.c	(revision 612)
+++ nm-device.c	(revision 635)
@@ -248,7 +248,6 @@
 	return self->priv->iface;
 }
 
-
 const char *
 nm_device_get_ip_iface (NMDevice *self)
 {
@@ -263,12 +262,10 @@
 nm_device_set_ip_iface (NMDevice *self, const char *iface)
 {
 	g_return_if_fail (NM_IS_DEVICE (self));
-
 	g_free (self->priv->ip_iface);
 	self->priv->ip_iface = iface ? g_strdup (iface) : NULL;
 }
 
-
 /*
  * Get/set functions for driver
  */
@@ -826,11 +823,11 @@
 	NMSettingIP4Config *s_ip4;
 	NMActRequest *req;
 	NMActStageReturn ret = NM_ACT_STAGE_RETURN_SUCCESS;
-	const char *iface;
+	const char *ip_iface;
 
 	g_return_val_if_fail (reason != NULL, NM_ACT_STAGE_RETURN_FAILURE);
 
-	iface = nm_device_get_iface (self);
+	ip_iface = nm_device_get_ip_iface (self);
 
 	req = nm_device_get_act_request (self);
 	s_ip4 = (NMSettingIP4Config *) nm_connection_get_setting (nm_act_request_get_connection (req),
@@ -847,7 +844,7 @@
 		/* DHCP manager will cancel any transaction already in progress and we do not
 		   want to cancel this activation if we get "down" state from that. */
 		g_signal_handler_block (priv->dhcp_manager, priv->dhcp_state_sigid);
-		success = nm_dhcp_manager_begin_transaction (priv->dhcp_manager, iface, s_ip4, 45);
+		success = nm_dhcp_manager_begin_transaction (priv->dhcp_manager, ip_iface, s_ip4, 45);
 		g_signal_handler_unblock (priv->dhcp_manager, priv->dhcp_state_sigid);
 
 		if (success) {
@@ -865,11 +862,11 @@
 		/* Start avahi-autoipd */
 		if (aipd_exec (self, &error)) {
 			nm_info ("Activation (%s) Stage 3 of 5 (IP Configure Start) started"
-			         " avahi-autoipd...", iface);
+			         " avahi-autoipd...", ip_iface);
 			ret = NM_ACT_STAGE_RETURN_POSTPONE;
 		} else {
 			nm_info ("Activation (%s) Stage 3 of 5 (IP Configure Start) failed"
-			         " to start avahi-autoipd: %s", iface, error->message);
+			         " to start avahi-autoipd: %s", ip_iface, error->message);
 			g_error_free (error);
 			aipd_cleanup (self);
 			*reason = NM_DEVICE_STATE_REASON_AUTOIP_START_FAILED;
@@ -891,7 +888,7 @@
 nm_device_activate_stage3_ip_config_start (gpointer user_data)
 {
 	NMDevice *self = NM_DEVICE (user_data);
-	const char *iface;
+	const char *ip_iface;
 	NMActStageReturn ret;
 	NMDeviceStateReason reason = NM_DEVICE_STATE_REASON_NONE;
 
@@ -899,8 +896,8 @@
 	if (self->priv->act_source_id > 0)
 		self->priv->act_source_id = 0;
 
-	iface = nm_device_get_iface (self);
-	nm_info ("Activation (%s) Stage 3 of 5 (IP Configure Start) started...", iface);
+	ip_iface = nm_device_get_iface (self);
+	nm_info ("Activation (%s) Stage 3 of 5 (IP Configure Start) started...", ip_iface);
 	nm_device_state_changed (self, NM_DEVICE_STATE_IP_CONFIG, NM_DEVICE_STATE_REASON_NONE);
 
 	ret = NM_DEVICE_GET_CLASS (self)->act_stage3_ip_config_start (self, &reason);
@@ -916,7 +913,7 @@
 	nm_device_activate_schedule_stage4_ip_config_get (self);
 
 out:
-	nm_info ("Activation (%s) Stage 3 of 5 (IP Configure Start) complete.", iface);
+	nm_info ("Activation (%s) Stage 3 of 5 (IP Configure Start) complete.", ip_iface);
 	return FALSE;
 }
 
@@ -939,7 +936,7 @@
 	self->priv->act_source_id = g_idle_add (nm_device_activate_stage3_ip_config_start, self);
 
 	nm_info ("Activation (%s) Stage 3 of 5 (IP Configure Start) scheduled.",
-	         nm_device_get_iface (self));
+	         nm_device_get_ip_iface (self));
 }
 
 static GHashTable *shared_ips = NULL;
@@ -1008,13 +1005,13 @@
 	NMActStageReturn ret = NM_ACT_STAGE_RETURN_FAILURE;
 	NMConnection *connection;
 	NMSettingIP4Config *s_ip4;
-	const char *iface;
+	const char *ip_iface;
 
 	g_return_val_if_fail (config != NULL, NM_ACT_STAGE_RETURN_FAILURE);
 	g_return_val_if_fail (*config == NULL, NM_ACT_STAGE_RETURN_FAILURE);
 	g_return_val_if_fail (reason != NULL, NM_ACT_STAGE_RETURN_FAILURE);
 
-	iface = nm_device_get_iface (self);
+	ip_iface = nm_device_get_ip_iface (self);
 
 	connection = nm_act_request_get_connection (nm_device_get_act_request (self));
 	g_assert (connection);
@@ -1022,11 +1019,11 @@
 	s_ip4 = (NMSettingIP4Config *) nm_connection_get_setting (connection, NM_TYPE_SETTING_IP4_CONFIG);
 
 	if (nm_device_get_use_dhcp (self)) {
-		*config = nm_dhcp_manager_get_ip4_config (priv->dhcp_manager, iface);
+		*config = nm_dhcp_manager_get_ip4_config (priv->dhcp_manager, ip_iface);
 		if (*config) {
 			nm_utils_merge_ip4_config (*config, s_ip4);
 
-			nm_dhcp_manager_set_dhcp4_config (priv->dhcp_manager, iface, priv->dhcp4_config);
+			nm_dhcp_manager_set_dhcp4_config (priv->dhcp_manager, ip_iface, priv->dhcp4_config);
 			/* Notify of new DHCP4 config */
 			g_object_notify (G_OBJECT (self), NM_DEVICE_INTERFACE_DHCP4_CONFIG);
 		} else
@@ -1046,7 +1043,7 @@
 		} else if (!strcmp (s_ip4->method, NM_SETTING_IP4_CONFIG_METHOD_SHARED)) {
 			*config = nm_device_new_ip4_shared_config (self, reason);
 			if (*config)
-				priv->dnsmasq_manager = nm_dnsmasq_manager_new (iface);
+				priv->dnsmasq_manager = nm_dnsmasq_manager_new (ip_iface);
 		}
 	}
 
@@ -1072,15 +1069,15 @@
 	NMDevice *self = NM_DEVICE (user_data);
 	NMIP4Config *ip4_config = NULL;
 	NMActStageReturn ret;
-	const char *iface = NULL;
+	const char *ip_iface = NULL;
 	NMDeviceStateReason reason = NM_DEVICE_STATE_REASON_NONE;
 
 	/* Clear the activation source ID now that this stage has run */
 	if (self->priv->act_source_id > 0)
 		self->priv->act_source_id = 0;
 
-	iface = nm_device_get_iface (self);
-	nm_info ("Activation (%s) Stage 4 of 5 (IP Configure Get) started...", iface);
+	ip_iface = nm_device_get_ip_iface (self);
+	nm_info ("Activation (%s) Stage 4 of 5 (IP Configure Get) started...", ip_iface);
 
 	ret = NM_DEVICE_GET_CLASS (self)->act_stage4_get_ip4_config (self, &ip4_config, &reason);
 	if (ret == NM_ACT_STAGE_RETURN_POSTPONE)
@@ -1098,7 +1095,7 @@
 	nm_device_activate_schedule_stage5_ip_config_commit (self);
 
 out:
-	nm_info ("Activation (%s) Stage 4 of 5 (IP Configure Get) complete.", iface);
+	nm_info ("Activation (%s) Stage 4 of 5 (IP Configure Get) complete.", ip_iface);
 	return FALSE;
 }
 
@@ -1122,7 +1119,7 @@
 	priv->act_source_id = g_idle_add (nm_device_activate_stage4_ip_config_get, self);
 
 	nm_info ("Activation (%s) Stage 4 of 5 (IP Configure Get) scheduled...",
-	         nm_device_get_iface (self));
+	         nm_device_get_ip_iface (self));
 }
 
 
@@ -1154,7 +1151,7 @@
 {
 	NMDevice *self = NM_DEVICE (user_data);
 	NMIP4Config *ip4_config = NULL;
-	const char *iface;
+	const char *ip_iface;
 	NMActStageReturn ret = NM_ACT_STAGE_RETURN_FAILURE;
 	NMDeviceStateReason reason = NM_DEVICE_STATE_REASON_NONE;
 
@@ -1162,8 +1159,8 @@
 	if (self->priv->act_source_id > 0)
 		self->priv->act_source_id = 0;
 
-	iface = nm_device_get_iface (self);
-	nm_info ("Activation (%s) Stage 4 of 5 (IP Configure Timeout) started...", iface);
+	ip_iface = nm_device_get_ip_iface (self);
+	nm_info ("Activation (%s) Stage 4 of 5 (IP Configure Timeout) started...", ip_iface);
 
 	ret = NM_DEVICE_GET_CLASS (self)->act_stage4_ip_config_timeout (self, &ip4_config, &reason);
 	if (ret == NM_ACT_STAGE_RETURN_POSTPONE) {
@@ -1181,7 +1178,7 @@
 	nm_device_activate_schedule_stage5_ip_config_commit (self);
 
 out:
-	nm_info ("Activation (%s) Stage 4 of 5 (IP Configure Timeout) complete.", iface);
+	nm_info ("Activation (%s) Stage 4 of 5 (IP Configure Timeout) complete.", ip_iface);
 	return FALSE;
 }
 
@@ -1205,7 +1202,7 @@
 	priv->act_source_id = g_idle_add (nm_device_activate_stage4_ip_config_timeout, self);
 
 	nm_info ("Activation (%s) Stage 4 of 5 (IP Configure Timeout) scheduled...",
-	         nm_device_get_iface (self));
+	         nm_device_get_ip_iface (self));
 }
 
 static void
@@ -1289,11 +1286,11 @@
 	guint32 netmask, network;
 	NMIP4Config *ip4_config;
 	const NMSettingIP4Address *ip4_addr;
-	const char *iface;
+	const char *ip_iface;
 
-	iface = nm_device_get_ip_iface (self);
-	if (!iface)
-		iface = nm_device_get_iface (self);
+	ip_iface = nm_device_get_ip_iface (self);
+	if (!ip_iface)
+		ip_iface = nm_device_get_ip_iface (self);
 
 	ip4_config = nm_device_get_ip4_config (self);
 	if (!ip4_config)
@@ -1317,21 +1314,21 @@
 	req = nm_device_get_act_request (self);
 	g_assert (req);
 
-	add_share_rule (req, "filter", "INPUT --in-interface %s --protocol tcp --destination-port 53 --jump ACCEPT", iface);
-	add_share_rule (req, "filter", "INPUT --in-interface %s --protocol udp --destination-port 53 --jump ACCEPT", iface);
-	add_share_rule (req, "filter", "INPUT --in-interface %s --protocol tcp --destination-port 67 --jump ACCEPT", iface);
-	add_share_rule (req, "filter", "INPUT --in-interface %s --protocol udp --destination-port 67 --jump ACCEPT", iface);
-	add_share_rule (req, "filter", "FORWARD --in-interface %s --jump REJECT", iface);
-	add_share_rule (req, "filter", "FORWARD --out-interface %s --jump REJECT", iface);
-	add_share_rule (req, "filter", "FORWARD --in-interface %s --out-interface %s --jump ACCEPT", iface, iface);
-	add_share_rule (req, "filter", "FORWARD --source %s/%s --in-interface %s --jump ACCEPT", str_addr, str_mask, iface);
-	add_share_rule (req, "filter", "FORWARD --destination %s/%s --out-interface %s --match state --state ESTABLISHED,RELATED --jump ACCEPT", str_addr, str_mask, iface);
+	add_share_rule (req, "filter", "INPUT --in-interface %s --protocol tcp --destination-port 53 --jump ACCEPT", ip_iface);
+	add_share_rule (req, "filter", "INPUT --in-interface %s --protocol udp --destination-port 53 --jump ACCEPT", ip_iface);
+	add_share_rule (req, "filter", "INPUT --in-interface %s --protocol tcp --destination-port 67 --jump ACCEPT", ip_iface);
+	add_share_rule (req, "filter", "INPUT --in-interface %s --protocol udp --destination-port 67 --jump ACCEPT", ip_iface);
+	add_share_rule (req, "filter", "FORWARD --in-interface %s --jump REJECT", ip_iface);
+	add_share_rule (req, "filter", "FORWARD --out-interface %s --jump REJECT", ip_iface);
+	add_share_rule (req, "filter", "FORWARD --in-interface %s --out-interface %s --jump ACCEPT", ip_iface, ip_iface);
+	add_share_rule (req, "filter", "FORWARD --source %s/%s --in-interface %s --jump ACCEPT", str_addr, str_mask, ip_iface);
+	add_share_rule (req, "filter", "FORWARD --destination %s/%s --out-interface %s --match state --state ESTABLISHED,RELATED --jump ACCEPT", str_addr, str_mask, ip_iface);
 	add_share_rule (req, "nat", "POSTROUTING --source %s/%s --destination ! %s/%s --jump MASQUERADE", str_addr, str_mask, str_addr, str_mask);
 
 	nm_act_request_set_shared (req, TRUE);
 
 	if (!nm_dnsmasq_manager_start (priv->dnsmasq_manager, ip4_config, &error)) {
-		nm_warning ("(%s): failed to start dnsmasq: %s", iface, error->message);
+		nm_warning ("(%s): failed to start dnsmasq: %s", ip_iface, error->message);
 		g_error_free (error);
 		nm_act_request_set_shared (req, FALSE);
 		return FALSE;
@@ -1355,7 +1352,7 @@
 	NMDevice *self = NM_DEVICE (user_data);
 	NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self);
 	NMIP4Config *ip4_config = NULL;
-	const char *iface;
+	const char *ip_iface;
 	NMConnection *connection;
 	NMSettingIP4Config *s_ip4;
 	NMDeviceStateReason reason = NM_DEVICE_STATE_REASON_NONE;
@@ -1368,9 +1365,9 @@
 	if (priv->act_source_id > 0)
 		priv->act_source_id = 0;
 
-	iface = nm_device_get_iface (self);
+	ip_iface = nm_device_get_ip_iface (self);
 	nm_info ("Activation (%s) Stage 5 of 5 (IP Configure Commit) started...",
-	         iface);
+	         ip_iface);
 
 	if (!nm_device_set_ip4_config (self, ip4_config, &reason)) {
 		nm_device_state_changed (self, NM_DEVICE_STATE_FAILED, reason);
@@ -1381,7 +1378,7 @@
 	s_ip4 = (NMSettingIP4Config *) nm_connection_get_setting (connection, NM_TYPE_SETTING_IP4_CONFIG);
 	if (s_ip4 && !strcmp (s_ip4->method, "shared")) {
 		if (!start_sharing (self)) {
-			nm_warning ("Activation (%s) Stage 5 of 5 (IP Configure Commit) start sharing failed.", iface);
+			nm_warning ("Activation (%s) Stage 5 of 5 (IP Configure Commit) start sharing failed.", ip_iface);
 			nm_device_state_changed (self, NM_DEVICE_STATE_FAILED, NM_DEVICE_STATE_REASON_SHARED_START_FAILED);
 			goto out;
 		}
@@ -1391,7 +1388,7 @@
 
 out:
 	nm_info ("Activation (%s) Stage 5 of 5 (IP Configure Commit) complete.",
-	         iface);
+	         ip_iface);
 
 	/* Balance IP4Config creation; device takes ownership in set_ip4_config() */
 	g_object_unref (ip4_config);
@@ -1418,7 +1415,7 @@
 	priv->act_source_id = g_idle_add (nm_device_activate_stage5_ip_config_commit, self);
 
 	nm_info ("Activation (%s) Stage 5 of 5 (IP Configure Commit) scheduled...",
-	         nm_device_get_iface (self));
+	         nm_device_get_ip_iface (self));
 }
 
 
@@ -1483,7 +1480,7 @@
 	/* Stop any ongoing DHCP transaction on this device */
 	if (nm_device_get_act_request (self)) {
 		if (nm_device_get_use_dhcp (self)) {
-			nm_dhcp_manager_cancel_transaction (priv->dhcp_manager, nm_device_get_iface (self));
+			nm_dhcp_manager_cancel_transaction (priv->dhcp_manager, nm_device_get_ip_iface (self));
 			nm_device_set_use_dhcp (self, FALSE);
 			/* Notify of invalid DHCP4 config */
 			g_object_notify (G_OBJECT (self), NM_DEVICE_INTERFACE_DHCP4_CONFIG);
@@ -1707,8 +1704,7 @@
 		return;
 	}
 
-	config = nm_dhcp_manager_get_ip4_config (NM_DEVICE_GET_PRIVATE (device)->dhcp_manager,
-											 nm_device_get_iface (device));
+	config = nm_dhcp_manager_get_ip4_config (NM_DEVICE_GET_PRIVATE (device)->dhcp_manager, nm_device_get_ip_iface (device));
 	if (!config) {
 		nm_warning ("failed to get DHCP config for rebind");
 		nm_device_state_changed (device, NM_DEVICE_STATE_FAILED, NM_DEVICE_STATE_REASON_IP_CONFIG_EXPIRED);
@@ -1730,12 +1726,12 @@
 		nm_device_state_changed (device, NM_DEVICE_STATE_FAILED, reason);
 	}
 
-	nm_dhcp_manager_set_dhcp4_config (priv->dhcp_manager, nm_device_get_iface (device), priv->dhcp4_config);
+	nm_dhcp_manager_set_dhcp4_config (priv->dhcp_manager, nm_device_get_ip_iface (device), priv->dhcp4_config);
 }
 
 static void
 dhcp_state_changed (NMDHCPManager *dhcp_manager,
-					const char *iface,
+					const char *ip_iface,
 					NMDHCPState state,
 					gpointer user_data)
 {
@@ -1743,7 +1739,7 @@
 	NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (device);
 	NMDeviceState dev_state;
 
-	if (strcmp (nm_device_get_iface (device), iface) != 0)
+	if (strcmp (nm_device_get_ip_iface (device), ip_iface) != 0)
 		return;
 
 	if (!nm_device_get_act_request (device))
@@ -1788,12 +1784,12 @@
 
 static void
 dhcp_timeout (NMDHCPManager *dhcp_manager,
-              const char *iface,
+              const char *ip_iface,
               gpointer user_data)
 {
 	NMDevice * device = NM_DEVICE (user_data);
 
-	if (strcmp (nm_device_get_iface (device), iface) != 0)
+	if (strcmp (nm_device_get_ip_iface (device), ip_iface) != 0)
 		return;
 
 	if (nm_device_get_state (device) == NM_DEVICE_STATE_IP_CONFIG)
@@ -1938,7 +1934,7 @@
 	}
 
 	memset (&req, 0, sizeof (struct ifreq));
-	strncpy (req.ifr_name, nm_device_get_iface (self), IFNAMSIZ);
+	strncpy (req.ifr_name, nm_device_get_ip_iface (self), IFNAMSIZ);
 	err = ioctl (fd, SIOCGIFADDR, &req);
 	close (fd);
 
@@ -1972,7 +1968,7 @@
 	if (nm_device_hw_is_up (self))
 		goto out;
 
-	nm_info ("(%s): bringing up device.", nm_device_get_iface (self));
+	nm_info ("(%s): bringing up device.", nm_device_get_ip_iface (self));
 
 	if (NM_DEVICE_GET_CLASS (self)->hw_bring_up) {
 		success = NM_DEVICE_GET_CLASS (self)->hw_bring_up (self, no_firmware);
@@ -1985,7 +1981,7 @@
 		g_usleep (200);
 
 	if (!nm_device_hw_is_up (self)) {
-		nm_warning ("(%s): device not up after timeout!", nm_device_get_iface (self));
+		nm_warning ("(%s): device not up after timeout!", nm_device_get_ip_iface (self));
 		return FALSE;
 	}
 
@@ -2008,7 +2004,7 @@
 	if (!nm_device_hw_is_up (self))
 		return;
 
-	nm_info ("(%s): taking down device.", nm_device_get_iface (self));
+	nm_info ("(%s): taking down device.", nm_device_get_ip_iface (self));
 
 	if (NM_DEVICE_GET_CLASS (self)->hw_take_down)
 		NM_DEVICE_GET_CLASS (self)->hw_take_down (self);
@@ -2031,7 +2027,7 @@
 	if (nm_device_is_up (self))
 		return TRUE;
 
-	nm_info ("(%s): preparing device.", nm_device_get_iface (self));
+	nm_info ("(%s): preparing device.", nm_device_get_ip_iface (self));
 
 	if (NM_DEVICE_GET_CLASS (self)->bring_up)
 		success = NM_DEVICE_GET_CLASS (self)->bring_up (self);
@@ -2048,7 +2044,7 @@
 		nm_device_interface_deactivate (NM_DEVICE_INTERFACE (self));
 
 	if (nm_device_is_up (self)) {
-		nm_info ("(%s): cleaning up...", nm_device_get_iface (self));
+		nm_info ("(%s): cleaning up...", nm_device_get_ip_iface (self));
 
 		if (NM_DEVICE_GET_CLASS (self)->take_down)
 			NM_DEVICE_GET_CLASS (self)->take_down (self);
@@ -2143,6 +2139,8 @@
 	case NM_DEVICE_INTERFACE_PROP_IFACE:
 		g_free (priv->iface);
 		priv->iface = g_value_dup_string (value);
+		g_free (priv->ip_iface);
+		priv->ip_iface = g_value_dup_string (value);
 		break;
 	case NM_DEVICE_INTERFACE_PROP_DRIVER:
 		priv->driver = g_strdup (g_value_get_string (value));