ubuntu-phone team mailing list archive

[Tony Espy <espy@xxxxxxxxxxxxx>]

If you work on Touch system components that use DBus, please read...

I've spent the last few months working on a bug #1480877 [1]:

"Access points' "PropertiesChanged" dbus signals freeze UI on mobile 
devices"

The DBus signals in question are generated and sent on the system bus by 
NM when a WiFi scan completes.  Initially I fixed a bug in NM that 
caused a 2-3x number of PropertiesChanged signals to be emitted every 
time a scan completed. Unfortunately, this didn't fix the UI freezes.

Eventually, after looking at the DBus source, and researching DBus 
performance, I was led to the a new debug method added to the DBus 
Statistics interface, called "GetAllMatchRules".  As this wasn't 
available in vivid, I backported it the version used in touch.  This 
package can be found in my PPA ( or wily & xenial ) for those interested 
in giving it a try [2].

A "match rule" is something that a DBus client sends to the DBus daemon 
in order to instruct the daemon which DBus messages it would like to see 
while connected to the bus.  Sometimes these rules are created 
explicitly by a client, and sometime they're generated by the DBus 
library used.

An example of a good rule is:

type='signal',
interface='org.freedesktop.NetworkManager.AccessPoint',
member='PropertiesChanged',
path='/org/freedesktop/NetworkManager/AccessPoint/0',
sender='org.freedesktop.NetworkManager'

This asks the daemon to route the signal 'PropertiesChanged', sent using 
the interface 'org.freedesktop.NetworkManager.AccessPoint' for the NM 
DBus object '/AccessPoint/0' by NetworkManager, to the my client.

Sometimes however, our clients don't specify fully qualified match rules 
( ie. one or more parameters are missing ).  This leads to less 
efficient use of the bus, as the daemon may need to consider more rules 
than necessary when determining where to route an incoming message.  In 
some cases, it can result in messages being sent to a client that are 
uncessary, which can lead to unecessary context switches/wakeups.

There are three types of non-fully-qualified match rules which can 
impact performance:

1. Not specifying a message 'type'

~99% of the time, match rules are used for signal notification. The only 
time a match rule is useful for other message types is when snooping the 
bus.  If a match rule is specified without type='signal', then the bus 
needs to consider it for other message types such as 'method_call', 
'method_return', or 'error'.  These are generally only listened for when 
snooping.  If you're really interested in only signals, then specify 'type'.

2. Not specifying a message 'sender'

Without sender ( eg. sender='org.freedesktop.NetworkManager' ), the bus 
needs to compare the match rule to messages from every sender on the bus 
vs. just the sender you're interested ( eg. NetworkManager ).

Not including a sender also has security implications, as it's possible 
for DBus interfaces/objects to be spoofed.

3. Match rule for 'NameOwnerChanged' without a arg0* criteria.

If your client just asks for 'NameOwnerChanged' without specifying an 
'arg0namespace', as this signal is is sent on the generic DBus interface 
( org.freedesktop.DBus ), your client will get notified for every name 
owner change on the bus vs. just the name(s) you're interested in.

There's a new script called GetAllMatchRules.py which was added to the 
dbus project's tools directory.  When this is run against a particular 
bus, it will generate a set of warning for any match rules that are 
found with the criteria mentioned above.  To run it on the system bus, 
you need to be root, and specify the argument "--system".  To run it 
against the session bus, just run as the phablet user with no args. 
Note, I've attached a modified version of this script, as I didn't 
package it in the version of DBus I pushed to my PPA.

Note, there's another facet of DBus client related performance, which is 
analyzing the match rules your client generates, and determining whether 
or not they're all really necessary.  You can ask the bus to dump *all* 
of it's current rules by specifying the "--all" parameter to the script. 
While debugging lp: #1480877 for instance, we discovered that we had 
many more processes listening to NM signals than was actually necessary. 
 This turned out to be caused by a platform-specific bug in Qt's 
networking code, which was just fixed last week.

I've gone ahead and filed bugs for the warnings currently generated on 
both buses for our current rc-proposed images ( as seen on krillin and 
mako ). I've tagged these bugs "dbus-match" [3].  If you're on the 
receiving end of one of these bugs, please let me know if you have 
questions/concerns/...

