[PATCH_v2 0/2] ACFC and PFC options implementation

[PATCH_v2 0/2] ACFC and PFC options implementation

[Guillaume Zajac]

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

Hi,

Change log from v2 is:
	- make compression options application independant from
	  sending or receiving frames
	- add an option into GAtPPP to disable completely ACFC and
	  PFC

Guillaume Zajac (2):
  GAtPPP: Add ACFC option support
  GAtPPP: Add PFC option support

 gatchat/gatppp.c  |  145 +++++++++++++++++++++++++++++++++++++++++++++++++----
 gatchat/gatppp.h  |    3 +
 gatchat/ppp.h     |   12 ++++
 gatchat/ppp_lcp.c |   49 ++++++++++++++++--
 4 files changed, 195 insertions(+), 14 deletions(-)

[PATCH_v2 1/2] GAtPPP: Add ACFC option support

[Guillaume Zajac]

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

---
 gatchat/gatppp.c  |   79 ++++++++++++++++++++++++++++++++++++++++++++++-------
 gatchat/gatppp.h  |    2 +
 gatchat/ppp.h     |    8 +++++
 gatchat/ppp_lcp.c |   28 ++++++++++++++++---
 4 files changed, 103 insertions(+), 14 deletions(-)

diff --git a/gatchat/gatppp.c b/gatchat/gatppp.c
index 5fb4146..2640985 100644
--- a/gatchat/gatppp.c
+++ b/gatchat/gatppp.c
@@ -83,6 +83,8 @@ struct _GAtPPP {
 	int fd;
 	guint guard_timeout_source;
 	gboolean suspended;
+	gboolean force_acfc_off;
+	gboolean acfc;
 };
 
 void ppp_debug(GAtPPP *ppp, const char *str)
@@ -168,8 +170,19 @@ static inline gboolean ppp_drop_packet(GAtPPP *ppp, guint16 protocol)
 static void ppp_receive(const unsigned char *buf, gsize len, void *data)
 {
 	GAtPPP *ppp = data;
-	guint16 protocol = ppp_proto(buf);
-	const guint8 *packet = ppp_info(buf);
+	struct ppp_header *header = (struct ppp_header *) buf;
+	gboolean acfc_frame = (header->address != PPP_ADDR_FIELD
+			|| header->control != PPP_CTRL);
+	guint16 protocol;
+	const guint8 *packet;
+
+	if (acfc_frame) {
+		protocol = ppp_acfc_proto(buf);
+		packet = ppp_acfc_info(buf);
+	} else {
+		protocol = ppp_proto(buf);
+		packet = ppp_info(buf);
+	}
 
 	if (ppp_drop_packet(ppp, protocol))
 		return;
@@ -196,17 +209,11 @@ static void ppp_receive(const unsigned char *buf, gsize len, void *data)
 	};
 }
 
-/*
- * transmit out through the lower layer interface
- *
- * infolen - length of the information part of the packet
- */
-void ppp_transmit(GAtPPP *ppp, guint8 *packet, guint infolen)
+static void ppp_send_common_frame(GAtPPP *ppp, guint8 *packet, guint infolen,
+					gboolean lcp)
 {
 	struct ppp_header *header = (struct ppp_header *) packet;
-	guint16 proto = ppp_proto(packet);
 	guint8 code;
-	gboolean lcp = (proto == LCP_PROTOCOL);
 	guint32 xmit_accm = 0;
 	gboolean sta = FALSE;
 
@@ -251,6 +258,44 @@ void ppp_transmit(GAtPPP *ppp, guint8 *packet, guint infolen)
 		g_at_hdlc_set_xmit_accm(ppp->hdlc, xmit_accm);
 }
 
+static void ppp_send_acfc_frame(GAtPPP *ppp, guint8 *packet,
+					guint infolen)
+{
+	struct ppp_header *header = (struct ppp_header *) packet;
+	guint offset = 0;
+
+	if (ppp->acfc)
+		offset = 2;
+	else
+		offset = 0;
+
+	if (g_at_hdlc_send(ppp->hdlc, packet + offset,
+				infolen + sizeof(*header) - offset)
+			== FALSE)
+		DBG(ppp, "Failed to send a frame\n");
+}
+
+/*
+ * transmit out through the lower layer interface
+ *
+ * infolen - length of the information part of the packet
+ */
+void ppp_transmit(GAtPPP *ppp, guint8 *packet, guint infolen)
+{
+	guint16 proto = ppp_proto(packet);
+
+	switch (proto) {
+	case LCP_PROTOCOL:
+		ppp_send_common_frame(ppp, packet, infolen, TRUE);
+		break;
+	case CHAP_PROTOCOL:
+	case IPCP_PROTO:
+	case PPP_IP_PROTO:
+		ppp_send_acfc_frame(ppp, packet, infolen);
+		break;
+	}
+}
+
 static inline void ppp_enter_phase(GAtPPP *ppp, enum ppp_phase phase)
 {
 	DBG(ppp, "%d", phase);
@@ -390,6 +435,14 @@ void ppp_set_mtu(GAtPPP *ppp, const guint8 *data)
 	ppp->mtu = mtu;
 }
 
