Re: [PATCH 1/5] monitor/rfcomm.c: Add support for MSC frame decoding

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Gowtham,

On Tue, Dec 2, 2014 at 4:39 PM, Gowtham Anandha Babu
<gowtham.ab@xxxxxxxxxxx> wrote:
> Hi Luiz,
>
>> -----Original Message-----
>> From: linux-bluetooth-owner@xxxxxxxxxxxxxxx [mailto:linux-bluetooth-
>> owner@xxxxxxxxxxxxxxx] On Behalf Of Luiz Augusto von Dentz
>> Sent: Tuesday, December 02, 2014 6:11 PM
>> To: Gowtham Anandha Babu
>> Cc: linux-bluetooth@xxxxxxxxxxxxxxx; Dmitry Kasatkin; Bharat Panda;
>> cpgs@xxxxxxxxxxx
>> Subject: Re: [PATCH 1/5] monitor/rfcomm.c: Add support for MSC frame
>> decoding
>>
>> Hi Gowtham,
>>
>> On Tue, Dec 2, 2014 at 2:37 PM, Gowtham Anandha Babu
>> <gowtham.ab@xxxxxxxxxxx> wrote:
>> > Hi Luiz,
>> >
>> >> -----Original Message-----
>> >> From: Gowtham Anandha Babu [mailto:gowtham.ab@xxxxxxxxxxx]
>> >> Sent: Tuesday, December 02, 2014 4:30 PM
>> >> To: 'Luiz Augusto von Dentz'
>> >> Cc: 'linux-bluetooth@xxxxxxxxxxxxxxx'; 'Dmitry Kasatkin'; 'Bharat
>> >> Panda'; 'cpgs@xxxxxxxxxxx'
>> >> Subject: RE: [PATCH 1/5] monitor/rfcomm.c: Add support for MSC frame
>> >> decoding
>> >>
>> >> Hi Luiz,
>> >>
>> >> > -----Original Message-----
>> >> > From: linux-bluetooth-owner@xxxxxxxxxxxxxxx
>> >> > [mailto:linux-bluetooth- owner@xxxxxxxxxxxxxxx] On Behalf Of Luiz
>> >> > Augusto von Dentz
>> >> > Sent: Tuesday, December 02, 2014 4:22 PM
>> >> > To: Gowtham Anandha Babu
>> >> > Cc: linux-bluetooth@xxxxxxxxxxxxxxx; Dmitry Kasatkin; Bharat Panda;
>> >> > cpgs@xxxxxxxxxxx
>> >> > Subject: Re: [PATCH 1/5] monitor/rfcomm.c: Add support for MSC
>> >> > frame decoding
>> >> >
>> >> > Hi Gowtham,
>> >> >
>> >> > On Tue, Dec 2, 2014 at 11:37 AM, Gowtham Anandha Babu
>> >> > <gowtham.ab@xxxxxxxxxxx> wrote:
>> >> > > Changes made to decode MSC frame in RFCOMM for btmon.
>> >> > >
>> >> > >       RFCOMM: Unnumbered Info with Header Check (UIH)(0xef)
>> >> > >          Address: 0x01 cr 0 dlci 0x00
>> >> > >          Control: 0xef poll/final 0
>> >> > >          Length: 4
>> >> > >          FCS: 0xaa
>> >> > >          MCC Message type: Modem Status Command CMD(0x38)
>> >> > >            Length: 2
>> >> > >            dlci 32
>> >> > >            fc 0 rtc 1 rtr 1 ic 0 dv 1
>> >> > > ---
>> >> > >  monitor/rfcomm.c | 76
>> >> > > ++++++++++++++++++++++++++++++++++++++++++++++++++-----
>> -
>> >> > >  1 file changed, 69 insertions(+), 7 deletions(-)
>> >> > >
>> >> > > diff --git a/monitor/rfcomm.c b/monitor/rfcomm.c index
>> >> > > b3db98b..969b29f 100644
>> >> > > --- a/monitor/rfcomm.c
>> >> > > +++ b/monitor/rfcomm.c
>> >> > > @@ -49,11 +49,24 @@ static char *cr_str[] = {
>> >> > >         "CMD"
>> >> > >  };
>> >> > >
>> >> > > -#define CR_STR(type) cr_str[GET_CR(type)] -#define
>> >> > > GET_LEN8(length) ((length & 0xfe) >> 1) -#define
>> >> > > GET_LEN16(length) ((length & 0xfffe)
>> >> > > >> 1)
>> >> > > -#define GET_CR(type)   ((type & 0x02) >> 1)
>> >> > > -#define GET_PF(ctr) (((ctr) >> 4) & 0x1)
>> >> > > +/* RFCOMM frame parsing macros */
>> >> > > +#define CR_STR(type)           cr_str[GET_CR(type)]
>> >> > > +#define GET_LEN8(length)       ((length & 0xfe) >> 1)
>> >> > > +#define GET_LEN16(length)      ((length & 0xfffe) >> 1)
>> >> > > +#define GET_CR(type)           ((type & 0x02) >> 1)
>> >> > > +#define GET_PF(ctr)            (((ctr) >> 4) & 0x1)
>> >> > > +
>> >> > > +/* MSC macros */
>> >> > > +#define GET_V24_FC(sigs)       ((sigs & 0x02) >> 1)
>> >> > > +#define GET_V24_RTC(sigs)      ((sigs & 0x04) >> 2)
>> >> > > +#define GET_V24_RTR(sigs)      ((sigs & 0x08) >> 3)
>> >> > > +#define GET_V24_IC(sigs)       ((sigs & 0x40) >> 6)
>> >> > > +#define GET_V24_DV(sigs)       ((sigs & 0x80) >> 7)
>> >> > > +
>> >> > > +#define GET_BRK_SIG1(sigs)     ((sigs & 0x02) >> 1)
>> >> > > +#define GET_BRK_SIG2(sigs)     ((sigs & 0x04) >> 2)
>> >> > > +#define GET_BRK_SIG3(sigs)     ((sigs & 0x08) >> 3)
>> >> > > +#define GET_BRK_SIG_LEN(sigs)  ((sigs & 0xf0) >> 4)
>> >> > >
>> >> > >  struct rfcomm_lhdr {
>> >> > >         uint8_t address;
>> >> > > @@ -63,6 +76,12 @@ struct rfcomm_lhdr {
>> >> > >         uint8_t credits; /* only for UIH frame */  }
>> >> > > __attribute__((packed));
>> >> > >
>> >> > > +struct rfcomm_lmsc {
>> >> > > +       uint8_t dlci;
>> >> > > +       uint8_t v24_sig;
>> >> > > +       uint8_t break_sig;
>> >> > > +} __attribute__((packed));
>> >> > > +
>> >> > >  struct rfcomm_lmcc {
>> >> > >         uint8_t type;
>> >> > >         uint16_t length;
>> >> > > @@ -92,6 +111,43 @@ static void print_rfcomm_hdr(struct
>> >> rfcomm_frame
>> >> > *rfcomm_frame, uint8_t indent)
>> >> > >         print_field("%*cFCS: 0x%2.2x", indent, ' ', hdr.fcs);  }
>> >> > >
>> >> > > +static inline bool mcc_msc(struct rfcomm_frame *rfcomm_frame,
>> >> > > +uint8_t
>> >> > > +indent) {
>> >> > > +       struct l2cap_frame *frame = &rfcomm_frame->l2cap_frame;
>> >> > > +       struct rfcomm_lmsc msc;
>> >> > > +
>> >> > > +       if (!l2cap_frame_get_u8(frame, &msc.dlci))
>> >> > > +               return false;
>> >> > > +
>> >> > > +       print_field("%*cdlci %d ", indent, ' ',
>> >> > > + RFCOMM_GET_DLCI(msc.dlci));
>> >> > > +
>> >> > > +       if (!l2cap_frame_get_u8(frame, &msc.v24_sig))
>> >> > > +               return false;
>> >> > > +
>> >> > > +       /* v24 control signals */
>> >> > > +       print_field("%*cfc %d rtc %d rtr %d ic %d dv %d", indent, ' ',
>> >> > > +               GET_V24_FC(msc.v24_sig), GET_V24_RTC(msc.v24_sig),
>> >> > > +               GET_V24_RTR(msc.v24_sig), GET_V24_IC(msc.v24_sig),
>> >> > > +                                       GET_V24_DV(msc.v24_sig));
>> >> > > +
>> >> > > +       if (frame->size < 2)
>> >> > > +               goto done;
>> >> > > +
>> >> > > +       /* break signals (optional) */
>> >> > > +
>> >> > > +       if (!l2cap_frame_get_u8(frame, &msc.break_sig))
>> >> > > +               return false;
>> >> > > +
>> >> > > +       printf("%2.2x", msc.break_sig);
>> >> >
>> >> > Im very suspicious the printf above will break indentation, you
>> >> > better add a frame that does exercise this code to make sure it is not
>> happening.
>> >> >
>> >>
>> >> That printf was added to cross check the break signals.
>> >> I Forgot to remove that line. Will update in v2.
>> >>
>> >
>> > I tried testing with many test cases, not able to capture the break signals.
>> > Any suggestions to capture the break signals?
>> > Btw the implementation here is exactly same as in hcidump.
>>
>> Perhaps you can try some test case with PTS, please check the TS if there is a
>> TC that tests break signals.
>>
>
> I referred the RFCOMM TS and tried all test cases for RFCOMM in PTS, still not able to capture the break signals.
> In the RFCOMM specs also, break signal fields are not explained much.
> Meanwhile I tried executing the MAP, PBAP, OPP, FTP connection establishment test cases, nothing worked out.
>
> Since this is optional, can we skip this (break signal decoding) for now? Or what do you think?

Then skip it and add a TODO to the code.


-- 
Luiz Augusto von Dentz
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux