diff mbox

[RFC,bluetooth-next,20/20] 6lowpan: bluetooth: add new implementation

Message ID 20160711195044.25343-21-aar@pengutronix.de (mailing list archive)
State Superseded
Headers show

Commit Message

Alexander Aring July 11, 2016, 7:50 p.m. UTC
This patch adds a new btle 6lowpan implementation in version 0.2. This
new implementation has a new userspace API for debugfs.

It's now possible to decide on which hci interface do you want to run
your 6LoWPAN interface, for that you can run:

echo "hci0 1" > /sys/kernel/debug/bluetooth/6lowpan_enable

to enable, or:

echo "hci0 0" > /sys/kernel/debug/bluetooth/6lowpan_enable

to disable it. A cat on this file will show the current hci <-> 6lowpan
interface mapping.

Additional the different is, that the interface will be created after
enable it. The old behaviour is that the interface will be created when
one first connection is established and deleted after all connections
(peers) will be disconnected. This handling is different now.

The reason (in my opinion) is that the existing of such interface should
not based on if there is a connection or not. At runtime of many IPv6
application an interface will be removed -> I think the most userspace
application can and will never handle that an IPv6 interface will be
deleted and recreating during runtime.

The new handling is that the interface will not removed and you can
re-establish the connection to your peers.

Connection to peers:

Connection to peers are not binded to hci interface, it's binded to your
6LoWPAN interface. This will prepare handling for multiple 6LoWPAN
interfaces on one hci (maybe also with netns support, which isn't
available yet) and you can choose which peers should be handled by
different BTLE 6LoWPAN interfaces. Example:

L2: hci0 is connected to node A, B, C, D

L3: 6lo0 and 6lo1 are two interfaces based on hci0.
    6lo0 is connected to nodes: A, B, C
    6lo1 is connected to nodes: D, C, B

as one possible example. Handle this peer interface will have a possible
more fine-granularity connection for peers.

To connect to peers, there exists no "6lowpan_control" anymore. The
control file is now peer interface, it's simple:

echo "connect $PEER_ADDRESS 1" > /sys/kernel/debug/bluetooth/6lo0

Also after connecting to peers it's still possible that another peer can
to connect to the peer which has already a connected peer (complicated
to describe). I saw that is somehow disabled in version before, see:

http://lxr.free-electrons.com/source/net/bluetooth/6lowpan.c#L1247

I am not a bluetooth expert, but I guess that is somehow if there exists
limitation to devices which are not "peers" that means devices which can
be run as master or slave only. So far I know such limitations for
devices doesn't exists for BTLE 6LoWPAN, or?

Nevertheless I disabled that stuff, it's still possible to connect to
some node which already run "connect ..." on his side.

Otherwise I fixed the ndisc stuff and removed a lot of races/side
effects. The old code had some static "lowpan_devices" list and most
time there was some lookup mapping according to bdaddr to get the right
6lo netdevice struct. I removed that, also the skb->cb is used a lot in
some callback which had some side-effects.

Signed-off-by: Alexander Aring <aar@pengutronix.de>
---
 include/net/bluetooth/hci_core.h |   10 +
 net/bluetooth/6lowpan.c          | 1015 ++++++++++++++++++++++++++++++++++++++
 net/bluetooth/Makefile           |    3 +
 3 files changed, 1028 insertions(+)
 create mode 100644 net/bluetooth/6lowpan.c

Comments

Alexander Aring July 12, 2016, 9:19 p.m. UTC | #1
Hi,

On 07/11/2016 09:50 PM, Alexander Aring wrote:
> This patch adds a new btle 6lowpan implementation in version 0.2. This
> new implementation has a new userspace API for debugfs.
> 
> It's now possible to decide on which hci interface do you want to run
> your 6LoWPAN interface, for that you can run:
> 
> echo "hci0 1" > /sys/kernel/debug/bluetooth/6lowpan_enable
> 
> to enable, or:
> 
> echo "hci0 0" > /sys/kernel/debug/bluetooth/6lowpan_enable
> 
> to disable it. A cat on this file will show the current hci <-> 6lowpan
> interface mapping.
> 
> Additional the different is, that the interface will be created after
> enable it. The old behaviour is that the interface will be created when
> one first connection is established and deleted after all connections
> (peers) will be disconnected. This handling is different now.
> 
> The reason (in my opinion) is that the existing of such interface should
> not based on if there is a connection or not. At runtime of many IPv6
> application an interface will be removed -> I think the most userspace
> application can and will never handle that an IPv6 interface will be
> deleted and recreating during runtime.
> 
> The new handling is that the interface will not removed and you can
> re-establish the connection to your peers.
> 
> Connection to peers:
> 
> Connection to peers are not binded to hci interface, it's binded to your
> 6LoWPAN interface. This will prepare handling for multiple 6LoWPAN
> interfaces on one hci (maybe also with netns support, which isn't
> available yet) and you can choose which peers should be handled by
> different BTLE 6LoWPAN interfaces. Example:
> 
> L2: hci0 is connected to node A, B, C, D
> 
> L3: 6lo0 and 6lo1 are two interfaces based on hci0.
>     6lo0 is connected to nodes: A, B, C
>     6lo1 is connected to nodes: D, C, B
> 
> as one possible example. Handle this peer interface will have a possible
> more fine-granularity connection for peers.
> 
> To connect to peers, there exists no "6lowpan_control" anymore. The
> control file is now peer interface, it's simple:
> 
> echo "connect $PEER_ADDRESS 1" > /sys/kernel/debug/bluetooth/6lo0
> 
> Also after connecting to peers it's still possible that another peer can
> to connect to the peer which has already a connected peer (complicated
> to describe). I saw that is somehow disabled in version before, see:
> 
> http://lxr.free-electrons.com/source/net/bluetooth/6lowpan.c#L1247
> 
> I am not a bluetooth expert, but I guess that is somehow if there exists
> limitation to devices which are not "peers" that means devices which can
> be run as master or slave only. So far I know such limitations for
> devices doesn't exists for BTLE 6LoWPAN, or?
> 

I think the following will explain somehow when it's a SLAVE and when
MASTER as my current view.

SLAVES:

They don't makes this echo "connect ..." foo in control files.
they only do "hciconfig hci0 leadv" that somebody other can connect to
that node (some MASTER node).

MASTERS:

can do the same what the SLAVES does but to be as MASTER they need to call
additional stuff:

hcitool lecc $SLAVE_ADDR (one or more times)

Additional for different lowpan interfaces (don't need the same addresses
which was used by previous one or more hcitool lecc calls):

echo "connect SLAVE_ADDR 1" > /sys/kernel/debug/bluetooth/6lo0

is this a correct view and can all BTLE transceivers run as SLAVE or
MASTER, or exists there some special transceivers outside which can be
run as SLAVE only?

