lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<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(&ethcmd, 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

Powered by Openwall GNU/*/Linux Powered by OpenVZ