Bluetooth: Implement the BTPROTO_HCI Protocol for AF_BLUETOOTH socket domain.

[btashton]

Right now we only implement the BTPROTO_L2CAP socket for AF_BLUETOOTH, we need to add support for BTPROTO_HCI to be able to really make use of the bluetooth stack for things like advertisement and GATT. Currently there is a network IOCTL but this is fragile and does not map well to the Linux socket interface.

[btashton]

@patacongo @v01d I opened this ticket to handle at least one of the missing interfaces. I'll try and wire this interface to the existing HCI code, I don't think that is actually too much work.

[protobits]

Cool. We can continue discussion here if you like.

[protobits]

@xiaoxiang781216 Please have a look here and the discussion on the list.

[xiaoxiang781216]

@anchao port the latest Zephyr stack here: https://github.com/anchao/zblue Since Zephyr bluetooth stack is still in the active developoment, we take a different approach to minimize the modification of zerphy base as small as possbile by adding a porting layer: https://github.com/anchao/zblue/tree/master/port Yes, it's hard to upstream to the mainline, we share the above code for the reference only.

[patacongo]

IP Clearance is another issue we need to consider with all third party software. Per @justinmclean we would need to get some permission from the Zephyr project to include their code. What is the chance of that?

The current Bluetooth stack did not come from Zephyr but from a separate Intel BSD package. This code did evolve to become the Zephyr Bluetooth stack and could also have IP clearance issues in the future.

An alternative might be to investigate incorporating the Apache MyNewt nimBLE Bluetooth 5.0 stack: https://mynewt.apache.org/. Justin is the chair on the Apache MyNewt project and might be the person to discuss that possibility with.

[patacongo]

I don't know how others feel, but I really dislike these kinds of hooks that hang "a bag off the side of NuttX" and then have a porting layer to work with the OS. I don't think this is the kind of OS the NuttX intends to be. I would much prefer to see a native solution owned and maintained by NuttX people.

We are going this too much: OpenAMP, LittleFS. These is all third party code, downloaded and build into the OS at build time. The end result is a pretty shitty OS architecture in my opinion. It is just lazy and reflects that fact that no one is willing to implement anything from scratch. Let's not let NuttX become a collections of bits and pieces of other OS's with no identity of its own.

[protobits]

I also do not like the idea of basing any improvements of the NuttX BLE stack by depending on external actively developed project such as Zephyr, which moves fast and its purpose is not to provide others with a BLE stack. This means that any internal reorganization of Zephyr stack will negatively impact NuttX (even if it is just adapting the shims). For something as critical and internal as a networking stack, I agree with Greg that this should be a fully contained and native solution. If it really feels like reinventing the wheel and there's a good generic BLE stack that can provide API stability over a long time (MyNewt?), I would consider using that, but only if the shim is clean.

Since we already have a good foundation for a 4.2 BLE stack in NuttX, maybe we should try to improve it with what was recently discussed in the list and what this issue is about first. If we want to then move to 5.0 I would try to improve the existing stack unless it proves to be a complete rewrite (I wouldn't think so since there's some degree of compatibility between spec versions). In any case, I would prioritize consolidating the current stack to upgrade to newer spec version. But I understand that companies with BLE products might want to prioritize 5.0 at some point.

[protobits]

The current Bluetooth stack did not come from Zephyr but from a separate Intel BSD package. This code did evolve to become the Zephyr Bluetooth stack and could also have IP clearance issues in the future.

That is a little more concerning. I don't know how easy would it be to get Intel to make a software grant. At least we know that Zephyr being now Apache licensed must've agreed to that at some point. But maybe keeping this as BSD is acceptable by Apache?

[patacongo]

The current Bluetooth stack did not come from Zephyr but from a separate Intel BSD package. This code did evolve to become the Zephyr Bluetooth stack and could also have IP clearance issues in the future.

That is a little more concerning. I don't know how easy would it be to get Intel to make a software grant. At least we know that Zephyr being now Apache licensed must've agreed to that at some point. But maybe keeping this as BSD is acceptable by Apache?