> Nevertheless I disabled that stuff, it's still possible to connect to
> some node which already run "connect ..." on his side.
> 
> Otherwise I fixed the ndisc stuff and removed a lot of races/side
> effects. The old code had some static "lowpan_devices" list and most
> time there was some lookup mapping according to bdaddr to get the right
> 6lo netdevice struct. I removed that, also the skb->cb is used a lot in
> some callback which had some side-effects.
> 
> Signed-off-by: Alexander Aring <aar@pengutronix.de>
> ---
>  include/net/bluetooth/hci_core.h |   10 +
>  net/bluetooth/6lowpan.c          | 1015 ++++++++++++++++++++++++++++++++++++++
>  net/bluetooth/Makefile           |    3 +
>  3 files changed, 1028 insertions(+)
>  create mode 100644 net/bluetooth/6lowpan.c
> 
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index a37027a..37a2256 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -399,6 +399,16 @@ struct hci_dev {
>  
>  	struct led_trigger	*power_led;
>  
> +#if IS_ENABLED(CONFIG_BT_6LOWPAN)
> +	/* TODO for netns support this need to be a list
> +	 * variables are protected by RTNL here
> +	 */
> +	struct net_device *ldev;
> +	bool lowpan_enable;
> +	/* delete lowpan iface via debugfs workaround */
> +	bool pending_deletion;
> +#endif
> +
>  	int (*open)(struct hci_dev *hdev);
>  	int (*close)(struct hci_dev *hdev);
>  	int (*flush)(struct hci_dev *hdev);
> diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
> new file mode 100644
> index 0000000..a8a3410
> --- /dev/null
> +++ b/net/bluetooth/6lowpan.c
> @@ -0,0 +1,1015 @@
> +/*
> +   (C) 2016 Pengutronix, Alexander Aring <aar@pengutronix.de>
> +   Copyright (c) 2013-2014 Intel Corp.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License version 2 and
> +   only version 2 as published by the Free Software Foundation.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +*/
> +
> +#include <linux/if_arp.h>
> +#include <linux/netdevice.h>
> +#include <linux/etherdevice.h>
> +#include <linux/module.h>
> +#include <linux/debugfs.h>
> +
> +#include <net/ipv6.h>
> +#include <net/ip6_route.h>
> +#include <net/addrconf.h>
> +
> +#include <net/bluetooth/bluetooth.h>
> +#include <net/bluetooth/hci_core.h>
> +#include <net/bluetooth/l2cap.h>
> +
> +#include <net/6lowpan.h>
> +
> +#define LOWPAN_BTLE_VERSION "0.2"
> +
> +#define lowpan_le48_to_be48(dst, src) baswap((bdaddr_t *)dst, (bdaddr_t *)src)
> +#define lowpan_be48_to_le48(dst, src) baswap((bdaddr_t *)dst, (bdaddr_t *)src)
> +
> +struct lowpan_btle_cb {
> +	struct l2cap_chan *chan;
> +};
> +
> +struct lowpan_btle_dev {
> +	struct hci_dev *hdev;
> +	struct workqueue_struct *workqueue;
> +	struct l2cap_chan *listen;
> +	struct list_head peers;
> +	struct dentry *control;
> +};
> +
> +struct lowpan_peer {
> +	struct l2cap_chan *chan;
> +
> +	struct list_head list;
> +};
> +
> +struct lowpan_chan_data {
> +	struct lowpan_peer peer;
> +	struct net_device *dev;
> +};
> +
> +struct lowpan_xmit_work {
> +	struct work_struct work;
> +	struct l2cap_chan *chan;
> +	struct net_device *dev;
> +	unsigned int true_len;
> +	struct sk_buff *skb;
> +};
> +
> +static inline struct lowpan_btle_cb *
> +lowpan_btle_cb(const struct sk_buff *skb)
> +{
> +	return (struct lowpan_btle_cb *)skb->cb;
> +}
> +
> +static inline struct lowpan_chan_data *
> +lowpan_chan_data(const struct l2cap_chan *chan)
> +{
> +	return (struct lowpan_chan_data *)chan->data;
> +}
> +
> +static inline struct lowpan_btle_dev *
> +lowpan_btle_dev(const struct net_device *dev)
> +{
> +	return (struct lowpan_btle_dev *)lowpan_dev(dev)->priv;
> +}
> +
> +static inline struct lowpan_peer *
> +lowpan_lookup_peer(struct lowpan_btle_dev *btdev, bdaddr_t *addr)
> +{
> +	struct lowpan_peer *entry;
> +
> +	list_for_each_entry_rcu(entry, &btdev->peers, list) {
> +		if (!bacmp(&entry->chan->dst, addr))
> +			return entry;
> +	}
> +
> +	return NULL;
> +}
> +
> +static int lowpan_give_skb_to_device(struct sk_buff *skb)
> +{
> +	skb->protocol = htons(ETH_P_IPV6);
> +	skb->dev->stats.rx_packets++;
> +	skb->dev->stats.rx_bytes += skb->len;
> +
> +	return netif_rx_ni(skb);
> +}
> +
> +static int lowpan_rx_handlers_result(struct sk_buff *skb, lowpan_rx_result res)
> +{
> +	switch (res) {
> +	case RX_CONTINUE:
> +		/* nobody cared about this packet */
> +		net_warn_ratelimited("%s: received unknown dispatch\n",
> +				     __func__);

that printout isn't corrected, for simplification I drop all non iphc
frames at recv callback.

I want to have this receive handling into "net/6lowpan" but I want to
not doing this now in this patch-series. It will prepare it.

After moving it to "net/6lowpan" unknown dispatches should be really
dispatches which are not known in the current state of implementation.

I think it's okay to leave this right there as it is, also somebody can
trigger this warning by sending lot of frames with the right dispatch,
not sure if that's normal that somebody from outside can trigger a lot
of system messages which are show up in dmesg. Need to think about it,
maybe some "ONCE" mechanism or using debug stuff.

> +
> +		/* fall-through */
> +	case RX_DROP_UNUSABLE:
> +		kfree_skb(skb);
> +
> +		/* fall-through */
> +	case RX_DROP:
> +		return NET_RX_DROP;
> +	case RX_QUEUED:
> +		return lowpan_give_skb_to_device(skb);
> +	default:
> +		break;
> +	}
> +
> +	return NET_RX_DROP;
> +}
> +
> +static lowpan_rx_result lowpan_rx_h_iphc(struct sk_buff *skb)
> +{
> +	struct l2cap_chan *chan = lowpan_btle_cb(skb)->chan;
> +	bdaddr_t daddr, saddr;
> +	int ret;
> +
> +	if (!lowpan_is_iphc(*skb_network_header(skb)))
> +		return RX_CONTINUE;
> +
> +	BT_DBG("recv %s dst: %pMR type %d src: %pMR chan %p",
> +	       skb->dev->name, &chan->dst, chan->dst_type, &chan->src, chan);
> +
> +	/* bluetooth chan view is vice-versa */
> +	bacpy(&daddr, &chan->src);
> +	bacpy(&saddr, &chan->dst);
> +
> +	ret = lowpan_header_decompress(skb, skb->dev, &daddr, &saddr);
> +	if (ret < 0)
> +		return RX_DROP_UNUSABLE;
> +
> +	return RX_QUEUED;
> +}
> +
> +static int lowpan_invoke_rx_handlers(struct sk_buff *skb)
> +{
> +	lowpan_rx_result res;
> +
> +#define CALL_RXH(rxh)			\
> +	do {				\
> +		res = rxh(skb);		\
> +		if (res != RX_CONTINUE)	\
> +			goto rxh_next;	\
> +	} while (0)
> +
> +	/* likely at first */
> +	CALL_RXH(lowpan_rx_h_iphc);
> +
> +rxh_next:
> +	return lowpan_rx_handlers_result(skb, res);
> +#undef CALL_RXH
> +}
> +
> +static int lowpan_chan_recv(struct l2cap_chan *chan, struct sk_buff *skb)
> +{
> +	struct lowpan_chan_data *data = lowpan_chan_data(chan);
> +	struct net_device *dev = data->dev;
> +	int ret;
> +
> +	/* TODO handle BT_CONNECTED in bluetooth subsytem? on
> +	 * when calling recv callback, I hit that case somehow
> +	 */
> +	if (!netif_running(dev) || chan->state != BT_CONNECTED ||
> +	    !skb->len || !lowpan_is_iphc(skb->data[0]))
> +		goto drop;
> +
> +	/* Replacing skb->dev and followed rx handlers will manipulate skb. */
> +	skb = skb_unshare(skb, GFP_ATOMIC);

can be GFP_KERNEL here.

> +	if (!skb)
> +		goto out;
> +
> +	skb->dev = dev;
> +	skb_reset_network_header(skb);
> +
> +	/* remember that one for dst bdaddr. TODO handle that as priv data for
> +	 * lowpan_invoke_rx_handlers parameter. Not necessary for skb->cb.
> +	 */

After fixing that, skb->cb isn't needed anymore in this implementation.
But I will do that when moving to receive handling to "net/6lowpan".

> +	lowpan_btle_cb(skb)->chan = chan;
> +
> +	ret = lowpan_invoke_rx_handlers(skb);
> +	if (ret == NET_RX_DROP)
> +		BT_DBG("recv %s dropped chan %p", skb->dev->name, chan);
> +
> +	return 0;
> +
> +drop:
> +	kfree_skb(skb);
> +out:
> +	/* we handle to free skb on error, so must 0
> +	 * seems that callback free the skb on error
> +	 * otherwise.
> +	 */

btw:

This is also a MUST to return 0 here always. Because netif_rx_ni will handle
also freeing on error and we cannot change this handling. (Or we doing
some crazy reference counting before foo, but will more confuse
everybody).

> +	return 0;
> +}
> +
> +static void lowpan_xmit_worker(struct work_struct *work)
> +{
> +	struct lowpan_btle_dev *btdev;
> +	struct lowpan_xmit_work *xw;
> +	struct l2cap_chan *chan;
> +	struct net_device *dev;
> +	struct msghdr msg = { };
> +	struct kvec iv;
> +	int ret;
> +
> +	xw = container_of(work, struct lowpan_xmit_work, work);
> +	dev = xw->dev;
> +	chan = xw->chan;
> +	btdev = lowpan_btle_dev(dev);
> +
> +	iv.iov_base = xw->skb->data;
> +	iv.iov_len = xw->skb->len;
> +	iov_iter_kvec(&msg.msg_iter, WRITE | ITER_KVEC, &iv, 1, xw->skb->len);
> +
> +	BT_DBG("l2cap_chan_send %s dst: %pMR type %d src: %pMR chan %p",
> +	       dev->name, &chan->dst, chan->dst_type, &chan->src, chan);
> +
> +	l2cap_chan_lock(chan);
> +
> +	ret = l2cap_chan_send(chan, &msg, xw->skb->len);
> +	BT_DBG("transmit return value %d", ret);
> +	if (ret < 0) {
> +		BT_DBG("send %s failed chan %p", dev->name, chan);
> +		kfree_skb(xw->skb);
> +	} else {
> +		consume_skb(xw->skb);
> +		dev->stats.tx_bytes += xw->true_len;
> +		dev->stats.tx_packets++;
> +	}
> +
> +	l2cap_chan_unlock(chan);
> +	l2cap_chan_put(chan);
> +
> +	kfree(xw);
> +}
> +
> +static void lowpan_send_unicast_pkt(struct net_device *dev,
> +				    struct l2cap_chan *chan,
> +				    struct sk_buff *skb,
> +				    unsigned int true_len)
> +{
> +	struct lowpan_xmit_work *xw;
> +
> +	/* copy to xmit work buffer */
> +	xw = kzalloc(sizeof(*xw), GFP_ATOMIC);
> +	if (!xw)
> +		return;
> +
> +	/* chan->lock mutex need to be hold so change context to workqueue */
> +	INIT_WORK(&xw->work, lowpan_xmit_worker);
> +	xw->true_len = true_len;
> +	/* freeing protected by ifdown workqueue sync */
> +	xw->dev = dev;
> +	/* disallow freeing of skb while context switch */
> +	xw->skb = skb_get(skb);
> +	/* disallow freeing while context switch */
> +	l2cap_chan_hold(chan);
> +	xw->chan = chan;
> +
> +	queue_work(lowpan_btle_dev(dev)->workqueue, &xw->work);
> +}
> +
> +static void lowpan_send_mcast_pkt(struct net_device *dev, struct sk_buff *skb,
> +				  unsigned int true_len)
> +{
> +	struct lowpan_btle_dev *btdev = lowpan_btle_dev(dev);
> +	struct lowpan_peer *peer;
> +
> +	rcu_read_lock();
> +
> +	BT_DBG("xmit %s starts multicasting", dev->name);
> +
> +	/* We need to send the packet to every device behind this
> +	 * interface, because multicasting.
> +	 */
> +	list_for_each_entry_rcu(peer, &btdev->peers, list)
> +		lowpan_send_unicast_pkt(dev, peer->chan, skb, true_len);
> +
> +	rcu_read_unlock();
> +}
> +
> +static netdev_tx_t lowpan_xmit(struct sk_buff *skb, struct net_device *dev)
> +{
> +	struct lowpan_addr_info *info = lowpan_addr_info(skb);
> +	struct lowpan_btle_dev *btdev = lowpan_btle_dev(dev);
> +	unsigned int true_len = skb->len;
> +	struct lowpan_peer *peer;
> +	bdaddr_t daddr, saddr;
> +	int ret;
> +
> +	/* We must take a copy of the skb before we modify/replace the ipv6
> +	 * header as the header could be used elsewhere.
> +	 */
> +	skb = skb_unshare(skb, GFP_ATOMIC);
> +	if (!skb) {
> +		kfree_skb(skb);
> +		goto out;
> +	}
> +
> +	lowpan_be48_to_le48(&daddr, info->daddr);
> +	lowpan_be48_to_le48(&saddr, info->saddr);
> +
> +	BT_DBG("xmit ndisc %s dst: %pMR src: %pMR",
> +	       dev->name, &daddr, &saddr);
> +
> +	ret = lowpan_header_compress(skb, dev, &daddr, &saddr);
> +	if (ret < 0) {
> +		kfree_skb(skb);
> +		goto out;
> +	}
> +
> +	/* this should never be the case, otherwise iphc is broken */
> +	WARN_ON_ONCE(skb->len > dev->mtu);
> +
> +	if (!memcmp(&daddr, dev->broadcast, dev->addr_len)) {
> +		lowpan_send_mcast_pkt(dev, skb, true_len);
> +	} else {
> +		btdev = lowpan_btle_dev(dev);
> +

btdev = can be removed here.

> +		rcu_read_lock();
> +
> +		peer = lowpan_lookup_peer(btdev, &daddr);
> +		if (peer)
> +			lowpan_send_unicast_pkt(dev, peer->chan, skb,
> +						true_len);
> +
> +		rcu_read_unlock();
> +	}
> +
> +	consume_skb(skb);
> +
> +out:
> +	return NETDEV_TX_OK;
> +}
> +
> +static int lowpan_stop(struct net_device *dev)
> +{
> +	struct lowpan_btle_dev *btdev = lowpan_btle_dev(dev);
> +
> +	/* synchronize with xmit worker */
> +	flush_workqueue(btdev->workqueue);
> +	return 0;
> +}
> +
> +static struct sk_buff *lowpan_chan_alloc_skb(struct l2cap_chan *chan,
> +					     unsigned long hdr_len,
> +					     unsigned long len, int nb)
> +{
> +	return bt_skb_alloc(hdr_len + len, GFP_KERNEL);
> +}
> +
> +static long lowpan_chan_get_sndtimeo(struct l2cap_chan *chan)
> +{
> +	return L2CAP_CONN_TIMEOUT;
> +}
> +
> +static struct l2cap_chan *lowpan_chan_create(struct net_device *dev)
> +{
> +	struct lowpan_chan_data *data;
> +	struct l2cap_chan *chan;
> +
> +	chan = l2cap_chan_create();
> +	if (!chan)
> +		return ERR_PTR(-ENOMEM);
> +
> +	data = kmalloc(sizeof(*data), GFP_KERNEL);
> +	if (!data) {
> +		l2cap_chan_put(chan);
> +		return ERR_PTR(-ENOMEM);
> +	}
> +
> +	l2cap_chan_set_defaults(chan);
> +	chan->chan_type = L2CAP_CHAN_CONN_ORIENTED;
> +	chan->mode = L2CAP_MODE_LE_FLOWCTL;
> +	chan->imtu = dev->mtu;
> +	chan->data = data;
> +
> +	data->peer.chan = chan;
> +	data->dev = dev;
> +
> +	return chan;
> +}
> +
> +static struct l2cap_chan *lowpan_chan_new_conn(struct l2cap_chan *pchan)
> +{
> +	struct lowpan_chan_data *data = lowpan_chan_data(pchan);
> +	struct l2cap_chan *chan;
> +
> +	chan = lowpan_chan_create(data->dev);
> +	if (IS_ERR(chan))
> +		return NULL;
> +
> +	chan->ops = pchan->ops;
> +	return chan;
> +}
> +
> +static void lowpan_chan_ready(struct l2cap_chan *chan)
> +{
> +	struct lowpan_chan_data *data = lowpan_chan_data(chan);
> +	struct lowpan_btle_dev *btdev = lowpan_btle_dev(data->dev);
> +
> +	BT_DBG("%s chan %p ready ", data->dev->name, chan);
> +
> +	/* make it visible for xmit */
> +	list_add_tail_rcu(&data->peer.list, &btdev->peers);
> +	synchronize_rcu();
> +}
> +
> +static void lowpan_chan_close(struct l2cap_chan *chan)
> +{
> +	struct lowpan_chan_data *data = lowpan_chan_data(chan);
> +
> +	BT_DBG("%s chan %p closed ", data->dev->name, chan);
> +
> +	/* make it unvisible for xmit */
> +	list_del_rcu(&data->peer.list);
> +	synchronize_rcu();
> +	kfree(data);

I currently think much about to run kfree here and I think I need
bluetooth experts to answer the correct handling here.

We allocate that data on chan_create, but doing a free in chan_close
requires that the chan state system will never run chan_ready
afterwards. Is this correct?

Correct handling for me is to do the free when the last chan_put was
called and free everything. Maybe it could be useful to add some
private_data _room_ to l2cap_chan_create, which sits after "struct
l2cap_chan" as something like "u8 data[0]", to allocate/free everything
at once.

I didn't hit any issues yet that after a close, the ready callback will
be called again so I think this can never happen.

btw:

previous implementation had kfree_rcu here, but we run synchronize_rcu
before and this is not needed anymore. I think kfree_rcu will not block,
but I feeling better to not leave that callback after running
synchronize_rcu. :-/

> +}
> +
> +static const struct l2cap_ops lowpan_chan_ops = {
> +	.name			= "L2CAP 6LoWPAN channel",
> +	.new_connection		= lowpan_chan_new_conn,
> +	.recv			= lowpan_chan_recv,
> +	.close			= lowpan_chan_close,
> +	.state_change		= l2cap_chan_no_state_change,
> +	.ready			= lowpan_chan_ready,
> +	.get_sndtimeo		= lowpan_chan_get_sndtimeo,
> +	.alloc_skb		= lowpan_chan_alloc_skb,
> +	.teardown		= l2cap_chan_no_teardown,
> +	.defer			= l2cap_chan_no_defer,
> +	.set_shutdown		= l2cap_chan_no_set_shutdown,
> +	.resume			= l2cap_chan_no_resume,
> +	.suspend		= l2cap_chan_no_suspend,
> +};
> +
> +static int lowpan_change_mtu(struct net_device *dev, int new_mtu)
> +{
> +	struct lowpan_btle_dev *btdev = lowpan_btle_dev(dev);
> +
> +	/* if dev is down, peers list are protected by RTNL */
> +	if (netif_running(dev) || !list_empty(&btdev->peers))
> +		return -EBUSY;
> +
> +	if (new_mtu < IPV6_MIN_MTU)
> +		return -EINVAL;
> +
> +	dev->mtu = new_mtu;
> +	return 0;
> +}
> +
> +static const struct net_device_ops netdev_ops = {
> +	.ndo_init		= lowpan_dev_init,
> +	.ndo_stop		= lowpan_stop,
> +	.ndo_start_xmit		= lowpan_xmit,
> +	.ndo_change_mtu		= lowpan_change_mtu,
> +};
> +
> +static void lowpan_free_netdev(struct net_device *dev)
> +{
> +	struct lowpan_btle_dev *btdev = lowpan_btle_dev(dev);
> +	struct lowpan_peer *peer, *tmp;
> +	struct l2cap_chan *chan;
> +
> +	l2cap_chan_close(btdev->listen, 0);
> +	l2cap_chan_put(btdev->listen);
> +
> +	/* no rcu here? should be safe netdev is down */
> +	list_for_each_entry_safe(peer, tmp, &btdev->peers, list) {
> +		/* close will free peer data, so save chan here */
> +		chan = peer->chan;
> +
> +		/* will del peer from list and and free.
> +		 * should be safe by tmp pointer which has
> +		 * a pointer for the next entry.
> +		 */
> +		l2cap_chan_close(chan, 0);
> +		l2cap_chan_put(chan);
> +	}
> +
> +	destroy_workqueue(btdev->workqueue);
> +	btdev->hdev->lowpan_enable = false;
> +	debugfs_remove(btdev->control);
> +	hci_dev_put(btdev->hdev);
> +}
> +
> +static void lowpan_setup(struct net_device *dev)
> +{
> +	memset(dev->broadcast, 0xff, sizeof(bdaddr_t));
> +
> +	dev->netdev_ops	= &netdev_ops;
> +	dev->destructor	= lowpan_free_netdev;
> +	dev->features	|= NETIF_F_NETNS_LOCAL;
> +}
> +
> +static struct device_type bt_type = {
> +	.name	= "bluetooth",
> +};
> +
> +static struct l2cap_chan *lowpan_listen_chan_new_conn(struct l2cap_chan *pchan)
> +{
> +	struct l2cap_chan *chan;
> +
> +	chan = lowpan_chan_create(pchan->data);
> +	if (IS_ERR(chan))
> +		return NULL;
> +
> +	/* change ops with more functionality than listen,
> +	 * which also free chan->data stuff.
> +	 */
> +	chan->ops = &lowpan_chan_ops;
> +
> +	return chan;
> +}
> +
> +static const struct l2cap_ops lowpan_listen_chan_ops = {
> +	.name			= "L2CAP 6LoWPAN listen channel",
> +	.new_connection		= lowpan_listen_chan_new_conn,
> +	.recv			= l2cap_chan_no_recv,
> +	.close			= l2cap_chan_no_close,
> +	.state_change		= l2cap_chan_no_state_change,
> +	.ready			= l2cap_chan_no_ready,
> +	.get_sndtimeo		= l2cap_chan_no_get_sndtimeo,
> +	.alloc_skb		= l2cap_chan_no_alloc_skb,
> +	.teardown		= l2cap_chan_no_teardown,
> +	.defer			= l2cap_chan_no_defer,
> +	.set_shutdown		= l2cap_chan_no_set_shutdown,
> +	.resume			= l2cap_chan_no_resume,
> +	.suspend		= l2cap_chan_no_suspend,
> +};
> +
> +static int lowpan_create_listen_chan(struct net_device *dev)
> +{
> +	struct lowpan_btle_dev *btdev = lowpan_btle_dev(dev);
> +	struct l2cap_chan *chan;
> +	u8 bdaddr_type;
> +	int ret;
> +
> +	/* don't use lowpan_chan_create here, because less functionality */
> +	chan = l2cap_chan_create();
> +	if (!chan)
> +		return -ENOMEM;
> +
> +	chan->data = dev;

I will add a comment here that this private_data of chan is really
"struct net_device *dev" only, it's for listen stuff only.

Looks wrong at the first sight.

> +	chan->ops = &lowpan_listen_chan_ops;
> +	hci_copy_identity_address(btdev->hdev, &chan->src, &bdaddr_type);
> +	if (bdaddr_type == ADDR_LE_DEV_PUBLIC)
> +		chan->src_type = BDADDR_LE_PUBLIC;
> +	else
> +		chan->src_type = BDADDR_LE_RANDOM;
> +
> +	chan->state = BT_LISTEN;
> +	atomic_set(&chan->nesting, L2CAP_NESTING_PARENT);
> +
> +	BT_DBG("chan %p src type %d", chan, chan->src_type);
> +	ret = l2cap_add_psm(chan, BDADDR_ANY, cpu_to_le16(L2CAP_PSM_IPSP));
> +	if (ret) {
> +		l2cap_chan_put(chan);
> +		BT_ERR("psm cannot be added err %d", ret);
> +		return ret;
> +	}
> +	btdev->listen = chan;
> +
> +	return 0;
> +}
> +
> +static const struct file_operations lowpan_control_fops;
> +
> +static struct net_device *lowpan_btle_newlink(struct hci_dev *hdev)
> +{
> +	struct lowpan_btle_dev *btdev;
> +	struct net_device *dev;
> +	int err = -ENOMEM;
> +
> +	dev = alloc_netdev(LOWPAN_PRIV_SIZE(sizeof(struct lowpan_btle_dev)),
> +			   LOWPAN_IFNAME_TEMPLATE, NET_NAME_ENUM, lowpan_setup);
> +	if (!dev)
> +		goto out;
> +
> +	dev->addr_assign_type = NET_ADDR_PERM;
> +	dev->addr_len = sizeof(hdev->bdaddr.b);
> +	lowpan_le48_to_be48(dev->dev_addr, &hdev->bdaddr);
> +
> +	SET_NETDEV_DEV(dev, &hdev->dev);
> +	SET_NETDEV_DEVTYPE(dev, &bt_type);
> +
> +	btdev = lowpan_btle_dev(dev);
> +	/* avoid freeing */
> +	btdev->hdev = hci_dev_hold(hdev);
> +	INIT_LIST_HEAD(&btdev->peers);
> +
> +	btdev->workqueue = create_singlethread_workqueue(dev->name);
> +	if (!btdev->workqueue) {
> +		free_netdev(dev);
> +		goto out;
> +	}
> +
> +	err = lowpan_create_listen_chan(dev);
> +	if (err < 0) {
> +		destroy_workqueue(btdev->workqueue);
> +		free_netdev(dev);
> +		goto out;
> +	}
> +
> +	err = lowpan_register_netdevice(dev, LOWPAN_LLTYPE_BTLE);
> +	if (err < 0) {
> +		BT_ERR("register_netdev failed %d", err);
> +		l2cap_chan_close(btdev->listen, 0);
> +		l2cap_chan_put(btdev->listen);
> +		free_netdev(dev);
> +		goto out;
> +	}
> +	hdev->ldev = dev;
> +
> +	/* ignore error handling here, we cannot call unregister anymore
> +	 * It's a bug, but it's debugfs... so ignore it.
> +	 */
> +	btdev->control = debugfs_create_file(dev->name, 0644,
> +					     bt_debugfs, btdev,
> +					     &lowpan_control_fops);
> +	if (!btdev->control)
> +		BT_ERR("debugfs error for %s, disable/enable 6lowpan again",
> +		       dev->name);
> +
> +	return dev;
> +
> +out:
> +	return ERR_PTR(err);
> +}
> +
> +static void lowpan_btle_dellink(struct net_device *dev)
> +{
> +	lowpan_unregister_netdevice(dev);
> +}
> +
> +static int lowpan_parse_le_bdaddr(struct hci_dev *hdev, unsigned char *buf,
> +				  bdaddr_t *addr, u8 *addr_type)
> +{
> +	int n;
> +
> +	n = sscanf(buf, "%hhx:%hhx:%hhx:%hhx:%hhx:%hhx %hhu",
> +		   &addr->b[5], &addr->b[4], &addr->b[3],
> +		   &addr->b[2], &addr->b[1], &addr->b[0],
> +		   addr_type);
> +	if (n < 7)
> +		return -EINVAL;
> +
> +	/* check if we handle le addresses and not own source address */
> +	if (!bdaddr_type_is_le(*addr_type) ||
> +	    !bacmp(&hdev->bdaddr, addr))
> +		return -EINVAL;
> +
> +	return n;
> +}
> +
> +static ssize_t __lowpan_control_write(struct file *fp,
> +				      const char __user *user_buffer,
> +				      size_t count, loff_t *position)
> +{
> +	char buf[32] = { };
> +	size_t buf_size = min(count, sizeof(buf) - 1);
> +	struct seq_file *file = fp->private_data;
> +	struct lowpan_btle_dev *btdev = file->private;
> +	struct hci_dev *hdev = btdev->hdev;
> +	struct lowpan_peer *peer;
> +	/* slave address */
> +	bdaddr_t addr;
> +	u8 addr_type;
> +	int ret;
> +
> +	if (copy_from_user(buf, user_buffer, buf_size))
> +		return -EFAULT;
> +
> +	if (memcmp(buf, "connect ", 8) == 0) {
> +		struct lowpan_peer *peer;

peer can be removed here.

> +		struct l2cap_chan *chan;
> +
> +		ret = lowpan_parse_le_bdaddr(hdev, &buf[8], &addr, &addr_type);
> +		if (ret < 0)
> +			return ret;
> +
> +		/* check if we already know that slave */
> +		rcu_read_lock();
> +		peer = lowpan_lookup_peer(btdev, &addr);
> +		if (peer) {
> +			rcu_read_unlock();
> +			BT_DBG("6LoWPAN connection already exists");
> +			return -EEXIST;
> +		}
> +		rcu_read_unlock();
> +
> +		chan = lowpan_chan_create(hdev->ldev);
> +		if (IS_ERR(chan))
> +			return PTR_ERR(chan);
> +		chan->ops = &lowpan_chan_ops;
> +
> +		ret = l2cap_hdev_chan_connect(hdev, chan,
> +					      cpu_to_le16(L2CAP_PSM_IPSP),
> +					      0, &addr, addr_type);
> +		if (ret < 0) {
> +			l2cap_chan_put(chan);
> +			return ret;
> +		}
> +
> +		return count;
> +	} else if (memcmp(buf, "disconnect ", 11) == 0) {
> +		ret = lowpan_parse_le_bdaddr(hdev, &buf[11], &addr, &addr_type);
> +		if (ret < 0)
> +			return ret;
> +
> +		/* check if we don't know that slave */
> +		rcu_read_lock();
> +		peer = lowpan_lookup_peer(btdev, &addr);
> +		if (!peer) {
> +			rcu_read_unlock();
> +			BT_DBG("6LoWPAN connection not found in peers");
> +			return -ENOENT;
> +		}
> +		rcu_read_unlock();
> +
> +		/* first delete the peer out of list,
> +		 * which makes it visiable to netdev,
> +		 * will be done by close callback.
> +		 */
> +		l2cap_chan_close(peer->chan, 0);
> +		l2cap_chan_put(peer->chan);
> +	} else {
> +		return -EINVAL;
> +	}
> +
> +	return count;
> +}
> +
> +static ssize_t lowpan_control_write(struct file *fp,
> +				    const char __user *user_buffer,
> +				    size_t count, loff_t *position)
> +{
> +	ssize_t ret;
> +
> +	rtnl_lock();
> +	ret = __lowpan_control_write(fp, user_buffer, count, position);
> +	rtnl_unlock();
> +
> +	return ret;
> +}
> +
> +static int lowpan_control_show(struct seq_file *f, void *ptr)
> +{
> +	struct lowpan_btle_dev *btdev = f->private;
> +	struct hci_dev *hdev = btdev->hdev;
> +	struct lowpan_peer *peer;
> +
> +	rtnl_lock();
> +
> +	if (!hdev->lowpan_enable) {
> +		rtnl_unlock();
> +		return 0;
> +	}
> +
> +	rcu_read_lock();
> +
> +	list_for_each_entry_rcu(peer, &btdev->peers, list) {
> +		seq_printf(f, "%pMR (type %u) state: %s\n",
> +			   &peer->chan->dst, peer->chan->dst_type,
> +			   state_to_string(peer->chan->state));
> +	}
> +
> +	rcu_read_unlock();
> +
> +	rtnl_unlock();
> +	return 0;
> +}
> +
> +static int lowpan_control_open(struct inode *inode, struct file *file)
> +{
> +	return single_open(file, lowpan_control_show, inode->i_private);
> +}
> +
> +static const struct file_operations lowpan_control_fops = {
> +	.open		= lowpan_control_open,
> +	.read		= seq_read,
> +	.write		= lowpan_control_write,
> +	.llseek		= seq_lseek,
> +	.release	= single_release,
> +};
> +
> +/* TODO remove this when add non debugfs API, it's a workaround */
> +static struct workqueue_struct *workqueue_cleanup;
> +
> +struct lowpan_cleanup_work {
> +	struct work_struct work;
> +	struct net_device *dev;
> +};
> +
> +static void lowpan_cleanup_worker(struct work_struct *work)
> +{
> +	struct lowpan_cleanup_work *xw;
> +
> +	xw = container_of(work, struct lowpan_cleanup_work, work);
> +
> +	rtnl_lock();
> +	lowpan_btle_dellink(xw->dev);
> +	lowpan_btle_dev(xw->dev)->hdev->pending_deletion = false;
> +	rtnl_unlock();
> +
> +	kfree(xw);
> +}
> +
> +/* TODO workaround, debugfs doesn't like recursion remove while debugfs
> + * handling, this should be removed for by using real UAPI.
> + */
> +static int __lowpan_btle_dellink(struct net_device *dev,
> +				 struct hci_dev *hdev)
> +{
> +	struct lowpan_cleanup_work *xw;
> +
> +	xw = kzalloc(sizeof(*xw), GFP_KERNEL);
> +	if (!xw)
> +		return -ENOMEM;
> +
> +	INIT_WORK(&xw->work, lowpan_cleanup_worker);
> +	xw->dev = dev;
> +
> +	hdev->pending_deletion = true;
> +	queue_work(workqueue_cleanup, &xw->work);
> +	return 0;
> +}
> +
> +static int lowpan_enable_set(struct hci_dev *hdev, bool enabled)
> +{
> +	int ret = 0;
> +
> +	if (hdev->lowpan_enable == enabled)
> +		goto out;
> +
> +	if (enabled) {
> +		struct net_device *dev;
> +
> +		/* create one interface by default */
> +		dev = lowpan_btle_newlink(hdev);
> +		if (IS_ERR(dev))
> +			return PTR_ERR(dev);
> +
> +		/* TODO should be done by user network managers? */
> +		ret = dev_open(dev);
> +		if (ret < 0) {
> +			BT_ERR("iface %s cannot be opened %d", dev->name, ret);
> +			ret = __lowpan_btle_dellink(dev, hdev);
> +			if (ret < 0)
> +				BT_ERR("failed to remove interface %s",
> +				       hdev->ldev->name);
> +
> +			return ret;
> +		}
> +
> +		hdev->lowpan_enable = true;
> +	} else {
> +		/* ignore pending deletions */
> +		if (hdev->pending_deletion)
> +			return 0;
> +
> +		__lowpan_btle_dellink(hdev->ldev, hdev);
> +	}
> +
> +out:
> +	return ret;
> +}
> +
> +static void lowpan_btle_hci_unregister(struct hci_dev *hdev)
> +{
> +	/* no need to cleanup debugfs, will be done by hci_core */

comment can be removed.

Well..., I had this comment because I did some stuff before per
"hci->debugfs". This is also the question here.

Maybe "6lowpan_enable" makes no sense to move it to per hci_dev debugfs
entry. For write, it makes sense - but read, you want some "global"
mapping of "hci_dev <-> lowpan interface", this may easy realizied for
netlink UAPI but for debugfs it's complicated because you need to
iterate over all hci%d entries and making cat on "6lowpan_enable".

That's the current reason why "6lowpan_enable" is global.

---

The control file for btle lowpan interface could be moved per hci_dev
and maybe I will change it again, then the above comment makes sense
again. :-)

> +
> +	/* TODO netns support with multiple lowpan interfaces */
> +	if (hdev->lowpan_enable)
> +		lowpan_btle_dellink(hdev->ldev);
> +}
> +
> +static int lowpan_hci_dev_event(struct notifier_block *unused,
> +				unsigned long event, void *ptr)
> +{
> +	struct hci_dev *hdev = ptr;
> +	int ret = NOTIFY_OK;
> +
> +	rtnl_lock();
> +
> +	/* bluetooth handles that event type as int,
> +	 * but there is no overflow yet.
> +	 */
> +	switch (event) {
> +	case HCI_DEV_UNREG:
> +		lowpan_btle_hci_unregister(hdev);
> +		ret = NOTIFY_DONE;
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	rtnl_unlock();
> +
> +	return ret;
> +}
> +
> +static struct notifier_block lowpan_hcI_dev_notifier = {
> +	.notifier_call = lowpan_hci_dev_event,
> +};
> +
> +static ssize_t lowpan_enable_write(struct file *fp,
> +				   const char __user *user_buffer,
> +				   size_t count, loff_t *position)
> +{
> +	char buf[32] = { };
> +	size_t buf_size = min(count, sizeof(buf) - 1);
> +	struct hci_dev *hdev;
> +	int idx, enabled, n, ret;
> +
> +	if (copy_from_user(buf, user_buffer, buf_size))
> +		return -EFAULT;
> +
> +	n = sscanf(buf, "hci%d %d", &idx, &enabled);
> +	if (n != 2)
> +		return -EINVAL;
> +
> +	hdev = hci_dev_get(idx);
> +	if (!hdev)
> +		return -EINVAL;
> +
> +	rtnl_lock();
> +	ret = lowpan_enable_set(hdev, enabled);
> +	rtnl_unlock();
> +
> +	hci_dev_put(hdev);
> +
> +	return count;
> +}
> +
> +static int lowpan_enable_show(struct seq_file *f, void *ptr)
> +{
> +	struct hci_dev *hdev;
> +
> +	rtnl_lock();
> +	read_lock(&hci_dev_list_lock);
> +	list_for_each_entry(hdev, &hci_dev_list, list) {
> +		if (hdev->lowpan_enable)
> +			seq_printf(f, "hci%d -> %s\n", hdev->id,
> +				   hdev->ldev->name);
> +	}
> +	read_unlock(&hci_dev_list_lock);
> +	rtnl_unlock();
> +
> +	return 0;
> +}
> +
> +static int lowpan_enable_open(struct inode *inode, struct file *file)
> +{
> +	return single_open(file, lowpan_enable_show, inode->i_private);
> +}
> +
> +static const struct file_operations lowpan_enable_fops = {
> +	.open		= lowpan_enable_open,
> +	.read		= seq_read,
> +	.write		= lowpan_enable_write,
> +	.llseek		= seq_lseek,
> +	.release	= single_release,
> +};
> +
> +static int __init bt_6lowpan_init(void)
> +{
> +	int err;
> +
> +	workqueue_cleanup = create_singlethread_workqueue("btle 6lowpan");
> +	if (!workqueue_cleanup)
> +		return -ENOMEM;
> +
> +	debugfs_create_file("6lowpan_enable", 0644,  bt_debugfs, NULL,
> +			    &lowpan_enable_fops);
> +
> +	err = register_hci_dev_notifier(&lowpan_hcI_dev_notifier);
> +	if (err < 0)
> +		destroy_workqueue(workqueue_cleanup);
> +
> +	return err;
> +}
> +
> +static void __exit bt_6lowpan_exit(void)
> +{
> +	destroy_workqueue(workqueue_cleanup);
> +	unregister_hci_dev_notifier(&lowpan_hcI_dev_notifier);
> +}
> +
> +module_init(bt_6lowpan_init);
> +module_exit(bt_6lowpan_exit);
> +
> +MODULE_AUTHOR("Jukka Rissanen <jukka.rissanen@linux.intel.com>");
> +MODULE_DESCRIPTION("Bluetooth 6LoWPAN");
> +MODULE_VERSION(LOWPAN_BTLE_VERSION);
> +MODULE_LICENSE("GPL");
> diff --git a/net/bluetooth/Makefile b/net/bluetooth/Makefile
> index e347828..b3ff12e 100644
> --- a/net/bluetooth/Makefile
> +++ b/net/bluetooth/Makefile
> @@ -7,6 +7,9 @@ obj-$(CONFIG_BT_RFCOMM)	+= rfcomm/
>  obj-$(CONFIG_BT_BNEP)	+= bnep/
>  obj-$(CONFIG_BT_CMTP)	+= cmtp/
>  obj-$(CONFIG_BT_HIDP)	+= hidp/
> +obj-$(CONFIG_BT_6LOWPAN) += bluetooth_6lowpan.o
> +
> +bluetooth_6lowpan-y := 6lowpan.o
>  
>  bluetooth-y := af_bluetooth.o hci_core.o hci_conn.o hci_event.o mgmt.o \
>  	hci_sock.o hci_sysfs.o l2cap_core.o l2cap_sock.o smp.o lib.o \
> 

- Alex
--
To unsubscribe from this list: send the line "unsubscribe linux-wpan" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Luiz Augusto von Dentz July 14, 2016, 11:40 a.m. UTC | #2
Hi Alex,

On Wed, Jul 13, 2016 at 12:19 AM, Alexander Aring <aar@pengutronix.de> wrote:
>
> Hi,
>
> On 07/11/2016 09:50 PM, Alexander Aring wrote:
>> This patch adds a new btle 6lowpan implementation in version 0.2. This
>> new implementation has a new userspace API for debugfs.
>>
>> It's now possible to decide on which hci interface do you want to run
>> your 6LoWPAN interface, for that you can run:
>>
>> echo "hci0 1" > /sys/kernel/debug/bluetooth/6lowpan_enable
>>
>> to enable, or:
>>
>> echo "hci0 0" > /sys/kernel/debug/bluetooth/6lowpan_enable

No please no more designs for debugfs, we are past that and moving
towards having proper management commands, so what you did here is a
huge waste of time and you have instead just ask before hand.

>> to disable it. A cat on this file will show the current hci <-> 6lowpan
>> interface mapping.
>>
>> Additional the different is, that the interface will be created after
>> enable it. The old behaviour is that the interface will be created when
>> one first connection is established and deleted after all connections
>> (peers) will be disconnected. This handling is different now.
>>
>> The reason (in my opinion) is that the existing of such interface should
>> not based on if there is a connection or not. At runtime of many IPv6
>> application an interface will be removed -> I think the most userspace
>> application can and will never handle that an IPv6 interface will be
>> deleted and recreating during runtime.
>>
>> The new handling is that the interface will not removed and you can
>> re-establish the connection to your peers.
>>
>> Connection to peers:
>>
>> Connection to peers are not binded to hci interface, it's binded to your
>> 6LoWPAN interface. This will prepare handling for multiple 6LoWPAN
>> interfaces on one hci (maybe also with netns support, which isn't
>> available yet) and you can choose which peers should be handled by
>> different BTLE 6LoWPAN interfaces. Example:
>>
>> L2: hci0 is connected to node A, B, C, D
>>
>> L3: 6lo0 and 6lo1 are two interfaces based on hci0.
>>     6lo0 is connected to nodes: A, B, C
>>     6lo1 is connected to nodes: D, C, B

Is this is useful for what exactly? I much rather have a simple module
that just deal with 1 connection than have to deal with complex
topology in the beginning, so this is moving in the exact opposite
direction.

>> as one possible example. Handle this peer interface will have a possible
>> more fine-granularity connection for peers.

Not useful when we most likely we will not have that many peers, this
is no Wifi or ethernet, this is for very embedded devices lets not
over engineer here please.

>> To connect to peers, there exists no "6lowpan_control" anymore. The
>> control file is now peer interface, it's simple:
>>
>> echo "connect $PEER_ADDRESS 1" > /sys/kernel/debug/bluetooth/6lo0
>>
>> Also after connecting to peers it's still possible that another peer can
>> to connect to the peer which has already a connected peer (complicated
>> to describe). I saw that is somehow disabled in version before, see:
>>
>> http://lxr.free-electrons.com/source/net/bluetooth/6lowpan.c#L1247
>>
>> I am not a bluetooth expert, but I guess that is somehow if there exists
>> limitation to devices which are not "peers" that means devices which can
>> be run as master or slave only. So far I know such limitations for
>> devices doesn't exists for BTLE 6LoWPAN, or?
>>

I guess that is because dual role is not specified, you either are a
node or a router but not both at the same time.

> I think the following will explain somehow when it's a SLAVE and when
> MASTER as my current view.
>
> SLAVES:
>
> They don't makes this echo "connect ..." foo in control files.
> they only do "hciconfig hci0 leadv" that somebody other can connect to
> that node (some MASTER node).
>
> MASTERS:
>
> can do the same what the SLAVES does but to be as MASTER they need to call
> additional stuff:
>
> hcitool lecc $SLAVE_ADDR (one or more times)

Don't use hcitool, actually never mention any of hci related
interfaces on the patches there are deprecated, btmgmt shall be used
to enable things like advertising, I will be working on adding
advertising support for bluetoothctl but we actually need IPSP UUID
support in order to make this work properly so not even btmgmt will be
enough in the end.

> Additional for different lowpan interfaces (don't need the same addresses
> which was used by previous one or more hcitool lecc calls):
>
> echo "connect SLAVE_ADDR 1" > /sys/kernel/debug/bluetooth/6lo0
>
> is this a correct view and can all BTLE transceivers run as SLAVE or
> MASTER, or exists there some special transceivers outside which can be
> run as SLAVE only?
>
>> Nevertheless I disabled that stuff, it's still possible to connect to
>> some node which already run "connect ..." on his side.
>>
>> Otherwise I fixed the ndisc stuff and removed a lot of races/side
>> effects. The old code had some static "lowpan_devices" list and most
>> time there was some lookup mapping according to bdaddr to get the right
>> 6lo netdevice struct. I removed that, also the skb->cb is used a lot in
>> some callback which had some side-effects.

Send the fixes, and the fixes only.

>> Signed-off-by: Alexander Aring <aar@pengutronix.de>
>> ---
>>  include/net/bluetooth/hci_core.h |   10 +
>>  net/bluetooth/6lowpan.c          | 1015 ++++++++++++++++++++++++++++++++++++++
>>  net/bluetooth/Makefile           |    3 +
>>  3 files changed, 1028 insertions(+)
>>  create mode 100644 net/bluetooth/6lowpan.c
>>
>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
>> index a37027a..37a2256 100644
>> --- a/include/net/bluetooth/hci_core.h
>> +++ b/include/net/bluetooth/hci_core.h
>> @@ -399,6 +399,16 @@ struct hci_dev {
>>
>>       struct led_trigger      *power_led;
>>
>> +#if IS_ENABLED(CONFIG_BT_6LOWPAN)
>> +     /* TODO for netns support this need to be a list
>> +      * variables are protected by RTNL here
>> +      */
>> +     struct net_device *ldev;
>> +     bool lowpan_enable;
>> +     /* delete lowpan iface via debugfs workaround */
>> +     bool pending_deletion;
>> +#endif
>> +
>>       int (*open)(struct hci_dev *hdev);
>>       int (*close)(struct hci_dev *hdev);
>>       int (*flush)(struct hci_dev *hdev);
>> diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
>> new file mode 100644
>> index 0000000..a8a3410
>> --- /dev/null
>> +++ b/net/bluetooth/6lowpan.c
>> @@ -0,0 +1,1015 @@
>> +/*
>> +   (C) 2016 Pengutronix, Alexander Aring <aar@pengutronix.de>
>> +   Copyright (c) 2013-2014 Intel Corp.
>> +
>> +   This program is free software; you can redistribute it and/or modify
>> +   it under the terms of the GNU General Public License version 2 and
>> +   only version 2 as published by the Free Software Foundation.
>> +
>> +   This program is distributed in the hope that it will be useful,
>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +   GNU General Public License for more details.
>> +*/
>> +
>> +#include <linux/if_arp.h>
>> +#include <linux/netdevice.h>
>> +#include <linux/etherdevice.h>
>> +#include <linux/module.h>
>> +#include <linux/debugfs.h>
>> +
>> +#include <net/ipv6.h>
>> +#include <net/ip6_route.h>
>> +#include <net/addrconf.h>
>> +
>> +#include <net/bluetooth/bluetooth.h>
>> +#include <net/bluetooth/hci_core.h>
>> +#include <net/bluetooth/l2cap.h>
>> +
>> +#include <net/6lowpan.h>
>> +
>> +#define LOWPAN_BTLE_VERSION "0.2"
>> +
>> +#define lowpan_le48_to_be48(dst, src) baswap((bdaddr_t *)dst, (bdaddr_t *)src)
>> +#define lowpan_be48_to_le48(dst, src) baswap((bdaddr_t *)dst, (bdaddr_t *)src)
>> +
>> +struct lowpan_btle_cb {
>> +     struct l2cap_chan *chan;
>> +};
>> +
>> +struct lowpan_btle_dev {
>> +     struct hci_dev *hdev;
>> +     struct workqueue_struct *workqueue;
>> +     struct l2cap_chan *listen;
>> +     struct list_head peers;
>> +     struct dentry *control;
>> +};
>> +
>> +struct lowpan_peer {
>> +     struct l2cap_chan *chan;
>> +
>> +     struct list_head list;
>> +};
>> +
>> +struct lowpan_chan_data {
>> +     struct lowpan_peer peer;
>> +     struct net_device *dev;
>> +};
>> +
>> +struct lowpan_xmit_work {
>> +     struct work_struct work;
>> +     struct l2cap_chan *chan;
>> +     struct net_device *dev;
>> +     unsigned int true_len;
>> +     struct sk_buff *skb;
>> +};
>> +
>> +static inline struct lowpan_btle_cb *
>> +lowpan_btle_cb(const struct sk_buff *skb)
>> +{
>> +     return (struct lowpan_btle_cb *)skb->cb;
>> +}
>> +
>> +static inline struct lowpan_chan_data *
>> +lowpan_chan_data(const struct l2cap_chan *chan)
>> +{
>> +     return (struct lowpan_chan_data *)chan->data;
>> +}
>> +
>> +static inline struct lowpan_btle_dev *
>> +lowpan_btle_dev(const struct net_device *dev)
>> +{
>> +     return (struct lowpan_btle_dev *)lowpan_dev(dev)->priv;
>> +}
>> +
>> +static inline struct lowpan_peer *
>> +lowpan_lookup_peer(struct lowpan_btle_dev *btdev, bdaddr_t *addr)
>> +{
>> +     struct lowpan_peer *entry;
>> +
>> +     list_for_each_entry_rcu(entry, &btdev->peers, list) {
>> +             if (!bacmp(&entry->chan->dst, addr))
>> +                     return entry;
>> +     }
>> +
>> +     return NULL;
>> +}
>> +
>> +static int lowpan_give_skb_to_device(struct sk_buff *skb)
>> +{
>> +     skb->protocol = htons(ETH_P_IPV6);
>> +     skb->dev->stats.rx_packets++;
>> +     skb->dev->stats.rx_bytes += skb->len;
>> +
>> +     return netif_rx_ni(skb);
>> +}
>> +
>> +static int lowpan_rx_handlers_result(struct sk_buff *skb, lowpan_rx_result res)
>> +{
>> +     switch (res) {
>> +     case RX_CONTINUE:
>> +             /* nobody cared about this packet */
>> +             net_warn_ratelimited("%s: received unknown dispatch\n",
>> +                                  __func__);
>
> that printout isn't corrected, for simplification I drop all non iphc
> frames at recv callback.
>
> I want to have this receive handling into "net/6lowpan" but I want to
> not doing this now in this patch-series. It will prepare it.
>
> After moving it to "net/6lowpan" unknown dispatches should be really
> dispatches which are not known in the current state of implementation.
>
> I think it's okay to leave this right there as it is, also somebody can
> trigger this warning by sending lot of frames with the right dispatch,
> not sure if that's normal that somebody from outside can trigger a lot
> of system messages which are show up in dmesg. Need to think about it,
> maybe some "ONCE" mechanism or using debug stuff.
>
>> +
>> +             /* fall-through */
>> +     case RX_DROP_UNUSABLE:
>> +             kfree_skb(skb);
>> +
>> +             /* fall-through */
>> +     case RX_DROP:
>> +             return NET_RX_DROP;
>> +     case RX_QUEUED:
>> +             return lowpan_give_skb_to_device(skb);
>> +     default:
>> +             break;
>> +     }
>> +
>> +     return NET_RX_DROP;
>> +}
>> +
>> +static lowpan_rx_result lowpan_rx_h_iphc(struct sk_buff *skb)
>> +{
>> +     struct l2cap_chan *chan = lowpan_btle_cb(skb)->chan;
>> +     bdaddr_t daddr, saddr;
>> +     int ret;
>> +
>> +     if (!lowpan_is_iphc(*skb_network_header(skb)))
>> +             return RX_CONTINUE;
>> +
>> +     BT_DBG("recv %s dst: %pMR type %d src: %pMR chan %p",
>> +            skb->dev->name, &chan->dst, chan->dst_type, &chan->src, chan);
>> +
>> +     /* bluetooth chan view is vice-versa */
>> +     bacpy(&daddr, &chan->src);
>> +     bacpy(&saddr, &chan->dst);
>> +
>> +     ret = lowpan_header_decompress(skb, skb->dev, &daddr, &saddr);
>> +     if (ret < 0)
>> +             return RX_DROP_UNUSABLE;
>> +
>> +     return RX_QUEUED;
>> +}
>> +
>> +static int lowpan_invoke_rx_handlers(struct sk_buff *skb)
>> +{
>> +     lowpan_rx_result res;
>> +
>> +#define CALL_RXH(rxh)                        \
>> +     do {                            \
>> +             res = rxh(skb);         \
>> +             if (res != RX_CONTINUE) \
>> +                     goto rxh_next;  \
>> +     } while (0)
>> +
>> +     /* likely at first */
>> +     CALL_RXH(lowpan_rx_h_iphc);
>> +
>> +rxh_next:
>> +     return lowpan_rx_handlers_result(skb, res);
>> +#undef CALL_RXH
>> +}
>> +
>> +static int lowpan_chan_recv(struct l2cap_chan *chan, struct sk_buff *skb)
>> +{
>> +     struct lowpan_chan_data *data = lowpan_chan_data(chan);
>> +     struct net_device *dev = data->dev;
>> +     int ret;
>> +
>> +     /* TODO handle BT_CONNECTED in bluetooth subsytem? on
>> +      * when calling recv callback, I hit that case somehow
>> +      */
>> +     if (!netif_running(dev) || chan->state != BT_CONNECTED ||
>> +         !skb->len || !lowpan_is_iphc(skb->data[0]))
>> +             goto drop;
>> +
>> +     /* Replacing skb->dev and followed rx handlers will manipulate skb. */
>> +     skb = skb_unshare(skb, GFP_ATOMIC);
>
> can be GFP_KERNEL here.
>
>> +     if (!skb)
>> +             goto out;
>> +
>> +     skb->dev = dev;
>> +     skb_reset_network_header(skb);
>> +
>> +     /* remember that one for dst bdaddr. TODO handle that as priv data for
>> +      * lowpan_invoke_rx_handlers parameter. Not necessary for skb->cb.
>> +      */
>
> After fixing that, skb->cb isn't needed anymore in this implementation.
> But I will do that when moving to receive handling to "net/6lowpan".
>
>> +     lowpan_btle_cb(skb)->chan = chan;
>> +
>> +     ret = lowpan_invoke_rx_handlers(skb);
>> +     if (ret == NET_RX_DROP)
>> +             BT_DBG("recv %s dropped chan %p", skb->dev->name, chan);
>> +
>> +     return 0;
>> +
>> +drop:
>> +     kfree_skb(skb);
>> +out:
>> +     /* we handle to free skb on error, so must 0
>> +      * seems that callback free the skb on error
>> +      * otherwise.
>> +      */
>
> btw:
>
> This is also a MUST to return 0 here always. Because netif_rx_ni will handle
> also freeing on error and we cannot change this handling. (Or we doing
> some crazy reference counting before foo, but will more confuse
> everybody).
>
>> +     return 0;
>> +}
>> +
>> +static void lowpan_xmit_worker(struct work_struct *work)
>> +{
>> +     struct lowpan_btle_dev *btdev;
>> +     struct lowpan_xmit_work *xw;
>> +     struct l2cap_chan *chan;
>> +     struct net_device *dev;
>> +     struct msghdr msg = { };
>> +     struct kvec iv;
>> +     int ret;
>> +
>> +     xw = container_of(work, struct lowpan_xmit_work, work);
>> +     dev = xw->dev;
>> +     chan = xw->chan;
>> +     btdev = lowpan_btle_dev(dev);
>> +
>> +     iv.iov_base = xw->skb->data;
>> +     iv.iov_len = xw->skb->len;
>> +     iov_iter_kvec(&msg.msg_iter, WRITE | ITER_KVEC, &iv, 1, xw->skb->len);
>> +
>> +     BT_DBG("l2cap_chan_send %s dst: %pMR type %d src: %pMR chan %p",
>> +            dev->name, &chan->dst, chan->dst_type, &chan->src, chan);
>> +
>> +     l2cap_chan_lock(chan);
>> +
>> +     ret = l2cap_chan_send(chan, &msg, xw->skb->len);
>> +     BT_DBG("transmit return value %d", ret);
>> +     if (ret < 0) {
>> +             BT_DBG("send %s failed chan %p", dev->name, chan);
>> +             kfree_skb(xw->skb);
>> +     } else {
>> +             consume_skb(xw->skb);
>> +             dev->stats.tx_bytes += xw->true_len;
>> +             dev->stats.tx_packets++;
>> +     }
>> +
>> +     l2cap_chan_unlock(chan);
>> +     l2cap_chan_put(chan);
>> +
>> +     kfree(xw);
>> +}
>> +
>> +static void lowpan_send_unicast_pkt(struct net_device *dev,
>> +                                 struct l2cap_chan *chan,
>> +                                 struct sk_buff *skb,
>> +                                 unsigned int true_len)
>> +{
>> +     struct lowpan_xmit_work *xw;
>> +
>> +     /* copy to xmit work buffer */
>> +     xw = kzalloc(sizeof(*xw), GFP_ATOMIC);
>> +     if (!xw)
>> +             return;
>> +
>> +     /* chan->lock mutex need to be hold so change context to workqueue */
>> +     INIT_WORK(&xw->work, lowpan_xmit_worker);
>> +     xw->true_len = true_len;
>> +     /* freeing protected by ifdown workqueue sync */
>> +     xw->dev = dev;
>> +     /* disallow freeing of skb while context switch */
>> +     xw->skb = skb_get(skb);
>> +     /* disallow freeing while context switch */
>> +     l2cap_chan_hold(chan);
>> +     xw->chan = chan;
>> +
>> +     queue_work(lowpan_btle_dev(dev)->workqueue, &xw->work);
>> +}
>> +
>> +static void lowpan_send_mcast_pkt(struct net_device *dev, struct sk_buff *skb,
>> +                               unsigned int true_len)
>> +{
>> +     struct lowpan_btle_dev *btdev = lowpan_btle_dev(dev);
>> +     struct lowpan_peer *peer;
>> +
>> +     rcu_read_lock();
>> +
>> +     BT_DBG("xmit %s starts multicasting", dev->name);
>> +
>> +     /* We need to send the packet to every device behind this
>> +      * interface, because multicasting.
>> +      */
>> +     list_for_each_entry_rcu(peer, &btdev->peers, list)
>> +             lowpan_send_unicast_pkt(dev, peer->chan, skb, true_len);
>> +
>> +     rcu_read_unlock();
>> +}
>> +
>> +static netdev_tx_t lowpan_xmit(struct sk_buff *skb, struct net_device *dev)
>> +{
>> +     struct lowpan_addr_info *info = lowpan_addr_info(skb);
>> +     struct lowpan_btle_dev *btdev = lowpan_btle_dev(dev);
>> +     unsigned int true_len = skb->len;
>> +     struct lowpan_peer *peer;
>> +     bdaddr_t daddr, saddr;
>> +     int ret;
>> +
>> +     /* We must take a copy of the skb before we modify/replace the ipv6
>> +      * header as the header could be used elsewhere.
>> +      */
>> +     skb = skb_unshare(skb, GFP_ATOMIC);
>> +     if (!skb) {
>> +             kfree_skb(skb);
>> +             goto out;
>> +     }
>> +
>> +     lowpan_be48_to_le48(&daddr, info->daddr);
>> +     lowpan_be48_to_le48(&saddr, info->saddr);
>> +
>> +     BT_DBG("xmit ndisc %s dst: %pMR src: %pMR",
>> +            dev->name, &daddr, &saddr);
>> +
>> +     ret = lowpan_header_compress(skb, dev, &daddr, &saddr);
>> +     if (ret < 0) {
>> +             kfree_skb(skb);
>> +             goto out;
>> +     }
>> +
>> +     /* this should never be the case, otherwise iphc is broken */
>> +     WARN_ON_ONCE(skb->len > dev->mtu);
>> +
>> +     if (!memcmp(&daddr, dev->broadcast, dev->addr_len)) {
>> +             lowpan_send_mcast_pkt(dev, skb, true_len);
>> +     } else {
>> +             btdev = lowpan_btle_dev(dev);
>> +
>
> btdev = can be removed here.
>
>> +             rcu_read_lock();
>> +
>> +             peer = lowpan_lookup_peer(btdev, &daddr);
>> +             if (peer)
>> +                     lowpan_send_unicast_pkt(dev, peer->chan, skb,
>> +                                             true_len);
>> +
>> +             rcu_read_unlock();
>> +     }
>> +
>> +     consume_skb(skb);
>> +
>> +out:
>> +     return NETDEV_TX_OK;
>> +}
>> +
>> +static int lowpan_stop(struct net_device *dev)
>> +{
>> +     struct lowpan_btle_dev *btdev = lowpan_btle_dev(dev);
>> +
>> +     /* synchronize with xmit worker */
>> +     flush_workqueue(btdev->workqueue);
>> +     return 0;
>> +}
>> +
>> +static struct sk_buff *lowpan_chan_alloc_skb(struct l2cap_chan *chan,
>> +                                          unsigned long hdr_len,
>> +                                          unsigned long len, int nb)
>> +{
>> +     return bt_skb_alloc(hdr_len + len, GFP_KERNEL);
>> +}
>> +
>> +static long lowpan_chan_get_sndtimeo(struct l2cap_chan *chan)
>> +{
>> +     return L2CAP_CONN_TIMEOUT;
>> +}
>> +
>> +static struct l2cap_chan *lowpan_chan_create(struct net_device *dev)
>> +{
>> +     struct lowpan_chan_data *data;
>> +     struct l2cap_chan *chan;
>> +
>> +     chan = l2cap_chan_create();
>> +     if (!chan)
>> +             return ERR_PTR(-ENOMEM);
>> +
>> +     data = kmalloc(sizeof(*data), GFP_KERNEL);
>> +     if (!data) {
>> +             l2cap_chan_put(chan);
>> +             return ERR_PTR(-ENOMEM);
>> +     }
>> +
>> +     l2cap_chan_set_defaults(chan);
>> +     chan->chan_type = L2CAP_CHAN_CONN_ORIENTED;
>> +     chan->mode = L2CAP_MODE_LE_FLOWCTL;
>> +     chan->imtu = dev->mtu;
>> +     chan->data = data;
>> +
>> +     data->peer.chan = chan;
>> +     data->dev = dev;
>> +
>> +     return chan;
>> +}
>> +
>> +static struct l2cap_chan *lowpan_chan_new_conn(struct l2cap_chan *pchan)
>> +{
>> +     struct lowpan_chan_data *data = lowpan_chan_data(pchan);
>> +     struct l2cap_chan *chan;
>> +
>> +     chan = lowpan_chan_create(data->dev);
>> +     if (IS_ERR(chan))
>> +             return NULL;
>> +
>> +     chan->ops = pchan->ops;
>> +     return chan;
>> +}
>> +
>> +static void lowpan_chan_ready(struct l2cap_chan *chan)
>> +{
>> +     struct lowpan_chan_data *data = lowpan_chan_data(chan);
>> +     struct lowpan_btle_dev *btdev = lowpan_btle_dev(data->dev);
>> +
>> +     BT_DBG("%s chan %p ready ", data->dev->name, chan);
>> +
>> +     /* make it visible for xmit */
>> +     list_add_tail_rcu(&data->peer.list, &btdev->peers);
>> +     synchronize_rcu();
>> +}
>> +
>> +static void lowpan_chan_close(struct l2cap_chan *chan)
>> +{
>> +     struct lowpan_chan_data *data = lowpan_chan_data(chan);
>> +
>> +     BT_DBG("%s chan %p closed ", data->dev->name, chan);
>> +
>> +     /* make it unvisible for xmit */
>> +     list_del_rcu(&data->peer.list);
>> +     synchronize_rcu();
>> +     kfree(data);
>
> I currently think much about to run kfree here and I think I need
> bluetooth experts to answer the correct handling here.
>
> We allocate that data on chan_create, but doing a free in chan_close
> requires that the chan state system will never run chan_ready
> afterwards. Is this correct?
>
> Correct handling for me is to do the free when the last chan_put was
> called and free everything. Maybe it could be useful to add some
> private_data _room_ to l2cap_chan_create, which sits after "struct
> l2cap_chan" as something like "u8 data[0]", to allocate/free everything
> at once.
>
> I didn't hit any issues yet that after a close, the ready callback will
> be called again so I think this can never happen.
>
> btw:
>
> previous implementation had kfree_rcu here, but we run synchronize_rcu
> before and this is not needed anymore. I think kfree_rcu will not block,
> but I feeling better to not leave that callback after running
> synchronize_rcu. :-/
>
>> +}
>> +
>> +static const struct l2cap_ops lowpan_chan_ops = {
>> +     .name                   = "L2CAP 6LoWPAN channel",
>> +     .new_connection         = lowpan_chan_new_conn,
>> +     .recv                   = lowpan_chan_recv,
>> +     .close                  = lowpan_chan_close,
>> +     .state_change           = l2cap_chan_no_state_change,
>> +     .ready                  = lowpan_chan_ready,
>> +     .get_sndtimeo           = lowpan_chan_get_sndtimeo,
>> +     .alloc_skb              = lowpan_chan_alloc_skb,
>> +     .teardown               = l2cap_chan_no_teardown,
>> +     .defer                  = l2cap_chan_no_defer,
>> +     .set_shutdown           = l2cap_chan_no_set_shutdown,
>> +     .resume                 = l2cap_chan_no_resume,
>> +     .suspend                = l2cap_chan_no_suspend,
>> +};
>> +
>> +static int lowpan_change_mtu(struct net_device *dev, int new_mtu)
>> +{
>> +     struct lowpan_btle_dev *btdev = lowpan_btle_dev(dev);
>> +
>> +     /* if dev is down, peers list are protected by RTNL */
>> +     if (netif_running(dev) || !list_empty(&btdev->peers))
>> +             return -EBUSY;
>> +
>> +     if (new_mtu < IPV6_MIN_MTU)
>> +             return -EINVAL;
>> +
>> +     dev->mtu = new_mtu;
>> +     return 0;
>> +}
>> +
>> +static const struct net_device_ops netdev_ops = {
>> +     .ndo_init               = lowpan_dev_init,
>> +     .ndo_stop               = lowpan_stop,
>> +     .ndo_start_xmit         = lowpan_xmit,
>> +     .ndo_change_mtu         = lowpan_change_mtu,
>> +};
>> +
>> +static void lowpan_free_netdev(struct net_device *dev)
>> +{
>> +     struct lowpan_btle_dev *btdev = lowpan_btle_dev(dev);
>> +     struct lowpan_peer *peer, *tmp;
>> +     struct l2cap_chan *chan;
>> +
>> +     l2cap_chan_close(btdev->listen, 0);
>> +     l2cap_chan_put(btdev->listen);
>> +
>> +     /* no rcu here? should be safe netdev is down */
>> +     list_for_each_entry_safe(peer, tmp, &btdev->peers, list) {
>> +             /* close will free peer data, so save chan here */
>> +             chan = peer->chan;
>> +
>> +             /* will del peer from list and and free.
>> +              * should be safe by tmp pointer which has
>> +              * a pointer for the next entry.
>> +              */
>> +             l2cap_chan_close(chan, 0);
>> +             l2cap_chan_put(chan);
>> +     }
>> +
>> +     destroy_workqueue(btdev->workqueue);
>> +     btdev->hdev->lowpan_enable = false;
>> +     debugfs_remove(btdev->control);
>> +     hci_dev_put(btdev->hdev);
>> +}
>> +
>> +static void lowpan_setup(struct net_device *dev)
>> +{
>> +     memset(dev->broadcast, 0xff, sizeof(bdaddr_t));
>> +
>> +     dev->netdev_ops = &netdev_ops;
>> +     dev->destructor = lowpan_free_netdev;
>> +     dev->features   |= NETIF_F_NETNS_LOCAL;
>> +}
>> +
>> +static struct device_type bt_type = {
>> +     .name   = "bluetooth",
>> +};
>> +
>> +static struct l2cap_chan *lowpan_listen_chan_new_conn(struct l2cap_chan *pchan)
>> +{
>> +     struct l2cap_chan *chan;
>> +
>> +     chan = lowpan_chan_create(pchan->data);
>> +     if (IS_ERR(chan))
>> +             return NULL;
>> +
>> +     /* change ops with more functionality than listen,
>> +      * which also free chan->data stuff.
>> +      */
>> +     chan->ops = &lowpan_chan_ops;
>> +
>> +     return chan;
>> +}
>> +
>> +static const struct l2cap_ops lowpan_listen_chan_ops = {
>> +     .name                   = "L2CAP 6LoWPAN listen channel",
>> +     .new_connection         = lowpan_listen_chan_new_conn,
>> +     .recv                   = l2cap_chan_no_recv,
>> +     .close                  = l2cap_chan_no_close,
>> +     .state_change           = l2cap_chan_no_state_change,
>> +     .ready                  = l2cap_chan_no_ready,
>> +     .get_sndtimeo           = l2cap_chan_no_get_sndtimeo,
>> +     .alloc_skb              = l2cap_chan_no_alloc_skb,
>> +     .teardown               = l2cap_chan_no_teardown,
>> +     .defer                  = l2cap_chan_no_defer,
>> +     .set_shutdown           = l2cap_chan_no_set_shutdown,
>> +     .resume                 = l2cap_chan_no_resume,
>> +     .suspend                = l2cap_chan_no_suspend,
>> +};
>> +
>> +static int lowpan_create_listen_chan(struct net_device *dev)
>> +{
>> +     struct lowpan_btle_dev *btdev = lowpan_btle_dev(dev);
>> +     struct l2cap_chan *chan;
>> +     u8 bdaddr_type;
>> +     int ret;
>> +
>> +     /* don't use lowpan_chan_create here, because less functionality */
>> +     chan = l2cap_chan_create();
>> +     if (!chan)
>> +             return -ENOMEM;
>> +
>> +     chan->data = dev;
>
> I will add a comment here that this private_data of chan is really
> "struct net_device *dev" only, it's for listen stuff only.
>
> Looks wrong at the first sight.
>
>> +     chan->ops = &lowpan_listen_chan_ops;
>> +     hci_copy_identity_address(btdev->hdev, &chan->src, &bdaddr_type);
>> +     if (bdaddr_type == ADDR_LE_DEV_PUBLIC)
>> +             chan->src_type = BDADDR_LE_PUBLIC;
>> +     else
>> +             chan->src_type = BDADDR_LE_RANDOM;
>> +
>> +     chan->state = BT_LISTEN;
>> +     atomic_set(&chan->nesting, L2CAP_NESTING_PARENT);
>> +
>> +     BT_DBG("chan %p src type %d", chan, chan->src_type);
>> +     ret = l2cap_add_psm(chan, BDADDR_ANY, cpu_to_le16(L2CAP_PSM_IPSP));
>> +     if (ret) {
>> +             l2cap_chan_put(chan);
>> +             BT_ERR("psm cannot be added err %d", ret);
>> +             return ret;
>> +     }
>> +     btdev->listen = chan;
>> +
>> +     return 0;
>> +}
>> +
>> +static const struct file_operations lowpan_control_fops;
>> +
>> +static struct net_device *lowpan_btle_newlink(struct hci_dev *hdev)
>> +{
>> +     struct lowpan_btle_dev *btdev;
>> +     struct net_device *dev;
>> +     int err = -ENOMEM;
>> +
>> +     dev = alloc_netdev(LOWPAN_PRIV_SIZE(sizeof(struct lowpan_btle_dev)),
>> +                        LOWPAN_IFNAME_TEMPLATE, NET_NAME_ENUM, lowpan_setup);
>> +     if (!dev)
>> +             goto out;
>> +
>> +     dev->addr_assign_type = NET_ADDR_PERM;
>> +     dev->addr_len = sizeof(hdev->bdaddr.b);
>> +     lowpan_le48_to_be48(dev->dev_addr, &hdev->bdaddr);
>> +
>> +     SET_NETDEV_DEV(dev, &hdev->dev);
>> +     SET_NETDEV_DEVTYPE(dev, &bt_type);
>> +
>> +     btdev = lowpan_btle_dev(dev);
>> +     /* avoid freeing */
>> +     btdev->hdev = hci_dev_hold(hdev);
>> +     INIT_LIST_HEAD(&btdev->peers);
>> +
>> +     btdev->workqueue = create_singlethread_workqueue(dev->name);
>> +     if (!btdev->workqueue) {
>> +             free_netdev(dev);
>> +             goto out;
>> +     }
>> +
>> +     err = lowpan_create_listen_chan(dev);
>> +     if (err < 0) {
>> +             destroy_workqueue(btdev->workqueue);
>> +             free_netdev(dev);
>> +             goto out;
>> +     }
>> +
>> +     err = lowpan_register_netdevice(dev, LOWPAN_LLTYPE_BTLE);
>> +     if (err < 0) {
>> +             BT_ERR("register_netdev failed %d", err);
>> +             l2cap_chan_close(btdev->listen, 0);
>> +             l2cap_chan_put(btdev->listen);
>> +             free_netdev(dev);
>> +             goto out;
>> +     }
>> +     hdev->ldev = dev;
>> +
>> +     /* ignore error handling here, we cannot call unregister anymore
>> +      * It's a bug, but it's debugfs... so ignore it.
>> +      */
>> +     btdev->control = debugfs_create_file(dev->name, 0644,
>> +                                          bt_debugfs, btdev,
>> +                                          &lowpan_control_fops);
>> +     if (!btdev->control)
>> +             BT_ERR("debugfs error for %s, disable/enable 6lowpan again",
>> +                    dev->name);
>> +
>> +     return dev;
>> +
>> +out:
>> +     return ERR_PTR(err);
>> +}
>> +
>> +static void lowpan_btle_dellink(struct net_device *dev)
>> +{
>> +     lowpan_unregister_netdevice(dev);
>> +}
>> +
>> +static int lowpan_parse_le_bdaddr(struct hci_dev *hdev, unsigned char *buf,
>> +                               bdaddr_t *addr, u8 *addr_type)
>> +{
>> +     int n;
>> +
>> +     n = sscanf(buf, "%hhx:%hhx:%hhx:%hhx:%hhx:%hhx %hhu",
>> +                &addr->b[5], &addr->b[4], &addr->b[3],
>> +                &addr->b[2], &addr->b[1], &addr->b[0],
>> +                addr_type);
>> +     if (n < 7)
>> +             return -EINVAL;
>> +
>> +     /* check if we handle le addresses and not own source address */
>> +     if (!bdaddr_type_is_le(*addr_type) ||
>> +         !bacmp(&hdev->bdaddr, addr))
>> +             return -EINVAL;
>> +
>> +     return n;
>> +}
>> +
>> +static ssize_t __lowpan_control_write(struct file *fp,
>> +                                   const char __user *user_buffer,
>> +                                   size_t count, loff_t *position)
>> +{
>> +     char buf[32] = { };
>> +     size_t buf_size = min(count, sizeof(buf) - 1);
>> +     struct seq_file *file = fp->private_data;
>> +     struct lowpan_btle_dev *btdev = file->private;
>> +     struct hci_dev *hdev = btdev->hdev;
>> +     struct lowpan_peer *peer;
>> +     /* slave address */
>> +     bdaddr_t addr;
>> +     u8 addr_type;
>> +     int ret;
>> +
>> +     if (copy_from_user(buf, user_buffer, buf_size))
>> +             return -EFAULT;
>> +
>> +     if (memcmp(buf, "connect ", 8) == 0) {
>> +             struct lowpan_peer *peer;
>
> peer can be removed here.
>
>> +             struct l2cap_chan *chan;
>> +
>> +             ret = lowpan_parse_le_bdaddr(hdev, &buf[8], &addr, &addr_type);
>> +             if (ret < 0)
>> +                     return ret;
>> +
>> +             /* check if we already know that slave */
>> +             rcu_read_lock();
>> +             peer = lowpan_lookup_peer(btdev, &addr);
>> +             if (peer) {
>> +                     rcu_read_unlock();
>> +                     BT_DBG("6LoWPAN connection already exists");
>> +                     return -EEXIST;
>> +             }
>> +             rcu_read_unlock();
>> +
>> +             chan = lowpan_chan_create(hdev->ldev);
>> +             if (IS_ERR(chan))
>> +                     return PTR_ERR(chan);
>> +             chan->ops = &lowpan_chan_ops;
>> +
>> +             ret = l2cap_hdev_chan_connect(hdev, chan,
>> +                                           cpu_to_le16(L2CAP_PSM_IPSP),
>> +                                           0, &addr, addr_type);
>> +             if (ret < 0) {
>> +                     l2cap_chan_put(chan);
>> +                     return ret;
>> +             }
>> +
>> +             return count;
>> +     } else if (memcmp(buf, "disconnect ", 11) == 0) {
>> +             ret = lowpan_parse_le_bdaddr(hdev, &buf[11], &addr, &addr_type);
>> +             if (ret < 0)
>> +                     return ret;
>> +
>> +             /* check if we don't know that slave */
>> +             rcu_read_lock();
>> +             peer = lowpan_lookup_peer(btdev, &addr);
>> +             if (!peer) {
>> +                     rcu_read_unlock();
>> +                     BT_DBG("6LoWPAN connection not found in peers");
>> +                     return -ENOENT;
>> +             }
>> +             rcu_read_unlock();
>> +
>> +             /* first delete the peer out of list,
>> +              * which makes it visiable to netdev,
>> +              * will be done by close callback.
>> +              */
>> +             l2cap_chan_close(peer->chan, 0);
>> +             l2cap_chan_put(peer->chan);
>> +     } else {
>> +             return -EINVAL;
>> +     }
>> +
>> +     return count;
>> +}
>> +
>> +static ssize_t lowpan_control_write(struct file *fp,
>> +                                 const char __user *user_buffer,
>> +                                 size_t count, loff_t *position)
>> +{
>> +     ssize_t ret;
>> +
>> +     rtnl_lock();
>> +     ret = __lowpan_control_write(fp, user_buffer, count, position);
>> +     rtnl_unlock();
>> +
>> +     return ret;
>> +}
>> +
>> +static int lowpan_control_show(struct seq_file *f, void *ptr)
>> +{
>> +     struct lowpan_btle_dev *btdev = f->private;
>> +     struct hci_dev *hdev = btdev->hdev;
>> +     struct lowpan_peer *peer;
>> +
>> +     rtnl_lock();
>> +
>> +     if (!hdev->lowpan_enable) {
>> +             rtnl_unlock();
>> +             return 0;
>> +     }
>> +
>> +     rcu_read_lock();
>> +
>> +     list_for_each_entry_rcu(peer, &btdev->peers, list) {
>> +             seq_printf(f, "%pMR (type %u) state: %s\n",
>> +                        &peer->chan->dst, peer->chan->dst_type,
>> +                        state_to_string(peer->chan->state));
>> +     }
>> +
>> +     rcu_read_unlock();
>> +
>> +     rtnl_unlock();
>> +     return 0;
>> +}
>> +
>> +static int lowpan_control_open(struct inode *inode, struct file *file)
>> +{
>> +     return single_open(file, lowpan_control_show, inode->i_private);
>> +}
>> +
>> +static const struct file_operations lowpan_control_fops = {
>> +     .open           = lowpan_control_open,
>> +     .read           = seq_read,
>> +     .write          = lowpan_control_write,
>> +     .llseek         = seq_lseek,
>> +     .release        = single_release,
>> +};
>> +
>> +/* TODO remove this when add non debugfs API, it's a workaround */
>> +static struct workqueue_struct *workqueue_cleanup;
>> +
>> +struct lowpan_cleanup_work {
>> +     struct work_struct work;
>> +     struct net_device *dev;
>> +};
>> +
>> +static void lowpan_cleanup_worker(struct work_struct *work)
>> +{
>> +     struct lowpan_cleanup_work *xw;
>> +
>> +     xw = container_of(work, struct lowpan_cleanup_work, work);
>> +
>> +     rtnl_lock();
>> +     lowpan_btle_dellink(xw->dev);
>> +     lowpan_btle_dev(xw->dev)->hdev->pending_deletion = false;
>> +     rtnl_unlock();
>> +
>> +     kfree(xw);
>> +}
>> +
>> +/* TODO workaround, debugfs doesn't like recursion remove while debugfs
>> + * handling, this should be removed for by using real UAPI.
>> + */
>> +static int __lowpan_btle_dellink(struct net_device *dev,
>> +                              struct hci_dev *hdev)
>> +{
>> +     struct lowpan_cleanup_work *xw;
>> +
>> +     xw = kzalloc(sizeof(*xw), GFP_KERNEL);
>> +     if (!xw)
>> +             return -ENOMEM;
>> +
>> +     INIT_WORK(&xw->work, lowpan_cleanup_worker);
>> +     xw->dev = dev;
>> +
>> +     hdev->pending_deletion = true;
>> +     queue_work(workqueue_cleanup, &xw->work);
>> +     return 0;
>> +}
>> +
>> +static int lowpan_enable_set(struct hci_dev *hdev, bool enabled)
>> +{
>> +     int ret = 0;
>> +
>> +     if (hdev->lowpan_enable == enabled)
>> +             goto out;
>> +
>> +     if (enabled) {
>> +             struct net_device *dev;
>> +
>> +             /* create one interface by default */
>> +             dev = lowpan_btle_newlink(hdev);
>> +             if (IS_ERR(dev))
>> +                     return PTR_ERR(dev);
>> +
>> +             /* TODO should be done by user network managers? */
>> +             ret = dev_open(dev);
>> +             if (ret < 0) {
>> +                     BT_ERR("iface %s cannot be opened %d", dev->name, ret);
>> +                     ret = __lowpan_btle_dellink(dev, hdev);
>> +                     if (ret < 0)
>> +                             BT_ERR("failed to remove interface %s",
>> +                                    hdev->ldev->name);
>> +
>> +                     return ret;
>> +             }
>> +
>> +             hdev->lowpan_enable = true;
>> +     } else {
>> +             /* ignore pending deletions */
>> +             if (hdev->pending_deletion)
>> +                     return 0;
>> +
>> +             __lowpan_btle_dellink(hdev->ldev, hdev);
>> +     }
>> +
>> +out:
>> +     return ret;
>> +}
>> +
>> +static void lowpan_btle_hci_unregister(struct hci_dev *hdev)
>> +{
>> +     /* no need to cleanup debugfs, will be done by hci_core */
>
> comment can be removed.
>
> Well..., I had this comment because I did some stuff before per
> "hci->debugfs". This is also the question here.
>
> Maybe "6lowpan_enable" makes no sense to move it to per hci_dev debugfs
> entry. For write, it makes sense - but read, you want some "global"
> mapping of "hci_dev <-> lowpan interface", this may easy realizied for
> netlink UAPI but for debugfs it's complicated because you need to
> iterate over all hci%d entries and making cat on "6lowpan_enable".
>
> That's the current reason why "6lowpan_enable" is global.
>
> ---
>
> The control file for btle lowpan interface could be moved per hci_dev
> and maybe I will change it again, then the above comment makes sense
> again. :-)
>
>> +
>> +     /* TODO netns support with multiple lowpan interfaces */
>> +     if (hdev->lowpan_enable)
>> +             lowpan_btle_dellink(hdev->ldev);
>> +}
>> +
>> +static int lowpan_hci_dev_event(struct notifier_block *unused,
>> +                             unsigned long event, void *ptr)
>> +{
>> +     struct hci_dev *hdev = ptr;
>> +     int ret = NOTIFY_OK;
>> +
>> +     rtnl_lock();
>> +
>> +     /* bluetooth handles that event type as int,
>> +      * but there is no overflow yet.
>> +      */
>> +     switch (event) {
>> +     case HCI_DEV_UNREG:
>> +             lowpan_btle_hci_unregister(hdev);
>> +             ret = NOTIFY_DONE;
>> +             break;
>> +     default:
>> +             break;
>> +     }
>> +
>> +     rtnl_unlock();
>> +
>> +     return ret;
>> +}
>> +
>> +static struct notifier_block lowpan_hcI_dev_notifier = {
>> +     .notifier_call = lowpan_hci_dev_event,
>> +};
>> +
>> +static ssize_t lowpan_enable_write(struct file *fp,
>> +                                const char __user *user_buffer,
>> +                                size_t count, loff_t *position)
>> +{
>> +     char buf[32] = { };
>> +     size_t buf_size = min(count, sizeof(buf) - 1);
>> +     struct hci_dev *hdev;
>> +     int idx, enabled, n, ret;
>> +
>> +     if (copy_from_user(buf, user_buffer, buf_size))
>> +             return -EFAULT;
>> +
>> +     n = sscanf(buf, "hci%d %d", &idx, &enabled);
>> +     if (n != 2)
>> +             return -EINVAL;
>> +
>> +     hdev = hci_dev_get(idx);
>> +     if (!hdev)
>> +             return -EINVAL;
>> +
>> +     rtnl_lock();
>> +     ret = lowpan_enable_set(hdev, enabled);
>> +     rtnl_unlock();
>> +
>> +     hci_dev_put(hdev);
>> +
>> +     return count;
>> +}
>> +
>> +static int lowpan_enable_show(struct seq_file *f, void *ptr)
>> +{
>> +     struct hci_dev *hdev;
>> +
>> +     rtnl_lock();
>> +     read_lock(&hci_dev_list_lock);
>> +     list_for_each_entry(hdev, &hci_dev_list, list) {
>> +             if (hdev->lowpan_enable)
>> +                     seq_printf(f, "hci%d -> %s\n", hdev->id,
>> +                                hdev->ldev->name);
>> +     }
>> +     read_unlock(&hci_dev_list_lock);
>> +     rtnl_unlock();
>> +
>> +     return 0;
>> +}
>> +
>> +static int lowpan_enable_open(struct inode *inode, struct file *file)
>> +{
>> +     return single_open(file, lowpan_enable_show, inode->i_private);
>> +}
>> +
>> +static const struct file_operations lowpan_enable_fops = {
>> +     .open           = lowpan_enable_open,
>> +     .read           = seq_read,
>> +     .write          = lowpan_enable_write,
>> +     .llseek         = seq_lseek,
>> +     .release        = single_release,
>> +};
>> +
>> +static int __init bt_6lowpan_init(void)
>> +{
>> +     int err;
>> +
>> +     workqueue_cleanup = create_singlethread_workqueue("btle 6lowpan");
>> +     if (!workqueue_cleanup)
>> +             return -ENOMEM;
>> +
>> +     debugfs_create_file("6lowpan_enable", 0644,  bt_debugfs, NULL,
>> +                         &lowpan_enable_fops);
>> +
>> +     err = register_hci_dev_notifier(&lowpan_hcI_dev_notifier);
>> +     if (err < 0)
>> +             destroy_workqueue(workqueue_cleanup);
>> +
>> +     return err;
>> +}
>> +
>> +static void __exit bt_6lowpan_exit(void)
>> +{
>> +     destroy_workqueue(workqueue_cleanup);
>> +     unregister_hci_dev_notifier(&lowpan_hcI_dev_notifier);
>> +}
>> +
>> +module_init(bt_6lowpan_init);
>> +module_exit(bt_6lowpan_exit);
>> +
>> +MODULE_AUTHOR("Jukka Rissanen <jukka.rissanen@linux.intel.com>");
>> +MODULE_DESCRIPTION("Bluetooth 6LoWPAN");
>> +MODULE_VERSION(LOWPAN_BTLE_VERSION);
>> +MODULE_LICENSE("GPL");
>> diff --git a/net/bluetooth/Makefile b/net/bluetooth/Makefile
>> index e347828..b3ff12e 100644
>> --- a/net/bluetooth/Makefile
>> +++ b/net/bluetooth/Makefile
>> @@ -7,6 +7,9 @@ obj-$(CONFIG_BT_RFCOMM)       += rfcomm/
>>  obj-$(CONFIG_BT_BNEP)        += bnep/
>>  obj-$(CONFIG_BT_CMTP)        += cmtp/
>>  obj-$(CONFIG_BT_HIDP)        += hidp/
>> +obj-$(CONFIG_BT_6LOWPAN) += bluetooth_6lowpan.o
>> +
>> +bluetooth_6lowpan-y := 6lowpan.o
>>
>>  bluetooth-y := af_bluetooth.o hci_core.o hci_conn.o hci_event.o mgmt.o \
>>       hci_sock.o hci_sysfs.o l2cap_core.o l2cap_sock.o smp.o lib.o \
>>
>
> - Alex
Alexander Aring July 17, 2016, 3:52 p.m. UTC | #3
Hi,

On 07/14/2016 01:40 PM, Luiz Augusto von Dentz wrote:
> Hi Alex,
> 
> On Wed, Jul 13, 2016 at 12:19 AM, Alexander Aring <aar@pengutronix.de> wrote:
>>
>> Hi,
>>
>> On 07/11/2016 09:50 PM, Alexander Aring wrote:
>>> This patch adds a new btle 6lowpan implementation in version 0.2. This
>>> new implementation has a new userspace API for debugfs.
>>>
>>> It's now possible to decide on which hci interface do you want to run
>>> your 6LoWPAN interface, for that you can run:
>>>
>>> echo "hci0 1" > /sys/kernel/debug/bluetooth/6lowpan_enable
>>>
>>> to enable, or:
>>>
>>> echo "hci0 0" > /sys/kernel/debug/bluetooth/6lowpan_enable
> 
> No please no more designs for debugfs, we are past that and moving
> towards having proper management commands, so what you did here is a
> huge waste of time and you have instead just ask before hand.
> 

The first time when I used BTLE 6LoWPAN I complaint about that I can not
say "use this special hci%d for lowpan interface xy", I did that not on
the mailinglist it was on the irc channel and everybody agreed that it
need to be fixed. The basic idea on my side was to run only one Virtual
Machine with two BTLE usb dongles to communicate to each other, so I
needed to switch to two virtual machines to workaround this issue.

If you have the current debugfs UAPI 1:1 already in some kind of stable
bluetooth API then I would give you the BIG advice to fix that and let
me look into that. (As far I know there exists idea's only, no
implementations).

Another really BIG thing is (it comes to the direction "breaking UAPI"
as well) that the interface depends on if Slaves are connected. So if
the last Slave will be disconnected then the interface will be deregistered.
Every IPv6 userspace application cannot deal with that. The interface
lifetime must not depend on if slaves connected or not.

At this case, I think every netdev people would agree.

>>> to disable it. A cat on this file will show the current hci <-> 6lowpan
>>> interface mapping.
>>>
>>> Additional the different is, that the interface will be created after
>>> enable it. The old behaviour is that the interface will be created when
>>> one first connection is established and deleted after all connections
>>> (peers) will be disconnected. This handling is different now.
>>>
>>> The reason (in my opinion) is that the existing of such interface should
>>> not based on if there is a connection or not. At runtime of many IPv6
>>> application an interface will be removed -> I think the most userspace
>>> application can and will never handle that an IPv6 interface will be
>>> deleted and recreating during runtime.
>>>
>>> The new handling is that the interface will not removed and you can
>>> re-establish the connection to your peers.
>>>
>>> Connection to peers:
>>>
>>> Connection to peers are not binded to hci interface, it's binded to your
>>> 6LoWPAN interface. This will prepare handling for multiple 6LoWPAN
>>> interfaces on one hci (maybe also with netns support, which isn't
>>> available yet) and you can choose which peers should be handled by
>>> different BTLE 6LoWPAN interfaces. Example:
>>>
>>> L2: hci0 is connected to node A, B, C, D
>>>
>>> L3: 6lo0 and 6lo1 are two interfaces based on hci0.
>>>     6lo0 is connected to nodes: A, B, C
>>>     6lo1 is connected to nodes: D, C, B
> 
> Is this is useful for what exactly? I much rather have a simple module
> that just deal with 1 connection than have to deal with complex
> topology in the beginning, so this is moving in the exact opposite
> direction.
> 

Well, I learned now to not talking about crazy use-cases which you could
do with that. I need more to talk about why this is really needed. I
will do that in future.

I observed, what I explained above. There exists two "connect" mechanism,
these are L2 and L3.

With L2 I mean $FIND_OUT_RIGHT_NOT_DEPRECATED_TOOL_IN_BT_UTILITY_DSCHUNGEL
leadv/lecc stuff. This prepares somehow that you can say
l2cap_chan_connect in the upper-layer to connect to slaves.

With l2cap_chan_connect I mean L3 connection, which is the same like
what you doing in L2 for L3. It's also not be a MUST, because you can
run e.g. above example Node A, B, C, D on L2 and:

BTLE 6LoWPAN: A <-> B

L2CAP_SOCK:   C <-> D

or some other mixed scenario. I think you want to still have such behaviour.
This would be the same like: A, B, C, D and 6lo0 is connected to A, B.
Forget the C, D, make with them whatever you want (_maybe_ namespace with
C, D or l2cap_sock).

At least if you want to have netns support with mutliple lowpan then
there will nothing to complex, it's just the same interface handling
like having one interface in init_net namespace. Just you can deal with
different namespaces with that (UAPI need to support namespaces for that).

It makes completely more sense to say to "lowpan interface 6lo%d connect
to slave xy".

The current way is that the user has no control of it and
l2cap_chan_connect call this "hci_get_route" function behind and with
bluetooth magic the user get one hci interface which best-fits and maybe
not that what you want.

>>> as one possible example. Handle this peer interface will have a possible
>>> more fine-granularity connection for peers.
> 
> Not useful when we most likely we will not have that many peers, this
> is no Wifi or ethernet, this is for very embedded devices lets not
> over engineer here please.
> 

Okay, I will not talk about any crazy use-cases which I could have. But
such setup is currently also possible but you can't control it because
bluetooth "hci_get_route" magic. On L2 you can connect to a number of Nodes
in some set. The funny thing is on L3 the "connect" command you cannot
say which interface connects now to which slave. If you have two lowpan
interfaces it will be decided by the magic "hci_get_route" function.

At last the question is not here to make such use-case the question is
here to make the "connect" command per interface. To doing that it will
not increase the number of "connect" commands which you need to use
anyway. What we remove is the bluetooth magic "hci_get_route", so now
you can "please connect from 6lo0 interface to $SLAVE, and this
interface is bounded to hci0", as exmaple.

>>> To connect to peers, there exists no "6lowpan_control" anymore. The
>>> control file is now peer interface, it's simple:
>>>
>>> echo "connect $PEER_ADDRESS 1" > /sys/kernel/debug/bluetooth/6lo0
>>>
>>> Also after connecting to peers it's still possible that another peer can
>>> to connect to the peer which has already a connected peer (complicated
>>> to describe). I saw that is somehow disabled in version before, see:
>>>
>>> http://lxr.free-electrons.com/source/net/bluetooth/6lowpan.c#L1247
>>>
>>> I am not a bluetooth expert, but I guess that is somehow if there exists
>>> limitation to devices which are not "peers" that means devices which can
>>> be run as master or slave only. So far I know such limitations for
>>> devices doesn't exists for BTLE 6LoWPAN, or?
>>>
> 
> I guess that is because dual role is not specified, you either are a
> node or a router but not both at the same time.
> 

Okay, then I will close the "listen" l2cap channel after somebody did a
connect. But I bet there was some graphic somewhere which had a

SLAVE.... <---- MASTER <----> MASTER ----> SLAVE...

So with that we forbid such MASTER <-> MASTER connections.

>> I think the following will explain somehow when it's a SLAVE and when
>> MASTER as my current view.
>>
>> SLAVES:
>>
>> They don't makes this echo "connect ..." foo in control files.
>> they only do "hciconfig hci0 leadv" that somebody other can connect to
>> that node (some MASTER node).
>>
>> MASTERS:
>>
>> can do the same what the SLAVES does but to be as MASTER they need to call
>> additional stuff:
>>
>> hcitool lecc $SLAVE_ADDR (one or more times)
> 
> Don't use hcitool, actually never mention any of hci related
> interfaces on the patches there are deprecated, btmgmt shall be used
> to enable things like advertising, I will be working on adding
> advertising support for bluetoothctl but we actually need IPSP UUID
> support in order to make this work properly so not even btmgmt will be
> enough in the end.
> 

Yes, Marcel already told me some tools are deprecated and I should not
use them. You don't want to know how long I took when I had some working
setup with my broadcom dongles. :-)

