Skip to contentSkip to conversation

lte atom auth and IP protocol

PATCH In review Giacinto Cifelli 2 participants 9 messages Oct 19, 2018 – Oct 22, 2018

lore
Share & tools
lore
#0 lore raw

v8 cover lte atom auth and IP protocol

I would like to submit this patch for the lte atom

This patch adds in core atom:
- the protocol for the default LTE APN, ip, ipv6 and both
- the authentication handling, with 3 properties:
method, username, password

then these are used in the atmodem atom:

The behavior of the patch is described in the api document (part 1), and in the core atom header (part 2).

Related to the previous version, this version:
- changes the name of the function at_auth_type_from_method in atmodem/atutils, as suggested by Jonas, and calls it without passing from an intermediate local variable.

Giacinto Cifelli (5):
lte-api: protocol and authentication properties lte.h: added proto and authentication handling
lte: protocol and authentication for default ctx atmodem/atutil: shared functions for cgdcont atmodem/lte: proto and authentication handling

 doc/lte-api.txt          |  39 +++++++
 drivers/atmodem/atutil.c |  49 +++++++++
 drivers/atmodem/atutil.h |   6 ++
 drivers/atmodem/lte.c    | 118 ++++++++++++++++++---
 include/lte.h            |   5 +
 src/lte.c                | 222 +++++++++++++++++++++++++++++----------
 6 files changed, 367 insertions(+), 72 deletions(-)
Signature · 1 line
2.17.1
#1 lore raw

PATCH v8 1/5 lte-api: protocol and authentication properties

added 4 properties for handling the type of context and the authentication method, exactly like in any gprs context handling. The properties are named after the equivalent gprs-context one, for compatibility and uniformity.

---
 doc/lte-api.txt | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)
doc/lte-api.txt +39 -0
index 8a2a97d9..c9544f60 100644
@@ -33,3 +33,42 @@ Properties	string DefaultAccessPointName [readwrite]
 
 			Setting this property to an empty string clears the
 			default APN from the modem.
+
+		string Protocol [readwrite]
+
+			Holds the protocol for this context.  Valid values
+			are: "ip", "ipv6" and "dual". Default value is "ip".
+
+		string AuthenticationMethod [readwrite]
+
+			Sets the Method used for the authentication
+			for the default APN.
+
+			Available values are "none", "pap" and "chap".
+			Default is "none".
+
+			If the AuthenticationMethod is set to 'none' it remove
+			the authentication for the DefaultAPN.
+			In case of AuthenticationMethod 'none',
+			if the Username and Password properties are not empty,
+			the values are preserved in the properties, but they
+			are not used or transmitted to the module.
+			Conversely, if Username or Password are empty, the
+			authentication method selected internally is 'none',
+			but the property AuthenticationMethod is left unchanged.
+
+			If the default APN supports authentication and it
+			fails, then it is up to the network how to proceed.
+			In general LTE access is denied and the modem can
+			fallback to a legacy technology if capable and another
+			radio technology is available.
+
+		string Username [readwrite]
+
+			Holds the username to be used for authentication
+			purposes.
+
+		string Password [readwrite]
+
+			Holds the password to be used for authentication
+			purposes.
Signature · 1 line
2.17.1
#2 lore raw

PATCH v8 2/5 lte.h: added proto and authentication handling

In reply to Giacinto Cifelli

The ofono_lte_default_attach_info is extended with protocol, authentication method, username and password.
The transmission of this info from the src to the atom happens through the existing set_default_attach_info.
A signal is emitted when one of these properties changes

---
 include/lte.h | 5 +++++
 1 file changed, 5 insertions(+)
include/lte.h +5 -0
index 0f2501c0..2f12ac29 100644
@@ -3,6 +3,7 @@
  *  oFono - Open Source Telephony
  *
  *  Copyright (C) 2016  Endocode AG. All rights reserved.
+ *  Copyright (C) 2018 Gemalto M2M
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License version 2 as
@@ -32,6 +33,10 @@ struct ofono_lte;
 
 struct ofono_lte_default_attach_info {
 	char apn[OFONO_GPRS_MAX_APN_LENGTH + 1];
+	enum ofono_gprs_proto proto;
+	enum ofono_gprs_auth_method auth_method;
+	char username[OFONO_GPRS_MAX_USERNAME_LENGTH + 1];
+	char password[OFONO_GPRS_MAX_PASSWORD_LENGTH + 1];
 };
 
 typedef void (*ofono_lte_cb_t)(const struct ofono_error *error, void *data);
Signature · 1 line
2.17.1
#3 lore raw

PATCH v8 3/5 lte: protocol and authentication for default ctx

In reply to Giacinto Cifelli

Many LTE networks require user authentication, even for the default context. In particular, most of the private APNs use this facility to add some control on top of the MNO providing the service, so that another user of the same network cannot access the private one. As such, we add these parameters to the default context settings that will attempt to use when registering to the network.

