Re: [MM] [PATCH v2] iface-modem-3gpp: clear deferred registration state update when disabling

[Aleksander Morgado <aleksander lanedo com>]

On 03/01/2013 06:09 PM, Ben Chan wrote:
> Yep, that should work.

Pushed that fix as well; thanks for reviewing it.

Cheers!

> On Fri, Mar 1, 2013 at 12:18 AM, Aleksander Morgado
> <aleksander lanedo com <mailto:aleksander lanedo com>> wrote:
>
>     On 03/01/2013 09:06 AM, Ben Chan wrote:
>     >
>     >
>     >
>     > On Thu, Feb 28, 2013 at 11:46 PM, Aleksander Morgado
>     > <aleksander lanedo com <mailto:aleksander lanedo com>
>     <mailto:aleksander lanedo com <mailto:aleksander lanedo com>>> wrote:
>     >
>     >     On 02/28/2013 08:11 PM, Ben Chan wrote:
>     >     > ---
>     >     >  src/mm-iface-modem-3gpp.c | 15 +++++++++++++++
>     >     >  1 file changed, 15 insertions(+)
>     >     >
>     >
>     >     I pushed it after modifying it to include the
>     >     clear_deferred_registration_state_update() call in the first
>     >     DISABLING_STEP_PERIODIC_REGISTRATION_CHECKS step. As said
>     previously, we
>     >     don't really mind if we get an unsolicited registration event
>     during the
>     >     disabling process, as we're disabling anyway.
>     >
>     >
>     > I'm a bit confused. If an unsolicited registration event is received
>     > after clearing deferred_update_id, deferred_update_id is set to a new
>     > value, which will be scheduled after the modem is disabled. That will
>     > cause the modem state to become enabled due to the following code in
>     > mm-iface-modem-3gpp.c:
>     >
>     >     mm_iface_modem_update_subsystem_state (
>     >         MM_IFACE_MODEM (self),
>     >         SUBSYSTEM_3GPP,
>     >         (new_state == MM_MODEM_3GPP_REGISTRATION_STATE_SEARCHING ?
>     >          MM_MODEM_STATE_SEARCHING :
>     >          MM_MODEM_STATE_ENABLED),
>     >         MM_MODEM_STATE_CHANGE_REASON_UNKNOWN);
>     >
>
>     Ah, good point; didn't consider that the deferred update could get
>     re-scheduled... In that case we should actually clear it *after*
>     DISABLING_STEP_CLEANUP_UNSOLICITED_REGISTRATION_EVENTS, once we know we
>     will not process those unsolicited events. How does this look?
>
>     diff --git a/src/mm-iface-modem-3gpp.c b/src/mm-iface-modem-3gpp.c
>     index 1d3fdce..6c8bb14 100644
>     --- a/src/mm-iface-modem-3gpp.c
>     +++ b/src/mm-iface-modem-3gpp.c
>     @@ -1347,6 +1347,7 @@ typedef enum {
>          DISABLING_STEP_PERIODIC_REGISTRATION_CHECKS,
>          DISABLING_STEP_DISABLE_UNSOLICITED_REGISTRATION_EVENTS,
>          DISABLING_STEP_CLEANUP_UNSOLICITED_REGISTRATION_EVENTS,
>     +    DISABLING_STEP_CLEANUP_DEFERRED_REGISTRATION_UPDATE,
>          DISABLING_STEP_CLEANUP_UNSOLICITED_EVENTS,
>          DISABLING_STEP_DISABLE_UNSOLICITED_EVENTS,
>          DISABLING_STEP_REGISTRATION_STATE,
>     @@ -1419,8 +1420,6 @@ interface_disabling_step (DisablingContext *ctx)
>          case DISABLING_STEP_PERIODIC_REGISTRATION_CHECKS:
>              /* Disable periodic registration checks, if they were set */
>              periodic_registration_check_disable (ctx->self);
>     -        /* Prevent any deferred registration state update from
>     happening after the modem is disabled */
>     -        clear_deferred_registration_state_update (ctx->self);
>              /* Fall down to next step */
>              ctx->step++;
>
>     @@ -1462,6 +1461,12 @@ interface_disabling_step (DisablingContext *ctx)
>              /* Fall down to next step */
>              ctx->step++;
>
>     +    case DISABLING_STEP_CLEANUP_DEFERRED_REGISTRATION_UPDATE:
>     +        /* Prevent any deferred registration state update from
>     happening after the modem is disabled */
>     +        clear_deferred_registration_state_update (ctx->self);
>     +        /* Fall down to next step */
>     +        ctx->step++;
>     +
>          case DISABLING_STEP_CLEANUP_UNSOLICITED_EVENTS:
>              if (MM_IFACE_MODEM_3GPP_GET_INTERFACE
>     (ctx->self)->cleanup_unsolicited_events &&
>                  MM_IFACE_MODEM_3GPP_GET_INTERFACE
>     (ctx->self)->cleanup_unsolicited_events_finish) {
>
>     --
>     Aleksander
>
>
>
>
> _______________________________________________
> networkmanager-list mailing list
> networkmanager-list gnome org
> https://mail.gnome.org/mailman/listinfo/networkmanager-list


-- 
Aleksander