I am too lazy to switch, I will use now btmgmt. I hope it can do the
same suff up/down (I think power on/off) and leadv and lecc.

Thanks.

>> Additional for different lowpan interfaces (don't need the same addresses
>> which was used by previous one or more hcitool lecc calls):
>>
>> echo "connect SLAVE_ADDR 1" > /sys/kernel/debug/bluetooth/6lo0
>>
>> is this a correct view and can all BTLE transceivers run as SLAVE or
>> MASTER, or exists there some special transceivers outside which can be
>> run as SLAVE only?
>>
>>> Nevertheless I disabled that stuff, it's still possible to connect to
>>> some node which already run "connect ..." on his side.
>>>
>>> Otherwise I fixed the ndisc stuff and removed a lot of races/side
>>> effects. The old code had some static "lowpan_devices" list and most
>>> time there was some lookup mapping according to bdaddr to get the right
>>> 6lo netdevice struct. I removed that, also the skb->cb is used a lot in
>>> some callback which had some side-effects.
> 
> Send the fixes, and the fixes only.
> 

There exists fixes only, no new features. In my opinion, UAPI is broken
because you can't control what you want. Again, because "hci_get_route" magic.
I added no new features, I fixed the broken things. Maybe there are some
little small cleanups if you say changing function namens is not a
bugfix it's a cleanup.

