Closed Bug 1177236 Opened 5 years ago Closed 4 years ago

[Cost Control] Usage Alert does not fire when data is consumed by second device connected through hotspot.

Categories

(Firefox OS Graveyard :: Gaia::Cost Control, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(firefox42 fixed, b2g-v2.0 affected, b2g-v2.1 affected, b2g-v2.2 affected, b2g-master verified)

RESOLVED FIXED
FxOS-S4 (07Aug)
Tracking Status
firefox42 --- fixed
b2g-v2.0 --- affected
b2g-v2.1 --- affected
b2g-v2.2 --- affected
b2g-master --- verified

People

(Reporter: NicholasN, Assigned: albert)

References

()

Details

(Whiteboard: [3.0-Daily-Testing][Spark])

Attachments

(3 files, 1 obsolete file)

Attached file logcat_usage.txt
Description:
Device 1 has cellular data and hotspot enabled, and has a usage alert for 1 MB. Device 2 has wifi enabled and is connected to the hotspot. Device 2 streams video from youtube, exceeding the limit set by device 1. No notification occurs, except when device 1 disables cellular data.

Note: If only device 1 is consuming data the alert triggers right when the limit is reached.


Repro Steps:
1) Update a Aries to 20150624143841
2) Enable cellular data and turn on hotspot in settings.
3) Go to the Usage app and set a data alert for 1 MB.
4) On another device, enable wifi and connect to the first device's hotspot.
5) On the second device, stream video from youtube to use more than 1 MB of data.



Actual:
The data alert on device 1 does not trigger, at least until that device disables cellular data.


Expected:
Data alert triggers as soon as the set limit is reached.


Notes:

Environmental Variables:
Device: Aries 3.0
Build ID: 20150624143841
Gaia: eb0d4aefa62b20420d6fa0642515a110daca5d97
Gecko: 4cdc1a95a672
Gonk: 2916e2368074b5383c80bf5a0fba3fc83ba310bd
Version: 41.0a1 (3.0)
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:41.0) Gecko/41.0 Firefox/41.0

Repro frequency: 6/7
See attached: video clip, logcat
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(onelson)
Whiteboard: [3.0-Daily-Testing][Spark]
This also occurred on Flame 3.0 and Flame 2.0

Flame 3.0

Actual result:
Usage alert did not occur after a 2nd device connected to the hotspot used more than the data limit. The alert did trigger after cellular data was disabled on device 1. 
(The alert took a little longer to show up on this device)

Environmental Variables:
Device: Flame 3.0
Build ID: 20150624010204
Gaia: 311c4e59936a407e64509f54fecb440d8a78e3c8
Gecko: be81b8d6fae9
Gonk: a4f6f31d1fe213ac935ca8ede7d05e47324101a4
Version: 41.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:41.0) Gecko/41.0 Firefox/41.0

Flame 2.0

Actual result:
Usage alert did not occur after a 2nd device connected to the hotspot used more than the data limit. The alert did trigger after cellular data was disabled on device 1. 

