Closed Bug 776212 Opened 7 years ago Closed 7 years ago

Get usb rndis interface's name automatically

Categories

(Firefox OS Graveyard :: General, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:leo+, firefox21 wontfix, firefox22 wontfix, firefox23 fixed, b2g18 fixed, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 wontfix)

RESOLVED FIXED
blocking-b2g leo+
Tracking Status
firefox21 --- wontfix
firefox22 --- wontfix
firefox23 --- fixed
b2g18 --- fixed
b2g18-v1.0.0 --- wontfix
b2g18-v1.0.1 --- wontfix

People

(Reporter: vchang, Assigned: vchang)

References

Details

Attachments

(1 file, 2 obsolete files)

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".
Blocks: 735172
Blocks: 864588
Assignee: nobody → vchang
The interface name could be one of rndis0/usb0.... My approach is to get network interface lists and compare them with possible name lists.
Attached patch Possible fix, patch v1.0 (obsolete) — Splinter Review
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.
Attachment #743539 - Flags: review?(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.
Attachment #743539 - Flags: review?(dhylands) → review+
(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.
Attachment #743539 - Attachment is obsolete: true
Attachment #744410 - Flags: review?(dhylands)
This should be mark as LEO+, we need this patch to make usb tethering work on LEO.
blocking-b2g: --- → leo?
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)
Attached patch patch v1.2Splinter Review
Attachment #744410 - Attachment is obsolete: true
Attachment #744410 - Flags: review?(dhylands)
Attachment #744579 - Flags: review?(dhylands)
Attachment #744579 - Flags: review?(dhylands) → review+
Triage 5/3- Leo+ base on comment 6
blocking-b2g: leo? → leo+
https://hg.mozilla.org/mozilla-central/rev/2188061519bb
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Flags: in-moztrap-
You need to log in before you can comment on or make changes to this bug.