+void ppp_set_acfc_enabled(GAtPPP *ppp, gboolean acfc)
+{
+	if (ppp->force_acfc_off)
+		ppp->acfc = FALSE;
+	else
+		ppp->acfc = acfc;
+}
+
 static void io_disconnect(gpointer user_data)
 {
 	GAtPPP *ppp = user_data;
@@ -658,6 +711,12 @@ void g_at_ppp_set_server_info(GAtPPP *ppp, const char *remote,
 	ipcp_set_server_info(ppp->ipcp, r, d1, d2);
 }
 
+void g_at_ppp_force_acfc_off(GAtPPP *ppp, gboolean off)
+{
+	ppp->force_acfc_off = off;
+	lcp_turn_off_acfc(ppp->lcp);
+}
+
 static GAtPPP *ppp_init_common(gboolean is_server, guint32 ip)
 {
 	GAtPPP *ppp;
diff --git a/gatchat/gatppp.h b/gatchat/gatppp.h
index f0930a7..d0231fc 100644
--- a/gatchat/gatppp.h
+++ b/gatchat/gatppp.h
@@ -79,6 +79,8 @@ void g_at_ppp_set_recording(GAtPPP *ppp, const char *filename);
 void g_at_ppp_set_server_info(GAtPPP *ppp, const char *remote_ip,
 				const char *dns1, const char *dns2);
 
+void g_at_ppp_force_acfc_off(GAtPPP *ppp, gboolean off);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/gatchat/ppp.h b/gatchat/ppp.h
index 023d779..116b5db 100644
--- a/gatchat/ppp.h
+++ b/gatchat/ppp.h
@@ -85,10 +85,17 @@ static inline void __put_unaligned_short(void *p, guint16 val)
 #define ppp_proto(packet) \
 	(get_host_short(packet + 2))
 
+#define ppp_acfc_info(packet) \
+	(packet + 2)
+
+#define ppp_acfc_proto(packet) \
+	(get_host_short(packet))
+
 /* LCP related functions */
 struct pppcp_data *lcp_new(GAtPPP *ppp, gboolean dormant);
 void lcp_free(struct pppcp_data *lcp);
 void lcp_protocol_reject(struct pppcp_data *lcp, guint8 *packet, gsize len);
+void lcp_turn_off_acfc(struct pppcp_data *pppcp);
 
 /* IPCP related functions */
 struct pppcp_data *ipcp_new(GAtPPP *ppp, gboolean is_server, guint32 ip);
@@ -125,4 +132,5 @@ void ppp_lcp_finished_notify(GAtPPP *ppp);
 void ppp_set_recv_accm(GAtPPP *ppp, guint32 accm);
 void ppp_set_xmit_accm(GAtPPP *ppp, guint32 accm);
 void ppp_set_mtu(GAtPPP *ppp, const guint8 *data);
+void ppp_set_acfc_enabled(GAtPPP *ppp, gboolean acfc);
 struct ppp_header *ppp_packet_new(gsize infolen, guint16 protocol);
diff --git a/gatchat/ppp_lcp.c b/gatchat/ppp_lcp.c
index ce9dae2..3814ddf 100644
--- a/gatchat/ppp_lcp.c
+++ b/gatchat/ppp_lcp.c
@@ -58,11 +58,12 @@ enum lcp_options {
 	ACFC			= 8,
 };
 
-/* Maximum size of all options, we only ever request ACCM and MRU */ 
-#define MAX_CONFIG_OPTION_SIZE 10
+/* Maximum size of all options, we only ever request ACCM, MRU and ACFC */
+#define MAX_CONFIG_OPTION_SIZE 12
 
 #define REQ_OPTION_ACCM	0x1
 #define REQ_OPTION_MRU	0x2
+#define REQ_OPTION_ACFC	0x4
 
 struct lcp_data {
 	guint8 options[MAX_CONFIG_OPTION_SIZE];
@@ -100,13 +101,20 @@ static void lcp_generate_config_options(struct lcp_data *lcp)
 		len += 4;
 	}
 
+	if (lcp->req_options & REQ_OPTION_ACFC) {
+		lcp->options[len] = ACFC;
+		lcp->options[len + 1] = 2;
+
+		len += 2;
+	}
+
 	lcp->options_len = len;
 }
 
 static void lcp_reset_config_options(struct lcp_data *lcp)
 {
 	/* Using the default ACCM */
-
+	lcp->req_options |= REQ_OPTION_ACFC;
 	lcp_generate_config_options(lcp);
 }
 
@@ -286,9 +294,11 @@ static enum rcr_result lcp_rcr(struct pppcp_data *pppcp,
 			break;
 		case MAGIC_NUMBER:
 		case PFC:
-		case ACFC:
 			/* don't care */
 			break;
+		case ACFC:
+			ppp_set_acfc_enabled(ppp, TRUE);
+			break;
 		}
 	}
 
@@ -338,3 +348,13 @@ struct pppcp_data *lcp_new(GAtPPP *ppp, gboolean is_server)
 
 	return pppcp;
 }
+
+void lcp_turn_off_acfc(struct pppcp_data *pppcp)
+{
+	struct lcp_data *lcp = pppcp_get_data(pppcp);
+
+	lcp->req_options &= ~REQ_OPTION_ACFC;
+	lcp_generate_config_options(lcp);
+
+	pppcp_set_local_options(pppcp, lcp->options, lcp->options_len);
+}
-- 
1.7.1

[PATCH_v2 2/2] GAtPPP: Add PFC option support

[Guillaume Zajac]

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

---
 gatchat/gatppp.c  |   68 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
 gatchat/gatppp.h  |    1 +
 gatchat/ppp.h     |    4 +++
 gatchat/ppp_lcp.c |   27 ++++++++++++++++++--
 4 files changed, 96 insertions(+), 4 deletions(-)

