Hi Naga, On Thu, Jul 25, 2024 at 3:21 AM Naga Bhavani Akella <quic_nakella@xxxxxxxxxxx> wrote: > > Hi Luiz Augusto von Dentz, > > Thank you for the comments. > > On 7/24/2024 9:53 PM, Luiz Augusto von Dentz wrote: > > Hi, > > > > On Wed, Jul 24, 2024 at 7:20 AM Naga Bhavani Akella > > <quic_nakella@xxxxxxxxxxx> wrote: > >> > >> There is no option in hcitool when Peripheral wants to > >> initiate Connection Parameter Update Request, hence > >> added provision to be able to send LL_CONNECTION_PARAM_REQ > >> using hcitool. > >> > >> Required for PTS TC - GAP/CONN/CPUP/BV-02-C > >> > >> Reference link for discussion : > >> https://lore.kernel.org/linux-bluetooth/ > >> Search for Subject - LE Connection Update Disallowed > >> git code link : > >> https://gist.github.com/SandyChapman/4a64c9ea22cd27d935e3 > >> --- > >> lib/hci.c | 81 +++++++++++++++++++++++++++++++++++++++++++ > >> lib/hci_lib.h | 35 +++++++++++++++++++ > >> tools/hcitool.c | 92 +++++++++++++++++++++++++++++++++++++++++++++++++ > >> 3 files changed, 208 insertions(+) > >> > >> diff --git a/lib/hci.c b/lib/hci.c > >> index 937e65d48..294b0bbd8 100644 > >> --- a/lib/hci.c > >> +++ b/lib/hci.c > >> @@ -1119,6 +1119,87 @@ int hci_send_cmd(int dd, uint16_t ogf, uint16_t ocf, uint8_t plen, void *param) > >> return 0; > >> } > >> > >> +int hci_send_acl_data(int dd, uint16_t handle, uint8_t dlen, > >> + struct signal_hdr *sh, struct signal_payload_hdr *plh, > >> + void *pl) > >> +{ > >> + uint8_t type = HCI_ACLDATA_PKT; > >> + hci_acl_hdr ha; > >> + struct iovec iv[5]; > >> + int ivn; > >> + > >> + ha.handle = handle; > >> + ha.dlen = dlen; > >> + > >> + iv[0].iov_base = &type; > >> + iv[0].iov_len = 1; > >> + iv[1].iov_base = &ha; > >> + iv[1].iov_len = HCI_ACL_HDR_SIZE; > >> + ivn = 2; > >> + > >> + printf("\nACL Packet details[handle:%x, length:%d]\n", > >> + ha.handle, ha.dlen); > >> + > >> + if (dlen) { > >> + iv[2].iov_base = sh; > >> + iv[2].iov_len = 4; //HCI_SIGNAL_HDR_SIZE; > >> + ivn = 3; > >> + printf("\nACL signal command details[length:%d, cid:%d]\n", > >> + sh->len, sh->cid); > >> + if (sh->len > 0) { > >> + iv[3].iov_base = plh; > >> + iv[3].iov_len = 4; //HCI_SIGNAL_PAYLOAD_HDR_SIZE; > >> + ivn = 4; > >> + if (plh->len > 0) { > >> + iv[4].iov_base = pl; > >> + iv[4].iov_len = plh->len; > >> + ivn = 5; > >> + } > >> + } > >> + } > >> + > >> + while (writev(dd, iv, ivn) < 0) { > >> + if (errno == EAGAIN || errno == EINTR) > >> + continue; > >> + return -1; > >> + } > >> + return 0; > >> +} > >> + > >> +int hci_signal_le_con_param_update_req(int dd, uint16_t handle, > >> + uint16_t interval_min, > >> + uint16_t interval_max, > >> + uint16_t slave_latency, > >> + uint16_t timeout_multiplier) > >> +{ > >> + struct signal_hdr sh; > >> + struct signal_payload_hdr pl; > >> + struct le_con_param_update_req ur; > >> + > >> + uint16_t length = 0x0010; > >> + > >> + memset(&sh, 0, sizeof(sh)); > >> + memset(&pl, 0, sizeof(pl)); > >> + memset(&ur, 0, sizeof(ur)); > >> + > >> + sh.len = HCI_SIGNAL_LE_CON_PARAM_UPDATE_REQ_SIZE; > >> + sh.cid = HCI_LE_CHANNEL_ID; > >> + > >> + pl.code = LE_CON_PARAM_UPDATE_REQ_CODE; > >> + pl.id = 0x77; > >> + pl.len = LE_CON_PARAM_UPDATE_LEN; > >> + > >> + ur.interval_min = interval_min; > >> + ur.interval_max = interval_max; > >> + ur.slave_latency = slave_latency; > >> + ur.timeout_multiplier = timeout_multiplier; > >> + > >> + if (hci_send_acl_data(dd, handle, length, &sh, &pl, &ur) < 0) > >> + return -1; > >> + > >> + return 0; > >> +} > >> + > >> int hci_send_req(int dd, struct hci_request *r, int to) > >> { > >> unsigned char buf[HCI_MAX_EVENT_SIZE], *ptr; > >> diff --git a/lib/hci_lib.h b/lib/hci_lib.h > >> index baf3d3e12..fe0458a1b 100644 > >> --- a/lib/hci_lib.h > >> +++ b/lib/hci_lib.h > >> @@ -35,12 +35,47 @@ struct hci_version { > >> uint16_t lmp_subver; > >> }; > >> > >> +struct hci_acl_hdr { > >> + uint16_t handle; > >> + uint16_t len; > >> +}; > >> + > >> +struct signal_hdr { > >> + uint16_t len; > >> + uint16_t cid; > >> +}; > >> + > >> +struct signal_payload_hdr { > >> + uint8_t code; > >> + uint8_t id; > >> + uint16_t len; > >> +}; > >> + > >> +struct le_con_param_update_req { > >> + uint16_t interval_min; > >> + uint16_t interval_max; > >> + uint16_t slave_latency; > >> + uint16_t timeout_multiplier; > >> +}; > >> +#define HCI_SIGNAL_LE_CON_PARAM_UPDATE_REQ_SIZE 0x000C > >> +#define HCI_LE_CHANNEL_ID 0x0005 > >> +#define LE_CON_PARAM_UPDATE_REQ_CODE 0x12 > >> +#define LE_CON_PARAM_UPDATE_LEN 0x0008 > >> + > >> int hci_open_dev(int dev_id); > >> int hci_close_dev(int dd); > >> int hci_send_cmd(int dd, uint16_t ogf, uint16_t ocf, uint8_t plen, void *param); > >> +int hci_send_acl_data(int dd, uint16_t handle, uint8_t dlen, > >> + struct signal_hdr *sh, > >> + struct signal_payload_hdr *plh, void *pl); > >> int hci_send_req(int dd, struct hci_request *req, int timeout); > >> > >> int hci_create_connection(int dd, const bdaddr_t *bdaddr, uint16_t ptype, uint16_t clkoffset, uint8_t rswitch, uint16_t *handle, int to); > >> +int hci_signal_le_con_param_update_req(int dd, uint16_t handle, > >> + uint16_t interval_min, > >> + uint16_t interval_max, > >> + uint16_t slave_latency, > >> + uint16_t timeout_multiplier); > >> int hci_disconnect(int dd, uint16_t handle, uint8_t reason, int to); > >> > >> int hci_inquiry(int dev_id, int len, int num_rsp, const uint8_t *lap, inquiry_info **ii, long flags); > >> diff --git a/tools/hcitool.c b/tools/hcitool.c > >> index 639ee6a51..ce611bb72 100644 > >> --- a/tools/hcitool.c > >> +++ b/tools/hcitool.c > >> @@ -3369,6 +3369,97 @@ static void cmd_lecup(int dev_id, int argc, char **argv) > >> hci_close_dev(dd); > >> } > >> > >> +static const char *acl_lecup_help = > >> + "Usage:\n" > >> + "\tacllecup <handle> <min> <max> <latency> <timeout>\n" > >> + "\tOptions:\n" > >> + "\t -H, --handle <0xXXXX> LE connection handle\n" > >> + "\t -m, --min <interval> Range: 0x0006 to 0x0C80\n" > >> + "\t -M, --max <interval> Range: 0x0006 to 0x0C80\n" > >> + "\t -l, --latency <range> Slave latency. Range: 0x0000 to 0x03E8\n" > >> + "\t -t, --timeout <time> N * 10ms. Range: 0x000A to 0x0C80\n" > >> + "\n\t min/max range: 7.5ms to 4s. Multiply factor: 1.25ms" > >> + "\n\t timeout range: 100ms to 32.0s. Larger than max interval\n"; > > > > Since to be the same as lecup, is the only difference being a > > peripheral? We could perhaps attempt to detect if the handle is for a > > peripheral, or add another parameter. Also perhaps this should have > > been done via main.conf: > > > > # LE default connection parameters. These values are superceeded by any > > # specific values provided via the Load Connection Parameters interface > > #MinConnectionInterval= > > #MaxConnectionInterval= > > #ConnectionLatency= > > #ConnectionSupervisionTimeout= > > > > But perhaps the kernel is not attempting to use them when connected as > > a peripheral? It probably should though. > After connection is established successfully, there is L2CAP_CONNECTION_PARAMETER_UPDATE_REQ sent by peripheral, > it has the values that were given in main.conf file. > > PTS requires us to initiate connection parameter request way after this is done, > to initiate this again currently we don't have any option in hcitool > (if central doesn't support Connection Parameters Request Procedure) hence added this option. > > This option is to send L2CAP_CONNECTION_PARAMETER_UPDATE_REQ (CODE 0x12) from the peripheral. > This is required in the case where Central doesn't support Connection Parameters Request Procedure. > As per Spec statement - > > "If the Central, the Peripheral, or both do not support the Connection > Parameters Request procedure, then the Central shall send an > LL_CONNECTION_UPDATE_IND PDU (the Peripheral shall not send this > PDU) while the Peripheral shall use the L2CAP LE Signaling channel (see [Vol > 3] Part A, Section 4.20 and Section 4.21)." I know what the procedure is, no need to keep quoting it, what perhaps you don't realize is that this procedure is in fact implemented: https://github.com/bluez/bluetooth-next/blob/master/net/bluetooth/l2cap_core.c#L1587 So it should be sending that L2CAP PDU if you had configured a connection interval range that doesn't include the current connection interval. > > > >> +static void cmd_acl_lecup(int dev_id, int argc, char **argv) > >> +{ > >> + uint16_t handle = 0, min, max, latency, timeout; > >> + int opt, dd, base; > >> + int options = 0; > >> + > >> + /* Aleatory valid values */ > >> + min = 0x0C8; > >> + max = 0x0960; > >> + latency = 0x0007; > >> + timeout = 0x0C80; > >> + > >> + for_each_opt(opt, lecup_options, NULL) { > >> + if (optarg && strncasecmp("0x", optarg, 2) == 0) > >> + base = 16; > >> + else > >> + base = 10; > >> + > >> + switch (opt) { > >> + case 'H': > >> + handle = strtoul(optarg, NULL, base); > >> + break; > >> + case 'm': > >> + min = strtoul(optarg, NULL, base); > >> + break; > >> + case 'M': > >> + max = strtoul(optarg, NULL, base); > >> + break; > >> + case 'l': > >> + latency = strtoul(optarg, NULL, base); > >> + break; > >> + case 't': > >> + timeout = strtoul(optarg, NULL, base); > >> + break; > >> + default: > >> + printf("%s", acl_lecup_help); > >> + return; > >> + } > >> + options = 1; > >> + } > >> + > >> + if (options == 0) { > >> + helper_arg(5, 5, &argc, &argv, acl_lecup_help); > >> + > >> + handle = strtoul(argv[0], NULL, 0); > >> + min = strtoul(argv[1], NULL, 0); > >> + max = strtoul(argv[2], NULL, 0); > >> + latency = strtoul(argv[3], NULL, 0); > >> + timeout = strtoul(argv[4], NULL, 0); > >> + } > >> + > >> + if (handle == 0 || handle > 0x0EFF) { > >> + printf("%s", acl_lecup_help); > >> + return; > >> + } > >> + > >> + if (dev_id < 0) > >> + dev_id = hci_get_route(NULL); > >> + > >> + dd = hci_open_dev(dev_id); > >> + if (dd < 0) { > >> + fprintf(stderr, "HCI device open failed\n"); > >> + exit(1); > >> + } > >> + > >> + fprintf(stderr, "Signal LE Connection Update Request: %d %d %d %d %d\n", > >> + handle, min, max, latency, timeout); > >> + if (hci_signal_le_con_param_update_req(dd, htobs(handle), htobs(min), > >> + htobs(max), htobs(latency), > >> + htobs(timeout)) < 0) { > >> + int err = -errno; > >> + > >> + fprintf(stderr, "Could not change connection params: %s(%d)\n", > >> + strerror(-err), -err); > >> + } > >> + > >> + hci_close_dev(dd); > >> +} > >> + > >> static struct { > >> char *cmd; > >> void (*func)(int dev_id, int argc, char **argv); > >> @@ -3417,6 +3508,7 @@ static struct { > >> { "lecc", cmd_lecc, "Create a LE Connection" }, > >> { "ledc", cmd_ledc, "Disconnect a LE Connection" }, > >> { "lecup", cmd_lecup, "LE Connection Update" }, > >> + { "acllecup", cmd_acl_lecup, "LE ACL Connection Param Update Req" }, > >> { NULL, NULL, 0 } > >> }; > >> > >> -- > >> > > > > > -Bhavani -- Luiz Augusto von Dentz
