Re: [PATCH v2] gdbus: Remove broad match rule and add proxy filter support

[Xiuzhuo Shang <xiuzhuo.shang@oss.qualcomm.com>]

On 7/21/2026 2:42 AM, Luiz Augusto von Dentz wrote:
> Hi Xiuzhuo,
> 
> On Wed, Jul 15, 2026 at 4:59 AM Xiuzhuo Shang
> <xiuzhuo.shang@oss.qualcomm.com> wrote:
>>
>> Problem
>> -------
>> On embedded platforms running continuous BLE scanning, bluetoothd
>> eventually stalls with its D-Bus socket to dbus-daemon full. strace
>> on a hung bluetoothd shows a repeating pattern:
>>
>>   sendmsg(7, {org.bluez.Device1 PropertiesChanged}, MSG_NOSIGNAL)
>>       = -1 EAGAIN (Resource temporarily unavailable)
>>   ppoll([{fd=7, events=POLLOUT}], 1, {tv_sec=0, tv_nsec=0}) = 0 (Timeout)
>>
>> fd=7 never becomes writable; bluetoothd's GMainLoop remains stuck
>> waiting for POLLOUT and cannot dispatch any further D-Bus events,
>> making the daemon appear hung and unresponsive to commands.
>>
>> The backpressure chain that causes this:
>>   1. BLE scanning generates high-rate PropertiesChanged(RSSI) signals
>>      (~400/s with typical BLE traffic).
>>   2. ofono's broad path_namespace='/' match rule causes dbus-daemon to
>>      route all these signals to ofono even though ofono has no use for
>>      BLE RSSI data.
>>   3. ofono's single-threaded GLib loop cannot consume them fast enough;
>>      undelivered messages accumulate inside dbus-daemon (457 MB
>>      observed after ~3 hours of scanning).
>>   4. dbus-daemon, busy draining its write queue toward ofono, stops
>>      reading from bluetoothd's socket in time; bluetoothd's kernel
>>      send buffer fills up and sendmsg() returns EAGAIN.
>>   5. With POLLOUT registered on fd=7, bluetoothd's GMainLoop stalls
>>      and can no longer send D-Bus replies or signals.
>>
>> Fix
>> ---
>> Three related changes:
>>
>> 1. Remove the broad type='signal',sender=<svc>,path_namespace=<path>
>>    match rule from g_dbus_client_new_full(). This rule was the sole
>>    feeder for the signal_func path in message_filter(). ofono never
>>    calls g_dbus_client_set_signal_watch() so signal_func is always
>>    NULL; the broad rule therefore served no purpose and caused
>>    dbus-daemon to route every bluetoothd signal to ofono.
>>
>> 2. Remove the now-empty match_rules GPtrArray infrastructure
>>    (field declaration, init, AddMatch loop, RemoveMatch loop, free).
>>    No match rules are added to this array any more.
>>
>> 3. Add a generic GDBusProxyFilterFunction callback and
>>    g_dbus_client_set_proxy_filter() API to GDBusClient. The filter
>>    is called from parse_properties() before proxy_new(), so a FALSE
>>    return prevents both proxy creation and per-device
>>    PropertiesChanged watch registration. This keeps all BlueZ-
>>    specific logic out of the gdbus layer.
>>
>>    Use this in hfp_hf_bluez5.c to skip Device1 proxies for BLE
>>    random-address devices: ofono only needs BR/EDR (AddressType=
>>    'public') devices for HFP/HSP. Skipping BLE proxies prevents
>>    dbus-daemon from registering per-device PropertiesChanged match
>>    rules for advertising peripherals and eliminates the remaining
>>    RSSI signal delivery to ofono.
> 
>> Together these changes prevent dbus-daemon from routing BLE
>> advertising signals to ofono, breaking the backpressure chain:
>> dbus-daemon memory stops growing, its write queue drains, and
>> bluetoothd's send buffer clears so that sendmsg() no longer returns
>> EAGAIN and the GMainLoop stall is resolved.
>>
>> Signed-off-by: Xiuzhuo Shang <xiuzhuo.shang@oss.qualcomm.com>
>> ---
>> Changes in v2:
>>  - Drop Change 1 (BLE address-type filter in parse_properties()) per
>>    review feedback; BlueZ-specific logic does not belong in the gdbus
>>    layer.
>>  - Add generic GDBusProxyFilterFunction callback and
>>    g_dbus_client_set_proxy_filter() API to GDBusClient. The filter is
>>    invoked before proxy_new() so a FALSE return prevents both proxy
>>    creation and per-device PropertiesChanged watch registration.
>>  - Use the new filter in hfp_hf_bluez5.c to skip Device1 proxies for
>>    BLE random-address devices, keeping all BlueZ-specific logic in the
>>    plugin as suggested.
>>  - Remove now-empty match_rules GPtrArray infrastructure (field,
>>    init, AddMatch loop, RemoveMatch loop, free) and unused variables.
>>  - Link to v1:
>>    https://lore.kernel.org/ofono/20260710075548.1072741-1-xiuzhuo.shang@oss.qualcomm.com/
>>
>>  gdbus/client.c          | 45 +++++++++++++++++++++--------------------
>>  gdbus/gdbus.h           |  8 ++++++++
>>  plugins/hfp_hf_bluez5.c | 38 ++++++++++++++++++++++++++++++++++
>>  3 files changed, 69 insertions(+), 22 deletions(-)
>>
>> diff --git a/gdbus/client.c b/gdbus/client.c
>> index 48711ae8..fa2e75c0 100644
>> --- a/gdbus/client.c
>> +++ b/gdbus/client.c
>> @@ -46,7 +46,6 @@ struct GDBusClient {
>>         guint watch;
>>         guint added_watch;
>>         guint removed_watch;
>> -       GPtrArray *match_rules;
>>         DBusPendingCall *pending_call;
>>         DBusPendingCall *get_objects_call;
>>         GDBusWatchFunction connect_func;
>> @@ -61,6 +60,8 @@ struct GDBusClient {
>>         GDBusClientFunction ready;
>>         void *ready_data;
>>         GDBusPropertyFunction property_changed;
>> +       GDBusProxyFilterFunction proxy_filter;
>> +       void *filter_user_data;
>>         void *user_data;
>>         GList *proxy_list;
>>  };
>> @@ -943,6 +944,14 @@ static void parse_properties(GDBusClient *client, const char *path,
>>                 return;
>>         }
>>
>> +       if (client->proxy_filter) {
>> +               DBusMessageIter copy = *iter;
>> +
>> +               if (!client->proxy_filter(client, path, interface,
>> +                                       &copy, client->filter_user_data))
>> +                       return;
>> +       }
>> +
>>         proxy = proxy_new(client, path, interface);
>>         if (proxy == NULL)
>>                 return;
>> @@ -1211,7 +1220,6 @@ GDBusClient *g_dbus_client_new_full(DBusConnection *connection,
>>                                                         const char *root_path)
>>  {
>>         GDBusClient *client;
>> -       unsigned int i;
>>
>>         if (!connection || !service)
>>                 return NULL;
>> @@ -1232,9 +1240,6 @@ GDBusClient *g_dbus_client_new_full(DBusConnection *connection,
>>         client->root_path = g_strdup(root_path);
>>         client->connected = FALSE;
>>
>> -       client->match_rules = g_ptr_array_sized_new(1);
>> -       g_ptr_array_set_free_func(client->match_rules, g_free);
>> -
>>         client->watch = g_dbus_add_service_watch(connection, service,
>>                                                 service_connect,
>>                                                 service_disconnect,
>> @@ -1255,14 +1260,6 @@ GDBusClient *g_dbus_client_new_full(DBusConnection *connection,
>>                                                 "InterfacesRemoved",
>>                                                 interfaces_removed,
>>                                                 client, NULL);
>> -       g_ptr_array_add(client->match_rules, g_strdup_printf("type='signal',"
>> -                               "sender='%s',path_namespace='%s'",
>> -                               client->service_name, client->base_path));
>> -
>> -       for (i = 0; i < client->match_rules->len; i++) {
>> -               modify_match(client->dbus_conn, "AddMatch",
>> -                               g_ptr_array_index(client->match_rules, i));
>> -       }
>>
>>         return g_dbus_client_ref(client);
>>  }
>> @@ -1279,8 +1276,6 @@ GDBusClient *g_dbus_client_ref(GDBusClient *client)
>>
>>  void g_dbus_client_unref(GDBusClient *client)
>>  {
>> -       unsigned int i;
>> -
>>         if (client == NULL)
>>                 return;
>>
>> @@ -1297,13 +1292,6 @@ void g_dbus_client_unref(GDBusClient *client)
>>                 dbus_pending_call_unref(client->get_objects_call);
>>         }
>>
>> -       for (i = 0; i < client->match_rules->len; i++) {
>> -               modify_match(client->dbus_conn, "RemoveMatch",
>> -                               g_ptr_array_index(client->match_rules, i));
>> -       }
>> -
>> -       g_ptr_array_free(client->match_rules, TRUE);
>> -
>>         dbus_connection_remove_filter(client->dbus_conn,
>>                                                 message_filter, client);
>>
>> @@ -1396,3 +1384,16 @@ gboolean g_dbus_client_set_proxy_handlers(GDBusClient *client,
>>
>>         return TRUE;
>>  }
>> +
>> +gboolean g_dbus_client_set_proxy_filter(GDBusClient *client,
>> +                                       GDBusProxyFilterFunction proxy_filter,
>> +                                       void *user_data)
>> +{
>> +       if (client == NULL)
>> +               return FALSE;
>> +
>> +       client->proxy_filter = proxy_filter;
>> +       client->filter_user_data = user_data;
>> +
>> +       return TRUE;
>> +}
>> diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
>> index d99c2549..cc3c4e16 100644
>> --- a/gdbus/gdbus.h
>> +++ b/gdbus/gdbus.h
>> @@ -347,6 +347,11 @@ typedef void (* GDBusClientFunction) (GDBusClient *client, void *user_data);
>>  typedef void (* GDBusProxyFunction) (GDBusProxy *proxy, void *user_data);
>>  typedef void (* GDBusPropertyFunction) (GDBusProxy *proxy, const char *name,
>>                                         DBusMessageIter *iter, void *user_data);
>> +typedef gboolean (* GDBusProxyFilterFunction) (GDBusClient *client,
>> +                                       const char *path,
>> +                                       const char *interface,
>> +                                       DBusMessageIter *iter,
>> +                                       void *user_data);
>>
>>  gboolean g_dbus_proxy_set_property_watch(GDBusProxy *proxy,
>>                         GDBusPropertyFunction function, void *user_data);
>> @@ -377,6 +382,9 @@ gboolean g_dbus_client_set_proxy_handlers(GDBusClient *client,
>>                                         GDBusProxyFunction proxy_removed,
>>                                         GDBusPropertyFunction property_changed,
>>                                         void *user_data);
>> +gboolean g_dbus_client_set_proxy_filter(GDBusClient *client,
>> +                                       GDBusProxyFilterFunction proxy_filter,
>> +                                       void *user_data);
>>
>>  #ifdef __cplusplus
>>  }
>> diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
>> index 5ad1674f..141dc5c4 100644
>> --- a/plugins/hfp_hf_bluez5.c
>> +++ b/plugins/hfp_hf_bluez5.c
>> @@ -791,6 +791,43 @@ static void proxy_added(GDBusProxy *proxy, void *user_data)
>>         device_changed(proxy, path);
>>  }
>>
>> +static gboolean proxy_filter(GDBusClient *client, const char *path,
>> +                               const char *interface, DBusMessageIter *iter,
>> +                               void *user_data)
>> +{
>> +       DBusMessageIter props, entry;
>> +
>> +       if (g_str_equal(BLUEZ_DEVICE_INTERFACE, interface) == FALSE)
>> +               return TRUE;
>> +
>> +       if (dbus_message_iter_get_arg_type(iter) != DBUS_TYPE_ARRAY)
>> +               return TRUE;
>> +
>> +       dbus_message_iter_recurse(iter, &props);
>> +
>> +       while (dbus_message_iter_get_arg_type(&props) == DBUS_TYPE_DICT_ENTRY) {
>> +               const char *key;
>> +
>> +               dbus_message_iter_recurse(&props, &entry);
>> +               dbus_message_iter_get_basic(&entry, &key);
>> +
>> +               if (g_str_equal(key, "AddressType") == TRUE) {
>> +                       DBusMessageIter var;
>> +                       const char *addr_type;
>> +
>> +                       dbus_message_iter_next(&entry);
>> +                       dbus_message_iter_recurse(&entry, &var);
>> +                       dbus_message_iter_get_basic(&var, &addr_type);
>> +
>> +                       return !g_str_equal(addr_type, "random");
>> +               }
>> +
>> +               dbus_message_iter_next(&props);
>> +       }
>> +
>> +       return TRUE;
>> +}
>> +
>>  static void property_changed(GDBusProxy *proxy, const char *name,
>>                                         DBusMessageIter *iter, void *user_data)
>>  {
>> @@ -844,6 +881,7 @@ static int hfp_init(void)
>>         g_dbus_client_set_connect_watch(bluez, connect_handler, NULL);
>>         g_dbus_client_set_proxy_handlers(bluez, proxy_added, NULL,
>>                                                 property_changed, NULL);
>> +       g_dbus_client_set_proxy_filter(bluez, proxy_filter, NULL);
> 
> I don't really follow; would this register a proxy filter and
> automatically remove it on the first match of an AddressType=random??
> Sounds not really useful to me, what is the difference if we don't use
> set_proxy_filter above?

Thank you for the review. Let me clarify how proxy_filter works.

The filter is a persistent callback registered on the GDBusClient
instance. It is called from parse_properties() for every device
object as it appears (via InterfacesAdded or GetManagedObjects).
Returning FALSE means "do not create a proxy for this specific
device" -- it does NOT remove or deregister the filter itself.

So the behaviour is:

  Device A (AddressType=random) -> proxy_filter called -> FALSE
                                -> proxy_new() skipped
  Device B (AddressType=random) -> proxy_filter called again -> FALSE
                                -> proxy_new() skipped
  Device C (AddressType=public) -> proxy_filter called -> TRUE
                                -> proxy created as normal

The filter stays registered for the lifetime of the GDBusClient and
is invoked once per device per interface, not just once globally.

Without set_proxy_filter:

  Every Device1 object, including BLE random-address devices, goes
  through proxy_new(). Each proxy registers a per-device
  PropertiesChanged watch via g_dbus_add_properties_watch(). In a
  dense BLE environment with 200-300 advertising peripherals, this
  results in 200-300 per-device match rules registered with
  dbus-daemon, which routes every RSSI PropertiesChanged signal for
  each of those devices to ofono. None of these signals are useful
  to ofono (it only needs BR/EDR devices for HFP/HSP), but the
  routing overhead causes dbus-daemon memory growth and eventually
  backpressures the bluetoothd socket.

With set_proxy_filter:

  BLE random-address devices are rejected before proxy_new() is
  called, so no per-device PropertiesChanged watch is registered for
  them. Only BR/EDR devices (AddressType='public') get proxies and
  watches, which is all ofono needs.

To summarise, the proxy_filter is not a one-shot mechanism; it
persistently gates every proxy creation for the lifetime of the
client. Without it, per-device watches accumulate for all BLE
peripherals in the vicinity, not just the first one encountered.

> 
>>
>>         ofono_handsfree_audio_ref();
>>
>> --
>> 2.43.0
>>
> 
>