They don't need to do an SGA. We can carry the stack as third party code by retaining the Intel copyrights in the BSD hardware and adding the files to the LICENSE file.

The only obstacle is that Justin has claimed that license compliance is insufficient and that the ASF wants some kind of (unspecified) permission to use the third party code. I am unsure of all of this stuff but we need to take care so that any additional effort put into the stack is not lost due to some future decision. It was be nice to have this all in order before making such investments.

But, I don't know how we would accomplish that. We are not going to get an SGA, CCLA, or any kind of approval from Intel. I am 100% certain that will never happen. Even large corporations that are friendly to NuttX are unable to get their legal departments to do such things. And I doubt that Intel is friendly in this way.

[btashton]

The HCI socket does not really exist so I'm not too worried since it would come from us and could be ported to swap out the underlying layer if needed. Honestly the existing stack is on the minimal side and has reasonable boundaries to be replaced if needed.

I'll keep plugging on the next couple days. Just want to invest in getting the sim working first.

[acassis]

...

Since we already have a good foundation for a 4.2 BLE stack in NuttX, maybe we should try to improve it with what was recently discussed in the list and what this issue is about first. If we want to then move to 5.0 I would try to improve the existing stack unless it proves to be a complete rewrite (I wouldn't think so since there's some degree of compatibility between spec versions). In any case, I would prioritize consolidating the current stack to upgrade to newer spec version. But I understand that companies with BLE products might want to prioritize 5.0 at some point.

I agree! This approach is safer than taking an external code (moving target) and trying to adapt it to NuttX.

[protobits]

@btashton I'm trying to understand how the code would look like once this new socket interface is added and, specifically, how would the GATT layer use it. For example, to read a GATT characteristic the code path right now is:

1. SIOCBTGATTRD ioctl on the socket
2. bt_gatt_read
3. gatt_send
4. bt_att_send
5. bt_l2cap_send
6. bt_conn_send (pushes to HCI tx queue)
7. hci_tx_kthread (pops from HCI tx queue) calls into bt driver send callback

This last send ends up in the Link Layer I created.

What I'm missing is how the existing L2CAP socket and the new HCI socket would be used here, since for this code path I don't see any socket used at all, it all starts on an ioctl and uses direct calls to all internal APIs. If the HCI socket where to sit between 6 and 7 I understand you would gain access to, for example, enabling advertising from userspace. But to send ATT/GATT you would use the L2CAP socket. Is this correct?

What is also confusing is that the btsak application does not really use the L2CAP socket for more than ioctl calls. I'm guessing that it should already be possible to send L2CAP packets from userspace in NuttX, as I see on bt_netdev,c another call to bt_l2cap_send which I would think it will be called when one write()s the socket. Is that correct?

In that case, besides adding the HCI socket, we also should make the ATT/GATT code into a library that speaks to the L2CAP socket via read/write. Am I understanding this correctly? I could work on that if so, unless you had plans for it or you have a clearer idea on how to do so.

[btashton]

Building on top of L2CAP socket would be the correct way. The netdev ioctrl does not align with anything else and should be moved into the proper socket abstraction HCI L2CAP SCO etc...

In the GATT context, the HCI socket will be required for things like advertising services, but it serves other purposes as well.

[protobits]

Great, I'm starting to understand how it all works together. I've found an example of gatt server code using the BTPROTO_L2CAP: https://github.com/Vudentz/BlueZ/blob/e8204d987d862b1b44fbc85470154ab4eb02cce3/peripheral/gatt.c#L243

[btashton]

@patacongo I started to dig into this a little around handling the socket protocol logic. This is what I am thinking

Currently when the socket is created we call net_sockif(sa_family_t family, int type, int protocol) which gives us a lot of what we need control of.

Linux defines approximately this (I removed some) as protocol dependent

struct proto_ops {
    int     family;
    int     (*release)
    int     (*bind)
    int     (*connect)
    int     (*socketpair)
    int     (*accept)
    int     (*getname)
    int     (*ioctl)
    int     (*gettstamp)
    int     (*listen)
    int     (*shutdown)
    int     (*setsockopt)
    int     (*getsockopt)
    int     (*sendmsg)
    int     (*recvmsg)
};

