* [PATCH 16/22] Store new configuration values in l2cap_pinfo
@ 2010-08-25 22:37 haijun liu
2010-08-26 1:21 ` Gustavo F. Padovan
2010-09-02 23:02 ` Mat Martineau
0 siblings, 2 replies; 8+ messages in thread
From: haijun liu @ 2010-08-25 22:37 UTC (permalink / raw)
To: linux-bluetooth
>From d093975dde6d85c824a5aaac943d676100810010 Mon Sep 17 00:00:00 2001
From: haijun.liu <haijun.liu@atheros.com>
Date: Mon, 23 Aug 2010 00:09:56 +0800
Subject: [PATCH 16/22] Store new configuration values in l2cap_pinfo.
---
include/net/bluetooth/l2cap.h | 9 +++++++++
1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 2d864d4..f2dd65d 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -406,6 +406,15 @@ struct l2cap_pinfo {
__u16 remote_mps;
__u16 mps;
+ __u8 ext_flowspec_enable;
+ struct ext_flow_spec loc_efs;
+ struct ext_flow_spec rem_efs;
+
+ __u8 extwin_enable;
+ __u16 extwin_size;
+ __u8 rem_extwin_enable;
+ __u16 rem_extwin_size;
+
__le16 sport;
struct timer_list retrans_timer;
--
1.6.3.3
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 16/22] Store new configuration values in l2cap_pinfo
2010-08-25 22:37 [PATCH 16/22] Store new configuration values in l2cap_pinfo haijun liu
@ 2010-08-26 1:21 ` Gustavo F. Padovan
2010-09-02 23:02 ` Mat Martineau
1 sibling, 0 replies; 8+ messages in thread
From: Gustavo F. Padovan @ 2010-08-26 1:21 UTC (permalink / raw)
To: haijun liu; +Cc: linux-bluetooth
Hi Haijun,
* haijun liu <liuhaijun.er@gmail.com> [2010-08-26 06:37:30 +0800]:
> From d093975dde6d85c824a5aaac943d676100810010 Mon Sep 17 00:00:00 2001
> From: haijun.liu <haijun.liu@atheros.com>
> Date: Mon, 23 Aug 2010 00:09:56 +0800
> Subject: [PATCH 16/22] Store new configuration values in l2cap_pinfo.
>
> ---
> include/net/bluetooth/l2cap.h | 9 +++++++++
> 1 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index 2d864d4..f2dd65d 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -406,6 +406,15 @@ struct l2cap_pinfo {
> __u16 remote_mps;
> __u16 mps;
>
> + __u8 ext_flowspec_enable;
> + struct ext_flow_spec loc_efs;
> + struct ext_flow_spec rem_efs;
> +
> + __u8 extwin_enable;
> + __u16 extwin_size;
> + __u8 rem_extwin_enable;
> + __u16 rem_extwin_size;
The same comment in 14/22 goes here and others patches in the patch set.
--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 16/22] Store new configuration values in l2cap_pinfo
2010-08-25 22:37 [PATCH 16/22] Store new configuration values in l2cap_pinfo haijun liu
2010-08-26 1:21 ` Gustavo F. Padovan
@ 2010-09-02 23:02 ` Mat Martineau
2010-09-03 1:30 ` haijun liu
1 sibling, 1 reply; 8+ messages in thread
From: Mat Martineau @ 2010-09-02 23:02 UTC (permalink / raw)
To: haijun liu; +Cc: linux-bluetooth
On Thu, 26 Aug 2010, haijun liu wrote:
> From d093975dde6d85c824a5aaac943d676100810010 Mon Sep 17 00:00:00 2001
> From: haijun.liu <haijun.liu@atheros.com>
> Date: Mon, 23 Aug 2010 00:09:56 +0800
> Subject: [PATCH 16/22] Store new configuration values in l2cap_pinfo.
>
> ---
> include/net/bluetooth/l2cap.h | 9 +++++++++
> 1 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index 2d864d4..f2dd65d 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -406,6 +406,15 @@ struct l2cap_pinfo {
> __u16 remote_mps;
> __u16 mps;
>
> + __u8 ext_flowspec_enable;
> + struct ext_flow_spec loc_efs;
> + struct ext_flow_spec rem_efs;
> +
> + __u8 extwin_enable;
> + __u16 extwin_size;
> + __u8 rem_extwin_enable;
> + __u16 rem_extwin_size;
> +
> __le16 sport;
>
> struct timer_list retrans_timer;
Regarding the new "extwin" structure members, have you considered
changing the existing tx_win and remote_tx_win members to __u16 and
using them with both standard and extended window sizes?
The spec also requires that both directions of the link use the same
type of control field (standard or extended). After L2CAP
configuration is done, all the information required for the transmit
window is the control field type, tx_win, and remote_tx_win. The
control field would be set to 'extended' if a successful configuration
response is sent or received for the extended window size option.
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 16/22] Store new configuration values in l2cap_pinfo
2010-09-02 23:02 ` Mat Martineau
@ 2010-09-03 1:30 ` haijun liu
2010-09-03 20:58 ` Mat Martineau
0 siblings, 1 reply; 8+ messages in thread
From: haijun liu @ 2010-09-03 1:30 UTC (permalink / raw)
To: Mat Martineau, linux-bluetooth
On Fri, Sep 3, 2010 at 7:02 AM, Mat Martineau <mathewm@codeaurora.org> wrote:
>
> On Thu, 26 Aug 2010, haijun liu wrote:
>
>> From d093975dde6d85c824a5aaac943d676100810010 Mon Sep 17 00:00:00 2001
>> From: haijun.liu <haijun.liu@atheros.com>
>> Date: Mon, 23 Aug 2010 00:09:56 +0800
>> Subject: [PATCH 16/22] Store new configuration values in l2cap_pinfo.
>>
>> ---
>> include/net/bluetooth/l2cap.h | 9 +++++++++
>> 1 files changed, 9 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
>> index 2d864d4..f2dd65d 100644
>> --- a/include/net/bluetooth/l2cap.h
>> +++ b/include/net/bluetooth/l2cap.h
>> @@ -406,6 +406,15 @@ struct l2cap_pinfo {
>> __u16 remote_mps;
>> __u16 mps;
>>
>> + __u8 ext_flowspec_enable;
>> + struct ext_flow_spec loc_efs;
>> + struct ext_flow_spec rem_efs;
>> +
>> + __u8 extwin_enable;
>> + __u16 extwin_size;
>> + __u8 rem_extwin_enable;
>> + __u16 rem_extwin_size;
>> +
>> __le16 sport;
>>
>> struct timer_list retrans_timer;
>
> Regarding the new "extwin" structure members, have you considered changing
> the existing tx_win and remote_tx_win members to __u16 and using them with
> both standard and extended window sizes?
>
> The spec also requires that both directions of the link use the same type of
> control field (standard or extended). After L2CAP configuration is done,
> all the information required for the transmit window is the control field
> type, tx_win, and remote_tx_win. The control field would be set to
> 'extended' if a successful configuration response is sent or received for
> the extended window size option.
>
>
> --
> Mat Martineau
> Employee of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
>
>
Yes, we do, please look into the patch, we use __u16 for extwin_size &
rem_extwin_size
+ __u8 extwin_enable;
+ __u16 extwin_size;
+ __u8 rem_extwin_enable;
+ __u16 rem_extwin_size;
You are exactly right, in our implementation, choosing standard or
extended window,
it depends whether successful configuration response contain the
extended window size option.
--
Haijun Liu
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 16/22] Store new configuration values in l2cap_pinfo
2010-09-03 1:30 ` haijun liu
@ 2010-09-03 20:58 ` Mat Martineau
2010-09-03 21:04 ` Gustavo F. Padovan
0 siblings, 1 reply; 8+ messages in thread
From: Mat Martineau @ 2010-09-03 20:58 UTC (permalink / raw)
To: haijun liu, haijun.liu; +Cc: linux-bluetooth
On Fri, 3 Sep 2010, haijun liu wrote:
> On Fri, Sep 3, 2010 at 7:02 AM, Mat Martineau <mathewm@codeaurora.org> wrote:
>>
>> On Thu, 26 Aug 2010, haijun liu wrote:
>>
>>> From d093975dde6d85c824a5aaac943d676100810010 Mon Sep 17 00:00:00 2001
>>> From: haijun.liu <haijun.liu@atheros.com>
>>> Date: Mon, 23 Aug 2010 00:09:56 +0800
>>> Subject: [PATCH 16/22] Store new configuration values in l2cap_pinfo.
>>>
>>> ---
>>> include/net/bluetooth/l2cap.h | 9 +++++++++
>>> 1 files changed, 9 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
>>> index 2d864d4..f2dd65d 100644
>>> --- a/include/net/bluetooth/l2cap.h
>>> +++ b/include/net/bluetooth/l2cap.h
>>> @@ -406,6 +406,15 @@ struct l2cap_pinfo {
>>> __u16 remote_mps;
>>> __u16 mps;
>>>
>>> + __u8 ext_flowspec_enable;
>>> + struct ext_flow_spec loc_efs;
>>> + struct ext_flow_spec rem_efs;
>>> +
>>> + __u8 extwin_enable;
>>> + __u16 extwin_size;
>>> + __u8 rem_extwin_enable;
>>> + __u16 rem_extwin_size;
>>> +
>>> __le16 sport;
>>>
>>> struct timer_list retrans_timer;
>>
>> Regarding the new "extwin" structure members, have you considered changing
>> the existing tx_win and remote_tx_win members to __u16 and using them with
>> both standard and extended window sizes?
>>
>> The spec also requires that both directions of the link use the same type of
>> control field (standard or extended). After L2CAP configuration is done,
>> all the information required for the transmit window is the control field
>> type, tx_win, and remote_tx_win. The control field would be set to
>> 'extended' if a successful configuration response is sent or received for
>> the extended window size option.
>
> Yes, we do, please look into the patch, we use __u16 for extwin_size &
> rem_extwin_size
> + __u8 extwin_enable;
> + __u16 extwin_size;
> + __u8 rem_extwin_enable;
> + __u16 rem_extwin_size;
>
> You are exactly right, in our implementation, choosing standard or
> extended window, it depends whether successful configuration
> response contain the extended window size option.
Haijin -
Thank you for your explanation. I was trying to suggest something
different - sorry I did not explain myself well.
I think that extwin_enable, extwin_size, rem_extwin_enable, and
rem_extwin_size are not needed in l2cap_pinfo. Instead, I suggest
this:
@@ -349,15 +349,17 @@ struct l2cap_pinfo {
__u8 ident;
- __u8 tx_win;
+ __u16 tx_win;
__u8 max_tx;
- __u8 remote_tx_win;
+ __u16 remote_tx_win;
__u8 remote_max_tx;
__u16 retrans_timeout;
__u16 monitor_timeout;
__u16 remote_mps;
__u16 mps;
+ __u8 extended_control;
+
__le16 sport;
struct timer_list retrans_timer;
Here, tx_win and remote_tx_win are always used to record the window
size, whether extended or standard. extended_control is used to
specify standard or extended control fields.
Regards,
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 16/22] Store new configuration values in l2cap_pinfo
2010-09-03 20:58 ` Mat Martineau
@ 2010-09-03 21:04 ` Gustavo F. Padovan
2010-09-03 21:46 ` Mat Martineau
0 siblings, 1 reply; 8+ messages in thread
From: Gustavo F. Padovan @ 2010-09-03 21:04 UTC (permalink / raw)
To: Mat Martineau; +Cc: haijun liu, haijun.liu, linux-bluetooth
Hi Mat,
* Mat Martineau <mathewm@codeaurora.org> [2010-09-03 13:58:46 -0700]:
>
> On Fri, 3 Sep 2010, haijun liu wrote:
>
> > On Fri, Sep 3, 2010 at 7:02 AM, Mat Martineau <mathewm@codeaurora.org> wrote:
> >>
> >> On Thu, 26 Aug 2010, haijun liu wrote:
> >>
> >>> From d093975dde6d85c824a5aaac943d676100810010 Mon Sep 17 00:00:00 2001
> >>> From: haijun.liu <haijun.liu@atheros.com>
> >>> Date: Mon, 23 Aug 2010 00:09:56 +0800
> >>> Subject: [PATCH 16/22] Store new configuration values in l2cap_pinfo.
> >>>
> >>> ---
> >>> include/net/bluetooth/l2cap.h | 9 +++++++++
> >>> 1 files changed, 9 insertions(+), 0 deletions(-)
> >>>
> >>> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> >>> index 2d864d4..f2dd65d 100644
> >>> --- a/include/net/bluetooth/l2cap.h
> >>> +++ b/include/net/bluetooth/l2cap.h
> >>> @@ -406,6 +406,15 @@ struct l2cap_pinfo {
> >>> __u16 remote_mps;
> >>> __u16 mps;
> >>>
> >>> + __u8 ext_flowspec_enable;
> >>> + struct ext_flow_spec loc_efs;
> >>> + struct ext_flow_spec rem_efs;
> >>> +
> >>> + __u8 extwin_enable;
> >>> + __u16 extwin_size;
> >>> + __u8 rem_extwin_enable;
> >>> + __u16 rem_extwin_size;
> >>> +
> >>> __le16 sport;
> >>>
> >>> struct timer_list retrans_timer;
> >>
> >> Regarding the new "extwin" structure members, have you considered changing
> >> the existing tx_win and remote_tx_win members to __u16 and using them with
> >> both standard and extended window sizes?
> >>
> >> The spec also requires that both directions of the link use the same type of
> >> control field (standard or extended). After L2CAP configuration is done,
> >> all the information required for the transmit window is the control field
> >> type, tx_win, and remote_tx_win. The control field would be set to
> >> 'extended' if a successful configuration response is sent or received for
> >> the extended window size option.
> >
> > Yes, we do, please look into the patch, we use __u16 for extwin_size &
> > rem_extwin_size
> > + __u8 extwin_enable;
> > + __u16 extwin_size;
> > + __u8 rem_extwin_enable;
> > + __u16 rem_extwin_size;
> >
> > You are exactly right, in our implementation, choosing standard or
> > extended window, it depends whether successful configuration
> > response contain the extended window size option.
>
> Haijin -
>
> Thank you for your explanation. I was trying to suggest something
> different - sorry I did not explain myself well.
>
> I think that extwin_enable, extwin_size, rem_extwin_enable, and
> rem_extwin_size are not needed in l2cap_pinfo. Instead, I suggest
> this:
>
> @@ -349,15 +349,17 @@ struct l2cap_pinfo {
>
> __u8 ident;
>
> - __u8 tx_win;
> + __u16 tx_win;
> __u8 max_tx;
> - __u8 remote_tx_win;
> + __u16 remote_tx_win;
> __u8 remote_max_tx;
> __u16 retrans_timeout;
> __u16 monitor_timeout;
> __u16 remote_mps;
> __u16 mps;
>
> + __u8 extended_control;
> +
> __le16 sport;
>
> struct timer_list retrans_timer;
>
> Here, tx_win and remote_tx_win are always used to record the window
> size, whether extended or standard. extended_control is used to
> specify standard or extended control fields.
Then you don't need extended_control here, a bit in the conf_state field
should be enough.
--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 16/22] Store new configuration values in l2cap_pinfo
2010-09-03 21:04 ` Gustavo F. Padovan
@ 2010-09-03 21:46 ` Mat Martineau
2010-09-03 21:49 ` Gustavo F. Padovan
0 siblings, 1 reply; 8+ messages in thread
From: Mat Martineau @ 2010-09-03 21:46 UTC (permalink / raw)
To: Gustavo F. Padovan; +Cc: haijun liu, haijun.liu, linux-bluetooth
Gustavo -
On Fri, 3 Sep 2010, Gustavo F. Padovan wrote:
> Hi Mat,
>
> * Mat Martineau <mathewm@codeaurora.org> [2010-09-03 13:58:46 -0700]:
>
>>
>> On Fri, 3 Sep 2010, haijun liu wrote:
>>
>>> On Fri, Sep 3, 2010 at 7:02 AM, Mat Martineau <mathewm@codeaurora.org> wrote:
>>>>
>>>> On Thu, 26 Aug 2010, haijun liu wrote:
>>>>
>>>>> From d093975dde6d85c824a5aaac943d676100810010 Mon Sep 17 00:00:00 2001
>>>>> From: haijun.liu <haijun.liu@atheros.com>
>>>>> Date: Mon, 23 Aug 2010 00:09:56 +0800
>>>>> Subject: [PATCH 16/22] Store new configuration values in l2cap_pinfo.
>>>>>
>>>>> ---
>>>>> include/net/bluetooth/l2cap.h | 9 +++++++++
>>>>> 1 files changed, 9 insertions(+), 0 deletions(-)
>>>>>
>>>>> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
>>>>> index 2d864d4..f2dd65d 100644
>>>>> --- a/include/net/bluetooth/l2cap.h
>>>>> +++ b/include/net/bluetooth/l2cap.h
>>>>> @@ -406,6 +406,15 @@ struct l2cap_pinfo {
>>>>> __u16 remote_mps;
>>>>> __u16 mps;
>>>>>
>>>>> + __u8 ext_flowspec_enable;
>>>>> + struct ext_flow_spec loc_efs;
>>>>> + struct ext_flow_spec rem_efs;
>>>>> +
>>>>> + __u8 extwin_enable;
>>>>> + __u16 extwin_size;
>>>>> + __u8 rem_extwin_enable;
>>>>> + __u16 rem_extwin_size;
>>>>> +
>>>>> __le16 sport;
>>>>>
>>>>> struct timer_list retrans_timer;
>>>>
>>>> Regarding the new "extwin" structure members, have you considered changing
>>>> the existing tx_win and remote_tx_win members to __u16 and using them with
>>>> both standard and extended window sizes?
>>>>
>>>> The spec also requires that both directions of the link use the same type of
>>>> control field (standard or extended). After L2CAP configuration is done,
>>>> all the information required for the transmit window is the control field
>>>> type, tx_win, and remote_tx_win. The control field would be set to
>>>> 'extended' if a successful configuration response is sent or received for
>>>> the extended window size option.
>>>
>>> Yes, we do, please look into the patch, we use __u16 for extwin_size &
>>> rem_extwin_size
>>> + __u8 extwin_enable;
>>> + __u16 extwin_size;
>>> + __u8 rem_extwin_enable;
>>> + __u16 rem_extwin_size;
>>>
>>> You are exactly right, in our implementation, choosing standard or
>>> extended window, it depends whether successful configuration
>>> response contain the extended window size option.
>>
>> Haijin -
>>
>> Thank you for your explanation. I was trying to suggest something
>> different - sorry I did not explain myself well.
>>
>> I think that extwin_enable, extwin_size, rem_extwin_enable, and
>> rem_extwin_size are not needed in l2cap_pinfo. Instead, I suggest
>> this:
>>
>> @@ -349,15 +349,17 @@ struct l2cap_pinfo {
>>
>> __u8 ident;
>>
>> - __u8 tx_win;
>> + __u16 tx_win;
>> __u8 max_tx;
>> - __u8 remote_tx_win;
>> + __u16 remote_tx_win;
>> __u8 remote_max_tx;
>> __u16 retrans_timeout;
>> __u16 monitor_timeout;
>> __u16 remote_mps;
>> __u16 mps;
>>
>> + __u8 extended_control;
>> +
>> __le16 sport;
>>
>> struct timer_list retrans_timer;
>>
>> Here, tx_win and remote_tx_win are always used to record the window
>> size, whether extended or standard. extended_control is used to
>> specify standard or extended control fields.
>
> Then you don't need extended_control here, a bit in the conf_state field
> should be enough.
It looks like conf_state is not currently used once the connection is
made. The extended_control information is accessed when parsing
headers in every incoming PDU and generating every outgoing PDU.
Usage is a lot like l2cap_pinfo.fcs
If you still want to squeeze this extended control bit in an existing
l2cap_pinfo member, would conn_state be better?
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 16/22] Store new configuration values in l2cap_pinfo
2010-09-03 21:46 ` Mat Martineau
@ 2010-09-03 21:49 ` Gustavo F. Padovan
0 siblings, 0 replies; 8+ messages in thread
From: Gustavo F. Padovan @ 2010-09-03 21:49 UTC (permalink / raw)
To: Mat Martineau; +Cc: haijun liu, haijun.liu, linux-bluetooth
Hi Mat,
* Mat Martineau <mathewm@codeaurora.org> [2010-09-03 14:46:58 -0700]:
>
> Gustavo -
>
> On Fri, 3 Sep 2010, Gustavo F. Padovan wrote:
>
> > Hi Mat,
> >
> > * Mat Martineau <mathewm@codeaurora.org> [2010-09-03 13:58:46 -0700]:
> >
> >>
> >> On Fri, 3 Sep 2010, haijun liu wrote:
> >>
> >>> On Fri, Sep 3, 2010 at 7:02 AM, Mat Martineau <mathewm@codeaurora.org> wrote:
> >>>>
> >>>> On Thu, 26 Aug 2010, haijun liu wrote:
> >>>>
> >>>>> From d093975dde6d85c824a5aaac943d676100810010 Mon Sep 17 00:00:00 2001
> >>>>> From: haijun.liu <haijun.liu@atheros.com>
> >>>>> Date: Mon, 23 Aug 2010 00:09:56 +0800
> >>>>> Subject: [PATCH 16/22] Store new configuration values in l2cap_pinfo.
> >>>>>
> >>>>> ---
> >>>>> include/net/bluetooth/l2cap.h | 9 +++++++++
> >>>>> 1 files changed, 9 insertions(+), 0 deletions(-)
> >>>>>
> >>>>> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> >>>>> index 2d864d4..f2dd65d 100644
> >>>>> --- a/include/net/bluetooth/l2cap.h
> >>>>> +++ b/include/net/bluetooth/l2cap.h
> >>>>> @@ -406,6 +406,15 @@ struct l2cap_pinfo {
> >>>>> __u16 remote_mps;
> >>>>> __u16 mps;
> >>>>>
> >>>>> + __u8 ext_flowspec_enable;
> >>>>> + struct ext_flow_spec loc_efs;
> >>>>> + struct ext_flow_spec rem_efs;
> >>>>> +
> >>>>> + __u8 extwin_enable;
> >>>>> + __u16 extwin_size;
> >>>>> + __u8 rem_extwin_enable;
> >>>>> + __u16 rem_extwin_size;
> >>>>> +
> >>>>> __le16 sport;
> >>>>>
> >>>>> struct timer_list retrans_timer;
> >>>>
> >>>> Regarding the new "extwin" structure members, have you considered changing
> >>>> the existing tx_win and remote_tx_win members to __u16 and using them with
> >>>> both standard and extended window sizes?
> >>>>
> >>>> The spec also requires that both directions of the link use the same type of
> >>>> control field (standard or extended). After L2CAP configuration is done,
> >>>> all the information required for the transmit window is the control field
> >>>> type, tx_win, and remote_tx_win. The control field would be set to
> >>>> 'extended' if a successful configuration response is sent or received for
> >>>> the extended window size option.
> >>>
> >>> Yes, we do, please look into the patch, we use __u16 for extwin_size &
> >>> rem_extwin_size
> >>> + __u8 extwin_enable;
> >>> + __u16 extwin_size;
> >>> + __u8 rem_extwin_enable;
> >>> + __u16 rem_extwin_size;
> >>>
> >>> You are exactly right, in our implementation, choosing standard or
> >>> extended window, it depends whether successful configuration
> >>> response contain the extended window size option.
> >>
> >> Haijin -
> >>
> >> Thank you for your explanation. I was trying to suggest something
> >> different - sorry I did not explain myself well.
> >>
> >> I think that extwin_enable, extwin_size, rem_extwin_enable, and
> >> rem_extwin_size are not needed in l2cap_pinfo. Instead, I suggest
> >> this:
> >>
> >> @@ -349,15 +349,17 @@ struct l2cap_pinfo {
> >>
> >> __u8 ident;
> >>
> >> - __u8 tx_win;
> >> + __u16 tx_win;
> >> __u8 max_tx;
> >> - __u8 remote_tx_win;
> >> + __u16 remote_tx_win;
> >> __u8 remote_max_tx;
> >> __u16 retrans_timeout;
> >> __u16 monitor_timeout;
> >> __u16 remote_mps;
> >> __u16 mps;
> >>
> >> + __u8 extended_control;
> >> +
> >> __le16 sport;
> >>
> >> struct timer_list retrans_timer;
> >>
> >> Here, tx_win and remote_tx_win are always used to record the window
> >> size, whether extended or standard. extended_control is used to
> >> specify standard or extended control fields.
> >
> > Then you don't need extended_control here, a bit in the conf_state field
> > should be enough.
>
> It looks like conf_state is not currently used once the connection is
> made. The extended_control information is accessed when parsing
> headers in every incoming PDU and generating every outgoing PDU.
> Usage is a lot like l2cap_pinfo.fcs
>
> If you still want to squeeze this extended control bit in an existing
> l2cap_pinfo member, would conn_state be better?
Yes, this one is better.
--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2010-09-03 21:49 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-08-25 22:37 [PATCH 16/22] Store new configuration values in l2cap_pinfo haijun liu
2010-08-26 1:21 ` Gustavo F. Padovan
2010-09-02 23:02 ` Mat Martineau
2010-09-03 1:30 ` haijun liu
2010-09-03 20:58 ` Mat Martineau
2010-09-03 21:04 ` Gustavo F. Padovan
2010-09-03 21:46 ` Mat Martineau
2010-09-03 21:49 ` Gustavo F. Padovan
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).