Removing the whole implementation and adding a new one is the only way
which I would think it is possible, because we need to fix stuff in
IPHC and as well in IPv6 as well.

I know the zephyr people wants to be backwardscompatible I could say, I
add a big BROKEN_IMPLEMENTATION Kconfig switch which let's you compile
the old broken version which is still compatible with stacks outside.

btw: stacks outside I see currently pull-request in RIOT-OS which do the
same L2 address length is 8 instead 6, which means they are currently
bug compatible with Linux.

- Alex
--
To unsubscribe from this list: send the line "unsubscribe linux-wpan" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Luiz Augusto von Dentz July 18, 2016, 8:59 a.m. UTC | #4
Hi Alex,

On Sun, Jul 17, 2016 at 6:52 PM, Alexander Aring <aar@pengutronix.de> wrote:
>
> Hi,
>
> On 07/14/2016 01:40 PM, Luiz Augusto von Dentz wrote:
>> Hi Alex,
>>
>> On Wed, Jul 13, 2016 at 12:19 AM, Alexander Aring <aar@pengutronix.de> wrote:
>>>
>>> Hi,
>>>
>>> On 07/11/2016 09:50 PM, Alexander Aring wrote:
>>>> This patch adds a new btle 6lowpan implementation in version 0.2. This
>>>> new implementation has a new userspace API for debugfs.
>>>>
>>>> It's now possible to decide on which hci interface do you want to run
>>>> your 6LoWPAN interface, for that you can run:
>>>>
>>>> echo "hci0 1" > /sys/kernel/debug/bluetooth/6lowpan_enable
>>>>
>>>> to enable, or:
>>>>
>>>> echo "hci0 0" > /sys/kernel/debug/bluetooth/6lowpan_enable
>>
>> No please no more designs for debugfs, we are past that and moving
>> towards having proper management commands, so what you did here is a
>> huge waste of time and you have instead just ask before hand.
>>
>
> The first time when I used BTLE 6LoWPAN I complaint about that I can not
> say "use this special hci%d for lowpan interface xy", I did that not on
> the mailinglist it was on the irc channel and everybody agreed that it
> need to be fixed. The basic idea on my side was to run only one Virtual
> Machine with two BTLE usb dongles to communicate to each other, so I
> needed to switch to two virtual machines to workaround this issue.
>
> If you have the current debugfs UAPI 1:1 already in some kind of stable
> bluetooth API then I would give you the BIG advice to fix that and let
> me look into that. (As far I know there exists idea's only, no
> implementations).

