All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/6] Add PPP server support
@ 2010-06-23  9:46 Zhenhua Zhang
  2010-06-23  9:46 ` [PATCH 1/6] ppp: Fix incorrect packet length for little-endian Zhenhua Zhang
  0 siblings, 1 reply; 11+ messages in thread
From: Zhenhua Zhang @ 2010-06-23  9:46 UTC (permalink / raw)
  To: ofono

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

Hi,

Patch 1/6 fixes incorrect packet length in ipcp_rcr. Patch 2/6 is updated
according to comments. Other patches are unchanged.

Please review them. Thanks!

Best regards,
Zhenhua



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/6] ppp: Fix incorrect packet length for little-endian
  2010-06-23  9:46 [PATCH 0/6] Add PPP server support Zhenhua Zhang
@ 2010-06-23  9:46 ` Zhenhua Zhang
  2010-06-23  9:46   ` [PATCH 2/6] gatppp: Add PPP server extension Zhenhua Zhang
  2010-06-23 23:20   ` [PATCH 1/6] ppp: Fix incorrect packet length for little-endian Denis Kenzior
  0 siblings, 2 replies; 11+ messages in thread
From: Zhenhua Zhang @ 2010-06-23  9:46 UTC (permalink / raw)
  To: ofono

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

packet->length is in TCP/IP network byte order. It needs to call ntohs()
to convert to host byte order, which is little-endian.
---
 gatchat/ppp_ipcp.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/gatchat/ppp_ipcp.c b/gatchat/ppp_ipcp.c
