From: Marcel Holtmann <marcel@holtmann.org>
To: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Cc: "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>
Subject: Re: [PATCH v5 3/3] Bluetooth: Add BT_MODE socket option
Date: Tue, 24 Mar 2020 19:37:25 +0100 [thread overview]
Message-ID: <BE08FB57-ED55-486F-85F9-180025895569@holtmann.org> (raw)
In-Reply-To: <CABBYNZJefwjHOJdLHe_pj6g_sZzSdAqWgprfAs3bgQOY8=ESHA@mail.gmail.com>
Hi Luiz,
>>> This adds BT_MODE socket option which can be used to set L2CAP modes,
>>> including modes only supported over LE which were not supported using
>>> the L2CAP_OPTIONS.
>>>
>>> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>>> ---
>>> include/net/bluetooth/bluetooth.h | 8 ++
>>> include/net/bluetooth/l2cap.h | 6 ++
>>> net/bluetooth/l2cap_sock.c | 124 ++++++++++++++++++++++++++++++
>>> 3 files changed, 138 insertions(+)
>>>
>>> diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
>>> index 1576353a2773..3fa7b1e3c5d9 100644
>>> --- a/include/net/bluetooth/bluetooth.h
>>> +++ b/include/net/bluetooth/bluetooth.h
>>> @@ -139,6 +139,14 @@ struct bt_voice {
>>> #define BT_PHY_LE_CODED_TX 0x00002000
>>> #define BT_PHY_LE_CODED_RX 0x00004000
>>>
>>> +#define BT_MODE 15
>>> +
>>> +#define BT_MODE_BASIC 0x00
>>> +#define BT_MODE_ERTM 0x01
>>> +#define BT_MODE_STREAMING 0x02
>>> +#define BT_MODE_LE_FLOWCTL 0x03
>>> +#define BT_MODE_EXT_FLOWCTL 0x04
>>> +
>>> __printf(1, 2)
>>> void bt_info(const char *fmt, ...);
>>> __printf(1, 2)
>>> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
>>> index dada14d0622c..56f727ba23bd 100644
>>> --- a/include/net/bluetooth/l2cap.h
>>> +++ b/include/net/bluetooth/l2cap.h
>>> @@ -720,9 +720,15 @@ struct l2cap_user {
>>> /* ----- L2CAP socket info ----- */
>>> #define l2cap_pi(sk) ((struct l2cap_pinfo *) sk)
>>>
>>> +#define L2CAP_PI_OPTION_UNSET 0x00
>>> +#define L2CAP_PI_OPTION_LEGACY 0x01
>>> +#define L2CAP_PI_OPTION_BT_MODE 0x02
>>> +
>>> struct l2cap_pinfo {
>>> struct bt_sock bt;
>>> struct l2cap_chan *chan;
>>> + u8 option;
>>> + u8 bt_mode;
>>> struct sk_buff *rx_busy_skb;
>>> };
>>
>> why do you want to store bt_mode here. Whatever we have in l2cap_chan should be plenty.
>
> Ive thought it would be cleaner to mess with types that comes from the
> spec itself so l2cap_chan would continue to use them.
Actually the LE_FLOWCTL and EXT_FLOWCTL are not spec values either. Frankly I don’t care which ones we store right now. I am fine keeping the spec one for now and just translate it to BT_MODE.
>> I also looked at l2cap_sock_setsockopt_old and if you use L2CAP_OPTIONS and want to read BT_MODE, then everything should be fine. Same as setting BT_MODE (except EXT_FLOWCTL) and then reading L2CAP_OPTIONS is fine as well. We can all translate this properly and with have EINVAL return errors for not supported / disabled modes.
>>
>> So the only time L2CAP_OPTIONS read should fail is if you use BT_MODE with EXT_FLOWCTL as mode. So you can just check the mode set in l2cap_chan. And we start using our new mode definition there and then convert it for L2CAP_OPTIONS.
>
> Sure, I thought it would be more efficient to not have conversions
> back and forth but Im fine either way.
In the end it makes the code simpler to just do the conversion. And once we add a kernel option to disable setsockopt_old, we have less code.
Regards
Marcel
prev parent reply other threads:[~2020-03-24 18:37 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-23 20:34 [PATCH v5 0/3] Enable use of L2CAP_MODE_EXT_FLOWCTL Luiz Augusto von Dentz
2020-03-23 20:34 ` [PATCH v5 1/3] Bluetooth: L2CAP: Add get_peer_pid callback Luiz Augusto von Dentz
2020-03-24 8:48 ` Marcel Holtmann
2020-03-23 20:34 ` [PATCH v5 2/3] Bluetooth: L2CAP: Use DEFER_SETUP to group ECRED connections Luiz Augusto von Dentz
2020-03-24 8:47 ` Marcel Holtmann
2020-03-24 23:10 ` Luiz Augusto von Dentz
2020-03-23 20:34 ` [PATCH v5 3/3] Bluetooth: Add BT_MODE socket option Luiz Augusto von Dentz
2020-03-24 8:44 ` Marcel Holtmann
2020-03-24 16:56 ` Luiz Augusto von Dentz
2020-03-24 18:37 ` Marcel Holtmann [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=BE08FB57-ED55-486F-85F9-180025895569@holtmann.org \
--to=marcel@holtmann.org \
--cc=linux-bluetooth@vger.kernel.org \
--cc=luiz.dentz@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).