If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Get usb rndis interface's name automatically

RESOLVED FIXED

Status

Firefox OS
General
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: vchang, Assigned: vchang)

Tracking

(Blocks: 1 bug)

unspecified
ARM
Gonk (Firefox OS)
Dependency tree / graph
Bug Flags:
in-moztrap -

Firefox Tracking Flags

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

Details

Attachments

(1 attachment, 2 obsolete attachments)

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".
(Assignee)

Updated

5 years ago
Blocks: 735172
(Assignee)

Updated

5 years ago
Blocks: 864588
(Assignee)

Updated

5 years ago
Assignee: nobody → vchang
(Assignee)

Comment 1

5 years ago
The interface name could be one of rndis0/usb0.... My approach is to get network interface lists and compare them with possible name lists.
(Assignee)

Comment 2

5 years ago
Created attachment 743539 [details] [diff] [review]
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.
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+
(Assignee)

Comment 4

5 years ago
(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.
(Assignee)

Comment 5

5 years ago
Created attachment 744410 [details] [diff] [review]
Patch to address review comment 3, patch v1.1
Attachment #743539 - Attachment is obsolete: true
Attachment #744410 - Flags: review?(dhylands)
(Assignee)

Comment 6

5 years ago
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)
(Assignee)

Comment 8

5 years ago
Created attachment 744579 [details] [diff] [review]
patch v1.2
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+
(Assignee)

Comment 10

4 years ago
https://hg.mozilla.org/projects/birch/rev/2188061519bb
https://hg.mozilla.org/mozilla-central/rev/2188061519bb
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
https://hg.mozilla.org/releases/mozilla-b2g18/rev/884ad1bbe24e
status-b2g18: --- → fixed
status-b2g18-v1.0.0: --- → wontfix
status-b2g18-v1.0.1: --- → wontfix
status-firefox21: --- → wontfix
status-firefox22: --- → wontfix
status-firefox23: --- → fixed

Updated

4 years ago
Flags: in-moztrap-
You need to log in before you can comment on or make changes to this bug.