index a1eacdf..0b3c381 100644
--- a/gatchat/ppp_ipcp.c
+++ b/gatchat/ppp_ipcp.c
@@ -281,7 +281,7 @@ static enum rcr_result ipcp_rcr(struct pppcp_data *pppcp,
 		return RCR_ACCEPT;
 
 	/* Reject all options */
-	*new_len = packet->length - sizeof(*packet);
+	*new_len = ntohs(packet->length) - sizeof(*packet);
 	*new_options = g_memdup(packet->data, *new_len);
 
 	return RCR_REJECT;
-- 
1.6.3.3


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/6] gatppp: Add PPP server extension
  2010-06-23  9:46 ` [PATCH 1/6] ppp: Fix incorrect packet length for little-endian Zhenhua Zhang
@ 2010-06-23  9:46   ` Zhenhua Zhang
  2010-06-23  9:46     ` [PATCH 3/6] atmodem: Fix GAtPPPConnectFunc interface change Zhenhua Zhang
  2010-06-23 23:19     ` [PATCH 2/6] gatppp: Add PPP server extension Denis Kenzior
  2010-06-23 23:20   ` [PATCH 1/6] ppp: Fix incorrect packet length for little-endian Denis Kenzior
  1 sibling, 2 replies; 11+ messages in thread
From: Zhenhua Zhang @ 2010-06-23  9:46 UTC (permalink / raw)
  To: ofono

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

1. Add interface to set PPP server info by g_at_ppp_set_server_info.
2. Pass local and peer address through IPCP handshaking.
---
 gatchat/gatppp.c   |   13 +++-
 gatchat/gatppp.h   |    7 ++-
 gatchat/ppp.h      |    6 ++-
 gatchat/ppp_ipcp.c |  183 ++++++++++++++++++++++++++++++++++++++++++++--------
 4 files changed, 178 insertions(+), 31 deletions(-)

diff --git a/gatchat/gatppp.c b/gatchat/gatppp.c
index e92fe5d..05136e0 100644
--- a/gatchat/gatppp.c
+++ b/gatchat/gatppp.c
@@ -246,7 +246,7 @@ void ppp_auth_notify(GAtPPP *ppp, gboolean success)
 	pppcp_signal_up(ppp->ipcp);
 }
 
-void ppp_ipcp_up_notify(GAtPPP *ppp, const char *ip,
+void ppp_ipcp_up_notify(GAtPPP *ppp, const char *local, const char *peer,
 					const char *dns1, const char *dns2)
 {
 	ppp->net = ppp_net_new(ppp);
@@ -264,7 +264,8 @@ void ppp_ipcp_up_notify(GAtPPP *ppp, const char *ip,
 
 	if (ppp->connect_cb)
 		ppp->connect_cb(ppp_net_get_interface(ppp->net),
-					ip, dns1, dns2, ppp->connect_data);
+					local, peer, dns1, dns2,
+					ppp->connect_data);
 }
 
 void ppp_ipcp_down_notify(GAtPPP *ppp)
@@ -464,6 +465,14 @@ void g_at_ppp_unref(GAtPPP *ppp)
 	g_free(ppp);
 }
 
+void g_at_ppp_set_server_info(GAtPPP *ppp, guint32 local, guint32 peer,
+				guint32 dns1, guint32 dns2,
+				guint32 nbns1, guint32 nbns2)
+{
+	ipcp_set_server_info(ppp->ipcp, local, peer, dns1, dns2,
+				nbns1, nbns2);
+}
+
 static GAtPPP *ppp_init_common(GAtHDLC *hdlc)
 {
 	GAtPPP *ppp;
diff --git a/gatchat/gatppp.h b/gatchat/gatppp.h
index 438b952..86b3081 100644
--- a/gatchat/gatppp.h
+++ b/gatchat/gatppp.h
@@ -43,7 +43,8 @@ typedef enum _GAtPPPDisconnectReason {
 	G_AT_PPP_REASON_LOCAL_CLOSE,	/* Normal user close */
 } GAtPPPDisconnectReason;
 
-typedef void (*GAtPPPConnectFunc)(const char *iface, const char *ip,
+typedef void (*GAtPPPConnectFunc)(const char *iface, const char *local,
+					const char *peer,
 					const char *dns1, const char *dns2,
 					gpointer user_data);
 typedef void (*GAtPPPDisconnectFunc)(GAtPPPDisconnectReason reason,
@@ -68,6 +69,10 @@ const char *g_at_ppp_get_password(GAtPPP *ppp);
 
 void g_at_ppp_set_recording(GAtPPP *ppp, const char *filename);
 
+void g_at_ppp_set_server_info(GAtPPP *ppp, guint32 local, guint32 peer,
+				guint32 dns1, guint32 dns2,
+				guint32 nbns1, guint32 nbns2);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/gatchat/ppp.h b/gatchat/ppp.h
index b6c5f4a..56da8a9 100644
--- a/gatchat/ppp.h
+++ b/gatchat/ppp.h
@@ -86,6 +86,10 @@ void lcp_protocol_reject(struct pppcp_data *lcp, guint8 *packet, gsize len);
 /* IPCP related functions */
 struct pppcp_data *ipcp_new(GAtPPP *ppp);
 void ipcp_free(struct pppcp_data *data);
+void ipcp_set_server_info(struct pppcp_data *ipcp, guint32 local_addr,
+				guint32 peer_addr,
+				guint32 dns1, guint32 dns2,
+				guint32 nbns1, guint32 nbns2);
 
 /* CHAP related functions */
 struct ppp_chap *ppp_chap_new(GAtPPP *ppp, guint8 method);
@@ -104,7 +108,7 @@ void ppp_debug(GAtPPP *ppp, const char *str);
 void ppp_transmit(GAtPPP *ppp, guint8 *packet, guint infolen);
 void ppp_set_auth(GAtPPP *ppp, const guint8 *auth_data);
 void ppp_auth_notify(GAtPPP *ppp, gboolean success);
-void ppp_ipcp_up_notify(GAtPPP *ppp, const char *ip,
+void ppp_ipcp_up_notify(GAtPPP *ppp, const char *local, const char *peer,
 					const char *dns1, const char *dns2);
 void ppp_ipcp_down_notify(GAtPPP *ppp);
 void ppp_ipcp_finished_notify(GAtPPP *ppp);
diff --git a/gatchat/ppp_ipcp.c b/gatchat/ppp_ipcp.c
index 0b3c381..f38d4c1 100644
--- a/gatchat/ppp_ipcp.c
+++ b/gatchat/ppp_ipcp.c
@@ -67,46 +67,49 @@ struct ipcp_data {
 	guint8 options[MAX_CONFIG_OPTION_SIZE];
 	guint16 options_len;
 	guint8 req_options;
-	guint32 ipaddr;
+	guint32 local_addr;
+	guint32 peer_addr;
 	guint32 dns1;
 	guint32 dns2;
 	guint32 nbns1;
 	guint32 nbns2;
+	gboolean is_server;
 };
 
-#define FILL_IP(req, type, var) 				\
-	if (req) {						\
-		ipcp->options[len] = type;			\
-		ipcp->options[len + 1] = 6;			\
-		memcpy(ipcp->options + len + 2, var, 4);	\
-								\
-		len += 6;					\
-	}							\
+#define FILL_IP(options, req, type, var) 		\
+	if (req) {					\
+		options[len] = type;			\
+		options[len + 1] = 6;			\
+		memcpy(options + len + 2, var, 4);	\
+							\
+		len += 6;				\
+	}						\
 
 static void ipcp_generate_config_options(struct ipcp_data *ipcp)
 {
 	guint16 len = 0;
 
-	FILL_IP(ipcp->req_options & REQ_OPTION_IPADDR,
-					IP_ADDRESS, &ipcp->ipaddr);
-	FILL_IP(ipcp->req_options & REQ_OPTION_DNS1,
+	FILL_IP(ipcp->options, ipcp->req_options & REQ_OPTION_IPADDR,
+					IP_ADDRESS, &ipcp->local_addr);
+	FILL_IP(ipcp->options, ipcp->req_options & REQ_OPTION_DNS1,
 					PRIMARY_DNS_SERVER, &ipcp->dns1);
-	FILL_IP(ipcp->req_options & REQ_OPTION_DNS2,
+	FILL_IP(ipcp->options, ipcp->req_options & REQ_OPTION_DNS2,
 					SECONDARY_DNS_SERVER, &ipcp->dns2);
-	FILL_IP(ipcp->req_options & REQ_OPTION_NBNS1,
+	FILL_IP(ipcp->options, ipcp->req_options & REQ_OPTION_NBNS1,
 					PRIMARY_NBNS_SERVER, &ipcp->nbns1);
-	FILL_IP(ipcp->req_options & REQ_OPTION_NBNS2,
+	FILL_IP(ipcp->options, ipcp->req_options & REQ_OPTION_NBNS2,
 					SECONDARY_NBNS_SERVER, &ipcp->nbns2);
 
 	ipcp->options_len = len;
 }
 
-static void ipcp_reset_config_options(struct ipcp_data *ipcp)
+static void ipcp_reset_client_config_options(struct ipcp_data *ipcp)
 {
 	ipcp->req_options = REQ_OPTION_IPADDR | REQ_OPTION_DNS1 |
 				REQ_OPTION_DNS2 | REQ_OPTION_NBNS1 |
 				REQ_OPTION_NBNS2;
-	ipcp->ipaddr = 0;
+	ipcp->local_addr = 0;
+	ipcp->peer_addr = 0;
 	ipcp->dns1 = 0;
 	ipcp->dns2 = 0;
 	ipcp->nbns1 = 0;
@@ -115,17 +118,53 @@ static void ipcp_reset_config_options(struct ipcp_data *ipcp)
 	ipcp_generate_config_options(ipcp);
 }
 
+static void ipcp_reset_server_config_options(struct ipcp_data *ipcp)
+{
+	ipcp->req_options = REQ_OPTION_IPADDR;
+
+	ipcp_generate_config_options(ipcp);
+}
+
+void ipcp_set_server_info(struct pppcp_data *pppcp, guint32 local_addr,
+				guint32 peer_addr,
+				guint32 dns1, guint32 dns2,
+				guint32 nbns1, guint32 nbns2)
+{
+	struct ipcp_data *ipcp = pppcp_get_data(pppcp);
+
+	if (local_addr != 0)
+		ipcp->req_options = REQ_OPTION_IPADDR;
+	else
+		ipcp->req_options = 0;
+
+	ipcp->local_addr = local_addr;
+	ipcp->peer_addr = peer_addr;
+	ipcp->dns1 = dns1;
+	ipcp->dns2 = dns2;
+	ipcp->nbns1 = nbns1;
+	ipcp->nbns2 = nbns2;
+	ipcp->is_server = TRUE;
+
+	ipcp_generate_config_options(ipcp);
+	pppcp_set_local_options(pppcp, ipcp->options, ipcp->options_len);
+}
+
 static void ipcp_up(struct pppcp_data *pppcp)
 {
 	struct ipcp_data *ipcp = pppcp_get_data(pppcp);
-	char ip[INET_ADDRSTRLEN];
+	char local[INET_ADDRSTRLEN];
+	char peer[INET_ADDRSTRLEN];
 	char dns1[INET_ADDRSTRLEN];
 	char dns2[INET_ADDRSTRLEN];
 	struct in_addr addr;
 
-	memset(ip, 0, sizeof(ip));
-	addr.s_addr = ipcp->ipaddr;
-	inet_ntop(AF_INET, &addr, ip, INET_ADDRSTRLEN);
+	memset(local, 0, sizeof(local));
+	addr.s_addr = ipcp->local_addr;
+	inet_ntop(AF_INET, &addr, local, INET_ADDRSTRLEN);
+
+	memset(peer, 0, sizeof(peer));
+	addr.s_addr = ipcp->peer_addr;
+	inet_ntop(AF_INET, &addr, peer, INET_ADDRSTRLEN);
 
 	memset(dns1, 0, sizeof(dns1));
 	addr.s_addr = ipcp->dns1;
@@ -135,7 +174,8 @@ static void ipcp_up(struct pppcp_data *pppcp)
 	addr.s_addr = ipcp->dns2;
 	inet_ntop(AF_INET, &addr, dns2, INET_ADDRSTRLEN);
 
-	ppp_ipcp_up_notify(pppcp_get_ppp(pppcp), ip[0] ? ip : NULL,
+	ppp_ipcp_up_notify(pppcp_get_ppp(pppcp), local[0] ? local : NULL,
+					peer[0] ? peer : NULL,
 					dns1[0] ? dns1 : NULL,
 					dns2[0] ? dns2 : NULL);
 }
@@ -144,7 +184,11 @@ static void ipcp_down(struct pppcp_data *pppcp)
 {
 	struct ipcp_data *ipcp = pppcp_get_data(pppcp);
 
-	ipcp_reset_config_options(ipcp);
+	if (ipcp->is_server)
+		ipcp_reset_server_config_options(ipcp);
+	else
+		ipcp_reset_client_config_options(ipcp);
+
 	pppcp_set_local_options(pppcp, ipcp->options, ipcp->options_len);
 	ppp_ipcp_down_notify(pppcp_get_ppp(pppcp));
 }
@@ -167,7 +211,7 @@ static void ipcp_rca(struct pppcp_data *pppcp,
 
 		switch (ppp_option_iter_get_type(&iter)) {
 		case IP_ADDRESS:
-			memcpy(&ipcp->ipaddr, data, 4);
+			memcpy(&ipcp->local_addr, data, 4);
 			break;
 		case PRIMARY_DNS_SERVER:
 			memcpy(&ipcp->dns1, data, 4);
@@ -204,7 +248,7 @@ static void ipcp_rcn_nak(struct pppcp_data *pppcp,
 		case IP_ADDRESS:
 			g_print("Setting suggested ip addr\n");
 			ipcp->req_options |= REQ_OPTION_IPADDR;
-			memcpy(&ipcp->ipaddr, data, 4);
+			memcpy(&ipcp->local_addr, data, 4);
 			break;
 		case PRIMARY_DNS_SERVER:
 			g_print("Setting suggested dns1\n");
@@ -269,17 +313,102 @@ static void ipcp_rcn_rej(struct pppcp_data *pppcp,
 	pppcp_set_local_options(pppcp, ipcp->options, ipcp->options_len);
 }
 
+static guint8 *ipcp_generate_peer_config_options(struct ipcp_data *ipcp,
+							guint16 *new_len)
+{
+	guint8 *options;
+	guint16 len = 0;
+
+	options = g_try_new0(guint8, MAX_CONFIG_OPTION_SIZE);
+	if (!options)
+		return NULL;
+
+	FILL_IP(options, TRUE, IP_ADDRESS, &ipcp->peer_addr);
+	FILL_IP(options, TRUE, PRIMARY_DNS_SERVER, &ipcp->dns1);
+	FILL_IP(options, TRUE, SECONDARY_DNS_SERVER, &ipcp->dns2);
+	FILL_IP(options, TRUE, PRIMARY_NBNS_SERVER, &ipcp->nbns1);
+	FILL_IP(options, TRUE, SECONDARY_NBNS_SERVER, &ipcp->nbns2);
+
+	*new_len = MAX_CONFIG_OPTION_SIZE;
+
+	return options;
+}
+
 static enum rcr_result ipcp_rcr(struct pppcp_data *pppcp,
 					const struct pppcp_packet *packet,
 					guint8 **new_options, guint16 *new_len)
 {
 	struct ppp_option_iter iter;
+	struct ipcp_data *ipcp = pppcp_get_data(pppcp);
+	guint32 peer_addr = 0;
+	guint32 dns1 = 0;
+	guint32 dns2 = 0;
+	guint32 nbns1 = 0;
+	guint32 nbns2 = 0;
 
 	ppp_option_iter_init(&iter, packet);
 
-	if (ppp_option_iter_next(&iter) == FALSE)
+	while (ppp_option_iter_next(&iter) == TRUE) {
+		const guint8 *data = ppp_option_iter_get_data(&iter);
+
+		switch (ppp_option_iter_get_type(&iter)) {
+		case IP_ADDRESS:
+			memcpy(&peer_addr, data, 4);
+			break;
+		case PRIMARY_DNS_SERVER:
+			memcpy(&dns1, data, 4);
+			break;
+		case SECONDARY_DNS_SERVER:
+			memcpy(&dns2, data, 4);
+			break;
+		case PRIMARY_NBNS_SERVER:
+			memcpy(&nbns1, data, 4);
+			break;
+		case SECONDARY_NBNS_SERVER:
+			memcpy(&nbns2, data, 4);
+			break;
+		default:
+			break;
+		}
+	}
+
+	if (ipcp->is_server) {
+		guint8 *options;
+		guint16 len;
+
+		/* Reject if we have not assign client address yet */
+		if (ipcp->peer_addr == 0 && ipcp->dns1 == 0 && ipcp->dns2 == 0)
+			goto reject;
+
+		/* Acknowledge client options if it matches with server options
+		 */
+		if (ipcp->peer_addr == peer_addr &&
+				ipcp->dns1 == dns1 && ipcp->dns2 == dns2 &&
+				ipcp->nbns1 == nbns1 && ipcp->nbns2 == nbns2)
+			return RCR_ACCEPT;
+
+		/* Send client IP/DNS/NBNS information in the config options */
+		options = ipcp_generate_peer_config_options(ipcp, &len);
+		if (!options)
+			goto reject;
+
+		*new_len = len;
+		*new_options = options;
+
+		return RCR_NAK;
+	}
+
+	/* Client */
+	if (peer_addr && ipcp->peer_addr == 0) {
+		/* RFC 1332 section 3.3
+		 * As client, accept the server IP as peer's address
+		 */
+		ipcp->peer_addr = peer_addr;
+
 		return RCR_ACCEPT;
+	}
 
+reject:
 	/* Reject all options */
 	*new_len = ntohs(packet->length) - sizeof(*packet);
 	*new_options = g_memdup(packet->data, *new_len);
@@ -317,7 +446,7 @@ struct pppcp_data *ipcp_new(GAtPPP *ppp)
 	}
 
 	pppcp_set_data(pppcp, ipcp);
-	ipcp_reset_config_options(ipcp);
+	ipcp_reset_client_config_options(ipcp);
 	pppcp_set_local_options(pppcp, ipcp->options, ipcp->options_len);
 
 	return pppcp;
-- 
1.6.3.3


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/6] atmodem: Fix GAtPPPConnectFunc interface change
  2010-06-23  9:46   ` [PATCH 2/6] gatppp: Add PPP server extension Zhenhua Zhang