The additional parameters added by this patch are: protocol, user, and password. These are sufficient to allow to connect to networks available to the patch author where ofono previously failed to register to the network at all.

---
 src/lte.c | 222 ++++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 167 insertions(+), 55 deletions(-)
src/lte.c +167 -55
index 23fe8e1c..fc0a3442 100644
@@ -3,6 +3,7 @@
  *  oFono - Open Source Telephony
  *
  *  Copyright (C) 2016  Endocode AG. All rights reserved.
+ *  Copyright (C) 2018 Gemalto M2M
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License version 2 as
@@ -39,7 +40,11 @@
 
 #define SETTINGS_STORE "lte"
 #define SETTINGS_GROUP "Settings"
-#define DEFAULT_APN_KEY "DefaultAccessPointName"
+#define LTE_APN "DefaultAccessPointName"
+#define LTE_PROTO "Protocol"
+#define LTE_USERNAME "Username"
+#define LTE_PASSWORD "Password"
+#define LTE_AUTH_METHOD "AuthenticationMethod"
 
 struct ofono_lte {
 	const struct ofono_lte_driver *driver;
@@ -57,6 +62,10 @@ static GSList *g_drivers = NULL;
 static void lte_load_settings(struct ofono_lte *lte)
 {
 	char *apn;
+	char *proto_str;
+	char *auth_method_str;
+	char *username;
+	char *password;
 
 	if (lte->imsi == NULL)
 		return;
@@ -69,19 +78,57 @@ static void lte_load_settings(struct ofono_lte *lte)
 		return;
 	}
 
-	apn = g_key_file_get_string(lte->settings, SETTINGS_GROUP ,
-					DEFAULT_APN_KEY, NULL);
-	if (apn) {
+	apn = g_key_file_get_string(lte->settings, SETTINGS_GROUP,
+				LTE_APN, NULL);
+	proto_str = g_key_file_get_string(lte->settings, SETTINGS_GROUP,
+				LTE_PROTO, NULL);
+	auth_method_str = g_key_file_get_string(lte->settings, SETTINGS_GROUP,
+				LTE_AUTH_METHOD, NULL);
+	username = g_key_file_get_string(lte->settings, SETTINGS_GROUP,
+				LTE_USERNAME, NULL);
+	password = g_key_file_get_string(lte->settings, SETTINGS_GROUP,
+				LTE_PASSWORD, NULL);
+	if (apn && is_valid_apn(apn))
 		strcpy(lte->info.apn, apn);
-		g_free(apn);
-	}
+
+	if (proto_str == NULL)
+		proto_str = g_strdup("ip");
+
+	/* this must have a valid default */
+	if (!gprs_proto_from_string(proto_str, &lte->info.proto))
+		lte->info.proto = OFONO_GPRS_PROTO_IP;
+
+	if (auth_method_str == NULL)
+		auth_method_str = g_strdup("none");
+
+	/* this must have a valid default */
+	if (!gprs_auth_method_from_string(auth_method_str,
+							&lte->info.auth_method))
+		lte->info.auth_method = OFONO_GPRS_AUTH_METHOD_NONE;
+
+	if (username && strlen(username) <= OFONO_GPRS_MAX_USERNAME_LENGTH)
+		strcpy(lte->info.username, username);
+
+	if (password && strlen(password) <= OFONO_GPRS_MAX_PASSWORD_LENGTH)
+		strcpy(lte->info.password, password);
+
+	g_free(apn);
+	g_free(proto_str);
+	g_free(auth_method_str);
+	g_free(username);
+	g_free(password);
 }
 
 static DBusMessage *lte_get_properties(DBusConnection *conn,
 					DBusMessage *msg, void *data)
 {
 	struct ofono_lte *lte = data;
+	const char *proto = gprs_proto_to_string(lte->info.proto);
 	const char *apn = lte->info.apn;
+	const char* auth_method =
+			gprs_auth_method_to_string(lte->info.auth_method);
+	const char *username = lte->info.username;
+	const char *password = lte->info.password;
 	DBusMessage *reply;
 	DBusMessageIter iter;
 	DBusMessageIter dict;
@@ -95,20 +142,31 @@ static DBusMessage *lte_get_properties(DBusConnection *conn,
 	dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
 					OFONO_PROPERTIES_ARRAY_SIGNATURE,
 					&dict);
-	ofono_dbus_dict_append(&dict, DEFAULT_APN_KEY, DBUS_TYPE_STRING, &apn);
+	ofono_dbus_dict_append(&dict, LTE_APN, DBUS_TYPE_STRING, &apn);
+	ofono_dbus_dict_append(&dict, LTE_PROTO, DBUS_TYPE_STRING, &proto);
+	ofono_dbus_dict_append(&dict, LTE_AUTH_METHOD, DBUS_TYPE_STRING,
+					&auth_method);
+	ofono_dbus_dict_append(&dict, LTE_USERNAME, DBUS_TYPE_STRING,
+					&username);
+	ofono_dbus_dict_append(&dict, LTE_PASSWORD, DBUS_TYPE_STRING,
+					&password);
 	dbus_message_iter_close_container(&iter, &dict);
 
 	return reply;
 }
 
 static void lte_set_default_attach_info_cb(const struct ofono_error *error,
-						void *data)
+								void *data)
 {
 	struct ofono_lte *lte = data;
 	const char *path = __ofono_atom_get_path(lte->atom);
 	DBusConnection *conn = ofono_dbus_get_connection();
 	DBusMessage *reply;
-	const char *apn = lte->info.apn;
+	char *key;
+	char *value;
+	const char *str;
+	DBusMessageIter iter;
+	DBusMessageIter var;
 
 	DBG("%s error %d", path, error->type);
 
@@ -118,55 +176,48 @@ static void lte_set_default_attach_info_cb(const struct ofono_error *error,
 		return;
 	}
 
-	g_strlcpy(lte->info.apn, lte->pending_info.apn,
-			OFONO_GPRS_MAX_APN_LENGTH + 1);
+	/*
+	 * Reparsing of the message to extract the key and value
+	 * No error checking needed since we already validated pending
+	 */
+	dbus_message_iter_init(lte->pending, &iter);
+	dbus_message_iter_get_basic(&iter, &str);
+	key = strdup(str);
+
+	dbus_message_iter_next(&iter);
+	dbus_message_iter_recurse(&iter, &var);
+	dbus_message_iter_get_basic(&var, &str);
+	value = strdup(str);
+
+	memcpy(&lte->info, &lte->pending_info, sizeof(lte->info));
+
+	reply = dbus_message_new_method_return(lte->pending);
+	__ofono_dbus_pending_reply(&lte->pending, reply);
 
 	if (lte->settings) {
-		if (strlen(lte->info.apn) == 0)
-			/* Clear entry on empty APN. */
-			g_key_file_remove_key(lte->settings, SETTINGS_GROUP,
-						DEFAULT_APN_KEY, NULL);
+		/*
+		 * the following code removes from storage empty APN, user, pwd
+		 * for proto and auth_method, given that they always
+		 * have defaults, it will not do anything.
+		 */
+		if (!*value)
+			/* Clear entry on empty string. */
+			g_key_file_remove_key(lte->settings,
+				SETTINGS_GROUP, key, NULL);
 		else
-			g_key_file_set_string(lte->settings, SETTINGS_GROUP,
-						DEFAULT_APN_KEY, lte->info.apn);
+			g_key_file_set_string(lte->settings,
+				SETTINGS_GROUP, key, value);
 
 		storage_sync(lte->imsi, SETTINGS_STORE, lte->settings);
 	}
 
-	reply = dbus_message_new_method_return(lte->pending);
-	__ofono_dbus_pending_reply(&lte->pending, reply);
-
 	ofono_dbus_signal_property_changed(conn, path,
 					OFONO_CONNECTION_CONTEXT_INTERFACE,
-					DEFAULT_APN_KEY,
-					DBUS_TYPE_STRING, &apn);
-}
-
-static DBusMessage *lte_set_default_apn(struct ofono_lte *lte,
-				DBusConnection *conn, DBusMessage *msg,
-				const char *apn)
-{
-	if (lte->driver->set_default_attach_info == NULL)
-		return __ofono_error_not_implemented(msg);
+					key,
+					DBUS_TYPE_STRING, &value);
 
-	if (lte->pending)
-		return __ofono_error_busy(msg);
-
-	if (g_str_equal(apn, lte->info.apn))
-		return dbus_message_new_method_return(msg);
-
-	/* We do care about empty value: it can be used for reset. */
-	if (is_valid_apn(apn) == FALSE && apn[0] != '\0')
-		return __ofono_error_invalid_format(msg);
-
-	lte->pending = dbus_message_ref(msg);
-
-	g_strlcpy(lte->pending_info.apn, apn, OFONO_GPRS_MAX_APN_LENGTH + 1);
-
-	lte->driver->set_default_attach_info(lte, &lte->pending_info,
-					lte_set_default_attach_info_cb, lte);
-
-	return NULL;
+	g_free(value);
+	g_free(key);
 }
 
 static DBusMessage *lte_set_property(DBusConnection *conn,
@@ -177,6 +228,14 @@ static DBusMessage *lte_set_property(DBusConnection *conn,
 	DBusMessageIter var;
 	const char *property;
 	const char *str;
+	enum ofono_gprs_auth_method auth_method;
+	enum ofono_gprs_proto proto;
+
+	if (lte->driver->set_default_attach_info == NULL)
+		return __ofono_error_not_implemented(msg);
+
+	if (lte->pending)
+		return __ofono_error_busy(msg);
 
 	if (!dbus_message_iter_init(msg, &iter))
 		return __ofono_error_invalid_args(msg);
@@ -192,16 +251,69 @@ static DBusMessage *lte_set_property(DBusConnection *conn,
 
 	dbus_message_iter_recurse(&iter, &var);
 
-	if (!strcmp(property, DEFAULT_APN_KEY)) {
-		if (dbus_message_iter_get_arg_type(&var) != DBUS_TYPE_STRING)
-			return __ofono_error_invalid_args(msg);
+	if (dbus_message_iter_get_arg_type(&var) != DBUS_TYPE_STRING)
+		return __ofono_error_invalid_args(msg);
 
-		dbus_message_iter_get_basic(&var, &str);
+	dbus_message_iter_get_basic(&var, &str);
 
-		return lte_set_default_apn(lte, conn, msg, str);
-	}
+	memcpy(&lte->pending_info, &lte->info, sizeof(lte->info));
+
+	if ((strcmp(property, LTE_APN) == 0)) {
+
+		if (g_str_equal(str, lte->info.apn))
+			return dbus_message_new_method_return(msg);
+
+		/* We do care about empty value: it can be used for reset. */
+		if (is_valid_apn(str) == FALSE && str[0] != '\0')
+			return __ofono_error_invalid_format(msg);
+
+		g_strlcpy(lte->pending_info.apn, str,
+					OFONO_GPRS_MAX_APN_LENGTH + 1);
+
+	} else if ((strcmp(property, LTE_PROTO) == 0)) {
+
+		if (!gprs_proto_from_string(str, &proto))
+			return __ofono_error_invalid_format(msg);
+
+		if (proto == lte->info.proto)
+			return dbus_message_new_method_return(msg);
 
-	return __ofono_error_invalid_args(msg);
+		lte->pending_info.proto = proto;
+
+	} else if (strcmp(property, LTE_AUTH_METHOD) == 0) {
+
+		if (!gprs_auth_method_from_string(str, &auth_method))
+			return __ofono_error_invalid_format(msg);
+
+		if (auth_method == lte->info.auth_method)
+			return dbus_message_new_method_return(msg);
+
+		lte->pending_info.auth_method = auth_method;
+
+	} else if (strcmp(property, LTE_USERNAME) == 0) {
+
+		if (g_str_equal(str, lte->info.username))
+			return dbus_message_new_method_return(msg);
+
+		g_strlcpy(lte->pending_info.username, str,
+					OFONO_GPRS_MAX_USERNAME_LENGTH + 1);
+
+	} else if (strcmp(property, LTE_PASSWORD) == 0) {
+
+		if (g_str_equal(str, lte->info.password))
+			return dbus_message_new_method_return(msg);
+
+		g_strlcpy(lte->pending_info.password, str,
+					OFONO_GPRS_MAX_PASSWORD_LENGTH + 1);
+
+	} else
+		return __ofono_error_invalid_args(msg);
+
+	lte->pending = dbus_message_ref(msg);
+	lte->driver->set_default_attach_info(lte, &lte->pending_info,
+					lte_set_default_attach_info_cb, lte);
+
+	return NULL;
 }
 
 static const GDBusMethodTable lte_methods[] = {
Signature · 1 line
2.17.1
#4 lore raw

PATCH v8 4/5 atmodem/atutil: shared functions for cgdcont

In reply to Giacinto Cifelli

included two new functions:

at_auth_type_from_method: converts the ofono enum ofono_gprs_auth_method
into the value of the 3GPP 27.007 to pass to the AT command

at_get_cgdcont_command: computes the AT+CGDCONT string, standard version

---
 drivers/atmodem/atutil.c | 49 ++++++++++++++++++++++++++++++++++++++++
 drivers/atmodem/atutil.h |  6 +++++
 2 files changed, 55 insertions(+)
drivers/atmodem/atutil.c +49 -0
index 6f4e8a20..ed19e91b 100644
@@ -3,6 +3,7 @@
  *  oFono - Open Source Telephony
  *
  *  Copyright (C) 2008-2011  Intel Corporation. All rights reserved.
+ *  Copyright (C) 2018 Gemalto M2M
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License version 2 as
@@ -27,6 +28,7 @@
 #include <gatchat.h>
 #include <string.h>
 #include <stdlib.h>
+#include <stdio.h>
 #include <errno.h>
 
 #define OFONO_API_SUBJECT_TO_CHANGE
@@ -654,3 +656,50 @@ int at_util_get_ipv4_address_and_netmask(const char *addrnetmask,
 
 	return ret;
 }
+
+int at_auth_type_from_method(enum ofono_gprs_auth_method auth_method)
+{
+	switch (auth_method) {
+	case OFONO_GPRS_AUTH_METHOD_PAP:
+		return 1;
+	case OFONO_GPRS_AUTH_METHOD_CHAP:
+		return 2;
+	case OFONO_GPRS_AUTH_METHOD_NONE:
+		return 0;
+	}
+
+	return 0;
+}
+
+char *at_get_cgdcont_command(guint cid, enum ofono_gprs_proto proto,
+							const char *apn)
+{
+	size_t buflen = 32 + OFONO_GPRS_MAX_APN_LENGTH + 1;
+	char *buf = g_new(char, buflen);
+	int len;
+
+	len = snprintf(buf, buflen, "AT+CGDCONT=%u", cid);
+	buflen -= len;
+
+	/*
+	 * if apn is null, it will remove the context.
+	 * but if apn is empty, it will create a context with empty apn
+	 */
+	if (!apn)
+		goto finished;
+
+	switch (proto) {
+	case OFONO_GPRS_PROTO_IPV6:
+		snprintf(buf+len, buflen, ",\"IPV6\",\"%s\"", apn);
+		break;
+	case OFONO_GPRS_PROTO_IPV4V6:
+		snprintf(buf+len, buflen, ",\"IPV4V6\",\"%s\"", apn);
+		break;
+	case OFONO_GPRS_PROTO_IP:
+		snprintf(buf+len, buflen, ",\"IP\",\"%s\"", apn);
+		break;
+	}
+
+finished:
+	return buf;
+}
drivers/atmodem/atutil.h +6 -0
index 7113a4cd..447ce56b 100644
@@ -3,6 +3,7 @@
  *  oFono - Open Source Telephony
  *
  *  Copyright (C) 2008-2011  Intel Corporation. All rights reserved.
+ *  Copyright (C) 2018 Gemalto M2M
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License version 2 as
@@ -86,6 +87,11 @@ void at_util_sim_state_query_free(struct at_util_sim_state_query *req);
 int at_util_get_ipv4_address_and_netmask(const char *addrnetmask,
 						char *address, char *netmask);
 
+int at_auth_type_from_method(enum ofono_gprs_auth_method auth_method);
+
+char *at_get_cgdcont_command(guint cid, enum ofono_gprs_proto proto,
+							const char *apn);
+
 struct cb_data {
 	void *cb;
 	void *data;
Signature · 1 line
2.17.1
#5 lore raw

Re: [PATCH v8 4/5] atmodem/atutil: shared functions for cgdcont

Hi Giacinto,

Quoted 6 lines · Giacinto Cifelli

included two new functions:

at_auth_type_from_method: converts the ofono enum ofono_gprs_auth_method
into the value of the 3GPP 27.007 to pass to the AT command

at_get_cgdcont_command: computes the AT+CGDCONT string, standard version

That just screams for 2 separate commits.

Also, everything in atutil.[ch] is prefixed by at_util_, so please follow that convention.

Quoted 62 lines
---
  drivers/atmodem/atutil.c | 49 ++++++++++++++++++++++++++++++++++++++++
  drivers/atmodem/atutil.h |  6 +++++
  2 files changed, 55 insertions(+)
drivers/atmodem/atutil.c +35 -0
index 6f4e8a20..ed19e91b 100644
@@ -3,6 +3,7 @@
   *  oFono - Open Source Telephony
   *
   *  Copyright (C) 2008-2011  Intel Corporation. All rights reserved.
+ *  Copyright (C) 2018 Gemalto M2M
   *
   *  This program is free software; you can redistribute it and/or modify
   *  it under the terms of the GNU General Public License version 2 as
@@ -27,6 +28,7 @@
  #include <gatchat.h>
  #include <string.h>
  #include <stdlib.h>
+#include <stdio.h>
  #include <errno.h>
 
  #define OFONO_API_SUBJECT_TO_CHANGE
@@ -654,3 +656,50 @@ int at_util_get_ipv4_address_and_netmask(const char *addrnetmask,
 
  	return ret;
  }
+
+int at_auth_type_from_method(enum ofono_gprs_auth_method auth_method)
+{
+	switch (auth_method) {
+	case OFONO_GPRS_AUTH_METHOD_PAP:
+		return 1;
+	case OFONO_GPRS_AUTH_METHOD_CHAP:
+		return 2;
+	case OFONO_GPRS_AUTH_METHOD_NONE:
+		return 0;
+	}
+
+	return 0;
+}
+
+char *at_get_cgdcont_command(guint cid, enum ofono_gprs_proto proto,
+							const char *apn)
+{
+	size_t buflen = 32 + OFONO_GPRS_MAX_APN_LENGTH + 1;
+	char *buf = g_new(char, buflen);
+	int len;
+
+	len = snprintf(buf, buflen, "AT+CGDCONT=%u", cid);
+	buflen -= len;
+
+	/*
+	 * if apn is null, it will remove the context.
+	 * but if apn is empty, it will create a context with empty apn
+	 */
+	if (!apn)
+		goto finished;
+
+	switch (proto) {

drivers/atmodem/atutil.c int at_util_get_ipv4_address_and_netmask(const char *addrnetmask, · line 701

Quoted 10 lines
drivers/atmodem/atutil.c +10 -0
+	case OFONO_GPRS_PROTO_IPV6:
+		snprintf(buf+len, buflen, ",\"IPV6\",\"%s\"", apn);
+		break;
+	case OFONO_GPRS_PROTO_IPV4V6:
+		snprintf(buf+len, buflen, ",\"IPV4V6\",\"%s\"", apn);
+		break;
+	case OFONO_GPRS_PROTO_IP:
+		snprintf(buf+len, buflen, ",\"IP\",\"%s\"", apn);
+		break;
+	}

Ugh. Since you're allocating a new string anyway, why not something like:

const char *pdp_type = at_util_gprs_proto_to_pdp_type(proto);

if (!apn || apn[0] == '\0')

	return g_strdup_printf("AT+CGDCONT=%u", cid);

return g_strdup_printf("AT+CGDCONT=%u,\"%s\",\"%s\"", cid, pdp_type, apn);

Quoted 19 lines
drivers/atmodem/atutil.c +4 -0
+
+finished:
+	return buf;
+}
drivers/atmodem/atutil.h +1 -0
index 7113a4cd..447ce56b 100644
@@ -3,6 +3,7 @@
   *  oFono - Open Source Telephony
   *
   *  Copyright (C) 2008-2011  Intel Corporation. All rights reserved.
+ *  Copyright (C) 2018 Gemalto M2M
   *
   *  This program is free software; you can redistribute it and/or modify
   *  it under the terms of the GNU General Public License version 2 as
@@ -86,6 +87,11 @@ void at_util_sim_state_query_free(struct at_util_sim_state_query *req);
  int at_util_get_ipv4_address_and_netmask(const char *addrnetmask,

drivers/atmodem/atutil.h void at_util_sim_state_query_free(struct at_util_sim_state_query *req); · line 97

Quoted 10 lines
drivers/atmodem/atutil.h +5 -0
  						char *address, char *netmask);
 
+int at_auth_type_from_method(enum ofono_gprs_auth_method auth_method);
+
+char *at_get_cgdcont_command(guint cid, enum ofono_gprs_proto proto,
+							const char *apn);
+
  struct cb_data {
  	void *cb;
  	void *data;

Regards,
-Denis

#6 lore raw

Re: [PATCH v8 4/5] atmodem/atutil: shared functions for cgdcont

Hi Denis,

ok for your comments, except ...

Quoted 91 lines · Denis Kenzior

Hi Giacinto,

Quoted 6 lines · Giacinto Cifelli

included two new functions:

at_auth_type_from_method: converts the ofono enum ofono_gprs_auth_method
into the value of the 3GPP 27.007 to pass to the AT command

at_get_cgdcont_command: computes the AT+CGDCONT string, standard version

That just screams for 2 separate commits.

Also, everything in atutil.[ch] is prefixed by at_util_, so please follow that convention.

Quoted 62 lines
---
  drivers/atmodem/atutil.c | 49 ++++++++++++++++++++++++++++++++++++++++
  drivers/atmodem/atutil.h |  6 +++++
  2 files changed, 55 insertions(+)
drivers/atmodem/atutil.c +35 -0
index 6f4e8a20..ed19e91b 100644
@@ -3,6 +3,7 @@
   *  oFono - Open Source Telephony
   *
   *  Copyright (C) 2008-2011  Intel Corporation. All rights reserved.
+ *  Copyright (C) 2018 Gemalto M2M
   *
   *  This program is free software; you can redistribute it and/or modify
   *  it under the terms of the GNU General Public License version 2 as
@@ -27,6 +28,7 @@
  #include <gatchat.h>
  #include <string.h>
  #include <stdlib.h>
+#include <stdio.h>
  #include <errno.h>
 
  #define OFONO_API_SUBJECT_TO_CHANGE
@@ -654,3 +656,50 @@ int at_util_get_ipv4_address_and_netmask(const char *addrnetmask,
 
      return ret;
  }
+
+int at_auth_type_from_method(enum ofono_gprs_auth_method auth_method)
+{
+     switch (auth_method) {
+     case OFONO_GPRS_AUTH_METHOD_PAP:
+             return 1;
+     case OFONO_GPRS_AUTH_METHOD_CHAP:
+             return 2;
+     case OFONO_GPRS_AUTH_METHOD_NONE:
+             return 0;
+     }
+
+     return 0;
+}
+
+char *at_get_cgdcont_command(guint cid, enum ofono_gprs_proto proto,
+                                                     const char *apn)
+{
+     size_t buflen = 32 + OFONO_GPRS_MAX_APN_LENGTH + 1;
+     char *buf = g_new(char, buflen);
+     int len;
+
+     len = snprintf(buf, buflen, "AT+CGDCONT=%u", cid);
+     buflen -= len;
+
+     /*
+      * if apn is null, it will remove the context.
+      * but if apn is empty, it will create a context with empty apn
+      */
+     if (!apn)
+             goto finished;
+
+     switch (proto) {

drivers/atmodem/atutil.c int at_util_get_ipv4_address_and_netmask(const char *addrnetmask, · line 701

Quoted 10 lines
drivers/atmodem/atutil.c +10 -0
+     case OFONO_GPRS_PROTO_IPV6:
+             snprintf(buf+len, buflen, ",\"IPV6\",\"%s\"", apn);
+             break;
+     case OFONO_GPRS_PROTO_IPV4V6:
+             snprintf(buf+len, buflen, ",\"IPV4V6\",\"%s\"", apn);
+             break;
+     case OFONO_GPRS_PROTO_IP:
+             snprintf(buf+len, buflen, ",\"IP\",\"%s\"", apn);
+             break;
+     }

Ugh. Since you're allocating a new string anyway, why not something like:

Quoted 4 lines

const char *pdp_type = at_util_gprs_proto_to_pdp_type(proto);

if (!apn || apn[0] == '\0')

        return g_strdup_printf("AT+CGDCONT=%u", cid);

... these are 2 different cases:
if !apn (null string), then the PDP context is to be deleted, and the command is like: AT+CGDCONT=7
but if apn is empty, then the DPD context is to be provisioned with a default apn: AT+CGDCONT=7,"IP",""

I will add a comment in the header file to explain that.

Quoted 23 lines

return g_strdup_printf("AT+CGDCONT=%u,\"%s\",\"%s\"", cid, pdp_type, apn);

Quoted 20 lines
drivers/atmodem/atutil.c +4 -0
+
+finished:
+     return buf;
+}
drivers/atmodem/atutil.h +1 -0
index 7113a4cd..447ce56b 100644
@@ -3,6 +3,7 @@
   *  oFono - Open Source Telephony
   *
   *  Copyright (C) 2008-2011  Intel Corporation. All rights reserved.
+ *  Copyright (C) 2018 Gemalto M2M
   *
   *  This program is free software; you can redistribute it and/or modify
   *  it under the terms of the GNU General Public License version 2 as
@@ -86,6 +87,11 @@ void at_util_sim_state_query_free(struct at_util_sim_state_query *req);
  int at_util_get_ipv4_address_and_netmask(const char *addrnetmask,
                                              char *address, char *netmask);
 
Quoted 12 lines

drivers/atmodem/atutil.h void at_util_sim_state_query_free(struct at_util_sim_state_query *req); · line 97

Quoted 9 lines
drivers/atmodem/atutil.h +5 -0
+int at_auth_type_from_method(enum ofono_gprs_auth_method auth_method);
+
+char *at_get_cgdcont_command(guint cid, enum ofono_gprs_proto proto,
+                                                     const char *apn);
+
  struct cb_data {
      void *cb;
      void *data;

Regards,
-Denis

and I will split them in 2 commits.

Regards,
Giacinto

#7 lore raw

PATCH v8 5/5 atmodem/lte: proto and authentication handling

In reply to Giacinto Cifelli

The ofono_lte_default_attach_info now handles also the protocol and the authentication method, username and password.

---
 drivers/atmodem/lte.c | 118 ++++++++++++++++++++++++++++++++++++------
 1 file changed, 101 insertions(+), 17 deletions(-)
drivers/atmodem/lte.c +101 -17
index c4866623..54c8de54 100644
@@ -3,6 +3,7 @@
  *  oFono - Open Source Telephony
  *
  *  Copyright (C) 2017  Intel Corporation. All rights reserved.
+ *  Copyright (C) 2018 Gemalto M2M
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License version 2 as
@@ -44,41 +45,124 @@ struct lte_driver_data {
 	GAtChat *chat;
 };
 
-static void at_lte_set_default_attach_info_cb(gboolean ok, GAtResult *result,
+struct lte_cbd {
+	gint ref_count; /* Ref count */
+	ofono_lte_cb_t cb;
+	void *data;
+	GAtChat *chat;
+	const struct ofono_lte_default_attach_info *info;
+	struct ofono_modem *modem;
+};
+
+static struct lte_cbd *lte_cb_data_new0(void *cb, void *data,
+		GAtChat *chat, const struct ofono_lte_default_attach_info *info)
+{
+	struct lte_cbd *cbd = g_new0(struct lte_cbd, 1);
+
+	cbd->ref_count = 1;
+	cbd->cb = cb;
+	cbd->data = data;
+	cbd->chat = chat;
+	cbd->info = info;
+
+	return cbd;
+}
+
+static inline struct lte_cbd *lte_cb_data_ref(struct lte_cbd *cbd)
+{
+	if (cbd == NULL)
+		return NULL;
+
+	g_atomic_int_inc(&cbd->ref_count);
+
+	return cbd;
+}
+
+static void lte_cb_data_unref(gpointer user_data)
+{
+	gboolean is_zero;
+	struct lte_cbd *cbd = user_data;
+
+	if (cbd == NULL)
+		return;
+
+	is_zero = g_atomic_int_dec_and_test(&cbd->ref_count);
+
+	if (is_zero == TRUE)
+		g_free(cbd);
+}
+
+static void at_lte_set_auth_cb(gboolean ok, GAtResult *result,
 							gpointer user_data)
 {
-	struct cb_data *cbd = user_data;
+	struct lte_cbd *cbd = user_data;
 	ofono_lte_cb_t cb = cbd->cb;
 	struct ofono_error error;
 
-	DBG("ok %d", ok);
-
 	decode_at_error(&error, g_at_result_final_response(result));
 	cb(&error, cbd->data);
 }
 
+static void at_lte_set_default_attach_info_cb(gboolean ok, GAtResult *result,
+							gpointer user_data)
+{
+	struct lte_cbd *cbd = user_data;
+	ofono_lte_cb_t cb = cbd->cb;
+	void *data = cbd->data;
+	struct ofono_error error;
+	char buf[32 + OFONO_GPRS_MAX_USERNAME_LENGTH +
+					OFONO_GPRS_MAX_PASSWORD_LENGTH  + 1];
+	size_t buflen = sizeof(buf);
+	size_t len;
+	enum ofono_gprs_auth_method auth_method;
+
+	if (!ok) {
+		lte_cb_data_unref(cbd);
+		decode_at_error(&error, g_at_result_final_response(result));
+		cb(&error, data);
+		return;
+	}
+
+	auth_method = cbd->info->auth_method;
+
+	/* change the authentication method if the  parameters are invalid */
+	if (!*cbd->info->username || !*cbd->info->password)
+		auth_method = OFONO_GPRS_AUTH_METHOD_NONE;
+
+	len = snprintf(buf, buflen, "AT+CGAUTH=0,%d",
+					at_auth_type_from_method(auth_method));
+	buflen -= len;
+
+	if (auth_method != OFONO_GPRS_AUTH_METHOD_NONE)
+		snprintf(buf + len, buflen, ",\"%s\",\"%s\"",
+				cbd->info->username, cbd->info->password);
+
+	cbd = lte_cb_data_ref(cbd);
+	if (g_at_chat_send(cbd->chat, buf, NULL,
+			at_lte_set_auth_cb, cbd, lte_cb_data_unref) > 0)
+		return;
+
+	lte_cb_data_unref(cbd);
+	CALLBACK_WITH_FAILURE(cb, data);
+}
+
 static void at_lte_set_default_attach_info(const struct ofono_lte *lte,
 			const struct ofono_lte_default_attach_info *info,
 			ofono_lte_cb_t cb, void *data)
 {
 	struct lte_driver_data *ldd = ofono_lte_get_data(lte);
-	char buf[32 + OFONO_GPRS_MAX_APN_LENGTH + 1];
-	struct cb_data *cbd = cb_data_new(cb, data);
-
-	DBG("LTE config with APN: %s", info->apn);
+	struct lte_cbd *cbd = lte_cb_data_new0(cb, data, ldd->chat, info);
+	char *buf = at_get_cgdcont_command(0, info->proto, info->apn);
 
-	if (strlen(info->apn) > 0)
-		snprintf(buf, sizeof(buf), "AT+CGDCONT=0,\"IP\",\"%s\"",
-				info->apn);
-	else
-		snprintf(buf, sizeof(buf), "AT+CGDCONT=0,\"IP\"");
-
-	/* We can't do much in case of failure so don't check response. */
 	if (g_at_chat_send(ldd->chat, buf, NULL,
-			at_lte_set_default_attach_info_cb, cbd, g_free) > 0)
-		return;
+					at_lte_set_default_attach_info_cb,
+					cbd, lte_cb_data_unref) > 0)
+		goto end;
 
+	lte_cb_data_unref(cbd);
 	CALLBACK_WITH_FAILURE(cb, data);
+end:
+	g_free(buf);
 }
 
 static gboolean lte_delayed_register(gpointer user_data)
Signature · 1 line
2.17.1
#8 lore raw
In reply to Giacinto Cifelli

Hi Giacinto,

Quoted 9 lines · Giacinto Cifelli

I would like to submit this patch for the lte atom

This patch adds in core atom:
- the protocol for the default LTE APN, ip, ipv6 and both
- the authentication handling, with 3 properties:
method, username, password

then these are used in the atmodem atom:

Quoted 8 lines

The behavior of the patch is described in the api document (part 1), and in the core atom header (part 2).

Related to the previous version, this version:
- changes the name of the function at_auth_type_from_method in atmodem/atutils, as suggested by Jonas, and calls it without passing from an intermediate local variable.

I've pushed out the first three patches of this series with a few trivial tweaks added. Thanks!

Regards,
-Denis