1022460 - Enable wifi tethering on emulator

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Wifi, defect)

Description

[Henry Chang [:hchang]]

With wifi tethering function enabled on emulator, we can have a marionette test to test the logic of wifi/tethering exclusively enable/disable in WifiWorker.js.

Some places need to be modified:

1) Add the following to build/target/board/generic/BoardConfig.mk

WIFI_DRIVER_FW_PATH_AP := "ap"
WIFI_DRIVER_FW_PATH_STA := "sta"
WIFI_DRIVER_FW_PATH_PARAM := "/data/misc/wifi/fake_fwpath"

2) Change the path of goldfish-hostapd from "/data/misc/wifi/hostapd" to "/data/misc/wifi/goldfish-hostapd" to avoid being connected [1] when tethering is running.

[1] http://hg.mozilla.org/mozilla-central/file/9305a8ec77fe/dom/wifi/WifiHotspotUtils.cpp#l57

3) Create writable file "/data/misc/wifi/fake_fwpath" and directory "/data/misc/wifi/goldfish-hostapd"

4) Add CONFIG_IP_ADVANCED_ROUTER=y and CONFIG_IP_MULTIPLE_TABLES=y to goldfish kernel

Comment 1

[Henry Chang [:hchang]]

Attachment: Part 1: Add necessary dummy config variables used by wifi.c

Comment 2

[Henry Chang [:hchang]]

Attachment: Part 2: Create fake fw path and change the goldfish-hostapd working directory

Comment 3

[Henry Chang [:hchang]]

Comment on attachment 8436748 [details]
Part 1: Add necessary dummy config variables used by wifi.c

https://github.com/mozilla-b2g/platform_build/pull/57

Comment 4

[Henry Chang [:hchang]]

Attachment: Part 1: Add necessary dummy config variables used by wifi.c

Comment 5

[Henry Chang [:hchang]]

Attachment: Part 3: Enable "ip rule" used by NatController (netd)

Comment 6

[Henry Chang [:hchang]]

Attachment: Part 4: Update kernel image for armv5 and armv7

Comment 7

[Henry Chang [:hchang]]

Attached all required PRs for enabling wifi tethering on ICS emulator. Note that netd starts wifi tethering with its handmade [1] hostapd.conf which would look like:

interface=wlan0
driver=nl80211
ctrl_interface=/data/misc/wifi/hostapd
ssid=FirefoxHotspot
channel=6
wpa=2
rsn_pairwise=CCMP
wpa_psk=f06a4df11a61066d3ee0c728b540cac18b862556e970234b8244749d4679fc4e

where the used driver "nl80211" is not available on emulator. Therefore we are not able to actually start another wpa_supplicant and connect to the hostapd. That being said, it's still complete enough for us to test the routing table setup and wifi internal machinery.

[1] http://androidxref.com/4.0.4/xref/system/netd/SoftapController.cpp#295

I've tested the following scenarios:

1) Mobile connection active, wifi disabled.
2) Mobile connection active, wifi enabled but not active.
3) Mobile connection enabled, wifi enabled and connected to stock AP.

Comment 8

[Henry Chang [:hchang]]

Hi Vicamo, Vincent, 

Do you have any suggestion to this bug based on all the PRs and comment 7? Regarding the kernel mods, only arm version needs to be changed and rebuilt since x86 already has enabled CONFIG_IP_ADVANCED_ROUTER and CONFIG_IP_MULTIPLE_TABLES. Besides, I also enable CONFIG_IPV6_MULTIPLE_TABLES because most of device kernels like hamachi and flame enable it as well.

Comment 9

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Henry Chang [:henry] from comment #0)
> 1) Add the following to build/target/board/generic/BoardConfig.mk
> 
> WIFI_DRIVER_FW_PATH_AP := "ap"
> WIFI_DRIVER_FW_PATH_STA := "sta"
> WIFI_DRIVER_FW_PATH_PARAM := "/data/misc/wifi/fake_fwpath"

Do we really need these?  What happens if not defined?

Comment 10

[Henry Chang [:hchang]]

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #9)
> (In reply to Henry Chang [:henry] from comment #0)
> > 1) Add the following to build/target/board/generic/BoardConfig.mk
> > 
> > WIFI_DRIVER_FW_PATH_AP := "ap"
> > WIFI_DRIVER_FW_PATH_STA := "sta"
> > WIFI_DRIVER_FW_PATH_PARAM := "/data/misc/wifi/fake_fwpath"
> 
> Do we really need these?  What happens if not defined?

We need WIFI_DRIVER_FW_PATH_AP/WIFI_DRIVER_FW_PATH_STA because of [1].
The empty string returned by wifi_get_fw_path() results in ResponseCode::CommandParameterError.