@ 2010-06-23  9:46     ` Zhenhua Zhang
  2010-06-23  9:46       ` [PATCH 4/6] test-server: Add PPP server support Zhenhua Zhang
  2010-06-23 23:19     ` [PATCH 2/6] gatppp: Add PPP server extension Denis Kenzior
  1 sibling, 1 reply; 11+ messages in thread
From: Zhenhua Zhang @ 2010-06-23  9:46 UTC (permalink / raw)
  To: ofono

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

---
 drivers/atmodem/gprs-context.c |    5 +++--
 gatchat/gsmdial.c              |    5 +++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/atmodem/gprs-context.c b/drivers/atmodem/gprs-context.c
index 4ddf88e..fea80b0 100644
--- a/drivers/atmodem/gprs-context.c
+++ b/drivers/atmodem/gprs-context.c
@@ -65,7 +65,8 @@ struct gprs_context_data {
 	void *cb_data;                                  /* Callback data */
 };
 
-static void ppp_connect(const char *interface, const char *ip,
+static void ppp_connect(const char *interface, const char *local,
+			const char *remote,
 			const char *dns1, const char *dns2,
 			gpointer user_data)
 {
@@ -78,7 +79,7 @@ static void ppp_connect(const char *interface, const char *ip,
 	dns[2] = 0;
 
 	gcd->state = STATE_ACTIVE;
-	CALLBACK_WITH_SUCCESS(gcd->up_cb, interface, TRUE, ip,
+	CALLBACK_WITH_SUCCESS(gcd->up_cb, interface, TRUE, local,
 					STATIC_IP_NETMASK, NULL,
 					dns, gcd->cb_data);
 }
diff --git a/gatchat/gsmdial.c b/gatchat/gsmdial.c
index 62c3b3d..a69a610 100644
--- a/gatchat/gsmdial.c
+++ b/gatchat/gsmdial.c
@@ -221,13 +221,14 @@ out:
 	return FALSE;
 }
 
-static void ppp_connect(const char *iface, const char *ip,
+static void ppp_connect(const char *iface, const char *local, const char *peer,
 			const char *dns1, const char *dns2,
 			gpointer user_data)
 {
 	/* print out the negotiated address and dns server */
 	g_print("Network Device: %s\n", iface);
-	g_print("IP Address: %s\n", ip);
+	g_print("IP Address: %s\n", local);
+	g_print("Peer IP Address: %s\n", peer);
 	g_print("Primary DNS Server: %s\n", dns1);
 	g_print("Secondary DNS Server: %s\n", dns2);
 }
-- 
1.6.3.3


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 4/6] test-server: Add PPP server support
  2010-06-23  9:46     ` [PATCH 3/6] atmodem: Fix GAtPPPConnectFunc interface change Zhenhua Zhang
