Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <CABBYNZJ=RirU3of1pCUjgBwKTP0Rmv5g0uGP-CKJnYLvdDUwOQ@mail.gmail.com>
 <20200608134036.3242711-1-marijns95@gmail.com>
In-Reply-To: <20200608134036.3242711-1-marijns95@gmail.com>
From:   Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date:   Mon, 8 Jun 2020 08:54:40 -0700
Message-ID: <CABBYNZJ-BBGRz8BATC45Un+6rxCLj2zDVe=m-ReepP974RwDww@mail.gmail.com>
Subject: Re: [PATCH BlueZ] btio: Fix not translation mode to L2CAP mode
To:     Marijn Suijten <marijns95@gmail.com>
Cc:     "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk

Hi Marijn,

On Mon, Jun 8, 2020 at 6:40 AM Marijn Suijten <marijns95@gmail.com> wrote:
>
> Hi Luiz,
>
> > Hi,
> >
> > On Fri, Jun 5, 2020 at 10:59 AM Luiz Augusto von Dentz
> > <luiz.dentz@gmail.com> wrote:
> > >
> > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > >
> > > When using L2CAP_OPTIONS legacy modes need to be used since they are
> > > not compatible with BT_MODE.
> > > ---
> > >  btio/btio.c | 24 ++++++++++++++++++++++--
> > >  1 file changed, 22 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/btio/btio.c b/btio/btio.c
> > > index 13c731062..844d6007f 100644
> > > --- a/btio/btio.c
> > > +++ b/btio/btio.c
> > > @@ -597,6 +597,20 @@ static gboolean get_key_size(int sock, int *size, GError **err)
> > >         return FALSE;
> > >  }
> > >
> > > +static uint8_t mode_l2mode(uint8_t mode)
> > > +{
> > > +       switch (mode) {
> > > +       case BT_IO_MODE_BASIC:
> > > +               return L2CAP_MODE_BASIC;
> > > +       case BT_IO_MODE_ERTM:
> > > +               return L2CAP_MODE_ERTM;
> > > +       case BT_IO_MODE_STREAMING:
> > > +               return L2CAP_MODE_STREAMING;
> > > +       default:
> > > +               return UINT8_MAX;
> > > +       }
> > > +}
> > > +
> > >  static gboolean set_l2opts(int sock, uint16_t imtu, uint16_t omtu,
> > >                                                 uint8_t mode, GError **err)
> > >  {
> > > @@ -614,8 +628,14 @@ static gboolean set_l2opts(int sock, uint16_t imtu, uint16_t omtu,
> > >                 l2o.imtu = imtu;
> > >         if (omtu)
> > >                 l2o.omtu = omtu;
> > > -       if (mode)
> > > -               l2o.mode = mode;
> > > +
> > > +       if (mode) {
> > > +               l2o.mode = mode_l2mode(mode);
> > > +               if (l2o.mode == UINT8_MAX) {
> > > +                       ERROR_FAILED(err, "Unsupported mode", errno);
> > > +                       return FALSE;
> > > +               }
> > > +       }
> > >
> > >         if (setsockopt(sock, SOL_L2CAP, L2CAP_OPTIONS, &l2o, sizeof(l2o)) < 0) {
> > >                 ERROR_FAILED(err, "setsockopt(L2CAP_OPTIONS)", errno);
> > > --
> > > 2.25.3
> >
> > Pushed.
> >
> > --
> > Luiz Augusto von Dentz
>
> This patch seems to break avctp browsing. The creation of browsing_io in
> avctp_register already uses L2CAP_MODE_ERTM which is not in the
> switch-case hence results in "Unsupported mode". What is the desired way
> to fix this? Should all those calls use BT_IO_MODE_* instead? Not to
> mention the uses of these functions should be checked for their enum
> usage as well.

I will fix it, bt_io API shall only operate with the its mode not
L2CAP mode so it is a bug to pass them.

-- 
Luiz Augusto von Dentz