diff --git a/gatchat/gatppp.c b/gatchat/gatppp.c
index 2640985..2a0159f 100644
--- a/gatchat/gatppp.c
+++ b/gatchat/gatppp.c
@@ -84,7 +84,9 @@ struct _GAtPPP {
 	guint guard_timeout_source;
 	gboolean suspended;
 	gboolean force_acfc_off;
+	gboolean force_pfc_off;
 	gboolean acfc;
+	gboolean pfc;
 };
 
 void ppp_debug(GAtPPP *ppp, const char *str)
@@ -173,6 +175,7 @@ static void ppp_receive(const unsigned char *buf, gsize len, void *data)
 	struct ppp_header *header = (struct ppp_header *) buf;
 	gboolean acfc_frame = (header->address != PPP_ADDR_FIELD
 			|| header->control != PPP_CTRL);
+	gboolean pfc_frame = FALSE;
 	guint16 protocol;
 	const guint8 *packet;
 
@@ -184,6 +187,20 @@ static void ppp_receive(const unsigned char *buf, gsize len, void *data)
 		packet = ppp_info(buf);
 	}
 
+	pfc_frame = (protocol != LCP_PROTOCOL && protocol != CHAP_PROTOCOL &&
+			protocol != IPCP_PROTO && protocol != PPP_IP_PROTO);
+
+	if (pfc_frame) {
+		guint8 proto = (protocol >> 8) & 0xFF ;
+		packet = packet - 1;
+		/*
+		 * The only protocol that can be compressed is PPP_IP_PROTO
+		 * because first byte is 0x00.
+		 */
+		if (proto == PPP_IP_COMPRESSED_PROTO)
+			protocol = PPP_IP_PROTO;
+	}
+
 	if (ppp_drop_packet(ppp, protocol))
 		return;
 
@@ -268,6 +285,32 @@ static void ppp_send_acfc_frame(GAtPPP *ppp, guint8 *packet,
 		offset = 2;
 	else
 		offset = 0;
+	/* We remove the only address and control field */
+	if (g_at_hdlc_send(ppp->hdlc, packet + offset,
+				infolen + sizeof(*header) - offset)
+			== FALSE)
+		DBG(ppp, "Failed to send a frame\n");
+}
+
+static void ppp_send_acfc_pfc_frame(GAtPPP *ppp, guint8 *packet,
+					guint infolen)
+{
+	struct ppp_header *header = (struct ppp_header *) packet;
+	guint offset = 0;
+
+	if (ppp->acfc && ppp->pfc)
+		offset = 3;
+	else if (ppp->acfc)
+		offset = 2;
+	else if (ppp->pfc) {
+		/*
+		 * We remove only the 1st byte that is 0x00 of protocol field.
+		 */
+		packet[2] = packet[1];
+		packet[1] = packet[0];
+		offset = 1;
+	} else
+		offset = 0;
 
 	if (g_at_hdlc_send(ppp->hdlc, packet + offset,
 				infolen + sizeof(*header) - offset)
@@ -286,13 +329,22 @@ void ppp_transmit(GAtPPP *ppp, guint8 *packet, guint infolen)
 
 	switch (proto) {
 	case LCP_PROTOCOL:
+		/* Don't use any compression for LCP packets */
 		ppp_send_common_frame(ppp, packet, infolen, TRUE);
 		break;
 	case CHAP_PROTOCOL:
 	case IPCP_PROTO:
-	case PPP_IP_PROTO:
+		/*
+		 * We can't use PFC option because first byte of CHAP_PROTOCOL
+		 * and IPCP_PROTO is not equal to 0x00
+		 */
 		ppp_send_acfc_frame(ppp, packet, infolen);
 		break;
+	case PPP_IP_PROTO:
+		/*
+		 * We can't use both compression options if they are negotiated
+		 */
+		ppp_send_acfc_pfc_frame(ppp, packet, infolen);
 	}
 }
 
@@ -443,6 +495,14 @@ void ppp_set_acfc_enabled(GAtPPP *ppp, gboolean acfc)
 		ppp->acfc = acfc;
 }
 
+void ppp_set_pfc_enabled(GAtPPP *ppp, gboolean pfc)
+{
+	if (ppp->force_pfc_off)
+		ppp->pfc = FALSE;
+	else
+		ppp->pfc = pfc;
+}
+
 static void io_disconnect(gpointer user_data)
 {
 	GAtPPP *ppp = user_data;
@@ -717,6 +777,12 @@ void g_at_ppp_force_acfc_off(GAtPPP *ppp, gboolean off)
 	lcp_turn_off_acfc(ppp->lcp);
 }
 
+void g_at_ppp_force_pfc_off(GAtPPP *ppp, gboolean off)
+{
+	ppp->force_acfc_off = off;
+	lcp_turn_off_pfc(ppp->lcp);
+}
+
 static GAtPPP *ppp_init_common(gboolean is_server, guint32 ip)
 {
 	GAtPPP *ppp;
diff --git a/gatchat/gatppp.h b/gatchat/gatppp.h
index d0231fc..b7933d3 100644
--- a/gatchat/gatppp.h
+++ b/gatchat/gatppp.h
@@ -80,6 +80,7 @@ void g_at_ppp_set_server_info(GAtPPP *ppp, const char *remote_ip,
 				const char *dns1, const char *dns2);
 
 void g_at_ppp_force_acfc_off(GAtPPP *ppp, gboolean off);
+void g_at_ppp_force_pfc_off(GAtPPP *ppp, gboolean off);
 
 #ifdef __cplusplus
 }
