linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/4] Bluetooth: Add sockopt configuration for txWindow on L2CAP
@ 2010-03-17 18:53 Gustavo F. Padovan
  2010-03-17 18:53 ` [PATCH 2/4] Bluetooth: Change acknowledgement to use the value of txWindow Gustavo F. Padovan
  0 siblings, 1 reply; 8+ messages in thread
From: Gustavo F. Padovan @ 2010-03-17 18:53 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: marcel, gustavo, mathewm

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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/4] Bluetooth: Change acknowledgement to use the value of txWindow
  2010-03-17 18:53 [PATCH 1/4] Bluetooth: Add sockopt configuration for txWindow on L2CAP Gustavo F. Padovan
@ 2010-03-17 18:53 ` Gustavo F. Padovan
  2010-03-17 18:53   ` [PATCH 3/4] Bluetooth: Add module parameter for txWindow size on L2CAP Gustavo F. Padovan
  2010-03-17 21:47   ` [PATCH 2/4] Bluetooth: Change acknowledgement to use the value of txWindow Mike Tsai
  0 siblings, 2 replies; 8+ messages in thread
From: Gustavo F. Padovan @ 2010-03-17 18:53 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: marcel, gustavo, mathewm

Now that we can set the txWindow we need to change the acknowledgement
procedure to ack after each (pi->txWindow/6 + 1). The plus 1 is to avoid
the zero value.
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 |    3 +--
 net/bluetooth/l2cap.c         |    9 +++++----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index c7bf676..9358b9e 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;
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 6679418..959be0f 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->tx_win/6) + 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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/4] Bluetooth: Add module parameter for txWindow size on L2CAP
  2010-03-17 18:53 ` [PATCH 2/4] Bluetooth: Change acknowledgement to use the value of txWindow Gustavo F. Padovan
@ 2010-03-17 18:53   ` Gustavo F. Padovan
  2010-03-17 18:53     ` [PATCH 4/4] Bluetooth: Enable option to configure Max Transmission value via sockopt Gustavo F. Padovan
  2010-03-17 21:47   ` [PATCH 2/4] Bluetooth: Change acknowledgement to use the value of txWindow Mike Tsai
  1 sibling, 1 reply; 8+ messages in thread
From: Gustavo F. Padovan @ 2010-03-17 18:53 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: marcel, gustavo, mathewm

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 959be0f..11ea353 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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 4/4] Bluetooth: Enable option to configure Max Transmission value via sockopt
  2010-03-17 18:53   ` [PATCH 3/4] Bluetooth: Add module parameter for txWindow size on L2CAP Gustavo F. Padovan
@ 2010-03-17 18:53     ` Gustavo F. Padovan
  0 siblings, 0 replies; 8+ messages in thread
From: Gustavo F. Padovan @ 2010-03-17 18:53 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: marcel, gustavo, mathewm

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 9358b9e..08c5bb0 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 11ea353..cfa0b80 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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* RE: [PATCH 2/4] Bluetooth: Change acknowledgement to use the value of txWindow
  2010-03-17 18:53 ` [PATCH 2/4] Bluetooth: Change acknowledgement to use the value of txWindow Gustavo F. Padovan
  2010-03-17 18:53   ` [PATCH 3/4] Bluetooth: Add module parameter for txWindow size on L2CAP Gustavo F. Padovan
@ 2010-03-17 21:47   ` Mike Tsai
  2010-03-22  3:33     ` Gustavo F. Padovan
  1 sibling, 1 reply; 8+ messages in thread
From: Mike Tsai @ 2010-03-17 21:47 UTC (permalink / raw)
  To: Gustavo F. Padovan, linux-bluetooth@vger.kernel.org
  Cc: marcel@holtmann.org, mathewm@codeaurora.org

Just out of curiosity, where does this (pi->txWindow/6 + 1) come from?

Thanks,

Mike


-----Original Message-----
From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-owner@v=
ger.kernel.org] On Behalf Of Gustavo F. Padovan
Sent: Wednesday, March 17, 2010 11:54 AM
To: linux-bluetooth@vger.kernel.org
Cc: marcel@holtmann.org; gustavo@padovan.org; mathewm@codeaurora.org
Subject: [PATCH 2/4] Bluetooth: Change acknowledgement to use the value of =
txWindow

Now that we can set the txWindow we need to change the acknowledgement
procedure to ack after each (pi->txWindow/6 + 1). The plus 1 is to avoid
the zero value.
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 |    3 +--
 net/bluetooth/l2cap.c         |    9 +++++----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index c7bf676..9358b9e 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;
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 6679418..959be0f 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 =3D 0;
 	l2cap_pi(sk)->unacked_frames =3D 0;
 	l2cap_pi(sk)->buffer_seq =3D 0;
-	l2cap_pi(sk)->num_to_ack =3D 0;
+	l2cap_pi(sk)->num_acked =3D 0;
 	l2cap_pi(sk)->frames_sent =3D 0;