The need of a fake WIFI_DRIVER_FW_PATH_PARAM is due to [2].
Even though the empty |fwpath| would bypass the check, the caller of wifi_change_fw_path [3]
will always pass a non-empty |fwpath| to it.

Thanks!

[1] http://androidxref.com/4.4.2_r2/xref/system/netd/SoftapController.cpp#220
[2] http://androidxref.com/4.4.2_r2/xref/hardware/libhardware_legacy/wifi/wifi.c#832
[3] http://androidxref.com/4.4.2_r2/xref/system/netd/SoftapController.cpp#221

Comment 11

[Henry Chang [:hchang]]

Attachment: Tethering test cases WIP

Comment 12

[Henry Chang [:hchang]]

(For reference)
When wifi tethering is enabled, the output of netcfg and ip route should be

lo       UP                                   127.0.0.1/8   0x00000049 00:00:00:00:00:00
eth0     UP                                   10.0.2.15/24  0x00001043 52:54:00:12:34:56
rmnet1   DOWN                                   0.0.0.0/0   0x00001002 52:54:00:12:34:58
rmnet2   DOWN                                   0.0.0.0/0   0x00001002 52:54:00:12:34:59
rmnet3   DOWN                                   0.0.0.0/0   0x00001002 52:54:00:12:34:5a
wlan0    UP                                 192.168.1.1/24  0x00001043 52:54:00:12:34:5b
sit0     DOWN                                   0.0.0.0/0   0x00000080 00:00:00:00:00:00
rmnet0   UP                                  10.0.2.100/24  0x00001043 52:54:00:12:34:57

and 

10.0.2.4 via 10.0.2.2 dev rmnet0 
10.0.2.3 via 10.0.2.2 dev rmnet0 
192.168.1.0/24 dev wlan0  proto kernel  scope link  src 192.168.1.1 
10.0.2.0/24 dev eth0  proto kernel  scope link  src 10.0.2.15 
10.0.2.0/24 dev rmnet0  proto kernel  scope link  src 10.0.2.100 
default via 10.0.2.2 dev rmnet0 
default via 10.0.2.2 dev eth0  metric 2

respectively

Comment 13

[Henry Chang [:hchang]]

netcfg/ip route/ip rule on android device for reference.

shell@d2tmo:/ $ netcfg
rev_rmnet0 DOWN                                   0.0.0.0/0   0x00001002 9e:80:22:79:46:4a
rev_rmnet1 DOWN                                   0.0.0.0/0   0x00001002 76:8d:31:57:96:ca
rev_rmnet5 DOWN                                   0.0.0.0/0   0x00001002 02:2d:8c:57:e4:86
rev_rmnet6 DOWN                                   0.0.0.0/0   0x00001002 e6:96:a3:d5:59:9f
rev_rmnet7 DOWN                                   0.0.0.0/0   0x00001002 62:a3:e0:d1:93:a7
rev_rmnet2 DOWN                                   0.0.0.0/0   0x00001002 da:bd:08:f6:10:35
rev_rmnet3 DOWN                                   0.0.0.0/0   0x00001002 ca:d1:c1:04:a6:91
rev_rmnet4 DOWN                                   0.0.0.0/0   0x00001002 6a:6f:88:30:81:60
rev_rmnet8 DOWN                                   0.0.0.0/0   0x00001002 4a:91:16:c2:ef:82
rmnet0   UP                               100.73.249.72/30  0x00000041 00:00:00:00:00:00
rmnet1   DOWN                                   0.0.0.0/0   0x00000000 00:00:00:00:00:00
rmnet5   DOWN                                   0.0.0.0/0   0x00000000 00:00:00:00:00:00
rmnet6   DOWN                                   0.0.0.0/0   0x00000000 00:00:00:00:00:00
rmnet7   DOWN                                   0.0.0.0/0   0x00000000 00:00:00:00:00:00
rmnet2   DOWN                                   0.0.0.0/0   0x00000000 00:00:00:00:00:00
rmnet3   DOWN                                   0.0.0.0/0   0x00000000 00:00:00:00:00:00
rmnet4   DOWN                                   0.0.0.0/0   0x00000000 00:00:00:00:00:00
sit0     DOWN                                   0.0.0.0/0   0x00000080 00:00:00:00:00:00
p2p0     DOWN                                   0.0.0.0/0   0x00001002 00:90:4c:33:22:11
lo       UP                                   127.0.0.1/8   0x00000049 00:00:00:00:00:00
wlan0    UP                                192.168.43.1/24  0x00001043 5c:0a:5b:15:1f:80
dummy0   DOWN                                   0.0.0.0/0   0x00000082 22:75:9c:44:e1:70
shell@d2tmo:/ $ ip route
default via 100.73.249.73 dev rmnet0 
74.125.203.113 via 100.73.249.73 dev rmnet0 
100.73.249.72/30 dev rmnet0  proto kernel  scope link  src 100.73.249.72 
100.73.249.73 dev rmnet0  scope link 
192.168.43.0/24 dev wlan0  proto kernel  scope link  src 192.168.43.1 
shell@d2tmo:/ $ ip rule
0:	from all lookup local 
98:	from 192.168.43.0/24 lookup 60 
99:	from all to 100.73.166.94 lookup main 
99:	from all to 74.125.204.102 lookup main 
99:	from all to 111.81.59.109 lookup main 
99:	from all to 74.125.204.139 lookup main 
99:	from all to 100.72.64.133 lookup main 
99:	from all to 173.194.72.138 lookup main 
99:	from all to 100.74.162.225 lookup main 
99:	from all to 100.77.231.229 lookup main 
99:	from all to 100.68.188.13 lookup main 
99:	from all to 210.242.125.55 lookup main 
99:	from all to 74.125.23.188 lookup main 
99:	from all to 100.68.105.41 lookup main 
99:	from all to 100.68.230.225 lookup main 
99:	from all to 100.72.17.81 lookup main 
99:	from all to 173.194.72.101 lookup main 
99:	from all to 173.194.72.188 lookup main 
99:	from all to 100.73.249.73 lookup main 
99:	from all to 74.125.203.113 lookup main 
32766:	from all lookup main 
32767:	from all lookup default