diff --git a/gatchat/ppp.h b/gatchat/ppp.h
index 116b5db..3a0b2bf 100644
--- a/gatchat/ppp.h
+++ b/gatchat/ppp.h
@@ -27,6 +27,8 @@
 #define PPP_IP_PROTO	0x0021
 #define MD5		5
 
+#define PPP_IP_COMPRESSED_PROTO 0x21
+
 #define DBG(p, fmt, arg...) do {				\
 	char *str = g_strdup_printf("%s:%s() " fmt, __FILE__,	\
 					__FUNCTION__ , ## arg); \
@@ -96,6 +98,7 @@ struct pppcp_data *lcp_new(GAtPPP *ppp, gboolean dormant);
 void lcp_free(struct pppcp_data *lcp);
 void lcp_protocol_reject(struct pppcp_data *lcp, guint8 *packet, gsize len);
 void lcp_turn_off_acfc(struct pppcp_data *pppcp);
+void lcp_turn_off_pfc(struct pppcp_data *pppcp);
 
 /* IPCP related functions */
 struct pppcp_data *ipcp_new(GAtPPP *ppp, gboolean is_server, guint32 ip);
@@ -133,4 +136,5 @@ void ppp_set_recv_accm(GAtPPP *ppp, guint32 accm);
 void ppp_set_xmit_accm(GAtPPP *ppp, guint32 accm);
 void ppp_set_mtu(GAtPPP *ppp, const guint8 *data);
 void ppp_set_acfc_enabled(GAtPPP *ppp, gboolean acfc);
+void ppp_set_pfc_enabled(GAtPPP *ppp, gboolean acfc);
 struct ppp_header *ppp_packet_new(gsize infolen, guint16 protocol);
diff --git a/gatchat/ppp_lcp.c b/gatchat/ppp_lcp.c
index 3814ddf..18dfea4 100644
--- a/gatchat/ppp_lcp.c
+++ b/gatchat/ppp_lcp.c
@@ -58,12 +58,13 @@ enum lcp_options {
 	ACFC			= 8,
 };
 
-/* Maximum size of all options, we only ever request ACCM, MRU and ACFC */
-#define MAX_CONFIG_OPTION_SIZE 12
+/* Maximum size of all options, we only ever request ACCM, MRU, ACFC and PFC */
+#define MAX_CONFIG_OPTION_SIZE 14
 
 #define REQ_OPTION_ACCM	0x1
 #define REQ_OPTION_MRU	0x2
 #define REQ_OPTION_ACFC	0x4
+#define REQ_OPTION_PFC	0x8
 
 struct lcp_data {
 	guint8 options[MAX_CONFIG_OPTION_SIZE];
@@ -108,6 +109,13 @@ static void lcp_generate_config_options(struct lcp_data *lcp)
 		len += 2;
 	}
 
+	if (lcp->req_options & REQ_OPTION_PFC) {
+		lcp->options[len] = PFC;
+		lcp->options[len + 1] = 2;
+
+		len += 2;
+	}
+
 	lcp->options_len = len;
 }
 
@@ -115,6 +123,7 @@ static void lcp_reset_config_options(struct lcp_data *lcp)
 {
 	/* Using the default ACCM */
 	lcp->req_options |= REQ_OPTION_ACFC;
+	lcp->req_options |= REQ_OPTION_PFC;
 	lcp_generate_config_options(lcp);
 }
 
@@ -293,9 +302,11 @@ static enum rcr_result lcp_rcr(struct pppcp_data *pppcp,
 			ppp_set_mtu(ppp, ppp_option_iter_get_data(&iter));
 			break;
 		case MAGIC_NUMBER:
-		case PFC:
 			/* don't care */
 			break;
+		case PFC:
+			ppp_set_pfc_enabled(ppp, TRUE);
+			break;
 		case ACFC:
 			ppp_set_acfc_enabled(ppp, TRUE);
 			break;
@@ -358,3 +369,13 @@ void lcp_turn_off_acfc(struct pppcp_data *pppcp)
 
 	pppcp_set_local_options(pppcp, lcp->options, lcp->options_len);
 }
+
+void lcp_turn_off_pfc(struct pppcp_data *pppcp)
+{
+	struct lcp_data *lcp = pppcp_get_data(pppcp);
+
+	lcp->req_options &= ~REQ_OPTION_PFC;
+	lcp_generate_config_options(lcp);
+
+	pppcp_set_local_options(pppcp, lcp->options, lcp->options_len);
+}
-- 
1.7.1

Re: [PATCH_v2 1/2] GAtPPP: Add ACFC option support

[Denis Kenzior]

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

Hi Guillaume,

