Hi Luiz,
>>> This introduces mgmt_alloc_skb and mgmt_send_event_skb which are
>>> convenient when building MGMT events that have variable length as the
>>> likes of skb_put_data can be used to insert portion directly on the skb
>>> instead of having to first build an intermediate buffer just to be
>>> copied over the skb.
>>>
>>> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx>
>>> ---
>>> net/bluetooth/mgmt_util.c | 54 +++++++++++++++++++++++++++------------
>>> net/bluetooth/mgmt_util.h | 3 +++
>>> 2 files changed, 40 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/net/bluetooth/mgmt_util.c b/net/bluetooth/mgmt_util.c
>>> index 83875f2a0604..4774e993d774 100644
>>> --- a/net/bluetooth/mgmt_util.c
>>> +++ b/net/bluetooth/mgmt_util.c
>>> @@ -56,40 +56,60 @@ static struct sk_buff *create_monitor_ctrl_event(__le16 index, u32 cookie,
>>> return skb;
>>> }
>>>
>>> -int mgmt_send_event(u16 event, struct hci_dev *hdev, unsigned short channel,
>>> - void *data, u16 data_len, int flag, struct sock *skip_sk)
>>> +struct sk_buff *mgmt_alloc_skb(unsigned int size)
>>> {
>>> struct sk_buff *skb;
>>> +
>>> + skb = alloc_skb(sizeof(struct mgmt_hdr) + size, GFP_KERNEL);
>>> + if (skb)
>>> + skb_reserve(skb, sizeof(struct mgmt_hdr));
>>> +
>>> + return skb;
>>> +}
>>> +
>>> +int mgmt_send_event_skb(u16 event, struct hci_dev *hdev, unsigned short channel,
>>> + struct sk_buff *skb, int flag, struct sock *skip_sk)
>>> +{
>>> struct mgmt_hdr *hdr;
>>> + int len = skb->len;
>>>
>>> - skb = alloc_skb(sizeof(*hdr) + data_len, GFP_KERNEL);
>>> - if (!skb)
>>> - return -ENOMEM;
>>> + /* Time stamp */
>>> + __net_timestamp(skb);
>>>
>>> - hdr = skb_put(skb, sizeof(*hdr));
>>> + /* Send just the data, without headers, to the monitor */
>>> + if (channel == HCI_CHANNEL_CONTROL)
>>> + hci_send_monitor_ctrl_event(hdev, event, skb->data, skb->len,
>>> + skb_get_ktime(skb), flag, skip_sk);
>>> +
>>> + hdr = skb_push(skb, sizeof(*hdr));
>>> hdr->opcode = cpu_to_le16(event);
>>> if (hdev)
>>> hdr->index = cpu_to_le16(hdev->id);
>>> else
>>> hdr->index = cpu_to_le16(MGMT_INDEX_NONE);
>>> - hdr->len = cpu_to_le16(data_len);
>>> -
>>> - if (data)
>>> - skb_put_data(skb, data, data_len);
>>> -
>>> - /* Time stamp */
>>> - __net_timestamp(skb);
>>> + hdr->len = cpu_to_le16(len);
>>>
>>> hci_send_to_channel(channel, skb, flag, skip_sk);
>>>
>>> - if (channel == HCI_CHANNEL_CONTROL)
>>> - hci_send_monitor_ctrl_event(hdev, event, data, data_len,
>>> - skb_get_ktime(skb), flag, skip_sk);
>>> -
>>> kfree_skb(skb);
>>> return 0;
>>> }
>>>
>>> +int mgmt_send_event(u16 event, struct hci_dev *hdev, unsigned short channel,
>>> + void *data, u16 data_len, int flag, struct sock *skip_sk)
>>> +{
>>> + struct sk_buff *skb;
>>> +
>>> + skb = mgmt_alloc_skb(data_len);
>>> + if (!skb)
>>> + return -ENOMEM;
>>> +
>>> + if (data)
>>> + skb_put_data(skb, data, data_len);
>>> +
>>> + return mgmt_send_event_skb(event, hdev, channel, skb, flag, skip_sk);
>>> +}
>>> +
>>> int mgmt_cmd_status(struct sock *sk, u16 index, u16 cmd, u8 status)
>>> {
>>> struct sk_buff *skb, *mskb;
>>> diff --git a/net/bluetooth/mgmt_util.h b/net/bluetooth/mgmt_util.h
>>> index 63b965eaaaac..9f8692d4ce90 100644
>>> --- a/net/bluetooth/mgmt_util.h
>>> +++ b/net/bluetooth/mgmt_util.h
>>> @@ -32,6 +32,9 @@ struct mgmt_pending_cmd {
>>> int (*cmd_complete)(struct mgmt_pending_cmd *cmd, u8 status);
>>> };
>>>
>>> +struct sk_buff *mgmt_alloc_skb(unsigned int size);
>>> +int mgmt_send_event_skb(u16 event, struct hci_dev *hdev, unsigned short channel,
>>> + struct sk_buff *skb, int flag, struct sock *skip_sk);
>>> int mgmt_send_event(u16 event, struct hci_dev *hdev, unsigned short channel,
>>> void *data, u16 data_len, int flag, struct sock *skip_sk);
>>> int mgmt_cmd_status(struct sock *sk, u16 index, u16 cmd, u8 status);
>>
>> this altogether doesn’t create nicer to read code. What about doing this:
>>
>> diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
>> index 2f31e571f34c..fd0a114a69c6 100644
>> --- a/include/net/bluetooth/bluetooth.h
>> +++ b/include/net/bluetooth/bluetooth.h
>> @@ -399,6 +399,7 @@ struct bt_skb_cb {
>> struct l2cap_ctrl l2cap;
>> struct sco_ctrl sco;
>> struct hci_ctrl hci;
>> + struct mgmt_ctrl mgmt;
>> };
>> };
>>
>> And then adding hci_dev and event to mgmt_ctrl. So you can do something like:
>>
>> struct sk_buff *mgmt_alloc_skb(struct hci_dev *hdev, u16 event, unsigned int size);
>
> So you want bt_cb to be used when sending? We could as well push the
> mgmt_header as well and then update the length at send since the
> header is already at the start in skb->data.
but hdev->id is your index and even you feed in. Doing the skb_push last is easier, then doing it early and then fiddle with skb->head.
Regards
Marcel
