[PATCH c2] huawei: handle disconnection via ^NDISSTAT unsolicited message

Aleksander Morgado aleksander at lanedo.com
Tue Sep 17 23:48:12 PDT 2013


On 18/09/13 05:45, Ben Chan wrote:
> This patch changes MMBroadbandModemHuawei to use ^NDISSTAT unsolicited
> messages to handle network-initiated disconnection. As a ^NDISSTAT
> unsolicited message is similar to a ^NDISSTATQRY response, the patch
> extends the ^NDISSTATQRY parser code to handle both ^NDISSTAT and
> ^NDISSTATQRY responses.


Still, isn't it possible to avoid the _is_connect_pending() and
_is_disconnect_pending() calls? Whenever a NDISSTAT message arrives,the
modem object can report the new connection status to the bearer, and
then the bearer decide internally whether to ignore the reported status
(e.g. if a connect or disconnect was pending). Check e.g.
mm_broadband_bearer_hso_report_connection_status() in the option/hso
plugin. I think that would simplify the logic as it would only be in one
place (the bearer) as opposed to in two places (modem and bearer). At
the end, the idea of ignoring NDISSTAT unsolicited messages during a
connection/disconnection sequence is applicable to the actual
connection/disconnection logic, which is implemented in the bearer.


> ---
>  plugins/huawei/mm-broadband-bearer-huawei.c      | 18 +++++
>  plugins/huawei/mm-broadband-bearer-huawei.h      |  3 +
>  plugins/huawei/mm-broadband-modem-huawei.c       | 92 +++++++++++++++++++++---
>  plugins/huawei/mm-modem-helpers-huawei.c         | 18 +++--
>  plugins/huawei/mm-modem-helpers-huawei.h         |  2 +-
>  plugins/huawei/tests/test-modem-helpers-huawei.c | 34 ++++++++-
>  6 files changed, 150 insertions(+), 17 deletions(-)
> 
> diff --git a/plugins/huawei/mm-broadband-bearer-huawei.c b/plugins/huawei/mm-broadband-bearer-huawei.c
> index eec37f3..569ddf1 100644
> --- a/plugins/huawei/mm-broadband-bearer-huawei.c
> +++ b/plugins/huawei/mm-broadband-bearer-huawei.c
> @@ -639,6 +639,24 @@ disconnect_3gpp (MMBroadbandBearer *self,
>  
>  /*****************************************************************************/
>  
> +gboolean
> +mm_broadband_bearer_huawei_is_connect_pending (MMBroadbandBearerHuawei *self)
> +{
> +    g_return_val_if_fail (MM_BROADBAND_BEARER_HUAWEI (self), FALSE);
> +
> +    return !!self->priv->connect_pending;
> +}
> +
> +gboolean
> +mm_broadband_bearer_huawei_is_disconnect_pending (MMBroadbandBearerHuawei *self)
> +{
> +    g_return_val_if_fail (MM_BROADBAND_BEARER_HUAWEI (self), FALSE);
> +
> +    return !!self->priv->disconnect_pending;
> +}
> +
> +/*****************************************************************************/
> +
>  MMBearer *
>  mm_broadband_bearer_huawei_new_finish (GAsyncResult *res,
>                                         GError **error)
> diff --git a/plugins/huawei/mm-broadband-bearer-huawei.h b/plugins/huawei/mm-broadband-bearer-huawei.h
> index 4c87d9c..3f514fb 100644
> --- a/plugins/huawei/mm-broadband-bearer-huawei.h
> +++ b/plugins/huawei/mm-broadband-bearer-huawei.h
> @@ -56,4 +56,7 @@ void      mm_broadband_bearer_huawei_new        (MMBroadbandModemHuawei *modem,
>  MMBearer *mm_broadband_bearer_huawei_new_finish (GAsyncResult *res,
>                                                   GError **error);
>  
> +gboolean mm_broadband_bearer_huawei_is_connect_pending (MMBroadbandBearerHuawei *self);
> +gboolean mm_broadband_bearer_huawei_is_disconnect_pending (MMBroadbandBearerHuawei *self);
> +
>  #endif /* MM_BROADBAND_BEARER_HUAWEI_H */
> diff --git a/plugins/huawei/mm-broadband-modem-huawei.c b/plugins/huawei/mm-broadband-modem-huawei.c
> index 73323e2..614df7c 100644
> --- a/plugins/huawei/mm-broadband-modem-huawei.c
> +++ b/plugins/huawei/mm-broadband-modem-huawei.c
> @@ -34,6 +34,7 @@
>  #include "mm-log.h"
>  #include "mm-errors-types.h"
>  #include "mm-modem-helpers.h"
> +#include "mm-modem-helpers-huawei.h"
>  #include "mm-base-modem-at.h"
>  #include "mm-iface-modem.h"
>  #include "mm-iface-modem-3gpp.h"
> @@ -43,6 +44,7 @@
>  #include "mm-broadband-modem-huawei.h"
>  #include "mm-broadband-bearer-huawei.h"
>  #include "mm-broadband-bearer.h"
> +#include "mm-bearer-list.h"
>  #include "mm-sim-huawei.h"
>  
>  static void iface_modem_init (MMIfaceModem *iface);
> @@ -85,6 +87,7 @@ struct _MMBroadbandModemHuaweiPrivate {
>  
>      /* Regex for connection status related notifications */
>      GRegex *dsflowrpt_regex;
> +    GRegex *ndisstat_regex;
>  
>      /* Regex to ignore */
>      GRegex *boot_regex;
> @@ -97,7 +100,6 @@ struct _MMBroadbandModemHuaweiPrivate {
>      GRegex *srvst_regex;
>      GRegex *stin_regex;
>      GRegex *hcsq_regex;
> -    GRegex *ndisstat_regex;
>      GRegex *pdpdeact_regex;
>      GRegex *ndisend_regex;
>      GRegex *rfswitch_regex;
> @@ -1510,6 +1512,77 @@ huawei_status_changed (MMAtSerialPort *port,
>      g_free (str);
>  }
>  
> +typedef struct {
> +    gboolean ipv4_available;
> +    gboolean ipv4_connected;
> +    gboolean ipv6_available;
> +    gboolean ipv6_connected;
> +} NdisstatResult;
> +
> +static void
> +bearer_report_connection_status (MMBearer *bearer,
> +                                 NdisstatResult *ndisstat_result)
> +{
> +    /* When a pending connection / disconnection attempt is in progress, we use
> +     * ^NDISSTATQRY? to check the connection status and thus temporarily ignore
> +     * ^NDISSTAT unsolicited messages */
> +    if (mm_broadband_bearer_huawei_is_connect_pending (MM_BROADBAND_BEARER_HUAWEI (bearer)) ||
> +        mm_broadband_bearer_huawei_is_disconnect_pending (MM_BROADBAND_BEARER_HUAWEI (bearer)))
> +        return;
> +
> +    /* We already use ^NDISSTATQRY? to poll the connection status, so we only
> +     * handle network-initiated disconnection here. */
> +    if (ndisstat_result->ipv4_available && !ndisstat_result->ipv4_connected) {
> +        /* TODO: MMBroadbandBearerHuawei does not currently support IPv6.
> +         * When it does, we should check the IP family associated with each bearer. */
> +        mm_dbg ("Disconnect bearer '%s'", mm_bearer_get_path (bearer));
> +        mm_bearer_report_disconnection (bearer);
> +    }
> +}
> +
> +static void
> +huawei_ndisstat_changed (MMAtSerialPort *port,
> +                         GMatchInfo *match_info,
> +                         MMBroadbandModemHuawei *self)
> +{
> +    gchar *str;
> +    NdisstatResult ndisstat_result;
> +    GError *error = NULL;
> +    MMBearerList *list = NULL;
> +
> +    str = g_match_info_fetch (match_info, 1);
> +    if (!mm_huawei_parse_ndisstatqry_response (str,
> +                                               &ndisstat_result.ipv4_available,
> +                                               &ndisstat_result.ipv4_connected,
> +                                               &ndisstat_result.ipv6_available,
> +                                               &ndisstat_result.ipv6_connected,
> +                                               &error)) {
> +        mm_dbg ("Ignore invalid ^NDISSTAT unsolicited message: '%s' (error %s)",
> +                str, error->message);
> +        g_error_free (error);
> +        return;
> +    }
> +
> +    mm_dbg ("NDIS status: IPv4 %s, IPv6 %s",
> +            ndisstat_result.ipv4_available ?
> +            (ndisstat_result.ipv4_connected ? "connected" : "disconnected") : "not available",
> +            ndisstat_result.ipv6_available ?
> +            (ndisstat_result.ipv6_connected ? "connected" : "disconnected") : "not available");
> +
> +    /* If empty bearer list, nothing else to do */
> +    g_object_get (self,
> +                  MM_IFACE_MODEM_BEARER_LIST, &list,
> +                  NULL);
> +    if (!list)
> +        return;
> +
> +    mm_bearer_list_foreach (list,
> +                            (MMBearerListForeachFunc)bearer_report_connection_status,
> +                            &ndisstat_result);
> +
> +    g_object_unref (list);
> +}
> +
>  static void
>  set_3gpp_unsolicited_events_handlers (MMBroadbandModemHuawei *self,
>                                        gboolean enable)
> @@ -1548,6 +1621,13 @@ set_3gpp_unsolicited_events_handlers (MMBroadbandModemHuawei *self,
>              enable ? (MMAtSerialUnsolicitedMsgFn)huawei_status_changed : NULL,
>              enable ? self : NULL,
>              NULL);
> +
> +        mm_at_serial_port_add_unsolicited_msg_handler (
> +            ports[i],
> +            self->priv->ndisstat_regex,
> +            enable ? (MMAtSerialUnsolicitedMsgFn)huawei_ndisstat_changed : NULL,
> +            enable ? self : NULL,
> +            NULL);
>      }
>  }
>  
> @@ -2992,10 +3072,6 @@ set_ignored_unsolicited_events_handlers (MMBroadbandModemHuawei *self)
>              NULL, NULL, NULL);
>          mm_at_serial_port_add_unsolicited_msg_handler (
>              ports[i],
> -            self->priv->ndisstat_regex,
> -            NULL, NULL, NULL);
> -        mm_at_serial_port_add_unsolicited_msg_handler (
> -            ports[i],
>              self->priv->pdpdeact_regex,
>              NULL, NULL, NULL);
>          mm_at_serial_port_add_unsolicited_msg_handler (
> @@ -3063,6 +3139,8 @@ mm_broadband_modem_huawei_init (MMBroadbandModemHuawei *self)
>                                            G_REGEX_RAW | G_REGEX_OPTIMIZE, 0, NULL);
>      self->priv->dsflowrpt_regex = g_regex_new ("\\r\\n\\^DSFLOWRPT:(.+)\\r\\n",
>                                                 G_REGEX_RAW | G_REGEX_OPTIMIZE, 0, NULL);
> +    self->priv->ndisstat_regex = g_regex_new ("\\r\\n(\\^NDISSTAT:.+)\\r+\\n",
> +                                              G_REGEX_RAW | G_REGEX_OPTIMIZE, 0, NULL);
>      self->priv->boot_regex = g_regex_new ("\\r\\n\\^BOOT:.+\\r\\n",
>                                            G_REGEX_RAW | G_REGEX_OPTIMIZE, 0, NULL);
>      self->priv->connect_regex = g_regex_new ("\\r\\n\\^CONNECT .+\\r\\n",
> @@ -3083,8 +3161,6 @@ mm_broadband_modem_huawei_init (MMBroadbandModemHuawei *self)
>                                            G_REGEX_RAW | G_REGEX_OPTIMIZE, 0, NULL);
>      self->priv->hcsq_regex = g_regex_new ("\\r\\n\\^HCSQ:.+\\r+\\n",
>                                            G_REGEX_RAW | G_REGEX_OPTIMIZE, 0, NULL);
> -    self->priv->ndisstat_regex = g_regex_new ("\\r\\n\\^NDISSTAT:.+\\r+\\n",
> -                                              G_REGEX_RAW | G_REGEX_OPTIMIZE, 0, NULL);
>      self->priv->pdpdeact_regex = g_regex_new ("\\r\\n\\^PDPDEACT:.+\\r+\\n",
>                                                G_REGEX_RAW | G_REGEX_OPTIMIZE, 0, NULL);
>      self->priv->ndisend_regex = g_regex_new ("\\r\\n\\^NDISEND:.+\\r+\\n",
> @@ -3109,6 +3185,7 @@ finalize (GObject *object)
>      g_regex_unref (self->priv->hrssilvl_regex);
>      g_regex_unref (self->priv->mode_regex);
>      g_regex_unref (self->priv->dsflowrpt_regex);
> +    g_regex_unref (self->priv->ndisstat_regex);
>      g_regex_unref (self->priv->boot_regex);
>      g_regex_unref (self->priv->connect_regex);
>      g_regex_unref (self->priv->csnr_regex);
> @@ -3119,7 +3196,6 @@ finalize (GObject *object)
>      g_regex_unref (self->priv->srvst_regex);
>      g_regex_unref (self->priv->stin_regex);
>      g_regex_unref (self->priv->hcsq_regex);
> -    g_regex_unref (self->priv->ndisstat_regex);
>      g_regex_unref (self->priv->pdpdeact_regex);
>      g_regex_unref (self->priv->ndisend_regex);
>      g_regex_unref (self->priv->rfswitch_regex);
> diff --git a/plugins/huawei/mm-modem-helpers-huawei.c b/plugins/huawei/mm-modem-helpers-huawei.c
> index f5abe57..bb977c2 100644
> --- a/plugins/huawei/mm-modem-helpers-huawei.c
> +++ b/plugins/huawei/mm-modem-helpers-huawei.c
> @@ -23,7 +23,7 @@
>  #include "mm-modem-helpers-huawei.h"
>  
>  /*****************************************************************************/
> -/* ^NDISSTATQRY response parser */
> +/* ^NDISSTAT /  ^NDISSTATQRY response parser */
>  
>  gboolean
>  mm_huawei_parse_ndisstatqry_response (const gchar *response,
> @@ -37,8 +37,10 @@ mm_huawei_parse_ndisstatqry_response (const gchar *response,
>      GMatchInfo *match_info;
>      GError *inner_error = NULL;
>  
> -    if (!response || !g_str_has_prefix (response, "^NDISSTATQRY:")) {
> -        g_set_error (error, MM_CORE_ERROR, MM_CORE_ERROR_FAILED, "Missing ^NDISSTATQRY prefix");
> +    if (!response ||
> +        !(g_str_has_prefix (response, "^NDISSTAT:") ||
> +          g_str_has_prefix (response, "^NDISSTATQRY:"))) {
> +        g_set_error (error, MM_CORE_ERROR, MM_CORE_ERROR_FAILED, "Missing ^NDISSTAT / ^NDISSTATQRY prefix");
>          return FALSE;
>      }
>  
> @@ -46,6 +48,8 @@ mm_huawei_parse_ndisstatqry_response (const gchar *response,
>      *ipv6_available = FALSE;
>  
>      /* The response maybe as:
> +     *     ^NDISSTAT: 1,,,IPV4
> +     *     ^NDISSTAT: 0,33,,IPV6
>       *     ^NDISSTATQRY: 1,,,IPV4
>       *     ^NDISSTATQRY: 0,33,,IPV6
>       *     OK
> @@ -54,8 +58,8 @@ mm_huawei_parse_ndisstatqry_response (const gchar *response,
>       *     ^NDISSTATQRY:0,,,"IPV4",0,,,"IPV6"
>       *     OK
>       */
> -    r = g_regex_new ("\\^NDISSTATQRY:\\s*(\\d),([^,]*),([^,]*),([^,\\r\\n]*)(?:\\r\\n)?"
> -                     "(?:\\^NDISSTATQRY:)?\\s*,?(\\d)?,?([^,]*)?,?([^,]*)?,?([^,\\r\\n]*)?(?:\\r\\n)?",
> +    r = g_regex_new ("\\^NDISSTAT(?:QRY)?:\\s*(\\d),([^,]*),([^,]*),([^,\\r\\n]*)(?:\\r\\n)?"
> +                     "(?:\\^NDISSTAT:|\\^NDISSTATQRY:)?\\s*,?(\\d)?,?([^,]*)?,?([^,]*)?,?([^,\\r\\n]*)?(?:\\r\\n)?",
>                       G_REGEX_DOLLAR_ENDONLY | G_REGEX_RAW,
>                       0, NULL);
>      g_assert (r != NULL);
> @@ -78,7 +82,7 @@ mm_huawei_parse_ndisstatqry_response (const gchar *response,
>                  (connected != 0 && connected != 1)) {
>                  inner_error = g_error_new (MM_CORE_ERROR,
>                                             MM_CORE_ERROR_FAILED,
> -                                           "Couldn't parse ^NDISSTATQRY fields");
> +                                           "Couldn't parse ^NDISSTAT / ^NDISSTATQRY fields");
>              } else if (g_ascii_strcasecmp (ip_type_str, "IPV4") == 0) {
>                  *ipv4_available = TRUE;
>                  *ipv4_connected = (gboolean)connected;
> @@ -98,7 +102,7 @@ mm_huawei_parse_ndisstatqry_response (const gchar *response,
>      if (!ipv4_available && !ipv6_available) {
>          inner_error = g_error_new (MM_CORE_ERROR,
>                                     MM_CORE_ERROR_FAILED,
> -                                   "Couldn't find IPv4 or IPv6 info in ^NDISSTATQRY response");
> +                                   "Couldn't find IPv4 or IPv6 info in ^NDISSTAT / ^NDISSTATQRY response");
>      }
>  
>      if (inner_error) {
> diff --git a/plugins/huawei/mm-modem-helpers-huawei.h b/plugins/huawei/mm-modem-helpers-huawei.h
> index cc32087..4d15e8a 100644
> --- a/plugins/huawei/mm-modem-helpers-huawei.h
> +++ b/plugins/huawei/mm-modem-helpers-huawei.h
> @@ -19,7 +19,7 @@
>  
>  #include "glib.h"
>  
> -/* ^NDISSTATQRY response parser */
> +/* ^NDISSTAT / ^NDISSTATQRY response parser */
>  gboolean mm_huawei_parse_ndisstatqry_response (const gchar *response,
>                                                 gboolean *ipv4_available,
>                                                 gboolean *ipv4_connected,
> diff --git a/plugins/huawei/tests/test-modem-helpers-huawei.c b/plugins/huawei/tests/test-modem-helpers-huawei.c
> index ea4cd95..8d99215 100644
> --- a/plugins/huawei/tests/test-modem-helpers-huawei.c
> +++ b/plugins/huawei/tests/test-modem-helpers-huawei.c
> @@ -20,7 +20,7 @@
>  #include "mm-modem-helpers-huawei.h"
>  
>  /*****************************************************************************/
> -/* Test ^NDISSTATQRY responses */
> +/* Test ^NDISSTAT / ^NDISSTATQRY responses */
>  
>  typedef struct {
>      const gchar *str;
> @@ -31,6 +31,38 @@ typedef struct {
>  } NdisstatqryTest;
>  
>  static const NdisstatqryTest ndisstatqry_tests[] = {
> +    { "^NDISSTAT: 1,,,IPV4\r\n", TRUE,  TRUE,  FALSE, FALSE },
> +    { "^NDISSTAT: 0,,,IPV4\r\n", TRUE,  FALSE, FALSE, FALSE },
> +    { "^NDISSTAT: 1,,,IPV6\r\n", FALSE, FALSE, TRUE,  TRUE  },
> +    { "^NDISSTAT: 0,,,IPV6\r\n", FALSE, FALSE, TRUE,  FALSE },
> +    { "^NDISSTAT: 1,,,IPV4\r\n"
> +      "^NDISSTAT: 1,,,IPV6\r\n", TRUE,  TRUE,  TRUE,  TRUE  },
> +    { "^NDISSTAT: 1,,,IPV4\r\n"
> +      "^NDISSTAT: 0,,,IPV6\r\n", TRUE,  TRUE,  TRUE,  FALSE },
> +    { "^NDISSTAT: 0,,,IPV4\r\n"
> +      "^NDISSTAT: 1,,,IPV6\r\n", TRUE,  FALSE, TRUE,  TRUE  },
> +    { "^NDISSTAT: 0,,,IPV4\r\n"
> +      "^NDISSTAT: 0,,,IPV6\r\n", TRUE,  FALSE, TRUE,  FALSE },
> +    { "^NDISSTAT: 1,,,IPV4",     TRUE,  TRUE,  FALSE, FALSE },
> +    { "^NDISSTAT: 0,,,IPV4",     TRUE,  FALSE, FALSE, FALSE },
> +    { "^NDISSTAT: 1,,,IPV6",     FALSE, FALSE, TRUE,  TRUE  },
> +    { "^NDISSTAT: 0,,,IPV6",     FALSE, FALSE, TRUE,  FALSE },
> +    { "^NDISSTAT: 1,,,IPV4\r\n"
> +      "^NDISSTAT: 1,,,IPV6",     TRUE,  TRUE,  TRUE,  TRUE  },
> +    { "^NDISSTAT: 1,,,IPV4\r\n"
> +      "^NDISSTAT: 0,,,IPV6",     TRUE,  TRUE,  TRUE,  FALSE },
> +    { "^NDISSTAT: 0,,,IPV4\r\n"
> +      "^NDISSTAT: 1,,,IPV6",     TRUE,  FALSE, TRUE,  TRUE  },
> +    { "^NDISSTAT: 0,,,IPV4\r\n"
> +      "^NDISSTAT: 0,,,IPV6",     TRUE,  FALSE, TRUE,  FALSE },
> +    { "^NDISSTAT: 1,,,\"IPV4\",1,,,\"IPV6\"",     TRUE,  TRUE,  TRUE,  TRUE  },
> +    { "^NDISSTAT: 1,,,\"IPV4\",0,,,\"IPV6\"",     TRUE,  TRUE,  TRUE,  FALSE },
> +    { "^NDISSTAT: 0,,,\"IPV4\",1,,,\"IPV6\"",     TRUE,  FALSE, TRUE,  TRUE  },
> +    { "^NDISSTAT: 0,,,\"IPV4\",0,,,\"IPV6\"",     TRUE,  FALSE, TRUE,  FALSE },
> +    { "^NDISSTAT: 1,,,\"IPV4\",1,,,\"IPV6\"\r\n", TRUE,  TRUE,  TRUE,  TRUE  },
> +    { "^NDISSTAT: 1,,,\"IPV4\",0,,,\"IPV6\"\r\n", TRUE,  TRUE,  TRUE,  FALSE },
> +    { "^NDISSTAT: 0,,,\"IPV4\",1,,,\"IPV6\"\r\n", TRUE,  FALSE, TRUE,  TRUE  },
> +    { "^NDISSTAT: 0,,,\"IPV4\",0,,,\"IPV6\"\r\n", TRUE,  FALSE, TRUE,  FALSE },
>      { "^NDISSTATQRY: 1,,,IPV4\r\n", TRUE,  TRUE,  FALSE, FALSE },
>      { "^NDISSTATQRY: 0,,,IPV4\r\n", TRUE,  FALSE, FALSE, FALSE },
>      { "^NDISSTATQRY: 1,,,IPV6\r\n", FALSE, FALSE, TRUE,  TRUE  },
> 


-- 
Aleksander


More information about the ModemManager-devel mailing list