Comment 14

[Henry Chang [:hchang]]

Attachment: Tethering test cases WIP V2 (with routing verification)

Comment 15

[Henry Chang [:hchang]]

Attachment: Tethering test cases WIP V3 (with routing verification)

Try run with this patch and all PRs: https://tbpl.mozilla.org/?tree=Try&rev=bf1b4e3ccaa6

Comment 16

[Henry Chang [:hchang]]

Attachment: Tethering test cases WIP V4

Comment 17

[Henry Chang [:hchang]]

Hi Vincent, Vicamo,

Please refer to https://tbpl.mozilla.org/?tree=Try&rev=8f316c5f3754 for the try run with all the PRs and test cases applied. Please also refer to comment 1 for what each PR does for enabling wifi tethering.

Thanks!

Comment 18

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 8436795 [details] [review]
Part 4: Update kernel image for armv5 and armv7

Please provide info about which kernel-goldfish commit is this PR based on. For example

    Bug 1022460: update 2.6 kernel for ARM
    
    -from commit https://github.com/mozilla-b2g/kernel_goldfish/commit/41a03df
    -add CONFIG_IP_ADVANCED_ROUTER=y and CONFIG_IP_MULTIPLE_TABLES=y

Comment 19

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 8447838 [details] [diff] [review]
Tethering test cases WIP V4

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

::: dom/wifi/test/marionette/head.js
@@ +13,5 @@
> +
> +// If the value of 'tethering.wifi.enable' doesn't change after
> +// this amount of time, we consider the enable/disable action we've
> +// just requested is successful.
> +const TETHERING_ENABLE_CHECK_TIME_MS = 5 * 1000;

Per offline discuss, I'd wish for some more deterministic way for checking tethering function. For example, since we have rulEmulatorShell() to execute arbitrary adb shell commands, maybe we can have an evaluation on kernel routing table. Timeouts usually don't work well in emulator.

Comment 20

[Henry Chang [:hchang]]

Attachment: Tethering test cases WIP V5

Test case V5 removes the use of timeout. Instead, we verify 'netcfg', 'ip route' and 'iptables -t nat --list' after enabling/disabling wifi tethering.

Comment 21

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 8448659 [details] [diff] [review]
Tethering test cases WIP V5

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

::: dom/wifi/test/marionette/head.js
@@ +979,5 @@
> +            if ('' === aLine) {
> +              return;
> +            }
> +
> +            let tokens = aLine.split(/\s+/);

Can we test |if (tokens.length >= 5) { break; }| instead?