On 06/27/2011 11:15 AM, Guillaume Zajac wrote:
> ---
>  gatchat/gatppp.c  |   79 ++++++++++++++++++++++++++++++++++++++++++++++-------
>  gatchat/gatppp.h  |    2 +
>  gatchat/ppp.h     |    8 +++++
>  gatchat/ppp_lcp.c |   28 ++++++++++++++++---
>  4 files changed, 103 insertions(+), 14 deletions(-)
> 
> diff --git a/gatchat/gatppp.c b/gatchat/gatppp.c
> index 5fb4146..2640985 100644
> --- a/gatchat/gatppp.c
> +++ b/gatchat/gatppp.c
> @@ -83,6 +83,8 @@ struct _GAtPPP {
>  	int fd;
>  	guint guard_timeout_source;
>  	gboolean suspended;
> +	gboolean force_acfc_off;

You don't need this variable, see below for more

> +	gboolean acfc;

Please name this xmit_acfc.

>  };
>  
>  void ppp_debug(GAtPPP *ppp, const char *str)
> @@ -168,8 +170,19 @@ static inline gboolean ppp_drop_packet(GAtPPP *ppp, guint16 protocol)
>  static void ppp_receive(const unsigned char *buf, gsize len, void *data)
>  {
>  	GAtPPP *ppp = data;
> -	guint16 protocol = ppp_proto(buf);
> -	const guint8 *packet = ppp_info(buf);
> +	struct ppp_header *header = (struct ppp_header *) buf;
> +	gboolean acfc_frame = (header->address != PPP_ADDR_FIELD
> +			|| header->control != PPP_CTRL);
> +	guint16 protocol;
> +	const guint8 *packet;
> +
> +	if (acfc_frame) {
> +		protocol = ppp_acfc_proto(buf);
> +		packet = ppp_acfc_info(buf);
> +	} else {
> +		protocol = ppp_proto(buf);
> +		packet = ppp_info(buf);
> +	}
>  
>  	if (ppp_drop_packet(ppp, protocol))
>  		return;
> @@ -196,17 +209,11 @@ static void ppp_receive(const unsigned char *buf, gsize len, void *data)
>  	};
>  }
>  
> -/*
> - * transmit out through the lower layer interface
> - *
> - * infolen - length of the information part of the packet
> - */
> -void ppp_transmit(GAtPPP *ppp, guint8 *packet, guint infolen)
> +static void ppp_send_common_frame(GAtPPP *ppp, guint8 *packet, guint infolen,
> +					gboolean lcp)

Shouldn't this be called send_lcp_frame?

>  {
>  	struct ppp_header *header = (struct ppp_header *) packet;
> -	guint16 proto = ppp_proto(packet);
>  	guint8 code;
> -	gboolean lcp = (proto == LCP_PROTOCOL);
>  	guint32 xmit_accm = 0;
>  	gboolean sta = FALSE;
>  
> @@ -251,6 +258,44 @@ void ppp_transmit(GAtPPP *ppp, guint8 *packet, guint infolen)
>  		g_at_hdlc_set_xmit_accm(ppp->hdlc, xmit_accm);
>  }
>  
> +static void ppp_send_acfc_frame(GAtPPP *ppp, guint8 *packet,
> +					guint infolen)
> +{
> +	struct ppp_header *header = (struct ppp_header *) packet;
> +	guint offset = 0;
> +
> +	if (ppp->acfc)
> +		offset = 2;
> +	else
> +		offset = 0;
> +
> +	if (g_at_hdlc_send(ppp->hdlc, packet + offset,
> +				infolen + sizeof(*header) - offset)
> +			== FALSE)
> +		DBG(ppp, "Failed to send a frame\n");
> +}
> +
> +/*
> + * transmit out through the lower layer interface
> + *
> + * infolen - length of the information part of the packet
> + */
> +void ppp_transmit(GAtPPP *ppp, guint8 *packet, guint infolen)
> +{
> +	guint16 proto = ppp_proto(packet);
> +
> +	switch (proto) {
> +	case LCP_PROTOCOL:
> +		ppp_send_common_frame(ppp, packet, infolen, TRUE);
> +		break;
> +	case CHAP_PROTOCOL:
> +	case IPCP_PROTO:
> +	case PPP_IP_PROTO:
> +		ppp_send_acfc_frame(ppp, packet, infolen);
> +		break;
> +	}
> +}
> +
>  static inline void ppp_enter_phase(GAtPPP *ppp, enum ppp_phase phase)
>  {
>  	DBG(ppp, "%d", phase);
> @@ -390,6 +435,14 @@ void ppp_set_mtu(GAtPPP *ppp, const guint8 *data)
>  	ppp->mtu = mtu;
>  }
>  
> +void ppp_set_acfc_enabled(GAtPPP *ppp, gboolean acfc)

Please name this function ppp_set_xmit_acfc(GAtPPP *ppp, gboolean acfc)