There is the patches posted by Patrik implementing the management interface:

http://www.spinics.net/lists/linux-bluetooth/msg66486.html

Note that these commands are per interface index, so you would be able
to have two dongle talking to each other, we could in fact even
automate this to test with 2 virtual controllers.

> Another really BIG thing is (it comes to the direction "breaking UAPI"
> as well) that the interface depends on if Slaves are connected. So if
> the last Slave will be disconnected then the interface will be deregistered.
> Every IPv6 userspace application cannot deal with that. The interface
> lifetime must not depend on if slaves connected or not.

This is tricky because the even using privacy the address of the
interface is defined by the time the connection is established, but
perhaps there is a way to set this properly on the network interface,
anyway I do agree the making the interface persistent is probably much
more convenient.

> At this case, I think every netdev people would agree.
>
>>>> to disable it. A cat on this file will show the current hci <-> 6lowpan
>>>> interface mapping.
>>>>
>>>> Additional the different is, that the interface will be created after
>>>> enable it. The old behaviour is that the interface will be created when
>>>> one first connection is established and deleted after all connections
>>>> (peers) will be disconnected. This handling is different now.
>>>>
>>>> The reason (in my opinion) is that the existing of such interface should
>>>> not based on if there is a connection or not. At runtime of many IPv6
>>>> application an interface will be removed -> I think the most userspace
>>>> application can and will never handle that an IPv6 interface will be
>>>> deleted and recreating during runtime.
>>>>
>>>> The new handling is that the interface will not removed and you can
>>>> re-establish the connection to your peers.
>>>>
>>>> Connection to peers:
>>>>
>>>> Connection to peers are not binded to hci interface, it's binded to your
>>>> 6LoWPAN interface. This will prepare handling for multiple 6LoWPAN
>>>> interfaces on one hci (maybe also with netns support, which isn't
>>>> available yet) and you can choose which peers should be handled by
>>>> different BTLE 6LoWPAN interfaces. Example:
>>>>
>>>> L2: hci0 is connected to node A, B, C, D
>>>>
>>>> L3: 6lo0 and 6lo1 are two interfaces based on hci0.
>>>>     6lo0 is connected to nodes: A, B, C
>>>>     6lo1 is connected to nodes: D, C, B
>>
>> Is this is useful for what exactly? I much rather have a simple module
>> that just deal with 1 connection than have to deal with complex
>> topology in the beginning, so this is moving in the exact opposite
>> direction.
>>
>
> Well, I learned now to not talking about crazy use-cases which you could
> do with that. I need more to talk about why this is really needed. I
> will do that in future.

It is about priorities, we should concentrate in making it work for
simpler cases, besides the RFC don't talk about subnets either.


> I observed, what I explained above. There exists two "connect" mechanism,
> these are L2 and L3.
>
> With L2 I mean $FIND_OUT_RIGHT_NOT_DEPRECATED_TOOL_IN_BT_UTILITY_DSCHUNGEL
> leadv/lecc stuff. This prepares somehow that you can say
> l2cap_chan_connect in the upper-layer to connect to slaves.
>
> With l2cap_chan_connect I mean L3 connection, which is the same like
> what you doing in L2 for L3. It's also not be a MUST, because you can
> run e.g. above example Node A, B, C, D on L2 and:
>
> BTLE 6LoWPAN: A <-> B
>
> L2CAP_SOCK:   C <-> D
>
> or some other mixed scenario. I think you want to still have such behaviour.
> This would be the same like: A, B, C, D and 6lo0 is connected to A, B.
> Forget the C, D, make with them whatever you want (_maybe_ namespace with
> C, D or l2cap_sock).

Im not sure where you are going with l2cap_sock here, yes we can make
it connect to different peers but usually each peer assumes a role in
BLE, the central role scans and connect and the peripheral only
advertise. If you do have more than one controller it should be fine
to have multiple network interfaces and then perhaps you can bridge
them together, or not, but this should be controlled via userspace,
the kernel shall only manage the network interface.

> At least if you want to have netns support with mutliple lowpan then
> there will nothing to complex, it's just the same interface handling
> like having one interface in init_net namespace. Just you can deal with
> different namespaces with that (UAPI need to support namespaces for that).
>
> It makes completely more sense to say to "lowpan interface 6lo%d connect
> to slave xy".
>
> The current way is that the user has no control of it and
> l2cap_chan_connect call this "hci_get_route" function behind and with
> bluetooth magic the user get one hci interface which best-fits and maybe
> not that what you want.

See http://www.spinics.net/lists/linux-bluetooth/msg66495.html, it
takes the index of the interface upfront so you can specify which
adapter to use so it shouldn't rely on hci_get_route anymore.

>>>> To connect to peers, there exists no "6lowpan_control" anymore. The
>>>> control file is now peer interface, it's simple:
>>>>
>>>> echo "connect $PEER_ADDRESS 1" > /sys/kernel/debug/bluetooth/6lo0
>>>>
>>>> Also after connecting to peers it's still possible that another peer can
>>>> to connect to the peer which has already a connected peer (complicated
>>>> to describe). I saw that is somehow disabled in version before, see:
>>>>
>>>> http://lxr.free-electrons.com/source/net/bluetooth/6lowpan.c#L1247
>>>>
>>>> I am not a bluetooth expert, but I guess that is somehow if there exists
>>>> limitation to devices which are not "peers" that means devices which can
>>>> be run as master or slave only. So far I know such limitations for
>>>> devices doesn't exists for BTLE 6LoWPAN, or?
>>>>
>>
>> I guess that is because dual role is not specified, you either are a
>> node or a router but not both at the same time.
>>
>
> Okay, then I will close the "listen" l2cap channel after somebody did a
> connect. But I bet there was some graphic somewhere which had a
>
> SLAVE.... <---- MASTER <----> MASTER ----> SLAVE...
>
> So with that we forbid such MASTER <-> MASTER connections.

This master to master connection doesn't make much sense, usually a
central cannot connect to another central since none advertise
anything, but indeed we can have more than one slave/none connected to
a master/router. Note we can in fact switch between peripheral and
central roles, so in theory it is still possible to have subnets, but
it is just very unlikely.

>>> I think the following will explain somehow when it's a SLAVE and when
>>> MASTER as my current view.
>>>
>>> SLAVES:
>>>
>>> They don't makes this echo "connect ..." foo in control files.
>>> they only do "hciconfig hci0 leadv" that somebody other can connect to
>>> that node (some MASTER node).
>>>
>>> MASTERS:
>>>
>>> can do the same what the SLAVES does but to be as MASTER they need to call
>>> additional stuff:
>>>
>>> hcitool lecc $SLAVE_ADDR (one or more times)
>>
>> Don't use hcitool, actually never mention any of hci related
>> interfaces on the patches there are deprecated, btmgmt shall be used
>> to enable things like advertising, I will be working on adding
>> advertising support for bluetoothctl but we actually need IPSP UUID
>> support in order to make this work properly so not even btmgmt will be
>> enough in the end.
>>
>
> Yes, Marcel already told me some tools are deprecated and I should not
> use them. You don't want to know how long I took when I had some working
> setup with my broadcom dongles. :-)
>
> I am too lazy to switch, I will use now btmgmt. I hope it can do the
> same suff up/down (I think power on/off) and leadv and lecc.

Yes, but if we keep using those people will never learn there deprecated.

>
>>> Additional for different lowpan interfaces (don't need the same addresses
>>> which was used by previous one or more hcitool lecc calls):
>>>
>>> echo "connect SLAVE_ADDR 1" > /sys/kernel/debug/bluetooth/6lo0
>>>
>>> is this a correct view and can all BTLE transceivers run as SLAVE or
>>> MASTER, or exists there some special transceivers outside which can be
>>> run as SLAVE only?
>>>
>>>> Nevertheless I disabled that stuff, it's still possible to connect to
>>>> some node which already run "connect ..." on his side.
>>>>
>>>> Otherwise I fixed the ndisc stuff and removed a lot of races/side
>>>> effects. The old code had some static "lowpan_devices" list and most
>>>> time there was some lookup mapping according to bdaddr to get the right
>>>> 6lo netdevice struct. I removed that, also the skb->cb is used a lot in
>>>> some callback which had some side-effects.
>>
>> Send the fixes, and the fixes only.
>>
>
> There exists fixes only, no new features. In my opinion, UAPI is broken
> because you can't control what you want. Again, because "hci_get_route" magic.
> I added no new features, I fixed the broken things. Maybe there are some
> little small cleanups if you say changing function namens is not a
> bugfix it's a cleanup.

See the RFC sent by Patrik, the MGMT commands use interface index so
hci_get_route should not be needed.

> Removing the whole implementation and adding a new one is the only way
> which I would think it is possible, because we need to fix stuff in
> IPHC and as well in IPv6 as well.

I guess these are separate fixes aren't they? Or are they depending on
this patch?

> I know the zephyr people wants to be backwardscompatible I could say, I
> add a big BROKEN_IMPLEMENTATION Kconfig switch which let's you compile
> the old broken version which is still compatible with stacks outside.

No, in fact we need these fixes in order to work properly with zephyr.

> btw: stacks outside I see currently pull-request in RIOT-OS which do the
> same L2 address length is 8 instead 6, which means they are currently
> bug compatible with Linux.