@@ +1045,5 @@
> +    // is disabled.
> +    function verifyIptables() {
> +      return runEmulatorShellSafe(['iptables', '-t', 'nat', '--list'])
> +        .then(function(aLines) {
> +          // $ iptables -t nat --list

Do iptables on AOSP accept `iptables -t nat -L POSTROUTING`? Then you don't have to skip other chains.

@@ +1272,5 @@
> +        .then(initTetheringTestEnvironment)
> +        .then(aTestCaseChain)
> +        .then(restoreToInitialState, function onreject(aReason) {
> +          return restoreToInitialState()
> +            .then(() => { throw aReason; }); // Re-throw the orignal reject reason.

Can this become a busy infinite-loop when restoreToInitialState() rejects?

Comment 22

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #21)
> Comment on attachment 8448659 [details] [diff] [review]
> @@ +1272,5 @@
> > +        .then(initTetheringTestEnvironment)
> > +        .then(aTestCaseChain)
> > +        .then(restoreToInitialState, function onreject(aReason) {
> > +          return restoreToInitialState()
> > +            .then(() => { throw aReason; }); // Re-throw the orignal reject reason.
> 
> Can this become a busy infinite-loop when restoreToInitialState() rejects?

Per offline discuss, I misunderstood these lines. Thank you.

Comment 23

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 8436751 [details] [review]
Part 2: Create fake fw path and change the goldfish-hostapd working directory

I'd like to have control sockets of all virtual remote APs in the same folder. A special case always brings troubles when we need to have some more experiments and/or follow-ups.

Comment 24

[Henry Chang [:hchang]]

Attachment: Tethering test cases WIP V6

V6 Changes:

1) Use /data/misc/wifi/remote-hostapd/ as the remote hostapd working directory to avoid being accidentally connected by [1].
2) Use 'iptables -t nat -L POSTROUTING' instead of 'iptables -t nat -L'
3) Remove the use of empty line check. It still works after simply removing it.
4) Use is() rather than isOrThrow() in restoreToInitialState().

[1] http://hg.mozilla.org/mozilla-central/file/7075808c3306/dom/wifi/WifiHotspotUtils.cpp#l124

Comment 25

[Henry Chang [:hchang]]

Another try run with all new PRs and test cases: 

https://tbpl.mozilla.org/?tree=Try&rev=373e6171d29b

Comment 26

[Henry Chang [:hchang]]

Comment on attachment 8436751 [details] [review]
Part 2: Create fake fw path and change the goldfish-hostapd working directory

Change emulator hostapd working directory to data/misc/wifi/remote_hostapd.

Comment 27

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 8449317 [details] [diff] [review]
Tethering test cases WIP V6

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

::: dom/wifi/test/marionette/head.js
@@ +979,5 @@
> +            if ('' === aLine) {
> +              return;
> +            }
> +
> +            let tokens = aLine.split(/\s+/);

if (tokens.length < 5) {
  return;
}

@@ +1006,5 @@
> +
> +          // Parse source ip for each interface.
> +          aLines.forEach(function (aLine) {
> +            let tokens = aLine.trim().split(/\s+/);
> +            if (tokens.length >= 2 && 'src' === tokens[tokens.length - 2]) {

let index = tokens.indexOf('src');
if (index < 0) {
  return;
}

...

@@ +1049,5 @@
> +          // Chain POSTROUTING (policy ACCEPT)
> +          // target     prot opt source               destination
> +          // MASQUERADE  all  --  anywhere             anywhere
> +          //
> +          let found = (function find_MASQUERADE() {

Why not have |let found = (aLines.length > 2) && aLines[2].startsWith("MASQUERADE");|?

Comment 28

[Vicamo Yang [:vicamo][:vyang]]

https://github.com/mozilla-b2g/platform_build/commit/0d616942c300d9fb142483210f1dda9096c9a9fc
https://github.com/mozilla-b2g/device_generic_goldfish/commit/3c42f55d85fd311de2ed9268a59ba1db30be2b21
https://github.com/mozilla-b2g/kernel_goldfish/commit/7909b6130e771f07247c987dbdaf68dbc8877970
https://github.com/mozilla-b2g/platform_prebuilts_qemu-kernel/commit/02c32feb2fe97037be0ac4dace3a6a5025ac895d

Comment 29

[Henry Chang [:hchang]]

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #28)
> https://github.com/mozilla-b2g/platform_build/commit/
> 0d616942c300d9fb142483210f1dda9096c9a9fc
> https://github.com/mozilla-b2g/device_generic_goldfish/commit/
> 3c42f55d85fd311de2ed9268a59ba1db30be2b21
> https://github.com/mozilla-b2g/kernel_goldfish/commit/
> 7909b6130e771f07247c987dbdaf68dbc8877970
> https://github.com/mozilla-b2g/platform_prebuilts_qemu-kernel/commit/
> 02c32feb2fe97037be0ac4dace3a6a5025ac895d

Thanks Vicamo!

Comment 30

[Henry Chang [:hchang]]

Attachment: Tethering test cases WIP V7 (r+'d)

Waiting for try run result: https://tbpl.mozilla.org/?tree=Try&rev=10fbb5ad872e

Comment 31

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/c3af6e7d15d0

Comment 32

[Ryan VanderMeulen [:RyanVM]]

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