After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 733696 - [PATCH] PDP type fallback logic needed
[PATCH] PDP type fallback logic needed
Status: RESOLVED FIXED
Product: NetworkManager
Classification: Platform
Component: Mobile broadband
git master
Other All
: Normal major
: ---
Assigned To: NetworkManager maintainer(s)
NetworkManager maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2014-07-24 19:06 UTC by Tore Anderson
Modified: 2015-02-25 14:09 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
0001-wwan-fall-back-to-secondary-IP-family-if-first-one-f.patch (11.57 KB, patch)
2015-02-07 13:08 UTC, Dan Williams
none Details | Review
debug log (7.86 KB, text/plain)
2015-02-07 13:51 UTC, Tore Anderson
  Details
debug log (7.85 KB, text/plain)
2015-02-07 18:16 UTC, Tore Anderson
  Details
Patch. (18.82 KB, patch)
2015-02-19 18:04 UTC, Aleksander Morgado
none Details | Review
Debug log of test case 2.3 (33.49 KB, text/plain)
2015-02-20 08:54 UTC, Tore Anderson
  Details
Patch, v2 (18.87 KB, patch)
2015-02-20 12:08 UTC, Aleksander Morgado
none Details | Review
Patch v3 [for nm-1-0] (18.72 KB, patch)
2015-02-24 17:54 UTC, Aleksander Morgado
none Details | Review
Patch v3 [for git master] (18.73 KB, patch)
2015-02-24 17:54 UTC, Aleksander Morgado
none Details | Review
Additional patch for git master (alignment/indentation issues) (6.33 KB, patch)
2015-02-24 17:56 UTC, Aleksander Morgado
none Details | Review

Description Tore Anderson 2014-07-24 19:06:29 UTC
With the introduction of IPv6 support, when connecting to a mobile broadband connection with everything left at the defaults (in particular, with both [ipv4] and [ipv6] set to "auto") NM will prefer to use PDP context type IPV4V6 (corresponds to "ip-type=ipv4v6" in MM) if MM reports that the modem supports this. If it doesn't, NM will try PDP context type IP ("ip-type=ipv4"), and if MM claims it doesn't support that either (probably extremely unlikely), it wil finally try PDP context type IPV6 ("ip-type=ipv6").

Once NM has decided which PDP context type (ip-type) to use, it will not try any other. If the connection attempt fails using the chosen ip-type, then the overall connection attempt fails.

This is problematic for two reasons that I can think of:

1) Even if the modem does support a certain PDP context type, the mobile provider (or more specifically, the given APN) might not. If that is the case, the connection attempt will be refused by the network. This is a situation that cannot be detected before actually trying and failing, as far as I can tell.

This is shown thusly in the logs when connecting to the APN "telenor.mobil" which only support the IPV6 PDP type, using an Ericsson F5321gw in non-MBIM mode. In this mode, NM will try using "ip-type=ipv4" as MM correctly does not advertise modem support for ip-type=ipv4v6:

<warn> (ttyACM3) failed to connect modem: Call setup failed
<warn> Activation (ttyACM3) failed for connection 'Default (telenor.mobil)'

In this case, if NM had responded to this failure by attempting the next PDP type in its original preference list (i.e., "ip-type=ipv6"), the connection attempt would have been a success.

2) It appears that in some cases, MM will report that modem supports PDP context types it in actuality does not. Currently, this happens with all MBIM modems that does not really support IPV4V6 (including the F5321gw):

http://cgit.freedesktop.org/ModemManager/ModemManager/tree/src/mm-broadband-modem-mbim.c?id=0284daf87e8f0ba780a4dcbe4741713cb3c779eb#n486

Any connection attempt using such a modem will fail:

<warn> (cdc-wdm1) failed to connect modem: NoDeviceSupport
<warn> Activation (cdc-wdm1) failed for connection 'Default (telenor.mobil)'

It will fail regardless of what the network/APN support in this case, as it fails before even communicating with the network. Again, if NM had dealt with this failure by retrying the connection attempt using the ip-types "ipv4" and/or "ipv6", the overall connection attempt would have succeeded.

