| Message ID | 20200525184343.8532-1-alainm@chromium.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Marcel Holtmann |
| Headers | show |
| Series | [v2] sco:Add support for BT_PKT_STATUS CMSG data | expand |
Hi Alain, > This change adds support for reporting the BT_PKT_STATUS to the socket > CMSG data to allow the implementation of a packet loss correction on > erronous data received on the SCO socket. > > The patch was partially developed by Marcel Holtmann and validated by > Hsin-yu Chao > > Signed-off-by: Alain Michaud <alainm@chromium.org> > > --- > > include/net/bluetooth/bluetooth.h | 8 ++++++++ > include/net/bluetooth/sco.h | 3 +++ > net/bluetooth/af_bluetooth.c | 3 +++ > net/bluetooth/hci_core.c | 1 + > net/bluetooth/sco.c | 28 ++++++++++++++++++++++++++++ > 5 files changed, 43 insertions(+) > > diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h > index 3fa7b1e3c5d9..85e6c5754448 100644 > --- a/include/net/bluetooth/bluetooth.h > +++ b/include/net/bluetooth/bluetooth.h > @@ -147,6 +147,8 @@ struct bt_voice { > #define BT_MODE_LE_FLOWCTL 0x03 > #define BT_MODE_EXT_FLOWCTL 0x04 > > +#define BT_PKT_STATUS 16 > + we need to agree on if this is an int or u8 value. Otherwise this looks good. Regards Marcel
On Thu, May 28, 2020 at 4:26 AM Marcel Holtmann <marcel@holtmann.org> wrote: > > Hi Alain, > > > This change adds support for reporting the BT_PKT_STATUS to the socket > > CMSG data to allow the implementation of a packet loss correction on > > erronous data received on the SCO socket. > > > > The patch was partially developed by Marcel Holtmann and validated by > > Hsin-yu Chao > > > > Signed-off-by: Alain Michaud <alainm@chromium.org> > > > > --- > > > > include/net/bluetooth/bluetooth.h | 8 ++++++++ > > include/net/bluetooth/sco.h | 3 +++ > > net/bluetooth/af_bluetooth.c | 3 +++ > > net/bluetooth/hci_core.c | 1 + > > net/bluetooth/sco.c | 28 ++++++++++++++++++++++++++++ > > 5 files changed, 43 insertions(+) > > > > diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h > > index 3fa7b1e3c5d9..85e6c5754448 100644 > > --- a/include/net/bluetooth/bluetooth.h > > +++ b/include/net/bluetooth/bluetooth.h > > @@ -147,6 +147,8 @@ struct bt_voice { > > #define BT_MODE_LE_FLOWCTL 0x03 > > #define BT_MODE_EXT_FLOWCTL 0x04 > > > > +#define BT_PKT_STATUS 16 > > + > > we need to agree on if this is an int or u8 value. Otherwise this looks good. Since doing anything beyond a u8 value would require a core spec change and likely a new definition for what the packet flags are, I think it is likely safe to keep as u8. Do you have a different perspective? > > Regards > > Marcel >
diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h index 3fa7b1e3c5d9..85e6c5754448 100644 --- a/include/net/bluetooth/bluetooth.h +++ b/include/net/bluetooth/bluetooth.h @@ -147,6 +147,8 @@ struct bt_voice { #define BT_MODE_LE_FLOWCTL 0x03 #define BT_MODE_EXT_FLOWCTL 0x04 +#define BT_PKT_STATUS 16 + __printf(1, 2) void bt_info(const char *fmt, ...); __printf(1, 2) @@ -275,6 +277,7 @@ struct bt_sock { struct sock *parent; unsigned long flags; void (*skb_msg_name)(struct sk_buff *, void *, int *); + void (*skb_put_cmsg)(struct sk_buff *, struct msghdr *, struct sock *); }; enum { @@ -324,6 +327,10 @@ struct l2cap_ctrl { struct l2cap_chan *chan; }; +struct sco_ctrl { + u8 pkt_status; +}; + struct hci_dev; typedef void (*hci_req_complete_t)(struct hci_dev *hdev, u8 status, u16 opcode); @@ -350,6 +357,7 @@ struct bt_skb_cb { u8 incoming:1; union { struct l2cap_ctrl l2cap; + struct sco_ctrl sco; struct hci_ctrl hci; }; }; diff --git a/include/net/bluetooth/sco.h b/include/net/bluetooth/sco.h index f40ddb4264fc..7f0d7bdc53f6 100644 --- a/include/net/bluetooth/sco.h +++ b/include/net/bluetooth/sco.h @@ -46,4 +46,7 @@ struct sco_conninfo { __u8 dev_class[3]; }; +/* CMSG flags */ +#define SCO_CMSG_PKT_STATUS 0x0001 + #endif /* __SCO_H */ diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c index 3fd124927d4d..d0abea8d08cc 100644 --- a/net/bluetooth/af_bluetooth.c +++ b/net/bluetooth/af_bluetooth.c @@ -286,6 +286,9 @@ int bt_sock_recvmsg(struct socket *sock, struct msghdr *msg, size_t len, if (msg->msg_name && bt_sk(sk)->skb_msg_name) bt_sk(sk)->skb_msg_name(skb, msg->msg_name, &msg->msg_namelen); + + if (bt_sk(sk)->skb_put_cmsg) + bt_sk(sk)->skb_put_cmsg(skb, msg, sk); } skb_free_datagram(sk, skb); diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index 51d399273276..7b5e46198d99 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -4549,6 +4549,7 @@ static void hci_scodata_packet(struct hci_dev *hdev, struct sk_buff *skb) if (conn) { /* Send to upper protocol */ + bt_cb(skb)->sco.pkt_status = flags & 0x03; sco_recv_scodata(conn, skb); return; } else { diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c index c8c3d38cdc7b..3d7b973e5b7b 100644 --- a/net/bluetooth/sco.c +++ b/net/bluetooth/sco.c @@ -66,6 +66,7 @@ struct sco_pinfo { bdaddr_t dst; __u32 flags; __u16 setting; + __u32 cmsg_mask; struct sco_conn *conn; }; @@ -449,6 +450,19 @@ static void sco_sock_close(struct sock *sk) sco_sock_kill(sk); } +static void sco_skb_put_cmsg(struct sk_buff *skb, struct msghdr *msg, + struct sock *sk) +{ + __u32 mask = sco_pi(sk)->cmsg_mask; + + if (mask & SCO_CMSG_PKT_STATUS) { + int pkt_status = bt_cb(skb)->sco.pkt_status; + + put_cmsg(msg, SOL_BLUETOOTH, BT_PKT_STATUS, sizeof(pkt_status), + &pkt_status); + } +} + static void sco_sock_init(struct sock *sk, struct sock *parent) { BT_DBG("sk %p", sk); @@ -457,6 +471,8 @@ static void sco_sock_init(struct sock *sk, struct sock *parent) sk->sk_type = parent->sk_type; bt_sk(sk)->flags = bt_sk(parent)->flags; security_sk_clone(parent, sk); + } else { + bt_sk(sk)->skb_put_cmsg = sco_skb_put_cmsg; } } @@ -846,6 +862,18 @@ static int sco_sock_setsockopt(struct socket *sock, int level, int optname, sco_pi(sk)->setting = voice.setting; break; + case BT_PKT_STATUS: + if (get_user(opt, (int __user *)optval)) { + err = -EFAULT; + break; + } + + if (opt) + sco_pi(sk)->cmsg_mask |= SCO_CMSG_PKT_STATUS; + else + sco_pi(sk)->cmsg_mask &= ~SCO_CMSG_PKT_STATUS; + break; + default: err = -ENOPROTOOPT; break;
This change adds support for reporting the BT_PKT_STATUS to the socket CMSG data to allow the implementation of a packet loss correction on erronous data received on the SCO socket. The patch was partially developed by Marcel Holtmann and validated by Hsin-yu Chao Signed-off-by: Alain Michaud <alainm@chromium.org> --- include/net/bluetooth/bluetooth.h | 8 ++++++++ include/net/bluetooth/sco.h | 3 +++ net/bluetooth/af_bluetooth.c | 3 +++ net/bluetooth/hci_core.c | 1 + net/bluetooth/sco.c | 28 ++++++++++++++++++++++++++++ 5 files changed, 43 insertions(+)