Re: [PATCH v5 3/5] Bluetooth: ISO: add TX timestamping

[Willem de Bruijn <willemdebruijn.kernel@gmail.com>]

Pauli Virtanen wrote:
> Add BT_SCM_ERROR socket CMSG type.
> 
> Support TX timestamping in ISO sockets.
> 
> Support MSG_ERRQUEUE in ISO recvmsg.
> 
> If a packet from sendmsg() is fragmented, only the first ACL fragment is
> timestamped.
> 
> Signed-off-by: Pauli Virtanen <pav@iki.fi>
> ---
> 
> Notes:
>     v5:
>     - use sockcm_init -> hci_sockcm_init
> 
>  include/net/bluetooth/bluetooth.h |  1 +
>  net/bluetooth/iso.c               | 24 ++++++++++++++++++++----
>  2 files changed, 21 insertions(+), 4 deletions(-)
> 
> diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> index 435250c72d56..bbefde319f95 100644
> --- a/include/net/bluetooth/bluetooth.h
> +++ b/include/net/bluetooth/bluetooth.h
> @@ -156,6 +156,7 @@ struct bt_voice {
>  #define BT_PKT_STATUS           16
>  
>  #define BT_SCM_PKT_STATUS	0x03
> +#define BT_SCM_ERROR		0x04
>  
>  #define BT_ISO_QOS		17
>  
> diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
> index 0cb52a3308ba..3501a991f1c6 100644
> --- a/net/bluetooth/iso.c
> +++ b/net/bluetooth/iso.c
> @@ -518,7 +518,8 @@ static struct bt_iso_qos *iso_sock_get_qos(struct sock *sk)
>  	return &iso_pi(sk)->qos;
>  }
>  
> -static int iso_send_frame(struct sock *sk, struct sk_buff *skb)
> +static int iso_send_frame(struct sock *sk, struct sk_buff *skb,
> +			  const struct sockcm_cookie *sockc)
>  {
>  	struct iso_conn *conn = iso_pi(sk)->conn;
>  	struct bt_iso_qos *qos = iso_sock_get_qos(sk);
> @@ -538,10 +539,12 @@ static int iso_send_frame(struct sock *sk, struct sk_buff *skb)
>  	hdr->slen = cpu_to_le16(hci_iso_data_len_pack(len,
>  						      HCI_ISO_STATUS_VALID));
>  
> -	if (sk->sk_state == BT_CONNECTED)
> +	if (sk->sk_state == BT_CONNECTED) {
> +		hci_setup_tx_timestamp(skb, 1, sockc);
>  		hci_send_iso(conn->hcon, skb);
> -	else
> +	} else {
>  		len = -ENOTCONN;
> +	}
>  
>  	return len;
>  }
> @@ -1348,6 +1351,7 @@ static int iso_sock_sendmsg(struct socket *sock, struct msghdr *msg,
>  {
>  	struct sock *sk = sock->sk;
>  	struct sk_buff *skb, **frag;
> +	struct sockcm_cookie sockc;
>  	size_t mtu;
>  	int err;
>  
> @@ -1360,6 +1364,14 @@ static int iso_sock_sendmsg(struct socket *sock, struct msghdr *msg,
>  	if (msg->msg_flags & MSG_OOB)
>  		return -EOPNOTSUPP;
>  
> +	hci_sockcm_init(&sockc, sk);
> +
> +	if (msg->msg_controllen) {
> +		err = sock_cmsg_send(sk, msg, &sockc);
> +		if (err)
> +			return err;
> +	}
> +

Minor: do you want to return an error if the process set unexpected
sockc fields?

If allowing to set them now, but ignoring them, it will be harder to
support them later. As then it will result in a kernel behavioral
change for the same sendmsg() call.

Though I suppose that is already the case if all cmsg are ignored
currently. So fine to keep if you don't care.