[RFC 1/2] Bluetooth: make use of MSG_DONTROUTE flag in L2CAP sockets

[RFC 1/2] Bluetooth: make use of MSG_DONTROUTE flag in L2CAP sockets

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This use MSG_DONTROUTE to send a single message with highest possible
priority without having to rely on socket options.

This is useful for RFCOMM which relies on socket interface to send frames
to L2CAP, but since some of this frames are actual commands that can
timeout they should be sent as soon as possible.

For userspace this also offer the convenience of not having to use
setsockopt several times when only a single/some messages needs to be
prioritize.

Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
 net/bluetooth/l2cap_sock.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index c57027f..133c1f2 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -699,6 +699,7 @@ static int l2cap_sock_sendmsg(struct kiocb *iocb, struct socket *sock, struct ms
 	struct sock *sk = sock->sk;
 	struct l2cap_chan *chan = l2cap_pi(sk)->chan;
 	int err;
+	u32 priority;
 
 	BT_DBG("sock %p, sk %p", sock, sk);
 
@@ -711,12 +712,18 @@ static int l2cap_sock_sendmsg(struct kiocb *iocb, struct socket *sock, struct ms
 
 	lock_sock(sk);
 
+	priority = sk->sk_priority;
+
+	/* MSG_DONTROUTE send message at maximum possible priority */
+	if ((msg->msg_flags & MSG_DONTROUTE) && priority < HCI_PRIO_MAX - 1)
+		priority = HCI_PRIO_MAX - 1;
+
 	if (sk->sk_state != BT_CONNECTED) {
 		release_sock(sk);
 		return -ENOTCONN;
 	}
 
-	err = l2cap_chan_send(chan, msg, len, sk->sk_priority);
+	err = l2cap_chan_send(chan, msg, len, priority);
 
 	release_sock(sk);
 	return err;
-- 
1.7.7.5

[RFC 2/2] Bluetooth: Fix possible timeouts while sending RFCOMM commands

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

Make use of MSG_DONTROUTE when sending RFCOMM commands to avoid possible
timeouts during connection attempts.

Note that this do not revert commit
5436538fb5f2a12e5328dcaa2e3a1742be25c2e0, it will continue ignoring
priorities (priority 0) for data sent by userspace via RFCOMM sockets
only commands are affected.

Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
 net/bluetooth/rfcomm/core.c |   27 +++++++++++++++------------
 1 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index 09a3cbc..c4a7f8c 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -65,7 +65,8 @@ static DEFINE_MUTEX(rfcomm_mutex);
 
 static LIST_HEAD(session_list);
 
-static int rfcomm_send_frame(struct rfcomm_session *s, u8 *data, int len);
+static int rfcomm_send_frame(struct rfcomm_session *s, u8 *data, int len,
+				unsigned flags);
 static int rfcomm_send_sabm(struct rfcomm_session *s, u8 dlci);
 static int rfcomm_send_disc(struct rfcomm_session *s, u8 dlci);
 static int rfcomm_queue_disc(struct rfcomm_dlc *d);
@@ -747,7 +748,8 @@ void rfcomm_session_getaddr(struct rfcomm_session *s, bdaddr_t *src, bdaddr_t *d
 }
 
 /* ---- RFCOMM frame sending ---- */
-static int rfcomm_send_frame(struct rfcomm_session *s, u8 *data, int len)
+static int rfcomm_send_frame(struct rfcomm_session *s, u8 *data, int len,
+				unsigned flags)
 {
 	struct kvec iv = { data, len };
 	struct msghdr msg;
@@ -755,6 +757,7 @@ static int rfcomm_send_frame(struct rfcomm_session *s, u8 *data, int len)
 	BT_DBG("session %p len %d", s, len);
 
 	memset(&msg, 0, sizeof(msg));
+	msg.msg_flags = flags;
 
 	return kernel_sendmsg(s->sock, &msg, &iv, 1, len);
 }
@@ -763,7 +766,7 @@ static int rfcomm_send_cmd(struct rfcomm_session *s, struct rfcomm_cmd *cmd)
 {
 	BT_DBG("%p cmd %u", s, cmd->ctrl);
 
-	return rfcomm_send_frame(s, (void *) cmd, sizeof(*cmd));
+	return rfcomm_send_frame(s, (void *) cmd, sizeof(*cmd), MSG_DONTROUTE);
 }
 
 static int rfcomm_send_sabm(struct rfcomm_session *s, u8 dlci)
@@ -866,7 +869,7 @@ static int rfcomm_send_nsc(struct rfcomm_session *s, int cr, u8 type)
 
 	*ptr = __fcs(buf); ptr++;
 
-	return rfcomm_send_frame(s, buf, ptr - buf);
+	return rfcomm_send_frame(s, buf, ptr - buf, MSG_DONTROUTE);
 }
 
 static int rfcomm_send_pn(struct rfcomm_session *s, int cr, struct rfcomm_dlc *d)
@@ -908,7 +911,7 @@ static int rfcomm_send_pn(struct rfcomm_session *s, int cr, struct rfcomm_dlc *d
 
 	*ptr = __fcs(buf); ptr++;
 
-	return rfcomm_send_frame(s, buf, ptr - buf);
+	return rfcomm_send_frame(s, buf, ptr - buf, MSG_DONTROUTE);
 }
 
 int rfcomm_send_rpn(struct rfcomm_session *s, int cr, u8 dlci,
@@ -946,7 +949,7 @@ int rfcomm_send_rpn(struct rfcomm_session *s, int cr, u8 dlci,
 
 	*ptr = __fcs(buf); ptr++;
 
-	return rfcomm_send_frame(s, buf, ptr - buf);
+	return rfcomm_send_frame(s, buf, ptr - buf, MSG_DONTROUTE);
 }
 
 static int rfcomm_send_rls(struct rfcomm_session *s, int cr, u8 dlci, u8 status)
@@ -973,7 +976,7 @@ static int rfcomm_send_rls(struct rfcomm_session *s, int cr, u8 dlci, u8 status)
 
 	*ptr = __fcs(buf); ptr++;
 
-	return rfcomm_send_frame(s, buf, ptr - buf);
+	return rfcomm_send_frame(s, buf, ptr - buf, MSG_DONTROUTE);
 }
 
 static int rfcomm_send_msc(struct rfcomm_session *s, int cr, u8 dlci, u8 v24_sig)
@@ -1000,7 +1003,7 @@ static int rfcomm_send_msc(struct rfcomm_session *s, int cr, u8 dlci, u8 v24_sig
 
 	*ptr = __fcs(buf); ptr++;
 
-	return rfcomm_send_frame(s, buf, ptr - buf);
+	return rfcomm_send_frame(s, buf, ptr - buf, MSG_DONTROUTE);
 }
 
 static int rfcomm_send_fcoff(struct rfcomm_session *s, int cr)
@@ -1022,7 +1025,7 @@ static int rfcomm_send_fcoff(struct rfcomm_session *s, int cr)
 
 	*ptr = __fcs(buf); ptr++;
 
-	return rfcomm_send_frame(s, buf, ptr - buf);
+	return rfcomm_send_frame(s, buf, ptr - buf, MSG_DONTROUTE);
 }
 
 static int rfcomm_send_fcon(struct rfcomm_session *s, int cr)