@ 2010-06-23  9:46       ` Zhenhua Zhang
  2010-06-23  9:46         ` [PATCH 5/6] test-server: Configure network interface Zhenhua Zhang
  0 siblings, 1 reply; 11+ messages in thread
From: Zhenhua Zhang @ 2010-06-23  9:46 UTC (permalink / raw)
  To: ofono

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

So that gsmdial and wvdial could talk to test-server and establish PPP
connection.
---
 gatchat/test-server.c |  131 ++++++++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 123 insertions(+), 8 deletions(-)

diff --git a/gatchat/test-server.c b/gatchat/test-server.c
index 5c1cfa4..68371c2 100644
--- a/gatchat/test-server.c
+++ b/gatchat/test-server.c
@@ -35,6 +35,7 @@
 #include <arpa/inet.h>
 #include <signal.h>
 #include <sys/signalfd.h>
+#include <errno.h>
 
 #include <glib.h>
 #include <utmp.h>
@@ -44,6 +45,7 @@
 #include <sys/stat.h>
 
 #include "gatserver.h"
+#include "gatppp.h"
 #include "ringbuffer.h"
 
 #define DEFAULT_TCP_PORT 12346
@@ -61,6 +63,7 @@ struct sock_server{
 
 static GMainLoop *mainloop;
 static GAtServer *server;
+static GAtPPP *ppp;
 unsigned int server_watch;
 
 static gboolean server_cleanup()
@@ -68,6 +71,11 @@ static gboolean server_cleanup()
 	if (server_watch)
 		g_source_remove(server_watch);
 
+	if (ppp) {
+		g_at_ppp_unref(ppp);
+		ppp = NULL;
+	}
+
 	g_at_server_unref(server);
 	server = NULL;
 
@@ -83,6 +91,81 @@ static void server_debug(const char *str, void *data)
 	g_print("%s: %s\n", (char *) data, str);
 }
 
+static void ppp_connect(const char *iface, const char *local, const char *peer,
+			const char *dns1, const char *dns2,
+			gpointer user)
+{
+	g_print("Network Device: %s\n", iface);
+	g_print("IP Address: %s\n", local);
+	g_print("Peer IP Address: %s\n", peer);
+	g_print("Primary DNS Server: %s\n", dns1);
+	g_print("Secondary DNS Server: %s\n", dns2);
+}
+
+static void ppp_disconnect(GAtPPPDisconnectReason reason, gpointer user)
+{
+	GAtServer *server = user;
+
+	g_print("PPP Link down: %d\n", reason);
+
+	g_at_ppp_unref(ppp);
+	ppp = NULL;
+
+	g_at_server_resume(server);
+	g_at_server_set_debug(server, server_debug, "Server");
+}
+
+static gboolean update_ppp(gpointer user)
+{
+	GAtPPP *ppp = user;
+	char local_ip[INET_ADDRSTRLEN] = "192.168.1.1";
+	char remote_ip[INET_ADDRSTRLEN] = "192.168.1.2";
+	char dns1[INET_ADDRSTRLEN] = "10.10.10.0";
+	char dns2[INET_ADDRSTRLEN] = "127.0.0.1";
+	guint32 l, r, d1, d2;
+
+	inet_pton(AF_INET, local_ip, &l);
+	inet_pton(AF_INET, remote_ip, &r);
+	inet_pton(AF_INET, dns1, &d1);
+	inet_pton(AF_INET, dns2, &d2);
+
+	g_at_ppp_set_server_info(ppp, l, r, d1, d2, 0, 0);
+
+	return FALSE;
+}
+
+static gboolean setup_ppp(gpointer user)
+{
+	GAtServer *server = user;
+	GAtIO *io;
+
+	io = g_at_server_get_io(server);
+
+	g_at_server_suspend(server);
+
+	/* open ppp */
+	ppp = g_at_ppp_new_from_io(io);
+	if (ppp == NULL) {
+		g_at_server_resume(server);
+		return FALSE;
+	}
+
+	g_at_ppp_set_debug(ppp, server_debug, "PPP");
+
+	g_at_ppp_set_credentials(ppp, "", "");
+
+	/* set connect and disconnect callbacks */
+	g_at_ppp_set_connect_function(ppp, ppp_connect, server);
+	g_at_ppp_set_disconnect_function(ppp, ppp_disconnect, server);
+
+	/* open the ppp connection */
+	g_at_ppp_open(ppp);
+
+	g_idle_add(update_ppp, ppp);
+
+	return FALSE;
+}
+
 static void cgmi_cb(GAtServerRequestType type, GAtResult *cmd, gpointer user)
 {
 	GAtServer *server = user;
@@ -468,6 +551,7 @@ static void cgdata_cb(GAtServerRequestType type, GAtResult *cmd, gpointer user)
 		break;
 	case G_AT_SERVER_REQUEST_TYPE_SET:
 		g_at_server_send_final(server, G_AT_SERVER_RESULT_CONNECT);
+		g_idle_add(setup_ppp, server);
 		break;
 	default:
 		g_at_server_send_final(server, G_AT_SERVER_RESULT_ERROR);
@@ -671,6 +755,40 @@ static void cpbs_cb(GAtServerRequestType type, GAtResult *cmd, gpointer user)
 	}
 }
 
+static void dial_cb(GAtServerRequestType type, GAtResult *cmd, gpointer user)
+{
+	GAtServer *server = user;
+	GAtServerResult res = G_AT_SERVER_RESULT_ERROR;
+	GAtResultIter iter;
+	const char *dial_str;
+	char c;
+
+	if (type != G_AT_SERVER_REQUEST_TYPE_SET)
+		goto error;
+
+	g_at_result_iter_init(&iter, cmd);
+
+	if (!g_at_result_iter_next(&iter, "D"))
+		goto error;
+
+	dial_str = g_at_result_iter_raw_line(&iter);
+	if (!dial_str)
+		goto error;
+
+	g_print("dial call %s\n", dial_str);
+
+	c = *dial_str;
+	if (c == '*' || c == '#' || c == 'T' || c == 't') {
+		g_at_server_send_final(server, G_AT_SERVER_RESULT_CONNECT);
+		g_idle_add(setup_ppp, server);
+	}
+
+	return;
+
+error:
+	g_at_server_send_final(server, res);
+}
+
 static void add_handler(GAtServer *server)
 {
 	g_at_server_set_debug(server, server_debug, "Server");
@@ -695,6 +813,7 @@ static void add_handler(GAtServer *server)
 	g_at_server_register(server, "+CSCS",    cscs_cb,    server, NULL);
 	g_at_server_register(server, "+CMGL",    cmgl_cb,    server, NULL);
 	g_at_server_register(server, "+CPBS",    cpbs_cb,    server, NULL);
+	g_at_server_register(server, "D",        dial_cb,    server, NULL);
 }
 
 static void server_destroy(gpointer user)
@@ -706,15 +825,11 @@ static void server_destroy(gpointer user)
 
 static void set_raw_mode(int fd)
 {
-	struct termios options;
-
-	tcgetattr(fd, &options);
-
-	/* Set TTY as raw mode to disable echo back of input characters
-	 * when they are received from Modem to avoid feedback loop */
-	options.c_lflag &= ~(ICANON | ECHO | ECHOE | ISIG);
+	struct termios ti;
 
-	tcsetattr(fd, TCSANOW, &options);
+	tcflush(fd, TCIOFLUSH);
+	cfmakeraw(&ti);
+	tcsetattr(fd, TCSANOW, &ti);
 }
 
 static gboolean create_tty(const char *modem_path)
