Cellular fix to PPP only: recover from service denial.

WARNING: this change is well tested on ESP-IDF, less well tested on Zephyr/Linux.  Should you have issues with PPP not reconnecting across a service outage, the place to look is the uPortPppReconnect() function in the file <ubxlib_root>/port/platform/<your platform>/u_port_ppp.c, which now performs a PPP disconnect before it remakes the PPP connection.

If either the network or the cellular module causes the PDP context to be deactivated while a PPP connection is active, that PPP connection will be deactivated by the cellular module sending an LCP Terminate Request over the PPP interface to the peer entity.  At this point the PPP connection is dead, it will never be reactivated ever, and so when the PDP context comes back the inactive connection needs to be torn down and a new one brought up,

What now happens:

- on the CMUX side, don't leave the old "direct" AT client locked; this means that CMUX can be taken down again by any task (otherwise FreeRTOS complains that the task unlocking the mutex is not the same as the task that had the mutex locked),
- for ESP-IDF, on the PPP side, no longer use the IP addresses passed in: ESP-IDF PPP picks these up during PPP LCP negotiation anyway, and not needing these allows the next bullet point,
- uPortPppReconnect() now stops the PPP link up into ESP-IDF/Zephyr/Linux, disconnects and reconnects PPP from a cellular module standpoint and then restarts the PPP link up into ESP-IDF/Zephyr/Linux afterwards; this does a proper/full reconnection, which might not _always_ be necessary but sometimes is: better safe than sorry.

cell/src/u_cell_mux.c

@@ -1761,6 +1761,11 @@ int32_t uCellMuxPrivateEnable(uCellPrivateInstance_t *pInstance)
                                         // of our instance to the new AT handle, leaving the
                                         // old AT handle locked
                                         pInstance->atHandle = atHandle;
+                                        // Unlock the now inactive AT client handle from this task,
+                                        // just in case we want to switch CMUX off from some
+                                        // other task (FreeRTOS doesn't like mutexes being unlocked
+                                        // by a task that didn't lock them).
+                                        uAtClientUnlock(pContext->savedAtHandle);
                                         // The setting of echo-off and AT+CMEE is port-specific,
                                         // so we need to set those here for the new port
 #ifdef U_CFG_CELL_ENABLE_NUMERIC_ERROR
