All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] PPP: Fix transmit ACCM and receive ACCM setting issue
@ 2011-01-20  5:45 martin.xu
  2011-01-20  5:45 ` [PATCH 2/3] Use default ACCM (0xffffffff) to trasmit package martin.xu
  2011-01-20 19:08 ` [PATCH 1/3] PPP: Fix transmit ACCM and receive ACCM setting issue Denis Kenzior
  0 siblings, 2 replies; 10+ messages in thread
From: martin.xu @ 2011-01-20  5:45 UTC (permalink / raw)
  To: ofono

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

From: Martin Xu <martin.xu@intel.com>

Tramsmit ACCM and receive ACCM is mixed up.
According to RFC1662 Section 7.1, ACCM Configuration Option is
used to inform the peer which control characters MUST remain
mapped when the peer sends them.
---
 gatchat/ppp_lcp.c |   16 ++++++++++++++--
 1 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/gatchat/ppp_lcp.c b/gatchat/ppp_lcp.c
index 3a80a62..cc3e231 100644
--- a/gatchat/ppp_lcp.c
+++ b/gatchat/ppp_lcp.c
@@ -149,7 +149,13 @@ static void lcp_rca(struct pppcp_data *pppcp, const struct pppcp_packet *packet)
 	while (ppp_option_iter_next(&iter) == TRUE) {
 		switch (ppp_option_iter_get_type(&iter)) {
 		case ACCM:
-			ppp_set_xmit_accm(pppcp_get_ppp(pppcp), 0);
+			/*
+			 * RFC1662 Section 7.1
+			 * The Configuration Option is used to inform the peer
+			 * which control characters MUST remain mapped when
+			 * the peer sends them.
+			 */
+			ppp_set_recv_accm(pppcp_get_ppp(pppcp), 0);
 			break;
 		default:
 			break;
@@ -263,7 +269,13 @@ static enum rcr_result lcp_rcr(struct pppcp_data *pppcp,
 	while (ppp_option_iter_next(&iter) == TRUE) {
 		switch (ppp_option_iter_get_type(&iter)) {
 		case ACCM:
-			ppp_set_recv_accm(ppp,
+			/*
+			 * RFC1662 Section 7.1
+			 * The Configuration Option is used to inform the peer
+			 * which control characters MUST remain mapped when
+			 * the peer sends them.
+			 */
+			ppp_set_xmit_accm(ppp,
 				get_host_long(ppp_option_iter_get_data(&iter)));
 			break;
 		case AUTH_PROTO:
-- 
1.6.1.3


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

* [PATCH 2/3] Use default ACCM (0xffffffff) to trasmit package
  2011-01-20  5:45 [PATCH 1/3] PPP: Fix transmit ACCM and receive ACCM setting issue martin.xu
@ 2011-01-20  5:45 ` martin.xu
  2011-01-20  5:45   ` [PATCH 3/3] Huawei: set Huawei EM770W modem device to 00 martin.xu
  2011-01-20 19:10   ` [PATCH 2/3] Use default ACCM (0xffffffff) to trasmit package Denis Kenzior
  2011-01-20 19:08 ` [PATCH 1/3] PPP: Fix transmit ACCM and receive ACCM setting issue Denis Kenzior
  1 sibling, 2 replies; 10+ messages in thread
From: martin.xu @ 2011-01-20  5:45 UTC (permalink / raw)
  To: ofono

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

From: Martin Xu <martin.xu@intel.com>

Using my Huawei EM770W modem, if set ACCM as 0x00000000, RXJ-
event breaks PPP link, after IP package transmit for a while.
Using default ACCM, the issue can be fixed.
I tested it at China Unicom networks.
---
 gatchat/ppp_lcp.c |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/gatchat/ppp_lcp.c b/gatchat/ppp_lcp.c
index cc3e231..2264731 100644
--- a/gatchat/ppp_lcp.c
+++ b/gatchat/ppp_lcp.c
@@ -105,8 +105,7 @@ static void lcp_generate_config_options(struct lcp_data *lcp)
 
 static void lcp_reset_config_options(struct lcp_data *lcp)
 {
-	lcp->req_options = REQ_OPTION_ACCM;
-	lcp->accm = 0;
+	/* Using the default ACCM */
 
 	lcp_generate_config_options(lcp);
 }
@@ -155,7 +154,11 @@ static void lcp_rca(struct pppcp_data *pppcp, const struct pppcp_packet *packet)
 			 * which control characters MUST remain mapped when
 			 * the peer sends them.
 			 */
-			ppp_set_recv_accm(pppcp_get_ppp(pppcp), 0);
+
+			/*
+			 * Using the default ACCM
+			 * ppp_set_recv_accm(pppcp_get_ppp(pppcp), ~0U);
+			 */
 			break;
 		default:
 			break;
-- 
1.6.1.3


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

* [PATCH 3/3] Huawei: set Huawei EM770W modem device to 00
  2011-01-20  5:45 ` [PATCH 2/3] Use default ACCM (0xffffffff) to trasmit package martin.xu
@ 2011-01-20  5:45   ` martin.xu
  2011-01-20 18:22     ` Lucas De Marchi
  2011-01-20 19:10   ` [PATCH 2/3] Use default ACCM (0xffffffff) to trasmit package Denis Kenzior
  1 sibling, 1 reply; 10+ messages in thread
From: martin.xu @ 2011-01-20  5:45 UTC (permalink / raw)
  To: ofono

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

From: Martin Xu <martin.xu@intel.com>

Device 00 is ppp port. Setting it as 01, my Huawei EM770W modem PPP
connection can't work
---
 plugins/ofono.rules |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/plugins/ofono.rules b/plugins/ofono.rules
index 111f071..7dec243 100644
--- a/plugins/ofono.rules
+++ b/plugins/ofono.rules
@@ -37,7 +37,7 @@ ATTRS{idVendor}=="12d1", ATTRS{idProduct}=="1401", ENV{OFONO_IFACE_NUM}=="02", E
 ATTRS{idVendor}=="12d1", ATTRS{idProduct}=="1402", ENV{OFONO_IFACE_NUM}=="00", ENV{OFONO_HUAWEI_TYPE}="Modem"
 ATTRS{idVendor}=="12d1", ATTRS{idProduct}=="1402", ENV{OFONO_IFACE_NUM}=="02", ENV{OFONO_HUAWEI_TYPE}="Pcui"
 
-ATTRS{idVendor}=="12d1", ATTRS{idProduct}=="1404", ENV{OFONO_IFACE_NUM}=="01", ENV{OFONO_HUAWEI_TYPE}="Modem"
+ATTRS{idVendor}=="12d1", ATTRS{idProduct}=="1404", ENV{OFONO_IFACE_NUM}=="00", ENV{OFONO_HUAWEI_TYPE}="Modem"
 ATTRS{idVendor}=="12d1", ATTRS{idProduct}=="1404", ENV{OFONO_IFACE_NUM}=="02", ENV{OFONO_HUAWEI_TYPE}="Pcui"
 ATTRS{idVendor}=="12d1", ATTRS{idProduct}=="1404", ENV{OFONO_HUAWEI_VOICE}="1"
 
-- 
1.6.1.3


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

* Re: [PATCH 3/3] Huawei: set Huawei EM770W modem device to 00
  2011-01-20  5:45   ` [PATCH 3/3] Huawei: set Huawei EM770W modem device to 00 martin.xu
@ 2011-01-20 18:22     ` Lucas De Marchi
  0 siblings, 0 replies; 10+ messages in thread
From: Lucas De Marchi @ 2011-01-20 18:22 UTC (permalink / raw)
  To: ofono

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

Hi Martin

On Thu, Jan 20, 2011 at 3:45 AM,  <martin.xu@intel.com> wrote:
> From: Martin Xu <martin.xu@intel.com>
>
> Device 00 is ppp port. Setting it as 01, my Huawei EM770W modem PPP
> connection can't work
> ---
>  plugins/ofono.rules |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)

We have to decide what to do here. Look at this commit:

commit bb58a729073abcb6c5b0b2fb8b120973dfa42a07
Author: M. Dietrich <mdt@emdete.de>
Date:   Fri Dec 10 19:25:17 2010 +0100

    huawei: set huawei em770 modem device to 01

    device 00 is ppp capable while 01 is not. 01 does everything
    else fine so ofono works with it flawlessly.



regards,
Lucas De Marchi

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

* Re: [PATCH 1/3] PPP: Fix transmit ACCM and receive ACCM setting issue
  2011-01-20  5:45 [PATCH 1/3] PPP: Fix transmit ACCM and receive ACCM setting issue martin.xu
  2011-01-20  5:45 ` [PATCH 2/3] Use default ACCM (0xffffffff) to trasmit package martin.xu
@ 2011-01-20 19:08 ` Denis Kenzior
  2011-01-21  2:09   ` Xu, Martin
  1 sibling, 1 reply; 10+ messages in thread
From: Denis Kenzior @ 2011-01-20 19:08 UTC (permalink / raw)
  To: ofono

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

Hi Martin,

On 01/19/2011 11:45 PM, martin.xu(a)intel.com wrote:
> From: Martin Xu <martin.xu@intel.com>
> 
> Tramsmit ACCM and receive ACCM is mixed up.
> According to RFC1662 Section 7.1, ACCM Configuration Option is
> used to inform the peer which control characters MUST remain
> mapped when the peer sends them.
> ---
>  gatchat/ppp_lcp.c |   16 ++++++++++++++--
>  1 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/gatchat/ppp_lcp.c b/gatchat/ppp_lcp.c
> index 3a80a62..cc3e231 100644
> --- a/gatchat/ppp_lcp.c
> +++ b/gatchat/ppp_lcp.c
> @@ -149,7 +149,13 @@ static void lcp_rca(struct pppcp_data *pppcp, const struct pppcp_packet *packet)
>  	while (ppp_option_iter_next(&iter) == TRUE) {
>  		switch (ppp_option_iter_get_type(&iter)) {
>  		case ACCM:
> -			ppp_set_xmit_accm(pppcp_get_ppp(pppcp), 0);
> +			/*
> +			 * RFC1662 Section 7.1
> +			 * The Configuration Option is used to inform the peer
> +			 * which control characters MUST remain mapped when
> +			 * the peer sends them.
> +			 */
> +			ppp_set_recv_accm(pppcp_get_ppp(pppcp), 0);

You still forgot to apply my earlier comment.  The recv ACCM should be
set to what is acked by the peer.  e.g.:

ppp_set_recv_accm(pppcp, get_host_long());

>  			break;
>  		default:
>  			break;

Regards,
-Denis

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

* Re: [PATCH 2/3] Use default ACCM (0xffffffff) to trasmit package
  2011-01-20  5:45 ` [PATCH 2/3] Use default ACCM (0xffffffff) to trasmit package martin.xu
  2011-01-20  5:45   ` [PATCH 3/3] Huawei: set Huawei EM770W modem device to 00 martin.xu
@ 2011-01-20 19:10   ` Denis Kenzior
  2011-01-21  1:51     ` Xu, Martin
  1 sibling, 1 reply; 10+ messages in thread
From: Denis Kenzior @ 2011-01-20 19:10 UTC (permalink / raw)
  To: ofono

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

Hi Martin,

On 01/19/2011 11:45 PM, martin.xu(a)intel.com wrote:
> From: Martin Xu <martin.xu@intel.com>
> 
> Using my Huawei EM770W modem, if set ACCM as 0x00000000, RXJ-
> event breaks PPP link, after IP package transmit for a while.
> Using default ACCM, the issue can be fixed.
> I tested it at China Unicom networks.

Does the Huawei NAK our ACCM by any chance?

> ---
>  gatchat/ppp_lcp.c |    9 ++++++---
>  1 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/gatchat/ppp_lcp.c b/gatchat/ppp_lcp.c
> index cc3e231..2264731 100644
> --- a/gatchat/ppp_lcp.c
> +++ b/gatchat/ppp_lcp.c
> @@ -105,8 +105,7 @@ static void lcp_generate_config_options(struct lcp_data *lcp)
>  
>  static void lcp_reset_config_options(struct lcp_data *lcp)
>  {
> -	lcp->req_options = REQ_OPTION_ACCM;
> -	lcp->accm = 0;
> +	/* Using the default ACCM */
>  
>  	lcp_generate_config_options(lcp);
>  }
> @@ -155,7 +154,11 @@ static void lcp_rca(struct pppcp_data *pppcp, const struct pppcp_packet *packet)
>  			 * which control characters MUST remain mapped when
>  			 * the peer sends them.
>  			 */
> -			ppp_set_recv_accm(pppcp_get_ppp(pppcp), 0);
> +
> +			/*
> +			 * Using the default ACCM
> +			 * ppp_set_recv_accm(pppcp_get_ppp(pppcp), ~0U);
> +			 */

If you follow my advice for path 1, then this chunk isn't necessary.

>  			break;
>  		default:
>  			break;

Regards,
-Denis

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

* RE: [PATCH 2/3] Use default ACCM (0xffffffff) to trasmit package
  2011-01-20 19:10   ` [PATCH 2/3] Use default ACCM (0xffffffff) to trasmit package Denis Kenzior
@ 2011-01-21  1:51     ` Xu, Martin
  2011-01-21 20:07       ` Denis Kenzior
  0 siblings, 1 reply; 10+ messages in thread
From: Xu, Martin @ 2011-01-21  1:51 UTC (permalink / raw)
  To: ofono

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

> -----Original Message-----
> From: Denis Kenzior [mailto:denkenz(a)gmail.com]
> Sent: Friday, January 21, 2011 3:11 AM
> To: ofono(a)ofono.org
> Cc: Xu, Martin
> Subject: Re: [PATCH 2/3] Use default ACCM (0xffffffff) to trasmit package
> 
> Hi Martin,
> 
> On 01/19/2011 11:45 PM, martin.xu(a)intel.com wrote:
> > From: Martin Xu <martin.xu@intel.com>
> >
> > Using my Huawei EM770W modem, if set ACCM as 0x00000000, RXJ-
> > event breaks PPP link, after IP package transmit for a while.
> > Using default ACCM, the issue can be fixed.
> > I tested it at China Unicom networks.
> 
> Does the Huawei NAK our ACCM by any chance?
The modem can accept our 0x0 ACCM. But with the ACCM, PPP does not work.
If there is not any issue at the modem firmware, can we understand it as:
Modem can transmit ACCM with 0x0, but we have no capability to receive it, so we can't ask modem transmit as 0x0 ACCM?

> 
> > ---
> >  gatchat/ppp_lcp.c |    9 ++++++---
> >  1 files changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/gatchat/ppp_lcp.c b/gatchat/ppp_lcp.c
> > index cc3e231..2264731 100644
> > --- a/gatchat/ppp_lcp.c
> > +++ b/gatchat/ppp_lcp.c
> > @@ -105,8 +105,7 @@ static void lcp_generate_config_options(struct
> lcp_data *lcp)
> >
> >  static void lcp_reset_config_options(struct lcp_data *lcp)
> >  {
> > -	lcp->req_options = REQ_OPTION_ACCM;
> > -	lcp->accm = 0;
> > +	/* Using the default ACCM */
> >
> >  	lcp_generate_config_options(lcp);
> >  }
> > @@ -155,7 +154,11 @@ static void lcp_rca(struct pppcp_data *pppcp, const
> struct pppcp_packet *packet)
> >  			 * which control characters MUST remain mapped when
> >  			 * the peer sends them.
> >  			 */
> > -			ppp_set_recv_accm(pppcp_get_ppp(pppcp), 0);
> > +
> > +			/*
> > +			 * Using the default ACCM
> > +			 * ppp_set_recv_accm(pppcp_get_ppp(pppcp), ~0U);
> > +			 */
> 
> If you follow my advice for path 1, then this chunk isn't necessary.
> 
> >  			break;
> >  		default:
> >  			break;
> 
> Regards,
> -Denis

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

* RE: [PATCH 1/3] PPP: Fix transmit ACCM and receive ACCM setting issue
  2011-01-20 19:08 ` [PATCH 1/3] PPP: Fix transmit ACCM and receive ACCM setting issue Denis Kenzior
@ 2011-01-21  2:09   ` Xu, Martin
  2011-01-21 20:10     ` Denis Kenzior
  0 siblings, 1 reply; 10+ messages in thread
From: Xu, Martin @ 2011-01-21  2:09 UTC (permalink / raw)
  To: ofono

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

Hi Denis:
> -----Original Message-----
> From: Denis Kenzior [mailto:denkenz(a)gmail.com]
> Sent: Friday, January 21, 2011 3:09 AM
> To: ofono(a)ofono.org
> Cc: Xu, Martin
> Subject: Re: [PATCH 1/3] PPP: Fix transmit ACCM and receive ACCM setting
> issue
> 
> Hi Martin,
> 
> On 01/19/2011 11:45 PM, martin.xu(a)intel.com wrote:
> > From: Martin Xu <martin.xu@intel.com>
> >
> > Tramsmit ACCM and receive ACCM is mixed up.
> > According to RFC1662 Section 7.1, ACCM Configuration Option is
> > used to inform the peer which control characters MUST remain
> > mapped when the peer sends them.
> > ---
> >  gatchat/ppp_lcp.c |   16 ++++++++++++++--
> >  1 files changed, 14 insertions(+), 2 deletions(-)
> >
> > diff --git a/gatchat/ppp_lcp.c b/gatchat/ppp_lcp.c
> > index 3a80a62..cc3e231 100644
> > --- a/gatchat/ppp_lcp.c
> > +++ b/gatchat/ppp_lcp.c
> > @@ -149,7 +149,13 @@ static void lcp_rca(struct pppcp_data *pppcp, const
> struct pppcp_packet *packet)
> >  	while (ppp_option_iter_next(&iter) == TRUE) {
> >  		switch (ppp_option_iter_get_type(&iter)) {
> >  		case ACCM:
> > -			ppp_set_xmit_accm(pppcp_get_ppp(pppcp), 0);
> > +			/*
> > +			 * RFC1662 Section 7.1
> > +			 * The Configuration Option is used to inform the peer
> > +			 * which control characters MUST remain mapped when
> > +			 * the peer sends them.
> > +			 */
> > +			ppp_set_recv_accm(pppcp_get_ppp(pppcp), 0);
> 
> You still forgot to apply my earlier comment.  The recv ACCM should be
> set to what is acked by the peer.  e.g.:
That is not this patch want to resolve. This patch is just used to fix the mixing up of setting recv ACCM and transit ACCM, And fix Huawei modem issue.

1. Currently, it is just ACKed by peer. You know It is called at lcp_rca, and here the code always use ACCM as 0x0. So that is just the acked ACCM.

2. I think current implementation of negotiation of ACCM needs enhancement:
1. we needs to set ACCM according to each modem
2. We needs to use ACKed ACCM as you mentioned. 


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

* Re: [PATCH 2/3] Use default ACCM (0xffffffff) to trasmit package
  2011-01-21  1:51     ` Xu, Martin
@ 2011-01-21 20:07       ` Denis Kenzior
  0 siblings, 0 replies; 10+ messages in thread
From: Denis Kenzior @ 2011-01-21 20:07 UTC (permalink / raw)
  To: ofono

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

Hi Martin,

On 01/20/2011 07:51 PM, Xu, Martin wrote:
>> -----Original Message-----
>> From: Denis Kenzior [mailto:denkenz(a)gmail.com]
>> Sent: Friday, January 21, 2011 3:11 AM
>> To: ofono(a)ofono.org
>> Cc: Xu, Martin
>> Subject: Re: [PATCH 2/3] Use default ACCM (0xffffffff) to trasmit package
>>
>> Hi Martin,
>>
>> On 01/19/2011 11:45 PM, martin.xu(a)intel.com wrote:
>>> From: Martin Xu <martin.xu@intel.com>
>>>
>>> Using my Huawei EM770W modem, if set ACCM as 0x00000000, RXJ-
>>> event breaks PPP link, after IP package transmit for a while.
>>> Using default ACCM, the issue can be fixed.
>>> I tested it at China Unicom networks.
>>
>> Does the Huawei NAK our ACCM by any chance?
> The modem can accept our 0x0 ACCM. But with the ACCM, PPP does not work.
> If there is not any issue at the modem firmware, can we understand it as:
> Modem can transmit ACCM with 0x0, but we have no capability to receive it, so we can't ask modem transmit as 0x0 ACCM?
> 

We should be able to receive ACCM of 0 just fine.  I double checked the
code, but maybe you want to put a pair of eyeballs on it as well.

Regards,
-Denis

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

* Re: [PATCH 1/3] PPP: Fix transmit ACCM and receive ACCM setting issue
  2011-01-21  2:09   ` Xu, Martin
@ 2011-01-21 20:10     ` Denis Kenzior
  0 siblings, 0 replies; 10+ messages in thread
From: Denis Kenzior @ 2011-01-21 20:10 UTC (permalink / raw)
  To: ofono

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

Hi Martin,

>> You still forgot to apply my earlier comment.  The recv ACCM should be
>> set to what is acked by the peer.  e.g.:
> That is not this patch want to resolve. This patch is just used to fix the mixing up of setting recv ACCM and transit ACCM, And fix Huawei modem issue.
> 
> 1. Currently, it is just ACKed by peer. You know It is called at lcp_rca, and here the code always use ACCM as 0x0. So that is just the acked ACCM.

Then please send a 2nd patch that just sets the recv ACCM to the one
acked by the peer if you want to be pedantic.  But changing the chunk
once, then changing it in the second chunk to some other value will only
serve to confuse me or anyone who reviews your patch submissions.

Regards,
-Denis

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

end of thread, other threads:[~2011-01-21 20:10 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-01-20  5:45 [PATCH 1/3] PPP: Fix transmit ACCM and receive ACCM setting issue martin.xu
2011-01-20  5:45 ` [PATCH 2/3] Use default ACCM (0xffffffff) to trasmit package martin.xu
2011-01-20  5:45   ` [PATCH 3/3] Huawei: set Huawei EM770W modem device to 00 martin.xu
2011-01-20 18:22     ` Lucas De Marchi
2011-01-20 19:10   ` [PATCH 2/3] Use default ACCM (0xffffffff) to trasmit package Denis Kenzior
2011-01-21  1:51     ` Xu, Martin
2011-01-21 20:07       ` Denis Kenzior
2011-01-20 19:08 ` [PATCH 1/3] PPP: Fix transmit ACCM and receive ACCM setting issue Denis Kenzior
2011-01-21  2:09   ` Xu, Martin
2011-01-21 20:10     ` Denis Kenzior

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