Skip to content

Commit e70917c

Browse files
scott-brustscott-brust
authored
Merge pull request #2571 from particle-iot/test/boron-bsom-r410-ppp-crash-in-network-workaround
[Boron / B SoM] R410 PPP crash in network phase workaround
2 parents 1eba189 + 77f03d3 commit e70917c

File tree

15 files changed

+253
-57
lines changed

15 files changed

+253
-57
lines changed

‎hal/network/lwip/cellular/pppncpnetif.cpp

Lines changed: 56 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,56 @@ namespace {
4848
// 5 minutes
4949
const unsigned PPP_CONNECT_TIMEOUT = 5 * 60 * 1000;
5050

51+
int pppErrorToSystem(int err) {
52+
using namespace particle::net::ppp;
53+
switch(err) {
54+
case Client::ERROR_NONE: {
55+
return SYSTEM_ERROR_NONE;
56+
}
57+
case Client::ERROR_PARAM: {
58+
return SYSTEM_ERROR_PPP_PARAM;
59+
}
60+
case Client::ERROR_OPEN: {
61+
return SYSTEM_ERROR_PPP_OPEN;
62+
}
63+
case Client::ERROR_DEVICE: {
64+
return SYSTEM_ERROR_PPP_DEVICE;
65+
}
66+
case Client::ERROR_ALLOC: {
67+
return SYSTEM_ERROR_PPP_ALLOC;
68+
}
69+
case Client::ERROR_USER: {
70+
return SYSTEM_ERROR_PPP_USER;
71+
}
72+
case Client::ERROR_CONNECT: {
73+
return SYSTEM_ERROR_PPP_CONNECT;
74+
}
75+
case Client::ERROR_AUTHFAIL: {
76+
return SYSTEM_ERROR_PPP_AUTH_FAIL;
77+
}
78+
case Client::ERROR_PROTOCOL: {
79+
return SYSTEM_ERROR_PPP_PROTOCOL;
80+
}
81+
case Client::ERROR_PEERDEAD: {
82+
return SYSTEM_ERROR_PPP_PEER_DEAD;
83+
}
84+
case Client::ERROR_IDLETIMEOUT: {
85+
return SYSTEM_ERROR_PPP_IDLE_TIMEOUT;
86+
}
87+
case Client::ERROR_CONNECTTIME: {
88+
return SYSTEM_ERROR_PPP_CONNECT_TIME;
89+
}
90+
case Client::ERROR_LOOPBACK: {
91+
return SYSTEM_ERROR_PPP_LOOPBACK;
92+
}
93+
case Client::ERROR_NO_CARRIER_IN_NETWORK_PHASE: {
94+
return SYSTEM_ERROR_PPP_NO_CARRIER_IN_NETWORK_PHASE;
95+
}
96+
}
97+
98+
return SYSTEM_ERROR_UNKNOWN;
99+
}
100+
51101
} // anonymous
52102

53103

@@ -311,12 +361,12 @@ void PppNcpNetif::netifEventHandler(netif_nsc_reason_t reason, const netif_ext_c
311361
/* Nothing to do here */
312362
}
313363