Sounds like a bug there, Bluetooth shall use MAC addresses of 6 bytes
long the conversion for 8 bytes (64 bits) is just for the link local
to form the so called IID (https://tools.ietf.org/html/rfc7668) I
don't think it should be transmitted with 0xff, 0xfe encoded in the
address.
Alexander Aring July 18, 2016, 9:52 p.m. UTC | #5
Hi,

On 07/18/2016 10:59 AM, Luiz Augusto von Dentz wrote:
> Hi Alex,
> 
> On Sun, Jul 17, 2016 at 6:52 PM, Alexander Aring <aar@pengutronix.de> wrote:
>>
>> Hi,
>>
>> On 07/14/2016 01:40 PM, Luiz Augusto von Dentz wrote:
>>> Hi Alex,
>>>
>>> On Wed, Jul 13, 2016 at 12:19 AM, Alexander Aring <aar@pengutronix.de> wrote:
>>>>
>>>> Hi,
>>>>
>>>> On 07/11/2016 09:50 PM, Alexander Aring wrote:
>>>>> This patch adds a new btle 6lowpan implementation in version 0.2. This
>>>>> new implementation has a new userspace API for debugfs.
>>>>>
>>>>> It's now possible to decide on which hci interface do you want to run
>>>>> your 6LoWPAN interface, for that you can run:
>>>>>
>>>>> echo "hci0 1" > /sys/kernel/debug/bluetooth/6lowpan_enable
>>>>>
>>>>> to enable, or:
>>>>>
>>>>> echo "hci0 0" > /sys/kernel/debug/bluetooth/6lowpan_enable
>>>
>>> No please no more designs for debugfs, we are past that and moving
>>> towards having proper management commands, so what you did here is a
>>> huge waste of time and you have instead just ask before hand.
>>>
>>
>> The first time when I used BTLE 6LoWPAN I complaint about that I can not
>> say "use this special hci%d for lowpan interface xy", I did that not on
>> the mailinglist it was on the irc channel and everybody agreed that it
>> need to be fixed. The basic idea on my side was to run only one Virtual
>> Machine with two BTLE usb dongles to communicate to each other, so I
>> needed to switch to two virtual machines to workaround this issue.
>>
>> If you have the current debugfs UAPI 1:1 already in some kind of stable
>> bluetooth API then I would give you the BIG advice to fix that and let
>> me look into that. (As far I know there exists idea's only, no
>> implementations).
> 
> There is the patches posted by Patrik implementing the management interface:
> 
> http://www.spinics.net/lists/linux-bluetooth/msg66486.html
> 
Okay, just to be sure here, in the above document:

Controller ID = hdev(usually var name, struct hci_dev)->id

Interface Index = dev(usualy var name, struct net_device)->idx

Is this right? I think so, so my next stuff will base on that.

> Note that these commands are per interface index, so you would be able
> to have two dongle talking to each other, we could in fact even
> automate this to test with 2 virtual controllers.
> 

We have similar automating testing in 802.15.4 6LoWPAN with virtual
transceiver drivers.

I always looked somehow for some virtual driver in bluetooth, does this
exist mainline?

>> Another really BIG thing is (it comes to the direction "breaking UAPI"
>> as well) that the interface depends on if Slaves are connected. So if
>> the last Slave will be disconnected then the interface will be deregistered.
>> Every IPv6 userspace application cannot deal with that. The interface
>> lifetime must not depend on if slaves connected or not.
> 
> This is tricky because the even using privacy the address of the
> interface is defined by the time the connection is established, but
> perhaps there is a way to set this properly on the network interface,
> anyway I do agree the making the interface persistent is probably much
> more convenient.
> 

This is currently something which I don't understand. With "privacy" you
mean the LE_PUBLIC and LE_PRIVATE address, the address type?

I think, we don't need that when we doing an interface register. This is
currently one of the big issue which this RFC fixes, the 8 byte vs 6
bytes address.

At Interface register we need to know the source address only, which is
my opinion the hdev->bdaddr. This address has (in my opinion) nothing
todo with the address type.

I can argument here with rfc7668:

 "This means that no bit in the IID indicates whether the
  underlying Bluetooth device address is public or random."

The IID is for autoconfiguration the last 8 bytes of IPv6 address and
usually generated by "dev->dev_addr" (dev = struct net_device).
Also the "dev->dev_addr" will be used in address options for
NS/NA/RS/RA...

In the current implementation the IID generation is mixed with the
"dev->addr" (dev as struct net_device) field which is wrong.

In summary:

We don't need to know to wait for connection to setting the
"dev->dev_addr" (struct net_device). This field should be "hdev->bdaddr"
and hdev should be the hdev which is bounded to the dev(lowpan struct
net_device).

I see no issues to create interfaces before, except you saying that
"hdev->bdaddr" is unknown.

>> At this case, I think every netdev people would agree.
>>
>>>>> to disable it. A cat on this file will show the current hci <-> 6lowpan
>>>>> interface mapping.
>>>>>
>>>>> Additional the different is, that the interface will be created after
>>>>> enable it. The old behaviour is that the interface will be created when
>>>>> one first connection is established and deleted after all connections
>>>>> (peers) will be disconnected. This handling is different now.
>>>>>
>>>>> The reason (in my opinion) is that the existing of such interface should
>>>>> not based on if there is a connection or not. At runtime of many IPv6
>>>>> application an interface will be removed -> I think the most userspace
>>>>> application can and will never handle that an IPv6 interface will be
>>>>> deleted and recreating during runtime.
>>>>>
>>>>> The new handling is that the interface will not removed and you can
>>>>> re-establish the connection to your peers.
>>>>>
>>>>> Connection to peers:
>>>>>
>>>>> Connection to peers are not binded to hci interface, it's binded to your
>>>>> 6LoWPAN interface. This will prepare handling for multiple 6LoWPAN
>>>>> interfaces on one hci (maybe also with netns support, which isn't
>>>>> available yet) and you can choose which peers should be handled by
>>>>> different BTLE 6LoWPAN interfaces. Example:
>>>>>
>>>>> L2: hci0 is connected to node A, B, C, D
>>>>>
>>>>> L3: 6lo0 and 6lo1 are two interfaces based on hci0.
>>>>>     6lo0 is connected to nodes: A, B, C
>>>>>     6lo1 is connected to nodes: D, C, B
>>>
>>> Is this is useful for what exactly? I much rather have a simple module
>>> that just deal with 1 connection than have to deal with complex
>>> topology in the beginning, so this is moving in the exact opposite
>>> direction.
>>>
>>
>> Well, I learned now to not talking about crazy use-cases which you could
>> do with that. I need more to talk about why this is really needed. I
>> will do that in future.
> 
> It is about priorities, we should concentrate in making it work for
> simpler cases, besides the RFC don't talk about subnets either.
> 

I don't talking about subnets.

I am talking about "make the connect command per interface". Patrik
works shows the following:

Add Network Command
===================

	Command Code:		0x0043
	Controller Index:	<controller id>
	Command Parameters:	Address (6 Octets)
				Address_Type (1 Octet)
	Return Parameters:	Address (6 Octets)
				Address_Type (1 Octet)
				Interface Index (4 Octets)

	This command is used to connect to establish a network connection
	to a remote BTLE node. A pre-requisite is that LE is supported
	by the controller, otherwise this command will return a
	"not supported" response.

	Possible values for the Address_Type parameter:
		0	Reserved
		1	LE Public
		2	LE Random

	This command generates a Command Complete event on success
	or failure.

	Possible errors:	Busy
				Refused
				Invalid Parameters
				Not Supported
				Invalid Index
				Already Connected

So far I understand, this is "make the connect command per device", the
device in this case is not a "struct net_device" it's "struct hci_dev".

In the "possible" future case to having multiple interfaces you cannot
say on which interface this connect should be done. With "connect" I
mean the l2cap_chan_connect call.

btw:
Also I don't know what's now the parameters "Address/Address_Type"
source/destination? Is the Input of them equal to the output?

> 
>> I observed, what I explained above. There exists two "connect" mechanism,
>> these are L2 and L3.
>>
>> With L2 I mean $FIND_OUT_RIGHT_NOT_DEPRECATED_TOOL_IN_BT_UTILITY_DSCHUNGEL
>> leadv/lecc stuff. This prepares somehow that you can say
>> l2cap_chan_connect in the upper-layer to connect to slaves.
>>
>> With l2cap_chan_connect I mean L3 connection, which is the same like
>> what you doing in L2 for L3. It's also not be a MUST, because you can
>> run e.g. above example Node A, B, C, D on L2 and:
>>
>> BTLE 6LoWPAN: A <-> B
>>
>> L2CAP_SOCK:   C <-> D
>>
>> or some other mixed scenario. I think you want to still have such behaviour.
>> This would be the same like: A, B, C, D and 6lo0 is connected to A, B.
>> Forget the C, D, make with them whatever you want (_maybe_ namespace with
>> C, D or l2cap_sock).
> 
> Im not sure where you are going with l2cap_sock here, yes we can make
> it connect to different peers but usually each peer assumes a role in
> BLE, the central role scans and connect and the peripheral only
> advertise. If you do have more than one controller it should be fine
> to have multiple network interfaces and then perhaps you can bridge
> them together, or not, but this should be controlled via userspace,
> the kernel shall only manage the network interface.
> 

Yes, but the kernel needs an UAPI so the user can control it over userspace.
See above my "make connect per interface" suggestion.

>> At least if you want to have netns support with mutliple lowpan then
>> there will nothing to complex, it's just the same interface handling
>> like having one interface in init_net namespace. Just you can deal with
>> different namespaces with that (UAPI need to support namespaces for that).
>>
>> It makes completely more sense to say to "lowpan interface 6lo%d connect
>> to slave xy".
>>
>> The current way is that the user has no control of it and
>> l2cap_chan_connect call this "hci_get_route" function behind and with
>> bluetooth magic the user get one hci interface which best-fits and maybe
>> not that what you want.
> 
> See http://www.spinics.net/lists/linux-bluetooth/msg66495.html, it
> takes the index of the interface upfront so you can specify which
> adapter to use so it shouldn't rely on hci_get_route anymore.
> 

See above, make the "connect per hci device" will make it not clear if
having multiple interfaces on one hci dev.

>>>>> To connect to peers, there exists no "6lowpan_control" anymore. The
>>>>> control file is now peer interface, it's simple:
>>>>>
>>>>> echo "connect $PEER_ADDRESS 1" > /sys/kernel/debug/bluetooth/6lo0
>>>>>
>>>>> Also after connecting to peers it's still possible that another peer can
>>>>> to connect to the peer which has already a connected peer (complicated
>>>>> to describe). I saw that is somehow disabled in version before, see:
>>>>>
>>>>> http://lxr.free-electrons.com/source/net/bluetooth/6lowpan.c#L1247
>>>>>
>>>>> I am not a bluetooth expert, but I guess that is somehow if there exists
>>>>> limitation to devices which are not "peers" that means devices which can
>>>>> be run as master or slave only. So far I know such limitations for
>>>>> devices doesn't exists for BTLE 6LoWPAN, or?
>>>>>
>>>
>>> I guess that is because dual role is not specified, you either are a
>>> node or a router but not both at the same time.
>>>
>>
>> Okay, then I will close the "listen" l2cap channel after somebody did a
>> connect. But I bet there was some graphic somewhere which had a
>>
>> SLAVE.... <---- MASTER <----> MASTER ----> SLAVE...
>>
>> So with that we forbid such MASTER <-> MASTER connections.
> 
> This master to master connection doesn't make much sense, usually a
> central cannot connect to another central since none advertise
> anything, but indeed we can have more than one slave/none connected to
> a master/router. Note we can in fact switch between peripheral and
> central roles, so in theory it is still possible to have subnets, but
> it is just very unlikely.
> 

I rechecked that handling and I think closing that "listen" channel will
not forbid such MASTER <-> MASTER stuff. I previous mentioned that I
found in the current mainline implementation that the listen channel
will be closed somehow when running a "connect", I still get no the
reason why. I thought it's something that you cannot "accept other
connection" when running a "connect" command once.
But if I had the "listen" channel before running and already accept
connection and running "connect" afterwards, then I still have the
channels open which was accepted by the "listen" channel.

In short:

I need to know somehow what's possible. When running connect and when
doing the "listen" stuff for accepting incoming connection. We talking
here about l2cap layer.

Can it not be it's such simple that l2cap_chan_connect/etc. functionality
will drop an error if it's simple not possible? Then we let the
subsystem decide if such connections are possible or not. Depends on
what means the RFC and if we need to make more than the bluetooth
subsystem can handle.

Then it would be the question to map these errors to UAPI.

I am really confused about the closing "listen" stuff channel. :-/

>>>> I think the following will explain somehow when it's a SLAVE and when
>>>> MASTER as my current view.
>>>>
>>>> SLAVES:
>>>>
>>>> They don't makes this echo "connect ..." foo in control files.
>>>> they only do "hciconfig hci0 leadv" that somebody other can connect to
>>>> that node (some MASTER node).
>>>>
>>>> MASTERS:
>>>>
>>>> can do the same what the SLAVES does but to be as MASTER they need to call
>>>> additional stuff:
>>>>
>>>> hcitool lecc $SLAVE_ADDR (one or more times)
>>>
>>> Don't use hcitool, actually never mention any of hci related
>>> interfaces on the patches there are deprecated, btmgmt shall be used
>>> to enable things like advertising, I will be working on adding
>>> advertising support for bluetoothctl but we actually need IPSP UUID
>>> support in order to make this work properly so not even btmgmt will be
>>> enough in the end.
>>>
>>
>> Yes, Marcel already told me some tools are deprecated and I should not
>> use them. You don't want to know how long I took when I had some working
>> setup with my broadcom dongles. :-)
>>
>> I am too lazy to switch, I will use now btmgmt. I hope it can do the
>> same suff up/down (I think power on/off) and leadv and lecc.
> 
> Yes, but if we keep using those people will never learn there deprecated.
> 

I didn't know it either, is there some wiki webpage which says about
which tool is deprecated and which isn't?

>>
>>>> Additional for different lowpan interfaces (don't need the same addresses
>>>> which was used by previous one or more hcitool lecc calls):
>>>>
>>>> echo "connect SLAVE_ADDR 1" > /sys/kernel/debug/bluetooth/6lo0
>>>>
>>>> is this a correct view and can all BTLE transceivers run as SLAVE or
>>>> MASTER, or exists there some special transceivers outside which can be
>>>> run as SLAVE only?
>>>>
>>>>> Nevertheless I disabled that stuff, it's still possible to connect to
>>>>> some node which already run "connect ..." on his side.
>>>>>
>>>>> Otherwise I fixed the ndisc stuff and removed a lot of races/side
>>>>> effects. The old code had some static "lowpan_devices" list and most
>>>>> time there was some lookup mapping according to bdaddr to get the right
>>>>> 6lo netdevice struct. I removed that, also the skb->cb is used a lot in
>>>>> some callback which had some side-effects.
>>>
>>> Send the fixes, and the fixes only.
>>>
>>
>> There exists fixes only, no new features. In my opinion, UAPI is broken
>> because you can't control what you want. Again, because "hci_get_route" magic.
>> I added no new features, I fixed the broken things. Maybe there are some
>> little small cleanups if you say changing function namens is not a
>> bugfix it's a cleanup.
> 
> See the RFC sent by Patrik, the MGMT commands use interface index so
> hci_get_route should not be needed.
> 

See above, I would suggest to make some commands per interface and not
per hci_dev.

>> Removing the whole implementation and adding a new one is the only way
>> which I would think it is possible, because we need to fix stuff in
>> IPHC and as well in IPv6 as well.
> 
> I guess these are separate fixes aren't they? Or are they depending on
> this patch?
> 

yea, some of them. I will make a clearer version of the patch series and
send some patches mainline which I can do before.

The necessary stuff for btle 6lowpan begins at:

 - Remove bluetooth implementation

and ends at:

 - Add new bluetooth implementation

Patches between of them will changes something in different Layers, this
is IPHC and IPv6. This is mostly to change the stuff from 8 bytes
address to 6 byte address. If I would do it before, I would break the
btle 6lowpan stuff before. Besides that there are many races etc in btle
6lowpan so I just do a remove, prepare handling for 6 byte address and
add a new implementation. I didn't hit a race yet but I need to test
more with some unlikely testcases. The only issue which I get while
using 6lowpan is the "tx_credits" deadlock, which we need to talk about
more.

The current mainline implementation is very unstable on my side, means:
my kernel will crash.

>> I know the zephyr people wants to be backwardscompatible I could say, I
>> add a big BROKEN_IMPLEMENTATION Kconfig switch which let's you compile
>> the old broken version which is still compatible with stacks outside.
> 
> No, in fact we need these fixes in order to work properly with zephyr.
> 

Did you test this patch series with zephyr, or somebody else?

