[PATCH 0/6] Bluetooth: 6lowpan cleanups

[PATCH 0/6] Bluetooth: 6lowpan cleanups

[Johan Hedberg]

Hi,

Here's a set of initial cleanups for the 6lowpan code which I came up
with as I started preparing to implement the new mgmt channel.

Johan Hedberg (6):
      Bluetooth: 6lowpan: Fix imtu & omtu values
      Bluetooth: 6lowpan: Remove redundant (and incorrect) MPS assignments
      Bluetooth: 6lowpan: Remove redundant BT_CONNECTED assignment
      Bluetooth: 6lowpan: Remove unnecessary chan_open() function
      Bluetooth: 6lowpan: Rename confusing 'pchan' variables
      Bluetooth: 6lowpan: Remove unnecessary chan_get() function

 net/bluetooth/6lowpan.c | 72 ++++++++++++++++--------------------------------
 1 file changed, 24 insertions(+), 48 deletions(-)

Johan

[PATCH 1/6] Bluetooth: 6lowpan: Fix imtu & omtu values

[Johan Hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

The omtu value is determined by the remote peer so there's no point in
trying to hard-code it to any value. The IPSP specification otoh gives
a more reasonable value for the imtu, i.e. 1280.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/6lowpan.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
index 131e79cde350..3e20f7a60d61 100644
--- a/net/bluetooth/6lowpan.c
+++ b/net/bluetooth/6lowpan.c
@@ -775,8 +775,7 @@ static struct l2cap_chan *chan_create(void)
 
 	chan->chan_type = L2CAP_CHAN_CONN_ORIENTED;
 	chan->mode = L2CAP_MODE_LE_FLOWCTL;
-	chan->omtu = 65535;
-	chan->imtu = chan->omtu;
+	chan->imtu = 1280;
 
 	return chan;
 }
-- 
2.4.3

[PATCH 2/6] Bluetooth: 6lowpan: Remove redundant (and incorrect) MPS assignments

[Johan Hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

The L2CAP core code already sets the local MPS to a sane value. The
remote MPS value otoh comes from the remote side so there's no point
in trying to hard-code it to any value.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/6lowpan.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
index 3e20f7a60d61..3d951abfdf41 100644
--- a/net/bluetooth/6lowpan.c
+++ b/net/bluetooth/6lowpan.c
@@ -788,9 +788,6 @@ static struct l2cap_chan *chan_open(struct l2cap_chan *pchan)
 	if (!chan)
 		return NULL;
 
-	chan->remote_mps = chan->omtu;
-	chan->mps = chan->omtu;
-
 	chan->state = BT_CONNECTED;
 
 	return chan;
-- 
2.4.3

[PATCH 3/6] Bluetooth: 6lowpan: Remove redundant BT_CONNECTED assignment

[Johan Hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

The L2CAP core code makes sure of setting the channel state to
BT_CONNECTED, so there's no need for the implementation code (6lowpan
in this case) to do it.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/6lowpan.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
index 3d951abfdf41..023fa29db709 100644
--- a/net/bluetooth/6lowpan.c
+++ b/net/bluetooth/6lowpan.c
@@ -788,8 +788,6 @@ static struct l2cap_chan *chan_open(struct l2cap_chan *pchan)
 	if (!chan)
 		return NULL;
 
-	chan->state = BT_CONNECTED;
-
 	return chan;
 }
 
-- 
2.4.3

[PATCH 4/6] Bluetooth: 6lowpan: Remove unnecessary chan_open() function

[Johan Hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

All the chan_open() function now does is to call chan_create() so it
doesn't really add any value.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/6lowpan.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
index 023fa29db709..77eb698d898e 100644
--- a/net/bluetooth/6lowpan.c
+++ b/net/bluetooth/6lowpan.c
@@ -780,17 +780,6 @@ static struct l2cap_chan *chan_create(void)
 	return chan;
 }
 
