| Message ID | 20210407163723.Bluez.v2.1.I4ff127dde9bc6adb2a07507af2bf2cc6b6bcf0f2@changeid (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [Bluez,v2] input/hog: support multiple variable length notification | expand |
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=462197 ---Test result--- ############################## Test: CheckPatch - PASS ############################## Test: CheckGitLint - PASS ############################## Test: CheckBuild: Setup ELL - PASS ############################## Test: CheckBuild: Setup - PASS ############################## Test: CheckBuild - PASS ############################## Test: MakeCheck - PASS ############################## Test: CheckBuild w/external ell - PASS --- Regards, Linux Bluetooth
Hi Archie, On Wed, Apr 7, 2021 at 1:38 AM Archie Pusaka <apusaka@google.com> wrote: > > From: Archie Pusaka <apusaka@chromium.org> > > Processing Multiple Variable Length Notification is mandatory if EATT > is enabled (Core Spec Vol 3 Part G Sec 4.2). > > Reviewed-by: Sonny Sasaka <sonnysasaka@chromium.org> > --- > > Changes in v2: > * fix compilation error > > attrib/att.h | 1 + > profiles/input/hog-lib.c | 109 ++++++++++++++++++++++++++++----------- > src/attrib-server.c | 1 + > 3 files changed, 81 insertions(+), 30 deletions(-) > > diff --git a/attrib/att.h b/attrib/att.h > index 13a0c3a31f..0dbfb14b83 100644 > --- a/attrib/att.h > +++ b/attrib/att.h > @@ -42,6 +42,7 @@ > #define ATT_OP_HANDLE_NOTIFY 0x1B > #define ATT_OP_HANDLE_IND 0x1D > #define ATT_OP_HANDLE_CNF 0x1E > +#define ATT_OP_HANDLE_NOTIFY_MULTI 0x23 > #define ATT_OP_SIGNED_WRITE_CMD 0xD2 I rather not keep adding support for the attrib directory since we want to get rid of it at some point and actually the main reason it was not removed yet is because hog was not converted to use bt_gatt_client so I'd say this would be a good time for bite the bullet and make the conversion so we can proceed to remove it. > /* Error codes for Error response PDU */ > diff --git a/profiles/input/hog-lib.c b/profiles/input/hog-lib.c > index e5e3d3e7f5..aa6c64a3f2 100644 > --- a/profiles/input/hog-lib.c > +++ b/profiles/input/hog-lib.c > @@ -65,7 +65,6 @@ > > #define HOG_REPORT_MAP_MAX_SIZE 512 > #define HID_INFO_SIZE 4 > -#define ATT_NOTIFICATION_HEADER_SIZE 3 > > struct bt_hog { > int ref_count; > @@ -112,7 +111,8 @@ struct report { > uint16_t value_handle; > uint8_t properties; > uint16_t ccc_handle; > - guint notifyid; > + guint notify_id; > + guint notify_multi_id; > uint16_t len; > uint8_t *value; > }; > @@ -283,22 +283,14 @@ static void find_included(struct bt_hog *hog, GAttrib *attrib, > free(req); > } > > -static void report_value_cb(const guint8 *pdu, guint16 len, gpointer user_data) > +static void process_notification(struct report *report, guint16 len, > + const uint8_t *data) > { > - struct report *report = user_data; > struct bt_hog *hog = report->hog; > struct uhid_event ev; > uint8_t *buf; > int err; > > - if (len < ATT_NOTIFICATION_HEADER_SIZE) { > - error("Malformed ATT notification"); > - return; > - } > - > - pdu += ATT_NOTIFICATION_HEADER_SIZE; > - len -= ATT_NOTIFICATION_HEADER_SIZE; > - > memset(&ev, 0, sizeof(ev)); > ev.type = UHID_INPUT; > buf = ev.u.input.data; > @@ -306,19 +298,78 @@ static void report_value_cb(const guint8 *pdu, guint16 len, gpointer user_data) > if (hog->has_report_id) { > buf[0] = report->id; > len = MIN(len, sizeof(ev.u.input.data) - 1); > - memcpy(buf + 1, pdu, len); > - ev.u.input.size = ++len; > + memcpy(buf + 1, data, len); > + ev.u.input.size = len + 1; > } else { > len = MIN(len, sizeof(ev.u.input.data)); > - memcpy(buf, pdu, len); > + memcpy(buf, data, len); > ev.u.input.size = len; > } > > err = bt_uhid_send(hog->uhid, &ev); > - if (err < 0) { > + if (err < 0) > error("bt_uhid_send: %s (%d)", strerror(-err), -err); > - return; > +} > + > +static void report_value_cb(const guint8 *pdu, guint16 len, gpointer user_data) > +{ > + struct report *report = user_data; > + uint8_t opcode = pdu[0]; > + guint16 report_len; > + guint16 header_len; > + > + /* Skip opcode field */ > + pdu += 1; > + len -= 1; > + > + if (opcode == ATT_OP_HANDLE_NOTIFY_MULTI) > + header_len = 4; > + else > + header_len = 2; > + > + if (len < header_len) > + goto fail; > + > + while (len >= header_len) { > + /* Skip first 2 bytes (handle) */ > + pdu += 2; > + len -= 2; > + > + if (opcode == ATT_OP_HANDLE_NOTIFY_MULTI) { > + report_len = get_le16(pdu); > + pdu += 2; > + len -= 2; > + > + if (report_len > len) > + goto fail; > + } else { > + report_len = len; > + } > + > + process_notification(report, report_len, pdu); > + > + pdu += report_len; > + len -= report_len; > } > + > + if (len == 0) > + return; > + > +fail: > + error("Malformed ATT notification"); > +} > + > +static void register_notify_handler(struct bt_hog *hog, struct report *report) > +{ > + report->notify_id = g_attrib_register(hog->attrib, > + ATT_OP_HANDLE_NOTIFY, > + report->value_handle, > + report_value_cb, report, NULL); > + > + report->notify_multi_id = g_attrib_register(hog->attrib, > + ATT_OP_HANDLE_NOTIFY_MULTI, > + report->value_handle, > + report_value_cb, report, NULL); > } > > static void report_ccc_written_cb(guint8 status, const guint8 *pdu, > @@ -336,13 +387,10 @@ static void report_ccc_written_cb(guint8 status, const guint8 *pdu, > return; > } > > - if (report->notifyid) > + if (report->notify_id) > return; > > - report->notifyid = g_attrib_register(hog->attrib, > - ATT_OP_HANDLE_NOTIFY, > - report->value_handle, > - report_value_cb, report, NULL); > + register_notify_handler(hog, report); > > DBG("Report characteristic descriptor written: notifications enabled"); > } > @@ -1711,13 +1759,10 @@ bool bt_hog_attach(struct bt_hog *hog, void *gatt) > for (l = hog->reports; l; l = l->next) { > struct report *r = l->data; > > - if (r->notifyid) > + if (r->notify_id) > continue; > > - r->notifyid = g_attrib_register(hog->attrib, > - ATT_OP_HANDLE_NOTIFY, > - r->value_handle, > - report_value_cb, r, NULL); > + register_notify_handler(hog, r); > } > > return true; > @@ -1764,9 +1809,13 @@ void bt_hog_detach(struct bt_hog *hog) > for (l = hog->reports; l; l = l->next) { > struct report *r = l->data; > > - if (r->notifyid > 0) { > - g_attrib_unregister(hog->attrib, r->notifyid); > - r->notifyid = 0; > + if (r->notify_id > 0) { > + g_attrib_unregister(hog->attrib, r->notify_id); > + r->notify_id = 0; > + } > + if (r->notify_multi_id > 0) { > + g_attrib_unregister(hog->attrib, r->notify_multi_id); > + r->notify_multi_id = 0; > } > } > > diff --git a/src/attrib-server.c b/src/attrib-server.c > index 5a178f95ea..fb11d3db2d 100644 > --- a/src/attrib-server.c > +++ b/src/attrib-server.c > @@ -1085,6 +1085,7 @@ static void channel_handler(const uint8_t *ipdu, uint16_t len, > return; > case ATT_OP_HANDLE_IND: > case ATT_OP_HANDLE_NOTIFY: > + case ATT_OP_HANDLE_NOTIFY_MULTI: > /* The attribute client is already handling these */ > return; > case ATT_OP_READ_MULTI_REQ: > -- > 2.31.0.208.g409f899ff0-goog >
diff --git a/attrib/att.h b/attrib/att.h index 13a0c3a31f..0dbfb14b83 100644 --- a/attrib/att.h +++ b/attrib/att.h @@ -42,6 +42,7 @@ #define ATT_OP_HANDLE_NOTIFY 0x1B #define ATT_OP_HANDLE_IND 0x1D #define ATT_OP_HANDLE_CNF 0x1E +#define ATT_OP_HANDLE_NOTIFY_MULTI 0x23 #define ATT_OP_SIGNED_WRITE_CMD 0xD2 /* Error codes for Error response PDU */ diff --git a/profiles/input/hog-lib.c b/profiles/input/hog-lib.c index e5e3d3e7f5..aa6c64a3f2 100644 --- a/profiles/input/hog-lib.c +++ b/profiles/input/hog-lib.c @@ -65,7 +65,6 @@ #define HOG_REPORT_MAP_MAX_SIZE 512 #define HID_INFO_SIZE 4 -#define ATT_NOTIFICATION_HEADER_SIZE 3 struct bt_hog { int ref_count; @@ -112,7 +111,8 @@ struct report { uint16_t value_handle; uint8_t properties; uint16_t ccc_handle; - guint notifyid; + guint notify_id; + guint notify_multi_id; uint16_t len; uint8_t *value; }; @@ -283,22 +283,14 @@ static void find_included(struct bt_hog *hog, GAttrib *attrib, free(req); } -static void report_value_cb(const guint8 *pdu, guint16 len, gpointer user_data) +static void process_notification(struct report *report, guint16 len, + const uint8_t *data) { - struct report *report = user_data; struct bt_hog *hog = report->hog; struct uhid_event ev; uint8_t *buf; int err; - if (len < ATT_NOTIFICATION_HEADER_SIZE) { - error("Malformed ATT notification"); - return; - } - - pdu += ATT_NOTIFICATION_HEADER_SIZE; - len -= ATT_NOTIFICATION_HEADER_SIZE; - memset(&ev, 0, sizeof(ev)); ev.type = UHID_INPUT; buf = ev.u.input.data; @@ -306,19 +298,78 @@ static void report_value_cb(const guint8 *pdu, guint16 len, gpointer user_data) if (hog->has_report_id) { buf[0] = report->id; len = MIN(len, sizeof(ev.u.input.data) - 1); - memcpy(buf + 1, pdu, len); - ev.u.input.size = ++len; + memcpy(buf + 1, data, len); + ev.u.input.size = len + 1; } else { len = MIN(len, sizeof(ev.u.input.data)); - memcpy(buf, pdu, len); + memcpy(buf, data, len); ev.u.input.size = len; } err = bt_uhid_send(hog->uhid, &ev); - if (err < 0) { + if (err < 0) error("bt_uhid_send: %s (%d)", strerror(-err), -err); - return; +} + +static void report_value_cb(const guint8 *pdu, guint16 len, gpointer user_data) +{ + struct report *report = user_data; + uint8_t opcode = pdu[0]; + guint16 report_len; + guint16 header_len; + + /* Skip opcode field */ + pdu += 1; + len -= 1; + + if (opcode == ATT_OP_HANDLE_NOTIFY_MULTI) + header_len = 4; + else + header_len = 2; + + if (len < header_len) + goto fail; + + while (len >= header_len) { + /* Skip first 2 bytes (handle) */ + pdu += 2; + len -= 2; + + if (opcode == ATT_OP_HANDLE_NOTIFY_MULTI) { + report_len = get_le16(pdu); + pdu += 2; + len -= 2; + + if (report_len > len) + goto fail; + } else { + report_len = len; + } + + process_notification(report, report_len, pdu); + + pdu += report_len; + len -= report_len; } + + if (len == 0) + return; + +fail: + error("Malformed ATT notification"); +} + +static void register_notify_handler(struct bt_hog *hog, struct report *report) +{ + report->notify_id = g_attrib_register(hog->attrib, + ATT_OP_HANDLE_NOTIFY, + report->value_handle, + report_value_cb, report, NULL); + + report->notify_multi_id = g_attrib_register(hog->attrib, + ATT_OP_HANDLE_NOTIFY_MULTI, + report->value_handle, + report_value_cb, report, NULL); } static void report_ccc_written_cb(guint8 status, const guint8 *pdu, @@ -336,13 +387,10 @@ static void report_ccc_written_cb(guint8 status, const guint8 *pdu, return; } - if (report->notifyid) + if (report->notify_id) return; - report->notifyid = g_attrib_register(hog->attrib, - ATT_OP_HANDLE_NOTIFY, - report->value_handle, - report_value_cb, report, NULL); + register_notify_handler(hog, report); DBG("Report characteristic descriptor written: notifications enabled"); } @@ -1711,13 +1759,10 @@ bool bt_hog_attach(struct bt_hog *hog, void *gatt) for (l = hog->reports; l; l = l->next) { struct report *r = l->data; - if (r->notifyid) + if (r->notify_id) continue; - r->notifyid = g_attrib_register(hog->attrib, - ATT_OP_HANDLE_NOTIFY, - r->value_handle, - report_value_cb, r, NULL); + register_notify_handler(hog, r); } return true; @@ -1764,9 +1809,13 @@ void bt_hog_detach(struct bt_hog *hog) for (l = hog->reports; l; l = l->next) { struct report *r = l->data; - if (r->notifyid > 0) { - g_attrib_unregister(hog->attrib, r->notifyid); - r->notifyid = 0; + if (r->notify_id > 0) { + g_attrib_unregister(hog->attrib, r->notify_id); + r->notify_id = 0; + } + if (r->notify_multi_id > 0) { + g_attrib_unregister(hog->attrib, r->notify_multi_id); + r->notify_multi_id = 0; } } diff --git a/src/attrib-server.c b/src/attrib-server.c index 5a178f95ea..fb11d3db2d 100644 --- a/src/attrib-server.c +++ b/src/attrib-server.c @@ -1085,6 +1085,7 @@ static void channel_handler(const uint8_t *ipdu, uint16_t len, return; case ATT_OP_HANDLE_IND: case ATT_OP_HANDLE_NOTIFY: + case ATT_OP_HANDLE_NOTIFY_MULTI: /* The attribute client is already handling these */ return; case ATT_OP_READ_MULTI_REQ: