[PATCH] xmm7modem: handling of IPv6 address for activated context

[PATCH] xmm7modem: handling of IPv6 address for activated context

[Antara Borwankar]

[-- Attachment #1: Type: text/plain, Size: 2295 bytes --]

Handled IPv6 address after activating PDP context
---
 drivers/ifxmodem/gprs-context.c | 38 ++++++++++++++++++++++++++++----------
 1 file changed, 28 insertions(+), 10 deletions(-)

diff --git a/drivers/ifxmodem/gprs-context.c b/drivers/ifxmodem/gprs-context.c
index 6042004..2c5456b 100644
--- a/drivers/ifxmodem/gprs-context.c
+++ b/drivers/ifxmodem/gprs-context.c
@@ -352,10 +352,17 @@ static void cgcontrdp_cb(gboolean ok, GAtResult *result, gpointer user_data)
 
 	DBG("DNS: %s, %s\n", gcd->dns1, gcd->dns2);
 
-	if (!laddrnetmask || at_util_get_ipv4_address_and_netmask(laddrnetmask,
-					gcd->address, gcd->netmask) < 0) {
-		failed_setup(gc, NULL, TRUE);
-		return;
+	if (gcd->proto == OFONO_GPRS_PROTO_IP) {
+		if (!laddrnetmask || at_util_get_ipv4_address_and_netmask(laddrnetmask,
+						gcd->address, gcd->netmask) < 0) {
+			failed_setup(gc, NULL, TRUE);
+			return;
+		}
+	}
+
+	if (gcd->proto == OFONO_GPRS_PROTO_IPV6) {
+		if (laddrnetmask)
+			strncpy(gcd->address, laddrnetmask, sizeof(gcd->address));
 	}
 
 	if (gw)
@@ -373,15 +380,26 @@ static void cgcontrdp_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	interface = ofono_gprs_context_get_interface(gc);
 	datapath = get_datapath(modem, interface);
 
-	ofono_gprs_context_set_ipv4_address(gc, gcd->address, TRUE);
+	if (gcd->proto == OFONO_GPRS_PROTO_IP) {
+		ofono_gprs_context_set_ipv4_address(gc, gcd->address, TRUE);
 
-	if (gcd->netmask[0])
-		ofono_gprs_context_set_ipv4_netmask(gc, gcd->netmask);
+		if (gcd->netmask[0])
+			ofono_gprs_context_set_ipv4_netmask(gc, gcd->netmask);
 
-	if (gcd->gateway[0])
-		ofono_gprs_context_set_ipv4_gateway(gc, gcd->gateway);
+		if (gcd->gateway[0])
+			ofono_gprs_context_set_ipv4_gateway(gc, gcd->gateway);
 
-	ofono_gprs_context_set_ipv4_dns_servers(gc, dns);
+		ofono_gprs_context_set_ipv4_dns_servers(gc, dns);
+	}
+
+	if (gcd->proto == OFONO_GPRS_PROTO_IPV6) {
+		ofono_gprs_context_set_ipv6_address(gc, gcd->address);
+
+		if (gcd->gateway[0])
+			ofono_gprs_context_set_ipv6_gateway(gc, gcd->gateway);
+
+		ofono_gprs_context_set_ipv6_dns_servers(gc, dns);
+	}
 
 	snprintf(buf, sizeof(buf), "AT+XDATACHANNEL=1,1,\"%s\",\"%s\",2,%u",
 			ctrlpath, datapath, gcd->active_context);
-- 
1.9.1

Re: [PATCH] xmm7modem: handling of IPv6 address for activated context

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 2953 bytes --]

Hi Antara,

On 04/03/2019 03:46 AM, Antara Borwankar wrote:
> Handled IPv6 address after activating PDP context
> ---
>   drivers/ifxmodem/gprs-context.c | 38 ++++++++++++++++++++++++++++----------
>   1 file changed, 28 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/ifxmodem/gprs-context.c b/drivers/ifxmodem/gprs-context.c
> index 6042004..2c5456b 100644
> --- a/drivers/ifxmodem/gprs-context.c
> +++ b/drivers/ifxmodem/gprs-context.c
> @@ -352,10 +352,17 @@ static void cgcontrdp_cb(gboolean ok, GAtResult *result, gpointer user_data)
>   
>   	DBG("DNS: %s, %s\n", gcd->dns1, gcd->dns2);
>   
> -	if (!laddrnetmask || at_util_get_ipv4_address_and_netmask(laddrnetmask,
> -					gcd->address, gcd->netmask) < 0) {
> -		failed_setup(gc, NULL, TRUE);
> -		return;
> +	if (gcd->proto == OFONO_GPRS_PROTO_IP) {
> +		if (!laddrnetmask || at_util_get_ipv4_address_and_netmask(laddrnetmask,
> +						gcd->address, gcd->netmask) < 0) {
> +			failed_setup(gc, NULL, TRUE);
> +			return;
> +		}
> +	}
> +
> +	if (gcd->proto == OFONO_GPRS_PROTO_IPV6) {
> +		if (laddrnetmask)
> +			strncpy(gcd->address, laddrnetmask, sizeof(gcd->address));

So what +CGPIAF setting is being assumed here?  If 
IPv6_AddressFormat==0, then aren't you also copying parts of the netmask 
into gcd->address?  FYI, the core doesn't validate the ipv6 address from 
the driver and broadcasts it directly over D-Bus.

>   	}
>   
>   	if (gw)
> @@ -373,15 +380,26 @@ static void cgcontrdp_cb(gboolean ok, GAtResult *result, gpointer user_data)
>   	interface = ofono_gprs_context_get_interface(gc);
>   	datapath = get_datapath(modem, interface);
>   
> -	ofono_gprs_context_set_ipv4_address(gc, gcd->address, TRUE);
> +	if (gcd->proto == OFONO_GPRS_PROTO_IP) {
> +		ofono_gprs_context_set_ipv4_address(gc, gcd->address, TRUE);
>   
> -	if (gcd->netmask[0])
> -		ofono_gprs_context_set_ipv4_netmask(gc, gcd->netmask);
> +		if (gcd->netmask[0])
> +			ofono_gprs_context_set_ipv4_netmask(gc, gcd->netmask);
>   
> -	if (gcd->gateway[0])
> -		ofono_gprs_context_set_ipv4_gateway(gc, gcd->gateway);
> +		if (gcd->gateway[0])
> +			ofono_gprs_context_set_ipv4_gateway(gc, gcd->gateway);
>   
> -	ofono_gprs_context_set_ipv4_dns_servers(gc, dns);
> +		ofono_gprs_context_set_ipv4_dns_servers(gc, dns);
> +	}
> +
> +	if (gcd->proto == OFONO_GPRS_PROTO_IPV6) {
> +		ofono_gprs_context_set_ipv6_address(gc, gcd->address);
> +

What about the prefix length?

> +		if (gcd->gateway[0])
> +			ofono_gprs_context_set_ipv6_gateway(gc, gcd->gateway);
> +
> +		ofono_gprs_context_set_ipv6_dns_servers(gc, dns);
> +	}

Also, I think we're ending up with too many if statements.  They should 
be combined and the DBG statements moved into the relevant if block.

>   
>   	snprintf(buf, sizeof(buf), "AT+XDATACHANNEL=1,1,\"%s\",\"%s\",2,%u",
>   			ctrlpath, datapath, gcd->active_context);
>

Regards,
-Denis

Re: [PATCH] xmm7modem: handling of IPv6 address for activated context

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 905 bytes --]

Hi Antara,

On 04/24/2019 12:54 AM, Antara Borwankar wrote:
> Handled IPv6 address after activating PDP context.
> Received IPv6 address is of format addr + netmask in the same string
> in the form of "a1.a2.a3.a4.a5.a6.a7.a8.a9.a10.a11.a12.a13.a14.a15.a16.
> m1.m2.m3.m4.m5.m6.m7.m8.m9.m10.m11.m12.m13.m14.m15.m16"
> ---
>   drivers/atmodem/atutil.c        | 39 +++++++++++++++++++++++++++++++++
>   drivers/atmodem/atutil.h        |  3 +++
>   drivers/ifxmodem/gprs-context.c | 48 +++++++++++++++++++++++++++++------------
>   3 files changed, 76 insertions(+), 14 deletions(-)
> 

I went ahead and applied this version after fixing up some lines which 
had > 80 chars.  But:

> +		ofono_gprs_context_set_ipv6_prefix_length(gc, IPV6_DEFAULT_PREFIX_LEN);

This really should be addressed at some point.  Assuming a default 
netmask won't work in every situation.

Regards,
-Denis

[PATCH] xmm7modem: handling of IPv6 address for activated context

[Antara Borwankar]

[-- Attachment #1: Type: text/plain, Size: 4852 bytes --]

Handled IPv6 address after activating PDP context.
Received IPv6 address is of format addr + netmask in the same string
in the form of "a1.a2.a3.a4.a5.a6.a7.a8.a9.a10.a11.a12.a13.a14.a15.a16.
m1.m2.m3.m4.m5.m6.m7.m8.m9.m10.m11.m12.m13.m14.m15.m16"
---
 drivers/atmodem/atutil.c        | 39 +++++++++++++++++++++++++++++++++
 drivers/atmodem/atutil.h        |  3 +++
 drivers/ifxmodem/gprs-context.c | 48 +++++++++++++++++++++++++++++------------
 3 files changed, 76 insertions(+), 14 deletions(-)

diff --git a/drivers/atmodem/atutil.c b/drivers/atmodem/atutil.c
index 98e3a2f..adcca85 100644
--- a/drivers/atmodem/atutil.c
+++ b/drivers/atmodem/atutil.c
@@ -656,6 +656,45 @@ int at_util_get_ipv4_address_and_netmask(const char *addrnetmask,
 	return ret;
 }
 
+/*
+ * CGCONTRDP returns addr + netmask in the same string in the form
+ * of "a1.a2.a3.a4.a5.a6.a7.a8.a9.a10.a11.a12.a13.a14.a15.a16.m1.m2.
+ * m3.m4.m5.m6.m7.m8.m9.m10.m11.m12.m13.m14.m15.m16" for IPv6.
+ * address/netmask must be able to hold 64 characters.
+ */
+int at_util_get_ipv6_address_and_netmask(const char *addrnetmask,
+						char *address, char *netmask)
+{
+	const char *s = addrnetmask;
+	const char *net = NULL;
+
+	int ret = -EINVAL;
+	int i;
+
+	/* Count 31 dots for ipv6, less or more means error. */
+	for (i = 0; i < 33; i++, s++) {
+		s = strchr(s, '.');
+
+		if (!s)
+			break;
+
+		if (i == 15) {
+			/* set netmask ptr and break the string */
+			net = s + 1;
+		}
+	}
+
+	if (i == 31) {
+		memcpy(address, addrnetmask, net - addrnetmask);
+		address[net - addrnetmask - 1] = '\0';
+		strcpy(netmask, net);
+
+		ret = 0;
+	}
+
+	return ret;
+}
+
 int at_util_gprs_auth_method_to_auth_prot(
 					enum ofono_gprs_auth_method auth_method)
 {
diff --git a/drivers/atmodem/atutil.h b/drivers/atmodem/atutil.h
index 69e8b49..61800ff 100644
--- a/drivers/atmodem/atutil.h
+++ b/drivers/atmodem/atutil.h
@@ -86,6 +86,9 @@ 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_util_get_ipv6_address_and_netmask(const char *addrnetmask,
+						char *address, char *netmask);
+
 
 int at_util_gprs_auth_method_to_auth_prot(
 				enum ofono_gprs_auth_method auth_method);
diff --git a/drivers/ifxmodem/gprs-context.c b/drivers/ifxmodem/gprs-context.c
index 6042004..5336f71 100644
--- a/drivers/ifxmodem/gprs-context.c
+++ b/drivers/ifxmodem/gprs-context.c
@@ -44,6 +44,7 @@
 #define TUN_DEV "/dev/net/tun"
 
 #define STATIC_IP_NETMASK "255.255.255.255"
+#define IPV6_DEFAULT_PREFIX_LEN 8
 
 static const char *none_prefix[] = { NULL };
 static const char *xdns_prefix[] = { "+XDNS:", NULL };
@@ -352,10 +353,39 @@ static void cgcontrdp_cb(gboolean ok, GAtResult *result, gpointer user_data)
 
 	DBG("DNS: %s, %s\n", gcd->dns1, gcd->dns2);
 
-	if (!laddrnetmask || at_util_get_ipv4_address_and_netmask(laddrnetmask,
-					gcd->address, gcd->netmask) < 0) {
-		failed_setup(gc, NULL, TRUE);
-		return;
+	if (gcd->proto == OFONO_GPRS_PROTO_IP) {
+		if (!laddrnetmask || at_util_get_ipv4_address_and_netmask(laddrnetmask,
+						gcd->address, gcd->netmask) < 0) {
+			failed_setup(gc, NULL, TRUE);
+			return;
+		}
+
+		ofono_gprs_context_set_ipv4_address(gc, gcd->address, TRUE);
+
+		if (gcd->netmask[0])
+			ofono_gprs_context_set_ipv4_netmask(gc, gcd->netmask);
+
+		if (gcd->gateway[0])
+			ofono_gprs_context_set_ipv4_gateway(gc, gcd->gateway);
+
+		ofono_gprs_context_set_ipv4_dns_servers(gc, dns);
+	}
+
+	if (gcd->proto == OFONO_GPRS_PROTO_IPV6) {
+		if (!laddrnetmask || at_util_get_ipv6_address_and_netmask(laddrnetmask,
+						gcd->address, gcd->netmask) < 0) {
+			failed_setup(gc, NULL, TRUE);
+			return;
+		}
+
+		ofono_gprs_context_set_ipv6_address(gc, gcd->address);
+
+		if (gcd->gateway[0])
+			ofono_gprs_context_set_ipv6_gateway(gc, gcd->gateway);
+
+		ofono_gprs_context_set_ipv6_dns_servers(gc, dns);
+
+		ofono_gprs_context_set_ipv6_prefix_length(gc, IPV6_DEFAULT_PREFIX_LEN);
 	}
 
 	if (gw)
@@ -373,16 +403,6 @@ static void cgcontrdp_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	interface = ofono_gprs_context_get_interface(gc);
 	datapath = get_datapath(modem, interface);
 
-	ofono_gprs_context_set_ipv4_address(gc, gcd->address, TRUE);
-
-	if (gcd->netmask[0])
-		ofono_gprs_context_set_ipv4_netmask(gc, gcd->netmask);
-
-	if (gcd->gateway[0])
-		ofono_gprs_context_set_ipv4_gateway(gc, gcd->gateway);
-
-	ofono_gprs_context_set_ipv4_dns_servers(gc, dns);
-
 	snprintf(buf, sizeof(buf), "AT+XDATACHANNEL=1,1,\"%s\",\"%s\",2,%u",
 			ctrlpath, datapath, gcd->active_context);
 	g_at_chat_send(gcd->chat, buf, none_prefix, NULL, NULL, NULL);
-- 
1.9.1

Re: [PATCH] xmm7modem: handling of IPv6 address for activated context

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 2656 bytes --]

Hi Antara,

On 03/20/2019 04:15 AM, Antara Borwankar wrote:
> Handled IPv6 address after activating PDP context
> ---
>   drivers/ifxmodem/gprs-context.c | 33 +++++++++++++++++++++++----------
>   1 file changed, 23 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/ifxmodem/gprs-context.c b/drivers/ifxmodem/gprs-context.c
> index 6042004..55d72a6 100644
> --- a/drivers/ifxmodem/gprs-context.c
> +++ b/drivers/ifxmodem/gprs-context.c
> @@ -352,10 +352,12 @@ static void cgcontrdp_cb(gboolean ok, GAtResult *result, gpointer user_data)
>   
>   	DBG("DNS: %s, %s\n", gcd->dns1, gcd->dns2);
>   
> -	if (!laddrnetmask || at_util_get_ipv4_address_and_netmask(laddrnetmask,
> -					gcd->address, gcd->netmask) < 0) {
> -		failed_setup(gc, NULL, TRUE);
> -		return;
> +	if(gcd->proto == OFONO_GPRS_PROTO_IP) {

There really should be a space after 'if' here.

> +		if (!laddrnetmask || at_util_get_ipv4_address_and_netmask(laddrnetmask,
> +						gcd->address, gcd->netmask) < 0) {
> +			failed_setup(gc, NULL, TRUE);
> +			return;
> +		}
>   	}
>   
>   	if (gw)
> @@ -373,15 +375,26 @@ static void cgcontrdp_cb(gboolean ok, GAtResult *result, gpointer user_data)
>   	interface = ofono_gprs_context_get_interface(gc);
>   	datapath = get_datapath(modem, interface);
>   
> -	ofono_gprs_context_set_ipv4_address(gc, gcd->address, TRUE);
> +	if(gcd->proto == OFONO_GPRS_PROTO_IP) {

And here

> +		ofono_gprs_context_set_ipv4_address(gc, gcd->address, TRUE);
>   
> -	if (gcd->netmask[0])
> -		ofono_gprs_context_set_ipv4_netmask(gc, gcd->netmask);
> +		if (gcd->netmask[0])
> +			ofono_gprs_context_set_ipv4_netmask(gc, gcd->netmask);
>   
> -	if (gcd->gateway[0])
> -		ofono_gprs_context_set_ipv4_gateway(gc, gcd->gateway);
> +		if (gcd->gateway[0])
> +			ofono_gprs_context_set_ipv4_gateway(gc, gcd->gateway);
>   
> -	ofono_gprs_context_set_ipv4_dns_servers(gc, dns);
> +		ofono_gprs_context_set_ipv4_dns_servers(gc, dns);
> +	}
> +
> +	if(gcd->proto == OFONO_GPRS_PROTO_IPV6) {
> +		ofono_gprs_context_set_ipv6_address(gc, gcd->address);

So I'm not sure I understand what is happening here.  gcd->address is 
only set when gcd->proto is OFONO_GPRS_PROTO_IP.  E.g. when it is 
extracted by at_util_get_ipv4_address_and_netmask.  How are you 
obtaining it for IPV6?

> +
> +		if (gcd->gateway[0])
> +			ofono_gprs_context_set_ipv6_gateway(gc, gcd->gateway);
> +
> +		ofono_gprs_context_set_ipv6_dns_servers(gc, dns);
> +	}
>   
>   	snprintf(buf, sizeof(buf), "AT+XDATACHANNEL=1,1,\"%s\",\"%s\",2,%u",
>   			ctrlpath, datapath, gcd->active_context);
> 

Regards,
-Denis

[PATCH] xmm7modem: handling of IPv6 address for activated context

[Antara Borwankar]

[-- Attachment #1: Type: text/plain, Size: 2147 bytes --]

Handled IPv6 address after activating PDP context
---
 drivers/ifxmodem/gprs-context.c | 33 +++++++++++++++++++++++----------
 1 file changed, 23 insertions(+), 10 deletions(-)

diff --git a/drivers/ifxmodem/gprs-context.c b/drivers/ifxmodem/gprs-context.c
index 6042004..55d72a6 100644
--- a/drivers/ifxmodem/gprs-context.c
+++ b/drivers/ifxmodem/gprs-context.c
@@ -352,10 +352,12 @@ static void cgcontrdp_cb(gboolean ok, GAtResult *result, gpointer user_data)
 
 	DBG("DNS: %s, %s\n", gcd->dns1, gcd->dns2);
 
-	if (!laddrnetmask || at_util_get_ipv4_address_and_netmask(laddrnetmask,
-					gcd->address, gcd->netmask) < 0) {
-		failed_setup(gc, NULL, TRUE);
-		return;
+	if(gcd->proto == OFONO_GPRS_PROTO_IP) {
+		if (!laddrnetmask || at_util_get_ipv4_address_and_netmask(laddrnetmask,
+						gcd->address, gcd->netmask) < 0) {
+			failed_setup(gc, NULL, TRUE);
+			return;
+		}
 	}
 
 	if (gw)
@@ -373,15 +375,26 @@ static void cgcontrdp_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	interface = ofono_gprs_context_get_interface(gc);
 	datapath = get_datapath(modem, interface);
 
-	ofono_gprs_context_set_ipv4_address(gc, gcd->address, TRUE);
+	if(gcd->proto == OFONO_GPRS_PROTO_IP) {
+		ofono_gprs_context_set_ipv4_address(gc, gcd->address, TRUE);
 
-	if (gcd->netmask[0])
-		ofono_gprs_context_set_ipv4_netmask(gc, gcd->netmask);
+		if (gcd->netmask[0])
+			ofono_gprs_context_set_ipv4_netmask(gc, gcd->netmask);
 
-	if (gcd->gateway[0])
-		ofono_gprs_context_set_ipv4_gateway(gc, gcd->gateway);
+		if (gcd->gateway[0])
+			ofono_gprs_context_set_ipv4_gateway(gc, gcd->gateway);
 
-	ofono_gprs_context_set_ipv4_dns_servers(gc, dns);
+		ofono_gprs_context_set_ipv4_dns_servers(gc, dns);
+	}
+
+	if(gcd->proto == OFONO_GPRS_PROTO_IPV6) {
+		ofono_gprs_context_set_ipv6_address(gc, gcd->address);
+
+		if (gcd->gateway[0])
+			ofono_gprs_context_set_ipv6_gateway(gc, gcd->gateway);
+
+		ofono_gprs_context_set_ipv6_dns_servers(gc, dns);
+	}
 
 	snprintf(buf, sizeof(buf), "AT+XDATACHANNEL=1,1,\"%s\",\"%s\",2,%u",
 			ctrlpath, datapath, gcd->active_context);
-- 
1.9.1