@@ -1044,7 +1047,7 @@ static int rfcomm_send_fcon(struct rfcomm_session *s, int cr)
 
 	*ptr = __fcs(buf); ptr++;
 
-	return rfcomm_send_frame(s, buf, ptr - buf);
+	return rfcomm_send_frame(s, buf, ptr - buf, MSG_DONTROUTE);
 }
 
 static int rfcomm_send_test(struct rfcomm_session *s, int cr, u8 *pattern, int len)
@@ -1095,7 +1098,7 @@ static int rfcomm_send_credits(struct rfcomm_session *s, u8 addr, u8 credits)
 
 	*ptr = __fcs(buf); ptr++;
 
-	return rfcomm_send_frame(s, buf, ptr - buf);
+	return rfcomm_send_frame(s, buf, ptr - buf, MSG_DONTROUTE);
 }
 
 static void rfcomm_make_uih(struct sk_buff *skb, u8 addr)
@@ -1774,7 +1777,7 @@ static inline int rfcomm_process_tx(struct rfcomm_dlc *d)
 		return skb_queue_len(&d->tx_queue);
 
 	while (d->tx_credits && (skb = skb_dequeue(&d->tx_queue))) {
-		err = rfcomm_send_frame(d->session, skb->data, skb->len);
+		err = rfcomm_send_frame(d->session, skb->data, skb->len, 0);
 		if (err < 0) {
 			skb_queue_head(&d->tx_queue, skb);
 			break;
-- 
1.7.7.5

Re: [RFC 1/2] Bluetooth: make use of MSG_DONTROUTE flag in L2CAP sockets

[Marcel Holtmann]

Hi Luiz,

> This use MSG_DONTROUTE to send a single message with highest possible
> priority without having to rely on socket options.
> 
> This is useful for RFCOMM which relies on socket interface to send frames
> to L2CAP, but since some of this frames are actual commands that can
> timeout they should be sent as soon as possible.
> 
> For userspace this also offer the convenience of not having to use
> setsockopt several times when only a single/some messages needs to be
> prioritize.

are you sure that we are not mis-using the MSG_DONTROUTE flag?

> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> ---
>  net/bluetooth/l2cap_sock.c |    9 ++++++++-
>  1 files changed, 8 insertions(+), 1 deletions(-)
> 
> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> index c57027f..133c1f2 100644
> --- a/net/bluetooth/l2cap_sock.c
> +++ b/net/bluetooth/l2cap_sock.c
> @@ -699,6 +699,7 @@ static int l2cap_sock_sendmsg(struct kiocb *iocb, struct socket *sock, struct ms
>  	struct sock *sk = sock->sk;
>  	struct l2cap_chan *chan = l2cap_pi(sk)->chan;
>  	int err;
> +	u32 priority;
>  
>  	BT_DBG("sock %p, sk %p", sock, sk);
>  
> @@ -711,12 +712,18 @@ static int l2cap_sock_sendmsg(struct kiocb *iocb, struct socket *sock, struct ms
>  
>  	lock_sock(sk);
>  
> +	priority = sk->sk_priority;
> +
> +	/* MSG_DONTROUTE send message at maximum possible priority */
> +	if ((msg->msg_flags & MSG_DONTROUTE) && priority < HCI_PRIO_MAX - 1)
> +		priority = HCI_PRIO_MAX - 1;
> +

you need to enforce the same privilege check that SO_PRIORITY does.
Otherwise every single application can just go and overwrite it.

>  	if (sk->sk_state != BT_CONNECTED) {
>  		release_sock(sk);
>  		return -ENOTCONN;
>  	}
>  
> -	err = l2cap_chan_send(chan, msg, len, sk->sk_priority);
> +	err = l2cap_chan_send(chan, msg, len, priority);
>  
>  	release_sock(sk);
>  	return err;

Regards

Marcel

Re: [RFC 1/2] Bluetooth: make use of MSG_DONTROUTE flag in L2CAP sockets

[Luiz Augusto von Dentz]

Hi Marcel,

On Tue, Jan 10, 2012 at 6:50 PM, Marcel Holtmann <marcel@holtmann.org> wrot=
e:
> Hi Luiz,
>
>> This use MSG_DONTROUTE to send a single message with highest possible
>> priority without having to rely on socket options.
>>
>> This is useful for RFCOMM which relies on socket interface to send frame=
s
>> to L2CAP, but since some of this frames are actual commands that can
>> timeout they should be sent as soon as possible.
>>
>> For userspace this also offer the convenience of not having to use
>> setsockopt several times when only a single/some messages needs to be
>> prioritize.
>
> are you sure that we are not mis-using the MSG_DONTROUTE flag?

This is the reason Ive sent as RFC, man page says it is only define
for protocols that route, but note that MSG_TRYHARD does map to the
same value (4) so the meaning depend on the protocol. Perhaps creating
a new flag e.g. MSG_HIGHPRIO would be acceptable?

>> + =A0 =A0 priority =3D sk->sk_priority;
>> +
>> + =A0 =A0 /* MSG_DONTROUTE send message at maximum possible priority */
>> + =A0 =A0 if ((msg->msg_flags & MSG_DONTROUTE) && priority < HCI_PRIO_MA=
X - 1)
>> + =A0 =A0 =A0 =A0 =A0 =A0 priority =3D HCI_PRIO_MAX - 1;
>> +
>
> you need to enforce the same privilege check that SO_PRIORITY does.
> Otherwise every single application can just go and overwrite it.

Currently HCI_PRIO_MAX - 1 maps to the highest priority without
requiring privileges, but I can change that to HCI_PRIO_MAX so only
privileged processes such bluetoothd can make use of it.

--=20
Luiz Augusto von Dentz

Re: [RFC 1/2] Bluetooth: make use of MSG_DONTROUTE flag in L2CAP sockets

[Marcel Holtmann]

Hi Luiz,

> >> This use MSG_DONTROUTE to send a single message with highest possible
> >> priority without having to rely on socket options.
> >>
> >> This is useful for RFCOMM which relies on socket interface to send frames
> >> to L2CAP, but since some of this frames are actual commands that can
> >> timeout they should be sent as soon as possible.
> >>
> >> For userspace this also offer the convenience of not having to use
> >> setsockopt several times when only a single/some messages needs to be
> >> prioritize.
> >
> > are you sure that we are not mis-using the MSG_DONTROUTE flag?
> 
> This is the reason Ive sent as RFC, man page says it is only define
> for protocols that route, but note that MSG_TRYHARD does map to the
> same value (4) so the meaning depend on the protocol. Perhaps creating
> a new flag e.g. MSG_HIGHPRIO would be acceptable?

try to get this upstream via netdev mailing list first. And lets see
where this goes. I personally like the idea, but shoehorning it into
MSG_DONTROUTE seems not really a good idea.

> >> +     priority = sk->sk_priority;
> >> +
> >> +     /* MSG_DONTROUTE send message at maximum possible priority */
> >> +     if ((msg->msg_flags & MSG_DONTROUTE) && priority < HCI_PRIO_MAX - 1)
> >> +             priority = HCI_PRIO_MAX - 1;
> >> +
> >
> > you need to enforce the same privilege check that SO_PRIORITY does.
> > Otherwise every single application can just go and overwrite it.
> 
> Currently HCI_PRIO_MAX - 1 maps to the highest priority without
> requiring privileges, but I can change that to HCI_PRIO_MAX so only
> privileged processes such bluetoothd can make use of it.

If we wanna do then, we should give privileged daemons to use highest
priority as well.

Regards

Marcel