=20
 	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 =3D=3D L2CAP_CONF_SUCCESS) {
 		switch (rfc.mode) {
 		case L2CAP_MODE_ERTM:
-			pi->remote_tx_win   =3D rfc.txwin_size;
+			pi->tx_win   =3D rfc.txwin_size;
 			pi->retrans_timeout =3D rfc.retrans_timeout;
 			pi->monitor_timeout =3D rfc.monitor_timeout;
 			pi->max_pdu_size    =3D le16_to_cpu(rfc.max_pdu_size);
@@ -3408,6 +3408,7 @@ static inline int l2cap_data_channel_iframe(struct so=
ck *sk, u16 rx_control, str
 	u8 tx_seq =3D __get_txseq(rx_control);
 	u8 req_seq =3D __get_reqseq(rx_control);
 	u8 sar =3D rx_control >> L2CAP_CTRL_SAR_SHIFT;
+	int num_to_ack =3D (pi->tx_win/6) + 1;
 	int err =3D 0;
=20
 	BT_DBG("sk %p rx_control 0x%4.4x len %d", sk, rx_control, skb->len);
@@ -3501,8 +3502,8 @@ expected:
=20
 	__mod_ack_timer();
=20
-	pi->num_to_ack =3D (pi->num_to_ack + 1) % L2CAP_DEFAULT_NUM_TO_ACK;
-	if (pi->num_to_ack =3D=3D L2CAP_DEFAULT_NUM_TO_ACK - 1)
+	pi->num_acked =3D (pi->num_acked + 1) % num_to_ack;
+	if (pi->num_acked =3D=3D num_to_ack - 1)
 		l2cap_send_ack(pi);
=20
 	return 0;
--=20
1.6.4.4

--
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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/4] Bluetooth: Change acknowledgement to use the value of txWindow
  2010-03-17 21:47   ` [PATCH 2/4] Bluetooth: Change acknowledgement to use the value of txWindow Mike Tsai
@ 2010-03-22  3:33     ` Gustavo F. Padovan
  2010-03-23 22:10       ` Mike Tsai
  0 siblings, 1 reply; 8+ messages in thread
From: Gustavo F. Padovan @ 2010-03-22  3:33 UTC (permalink / raw)
  To: Mike Tsai
  Cc: linux-bluetooth@vger.kernel.org, marcel@holtmann.org,
	mathewm@codeaurora.org

Hi Mike,

* Mike Tsai <Mike.Tsai@Atheros.com> [2010-03-17 14:47:49 -0700]:

> Just out of curiosity, where does this (pi->txWindow/6 + 1) come from?

L2CAP spec says that we don't need to ack every packet received, so I
have choose this number for optimization to not send an ack for each
packet received.
We can do a study and choose the better value for this, but it's better
to have the Extended Tx Window Implemented.

> 
> Thanks,
> 
> Mike
> 
> 
> -----Original Message-----
> From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-owner@vger.kernel.org] On Behalf Of Gustavo F. Padovan
> Sent: Wednesday, March 17, 2010 11:54 AM
> To: linux-bluetooth@vger.kernel.org
> Cc: marcel@holtmann.org; gustavo@padovan.org; mathewm@codeaurora.org
> Subject: [PATCH 2/4] Bluetooth: Change acknowledgement to use the value of txWindow
> 
> Now that we can set the txWindow we need to change the acknowledgement
> procedure to ack after each (pi->txWindow/6 + 1). The plus 1 is to avoid
> the zero value.
> 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 |    3 +--
>  net/bluetooth/l2cap.c         |    9 +++++----
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index c7bf676..9358b9e 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;
> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> index 6679418..959be0f 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->tx_win/6) + 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
> 
> --
> 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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH 2/4] Bluetooth: Change acknowledgement to use the value of txWindow
  2010-03-22  3:33     ` Gustavo F. Padovan
@ 2010-03-23 22:10       ` Mike Tsai
  2010-03-23 22:35         ` Gustavo F. Padovan
  0 siblings, 1 reply; 8+ messages in thread
From: Mike Tsai @ 2010-03-23 22:10 UTC (permalink / raw)
  To: Gustavo F. Padovan
  Cc: linux-bluetooth@vger.kernel.org, marcel@holtmann.org,
	mathewm@codeaurora.org

Hi Gustavo,

	Thanks for your reply. It seems to be a reasonable approach. 

	There are no BT 3.0 products on the market that support extended window size yet (up to 16000+ TxWin?), and I don't think it will be supported in the near future either,

Best Regards,

Mike


-----Original Message-----
From: Gustavo F. Padovan [mailto:gfpadovan@gmail.com] On Behalf Of Gustavo F. Padovan
Sent: Sunday, March 21, 2010 8:34 PM
To: Mike Tsai
Cc: linux-bluetooth@vger.kernel.org; marcel@holtmann.org; mathewm@codeaurora.org
Subject: Re: [PATCH 2/4] Bluetooth: Change acknowledgement to use the value of txWindow

Hi Mike,

* Mike Tsai <Mike.Tsai@Atheros.com> [2010-03-17 14:47:49 -0700]:

> Just out of curiosity, where does this (pi->txWindow/6 + 1) come from?

L2CAP spec says that we don't need to ack every packet received, so I
have choose this number for optimization to not send an ack for each
packet received.
We can do a study and choose the better value for this, but it's better
to have the Extended Tx Window Implemented.

> 
> Thanks,
> 
> Mike
> 
> 
> -----Original Message-----
> From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-owner@vger.kernel.org] On Behalf Of Gustavo F. Padovan
> Sent: Wednesday, March 17, 2010 11:54 AM
> To: linux-bluetooth@vger.kernel.org
> Cc: marcel@holtmann.org; gustavo@padovan.org; mathewm@codeaurora.org
> Subject: [PATCH 2/4] Bluetooth: Change acknowledgement to use the value of txWindow
> 
> Now that we can set the txWindow we need to change the acknowledgement
> procedure to ack after each (pi->txWindow/6 + 1). The plus 1 is to avoid
> the zero value.
> 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 |    3 +--
>  net/bluetooth/l2cap.c         |    9 +++++----
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index c7bf676..9358b9e 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;
> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> index 6679418..959be0f 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->tx_win/6) + 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
> 
> --
> 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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/4] Bluetooth: Change acknowledgement to use the value of txWindow
  2010-03-23 22:10       ` Mike Tsai
@ 2010-03-23 22:35         ` Gustavo F. Padovan
  0 siblings, 0 replies; 8+ messages in thread