Thanks,
/tony

[1] https://bugs.launchpad.net/canonical-devices-system-image/+bug/1480877
[2] https://launchpad.net/~awe/+archive/ubuntu/ppa/+packages
[3] https://goo.gl/WChTvm

#!/usr/bin/python3

import sys
import argparse
import dbus
import time

def get_cmdline(pid):
  cmdline = ''
  if pid > 0:
    try:
      procpath = '/proc/' + str(pid) + '/cmdline'
      with open(procpath, 'r') as f:
        cmdline = " ".join(f.readline().split('\0'))
    except:
      pass
  return cmdline

# Parsing parameters

parser = argparse.ArgumentParser(description='Testing D-Bus match rules')
parser.add_argument('--session', help='session bus', action="store_true")
parser.add_argument('--system', help='system bus', action="store_true")
parser.add_argument('--all', help='print all match rules', action="store_true")
args = parser.parse_args()

if args.system and args.session:
  parser.print_help()
  sys.exit(1)

# Fetch data from the bus driver

if args.system:
  bus = dbus.SystemBus()
else:
  bus = dbus.SessionBus()

remote_object = bus.get_object("org.freedesktop.DBus",
                               "/org/freedesktop/DBus")
bus_iface = dbus.Interface(remote_object, "org.freedesktop.DBus")
stats_iface = dbus.Interface(remote_object, "org.freedesktop.DBus.Debug.Stats")

try:
  match_rules = stats_iface.GetAllMatchRules()
except:
  print("GetConnectionMatchRules failed: did you enable the Stats interface?")
  sys.exit(1)

names = bus_iface.ListNames()
unique_names = [ a for a in names if a.startswith(":") ]
pids = dict((name, bus_iface.GetConnectionUnixProcessID(name)) for name in unique_names)
cmds = dict((name, get_cmdline(pids[name])) for name in unique_names)
well_known_names = [ a for a in names if a not in unique_names ]
owners = dict((wkn, bus_iface.GetNameOwner(wkn)) for wkn in well_known_names)

rules = dict((k_rules,
              dict({
                'wkn': [k for k, v in owners.items() if v == k_rules],
                'pid': pids[k_rules],
                'cmd': cmds[k_rules] or "",
                'rules': v_rules,
                'warnings': dict({
                  'not_signal': [a for a in v_rules if "type='signal'" not in a],
                  'no_sender': [a for a in v_rules if "sender=" not in a],
                  'local': [a for a in v_rules if "org.freedesktop.DBus.Local" in a],
                  'NameOwnerChanged_arg0': [a for a in v_rules if "member='NameOwnerChanged'" in a and "arg0" not in a]
                })
              })
             ) for k_rules, v_rules in match_rules.items())

warnings = dict({
             'not_signal': 'Match rule without selecting signals',
             'no_sender': 'Match rule without a sender criteria',
             'local': 'Match rule on the org.freedesktop.DBus.Local interface',
             'NameOwnerChanged_arg0': 'Match rule on NameOwnerChanged without a arg0* criteria'
           })

# Print the match rules

# print all match rules without analysing them
if args.all:
  for name in rules:
    print("Connection %s with pid %d '%s' (%s): %d match rules, %d warnings"
          % (name, rules[name]['pid'], rules[name]['cmd'],
             ' '.join(rules[name]['wkn']), len(rules[name]['rules']),
             len(sum(rules[name]['warnings'].values(), []))))
    for rule in rules[name]['rules']:
      print("\t%s" % (rule))
    print("")
  sys.exit(0)

# analyse match rules and print only the suspicious ones
for conn,data in rules.items():
  warnings_count = len(sum(data['warnings'].values(), []))
  if warnings_count == 0:
    continue

  print("Connection %s with pid %d '%s' (%s): %d match rules, %d warnings"
        % (conn, data['pid'], data['cmd'], ' '.join(data['wkn']),
           len(data['rules']), warnings_count))

  for warn_code,rule_list in [(warn_code,rule_list) \
                              for warn_code, rule_list \
                              in data['warnings'].items() \
                              if len(rule_list) > 0]:
    print("   - %s:" % (warnings[warn_code]))
    for rule in rule_list:
      print("         - %s" % (rule))

  print("")

sys.exit(0)