314-
void PppNcpNetif::pppEventHandlerCb(particle::net::ppp::Client* c, uint64_t ev, void* ctx) {
364+
void PppNcpNetif::pppEventHandlerCb(particle::net::ppp::Client* c, uint64_t ev, int data, void* ctx) {
315365
auto self = (PppNcpNetif*)ctx;
316-
self->pppEventHandler(ev);
366+
self->pppEventHandler(ev, data);
317367
}
318368

319-
void PppNcpNetif::pppEventHandler(uint64_t ev) {
369+
void PppNcpNetif::pppEventHandler(uint64_t ev, int data) {
320370
if (ev == particle::net::ppp::Client::EVENT_UP) {
321371
unsigned mtu = client_.getIf()->mtu;
322372
LOG(TRACE, "Negotiated MTU: %u", mtu);
@@ -331,6 +381,9 @@ void PppNcpNetif::pppEventHandler(uint64_t ev) {
331381
if (connectStart_ == 0) {
332382
connectStart_ = HAL_Timer_Get_Milli_Seconds();
333383
}
384+
} else if (ev == particle::net::ppp::Client::EVENT_ERROR) {
385+
LOG(ERROR, "PPP error event data=%d", data);
386+
celMan_->ncpClient()->dataModeError(pppErrorToSystem(data));
334387
}
335388
}
336389

‎hal/network/lwip/cellular/pppncpnetif.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,8 +77,8 @@ class PppNcpNetif : public BaseNetif {
7777

7878
static void loop(void* arg);
7979

80-
static void pppEventHandlerCb(particle::net::ppp::Client* c, uint64_t ev, void* ctx);
81-
void pppEventHandler(uint64_t ev);
80+
static void pppEventHandlerCb(particle::net::ppp::Client* c, uint64_t ev, int data, void* ctx);
81+
void pppEventHandler(uint64_t ev, int data);
8282

8383
static void mempEventHandler(memp_t type, unsigned available, unsigned size, void* ctx);
8484

‎hal/network/lwip/ppp_client.cpp

Lines changed: 54 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,21 @@ int Client::netifClientDataIdx_ = -1;
5252
constexpr const char* Client::eventNames_[];
5353
constexpr const char* Client::stateNames_[];
5454

55+
namespace {
56+
57+
#if !PPP_DEBUG
58+
const char* pppPhaseToString(uint8_t phase) {
59+
const char* phases[] = {
60+
"Dead", "Master", "Holdoff", "Initialize", "SerialConn",
61+
"Dormant", "Establish", "Authenticate", "Callback", "Network",
62+
"Running", "Terminate", "Disconnect", "Unknown"
63+
};
64+
return phases[std::min<int>(phase, sizeof(phases) / sizeof(phases[0]) - 1)];
65+
}
66+
#endif // !PPP_DEBUG
67+
68+
} // anonymous
69+
5570
Client::Client() {
5671
std::call_once(once_, []() {
5772
LOCK_TCPIP_CORE();
@@ -109,7 +124,7 @@ void Client::init() {
109124

110125
pppapi_set_notify_phase_callback(pcb_, &Client::notifyPhaseCb);
111126

112-
os_queue_create(&queue_, sizeof(uint64_t), 5, nullptr);
127+
os_queue_create(&queue_, sizeof(QueueEvent), 5, nullptr);
113128
SPARK_ASSERT(queue_);
114129

115130
os_thread_create(&thread_, "ppp", OS_THREAD_PRIORITY_NETWORK, &Client::loopCb, this, OS_THREAD_STACK_SIZE_DEFAULT_HIGH);
@@ -199,11 +214,14 @@ bool Client::configure(void* config) {
199214
return true;
200215
}
201216

202-
bool Client::notifyEvent(uint64_t ev) {
217+
bool Client::notifyEvent(uint64_t ev, int data) {
203218
if (!running_) {
204219
return false;
205220
}
206-
return os_queue_put(queue_, &ev, CONCURRENT_WAIT_FOREVER, nullptr) == 0;
221+
QueueEvent event = {};
222+
event.ev = ev;
223+
event.data = data;
224+
return os_queue_put(queue_, &event, CONCURRENT_WAIT_FOREVER, nullptr) == 0;
207225
}
208226

209227
int Client::input(const uint8_t* data, size_t size) {
@@ -213,6 +231,18 @@ int Client::input(const uint8_t* data, size_t size) {
213231
case STATE_DISCONNECTING:
214232
case STATE_CONNECTED: {
215233
LOG_DEBUG(TRACE, "RX: %lu", size);
234+
235+
if (platform_primary_ncp_identifier() == PLATFORM_NCP_SARA_R410) {
236+
auto pppos = (pppos_pcb*)pcb_->link_ctx_cb;
237+
const char NO_CARRIER[] = "\r\nNO CARRIER\r\n";
238+
if (pppos && pppos->in_state == PDADDRESS && pcb_->phase == PPP_PHASE_NETWORK && data[0] != PPP_FLAG && size >= sizeof(NO_CARRIER) - 1 && !strncmp((const char*)data, NO_CARRIER, size)) {
239+
LOG(ERROR, "NO CARRIER in network PPP phase");
240+
pppapi_close(pcb_, 1);
241+
notifyEvent(EVENT_ERROR, ERROR_NO_CARRIER_IN_NETWORK_PHASE);
242+
break;
243+
}
244+
}
245+
216246
#if !PPP_INPROC_IRQ_SAFE
217247
err_t err = pppos_input_tcpip(pcb_, (u8_t*)data, size);
218248
#else
@@ -321,7 +351,7 @@ void Client::loop() {
321351
}
322352
err_t err = pppapi_connect(pcb_, 0);
323353
if (err != ERR_OK) {
324-
LOG(TRACE, "PPP error connecting: %x", err);
354+
LOG(TRACE, "PPP error connecting: %d", err);
325355
transition(STATE_CONNECT);
326356
}
327357
break;
@@ -361,11 +391,11 @@ void Client::loop() {
361391
}
362392
}
363393

364-
uint64_t ev = 0;
394+
QueueEvent ev = {};
365395
if (os_queue_take(queue_, &ev, qWait, nullptr) == 0) {
366-
LOG(TRACE, "PPP thread event %s", eventNames_[ev]);
396+
LOG(TRACE, "PPP thread event %s data=%d", eventNames_[ev.ev], ev.data);
367397
/* Incoming event */
368-
switch (ev) {
398+
switch (ev.ev) {
369399
case EVENT_LOWER_UP: {
370400
if (!lowerState_) {
371401
lowerState_ = true;
@@ -427,6 +457,16 @@ void Client::loop() {
427457
}
428458
break;
429459
}
460+
case EVENT_ERROR: {
461+
std::unique_lock<std::mutex> lk(mutex_);
462+
auto cb = cb_;
463+
auto ctx = cbCtx_;
464+
lk.unlock();
465+
if (cb) {
466+
cb(this, EVENT_ERROR, ev.data, ctx);
467+
}
468+
break;
469+
}
430470
}
431471
}
432472
}
@@ -464,7 +504,9 @@ void Client::notifyPhaseCb(ppp_pcb* pcb, uint8_t phase, void* ctx) {
464504
}
465505

466506
void Client::notifyPhase(uint8_t phase) {
467-
LOG(TRACE, "PPP phase -> %d", phase);
507+
#if !PPP_DEBUG
508+
LOG(TRACE, "PPP phase -> %s", pppPhaseToString(phase));
509+
#endif // !PPP_DEBUG
468510
}
469511

470512
void Client::notifyStatusCb(ppp_pcb* pcb, int err, void* ctx) {
@@ -484,6 +526,7 @@ void Client::notifyStatus(int err) {
484526
}
485527
default: {
486528
/* Error connecting */
529+
notifyEvent(EVENT_ERROR, err);
487530
notifyEvent(EVENT_DOWN);
488531
break;
489532
}
@@ -518,11 +561,11 @@ void Client::transition(State newState) {
518561
lk.unlock();
519562
if (cb) {
520563
if (state_ == STATE_CONNECTED) {
521-
cb(this, EVENT_UP, ctx);
564+
cb(this, EVENT_UP, ERROR_NONE, ctx);
522565
} else if (state_ == STATE_DISCONNECTED) {
523-
cb(this, EVENT_DOWN, ctx);
566+
cb(this, EVENT_DOWN, ERROR_NONE, ctx);
524567
} else if (state_ == STATE_CONNECTING) {
525-
cb(this, EVENT_CONNECTING, ctx);
568+
cb(this, EVENT_CONNECTING, ERROR_NONE, ctx);
526569
}
527570
}
528571
}

‎hal/network/lwip/ppp_client.h

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,23 @@ class Client {
6565
EVENT_MAX = 0x09
6666
};
6767

68+
enum Error {
69+
ERROR_NONE = PPPERR_NONE,
70+
ERROR_PARAM = PPPERR_PARAM,
71+
ERROR_OPEN = PPPERR_OPEN,
72+
ERROR_DEVICE = PPPERR_DEVICE,
73+
ERROR_ALLOC = PPPERR_ALLOC,
74+
ERROR_USER = PPPERR_USER,
75+
ERROR_CONNECT = PPPERR_CONNECT,
76+
ERROR_AUTHFAIL = PPPERR_AUTHFAIL,
77+
ERROR_PROTOCOL = PPPERR_PROTOCOL,
78+
ERROR_PEERDEAD = PPPERR_PEERDEAD,
79+
ERROR_IDLETIMEOUT = PPPERR_IDLETIMEOUT,
80+
ERROR_CONNECTTIME = PPPERR_CONNECTTIME,
81+
ERROR_LOOPBACK = PPPERR_LOOPBACK,
82+
ERROR_NO_CARRIER_IN_NETWORK_PHASE = 100
83+
};
84+
6885
enum State {
6986
STATE_NONE = 0,
7087
STATE_READY = 1,
@@ -77,15 +94,15 @@ class Client {
7794
STATE_MAX = 8
7895
};
7996

80-
bool notifyEvent(uint64_t ev);
97+
bool notifyEvent(uint64_t ev, int data = ERROR_NONE);
8198
int input(const uint8_t* data, size_t size);
8299

83100
typedef int (*OutputCallback)(const uint8_t* data, size_t size, void* ctx);
84101
typedef int (*EnterDataModeCallback)(void* ctx);
85102
void setOutputCallback(OutputCallback cb, void* ctx);
86103
void setEnterDataModeCallback(EnterDataModeCallback, void* ctx);
87104

88-
typedef void (*NotifyCallback)(Client* c, uint64_t ev, void* ctx);
105+
typedef void (*NotifyCallback)(Client* c, uint64_t ev, int data, void* ctx);
89106

90107
void setNotifyCallback(NotifyCallback cb, void* ctx);
91108

@@ -142,6 +159,12 @@ class Client {
142159
void transition(State newState);
143160

144161
private:
162+
163+
struct QueueEvent {
164+
uint64_t ev;
165+
int data;
166+
};
167+
145168
netif if_ = {};
146169
ppp_pcb* pcb_ = nullptr;
147170
#if PPP_IPCP_OVERRIDE

‎hal/network/ncp/cellular/cellular_ncp_client.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,7 @@ class CellularNcpClient: public NcpClient {
147147
virtual int getMtu() = 0;
148148
virtual int urcs(bool enable) = 0;
149149
virtual int startNcpFwUpdate(bool update) = 0;
150+
virtual int dataModeError(int error) = 0;
150151
};
151152

152153
inline CellularNcpClientConfig::CellularNcpClientConfig() :

‎hal/network/ncp_client/quectel/quectel_ncp_client.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1573,6 +1573,10 @@ int QuectelNcpClient::enterDataMode() {
15731573
return r;
15741574
}
15751575

1576+
int QuectelNcpClient::dataModeError(int error) {
1577+
return 0;
1578+
}
1579+
15761580
int QuectelNcpClient::getMtu() {
15771581
return 0;
15781582
}

‎hal/network/ncp_client/quectel/quectel_ncp_client.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ class QuectelNcpClient: public CellularNcpClient {
7474
virtual int getMtu() override;
7575
virtual int urcs(bool enable) override;
7676
virtual int startNcpFwUpdate(bool update) override;
77+
virtual int dataModeError(int error) override;
7778

7879
auto getMuxer() {
7980
return &muxer_;

0 commit comments

Comments
 (0)