Re: [RFC] IWD as wifi backend

[Andrew Zaborowski <andrew zaborowski intel com>]

Hi Thomas,

Thanks for the motivating response,

(sorry for breaking the threading, I only now confirmed subscription
to the list)

On Wed, 29 Nov 2017 09:12:20 +0100 Thomas Haller wrote:
> Can you shortly comment on the plan (timetable) for first release of
> iwd and whether there is an effort to package it for distributions?
>
> I think Lubomir might be willing to maintain it for Fedora.

So we still want a few features that will imply DBus API changes, but
I heard Denis Kenzior say "I see no reason why we can’t start making
iwd releases" yesterday.  I added Marcel Holtmann to CC to see if he
has a rough timetable.

> > ---
> >  Makefile.am                       |   13 +-
> >  configure.ac                      |   20 +
> >  src/devices/wifi/nm-device-iwd.c  | 1924
> > +++++++++++++++++++++++++++++++++++++
> >  src/devices/wifi/nm-device-iwd.h  |   58 ++
> >  src/devices/wifi/nm-iwd-manager.c |  343 +++++++
> >  src/devices/wifi/nm-iwd-manager.h |   53 +
> >  6 files changed, 2409 insertions(+), 2 deletions(-)
> >  create mode 100644 src/devices/wifi/nm-device-iwd.c
> >  create mode 100644 src/devices/wifi/nm-device-iwd.h
> >  create mode 100644 src/devices/wifi/nm-iwd-manager.c
> >  create mode 100644 src/devices/wifi/nm-iwd-manager.h
> >
> > diff --git a/Makefile.am b/Makefile.am
> > index 7bce597e9..dbe5a4e14 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -2632,8 +2632,6 @@ core_plugins += src/devices/wifi/libnm-device-
> > plugin-wifi.la
> >
> >  src_devices_wifi_libnm_device_plugin_wifi_la_SOURCES = \
> >      src/devices/wifi/nm-wifi-factory.c \
> > -     src/devices/wifi/nm-device-wifi.c \
> > -     src/devices/wifi/nm-device-wifi.h \
> >      src/devices/wifi/nm-wifi-ap.c \
> >      src/devices/wifi/nm-wifi-ap.h \
> >      src/devices/wifi/nm-wifi-utils.c \
> > @@ -2641,6 +2639,17 @@
> > src_devices_wifi_libnm_device_plugin_wifi_la_SOURCES = \
> >      src/devices/wifi/nm-device-olpc-mesh.c \
> >      src/devices/wifi/nm-device-olpc-mesh.h
> >
> > +if WITH_IWD
> > +src_devices_wifi_libnm_device_plugin_wifi_la_SOURCES += \
> > +     src/devices/wifi/nm-device-iwd.c \
> > +     src/devices/wifi/nm-device-iwd.h \
> > +     src/devices/wifi/nm-iwd-manager.c \
> > +     src/devices/wifi/nm-iwd-manager.h
> > +else
> > +     src/devices/wifi/nm-device-wifi.c \
> > +     src/devices/wifi/nm-device-wifi.h \
> > +endif
>
> Can we make it that IWD or SUPPLICANT can be both enabled (optionally)
> at configure time, and select the right backend via a configuration
> like main.wifi=iwd|wpa-supplicant in NetworkManager.conf?

That will probably require moving the wpa_supplicant support to a new
subclass of NMDeviceWifi so let me try to do that first.  I'll then
split the patch into pieces and send a new version.

> Also for
> developing and built-time-checks it's useful to build every part of the
> source, otherwise when refactoring you frequently might break the
> build, because you are unaware that you need to change something in the
> disabled Wi-Fi backend.

Yes, good point.

> I think it's right that there still is only one libnm-device-plugin-
> wifi.so, but it should (optionally) be able to handle both.
>
> Also, for a binary distribution that packages both wpa-supplicant and
> iwd, it would be bad if it would need ot build two (conflicting)
> versions of NetworkManager (of two versions of the Wi-Fi device
> plugins).
>
> Actually, the rest blow looks good enough for me for inital inclusion.
> I think it's better to merge it early and improve incrementally,
> instead of going back and forth with revisions. For me, there is clear
> that we eventually want to support it, so merge early seems a faster
> approach to me.

Great!

> > +
> >  src_devices_wifi_libnm_device_plugin_wifi_la_CPPFLAGS = \
> >      -I$(srcdir)/src \
> >      -I$(builddir)/src \
> > diff --git a/configure.ac b/configure.ac
> > index 79791ab76..a6af48366 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -264,6 +264,26 @@ else
> >  fi
> >
> >  dnl
> > +dnl Default to using wpa_supplicant but allow IWD as wifi backend
> > +dnl
> > +AC_ARG_WITH(iwd,
> > +            AS_HELP_STRING([--with-iwd=yes],
> > +                           [Use IWD instead of wpa_supplicant as
> > wifi backend]),
>
> Say " (experimental)"?
> Since the plugin doesn't include any new public API, every misbehavior
> is just a bug that we can fix later. We don't commit to new API here.
> So, "experimental" is just cosmetic to set user expectations straight.

Ok, do you prefer that this be enabled by default so that it gets
build-tested and the iwd backend be guarded by the NetworkManager.conf
setting (also with "experimental" comment)?

Many thanks