That maps fairly well to sock_intf_s

struct sock_intf_s
{
  CODE int        (*si_setup)(FAR struct socket *psock, int protocol);
  CODE sockcaps_t (*si_sockcaps)(FAR struct socket *psock);
  CODE void       (*si_addref)(FAR struct socket *psock);
  CODE int        (*si_bind)(FAR struct socket *psock,
                    FAR const struct sockaddr *addr, socklen_t addrlen);
  CODE int        (*si_getsockname)(FAR struct socket *psock,
                    FAR struct sockaddr *addr, FAR socklen_t *addrlen);
  CODE int        (*si_getpeername)(FAR struct socket *psock,
                    FAR struct sockaddr *addr, FAR socklen_t *addrlen);
  CODE int        (*si_listen)(FAR struct socket *psock, int backlog);
  CODE int        (*si_connect)(FAR struct socket *psock,
                    FAR const struct sockaddr *addr, socklen_t addrlen);
  CODE int        (*si_accept)(FAR struct socket *psock,
                    FAR struct sockaddr *addr, FAR socklen_t *addrlen,
                    FAR struct socket *newsock);
  CODE int        (*si_poll)(FAR struct socket *psock,
                    FAR struct pollfd *fds, bool setup);
  CODE ssize_t    (*si_send)(FAR struct socket *psock, FAR const void *buf,
                    size_t len, int flags);
  CODE ssize_t    (*si_sendto)(FAR struct socket *psock, FAR const void *buf,
                    size_t len, int flags, FAR const struct sockaddr *to,
                    socklen_t tolen);
#ifdef CONFIG_NET_SENDFILE
  CODE ssize_t    (*si_sendfile)(FAR struct socket *psock,
                    FAR struct file *infile, FAR off_t *offset,
                    size_t count);
#endif
  CODE ssize_t    (*si_recvfrom)(FAR struct socket *psock, FAR void *buf,
                    size_t len, int flags, FAR struct sockaddr *from,
                    FAR socklen_t *fromlen);
#ifdef CONFIG_NET_CMSG
  CODE ssize_t    (*si_recvmsg)(FAR struct socket *psock,
    FAR struct msghdr *msg, int flags);
  CODE ssize_t    (*si_sendmsg)(FAR struct socket *psock,
    FAR struct msghdr *msg, int flags);
#endif
  CODE int        (*si_close)(FAR struct socket *psock);
#ifdef CONFIG_NET_USRSOCK
  CODE int        (*si_ioctl)(FAR struct socket *psock, int cmd,
                    FAR void *arg, size_t arglen);
#endif
};

Some notable missing interfaces are getsockopt setsockopt and ioctl without NET_USRSOCK. Do you think it would be reasonable to adapt this interface?

[patacongo]

Do you think it would be reasonable to adapt this interface?

My preference would not be to make any major redesign of the networking. Change struct sock_intf_s effects everything. I don't think that is called for. I think it only requires extension of the existing interface.

The si_setup() method is the one that configures the socket for the address family, socket type, and protocol. That is where the additional protocols and channels would have to be handled. But I think it need only retain this information in struct bluetooth_conn_s which part of the socket data structure that holds information unique to an address family.

For Bluetooth sockets, none of this information matters until we get to the network driver layer. I don't think that the selected Bluetooth protocol necessarily has any impact on the network itself. However, when a frame is sent or received, then it does effect the network driver and Bluetooth stack significantly.

I think it is l2 layer where the additional protocols would have to be implemented; I think the socket layer just has to verify and remember the protocol and channel and coordinate this with the network driver / bluetooth stack.

NOTE: I tend to refer everything below the logic under net/ as L2. The architecture is odd for Bluetooth because the Bluetooth stack is not integrated into the NuttX network stack. Normally, the network driver is at the L2 layer, but not here. The network driver sits above the Bluetooth stack and the entire Bluetooth stack is treated as the MAC sub-layer. This is a little unusual, but I think think is a good partitioning.