> +{
> +	if (ppp->force_acfc_off)
> +		ppp->acfc = FALSE;
> +	else
> +		ppp->acfc = acfc;

And set ppp->xmit_acfc directly from acfc...

> +}
> +
>  static void io_disconnect(gpointer user_data)
>  {
>  	GAtPPP *ppp = user_data;
> @@ -658,6 +711,12 @@ void g_at_ppp_set_server_info(GAtPPP *ppp, const char *remote,
>  	ipcp_set_server_info(ppp->ipcp, r, d1, d2);
>  }
>  
> +void g_at_ppp_force_acfc_off(GAtPPP *ppp, gboolean off)

Please name this function g_at_ppp_set_acfc_enabled(GAtPPP *ppp,
gboolean enabled)

> +{
> +	ppp->force_acfc_off = off;
> +	lcp_turn_off_acfc(ppp->lcp);

To help in regression testing lets turn off ACFC by default, and only
enable it if someone has asked for it.

> +}
> +
>  static GAtPPP *ppp_init_common(gboolean is_server, guint32 ip)
>  {
>  	GAtPPP *ppp;
> diff --git a/gatchat/gatppp.h b/gatchat/gatppp.h
> index f0930a7..d0231fc 100644
> --- a/gatchat/gatppp.h
> +++ b/gatchat/gatppp.h
> @@ -79,6 +79,8 @@ void g_at_ppp_set_recording(GAtPPP *ppp, const char *filename);
>  void g_at_ppp_set_server_info(GAtPPP *ppp, const char *remote_ip,
>  				const char *dns1, const char *dns2);
>  
> +void g_at_ppp_force_acfc_off(GAtPPP *ppp, gboolean off);
> +
>  #ifdef __cplusplus
>  }
>  #endif
> diff --git a/gatchat/ppp.h b/gatchat/ppp.h
> index 023d779..116b5db 100644
> --- a/gatchat/ppp.h
> +++ b/gatchat/ppp.h
> @@ -85,10 +85,17 @@ static inline void __put_unaligned_short(void *p, guint16 val)
>  #define ppp_proto(packet) \
>  	(get_host_short(packet + 2))
>  
> +#define ppp_acfc_info(packet) \
> +	(packet + 2)
> +
> +#define ppp_acfc_proto(packet) \
> +	(get_host_short(packet))
> +
>  /* LCP related functions */
>  struct pppcp_data *lcp_new(GAtPPP *ppp, gboolean dormant);
>  void lcp_free(struct pppcp_data *lcp);
>  void lcp_protocol_reject(struct pppcp_data *lcp, guint8 *packet, gsize len);
> +void lcp_turn_off_acfc(struct pppcp_data *pppcp);
>  
>  /* IPCP related functions */
>  struct pppcp_data *ipcp_new(GAtPPP *ppp, gboolean is_server, guint32 ip);
> @@ -125,4 +132,5 @@ void ppp_lcp_finished_notify(GAtPPP *ppp);
>  void ppp_set_recv_accm(GAtPPP *ppp, guint32 accm);
>  void ppp_set_xmit_accm(GAtPPP *ppp, guint32 accm);
>  void ppp_set_mtu(GAtPPP *ppp, const guint8 *data);
> +void ppp_set_acfc_enabled(GAtPPP *ppp, gboolean acfc);
>  struct ppp_header *ppp_packet_new(gsize infolen, guint16 protocol);
> diff --git a/gatchat/ppp_lcp.c b/gatchat/ppp_lcp.c
> index ce9dae2..3814ddf 100644
> --- a/gatchat/ppp_lcp.c
> +++ b/gatchat/ppp_lcp.c
> @@ -58,11 +58,12 @@ enum lcp_options {
>  	ACFC			= 8,
>  };
>  
> -/* Maximum size of all options, we only ever request ACCM and MRU */ 
> -#define MAX_CONFIG_OPTION_SIZE 10
> +/* Maximum size of all options, we only ever request ACCM, MRU and ACFC */
> +#define MAX_CONFIG_OPTION_SIZE 12
>  
>  #define REQ_OPTION_ACCM	0x1
>  #define REQ_OPTION_MRU	0x2
> +#define REQ_OPTION_ACFC	0x4
>  
>  struct lcp_data {
>  	guint8 options[MAX_CONFIG_OPTION_SIZE];
> @@ -100,13 +101,20 @@ static void lcp_generate_config_options(struct lcp_data *lcp)
>  		len += 4;
>  	}
>  
> +	if (lcp->req_options & REQ_OPTION_ACFC) {
> +		lcp->options[len] = ACFC;
> +		lcp->options[len + 1] = 2;
> +
> +		len += 2;
> +	}
> +
>  	lcp->options_len = len;
>  }
>  
>  static void lcp_reset_config_options(struct lcp_data *lcp)
>  {
>  	/* Using the default ACCM */
> -
> +	lcp->req_options |= REQ_OPTION_ACFC;
>  	lcp_generate_config_options(lcp);
>  }
>  
> @@ -286,9 +294,11 @@ static enum rcr_result lcp_rcr(struct pppcp_data *pppcp,
>  			break;
>  		case MAGIC_NUMBER:
>  		case PFC:
> -		case ACFC:
>  			/* don't care */
>  			break;
> +		case ACFC:
> +			ppp_set_acfc_enabled(ppp, TRUE);

You can check whether we turned on ACFC by checking whether ACFC option
is being advertised to the peer.  E.g. lcp->req_options & REQ_OPTION_ACFC...

> +			break;
>  		}
>  	}
>  
> @@ -338,3 +348,13 @@ struct pppcp_data *lcp_new(GAtPPP *ppp, gboolean is_server)
>  
>  	return pppcp;
>  }
> +
> +void lcp_turn_off_acfc(struct pppcp_data *pppcp)
> +{
> +	struct lcp_data *lcp = pppcp_get_data(pppcp);
> +
> +	lcp->req_options &= ~REQ_OPTION_ACFC;
> +	lcp_generate_config_options(lcp);
> +
> +	pppcp_set_local_options(pppcp, lcp->options, lcp->options_len);
> +}

Regards,
-Denis

Re: [PATCH_v2 1/2] GAtPPP: Add ACFC option support

[Guillaume Zajac]

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



