Closed Bug 1022460 Opened 5 years ago Closed 5 years ago

Enable wifi tethering on emulator

Categories

(Firefox OS Graveyard :: Wifi, defect)

x86_64
Gonk (Firefox OS)
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
2.0 S5 (4july)

People

(Reporter: hchang, Assigned: hchang)

Details

(Whiteboard: [p=3])

Attachments

(5 files, 7 obsolete files)

62 bytes, text/x-github-pull-request
vicamo
: review+
Details | Review
53 bytes, text/x-github-pull-request
vicamo
: review+
Details | Review
54 bytes, text/x-github-pull-request
vicamo
: review+
Details | Review
68 bytes, text/x-github-pull-request
vicamo
: review+
Details | Review
23.11 KB, patch
Details | Diff | Splinter Review
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
Whiteboard: [p=3]
Target Milestone: --- → 2.0 S4 (20june)
Assignee: nobody → 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
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.
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.
Flags: needinfo?(vyang)
Flags: needinfo?(vchang)
(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?
(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
Attached patch Tethering test cases WIP (obsolete) — Splinter Review
(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
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
Target Milestone: 2.0 S4 (20june) → 2.0 S5 (4july)
Try run with this patch and all PRs: https://tbpl.mozilla.org/?tree=Try&rev=bf1b4e3ccaa6
Attachment #8442737 - Attachment is obsolete: true
Attachment #8443399 - Attachment is obsolete: true
Attached patch Tethering test cases WIP V4 (obsolete) — Splinter Review
Attachment #8445036 - Attachment is obsolete: true
Attachment #8436751 - Flags: review?(vyang)
Flags: needinfo?(vyang)
Flags: needinfo?(vchang)
Attachment #8436754 - Flags: review?(vyang)
Attachment #8436757 - Flags: review?(vyang)
Attachment #8436795 - Flags: review?(vyang)
Attachment #8447838 - Flags: review?(vyang)
Attachment #8447838 - Flags: review?(vchang)
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!
Attachment #8436754 - Flags: review?(vyang) → review+
Attachment #8436757 - Flags: review?(vyang) → review+
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 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.
Attachment #8447838 - Flags: review?(vyang)
Attachment #8436795 - Flags: review?(vyang)
Attached patch Tethering test cases WIP V5 (obsolete) — Splinter Review
Test case V5 removes the use of timeout. Instead, we verify 'netcfg', 'ip route' and 'iptables -t nat --list' after enabling/disabling wifi tethering.
Attachment #8448659 - Flags: review?(vyang)
Attachment #8448659 - Flags: review?(vchang)
Attachment #8447838 - Attachment is obsolete: true
Attachment #8447838 - Flags: review?(vchang)
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?
Attachment #8448659 - Flags: review?(vyang)
Attachment #8448659 - Flags: review?(vchang)
Attachment #8448659 - Flags: review+
(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 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.
Attachment #8436751 - Flags: review?(vyang)
Attached patch Tethering test cases WIP V6 (obsolete) — Splinter Review
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
Another try run with all new PRs and test cases: 

https://tbpl.mozilla.org/?tree=Try&rev=373e6171d29b
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.
Attachment #8436751 - Flags: review?(vyang)
Attachment #8436751 - Flags: review?(vyang) → review+
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");|?
(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!
Attachment #8453652 - Attachment description: Tethering test cases WIP V7 → Tethering test cases WIP V7 (r+'d)
Attachment #8448659 - Attachment is obsolete: true
Attachment #8449317 - Attachment is obsolete: true
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/c3af6e7d15d0
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.