Re: [PATCH 3/4] devices/wifi: Add the wifi-backend config option

[Dan Williams <dcbw redhat com>]

On Tue, 2017-12-05 at 16:26 +0100, Andrew Zaborowski wrote:
> Let the config file select between creating classes of NMDeviceWifi
> (for the usual wpa_supplicant based devices) and NMDeviceIwd
> depending
> on the new NetworkManager.conf setting.

This seems wrong to me.  We usually try to keep things runtime enabled
rather than add config options like this.  Is there any reason you
would have both wpa_supplicant and iwd running on the same system at
the same time?  Could we check whether the iwd dbus service is claimed
and just use iwd if it's running?

Dan

> ---
>  man/NetworkManager.conf.xml        | 13 +++++++++++++
>  src/devices/wifi/nm-wifi-factory.c | 23 ++++++++++++++++++++---
>  src/nm-config.h                    |  1 +
>  3 files changed, 34 insertions(+), 3 deletions(-)
>
> diff --git a/man/NetworkManager.conf.xml
> b/man/NetworkManager.conf.xml
> index 94465a019..a7fa752e9 100644
> --- a/man/NetworkManager.conf.xml
> +++ b/man/NetworkManager.conf.xml
> @@ -426,6 +426,19 @@ no-auto-default=*
>            </para>
>          </listitem>
>        </varlistentry>
> +
> +      <varlistentry>
> +        <term><varname>wifi-backend</varname></term>
> +        <listitem>
> +          <para>
> +            If present, specifies the WiFi backend to use. Allowed
> +            values are <literal>wpa_supplicant</literal> and, if
> +            enabled during compilation, <literal>iwd</literal>
> +         (expermiental). <literal>wpa_supplicant</literal> is
> also
> +            the default backend.
> +          </para>
> +        </listitem>
> +      </varlistentry>
>      </variablelist>
>    </refsect1>
>  
> diff --git a/src/devices/wifi/nm-wifi-factory.c
> b/src/devices/wifi/nm-wifi-factory.c
> index a1752634b..6c6984eab 100644
> --- a/src/devices/wifi/nm-wifi-factory.c
> +++ b/src/devices/wifi/nm-wifi-factory.c
> @@ -27,8 +27,10 @@
>  #include "nm-setting-olpc-mesh.h"
>  #include "nm-device-wifi.h"
>  #include "nm-device-olpc-mesh.h"
> +#include "nm-device-iwd.h"
>  #include "settings/nm-settings-connection.h"
>  #include "platform/nm-platform.h"
> +#include "nm-config.h"
>  
>  /*******************************************************************
> **********/
>  
> @@ -75,6 +77,7 @@ create_device (NMDeviceFactory *factory,
>  {
>       NMDeviceWifiCapabilities capabilities;
>       NM80211Mode mode;
> +     const char *backend;
>  
>       g_return_val_if_fail (iface != NULL, NULL);
>       g_return_val_if_fail (plink != NULL, NULL);
> @@ -98,10 +101,24 @@ create_device (NMDeviceFactory *factory,
>               return NULL;
>       }
>  
> -     if (plink->type == NM_LINK_TYPE_WIFI)
> -             return nm_device_wifi_new (iface, capabilities);
> -     else
> +     if (plink->type != NM_LINK_TYPE_WIFI)
>               return nm_device_olpc_mesh_new (iface);
> +
> +     backend = nm_config_data_get_value (NM_CONFIG_GET_DATA_ORIG,
> +                                         NM_CONFIG_KEYFILE_GROUP_
> MAIN,
> +                                         NM_CONFIG_KEYFILE_KEY_MA
> IN_WIFI_BACKEND,
> +                                         NM_CONFIG_GET_VALUE_STRI
> P);
> +
> +     nm_log_warn (LOGD_PLATFORM | LOGD_WIFI, "(%s) config:
> backend is %s, %i", iface, backend, WITH_IWD);
> +     if (!backend || !strcasecmp (backend, "wpa_supplicant"))
> +             return nm_device_wifi_new (iface, capabilities);
> +#if WITH_IWD
> +     else if (!strcasecmp (backend, "iwd"))
> +             return nm_device_iwd_new (iface, capabilities);
> +#endif
> +
> +     nm_log_warn (LOGD_PLATFORM | LOGD_WIFI, "(%s) config:
> unknown or unsupported wifi-backend %s", iface, backend);
> +     return NULL;
>  }
>  
>  /*******************************************************************
> **********/
> diff --git a/src/nm-config.h b/src/nm-config.h
> index 47e929884..d94a279ca 100644
> --- a/src/nm-config.h
> +++ b/src/nm-config.h
> @@ -64,6 +64,7 @@
>  #define NM_CONFIG_KEYFILE_KEY_MAIN_DEBUG                    "debug"
>  #define
> NM_CONFIG_KEYFILE_KEY_MAIN_HOSTNAME_MODE            "hostname-mode"
>  #define NM_CONFIG_KEYFILE_KEY_MAIN_SLAVES_ORDER             "slaves-
> order"
> +#define NM_CONFIG_KEYFILE_KEY_MAIN_WIFI_BACKEND             "wifi-
> backend"
>  #define
> NM_CONFIG_KEYFILE_KEY_LOGGING_BACKEND               "backend"
>  #define NM_CONFIG_KEYFILE_KEY_CONFIG_ENABLE                 "enable"
>  #define NM_CONFIG_KEYFILE_KEY_ATOMIC_SECTION_WAS            ".was"