Re: [PATCH 1/3] PPP: Fix transmit ACCM and receive ACCM setting issue

2011-01-21 Thread Denis Kenzior 
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
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono

Re: [PATCH 1/3] PPP: Fix transmit ACCM and receive ACCM setting issue

2011-01-20 Thread Denis Kenzior 
Hi Martin,

On 01/19/2011 11:45 PM, martin...@intel.com wrote:
 From: Martin Xu martin...@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
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono

RE: [PATCH 1/3] PPP: Fix transmit ACCM and receive ACCM setting issue

2011-01-20 Thread Xu, Martin 
Hi Denis:
 -Original Message-
 From: Denis Kenzior [mailto:denk...@gmail.com]
 Sent: Friday, January 21, 2011 3:09 AM
 To: ofono@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...@intel.com wrote:
  From: Martin Xu martin...@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. 

___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono