[PATCH 1/4] Bluetooth: Add sockopt configuration for txWindow on L2CAP

[PATCH 1/4] Bluetooth: Add sockopt configuration for txWindow on L2CAP

[Gustavo F. Padovan]

Now can set/get Transmission Window size via sockopt.

Signed-off-by: Gustavo F. Padovan <gustavo@padovan.org>
---
 include/net/bluetooth/l2cap.h |    2 ++
 net/bluetooth/l2cap.c         |    7 ++++++-
 2 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 48f10f4..c7bf676 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -56,6 +56,7 @@ struct l2cap_options {
 	__u16 flush_to;
 	__u8  mode;
 	__u8  fcs;
+	__u8  txwin_size;
 };
 
 #define L2CAP_CONNINFO	0x02
@@ -339,6 +340,7 @@ struct l2cap_pinfo {
 
 	__u8		ident;
 
+	__u8		tx_win;
 	__u8		remote_tx_win;
 	__u8		remote_max_tx;
 	__u16		retrans_timeout;
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 930f987..6679418 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -780,6 +780,7 @@ static void l2cap_sock_init(struct sock *sk, struct sock *parent)
 		pi->omtu = l2cap_pi(parent)->omtu;
 		pi->mode = l2cap_pi(parent)->mode;
 		pi->fcs  = l2cap_pi(parent)->fcs;
+		pi->tx_win = l2cap_pi(parent)->tx_win;
 		pi->sec_level = l2cap_pi(parent)->sec_level;
 		pi->role_switch = l2cap_pi(parent)->role_switch;
 		pi->force_reliable = l2cap_pi(parent)->force_reliable;
@@ -788,6 +789,7 @@ static void l2cap_sock_init(struct sock *sk, struct sock *parent)
 		pi->omtu = 0;
 		pi->mode = L2CAP_MODE_BASIC;
 		pi->fcs  = L2CAP_FCS_CRC16;
+		pi->tx_win = L2CAP_DEFAULT_TX_WINDOW;
 		pi->sec_level = BT_SECURITY_LOW;
 		pi->role_switch = 0;
 		pi->force_reliable = 0;
@@ -1776,6 +1778,7 @@ static int l2cap_sock_setsockopt_old(struct socket *sock, int optname, char __us
 		opts.flush_to = l2cap_pi(sk)->flush_to;
 		opts.mode     = l2cap_pi(sk)->mode;
 		opts.fcs      = l2cap_pi(sk)->fcs;
+		opts.txwin_size = l2cap_pi(sk)->tx_win;
 
 		len = min_t(unsigned int, sizeof(opts), optlen);
 		if (copy_from_user((char *) &opts, optval, len)) {
@@ -1787,6 +1790,7 @@ static int l2cap_sock_setsockopt_old(struct socket *sock, int optname, char __us
 		l2cap_pi(sk)->omtu = opts.omtu;
 		l2cap_pi(sk)->mode = opts.mode;
 		l2cap_pi(sk)->fcs  = opts.fcs;
+		l2cap_pi(sk)->tx_win = opts.txwin_size;
 		break;
 
 	case L2CAP_LM:
@@ -1901,6 +1905,7 @@ static int l2cap_sock_getsockopt_old(struct socket *sock, int optname, char __us
 		opts.flush_to = l2cap_pi(sk)->flush_to;
 		opts.mode     = l2cap_pi(sk)->mode;
 		opts.fcs      = l2cap_pi(sk)->fcs;
+		opts.txwin_size = l2cap_pi(sk)->tx_win;
 
 		len = min_t(unsigned int, len, sizeof(opts));
 		if (copy_to_user(optval, (char *) &opts, len))
@@ -2318,7 +2323,7 @@ done:
 
 	case L2CAP_MODE_ERTM:
 		rfc.mode            = L2CAP_MODE_ERTM;
-		rfc.txwin_size      = L2CAP_DEFAULT_TX_WINDOW;
+		rfc.txwin_size      = pi->tx_win;
 		rfc.max_transmit    = max_transmit;
 		rfc.retrans_timeout = 0;
 		rfc.monitor_timeout = 0;
-- 
1.6.4.4

[PATCH 2/4] Bluetooth: Fix remote and local txWindow roles on L2CAP

[Gustavo F. Padovan]

L2CAP was misusing the values of txWindow. Now the remote txWindow is used
on the acknowledgement of I-frames and the local txWindow on the sending
of I-frames. It also renames pi->num_to_ack to a better name:
pi->num_acked.

Signed-off-by: Gustavo F. Padovan <gustavo@padovan.org>
---
 include/net/bluetooth/l2cap.h |    5 ++---
 net/bluetooth/l2cap.c         |    9 +++++----
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index c7bf676..f1eca05 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -30,7 +30,6 @@
 #define L2CAP_DEFAULT_MIN_MTU		48
 #define L2CAP_DEFAULT_FLUSH_TO		0xffff
 #define L2CAP_DEFAULT_TX_WINDOW		63
-#define L2CAP_DEFAULT_NUM_TO_ACK        (L2CAP_DEFAULT_TX_WINDOW/5)
 #define L2CAP_DEFAULT_MAX_TX		3
 #define L2CAP_DEFAULT_RETRANS_TO	1000    /* 1 second */
 #define L2CAP_DEFAULT_MONITOR_TO	12000   /* 12 seconds */
@@ -333,7 +332,7 @@ struct l2cap_pinfo {
 	__u8		frames_sent;
 	__u8		unacked_frames;
 	__u8		retry_count;
-	__u8		num_to_ack;
+	__u8		num_acked;
 	__u16		sdu_len;
 	__u16		partial_sdu_len;
 	struct sk_buff	*sdu;
@@ -399,7 +398,7 @@ static inline int l2cap_tx_window_full(struct sock *sk)
 	if (sub < 0)
 		sub += 64;
 
-	return (sub == pi->remote_tx_win);
+	return (sub == pi->tx_win);
 }
 
 #define __get_txseq(ctrl) ((ctrl) & L2CAP_CTRL_TXSEQ) >> 1
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 6679418..85ec8b2 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -2248,7 +2248,7 @@ static inline void l2cap_ertm_init(struct sock *sk)
 	l2cap_pi(sk)->expected_ack_seq = 0;
 	l2cap_pi(sk)->unacked_frames = 0;
 	l2cap_pi(sk)->buffer_seq = 0;
-	l2cap_pi(sk)->num_to_ack = 0;
+	l2cap_pi(sk)->num_acked = 0;
 	l2cap_pi(sk)->frames_sent = 0;
 
 	setup_timer(&l2cap_pi(sk)->retrans_timer,
@@ -2571,7 +2571,7 @@ static int l2cap_parse_conf_rsp(struct sock *sk, void *rsp, int len, void *data,
 	if (*result == L2CAP_CONF_SUCCESS) {
 		switch (rfc.mode) {
 		case L2CAP_MODE_ERTM:
-			pi->remote_tx_win   = rfc.txwin_size;
+			pi->tx_win   = rfc.txwin_size;
 			pi->retrans_timeout = rfc.retrans_timeout;
 			pi->monitor_timeout = rfc.monitor_timeout;
 			pi->max_pdu_size    = le16_to_cpu(rfc.max_pdu_size);
@@ -3408,6 +3408,7 @@ static inline int l2cap_data_channel_iframe(struct sock *sk, u16 rx_control, str
 	u8 tx_seq = __get_txseq(rx_control);
 	u8 req_seq = __get_reqseq(rx_control);
 	u8 sar = rx_control >> L2CAP_CTRL_SAR_SHIFT;
+	int num_to_ack = (pi->remote_tx_win/5) + 1;
 	int err = 0;
 
 	BT_DBG("sk %p rx_control 0x%4.4x len %d", sk, rx_control, skb->len);
@@ -3501,8 +3502,8 @@ expected:
 
 	__mod_ack_timer();
 
-	pi->num_to_ack = (pi->num_to_ack + 1) % L2CAP_DEFAULT_NUM_TO_ACK;
-	if (pi->num_to_ack == L2CAP_DEFAULT_NUM_TO_ACK - 1)
+	pi->num_acked = (pi->num_acked + 1) % num_to_ack;
+	if (pi->num_acked == num_to_ack - 1)
 		l2cap_send_ack(pi);
 
 	return 0;
-- 
1.6.4.4

[PATCH 3/4] Bluetooth: Add module parameter for txWindow size on L2CAP

[Gustavo F. Padovan]

Very useful for testing purposes.

Signed-off-by: Gustavo F. Padovan <gustavo@padovan.org>
---
 net/bluetooth/l2cap.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 85ec8b2..6f3dc6e 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -55,6 +55,7 @@
 
 static int enable_ertm = 0;
 static int max_transmit = L2CAP_DEFAULT_MAX_TX;
+static int tx_window = L2CAP_DEFAULT_TX_WINDOW;
 
 static u32 l2cap_feat_mask = L2CAP_FEAT_FIXED_CHAN;
 static u8 l2cap_fixed_chan[8] = { 0x02, };
@@ -789,7 +790,7 @@ static void l2cap_sock_init(struct sock *sk, struct sock *parent)
 		pi->omtu = 0;
 		pi->mode = L2CAP_MODE_BASIC;
 		pi->fcs  = L2CAP_FCS_CRC16;
-		pi->tx_win = L2CAP_DEFAULT_TX_WINDOW;
+		pi->tx_win = tx_window;
 		pi->sec_level = BT_SECURITY_LOW;
 		pi->role_switch = 0;
 		pi->force_reliable = 0;
@@ -4231,6 +4232,9 @@ MODULE_PARM_DESC(enable_ertm, "Enable enhanced retransmission mode");
 module_param(max_transmit, uint, 0644);
 MODULE_PARM_DESC(max_transmit, "Max transmit value (default = 3)");
 
+module_param(tx_window, uint, 0644);
+MODULE_PARM_DESC(tx_window, "Transmission window value (default = 63)");
+
 MODULE_AUTHOR("Marcel Holtmann <marcel@holtmann.org>");
 MODULE_DESCRIPTION("Bluetooth L2CAP ver " VERSION);
 MODULE_VERSION(VERSION);
-- 
1.6.4.4

[PATCH 4/4] Bluetooth: Enable option to configure Max Transmission value via sockopt

[Gustavo F. Padovan]

With the sockopt extension we can set a per-channel MaxTx value.

Signed-off-by: Gustavo F. Padovan <gustavo@padovan.org>
---
 include/net/bluetooth/l2cap.h |    2 ++
 net/bluetooth/l2cap.c         |    7 ++++++-
 2 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index f1eca05..0366a84 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -56,6 +56,7 @@ struct l2cap_options {
 	__u8  mode;
 	__u8  fcs;
 	__u8  txwin_size;
+	__u8  max_tx;
 };
 
 #define L2CAP_CONNINFO	0x02
@@ -340,6 +341,7 @@ struct l2cap_pinfo {
 	__u8		ident;
 
 	__u8		tx_win;
+	__u8		max_tx;
 	__u8		remote_tx_win;
 	__u8		remote_max_tx;
 	__u16		retrans_timeout;
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 6f3dc6e..5ff0ca0 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -781,6 +781,7 @@ static void l2cap_sock_init(struct sock *sk, struct sock *parent)
 		pi->omtu = l2cap_pi(parent)->omtu;
 		pi->mode = l2cap_pi(parent)->mode;
 		pi->fcs  = l2cap_pi(parent)->fcs;
+		pi->max_tx = l2cap_pi(parent)->max_tx;
 		pi->tx_win = l2cap_pi(parent)->tx_win;
 		pi->sec_level = l2cap_pi(parent)->sec_level;
 		pi->role_switch = l2cap_pi(parent)->role_switch;
@@ -790,6 +791,7 @@ static void l2cap_sock_init(struct sock *sk, struct sock *parent)
 		pi->omtu = 0;
 		pi->mode = L2CAP_MODE_BASIC;
 		pi->fcs  = L2CAP_FCS_CRC16;
+		pi->max_tx = max_transmit;
 		pi->tx_win = tx_window;
 		pi->sec_level = BT_SECURITY_LOW;
 		pi->role_switch = 0;
@@ -1780,6 +1782,7 @@ static int l2cap_sock_setsockopt_old(struct socket *sock, int optname, char __us
 		opts.mode     = l2cap_pi(sk)->mode;
 		opts.fcs      = l2cap_pi(sk)->fcs;
 		opts.txwin_size = l2cap_pi(sk)->tx_win;
+		opts.max_tx   = l2cap_pi(sk)->max_tx;
 
 		len = min_t(unsigned int, sizeof(opts), optlen);
 		if (copy_from_user((char *) &opts, optval, len)) {
@@ -1792,6 +1795,7 @@ static int l2cap_sock_setsockopt_old(struct socket *sock, int optname, char __us
 		l2cap_pi(sk)->mode = opts.mode;
 		l2cap_pi(sk)->fcs  = opts.fcs;
 		l2cap_pi(sk)->tx_win = opts.txwin_size;
+		l2cap_pi(sk)->max_tx = opts.max_tx;
 		break;
 
 	case L2CAP_LM:
@@ -1907,6 +1911,7 @@ static int l2cap_sock_getsockopt_old(struct socket *sock, int optname, char __us
 		opts.mode     = l2cap_pi(sk)->mode;
 		opts.fcs      = l2cap_pi(sk)->fcs;
 		opts.txwin_size = l2cap_pi(sk)->tx_win;
+		opts.max_tx   = l2cap_pi(sk)->max_tx;
 
 		len = min_t(unsigned int, len, sizeof(opts));
 		if (copy_to_user(optval, (char *) &opts, len))
@@ -2325,7 +2330,7 @@ done:
 	case L2CAP_MODE_ERTM:
 		rfc.mode            = L2CAP_MODE_ERTM;
 		rfc.txwin_size      = pi->tx_win;
-		rfc.max_transmit    = max_transmit;
+		rfc.max_transmit    = pi->max_tx;
 		rfc.retrans_timeout = 0;
 		rfc.monitor_timeout = 0;
 		rfc.max_pdu_size    = cpu_to_le16(L2CAP_DEFAULT_MAX_PDU_SIZE);
-- 
1.6.4.4

RE: [PATCH 1/4] Bluetooth: Add sockopt configuration for txWindow on L2CAP

[Mat Martineau]

Gustavo -

> -----Original Message-----
> From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-
> owner@vger.kernel.org] On Behalf Of Gustavo F. Padovan
> Sent: Monday, March 15, 2010 5:26 PM
> To: linux-bluetooth@vger.kernel.org
> Cc: marcel@holtmann.org; gustavo@padovan.org
> Subject: [PATCH 1/4] Bluetooth: Add sockopt configuration for txWindow
> on L2CAP
> 
> Now can set/get Transmission Window size via sockopt.

It would be better to use __u16 for the Tx Window size, so we can use
extended window sizes in the future.  This is important for AMP, where the
amount of data in-flight can be large enough for the extended Tx Window to
matter.

> 
> Signed-off-by: Gustavo F. Padovan <gustavo@padovan.org>
> ---
>  include/net/bluetooth/l2cap.h |    2 ++
>  net/bluetooth/l2cap.c         |    7 ++++++-
>  2 files changed, 8 insertions(+), 1 deletions(-)
> 
> diff --git a/include/net/bluetooth/l2cap.h
> b/include/net/bluetooth/l2cap.h
> index 48f10f4..c7bf676 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -56,6 +56,7 @@ struct l2cap_options {
>  	__u16 flush_to;
>  	__u8  mode;
>  	__u8  fcs;
> +	__u8  txwin_size;
>  };
> 
>  #define L2CAP_CONNINFO	0x02
> @@ -339,6 +340,7 @@ struct l2cap_pinfo {
> 
>  	__u8		ident;
> 
> +	__u8		tx_win;
>  	__u8		remote_tx_win;
>  	__u8		remote_max_tx;
>  	__u16		retrans_timeout;
> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> index 930f987..6679418 100644
> --- a/net/bluetooth/l2cap.c
> +++ b/net/bluetooth/l2cap.c
> @@ -780,6 +780,7 @@ static void l2cap_sock_init(struct sock *sk, struct
> sock *parent)
>  		pi->omtu = l2cap_pi(parent)->omtu;
>  		pi->mode = l2cap_pi(parent)->mode;
>  		pi->fcs  = l2cap_pi(parent)->fcs;
> +		pi->tx_win = l2cap_pi(parent)->tx_win;
>  		pi->sec_level = l2cap_pi(parent)->sec_level;
>  		pi->role_switch = l2cap_pi(parent)->role_switch;
>  		pi->force_reliable = l2cap_pi(parent)->force_reliable;
> @@ -788,6 +789,7 @@ static void l2cap_sock_init(struct sock *sk, struct
> sock *parent)
>  		pi->omtu = 0;
>  		pi->mode = L2CAP_MODE_BASIC;
>  		pi->fcs  = L2CAP_FCS_CRC16;
> +		pi->tx_win = L2CAP_DEFAULT_TX_WINDOW;
>  		pi->sec_level = BT_SECURITY_LOW;
>  		pi->role_switch = 0;
>  		pi->force_reliable = 0;
> @@ -1776,6 +1778,7 @@ static int l2cap_sock_setsockopt_old(struct
> socket *sock, int optname, char __us
>  		opts.flush_to = l2cap_pi(sk)->flush_to;
>  		opts.mode     = l2cap_pi(sk)->mode;
>  		opts.fcs      = l2cap_pi(sk)->fcs;
> +		opts.txwin_size = l2cap_pi(sk)->tx_win;
> 
>  		len = min_t(unsigned int, sizeof(opts), optlen);
>  		if (copy_from_user((char *) &opts, optval, len)) {
> @@ -1787,6 +1790,7 @@ static int l2cap_sock_setsockopt_old(struct
> socket *sock, int optname, char __us
>  		l2cap_pi(sk)->omtu = opts.omtu;
>  		l2cap_pi(sk)->mode = opts.mode;
>  		l2cap_pi(sk)->fcs  = opts.fcs;
> +		l2cap_pi(sk)->tx_win = opts.txwin_size;
>  		break;
> 
>  	case L2CAP_LM:
> @@ -1901,6 +1905,7 @@ static int l2cap_sock_getsockopt_old(struct
> socket *sock, int optname, char __us
>  		opts.flush_to = l2cap_pi(sk)->flush_to;
>  		opts.mode     = l2cap_pi(sk)->mode;
>  		opts.fcs      = l2cap_pi(sk)->fcs;
> +		opts.txwin_size = l2cap_pi(sk)->tx_win;
> 
>  		len = min_t(unsigned int, len, sizeof(opts));
>  		if (copy_to_user(optval, (char *) &opts, len))
> @@ -2318,7 +2323,7 @@ done:
> 
>  	case L2CAP_MODE_ERTM:
>  		rfc.mode            = L2CAP_MODE_ERTM;
> -		rfc.txwin_size      = L2CAP_DEFAULT_TX_WINDOW;
> +		rfc.txwin_size      = pi->tx_win;
>  		rfc.max_transmit    = max_transmit;
>  		rfc.retrans_timeout = 0;
>  		rfc.monitor_timeout = 0;


Mat Martineau
Qualcomm Innovation Center, Inc.,
A member of the Code Aurora Forum

Re: [PATCH 1/4] Bluetooth: Add sockopt configuration for txWindow on L2CAP

[Gustavo F. Padovan]

Hi Mat,

On Tue, Mar 16, 2010 at 12:15 PM, Mat Martineau <mathewm@codeaurora.org> wrote:
> Gustavo -
>
>> -----Original Message-----
>> From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-
>> owner@vger.kernel.org] On Behalf Of Gustavo F. Padovan
>> Sent: Monday, March 15, 2010 5:26 PM
>> To: linux-bluetooth@vger.kernel.org
>> Cc: marcel@holtmann.org; gustavo@padovan.org
>> Subject: [PATCH 1/4] Bluetooth: Add sockopt configuration for txWindow
>> on L2CAP
>>
>> Now can set/get Transmission Window size via sockopt.
>
> It would be better to use __u16 for the Tx Window size, so we can use
> extended window sizes in the future.  This is important for AMP, where the
> amount of data in-flight can be large enough for the extended Tx Window to
> matter.

It's better to update it to __u16 when we actually implement the
Extended Tx Window.

>
>>
>> Signed-off-by: Gustavo F. Padovan <gustavo@padovan.org>
>> ---
>>  include/net/bluetooth/l2cap.h |    2 ++
>>  net/bluetooth/l2cap.c         |    7 ++++++-
>>  2 files changed, 8 insertions(+), 1 deletions(-)
>>
>> diff --git a/include/net/bluetooth/l2cap.h
>> b/include/net/bluetooth/l2cap.h
>> index 48f10f4..c7bf676 100644
>> --- a/include/net/bluetooth/l2cap.h
>> +++ b/include/net/bluetooth/l2cap.h
>> @@ -56,6 +56,7 @@ struct l2cap_options {
>>       __u16 flush_to;
>>       __u8  mode;
>>       __u8  fcs;
>> +     __u8  txwin_size;
>>  };
>>
>>  #define L2CAP_CONNINFO       0x02
>> @@ -339,6 +340,7 @@ struct l2cap_pinfo {
>>
>>       __u8            ident;
>>
>> +     __u8            tx_win;
>>       __u8            remote_tx_win;
>>       __u8            remote_max_tx;
>>       __u16           retrans_timeout;
>> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
>> index 930f987..6679418 100644
>> --- a/net/bluetooth/l2cap.c
>> +++ b/net/bluetooth/l2cap.c
>> @@ -780,6 +780,7 @@ static void l2cap_sock_init(struct sock *sk, struct
>> sock *parent)
>>               pi->omtu = l2cap_pi(parent)->omtu;
>>               pi->mode = l2cap_pi(parent)->mode;
>>               pi->fcs  = l2cap_pi(parent)->fcs;
>> +             pi->tx_win = l2cap_pi(parent)->tx_win;
>>               pi->sec_level = l2cap_pi(parent)->sec_level;
>>               pi->role_switch = l2cap_pi(parent)->role_switch;
>>               pi->force_reliable = l2cap_pi(parent)->force_reliable;
>> @@ -788,6 +789,7 @@ static void l2cap_sock_init(struct sock *sk, struct
>> sock *parent)
>>               pi->omtu = 0;
>>               pi->mode = L2CAP_MODE_BASIC;
>>               pi->fcs  = L2CAP_FCS_CRC16;
>> +             pi->tx_win = L2CAP_DEFAULT_TX_WINDOW;
>>               pi->sec_level = BT_SECURITY_LOW;
>>               pi->role_switch = 0;
>>               pi->force_reliable = 0;
>> @@ -1776,6 +1778,7 @@ static int l2cap_sock_setsockopt_old(struct
>> socket *sock, int optname, char __us
>>               opts.flush_to = l2cap_pi(sk)->flush_to;
>>               opts.mode     = l2cap_pi(sk)->mode;
>>               opts.fcs      = l2cap_pi(sk)->fcs;
>> +             opts.txwin_size = l2cap_pi(sk)->tx_win;
>>
>>               len = min_t(unsigned int, sizeof(opts), optlen);
>>               if (copy_from_user((char *) &opts, optval, len)) {
>> @@ -1787,6 +1790,7 @@ static int l2cap_sock_setsockopt_old(struct
>> socket *sock, int optname, char __us
>>               l2cap_pi(sk)->omtu = opts.omtu;
>>               l2cap_pi(sk)->mode = opts.mode;
>>               l2cap_pi(sk)->fcs  = opts.fcs;
>> +             l2cap_pi(sk)->tx_win = opts.txwin_size;
>>               break;
>>
>>       case L2CAP_LM:
>> @@ -1901,6 +1905,7 @@ static int l2cap_sock_getsockopt_old(struct
>> socket *sock, int optname, char __us
>>               opts.flush_to = l2cap_pi(sk)->flush_to;
>>               opts.mode     = l2cap_pi(sk)->mode;
>>               opts.fcs      = l2cap_pi(sk)->fcs;
>> +             opts.txwin_size = l2cap_pi(sk)->tx_win;
>>
>>               len = min_t(unsigned int, len, sizeof(opts));
>>               if (copy_to_user(optval, (char *) &opts, len))
>> @@ -2318,7 +2323,7 @@ done:
>>
>>       case L2CAP_MODE_ERTM:
>>               rfc.mode            = L2CAP_MODE_ERTM;
>> -             rfc.txwin_size      = L2CAP_DEFAULT_TX_WINDOW;
>> +             rfc.txwin_size      = pi->tx_win;
>>               rfc.max_transmit    = max_transmit;
>>               rfc.retrans_timeout = 0;
>>               rfc.monitor_timeout = 0;
>
>
> Mat Martineau
> Qualcomm Innovation Center, Inc.,
> A member of the Code Aurora Forum
>
>
>



-- 
Gustavo F. Padovan
http://padovan.org

RE: [PATCH 1/4] Bluetooth: Add sockopt configuration for txWindow on L2CAP

[Mat Martineau]

> -----Original Message-----
> From: linux-bluetooth-owner@vger.kernel.org
> Sent: Tuesday, March 16, 2010 8:30 AM
> To: Mat Martineau
> Cc: linux-bluetooth@vger.kernel.org
> Subject: Re: [PATCH 1/4] Bluetooth: Add sockopt configuration for txWindow
on L2CAP
> 
> Hi Mat,
> 
> On Tue, Mar 16, 2010 at 12:15 PM, Mat Martineau
> <mathewm@codeaurora.org> wrote:
> > Gustavo -
> >
> >> -----Original Message-----
> >> From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-
> >> owner@vger.kernel.org] On Behalf Of Gustavo F. Padovan
> >> Sent: Monday, March 15, 2010 5:26 PM
> >> To: linux-bluetooth@vger.kernel.org
> >> Cc: marcel@holtmann.org; gustavo@padovan.org
> >> Subject: [PATCH 1/4] Bluetooth: Add sockopt configuration for txWindow
> >> on L2CAP
> >>
> >> Now can set/get Transmission Window size via sockopt.
> >
> > It would be better to use __u16 for the Tx Window size, so we can use
> > extended window sizes in the future.  This is important for AMP, where
the
> > amount of data in-flight can be large enough for the extended Tx Window
to
> > matter.
> 
> It's better to update it to __u16 when we actually implement the
> Extended Tx Window.

The advantage of doing it now is that the data type visible to userspace
will not ever have to change.  Applications using these sockopts will not
want to worry about this parameter being different types on different kernel
versions.  Since this value needs to be range-checked anyway (normal Tx
Window must fit in 6 bits), there isn't much downside to going with __u16
for the sockopt interface now.  Extended Tx Window support is coming in the
near term.

Another possibility is that we determine the Tx Window automatically, and
have no sockopt interface for it.  The current window of 63 could continue
to be used for BR/EDR connections, and each AMP controller type could have a
sensible extended Tx Window that considers its data rate and round trip
time.


Regards,
Mat Martineau
Qualcomm Innovation Center, Inc.,
A member of the Code Aurora Forum

Re: [PATCH 1/4] Bluetooth: Add sockopt configuration for txWindow on L2CAP

[Gustavo F. Padovan]

On Tue, Mar 16, 2010 at 1:26 PM, Mat Martineau <mathewm@codeaurora.org> wrote:
>
>
>> -----Original Message-----
>> From: linux-bluetooth-owner@vger.kernel.org
>> Sent: Tuesday, March 16, 2010 8:30 AM
>> To: Mat Martineau
>> Cc: linux-bluetooth@vger.kernel.org
>> Subject: Re: [PATCH 1/4] Bluetooth: Add sockopt configuration for txWindow
> on L2CAP
>>
>> Hi Mat,
>>
>> On Tue, Mar 16, 2010 at 12:15 PM, Mat Martineau
>> <mathewm@codeaurora.org> wrote:
>> > Gustavo -
>> >
>> >> -----Original Message-----
>> >> From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-
>> >> owner@vger.kernel.org] On Behalf Of Gustavo F. Padovan
>> >> Sent: Monday, March 15, 2010 5:26 PM
>> >> To: linux-bluetooth@vger.kernel.org
>> >> Cc: marcel@holtmann.org; gustavo@padovan.org
>> >> Subject: [PATCH 1/4] Bluetooth: Add sockopt configuration for txWindow
>> >> on L2CAP
>> >>
>> >> Now can set/get Transmission Window size via sockopt.
>> >
>> > It would be better to use __u16 for the Tx Window size, so we can use
>> > extended window sizes in the future.  This is important for AMP, where
> the
>> > amount of data in-flight can be large enough for the extended Tx Window
> to
>> > matter.
>>
>> It's better to update it to __u16 when we actually implement the
>> Extended Tx Window.
>
> The advantage of doing it now is that the data type visible to userspace
> will not ever have to change.  Applications using these sockopts will not
> want to worry about this parameter being different types on different kernel
> versions.  Since this value needs to be range-checked anyway (normal Tx
> Window must fit in 6 bits), there isn't much downside to going with __u16
> for the sockopt interface now.  Extended Tx Window support is coming in the
> near term.

So we can just avoid to apply the patch for txWindow on userspace
BlueZ until we have Extended txWindow done. That will be fine to me,
since the next will be released only on 2.6.35 kernel, and Extended
txWindow is coming soon.

>
> Another possibility is that we determine the Tx Window automatically, and
> have no sockopt interface for it.  The current window of 63 could continue
> to be used for BR/EDR connections, and each AMP controller type could have a
> sensible extended Tx Window that considers its data rate and round trip
> time.

That's not a good idea, health profiles, for example,  will want to
use a txWindow = 1. They don't need the speed, just the reliability.

>
>
> Regards,
> Mat Martineau
> Qualcomm Innovation Center, Inc.,
> A member of the Code Aurora Forum
>
>
>



-- 
Gustavo F. Padovan
http://padovan.org

RE: [PATCH 2/4] Bluetooth: Fix remote and local txWindow roles on L2CAP

[Mat Martineau]

Gustavo -

> -----Original Message-----
> From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-
> owner@vger.kernel.org] On Behalf Of Gustavo F. Padovan
> Sent: Monday, March 15, 2010 5:26 PM
> To: linux-bluetooth@vger.kernel.org
> Cc: marcel@holtmann.org; gustavo@padovan.org
> Subject: [PATCH 2/4] Bluetooth: Fix remote and local txWindow roles on
> L2CAP
> 
> L2CAP was misusing the values of txWindow. Now the remote txWindow is used
> on the acknowledgement of I-frames and the local txWindow on the sending
> of I-frames. It also renames pi->num_to_ack to a better name:
> pi->num_acked.
> 

The XMIT state table for ERTM shows that I-Frames are not sent with the
RemWindow-Full condition, so either the previous implementation was correct
or the naming of remote & local txWindow in l2cap.c is opposite of that in
the spec.


Regards,
Mat Martineau
Qualcomm Innovation Center, Inc.,
A member of the Code Aurora Forum

Re: [PATCH 2/4] Bluetooth: Fix remote and local txWindow roles on L2CAP

[Gustavo F. Padovan]

Hi Mat,

* Mat Martineau <mathewm@codeaurora.org> [2010-03-16 10:00:44 -0700]:

> Gustavo -
> 
> > -----Original Message-----
> > From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-
> > owner@vger.kernel.org] On Behalf Of Gustavo F. Padovan
> > Sent: Monday, March 15, 2010 5:26 PM
> > To: linux-bluetooth@vger.kernel.org
> > Cc: marcel@holtmann.org; gustavo@padovan.org
> > Subject: [PATCH 2/4] Bluetooth: Fix remote and local txWindow roles on
> > L2CAP
> > 
> > L2CAP was misusing the values of txWindow. Now the remote txWindow is used
> > on the acknowledgement of I-frames and the local txWindow on the sending
> > of I-frames. It also renames pi->num_to_ack to a better name:
> > pi->num_acked.
> > 
> 
> The XMIT state table for ERTM shows that I-Frames are not sent with the
> RemWindow-Full condition, so either the previous implementation was correct
> or the naming of remote & local txWindow in l2cap.c is opposite of that in
> the spec.

Looks I misundestood the spec, I'll check that.

> 
> 
> Regards,
> Mat Martineau
> Qualcomm Innovation Center, Inc.,
> A member of the Code Aurora Forum
> 
> --
> 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

-- 
Gustavo F. Padovan
http://padovan.org