Return-Path: <luiz.dentz@gmail.com>
MIME-Version: 1.0
In-Reply-To: <1456146169.17625.70.camel@linux.intel.com>
References: <1455797845-27533-1-git-send-email-patrik.flykt@linux.intel.com>
	<08B4C3A1-DC90-4D77-B4B0-B15AC0CF05F4@holtmann.org>
	<1456146169.17625.70.camel@linux.intel.com>
Date: Mon, 22 Feb 2016 15:42:17 +0200
Message-ID: <CABBYNZJxTEitC_41ZCmneTz7TNzoYqSkhxirJapHANjWVcPh=g@mail.gmail.com>
Subject: Re: [RFC] doc: Add Management Interface IPSP API definitions
From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
To: Patrik Flykt <Patrik.Flykt@linux.intel.com>
Cc: Marcel Holtmann <marcel@holtmann.org>,
	"linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>
Content-Type: text/plain; charset=UTF-8
List-ID: <linux-bluetooth.vger.kernel.org>

Hi Patrik, Marcel,

On Mon, Feb 22, 2016 at 3:02 PM, Patrik Flykt
<Patrik.Flykt@linux.intel.com> wrote:
>
>         Hi,
>
> On Fri, 2016-02-19 at 23:59 -0800, Marcel Holtmann wrote:
>> > +Connect IPSP Command
>> > +====================
>> > +
>> > +   Command Code:           0x0042
>> > +   Controller Index:       <controller id>
>> > +   Command Parameters:     Address (6 Octets)
>> > +                           Address_Type (1 Octet)
>> > +   Return Parameters:      Interface Index (4 Octets)
>> > +                           Address (6 Octets)
>> > +                           Address_Type (1 Octet)
>>
>> I think you really want to swap this. Address + Type first and then
>> Interface Index.
>
> Ok.
>
>> > +   This command is used to connect to a remote IPSP Node.
>> > +
>> > +   Possible values for the Address_Type parameter:
>> > +           1       LE Public
>> > +           2       LE Random
>> > +
>> > +   This command generates a Command Complete event on success
>> > +   or failure.
>> > +
>> > +   Possible errors:        Busy
>> > +                           Refused
>> > +                           Invalid Parameters
>> > +                           Not Powered
>> > +                           Invalid Index
>> > +                           Already Connected
>> > +
>> > +
>> > +Disconnect IPSP Command
>> > +=======================
>> > +
>> > +   Command Code:           0x0043
>> > +   Controller Index:       <controller id>
>> > +   Command Parameters:     Address (6 Octets)
>> > +                           Address_Type (1 Octet)
>> > +   Return Parameters:      Address (6 Octets)
>> > +                           Address_Type (1 Octet)
>> > +
>> > +   This command is used to disconnect from a remote IPSP device.
>> > +
>> > +   Possible values for the Address_Type parameter:
>> > +           1       LE Public
>> > +           2       LE Random
>>
>> I actually prefer that we list 0 value as reserved like we have done
>> for the other LE only commands.
>>
>> In addition we might want to make clear that these commands are only
>> supported if LE is actually enabled first.
>
> Ok.
>
>> > +Get IPSP Connections Command
>> > +============================
>> > +
>> > +   Command Code:           0x0044
>> > +   Controller Index:       <controller id>
>> > +   Command Parameters:
>> > +   Return Parameters:      Interface Index (4 Octets)
>> > +                           Connection_Count (2 Octets)
>> > +                           Address1 {
>> > +                                   Address (6 Octets)
>> > +                                   Address_Type (1 Octet)
>> > +                           }
>> > +                           Address2 { }
>> > +                           ...
>>
>> I would normally assume that the Get Connections one comes before the
>> Connect / Disconnect commands.
>
> Swapped order here.
>
>> > +
>> > +   This command is used to retrieve a list of currently connected
>> > +   IPSP devices.
>> > +
>> > +   Possible values for the Address_Type parameter:
>> > +           1       LE Public
>> > +           2       LE Random
>> > +
>> > +   For devices using resolvable random addresses with a known
>> > +   identity resolving key, the Address and Address_Type will
>> > +   contain the identity information.
>> > +
>> > +   This command can only be used when the controller is powered.
>> > +
>> > +   This command generates a Command Complete event on success or
>> > +   a Command Status event on failure.
>> > +
>> > +   Possible errors:        Invalid Parameters
>> > +                           Not Powered
>> > +                           Invalid Index
>> > +
>> > +
>> > +Enable IPSP Server Command
>> > +==========================
>> > +
>> > +   Command Code:           0x0045
>> > +   Controller Index:       <controller id>
>> > +   Command Parameters:     Enable (1 Octet)
>> > +   Return Parameters:
>>
>> Not a big fan of the command name. We never used the Enable Something
>> term.
>
> Is 'Set IPSP Node Role Command' ok?

Btw what exactly is this command suppose to do? Just add a listening
channel or does it actually enables advertising? How do we actually
hook with the agent to authorize incoming connections? Perhaps it
actually a better idea to use the L2CAP socket API for this and make
bluetoothd listen on PSM 35 like we do for other sockets, once the
connection is authorized then we can have a mgmt command to add it to
the network interface, actually this may as well replace the
Connect/Disconnect commands with Add/Remove client so the connection
phase is actually handled by bluetoothd.

>
>> > +IPSP Connected Event
>> > +====================
>> > +
>> > +   Event Code:             0x0025
>> > +   Controller Index:       <controller id>
>> > +   Event Parameters:       Interface Index (4 Octets)
>> > +                           Address (6 Octets)
>> > +                           Address_Type (1 Octet)
>>
>> Same here. I think they are in the wrong order.
>
> Ok.
>
>> > +IPSP Disconnected Event
>> > +=======================
>> > +
>> > +   Event Code:             0x0026
>> > +   Controller Index:       <controller id>
>> > +   Event Parameters:       Address (6 Octets)
>> > +                           Address_Type (1 Octet)
>> > +
>> > +   This event indicates that the IPSP connection was lost to a
>> > +   remote device.
>> > +
>> > +   Possible values for the Address_Type parameter:
>> > +           1       LE Public
>> > +           2       LE Random
>> > +
>> > +   For devices using resolvable random addresses with a known
>> > +   identity resolving key, the Address and Address_Type will
>> > +   contain the identity information.
>>
>> I am not a huge fan of the IPSP in the commands and events. Can we
>> find something better?
>
> There was a discussion on naming in "Re: [RFC v3] doc: Add initial
> Network Management API definition" on the mailing list last June that
> ended a bit inconclusive. Meanwhile RFC 7668 has gravitated towards
> using "6LowPAN techniques" to describe how IPv6 is transported over
> BTLE, but the RFC itself is called "IPv6 over BLUETOOTH(R) Low Energy".
>
> Then again include/net/bluetooth/l2cap.h defines a value called
> L2CAP_PSM_IPSP.
>
> Shall we continue using 6lowpan as before in RFC v3?
>
>
> Cheers,
>
>         Patrik
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Luiz Augusto von Dentz