[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <ceef0e19687d24fbb7847cbf95a2421414fb1fb5.camel@iki.fi>
Date: Tue, 22 Apr 2025 22:08:35 +0300
From: Pauli Virtanen <pav@....fi>
To: Willem de Bruijn <willemdebruijn.kernel@...il.com>,
linux-bluetooth@...r.kernel.org
Cc: luiz.dentz@...il.com, netdev@...r.kernel.org, kernelxing@...cent.com,
andrew@...n.ch
Subject: Re: [PATCH] Bluetooth: add support for SIOCETHTOOL
ETHTOOL_GET_TS_INFO
ti, 2025-04-22 kello 09:31 -0400, Willem de Bruijn kirjoitti:
> Pauli Virtanen wrote:
> > Bluetooth needs some way for user to get supported so_timestamping flags
> > for the different socket types.
> >
> > Use SIOCETHTOOL API for this purpose. As hci_dev is not associated with
> > struct net_device, the existing implementation can't be reused, so we
> > add a small one here.
> >
> > Add support (only) for ETHTOOL_GET_TS_INFO command. The API differs
> > slightly from netdev in that the result depends also on socket proto,
> > not just hardware.
> >
> > Signed-off-by: Pauli Virtanen <pav@....fi>
> > ---
> >
> > Notes:
> > Another option could be a new socket option, not sure what would be best
> > here. Using SIOCETHTOOL may not be that great since the 'ethtool' program
> > can't query these as the net_device doesn't actually exist.
> >
> > include/net/bluetooth/bluetooth.h | 4 ++
> > net/bluetooth/af_bluetooth.c | 87 +++++++++++++++++++++++++++++++
> > net/bluetooth/hci_conn.c | 40 ++++++++++++++
> > 3 files changed, 131 insertions(+)
> >
> > diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> > index bbefde319f95..114299bd8b98 100644
> > --- a/include/net/bluetooth/bluetooth.h
> > +++ b/include/net/bluetooth/bluetooth.h
> > @@ -29,6 +29,7 @@
> > #include <linux/poll.h>
> > #include <net/sock.h>
> > #include <linux/seq_file.h>
> > +#include <linux/ethtool.h>
> >
> > #define BT_SUBSYS_VERSION 2
> > #define BT_SUBSYS_REVISION 22
> > @@ -448,6 +449,9 @@ void hci_req_cmd_complete(struct hci_dev *hdev, u16 opcode, u8 status,
> > hci_req_complete_t *req_complete,
> > hci_req_complete_skb_t *req_complete_skb);
> >
> > +int hci_ethtool_ts_info(unsigned int index, int sk_proto,
> > + struct kernel_ethtool_ts_info *ts_info);
> > +
> > #define HCI_REQ_START BIT(0)
> > #define HCI_REQ_SKB BIT(1)
> >
> > diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
> > index 0b4d0a8bd361..6ad2f72f53f4 100644
> > --- a/net/bluetooth/af_bluetooth.c
> > +++ b/net/bluetooth/af_bluetooth.c
> > @@ -34,6 +34,9 @@
> > #include <net/bluetooth/bluetooth.h>
> > #include <linux/proc_fs.h>
> >
> > +#include <linux/ethtool.h>
> > +#include <linux/sockios.h>
> > +
> > #include "leds.h"
> > #include "selftest.h"
> >
> > @@ -563,6 +566,86 @@ __poll_t bt_sock_poll(struct file *file, struct socket *sock,
> > }
> > EXPORT_SYMBOL(bt_sock_poll);
> >
> > +static int bt_ethtool_get_ts_info(struct sock *sk, unsigned int index,
> > + void __user *useraddr)
> > +{
> > + struct ethtool_ts_info info;
> > + struct kernel_ethtool_ts_info ts_info = {};
> > + int ret;
> > +
> > + ret = hci_ethtool_ts_info(index, sk->sk_protocol, &ts_info);
> > + if (ret == -ENODEV)
> > + return ret;
> > + else if (ret < 0)
> > + return -EIO;
> > +
> > + memset(&info, 0, sizeof(info));
> > +
> > + info.cmd = ETHTOOL_GET_TS_INFO;
> > + info.so_timestamping = ts_info.so_timestamping;
> > + info.phc_index = ts_info.phc_index;
> > + info.tx_types = ts_info.tx_types;
> > + info.rx_filters = ts_info.rx_filters;
> > +
> > + if (copy_to_user(useraddr, &info, sizeof(info)))
> > + return -EFAULT;
> > +
> > + return 0;
> > +}
> > +
> > +static int bt_ethtool(struct sock *sk, const struct ifreq *ifr,
> > + void __user *useraddr)
> > +{
> > + unsigned int index;
> > + u32 ethcmd;
> > + int n;
> > +
> > + if (copy_from_user(ðcmd, useraddr, sizeof(ethcmd)))
> > + return -EFAULT;
> > +
> > + if (sscanf(ifr->ifr_name, "hci%u%n", &index, &n) != 1 ||
> > + n != strlen(ifr->ifr_name))
> > + return -ENODEV;
> > +
> > + switch (ethcmd) {
> > + case ETHTOOL_GET_TS_INFO:
> > + return bt_ethtool_get_ts_info(sk, index, useraddr);
> > + }
> > +
> > + return -EOPNOTSUPP;
> > +}
> > +
> > +static int bt_dev_ioctl(struct socket *sock, unsigned int cmd, void __user *arg)
> > +{
> > + struct sock *sk = sock->sk;
> > + struct ifreq ifr = {};
> > + void __user *data;
> > + char *colon;
> > + int ret = -ENOIOCTLCMD;
> > +
> > + if (get_user_ifreq(&ifr, &data, arg))
> > + return -EFAULT;
> > +
> > + ifr.ifr_name[IFNAMSIZ - 1] = 0;
> > + colon = strchr(ifr.ifr_name, ':');
> > + if (colon)
> > + *colon = 0;
> > +
> > + switch (cmd) {
> > + case SIOCETHTOOL:
> > + ret = bt_ethtool(sk, &ifr, data);
> > + break;
> > + }
> > +
> > + if (colon)
> > + *colon = ':';
> > +
> > + if (put_user_ifreq(&ifr, arg))
> > + return -EFAULT;
> > +
> > + return ret;
> > +}
> > +
> > int bt_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
> > {
> > struct sock *sk = sock->sk;
> > @@ -595,6 +678,10 @@ int bt_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
> > err = put_user(amount, (int __user *)arg);
> > break;
> >
> > + case SIOCETHTOOL:
> > + err = bt_dev_ioctl(sock, cmd, (void __user *)arg);
> > + break;
> > +
> > default:
> > err = -ENOIOCTLCMD;
> > break;
> > diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> > index 95972fd4c784..55f703076e25 100644
> > --- a/net/bluetooth/hci_conn.c
> > +++ b/net/bluetooth/hci_conn.c
> > @@ -3186,3 +3186,43 @@ void hci_conn_tx_dequeue(struct hci_conn *conn)
> >
> > kfree_skb(skb);
> > }
> > +
> > +int hci_ethtool_ts_info(unsigned int index, int sk_proto,
> > + struct kernel_ethtool_ts_info *info)
> > +{
> > + struct hci_dev *hdev;
> > +
> > + hdev = hci_dev_get(index);
> > + if (!hdev)
> > + return -ENODEV;
> > +
> > + info->so_timestamping =
> > + SOF_TIMESTAMPING_TX_SOFTWARE |
> > + SOF_TIMESTAMPING_SOFTWARE |
> > + SOF_TIMESTAMPING_OPT_ID |
> > + SOF_TIMESTAMPING_OPT_CMSG |
> > + SOF_TIMESTAMPING_OPT_TSONLY;
>
> Options are universally supported, do not have to be advertised
> per device.
Ok.
> > + info->phc_index = -1;
> > + info->tx_types = BIT(HWTSTAMP_TX_OFF);
> > + info->rx_filters = BIT(HWTSTAMP_FILTER_NONE);
> > +
> > + switch (sk_proto) {
> > + case BTPROTO_ISO:
> > + info->so_timestamping |= SOF_TIMESTAMPING_RX_SOFTWARE;
> > + info->so_timestamping |= SOF_TIMESTAMPING_TX_COMPLETION;
> > + break;
> > + case BTPROTO_L2CAP:
> > + info->so_timestamping |= SOF_TIMESTAMPING_TX_COMPLETION;
>
> For netdev, SOF_TIMESTAMPING_RX_SOFTWARE is universally supported.
> Because implemented not in the drivers, but in
> __netif_receive_skb_core. Does the same not hold for BT?
There is a timestamp assigned to all skbs when core gets them from
driver, so it should work in the same way.
I'll need to look a bit more into what is going on with L2CAP, I have
one use case where RX timestamps for L2CAP are missing, but I see now
in other tests it works as expected.
>
> > + break;
> > + case BTPROTO_SCO:
> > + info->so_timestamping |= SOF_TIMESTAMPING_RX_SOFTWARE;
> > + if (hci_dev_test_flag(hdev, HCI_SCO_FLOWCTL))
> > + info->so_timestamping |= SOF_TIMESTAMPING_TX_COMPLETION;
> > + break;
> > + default:
> > + info->so_timestamping = 0;
> > + }
> > +
> > + hci_dev_put(hdev);
> > + return 0;
> > +}
> > --
> > 2.49.0
> >
>
--
Pauli Virtanen
Powered by blists - more mailing lists