@@ -1892,6 +1897,7 @@ int32_t uCellMuxPrivateDisable(uCellPrivateInstance_t *pInstance)
             if (pContext->savedAtHandle != NULL) {
                 // Copy the settings of the AT handler on channel 1
                 // back into the original one, in case they have changed
+                uAtClientLock(pContext->savedAtHandle);
                 errorCode = uCellMuxPrivateCopyAtClient(atHandle, pContext->savedAtHandle);
                 // While we set the error code above, there's not a whole lot
                 // we can do if this fails, so continue anyway; close the

cell/src/u_cell_net.c

@@ -67,6 +67,7 @@
 #include "u_cell_mno_db.h"
 #include "u_cell_ppp_shared.h"
 
+#include "u_cell_ppp_private.h"
 #include "u_cell_pwr_private.h"
 
 /* ----------------------------------------------------------------
@@ -202,6 +203,7 @@ static const uCellNetRegDomain_t gRegTypeToDomain[] = {
     U_CELL_NET_REG_DOMAIN_PS, // U_CELL_PRIVATE_NET_REG_TYPE_CGREG
     U_CELL_NET_REG_DOMAIN_PS // U_CELL_PRIVATE_NET_REG_TYPE_CEREG
 };
+
 #if U_CFG_ENABLE_LOGGING
 /** Strings that describe the possible authentication modes;
  * used in a debug print only, MUST have the same number of
@@ -215,6 +217,13 @@ static const char *gpAuthenticationModeStr[] = {
 };
 #endif
 
+/** The [start of] strings returned by +CGEV which mean that
+ * the PDP context has been removed unexpectedly by the network.
+ */
+static const char *gpCgevPdpContextFailure[] = {
+    "NW PDN DEACT", "NW DETACH"
+};
+
 /* ----------------------------------------------------------------
  * STATIC FUNCTIONS: FORWARD DECLARATIONS
  * -------------------------------------------------------------- */
@@ -291,10 +300,16 @@ static void activateContextCallback(uAtClientHandle_t atHandle,
 
     if (U_CELL_PRIVATE_HAS(pInstance->pModule, U_CELL_PRIVATE_FEATURE_PPP)) {
         if ((uCellNetGetIpAddressStr(cellHandle, buffer) > 0) &&
-            (uSockStringToAddress(buffer, &address) > 0)) {
+            (uSockStringToAddress(buffer, &address) == 0)) {
             pIpAddress = &address.ipAddress;
         }
+        // Reconnect PPP, making sure not to disable CMUX while doing
+        // so since it is pointless taking it down and up again when
+        // that would also result in the removal of the AT client
+        // that it is actually using at the time
+        uCellPppSetDoNotDisableCmuxOnClose(cellHandle, true);
         uPortPppReconnect(cellHandle, pIpAddress);
+        uCellPppSetDoNotDisableCmuxOnClose(cellHandle, false);
     }
 }
 
@@ -308,6 +323,7 @@ static void setNetworkStatus(uCellPrivateInstance_t *pInstance,
 {
     uCellNetRegistationStatus_t *pStatus;
     bool printAllowed = true;
+    uCellNetRat_t previousRat = pInstance->rat[regType];
 #if U_CFG_OS_CLIB_LEAKS
     // If we're in a URC and the C library leaks memory
     // when printf() is called from a dynamically
@@ -421,12 +437,18 @@ static void setNetworkStatus(uCellPrivateInstance_t *pInstance,
             // support LTE but does support Cat-M1, switch it
             pInstance->rat[regType] = U_CELL_NET_RAT_CATM1;
         }
-        if (pInstance->profileState == U_CELL_PRIVATE_PROFILE_STATE_REQUIRES_REACTIVATION) {
-            // This flag will be set if we had been knocked out
-            // of our PDP context by a network outage and need
-            // to get it back again; make sure to get this in the
-            // queue before any user registratioon status callback
-            // so that everything is sorted for them
+        if ((pInstance->profileState == U_CELL_PRIVATE_PROFILE_STATE_REQUIRES_REACTIVATION) ||
+            ((previousRat != U_CELL_NET_RAT_UNKNOWN_OR_NOT_USED) &&
+             (pInstance->rat[regType] != previousRat) && (uCellPppPrivateIsOpen(pInstance)))) {
+            // profileState will have been set to "reactivation required"
+            // if we had been knocked out of our PDP context by a network
+            // outage and need to get it back again.  We also need to do
+            // this if the module has changed RAT underneath us, potentially
+            // between 2G and LTE, and we have a PPP connection up; the PPP
+            // connection could have become inactive, without notification,
+            // on such a RAT change, activateContextCallback() will reconnect it.
+            // Make sure this is in the queue before any user registration
+            // status callback so that everything is sorted for the user.
             if (!U_CELL_PRIVATE_HAS(pInstance->pModule,
                                     U_CELL_PRIVATE_FEATURE_USE_UPSD_CONTEXT_ACTIVATION)) {
                 // Use the AT client's callback mechanism to do the operation
@@ -725,22 +747,58 @@ static void CSCON_urc(uAtClientHandle_t atHandle, void *pParameter)
     }
 }
 
+// Take action on a PDP context being deactivated by the network/module,
+// which will be signalled by a +UUPSDD or +CGEV URC.
+static void contextDeactivated(uCellPrivateInstance_t *pInstance)
+{
+    if (pInstance->profileState == U_CELL_PRIVATE_PROFILE_STATE_SHOULD_BE_UP) {
+        // Set the state so that, should we re-register with the network,
+        // we will reactivate the internal profile
+        pInstance->profileState = U_CELL_PRIVATE_PROFILE_STATE_REQUIRES_REACTIVATION;
+    }
+}
+
+// Detect a change in the state of context activation
+static void CGEV_urc(uAtClientHandle_t atHandle, void *pParameter)
+{
+    uCellPrivateInstance_t *pInstance = (uCellPrivateInstance_t *) pParameter;
+    char buffer[32];
+    int32_t x;
+    bool pdpContextFailure = false;
+    int32_t failureStringLength;
+
+    x = uAtClientReadString(atHandle, buffer, sizeof(buffer), false);
+
+    // The +CGEV URCs of interest are the ones that indicate a context
+    // has been deactivated, which are:
+    //
+    // NW PDN DEACT <cid>
+    // NW DETACH
+
+    for (size_t y = 0; !pdpContextFailure && (x > 0) &&
+         (y < sizeof(gpCgevPdpContextFailure) /
+          sizeof(gpCgevPdpContextFailure[0])); y++) {
+        failureStringLength = strlen(gpCgevPdpContextFailure[y]);
+        pdpContextFailure = (x >= failureStringLength) &&
+                            (memcmp(buffer, gpCgevPdpContextFailure[y], failureStringLength) == 0);
+    }
+
+    if (pdpContextFailure) {
+        contextDeactivated(pInstance);
+    }
+}
+
 // Detect deactivation of an internal profile, which will occur if we
 // fall out of service.
-static void UUPSDD_urc(uAtClientHandle_t atHandle,
-                       void *pParameter)
+static void UUPSDD_urc(uAtClientHandle_t atHandle, void *pParameter)
 {
     uCellPrivateInstance_t *pInstance = (uCellPrivateInstance_t *) pParameter;
 
     // Skip the parameter; we don't care since we only ever
     // activate a single internal profile
     uAtClientSkipParameters(atHandle, 1);
 
-    if (pInstance->profileState == U_CELL_PRIVATE_PROFILE_STATE_SHOULD_BE_UP) {
-        // Set the state so that, should we re-register with the network,
-        // we will reactivate the internal profile
-        pInstance->profileState = U_CELL_PRIVATE_PROFILE_STATE_REQUIRES_REACTIVATION;
-    }
+    contextDeactivated(pInstance);
 }
 
 /* ----------------------------------------------------------------
@@ -827,6 +885,7 @@ static int32_t prepareConnect(uCellPrivateInstance_t *pInstance)
     uAtClientSetUrcHandler(atHandle, "+CGREG:", CGREG_urc, pInstance);
     uAtClientSetUrcHandler(atHandle, "+CEREG:", CEREG_urc, pInstance);
     uAtClientSetUrcHandler(atHandle, "+UUPSDD:", UUPSDD_urc, pInstance);
+    uAtClientSetUrcHandler(atHandle, "+CGEV:", CGEV_urc, pInstance);
 
     // Switch on the unsolicited result codes for registration
     // and also ask for the additional parameters <lac>, <ci> and
@@ -2401,6 +2460,7 @@ static int32_t disconnect(uCellPrivateInstance_t *pInstance,
         uAtClientRemoveUrcHandler(atHandle, "+CGREG:");
         uAtClientRemoveUrcHandler(atHandle, "+CEREG:");
         uAtClientRemoveUrcHandler(atHandle, "+UUPSDD:");
+        uAtClientRemoveUrcHandler(atHandle, "+CGEV:");
         uPortLog("U_CELL_NET: disconnected.\n");
     } else {
         uPortLog("U_CELL_NET: unable to disconnect.\n");

cell/src/u_cell_ppp.c

@@ -150,6 +150,7 @@ typedef struct {
     bool receiveBufferIsMalloced;
     bool muxAlreadyEnabled;
     bool uartSleepWakeOnDataWasEnabled;
+    bool doNotDisableCmux;
 } uCellPppContext_t;
 
 /* ----------------------------------------------------------------
@@ -347,7 +348,7 @@ static void closePpp(uCellPrivateInstance_t *pInstance,
                     pContext->pDeviceSerial = NULL;
                 }
             }
-            if (!pContext->muxAlreadyEnabled) {
+            if (!pContext->muxAlreadyEnabled && !pContext->doNotDisableCmux) {
                 // Disable the multiplexer if one was in use
                 // and it was us who started it
                 uCellMuxPrivateDisable(pInstance);
@@ -406,6 +407,20 @@ void uCellPppPrivateRemoveContext(uCellPrivateInstance_t *pInstance)
     }
 }
 
+// Determine if PPP is up and running.
+bool uCellPppPrivateIsOpen(uCellPrivateInstance_t *pInstance)
+{
+    bool isRunning = false;
+    uCellPppContext_t *pContext;
+
+    if ((pInstance != NULL) && (pInstance->pPppContext != NULL)) {
+        pContext = (uCellPppContext_t *) pInstance->pPppContext;
+        isRunning = pContext->pDeviceSerial != NULL;
+    }
+
+    return isRunning;
+}
+
 /* ----------------------------------------------------------------
  * PUBLIC FUNCTIONS KEPT WITHIN THE SRC DIRECTORY
  * -------------------------------------------------------------- */
@@ -550,16 +565,14 @@ bool uCellPppIsOpen(uDeviceHandle_t cellHandle)
 {
     bool isRunning = false;
     uCellPrivateInstance_t *pInstance;
-    uCellPppContext_t *pContext;
 
     if (gUCellPrivateMutex != NULL) {
 
         U_PORT_MUTEX_LOCK(gUCellPrivateMutex);
 
         pInstance = pUCellPrivateGetInstance(cellHandle);
-        if ((pInstance != NULL) && (pInstance->pPppContext != NULL)) {
-            pContext = (uCellPppContext_t *) pInstance->pPppContext;
-            isRunning = pContext->pDeviceSerial != NULL;
+        if (pInstance != NULL) {
+            isRunning = uCellPppPrivateIsOpen(pInstance);
         }
 
         U_PORT_MUTEX_UNLOCK(gUCellPrivateMutex);
@@ -596,6 +609,32 @@ int32_t uCellPppClose(uDeviceHandle_t cellHandle,
     return errorCode;
 }
 
+// Set whether CMUX is disabled on PPP closure or not.
+void uCellPppSetDoNotDisableCmuxOnClose(uDeviceHandle_t cellHandle,
+                                        bool doNotDisableCmux)
+{
+    uCellPrivateInstance_t *pInstance;
+    uCellPppContext_t *pContext;
+
+    if (gUCellPrivateMutex != NULL) {
+
+        U_PORT_MUTEX_LOCK(gUCellPrivateMutex);
+
+        pInstance = pUCellPrivateGetInstance(cellHandle);
+        if (pInstance != NULL) {
+            if (U_CELL_PRIVATE_HAS(pInstance->pModule,
+                                   U_CELL_PRIVATE_FEATURE_PPP)) {
+                pContext = (uCellPppContext_t *) pInstance->pPppContext;
+                if (pContext != NULL) {
+                    pContext->doNotDisableCmux = doNotDisableCmux;
+                }
+            }
+        }
+
+        U_PORT_MUTEX_UNLOCK(gUCellPrivateMutex);
+    }
+}
+
 // Transmit a buffer of data over the PPP interface.
 int32_t uCellPppTransmit(uDeviceHandle_t cellHandle,
                          const char *pData, size_t dataSize)

cell/src/u_cell_ppp_private.h

@@ -57,6 +57,15 @@ extern "C" {
  */
 void uCellPppPrivateRemoveContext(uCellPrivateInstance_t *pInstance);
 
+/** Determine if a PPP connection is currently up.
+ *
+ * Note: gUCellPrivateMutex should be locked before this is called.
+ *
+ * @param[in] pInstance a pointer to the cellular instance.
+ * @return    true if a PPP connection is up, else false.
+ */
+bool uCellPppPrivateIsOpen(uCellPrivateInstance_t *pInstance);
+
 #ifdef __cplusplus
 }
 #endif

cell/src/u_cell_ppp_shared.h

@@ -158,6 +158,26 @@ bool uCellPppIsOpen(uDeviceHandle_t cellHandle);
 int32_t uCellPppClose(uDeviceHandle_t cellHandle,
                       bool pppTerminateRequired);
 
+/** Normally, closure of PPP would also cause CMUX to be
+ * deactivated if this code knows that CMUX was only
+ * activated in order to run PPP.  This is unnecesary if the
+ * closure of PPP is only temporary, e.g. to recover from a
+ * service outage, and it is going to be brought back up again
+ * immediately.  Call this function with doNotDisableCmux set
+ * to true before calling uCellPppClose() where this is the
+ * case (though don't forget to call it again with
+ * doNotDisableCmux set to false before you call uCellPppClose()
+ * with no intention of calling uCellPppOpen() again afterwards).
+ *
+ * @param cellHandle         the handle of the cellular instance.
+ * @param doNotDisableCmux   if true then uCellPppClose() will
+ *                           leave CMUX up, else it will close
+ *                           CMUX if CMUX was only brought up
+ *                           to run PPP.
+ */
+void uCellPppSetDoNotDisableCmuxOnClose(uDeviceHandle_t cellHandle,
+                                        bool doNotDisableCmux);
+
 /** Transmit data over the PPP interface of the cellular module.
  * This may be integrated into a higher layer, e.g. the PPP
  * interface at the bottom of an IP stack, to permit it to send

common/network/api/u_network.h

@@ -56,14 +56,6 @@
  *                          powered-up and may be reconfigured etc.: you
  *                          must call uNetworkInterfaceUp() to talk with
  *                          it again.
- *                          IMPORTANT: under some circumstances, e.g. when
- *                          using a GNSS device inside a cellular module
- *                          or when using PPP with cellular,
- *                          uNetworkInterfaceUp() may leave a mutex locked
- *                          that will be released by uNetworkInterfaceDown(),
- *                          hence it is important that uNetworkInterfaceDown()
- *                          is called from the same task that called
- *                          uNetworkInterfaceUp().
  * uDeviceClose():          call this to power the device down and clear
  *                          up any resources belonging to it; uDeviceOpen()
  *                          must be called to re-instantiate the device.
@@ -236,21 +228,6 @@ int32_t uNetworkInterfaceUp(uDeviceHandle_t devHandle, uNetworkType_t netType,
  * Note: for a Wi-Fi network, this function uses the
  * uWifiSetConnectionStatusCallback() callback.
  *
- * Note: if you call uNetworkInterfaceDown() on a GNSS network that
- * is inside a cellular device, or if you have been using PPP with
- * a cellular module (i.e. U_CFG_PPP_ENABLE is defined), it is possible
- * your RTOS will assert that a mutex is being released by a task that
- * does not own it; for example FreeRTOS may do this.  The reason for
- * this is that, when uNetworkInterfaceUp() was called, the cellular
- * module will have been told to create a CMUX channel (to carry AT
- * and either PPP or GNSS traffic simultaneously) and the original AT
- * client will have been left locked.  If uNetworkInterfaceDown() is
- * called from a _different_ _task_ to the one that called
- * uNetworkInterfaceUp(), the assert will be triggered when the original
- * AT client is unlocked.  A fix for this is to call uNetworkInterfaceDown()
- * from the same task that called uNetworkInterfaceUp().  We are
- * investigating whether there is a way to remove this restriction.
- *
  * @param devHandle the handle of the device that is carrying the
  *                  network.
  * @param netType   which of the module interfaces to take down.