2024-04-05 21:12:02

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH BlueZ v6 1/3] input/device: Fix not handling IdleTimeout when uhid is in use

From: Luiz Augusto von Dentz <[email protected]>

When uhid is in use IdleTimeout was not taking any effect, this also
attempt to force the destroy the input device node to make it useful
for users that don't want to keep the input node forever.
---
profiles/input/device.c | 86 +++++++++++++++++++++++++++------------
profiles/input/input.conf | 2 +-
2 files changed, 61 insertions(+), 27 deletions(-)

diff --git a/profiles/input/device.c b/profiles/input/device.c
index 1b28cdc174b1..b622ee8cd681 100644
--- a/profiles/input/device.c
+++ b/profiles/input/device.c
@@ -77,6 +77,7 @@ struct input_device {
unsigned int report_req_timer;
uint32_t report_rsp_id;
bool virtual_cable_unplug;
+ unsigned int idle_timer;
};

static int idle_timeout = 0;
@@ -139,6 +140,9 @@ static void input_device_free(struct input_device *idev)
g_free(idev->req);
}

+ if (idev->idle_timer)
+ timeout_remove(idev->idle_timer);
+
if (idev->reconnect_timer > 0)
timeout_remove(idev->reconnect_timer);

@@ -156,8 +160,54 @@ static void virtual_cable_unplug(struct input_device *idev)
idev->virtual_cable_unplug = false;
}

-static bool hidp_send_message(GIOChannel *chan, uint8_t hdr,
- const uint8_t *data, size_t size)
+static int uhid_disconnect(struct input_device *idev, bool force)
+{
+ int err;
+
+ if (!bt_uhid_created(idev->uhid))
+ return 0;
+
+ /* Only destroy the node if virtual cable unplug flag has been set */
+ if (!idev->virtual_cable_unplug && !force)
+ return 0;
+
+ bt_uhid_unregister_all(idev->uhid);
+
+ err = bt_uhid_destroy(idev->uhid);
+ if (err < 0) {
+ error("bt_uhid_destroy: %s", strerror(-err));
+ return err;
+ }
+
+ return err;
+}
+
+static bool input_device_idle_timeout(gpointer user_data)
+{
+ struct input_device *idev = user_data;
+
+ idev->idle_timer = 0;
+
+ DBG("path=%s", idev->path);
+
+ uhid_disconnect(idev, true);
+ connection_disconnect(idev, 0);
+
+ return false;
+}
+
+static void input_device_idle_reset(struct input_device *idev)
+{
+ timeout_remove(idev->idle_timer);
+
+ if (idle_timeout)
+ idev->idle_timer = timeout_add_seconds(idle_timeout,
+ input_device_idle_timeout, idev,
+ NULL);
+}
+
+static bool hidp_send_message(struct input_device *idev, GIOChannel *chan,
+ uint8_t hdr, const uint8_t *data, size_t size)
{
int fd;
ssize_t len;
@@ -191,6 +241,8 @@ static bool hidp_send_message(GIOChannel *chan, uint8_t hdr,
return false;
}

+ input_device_idle_reset(idev);
+
return true;
}

@@ -200,13 +252,13 @@ static bool hidp_send_ctrl_message(struct input_device *idev, uint8_t hdr,
if (hdr == (HIDP_TRANS_HID_CONTROL | HIDP_CTRL_VIRTUAL_CABLE_UNPLUG))
idev->virtual_cable_unplug = true;

- return hidp_send_message(idev->ctrl_io, hdr, data, size);
+ return hidp_send_message(idev, idev->ctrl_io, hdr, data, size);
}

static bool hidp_send_intr_message(struct input_device *idev, uint8_t hdr,
const uint8_t *data, size_t size)
{
- return hidp_send_message(idev->intr_io, hdr, data, size);
+ return hidp_send_message(idev, idev->intr_io, hdr, data, size);
}

static bool uhid_send_get_report_reply(struct input_device *idev,
@@ -297,6 +349,8 @@ static bool hidp_recv_intr_data(GIOChannel *chan, struct input_device *idev)
return true;
}