-- 
1.6.3.3


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 5/6] test-server: Configure network interface
  2010-06-23  9:46       ` [PATCH 4/6] test-server: Add PPP server support Zhenhua Zhang
@ 2010-06-23  9:46         ` Zhenhua Zhang
  2010-06-23  9:46           ` [PATCH 6/6] gsmdial: Configure network interface for PPP Zhenhua Zhang
  0 siblings, 1 reply; 11+ messages in thread
From: Zhenhua Zhang @ 2010-06-23  9:46 UTC (permalink / raw)
  To: ofono

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

Require ROOT priviledge to:
1. Run external command to configure and bring up network interface.
2. Enable kernel IP forwarding.
---
 gatchat/test-server.c |   31 +++++++++++++++++++++++++++++++
 1 files changed, 31 insertions(+), 0 deletions(-)

diff --git a/gatchat/test-server.c b/gatchat/test-server.c
index 68371c2..df0bab8 100644
--- a/gatchat/test-server.c
+++ b/gatchat/test-server.c
@@ -50,6 +50,7 @@
 
 #define DEFAULT_TCP_PORT 12346
 #define DEFAULT_SOCK_PATH "./server_sock"
+#define IFCONFIG_PATH "/sbin/ifconfig"
 
 static int modem_mode = 0;
 static int modem_creg = 0;
@@ -91,15 +92,40 @@ static void server_debug(const char *str, void *data)
 	g_print("%s: %s\n", (char *) data, str);
 }
 
+static gboolean execute(const char *cmd)
+{
+	int status;
+
+	status = system(cmd);
+	if (status < 0) {
+		g_print("Failed to execute command: %s\n", strerror(errno));
+		return FALSE;
+	}
+
+	return TRUE;
+}
+
 static void ppp_connect(const char *iface, const char *local, const char *peer,
 			const char *dns1, const char *dns2,
 			gpointer user)
 {
+	char buf[512];
+
 	g_print("Network Device: %s\n", iface);
 	g_print("IP Address: %s\n", local);
 	g_print("Peer IP Address: %s\n", peer);
 	g_print("Primary DNS Server: %s\n", dns1);
 	g_print("Secondary DNS Server: %s\n", dns2);
+
+	snprintf(buf, sizeof(buf), "%s %s up", IFCONFIG_PATH, iface);
+	execute(buf);
+
+	snprintf(buf, sizeof(buf), "%s %s %s pointopoint %s", IFCONFIG_PATH,
+				iface, local, peer);
+	execute(buf);
+
+	snprintf(buf, sizeof(buf), "echo 1 > /proc/sys/net/ipv4/ip_forward");
+	execute(buf);
 }
 
 static void ppp_disconnect(GAtPPPDisconnectReason reason, gpointer user)
@@ -139,6 +165,11 @@ static gboolean setup_ppp(gpointer user)
 	GAtServer *server = user;
 	GAtIO *io;
 
+	if (getuid() != 0) {
+		g_print("Need root priviledge for PPP connection\n");
+		return FALSE;
+	}
+
 	io = g_at_server_get_io(server);
 
 	g_at_server_suspend(server);
-- 
1.6.3.3


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 6/6] gsmdial: Configure network interface for PPP
  2010-06-23  9:46         ` [PATCH 5/6] test-server: Configure network interface Zhenhua Zhang