More info here (including a debug-level log from a failed connection attempt): https://bugzilla.gnome.org/show_bug.cgi?id=682623#c28

I consider it quite important that this is implemented before releasing the next version of NM and incorporating it in the major distributions, otherwise a large number of users will likely experience that their previously working connection profiles are no longer able to successfully connect to the network.

Tore
Comment 1 Marius Kotsbak 2014-12-22 13:38:30 UTC
What about adding information about what PDP context types the providers support together with the APNs here?:

https://git.gnome.org/browse/mobile-broadband-provider-info/tree/serviceproviders.2.dtd
Comment 2 Tore Anderson 2014-12-22 17:18:40 UTC
That's certainly possible, but I think it would be an incomplete solution:

1) AFAIK, the information is only consulted when the profile is created. So it won't help all those with non-IPV4V6-capable providers that have already created default profiles (as they will try IPV4V6 if MM reports modem support).

2) Similarly, if a new user creates an IPv4-only connection profile due to mobile-broadband-provider-info stating that the provider don't support IPV6/IPV4V6, then if that provider deploys IPV6 and/or IPV4V6 later then the user won't get to make use of.

3) It won't handle the case where MM misreports that an MBIM modem support IPV4V6, if the provider supports IPV4V6 (e.g., Telenor Norway on the "telenor.smart" APN). This results in a connection failure.

4) IPv6/dual-stack support is very much a moving target these days with deployments happening in many different countries. Therefore, the info provided by mobile-broadband-provider-info would necessarily become stale really quickly.

That said, if a fallback logic is implemented, then there's really not much to gain from including the info in mobile-broadband-provider info as the NM/MM team will figure it out anyway the connection attempts.

IMHO the sensible preference order would be IPV4V6 > IP > IPV6 for the forseeable future, or if 464XLAT gets implemented (bug#733716) then it might be changed to IPV4V6 > IPV6 > IP instead.

Tore
Comment 3 Dan Williams 2015-02-07 13:08:32 UTC
Created attachment 296325 [details] [review]
0001-wwan-fall-back-to-secondary-IP-family-if-first-one-f.patch

Tore, would you be able to try this patch out and see if it works like you expect?
Comment 4 Tore Anderson 2015-02-07 13:51:56 UTC
Created attachment 296327 [details]
debug log

Tested it, it does not work. This test case has an MBIM modem mis-reporting IPV4V6 support. The provider/APN supports all three PDP types, so any fallback should have worked. Debug logs attached.
 
Even if the above bug is fixed, I see another potential problem. In the logs, it says "connecting with ip-type ipv4v6, fallback ipv4". However, if the APN only supports IPv6 (my provider has one of these), then the fallback will also fail. So I think you will need to potentially do three attempts before giving up:

1) ipv4v6
2) ipv4
3) ipv6

(the list would possibly be altered by the may-fail setting).

Tore
Comment 5 Tore Anderson 2015-02-07 18:16:50 UTC
Created attachment 296340 [details]
debug log

I found another issue with the patch too, I cannot connect to IPv6-only mobile broadband any longer. I have forced NM to use IPv6 ip-type by disabling IPv4 in the connection profile, so no fallback logic is necessary in this case.

It ends up failing with "NetworkManager-wwan-CRITICAL **: file nm-modem-broadband.c: line 197 (_ip_family_to_ip_type): should not be reached". See attached debug-level log.

Tore
Comment 6 Aleksander Morgado 2015-02-19 08:07:54 UTC
Reported in the MM bugzilla as well:
https://bugs.freedesktop.org/show_bug.cgi?id=89212
Comment 7 Aleksander Morgado 2015-02-19 08:49:37 UTC
>  
> Even if the above bug is fixed, I see another potential problem. In the
> logs, it says "connecting with ip-type ipv4v6, fallback ipv4". However, if
> the APN only supports IPv6 (my provider has one of these), then the fallback
> will also fail. So I think you will need to potentially do three attempts
> before giving up:
> 
> 1) ipv4v6
> 2) ipv4
> 3) ipv6
> 