-static struct l2cap_chan *chan_open(struct l2cap_chan *pchan)
-{
-	struct l2cap_chan *chan;
-
-	chan = chan_create();
-	if (!chan)
-		return NULL;
-
-	return chan;
-}
-
 static void set_ip_addr_bits(u8 addr_type, u8 *addr)
 {
 	if (addr_type == BDADDR_LE_PUBLIC)
@@ -913,7 +902,10 @@ static inline struct l2cap_chan *chan_new_conn_cb(struct l2cap_chan *pchan)
 {
 	struct l2cap_chan *chan;
 
-	chan = chan_open(pchan);
+	chan = chan_create();
+	if (!chan)
+		return NULL;
+
 	chan->ops = pchan->ops;
 
 	BT_DBG("chan %p pchan %p", chan, pchan);
-- 
2.4.3

[PATCH 5/6] Bluetooth: 6lowpan: Rename confusing 'pchan' variables

[Johan Hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

The typical convention when having both a child and a parent channel
variable is to call the former 'chan' and the latter 'pchan'. When
there's only one variable it's called chan. Rename the 'pchan'
variables in the 6lowpan code to follow this convention.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/6lowpan.c | 42 +++++++++++++++++++++---------------------
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
index 77eb698d898e..e20b97297a15 100644
--- a/net/bluetooth/6lowpan.c
+++ b/net/bluetooth/6lowpan.c
@@ -1053,32 +1053,32 @@ static inline __u8 bdaddr_type(__u8 type)
 
 static struct l2cap_chan *chan_get(void)
 {
-	struct l2cap_chan *pchan;
+	struct l2cap_chan *chan;
 
-	pchan = chan_create();
-	if (!pchan)
+	chan = chan_create();
+	if (!chan)
 		return NULL;
 
-	pchan->ops = &bt_6lowpan_chan_ops;
+	chan->ops = &bt_6lowpan_chan_ops;
 
-	return pchan;
+	return chan;
 }
 
 static int bt_6lowpan_connect(bdaddr_t *addr, u8 dst_type)
 {
-	struct l2cap_chan *pchan;
+	struct l2cap_chan *chan;
 	int err;
 
-	pchan = chan_get();
-	if (!pchan)
+	chan = chan_get();
+	if (!chan)
 		return -EINVAL;
 
-	err = l2cap_chan_connect(pchan, cpu_to_le16(L2CAP_PSM_IPSP), 0,
+	err = l2cap_chan_connect(chan, cpu_to_le16(L2CAP_PSM_IPSP), 0,
 				 addr, dst_type);
 
-	BT_DBG("chan %p err %d", pchan, err);
+	BT_DBG("chan %p err %d", chan, err);
 	if (err < 0)
-		l2cap_chan_put(pchan);
+		l2cap_chan_put(chan);
 
 	return err;
 }
@@ -1103,31 +1103,31 @@ static int bt_6lowpan_disconnect(struct l2cap_conn *conn, u8 dst_type)
 static struct l2cap_chan *bt_6lowpan_listen(void)
 {
 	bdaddr_t *addr = BDADDR_ANY;
-	struct l2cap_chan *pchan;
+	struct l2cap_chan *chan;
 	int err;
 
 	if (!enable_6lowpan)
 		return NULL;
 
-	pchan = chan_get();
-	if (!pchan)
+	chan = chan_get();
+	if (!chan)
 		return NULL;
 
-	pchan->state = BT_LISTEN;
-	pchan->src_type = BDADDR_LE_PUBLIC;
+	chan->state = BT_LISTEN;
+	chan->src_type = BDADDR_LE_PUBLIC;
 
-	atomic_set(&pchan->nesting, L2CAP_NESTING_PARENT);
+	atomic_set(&chan->nesting, L2CAP_NESTING_PARENT);
 
-	BT_DBG("chan %p src type %d", pchan, pchan->src_type);
+	BT_DBG("chan %p src type %d", chan, chan->src_type);
 
-	err = l2cap_add_psm(pchan, addr, cpu_to_le16(L2CAP_PSM_IPSP));
+	err = l2cap_add_psm(chan, addr, cpu_to_le16(L2CAP_PSM_IPSP));
 	if (err) {
-		l2cap_chan_put(pchan);
+		l2cap_chan_put(chan);
 		BT_ERR("psm cannot be added err %d", err);
 		return NULL;
 	}
 
-	return pchan;
+	return chan;
 }
 
 static int get_l2cap_conn(char *buf, bdaddr_t *addr, u8 *addr_type,
-- 
2.4.3

[PATCH 6/6] Bluetooth: 6lowpan: Remove unnecessary chan_get() function

[Johan Hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

The chan_get() function just adds unnecessary indirection to calling
the chan_create() call. The only added value it gives is the chan->ops
assignment, but that can equally well be done in the calling code.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/6lowpan.c | 20 +++++---------------
 1 file changed, 5 insertions(+), 15 deletions(-)

diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
index e20b97297a15..9363f05275f4 100644
--- a/net/bluetooth/6lowpan.c
+++ b/net/bluetooth/6lowpan.c
@@ -1051,28 +1051,17 @@ static inline __u8 bdaddr_type(__u8 type)
 		return BDADDR_LE_RANDOM;
 }
 
-static struct l2cap_chan *chan_get(void)
-{
-	struct l2cap_chan *chan;
-
-	chan = chan_create();
-	if (!chan)
-		return NULL;
-
-	chan->ops = &bt_6lowpan_chan_ops;
-
-	return chan;
-}
-
 static int bt_6lowpan_connect(bdaddr_t *addr, u8 dst_type)
 {
 	struct l2cap_chan *chan;
 	int err;
 
-	chan = chan_get();
+	chan = chan_create();
 	if (!chan)
 		return -EINVAL;
 
+	chan->ops = &bt_6lowpan_chan_ops;
+
 	err = l2cap_chan_connect(chan, cpu_to_le16(L2CAP_PSM_IPSP), 0,
 				 addr, dst_type);
 
@@ -1109,10 +1098,11 @@ static struct l2cap_chan *bt_6lowpan_listen(void)
 	if (!enable_6lowpan)
 		return NULL;
 
-	chan = chan_get();
+	chan = chan_create();
 	if (!chan)
 		return NULL;
 
+	chan->ops = &bt_6lowpan_chan_ops;
 	chan->state = BT_LISTEN;
 	chan->src_type = BDADDR_LE_PUBLIC;
 
-- 
2.4.3

Re: [PATCH 0/6] Bluetooth: 6lowpan cleanups

[Jukka Rissanen]

Hi Johan,

Acked-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>

On ti, 2015-10-06 at 13:03 +0300, Johan Hedberg wrote:
> Hi,
> 
> Here's a set of initial cleanups for the 6lowpan code which I came up
> with as I started preparing to implement the new mgmt channel.
> 
> Johan Hedberg (6):
>       Bluetooth: 6lowpan: Fix imtu & omtu values
>       Bluetooth: 6lowpan: Remove redundant (and incorrect) MPS assignments
>       Bluetooth: 6lowpan: Remove redundant BT_CONNECTED assignment
>       Bluetooth: 6lowpan: Remove unnecessary chan_open() function
>       Bluetooth: 6lowpan: Rename confusing 'pchan' variables
>       Bluetooth: 6lowpan: Remove unnecessary chan_get() function
> 
>  net/bluetooth/6lowpan.c | 72 ++++++++++++++++--------------------------------
>  1 file changed, 24 insertions(+), 48 deletions(-)
> 
> Johan
> 

Cheers,
Jukka

Re: [PATCH 0/6] Bluetooth: 6lowpan cleanups

[Marcel Holtmann]

Hi Johan,

> Here's a set of initial cleanups for the 6lowpan code which I came up
> with as I started preparing to implement the new mgmt channel.
> 
> Johan Hedberg (6):
>      Bluetooth: 6lowpan: Fix imtu & omtu values
>      Bluetooth: 6lowpan: Remove redundant (and incorrect) MPS assignments
>      Bluetooth: 6lowpan: Remove redundant BT_CONNECTED assignment
>      Bluetooth: 6lowpan: Remove unnecessary chan_open() function
>      Bluetooth: 6lowpan: Rename confusing 'pchan' variables
>      Bluetooth: 6lowpan: Remove unnecessary chan_get() function
> 
> net/bluetooth/6lowpan.c | 72 ++++++++++++++++--------------------------------
> 1 file changed, 24 insertions(+), 48 deletions(-)

all 6 patches have been applied to bluetooth-next tree.

Regards

Marcel