@ 2010-06-23  9:46           ` Zhenhua Zhang
  0 siblings, 0 replies; 11+ messages in thread
From: Zhenhua Zhang @ 2010-06-23  9:46 UTC (permalink / raw)
  To: ofono

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

---
 gatchat/gsmdial.c |   29 +++++++++++++++++++++++++++++
 1 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/gatchat/gsmdial.c b/gatchat/gsmdial.c
index a69a610..59f1a5c 100644
--- a/gatchat/gsmdial.c
+++ b/gatchat/gsmdial.c
@@ -38,6 +38,8 @@
 #include <gattty.h>
 #include <gatppp.h>
 
+#define IFCONFIG_PATH "/sbin/ifconfig"
+
 static const char *none_prefix[] = { NULL };
 static const char *cfun_prefix[] = { "+CFUN:", NULL };
 static const char *creg_prefix[] = { "+CREG:", NULL };
@@ -221,16 +223,43 @@ out:
 	return FALSE;
 }
 
+static gboolean execute(const char *cmd)
+{
+	int status;
+
+	status = system(cmd);
+	if (status < 0) {
+		g_print("Failed to execute command: %s\n", strerror(errno));
+		return FALSE;
+	}
+
+	return TRUE;
+}
+
 static void ppp_connect(const char *iface, const char *local, const char *peer,
 			const char *dns1, const char *dns2,
 			gpointer user_data)
 {
+	char buf[512];
+
 	/* print out the negotiated address and dns server */
 	g_print("Network Device: %s\n", iface);
 	g_print("IP Address: %s\n", local);
 	g_print("Peer IP Address: %s\n", peer);
 	g_print("Primary DNS Server: %s\n", dns1);
 	g_print("Secondary DNS Server: %s\n", dns2);
+
+	if (getuid() != 0) {
+		g_print("Need root priviledge to config PPP interface\n");
+		return;
+	}
+
+	snprintf(buf, sizeof(buf), "%s %s up", IFCONFIG_PATH, iface);
+	execute(buf);
+
+	snprintf(buf, sizeof(buf), "%s %s %s pointopoint %s", IFCONFIG_PATH,
+				iface, local, peer);
+	execute(buf);
 }
 
 static void ppp_disconnect(GAtPPPDisconnectReason reason, gpointer user_data)
-- 
1.6.3.3


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/6] gatppp: Add PPP server extension
  2010-06-23  9:46   ` [PATCH 2/6] gatppp: Add PPP server extension Zhenhua Zhang
  2010-06-23  9:46     ` [PATCH 3/6] atmodem: Fix GAtPPPConnectFunc interface change Zhenhua Zhang
@ 2010-06-23 23:19     ` Denis Kenzior
  2010-06-24  6:23       ` Zhang, Zhenhua
  1 sibling, 1 reply; 11+ messages in thread
From: Denis Kenzior @ 2010-06-23 23:19 UTC (permalink / raw)
  To: ofono

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

Hi Zhenhua,

> 1. Add interface to set PPP server info by g_at_ppp_set_server_info.
> 2. Pass local and peer address through IPCP handshaking.

Ok getting pretty close now :)

> +static void ipcp_reset_server_config_options(struct ipcp_data *ipcp)
> +{
> +	ipcp->req_options = REQ_OPTION_IPADDR;

Might want to only request IP Addr if local_addr is not zero.  Just like in 
set_server_info.

> +
> +	ipcp_generate_config_options(ipcp);
> +}
> +

<snip>

> @@ -167,7 +211,7 @@ static void ipcp_rca(struct pppcp_data *pppcp,
> 
>  		switch (ppp_option_iter_get_type(&iter)) {
>  		case IP_ADDRESS:
> -			memcpy(&ipcp->ipaddr, data, 4);
> +			memcpy(&ipcp->local_addr, data, 4);
>  			break;
>  		case PRIMARY_DNS_SERVER:
>  			memcpy(&ipcp->dns1, data, 4);

You might not want to do anything here in the case of a server role.

> @@ -204,7 +248,7 @@ static void ipcp_rcn_nak(struct pppcp_data *pppcp,
>  		case IP_ADDRESS:
>  			g_print("Setting suggested ip addr\n");
>  			ipcp->req_options |= REQ_OPTION_IPADDR;
> -			memcpy(&ipcp->ipaddr, data, 4);
> +			memcpy(&ipcp->local_addr, data, 4);
>  			break;
>  		case PRIMARY_DNS_SERVER:
>  			g_print("Setting suggested dns1\n");

Again, probably don't want to set the local_addr in the case of a server role.

> @@ -269,17 +313,102 @@ static void ipcp_rcn_rej(struct pppcp_data *pppcp,
>  	pppcp_set_local_options(pppcp, ipcp->options, ipcp->options_len);
>  }
> 
> +static guint8 *ipcp_generate_peer_config_options(struct ipcp_data *ipcp,
> +							guint16 *new_len)
> +{
> +	guint8 *options;
> +	guint16 len = 0;
> +
> +	options = g_try_new0(guint8, MAX_CONFIG_OPTION_SIZE);
> +	if (!options)
> +		return NULL;
> +
> +	FILL_IP(options, TRUE, IP_ADDRESS, &ipcp->peer_addr);
> +	FILL_IP(options, TRUE, PRIMARY_DNS_SERVER, &ipcp->dns1);
> +	FILL_IP(options, TRUE, SECONDARY_DNS_SERVER, &ipcp->dns2);
> +	FILL_IP(options, TRUE, PRIMARY_NBNS_SERVER, &ipcp->nbns1);
> +	FILL_IP(options, TRUE, SECONDARY_NBNS_SERVER, &ipcp->nbns2);
> +
> +	*new_len = MAX_CONFIG_OPTION_SIZE;

Don't use MAX_CONFIG_OPTION_SIZE, instead use len (which is filled properly by 
the FILL_IP macro)  Also, we shouldn't bother supporting NBNS, lets never 
suggest those options as a server or set them in set_server_info.

> +
> +	return options;
> +}
> +
>  static enum rcr_result ipcp_rcr(struct pppcp_data *pppcp,
>  					const struct pppcp_packet *packet,
>  					guint8 **new_options, guint16 *new_len)
>  {
>  	struct ppp_option_iter iter;
> +	struct ipcp_data *ipcp = pppcp_get_data(pppcp);
> +	guint32 peer_addr = 0;
> +	guint32 dns1 = 0;
> +	guint32 dns2 = 0;
> +	guint32 nbns1 = 0;
> +	guint32 nbns2 = 0;
> 
>  	ppp_option_iter_init(&iter, packet);
> 
> -	if (ppp_option_iter_next(&iter) == FALSE)
> +	while (ppp_option_iter_next(&iter) == TRUE) {
> +		const guint8 *data = ppp_option_iter_get_data(&iter);
> +
> +		switch (ppp_option_iter_get_type(&iter)) {
> +		case IP_ADDRESS:
> +			memcpy(&peer_addr, data, 4);
> +			break;
> +		case PRIMARY_DNS_SERVER:
> +			memcpy(&dns1, data, 4);
> +			break;
> +		case SECONDARY_DNS_SERVER:
> +			memcpy(&dns2, data, 4);
> +			break;
> +		case PRIMARY_NBNS_SERVER:
> +			memcpy(&nbns1, data, 4);
> +			break;
> +		case SECONDARY_NBNS_SERVER:
> +			memcpy(&nbns2, data, 4);
> +			break;
> +		default:
> +			break;
> +		}
> +	}
> +

As mentioned above, if Primary / Secondary NBNS server is sent by the client, 
we need to Conf-Rej just those options to the client.  Any other unrecognized 
options should also be Conf-Rejected.  The order is important here, read the 
spec for details.  The IP Address, DNS1/DNS2 should not be Conf-Rejected.

> +	if (ipcp->is_server) {
> +		guint8 *options;
> +		guint16 len;
> +
> +		/* Reject if we have not assign client address yet */
> +		if (ipcp->peer_addr == 0 && ipcp->dns1 == 0 && ipcp->dns2 == 0)
> +			goto reject;

Actually you should be NAKing here, not Rejecting.  Reject means you don't 
support this option at all.

> +
> +		/* Acknowledge client options if it matches with server options
> +		 */
> +		if (ipcp->peer_addr == peer_addr &&
> +				ipcp->dns1 == dns1 && ipcp->dns2 == dns2 &&
> +				ipcp->nbns1 == nbns1 && ipcp->nbns2 == nbns2)
> +			return RCR_ACCEPT;
> +
> +		/* Send client IP/DNS/NBNS information in the config options */
> +		options = ipcp_generate_peer_config_options(ipcp, &len);
> +		if (!options)
> +			goto reject;
> +
> +		*new_len = len;
> +		*new_options = options;
> +
> +		return RCR_NAK;
> +	}
> +
> +	/* Client */
> +	if (peer_addr && ipcp->peer_addr == 0) {
> +		/* RFC 1332 section 3.3
> +		 * As client, accept the server IP as peer's address
> +		 */
> +		ipcp->peer_addr = peer_addr;
> +
>  		return RCR_ACCEPT;
> +	}
> 
> +reject:
>  	/* Reject all options */
>  	*new_len = ntohs(packet->length) - sizeof(*packet);
>  	*new_options = g_memdup(packet->data, *new_len);
> @@ -317,7 +446,7 @@ struct pppcp_data *ipcp_new(GAtPPP *ppp)
>  	}
> 
>  	pppcp_set_data(pppcp, ipcp);
> -	ipcp_reset_config_options(ipcp);
> +	ipcp_reset_client_config_options(ipcp);
>  	pppcp_set_local_options(pppcp, ipcp->options, ipcp->options_len);
> 
>  	return pppcp;
> 

Regards,
-Denis

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/6] ppp: Fix incorrect packet length for little-endian
  2010-06-23  9:46 ` [PATCH 1/6] ppp: Fix incorrect packet length for little-endian Zhenhua Zhang
  2010-06-23  9:46   ` [PATCH 2/6] gatppp: Add PPP server extension Zhenhua Zhang
@ 2010-06-23 23:20   ` Denis Kenzior
  1 sibling, 0 replies; 11+ messages in thread
From: Denis Kenzior @ 2010-06-23 23:20 UTC (permalink / raw)
  To: ofono

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

Hi Zhenhua,

> packet->length is in TCP/IP network byte order. It needs to call ntohs()
> to convert to host byte order, which is little-endian.

Nice catch.  Patch has been applied, thanks.

