960865 - B2G RIL: support dun apn type

Status: RESOLVED FIXED

(Firefox OS Graveyard :: RIL, defect)

Description

[Jessica Jong [:jessica] (inactive)]

Some carriers require the apn with type 'dun' for tethering.

Comment 1

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Guess boldly: there would be changes on ril internal interfaces. Setting the dependency for that. Please correct the dependency if not the case. Thanks!

Comment 2

[Jessica Jong [:jessica] (inactive)]

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.

Comment 3

[José Antonio Olivera Ortega [:jaoo]]

(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!

Comment 4

[Jessica Jong [:jessica] (inactive)]

(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. :)

Comment 5

[Ken Chang[:kenkai]]

Blocking 1.4 LTE feature.

Comment 6

[Ken Chang[:kenkai]]

This is a 1.4+ bug.

Comment 7

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

remove 1.4 flag as this is a feature work. let's use metabug (b2g-LTE-1.4) to keep tracking.

Comment 8

[Jessica Jong [:jessica] (inactive)]

Attachment: Part 1: add dun apn type constant in idl.

Comment 9

[Jessica Jong [:jessica] (inactive)]

Attachment: Part 2: handle dun apn type in RadioInterfaceLayer.

Comment 10

[Jessica Jong [:jessica] (inactive)]

Attachment: Part 3: handle dun apn type in NetworkManager.

Comment 11

[Jessica Jong [:jessica] (inactive)]

Attachment: Part 4: modify nat command to support setting rules for secondary routing table.

Comment 12

[Jessica Jong [:jessica] (inactive)]

Attachment: Part 5.1: dun support implementaion (usb tethering).

Comment 13

[Jessica Jong [:jessica] (inactive)]

Attachment: Part 5.2: dun support implementaion (wifi tethering).

Comment 14

[Jessica Jong [:jessica] (inactive)]

Attachment: Part 6: add dun connection timer.

Comment 15

[Jessica Jong [:jessica] (inactive)]

Attachment: Part 7: add dun retry timer.

Comment 16

[Jessica Jong [:jessica] (inactive)]

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

Comment 17

[Jessica Jong [:jessica] (inactive)]

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.

Comment 18

[Hsin-Yi Tsai (she/her) [:hsinyi]]

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 :)

Comment 19

[Hsin-Yi Tsai (she/her) [:hsinyi]]

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?

Comment 20

[Jessica Jong [:jessica] (inactive)]

(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.

Comment 21

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(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. :)

Comment 22

[Jessica Jong [:jessica] (inactive)]

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 23

[Hsin-Yi Tsai (she/her) [:hsinyi]]

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 ;)

Comment 24

[Hsin-Yi Tsai (she/her) [:hsinyi]]

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 ;)

Comment 25

[Hsin-Yi Tsai (she/her) [:hsinyi]]

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!

Comment 26

[Jessica Jong [:jessica] (inactive)]

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 27

[Vincent Chang[:vchang][changyihsin]]

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 ?

Comment 28

[Vincent Chang[:vchang][changyihsin]]

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.

Comment 29

[Vincent Chang[:vchang][changyihsin]]

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.

Comment 30

[Vincent Chang[:vchang][changyihsin]]

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. :-)

Comment 31

[Vincent Chang[:vchang][changyihsin]]

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 ?

Comment 32

[Jessica Jong [:jessica] (inactive)]

