776212 - Get usb rndis interface's name automatically

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Vincent Chang[:vchang][changyihsin]]

When enable usb tethering, the usb driver will create a rndis network interface.
Rather than hard code the name, we need to get it automatically. 
The default name for ICS is "rndis0".

Comment 1

[Vincent Chang[:vchang][changyihsin]]

The interface name could be one of rndis0/usb0.... My approach is to get network interface lists and compare them with possible name lists.

Comment 2

[Vincent Chang[:vchang][changyihsin]]

Attachment: Possible fix, patch v1.0

The interface name of rndis could be one of rndis0(unagi)/usb0(leo).... My approach is to get network interface lists from /sys/class/net and compare them with possible name lists.

Comment 3

[Dave Hylands [:dhylands]]

Comment on attachment 743539 [details] [diff] [review]
Possible fix, patch v1.0

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

::: dom/system/gonk/NetworkManager.js
@@ +848,5 @@
> +  getUsbInterface: function getUsbInterface() {
> +    let array = [];
> +    try {
> +      let file = new FileUtils.File(KERNEL_NETWORK_ENTRY);
> +      let entries = file.directoryEntries;

Wouldn't it make more sense to just iterate over this.possibleInterface and check to see if /sys/class/net/NAME exists?

At least, then you can prioritize, and stop when you find the first entry that matches.

Fundamentally, I don't have any issues with this approach either, it just seemed odd to me.

Comment 4

[Vincent Chang[:vchang][changyihsin]]

(In reply to Dave Hylands [:dhylands] from comment #3)
> Comment on attachment 743539 [details] [diff] [review]
> Possible fix, patch v1.0
> 
> Review of attachment 743539 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/system/gonk/NetworkManager.js
> @@ +848,5 @@
> > +  getUsbInterface: function getUsbInterface() {
> > +    let array = [];
> > +    try {
> > +      let file = new FileUtils.File(KERNEL_NETWORK_ENTRY);
> > +      let entries = file.directoryEntries;
> 
> Wouldn't it make more sense to just iterate over this.possibleInterface and
> check to see if /sys/class/net/NAME exists?
> 
> At least, then you can prioritize, and stop when you find the first entry
> that matches.
> 
> Fundamentally, I don't have any issues with this approach either, it just
> seemed odd to me.

Thanks for your quick feedback, I am going to post a new patch to address your comment.
Please help me to double check it.

Comment 5

[Vincent Chang[:vchang][changyihsin]]

Attachment: Patch to address review comment 3, patch v1.1

Comment 6

[Vincent Chang[:vchang][changyihsin]]

This should be mark as LEO+, we need this patch to make usb tethering work on LEO.

Comment 7

[Dave Hylands [:dhylands]]

Comment on attachment 744410 [details] [diff] [review]
Patch to address review comment 3, patch v1.1

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

::: dom/system/gonk/NetworkManager.js
@@ +863,5 @@
> +    }
> +
> +    // Find the rndis interface.
> +    for (let i = 0; (array.length > 0 &&
> +         i < this.possibleInterface.length); i++) {

I was thinking more along the lines of removing lines 849 thru 863 and replacing lines 868 to 869 with:

let file = new FileUtils.File(KERNEL_NETWORK_ENTRY + "/" + this.possibleInterface[i]);
if (file.IsDirectory()) {
 return this.possibleInterface[i];
}

(and adding in an appropriate try/catch block)

Comment 8

[Vincent Chang[:vchang][changyihsin]]

Attachment: patch v1.2

Comment 9

[Wayne Chang [:wchang]]

Triage 5/3- Leo+ base on comment 6

Comment 10

[Vincent Chang[:vchang][changyihsin]]

https://hg.mozilla.org/projects/birch/rev/2188061519bb

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/2188061519bb

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/884ad1bbe24e