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
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
Hi Johan,
Acked-by: Jukka Rissanen <[email protected]>
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
From: Johan Hedberg <[email protected]>
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 <[email protected]>
---
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
From: Johan Hedberg <[email protected]>
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 <[email protected]>
---
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
From: Johan Hedberg <[email protected]>
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 <[email protected]>
---
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
From: Johan Hedberg <[email protected]>
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 <[email protected]>
---
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
From: Johan Hedberg <[email protected]>
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 <[email protected]>
---
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
From: Johan Hedberg <[email protected]>
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 <[email protected]>
---
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