[review] dcbw/ipv6-fixes: fix IPv6 support in core and plugins

[Aleksander Morgado]

On Tue, Jun 10, 2014 at 8:08 PM, Dan Williams <dcbw at redhat.com> wrote:
> For QMI, static configuration is always used, because we can always read
> the IP addresses and DNS servers from the firmware.

One issue with the IPv4 static config.

I have this Huawei device which doesn't return IP Family TLV in the
Get Current Settings reply:

ModemManager[8148]: <debug> [1402483095.253942] [mm-bearer-qmi.c:476]
get_current_settings_ready():  IP Family: failed (Field 'IP Family'
was not found in the message)
ModemManager[8148]: <debug> [1402483095.253965] [mm-bearer-qmi.c:493]
get_current_settings_ready():    Domains: failed (Field 'Domain Name
List' was not found in the message)
ModemManager[8148]: <debug> [1402483095.253990] [mm-port.c:93]
mm_port_set_connected(): (wwp0s29u1u2i1): port now connected

The bearer is created, but it shows 'unknown' method for IPv4 and IPv6:
  -------------------------
  IPv4 configuration |   method: 'unknown'
  -------------------------
  IPv6 configuration |   method: 'unknown'

So NetworkManager fails to connect:
jun 11 12:37:16 athena NetworkManager[419]: <warn> (cdc-wdm0) failed
to connect modem: invalid IP config
jun 11 12:37:16 athena NetworkManager[419]: <info> (cdc-wdm0): device
state change: prepare -> failed (reason 'unknown') [40 120 1]


The culprit lines seem to be around here:

        if (qmi_message_wds_get_current_settings_output_get_ip_family
(output, &ip_family, &error)) {
            mm_dbg (" IP Family: %s",
                    (ip_family == QMI_WDS_IP_FAMILY_IPV4) ? "IPv4" :
                       (ip_family == QMI_WDS_IP_FAMILY_IPV6) ? "IPv6"
: "unknown");

            if (!qmi_message_wds_get_current_settings_output_get_mtu
(output, &mtu, &error)) {
                mm_dbg ("       MTU: failed (%s)", error->message);
                g_clear_error (&error);
            }

            if (ip_family == QMI_WDS_IP_FAMILY_IPV4)
                ctx->ipv4_config = get_ipv4_config (output, mtu);
            else if (ip_family == QMI_WDS_IP_FAMILY_IPV6)
                ctx->ipv6_config = get_ipv6_config (output, mtu);
        } else {
            mm_dbg (" IP Family: failed (%s)", error->message);
            g_clear_error (&error);
        }

When no IP Family TLV is given, I guess we should assume it will be
IPv4? Something like:

diff --git a/src/mm-bearer-qmi.c b/src/mm-bearer-qmi.c
index 5c0a9e6..7f321f3 100644
--- a/src/mm-bearer-qmi.c
+++ b/src/mm-bearer-qmi.c
@@ -473,8 +473,9 @@ get_current_settings_ready (QmiClientWds *client,
             else if (ip_family == QMI_WDS_IP_FAMILY_IPV6)
                 ctx->ipv6_config = get_ipv6_config (output, mtu);
         } else {
-            mm_dbg (" IP Family: failed (%s)", error->message);
+            mm_dbg (" IP Family: failed (%s). Assuming IPv4.", error->message);
             g_clear_error (&error);
+            ctx->ipv4_config = get_ipv4_config (output, mtu);
         }

And BTW; if reading the MTU fails, the 'mtu' variable won't get
initialized, but will still be used afterwards:

        guint32 mtu;
        ....
            if (!qmi_message_wds_get_current_settings_output_get_mtu
(output, &mtu, &error)) {
                mm_dbg ("       MTU: failed (%s)", error->message);
                g_clear_error (&error);
            }
            if (ip_family == QMI_WDS_IP_FAMILY_IPV4)
                ctx->ipv4_config = get_ipv4_config (output, mtu);

I'm not sure if it may ever happen, but at least we should initialize
to some default value, even if it's 0.


-- 
Aleksander
https://aleksander.es