Regards,
-Denis

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH 2/6] gatppp: Add PPP server extension
  2010-06-23 23:19     ` [PATCH 2/6] gatppp: Add PPP server extension Denis Kenzior
@ 2010-06-24  6:23       ` Zhang, Zhenhua
  2010-06-24  6:26         ` Zhang, Zhenhua
  0 siblings, 1 reply; 11+ messages in thread
From: Zhang, Zhenhua @ 2010-06-24  6:23 UTC (permalink / raw)
  To: ofono

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

Hi Denis,

Denis Kenzior wrote:
> Hi Zhenhua,
> 
>> 1. Add interface to set PPP server info by g_at_ppp_set_server_info.
>> 2. Pass local and peer address through IPCP handshaking.
> 
> Ok getting pretty close now :)
> 
>> +static void ipcp_reset_server_config_options(struct ipcp_data
>> *ipcp) +{ +	ipcp->req_options = REQ_OPTION_IPADDR;
> 
> Might want to only request IP Addr if local_addr is not zero.
> Just like in
> set_server_info.

Ok. Local updated.
 
>> +
>> +	ipcp_generate_config_options(ipcp);
>> +}
>> +
> 
> <snip>
> 
>> @@ -167,7 +211,7 @@ static void ipcp_rca(struct pppcp_data *pppcp,
>> 
>>  		switch (ppp_option_iter_get_type(&iter)) {
>>  		case IP_ADDRESS:
>> -			memcpy(&ipcp->ipaddr, data, 4);
>> +			memcpy(&ipcp->local_addr, data, 4);
>>  			break;
>>  		case PRIMARY_DNS_SERVER:
>>  			memcpy(&ipcp->dns1, data, 4);
> 
> You might not want to do anything here in the case of a server role.

Agree. So in case of a server role, we simply do nothing in ipcp_rca and ipcp_rcn_nak. Good catch.

>> @@ -204,7 +248,7 @@ static void ipcp_rcn_nak(struct pppcp_data
>>  			*pppcp,  		case IP_ADDRESS: g_print("Setting suggested ip
>>  			addr\n"); ipcp->req_options |= REQ_OPTION_IPADDR;
>> -			memcpy(&ipcp->ipaddr, data, 4);
>> +			memcpy(&ipcp->local_addr, data, 4);
>>  			break;
>>  		case PRIMARY_DNS_SERVER:
>>  			g_print("Setting suggested dns1\n");
> 
> Again, probably don't want to set the local_addr in the case
> of a server role.
> 
>> @@ -269,17 +313,102 @@ static void ipcp_rcn_rej(struct pppcp_data
>>  	*pppcp, pppcp_set_local_options(pppcp, ipcp->options,
>> ipcp->options_len);  } 
>> 
>> +static guint8 *ipcp_generate_peer_config_options(struct ipcp_data
>> *ipcp, +							guint16
> *new_len)
>> +{
>> +	guint8 *options;
>> +	guint16 len = 0;
>> +
>> +	options = g_try_new0(guint8, MAX_CONFIG_OPTION_SIZE); +	if
>> (!options) +		return NULL;
>> +
>> +	FILL_IP(options, TRUE, IP_ADDRESS, &ipcp->peer_addr);
>> +	FILL_IP(options, TRUE, PRIMARY_DNS_SERVER, &ipcp->dns1);
>> +	FILL_IP(options, TRUE, SECONDARY_DNS_SERVER, &ipcp->dns2);
>> +	FILL_IP(options, TRUE, PRIMARY_NBNS_SERVER, &ipcp->nbns1);
>> +	FILL_IP(options, TRUE, SECONDARY_NBNS_SERVER, &ipcp->nbns2); +
>> +	*new_len = MAX_CONFIG_OPTION_SIZE;
> 
> Don't use MAX_CONFIG_OPTION_SIZE, instead use len (which is
> filled properly by
> the FILL_IP macro)  Also, we shouldn't bother supporting NBNS,
> lets never
> suggest those options as a server or set them in set_server_info.

Fixed, so I have removed NBNS parameters for set_server_info() at all.

>> +
>> +	return options;
>> +}
>> +
>>  static enum rcr_result ipcp_rcr(struct pppcp_data *pppcp,
>>  					const struct
> pppcp_packet *packet,
>>  					guint8 **new_options,
> guint16 *new_len)
>>  {
>>  	struct ppp_option_iter iter;
>> +	struct ipcp_data *ipcp = pppcp_get_data(pppcp);
>> +	guint32 peer_addr = 0;
>> +	guint32 dns1 = 0;
>> +	guint32 dns2 = 0;
>> +	guint32 nbns1 = 0;
>> +	guint32 nbns2 = 0;
>> 
>>  	ppp_option_iter_init(&iter, packet);
>> 
>> -	if (ppp_option_iter_next(&iter) == FALSE)
>> +	while (ppp_option_iter_next(&iter) == TRUE) {
>> +		const guint8 *data = ppp_option_iter_get_data(&iter); +
>> +		switch (ppp_option_iter_get_type(&iter)) {
>> +		case IP_ADDRESS:
>> +			memcpy(&peer_addr, data, 4);
>> +			break;
>> +		case PRIMARY_DNS_SERVER:
>> +			memcpy(&dns1, data, 4);
>> +			break;
>> +		case SECONDARY_DNS_SERVER:
>> +			memcpy(&dns2, data, 4);
>> +			break;
>> +		case PRIMARY_NBNS_SERVER:
>> +			memcpy(&nbns1, data, 4);
>> +			break;
>> +		case SECONDARY_NBNS_SERVER:
>> +			memcpy(&nbns2, data, 4);
>> +			break;
>> +		default:
>> +			break;
>> +		}
>> +	}
>> +
> 
> As mentioned above, if Primary / Secondary NBNS server is sent
> by the client,
> we need to Conf-Rej just those options to the client.  Any
> other unrecognized
> options should also be Conf-Rejected.  The order is important
> here, read the
> spec for details.  The IP Address, DNS1/DNS2 should not be
> Conf-Rejected. 

Looks like we need more Conf-Nak than Conf-Reject in ipcp_rcr. :-)

As PPP client, actually I think we don't need to send NBNS request at all. It's clear that pppd client only requests IP/DNS in Conf-Req. Secondly, if server sent us empty Conf-Req, the client should request server IP address in Conf-Nak response, instead of Conf-Rej all options. Once we get local_addr from server, we return Conf-Ack in ipcp_rcr and don't request server IP any more.

See attached pppd.log for details.

>> +	if (ipcp->is_server) {
>> +		guint8 *options;
>> +		guint16 len;
>> +
>> +		/* Reject if we have not assign client address yet */
>> +		if (ipcp->peer_addr == 0 && ipcp->dns1 == 0 && ipcp->dns2 == 0)
>> +			goto reject;
> 
> Actually you should be NAKing here, not Rejecting.  Reject
> means you don't
> support this option at all.

Okay.

>> +
>> +		/* Acknowledge client options if it matches with server options
>> +		 */ +		if (ipcp->peer_addr == peer_addr &&
>> +				ipcp->dns1 == dns1 &&
> ipcp->dns2 == dns2 &&
>> +				ipcp->nbns1 == nbns1 &&
> ipcp->nbns2 == nbns2)
>> +			return RCR_ACCEPT;
>> +
>> +		/* Send client IP/DNS/NBNS information in the config options */
>> +		options = ipcp_generate_peer_config_options(ipcp, &len); +		if
>> (!options) +			goto reject;
>> +
>> +		*new_len = len;
>> +		*new_options = options;
>> +
>> +		return RCR_NAK;
>> +	}
>> +
>> +	/* Client */
>> +	if (peer_addr && ipcp->peer_addr == 0) {
>> +		/* RFC 1332 section 3.3
>> +		 * As client, accept the server IP as peer's address +		 */
>> +		ipcp->peer_addr = peer_addr;
>> +

As client, can we just accept peer_addr as long as it's no zero, and return Conf-Ack? No matter what ipcp->peer_addr is.

>>  		return RCR_ACCEPT;
>> +	}
>> 
>> +reject:
>>  	/* Reject all options */
>>  	*new_len = ntohs(packet->length) - sizeof(*packet);
>>  	*new_options = g_memdup(packet->data, *new_len);
>> @@ -317,7 +446,7 @@ struct pppcp_data *ipcp_new(GAtPPP *ppp)  	}
>> 
>>  	pppcp_set_data(pppcp, ipcp);
>> -	ipcp_reset_config_options(ipcp);
>> +	ipcp_reset_client_config_options(ipcp);
>>  	pppcp_set_local_options(pppcp, ipcp->options, ipcp->options_len);
>> 
>>  	return pppcp;
>> 
> 
> Regards,
> -Denis



Regards,
Zhenhua


^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH 2/6] gatppp: Add PPP server extension
  2010-06-24  6:23       ` Zhang, Zhenhua