(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.

Comment 33

[Jessica Jong [:jessica] (inactive)]

(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.

Comment 34

[Jessica Jong [:jessica] (inactive)]

(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.

Comment 35

[Jessica Jong [:jessica] (inactive)]

Attachment: Part 1: add dun apn type in idls, v2.

Comment 36

[Jessica Jong [:jessica] (inactive)]

Attachment: Part 2: handle dun apn type in RadioInterfaceLayer, v2.

Add r=hsinyi.

Comment 37

[Jessica Jong [:jessica] (inactive)]

Attachment: Part 3: modify nat command to support setting rules for secondary routing table, v2.

Comment 38

[Jessica Jong [:jessica] (inactive)]

Attachment: Part 4: dun support implementation in NetworkManager, v2.

- Squashed commits per reviewer's request.
- Convert _pendingTetheringRequest into _pendingTetheringRequests array.
- Address review comment 28.

Comment 39

[Jessica Jong [:jessica] (inactive)]

Attachment: Part 5: add dun timers and ref count, v2.

- Squashed commits per reviewer's request.
- Added reference count for dun to avoid disconnecting dun when one of the tethering still needs it.

Comment 40

[Jessica Jong [:jessica] (inactive)]

Attachment: Part 1: add dun apn type in idls, v3.

Comment 41

[Jessica Jong [:jessica] (inactive)]

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.

Comment 42

[Jessica Jong [:jessica] (inactive)]

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 43

[Vincent Chang[:vchang][changyihsin]]

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.

Comment 44

[Vincent Chang[:vchang][changyihsin]]

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.

Comment 45

[Vincent Chang[:vchang][changyihsin]]

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.

Comment 46

[Jessica Jong [:jessica] (inactive)]

Attachment: Part 4: dun support implementation in NetworkManager, v3.

- Fix handleLastRequest() should be called last in usbTetheringResultReport().

Comment 47

[Jessica Jong [:jessica] (inactive)]

Attachment: Part 5: add dun timers and ref count, v3.

Address review comment 44:
- clear the retry and connection timer and pending requests when this._dunActiveUsers is zero.

Comment 48

[Jessica Jong [:jessica] (inactive)]

Comment on attachment 8382961 [details] [diff] [review]
Part 5: add dun timers and ref count, v3.

Thank you Vincent! :)

Comment 49

[Vincent Chang[:vchang][changyihsin]]

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.

Comment 50

[Jessica Jong [:jessica] (inactive)]

Attachment: Part 1: add dun apn type constant in idls. r=vchang (final)

Rebased after Bug 961571.

Comment 51

[Jessica Jong [:jessica] (inactive)]

Attachment: Part 2: handle dun apn type in RadioInterfaceLayer. r=hsinyi (final)

Rebased after Bug 961571.

Comment 52

[Jessica Jong [:jessica] (inactive)]

Attachment: Part 3: modify nat command to support setting rules for secondary routing table. r=vchang (final)

Rebased after Bug 961571.

Comment 53

[Jessica Jong [:jessica] (inactive)]

Attachment: Part 4: dun support implementation in NetworkManager. r=vchang (final)

Rebased after Bug 961571.

Comment 54

[Jessica Jong [:jessica] (inactive)]

Attachment: Part 5: add dun connection timer and reference count. r=vchang (final)

Rebased after Bug 961571.

Comment 55

[Jessica Jong [:jessica] (inactive)]

Full try: https://tbpl.mozilla.org/?tree=Try&rev=a738aba98bd8

Comment 56

[Jessica Jong [:jessica] (inactive)]

I think we are good to go! The failures in Linux's mochitest seems to be known issues.

Comment 57

[Vicamo Yang [:vicamo][:vyang]]

https://hg.mozilla.org/integration/b2g-inbound/rev/25dec088f33d
https://hg.mozilla.org/integration/b2g-inbound/rev/4307f548abf1
https://hg.mozilla.org/integration/b2g-inbound/rev/df6bbad379aa
https://hg.mozilla.org/integration/b2g-inbound/rev/56e53b005ea7
https://hg.mozilla.org/integration/b2g-inbound/rev/a17fb827ee9d

Comment 58

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/25dec088f33d
https://hg.mozilla.org/mozilla-central/rev/4307f548abf1
https://hg.mozilla.org/mozilla-central/rev/df6bbad379aa
https://hg.mozilla.org/mozilla-central/rev/56e53b005ea7
https://hg.mozilla.org/mozilla-central/rev/a17fb827ee9d

Comment 59

[Wang Rong]

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.

Comment 60

[Jessica Jong [:jessica] (inactive)]

(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.

Comment 61

[buri.blff]

how can we make sure iproute2 project is enabled??

Comment 62

[Jessica Jong [:jessica] (inactive)]

(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!

Comment 63

[buri.blff]

(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

Comment 64

[buri.blff]

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

Comment 65

[buri.blff]

Attachment: can't open wifi tethering.txt

Comment 66

[Jessica Jong [:jessica] (inactive)]

(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.

Comment 67

[guquan]

(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

Comment 68

[guquan]

(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