User-Space
`- BSD Socket Layer
   `- NuttX raw packet support (very minimal)
      `- Network driver
         `- Bluetooth stack
            `- HCI driver

It was originally done that way to support 6LoWPAN sockets on top of L2CAP.

User-Space
`- BSD Socket Layer
   `- IPv6
      `- 6LoWPAN
         `- Network driver
            `- Bluetooth stack
               `- HCI driver

Refs: https://infocenter.nordicsemi.com/index.jsp?topic=%2Fcom.nordic.infocenter.iotsdk.v0.9.0%2Flib_iot_6lowpan.html https://tools.ietf.org/html/draft-ietf-6lowpan-btle-12

struct sock_intf_s is the interface between the user BSD sockets and the per-address-family socket implementation. I don't think that should change. I think the changes need to be in struct netdriver_s which is the interface between the network and the Bluetooth network driver and stack.

It is only the Bluetooth network driver that can tap into the Bluetooth stack to support each protocol; It is only the Bluetooth stack that can manage the channel behavior.

Therefore, nothing much of substance has to change under net/. All of the changes have to occur in the interface between net/ and the network drivers, in the network driver, and the Bluetooth stack.

[btashton]

What If we don't change sock_intf_s would you be open to having multiple versions for the different socket protocols. The reason being that the HCI socket is very different than L2CAP. For example it does not support connections. The code is going to get really ugly handling both of these in the same place. This is further complicated because with the bind there are then multiple "channels" defined inside the address which changes how the writes and reads are handled.

If we store the protocol inside the socket struct it is fairly easy to handle the ioctl and socket option calls without changing sock_intf_s.

FAR const struct sock_intf_s *
net_sockif(sa_family_t family, int type, int protocol)
{
  FAR const struct sock_intf_s *sockif = NULL;
  switch (family)
    {
#ifdef CONFIG_NET_BLUETOOTH
    case PF_BLUETOOTH:
      switch (protocol)
        {
          case BTPROTO_HCI:
            sockif = &g_bluetooth_hcisockif;
            break
          case BTPROTO_L2CAP:
          default:
            sockif = &g_bluetooth_sockif;
            break
        }
      break;
#endif
    default:
      nerr("ERROR: Address family unsupported: %d\n", family);
    }
  return sockif;
}

[patacongo]

What If we don't change sock_intf_s would you be open to having multiple versions for the different socket protocols. The reason being that the HCI socket is very different than L2CAP. For example it does not support connections. The code is going to get really ugly handling both of these in the same place. This is further complicated because with the bind there are then multiple "channels" defined inside the address which changes how the writes and reads are handled.

If we store the protocol inside the socket struct it is fairly easy to handle the ioctl and socket option calls without changing sock_intf_s.

FAR const struct sock_intf_s *
net_sockif(sa_family_t family, int type, int protocol)
{
  FAR const struct sock_intf_s *sockif = NULL;
  switch (family)
    {
#ifdef CONFIG_NET_BLUETOOTH
    case PF_BLUETOOTH:
      switch (protocol)
        {
          case BTPROTO_HCI:
            sockif = &g_bluetooth_hcisockif;
            break
          case BTPROTO_L2CAP:
          default:
            sockif = &g_bluetooth_sockif;
            break
        }
      break;
#endif
    default:
      nerr("ERROR: Address family unsupported: %d\n", family);
    }
  return sockif;
}

All sockets support connection, even connection-less sockets like UDP. When you connect a UDP socket it just remembers the peer address so that you can use recv() instead of recvfrom().

The current L2CAP sockets are currently only connection-less, raw sockets so I am not sure what are getting at exactly. See net/bluetooth/bluetooth_sockif.c:

189   if (psock->s_type == SOCK_RAW)
190     {
191       return bluetooth_sockif_alloc(psock);
192     }
193   else
194     {
195       return -EPROTONOSUPPORT;
196     }

Connected L2CAP sockets are not supported. That would, indeed, probably have to be implemented in the net/ layer.

What you suggest a reasonable thing to do if you believe that the network layer needs to make a distinction between the protocols. That is basically how many of the other sockets work as well. Like PF_INET and PF_INET6 with SOCK_DGRAM or SOCK_STREAM.

It would make sense for the protocol to be stored in struct socket because that is common to all address families. But the address specific channel would need to be saved in the struct bluetooth_conn_s that is linked with the struct socket instance.

[btashton]

All sockets support connection, even connection-less sockets like UDP. When you connect a UDP socket it just remembers the peer address so that you can use recv() instead of recvfrom().

with BTPROTO_HCI there is no concept of accept, connect, listen, or you can only issue a bind This is because this socket is for controlling local hardware there is no remote.

You can see that Linux also does this here: https://elixir.bootlin.com/linux/latest/source/net/bluetooth/hci_sock.c#L1999

I think I can come up with something that will work within the existing network infrastructure with the slight addition of the protocol attached to the socket to help with some of the upper layers of the stack.

You can see an example of the User Channel in my PR for the sim, but here is an example of using the monitor channel

fd = socket(AF_BLUETOOTH, SOCK_RAW, BTPROTO_HCI);
memset(&addr, 0, sizeof(addr));
addr.hci_family = AF_BLUETOOTH;
addr.hci_dev = HCI_DEV_NONE;
addr.hci_channel = HCI_CHANNEL_MONITOR;
bind(fd, (struct sockaddr *) &addr, sizeof(addr));
setsockopt(fd, SOL_SOCKET, SO_TIMESTAMP, &opt, sizeof(opt));

while(1)
  {
    handle_msg(recvmsg(handle->fd, &msg, 0))
  }

[patacongo]

I think I can come up with something that will work within the existing network infrastructure with the slight addition of the protocol attached to the socket to help with some of the upper layers of the stack.

OK. I did offer to help. The offer is still open if you need any assistance.

[btashton]

I will take you up on that, just trying to get what is in my head down in code. The management channel will be the biggest (and most important) to implement I think. https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/doc/mgmt-api.txt

This socket API would replace much of the existing ioctl interface.

[patacongo]

Another trick available to you is to "inherit" the bluetooth device structure from struct netdriver_s (i.e., contain struct netdriver_s). All other radios have done this using struct radio_driver_s as defined in include/nuttx/net/radiodev.h. Using struct radio_driver_s (or perhaps an analogous struct bt_driver_s), you could add some custom fields and methods to the driver interface.

[patacongo]

The management channel will be the biggest (and most important) to implement I think. https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/doc/mgmt-api.txt

Which will also cause a major re-write of apps/wireless/bluetooth/btsak

[protobits]

The management channel will be the biggest (and most important) to implement I think. https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/doc/mgmt-api.txt

Which will also cause a major re-write of apps/wireless/bluetooth/btsak

btsak indeed will have to change to adapt to using both L2CAP and HCI sockets. Right now it opens an L2CAP socket but everything actually goes via ioctl()s to GATT functions which internally use L2CAP. Does adapting btsak actually make sense? Or should there be more modular applications? Having something like hcitool/gattool I think would be better.

For gattool, the more immediate adaptation should be to the ATT/GATT code to convert it into a library which interfaces with L2CAP via socket. This way it would be also possible to write a standalone GATT server. For hcitool I think the HCI socket is already sufficient.

[btashton]

The management channel will be the biggest (and most important) to implement I think. https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/doc/mgmt-api.txt

Which will also cause a major re-write of apps/wireless/bluetooth/btsak

btsak indeed will have to change to adapt to using both L2CAP and HCI sockets. Right now it opens an L2CAP socket but everything actually goes via ioctl()s to GATT functions which internally use L2CAP. Does adapting btsak actually make sense? Or should there be more modular applications? Having something like hcitool/gattool I think would be better.

For gattool, the more immediate adaptation should be to the ATT/GATT code to convert it into a library which interfaces with L2CAP via socket. This way it would be also possible to write a standalone GATT server. For hcitool I think the HCI socket is already sufficient.

My plan was to build something like btmon which is the modern replacement to hcidump that would leverage the HCI socket using the monitor channel. There is also btmgmt which fully exercises the HCI socket control channel, which fits in with btsak better, but it might be easier to just write something new here. For GATT I have always used bluetoothctl which I think has replaced gattool https://wiki.archlinux.org/index.php/bluetooth#Troubleshooting

I'm slowly stubbing things out in the stack trying to make the changes as minor as possible for now.

[btashton]

@patacongo I attached a PR to this that is the very early start to this to see what you think. I ended up keeping the single sockif and adding cases where needed to route to protocol specific logic. I added the protocol to the socket struct and defined a new address struct for HCI.

At this point it does not do anything useful.

[protobits]

My plan was to build something like btmon which is the modern replacement to hcidump that would leverage the HCI socket using the monitor channel. There is also btmgmt which fully exercises the HCI socket control channel, which fits in with btsak better, but it might be easier to just write something new here. For GATT I have always used bluetoothctl which I think has replaced gattool https://wiki.archlinux.org/index.php/bluetooth#Troubleshooting

I'm slowly stubbing things out in the stack trying to make the changes as minor as possible for now.

Sure, btmon will be really useful. I didn't know about btmgmt and I didn't try bluetootctl much as it appears less CLI friendly (more interactive) but it appears to support registering a GATT server, which gattool does not support. Eventually it will be useful to have all these applications similar to Linux, but I'm hoping to use something simple to start with.

I'm thinking I could use btsak as is for now, but start adapting bt_att.c which is the point of interface to l2cap functions so that it uses L2CAP sockets instead. Does this sound reasonable? I could use the simulator build with my BLE dongle and try to read characteristics from a GATT server on my smartphone.

[protobits]

Also, you're probably already aware of it, but on https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/src/shared/ there is a library + apps which directly use the socket interface. I will use it to learn how to operate the sockets. But it is also useful as an alternative explame of partitioning into applications (instead of single bluetoothctl): gatt-client, gatt-server, btmon, btmgmt, hciconfig, etc.

[btashton]

Also, you're probably already aware of it, but on https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/src/shared/ there is a library + apps which directly use the socket interface. I will use it to learn how to operate the sockets. But it is also useful as an alternative explame of partitioning into applications (instead of single bluetoothctl): gatt-client, gatt-server, btmon, btmgmt, hciconfig, etc.

Yep, just be aware that most of the applications that are hci* are considered for the most part deprecated, but still serve as examples of the socket interface, just that they may interfere with bluetoothd (you don't normally want two GATT servers running).

[btashton]

@v01d I am going to make the L2CAP socket addr also align better with the Linux implementation

struct sockaddr_l2
{
  sa_family_t  l2_family;       /* Must be AF_BLUETOOTH */
  uint16_t     l2_psm;          /* Protocol Service Multiplexer (PSM) */
  bt_addr_t    l2_bdaddr;       /* 6-byte Bluetooth address */
  uint16_t     l2_cid;          /* Channel identifier (CID) */
  uint8_t      l2_bdaddr_type;  /* Bluetooth address type */
};

vs

struct sockaddr_bt_s
{
  sa_family_t  bt_family;  /* Must be AF_BLUETOOTH */
  bt_addr_t    bt_bdaddr;  /* 6-byte Bluetooth address */
  uint8_t      bt_channel; /* Channel identifier (CID) */
};

[patacongo]

I am going to make the L2CAP socket addr also align better with the Linux implementation

Sounds fine to me. But I think we should bring these cosmetic kinds of things directory to master and rebase this PR so that we don't have to deal with the diffs when reviewing. There will be multiple review patches and things will become hard to review.

The renaming change is valid with or without the HCI socket support.

[btashton]

I am going to make the L2CAP socket addr also align better with the Linux implementation

Sounds fine to me. But I think we should bring these cosmetic kinds of things directory to master and rebase this PR so that we don't have to deal with the diffs when reviewing. There will be multiple review patches and things will become hard to review.

The renaming change is valid with or without the HCI socket support.

Yeah sounds good, I'll pull this out for the OS and Apps. That will also make it so its easier to keep CI happy.

[protobits]

I think that we can close this right? Or if you prefer to keep it to remind about the MGMT and monitor channel, the title could be changed.