@ 2010-06-24  6:26         ` Zhang, Zhenhua
  0 siblings, 0 replies; 11+ messages in thread
From: Zhang, Zhenhua @ 2010-06-24  6:26 UTC (permalink / raw)
  To: ofono

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

Hi Denis,

Zhang, Zhenhua wrote:
> Hi Denis,
> 
>> As mentioned above, if Primary / Secondary NBNS server is sent
>> by the client,
>> we need to Conf-Rej just those options to the client.  Any
>> other unrecognized
>> options should also be Conf-Rejected.  The order is important
>> here, read the
>> spec for details.  The IP Address, DNS1/DNS2 should not be
>> Conf-Rejected.
> 
> Looks like we need more Conf-Nak than Conf-Reject in ipcp_rcr. :-)
> 
> As PPP client, actually I think we don't need to send NBNS
> request at all. It's clear that pppd client only requests
> IP/DNS in Conf-Req. Secondly, if server sent us empty
> Conf-Req, the client should request server IP address in
> Conf-Nak response, instead of Conf-Rej all options. Once we
> get local_addr from server, we return Conf-Ack in ipcp_rcr and
> don't request server IP any more.
> 
> See attached pppd.log for details.

Ops, forgot the attachment. Attached again.
 
>>> +	if (ipcp->is_server) {
>>> +		guint8 *options;
>>> +		guint16 len;
>>> +
>>> +		/* Reject if we have not assign client address yet */
>>> +		if (ipcp->peer_addr == 0 && ipcp->dns1 == 0 && ipcp->dns2 == 0)
>>> +			goto reject;
>> 
>> Actually you should be NAKing here, not Rejecting.  Reject
>> means you don't
>> support this option at all.
> 
> Okay.
> 
>>> +
>>> +		/* Acknowledge client options if it matches with server options
>>> +		 */ +		if (ipcp->peer_addr == peer_addr &&
>>> +				ipcp->dns1 == dns1 &&
>> ipcp->dns2 == dns2 &&
>>> +				ipcp->nbns1 == nbns1 &&
>> ipcp->nbns2 == nbns2)
>>> +			return RCR_ACCEPT;
>>> +
>>> +		/* Send client IP/DNS/NBNS information in the config options */
>>> +		options =
> ipcp_generate_peer_config_options(ipcp, &len); +		if
>>> (!options) +			goto reject;
>>> +
>>> +		*new_len = len;
>>> +		*new_options = options;
>>> +
>>> +		return RCR_NAK;
>>> +	}
>>> +
>>> +	/* Client */
>>> +	if (peer_addr && ipcp->peer_addr == 0) {
>>> +		/* RFC 1332 section 3.3
>>> +		 * As client, accept the server IP as peer's address +		 */
>>> +		ipcp->peer_addr = peer_addr;
>>> +
> 
> As client, can we just accept peer_addr as long as it's no
> zero, and return Conf-Ack? No matter what ipcp->peer_addr is.
> 
>>>  		return RCR_ACCEPT;
>>> +	}
>>> 
>>> +reject:
>>>  	/* Reject all options */
>>>  	*new_len = ntohs(packet->length) - sizeof(*packet);
>>>  	*new_options = g_memdup(packet->data, *new_len);
>>> @@ -317,7 +446,7 @@ struct pppcp_data *ipcp_new(GAtPPP *ppp)  	}
>>> 
>>>  	pppcp_set_data(pppcp, ipcp);
>>> -	ipcp_reset_config_options(ipcp);
>>> +	ipcp_reset_client_config_options(ipcp);
>>>  	pppcp_set_local_options(pppcp, ipcp->options, ipcp->options_len);
>>> 
>>>  	return pppcp;
>>> 
>> 
>> Regards,
>> -Denis
> 
> 
> 
> Regards,
> Zhenhua
> 
> _______________________________________________
> ofono mailing list
> ofono(a)ofono.org
> http://lists.ofono.org/listinfo/ofono



Regards,
Zhenhua


[-- Attachment #2: pppd.log --]
[-- Type: application/octet-stream, Size: 1327 bytes --]

pppd[8701]: sent [IPCP ConfReq id=0x1 <compress VJ 0f 01> <addr 0.0.0.0> <ms-dns1 0.0.0.0> <ms-dns2 0.0.0.0>]
pppd[8701]: rcvd [LCP ProtRej id=0x1 80 fd 01 01 00 0f 1a 04 78 00 18 04 78 00 15 03 2f]
pppd[8701]: Protocol-Reject for 'Compression Control Protocol' (0x80fd) received
pppd[8701]: rcvd [IPCP ConfReq id=0x1]
pppd[8701]: sent [IPCP ConfNak id=0x1 <addr 0.0.0.0>]
pppd[8701]: rcvd [IPCP ConfNak id=0x1 <addr 10.85.15.142> <ms-dns1 211.136.112.50> <ms-dns2 211.136.20.203>]
pppd[8701]: sent [IPCP ConfReq id=0x2 <compress VJ 0f 01> <addr 10.85.15.142> <ms-dns1 211.136.112.50> <ms-dns2 211.136.20.203>]
pppd[8701]: rcvd [IPCP ConfReq id=0x2]
pppd[8701]: sent [IPCP ConfAck id=0x2]
pppd[8701]: rcvd [IPCP ConfAck id=0x2 <compress VJ 0f 01> <addr 10.85.15.142> <ms-dns1 211.136.112.50> <ms-dns2 211.136.20.203>]
pppd[8701]: Could not determine remote IP address: defaulting to 10.64.64.64
pppd[8701]: not replacing existing default route via 10.239.20.1
pppd[8701]: Cannot determine ethernet address for proxy ARP
pppd[8701]: local  IP address 10.85.15.142
pppd[8701]: remote IP address 10.64.64.64
pppd[8701]: primary   DNS address 211.136.112.50
pppd[8701]: secondary DNS address 211.136.20.203
pppd[8701]: Script /etc/ppp/ip-up started (pid 8710)
pppd[8701]: Script /etc/ppp/ip-up finished (pid 8710), status = 0x0




^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2010-06-24  6:26 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-06-23  9:46 [PATCH 0/6] Add PPP server support Zhenhua Zhang
2010-06-23  9:46 ` [PATCH 1/6] ppp: Fix incorrect packet length for little-endian Zhenhua Zhang
2010-06-23  9:46   ` [PATCH 2/6] gatppp: Add PPP server extension Zhenhua Zhang
2010-06-23  9:46     ` [PATCH 3/6] atmodem: Fix GAtPPPConnectFunc interface change Zhenhua Zhang
2010-06-23  9:46       ` [PATCH 4/6] test-server: Add PPP server support Zhenhua Zhang
2010-06-23  9:46         ` [PATCH 5/6] test-server: Configure network interface Zhenhua Zhang
2010-06-23  9:46           ` [PATCH 6/6] gsmdial: Configure network interface for PPP Zhenhua Zhang
2010-06-23 23:19     ` [PATCH 2/6] gatppp: Add PPP server extension Denis Kenzior
2010-06-24  6:23       ` Zhang, Zhenhua
2010-06-24  6:26         ` Zhang, Zhenhua
2010-06-23 23:20   ` [PATCH 1/6] ppp: Fix incorrect packet length for little-endian Denis Kenzior

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.