+ input_device_idle_reset(idev);
+
hdr = data[0];
if (hdr != (HIDP_TRANS_DATA | HIDP_DATA_RTYPE_INPUT)) {
DBG("unsupported HIDP protocol header 0x%02x", hdr);
@@ -313,28 +367,6 @@ static bool hidp_recv_intr_data(GIOChannel *chan, struct input_device *idev)
return true;
}

-static int uhid_disconnect(struct input_device *idev, bool force)
-{
- int err;
-
- if (!bt_uhid_created(idev->uhid))
- return 0;
-
- /* Only destroy the node if virtual cable unplug flag has been set */
- if (!idev->virtual_cable_unplug && !force)
- return 0;
-
- bt_uhid_unregister_all(idev->uhid);
-
- err = bt_uhid_destroy(idev->uhid);
- if (err < 0) {
- error("bt_uhid_destroy: %s", strerror(-err));
- return err;
- }
-
- return err;
-}
-
static gboolean intr_watch_cb(GIOChannel *chan, GIOCondition cond, gpointer data)
{
struct input_device *idev = data;
@@ -520,6 +552,8 @@ static bool hidp_recv_ctrl_message(GIOChannel *chan, struct input_device *idev)
return true;
}

+ input_device_idle_reset(idev);
+
hdr = data[0];
type = hdr & HIDP_HEADER_TRANS_MASK;
param = hdr & HIDP_HEADER_PARAM_MASK;
diff --git a/profiles/input/input.conf b/profiles/input/input.conf
index 00a34eb63de1..fc20c58b6b32 100644
--- a/profiles/input/input.conf
+++ b/profiles/input/input.conf
@@ -6,7 +6,7 @@

# Set idle timeout (in minutes) before the connection will
# be disconnect (defaults to 0 for no timeout)
-#IdleTimeout=30
+#IdleTimeout=0

# Enable HID protocol handling in userspace input profile
# Defaults to true (Use UHID instead of kernel HIDP)
--
2.44.0



2024-04-05 21:12:14

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH BlueZ v6 3/3] input/device: Add replay support

From: Luiz Augusto von Dentz <[email protected]>

This adds replay support when uhid is in use and the input node is keep
while disconnected:

Fixes: https://github.com/bluez/bluez/issues/777
---
profiles/input/device.c | 174 +++++++++++++++++++++++++++++++++++++++-
1 file changed, 173 insertions(+), 1 deletion(-)

diff --git a/profiles/input/device.c b/profiles/input/device.c
index b622ee8cd681..21da16155b0c 100644
--- a/profiles/input/device.c
+++ b/profiles/input/device.c
@@ -42,6 +42,8 @@
#include "src/sdp-client.h"
#include "src/shared/timeout.h"
#include "src/shared/uhid.h"
+#include "src/shared/util.h"
+#include "src/shared/queue.h"

#include "device.h"
#include "hidp_defs.h"
@@ -55,6 +57,19 @@ enum reconnect_mode_t {
RECONNECT_ANY
};

+struct hidp_msg {
+ uint8_t hdr;
+ struct iovec *iov;
+};
+
+struct hidp_replay {
+ bool replaying;
+ struct queue *out;
+ struct queue *in;
+ struct queue *re_out;
+ struct queue *re_in;
+};
+
struct input_device {
struct btd_service *service;
struct btd_device *device;
@@ -78,6 +93,7 @@ struct input_device {
uint32_t report_rsp_id;
bool virtual_cable_unplug;
unsigned int idle_timer;
+ struct hidp_replay *replay;
};

static int idle_timeout = 0;
@@ -113,8 +129,30 @@ static bool input_device_bonded(struct input_device *idev)
btd_device_get_bdaddr_type(idev->device));
}

