Issues with new D-Bus API

[Marcel Holtmann]

Hi Witold,

> > so I have been hacking on a client using the new D-Bus API and found a
> > couple of things that are sort of weird and could be done way simpler.
> > Some of them seems just copied from the old API and I think it is time
> > to fix this now before we actual get real users of this API.
> > 
> I started to work on some patches to address some of those issues. You
> can pull it from my git: git://repo.or.cz/hostap-gosc2009.git dbus
> branch. I haven't tested it yet and I haven't modified python test scrip
> to fit API changes too, but the documentation is up to date. There is
> some duplicated code cleaning too.

thanks for the work. I will test it as soon as possible.

> > 3) CreateInterface properties are weird
> > 
> > The specification list the "Bridge_ifname" property. That should be
> > called "BridgeIfname" to match the name of the interface properties.
> > 
> Where exactly did you find "Bridge_ifname" property name?

It was in the specification document. I actually never doubled checked
with the source code.

> > In addition I prefer if we can have "Ifindex" property and only "Ifname"
> > or "Ifindex are required to be provided. Same goes for including the
> > "Ifname" property for the interface.
> > 
> > Having the interface name is nice, but it is not guaranteed to be
> > stable. I looked through the wpa_supplicant code and it doesn't really
> > expose the index, but the nl80211 driver does. I am worried about
> > interface renames while WiFi operation is in place. The only stable
> > identifier is the index in Linux.
> > 
> Well, Dbus interface is just frontend. We can't expose variables which
> are not available from wpa_supplicant (i.e. we can't get properties
> values directly from driver). The other issue is that wpa_supplicant is
> supposed to work with other drivers than nl80211.

We might need to extend wpa_supplicant a bit. And if index != -1 then we
can use it. Otherwise we won't. So it will be a Linux specific thing,
but that is fine with me.

> > And just supporting both, "Ifname" and "Ifindex", doesn't bloat the API
> > at all. It actually does make it easier for the client.
> > 
> > 4) Changes to Scanning property
> > 
> > The Scanning property of an interface only changes if wpa_supplicant
> > triggers the scanning. Not if you trigger it via iw or other tools. The
> > kernel emits a signal when a scanning is started. So at least with the
> > nl80211 we should be using that one and change Scanning property to
> > match the actual scanning state of the device.
> > 
> Ditto.

We need some extra wpa_supplicant hooks, but it should be useful. Since
all drivers should actually know about it.

> > 6) Usefulness of ScanDone signal
> > 
> > The ScanDone signal is pretty nice to have, but currently it only shows
> > if the scan succeeded or failed. What about actually including the
> > information from the nl80211 driver on why the scan was triggered?
> > 
> And again. Is that information available directly from scan function or
> we would need to get if somehow from driver's private data?
> 
> > An alternative would be to make the Scan method actually return the
> > result. Especially with the BSSAdded and BSSRemoved doing all the work,
> > that might be a way better alternative. Essentially the client only
> > cares about the results of scans it triggered anyway.
> > 
> But then the result of scan method would arrive to client quite long
> after calling. I would prefer to not have any "blocking" methods.

Since D-Bus is ALWAYS async, nothing would be blocking. This is why I
think just returning the ScanDone result via the Scan method would be
beneficial for the clients. This way they only have to make a method
call. And not method call + listen on signal.

> > 8) Network properties
> > 
> > That thing is still a mess and we should do something about it. Having
> > everything as strings is kinda bad. I do remember we talked about it,
> 
> Heh, I do remember it too. IIRC, Dan wanted to have all options as
> strings, but I don't remember why :)
> 
> > but not being able to clearly match up a SSID from a network with a SSID
> > from a BSS. I do think we should have something cleaner and simpler
> > there and not necessary copy the syntax of the wpa_supplicant
> > configuration file. Especially the interface capabilities like KeyMgmt
> > should match the property names we use in the network configuration.
> > 
> We could do some mapping from config options to properties.
> Additionally, we could keep a dictionary with all options in strings too
> if it is really needed.

We have to figure something out here. I will be looking into some
details later. Still working on some other testing.

> > 9) Miscellaneous
> > 
> > I already mentioned that we should add the full dictionary of properties
> > as second argument to the signals InterfaceAdded, NetworkAdded and
> > BSSAdded. I do think that we can leave BlobAdded out of this since it
> > just is a data storage.
> > 
> Done, but still I'm not sure if it's right. Those signals are much
> bigger now and it probably may have some negative impact on performance.
> If a client is interested in object properties, it can always ask about
> those via GetAll.

It is one signal compared to one signal + GetAll method call. That is
preferred. Especially with the D-Bus bus daemon itself filtering out the
signal and not sending them to clients that don't care. So there is no
overhead, just gain. From a D-Bus roundtrip point of view and also for
the client complexity.

> 10) All BSSs properties at once.
> 
> As you proposed earlier, a method returning a dict with BSS objects and
> its properties would be quite useful for clients connecting to running
> wpa_supplicant. I'm not sure if DBus allows to make dictionaries with
> other keys than strings. In case it doesn't, we can store object paths
> as strings in dictionary keys or we can return an array of structs
> (pairs) like a(oa{sv}). Similar methods would be useful for networks and
>  interfaces.

Correct. I forgot about that one.

And yes, you can create as complex dicts as you want. You are using a
variant container and that can contain any other type. Including another
dictionary.

Regards

Marcel