How about this for now?:
 * If user asked for IPv4, only try IPv4.
 * If user asked for IPv6, only try IPv6.  No UI for this one yet anyway, right?
 * If user asked for IPv4v6:
   ** Try IPv4v6.
   ** Otherwise, fallback to IPv4.
   ** Otherwise, fallback to IPv6.
Comment 8 Tore Anderson 2015-02-19 09:03:38 UTC
(In reply to Aleksander Morgado from comment #7)
> >  
> > Even if the above bug is fixed, I see another potential problem. In the
> > logs, it says "connecting with ip-type ipv4v6, fallback ipv4". However, if
> > the APN only supports IPv6 (my provider has one of these), then the fallback
> > will also fail. So I think you will need to potentially do three attempts
> > before giving up:
> > 
> > 1) ipv4v6
> > 2) ipv4
> > 3) ipv6
> > 
> 
> How about this for now?:
>  * If user asked for IPv4, only try IPv4.

Yep.

>  * If user asked for IPv6, only try IPv6.  No UI for this one yet anyway,
> right?

Yes, there is. You simply set IPv4 method to "disabled" (or perhaps it was "off", can't remember), while leaving IPv6 method as "auto".

>  * If user asked for IPv4v6:

This is the NM default if modem advertises support for it, FWIW (ipv4 method = ipv6 method = "auto").

>    ** Try IPv4v6.

Yep.

>    ** Otherwise, fallback to IPv4.

Yep, except if ipv6 may-fail=false.

>    ** Otherwise, fallback to IPv6.

Yep, except if ipv4 may-fail=false.

However, if/when NM gains support for 464XLAT (RFC6877), then the order should be changes as follows:

 * If user asked for IPv4v6:
   ** Try IPv4v6.
   ** Otherwise, fallback to IPv6.
   *** If IPv6 is successfull and IPv4 mode = auto, try to enable 464XLAT
   ** If IPv6 fails, or 464XLAT fails when ipv4 may-fail=false, disconnect and retry with IPv4. (unless ipv6 may-fail=false, in that case, fail the connection completely)

This is because successful enabling of 464XLAT on an IPv6-only data bearer should be considered an overall IPv4 success. Most if not all mobile providers that support IPv6-only PDP contexts also support 464XLAT (i.e., they provide NAT64/DNS64).

Tore
Comment 9 Aleksander Morgado 2015-02-19 18:04:29 UTC
Created attachment 297292 [details] [review]
Patch.

Tore, could you retry with this patch?
Comment 10 Tore Anderson 2015-02-20 08:54:26 UTC
Created attachment 297353 [details]
Debug log of test case 2.3

(In reply to Aleksander Morgado from comment #9)
> Tore, could you retry with this patch?

Tested it, see below:

1) Ericsson MBM MBIM (misreports support for ipv4v6):

1.1) APN w/support for ip-types ipv4,ipv6,ipv4v6:
     => Connected OK (ip-type ipv4 used)

1.2) APN w/support for ip-type ipv4:
     => Connected OK (ip-type ipv4 used)

1.3) APN w/support for ip-type ipv6:
     => Connected OK (ip-type ipv6 used)

1.4) APN w/support for ip-types ipv4,ipv6,ipv4v6, ipv6 may-fail=false:
     => Connected OK (ip-type ipv6 used)

1.5) APN w/support for ip-types ipv4,ipv6,ipv4v6, ipv4 may-fail=false, ipv6 may-fail=false:
     => Connection failure (NoDeviceSupport)

2) Ericsson MBM NCM (reports support for ipv4,ipv6):

2.1) APN w/support for ip-types ipv4,ipv6,ipv4v6:
     => Connected OK (ip-type ipv4 used)

2.2) APN w/support for ip-type ipv4:
     => Connected OK (ip-type ipv4 used)

2.3) APN w/support for ip-type ipv6:
     => Connection failure (Call setup failed)

2.4) APN w/support for ip-types ipv4,ipv6,ipv4v6, ipv6 may-fail=false:
     => Connected OK (ip-type ipv6 used)

2.5) APN w/support for ip-types ipv4,ipv6,ipv4v6, ipv4 may-fail=false, ipv6 may-fail=false:
     => Connection failure (Connection requested both IPv4 and IPv6 but dual-stack addressing is unsupported by the modem)

