* [PATCH] test-server:Add test of PPP disconnect reason @ 2011-02-17 13:48 Guillaume Zajac 2011-02-17 16:14 ` Denis Kenzior 0 siblings, 1 reply; 5+ messages in thread From: Guillaume Zajac @ 2011-02-17 13:48 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 603 bytes --] --- gatchat/test-server.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/gatchat/test-server.c b/gatchat/test-server.c index e574d64..c0a5a4d 100644 --- a/gatchat/test-server.c +++ b/gatchat/test-server.c @@ -137,6 +137,12 @@ static void ppp_disconnect(GAtPPPDisconnectReason reason, gpointer user) g_at_ppp_unref(ppp); ppp = NULL; + if (reason == G_AT_PPP_REASON_NET_FAIL) { + g_at_server_unref(server); + server = NULL; + return; + } + g_at_server_resume(server); g_at_server_set_debug(server, server_debug, "Server"); -- 1.7.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] test-server:Add test of PPP disconnect reason 2011-02-17 13:48 [PATCH] test-server:Add test of PPP disconnect reason Guillaume Zajac @ 2011-02-17 16:14 ` Denis Kenzior 2011-02-17 16:37 ` Guillaume Zajac 0 siblings, 1 reply; 5+ messages in thread From: Denis Kenzior @ 2011-02-17 16:14 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 763 bytes --] Hi Guillaume, On 02/17/2011 07:48 AM, Guillaume Zajac wrote: > --- > gatchat/test-server.c | 6 ++++++ > 1 files changed, 6 insertions(+), 0 deletions(-) > > diff --git a/gatchat/test-server.c b/gatchat/test-server.c > index e574d64..c0a5a4d 100644 > --- a/gatchat/test-server.c > +++ b/gatchat/test-server.c > @@ -137,6 +137,12 @@ static void ppp_disconnect(GAtPPPDisconnectReason reason, gpointer user) > g_at_ppp_unref(ppp); > ppp = NULL; > > + if (reason == G_AT_PPP_REASON_NET_FAIL) { Don't you mean G_AT_PPP_REASON_LINK_DEAD? > + g_at_server_unref(server); > + server = NULL; > + return; > + } > + > g_at_server_resume(server); > g_at_server_set_debug(server, server_debug, "Server"); > Regards, -Denis ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] test-server:Add test of PPP disconnect reason 2011-02-17 16:14 ` Denis Kenzior @ 2011-02-17 16:37 ` Guillaume Zajac 2011-02-17 16:50 ` Denis Kenzior 0 siblings, 1 reply; 5+ messages in thread From: Guillaume Zajac @ 2011-02-17 16:37 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 1990 bytes --] Hi Denis, On 17/02/2011 17:14, Denis Kenzior wrote: > Hi Guillaume, > > On 02/17/2011 07:48 AM, Guillaume Zajac wrote: >> --- >> gatchat/test-server.c | 6 ++++++ >> 1 files changed, 6 insertions(+), 0 deletions(-) >> >> diff --git a/gatchat/test-server.c b/gatchat/test-server.c >> index e574d64..c0a5a4d 100644 >> --- a/gatchat/test-server.c >> +++ b/gatchat/test-server.c >> @@ -137,6 +137,12 @@ static void ppp_disconnect(GAtPPPDisconnectReason reason, gpointer user) >> g_at_ppp_unref(ppp); >> ppp = NULL; >> >> + if (reason == G_AT_PPP_REASON_NET_FAIL) { > Don't you mean G_AT_PPP_REASON_LINK_DEAD? > In case of test-server, I didn't reproduce the crash with/without the fix, I applied it to test server first as we don't have dial_cb for emulator yet: I did ./gsmdial -i localhost -p 12346 At the end of the exchange: ppp_lcp_down_notify() is called disconnect reason is G_AT_PPP_REASON_PEER_CLOSED then the GAtServer is well resumed. In case of emulator (based on Gustavo/ZhenZhua dial_cb), ppp_lcp_down_notify() is called but just after this one, ppp_ipcp_up_notify() fails and sets disconnect reason to G_AT_PPP_REASON_NET_FAIL. Then we get a crash after ppp_disconnect because we call the hdlc write handler e.g. can_write_data() into gathldc.c although we have unref the PPP server. So I called ofono_emulator_remove() that will call the unref GAtServer etc... when we get G_AT_PPP_REASON_NET_FAIL into ppp_disconnect I don't get G_AT_PPP_REASON_LINK_DEAD disconnect reason when emulator crashes. So should we call ofono_emulator_remove()/GAtServer_unref() in both disconnect reason cases? Or is there a specific action to do when G_AT_PPP_REASON_NET_FAIL? >> + g_at_server_unref(server); >> + server = NULL; >> + return; >> + } >> + >> g_at_server_resume(server); >> g_at_server_set_debug(server, server_debug, "Server"); >> > Regards, > -Denis > Kind regards, Guillaume ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] test-server:Add test of PPP disconnect reason 2011-02-17 16:37 ` Guillaume Zajac @ 2011-02-17 16:50 ` Denis Kenzior 2011-02-17 17:21 ` Guillaume Zajac 0 siblings, 1 reply; 5+ messages in thread From: Denis Kenzior @ 2011-02-17 16:50 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 2137 bytes --] Hi Guillaume, On 02/17/2011 10:37 AM, Guillaume Zajac wrote: > Hi Denis, > > On 17/02/2011 17:14, Denis Kenzior wrote: >> Hi Guillaume, >> >> On 02/17/2011 07:48 AM, Guillaume Zajac wrote: >>> --- >>> gatchat/test-server.c | 6 ++++++ >>> 1 files changed, 6 insertions(+), 0 deletions(-) >>> >>> diff --git a/gatchat/test-server.c b/gatchat/test-server.c >>> index e574d64..c0a5a4d 100644 >>> --- a/gatchat/test-server.c >>> +++ b/gatchat/test-server.c >>> @@ -137,6 +137,12 @@ static void >>> ppp_disconnect(GAtPPPDisconnectReason reason, gpointer user) >>> g_at_ppp_unref(ppp); >>> ppp = NULL; >>> >>> + if (reason == G_AT_PPP_REASON_NET_FAIL) { >> Don't you mean G_AT_PPP_REASON_LINK_DEAD? >> > > In case of test-server, I didn't reproduce the crash with/without the > fix, I applied it to test server first as we don't have dial_cb for > emulator yet: > > I did ./gsmdial -i localhost -p 12346 > > At the end of the exchange: > ppp_lcp_down_notify() is called > disconnect reason is G_AT_PPP_REASON_PEER_CLOSED > then the GAtServer is well resumed. > That is because gsmdial performs graceful shutdown, try killing the app with -9. > In case of emulator (based on Gustavo/ZhenZhua dial_cb), > ppp_lcp_down_notify() is called but just after this one, > ppp_ipcp_up_notify() fails and sets disconnect reason to > G_AT_PPP_REASON_NET_FAIL. Then we probably have a bug. > Then we get a crash after ppp_disconnect because we call the hdlc write > handler e.g. can_write_data() into gathldc.c although we have unref the > PPP server. > > So I called ofono_emulator_remove() that will call the unref GAtServer > etc... when we get G_AT_PPP_REASON_NET_FAIL into ppp_disconnect > > I don't get G_AT_PPP_REASON_LINK_DEAD disconnect reason when emulator > crashes. > > So should we call ofono_emulator_remove()/GAtServer_unref() in both > disconnect reason cases? > Or is there a specific action to do when G_AT_PPP_REASON_NET_FAIL? > Not necessarily, NET_FAIL basically means the IPCP negotiation failed. It is not fatal. Regards, -Denis ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] test-server:Add test of PPP disconnect reason 2011-02-17 16:50 ` Denis Kenzior @ 2011-02-17 17:21 ` Guillaume Zajac 0 siblings, 0 replies; 5+ messages in thread From: Guillaume Zajac @ 2011-02-17 17:21 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 2674 bytes --] Hi Denis, On 17/02/2011 17:50, Denis Kenzior wrote: > Hi Guillaume, > > On 02/17/2011 10:37 AM, Guillaume Zajac wrote: >> Hi Denis, >> >> On 17/02/2011 17:14, Denis Kenzior wrote: >>> Hi Guillaume, >>> >>> On 02/17/2011 07:48 AM, Guillaume Zajac wrote: >>>> --- >>>> gatchat/test-server.c | 6 ++++++ >>>> 1 files changed, 6 insertions(+), 0 deletions(-) >>>> >>>> diff --git a/gatchat/test-server.c b/gatchat/test-server.c >>>> index e574d64..c0a5a4d 100644 >>>> --- a/gatchat/test-server.c >>>> +++ b/gatchat/test-server.c >>>> @@ -137,6 +137,12 @@ static void >>>> ppp_disconnect(GAtPPPDisconnectReason reason, gpointer user) >>>> g_at_ppp_unref(ppp); >>>> ppp = NULL; >>>> >>>> + if (reason == G_AT_PPP_REASON_NET_FAIL) { >>> Don't you mean G_AT_PPP_REASON_LINK_DEAD? >>> >> In case of test-server, I didn't reproduce the crash with/without the >> fix, I applied it to test server first as we don't have dial_cb for >> emulator yet: >> >> I did ./gsmdial -i localhost -p 12346 >> >> At the end of the exchange: >> ppp_lcp_down_notify() is called >> disconnect reason is G_AT_PPP_REASON_PEER_CLOSED >> then the GAtServer is well resumed. >> > That is because gsmdial performs graceful shutdown, try killing the app > with -9. > Right, I just tested it and the fix with: if (reason == G_AT_PPP_REASON_LINK_DEAD) { g_at_server_unref(server); server = NULL; return; } is avoiding the crash in case the telnet is killed after we do a ATD*99. I will submit a new patch for fixing this crash. >> In case of emulator (based on Gustavo/ZhenZhua dial_cb), >> ppp_lcp_down_notify() is called but just after this one, >> ppp_ipcp_up_notify() fails and sets disconnect reason to >> G_AT_PPP_REASON_NET_FAIL. > Then we probably have a bug. Yes, I will try to investigate a bit more on this issue and the action to do. >> Then we get a crash after ppp_disconnect because we call the hdlc write >> handler e.g. can_write_data() into gathldc.c although we have unref the >> PPP server. >> >> So I called ofono_emulator_remove() that will call the unref GAtServer >> etc... when we get G_AT_PPP_REASON_NET_FAIL into ppp_disconnect >> >> I don't get G_AT_PPP_REASON_LINK_DEAD disconnect reason when emulator >> crashes. >> >> So should we call ofono_emulator_remove()/GAtServer_unref() in both >> disconnect reason cases? >> Or is there a specific action to do when G_AT_PPP_REASON_NET_FAIL? >> > Not necessarily, NET_FAIL basically means the IPCP negotiation failed. > It is not fatal. Ok. > Regards, > -Denis > Kind regards, Guillaume ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-02-17 17:21 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-02-17 13:48 [PATCH] test-server:Add test of PPP disconnect reason Guillaume Zajac 2011-02-17 16:14 ` Denis Kenzior 2011-02-17 16:37 ` Guillaume Zajac 2011-02-17 16:50 ` Denis Kenzior 2011-02-17 17:21 ` Guillaume Zajac
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.