[PATCH] broadband-bearer-icera: deactivate context before authentication

[Dan Williams]

On Fri, 2016-05-13 at 20:05 +0200, Aleksander Morgado wrote:
> On Wed, May 11, 2016 at 11:25 PM, Dan Williams <dcbw at redhat.com>
> wrote:
> > 
> > If the modem thinks a PDP context is already active it'll return
> > 583 errors from IPDPCFG and IPDPACT until the context is
> > deactivated.  Deactivation was previously done after
> > authentication,
> > but needs to be done before any part of the connect process to
> > ensure the PDP context is inactive.
> > 
> > The previous approach worked only if the context was being
> > deactivated already (which can take a bit of time) because it would
> > be deactivated after a few seconds and the connect could continue.
> > This approach works for more cases (like a MM crash and restart
> > while the modem is connected).
> > ---
> LGTM.

Thanks; pushed to git master and mm-1-4.

Dan

> > 
> >  plugins/icera/mm-broadband-bearer-icera.c | 99 ++++++++++++++++---
> > ------------
> >  1 file changed, 52 insertions(+), 47 deletions(-)
> > 
> > diff --git a/plugins/icera/mm-broadband-bearer-icera.c
> > b/plugins/icera/mm-broadband-bearer-icera.c
> > index 95e5722..ec5779c 100644
> > --- a/plugins/icera/mm-broadband-bearer-icera.c
> > +++ b/plugins/icera/mm-broadband-bearer-icera.c
> > @@ -752,43 +752,6 @@ activate_ready (MMBaseModem *modem,
> >                                                             self);
> >  }
> > 
> > -static void
> > -deactivate_ready (MMBaseModem *modem,
> > -                  GAsyncResult *res,
> > -                  Dial3gppContext *ctx)
> > -{
> > -    gchar *command;
> > -
> > -    /*
> > -     * Ignore any error here; %IPDPACT=ctx,0 will produce an error
> > 767
> > -     * if the context is not, in fact, connected. This is annoying
> > but
> > -     * harmless.
> > -     */
> > -    mm_base_modem_at_command_full_finish (modem, res, NULL);
> > -
> > -    /* The unsolicited response to %IPDPACT may come before the OK
> > does.
> > -     * We will keep the connection context in the bearer private
> > data so
> > -     * that it is accessible from the unsolicited message handler.
> > Note
> > -     * also that we do NOT pass the ctx to the
> > GAsyncReadyCallback, as it
> > -     * may not be valid any more when the callback is called (it
> > may be
> > -     * already completed in the unsolicited handling) */
> > -    g_assert (ctx->self->priv->connect_pending == NULL);
> > -    ctx->self->priv->connect_pending = ctx;
> > -
> > -    command = g_strdup_printf ("%%IPDPACT=%d,1", ctx->cid);
> > -    mm_base_modem_at_command_full (
> > -        ctx->modem,
> > -        ctx->primary,
> > -        command,
> > -        60,
> > -        FALSE,
> > -        FALSE, /* raw */
> > -        NULL, /* cancellable */
> > -        (GAsyncReadyCallback)activate_ready,
> > -        g_object_ref (ctx->self)); /* we pass the bearer object!
> > */
> > -    g_free (command);
> > -}
> > -
> >  static void authenticate (Dial3gppContext *ctx);
> > 
> >  static gboolean
> > @@ -827,13 +790,16 @@ authenticate_ready (MMBaseModem *modem,
> >          return;
> >      }
> > 
> > -    /*
> > -     * Deactivate the context we want to use before we try to
> > activate
> > -     * it. This handles the case where ModemManager crashed while
> > -     * connected and is now trying to reconnect. (Should some part
> > of
> > -     * the core or modem driver have made sure of this already?)
> > -     */
> > -    command = g_strdup_printf ("%%IPDPACT=%d,0", ctx->cid);
> > +    /* The unsolicited response to %IPDPACT may come before the OK
> > does.
> > +     * We will keep the connection context in the bearer private
> > data so
> > +     * that it is accessible from the unsolicited message handler.
> > Note
> > +     * also that we do NOT pass the ctx to the
> > GAsyncReadyCallback, as it
> > +     * may not be valid any more when the callback is called (it
> > may be
> > +     * already completed in the unsolicited handling) */
> > +    g_assert (ctx->self->priv->connect_pending == NULL);
> > +    ctx->self->priv->connect_pending = ctx;
> > +
> > +    command = g_strdup_printf ("%%IPDPACT=%d,1", ctx->cid);
> >      mm_base_modem_at_command_full (
> >          ctx->modem,
> >          ctx->primary,
> > @@ -842,8 +808,8 @@ authenticate_ready (MMBaseModem *modem,
> >          FALSE,
> >          FALSE, /* raw */
> >          NULL, /* cancellable */
> > -        (GAsyncReadyCallback)deactivate_ready,
> > -        ctx);
> > +        (GAsyncReadyCallback)activate_ready,
> > +        g_object_ref (ctx->self)); /* we pass the bearer object!
> > */
> >      g_free (command);
> >  }
> > 
> > @@ -913,6 +879,45 @@ authenticate (Dial3gppContext *ctx)
> >  }
> > 
> >  static void
> > +deactivate_ready (MMBaseModem *modem,
> > +                  GAsyncResult *res,
> > +                  Dial3gppContext *ctx)
> > +{
> > +    /*
> > +     * Ignore any error here; %IPDPACT=ctx,0 will produce an error
> > 767
> > +     * if the context is not, in fact, connected. This is annoying
> > but
> > +     * harmless.
> > +     */
> > +    mm_base_modem_at_command_full_finish (modem, res, NULL);
> > +
> > +    authenticate (ctx);
> > +}
> > +
> > +static void
> > +connect_deactivate (Dial3gppContext *ctx)
> > +{
> > +    gchar *command;
> > +
> > +    /* Deactivate the context we want to use before we try to
> > activate
> > +     * it. This handles the case where ModemManager crashed while
> > +     * connected and is now trying to reconnect. (Should some part
> > of
> > +     * the core or modem driver have made sure of this already?)
> > +     */
> > +    command = g_strdup_printf ("%%IPDPACT=%d,0", ctx->cid);
> > +    mm_base_modem_at_command_full (
> > +        ctx->modem,
> > +        ctx->primary,
> > +        command,
> > +        60,
> > +        FALSE,
> > +        FALSE, /* raw */
> > +        NULL, /* cancellable */
> > +        (GAsyncReadyCallback)deactivate_ready,
> > +        ctx);
> > +    g_free (command);
> > +}
> > +
> > +static void
> >  dial_3gpp (MMBroadbandBearer *self,
> >             MMBaseModem *modem,
> >             MMPortSerialAt *primary,
> > @@ -948,7 +953,7 @@ dial_3gpp (MMBroadbandBearer *self,
> >          return;
> >      }
> > 
> > -    authenticate (ctx);
> > +    connect_deactivate (ctx);
> >  }
> > 
> >  /*****************************************************************
> > ************/
> > --
> > 2.5.5
> > _______________________________________________
> > ModemManager-devel mailing list
> > ModemManager-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
> 
>