Closed Bug 960865 Opened 6 years ago Closed 6 years ago

B2G RIL: support dun apn type

Categories

(Firefox OS Graveyard :: RIL, defect)

x86_64
Gonk (Firefox OS)
defect
Not set

Tracking

(tracking-b2g:backlog)

RESOLVED FIXED
1.4 S2 (28feb)
tracking-b2g backlog

People

(Reporter: jessica, Assigned: jessica)

References

Details

(Whiteboard: [FT:RIL])

Attachments

(6 files, 16 obsolete files)

2.72 KB, patch
jessica
: review+
Details | Diff | Splinter Review
1.94 KB, patch
jessica
: review+
Details | Diff | Splinter Review
3.41 KB, patch
jessica
: review+
Details | Diff | Splinter Review
19.47 KB, patch
jessica
: review+
Details | Diff | Splinter Review
10.10 KB, patch
jessica
: review+
Details | Diff | Splinter Review
95.00 KB, text/plain
Details
Some carriers require the apn with type 'dun' for tethering.
Guess boldly: there would be changes on ril internal interfaces. Setting the dependency for that. Please correct the dependency if not the case. Thanks!
Hi José,

Could you help us parse apn for dun and ims (bug 961571) from the database and pass it to gecko through 'ril.data.apnSettings', just like the other apn types.
These apn types are needed for 1.4 features.