From: Gustavo F. Padovan @ 2010-03-23 22:35 UTC (permalink / raw)
  To: Mike Tsai; +Cc: linux-bluetooth@vger.kernel.org


Hi Mike,

Please do not top post on this mailing list.


* Mike Tsai <Mike.Tsai@Atheros.com> [2010-03-23 15:10:29 -0700]:

> Hi Gustavo,
> 
> 	Thanks for your reply. It seems to be a reasonable approach. 
> 
> 	There are no BT 3.0 products on the market that support extended window size yet (up to 16000+ TxWin?), and I don't think it will be supported in the near future either,

AFAIK we don't need a 3.0 hardware to have Extended TxWindow working.
It is an L2CAP protocol feature. The hardware lower layers has no idea
about the L2CAP txWin size.

> 
> Best Regards,
> 
> Mike
> 
> 
> -----Original Message-----
> From: Gustavo F. Padovan [mailto:gfpadovan@gmail.com] On Behalf Of Gustavo F. Padovan
> Sent: Sunday, March 21, 2010 8:34 PM
> To: Mike Tsai
> Cc: linux-bluetooth@vger.kernel.org; marcel@holtmann.org; mathewm@codeaurora.org
> Subject: Re: [PATCH 2/4] Bluetooth: Change acknowledgement to use the value of txWindow
> 
> Hi Mike,
> 
> * Mike Tsai <Mike.Tsai@Atheros.com> [2010-03-17 14:47:49 -0700]:
> 
> > Just out of curiosity, where does this (pi->txWindow/6 + 1) come from?
> 
> L2CAP spec says that we don't need to ack every packet received, so I
> have choose this number for optimization to not send an ack for each
> packet received.
> We can do a study and choose the better value for this, but it's better
> to have the Extended Tx Window Implemented.
> 
> > 
> > Thanks,
> > 
> > Mike
> > 
> > 
> > -----Original Message-----
> > From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-owner@vger.kernel.org] On Behalf Of Gustavo F. Padovan
> > Sent: Wednesday, March 17, 2010 11:54 AM
> > To: linux-bluetooth@vger.kernel.org
> > Cc: marcel@holtmann.org; gustavo@padovan.org; mathewm@codeaurora.org
> > Subject: [PATCH 2/4] Bluetooth: Change acknowledgement to use the value of txWindow
> > 
> > Now that we can set the txWindow we need to change the acknowledgement
> > procedure to ack after each (pi->txWindow/6 + 1). The plus 1 is to avoid
> > the zero value.
> > 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 |    3 +--
> >  net/bluetooth/l2cap.c         |    9 +++++----
> >  2 files changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> > index c7bf676..9358b9e 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;
> > diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> > index 6679418..959be0f 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->tx_win/6) + 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
> > 
> > --
> > 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
> --
> 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

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2010-03-23 22:35 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-03-17 18:53 [PATCH 1/4] Bluetooth: Add sockopt configuration for txWindow on L2CAP Gustavo F. Padovan
2010-03-17 18:53 ` [PATCH 2/4] Bluetooth: Change acknowledgement to use the value of txWindow Gustavo F. Padovan
2010-03-17 18:53   ` [PATCH 3/4] Bluetooth: Add module parameter for txWindow size on L2CAP Gustavo F. Padovan
2010-03-17 18:53     ` [PATCH 4/4] Bluetooth: Enable option to configure Max Transmission value via sockopt Gustavo F. Padovan
2010-03-17 21:47   ` [PATCH 2/4] Bluetooth: Change acknowledgement to use the value of txWindow Mike Tsai
2010-03-22  3:33     ` Gustavo F. Padovan
2010-03-23 22:10       ` Mike Tsai
2010-03-23 22:35         ` 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).