3:50 p.m.
The patches extend GAtPPP to support PPP server role, so that you use gsmdial or wvdial to
talk to test-server (Patch 6/11 adds PPP server support in test-server).
If you're interesting to try it, connect two machines by BlueZ serial proxy.
Server side:
1. Run ./test-server by 'root'. It will tell you the server port.
2. Update /etc/bluetooth/servial.conf. Create one if it doesn't exist.
[Proxy DUN] # UUID for DUN proxy service
UUID=00001103-0000-1000-8000-00805F9B34FB
# Address for device node
Address=/dev/pts/xxx
3. Restart bluetoothd.
Client side:
1. rfcomm bind -r <server bdaddr> 1
2. Run './gsmdial -n /dev/rfcomm0' by 'root'.
You may update client's /etc/resolve.conf with server DNS info.
3:50 p.m.
New subject: [PATCH 01/11] gatserver: Suspend/resume GAtServer with GAtIO
Support g_at_server_suspend and g_at_server_resume operation by using
GAtIO to handle IO related function.
---
gatchat/gatserver.c | 259 +++++++++++++++++++++-----------------------------
gatchat/gatserver.h | 5 +
2 files changed, 114 insertions(+), 150 deletions(-)
diff --git a/gatchat/gatserver.c b/gatchat/gatserver.c
index 148754a..6bb9244 100644
--- a/gatchat/gatserver.c
+++ b/gatchat/gatserver.c
@@ -31,6 +31,7 @@
#include "ringbuffer.h"
#include "gatserver.h"
+#include "gatio.h"
#define BUF_SIZE 4096
/* <cr><lf> + the max length of information text + <cr><lf> */
@@ -100,16 +101,13 @@ struct at_command {
struct _GAtServer {
gint ref_count; /* Ref count */
struct v250_settings v250; /* V.250 command setting */
- GIOChannel *channel; /* Server IO */
- guint read_watch; /* GSource read id, 0 if none */
- guint write_watch; /* GSource write id, 0 if none */
+ GAtIO *io; /* Server IO */
guint read_so_far; /* Number of bytes processed */
GAtDisconnectFunc user_disconnect; /* User disconnect func */
gpointer user_disconnect_data; /* User disconnect data */
GAtDebugFunc debugf; /* Debugging output function */
gpointer debug_data; /* Data to pass to debug func */
GHashTable *command_list; /* List of AT commands */
- struct ring_buffer *read_buf; /* Current read buffer */
GQueue *write_queue; /* Write buffer queue */
guint max_read_attempts; /* Max reads per select */
enum ParserState parser_state;
@@ -117,12 +115,13 @@ struct _GAtServer {
char *last_line; /* Last read line */
unsigned int cur_pos; /* Where we are on the line */
GAtServerResult last_result;
- gboolean processing_cmdline;
+ gboolean suspended;
gboolean final_sent;
gboolean final_async;
+ gboolean in_read_handler;
};
-static void g_at_server_wakeup_writer(GAtServer *server);
+static void server_wakeup_writer(GAtServer *server);
static void server_parse_line(GAtServer *server);
static struct ring_buffer *allocate_next(GAtServer *server)
@@ -162,7 +161,7 @@ static void send_common(GAtServer *server, const char *buf, unsigned
int len)
write_buf = allocate_next(server);
}
- g_at_server_wakeup_writer(server);
+ server_wakeup_writer(server);
}
static void send_result_common(GAtServer *server, const char *result)
@@ -198,14 +197,14 @@ void g_at_server_send_final(GAtServer *server, GAtServerResult
result)
server->final_sent = TRUE;
server->last_result = result;
- if (result == G_AT_SERVER_RESULT_OK && server->processing_cmdline) {
+ if (result == G_AT_SERVER_RESULT_OK && server->suspended) {
if (server->final_async)
server_parse_line(server);
return;
}
- server->processing_cmdline = FALSE;
+ g_at_server_resume(server);
if (server->v250.is_v1)
sprintf(buf, "%s", server_result_to_string(result));
@@ -219,7 +218,7 @@ void g_at_server_send_ext_final(GAtServer *server, const char
*result)
{
server->final_sent = TRUE;
server->last_result = G_AT_SERVER_RESULT_EXT_ERROR;
- server->processing_cmdline = FALSE;
+ g_at_server_resume(server);
send_result_common(server, result);
}
@@ -668,7 +667,7 @@ static void server_parse_line(GAtServer *server)
server->final_async = FALSE;
if (pos == 0)
- server->processing_cmdline = TRUE;
+ g_at_server_suspend(server);
while (pos < len) {
unsigned int consumed;
@@ -702,7 +701,7 @@ static void server_parse_line(GAtServer *server)
return;
}
- server->processing_cmdline = FALSE;
+ g_at_server_resume(server);
g_at_server_send_final(server, G_AT_SERVER_RESULT_OK);
}
@@ -780,11 +779,11 @@ out:
return res;
}
-static char *extract_line(GAtServer *p)
+static char *extract_line(GAtServer *p, struct ring_buffer *rbuf)
{
- unsigned int wrap = ring_buffer_len_no_wrap(p->read_buf);
+ unsigned int wrap = ring_buffer_len_no_wrap(rbuf);
unsigned int pos = 0;
- unsigned char *buf = ring_buffer_read_ptr(p->read_buf, pos);
+ unsigned char *buf = ring_buffer_read_ptr(rbuf, pos);
int strip_front = 0;
int line_length = 0;
gboolean in_string = FALSE;
@@ -806,7 +805,7 @@ static char *extract_line(GAtServer *p)
pos += 1;
if (pos == wrap)
- buf = ring_buffer_read_ptr(p->read_buf, pos);
+ buf = ring_buffer_read_ptr(rbuf, pos);
}
/* We will strip AT and S3 */
@@ -814,17 +813,17 @@ static char *extract_line(GAtServer *p)
line = g_try_new(char, line_length + 1);
if (!line) {
- ring_buffer_drain(p->read_buf, p->read_so_far);
+ ring_buffer_drain(rbuf, p->read_so_far);
return NULL;
}
/* Strip leading whitespace + AT */
- ring_buffer_drain(p->read_buf, strip_front + 2);
+ ring_buffer_drain(rbuf, strip_front + 2);
pos = 0;
i = 0;
- wrap = ring_buffer_len_no_wrap(p->read_buf);
- buf = ring_buffer_read_ptr(p->read_buf, pos);
+ wrap = ring_buffer_len_no_wrap(rbuf);
+ buf = ring_buffer_read_ptr(rbuf, pos);
while (pos < (p->read_so_far - strip_front - 2)) {
if (*buf == '"')
@@ -839,33 +838,39 @@ static char *extract_line(GAtServer *p)
pos += 1;
if (pos == wrap)
- buf = ring_buffer_read_ptr(p->read_buf, pos);
+ buf = ring_buffer_read_ptr(rbuf, pos);
}
/* Strip S3 */
- ring_buffer_drain(p->read_buf, p->read_so_far - strip_front - 2);
+ ring_buffer_drain(rbuf, p->read_so_far - strip_front - 2);
line[i] = '\0';
return line;
}
-static void new_bytes(GAtServer *p)
+static void new_bytes(struct ring_buffer *rbuf, gpointer user_data)
{
- unsigned int len = ring_buffer_len(p->read_buf);
- unsigned int wrap = ring_buffer_len_no_wrap(p->read_buf);
- unsigned char *buf = ring_buffer_read_ptr(p->read_buf, p->read_so_far);
+ GAtServer *p = user_data;
+ unsigned int len = ring_buffer_len(rbuf);
+ unsigned int wrap = ring_buffer_len_no_wrap(rbuf);
+ unsigned char *buf = ring_buffer_read_ptr(rbuf, p->read_so_far);
enum ParserResult result;
- while (p->channel && (p->read_so_far < len)) {
+ p->in_read_handler = TRUE;
+
+ while (p->io && (p->read_so_far < len)) {
gsize rbytes = MIN(len - p->read_so_far, wrap - p->read_so_far);
result = server_feed(p, (char *)buf, &rbytes);
+ if (p->v250.echo)
+ send_common(p, (char *)buf, rbytes);
+
buf += rbytes;
p->read_so_far += rbytes;
if (p->read_so_far == wrap) {
- buf = ring_buffer_read_ptr(p->read_buf, p->read_so_far);
+ buf = ring_buffer_read_ptr(rbuf, p->read_so_far);
wrap = len;
}
@@ -879,14 +884,14 @@ static void new_bytes(GAtServer *p)
* Empty commands must be OK by the DCE
*/
g_at_server_send_final(p, G_AT_SERVER_RESULT_OK);
- ring_buffer_drain(p->read_buf, p->read_so_far);
+ ring_buffer_drain(rbuf, p->read_so_far);
break;
case PARSER_RESULT_COMMAND:
{
g_free(p->last_line);
- p->last_line = extract_line(p);
+ p->last_line = extract_line(p, rbuf);
p->cur_pos = 0;
if (p->last_line)
@@ -905,11 +910,11 @@ static void new_bytes(GAtServer *p)
else
g_at_server_send_final(p,
G_AT_SERVER_RESULT_OK);
- ring_buffer_drain(p->read_buf, p->read_so_far);
+ ring_buffer_drain(rbuf, p->read_so_far);
break;
default:
- ring_buffer_drain(p->read_buf, p->read_so_far);
+ ring_buffer_drain(rbuf, p->read_so_far);
break;
}
@@ -918,72 +923,15 @@ static void new_bytes(GAtServer *p)
p->read_so_far = 0;
}
- /* We're overflowing the buffer, shutdown the socket */
- if (p->read_buf && ring_buffer_avail(p->read_buf) == 0)
- g_source_remove(p->read_watch);
-}
-
-static gboolean received_data(GIOChannel *channel, GIOCondition cond,
- gpointer data)
-{
- unsigned char *buf;
- GAtServer *server = data;
- GIOError err;
- gsize rbytes;
- gsize toread;
- guint total_read = 0;
- guint read_count = 0;
-
- if (cond & G_IO_NVAL)
- return FALSE;
-
- do {
- toread = ring_buffer_avail_no_wrap(server->read_buf);
-
- if (toread == 0)
- break;
-
- rbytes = 0;
- buf = ring_buffer_write_ptr(server->read_buf, 0);
-
- err = g_io_channel_read(channel, (char *) buf, toread, &rbytes);
- g_at_util_debug_chat(TRUE, (char *)buf, rbytes,
- server->debugf, server->debug_data);
-
- read_count++;
-
- if (rbytes == 0)
- break;
-
- if (server->v250.echo)
- send_common(server, (char *)buf, rbytes);
-
- /* Ignore incoming bytes when processing a command line */
- if (server->processing_cmdline)
- continue;
+ p->in_read_handler = FALSE;
- total_read += rbytes;
- ring_buffer_write_advance(server->read_buf, rbytes);
- } while (err == G_IO_ERROR_NONE &&
- read_count < server->max_read_attempts);
-
- if (total_read > 0)
- new_bytes(server);
-
- if (cond & (G_IO_HUP | G_IO_ERR))
- return FALSE;
-
- if (read_count > 0 && rbytes == 0 && err != G_IO_ERROR_AGAIN)
- return FALSE;
-
- return TRUE;
+ if (p->destroyed)
+ g_free(p);
}
-static gboolean can_write_data(GIOChannel *channel, GIOCondition cond,
- gpointer data)
+static gboolean can_write_data(gpointer data)
{
GAtServer *server = data;
- GIOError err;
gsize bytes_written;
gsize towrite;
struct ring_buffer *write_buf;
@@ -992,9 +940,6 @@ static gboolean can_write_data(GIOChannel *channel, GIOCondition
cond,
int limiter;
#endif
- if (cond & (G_IO_NVAL | G_IO_HUP | G_IO_ERR))
- return FALSE;
-
if (!server->write_queue)
return FALSE;
@@ -1012,22 +957,17 @@ static gboolean can_write_data(GIOChannel *channel, GIOCondition
cond,
limiter = 5;
#endif
- err = g_io_channel_write(server->channel,
+ bytes_written = g_at_io_write(server->io,
(char *)buf,
#ifdef WRITE_SCHEDULER_DEBUG
- limiter,
+ limiter
#else
- towrite,
+ towrite
#endif
- &bytes_written);
+ );
- if (err != G_IO_ERROR_NONE) {
- g_source_remove(server->read_watch);
+ if (bytes_written == 0)
return FALSE;
- }
-
- g_at_util_debug_chat(FALSE, (char *)buf, bytes_written, server->debugf,
- server->debug_data);
ring_buffer_drain(write_buf, bytes_written);
@@ -1059,10 +999,6 @@ static void write_queue_free(GQueue *write_queue)
static void g_at_server_cleanup(GAtServer *server)
{
- /* Cleanup all received data */
- ring_buffer_free(server->read_buf);
- server->read_buf = NULL;
-
/* Cleanup pending data to write */
write_queue_free(server->write_queue);
@@ -1071,15 +1007,15 @@ static void g_at_server_cleanup(GAtServer *server)
g_free(server->last_line);
- server->channel = NULL;
+ g_at_io_unref(server->io);
+ server->io = NULL;
}
-static void read_watcher_destroy_notify(gpointer user_data)
+static void io_disconnect(gpointer user_data)
{
GAtServer *server = user_data;
g_at_server_cleanup(server);
- server->read_watch = 0;
if (server->user_disconnect)
server->user_disconnect(server->user_disconnect_data);
@@ -1088,23 +1024,9 @@ static void read_watcher_destroy_notify(gpointer user_data)
g_free(server);
}
-static void write_watcher_destroy_notify(gpointer user_data)
+static void server_wakeup_writer(GAtServer *server)
{
- GAtServer *server = user_data;
-
- server->write_watch = 0;
-}
-
-static void g_at_server_wakeup_writer(GAtServer *server)
-{
- if (server->write_watch != 0)
- return;
-
- server->write_watch = g_io_add_watch_full(server->channel,
- G_PRIORITY_DEFAULT,
- G_IO_OUT | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
- can_write_data, server,
- write_watcher_destroy_notify);
+ g_at_io_set_write_handler(server->io, can_write_data, server);
}
static void v250_settings_create(struct v250_settings *v250)
@@ -1156,13 +1078,15 @@ GAtServer *g_at_server_new(GIOChannel *io)
server->ref_count = 1;
v250_settings_create(&server->v250);
- server->channel = io;
+ server->io = g_at_io_new(io);
+ if (!server->io)
+ goto error;
+
+ g_at_io_set_disconnect_function(server->io, io_disconnect, server);
+
server->command_list = g_hash_table_new_full(g_str_hash, g_str_equal,
g_free,
at_notify_node_destroy);
- server->read_buf = ring_buffer_new(BUF_SIZE);
- if (!server->read_buf)
- goto error;
server->write_queue = g_queue_new();
if (!server->write_queue)
@@ -1173,25 +1097,18 @@ GAtServer *g_at_server_new(GIOChannel *io)
server->max_read_attempts = 3;
- if (!g_at_util_setup_io(server->channel, G_IO_FLAG_NONBLOCK))
- goto error;
-
- server->read_watch = g_io_add_watch_full(io, G_PRIORITY_DEFAULT,
- G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
- received_data, server,
- read_watcher_destroy_notify);
+ g_at_io_set_read_handler(server->io, new_bytes, server);
basic_command_register(server);
return server;
error:
+ g_at_io_unref(server->io);
+
if (server->command_list)
g_hash_table_destroy(server->command_list);
- if (server->read_buf)
- ring_buffer_free(server->read_buf);
-
if (server->write_queue)
write_queue_free(server->write_queue);
@@ -1201,6 +1118,22 @@ error:
return NULL;
}
+GIOChannel *g_at_server_get_channel(GAtServer *server)
+{
+ if (server == NULL || server->io == NULL)
+ return NULL;
+
+ return g_at_io_get_channel(server->io);
+}
+
+GAtIO *g_at_server_get_io(GAtServer *server)
+{
+ if (server == NULL)
+ return NULL;
+
+ return server->io;
+}
+
GAtServer *g_at_server_ref(GAtServer *server)
{
if (server == NULL)
@@ -1211,6 +1144,33 @@ GAtServer *g_at_server_ref(GAtServer *server)
return server;
}
+void g_at_server_suspend(GAtServer *server)
+{
+ if (server == NULL)
+ return;
+
+ server->suspended = TRUE;
+
+ g_at_io_set_write_handler(server->io, NULL, NULL);
+ g_at_io_set_read_handler(server->io, NULL, NULL);
+
+ g_at_io_set_debug(server->io, NULL, NULL);
+}
+
+void g_at_server_resume(GAtServer *server)
+{
+ if (server == NULL)
+ return;
+
+ server->suspended = FALSE;
+
+ g_at_io_set_debug(server->io, server->debugf, server->debug_data);
+ g_at_io_set_read_handler(server->io, new_bytes, server);
+
+ if (g_queue_get_length(server->write_queue) > 0)
+ server_wakeup_writer(server);
+}
+
void g_at_server_unref(GAtServer *server)
{
gboolean is_zero;
@@ -1223,6 +1183,11 @@ void g_at_server_unref(GAtServer *server)
if (is_zero == FALSE)
return;
+ if (server->io) {
+ g_at_server_suspend(server);
+ g_at_server_cleanup(server);
+ }
+
g_at_server_shutdown(server);
/* glib delays the destruction of the watcher until it exits, this
@@ -1230,7 +1195,7 @@ void g_at_server_unref(GAtServer *server)
* destroyed already. We have to wait until the read_watcher
* destroy function gets called
*/
- if (server->read_watch != 0)
+ if (server->in_read_handler)
server->destroyed = TRUE;
else
g_free(server);
@@ -1245,12 +1210,6 @@ gboolean g_at_server_shutdown(GAtServer *server)
server->user_disconnect = NULL;
server->user_disconnect_data = NULL;
- if (server->write_watch)
- g_source_remove(server->write_watch);
-
- if (server->read_watch)
- g_source_remove(server->read_watch);
-
return TRUE;
}
diff --git a/gatchat/gatserver.h b/gatchat/gatserver.h
index b604c37..f0c19da 100644
--- a/gatchat/gatserver.h
+++ b/gatchat/gatserver.h
@@ -28,6 +28,7 @@ extern "C" {
#include "gatresult.h"
#include "gatutil.h"
+#include "gatio.h"
struct _GAtServer;
@@ -68,8 +69,12 @@ typedef void (*GAtServerNotifyFunc)(GAtServerRequestType type,
GAtResult *result, gpointer user_data);
GAtServer *g_at_server_new(GIOChannel *io);
+GIOChannel *g_at_server_get_channel(GAtServer *server);
+GAtIO *g_at_server_get_io(GAtServer *server);
GAtServer *g_at_server_ref(GAtServer *server);
+void g_at_server_suspend(GAtServer *server);
+void g_at_server_resume(GAtServer *server);
void g_at_server_unref(GAtServer *server);
gboolean g_at_server_shutdown(GAtServer *server);
--
1.6.3.3
3:50 p.m.
New subject: [PATCH 02/11] gatserver: Check for disconnection when resuming
If the internal GAtIO is no longer valid, treat it as if our channel was
disconnected.
---
gatchat/gatserver.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/gatchat/gatserver.c b/gatchat/gatserver.c
index 6bb9244..3fa26a0 100644
--- a/gatchat/gatserver.c
+++ b/gatchat/gatserver.c
@@ -1164,6 +1164,11 @@ void g_at_server_resume(GAtServer *server)
server->suspended = FALSE;
+ if (g_at_io_get_channel(server->io) == NULL) {
+ io_disconnect(server);
+ return;
+ }
+
g_at_io_set_debug(server->io, server->debugf, server->debug_data);
g_at_io_set_read_handler(server->io, new_bytes, server);
--
1.6.3.3
3:50 p.m.
New subject: [PATCH 03/11] gatppp: Add PPP server extension
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 | 114 ++++++++++++++++++++++++++++++++++++++++++++++-----
4 files changed, 124 insertions(+), 16 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 a1eacdf..8f13d85 100644
--- a/gatchat/ppp_ipcp.c
+++ b/gatchat/ppp_ipcp.c
@@ -67,7 +67,8 @@ 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;
@@ -88,7 +89,7 @@ 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);
+ IP_ADDRESS, &ipcp->local_addr);
FILL_IP(ipcp->req_options & REQ_OPTION_DNS1,
PRIMARY_DNS_SERVER, &ipcp->dns1);
FILL_IP(ipcp->req_options & REQ_OPTION_DNS2,
@@ -101,12 +102,52 @@ static void ipcp_generate_config_options(struct ipcp_data *ipcp)
ipcp->options_len = len;
}
+static void ipcp_generate_peer_config_options(struct ipcp_data *ipcp)
+{
+ guint16 len = 0;
+
+ FILL_IP(ipcp->req_options & REQ_OPTION_IPADDR,
+ IP_ADDRESS, &ipcp->peer_addr);
+ FILL_IP(ipcp->req_options & REQ_OPTION_DNS1,
+ PRIMARY_DNS_SERVER, &ipcp->dns1);
+ FILL_IP(ipcp->req_options & REQ_OPTION_DNS2,
+ SECONDARY_DNS_SERVER, &ipcp->dns2);
+ FILL_IP(ipcp->req_options & REQ_OPTION_NBNS1,
+ PRIMARY_NBNS_SERVER, &ipcp->nbns1);
+ FILL_IP(ipcp->req_options & REQ_OPTION_NBNS2,
+ SECONDARY_NBNS_SERVER, &ipcp->nbns2);
+
+ ipcp->options_len = len;
+}
+
+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);
+
+ ipcp->req_options = REQ_OPTION_IPADDR | REQ_OPTION_DNS1 |
+ REQ_OPTION_DNS2 | REQ_OPTION_NBNS1 |
+ REQ_OPTION_NBNS2;
+
+ ipcp->local_addr = local_addr;
+ ipcp->peer_addr = peer_addr;
+ ipcp->dns1 = dns1;
+ ipcp->dns2 = dns2;
+ ipcp->nbns1 = nbns1;
+ ipcp->nbns2 = nbns2;
+
+ ipcp_generate_config_options(ipcp);
+}
+
static void ipcp_reset_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;
@@ -118,14 +159,19 @@ static void ipcp_reset_config_options(struct ipcp_data *ipcp)
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 +181,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);
}
@@ -167,7 +214,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 +251,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");
@@ -274,11 +321,54 @@ static enum rcr_result ipcp_rcr(struct pppcp_data *pppcp,
guint8 **new_options, guint16 *new_len)
{
struct ppp_option_iter iter;
+ struct ipcp_data *ipcp = pppcp_get_data(pppcp);
+ guint32 peer_addr;
+ guint32 dns1;
+ guint32 dns2;
ppp_option_iter_init(&iter, packet);
- if (ppp_option_iter_next(&iter) == FALSE)
- return RCR_ACCEPT;
+ 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;
+ default:
+ break;
+ }
+ }
+
+ if (peer_addr && dns1 && dns2) {
+ if (ipcp->peer_addr == 0) {
+ /* RFC 1332 Section 3.3
+ * Accept the value of IP address as peer IP address
+ */
+ ipcp->peer_addr = peer_addr;
+
+ return RCR_ACCEPT;
+ } else if (ipcp->peer_addr == peer_addr)
+ return RCR_ACCEPT;
+ } else {
+ /* Peer requests us to send ip address in config options */
+ if (ipcp->peer_addr) {
+ struct ipcp_data *peer;
+
+ peer = g_memdup(ipcp, sizeof(struct ipcp_data));
+ ipcp_generate_peer_config_options(peer);
+
+ *new_len = peer->options_len;
+ *new_options = peer->options;
+ return RCR_NAK;
+ }
+ }
/* Reject all options */
*new_len = packet->length - sizeof(*packet);
--
1.6.3.3
3:50 p.m.
New subject: [PATCH 04/11] atmodem: Fix GAtPPPConnectFunc interface change
---
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 a7d38c9..35267e1 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
3:50 p.m.
New subject: [PATCH 05/11] test-server: Fix GIOChannel leak in create_tty
Remove client_io and unref server_io after creating GAtServer.
---
gatchat/test-server.c | 6 ++----
1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/gatchat/test-server.c b/gatchat/test-server.c
index 74ddd4d..5c1cfa4 100644
--- a/gatchat/test-server.c
+++ b/gatchat/test-server.c
@@ -722,7 +722,6 @@ static gboolean create_tty(const char *modem_path)
int master, slave;
char pty_name[256];
GIOChannel *server_io;
- GIOChannel *client_io;
if (!modem_path)
return FALSE;
@@ -732,9 +731,6 @@ static gboolean create_tty(const char *modem_path)
set_raw_mode(slave);
- client_io = g_io_channel_unix_new(slave);
- g_io_channel_set_close_on_unref(client_io, TRUE);
-
g_print("new pty is created at %s\n", pty_name);
server_io = g_io_channel_unix_new(master);
@@ -747,6 +743,8 @@ static gboolean create_tty(const char *modem_path)
return FALSE;
}
+ g_io_channel_unref(server_io);
+
return TRUE;
}
--
1.6.3.3
3:50 p.m.
New subject: [PATCH 06/11] test-server: Add PPP server support
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
3:50 p.m.
New subject: [PATCH 07/11] test-server: Configure network interface
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
3:50 p.m.
New subject: [PATCH 08/11] gsmdial: Configure network interface for PPP
---
gatchat/gsmdial.c | 29 +++++++++++++++++++++++++++++
1 files changed, 29 insertions(+), 0 deletions(-)
diff --git a/gatchat/gsmdial.c b/gatchat/gsmdial.c
index 35267e1..29e1d22 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
3:50 p.m.
New subject: [PATCH 09/11] gsmdial: Unref ppp when we get disconnected
---
gatchat/gsmdial.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/gatchat/gsmdial.c b/gatchat/gsmdial.c
index 29e1d22..59f1a5c 100644
--- a/gatchat/gsmdial.c
+++ b/gatchat/gsmdial.c
@@ -265,7 +265,12 @@ static void ppp_connect(const char *iface, const char *local, const
char *peer,
static void ppp_disconnect(GAtPPPDisconnectReason reason, gpointer user_data)
{
char buf[64];
+
g_print("PPP Link down: %d\n", reason);
+
+ g_at_ppp_unref(ppp);
+ ppp = NULL;
+
g_at_chat_resume(modem);
sprintf(buf, "AT+CFUN=%u", option_offmode);
--
1.6.3.3
3:50 p.m.
New subject: [PATCH 10/11] test-server: Delay ppp_unref after IO processing
The ppp_disconnect is invoked during ppp receives LCP packet and enter
PPP dead phase. It's not good to stop the IO processing in disconnect
function. Delay the ppp_unref after packet is processed.
---
gatchat/test-server.c | 19 +++++++++++++++++--
1 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/gatchat/test-server.c b/gatchat/test-server.c
index df0bab8..dac7234 100644
--- a/gatchat/test-server.c
+++ b/gatchat/test-server.c
@@ -57,6 +57,7 @@ static int modem_creg = 0;
static int modem_cgreg = 0;
static int network_status = 4;
static int network_attach = 0;
+static int resume_source = 0;
struct sock_server{
int server_sock;
@@ -72,6 +73,9 @@ static gboolean server_cleanup()
if (server_watch)
g_source_remove(server_watch);
+ if (resume_source)
+ g_source_remove(resume_source);
+
if (ppp) {
g_at_ppp_unref(ppp);
ppp = NULL;
@@ -128,17 +132,28 @@ static void ppp_connect(const char *iface, const char *local, const
char *peer,
execute(buf);
}
-static void ppp_disconnect(GAtPPPDisconnectReason reason, gpointer user)
+static gboolean resume_server(gpointer user)
{
GAtServer *server = user;
- g_print("PPP Link down: %d\n", reason);
+ resume_source = 0;
g_at_ppp_unref(ppp);
ppp = NULL;
g_at_server_resume(server);
g_at_server_set_debug(server, server_debug, "Server");
+
+ modem_mode = 0;
+
+ return FALSE;
+}
+
+static void ppp_disconnect(GAtPPPDisconnectReason reason, gpointer user)
+{
+ g_print("PPP Link down: %d\n", reason);
+
+ resume_source = g_idle_add(resume_server, user);
}
static gboolean update_ppp(gpointer user)
--
1.6.3.3
3:50 p.m.
New subject: [PATCH 11/11] gsmdial: Delay ppp_unref after IO processing
---
gatchat/gsmdial.c | 17 +++++++++++++++--
1 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/gatchat/gsmdial.c b/gatchat/gsmdial.c
index 59f1a5c..d6cacc5 100644
--- a/gatchat/gsmdial.c
+++ b/gatchat/gsmdial.c
@@ -71,6 +71,7 @@ enum state {
static int state = 0;
static int oldmode = 0;
+static int resume_source = 0;
static void gsmdial_debug(const char *str, void *data)
{
@@ -262,11 +263,11 @@ static void ppp_connect(const char *iface, const char *local, const
char *peer,
execute(buf);
}
-static void ppp_disconnect(GAtPPPDisconnectReason reason, gpointer user_data)
+static gboolean resume_modem(gpointer user_data)
{
char buf[64];
- g_print("PPP Link down: %d\n", reason);
+ resume_source = 0;
g_at_ppp_unref(ppp);
ppp = NULL;
@@ -275,6 +276,15 @@ static void ppp_disconnect(GAtPPPDisconnectReason reason, gpointer
user_data)
sprintf(buf, "AT+CFUN=%u", option_offmode);
g_at_chat_send(control, buf, none_prefix, power_down, NULL, NULL);
+
+ return FALSE;
+}
+
+static void ppp_disconnect(GAtPPPDisconnectReason reason, gpointer user_data)
+{
+ g_print("PPP Link down: %d\n", reason);
+
+ resume_source = g_idle_add(resume_modem, NULL);
}
static void connect_cb(gboolean ok, GAtResult *result, gpointer user_data)
@@ -703,6 +713,9 @@ int main(int argc, char **argv)
g_main_loop_unref(event_loop);
out:
+ if (resume_source)
+ g_source_remove(resume_source);
+
if (ppp == NULL) {
g_at_chat_unref(control);
g_at_chat_unref(modem);
--
1.6.3.3
4:03 p.m.
New subject: [PATCH 11/11] gsmdial: Delay ppp_unref after IO processing
Hi,
Zhenhua Zhang wrote:
---
gatchat/gsmdial.c | 17 +++++++++++++++--
1 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/gatchat/gsmdial.c b/gatchat/gsmdial.c
index 59f1a5c..d6cacc5 100644
--- a/gatchat/gsmdial.c
+++ b/gatchat/gsmdial.c
I think we should delay the ppp shutdown stage until the LCP packet was well processed. I
got valgrind result as below before I have patches 10/11 and 11/11.
Entering new phase: 0
PPP Link down: 6
==5332== Invalid write of size 2
==5332== at 0x805520D: new_bytes (gathdlc.c:154)
==5332== by 0x804F2E6: received_data (gatio.c:119)
==5332== by 0x40AFDAA: g_io_unix_dispatch (giounix.c:162)
==5332== by 0x4078E87: g_main_context_dispatch (gmain.c:1960)
==5332== by 0x407C72F: g_main_context_iterate (gmain.c:2591)
==5332== by 0x407CB9E: g_main_loop_run (gmain.c:2799)
==5332== by 0x804BE5D: main (gsmdial.c:702)
==5332== Address 0x4382130 is 24 bytes inside a block of size 88 free'd
==5332== at 0x4024886: free (vg_replace_malloc.c:325)
==5332== by 0x4081195: g_free (gmem.c:190)
==5332== by 0x805557D: g_at_hdlc_unref (gathdlc.c:270)
==5332== by 0x8056316: g_at_ppp_unref (gatppp.c:463)
==5332== by 0x804AF20: ppp_disconnect (gsmdial.c:272)
==5332== by 0x8055CE9: ppp_dead (gatppp.c:204)
==5332== by 0x8055D1E: ppp_enter_phase (gatppp.c:214)
==5332== by 0x8055FE0: ppp_lcp_finished_notify (gatppp.c:317)
==5332== by 0x8057E27: lcp_finished (ppp_lcp.c:140)
==5332== by 0x80568DF: pppcp_this_layer_finished (ppp_cp.c:340)
==5332== by 0x8057446: pppcp_generate_event (ppp_cp.c:688)
==5332== by 0x8057B5B: pppcp_process_packet (ppp_cp.c:965)
==5332==
==5332== Invalid write of size 4
==5332== at 0x8055216: new_bytes (gathdlc.c:155)
==5332== by 0x804F2E6: received_data (gatio.c:119)
==5332== by 0x40AFDAA: g_io_unix_dispatch (giounix.c:162)
==5332== by 0x4078E87: g_main_context_dispatch (gmain.c:1960)
==5332== by 0x407C72F: g_main_context_iterate (gmain.c:2591)
==5332== by 0x407CB9E: g_main_loop_run (gmain.c:2799)
==5332== by 0x804BE5D: main (gsmdial.c:702)
==5332== Address 0x438212c is 20 bytes inside a block of size 88 free'd
==5332== at 0x4024886: free (vg_replace_malloc.c:325)
==5332== by 0x4081195: g_free (gmem.c:190)
==5332== by 0x805557D: g_at_hdlc_unref (gathdlc.c:270)
==5332== by 0x8056316: g_at_ppp_unref (gatppp.c:463)
==5332== by 0x804AF20: ppp_disconnect (gsmdial.c:272)
==5332== by 0x8055CE9: ppp_dead (gatppp.c:204)
==5332== by 0x8055D1E: ppp_enter_phase (gatppp.c:214)
==5332== by 0x8055FE0: ppp_lcp_finished_notify (gatppp.c:317)
==5332== by 0x8057E27: lcp_finished (ppp_lcp.c:140)
==5332== by 0x80568DF: pppcp_this_layer_finished (ppp_cp.c:340)
==5332== by 0x8057446: pppcp_generate_event (ppp_cp.c:688)
==5332== by 0x8057B5B: pppcp_process_packet (ppp_cp.c:965)
==5332==
==5332== Invalid read of size 4
==5332== at 0x80552CB: new_bytes (gathdlc.c:167)
==5332== by 0x804F2E6: received_data (gatio.c:119)
==5332== by 0x40AFDAA: g_io_unix_dispatch (giounix.c:162)
==5332== by 0x4078E87: g_main_context_dispatch (gmain.c:1960)
==5332== by 0x407C72F: g_main_context_iterate (gmain.c:2591)
==5332== by 0x407CB9E: g_main_loop_run (gmain.c:2799)
==5332== by 0x804BE5D: main (gsmdial.c:702)
==5332== Address 0x438216c is 84 bytes inside a block of size 88 free'd
==5332== at 0x4024886: free (vg_replace_malloc.c:325)
==5332== by 0x4081195: g_free (gmem.c:190)
==5332== by 0x805557D: g_at_hdlc_unref (gathdlc.c:270)
==5332== by 0x8056316: g_at_ppp_unref (gatppp.c:463)
==5332== by 0x804AF20: ppp_disconnect (gsmdial.c:272)
==5332== by 0x8055CE9: ppp_dead (gatppp.c:204)
==5332== by 0x8055D1E: ppp_enter_phase (gatppp.c:214)
==5332== by 0x8055FE0: ppp_lcp_finished_notify (gatppp.c:317)
==5332== by 0x8057E27: lcp_finished (ppp_lcp.c:140)
==5332== by 0x80568DF: pppcp_this_layer_finished (ppp_cp.c:340)
==5332== by 0x8057446: pppcp_generate_event (ppp_cp.c:688)
==5332== by 0x8057B5B: pppcp_process_packet (ppp_cp.c:965)
==5332==
: > AT+CFUN=0\r
==5332==
==5332== HEAP SUMMARY:
==5332== in use at exit: 8,854 bytes in 31 blocks
==5332== total heap usage: 860 allocs, 829 frees, 3,139,527 bytes allocated
==5332==
==5332== 248 bytes in 1 blocks are possibly lost in loss record 13 of 23
==5332== at 0x4023E7E: memalign (vg_replace_malloc.c:532)
==5332== by 0x4023EDB: posix_memalign (vg_replace_malloc.c:660)
==5332== by 0x409661D: slab_allocator_alloc_chunk (gslice.c:1136)
==5332== by 0x4097580: g_slice_alloc (gslice.c:661)
==5332== by 0x4076D75: g_list_prepend (glist.c:169)
==5332== by 0x4085A82: add_pending_null (goption.c:1119)
==5332== by 0x40868C6: parse_short_option (goption.c:1397)
==5332== by 0x4087EC1: g_option_context_parse (goption.c:1858)
==5332== by 0x804BB35: main (gsmdial.c:631)
==5332==
==5332== 360 bytes in 3 blocks are possibly lost in loss record 14 of 23
==5332== at 0x4023E7E: memalign (vg_replace_malloc.c:532)
==5332== by 0x4023EDB: posix_memalign (vg_replace_malloc.c:660)
==5332== by 0x409661D: slab_allocator_alloc_chunk (gslice.c:1136)
==5332== by 0x40975AC: g_slice_alloc (gslice.c:666)
==5332== by 0x40987B1: g_slist_prepend (gslist.c:160)
==5332== by 0x4079859: g_source_add_poll (gmain.c:972)
==5332== by 0x40AFF04: g_io_unix_create_watch (giounix.c:357)
==5332== by 0x406D828: g_io_create_watch (giochannel.c:516)
==5332== by 0x4070712: g_io_add_watch_full (giochannel.c:550)
==5332== by 0x804F503: create_io (gatio.c:207)
==5332== by 0x804F555: g_at_io_new (gatio.c:225)
==5332== by 0x804D1E6: create_chat (gatchat.c:825)
==5332==
==5332== 496 bytes in 2 blocks are possibly lost in loss record 15 of 23
==5332== at 0x4023E7E: memalign (vg_replace_malloc.c:532)
==5332== by 0x4023EDB: posix_memalign (vg_replace_malloc.c:660)
==5332== by 0x409661D: slab_allocator_alloc_chunk (gslice.c:1136)
==5332== by 0x40975AC: g_slice_alloc (gslice.c:666)
==5332== by 0x4076D75: g_list_prepend (glist.c:169)
==5332== by 0x4085A82: add_pending_null (goption.c:1119)
==5332== by 0x40868C6: parse_short_option (goption.c:1397)
==5332== by 0x4087EC1: g_option_context_parse (goption.c:1858)
==5332== by 0x804BB35: main (gsmdial.c:631)
==5332==
==5332== 504 bytes in 1 blocks are possibly lost in loss record 17 of 23
==5332== at 0x4023E7E: memalign (vg_replace_malloc.c:532)
==5332== by 0x4023EDB: posix_memalign (vg_replace_malloc.c:660)
==5332== by 0x409661D: slab_allocator_alloc_chunk (gslice.c:1136)
==5332== by 0x40975AC: g_slice_alloc (gslice.c:666)
==5332== by 0x406A628: g_hash_table_new_full (ghash.c:512)
==5332== by 0x804D263: create_chat (gatchat.c:839)
==5332== by 0x804D310: g_at_chat_new (gatchat.c:863)
==5332== by 0x804B837: open_serial (gsmdial.c:517)
==5332== by 0x804BBDD: main (gsmdial.c:651)
==5332==
==5332== 1,240 bytes in 5 blocks are possibly lost in loss record 22 of 23
==5332== at 0x4023E7E: memalign (vg_replace_malloc.c:532)
==5332== by 0x4023EDB: posix_memalign (vg_replace_malloc.c:660)
==5332== by 0x409661D: slab_allocator_alloc_chunk (gslice.c:1136)
==5332== by 0x40975AC: g_slice_alloc (gslice.c:666)
==5332== by 0x40532D8: g_array_sized_new (garray.c:87)
==5332== by 0x40533F6: g_array_new (garray.c:79)
==5332== by 0x40A361E: g_static_private_set (gthread.c:451)
==5332== by 0x40AA883: g_get_charset (gutf8.c:545)
==5332== by 0x40834CC: g_print (gmessages.c:1043)
==5332== by 0x804BBBA: main (gsmdial.c:647)
==5332==
==5332== LEAK SUMMARY:
==5332== definitely lost: 0 bytes in 0 blocks
==5332== indirectly lost: 0 bytes in 0 blocks
==5332== possibly lost: 2,848 bytes in 12 blocks
==5332== still reachable: 6,006 bytes in 19 blocks
==5332== suppressed: 0 bytes in 0 blocks
==5332== Reachable blocks (those to which a pointer was found) are not shown.
==5332== To see them, rerun with: --leak-check=full --show-reachable=yes
==5332==
==5332== For counts of detected and suppressed errors, rerun with: -v
==5332== ERROR SUMMARY: 8 errors from 8 contexts (suppressed: 15 from 6)
Regards,
Zhenhua
3:51 a.m.
New subject: [PATCH 11/11] gsmdial: Delay ppp_unref after IO processing
Hi Zhenhua,
I think we should delay the ppp shutdown stage until the LCP packet
was
well processed. I got valgrind result as below before I have patches 10/11
and 11/11.
Entering new phase: 0
PPP Link down: 6
==5332== Invalid write of size 2
==5332== at 0x805520D: new_bytes (gathdlc.c:154)
==5332== by 0x804F2E6: received_data (gatio.c:119)
==5332== by 0x40AFDAA: g_io_unix_dispatch (giounix.c:162)
==5332== by 0x4078E87: g_main_context_dispatch (gmain.c:1960)
==5332== by 0x407C72F: g_main_context_iterate (gmain.c:2591)
==5332== by 0x407CB9E: g_main_loop_run (gmain.c:2799)
==5332== by 0x804BE5D: main (gsmdial.c:702)
==5332== Address 0x4382130 is 24 bytes inside a block of size 88 free'd
==5332== at 0x4024886: free (vg_replace_malloc.c:325)
==5332== by 0x4081195: g_free (gmem.c:190)
==5332== by 0x805557D: g_at_hdlc_unref (gathdlc.c:270)
==5332== by 0x8056316: g_at_ppp_unref (gatppp.c:463)
==5332== by 0x804AF20: ppp_disconnect (gsmdial.c:272)
==5332== by 0x8055CE9: ppp_dead (gatppp.c:204)
==5332== by 0x8055D1E: ppp_enter_phase (gatppp.c:214)
==5332== by 0x8055FE0: ppp_lcp_finished_notify (gatppp.c:317)
==5332== by 0x8057E27: lcp_finished (ppp_lcp.c:140)
==5332== by 0x80568DF: pppcp_this_layer_finished (ppp_cp.c:340)
==5332== by 0x8057446: pppcp_generate_event (ppp_cp.c:688)
==5332== by 0x8057B5B: pppcp_process_packet (ppp_cp.c:965)
Good catch, however I fixed it slightly differently in GAtHDLC itself. Can you
test and make sure my fix works?
Thanks,
-Denis
3:50 a.m.
New subject: [PATCH 05/11] test-server: Fix GIOChannel leak in create_tty
Hi Zhenhua,
Remove client_io and unref server_io after creating GAtServer.
---
gatchat/test-server.c | 6 ++----
1 files changed, 2 insertions(+), 4 deletions(-)
This patch has been applied, thanks.
Regards,
-Denis
3:49 a.m.
New subject: [PATCH 03/11] gatppp: Add PPP server extension
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.
---
Its great that you're working on the PPP Server extensions. Can I get you to
submit a patch taking ownership of this task? See ofono/TODO 'PPP Server
support'.
+static void ipcp_generate_peer_config_options(struct ipcp_data
*ipcp)
+{
+ guint16 len = 0;
+
+ FILL_IP(ipcp->req_options & REQ_OPTION_IPADDR,
+ IP_ADDRESS, &ipcp->peer_addr);
+ FILL_IP(ipcp->req_options & REQ_OPTION_DNS1,
+ PRIMARY_DNS_SERVER, &ipcp->dns1);
+ FILL_IP(ipcp->req_options & REQ_OPTION_DNS2,
+ SECONDARY_DNS_SERVER, &ipcp->dns2);
+ FILL_IP(ipcp->req_options & REQ_OPTION_NBNS1,
+ PRIMARY_NBNS_SERVER, &ipcp->nbns1);
+ FILL_IP(ipcp->req_options & REQ_OPTION_NBNS2,
+ SECONDARY_NBNS_SERVER, &ipcp->nbns2);
+
Using ipcp->req_options is the wrong thing here. That is used for options
sent in a Configure-Req. Here you're filling in options to be sent in a
Configure-Nak, Configure-Reject or a Configure-Ack. You should simply be filling
these in ipcp_rcr. Feel free to modify the FILL_IP macro to take an
additional destination parameter.
+ ipcp->options_len = len;
+}
+
+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);
+
+ ipcp->req_options = REQ_OPTION_IPADDR | REQ_OPTION_DNS1 |
+ REQ_OPTION_DNS2 | REQ_OPTION_NBNS1 |
+ REQ_OPTION_NBNS2;
+
+ ipcp->local_addr = local_addr;
+ ipcp->peer_addr = peer_addr;
+ ipcp->dns1 = dns1;
+ ipcp->dns2 = dns2;
+ ipcp->nbns1 = nbns1;
+ ipcp->nbns2 = nbns2;
+
+ ipcp_generate_config_options(ipcp);
Please note that this function is currently having no effect, you're missing a
call to pppcp_set_local_options. However, in the case of a server, we
actually do not want to request DNS or NBNS, and we should only send our local
address to the peer if it is non-zero. Please note that all PPP modems we
have encountered so far, do not send their local IP address in a Configure-
Request. Thus calling ipcp_set_server_info should end up generating an empty
Configure-Request or with only the local IP address present.
+}
+
static void ipcp_reset_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;
In the case of a server, the local_address should not be reset.
+ ipcp->peer_addr = 0;
ipcp->dns1 = 0;
ipcp->dns2 = 0;
ipcp->nbns1 = 0;
@@ -274,11 +321,54 @@ static enum rcr_result ipcp_rcr(struct pppcp_data
*pppcp, guint8 **new_options, guint16 *new_len)
{
struct ppp_option_iter iter;
+ struct ipcp_data *ipcp = pppcp_get_data(pppcp);
+ guint32 peer_addr;
+ guint32 dns1;
+ guint32 dns2;
ppp_option_iter_init(&iter, packet);
- if (ppp_option_iter_next(&iter) == FALSE)
- return RCR_ACCEPT;
Again, be careful here. In the case of a client we should only accept the
Server's IP-Address, nothing else.
+ 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;
+ default:
+ break;
+ }
+ }
+
+ if (peer_addr && dns1 && dns2) {
+ if (ipcp->peer_addr == 0) {
+ /* RFC 1332 Section 3.3
+ * Accept the value of IP address as peer IP address
+ */
+ ipcp->peer_addr = peer_addr;
+
This part doesn't really make any sense, if peer is sending 0, he's asking us
for an IP.
+ return RCR_ACCEPT;
+ } else if (ipcp->peer_addr == peer_addr)
+ return RCR_ACCEPT;
You need to ensure that not only the peer_addr matches, but also the dns /
nbns addresses. In our case we should be Configure-Rejecting the NBNS options
and Configure-Naking DNS ones if they're set to zero or don't match what the
peer is sending us.
+ } else {
+ /* Peer requests us to send ip address in config options */
+ if (ipcp->peer_addr) {
+ struct ipcp_data *peer;
+
+ peer = g_memdup(ipcp, sizeof(struct ipcp_data));
+ ipcp_generate_peer_config_options(peer);
Yuck, please don't do this. The main PPPCP layer will take care of freeing
new_options, however you're now leaking the rest of the ipcp_data structure.
+
+ *new_len = peer->options_len;
+ *new_options = peer->options;
+ return RCR_NAK;
+ }
+ }
/* Reject all options */
*new_len = packet->length - sizeof(*packet);
Regards,
-Denis
12:38 p.m.
New subject: [PATCH 03/11] gatppp: Add PPP server extension
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.
> ---
Its great that you're working on the PPP Server extensions.
Can I get you to
submit a patch taking ownership of this task? See ofono/TODO 'PPP
Server support'.
> +static void ipcp_generate_peer_config_options(struct ipcp_data
> *ipcp) +{ + guint16 len = 0;
> +
> + FILL_IP(ipcp->req_options & REQ_OPTION_IPADDR,
> + IP_ADDRESS, &ipcp->peer_addr);
> + FILL_IP(ipcp->req_options & REQ_OPTION_DNS1,
> + PRIMARY_DNS_SERVER,
&ipcp->dns1);
> + FILL_IP(ipcp->req_options & REQ_OPTION_DNS2,
> + SECONDARY_DNS_SERVER,
&ipcp->dns2);
> + FILL_IP(ipcp->req_options & REQ_OPTION_NBNS1,
> + PRIMARY_NBNS_SERVER,
&ipcp->nbns1);
> + FILL_IP(ipcp->req_options & REQ_OPTION_NBNS2,
> + SECONDARY_NBNS_SERVER,
&ipcp->nbns2);
> +
Using ipcp->req_options is the wrong thing here. That is used
for options
sent in a Configure-Req. Here you're filling in options to be
sent in a
Configure-Nak, Configure-Reject or a Configure-Ack. You
should simply be filling
these in ipcp_rcr. Feel free to modify the FILL_IP macro to take an
additional destination parameter.
Okay. I will do that.
> + ipcp->options_len = len;
> +}
> +
> +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);
> +
> + ipcp->req_options = REQ_OPTION_IPADDR | REQ_OPTION_DNS1 |
> + REQ_OPTION_DNS2 | REQ_OPTION_NBNS1 |
> + REQ_OPTION_NBNS2;
> +
> + ipcp->local_addr = local_addr;
> + ipcp->peer_addr = peer_addr;
> + ipcp->dns1 = dns1;
> + ipcp->dns2 = dns2;
> + ipcp->nbns1 = nbns1;
> + ipcp->nbns2 = nbns2;
> +
> + ipcp_generate_config_options(ipcp);
Please note that this function is currently having no effect,
you're missing a
call to pppcp_set_local_options. However, in the case of a server, we
actually do not want to request DNS or NBNS, and we should
only send our local
address to the peer if it is non-zero. Please note that all
PPP modems we
have encountered so far, do not send their local IP address in
a Configure-
Request. Thus calling ipcp_set_server_info should end up
generating an empty
Configure-Request or with only the local IP address present.
It's confused me a little. Please correct me if I am wrong. I think PPP modem should
send their local IP address to peer in a Configure-Request. So client could accept the
peer address in ipcp_rcr.
In the case of a server, ipcp_set_server_info() should set local, peer, DNS and NBNS info
and call pppcp_set_local_options(). And server only send our local address to peer in a
Configure-Request.
> +}
> +
> static void ipcp_reset_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;
In the case of a server, the local_address should not be reset.
It seems we need to add a flag in ipcp_data to indicate whether it's server or not.
Does it make sense?
> + ipcp->peer_addr = 0;
> ipcp->dns1 = 0;
> ipcp->dns2 = 0;
> ipcp->nbns1 = 0;
> @@ -274,11 +321,54 @@ static enum rcr_result ipcp_rcr(struct
> pppcp_data *pppcp, guint8 **new_options, guint16 *new_len)
> {
> struct ppp_option_iter iter;
> + struct ipcp_data *ipcp = pppcp_get_data(pppcp);
> + guint32 peer_addr;
> + guint32 dns1;
> + guint32 dns2;
>
> ppp_option_iter_init(&iter, packet);
>
> - if (ppp_option_iter_next(&iter) == FALSE)
> - return RCR_ACCEPT;
Again, be careful here. In the case of a client we should
only accept the
Server's IP-Address, nothing else.
> + 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;
> + default:
> + break;
> + }
> + }
> +
> + if (peer_addr && dns1 && dns2) {
> + if (ipcp->peer_addr == 0) {
> + /* RFC 1332 Section 3.3
> + * Accept the value of IP address as
peer IP address
> + */
> + ipcp->peer_addr = peer_addr;
> +
This part doesn't really make any sense, if peer is sending 0,
he's asking us
for an IP.
For client side, the peer server address is zero initially. So it need to accept the
server address immediately.
> + return RCR_ACCEPT;
> + } else if (ipcp->peer_addr == peer_addr)
> + return RCR_ACCEPT;
You need to ensure that not only the peer_addr matches, but
also the dns /
nbns addresses. In our case we should be Configure-Rejecting
the NBNS options
and Configure-Naking DNS ones if they're set to zero or don't
match what the
peer is sending us.
Agree. Will update that part.
> + } else {
> + /* Peer requests us to send ip address in config options */ + if
> (ipcp->peer_addr) { + struct ipcp_data *peer;
> +
> + peer = g_memdup(ipcp, sizeof(struct ipcp_data));
> + ipcp_generate_peer_config_options(peer);
Yuck, please don't do this. The main PPPCP layer will take
care of freeing
new_options, however you're now leaking the rest of the
ipcp_data structure.
Ok. I will take care about it and resend the patch later.
> +
> + *new_len = peer->options_len;
> + *new_options = peer->options;
> + return RCR_NAK;
> + }
> + }
>
> /* Reject all options */
> *new_len = packet->length - sizeof(*packet);
>
Regards,
-Denis
Regards,
Zhenhua
2:26 p.m.
New subject: [PATCH 03/11] gatppp: Add PPP server extension
Hi Zhenhua,
>> +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);
>> +
>> + ipcp->req_options = REQ_OPTION_IPADDR | REQ_OPTION_DNS1 |
>> + REQ_OPTION_DNS2 | REQ_OPTION_NBNS1 |
>> + REQ_OPTION_NBNS2;
>> +
>> + ipcp->local_addr = local_addr;
>> + ipcp->peer_addr = peer_addr;
>> + ipcp->dns1 = dns1;
>> + ipcp->dns2 = dns2;
>> + ipcp->nbns1 = nbns1;
>> + ipcp->nbns2 = nbns2;
>> +
>> + ipcp_generate_config_options(ipcp);
>
> Please note that this function is currently having no effect,
> you're missing a
> call to pppcp_set_local_options. However, in the case of a server, we
> actually do not want to request DNS or NBNS, and we should
> only send our local
> address to the peer if it is non-zero. Please note that all
> PPP modems we
> have encountered so far, do not send their local IP address in
> a Configure-
> Request. Thus calling ipcp_set_server_info should end up
> generating an empty
> Configure-Request or with only the local IP address present.
It's confused me a little. Please correct me if I am wrong. I think PPP
modem should send their local IP address to peer in a Configure-Request.
So client could accept the peer address in ipcp_rcr.
This is correct if the PPP server wants to do so, it is perfectly OK for the
server not to send its IP address (e.g. if it doesn't have one.) This is what
we observe on most PPP modems.
In the case of a server, ipcp_set_server_info() should set local, peer, DNS
and NBNS info and call pppcp_set_local_options(). And server only send our
local address to peer in a Configure-Request.
I'd modify this slightly. If ipcp_set_server_info is called with server's
local address as 0, then don't even send IP-Address in a configure Request.
This wouldn't make sense, in effect the server is asking the client to provide
its IP address. Otherwise, go ahead and send it (as the client will simply
ack).
>> +}
>> +
>> static void ipcp_reset_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;
>
> In the case of a server, the local_address should not be reset.
It seems we need to add a flag in ipcp_data to indicate whether it's server
or not. Does it make sense?
Might be a good idea.
Regards,
-Denis
4:31 p.m.
New subject: [PATCH 03/11] gatppp: Add PPP server extension
Hi Denis,
Denis Kenzior wrote:
Hi Zhenhua,
>>> +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);
>>> +
>>> + ipcp->req_options = REQ_OPTION_IPADDR | REQ_OPTION_DNS1 |
>>> + REQ_OPTION_DNS2 | REQ_OPTION_NBNS1 |
>>> + REQ_OPTION_NBNS2;
>>> +
>>> + ipcp->local_addr = local_addr;
>>> + ipcp->peer_addr = peer_addr;
>>> + ipcp->dns1 = dns1;
>>> + ipcp->dns2 = dns2;
>>> + ipcp->nbns1 = nbns1;
>>> + ipcp->nbns2 = nbns2;
>>> +
>>> + ipcp_generate_config_options(ipcp);
>>
>> Please note that this function is currently having no effect,
>> you're missing a call to pppcp_set_local_options. However, in the
>> case of a server, we actually do not want to request DNS or NBNS,
>> and we should only send our local address to the peer if it is
>> non-zero. Please note that all PPP modems we have encountered so
>> far, do not send their local IP address in a Configure- Request.
>> Thus calling ipcp_set_server_info should end up generating an empty
>> Configure-Request or with only the local IP address present.
>
> It's confused me a little. Please correct me if I am wrong. I think
> PPP modem should send their local IP address to peer in a
> Configure-Request. So client could accept the peer address in
> ipcp_rcr.
This is correct if the PPP server wants to do so, it is perfectly OK
for the server not to send its IP address (e.g. if it doesn't have
one.) This is what we observe on most PPP modems.
Okay. That's fine. At least, PPP server need to send peer_addr in Conf-Nak reply.
Client 'thinks' it's server address.
>
> In the case of a server, ipcp_set_server_info() should set local,
> peer, DNS and NBNS info and call pppcp_set_local_options(). And
> server only send our local address to peer in a Configure-Request.
I'd modify this slightly. If ipcp_set_server_info is called with
server's local address as 0, then don't even send IP-Address in a
configure Request. This wouldn't make sense, in effect the server is
asking the client to provide its IP address. Otherwise, go ahead and
send it (as the client will simply ack).
No problems. If server local address is zero, we don't send IP in a configure
request.
>
>>> +}
>>> +
>>> static void ipcp_reset_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;
>>
>> In the case of a server, the local_address should not be reset.
>
> It seems we need to add a flag in ipcp_data to indicate whether it's
> server or not. Does it make sense?
Might be a good idea.
Regards,
-Denis
Regards,
Zhenhua
4471
days inactive
4481
days old
21 comments
3 participants
participants (3)
-
Denis Kenzior -
Zhang, Zhenhua -
Zhenhua Zhang