Thanks a lot.
Flags: needinfo?(josea.olivera)
(In reply to Jessica Jong [:jjong] [:jessica] from comment #2)

> Could you help us parse apn for dun and ims (bug 961571) from the database
> and pass it to gecko through 'ril.data.apnSettings', just like the other apn
> types.

Sure, please file the bug and assign it to me. Due the 1.3+ blockers we have I guess we can postpone this one a bit, is that a problem? Thanks!
Flags: needinfo?(josea.olivera)
(In reply to José Antonio Olivera Ortega [:jaoo] from comment #3)
> (In reply to Jessica Jong [:jjong] [:jessica] from comment #2)
> 
> > Could you help us parse apn for dun and ims (bug 961571) from the database
> > and pass it to gecko through 'ril.data.apnSettings', just like the other apn
> > types.
> 
> Sure, please file the bug and assign it to me. Due the 1.3+ blockers we have
> I guess we can postpone this one a bit, is that a problem? Thanks!

Yes, 1.3+ blockers go first!
I will file the bug and let you know. Thanks for your help. :)
Blocking 1.4 LTE feature.
Assignee: nobody → jjong
Blocks: b2g-LTE-1.4
This is a 1.4+ bug.
blocking-b2g: --- → 1.4+
Depends on: 911713
Depends on: 959506
Blocks: 969298
remove 1.4 flag as this is a feature work. let's use metabug (b2g-LTE-1.4) to keep tracking.
blocking-b2g: 1.4+ → backlog
Whiteboard: [FT:RIL]
Target Milestone: --- → 1.4 S4 (28mar)
Depends on: 973842
Target Milestone: 1.4 S4 (28mar) → 1.4 S2 (28feb)
No longer depends on: 911713
No longer depends on: 959506
Attachment #8381226 - Attachment description: Bug 960865 - Part 7: add dun retry timer. → Part 7: add dun retry timer.
Attachment #8381217 - Flags: review?(htsai)
Attachment #8381218 - Flags: review?(htsai)
Attachment #8381220 - Flags: review?(vchang)
Attachment #8381220 - Flags: review?(htsai)
Comment on attachment 8381222 [details] [diff] [review]
Part 4: modify nat command to support setting rules for secondary routing table.

Netd expects the following nat command:
//  0    1       2       3       4            5
// nat enable intface extface addrcnt nated-ipaddr/prelength

However, nated-ipaddr/prelength should only take effect if secondary routing table exists.

Ref: https://android.googlesource.com/platform/system/netd/+/jb-mr2-dev/NatController.cpp
Attachment #8381222 - Flags: review?(vchang)
Attachment #8381223 - Flags: review?(vchang)
Comment on attachment 8381224 [details] [diff] [review]
Part 5.2: dun support implementaion (wifi tethering).

Sorry, will remove the extra line on the next version.
Attachment #8381224 - Flags: review?(vchang)
Attachment #8381225 - Flags: review?(vchang)
Attachment #8381225 - Flags: review?(htsai)
Attachment #8381226 - Flags: review?(vchang)
Attachment #8381226 - Flags: review?(htsai)
Comment on attachment 8381217 [details] [diff] [review]
Part 1: add dun apn type constant in idl.

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

Forward to Vincent :)
Attachment #8381217 - Flags: review?(htsai) → review?(vchang)
Comment on attachment 8381217 [details] [diff] [review]
Part 1: add dun apn type constant in idl.

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

::: dom/system/gonk/nsINetworkManager.idl
@@ +31,5 @@
>    const long NETWORK_TYPE_MOBILE      = 1;
>    const long NETWORK_TYPE_MOBILE_MMS  = 2;
>    const long NETWORK_TYPE_MOBILE_SUPL = 3;
> +  const long NETWORK_TYPE_MOBILE_DUN  = 4;
> +  const long NETWORK_TYPE_MOBILE_MAX  = NETWORK_TYPE_MOBILE_DUN;

I am not sure why we need to expose NETWORK_TYPE_MOBILE_MAX to .idl. It looks like an internal constant. Any nsINetworkInterface user needs this?
(In reply to Hsin-Yi Tsai  [:hsinyi] from comment #19)
> Comment on attachment 8381217 [details] [diff] [review]
> Part 1: add dun apn type constant in idl.
> 
> Review of attachment 8381217 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/system/gonk/nsINetworkManager.idl
> @@ +31,5 @@
> >    const long NETWORK_TYPE_MOBILE      = 1;
> >    const long NETWORK_TYPE_MOBILE_MMS  = 2;
> >    const long NETWORK_TYPE_MOBILE_SUPL = 3;
> > +  const long NETWORK_TYPE_MOBILE_DUN  = 4;
> > +  const long NETWORK_TYPE_MOBILE_MAX  = NETWORK_TYPE_MOBILE_DUN;
> 
> I am not sure why we need to expose NETWORK_TYPE_MOBILE_MAX to .idl. It
> looks like an internal constant. Any nsINetworkInterface user needs this?

Oh, I am using it in the next patch, see attachment 8381220 [details] [diff] [review].
I added here cause I think it is the only place we collect all the network types.
Attachment #8381218 - Flags: review?(htsai) → review+
(In reply to Jessica Jong [:jjong] [:jessica] from comment #20)
> (In reply to Hsin-Yi Tsai  [:hsinyi] from comment #19)
> > Comment on attachment 8381217 [details] [diff] [review]
> > Part 1: add dun apn type constant in idl.
> > 
> > Review of attachment 8381217 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > ::: dom/system/gonk/nsINetworkManager.idl
> > @@ +31,5 @@
> > >    const long NETWORK_TYPE_MOBILE      = 1;
> > >    const long NETWORK_TYPE_MOBILE_MMS  = 2;
> > >    const long NETWORK_TYPE_MOBILE_SUPL = 3;
> > > +  const long NETWORK_TYPE_MOBILE_DUN  = 4;
> > > +  const long NETWORK_TYPE_MOBILE_MAX  = NETWORK_TYPE_MOBILE_DUN;
> > 
> > I am not sure why we need to expose NETWORK_TYPE_MOBILE_MAX to .idl. It
> > looks like an internal constant. Any nsINetworkInterface user needs this?
> 
> Oh, I am using it in the next patch, see attachment 8381220 [details] [diff] [review]
> [review].
> I added here cause I think it is the only place we collect all the network
> types.

I think I got your point to have in on .idl, but it looks not that nature to me to have the attribute MOBILE_MAX on API. IMHO, it seems a variable used only within the network manager implementation and should be held by the manager itself. However, Vincent is more properer than I doing the review. I am providing my opinion and will wait for his comment. :)
To test the patches, we need the modify shared/resources/apn.json as follow:

-    {"carrier":"中華電信(Chunghwa) (Internet)","apn":"internet","type":["default","supl"]},
-    {"carrier":"中華電信(Chunghwa) (MMS)","apn":"emome","mmsc":"http://mms.emome.net:8002","mmsproxy":"10.1.1.1","mmsport":"8080","type":["mms"]}
+    {"carrier":"中華電信(Chunghwa) (Internet)","apn":"internet","type":["default","supl","ims"]},
+    {"carrier":"中華電信(Chunghwa) (MMS)","apn":"emome","mmsc":"http://mms.emome.net:8002","mmsproxy":"10.1.1.1","mmsport":"8080","type":["mms"]},
+    {"carrier":"中華電信(Chunghwa) (Emome)","apn":"emome","type":["dun"]}

Make sure gaia is updated to include Bug 968092 fix and iproute2 project is enabled.

What is more, I have set the 'dun-required' setting to true in attachment 8381223 [details] [diff] [review] for testing, we should read this from setting or property, not sure which is more secured? ro.* maybe?
Comment on attachment 8381220 [details] [diff] [review]
Part 3: handle dun apn type in NetworkManager.

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

Vincent's review should be enough ;)
Attachment #8381220 - Flags: review?(htsai)
Comment on attachment 8381225 [details] [diff] [review]
Part 6: add dun connection timer.

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

Vincent's review should be enough ;)
Attachment #8381225 - Flags: review?(htsai)
Comment on attachment 8381226 [details] [diff] [review]
Part 7: add dun retry timer.

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

Vincent's review should be enough ;) Thanks Jessica and Vincent!
Attachment #8381226 - Flags: review?(htsai)
Thanks for the reviewers effort in advance!

I must say that this is not a small change, but it should take effect only if 'dun required' setting or property is set.

There are still some issues to be considered that should be handled next:
- Maintain a reference count for dun in NetworkManager: if usb tethering and wifi tethering is enabled at the same time, when one of the tetherings is turned off, dun connection will go away with it.

- Dun data connection disconnected unexpectedly: what to do when dun connection is disconnected? should we report to the user (there is no path for this right now), retry or turn off tethering silently?

- There is a bug in data call where the network type of DISCONNECTED/UNKNOWN state event is wrong (see bug 939046 comment 0 item 2.b), this causes dun secondary routes not removed through netd, but removed due to network interface down. This will lead to secondary routing table mapping not cleared in netd.

- In the current design, dun data call can be set-up no matter data setting is enabled or not. This is because we use RadioInterface.setupDataCallByType(), which is used by mms, so there is no checking in that function.
Comment on attachment 8381222 [details] [diff] [review]
Part 4: modify nat command to support setting rules for secondary routing table.

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

::: dom/system/gonk/NetworkUtils.cpp
@@ +223,5 @@
> + * Helper function to get the network part of an ip from prefix.
> + */
> +static char* getNetworkAddr(const uint32_t ip, const uint32_t prefix)
> +{
> +  uint32_t mask, subnet;

You should initial local variables before using them. They are not guaranteed to be zero. r- because of this.

@@ +227,5 @@
> +  uint32_t mask, subnet;
> +
> +  mask = ~mask << (32 - prefix);
> +  mask = htonl(mask);
> +  subnet = ip & mask;

So the ip is always in network order ?
Attachment #8381222 - Flags: review?(vchang) → review-
Comment on attachment 8381220 [details] [diff] [review]
Part 3: handle dun apn type in NetworkManager.

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

::: dom/system/gonk/NetworkManager.js
@@ +424,5 @@
>  
>  #ifdef MOZ_B2G_RIL
> +  isNetworkTypeSecondaryMobile: function(type) {
> +    return (type > Ci.nsINetworkInterface.NETWORK_TYPE_MOBILE &&
> +            type <= Ci.nsINetworkInterface.NETWORK_TYPE_MOBILE_MAX);

Let's check the NETWORK_TYPE_MOBILE_XXX directly rather than using NETWORK_TYPE_MOBILE_MAX. So you can remove the the NETWORK_TYPE_MOBILE_MAX in idl definition. We can define NETWORK_TYPE in more generic form like "type + subtype + id" in followup.

@@ +430,5 @@
> +
> +  isNetworkTypeMobile: function(type) {
> +    return (type >= Ci.nsINetworkInterface.NETWORK_TYPE_MOBILE &&
> +            type <= Ci.nsINetworkInterface.NETWORK_TYPE_MOBILE_MAX);
> +  },

Ditto.

@@ +488,5 @@
> +    // Now we can add default route through gateway.
> +    route.ip = "0.0.0.0";
> +    route.prefix = "0";
> +    route.gateway = network.gateway;
> +    gNetworkService.addSecondaryRoute(network.name, route);

Can you add comments to explain a little bit more ? Why do we call addSecondaryRoute twice with different router parameters ?

@@ +503,5 @@
> +
> +    route.ip = "0.0.0.0";
> +    route.prefix = "0";
> +    route.gateway = network.gateway;
> +    gNetworkService.removeSecondaryRoute(network.name, route);

Ditto.
Attachment #8381220 - Flags: review?(vchang)
Comment on attachment 8381217 [details] [diff] [review]
Part 1: add dun apn type constant in idl.

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

::: dom/system/gonk/nsINetworkManager.idl
@@ +31,5 @@
>    const long NETWORK_TYPE_MOBILE      = 1;
>    const long NETWORK_TYPE_MOBILE_MMS  = 2;
>    const long NETWORK_TYPE_MOBILE_SUPL = 3;
> +  const long NETWORK_TYPE_MOBILE_DUN  = 4;
> +  const long NETWORK_TYPE_MOBILE_MAX  = NETWORK_TYPE_MOBILE_DUN;

I think we should remove NETWORK_TYPE_MOBILE_MAX here. See my comments in the other patch.
Attachment #8381217 - Flags: review?(vchang)
Comment on attachment 8381223 [details] [diff] [review]
Part 5.1: dun support implementaion (usb tethering).

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

::: dom/system/gonk/NetworkManager.js
@@ +920,5 @@
> +      // If enable is false, don't find usb interface cause it is already down,
> +      // just use the internal interface in settings.
> +      if (enable) {
> +        this._tetheringInterface[TETHERING_TYPE_USB].internalInterface = this.getUsbInterface();
> +      }

Good catch. :-)
Attachment #8381223 - Flags: review?(vchang) → review+
Attachment #8381224 - Flags: review?(vchang) → review+
Comment on attachment 8381225 [details] [diff] [review]
Part 6: add dun connection timer.

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

Why do we need this timer ? Any problem if we turn on wifi/usb hotspot together ?
(In reply to Vincent Chang[:vchang] from comment #27)
> Comment on attachment 8381222 [details] [diff] [review]
> Part 4: modify nat command to support setting rules for secondary routing
> table.
> 
> Review of attachment 8381222 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/system/gonk/NetworkUtils.cpp
> @@ +223,5 @@
> > + * Helper function to get the network part of an ip from prefix.
> > + */
> > +static char* getNetworkAddr(const uint32_t ip, const uint32_t prefix)
> > +{
> > +  uint32_t mask, subnet;
> 
> You should initial local variables before using them. They are not
> guaranteed to be zero. r- because of this.

Thank you for the remind. I will fix them in the next version.

> 
> @@ +227,5 @@
> > +  uint32_t mask, subnet;
> > +
> > +  mask = ~mask << (32 - prefix);
> > +  mask = htonl(mask);
> > +  subnet = ip & mask;
> 
> So the ip is always in network order ?

Yes, we expect param ip in network order. I will add this in the function comment.
(In reply to Vincent Chang[:vchang] from comment #28)
> Comment on attachment 8381220 [details] [diff] [review]
> Part 3: handle dun apn type in NetworkManager.
> 
> Review of attachment 8381220 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/system/gonk/NetworkManager.js
> @@ +424,5 @@
> >  
> >  #ifdef MOZ_B2G_RIL
> > +  isNetworkTypeSecondaryMobile: function(type) {
> > +    return (type > Ci.nsINetworkInterface.NETWORK_TYPE_MOBILE &&
> > +            type <= Ci.nsINetworkInterface.NETWORK_TYPE_MOBILE_MAX);
> 
> Let's check the NETWORK_TYPE_MOBILE_XXX directly rather than using
> NETWORK_TYPE_MOBILE_MAX. So you can remove the the NETWORK_TYPE_MOBILE_MAX
> in idl definition. We can define NETWORK_TYPE in more generic form like
> "type + subtype + id" in followup.

Will do.

> 
> @@ +430,5 @@
> > +
> > +  isNetworkTypeMobile: function(type) {
> > +    return (type >= Ci.nsINetworkInterface.NETWORK_TYPE_MOBILE &&
> > +            type <= Ci.nsINetworkInterface.NETWORK_TYPE_MOBILE_MAX);
> > +  },
> 
> Ditto.

Will do.

> 
> @@ +488,5 @@
> > +    // Now we can add default route through gateway.
> > +    route.ip = "0.0.0.0";
> > +    route.prefix = "0";
> > +    route.gateway = network.gateway;
> > +    gNetworkService.addSecondaryRoute(network.name, route);
> 
> Can you add comments to explain a little bit more ? Why do we call
> addSecondaryRoute twice with different router parameters ?

I will add some more comments, thank you.

> 
> @@ +503,5 @@
> > +
> > +    route.ip = "0.0.0.0";
> > +    route.prefix = "0";
> > +    route.gateway = network.gateway;
> > +    gNetworkService.removeSecondaryRoute(network.name, route);
> 
> Ditto.

Will do.
(In reply to Vincent Chang[:vchang] from comment #31)
> Comment on attachment 8381225 [details] [diff] [review]
> Part 6: add dun connection timer.
> 
> Review of attachment 8381225 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Why do we need this timer ? Any problem if we turn on wifi/usb hotspot
> together ?

This timer is needed in case of dun data connection not setup as expected. I think maybe we can cancel the timer before initing it again. Also, as per offline discussion, we need to convert _pendingTetheringRequest into an array to handle usb and wifi tethering turning on at the same time. Thank you.
Attachment #8381217 - Attachment is obsolete: true
Add r=hsinyi.
Attachment #8381218 - Attachment is obsolete: true
Attachment #8382808 - Flags: review+
- Squashed commits per reviewer's request.
- Convert _pendingTetheringRequest into _pendingTetheringRequests array.
- Address review comment 28.
Attachment #8381220 - Attachment is obsolete: true
Attachment #8381223 - Attachment is obsolete: true
Attachment #8381224 - Attachment is obsolete: true
- Squashed commits per reviewer's request.
- Added reference count for dun to avoid disconnecting dun when one of the tethering still needs it.
Attachment #8381225 - Attachment is obsolete: true
Attachment #8381226 - Attachment is obsolete: true
Attachment #8381225 - Flags: review?(vchang)
Attachment #8381226 - Flags: review?(vchang)
Attachment #8382807 - Attachment is obsolete: true
Attachment #8382822 - Flags: review?(vchang)
Attachment #8382810 - Flags: review?(vchang)
Attachment #8382812 - Flags: review?(vchang)
Attachment #8382822 - Flags: review?(vchang) → review+
Added reference count in v2 patches to avoid disconnecting dun when it is still need it. However, there is an existing bug that will disable ip forwarding even when another tethering is still active. Dimi is helping with existing tethering bugs, see bug 977469 and bug 977479 Thank you.
Attachment #8382816 - Flags: review?(vchang)
One more thing, we would like to handle dun retries and unexpected disconnection in DataConnectionManager/Handler in the future, maybe treat it like default data - to be always on when there are active users. Then, we can remove the timers for dun in Tethering.
Comment on attachment 8382810 [details] [diff] [review]
Part 3: modify nat command to support setting rules for secondary routing table, v2.

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

You have to rebase this patch. Otherwise, it looks good.
Attachment #8382810 - Flags: review?(vchang) → review+
Comment on attachment 8382816 [details] [diff] [review]
Part 5: add dun timers and ref count, v2.

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

::: dom/system/gonk/NetworkManager.js
@@ +813,5 @@
> +      this._dunActiveUsers--;
> +      if (this._dunActiveUsers > 0) {
> +        debug("Dun still needed by others, do not disconnect.")
> +        return;
> +      }

Per discussion offline, we should clear the retry and connection timer and pending requests when this._dunActiveUsers is zero.
Attachment #8382816 - Flags: review?(vchang)
Comment on attachment 8382812 [details] [diff] [review]
Part 4: dun support implementation in NetworkManager, v2.

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

Looks good. Thank you.
Attachment #8382812 - Flags: review?(vchang) → review+
- Fix handleLastRequest() should be called last in usbTetheringResultReport().
Attachment #8382812 - Attachment is obsolete: true
Attachment #8382960 - Flags: review+
Address review comment 44:
- clear the retry and connection timer and pending requests when this._dunActiveUsers is zero.
Attachment #8382816 - Attachment is obsolete: true
Comment on attachment 8382961 [details] [diff] [review]
Part 5: add dun timers and ref count, v3.

Thank you Vincent! :)
Attachment #8382961 - Flags: review?(vchang)
Comment on attachment 8382961 [details] [diff] [review]
Part 5: add dun timers and ref count, v3.

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

Looks good, thank you.
Attachment #8382961 - Flags: review?(vchang) → review+
Rebased after Bug 961571.
Attachment #8382822 - Attachment is obsolete: true
Attachment #8383471 - Flags: review+
I think we are good to go! The failures in Linux's mochitest seems to be known issues.
Keywords: checkin-needed
Dears,
We find that merge the patch for dun tethering, hotspot can't be open if dun connection can't set up. What if we want to use other apn? Thanks.
(In reply to Wang Rong from comment #59)
> Dears,
> We find that merge the patch for dun tethering, hotspot can't be open if dun
> connection can't set up. What if we want to use other apn? Thanks.

Actually, dun tethering is not required by default, it is required only if 'tethering.dun.required' setting is set to true. So if you want to use tethering with any apn, just set 'tethering.dun.required' to false.
how can we make sure iproute2 project is enabled??
Flags: needinfo?(jjong)
(In reply to buri.blff from comment #61)
> how can we make sure iproute2 project is enabled??

If you have the repository, you can check if directory external/iproute2 exists. If you have the device with adb enabled, you can enter the following:
# adb shell ip -V
and see if it returns something like: "ip utility, iproute2-ss120521".

Hope it helps!
Flags: needinfo?(jjong)
(In reply to Jessica Jong [:jjong] [:jessica] from comment #62)
> (In reply to buri.blff from comment #61)
> > how can we make sure iproute2 project is enabled??
> 
> If you have the repository, you can check if directory external/iproute2
> exists. If you have the device with adb enabled, you can enter the following:
> # adb shell ip -V
> and see if it returns something like: "ip utility, iproute2-ss120521".
> 
> Hope it helps!

yes, it returns like:ip utility, iproute2-ss12052
But with the patch, we find our device can't open wifitethering:

 After set  ro.tethering.dun_required  to true and define MOZ_B2G_RIL, we find the wifi tethering  can't be open.
   we have add the log:"can't open wifi tethering.txt"
  
   we find the log give follow error, then it disabled the wifi tethering.
 
  V/NatController(  206): runCmd(/system/bin/ip route flush cache) res=0
E/NatController(  206): Error setting route rules
(In reply to buri.blff from comment #64)
> But with the patch, we find our device can't open wifitethering:
> 
>  After set  ro.tethering.dun_required  to true and define MOZ_B2G_RIL, we
> find the wifi tethering  can't be open.
>    we have add the log:"can't open wifi tethering.txt"
>   
>    we find the log give follow error, then it disabled the wifi tethering.
>  
>   V/NatController(  206): runCmd(/system/bin/ip route flush cache) res=0
> E/NatController(  206): Error setting route rules

I think it met some problem when setting the rules for secondary table. What is strange to me is that I don't see any logs about dun data connection being connected, so I am not sure if secondary routing table is set up properly. Since you are using qcril and not mozril, could you please file a separate bug and cc anshulj? I'd like to know first if qcril can work properly with these dun changes. Thanks.
(In reply to Jessica Jong [:jjong] [:jessica] from comment #66)
> (In reply to buri.blff from comment #64)
> > But with the patch, we find our device can't open wifitethering:
> > 
> >  After set  ro.tethering.dun_required  to true and define MOZ_B2G_RIL, we
> > find the wifi tethering  can't be open.
> >    we have add the log:"can't open wifi tethering.txt"
> >   
> >    we find the log give follow error, then it disabled the wifi tethering.
> >  
> >   V/NatController(  206): runCmd(/system/bin/ip route flush cache) res=0
> > E/NatController(  206): Error setting route rules
> 
> I think it met some problem when setting the rules for secondary table. What
> is strange to me is that I don't see any logs about dun data connection
> being connected, so I am not sure if secondary routing table is set up
> properly. Since you are using qcril and not mozril, could you please file a
> separate bug and cc anshulj? I'd like to know first if qcril can work
> properly with these dun changes. Thanks.

I have create a new pr:1111520
(In reply to Jessica Jong [:jjong] [:jessica] from comment #66)
> (In reply to buri.blff from comment #64)
> > But with the patch, we find our device can't open wifitethering:
> > 
> >  After set  ro.tethering.dun_required  to true and define MOZ_B2G_RIL, we
> > find the wifi tethering  can't be open.
> >    we have add the log:"can't open wifi tethering.txt"
> >   
> >    we find the log give follow error, then it disabled the wifi tethering.
> >  
> >   V/NatController(  206): runCmd(/system/bin/ip route flush cache) res=0
> > E/NatController(  206): Error setting route rules
> 
> I think it met some problem when setting the rules for secondary table. What
> is strange to me is that I don't see any logs about dun data connection
> being connected, so I am not sure if secondary routing table is set up
> properly. Since you are using qcril and not mozril, could you please file a
> separate bug and cc anshulj? I'd like to know first if qcril can work
> properly with these dun changes. Thanks.

I have add a new log in pr;1111520:   cant open wifi tehthering with setupduncatacall.txt 
You can see the :
I/Gecko   (  224): -*- NetworkService: Going to add route to secondary table on rmnet0
I/Gecko   (  224): -*- NetworkService: Going to add route to secondary table on rmnet0

And the dun data call is connected:
D/DATA_CALL_MANAGER(  224): ManageDataCall :: check state of data call (apn='movistar.es', profile=1, networkType=6)
D/DATA_CALL_MANAGER(  224): ManageDataCall :: dataEnabled 0
D/DATA_CALL(  224): DataCall::IsInUse mUseCount =0
D/DATA_CALL_MANAGER(  224): ManageDataCall :: aDataCall->IsInUse()=0
D/DATA_CALL(  224): DataCall::IsInUse mUseCount =0
D/DATA_CALL_MANAGER(  224): ManageDataCall :: dataEnabled & 0
D/DATA_CALL_MANAGER(  224): ManageDataCall :: aDataCall->IsConnected() & 1
D/DATA_CALL_MANAGER(  224): ManageDataCall :: IsConnected() & 1
blocking-b2g: backlog → ---
You need to log in before you can comment on or make changes to this bug.