Test case 2.3 doesn't work as I would expect, it seems to fail instantly after initially trying ipv4 but getting refused by the network, rather than failing over to trying ipv6 instead. I am attaching a debug log.

FWIW, the patch doesn't apply cleanly to today's git master, and with NM 1.0 I had to use --enable-more-warnings=no in order to get it to build successfully.

Tore
Comment 11 Aleksander Morgado 2015-02-20 12:08:42 UTC
Created attachment 297364 [details] [review]
Patch, v2

Hey, thanks for the thorough tests.

I have now updated the patch with:
  * Fixes for the warnings, so you should no longer need to play with --enable-more-warnings.
  * Fix for the retries logic when IPv4v6 is not supported by the modem (there was no retries set in that case)
  * Fix for some error reporting, which was wrongly returning a GError when a NMDeviceStateReason was expected.

The new patch should be applied on top of nm-1-0. If it looks good, I'll write a new one for git master as well.

Could you please retry? Thanks!
Comment 12 Tore Anderson 2015-02-20 13:54:40 UTC
Looks good!

I can confirm that it now builds correctly with --enable-more-warnings=error (default in the specfile), and that test case #2.3 seems fixed (I now get an IPv6 connection, as expected). No regressions in any of the other test cases.


Tore
Comment 13 Aleksander Morgado 2015-02-20 14:15:48 UTC
Nice! I'll port it to git master.
Comment 14 Dan Williams 2015-02-24 16:51:11 UTC
Review of attachment 297364 [details] [review]:

Looks good, just some whitespace fixup and the IPV4->IPV6 fixup.  THanks!

::: src/devices/wwan/nm-modem-broadband.c
@@ +369,3 @@
+		nm_log_dbg (LOGD_MB, "(%s) launching connection with ip type '%s'",
+					nm_modem_get_uid (NM_MODEM (ctx->self)),
+					nm_modem_ip_type_to_string (current));

Whitespace

@@ +375,3 @@
+								 NULL,
+								 (GAsyncReadyCallback)connect_ready,
+								 ctx);

Whitespace

@@ +383,3 @@
+		ctx->first_error = g_error_new_literal (NM_DEVICE_ERROR,
+												NM_DEVICE_ERROR_INVALID_CONNECTION,
+												"invalid bearer IP configuration");

Whitespace.

@@ +387,3 @@
+	nm_log_warn (LOGD_MB, "(%s) failed to connect modem: %s",
+				 nm_modem_get_uid (NM_MODEM (ctx->self)),
+				 ctx->first_error->message);

whitespace

::: src/devices/wwan/nm-modem.c
@@ +335,2 @@
+		/* If IPv4 may-fail=false, we should NOT try IPv6 as fallback */
+		if ((priv->ip_types & NM_MODEM_IP_TYPE_IPV4) && ip4_may_fail) {

I think this should be TYPE_IPV6

@@ +349,3 @@
+							 "Connection requested both IPv4 and IPv6 "
+							 "but dual-stack addressing is unsupported "
+							 "by the modem.");

Whitespace.
Comment 15 Aleksander Morgado 2015-02-24 17:54:00 UTC
Created attachment 297791 [details] [review]
Patch v3 [for nm-1-0]

Fixed the whitespaces issues in the patch.
This patch is for nm-1-0.
Comment 16 Aleksander Morgado 2015-02-24 17:54:40 UTC
Created attachment 297792 [details] [review]
Patch v3 [for git master]

Patch v3, but this time for git master.
Comment 17 Aleksander Morgado 2015-02-24 17:56:16 UTC
Created attachment 297794 [details] [review]
Additional patch for git master (alignment/indentation issues)

This patch goes on top of the previous Patch v3 [for git master]; fixes several alignment/indentation issues in the WWAN sources. Didn't backport these trivial fixes to nm-1-0.
Comment 18 Dan Williams 2015-02-24 22:30:14 UTC
Pushed both patches to git master and nm-1-0:

85d9132464b76b75c0145376458c35f16472dc32
a84744324f0467e9106a94dd03841ecc8df53c0e

I chose to simply backport the WWAN async disconnect patches to 1.0 instead of using the modified patch.