stemodem: Create network interfaces statically

PATCH Superseded Sjur Brændeland 3 participants 13 messages Nov 14, 2010 – Nov 23, 2010

lore

Share & tools

lore

Sjur Brændeland

author

2010-11-14 18:51 UTC #0 lore raw

PATCH 1/2 stemodem: Create network interfaces statically

From: Sjur Brændeland <[email protected]>

Changes:
o Restructure code so interfaces are created statically when probe is called.
o Removed some of the unnecessary initializations at declaration.
o No longer reporting "default gateway" on PtP IP Interface.
o Bugfix: Handle network initiated deactivation.

---
 drivers/stemodem/gprs-context.c |  153 ++++++++++++++++++++++-----------------
 1 files changed, 85 insertions(+), 68 deletions(-)

drivers/stemodem/gprs-context.c +85 ~~-68~~

index 05fec3f..d8db23b 100644
@@ -28,6 +28,7 @@
 #include <string.h>
 #include <stdlib.h>
 #include <stdio.h>
+#include <errno.h>
 
 #include <glib.h>

@@ -65,10 +66,18 @@ struct gprs_context_data {
 };
 
 struct conn_info {
+	/*
+	 * cid is allocated in oFono Core and is identifying
+	 * the Account. cid = 0 indicates that it is currently unused.
+	 */
 	unsigned int cid;
-	unsigned int device;
+	/* Id used by CAIF and EPPSD to identify the CAIF channel*/
 	unsigned int channel_id;
-	char interface[10];
+	/* Linux Interface Id */
+	unsigned int ifindex;
+	/* Linux Interface name */
+	char interface[IF_NAMESIZE];
+	gboolean created;
 };
 
 struct eppsd_response {

@@ -76,7 +85,6 @@ struct eppsd_response {
 	char ip_address[IP_ADDR_LEN];
 	char subnet_mask[IP_ADDR_LEN];
 	char mtu[IP_ADDR_LEN];
-	char default_gateway[IP_ADDR_LEN];
 	char dns_server1[IP_ADDR_LEN];
 	char dns_server2[IP_ADDR_LEN];
 	char p_cscf_server[IP_ADDR_LEN];

@@ -96,8 +104,6 @@ static void start_element_handler(GMarkupParseContext *context,
 		rsp->current = rsp->subnet_mask;
 	else if (!strcmp(element_name, "mtu"))
 		rsp->current = rsp->mtu;
-	else if (!strcmp(element_name, "default_gateway"))
-		rsp->current = rsp->default_gateway;
 	else if (!strcmp(element_name, "dns_server") &&
 					rsp->dns_server1[0] == '\0')
 		rsp->current = rsp->dns_server1;

@@ -123,7 +129,7 @@ static void text_handler(GMarkupParseContext *context,
 
 	if (rsp->current) {
 		strncpy(rsp->current, text, IP_ADDR_LEN);
-		rsp->current[IP_ADDR_LEN] = 0;
+		rsp->current[IP_ADDR_LEN] = '\0';
 	}
 }

@@ -153,8 +159,7 @@ static gint conn_compare_by_cid(gconstpointer a, gconstpointer b)
 	return 0;
 }
 
-static struct conn_info *conn_info_create(unsigned int device,
-						unsigned int channel_id)
+static struct conn_info *conn_info_create(unsigned int channel_id)
 {
 	struct conn_info *connection = g_try_new0(struct conn_info, 1);

@@ -162,7 +167,6 @@ static struct conn_info *conn_info_create(unsigned int device,
 		return NULL;
 
 	connection->cid = 0;
-	connection->device = device;
 	connection->channel_id = channel_id;
 
 	return connection;

@@ -171,7 +175,7 @@ static struct conn_info *conn_info_create(unsigned int device,
 /*
  * Creates a new IP interface for CAIF.
  */
-static gboolean caif_if_create(const char *interface, unsigned int connid)
+static gboolean caif_if_create(struct conn_info *conn)
 {
 	return FALSE;
 }

@@ -179,9 +183,8 @@ static gboolean caif_if_create(const char *interface, unsigned int connid)
 /*
  * Removes IP interface for CAIF.
  */
-static gboolean caif_if_remove(const char *interface, unsigned int connid)
+static void caif_if_remove(struct conn_info *conn)
 {
-	return FALSE;
 }
 
 static void ste_eppsd_down_cb(gboolean ok, GAtResult *result,

@@ -210,21 +213,13 @@ static void ste_eppsd_down_cb(gboolean ok, GAtResult *result,
 		DBG("Did not find data (used caif device) for"
 					"connection with cid; %d",
 					gcd->active_context);
-		goto error;
+		CALLBACK_WITH_FAILURE(cb, cbd->data);
+		return;
 	}
 
 	conn = l->data;
-
-	if (!caif_if_remove(conn->interface, conn->channel_id)) {
-		DBG("Failed to remove caif interface %s.",
-				conn->interface);
-	}
-
 	conn->cid = 0;
-	return;
-
-error:
-	CALLBACK_WITH_FAILURE(cb, cbd->data);
+	CALLBACK_WITH_SUCCESS(cb, cbd->data);
 }
 
 static void ste_eppsd_up_cb(gboolean ok, GAtResult *result, gpointer user_data)

@@ -233,7 +228,7 @@ static void ste_eppsd_up_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	ofono_gprs_context_up_cb_t cb = cbd->cb;
 	struct ofono_gprs_context *gc = cbd->user;
 	struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
-	struct conn_info *conn = NULL;
+	struct conn_info *conn;
 	GAtResultIter iter;
 	GSList *l;
 	int i;

@@ -241,17 +236,15 @@ static void ste_eppsd_up_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	const char *res_string;
 	const char *dns[MAX_DNS + 1];
 	struct eppsd_response rsp;
-	GMarkupParseContext *context = NULL;
+	GMarkupParseContext *context;
 
 	l = g_slist_find_custom(g_caif_devices,
 				GUINT_TO_POINTER(gcd->active_context),
 				conn_compare_by_cid);
 
 	if (!l) {
-		DBG("Did not find data (device and channel id)"
-					"for connection with cid; %d",
-					gcd->active_context);
-		goto error;
+		DBG("CAIF Device gone missing (cid:%d)", gcd->active_context);
+		goto error_no_device;
 	}
 
 	conn = l->data;

@@ -291,31 +284,21 @@ static void ste_eppsd_up_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	dns[1] = rsp.dns_server2;
 	dns[2] = NULL;
 
-	sprintf(conn->interface, "caif%u", conn->device);
-
-	if (!caif_if_create(conn->interface, conn->channel_id)) {
-		ofono_error("Failed to create caif interface %s.",
-				conn->interface);
-		CALLBACK_WITH_SUCCESS(cb, NULL, FALSE, rsp.ip_address,
-				rsp.subnet_mask, rsp.default_gateway,
+	CALLBACK_WITH_SUCCESS(cb, conn->interface, TRUE, rsp.ip_address,
+				rsp.subnet_mask, NULL,
 				dns, cbd->data);
-	} else {
-		CALLBACK_WITH_SUCCESS(cb, conn->interface,
-				FALSE, rsp.ip_address, rsp.subnet_mask,
-				rsp.default_gateway, dns, cbd->data);
-	}
-
 	return;
 
 error:
-	DBG("ste_eppsd_up_cb error");
-
 	if (context)
 		g_markup_parse_context_free(context);
 
 	if (conn)
 		conn->cid = 0;
 
+error_no_device:
+	DBG("ste_eppsd_up_cb error");
+	gcd->active_context = 0;
 	CALLBACK_WITH_FAILURE(cb, NULL, 0, NULL, NULL, NULL, NULL, cbd->data);
 }

@@ -325,44 +308,47 @@ static void ste_cgdcont_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	ofono_gprs_context_up_cb_t cb = cbd->cb;
 	struct ofono_gprs_context *gc = cbd->user;
 	struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
-	struct cb_data *ncbd = NULL;
-	char buf[128];
+	struct cb_data *ncbd;
 	struct conn_info *conn;
+	char buf[128];
 	GSList *l;
 
+	l = g_slist_find_custom(g_caif_devices,
+				GUINT_TO_POINTER(gcd->active_context),
+				conn_compare_by_cid);
+
+	if (!l) {
+		DBG("CAIF Device gone missing (cid:%d)", gcd->active_context);
+		goto error_no_device;
+	}
+
+	conn = l->data;
+
 	if (!ok) {
 		struct ofono_error error;
 
+		conn->cid = 0;
 		gcd->active_context = 0;
 		decode_at_error(&error, g_at_result_final_response(result));
 		cb(&error, NULL, 0, NULL, NULL, NULL, NULL, cbd->data);
 		return;
 	}
 
-	ncbd = g_memdup(cbd, sizeof(struct cb_data));
-
-	l = g_slist_find_custom(g_caif_devices, GUINT_TO_POINTER(0),
-				conn_compare_by_cid);
-
-	if (!l) {
-		DBG("at_cgdcont_cb, no more available devices");
-		goto error;
-	}
-
-	conn = l->data;
-	conn->cid = gcd->active_context;
 	snprintf(buf, sizeof(buf), "AT*EPPSD=1,%u,%u",
 			conn->channel_id, conn->cid);
+	ncbd = g_memdup(cbd, sizeof(struct cb_data));
 
 	if (g_at_chat_send(gcd->chat, buf, NULL,
 				ste_eppsd_up_cb, ncbd, g_free) > 0)
 		return;
 
-error:
 	g_free(ncbd);
 
-	gcd->active_context = 0;
+error:
+	conn->cid = 0;
 
+error_no_device:
+	gcd->active_context = 0;
 	CALLBACK_WITH_FAILURE(cb, NULL, 0, NULL, NULL,
 				NULL, NULL, cbd->data);
 }

@@ -375,13 +361,32 @@ static void ste_gprs_activate_primary(struct ofono_gprs_context *gc,
 	struct cb_data *cbd = cb_data_new(cb, data);
 	char buf[AUTH_BUF_LENGTH];
 	int len;
+	GSList *l;
+	struct conn_info *conn;
 
 	if (!cbd)
-		goto error;
+		goto error_no_device;
 
 	gcd->active_context = ctx->cid;
 	cbd->user = gc;
 
+	/* Find free connection with cid zero */
+	l = g_slist_find_custom(g_caif_devices, GUINT_TO_POINTER(0),
+				conn_compare_by_cid);
+
+	if (!l) {
+		DBG("No more available CAIF devices");
+		goto error_no_device;
+	}
+
+	conn = l->data;
+	conn->cid = ctx->cid;
+
+	if (!conn->created) {
+		DBG("CAIF interface not created (rtnl error?)");
+		goto error;
+	}
+
 	len = snprintf(buf, sizeof(buf), "AT+CGDCONT=%u,\"IP\"", ctx->cid);
 
 	if (ctx->apn)

@@ -405,6 +410,9 @@ static void ste_gprs_activate_primary(struct ofono_gprs_context *gc,
 	return;
 
 error:
+	conn->cid = 0;
+
+error_no_device:
 	gcd->active_context = 0;
 	g_free(cbd);

@@ -431,8 +439,8 @@ static void ste_gprs_deactivate_primary(struct ofono_gprs_context *gc,
 				conn_compare_by_cid);
 
 	if (!l) {
-		DBG("at_gprs_deactivate_primary, did not find"
-			"data (channel id) for connection with cid; %d", id);
+		DBG("did not find data (channel id) "
+				"for connection with cid; %d", id);
 		goto error;
 	}

@@ -446,7 +454,6 @@ static void ste_gprs_deactivate_primary(struct ofono_gprs_context *gc,
 
 error:
 	g_free(cbd);
-
 	CALLBACK_WITH_FAILURE(cb, data);
 }

@@ -457,6 +464,7 @@ static void ste_cgact_read_cb(gboolean ok, GAtResult *result,
 	struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
 	gint cid, state;
 	GAtResultIter iter;
+	GSList *l;
 
 	if (!ok)
 		return;

@@ -480,7 +488,13 @@ static void ste_cgact_read_cb(gboolean ok, GAtResult *result,
 		ofono_gprs_context_deactivated(gc, gcd->active_context);
 		gcd->active_context = 0;
 
-		break;
+		/* Mark interface as unused */
+		l = g_slist_find_custom(g_caif_devices, GUINT_TO_POINTER(cid),
+				conn_compare_by_cid);
+		if (l) {
+			struct conn_info *conn = l->data;
+			conn->cid = 0;
+		}
 	}
 }

@@ -524,9 +538,11 @@ static int ste_gprs_context_probe(struct ofono_gprs_context *gc,
 	ofono_gprs_context_set_data(gc, gcd);
 
 	for (i = 0; i < MAX_CAIF_DEVICES; i++) {
-		ci = conn_info_create(i, i+1);
-		if (ci)
-			g_caif_devices = g_slist_append(g_caif_devices, ci);
+		ci = conn_info_create(i+1);
+		if (!ci)
+			return -ENOMEM;
+		caif_if_create(ci);
+		g_caif_devices = g_slist_append(g_caif_devices, ci);
 	}
 
 	return 0;

@@ -536,6 +552,7 @@ static void ste_gprs_context_remove(struct ofono_gprs_context *gc)
 {
 	struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
 
+	g_slist_foreach(g_caif_devices, (GFunc) caif_if_remove, NULL);
 	g_slist_foreach(g_caif_devices, (GFunc) g_free, NULL);
 	g_slist_free(g_caif_devices);
 	g_caif_devices = NULL;

Signature · 1 line

1.7.0.4

Sjur Brændeland

author

2010-11-14 18:51 UTC #1 lore raw

PATCH 2/2 stemodem: Use RTNL to create network interfaces.

From: Sjur Brændeland <[email protected]>

---
 drivers/stemodem/gprs-context.c |   43 ++++++++++++++++++++++++++++++++++++++-
 1 files changed, 42 insertions(+), 1 deletions(-)

drivers/stemodem/gprs-context.c +42 -1

index d8db23b..899379c 100644
@@ -47,6 +47,7 @@
 #include "stemodem.h"
 #include "caif_socket.h"
 #include "if_caif.h"
+#include "caif_rtnl.h"
 
 #define MAX_CAIF_DEVICES 4
 #define MAX_DNS 2

@@ -172,12 +173,38 @@ static struct conn_info *conn_info_create(unsigned int channel_id)
 	return connection;
 }
 
+static void rtnl_callback(int ifindex, char *ifname, void *user_data)
+{
+	struct conn_info *conn = user_data;
+
+	if (ifindex < 0) {
+		conn->created = FALSE;
+		ofono_error("Failed to create caif interface %s",
+			conn->interface);
+		return;
+	}
+
+	strncpy(conn->interface, ifname, sizeof(conn->interface));
+	conn->ifindex = ifindex;
+	conn->created = TRUE;
+}
+
 /*
  * Creates a new IP interface for CAIF.
  */
 static gboolean caif_if_create(struct conn_info *conn)
 {
-	return FALSE;
+	int err;
+
+	err = caif_rtnl_create_interface(IFLA_CAIF_IPV4_CONNID,
+						conn->channel_id, FALSE,
+						rtnl_callback, conn);
+	if (err < 0) {
+		DBG("Failed to create IP interface for CAIF");
+		return FALSE;
+	}
+
+	return TRUE;
 }
 
 /*

@@ -185,6 +212,18 @@ static gboolean caif_if_create(struct conn_info *conn)
  */
 static void caif_if_remove(struct conn_info *conn)
 {
+	if (!conn->created)
+		return;
+
+	if (caif_rtnl_delete_interface(conn->ifindex) < 0) {
+		ofono_error("Failed to delete caif interface %s",
+			conn->interface);
+		return;
+	}
+
+	DBG("removed CAIF interface ch:%d ifname:%s ifindex:%d\n",
+		conn->channel_id, conn->interface, conn->ifindex);
+	return;
 }
 
 static void ste_eppsd_down_cb(gboolean ok, GAtResult *result,

@@ -573,10 +612,12 @@ static struct ofono_gprs_context_driver driver = {
 
 void ste_gprs_context_init()
 {
+	caif_rtnl_init();
 	ofono_gprs_context_driver_register(&driver);
 }
 
 void ste_gprs_context_exit()
 {
+	caif_rtnl_exit();
 	ofono_gprs_context_driver_unregister(&driver);
 }

Signature · 1 line

1.7.0.4

Sjur Brændeland

author

2010-11-18 08:03 UTC #2 lore raw

In reply to Sjur Brændeland

Hi Denis & Marcel.

drivers/stemodem/gprs-context.c

Quoted 15 lines

From: Sjur Brændeland <[email protected]>
Changes:
o Restructure code so interfaces are created statically when probe is called.
o Removed some of the unnecessary initializations at declaration.
o No longer reporting "default gateway" on PtP IP Interface.
o Bugfix: Handle network initiated deactivation.
---
drivers/stemodem/gprs-context.c | 153 ++++++++++++++++++++++-----------------
 1 files changed, 85 insertions(+), 68 deletions(-)
drivers/stemodem/gprs-context.c +0 -0
index 05fec3f..d8db23b 100644

Have you received this patch in readable format?
I'm sending patches using git send-email over gmail,
and when I check sent items in gmail, it comes up as base64 encoded. Is this garbled for you as well?

BTW are you using patchwork or similar so I could check the patch status somewhere?

Regards,
Sjur

Denis Kenzior

2010-11-22 10:34 UTC #3 lore raw

Hi Sjur,

Quoted 13 lines · Sjur Brændeland

Hi Denis & Marcel.
Quoted 11 lines
From: Sjur Brændeland <[email protected]>
Changes:
o Restructure code so interfaces are created statically when probe is called.
o Removed some of the unnecessary initializations at declaration.
o No longer reporting "default gateway" on PtP IP Interface.
o Bugfix: Handle network initiated deactivation.
---
 drivers/stemodem/gprs-context.c |  153 ++++++++++++++++++++++-----------------
 1 files changed, 85 insertions(+), 68 deletions(-)

Quoted 9 lines

drivers/stemodem/gprs-context.c
Quoted 4 lines
drivers/stemodem/gprs-context.c +0 -0
index 05fec3f..d8db23b 100644
Have you received this patch in readable format?
I'm sending patches using git send-email over gmail,
and when I check sent items in gmail, it comes up as base64 encoded. Is this garbled for you as well?

They seem to come through fine for me at least.

Quoted 4 lines

BTW are you using patchwork or similar so I could check the patch status somewhere?

Nope, but it has been suggested that this might be a good idea. I'm open to the idea of trying this out, but don't promise I will like it ;)

Quoted 2 lines

Regards,
Sjur

Regards,
-Denis

Sjur Brændeland

author

2010-11-22 11:32 UTC #4 lore raw

Hi Denis.

Quoted 6 lines

Quoted 4 lines
Have you received this patch in readable format?
I'm sending patches using git send-email over gmail,
and when I check sent items in gmail, it comes up as base64 encoded. Is this garbled for you as well?
They seem to come through fine for me at least.

Good, that's a relief - thank you for checking this.

Regards,
Sjur

Sjur Brændeland

author

2010-11-21 20:55 UTC #5 lore raw

PATCH v2 1/2 stemodem: Create network interfaces statically

In reply to Sjur Brændeland

From: Sjur Brændeland <[email protected]>

 drivers/stemodem/gprs-context.c |  151 +++++++++++++++++++++-----------------
 1 files changed, 83 insertions(+), 68 deletions(-)

drivers/stemodem/gprs-context.c +83 ~~-68~~

index 05fec3f..3ccda87 100644
@@ -28,6 +28,7 @@
 #include <string.h>
 #include <stdlib.h>
 #include <stdio.h>
+#include <errno.h>
 
 #include <glib.h>

@@ -65,10 +66,18 @@ struct gprs_context_data {
 };
 
 struct conn_info {
+	/*
+	 * cid is allocated in oFono Core and is identifying
+	 * the Account. cid = 0 indicates that it is currently unused.
+	 */
 	unsigned int cid;
-	unsigned int device;
+	/* Id used by CAIF and EPPSD to identify the CAIF channel*/
 	unsigned int channel_id;
-	char interface[10];
+	/* Linux Interface Id */
+	unsigned int ifindex;
+	/* Linux Interface name */
+	char interface[IF_NAMESIZE];
+	gboolean created;
 };
 
 struct eppsd_response {

@@ -76,7 +85,6 @@ struct eppsd_response {
 	char ip_address[IP_ADDR_LEN];
 	char subnet_mask[IP_ADDR_LEN];
 	char mtu[IP_ADDR_LEN];
-	char default_gateway[IP_ADDR_LEN];
 	char dns_server1[IP_ADDR_LEN];
 	char dns_server2[IP_ADDR_LEN];
 	char p_cscf_server[IP_ADDR_LEN];

@@ -96,8 +104,6 @@ static void start_element_handler(GMarkupParseContext *context,
 		rsp->current = rsp->subnet_mask;
 	else if (!strcmp(element_name, "mtu"))
 		rsp->current = rsp->mtu;
-	else if (!strcmp(element_name, "default_gateway"))
-		rsp->current = rsp->default_gateway;
 	else if (!strcmp(element_name, "dns_server") &&
 					rsp->dns_server1[0] == '\0')
 		rsp->current = rsp->dns_server1;

@@ -123,7 +129,7 @@ static void text_handler(GMarkupParseContext *context,
 
 	if (rsp->current) {
 		strncpy(rsp->current, text, IP_ADDR_LEN);
-		rsp->current[IP_ADDR_LEN] = 0;
+		rsp->current[IP_ADDR_LEN] = '\0';
 	}
 }

@@ -153,8 +159,7 @@ static gint conn_compare_by_cid(gconstpointer a, gconstpointer b)
 	return 0;
 }
 
-static struct conn_info *conn_info_create(unsigned int device,
-						unsigned int channel_id)
+static struct conn_info *conn_info_create(unsigned int channel_id)
 {
 	struct conn_info *connection = g_try_new0(struct conn_info, 1);

@@ -162,7 +167,6 @@ static struct conn_info *conn_info_create(unsigned int device,
 		return NULL;
 
 	connection->cid = 0;
-	connection->device = device;
 	connection->channel_id = channel_id;
 
 	return connection;

@@ -171,7 +175,7 @@ static struct conn_info *conn_info_create(unsigned int device,
 /*
  * Creates a new IP interface for CAIF.
  */
-static gboolean caif_if_create(const char *interface, unsigned int connid)
+static gboolean caif_if_create(struct conn_info *conn)
 {
 	return FALSE;
 }

@@ -179,9 +183,8 @@ static gboolean caif_if_create(const char *interface, unsigned int connid)
 /*
  * Removes IP interface for CAIF.
  */
-static gboolean caif_if_remove(const char *interface, unsigned int connid)
+static void caif_if_remove(struct conn_info *conn)
 {
-	return FALSE;
 }
 
 static void ste_eppsd_down_cb(gboolean ok, GAtResult *result,

@@ -210,21 +213,13 @@ static void ste_eppsd_down_cb(gboolean ok, GAtResult *result,
 		DBG("Did not find data (used caif device) for"
 					"connection with cid; %d",
 					gcd->active_context);
-		goto error;
+		CALLBACK_WITH_FAILURE(cb, cbd->data);
+		return;
 	}
 
 	conn = l->data;
-
-	if (!caif_if_remove(conn->interface, conn->channel_id)) {
-		DBG("Failed to remove caif interface %s.",
-				conn->interface);
-	}
-
 	conn->cid = 0;
-	return;
-
-error:
-	CALLBACK_WITH_FAILURE(cb, cbd->data);
+	CALLBACK_WITH_SUCCESS(cb, cbd->data);
 }
 
 static void ste_eppsd_up_cb(gboolean ok, GAtResult *result, gpointer user_data)

@@ -233,7 +228,7 @@ static void ste_eppsd_up_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	ofono_gprs_context_up_cb_t cb = cbd->cb;
 	struct ofono_gprs_context *gc = cbd->user;
 	struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
-	struct conn_info *conn = NULL;
+	struct conn_info *conn;
 	GAtResultIter iter;
 	GSList *l;
 	int i;

@@ -241,17 +236,15 @@ static void ste_eppsd_up_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	const char *res_string;
 	const char *dns[MAX_DNS + 1];
 	struct eppsd_response rsp;
-	GMarkupParseContext *context = NULL;
+	GMarkupParseContext *context;
 
 	l = g_slist_find_custom(g_caif_devices,
 				GUINT_TO_POINTER(gcd->active_context),
 				conn_compare_by_cid);
 
 	if (!l) {
-		DBG("Did not find data (device and channel id)"
-					"for connection with cid; %d",
-					gcd->active_context);
-		goto error;
+		DBG("CAIF Device gone missing (cid:%d)", gcd->active_context);
+		goto error_no_device;
 	}
 
 	conn = l->data;

@@ -291,31 +284,21 @@ static void ste_eppsd_up_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	dns[1] = rsp.dns_server2;
 	dns[2] = NULL;
 
-	sprintf(conn->interface, "caif%u", conn->device);
-
-	if (!caif_if_create(conn->interface, conn->channel_id)) {
-		ofono_error("Failed to create caif interface %s.",
-				conn->interface);
-		CALLBACK_WITH_SUCCESS(cb, NULL, FALSE, rsp.ip_address,
-				rsp.subnet_mask, rsp.default_gateway,
+	CALLBACK_WITH_SUCCESS(cb, conn->interface, TRUE, rsp.ip_address,
+				rsp.subnet_mask, NULL,
 				dns, cbd->data);
-	} else {
-		CALLBACK_WITH_SUCCESS(cb, conn->interface,
-				FALSE, rsp.ip_address, rsp.subnet_mask,
-				rsp.default_gateway, dns, cbd->data);
-	}
-
 	return;
 
 error:
-	DBG("ste_eppsd_up_cb error");
-
 	if (context)
 		g_markup_parse_context_free(context);
 
 	if (conn)
 		conn->cid = 0;
 
+error_no_device:
+	DBG("ste_eppsd_up_cb error");
+	gcd->active_context = 0;
 	CALLBACK_WITH_FAILURE(cb, NULL, 0, NULL, NULL, NULL, NULL, cbd->data);
 }

@@ -325,44 +308,45 @@ static void ste_cgdcont_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	ofono_gprs_context_up_cb_t cb = cbd->cb;
 	struct ofono_gprs_context *gc = cbd->user;
 	struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
-	struct cb_data *ncbd = NULL;
-	char buf[128];
+	struct cb_data *ncbd;
 	struct conn_info *conn;
+	char buf[128];
 	GSList *l;
 
+	l = g_slist_find_custom(g_caif_devices,
+				GUINT_TO_POINTER(gcd->active_context),
+				conn_compare_by_cid);
+
+	if (!l) {
+		DBG("CAIF Device gone missing (cid:%d)", gcd->active_context);
+		goto error_no_device;
+	}
+
+	conn = l->data;
+
 	if (!ok) {
 		struct ofono_error error;
 
+		conn->cid = 0;
 		gcd->active_context = 0;
 		decode_at_error(&error, g_at_result_final_response(result));
 		cb(&error, NULL, 0, NULL, NULL, NULL, NULL, cbd->data);
 		return;
 	}
 
-	ncbd = g_memdup(cbd, sizeof(struct cb_data));
-
-	l = g_slist_find_custom(g_caif_devices, GUINT_TO_POINTER(0),
-				conn_compare_by_cid);
-
-	if (!l) {
-		DBG("at_cgdcont_cb, no more available devices");
-		goto error;
-	}
-
-	conn = l->data;
-	conn->cid = gcd->active_context;
 	snprintf(buf, sizeof(buf), "AT*EPPSD=1,%u,%u",
 			conn->channel_id, conn->cid);
+	ncbd = g_memdup(cbd, sizeof(struct cb_data));
 
 	if (g_at_chat_send(gcd->chat, buf, NULL,
 				ste_eppsd_up_cb, ncbd, g_free) > 0)
 		return;
 
-error:
 	g_free(ncbd);
+	conn->cid = 0;
 
+error_no_device:
 	gcd->active_context = 0;
-
 	CALLBACK_WITH_FAILURE(cb, NULL, 0, NULL, NULL,
 				NULL, NULL, cbd->data);
 }

@@ -375,13 +359,32 @@ static void ste_gprs_activate_primary(struct ofono_gprs_context *gc,
 	struct cb_data *cbd = cb_data_new(cb, data);
 	char buf[AUTH_BUF_LENGTH];
 	int len;
+	GSList *l;
+	struct conn_info *conn;
 
 	if (!cbd)
-		goto error;
+		goto error_no_device;
 
 	gcd->active_context = ctx->cid;
 	cbd->user = gc;
 
+	/* Find free connection with cid zero */
+	l = g_slist_find_custom(g_caif_devices, GUINT_TO_POINTER(0),
+				conn_compare_by_cid);
+
+	if (!l) {
+		DBG("No more available CAIF devices");
+		goto error_no_device;
+	}
+
+	conn = l->data;
+	conn->cid = ctx->cid;
+
+	if (!conn->created) {
+		DBG("CAIF interface not created (rtnl error?)");
+		goto error;
+	}
+
 	len = snprintf(buf, sizeof(buf), "AT+CGDCONT=%u,\"IP\"", ctx->cid);
 
 	if (ctx->apn)

@@ -405,6 +408,9 @@ static void ste_gprs_activate_primary(struct ofono_gprs_context *gc,
 	return;
 
 error:
+	conn->cid = 0;
+
+error_no_device:
 	gcd->active_context = 0;
 	g_free(cbd);

@@ -431,8 +437,8 @@ static void ste_gprs_deactivate_primary(struct ofono_gprs_context *gc,
 				conn_compare_by_cid);
 
 	if (!l) {
-		DBG("at_gprs_deactivate_primary, did not find"
-			"data (channel id) for connection with cid; %d", id);
+		DBG("did not find data (channel id) "
+				"for connection with cid; %d", id);
 		goto error;
 	}

@@ -446,7 +452,6 @@ static void ste_gprs_deactivate_primary(struct ofono_gprs_context *gc,
 
 error:
 	g_free(cbd);
-
 	CALLBACK_WITH_FAILURE(cb, data);
 }

@@ -457,6 +462,7 @@ static void ste_cgact_read_cb(gboolean ok, GAtResult *result,
 	struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
 	gint cid, state;
 	GAtResultIter iter;
+	GSList *l;
 
 	if (!ok)
 		return;

@@ -480,7 +486,13 @@ static void ste_cgact_read_cb(gboolean ok, GAtResult *result,
 		ofono_gprs_context_deactivated(gc, gcd->active_context);
 		gcd->active_context = 0;
 
-		break;
+		/* Mark interface as unused */
+		l = g_slist_find_custom(g_caif_devices, GUINT_TO_POINTER(cid),
+				conn_compare_by_cid);
+		if (l) {
+			struct conn_info *conn = l->data;
+			conn->cid = 0;
+		}
 	}
 }

@@ -524,9 +536,11 @@ static int ste_gprs_context_probe(struct ofono_gprs_context *gc,
 	ofono_gprs_context_set_data(gc, gcd);
 
 	for (i = 0; i < MAX_CAIF_DEVICES; i++) {
-		ci = conn_info_create(i, i+1);
-		if (ci)
-			g_caif_devices = g_slist_append(g_caif_devices, ci);
+		ci = conn_info_create(i+1);
+		if (!ci)
+			return -ENOMEM;
+		caif_if_create(ci);
+		g_caif_devices = g_slist_append(g_caif_devices, ci);
 	}
 
 	return 0;

@@ -536,6 +550,7 @@ static void ste_gprs_context_remove(struct ofono_gprs_context *gc)
 {
 	struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
 
+	g_slist_foreach(g_caif_devices, (GFunc) caif_if_remove, NULL);
 	g_slist_foreach(g_caif_devices, (GFunc) g_free, NULL);
 	g_slist_free(g_caif_devices);
 	g_caif_devices = NULL;

Signature · 1 line

1.7.0.4

Sjur Brændeland

author

2010-11-21 20:55 UTC #6 lore raw

PATCH v2 2/2 stemodem: Use RTNL to create network interfaces.

In reply to Sjur Brændeland

From: Sjur Brændeland <[email protected]>

---
 drivers/stemodem/gprs-context.c |   43 ++++++++++++++++++++++++++++++++++++++-
 1 files changed, 42 insertions(+), 1 deletions(-)

drivers/stemodem/gprs-context.c +42 -1

index 3ccda87..3859107 100644
@@ -47,6 +47,7 @@
 #include "stemodem.h"
 #include "caif_socket.h"
 #include "if_caif.h"
+#include "caif_rtnl.h"
 
 #define MAX_CAIF_DEVICES 4
 #define MAX_DNS 2

@@ -172,12 +173,38 @@ static struct conn_info *conn_info_create(unsigned int channel_id)
 	return connection;
 }
 
+static void rtnl_callback(int ifindex, const char *ifname, void *user_data)
+{
+	struct conn_info *conn = user_data;
+
+	if (ifindex < 0) {
+		conn->created = FALSE;
+		ofono_error("Failed to create caif interface %s",
+			conn->interface);
+		return;
+	}
+
+	strncpy(conn->interface, ifname, sizeof(conn->interface));
+	conn->ifindex = ifindex;
+	conn->created = TRUE;
+}
+
 /*
  * Creates a new IP interface for CAIF.
  */
 static gboolean caif_if_create(struct conn_info *conn)
 {
-	return FALSE;
+	int err;
+
+	err = caif_rtnl_create_interface(IFLA_CAIF_IPV4_CONNID,
+						conn->channel_id, FALSE,
+						rtnl_callback, conn);
+	if (err < 0) {
+		DBG("Failed to create IP interface for CAIF");
+		return FALSE;
+	}
+
+	return TRUE;
 }
 
 /*

@@ -185,6 +212,18 @@ static gboolean caif_if_create(struct conn_info *conn)
  */
 static void caif_if_remove(struct conn_info *conn)
 {
+	if (!conn->created)
+		return;
+
+	if (caif_rtnl_delete_interface(conn->ifindex) < 0) {
+		ofono_error("Failed to delete caif interface %s",
+			conn->interface);
+		return;
+	}
+
+	DBG("removed CAIF interface ch:%d ifname:%s ifindex:%d\n",
+		conn->channel_id, conn->interface, conn->ifindex);
+	return;
 }
 
 static void ste_eppsd_down_cb(gboolean ok, GAtResult *result,

@@ -571,10 +610,12 @@ static struct ofono_gprs_context_driver driver = {
 
 void ste_gprs_context_init()
 {
+	caif_rtnl_init();
 	ofono_gprs_context_driver_register(&driver);
 }
 
 void ste_gprs_context_exit()
 {
+	caif_rtnl_exit();
 	ofono_gprs_context_driver_unregister(&driver);
 }

Signature · 1 line

1.7.0.4

George Matveev

2010-11-21 22:20 UTC #7 lore raw

Re: [PATCH v2 2/2] stemodem: Use RTNL to create network interfaces.

Hi Sjur,

it looks as if two functions:

caif_if_create/remove (struct conn_info *conn)

do nothing but verify the return results of functions:

caif_rtnl_create_interface/caif_rtnl_delete_interface

declared in in caif_rtnl.h

Which means if you #include caif_rtnl.h in gprs-context.c create/delete_interface will be available directly without any wrappers and could make your code run a bit faster and easier to understand? Unless I missed something.

Regards,
George

Quoted 103 lines

From: Sjur BrÃ¦ndeland <[email protected]>

---
 drivers/stemodem/gprs-context.c |   43
++++++++++++++++++++++++++++++++++++++-
 1 files changed, 42 insertions(+), 1 deletions(-)

drivers/stemodem/gprs-context.c +28 -1

index 3ccda87..3859107 100644
@@ -47,6 +47,7 @@
 #include "stemodem.h"
 #include "caif_socket.h"
 #include "if_caif.h"
+#include "caif_rtnl.h"
 
 #define MAX_CAIF_DEVICES 4
 #define MAX_DNS 2

@@ -172,12 +173,38 @@ static struct conn_info *conn_info_create(unsigned
 int channel_id)
 	return connection;
 }
 
+static void rtnl_callback(int ifindex, const char *ifname, void
 *user_data)
+{
+	struct conn_info *conn = user_data;
+
+	if (ifindex < 0) {
+		conn->created = FALSE;
+		ofono_error("Failed to create caif interface %s",
+			conn->interface);
+		return;
+	}
+
+	strncpy(conn->interface, ifname, sizeof(conn->interface));
+	conn->ifindex = ifindex;
+	conn->created = TRUE;
+}
+
 /*
  * Creates a new IP interface for CAIF.
  */
 static gboolean caif_if_create(struct conn_info *conn)
 {
-	return FALSE;
+	int err;
+
+	err = caif_rtnl_create_interface(IFLA_CAIF_IPV4_CONNID,
+						conn->channel_id, FALSE,
+						rtnl_callback, conn);
+	if (err < 0) {
+		DBG("Failed to create IP interface for CAIF");
+		return FALSE;
+	}
+
+	return TRUE;
 }

drivers/stemodem/gprs-context.c +12 -0

@@ -185,6 +212,18 @@ static gboolean caif_if_create(struct conn_info
 *conn)
  */
 static void caif_if_remove(struct conn_info *conn)
 {
+	if (!conn->created)
+		return;
+
+	if (caif_rtnl_delete_interface(conn->ifindex) < 0) {
+		ofono_error("Failed to delete caif interface %s",
+			conn->interface);
+		return;
+	}
+
+	DBG("removed CAIF interface ch:%d ifname:%s ifindex:%d\n",
+		conn->channel_id, conn->interface, conn->ifindex);
+	return;
 }

static void ste_eppsd_down_cb(gboolean ok, GAtResult *result,

drivers/stemodem/gprs-context.c +2 -1

@@ -571,10 +610,12 @@ static struct ofono_gprs_context_driver driver = {
 
 void ste_gprs_context_init()
 {
+	caif_rtnl_init();
 	ofono_gprs_context_driver_register(&driver);
 }
 
 void ste_gprs_context_exit()
 {
+	caif_rtnl_exit();
 	ofono_gprs_context_driver_unregister(&driver);
 }
--

1.7.0.4

_______________________________________________
ofono mailing list
ofono(a)ofono.org
http://lists.ofono.org/listinfo/ofono

Sjur Brændeland

author

2010-11-21 22:54 UTC #8 lore raw

Re: [PATCH v2 2/2] stemodem: Use RTNL to create network interfaces.

Hi George.

Quoted 6 lines

it looks as if two functions:
caif_if_create/remove (struct conn_info *conn)
do nothing but verify the return results of functions:

Quoted 8 lines

caif_rtnl_create_interface/caif_rtnl_delete_interface
declared in in caif_rtnl.h
Which means if you #include caif_rtnl.h in gprs-context.c create/delete_interface will be available directly without any wrappers and could make your code run a bit faster and easier to understand? Unless I missed something.

Thank you for reviewing this patch :-)
Yes, you are right concerning caif_if_create, I'll change this. But caif_if_remove needs to stay as is,
it is called from g_slist_foreach and must have the list node as argument.

BTW you should take a look at http://ofono.org/wiki/ofono-etiquette.
Top-posting is not exactly popular on this list ;-)

Regards,
Sjur

Sjur Brændeland

author

2010-11-21 23:01 UTC #9 lore raw

PATCH v3 1/2 stemodem: Create network interfaces statically

In reply to George Matveev

From: Sjur Brændeland <[email protected]>

---
No changes from v2.

 drivers/stemodem/gprs-context.c |  151 +++++++++++++++++++++-----------------
 1 files changed, 83 insertions(+), 68 deletions(-)

drivers/stemodem/gprs-context.c +83 ~~-68~~

index 05fec3f..3ccda87 100644
@@ -28,6 +28,7 @@
 #include <string.h>
 #include <stdlib.h>
 #include <stdio.h>
+#include <errno.h>
 
 #include <glib.h>

@@ -65,10 +66,18 @@ struct gprs_context_data {
 };
 
 struct conn_info {
+	/*
+	 * cid is allocated in oFono Core and is identifying
+	 * the Account. cid = 0 indicates that it is currently unused.
+	 */
 	unsigned int cid;
-	unsigned int device;
+	/* Id used by CAIF and EPPSD to identify the CAIF channel*/
 	unsigned int channel_id;
-	char interface[10];
+	/* Linux Interface Id */
+	unsigned int ifindex;
+	/* Linux Interface name */
+	char interface[IF_NAMESIZE];
+	gboolean created;
 };
 
 struct eppsd_response {

@@ -76,7 +85,6 @@ struct eppsd_response {
 	char ip_address[IP_ADDR_LEN];
 	char subnet_mask[IP_ADDR_LEN];
 	char mtu[IP_ADDR_LEN];
-	char default_gateway[IP_ADDR_LEN];
 	char dns_server1[IP_ADDR_LEN];
 	char dns_server2[IP_ADDR_LEN];
 	char p_cscf_server[IP_ADDR_LEN];

@@ -96,8 +104,6 @@ static void start_element_handler(GMarkupParseContext *context,
 		rsp->current = rsp->subnet_mask;
 	else if (!strcmp(element_name, "mtu"))
 		rsp->current = rsp->mtu;
-	else if (!strcmp(element_name, "default_gateway"))
-		rsp->current = rsp->default_gateway;
 	else if (!strcmp(element_name, "dns_server") &&
 					rsp->dns_server1[0] == '\0')
 		rsp->current = rsp->dns_server1;

@@ -123,7 +129,7 @@ static void text_handler(GMarkupParseContext *context,
 
 	if (rsp->current) {
 		strncpy(rsp->current, text, IP_ADDR_LEN);
-		rsp->current[IP_ADDR_LEN] = 0;
+		rsp->current[IP_ADDR_LEN] = '\0';
 	}
 }

@@ -153,8 +159,7 @@ static gint conn_compare_by_cid(gconstpointer a, gconstpointer b)
 	return 0;
 }
 
-static struct conn_info *conn_info_create(unsigned int device,
-						unsigned int channel_id)
+static struct conn_info *conn_info_create(unsigned int channel_id)
 {
 	struct conn_info *connection = g_try_new0(struct conn_info, 1);

@@ -162,7 +167,6 @@ static struct conn_info *conn_info_create(unsigned int device,
 		return NULL;
 
 	connection->cid = 0;
-	connection->device = device;
 	connection->channel_id = channel_id;
 
 	return connection;

@@ -171,7 +175,7 @@ static struct conn_info *conn_info_create(unsigned int device,
 /*
  * Creates a new IP interface for CAIF.
  */
-static gboolean caif_if_create(const char *interface, unsigned int connid)
+static gboolean caif_if_create(struct conn_info *conn)
 {
 	return FALSE;
 }

@@ -179,9 +183,8 @@ static gboolean caif_if_create(const char *interface, unsigned int connid)
 /*
  * Removes IP interface for CAIF.
  */
-static gboolean caif_if_remove(const char *interface, unsigned int connid)
+static void caif_if_remove(struct conn_info *conn)
 {
-	return FALSE;
 }
 
 static void ste_eppsd_down_cb(gboolean ok, GAtResult *result,

@@ -210,21 +213,13 @@ static void ste_eppsd_down_cb(gboolean ok, GAtResult *result,
 		DBG("Did not find data (used caif device) for"
 					"connection with cid; %d",
 					gcd->active_context);
-		goto error;
+		CALLBACK_WITH_FAILURE(cb, cbd->data);
+		return;
 	}
 
 	conn = l->data;
-
-	if (!caif_if_remove(conn->interface, conn->channel_id)) {
-		DBG("Failed to remove caif interface %s.",
-				conn->interface);
-	}
-
 	conn->cid = 0;
-	return;
-
-error:
-	CALLBACK_WITH_FAILURE(cb, cbd->data);
+	CALLBACK_WITH_SUCCESS(cb, cbd->data);
 }
 
 static void ste_eppsd_up_cb(gboolean ok, GAtResult *result, gpointer user_data)

@@ -233,7 +228,7 @@ static void ste_eppsd_up_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	ofono_gprs_context_up_cb_t cb = cbd->cb;
 	struct ofono_gprs_context *gc = cbd->user;
 	struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
-	struct conn_info *conn = NULL;
+	struct conn_info *conn;
 	GAtResultIter iter;
 	GSList *l;
 	int i;

@@ -241,17 +236,15 @@ static void ste_eppsd_up_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	const char *res_string;
 	const char *dns[MAX_DNS + 1];
 	struct eppsd_response rsp;
-	GMarkupParseContext *context = NULL;
+	GMarkupParseContext *context;
 
 	l = g_slist_find_custom(g_caif_devices,
 				GUINT_TO_POINTER(gcd->active_context),
 				conn_compare_by_cid);
 
 	if (!l) {
-		DBG("Did not find data (device and channel id)"
-					"for connection with cid; %d",
-					gcd->active_context);
-		goto error;
+		DBG("CAIF Device gone missing (cid:%d)", gcd->active_context);
+		goto error_no_device;
 	}
 
 	conn = l->data;

@@ -291,31 +284,21 @@ static void ste_eppsd_up_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	dns[1] = rsp.dns_server2;
 	dns[2] = NULL;
 
-	sprintf(conn->interface, "caif%u", conn->device);
-
-	if (!caif_if_create(conn->interface, conn->channel_id)) {
-		ofono_error("Failed to create caif interface %s.",
-				conn->interface);
-		CALLBACK_WITH_SUCCESS(cb, NULL, FALSE, rsp.ip_address,
-				rsp.subnet_mask, rsp.default_gateway,
+	CALLBACK_WITH_SUCCESS(cb, conn->interface, TRUE, rsp.ip_address,
+				rsp.subnet_mask, NULL,
 				dns, cbd->data);
-	} else {
-		CALLBACK_WITH_SUCCESS(cb, conn->interface,
-				FALSE, rsp.ip_address, rsp.subnet_mask,
-				rsp.default_gateway, dns, cbd->data);
-	}
-
 	return;
 
 error:
-	DBG("ste_eppsd_up_cb error");
-
 	if (context)
 		g_markup_parse_context_free(context);
 
 	if (conn)
 		conn->cid = 0;
 
+error_no_device:
+	DBG("ste_eppsd_up_cb error");
+	gcd->active_context = 0;
 	CALLBACK_WITH_FAILURE(cb, NULL, 0, NULL, NULL, NULL, NULL, cbd->data);
 }

@@ -325,44 +308,45 @@ static void ste_cgdcont_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	ofono_gprs_context_up_cb_t cb = cbd->cb;
 	struct ofono_gprs_context *gc = cbd->user;
 	struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
-	struct cb_data *ncbd = NULL;
-	char buf[128];
+	struct cb_data *ncbd;
 	struct conn_info *conn;
+	char buf[128];
 	GSList *l;
 
+	l = g_slist_find_custom(g_caif_devices,
+				GUINT_TO_POINTER(gcd->active_context),
+				conn_compare_by_cid);
+
+	if (!l) {
+		DBG("CAIF Device gone missing (cid:%d)", gcd->active_context);
+		goto error_no_device;
+	}
+
+	conn = l->data;
+
 	if (!ok) {
 		struct ofono_error error;
 
+		conn->cid = 0;
 		gcd->active_context = 0;
 		decode_at_error(&error, g_at_result_final_response(result));
 		cb(&error, NULL, 0, NULL, NULL, NULL, NULL, cbd->data);
 		return;
 	}
 
-	ncbd = g_memdup(cbd, sizeof(struct cb_data));
-
-	l = g_slist_find_custom(g_caif_devices, GUINT_TO_POINTER(0),
-				conn_compare_by_cid);
-
-	if (!l) {
-		DBG("at_cgdcont_cb, no more available devices");
-		goto error;
-	}
-
-	conn = l->data;
-	conn->cid = gcd->active_context;
 	snprintf(buf, sizeof(buf), "AT*EPPSD=1,%u,%u",
 			conn->channel_id, conn->cid);
+	ncbd = g_memdup(cbd, sizeof(struct cb_data));
 
 	if (g_at_chat_send(gcd->chat, buf, NULL,
 				ste_eppsd_up_cb, ncbd, g_free) > 0)
 		return;
 
-error:
 	g_free(ncbd);
+	conn->cid = 0;
 
+error_no_device:
 	gcd->active_context = 0;
-
 	CALLBACK_WITH_FAILURE(cb, NULL, 0, NULL, NULL,
 				NULL, NULL, cbd->data);
 }

@@ -375,13 +359,32 @@ static void ste_gprs_activate_primary(struct ofono_gprs_context *gc,
 	struct cb_data *cbd = cb_data_new(cb, data);
 	char buf[AUTH_BUF_LENGTH];
 	int len;
+	GSList *l;
+	struct conn_info *conn;
 
 	if (!cbd)
-		goto error;
+		goto error_no_device;
 
 	gcd->active_context = ctx->cid;
 	cbd->user = gc;
 
+	/* Find free connection with cid zero */
+	l = g_slist_find_custom(g_caif_devices, GUINT_TO_POINTER(0),
+				conn_compare_by_cid);
+
+	if (!l) {
+		DBG("No more available CAIF devices");
+		goto error_no_device;
+	}
+
+	conn = l->data;
+	conn->cid = ctx->cid;
+
+	if (!conn->created) {
+		DBG("CAIF interface not created (rtnl error?)");
+		goto error;
+	}
+
 	len = snprintf(buf, sizeof(buf), "AT+CGDCONT=%u,\"IP\"", ctx->cid);
 
 	if (ctx->apn)

@@ -405,6 +408,9 @@ static void ste_gprs_activate_primary(struct ofono_gprs_context *gc,
 	return;
 
 error:
+	conn->cid = 0;
+
+error_no_device:
 	gcd->active_context = 0;
 	g_free(cbd);

@@ -431,8 +437,8 @@ static void ste_gprs_deactivate_primary(struct ofono_gprs_context *gc,
 				conn_compare_by_cid);
 
 	if (!l) {
-		DBG("at_gprs_deactivate_primary, did not find"
-			"data (channel id) for connection with cid; %d", id);
+		DBG("did not find data (channel id) "
+				"for connection with cid; %d", id);
 		goto error;
 	}

@@ -446,7 +452,6 @@ static void ste_gprs_deactivate_primary(struct ofono_gprs_context *gc,
 
 error:
 	g_free(cbd);
-
 	CALLBACK_WITH_FAILURE(cb, data);
 }

@@ -457,6 +462,7 @@ static void ste_cgact_read_cb(gboolean ok, GAtResult *result,
 	struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
 	gint cid, state;
 	GAtResultIter iter;
+	GSList *l;
 
 	if (!ok)
 		return;

@@ -480,7 +486,13 @@ static void ste_cgact_read_cb(gboolean ok, GAtResult *result,
 		ofono_gprs_context_deactivated(gc, gcd->active_context);
 		gcd->active_context = 0;
 
-		break;
+		/* Mark interface as unused */
+		l = g_slist_find_custom(g_caif_devices, GUINT_TO_POINTER(cid),
+				conn_compare_by_cid);
+		if (l) {
+			struct conn_info *conn = l->data;
+			conn->cid = 0;
+		}
 	}
 }

@@ -524,9 +536,11 @@ static int ste_gprs_context_probe(struct ofono_gprs_context *gc,
 	ofono_gprs_context_set_data(gc, gcd);
 
 	for (i = 0; i < MAX_CAIF_DEVICES; i++) {
-		ci = conn_info_create(i, i+1);
-		if (ci)
-			g_caif_devices = g_slist_append(g_caif_devices, ci);
+		ci = conn_info_create(i+1);
+		if (!ci)
+			return -ENOMEM;
+		caif_if_create(ci);
+		g_caif_devices = g_slist_append(g_caif_devices, ci);
 	}
 
 	return 0;

@@ -536,6 +550,7 @@ static void ste_gprs_context_remove(struct ofono_gprs_context *gc)
 {
 	struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
 
+	g_slist_foreach(g_caif_devices, (GFunc) caif_if_remove, NULL);
 	g_slist_foreach(g_caif_devices, (GFunc) g_free, NULL);
 	g_slist_free(g_caif_devices);
 	g_caif_devices = NULL;

Signature · 1 line

1.7.0.4

Sjur Brændeland

author

2010-11-21 23:01 UTC #10 lore raw

PATCH v3 2/2 stemodem: Use RTNL to create network interfaces.

In reply to George Matveev

From: Sjur Brændeland <[email protected]>

---
Changes from V2:
o Squashed functions caif_if_create info ste_gprs_context_probe

 drivers/stemodem/gprs-context.c |   44 ++++++++++++++++++++++++++++++++------
 1 files changed, 37 insertions(+), 7 deletions(-)

drivers/stemodem/gprs-context.c +37 -7

index 3ccda87..f118453 100644
@@ -47,6 +47,7 @@
 #include "stemodem.h"
 #include "caif_socket.h"
 #include "if_caif.h"
+#include "caif_rtnl.h"
 
 #define MAX_CAIF_DEVICES 4
 #define MAX_DNS 2

@@ -172,12 +173,20 @@ static struct conn_info *conn_info_create(unsigned int channel_id)
 	return connection;
 }
 
-/*
- * Creates a new IP interface for CAIF.
- */
-static gboolean caif_if_create(struct conn_info *conn)
+static void rtnl_callback(int ifindex, const char *ifname, void *user_data)
 {
-	return FALSE;
+	struct conn_info *conn = user_data;
+
+	if (ifindex < 0) {
+		conn->created = FALSE;
+		ofono_error("Failed to create caif interface %s",
+			conn->interface);
+		return;
+	}
+
+	strncpy(conn->interface, ifname, sizeof(conn->interface));
+	conn->ifindex = ifindex;
+	conn->created = TRUE;
 }
 
 /*

@@ -185,6 +194,18 @@ static gboolean caif_if_create(struct conn_info *conn)
  */
 static void caif_if_remove(struct conn_info *conn)
 {
+	if (!conn->created)
+		return;
+
+	if (caif_rtnl_delete_interface(conn->ifindex) < 0) {
+		ofono_error("Failed to delete caif interface %s",
+			conn->interface);
+		return;
+	}
+
+	DBG("removed CAIF interface ch:%d ifname:%s ifindex:%d\n",
+		conn->channel_id, conn->interface, conn->ifindex);
+	return;
 }
 
 static void ste_eppsd_down_cb(gboolean ok, GAtResult *result,

@@ -526,7 +547,7 @@ static int ste_gprs_context_probe(struct ofono_gprs_context *gc,
 	GAtChat *chat = data;
 	struct gprs_context_data *gcd;
 	struct conn_info *ci;
-	int i;
+	int i,err;
 
 	gcd = g_new0(struct gprs_context_data, 1);
 	gcd->chat = g_at_chat_clone(chat);

@@ -539,7 +560,14 @@ static int ste_gprs_context_probe(struct ofono_gprs_context *gc,
 		ci = conn_info_create(i+1);
 		if (!ci)
 			return -ENOMEM;
-		caif_if_create(ci);
+		err = caif_rtnl_create_interface(IFLA_CAIF_IPV4_CONNID,
+						ci->channel_id, FALSE,
+						rtnl_callback, ci);
+		if (err < 0) {
+			DBG("Failed to create IP interface for CAIF");
+			return err;
+		}
+
 		g_caif_devices = g_slist_append(g_caif_devices, ci);
 	}

@@ -571,10 +599,12 @@ static struct ofono_gprs_context_driver driver = {
 
 void ste_gprs_context_init()
 {
+	caif_rtnl_init();
 	ofono_gprs_context_driver_register(&driver);
 }
 
 void ste_gprs_context_exit()
 {
+	caif_rtnl_exit();
 	ofono_gprs_context_driver_unregister(&driver);
 }

Signature · 1 line

1.7.0.4

Sjur Brændeland

author

2010-11-21 20:55 UTC #11 lore raw

PATCH RESEND 1/1 stemodem: Change use of types

In reply to Sjur Brændeland

From: Sjur Brændeland <[email protected]>

Use the type __u32 for sequence counting rather than guint32, and "void *" instead of gpointer.
Reduce the size of RTNL message buffer from 4096 to 1024, as this should be sufficient to hold the NEWLINK message.

---
Last patch was sent with base64 encoding :-(

 drivers/stemodem/caif_rtnl.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

drivers/stemodem/caif_rtnl.c +7 -7

index 5d4073a..4c00446 100644
@@ -41,7 +41,7 @@
 #define NLMSG_TAIL(nmsg) \
 	((struct rtattr *) (((void *) (nmsg)) + NLMSG_ALIGN((nmsg)->nlmsg_len)))
 
-#define RTNL_MSG_SIZE 4096
+#define RTNL_MSG_SIZE 1024
 
 struct rtnl_msg {
 	struct nlmsghdr n;

@@ -50,17 +50,17 @@ struct rtnl_msg {
 };
 
 struct iplink_req {
-	guint32 rtnlmsg_seqnr;
-	gpointer user_data;
+	__u32 rtnlmsg_seqnr;
+	void *user_data;
 	caif_rtnl_create_cb_t callback;
 };
 
 static GSList *pending_requests;
-static guint32 rtnl_seqnr;
+static __u32 rtnl_seqnr;
 static guint rtnl_watch;
 static GIOChannel *rtnl_channel;
 
-static struct iplink_req *find_request(guint32 seq)
+static struct iplink_req *find_request(__u32 seq)
 {
 	GSList *list;

@@ -169,7 +169,7 @@ static int add_attribute(struct nlmsghdr *n, unsigned int maxlen, int type,
 	return 0;
 }
 
-static inline void prep_rtnl_req(struct rtnl_msg *msg, int reqtype, guint seqnr)
+static inline void prep_rtnl_req(struct rtnl_msg *msg, int reqtype, __u32 seqnr)
 {
 	msg->n.nlmsg_len = NLMSG_LENGTH(sizeof(struct ifinfomsg));
 	msg->n.nlmsg_flags = NLM_F_REQUEST|NLM_F_CREATE|NLM_F_EXCL;

@@ -179,7 +179,7 @@ static inline void prep_rtnl_req(struct rtnl_msg *msg, int reqtype, guint seqnr)
 }
 
 static gboolean netlink_event(GIOChannel *chan,
-				GIOCondition cond, gpointer data)
+				GIOCondition cond, void *data)
 {
 	unsigned char buf[RTNL_MSG_SIZE];
 	int len, sk;

Signature · 1 line

1.6.3.3

Denis Kenzior

2010-11-23 09:07 UTC #12 lore raw

Re: [PATCH RESEND] stemodem: Change use of types

Hi Sjur,

Quoted 7 lines · Sjur Brændeland

From: Sjur Brændeland <[email protected]>
Use the type __u32 for sequence counting rather than guint32, and "void *" instead of gpointer.
Reduce the size of RTNL message buffer from 4096 to 1024, as this should be sufficient to hold the NEWLINK message.

Quoted 6 lines

---
Last patch was sent with base64 encoding :-(
 drivers/stemodem/caif_rtnl.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

Patch has been applied, thanks.

Regards,
-Denis