Closed Bug 973842 Opened 6 years ago Closed 6 years ago

support adding routes in secondary routing tables

Categories

(Firefox OS Graveyard :: RIL, defect)

x86_64
Linux
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
1.4 S2 (28feb)

People

(Reporter: jessica, Assigned: jessica)

References

Details

Attachments

(3 files, 3 obsolete files)

we need this for adding rules and default routes for non-default apns, e.g. dun.
Routing tables would look like this (tested on Android, rmnet0 is dun's iface):

shell@tilapia:/ # ip rule show
0:	from all lookup local 
32765:	from 192.168.43.0/24 lookup 60 
32766:	from all lookup main 
32767:	from all lookup default 

shell@tilapia:/ # ip route show table 60
default via 10.32.245.37 dev rmnet0 
10.32.245.37 dev rmnet0 
192.168.43.0/24 dev wlan0  scope link
Blocks: 960865
(In reply to Jessica Jong [:jjong] [:jessica] from comment #0)
It also needs to enable multiple routing table in linux kernel. By the way, you may need to discuss with Vincent and Vicamo. They told me that iptable could support this without adding multiple routing table.
I'm trying to use the secondary table method. For this, we need to import external/iproute2 project since netd uses the 'ip' command to add routes to a secondary table.
But I am not sure for what project are we adding this? (I am using hamachi for development), and from which revision to import the project, ics_strawberry or b2g_ics_1.2? (system/netd's revision is b2g_ics_1.2 in hamachi).
Maybe, you should use Nexus 4 to develop this future. I don't think hamachi supports this feature.
Assignee: nobody → jjong
Component: Networking → RIL
Product: Core → Firefox OS
Target Milestone: --- → 1.4 S4 (28mar)
Target milestone is Feb 28?
Target Milestone: 1.4 S4 (28mar) → 1.4 S2 (28feb)
Attachment #8379470 - Attachment description: bug-973842-part1-secondary-idl.patch → Part 1: support adding secondary routes (idl/cid).
Attachment #8379470 - Flags: review?(vyang)
Attachment #8379471 - Flags: review?(vyang)
Comment on attachment 8379472 [details] [diff] [review]
Part 3: support adding secondary routes (NetworkUtils).

The following is expected by netd:
//     0       1       2        3          4           5     6      7
// interface route add/remove iface default/secondary dest prefix gateway

Ref: https://android.googlesource.com/platform/system/netd/+/jb-mr2-dev/CommandListener.cpp
Attachment #8379472 - Flags: review?(vyang)
Vicamo is very busy in IPv6. I think Vincent can help this review.
Comment on attachment 8379470 [details] [diff] [review]
Part 1: support adding secondary routes (idl/cid).

Changing reviewer per comment 9.
Attachment #8379470 - Flags: review?(vyang) → review?(vchang)
Comment on attachment 8379471 [details] [diff] [review]
Part 2: support adding secondary routes (NetworkService).

Changing reviewer per comment 9.
Attachment #8379471 - Flags: review?(vyang) → review?(vchang)
Comment on attachment 8379472 [details] [diff] [review]
Part 3: support adding secondary routes (NetworkUtils).

Changing reviewer per comment 9.
Attachment #8379472 - Flags: review?(vyang) → review?(vchang)
Comment on attachment 8379470 [details] [diff] [review]
Part 1: support adding secondary routes (idl/cid).

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

Please help to modity the comments.

::: dom/system/gonk/nsINetworkService.idl
@@ +298,5 @@
>    /**
> +   * Add route to secondary routing table.
> +   *
> +   * @param interfaceName
> +   *        The route's network interface we want add to secondary routing table.

The network interface for this route.

@@ +301,5 @@
> +   * @param interfaceName
> +   *        The route's network interface we want add to secondary routing table.
> +   * @param route
> +   *        The route to add to the secondary table. A route is an object
> +   *        composed of ip, prefix and gateway.

The route info should has following fields:
- ip: destination ip address
- prefix: destination prefix
- gateway: gateway ip address

@@ +310,5 @@
> +   * Remove route from secondary routing table.
> +   *
> +   * @param interfaceName
> +   *        The route's network interface we want remove from secondary routing
> +   *        table.

The network interface for the route we want to remove.

@@ +314,5 @@
> +   *        table.
> +   * @param route
> +   *        The route to remove from the secondary table. A route is an object
> +   *        composed of ip, prefix and gateway.
> +   */

The route info should has following fields:
- ip: destination ip address
- prefix: destination prefix
- gateway: gateway ip address
Attachment #8379470 - Flags: review?(vchang) → review+
Comment on attachment 8379471 [details] [diff] [review]
Part 2: support adding secondary routes (NetworkService).

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

Looks good. Thank you.
Attachment #8379471 - Flags: review?(vchang) → review+
Comment on attachment 8379472 [details] [diff] [review]
Part 3: support adding secondary routes (NetworkUtils).

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

Looks good. Thank you.
Attachment #8379472 - Flags: review?(vchang) → review+
Address review comment 13 and add r=vchang.

Thanks Vincent!
Attachment #8379470 - Attachment is obsolete: true
Attachment #8380578 - Flags: review+
Add r=vchang.
Attachment #8379471 - Attachment is obsolete: true
Attachment #8380579 - Flags: review+
Add r=vchang.
Attachment #8379472 - Attachment is obsolete: true
Attachment #8380580 - Flags: review+
Try: https://tbpl.mozilla.org/?tree=Try&rev=74e8a447da38
All green! \0/

This is needed by bug 960865, which is part of bug 952026.
Keywords: checkin-needed
You need to log in before you can comment on or make changes to this bug.