>> btw: stacks outside I see currently pull-request in RIOT-OS which do the
>> same L2 address length is 8 instead 6, which means they are currently
>> bug compatible with Linux.
> 
> Sounds like a bug there, Bluetooth shall use MAC addresses of 6 bytes
> long the conversion for 8 bytes (64 bits) is just for the link local
> to form the so called IID (https://tools.ietf.org/html/rfc7668) I
> don't think it should be transmitted with 0xff, 0xfe encoded in the
> address.
> 

This bug I mentioned earlier, see:
http://www.spinics.net/lists/linux-wpan/msg03899.html

Besides that another big bug is that BTLE ignores the neighbour
discovery cache. Only addresses which are generated by
autoconfiguration. Normally the ndisc cache has L3 -> L2 address
mapping (but includes more than just that). The mainline work stuff do
currently to reconstruct the L2 address from L3 address, which only for
autoconfiguration addresses.

In short:

If you set an own ipv6 address which is not based on mac address it will
not work. Also some other IPv6 mechanism in neighbour discovery will not
work.

This patch series will fix that and use the neighbour discovery.

btw:

I report that bug 3-4 months ago in irc channel.


IMPORTANT NOTE:

This means not that we using ARO DAC/DAR stuff from rfc6775.

---

At last I would like suggest some ideas for the UAPI handling, which is
some oriented (for interface generation) what we have in 802.15.4:

 1. Command to add an 6lowpan interface according to hci dev
    (You can add multiple interfaces then if calling more than once)
 2. Command to del an 6lowpan interface
 3. Command to connect to $PEER (or $SLAVE, whatever the right term is
    here), but this command is per 6lowpan interface. Not hci_dev.
 4. Command to disconnect to $PEER, per 6lowpan interface as well.

Maybe I we should make the UAPI more clear and I implement it for now as
debugfs. But you think it's waste of time, or isn't it?

I also detected that debugfs doesn't like to remove debugfs while
debugfs handling. So the second command is complicated to implement. (I
did some workqueue workaround for that which I mentioned many times
inside the comments that it is a workaround).

btw:

Some kernel hackers would also say thet doing the xmit workqueue is a
workaround because we need to use "async API". I don't saw any xmit
async functionality at bluetooth subsystem and I also need to hold the
l2cap chan->lock when I call l2cap_chan_send, so this is a mutex and I
need to run a workqueue. I try to deal with that in the current
implementation, so far it works... but running a workqueue to call
l2cap_chan_send which runs again a workqueue is bad.

- Alex
--
To unsubscribe from this list: send the line "unsubscribe linux-wpan" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Johan Hedberg July 19, 2016, 5:45 a.m. UTC | #6
Hi Alex,

On Mon, Jul 18, 2016, Alexander Aring wrote:
> > This is tricky because the even using privacy the address of the
> > interface is defined by the time the connection is established, but
> > perhaps there is a way to set this properly on the network interface,
> > anyway I do agree the making the interface persistent is probably much
> > more convenient.
> > 
> 
> This is currently something which I don't understand. With "privacy" you
> mean the LE_PUBLIC and LE_PRIVATE address, the address type?

Might be worth to do a quick read-through of section 1.3 in Vol 6, Part
B of the Bluetooth Core specification (v4.2) for a quick overview of LE
addresses. On the top-level you have public & random, but random itself
is further split into three different sub-types out of which two are
considered "private". The third one, a static random address, is
considered an identity address the same way a public address is.

> I think, we don't need that when we doing an interface register. This is
> currently one of the big issue which this RFC fixes, the 8 byte vs 6
> bytes address.
> 
> At Interface register we need to know the source address only, which is
> my opinion the hdev->bdaddr. This address has (in my opinion) nothing
> todo with the address type.
> 
> I can argument here with rfc7668:
> 
>  "This means that no bit in the IID indicates whether the
>   underlying Bluetooth device address is public or random."
> 
> The IID is for autoconfiguration the last 8 bytes of IPv6 address and
> usually generated by "dev->dev_addr" (dev = struct net_device).
> Also the "dev->dev_addr" will be used in address options for
> NS/NA/RS/RA...
> 
> In the current implementation the IID generation is mixed with the
> "dev->addr" (dev as struct net_device) field which is wrong.
> 
> In summary:
> 
> We don't need to know to wait for connection to setting the
> "dev->dev_addr" (struct net_device). This field should be "hdev->bdaddr"
> and hdev should be the hdev which is bounded to the dev(lowpan struct
> net_device).
> 
> I see no issues to create interfaces before, except you saying that
> "hdev->bdaddr" is unknown.

hdev->bdaddr tracks the public address of a device, however the
Bluetooth specification doesn't mandate that all devices have a public
address, i.e. it can also be all zeroes. In particular, single-mode
(LE-only) devices may only use a random address and have a static random
address as their identity address. Maybe it's the identity address that
you should be using instead of hdev->bdaddr?

Right now the identity address isn't stored in any variable, but we do
have a hci_copy_identity_address() function to fetch it (you'd just
ignore the addr_type if you don't need that).

Also note that the identity address isn't necessarily the same as what
was used to create the connection, so if the connection address is of
importance we track that in hci_conn->init_addr and hci_conn->resp_addr.

Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-wpan" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Luiz Augusto von Dentz July 19, 2016, 8:23 a.m. UTC | #7
Hi Alex, Johan,

On Tue, Jul 19, 2016 at 8:45 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi Alex,
>
> On Mon, Jul 18, 2016, Alexander Aring wrote:
>> > This is tricky because the even using privacy the address of the
>> > interface is defined by the time the connection is established, but
>> > perhaps there is a way to set this properly on the network interface,
>> > anyway I do agree the making the interface persistent is probably much
>> > more convenient.
>> >
>>
>> This is currently something which I don't understand. With "privacy" you
>> mean the LE_PUBLIC and LE_PRIVATE address, the address type?
>
> Might be worth to do a quick read-through of section 1.3 in Vol 6, Part
> B of the Bluetooth Core specification (v4.2) for a quick overview of LE
> addresses. On the top-level you have public & random, but random itself
> is further split into three different sub-types out of which two are
> considered "private". The third one, a static random address, is
> considered an identity address the same way a public address is.

The so called privacy, the use of RPA, is actually recommended by RFC 7668:

https://tools.ietf.org/html/rfc7668#section-3.2.2

>> I think, we don't need that when we doing an interface register. This is
>> currently one of the big issue which this RFC fixes, the 8 byte vs 6
>> bytes address.
>>
>> At Interface register we need to know the source address only, which is
>> my opinion the hdev->bdaddr. This address has (in my opinion) nothing
>> todo with the address type.
>>
>> I can argument here with rfc7668:
>>
>>  "This means that no bit in the IID indicates whether the
>>   underlying Bluetooth device address is public or random."

See above the rfc7668 actually recommends using RPAs.

>> The IID is for autoconfiguration the last 8 bytes of IPv6 address and
>> usually generated by "dev->dev_addr" (dev = struct net_device).
>> Also the "dev->dev_addr" will be used in address options for
>> NS/NA/RS/RA...
>>
>> In the current implementation the IID generation is mixed with the
>> "dev->addr" (dev as struct net_device) field which is wrong.
>>
>> In summary:
>>
>> We don't need to know to wait for connection to setting the
>> "dev->dev_addr" (struct net_device). This field should be "hdev->bdaddr"
>> and hdev should be the hdev which is bounded to the dev(lowpan struct
>> net_device).
>>
>> I see no issues to create interfaces before, except you saying that
>> "hdev->bdaddr" is unknown.
>
> hdev->bdaddr tracks the public address of a device, however the
> Bluetooth specification doesn't mandate that all devices have a public
> address, i.e. it can also be all zeroes. In particular, single-mode
> (LE-only) devices may only use a random address and have a static random
> address as their identity address. Maybe it's the identity address that
> you should be using instead of hdev->bdaddr?
>
> Right now the identity address isn't stored in any variable, but we do
> have a hci_copy_identity_address() function to fetch it (you'd just
> ignore the addr_type if you don't need that).
>
> Also note that the identity address isn't necessarily the same as what
> was used to create the connection, so if the connection address is of
> importance we track that in hci_conn->init_addr and hci_conn->resp_addr.

Following the recommendation of rfc7668 which say the node should
always change its address when connecting, which means advertise a
different address, also the router should periodically change is RPA,
thus the network interface may have to assume different MAC addresses
which I don't is currently possible or we are back to having the
interfaces created on demand.
Luiz Augusto von Dentz July 19, 2016, 8:49 a.m. UTC | #8
Hi Alex,

On Tue, Jul 19, 2016 at 12:52 AM, Alexander Aring <aar@pengutronix.de> wrote:
>>> If you have the current debugfs UAPI 1:1 already in some kind of stable
>>> bluetooth API then I would give you the BIG advice to fix that and let
>>> me look into that. (As far I know there exists idea's only, no
>>> implementations).
>>
>> There is the patches posted by Patrik implementing the management interface:
>>
>> http://www.spinics.net/lists/linux-bluetooth/msg66486.html
>>
> Okay, just to be sure here, in the above document:
>
> Controller ID = hdev(usually var name, struct hci_dev)->id
>
> Interface Index = dev(usualy var name, struct net_device)->idx
>
> Is this right? I think so, so my next stuff will base on that.

Yep.

>> Note that these commands are per interface index, so you would be able
>> to have two dongle talking to each other, we could in fact even
>> automate this to test with 2 virtual controllers.
>>
>
> We have similar automating testing in 802.15.4 6LoWPAN with virtual
> transceiver drivers.
>
> I always looked somehow for some virtual driver in bluetooth, does this
> exist mainline?

Take a look at the emulator directory:

https://git.kernel.org/cgit/bluetooth/bluez.git/tree/emulator

>> It is about priorities, we should concentrate in making it work for
>> simpler cases, besides the RFC don't talk about subnets either.
>>
>
> I don't talking about subnets.
>
> I am talking about "make the connect command per interface". Patrik
> works shows the following:
>
> Add Network Command
> ===================
>
>         Command Code:           0x0043
>         Controller Index:       <controller id>
>         Command Parameters:     Address (6 Octets)
>                                 Address_Type (1 Octet)
>         Return Parameters:      Address (6 Octets)
>                                 Address_Type (1 Octet)
>                                 Interface Index (4 Octets)
>
>         This command is used to connect to establish a network connection
>         to a remote BTLE node. A pre-requisite is that LE is supported
>         by the controller, otherwise this command will return a
>         "not supported" response.
>
>         Possible values for the Address_Type parameter:
>                 0       Reserved
>                 1       LE Public
>                 2       LE Random
>
>         This command generates a Command Complete event on success
>         or failure.
>
>         Possible errors:        Busy
>                                 Refused
>                                 Invalid Parameters
>                                 Not Supported
>                                 Invalid Index
>                                 Already Connected
>
> So far I understand, this is "make the connect command per device", the
> device in this case is not a "struct net_device" it's "struct hci_dev".
>
> In the "possible" future case to having multiple interfaces you cannot
> say on which interface this connect should be done. With "connect" I
> mean the l2cap_chan_connect call.

And why would we want more than one one net_dev per hci_dev, the
rfc7668 only talks about star topology:

https://tools.ietf.org/html/rfc7668#section-2.2

Bluetooth currently don't have support for a mesh topology, and anyway
I don't think the userspace would need to be involved in the selection
of the netdev since this is beyond the Bluetooth interface. In PAN for
example we end up doing one netdev per connection and the which the
userspace managing using a bridge.

> btw:
> Also I don't know what's now the parameters "Address/Address_Type"
> source/destination? Is the Input of them equal to the output?

That is always the destination address, and we needs its type to know
if it is public or private to act accordingly.

>>
>>> I observed, what I explained above. There exists two "connect" mechanism,
>>> these are L2 and L3.
>>>
>>> With L2 I mean $FIND_OUT_RIGHT_NOT_DEPRECATED_TOOL_IN_BT_UTILITY_DSCHUNGEL
>>> leadv/lecc stuff. This prepares somehow that you can say
>>> l2cap_chan_connect in the upper-layer to connect to slaves.
>>>
>>> With l2cap_chan_connect I mean L3 connection, which is the same like
>>> what you doing in L2 for L3. It's also not be a MUST, because you can
>>> run e.g. above example Node A, B, C, D on L2 and:
>>>
>>> BTLE 6LoWPAN: A <-> B
>>>
>>> L2CAP_SOCK:   C <-> D
>>>
>>> or some other mixed scenario. I think you want to still have such behaviour.
>>> This would be the same like: A, B, C, D and 6lo0 is connected to A, B.
>>> Forget the C, D, make with them whatever you want (_maybe_ namespace with
>>> C, D or l2cap_sock).
>>
>> Im not sure where you are going with l2cap_sock here, yes we can make
>> it connect to different peers but usually each peer assumes a role in
>> BLE, the central role scans and connect and the peripheral only
>> advertise. If you do have more than one controller it should be fine
>> to have multiple network interfaces and then perhaps you can bridge
>> them together, or not, but this should be controlled via userspace,
>> the kernel shall only manage the network interface.
>>
>
> Yes, but the kernel needs an UAPI so the user can control it over userspace.
> See above my "make connect per interface" suggestion.

This is API is for Bluetooth only, it shall not be mixed with network
interface management.
--
To unsubscribe from this list: send the line "unsubscribe linux-wpan" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Michael Richardson July 19, 2016, 2:48 p.m. UTC | #9
Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
    > And why would we want more than one one net_dev per hci_dev, the
    > rfc7668 only talks about star topology:

    > https://tools.ietf.org/html/rfc7668#section-2.2

    > Bluetooth currently don't have support for a mesh topology, and anyway
    > I don't think the userspace would need to be involved in the selection
    > of the netdev since this is beyond the Bluetooth interface. In PAN for
    > example we end up doing one netdev per connection and the which the
    > userspace managing using a bridge.

BTLE 4.2 moves quite far to supporting a mesh, but doesn't yet get there.
I'm told that it will come.  BTLE 4.2 does support switching to which
star one belongs quickly enough that one could actually route, but it's
probably not practical yet.

--
]               Never tell me the odds!                 | ipv6 mesh networks [
]   Michael Richardson, Sandelman Software Works        | network architect  [
]     mcr@sandelman.ca  http://www.sandelman.ca/        |   ruby on rails    [

--
To unsubscribe from this list: send the line "unsubscribe linux-wpan" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexander Aring July 19, 2016, 9:05 p.m. UTC | #10
Hi,

On 07/19/2016 10:23 AM, Luiz Augusto von Dentz wrote:
> Hi Alex, Johan,
> 
> On Tue, Jul 19, 2016 at 8:45 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
>> Hi Alex,
>>
...
> 
> Following the recommendation of rfc7668 which say the node should
> always change its address when connecting, which means advertise a
> different address, also the router should periodically change is RPA,
> thus the network interface may have to assume different MAC addresses
> which I don't is currently possible or we are back to having the
> interfaces created on demand.
> 

mhhh, I am not a bluetooth expert. What really means "periodically
change is RPA" and how is this done in the Linux bluetooth?

When "changing the RPA" happens, does that mean the all connections will
be lost (unregister interface). Then the previous connections will be
recreated (register interface) with a complete different MAC address?

I think it will be a complete different MAC address and this happens all
15 minutes.

"The recommended time interval before randomizing new private address is 15
 minutes, as determined by timer T_GAP(private_addr_int) in Table 17.1 of
 the Bluetooth Generic Access Profile [BTCorev4.1]."

If so, then I could understand the reason why recreation of interface.

The only question would for me, how does the bluetooth subsystem handle
such MAC address update with running l2cap channel connections. I
suppose it will close all connections, otherwise the current mainline
solution doesn't work here, because if all channels are disconnected we
get an interface deregister, so far I understood. Afterwards somehow all
connection will be re-established under a new interface, or?

---

Another idea would be stop the interface which allows to change mac
address. Maybe we can have some event handling here, that the mac
address will be changed. If so, such event occur change dev->dev_addr
and open the interface again. stop (ifdown) and open (ifup) with changing
the mac address should be handled by IPv6 applications.

We need at least to run [0] on mac address changes, this reacts on IFUP
and CHANGE netdev notifiers. This will generate the SLAAC address, but
it confuse me somehow to change this address all 15 minutes because the
MAC address changed. :-/

Deregister and register the interface is a no-go in my opinion and we
should have another solution. Especially when such deregister/register
mechanism happens periodically all 15 minutes.

---

I suppose the "RPA" will be the device address which also need to be
filled in as source/target address option in NS/NA/RS/RA then. It's just
difficult to deal with that when the address will be changed
periodically in 15 minutes.

btw: I talked with C. Gomez today, because I am at the IETF. He would
like to help here to make everything clear and agreed that I cc him here.

- Alex

[0] http://lxr.free-electrons.com/source/net/ipv6/addrconf.c#L3333
--
To unsubscribe from this list: send the line "unsubscribe linux-wpan" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexander Aring July 19, 2016, 9:24 p.m. UTC | #11
Hi,

On 07/19/2016 10:49 AM, Luiz Augusto von Dentz wrote:
> Hi Alex,
> 
> On Tue, Jul 19, 2016 at 12:52 AM, Alexander Aring <aar@pengutronix.de> wrote:
>>>> If you have the current debugfs UAPI 1:1 already in some kind of stable
>>>> bluetooth API then I would give you the BIG advice to fix that and let
>>>> me look into that. (As far I know there exists idea's only, no
>>>> implementations).
>>>
>>> There is the patches posted by Patrik implementing the management interface:
>>>
>>> http://www.spinics.net/lists/linux-bluetooth/msg66486.html
>>>
>> Okay, just to be sure here, in the above document:
>>
>> Controller ID = hdev(usually var name, struct hci_dev)->id
>>
>> Interface Index = dev(usualy var name, struct net_device)->idx
>>
>> Is this right? I think so, so my next stuff will base on that.
> 
> Yep.
> 
>>> Note that these commands are per interface index, so you would be able
>>> to have two dongle talking to each other, we could in fact even
>>> automate this to test with 2 virtual controllers.
>>>
>>
>> We have similar automating testing in 802.15.4 6LoWPAN with virtual
>> transceiver drivers.
>>
>> I always looked somehow for some virtual driver in bluetooth, does this
>> exist mainline?
> 
> Take a look at the emulator directory:
> 
> https://git.kernel.org/cgit/bluetooth/bluez.git/tree/emulator
> 

cool.

>>> It is about priorities, we should concentrate in making it work for
>>> simpler cases, besides the RFC don't talk about subnets either.
>>>
>>
>> I don't talking about subnets.
>>
>> I am talking about "make the connect command per interface". Patrik
>> works shows the following:
>>
>> Add Network Command
>> ===================
>>
>>         Command Code:           0x0043
>>         Controller Index:       <controller id>
>>         Command Parameters:     Address (6 Octets)
>>                                 Address_Type (1 Octet)
>>         Return Parameters:      Address (6 Octets)
>>                                 Address_Type (1 Octet)
>>                                 Interface Index (4 Octets)
>>
>>         This command is used to connect to establish a network connection
>>         to a remote BTLE node. A pre-requisite is that LE is supported
>>         by the controller, otherwise this command will return a
>>         "not supported" response.
>>
>>         Possible values for the Address_Type parameter:
>>                 0       Reserved
>>                 1       LE Public
>>                 2       LE Random
>>
>>         This command generates a Command Complete event on success
>>         or failure.
>>
>>         Possible errors:        Busy
>>                                 Refused
>>                                 Invalid Parameters
>>                                 Not Supported
>>                                 Invalid Index
>>                                 Already Connected
>>
>> So far I understand, this is "make the connect command per device", the
>> device in this case is not a "struct net_device" it's "struct hci_dev".
>>
>> In the "possible" future case to having multiple interfaces you cannot
>> say on which interface this connect should be done. With "connect" I
>> mean the l2cap_chan_connect call.
> 
> And why would we want more than one one net_dev per hci_dev, the
> rfc7668 only talks about star topology:
> 
> https://tools.ietf.org/html/rfc7668#section-2.2
> 
> Bluetooth currently don't have support for a mesh topology, and anyway
> I don't think the userspace would need to be involved in the selection
> of the netdev since this is beyond the Bluetooth interface. In PAN for
> example we end up doing one netdev per connection and the which the
> userspace managing using a bridge.
> 

I am sure that there will be some use-case with namespaces some day.

In general:

In my opinion, having hci_dev as input parameter and as output the
interface will do something hidden behind which you can't control.
The Output return value "interface" will indicate on which interface
the operation was operated on. For me it's something behind which you
cannot control as user. E.g. When having multiple interfaces. On 1:1
mapping, I agree this should work.

But why not simple drop the interface idx from the Output and move it to
Input?

         Command Code:           0x0043
         Controller Index:       <controller id>
         Command Parameters:     Address (6 Octets)
                                 Address_Type (1 Octet)
                                 Interface Index (4 Octets)
         Return Parameters:      Address (6 Octets)
                                 Address_Type (1 Octet)

If we get some way that the 6LoWPAN interface can be created before and
not when doing the first "connect command" then the "Controller Index"
would be unnecessary because you would give that information when
creating an interface.

>> btw:
>> Also I don't know what's now the parameters "Address/Address_Type"
>> source/destination? Is the Input of them equal to the output?
> 
> That is always the destination address, and we needs its type to know
> if it is public or private to act accordingly.
> 

okay.

- Alex
--
To unsubscribe from this list: send the line "unsubscribe linux-wpan" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Johan Hedberg July 20, 2016, 7:39 a.m. UTC | #12
Hi Alex,

On Tue, Jul 19, 2016, Alexander Aring wrote:
> mhhh, I am not a bluetooth expert. What really means "periodically
> change is RPA" and how is this done in the Linux bluetooth?
> 
> When "changing the RPA" happens, does that mean the all connections will
> be lost (unregister interface). Then the previous connections will be
> recreated (register interface) with a complete different MAC address?

Connections aren't lost when the random address is changed. That's why
we track the hci_conn->init_addr and hci_conn->resp_addr. The way this
is dealt with e.g. the Security Manager protocol is that the connection
creation address *is* the address for the remote device throughout the
entire connection, no matter what happens to the local and remote random
address during the connection. I would expect 6LoWPAN do do something
similar.

What was still unclear to me (maybe I missed it in the thread): does the
6LoWPAN for LE spec require using the connection creation address or the
identity address? If it would be the latter then that's something that
will never change and the whole interface recreation issue goes away.

Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-wpan" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Luiz Augusto von Dentz July 20, 2016, 8:14 a.m. UTC | #13
Hi Johan,

On Wed, Jul 20, 2016 at 10:39 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi Alex,
>
> On Tue, Jul 19, 2016, Alexander Aring wrote:
>> mhhh, I am not a bluetooth expert. What really means "periodically
>> change is RPA" and how is this done in the Linux bluetooth?
>>
>> When "changing the RPA" happens, does that mean the all connections will
>> be lost (unregister interface). Then the previous connections will be
>> recreated (register interface) with a complete different MAC address?
>
> Connections aren't lost when the random address is changed. That's why
> we track the hci_conn->init_addr and hci_conn->resp_addr. The way this
> is dealt with e.g. the Security Manager protocol is that the connection
> creation address *is* the address for the remote device throughout the
> entire connection, no matter what happens to the local and remote random
> address during the connection. I would expect 6LoWPAN do do something
> similar.
>
> What was still unclear to me (maybe I missed it in the thread): does the
> 6LoWPAN for LE spec require using the connection creation address or the
> identity address? If it would be the latter then that's something that
> will never change and the whole interface recreation issue goes away.

For the MAC address I don't really know, but for the link-local IPv6
address I think the RPA shall be used:

   'The IPv6 link-local address configuration described in Section 3.2.2
   only reveals information about the 6LN to the 6LBR that the 6LBR
   already knows from the link-layer connection.  This means that a
   device using Bluetooth privacy features reveals the same information
   in its IPv6 link-local addresses as in its device addresses.
   Respectively, a device not using privacy at the Bluetooth level will
   not have privacy at the IPv6 link-local address either.'
Marcel Holtmann July 20, 2016, 10:22 a.m. UTC | #14
Hi Johan,

>> mhhh, I am not a bluetooth expert. What really means "periodically
>> change is RPA" and how is this done in the Linux bluetooth?
>> 
>> When "changing the RPA" happens, does that mean the all connections will
>> be lost (unregister interface). Then the previous connections will be
>> recreated (register interface) with a complete different MAC address?
> 
> Connections aren't lost when the random address is changed. That's why
> we track the hci_conn->init_addr and hci_conn->resp_addr. The way this
> is dealt with e.g. the Security Manager protocol is that the connection
> creation address *is* the address for the remote device throughout the
> entire connection, no matter what happens to the local and remote random
> address during the connection. I would expect 6LoWPAN do do something
> similar.
> 
> What was still unclear to me (maybe I missed it in the thread): does the
> 6LoWPAN for LE spec require using the connection creation address or the
> identity address? If it would be the latter then that's something that
> will never change and the whole interface recreation issue goes away.

of course we do not want to leak the identity address if we don't have to. That is why I am thinking that the IPv6 link should always use something close to the link layer address.

The basic assumption should be that when hci0 is powered on, then you get a 6lo network interface. If you power the controller down, then the 6lo interface gets removed.

By default, the 6lo interface should not have a single address assigned to it if there are no connections. When a connection is created, then a new address should be assigned to it. If the connection drops, then it should be removed. And of course there needs to be usage counter attached to each address. If two connections use the same address, then it only gets removed once there is no connection with that source address active anymore (meaning it needs some sort of reference counting).

With usage of public address or static random address, then most likely only one address on 6lo interface is used. This is not strictly mandated by the spec since you can in theory have one connection with a public address and the other with the static random address. However ignore that fact for a bit since it will just work out when dealing with RPAs anyway. The importance is the link layer address that is used when establishing the connection.

In case you use RPAs, then every connection will have a new link layer address (we rotate the RPA when disabling advertising). As long as the connection is active, that source address should be represented on 6lo interface. However this is no difference either since per spec each new connection can have a different address from all the existing connections. That is always valid with the link layer.

Regards

Marcel

--
To unsubscribe from this list: send the line "unsubscribe linux-wpan" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index a37027a..37a2256 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -399,6 +399,16 @@  struct hci_dev {
 
 	struct led_trigger	*power_led;
 
+#if IS_ENABLED(CONFIG_BT_6LOWPAN)
+	/* TODO for netns support this need to be a list
+	 * variables are protected by RTNL here
+	 */
+	struct net_device *ldev;
+	bool lowpan_enable;
+	/* delete lowpan iface via debugfs workaround */
+	bool pending_deletion;
+#endif
+
 	int (*open)(struct hci_dev *hdev);
 	int (*close)(struct hci_dev *hdev);
 	int (*flush)(struct hci_dev *hdev);
diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
new file mode 100644
index 0000000..a8a3410
--- /dev/null
+++ b/net/bluetooth/6lowpan.c
@@ -0,0 +1,1015 @@ 
+/*
+   (C) 2016 Pengutronix, Alexander Aring <aar@pengutronix.de>
+   Copyright (c) 2013-2014 Intel Corp.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License version 2 and
+   only version 2 as published by the Free Software Foundation.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+*/
+
+#include <linux/if_arp.h>
+#include <linux/netdevice.h>
+#include <linux/etherdevice.h>
+#include <linux/module.h>
+#include <linux/debugfs.h>
+
+#include <net/ipv6.h>
+#include <net/ip6_route.h>
+#include <net/addrconf.h>
+
+#include <net/bluetooth/bluetooth.h>
+#include <net/bluetooth/hci_core.h>
+#include <net/bluetooth/l2cap.h>
+
+#include <net/6lowpan.h>
+
+#define LOWPAN_BTLE_VERSION "0.2"
+
+#define lowpan_le48_to_be48(dst, src) baswap((bdaddr_t *)dst, (bdaddr_t *)src)
+#define lowpan_be48_to_le48(dst, src) baswap((bdaddr_t *)dst, (bdaddr_t *)src)
+
+struct lowpan_btle_cb {
+	struct l2cap_chan *chan;
+};
+
+struct lowpan_btle_dev {
+	struct hci_dev *hdev;
+	struct workqueue_struct *workqueue;
+	struct l2cap_chan *listen;
+	struct list_head peers;
+	struct dentry *control;
+};
+
+struct lowpan_peer {
+	struct l2cap_chan *chan;
+
+	struct list_head list;
+};
+
+struct lowpan_chan_data {
+	struct lowpan_peer peer;
+	struct net_device *dev;
+};
+
+struct lowpan_xmit_work {
+	struct work_struct work;
+	struct l2cap_chan *chan;
+	struct net_device *dev;
+	unsigned int true_len;
+	struct sk_buff *skb;
+};
+
+static inline struct lowpan_btle_cb *
+lowpan_btle_cb(const struct sk_buff *skb)
+{
+	return (struct lowpan_btle_cb *)skb->cb;
+}
+
+static inline struct lowpan_chan_data *
+lowpan_chan_data(const struct l2cap_chan *chan)
+{
+	return (struct lowpan_chan_data *)chan->data;
+}
+
+static inline struct lowpan_btle_dev *
+lowpan_btle_dev(const struct net_device *dev)
+{
+	return (struct lowpan_btle_dev *)lowpan_dev(dev)->priv;
+}
+
+static inline struct lowpan_peer *
+lowpan_lookup_peer(struct lowpan_btle_dev *btdev, bdaddr_t *addr)
+{
+	struct lowpan_peer *entry;
+
+	list_for_each_entry_rcu(entry, &btdev->peers, list) {
+		if (!bacmp(&entry->chan->dst, addr))
+			return entry;
+	}
+
+	return NULL;
+}
+
+static int lowpan_give_skb_to_device(struct sk_buff *skb)
+{
+	skb->protocol = htons(ETH_P_IPV6);
+	skb->dev->stats.rx_packets++;
+	skb->dev->stats.rx_bytes += skb->len;
+
+	return netif_rx_ni(skb);
+}
+
+static int lowpan_rx_handlers_result(struct sk_buff *skb, lowpan_rx_result res)
+{
+	switch (res) {
+	case RX_CONTINUE:
+		/* nobody cared about this packet */
+		net_warn_ratelimited("%s: received unknown dispatch\n",
+				     __func__);
+
+		/* fall-through */
+	case RX_DROP_UNUSABLE:
+		kfree_skb(skb);
+
+		/* fall-through */
+	case RX_DROP:
+		return NET_RX_DROP;
+	case RX_QUEUED:
+		return lowpan_give_skb_to_device(skb);
+	default:
+		break;
+	}
+
+	return NET_RX_DROP;
+}
+
+static lowpan_rx_result lowpan_rx_h_iphc(struct sk_buff *skb)
+{
+	struct l2cap_chan *chan = lowpan_btle_cb(skb)->chan;
+	bdaddr_t daddr, saddr;
+	int ret;
+
+	if (!lowpan_is_iphc(*skb_network_header(skb)))
+		return RX_CONTINUE;
+
+	BT_DBG("recv %s dst: %pMR type %d src: %pMR chan %p",
+	       skb->dev->name, &chan->dst, chan->dst_type, &chan->src, chan);
+
+	/* bluetooth chan view is vice-versa */
+	bacpy(&daddr, &chan->src);
+	bacpy(&saddr, &chan->dst);
+
+	ret = lowpan_header_decompress(skb, skb->dev, &daddr, &saddr);
+	if (ret < 0)
+		return RX_DROP_UNUSABLE;
+
+	return RX_QUEUED;
+}
+
+static int lowpan_invoke_rx_handlers(struct sk_buff *skb)
+{
+	lowpan_rx_result res;
+
+#define CALL_RXH(rxh)			\
+	do {				\
+		res = rxh(skb);		\
+		if (res != RX_CONTINUE)	\
+			goto rxh_next;	\
+	} while (0)
+
+	/* likely at first */
+	CALL_RXH(lowpan_rx_h_iphc);
+
+rxh_next:
+	return lowpan_rx_handlers_result(skb, res);
+#undef CALL_RXH
+}
+
+static int lowpan_chan_recv(struct l2cap_chan *chan, struct sk_buff *skb)
+{
+	struct lowpan_chan_data *data = lowpan_chan_data(chan);
+	struct net_device *dev = data->dev;
+	int ret;
+
+	/* TODO handle BT_CONNECTED in bluetooth subsytem? on
+	 * when calling recv callback, I hit that case somehow
+	 */
+	if (!netif_running(dev) || chan->state != BT_CONNECTED ||
+	    !skb->len || !lowpan_is_iphc(skb->data[0]))
+		goto drop;
+
+	/* Replacing skb->dev and followed rx handlers will manipulate skb. */
+	skb = skb_unshare(skb, GFP_ATOMIC);
+	if (!skb)
+		goto out;
+
+	skb->dev = dev;
+	skb_reset_network_header(skb);
+
+	/* remember that one for dst bdaddr. TODO handle that as priv data for
+	 * lowpan_invoke_rx_handlers parameter. Not necessary for skb->cb.
+	 */
+	lowpan_btle_cb(skb)->chan = chan;
+
+	ret = lowpan_invoke_rx_handlers(skb);
+	if (ret == NET_RX_DROP)
+		BT_DBG("recv %s dropped chan %p", skb->dev->name, chan);
+
+	return 0;
+
+drop:
+	kfree_skb(skb);
+out:
+	/* we handle to free skb on error, so must 0
+	 * seems that callback free the skb on error
+	 * otherwise.
+	 */
+	return 0;
+}
+
+static void lowpan_xmit_worker(struct work_struct *work)
+{
+	struct lowpan_btle_dev *btdev;
+	struct lowpan_xmit_work *xw;
+	struct l2cap_chan *chan;
+	struct net_device *dev;
+	struct msghdr msg = { };
+	struct kvec iv;
+	int ret;
+
+	xw = container_of(work, struct lowpan_xmit_work, work);
+	dev = xw->dev;
+	chan = xw->chan;
+	btdev = lowpan_btle_dev(dev);
+
+	iv.iov_base = xw->skb->data;
+	iv.iov_len = xw->skb->len;
+	iov_iter_kvec(&msg.msg_iter, WRITE | ITER_KVEC, &iv, 1, xw->skb->len);
+
+	BT_DBG("l2cap_chan_send %s dst: %pMR type %d src: %pMR chan %p",
+	       dev->name, &chan->dst, chan->dst_type, &chan->src, chan);
+
+	l2cap_chan_lock(chan);
+
+	ret = l2cap_chan_send(chan, &msg, xw->skb->len);
+	BT_DBG("transmit return value %d", ret);
+	if (ret < 0) {
+		BT_DBG("send %s failed chan %p", dev->name, chan);
+		kfree_skb(xw->skb);
+	} else {
+		consume_skb(xw->skb);
+		dev->stats.tx_bytes += xw->true_len;
+		dev->stats.tx_packets++;
+	}
+
+	l2cap_chan_unlock(chan);
+	l2cap_chan_put(chan);
+
+	kfree(xw);
+}
+
+static void lowpan_send_unicast_pkt(struct net_device *dev,
+				    struct l2cap_chan *chan,
+				    struct sk_buff *skb,
+				    unsigned int true_len)
+{
+	struct lowpan_xmit_work *xw;
+
+	/* copy to xmit work buffer */
+	xw = kzalloc(sizeof(*xw), GFP_ATOMIC);
+	if (!xw)
+		return;
+
+	/* chan->lock mutex need to be hold so change context to workqueue */
+	INIT_WORK(&xw->work, lowpan_xmit_worker);
+	xw->true_len = true_len;
+	/* freeing protected by ifdown workqueue sync */
+	xw->dev = dev;
+	/* disallow freeing of skb while context switch */
+	xw->skb = skb_get(skb);
+	/* disallow freeing while context switch */
+	l2cap_chan_hold(chan);
+	xw->chan = chan;
+
+	queue_work(lowpan_btle_dev(dev)->workqueue, &xw->work);
+}
+
+static void lowpan_send_mcast_pkt(struct net_device *dev, struct sk_buff *skb,
+				  unsigned int true_len)
+{
+	struct lowpan_btle_dev *btdev = lowpan_btle_dev(dev);
+	struct lowpan_peer *peer;
+
+	rcu_read_lock();
+
+	BT_DBG("xmit %s starts multicasting", dev->name);
+
+	/* We need to send the packet to every device behind this
+	 * interface, because multicasting.
+	 */
+	list_for_each_entry_rcu(peer, &btdev->peers, list)
+		lowpan_send_unicast_pkt(dev, peer->chan, skb, true_len);
+
+	rcu_read_unlock();
+}
+
+static netdev_tx_t lowpan_xmit(struct sk_buff *skb, struct net_device *dev)
+{
+	struct lowpan_addr_info *info = lowpan_addr_info(skb);
+	struct lowpan_btle_dev *btdev = lowpan_btle_dev(dev);
+	unsigned int true_len = skb->len;
+	struct lowpan_peer *peer;
+	bdaddr_t daddr, saddr;
+	int ret;
+
+	/* We must take a copy of the skb before we modify/replace the ipv6
+	 * header as the header could be used elsewhere.
+	 */
+	skb = skb_unshare(skb, GFP_ATOMIC);
+	if (!skb) {
+		kfree_skb(skb);
+		goto out;
+	}
+
+	lowpan_be48_to_le48(&daddr, info->daddr);
+	lowpan_be48_to_le48(&saddr, info->saddr);
+
+	BT_DBG("xmit ndisc %s dst: %pMR src: %pMR",
+	       dev->name, &daddr, &saddr);
+
+	ret = lowpan_header_compress(skb, dev, &daddr, &saddr);
+	if (ret < 0) {
+		kfree_skb(skb);
+		goto out;
+	}
+
+	/* this should never be the case, otherwise iphc is broken */
+	WARN_ON_ONCE(skb->len > dev->mtu);
+
+	if (!memcmp(&daddr, dev->broadcast, dev->addr_len)) {
+		lowpan_send_mcast_pkt(dev, skb, true_len);
+	} else {
+		btdev = lowpan_btle_dev(dev);
+
+		rcu_read_lock();
+
+		peer = lowpan_lookup_peer(btdev, &daddr);
+		if (peer)
+			lowpan_send_unicast_pkt(dev, peer->chan, skb,
+						true_len);
+
+		rcu_read_unlock();
+	}
+
+	consume_skb(skb);
+
+out:
+	return NETDEV_TX_OK;
+}
+
+static int lowpan_stop(struct net_device *dev)
+{
+	struct lowpan_btle_dev *btdev = lowpan_btle_dev(dev);
+
+	/* synchronize with xmit worker */
+	flush_workqueue(btdev->workqueue);
+	return 0;
+}
+
+static struct sk_buff *lowpan_chan_alloc_skb(struct l2cap_chan *chan,
+					     unsigned long hdr_len,
+					     unsigned long len, int nb)
+{
+	return bt_skb_alloc(hdr_len + len, GFP_KERNEL);
+}
+
+static long lowpan_chan_get_sndtimeo(struct l2cap_chan *chan)
+{
+	return L2CAP_CONN_TIMEOUT;
+}
+
+static struct l2cap_chan *lowpan_chan_create(struct net_device *dev)
+{
+	struct lowpan_chan_data *data;
+	struct l2cap_chan *chan;
+
+	chan = l2cap_chan_create();
+	if (!chan)
+		return ERR_PTR(-ENOMEM);
+
+	data = kmalloc(sizeof(*data), GFP_KERNEL);
+	if (!data) {
+		l2cap_chan_put(chan);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	l2cap_chan_set_defaults(chan);
+	chan->chan_type = L2CAP_CHAN_CONN_ORIENTED;
+	chan->mode = L2CAP_MODE_LE_FLOWCTL;
+	chan->imtu = dev->mtu;
+	chan->data = data;
+
+	data->peer.chan = chan;
+	data->dev = dev;
+
+	return chan;
+}
+
+static struct l2cap_chan *lowpan_chan_new_conn(struct l2cap_chan *pchan)
+{
+	struct lowpan_chan_data *data = lowpan_chan_data(pchan);
+	struct l2cap_chan *chan;
+
+	chan = lowpan_chan_create(data->dev);
+	if (IS_ERR(chan))
+		return NULL;
+
+	chan->ops = pchan->ops;
+	return chan;
+}
+
+static void lowpan_chan_ready(struct l2cap_chan *chan)
+{
+	struct lowpan_chan_data *data = lowpan_chan_data(chan);
+	struct lowpan_btle_dev *btdev = lowpan_btle_dev(data->dev);
+
+	BT_DBG("%s chan %p ready ", data->dev->name, chan);
+
+	/* make it visible for xmit */
+	list_add_tail_rcu(&data->peer.list, &btdev->peers);
+	synchronize_rcu();
+}
+
+static void lowpan_chan_close(struct l2cap_chan *chan)
+{
+	struct lowpan_chan_data *data = lowpan_chan_data(chan);
+
+	BT_DBG("%s chan %p closed ", data->dev->name, chan);
+
+	/* make it unvisible for xmit */
+	list_del_rcu(&data->peer.list);
+	synchronize_rcu();
+	kfree(data);
+}
+
+static const struct l2cap_ops lowpan_chan_ops = {
+	.name			= "L2CAP 6LoWPAN channel",
+	.new_connection		= lowpan_chan_new_conn,
+	.recv			= lowpan_chan_recv,
+	.close			= lowpan_chan_close,
+	.state_change		= l2cap_chan_no_state_change,
+	.ready			= lowpan_chan_ready,
+	.get_sndtimeo		= lowpan_chan_get_sndtimeo,
+	.alloc_skb		= lowpan_chan_alloc_skb,
+	.teardown		= l2cap_chan_no_teardown,
+	.defer			= l2cap_chan_no_defer,
+	.set_shutdown		= l2cap_chan_no_set_shutdown,
+	.resume			= l2cap_chan_no_resume,
+	.suspend		= l2cap_chan_no_suspend,
+};
+
+static int lowpan_change_mtu(struct net_device *dev, int new_mtu)
+{
+	struct lowpan_btle_dev *btdev = lowpan_btle_dev(dev);
+
+	/* if dev is down, peers list are protected by RTNL */
+	if (netif_running(dev) || !list_empty(&btdev->peers))
+		return -EBUSY;
+
+	if (new_mtu < IPV6_MIN_MTU)
+		return -EINVAL;
+
+	dev->mtu = new_mtu;
+	return 0;
+}
+
+static const struct net_device_ops netdev_ops = {
+	.ndo_init		= lowpan_dev_init,
+	.ndo_stop		= lowpan_stop,
+	.ndo_start_xmit		= lowpan_xmit,
+	.ndo_change_mtu		= lowpan_change_mtu,
+};
+
+static void lowpan_free_netdev(struct net_device *dev)
+{
+	struct lowpan_btle_dev *btdev = lowpan_btle_dev(dev);
+	struct lowpan_peer *peer, *tmp;
+	struct l2cap_chan *chan;
+
+	l2cap_chan_close(btdev->listen, 0);
+	l2cap_chan_put(btdev->listen);
+
+	/* no rcu here? should be safe netdev is down */
+	list_for_each_entry_safe(peer, tmp, &btdev->peers, list) {
+		/* close will free peer data, so save chan here */
+		chan = peer->chan;
+
+		/* will del peer from list and and free.
+		 * should be safe by tmp pointer which has
+		 * a pointer for the next entry.
+		 */
+		l2cap_chan_close(chan, 0);
+		l2cap_chan_put(chan);
+	}
+
+	destroy_workqueue(btdev->workqueue);
+	btdev->hdev->lowpan_enable = false;
+	debugfs_remove(btdev->control);
+	hci_dev_put(btdev->hdev);
+}
+
+static void lowpan_setup(struct net_device *dev)
+{
+	memset(dev->broadcast, 0xff, sizeof(bdaddr_t));
+
+	dev->netdev_ops	= &netdev_ops;
+	dev->destructor	= lowpan_free_netdev;
+	dev->features	|= NETIF_F_NETNS_LOCAL;
+}
+
+static struct device_type bt_type = {
+	.name	= "bluetooth",
+};
+
+static struct l2cap_chan *lowpan_listen_chan_new_conn(struct l2cap_chan *pchan)
+{
+	struct l2cap_chan *chan;
+
+	chan = lowpan_chan_create(pchan->data);
+	if (IS_ERR(chan))
+		return NULL;
+
+	/* change ops with more functionality than listen,
+	 * which also free chan->data stuff.
+	 */
+	chan->ops = &lowpan_chan_ops;
+
+	return chan;
+}
+
+static const struct l2cap_ops lowpan_listen_chan_ops = {
+	.name			= "L2CAP 6LoWPAN listen channel",
+	.new_connection		= lowpan_listen_chan_new_conn,
+	.recv			= l2cap_chan_no_recv,
+	.close			= l2cap_chan_no_close,
+	.state_change		= l2cap_chan_no_state_change,
+	.ready			= l2cap_chan_no_ready,
+	.get_sndtimeo		= l2cap_chan_no_get_sndtimeo,
+	.alloc_skb		= l2cap_chan_no_alloc_skb,
+	.teardown		= l2cap_chan_no_teardown,
+	.defer			= l2cap_chan_no_defer,
+	.set_shutdown		= l2cap_chan_no_set_shutdown,
+	.resume			= l2cap_chan_no_resume,
+	.suspend		= l2cap_chan_no_suspend,
+};
+
+static int lowpan_create_listen_chan(struct net_device *dev)
+{
+	struct lowpan_btle_dev *btdev = lowpan_btle_dev(dev);
+	struct l2cap_chan *chan;
+	u8 bdaddr_type;
+	int ret;
+
+	/* don't use lowpan_chan_create here, because less functionality */
+	chan = l2cap_chan_create();
+	if (!chan)
+		return -ENOMEM;
+
+	chan->data = dev;
+	chan->ops = &lowpan_listen_chan_ops;
+	hci_copy_identity_address(btdev->hdev, &chan->src, &bdaddr_type);
+	if (bdaddr_type == ADDR_LE_DEV_PUBLIC)
+		chan->src_type = BDADDR_LE_PUBLIC;
+	else
+		chan->src_type = BDADDR_LE_RANDOM;
+
+	chan->state = BT_LISTEN;
+	atomic_set(&chan->nesting, L2CAP_NESTING_PARENT);
+
+	BT_DBG("chan %p src type %d", chan, chan->src_type);
+	ret = l2cap_add_psm(chan, BDADDR_ANY, cpu_to_le16(L2CAP_PSM_IPSP));
+	if (ret) {
+		l2cap_chan_put(chan);
+		BT_ERR("psm cannot be added err %d", ret);
+		return ret;
+	}
+	btdev->listen = chan;
+
+	return 0;
+}
+
+static const struct file_operations lowpan_control_fops;
+
+static struct net_device *lowpan_btle_newlink(struct hci_dev *hdev)
+{
+	struct lowpan_btle_dev *btdev;
+	struct net_device *dev;
+	int err = -ENOMEM;
+
+	dev = alloc_netdev(LOWPAN_PRIV_SIZE(sizeof(struct lowpan_btle_dev)),
+			   LOWPAN_IFNAME_TEMPLATE, NET_NAME_ENUM, lowpan_setup);
+	if (!dev)
+		goto out;
+
+	dev->addr_assign_type = NET_ADDR_PERM;
+	dev->addr_len = sizeof(hdev->bdaddr.b);
+	lowpan_le48_to_be48(dev->dev_addr, &hdev->bdaddr);
+
+	SET_NETDEV_DEV(dev, &hdev->dev);
+	SET_NETDEV_DEVTYPE(dev, &bt_type);
+
+	btdev = lowpan_btle_dev(dev);
+	/* avoid freeing */
+	btdev->hdev = hci_dev_hold(hdev);
+	INIT_LIST_HEAD(&btdev->peers);
+
+	btdev->workqueue = create_singlethread_workqueue(dev->name);
+	if (!btdev->workqueue) {
+		free_netdev(dev);
+		goto out;
+	}
+
+	err = lowpan_create_listen_chan(dev);
+	if (err < 0) {
+		destroy_workqueue(btdev->workqueue);
+		free_netdev(dev);
+		goto out;
+	}
+
+	err = lowpan_register_netdevice(dev, LOWPAN_LLTYPE_BTLE);
+	if (err < 0) {
+		BT_ERR("register_netdev failed %d", err);
+		l2cap_chan_close(btdev->listen, 0);
+		l2cap_chan_put(btdev->listen);
+		free_netdev(dev);
+		goto out;
+	}
+	hdev->ldev = dev;
+
+	/* ignore error handling here, we cannot call unregister anymore
+	 * It's a bug, but it's debugfs... so ignore it.
+	 */
+	btdev->control = debugfs_create_file(dev->name, 0644,
+					     bt_debugfs, btdev,
+					     &lowpan_control_fops);
+	if (!btdev->control)
+		BT_ERR("debugfs error for %s, disable/enable 6lowpan again",
+		       dev->name);
+
+	return dev;
+
+out:
+	return ERR_PTR(err);
+}
+
+static void lowpan_btle_dellink(struct net_device *dev)
+{
+	lowpan_unregister_netdevice(dev);
+}
+
+static int lowpan_parse_le_bdaddr(struct hci_dev *hdev, unsigned char *buf,
+				  bdaddr_t *addr, u8 *addr_type)
+{
+	int n;
+
+	n = sscanf(buf, "%hhx:%hhx:%hhx:%hhx:%hhx:%hhx %hhu",
+		   &addr->b[5], &addr->b[4], &addr->b[3],
+		   &addr->b[2], &addr->b[1], &addr->b[0],
+		   addr_type);
+	if (n < 7)
+		return -EINVAL;
+
+	/* check if we handle le addresses and not own source address */
+	if (!bdaddr_type_is_le(*addr_type) ||
+	    !bacmp(&hdev->bdaddr, addr))
+		return -EINVAL;
+
+	return n;
+}
+
+static ssize_t __lowpan_control_write(struct file *fp,
+				      const char __user *user_buffer,
+				      size_t count, loff_t *position)
+{
+	char buf[32] = { };
+	size_t buf_size = min(count, sizeof(buf) - 1);
+	struct seq_file *file = fp->private_data;
+	struct lowpan_btle_dev *btdev = file->private;
+	struct hci_dev *hdev = btdev->hdev;
+	struct lowpan_peer *peer;
+	/* slave address */
+	bdaddr_t addr;
+	u8 addr_type;
+	int ret;
+
+	if (copy_from_user(buf, user_buffer, buf_size))
+		return -EFAULT;
+
+	if (memcmp(buf, "connect ", 8) == 0) {
+		struct lowpan_peer *peer;
+		struct l2cap_chan *chan;
+
+		ret = lowpan_parse_le_bdaddr(hdev, &buf[8], &addr, &addr_type);
+		if (ret < 0)
+			return ret;
+
+		/* check if we already know that slave */
+		rcu_read_lock();
+		peer = lowpan_lookup_peer(btdev, &addr);
+		if (peer) {
+			rcu_read_unlock();
+			BT_DBG("6LoWPAN connection already exists");
+			return -EEXIST;
+		}
+		rcu_read_unlock();
+
+		chan = lowpan_chan_create(hdev->ldev);
+		if (IS_ERR(chan))
+			return PTR_ERR(chan);
+		chan->ops = &lowpan_chan_ops;
+
+		ret = l2cap_hdev_chan_connect(hdev, chan,
+					      cpu_to_le16(L2CAP_PSM_IPSP),
+					      0, &addr, addr_type);
+		if (ret < 0) {
+			l2cap_chan_put(chan);
+			return ret;
+		}
+
+		return count;
+	} else if (memcmp(buf, "disconnect ", 11) == 0) {
+		ret = lowpan_parse_le_bdaddr(hdev, &buf[11], &addr, &addr_type);
+		if (ret < 0)
+			return ret;
+
+		/* check if we don't know that slave */
+		rcu_read_lock();
+		peer = lowpan_lookup_peer(btdev, &addr);
+		if (!peer) {
+			rcu_read_unlock();
+			BT_DBG("6LoWPAN connection not found in peers");
+			return -ENOENT;
+		}
+		rcu_read_unlock();
+
+		/* first delete the peer out of list,
+		 * which makes it visiable to netdev,
+		 * will be done by close callback.
+		 */
+		l2cap_chan_close(peer->chan, 0);
+		l2cap_chan_put(peer->chan);
+	} else {
+		return -EINVAL;
+	}
+
+	return count;
+}
+
+static ssize_t lowpan_control_write(struct file *fp,
+				    const char __user *user_buffer,
+				    size_t count, loff_t *position)
+{
+	ssize_t ret;
+
+	rtnl_lock();
+	ret = __lowpan_control_write(fp, user_buffer, count, position);
+	rtnl_unlock();
+
+	return ret;
+}
+
+static int lowpan_control_show(struct seq_file *f, void *ptr)
+{
+	struct lowpan_btle_dev *btdev = f->private;
+	struct hci_dev *hdev = btdev->hdev;
+	struct lowpan_peer *peer;
+
+	rtnl_lock();
+
+	if (!hdev->lowpan_enable) {
+		rtnl_unlock();
+		return 0;
+	}
+
+	rcu_read_lock();
+
+	list_for_each_entry_rcu(peer, &btdev->peers, list) {
+		seq_printf(f, "%pMR (type %u) state: %s\n",
+			   &peer->chan->dst, peer->chan->dst_type,
+			   state_to_string(peer->chan->state));
+	}
+
+	rcu_read_unlock();
+
+	rtnl_unlock();
+	return 0;
+}
+
+static int lowpan_control_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, lowpan_control_show, inode->i_private);
+}
+
+static const struct file_operations lowpan_control_fops = {
+	.open		= lowpan_control_open,
+	.read		= seq_read,
+	.write		= lowpan_control_write,
+	.llseek		= seq_lseek,
+	.release	= single_release,
+};
+
+/* TODO remove this when add non debugfs API, it's a workaround */
+static struct workqueue_struct *workqueue_cleanup;
+
+struct lowpan_cleanup_work {
+	struct work_struct work;
+	struct net_device *dev;
+};
+
+static void lowpan_cleanup_worker(struct work_struct *work)
+{
+	struct lowpan_cleanup_work *xw;
+
+	xw = container_of(work, struct lowpan_cleanup_work, work);
+
+	rtnl_lock();
+	lowpan_btle_dellink(xw->dev);
+	lowpan_btle_dev(xw->dev)->hdev->pending_deletion = false;
+	rtnl_unlock();
+
+	kfree(xw);
+}
+
+/* TODO workaround, debugfs doesn't like recursion remove while debugfs
+ * handling, this should be removed for by using real UAPI.
+ */
+static int __lowpan_btle_dellink(struct net_device *dev,
+				 struct hci_dev *hdev)
+{
+	struct lowpan_cleanup_work *xw;
+
+	xw = kzalloc(sizeof(*xw), GFP_KERNEL);
+	if (!xw)
+		return -ENOMEM;
+
+	INIT_WORK(&xw->work, lowpan_cleanup_worker);
+	xw->dev = dev;
+
+	hdev->pending_deletion = true;
+	queue_work(workqueue_cleanup, &xw->work);
+	return 0;
+}
+
+static int lowpan_enable_set(struct hci_dev *hdev, bool enabled)
+{
+	int ret = 0;
+
+	if (hdev->lowpan_enable == enabled)
+		goto out;
+
+	if (enabled) {
+		struct net_device *dev;
+
+		/* create one interface by default */
+		dev = lowpan_btle_newlink(hdev);
+		if (IS_ERR(dev))
+			return PTR_ERR(dev);
+
+		/* TODO should be done by user network managers? */
+		ret = dev_open(dev);
+		if (ret < 0) {
+			BT_ERR("iface %s cannot be opened %d", dev->name, ret);
+			ret = __lowpan_btle_dellink(dev, hdev);
+			if (ret < 0)
+				BT_ERR("failed to remove interface %s",
+				       hdev->ldev->name);
+
+			return ret;
+		}
+
+		hdev->lowpan_enable = true;
+	} else {
+		/* ignore pending deletions */
+		if (hdev->pending_deletion)
+			return 0;
+
+		__lowpan_btle_dellink(hdev->ldev, hdev);
+	}
+
+out:
+	return ret;
+}
+
+static void lowpan_btle_hci_unregister(struct hci_dev *hdev)
+{
+	/* no need to cleanup debugfs, will be done by hci_core */
+
+	/* TODO netns support with multiple lowpan interfaces */
+	if (hdev->lowpan_enable)
+		lowpan_btle_dellink(hdev->ldev);
+}
+
+static int lowpan_hci_dev_event(struct notifier_block *unused,
+				unsigned long event, void *ptr)
+{
+	struct hci_dev *hdev = ptr;
+	int ret = NOTIFY_OK;
+
+	rtnl_lock();
+
+	/* bluetooth handles that event type as int,
+	 * but there is no overflow yet.
+	 */
+	switch (event) {
+	case HCI_DEV_UNREG:
+		lowpan_btle_hci_unregister(hdev);
+		ret = NOTIFY_DONE;
+		break;
+	default:
+		break;
+	}
+
+	rtnl_unlock();
+
+	return ret;
+}
+
+static struct notifier_block lowpan_hcI_dev_notifier = {
+	.notifier_call = lowpan_hci_dev_event,
+};
+
+static ssize_t lowpan_enable_write(struct file *fp,
+				   const char __user *user_buffer,
+				   size_t count, loff_t *position)
+{
+	char buf[32] = { };
+	size_t buf_size = min(count, sizeof(buf) - 1);
+	struct hci_dev *hdev;
+	int idx, enabled, n, ret;
+
+	if (copy_from_user(buf, user_buffer, buf_size))
+		return -EFAULT;
+
+	n = sscanf(buf, "hci%d %d", &idx, &enabled);
+	if (n != 2)
+		return -EINVAL;
+
+	hdev = hci_dev_get(idx);
+	if (!hdev)
+		return -EINVAL;
+
+	rtnl_lock();
+	ret = lowpan_enable_set(hdev, enabled);
+	rtnl_unlock();
+
+	hci_dev_put(hdev);
+
+	return count;
+}
+
+static int lowpan_enable_show(struct seq_file *f, void *ptr)
+{
+	struct hci_dev *hdev;
+
+	rtnl_lock();
+	read_lock(&hci_dev_list_lock);
+	list_for_each_entry(hdev, &hci_dev_list, list) {
+		if (hdev->lowpan_enable)
+			seq_printf(f, "hci%d -> %s\n", hdev->id,
+				   hdev->ldev->name);
+	}
+	read_unlock(&hci_dev_list_lock);
+	rtnl_unlock();
+
+	return 0;
+}
+
+static int lowpan_enable_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, lowpan_enable_show, inode->i_private);
+}
+
+static const struct file_operations lowpan_enable_fops = {
+	.open		= lowpan_enable_open,
+	.read		= seq_read,
+	.write		= lowpan_enable_write,
+	.llseek		= seq_lseek,
+	.release	= single_release,
+};
+
+static int __init bt_6lowpan_init(void)
+{
+	int err;
+
+	workqueue_cleanup = create_singlethread_workqueue("btle 6lowpan");
+	if (!workqueue_cleanup)
+		return -ENOMEM;
+
+	debugfs_create_file("6lowpan_enable", 0644,  bt_debugfs, NULL,
+			    &lowpan_enable_fops);
+
+	err = register_hci_dev_notifier(&lowpan_hcI_dev_notifier);
+	if (err < 0)
+		destroy_workqueue(workqueue_cleanup);
+
+	return err;
+}
+
+static void __exit bt_6lowpan_exit(void)
+{
+	destroy_workqueue(workqueue_cleanup);
+	unregister_hci_dev_notifier(&lowpan_hcI_dev_notifier);
+}
+
+module_init(bt_6lowpan_init);
+module_exit(bt_6lowpan_exit);
+
+MODULE_AUTHOR("Jukka Rissanen <jukka.rissanen@linux.intel.com>");
+MODULE_DESCRIPTION("Bluetooth 6LoWPAN");
+MODULE_VERSION(LOWPAN_BTLE_VERSION);
+MODULE_LICENSE("GPL");
diff --git a/net/bluetooth/Makefile b/net/bluetooth/Makefile
index e347828..b3ff12e 100644
--- a/net/bluetooth/Makefile
+++ b/net/bluetooth/Makefile
@@ -7,6 +7,9 @@  obj-$(CONFIG_BT_RFCOMM)	+= rfcomm/
 obj-$(CONFIG_BT_BNEP)	+= bnep/
 obj-$(CONFIG_BT_CMTP)	+= cmtp/
 obj-$(CONFIG_BT_HIDP)	+= hidp/
+obj-$(CONFIG_BT_6LOWPAN) += bluetooth_6lowpan.o
+
+bluetooth_6lowpan-y := 6lowpan.o
 
 bluetooth-y := af_bluetooth.o hci_core.o hci_conn.o hci_event.o mgmt.o \
 	hci_sock.o hci_sysfs.o l2cap_core.o l2cap_sock.o smp.o lib.o \