Bug 85012 - icera: fix parsing IPv6 config logic
Summary: icera: fix parsing IPv6 config logic
Status: RESOLVED FIXED
Alias: None
Product: ModemManager
Classification: Unclassified
Component: plugins (show other bugs)
Version: unspecified
Hardware: Other All
: medium major
Assignee: Aleksander Morgado
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-10-14 16:55 UTC by Aleksander Morgado
Modified: 2014-12-03 18:07 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
Patch (4.66 KB, patch)
2014-11-14 11:35 UTC, Aleksander Morgado
Details | Splinter Review
Patch v2 (8.35 KB, patch)
2014-11-14 13:43 UTC, Aleksander Morgado
Details | Splinter Review
Debug output for issue #1 (46.33 KB, text/plain)
2014-11-15 11:05 UTC, Tore Anderson
Details
Debug output for issue #2 (49.03 KB, text/plain)
2014-11-15 11:05 UTC, Tore Anderson
Details

Description Aleksander Morgado 2014-10-14 16:55:06 UTC
Originally reported at:
  https://bugzilla.gnome.org/show_bug.cgi?id=733394
Please refer to the original bug report if more details are needed.

The Icera plugin should treat "::" for IPv6 the same way as it treats "0.0.0.0" for IPv4. Currently the "::" string is being passed to inet_pton(), which doesn't parse it correctly, and en error is issued (which actually prevents connection):
  Couldn't connect bearer: 'Couldn't parse IPv6 address '::''

Plus, the logic for IPv4 and IPv6 should be the same in parse_ipdpaddr_v4() and parse_ipdpaddr_v6(). In particular:
  * An empty placeholder like "::" in the ADDRESS should end up returning NULL without error (i.e. no error parsing but no IPv6 configuration available).
  * If there is a valid ADDRESS, but an empty placeholder like "::" is found in the DNS addresses, these should just be ignored (instead of returning an error).
Comment 1 Aleksander Morgado 2014-11-14 11:35:41 UTC
Created attachment 109456 [details] [review]
Patch

Suggested patch, with unit test.

Review and/or tests welcome :)
Comment 2 Tore Anderson 2014-11-14 12:49:02 UTC
Hi, I can confirm that the patch works, now I can connect to an IPv4-only APN using the IPV4V6 PDP context type, and end up with working IPv4 connectivity. I could not before.

