[PATCH 0/2] dbus: Do Not Coalesce State Changed Signals

[Dan Williams]

On Mon, 2011-10-24 at 10:08 -0700, Sam Leffler wrote:
> On Mon, Oct 24, 2011 at 8:40 AM, Dan Williams <dcbw at redhat.com> wrote:
> > On Mon, 2011-10-24 at 18:26 +0300, Jouni Malinen wrote:
> >> On Mon, Oct 24, 2011 at 10:05:04AM -0500, Dan Williams wrote:
> >> > I used to have a problem with this, but I now see where it makes sense
> >> > for the State property alone.  Most properties *should* be coalesced to
> >> > reduce dbus traffic, but the State property should probably just trigger
> >> > an immediate emit of the interface's changed properties.  State doesn't
> >> > change that often so it wouldn't meaningfully increase D-Bus traffic.
> >>
> >> What exactly are these state changes used for? The main problem I see
> >> with this is that wpa_s->wpa_state is supposed to be internal data for
> >> wpa_supplicant and the values it has and the way they are used are
> >> subject to change whenever needed.. As such, I would really not want any
> >> external functional to depends on that for anything beyond maybe showing
> >> some debug information.
> >
> > NM uses it to determine what state the supplicant is in, obviously.  If
> > the supplicant is scanning, then we don't want to do certain operations.
> > If the supplicant is connecting, there are other operations we don't
> > want to do (like request a scan).
> 
> You get an explicit signal for scanning.

Yeah, because I added that, because scanning happens in other states
than SCANNING.

> >
> > If the supplicant fails during association/authentication, then the
> > connection manager wants to know that, because that could indicate
> > specific failures: a failure during 4-way handshake usually indicates
> > that a password is incorrect, and that's very useful information to a
> > connection manager.
> 
> We identify 4-way handshake failure when the CurrentBSS transitions to
> "/".  One might argue this is a hack but it's been as reliable as
> monitoring the state (both are unsatisfactory--there should be an
> explicit signal/property).

Well, that can happen for a number of reasons and could in the future,
and it relies on specific behavior of the D-Bus interface that might
change in the future.  In general you shouldn't be relying on signal
ordering for any sort of meaning.  It would be a lot clearer and more
robust to have an indication of 4-way handshake failure, or even better,
just some indication that the connection attempt failed due to invalid
X, where X is invalid certificates, invalid password, AP rejection,
whatever.  Doesn't need to be "4way handshake failed" so much as
"connection attempt failed and here's why..." with detailed reason codes
or something.

> >
> > Obviously we want notification of disconnection events so that in NM we
> > can start a timer for overall connection failure and after a certain
> > period of retries, terminate the attempt and look for a different AP.
> 
> Again, monitor CurrentBSS.

Again, seems like a hack.  CurrentBSS is useful when connected to detect
roaming behavior between APs, but really shouldn't be a proxy for
auth/assoc attempts and detection of failures.

> >
> > We need to know, after adding an interface, whether that interface is
> > now ready for action or not.  When the interface enters state COMPLETED,
> > we need to know to start IP configuration.  When the interface is DOWN
> > we need to know to clean stuff up and forget about the interface.  If
> > the interface is AUTHENTICATED and we're waiting for an EAP identity or
> > password, then when the interface transitions to DISCONNECTED we need to
> > know that we should stop waiting for user input.
> >
> > All sorts of stuff :)
> 
> We were able to accomplish all the above w/o using the state variable.
>  In fact it wasn't until we stopped using the state setting that we
> got reliable integration (even for the old dbus api).  The typical
> problem seems to there are two async state machines trying to control
> each other and that just causes horrible confusion.

What do you define "reliable" as?  The State variable has been pretty
solid for the last 3+ years for NetworkManager and I don't believe I've
encountered any of the cases that you've described here or in the past.
Are you by chance dumping all network blocks into the supplicant config
at the same time and letting the supplicant do network block selection?
That's something that NM does not do, but is something we'd like to do
in the future, and might be the cause of some of these issues.  So I'd
love to fix that for real, and some sort of synchronous state emission
would do that.

> >
> >> There are already some horrible hacks in place for wpa_state (just see
> >> how WPA_IDLE is generated in Android builds) and I really do not want to
> >> have to consider supporting something like that with the D-Bus
> >> interface.
> >
> > Well, it's extremely useful to have something like this, as opposed to a
> > plethora of signals for other stuff.  Having one state signal for an
> > interface means one centralized place that client code can perform
> > actions based on what's happening in the supplicant.  If not state
> > signals then I'm not sure what we'd use, but hopefully it's not a
> > combination of other signals since those arrive asynchronously and thus
> > are harder to handle.
> 
> Integration of supplicant w/ a connection manager is problematic given
> both want control under certain conditions.  Until supplicant stops
> wanting to do so much or connection managers _really_ defer control to
> supplicant there will continue to be madness.  We started w/ the
> connman approach that the connection manager should be in control but
> very quickly recognized this would never work except in the simplest
> scenarios.  We stabilized once we moved more control to supplicant and
> continue in this direction.

Hmm, again, it's worked pretty well so far for NM; I'm apparently just
not familiar with the problems you've encountered.  NM takes a more CM
oriented approach by doing network selection itself and only sending one
network block down the supplicant.  That does slightly increase
connection latency and we're moving in the direction of pushing all
network blocks down to the supplicant and letting it handle network
selection, but there's some features we need from the supplicant first
that I've started pushing patches for.

So I'm interested in what problems you had with the more CM oriented
approach, and what problems you have with more control in the
supplicant.

> >
> >> If there is desire to actually use this type of information for taking
> >> some actions, it would be much safer to define a new value that will be
> >> generated based on internal state (including wpa_s->wpa_state) and
> >> define that in a way that makes it easier to maintain this in a stable
> >> way even if the internal implementation changes.
> >
> > That's fine with me, we could simply keep the same state values and just
> > change how the D-Bus property value is calculated.  But having a
> > notification of supplicant interface state is really, really useful.
> 
> I asked for this to be optional because it just adds overhead for us
> and may introduce subtle breakage.  I still haven't seen a good reason
> to change the current behaviour (e.g. we've demonstrated there's no
> need for this change).  If there are explicit events clients want it
> would seem better add signals than expose internal supplicant state
> that will be used to inter these events.

I think mainly because other mechanisms aren't really appropriate as
proxies for actual interface state.  CurrentBSS is simply a pointer to
the current BSS the supplicant is working with, but that doesn't
actually give you any indication of what's really going on, or what the
failures actually were if there were failures.  Next up, how do you
determine when the supplicant has completed the process and is fully
associated, so you can being IP configuration?  AFAIK the only way to
figure that out is the State property, because CurrentBSS is (or if not,
should be) set during the association/authentication states as well.

Dan