On 27/06/2011 18:31, Denis Kenzior wrote:
> Hi Guillaume,
>
> On 06/27/2011 11:15 AM, Guillaume Zajac wrote:
>> ---
>>   gatchat/gatppp.c  |   79 ++++++++++++++++++++++++++++++++++++++++++++++-------
>>   gatchat/gatppp.h  |    2 +
>>   gatchat/ppp.h     |    8 +++++
>>   gatchat/ppp_lcp.c |   28 ++++++++++++++++---
>>   4 files changed, 103 insertions(+), 14 deletions(-)
>>
>> diff --git a/gatchat/gatppp.c b/gatchat/gatppp.c
>> index 5fb4146..2640985 100644
>> --- a/gatchat/gatppp.c
>> +++ b/gatchat/gatppp.c
>> @@ -83,6 +83,8 @@ struct _GAtPPP {
>>   	int fd;
>>   	guint guard_timeout_source;
>>   	gboolean suspended;
>> +	gboolean force_acfc_off;
> You don't need this variable, see below for more
>
>> +	gboolean acfc;
> Please name this xmit_acfc.
>
>>   };
>>
>>   void ppp_debug(GAtPPP *ppp, const char *str)
>> @@ -168,8 +170,19 @@ static inline gboolean ppp_drop_packet(GAtPPP *ppp, guint16 protocol)
>>   static void ppp_receive(const unsigned char *buf, gsize len, void *data)
>>   {
>>   	GAtPPP *ppp = data;
>> -	guint16 protocol = ppp_proto(buf);
>> -	const guint8 *packet = ppp_info(buf);
>> +	struct ppp_header *header = (struct ppp_header *) buf;
>> +	gboolean acfc_frame = (header->address != PPP_ADDR_FIELD
>> +			|| header->control != PPP_CTRL);
>> +	guint16 protocol;
>> +	const guint8 *packet;
>> +
>> +	if (acfc_frame) {
>> +		protocol = ppp_acfc_proto(buf);
>> +		packet = ppp_acfc_info(buf);
>> +	} else {
>> +		protocol = ppp_proto(buf);
>> +		packet = ppp_info(buf);
>> +	}
>>
>>   	if (ppp_drop_packet(ppp, protocol))
>>   		return;
>> @@ -196,17 +209,11 @@ static void ppp_receive(const unsigned char *buf, gsize len, void *data)
>>   	};
>>   }
>>
>> -/*
>> - * transmit out through the lower layer interface
>> - *
>> - * infolen - length of the information part of the packet
>> - */
>> -void ppp_transmit(GAtPPP *ppp, guint8 *packet, guint infolen)
>> +static void ppp_send_common_frame(GAtPPP *ppp, guint8 *packet, guint infolen,
>> +					gboolean lcp)
> Shouldn't this be called send_lcp_frame?

Yes sorry I had an issue with git reflog. I will fix it in next version.

