diff mbox series

[BlueZ,v1] hcitool - Added option for Peripheral Initiated Connection Parameter Update Request.

Message ID 20240724111747.5952-1-quic_nakella@quicinc.com (mailing list archive)
State New, archived
Headers show
Series [BlueZ,v1] hcitool - Added option for Peripheral Initiated Connection Parameter Update Request. | expand

Checks

Context Check Description
tedd_an/pre-ci_am success Success
tedd_an/CheckPatch success CheckPatch PASS
tedd_an/GitLint fail WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search 1: T1 Title exceeds max length (95>80): "[BlueZ,v1] hcitool - Added option for Peripheral Initiated Connection Parameter Update Request." 1: T3 Title has trailing punctuation (.): "[BlueZ,v1] hcitool - Added option for Peripheral Initiated Connection Parameter Update Request."
tedd_an/BuildEll success Build ELL PASS
tedd_an/BluezMake success Bluez Make PASS
tedd_an/MakeCheck success Bluez Make Check PASS
tedd_an/MakeDistcheck success Make Distcheck PASS
tedd_an/CheckValgrind success Check Valgrind PASS
tedd_an/CheckSmatch success CheckSparse PASS
tedd_an/bluezmakeextell success Make External ELL PASS
tedd_an/IncrementalBuild success Incremental Build PASS
tedd_an/ScanBuild warning ScanBuild: tools/hcitool.c:3404:13: warning: Null pointer passed to 1st parameter expecting 'nonnull' handle = strtoul(optarg, NULL, base); ^~~~~~~~~~~~~~~~~~~~~~~~~~~ tools/hcitool.c:3407:10: warning: Null pointer passed to 1st parameter expecting 'nonnull' min = strtoul(optarg, NULL, base); ^~~~~~~~~~~~~~~~~~~~~~~~~~~ tools/hcitool.c:3410:10: warning: Null pointer passed to 1st parameter expecting 'nonnull' max = strtoul(optarg, NULL, base); ^~~~~~~~~~~~~~~~~~~~~~~~~~~ tools/hcitool.c:3413:14: warning: Null pointer passed to 1st parameter expecting 'nonnull' latency = strtoul(optarg, NULL, base); ^~~~~~~~~~~~~~~~~~~~~~~~~~~ tools/hcitool.c:3416:14: warning: Null pointer passed to 1st parameter expecting 'nonnull' timeout = strtoul(optarg, NULL, base); ^~~~~~~~~~~~~~~~~~~~~~~~~~~ 5 warnings generated.

Commit Message

Naga Bhavani Akella July 24, 2024, 11:17 a.m. UTC
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(+)

--

Comments

bluez.test.bot@gmail.com July 24, 2024, 1:04 p.m. UTC | #1
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=873515

---Test result---

Test Summary:
CheckPatch                    PASS      0.47 seconds
GitLint                       FAIL      0.47 seconds
BuildEll                      PASS      24.98 seconds
BluezMake                     PASS      1736.79 seconds
MakeCheck                     PASS      12.84 seconds
MakeDistcheck                 PASS      180.57 seconds
CheckValgrind                 PASS      256.98 seconds
CheckSmatch                   PASS      359.78 seconds
bluezmakeextell               PASS      121.86 seconds
IncrementalBuild              PASS      1513.53 seconds
ScanBuild                     WARNING   1035.52 seconds

Details
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[BlueZ,v1] hcitool - Added option for Peripheral Initiated Connection Parameter Update Request.

WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
1: T1 Title exceeds max length (95>80): "[BlueZ,v1] hcitool - Added option for Peripheral Initiated Connection Parameter Update Request."
1: T3 Title has trailing punctuation (.): "[BlueZ,v1] hcitool - Added option for Peripheral Initiated Connection Parameter Update Request."
##############################
Test: ScanBuild - WARNING
Desc: Run Scan Build
Output:
tools/hcitool.c:3404:13: warning: Null pointer passed to 1st parameter expecting 'nonnull'
                        handle = strtoul(optarg, NULL, base);
                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~
tools/hcitool.c:3407:10: warning: Null pointer passed to 1st parameter expecting 'nonnull'
                        min = strtoul(optarg, NULL, base);
                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~
tools/hcitool.c:3410:10: warning: Null pointer passed to 1st parameter expecting 'nonnull'
                        max = strtoul(optarg, NULL, base);
                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~
tools/hcitool.c:3413:14: warning: Null pointer passed to 1st parameter expecting 'nonnull'
                        latency = strtoul(optarg, NULL, base);
                                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~
tools/hcitool.c:3416:14: warning: Null pointer passed to 1st parameter expecting 'nonnull'
                        timeout = strtoul(optarg, NULL, base);
                                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~
5 warnings generated.



---
Regards,
Linux Bluetooth
Luiz Augusto von Dentz July 24, 2024, 4:23 p.m. UTC | #2
Hi,

On Wed, Jul 24, 2024 at 7:20 AM Naga Bhavani Akella
<quic_nakella@quicinc.com> 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.

> +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 }
>  };
>
> --
>
Naga Bhavani Akella July 25, 2024, 7:21 a.m. UTC | #3
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@quicinc.com> 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)."
> 
>> +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 July 25, 2024, 1:50 p.m. UTC | #4
Hi Naga,

On Thu, Jul 25, 2024 at 3:21 AM Naga Bhavani Akella
<quic_nakella@quicinc.com> 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@quicinc.com> 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
Naga Bhavani Akella July 25, 2024, 4:38 p.m. UTC | #5
Hi Luiz,

On 7/25/2024 7:20 PM, Luiz Augusto von Dentz wrote:
> Hi Naga,
> 
> On Thu, Jul 25, 2024 at 3:21 AM Naga Bhavani Akella
> <quic_nakella@quicinc.com> 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@quicinc.com> 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.
Perhaps you have missed my comment,
"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."

Please let us know if there is a way we can delay sending this pdu or if there is a way to initiate the process again
at a later point of time (when PTS prompts us to initiate).
> 
>>>
>>>> +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
> 
> 
>
Naga Bhavani Akella Aug. 6, 2024, 10:17 a.m. UTC | #6
Hi Luiz,

Could you please help us with the below request.

On 7/25/2024 10:08 PM, Naga Bhavani Akella wrote:
> Hi Luiz,
> 
> On 7/25/2024 7:20 PM, Luiz Augusto von Dentz wrote:
>> Hi Naga,
>>
>> On Thu, Jul 25, 2024 at 3:21 AM Naga Bhavani Akella
>> <quic_nakella@quicinc.com> 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@quicinc.com> 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.
> Perhaps you have missed my comment,
> "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."
> 
> Please let us know if there is a way we can delay sending this pdu or if there is a way to initiate the process again
> at a later point of time (when PTS prompts us to initiate).
>>
>>>>
>>>>> +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
>>
>>
>>
-Bhavani
diff mbox series

Patch

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";
+
+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 }
 };