However I noticed a similar bug when connecting to a dual-stack APN (the dual-stack APN with network support for IPV4V6 is something my provider has recently set up, so I haven't tested that before):

tore@envy:~$ sudo mmcli -m 5 --simple-connect=apn=telenor.smart,ip-type=ipv4v6
error: couldn't connect the modem: 'GDBus.Error:org.freedesktop.ModemManager1.Error.Core.Failed: Couldn't get IP config: couldn't parse response '%IPDPADDR: 1, 0.0.0.0, 0.0.0.0, 193.213.112.4, 130.67.15.198, 0.0.0.0, 0.0.0.0, 0.0.0.0, 0.0.0.0, ::, ::, 2001:4600:4:fff::52, 2001:4600:4:1fff::52, ::, ::, ::, ::''

However I think this might be a modem firmware bug. It seems bogus for the modem to return IPv6 name servers but no IPv6 interface ID is returned. That said, it actually works if I enable the interface, as I do see my provider's router advertisements show up and I do get working IPv6 connectivity through the interface. So perhaps it would be possible for MM to say not fail in this situation?

Tore
Comment 3 Aleksander Morgado 2014-11-14 13:33:40 UTC
(In reply to Tore Anderson from comment #2)
> Hi, I can confirm that the patch works, now I can connect to an IPv4-only
> APN using the IPV4V6 PDP context type, and end up with working IPv4
> connectivity. I could not before.
> 
> However I noticed a similar bug when connecting to a dual-stack APN (the
> dual-stack APN with network support for IPV4V6 is something my provider has
> recently set up, so I haven't tested that before):
> 
> tore@envy:~$ sudo mmcli -m 5
> --simple-connect=apn=telenor.smart,ip-type=ipv4v6
> error: couldn't connect the modem:
> 'GDBus.Error:org.freedesktop.ModemManager1.Error.Core.Failed: Couldn't get
> IP config: couldn't parse response '%IPDPADDR: 1, 0.0.0.0, 0.0.0.0,
> 193.213.112.4, 130.67.15.198, 0.0.0.0, 0.0.0.0, 0.0.0.0, 0.0.0.0, ::, ::,
> 2001:4600:4:fff::52, 2001:4600:4:1fff::52, ::, ::, ::, ::''
> 
> However I think this might be a modem firmware bug. It seems bogus for the
> modem to return IPv6 name servers but no IPv6 interface ID is returned. That
> said, it actually works if I enable the interface, as I do see my provider's
> router advertisements show up and I do get working IPv6 connectivity through
> the interface. So perhaps it would be possible for MM to say not fail in
> this situation?
> 

Actually, the patch is wrong. Let me try to rewrite it...
Comment 4 Aleksander Morgado 2014-11-14 13:43:33 UTC
Created attachment 109466 [details] [review]
Patch v2

Bearer connection type will now default to DHCP if DNS IPv6 is given but no IPv6 address is given.
Comment 5 Tore Anderson 2014-11-15 11:04:03 UTC
Tested the new patch in various ways, here are the results:

+----------------+--------------------------------------+
|  Network/APN   |      Requested ip-type (by NM)       |
| PDP capability | IPv4-only  | IPv6-only  | Dual-Stack |
+----------------+------------+------------+------------+
| IP,IPV6,IPV4V6 | OK (IPv4)  | OK (IPv6)  | Issue #1   |
|    IP,IPV6     | OK (IPv4)  | OK (IPv6)  | OK (DS)    |
|      IPV6      | N/A        | OK (IPv6)  | Issue #2   |
|       IP       | OK (IPv4)  | N/A        | OK (IPv4)  |
+----------------+------------+------------+------------+

Issue #1: Even though the APN supports IPV4V6, only IPv6 addresses gets configured on the WWAN interface (and this works, I can successfully ping IPv6 addresses on the Internet). I get both IPv4 and IPv6 default routes pointing to the WWAN interface. No DNS servers are written to /etc/resolv.conf. This might well be caused by a modem firmware bug though, as I understand that support for the IPV4V6 PDP type is quite spotty on 3G equipment (the %IPDPADDR output does look fishy to me).

Issue #2: Even though this APN does not provide IPv4 connectivity, an IPv4 default route is added, using the WWAN interface as the next hop. Everything works, except for the fact that IPv4 socket calls must time out rather than fail quickly with "no route to host". This might be a NM bug, though. If you concur, I'll file a NM bug for it.

I'll attach MM/NM debug output from when reproducing the above two issues.

Tore
Comment 6 Tore Anderson 2014-11-15 11:05:34 UTC
Created attachment 109513 [details]
Debug output for issue #1
Comment 7 Tore Anderson 2014-11-15 11:05:49 UTC
Created attachment 109514 [details]
Debug output for issue #2
Comment 8 Aleksander Morgado 2014-11-18 14:00:42 UTC
For reference, this is be the IPADDR return for issue #1:
%IPDPADDR: 1, 0.0.0.0, 0.0.0.0, 193.213.112.4, 130.67.15.198, 0.0.0.0, 0.0.0.0, 0.0.0.0, 0.0.0.0, ::, ::, 2001:4600:4:fff::52, 2001:4600:4:1fff::52, ::, ::, ::, ::

And this is the IPDPADDR return for issue #2:
%IPDPADDR: 1, 0.0.0.0, 0.0.0.0, 0.0.0.0, 0.0.0.0, 0.0.0.0, 0.0.0.0, 0.0.0.0, 0.0.0.0, fe80::18:bfc4:3b01, ::, 2001:4600:4:fff::54, 2001:4600:4:1fff::54, ::, ::, ::, ::
Comment 9 Dan Williams 2014-11-24 22:14:33 UTC
Latest patch (109466) appears to work for me and looks like it does the right thing.  I don't have a modem that exhibits the no-IP-address behavior though.

Re issue #1:

nov. 15 11:53:27 envy.fud.no ModemManager[8326]: <debug> (ttyACM17): <-- '<CR><LF>%IPDPADDR: 1, 0.0.0.0, 0.0.0.0, 193.213.112.4, 130.67.15.198, 0.0.0.0, 0.0.0.0, 0.0.0.0, 0.0.0.0, ::, ::, 2001:4600:4:fff::52, 2001:4600:4:1fff::52, ::, ::, ::, ::<CR><LF>OK<CR><LF>'

The firmware didn't return an IPv4 address or gateway, just DNS servers.  So ModemManager won't return an IPv4 bearer object at all.  This leads you to:

nov. 15 11:53:27 envy.fud.no NetworkManager[771]: <info>  (ttyACM17): IPv4 configuration disabled

Which I cannot reproduce, but that may be due to bugfixes that the th/bgo735512_route_metric_v2 branch that Thomas merged on 2014-11-19 that your checkout doesn't appear to have.


Re issue #2: I think this is the same thing as issue #1, and I think that's fixed with latest git master after 2014-11-19.

Can you re-test the NetworkManager behavior with latest git master?
Comment 10 Tore Anderson 2014-11-25 17:29:16 UTC
(In reply to Dan Williams from comment #9)
> nov. 15 11:53:27 envy.fud.no ModemManager[8326]: <debug> (ttyACM17): <--
> '<CR><LF>%IPDPADDR: 1, 0.0.0.0, 0.0.0.0, 193.213.112.4, 130.67.15.198,
> 0.0.0.0, 0.0.0.0, 0.0.0.0, 0.0.0.0, ::, ::, 2001:4600:4:fff::52,
> 2001:4600:4:1fff::52, ::, ::, ::, ::<CR><LF>OK<CR><LF>'
> 
> The firmware didn't return an IPv4 address or gateway, just DNS servers.

Yep. Probably a firmware bug. 3G gear is known to support the true IPV4V6 PDP type poorly.

> Can you re-test the NetworkManager behavior with latest git master?

Yep, looks good now. No default IPv4 route shows up for either of the two cases that was noted with "issue 1" and "issue 2".

Tore
Comment 11 Aleksander Morgado 2014-11-25 17:36:43 UTC
(In reply to Tore Anderson from comment #10)
> (In reply to Dan Williams from comment #9)
> > nov. 15 11:53:27 envy.fud.no ModemManager[8326]: <debug> (ttyACM17): <--
> > '<CR><LF>%IPDPADDR: 1, 0.0.0.0, 0.0.0.0, 193.213.112.4, 130.67.15.198,
> > 0.0.0.0, 0.0.0.0, 0.0.0.0, 0.0.0.0, ::, ::, 2001:4600:4:fff::52,
> > 2001:4600:4:1fff::52, ::, ::, ::, ::<CR><LF>OK<CR><LF>'
> > 
> > The firmware didn't return an IPv4 address or gateway, just DNS servers.
> 
> Yep. Probably a firmware bug. 3G gear is known to support the true IPV4V6
> PDP type poorly.
> 
> > Can you re-test the NetworkManager behavior with latest git master?
> 
> Yep, looks good now. No default IPv4 route shows up for either of the two
> cases that was noted with "issue 1" and "issue 2".
> 

So, all good to merge patch v2?
Comment 12 Tore Anderson 2014-11-25 18:11:27 UTC
(In reply to Aleksander Morgado from comment #11)
> So, all good to merge patch v2?

As far as I'm concerned, at least...

Tore
Comment 13 Dan Williams 2014-11-26 20:44:56 UTC
(In reply to Aleksander Morgado from comment #11)
> So, all good to merge patch v2?

+1 from me.
Comment 14 Aleksander Morgado 2014-12-03 18:07:35 UTC
Pushed to git master and mm-1-4.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.