Environmental Variables:
Device: Flame 2.0
BuildID: 20150624000203
Gaia: 5552bf529d3d6775a968942e9afa6c1d4037362c
Gecko: 5a1353911c67
Gonk: bd9cb3af2a0354577a6903917bc826489050b40d
Version: 32.0 (2.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0
Also occurs on Flame 2.1 and Flame 2.2

Flame 2.1

Actual result:
Usage alert did not occur after a 2nd device connected to the hotspot used more than the data limit. The alert did trigger after cellular data was disabled on device 1. 

Environmental Variables:
Device: Flame 2.1
BuildID: 20150624001204
Gaia: f8b848c82d1ed589f7a1eb5cc099830c867ff1d4
Gecko: ed778b596fad
Gonk: bd9cb3af2a0354577a6903917bc826489050b40d
Version: 34.0 (2.1) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Flame 2.2

Actual result:
Usage alert did not occur after a 2nd device connected to the hotspot used more than the data limit. The alert did trigger after cellular data was disabled on device 1. 

Environmental Variables:
Device: Flame 2.2
BuildID: 20150624002503
Gaia: 1f8981d7872e3c0053571c26fb3edaf401844d75
Gecko: 3268ab642b70
Gonk: bd9cb3af2a0354577a6903917bc826489050b40d
Version: 37.0 (2.2) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0
Seems like a bad issue that has been occurring through the device lifetime. ni? on Cost Control owner to determine if this should block anywhere.
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(onelson) → needinfo?(slyu)
This looks like a bug.

Salva, could you refer someone to look at this? Thanks.
Flags: needinfo?(slyu) → needinfo?(salva)
IMHO, this is not severe enough to be a blocker, but feel free to block it if you want to.
Albert, do you have an idea of what would be happening?
Flags: needinfo?(salva) → needinfo?(alberto.crespellperez)
Alarms are set and managed in net daemon (gonk) through iptables using the bandwidth controller, a set of custom chains called bw_xxxx applied over the system chains INPUT, OUTPUT and FORWARD.

When tethering is enabled the net daemon uses the nat controller to forward the traffic coming from one interface to another with connectivity and it uses another set of custom chains called fw_xxxx, which are out of scope of bw_xxxx chains. Thats the reason why tethering traffic is not being counted.

As an example, if we remove the tethering counters added by the nat controller and add the bandwidth chain controlling the quota instead, it works as expected:

adb shell iptables -D natctrl_tether_counters 1
adb shell iptables -D natctrl_tether_counters 1
adb shell iptables -A natctrl_tether_counters -j bw_costly_rmnet0

However modifying rules in that way is not a solution because 'netd' module comes from the Android repo.
Flags: needinfo?(alberto.crespellperez)
Attached patch Patch v1 (obsolete) — Splinter Review
Added globalAlert when using tethering
Attachment #8630557 - Flags: review?(ettseng)
Comment on attachment 8630557 [details] [diff] [review]
Patch v1

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

Hi Albert,

Thanks for your patch. I read through it.

I have one basic question about the logic.
While switching on/off tethering and we set global and interface alarm respectively, will the previous counters be accumulated?

For example, suppose the threshold is set as 1GB and the device already consumes 500MB on WiFi interface. When we enable tethering on this device now, how much amount of traffic will trigger the alarm?
Would it be 500MB or 1GB?
Attachment #8630557 - Flags: review?(ettseng) → feedback+
(In reply to Ethan Tseng [:ethan] from comment #9)
> Comment on attachment 8630557 [details] [diff] [review]
> Patch v1
> 
> Review of attachment 8630557 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Hi Albert,
> 
> Thanks for your patch. I read through it.
> 
> I have one basic question about the logic.
> While switching on/off tethering and we set global and interface alarm
> respectively, will the previous counters be accumulated?
> 
> For example, suppose the threshold is set as 1GB and the device already
> consumes 500MB on WiFi interface. When we enable tethering on this device
> now, how much amount of traffic will trigger the alarm?
> Would it be 500MB or 1GB?

In that case, the alarm should trigger at 1GB
Attachment #8630557 - Flags: review?(changyihsin)
Attachment #8630557 - Flags: review?(changyihsin) → review?(ettseng)
(In reply to Albert [:albert] from comment #10)
> > For example, suppose the threshold is set as 1GB and the device already
> > consumes 500MB on WiFi interface. When we enable tethering on this device
> > now, how much amount of traffic will trigger the alarm?
> > Would it be 500MB or 1GB?
> 
> In that case, the alarm should trigger at 1GB

Confirmed with Alert offline. The traffic amount will be accumulated after switching on/off tethering, so my concern no longer exists.
Comment on attachment 8630557 [details] [diff] [review]
Patch v1

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

This patch adds some functions to NetworkUtils and NetworkService to support switching global/interface alert while enabling/disabling tethering. It looks good.
Just some nits and suggestions. r=me if they are addressed.

::: dom/system/gonk/NetworkService.js
@@ +282,5 @@
> +    if (!file) {
> +      return;
> +    }
> +
> +    let self = this;

People suggest to replace 'self' by arrow function in order to make code more expressive. I know my colleagues had replaced most 'self' by arrow functions in this file, and I encourage you to do that too.

@@ +294,5 @@
> +                          .split("\n");
> +        if (data) {
> +          let threshold = parseInt(data[0], 10);
> +
> +          self._setNetworkTetheringAlarm(aEnable, aInterface, threshold); 

Remove the trailing whitespace please.

@@ +617,5 @@
>      aConfig.wifictrlinterfacename = WIFI_CTRL_INTERFACE;
>      aConfig.cmd = "setWifiTethering";
>  
>      // The callback function in controlMessage may not be fired immediately.
> +    let self = this;

Replace 'self' by arrow function.

@@ +640,5 @@
>    // Enable/disable USB tethering by sending commands to netd.
>    setUSBTethering: function(aEnable, aConfig, aCallback) {
>      aConfig.cmd = "setUSBTethering";
>      // The callback function in controlMessage may not be fired immediately.
> +    let self = this;

Ditto.

::: dom/system/gonk/NetworkUtils.cpp
@@ +28,5 @@
>  #include <sys/socket.h> // getaddrinfo(), freeaddrinfo()
>  #include <netdb.h>
>  #include <arpa/inet.h>  // inet_ntop()
>  
> +#define _DEBUG 1

This flag should be turned off in formal patch, right?

@@ +281,5 @@
> +};
> +
> +const CommandFunc NetworkUtils::sTetheringGetStatusChain[] = {
> +  NetworkUtils::tetheringStatus,
> +  NetworkUtils::networkInterfaceAlarmSuccess

Use defaultAsyncSuccessHandler instead of networkInterfaceAlarmSuccess. They do almost the same thing. But the former is more appropriate for GetStatus function (from semantic perspective).
Attachment #8630557 - Flags: review?(ettseng) → review+
Assignee: nobody → alberto.crespell
Status: NEW → ASSIGNED
Attached patch Patch v2Splinter Review
nits from comment 12
Attachment #8630557 - Attachment is obsolete: true
Attachment #8638489 - Flags: review+
Keywords: checkin-needed
can we get a try run here ?
Flags: needinfo?(alberto.crespell)
Keywords: checkin-needed
(In reply to Carsten Book [:Tomcat] from comment #14)
> can we get a try run here ?

Sure

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2016952bb20c
Flags: needinfo?(alberto.crespell)
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/4d4d72da3671
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → FxOS-S4 (07Aug)
Attached video Aries v2.5.3gp
This bug has been verified as "pass" on latest build of Aries master by the STR in Comment 0.

Actual results: Data alert triggers timely when the set limit is reached.

See attachment: Aries v2.5.3gp

Reproduce rate: 0/5

DeviceBuild ID               20150826212128
Gaia Revision          c1ae9f02f2a9cfb89bf67aeea97e467c41c3362c
Gaia Date              2015-08-25 22:03:05
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/fea87cbeaa6b64510dff835549ed906fe405d558
Gecko Version          43.0a1
Device Name            aries
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.worker.20150826.204803
Firmware Date          Wed Aug 26 20:48:11 UTC 2015
Bootloader             s1: Aries KK 2.5(Pass)

Device: Flame KK v2.5(Pass):
Build ID               20150826150203
Gaia Revision          c1ae9f02f2a9cfb89bf67aeea97e467c41c3362c
Gaia Date              2015-08-25 22:03:05
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/fea87cbeaa6b64510dff835549ed906fe405d558
Gecko Version          43.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150826.182949
Firmware Date          Wed Aug 26 18:30:01 EDT 2015
Bootloader             L1TC000118D0
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage+][MGSEI-Triage+]
You need to log in before you can comment on or make changes to this bug.