785982 - B2G RIL: toggling on->off 3G data call quickly caused data call stay in UP state

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Shian-Yow Wu [:swu]]

I saw this on Otoro device.

STR:
1. Make sure Data Connection is disabled in the beginning.
2. Go to settings, toggle on->off "Data Connection" quickly.
3. Check "netcfg" in adb shell, the rmnet0 stays in UP state forever.  Further on/off no longer working.

When this happened, according to logcat, the data call state was 2 (GECKO_NETWORK_STATE_SUSPENDED)

I/Gecko   (  105): -*- RadioInterfaceLayer: Received message from worker: {"status":0,"suggestedRetryTime":-1,"cid":"0","active":1,"type":"IP","ifname":"rmnet0","ipaddr":"177.144.41.5/30","dns":["200.220.227.56","200.142.130.202"],"gw":"177.144.41.6","ip":"177.144.41.5","netmask":"255.255.255.252","broadcast":"177.144.41.7","state":2,"radioTech":1,"apn":"","user":"","passwd":"","chappap":3,"pdptype":"IP","rilMessageType":"datacallstatechange"}

Comment 1

[Philipp von Weitershausen [:philikon]]

I think what we should do here is ignore any changes to the 'ril.data.enabled' setting while RILNetworkInterface.state == nsINetworkInterface.NETWORK_STATE_CONNECTING or DISCONNECTING. Then, when the datacall set-up or tear-down is complete and we're changing RILNetworkInterface.state to nsINetworkInterface.NETWORK_STATE_CONNECTED or DISCONNECTED, we check 'ril.data.enabled' once more. If it's now inconsistent with RILNetworkInterface.state, we tear down or set up the data call again.


I think this block basecamp because

(a) it's entirely possible for users to fat-finger the "data enabled" setting and flip it very quickly. Leaving the data call in an inconsistent state in such cases should not happen.

(b) we actually require the UI layer to flip the 'ril.data.enabled' setting programmatically when other data settings change. Fixing this bug would make it easier to implement that reliably.

Comment 2

[Shian-Yow Wu [:swu]]

Attachment: WIP V1

This is a WIP patch.
I'm doing more tests before asking for review.

Comment 3

[Shian-Yow Wu [:swu]]

Test cases:
1. Toggle on
Expected result: data call stays in CONNECTED state

2. Toggle off
Expected result: data call stays in DISCONNECTED state

3. Quickly toggle on->off
Expected result: data call stays in DISCONNECTED state

4. Quickly toggle off->on
Expected result: data call stays in CONNECTED state

5. Quickly Toggling on/off repeatedly
Expected result: data call stays in correct state according to last on/off status

6. Toggle on and reboot
Expected result: after reboot, data call stays in CONNECTED state

Comment 4

[Shian-Yow Wu [:swu]]

Status update:

The current implementation can work properly by the test cases.
Need to rebase after bug 744700 landed and test again.

Comment 5

[Shian-Yow Wu [:swu]]

Attachment: WIP V2

Slightly changed and rebase on bug 782513.

Comment 6

[Philipp von Weitershausen [:philikon]]

Comment on attachment 663373 [details] [diff] [review]
WIP V2

Review of attachment 663373 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/system/gonk/RadioInterfaceLayer.js
@@ +1965,5 @@
>      }
>  
>      this.state = datacall.state;
>  
> +    this.mRIL.updateRILNetworkInterface(); 

Nice, this is elegant. Maybe we can add a small comment here to explain why we're doing this, e.g.:

  // In case the data setting changed while the datacall was being started or
  // ended, let's re-check the setting and potentially adjust the datacall
  // state again.

Also, since you mentioned you rebased it on bug 782513: should we make sure we only do this for the `dataNetworkInterface`? E.g.

  if (this == this.mRIL.dataNetworkInterface) {
    this.mRIL.updateRILNetworkInterface();
  }

(Not sure there's a more elegant way of doing this.)

Ahyway, r=me with those. Thanks!

Comment 7

[Shian-Yow Wu [:swu]]

Attachment: V3 (r=philikon)

Addressed review comments.

Comment 8

[Shian-Yow Wu [:swu]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ed7f4ba92ab0

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/ed7f4ba92ab0