+static void hidp_msg_free(void *data)
+{
+ struct hidp_msg *msg = data;
+
+ util_iov_free(msg->iov, 1);
+ free(msg);
+}
+
+static void hidp_replay_free(struct hidp_replay *replay)
+{
+ if (!replay)
+ return;
+
+ queue_destroy(replay->re_in, NULL);
+ queue_destroy(replay->in, hidp_msg_free);
+ queue_destroy(replay->re_out, NULL);
+ queue_destroy(replay->out, hidp_msg_free);
+ free(replay);
+}
+
static void input_device_free(struct input_device *idev)
{
+ hidp_replay_free(idev->replay);
+
bt_uhid_unref(idev->uhid);
btd_service_unref(idev->service);
btd_device_unref(idev->device);
@@ -171,6 +209,10 @@ static int uhid_disconnect(struct input_device *idev, bool force)
if (!idev->virtual_cable_unplug && !force)
return 0;

+ /* Destroy replay messages */
+ hidp_replay_free(idev->replay);
+ idev->replay = NULL;
+
bt_uhid_unregister_all(idev->uhid);

err = bt_uhid_destroy(idev->uhid);
@@ -246,12 +288,96 @@ static bool hidp_send_message(struct input_device *idev, GIOChannel *chan,
return true;
}

+static void hidp_replay_resend(struct input_device *idev)
+{
+ struct hidp_msg *msg;
+
+ if (!idev->replay || !idev->replay->replaying)
+ return;
+
+ msg = queue_pop_head(idev->replay->re_out);
+ if (!msg) {
+ DBG("uhid replay finished");
+ idev->replay->replaying = false;
+ return;
+ }
+
+ if (hidp_send_message(idev, NULL, msg->hdr, msg->iov->iov_base,
+ msg->iov->iov_len))
+ DBG("hdr 0x%02x size %zu", msg->hdr, msg->iov->iov_len);
+ else
+ error("uhid replay resend failed");
+}
+
+static void hidp_replay_recv(struct input_device *idev, uint8_t hdr,
+ const uint8_t *data, size_t size)
+{
+ struct hidp_msg *msg;
+
+ if (!idev->replay || !idev->replay->replaying)
+ return;
+
+ msg = queue_pop_head(idev->replay->re_in);
+
+ if (msg && (msg->hdr != hdr || msg->iov->iov_len != size ||
+ memcmp(msg->iov->iov_base, data, size)))
+ error("uhid replay input error... discarding");
+
+ hidp_replay_resend(idev);
+}
+
+static struct hidp_replay *hidp_replay_new(void)
+{
+ struct hidp_replay *replay = new0(struct hidp_replay, 1);
+
+ replay->out = queue_new();
+ replay->in = queue_new();
+
+ return replay;
+}
+
+static void hidp_record_message(struct input_device *idev, bool out,
+ uint8_t hdr, const uint8_t *data, size_t size)
+{
+ struct hidp_msg *msg;
+ struct iovec iov = { (void *)data, size };
+
+ /* Only record messages if uhid has been created */
+ if (!bt_uhid_created(idev->uhid))
+ return;
+
+ if (idev->replay && idev->replay->replaying) {
+ if (!out)
+ hidp_replay_recv(idev, hdr, data, size);
+ return;
+ }
+
+ if (!idev->replay)
+ idev->replay = hidp_replay_new();
+
+ msg = new0(struct hidp_msg, 1);
+ msg->hdr = hdr;
+ msg->iov = util_iov_dup(&iov, 1);
+
+ if (out) {
+ DBG("output[%u]: hdr 0x%02x size %zu",
+ queue_length(idev->replay->out), hdr, size);
+ queue_push_tail(idev->replay->out, msg);
+ } else {
+ DBG("input[%u]: hdr 0x%02x size %zu",
+ queue_length(idev->replay->in), hdr, size);
+ queue_push_tail(idev->replay->in, msg);
+ }
+}
+
static bool hidp_send_ctrl_message(struct input_device *idev, uint8_t hdr,
const uint8_t *data, size_t size)
{
if (hdr == (HIDP_TRANS_HID_CONTROL | HIDP_CTRL_VIRTUAL_CABLE_UNPLUG))
idev->virtual_cable_unplug = true;

+ hidp_record_message(idev, true, hdr, data, size);
+
return hidp_send_message(idev, idev->ctrl_io, hdr, data, size);
}

@@ -558,6 +684,12 @@ static bool hidp_recv_ctrl_message(GIOChannel *chan, struct input_device *idev)
type = hdr & HIDP_HEADER_TRANS_MASK;
param = hdr & HIDP_HEADER_PARAM_MASK;

+ /* While replaying don't involve the driver since it will likely get
+ * confused with messages it already things it has received.
+ */
+ if (idev->replay && idev->replay->replaying)
+ goto done;
+
switch (type) {
case HIDP_TRANS_HANDSHAKE:
hidp_recv_ctrl_handshake(idev, param);
@@ -575,6 +707,9 @@ static bool hidp_recv_ctrl_message(GIOChannel *chan, struct input_device *idev)
break;
}

+done:
+ hidp_record_message(idev, false, hdr, data + 1, len - 1);
+
return true;
}