>>   {
>>   	struct ppp_header *header = (struct ppp_header *) packet;
>> -	guint16 proto = ppp_proto(packet);
>>   	guint8 code;
>> -	gboolean lcp = (proto == LCP_PROTOCOL);
>>   	guint32 xmit_accm = 0;
>>   	gboolean sta = FALSE;
>>
>> @@ -251,6 +258,44 @@ void ppp_transmit(GAtPPP *ppp, guint8 *packet, guint infolen)
>>   		g_at_hdlc_set_xmit_accm(ppp->hdlc, xmit_accm);
>>   }
>>
>> +static void ppp_send_acfc_frame(GAtPPP *ppp, guint8 *packet,
>> +					guint infolen)
>> +{
>> +	struct ppp_header *header = (struct ppp_header *) packet;
>> +	guint offset = 0;
>> +
>> +	if (ppp->acfc)
>> +		offset = 2;
>> +	else
>> +		offset = 0;
>> +
>> +	if (g_at_hdlc_send(ppp->hdlc, packet + offset,
>> +				infolen + sizeof(*header) - offset)
>> +			== FALSE)
>> +		DBG(ppp, "Failed to send a frame\n");
>> +}
>> +
>> +/*
>> + * transmit out through the lower layer interface
>> + *
>> + * infolen - length of the information part of the packet
>> + */
>> +void ppp_transmit(GAtPPP *ppp, guint8 *packet, guint infolen)
>> +{
>> +	guint16 proto = ppp_proto(packet);
>> +
>> +	switch (proto) {
>> +	case LCP_PROTOCOL:
>> +		ppp_send_common_frame(ppp, packet, infolen, TRUE);
>> +		break;
>> +	case CHAP_PROTOCOL:
>> +	case IPCP_PROTO:
>> +	case PPP_IP_PROTO:
>> +		ppp_send_acfc_frame(ppp, packet, infolen);
>> +		break;
>> +	}
>> +}
>> +
>>   static inline void ppp_enter_phase(GAtPPP *ppp, enum ppp_phase phase)
>>   {
>>   	DBG(ppp, "%d", phase);
>> @@ -390,6 +435,14 @@ void ppp_set_mtu(GAtPPP *ppp, const guint8 *data)
>>   	ppp->mtu = mtu;
>>   }
>>
>> +void ppp_set_acfc_enabled(GAtPPP *ppp, gboolean acfc)
> Please name this function ppp_set_xmit_acfc(GAtPPP *ppp, gboolean acfc)
>
>> +{
>> +	if (ppp->force_acfc_off)
>> +		ppp->acfc = FALSE;
>> +	else
>> +		ppp->acfc = acfc;
> And set ppp->xmit_acfc directly from acfc...
>
>> +}
>> +
>>   static void io_disconnect(gpointer user_data)
>>   {
>>   	GAtPPP *ppp = user_data;
>> @@ -658,6 +711,12 @@ void g_at_ppp_set_server_info(GAtPPP *ppp, const char *remote,
>>   	ipcp_set_server_info(ppp->ipcp, r, d1, d2);
>>   }
>>
>> +void g_at_ppp_force_acfc_off(GAtPPP *ppp, gboolean off)
> Please name this function g_at_ppp_set_acfc_enabled(GAtPPP *ppp,
> gboolean enabled)
>
>> +{
>> +	ppp->force_acfc_off = off;
>> +	lcp_turn_off_acfc(ppp->lcp);
> To help in regression testing lets turn off ACFC by default, and only
> enable it if someone has asked for it.
>
>> +}
>> +
>>   static GAtPPP *ppp_init_common(gboolean is_server, guint32 ip)
>>   {
>>   	GAtPPP *ppp;
>> diff --git a/gatchat/gatppp.h b/gatchat/gatppp.h
>> index f0930a7..d0231fc 100644
>> --- a/gatchat/gatppp.h
>> +++ b/gatchat/gatppp.h
>> @@ -79,6 +79,8 @@ void g_at_ppp_set_recording(GAtPPP *ppp, const char *filename);
>>   void g_at_ppp_set_server_info(GAtPPP *ppp, const char *remote_ip,
>>   				const char *dns1, const char *dns2);
>>
>> +void g_at_ppp_force_acfc_off(GAtPPP *ppp, gboolean off);
>> +
>>   #ifdef __cplusplus
>>   }
>>   #endif
>> diff --git a/gatchat/ppp.h b/gatchat/ppp.h
>> index 023d779..116b5db 100644
>> --- a/gatchat/ppp.h
>> +++ b/gatchat/ppp.h
>> @@ -85,10 +85,17 @@ static inline void __put_unaligned_short(void *p, guint16 val)
>>   #define ppp_proto(packet) \
>>   	(get_host_short(packet + 2))
>>
>> +#define ppp_acfc_info(packet) \
>> +	(packet + 2)
>> +
>> +#define ppp_acfc_proto(packet) \
>> +	(get_host_short(packet))
>> +
>>   /* LCP related functions */
>>   struct pppcp_data *lcp_new(GAtPPP *ppp, gboolean dormant);
>>   void lcp_free(struct pppcp_data *lcp);
>>   void lcp_protocol_reject(struct pppcp_data *lcp, guint8 *packet, gsize len);
>> +void lcp_turn_off_acfc(struct pppcp_data *pppcp);
>>
>>   /* IPCP related functions */
>>   struct pppcp_data *ipcp_new(GAtPPP *ppp, gboolean is_server, guint32 ip);
>> @@ -125,4 +132,5 @@ void ppp_lcp_finished_notify(GAtPPP *ppp);
>>   void ppp_set_recv_accm(GAtPPP *ppp, guint32 accm);
>>   void ppp_set_xmit_accm(GAtPPP *ppp, guint32 accm);
>>   void ppp_set_mtu(GAtPPP *ppp, const guint8 *data);
>> +void ppp_set_acfc_enabled(GAtPPP *ppp, gboolean acfc);
>>   struct ppp_header *ppp_packet_new(gsize infolen, guint16 protocol);
>> diff --git a/gatchat/ppp_lcp.c b/gatchat/ppp_lcp.c
>> index ce9dae2..3814ddf 100644
>> --- a/gatchat/ppp_lcp.c
>> +++ b/gatchat/ppp_lcp.c
>> @@ -58,11 +58,12 @@ enum lcp_options {
>>   	ACFC			= 8,
>>   };
>>
>> -/* Maximum size of all options, we only ever request ACCM and MRU */
>> -#define MAX_CONFIG_OPTION_SIZE 10
>> +/* Maximum size of all options, we only ever request ACCM, MRU and ACFC */
>> +#define MAX_CONFIG_OPTION_SIZE 12
>>
>>   #define REQ_OPTION_ACCM	0x1
>>   #define REQ_OPTION_MRU	0x2
>> +#define REQ_OPTION_ACFC	0x4
>>
>>   struct lcp_data {
>>   	guint8 options[MAX_CONFIG_OPTION_SIZE];
>> @@ -100,13 +101,20 @@ static void lcp_generate_config_options(struct lcp_data *lcp)
>>   		len += 4;
>>   	}
>>
>> +	if (lcp->req_options&  REQ_OPTION_ACFC) {
>> +		lcp->options[len] = ACFC;
>> +		lcp->options[len + 1] = 2;
>> +
>> +		len += 2;
>> +	}
>> +
>>   	lcp->options_len = len;
>>   }
>>
>>   static void lcp_reset_config_options(struct lcp_data *lcp)
>>   {
>>   	/* Using the default ACCM */
>> -
>> +	lcp->req_options |= REQ_OPTION_ACFC;
>>   	lcp_generate_config_options(lcp);
>>   }
>>
>> @@ -286,9 +294,11 @@ static enum rcr_result lcp_rcr(struct pppcp_data *pppcp,
>>   			break;
>>   		case MAGIC_NUMBER:
>>   		case PFC:
>> -		case ACFC:
>>   			/* don't care */
>>   			break;
>> +		case ACFC:
>> +			ppp_set_acfc_enabled(ppp, TRUE);
> You can check whether we turned on ACFC by checking whether ACFC option
> is being advertised to the peer.  E.g. lcp->req_options&  REQ_OPTION_ACFC...

Ok I will do it this way.
Then I will add ACFC and PFC options activation into emulator and gsmdial.

Kind regards,
Guillaume