@@ -973,12 +1108,49 @@ static int ioctl_disconnect(struct input_device *idev, uint32_t flags)
return err;
}

+static void queue_append(void *data, void *user_data)
+{
+ queue_push_tail(user_data, data);
+}
+
+static struct queue *queue_dup(struct queue *q)
+{
+ struct queue *dup;
+
+ if (!q || queue_isempty(q))
+ return NULL;
+
+ dup = queue_new();
+
+ queue_foreach(q, queue_append, dup);
+
+ return dup;
+}
+
+static void hidp_replay_init(struct input_device *idev)
+{
+ if (!idev->replay || idev->replay->replaying)
+ return;
+
+ idev->replay->replaying = true;
+
+ queue_destroy(idev->replay->re_in, NULL);
+ idev->replay->re_in = queue_dup(idev->replay->in);
+
+ queue_destroy(idev->replay->re_out, NULL);
+ idev->replay->re_out = queue_dup(idev->replay->out);
+
+ hidp_replay_resend(idev);
+}
+
static int uhid_connadd(struct input_device *idev, struct hidp_connadd_req *req)
{
int err;

- if (bt_uhid_created(idev->uhid))
+ if (bt_uhid_created(idev->uhid)) {
+ hidp_replay_init(idev);
return 0;
+ }

err = bt_uhid_create(idev->uhid, req->name, &idev->src, &idev->dst,
req->vendor, req->product, req->version,
--
2.44.0


2024-04-05 23:52:33

by bluez.test.bot

[permalink] [raw]
Subject: RE: [BlueZ,v6,1/3] input/device: Fix not handling IdleTimeout when uhid is in use

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=841948

---Test result---

Test Summary:
CheckPatch PASS 0.95 seconds
GitLint PASS 0.58 seconds
BuildEll PASS 24.19 seconds
BluezMake PASS 1637.10 seconds
MakeCheck PASS 13.08 seconds
MakeDistcheck PASS 174.11 seconds
CheckValgrind PASS 244.85 seconds
CheckSmatch PASS 349.91 seconds
bluezmakeextell PASS 119.93 seconds
IncrementalBuild PASS 4565.34 seconds
ScanBuild PASS 967.50 seconds



---
Regards,
Linux Bluetooth

2024-04-08 19:30:36

by patchwork-bot+bluetooth

[permalink] [raw]
Subject: Re: [PATCH BlueZ v6 1/3] input/device: Fix not handling IdleTimeout when uhid is in use

Hello:

This series was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <[email protected]>:

On Fri, 5 Apr 2024 17:11:43 -0400 you wrote:
> From: Luiz Augusto von Dentz <[email protected]>
>
> When uhid is in use IdleTimeout was not taking any effect, this also
> attempt to force the destroy the input device node to make it useful
> for users that don't want to keep the input node forever.
> ---
> profiles/input/device.c | 86 +++++++++++++++++++++++++++------------
> profiles/input/input.conf | 2 +-
> 2 files changed, 61 insertions(+), 27 deletions(-)

Here is the summary with links:
- [BlueZ,v6,1/3] input/device: Fix not handling IdleTimeout when uhid is in use
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=ea96d7d18bff
- [BlueZ,v6,2/3] input.conf: Change IdleTimeout unit to be in seconds
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=94c4f445af66
- [BlueZ,v6,3/3] input/device: Add replay support
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=c6dea886985e

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html