All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] Revert "Bluetooth: Fix passing NULL to PTR_ERR"
@ 2022-02-08 22:19 Paul Menzel
  2022-02-08 22:19 ` [PATCH 2/2] Revert "Bluetooth: RFCOMM: Replace use of memcpy_from_msg with bt_skb_sendmmsg" Paul Menzel
  2022-02-08 23:02 ` [1/2] Revert "Bluetooth: Fix passing NULL to PTR_ERR" bluez.test.bot
  0 siblings, 2 replies; 5+ messages in thread
From: Paul Menzel @ 2022-02-08 22:19 UTC (permalink / raw)
  To: Marcel Holtmann, Johan Hedberg, Luiz Augusto von Dentz,
	David S. Miller, Jakub Kicinski
  Cc: Paul Menzel, linux-bluetooth, netdev, linux-kernel

This reverts commit 266191aa8d14b84958aaeb5e96ee4e97839e3d87, so that
commit 81be03e026dc ("Bluetooth: RFCOMM: Replace use of memcpy_from_msg
with bt_skb_sendmmsg"), introducing a regression, can be reverted.

Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
---
 include/net/bluetooth/bluetooth.h | 2 +-
 net/bluetooth/rfcomm/sock.c       | 2 +-
 net/bluetooth/sco.c               | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index 4b3d0b16c185..318df161e6d3 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -505,7 +505,7 @@ static inline struct sk_buff *bt_skb_sendmmsg(struct sock *sk,
 		struct sk_buff *tmp;
 
 		tmp = bt_skb_sendmsg(sk, msg, len, mtu, headroom, tailroom);
-		if (IS_ERR(tmp)) {
+		if (IS_ERR_OR_NULL(tmp)) {
 			kfree_skb(skb);
 			return tmp;
 		}
diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index 4bf4ea6cbb5e..5938af3e9936 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -583,7 +583,7 @@ static int rfcomm_sock_sendmsg(struct socket *sock, struct msghdr *msg,
 
 	skb = bt_skb_sendmmsg(sk, msg, len, d->mtu, RFCOMM_SKB_HEAD_RESERVE,
 			      RFCOMM_SKB_TAIL_RESERVE);
-	if (IS_ERR(skb))
+	if (IS_ERR_OR_NULL(skb))
 		return PTR_ERR(skb);
 
 	sent = rfcomm_dlc_send(d, skb);
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index 8eabf41b2993..7ebc9d294f0c 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -734,7 +734,7 @@ static int sco_sock_sendmsg(struct socket *sock, struct msghdr *msg,
 		return -EOPNOTSUPP;
 
 	skb = bt_skb_sendmsg(sk, msg, len, len, 0, 0);
-	if (IS_ERR(skb))
+	if (IS_ERR_OR_NULL(skb))
 		return PTR_ERR(skb);
 
 	lock_sock(sk);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] Revert "Bluetooth: RFCOMM: Replace use of memcpy_from_msg with bt_skb_sendmmsg"
  2022-02-08 22:19 [PATCH 1/2] Revert "Bluetooth: Fix passing NULL to PTR_ERR" Paul Menzel
@ 2022-02-08 22:19 ` Paul Menzel
  2022-02-08 23:39   ` Luiz Augusto von Dentz
  2022-02-09  1:06   ` Luiz Augusto von Dentz
  2022-02-08 23:02 ` [1/2] Revert "Bluetooth: Fix passing NULL to PTR_ERR" bluez.test.bot
  1 sibling, 2 replies; 5+ messages in thread
From: Paul Menzel @ 2022-02-08 22:19 UTC (permalink / raw)
  To: Marcel Holtmann, Johan Hedberg, Luiz Augusto von Dentz,
	David S. Miller, Jakub Kicinski
  Cc: Paul Menzel, linux-bluetooth, netdev, linux-kernel

This reverts commit 81be03e026dc0c16dc1c64e088b2a53b73caa895.

Since the commit, transferring files greater than some bytes to the
Nokia N9 (MeeGo) or Jolla (Sailfish OS) is not possible anymore.

    # obexctl
    [NEW] Client /org/bluez/obex
    [obex]# connect 40:98:4E:5B:CE:XX
    Attempting to connect to 40:98:4E:5B:CE:XX
    [NEW] Session /org/bluez/obex/client/session0 [default]
    [NEW] ObjectPush /org/bluez/obex/client/session0
    Connection successful
    [40:98:4E:5B:CE:XX]# send /lib/systemd/systemd
    Attempting to send /lib/systemd/systemd to /org/bluez/obex/client/session0
    [NEW] Transfer /org/bluez/obex/client/session0/transfer0
    Transfer /org/bluez/obex/client/session0/transfer0
        Status: queued
        Name: systemd
        Size: 1841712
        Filename: /lib/systemd/systemd
        Session: /org/bluez/obex/client/session0
    [CHG] Transfer /org/bluez/obex/client/session0/transfer0 Status: active
    [CHG] Transfer /org/bluez/obex/client/session0/transfer0 Transferred: 32737 (@32KB/s 00:55)
    [CHG] Transfer /org/bluez/obex/client/session0/transfer0 Status: error
    [DEL] Transfer /org/bluez/obex/client/session0/transfer0

Reverting it, fixes the regression.

Link: https://lore.kernel.org/linux-bluetooth/aa3ee7ac-6c52-3861-1798-3cc1a37f6ebf@molgen.mpg.de/T/#m1f9673e4ab0d55a7dccf87905337ab2e67d689f1
Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
---
 net/bluetooth/rfcomm/core.c | 50 ++++++-------------------------------
 net/bluetooth/rfcomm/sock.c | 46 ++++++++++++++++++++++++++--------
 2 files changed, 43 insertions(+), 53 deletions(-)

diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index 7324764384b6..f2bacb464ccf 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -549,58 +549,22 @@ struct rfcomm_dlc *rfcomm_dlc_exists(bdaddr_t *src, bdaddr_t *dst, u8 channel)
 	return dlc;
 }
 
-static int rfcomm_dlc_send_frag(struct rfcomm_dlc *d, struct sk_buff *frag)
-{
-	int len = frag->len;
-
-	BT_DBG("dlc %p mtu %d len %d", d, d->mtu, len);
-
-	if (len > d->mtu)
-		return -EINVAL;
-
-	rfcomm_make_uih(frag, d->addr);
-	__skb_queue_tail(&d->tx_queue, frag);
-
-	return len;
-}
-
 int rfcomm_dlc_send(struct rfcomm_dlc *d, struct sk_buff *skb)
 {
-	unsigned long flags;
-	struct sk_buff *frag, *next;
-	int len;
+	int len = skb->len;
 
 	if (d->state != BT_CONNECTED)
 		return -ENOTCONN;
 
-	frag = skb_shinfo(skb)->frag_list;
-	skb_shinfo(skb)->frag_list = NULL;
-
-	/* Queue all fragments atomically. */
-	spin_lock_irqsave(&d->tx_queue.lock, flags);
-
-	len = rfcomm_dlc_send_frag(d, skb);
-	if (len < 0 || !frag)
-		goto unlock;
-
-	for (; frag; frag = next) {
-		int ret;
-
-		next = frag->next;
-
-		ret = rfcomm_dlc_send_frag(d, frag);
-		if (ret < 0) {
-			kfree_skb(frag);
-			goto unlock;
-		}
+	BT_DBG("dlc %p mtu %d len %d", d, d->mtu, len);
 
-		len += ret;
-	}
+	if (len > d->mtu)
+		return -EINVAL;
 
-unlock:
-	spin_unlock_irqrestore(&d->tx_queue.lock, flags);
+	rfcomm_make_uih(skb, d->addr);
+	skb_queue_tail(&d->tx_queue, skb);
 
-	if (len > 0 && !test_bit(RFCOMM_TX_THROTTLED, &d->flags))
+	if (!test_bit(RFCOMM_TX_THROTTLED, &d->flags))
 		rfcomm_schedule();
 	return len;
 }
diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index 5938af3e9936..2c95bb58f901 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -575,20 +575,46 @@ static int rfcomm_sock_sendmsg(struct socket *sock, struct msghdr *msg,
 	lock_sock(sk);
 
 	sent = bt_sock_wait_ready(sk, msg->msg_flags);
+	if (sent)
+		goto done;
 
-	release_sock(sk);
+	while (len) {
+		size_t size = min_t(size_t, len, d->mtu);
+		int err;
 
-	if (sent)
-		return sent;
+		skb = sock_alloc_send_skb(sk, size + RFCOMM_SKB_RESERVE,
+				msg->msg_flags & MSG_DONTWAIT, &err);
+		if (!skb) {
+			if (sent == 0)
+				sent = err;
+			break;
+		}
+		skb_reserve(skb, RFCOMM_SKB_HEAD_RESERVE);
+
+		err = memcpy_from_msg(skb_put(skb, size), msg, size);
+		if (err) {
+			kfree_skb(skb);
+			if (sent == 0)
+				sent = err;
+			break;
+		}
+
+		skb->priority = sk->sk_priority;
+
+		err = rfcomm_dlc_send(d, skb);
+		if (err < 0) {
+			kfree_skb(skb);
+			if (sent == 0)
+				sent = err;
+			break;
+		}
 
-	skb = bt_skb_sendmmsg(sk, msg, len, d->mtu, RFCOMM_SKB_HEAD_RESERVE,
-			      RFCOMM_SKB_TAIL_RESERVE);
-	if (IS_ERR_OR_NULL(skb))
-		return PTR_ERR(skb);
+		sent += size;
+		len  -= size;
+	}
 
-	sent = rfcomm_dlc_send(d, skb);
-	if (sent < 0)
-		kfree_skb(skb);
+done:
+	release_sock(sk);
 
 	return sent;
 }
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* RE: [1/2] Revert "Bluetooth: Fix passing NULL to PTR_ERR"
  2022-02-08 22:19 [PATCH 1/2] Revert "Bluetooth: Fix passing NULL to PTR_ERR" Paul Menzel
  2022-02-08 22:19 ` [PATCH 2/2] Revert "Bluetooth: RFCOMM: Replace use of memcpy_from_msg with bt_skb_sendmmsg" Paul Menzel
@ 2022-02-08 23:02 ` bluez.test.bot
  1 sibling, 0 replies; 5+ messages in thread
From: bluez.test.bot @ 2022-02-08 23:02 UTC (permalink / raw)
  To: linux-bluetooth, pmenzel

[-- Attachment #1: Type: text/plain, Size: 2814 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=612351

---Test result---

Test Summary:
CheckPatch                    FAIL      2.11 seconds
GitLint                       FAIL      0.74 seconds
SubjectPrefix                 PASS      0.51 seconds
BuildKernel                   PASS      37.46 seconds
BuildKernel32                 PASS      33.75 seconds
Incremental Build with patchesPASS      51.88 seconds
TestRunner: Setup             PASS      597.35 seconds
TestRunner: l2cap-tester      PASS      15.94 seconds
TestRunner: bnep-tester       PASS      7.22 seconds
TestRunner: mgmt-tester       PASS      128.61 seconds
TestRunner: rfcomm-tester     PASS      9.02 seconds
TestRunner: sco-tester        PASS      9.47 seconds
TestRunner: smp-tester        PASS      9.21 seconds
TestRunner: userchan-tester   PASS      7.54 seconds

Details
##############################
Test: CheckPatch - FAIL - 2.11 seconds
Run checkpatch.pl script with rule in .checkpatch.conf
[2/2] Revert "Bluetooth: RFCOMM: Replace use of memcpy_from_msg with bt_skb_sendmmsg"\WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#69: 
    Attempting to send /lib/systemd/systemd to /org/bluez/obex/client/session0

CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#180: FILE: net/bluetooth/rfcomm/sock.c:586:
+		skb = sock_alloc_send_skb(sk, size + RFCOMM_SKB_RESERVE,
+				msg->msg_flags & MSG_DONTWAIT, &err);

total: 0 errors, 1 warnings, 1 checks, 121 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

/github/workspace/src/12739414.patch has style problems, please review.

NOTE: Ignored message types: UNKNOWN_COMMIT_ID

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.


##############################
Test: GitLint - FAIL - 0.74 seconds
Run gitlint with rule in .gitlint
[2/2] Revert "Bluetooth: RFCOMM: Replace use of memcpy_from_msg with bt_skb_sendmmsg"
1: T1 Title exceeds max length (85>80): "[2/2] Revert "Bluetooth: RFCOMM: Replace use of memcpy_from_msg with bt_skb_sendmmsg""
25: B1 Line exceeds max length (95>80): "    [CHG] Transfer /org/bluez/obex/client/session0/transfer0 Transferred: 32737 (@32KB/s 00:55)"
31: B1 Line exceeds max length (141>80): "Link: https://lore.kernel.org/linux-bluetooth/aa3ee7ac-6c52-3861-1798-3cc1a37f6ebf@molgen.mpg.de/T/#m1f9673e4ab0d55a7dccf87905337ab2e67d689f1"




---
Regards,
Linux Bluetooth


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] Revert "Bluetooth: RFCOMM: Replace use of memcpy_from_msg with bt_skb_sendmmsg"
  2022-02-08 22:19 ` [PATCH 2/2] Revert "Bluetooth: RFCOMM: Replace use of memcpy_from_msg with bt_skb_sendmmsg" Paul Menzel
@ 2022-02-08 23:39   ` Luiz Augusto von Dentz
  2022-02-09  1:06   ` Luiz Augusto von Dentz
  1 sibling, 0 replies; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2022-02-08 23:39 UTC (permalink / raw)
  To: Paul Menzel
  Cc: Marcel Holtmann, Johan Hedberg, David S. Miller, Jakub Kicinski,
	linux-bluetooth, open list:NETWORKING [GENERAL],
	Linux Kernel Mailing List

Hi Paul,

On Tue, Feb 8, 2022 at 2:20 PM Paul Menzel <pmenzel@molgen.mpg.de> wrote:
>
> This reverts commit 81be03e026dc0c16dc1c64e088b2a53b73caa895.
>
> Since the commit, transferring files greater than some bytes to the
> Nokia N9 (MeeGo) or Jolla (Sailfish OS) is not possible anymore.
>
>     # obexctl
>     [NEW] Client /org/bluez/obex
>     [obex]# connect 40:98:4E:5B:CE:XX
>     Attempting to connect to 40:98:4E:5B:CE:XX
>     [NEW] Session /org/bluez/obex/client/session0 [default]
>     [NEW] ObjectPush /org/bluez/obex/client/session0
>     Connection successful
>     [40:98:4E:5B:CE:XX]# send /lib/systemd/systemd
>     Attempting to send /lib/systemd/systemd to /org/bluez/obex/client/session0
>     [NEW] Transfer /org/bluez/obex/client/session0/transfer0
>     Transfer /org/bluez/obex/client/session0/transfer0
>         Status: queued
>         Name: systemd
>         Size: 1841712
>         Filename: /lib/systemd/systemd
>         Session: /org/bluez/obex/client/session0
>     [CHG] Transfer /org/bluez/obex/client/session0/transfer0 Status: active
>     [CHG] Transfer /org/bluez/obex/client/session0/transfer0 Transferred: 32737 (@32KB/s 00:55)
>     [CHG] Transfer /org/bluez/obex/client/session0/transfer0 Status: error
>     [DEL] Transfer /org/bluez/obex/client/session0/transfer0
>
> Reverting it, fixes the regression.
>
> Link: https://lore.kernel.org/linux-bluetooth/aa3ee7ac-6c52-3861-1798-3cc1a37f6ebf@molgen.mpg.de/T/#m1f9673e4ab0d55a7dccf87905337ab2e67d689f1
> Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>

We would be much better off with the explanation on why it is causing
a regression on these, is there an error? On top of that we can avoid
such regressions by introducing a test to rfcomm-tester to transfer
big PDUs.

> ---
>  net/bluetooth/rfcomm/core.c | 50 ++++++-------------------------------
>  net/bluetooth/rfcomm/sock.c | 46 ++++++++++++++++++++++++++--------
>  2 files changed, 43 insertions(+), 53 deletions(-)
>
> diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
> index 7324764384b6..f2bacb464ccf 100644
> --- a/net/bluetooth/rfcomm/core.c
> +++ b/net/bluetooth/rfcomm/core.c
> @@ -549,58 +549,22 @@ struct rfcomm_dlc *rfcomm_dlc_exists(bdaddr_t *src, bdaddr_t *dst, u8 channel)
>         return dlc;
>  }
>
> -static int rfcomm_dlc_send_frag(struct rfcomm_dlc *d, struct sk_buff *frag)
> -{
> -       int len = frag->len;
> -
> -       BT_DBG("dlc %p mtu %d len %d", d, d->mtu, len);
> -
> -       if (len > d->mtu)
> -               return -EINVAL;
> -
> -       rfcomm_make_uih(frag, d->addr);
> -       __skb_queue_tail(&d->tx_queue, frag);
> -
> -       return len;
> -}
> -
>  int rfcomm_dlc_send(struct rfcomm_dlc *d, struct sk_buff *skb)
>  {
> -       unsigned long flags;
> -       struct sk_buff *frag, *next;
> -       int len;
> +       int len = skb->len;
>
>         if (d->state != BT_CONNECTED)
>                 return -ENOTCONN;
>
> -       frag = skb_shinfo(skb)->frag_list;
> -       skb_shinfo(skb)->frag_list = NULL;
> -
> -       /* Queue all fragments atomically. */
> -       spin_lock_irqsave(&d->tx_queue.lock, flags);
> -
> -       len = rfcomm_dlc_send_frag(d, skb);
> -       if (len < 0 || !frag)
> -               goto unlock;
> -
> -       for (; frag; frag = next) {
> -               int ret;
> -
> -               next = frag->next;
> -
> -               ret = rfcomm_dlc_send_frag(d, frag);
> -               if (ret < 0) {
> -                       kfree_skb(frag);
> -                       goto unlock;
> -               }
> +       BT_DBG("dlc %p mtu %d len %d", d, d->mtu, len);
>
> -               len += ret;
> -       }
> +       if (len > d->mtu)
> +               return -EINVAL;
>
> -unlock:
> -       spin_unlock_irqrestore(&d->tx_queue.lock, flags);
> +       rfcomm_make_uih(skb, d->addr);
> +       skb_queue_tail(&d->tx_queue, skb);
>
> -       if (len > 0 && !test_bit(RFCOMM_TX_THROTTLED, &d->flags))
> +       if (!test_bit(RFCOMM_TX_THROTTLED, &d->flags))
>                 rfcomm_schedule();
>         return len;
>  }
> diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
> index 5938af3e9936..2c95bb58f901 100644
> --- a/net/bluetooth/rfcomm/sock.c
> +++ b/net/bluetooth/rfcomm/sock.c
> @@ -575,20 +575,46 @@ static int rfcomm_sock_sendmsg(struct socket *sock, struct msghdr *msg,
>         lock_sock(sk);
>
>         sent = bt_sock_wait_ready(sk, msg->msg_flags);
> +       if (sent)
> +               goto done;
>
> -       release_sock(sk);
> +       while (len) {
> +               size_t size = min_t(size_t, len, d->mtu);
> +               int err;
>
> -       if (sent)
> -               return sent;
> +               skb = sock_alloc_send_skb(sk, size + RFCOMM_SKB_RESERVE,
> +                               msg->msg_flags & MSG_DONTWAIT, &err);
> +               if (!skb) {
> +                       if (sent == 0)
> +                               sent = err;
> +                       break;
> +               }
> +               skb_reserve(skb, RFCOMM_SKB_HEAD_RESERVE);
> +
> +               err = memcpy_from_msg(skb_put(skb, size), msg, size);
> +               if (err) {
> +                       kfree_skb(skb);
> +                       if (sent == 0)
> +                               sent = err;
> +                       break;
> +               }
> +
> +               skb->priority = sk->sk_priority;
> +
> +               err = rfcomm_dlc_send(d, skb);
> +               if (err < 0) {
> +                       kfree_skb(skb);
> +                       if (sent == 0)
> +                               sent = err;
> +                       break;
> +               }
>
> -       skb = bt_skb_sendmmsg(sk, msg, len, d->mtu, RFCOMM_SKB_HEAD_RESERVE,
> -                             RFCOMM_SKB_TAIL_RESERVE);
> -       if (IS_ERR_OR_NULL(skb))
> -               return PTR_ERR(skb);
> +               sent += size;
> +               len  -= size;
> +       }
>
> -       sent = rfcomm_dlc_send(d, skb);
> -       if (sent < 0)
> -               kfree_skb(skb);
> +done:
> +       release_sock(sk);
>
>         return sent;
>  }
> --
> 2.34.1
>


-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] Revert "Bluetooth: RFCOMM: Replace use of memcpy_from_msg with bt_skb_sendmmsg"
  2022-02-08 22:19 ` [PATCH 2/2] Revert "Bluetooth: RFCOMM: Replace use of memcpy_from_msg with bt_skb_sendmmsg" Paul Menzel
  2022-02-08 23:39   ` Luiz Augusto von Dentz
@ 2022-02-09  1:06   ` Luiz Augusto von Dentz
  1 sibling, 0 replies; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2022-02-09  1:06 UTC (permalink / raw)
  To: Paul Menzel
  Cc: Marcel Holtmann, Johan Hedberg, David S. Miller, Jakub Kicinski,
	linux-bluetooth, open list:NETWORKING [GENERAL],
	Linux Kernel Mailing List

Hi Paul,

On Tue, Feb 8, 2022 at 2:20 PM Paul Menzel <pmenzel@molgen.mpg.de> wrote:
>
> This reverts commit 81be03e026dc0c16dc1c64e088b2a53b73caa895.
>
> Since the commit, transferring files greater than some bytes to the
> Nokia N9 (MeeGo) or Jolla (Sailfish OS) is not possible anymore.
>
>     # obexctl
>     [NEW] Client /org/bluez/obex
>     [obex]# connect 40:98:4E:5B:CE:XX
>     Attempting to connect to 40:98:4E:5B:CE:XX
>     [NEW] Session /org/bluez/obex/client/session0 [default]
>     [NEW] ObjectPush /org/bluez/obex/client/session0
>     Connection successful
>     [40:98:4E:5B:CE:XX]# send /lib/systemd/systemd
>     Attempting to send /lib/systemd/systemd to /org/bluez/obex/client/session0
>     [NEW] Transfer /org/bluez/obex/client/session0/transfer0
>     Transfer /org/bluez/obex/client/session0/transfer0
>         Status: queued
>         Name: systemd
>         Size: 1841712
>         Filename: /lib/systemd/systemd
>         Session: /org/bluez/obex/client/session0
>     [CHG] Transfer /org/bluez/obex/client/session0/transfer0 Status: active
>     [CHG] Transfer /org/bluez/obex/client/session0/transfer0 Transferred: 32737 (@32KB/s 00:55)
>     [CHG] Transfer /org/bluez/obex/client/session0/transfer0 Status: error
>     [DEL] Transfer /org/bluez/obex/client/session0/transfer0

Would you please create a github issue
(https://github.com/bluez/bluez/issues/) and attach the btmon trace so
we can check what is the error, you might as well attach the obexd
logs.

> Reverting it, fixes the regression.
>
> Link: https://lore.kernel.org/linux-bluetooth/aa3ee7ac-6c52-3861-1798-3cc1a37f6ebf@molgen.mpg.de/T/#m1f9673e4ab0d55a7dccf87905337ab2e67d689f1
> Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
> ---
>  net/bluetooth/rfcomm/core.c | 50 ++++++-------------------------------
>  net/bluetooth/rfcomm/sock.c | 46 ++++++++++++++++++++++++++--------
>  2 files changed, 43 insertions(+), 53 deletions(-)
>
> diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
> index 7324764384b6..f2bacb464ccf 100644
> --- a/net/bluetooth/rfcomm/core.c
> +++ b/net/bluetooth/rfcomm/core.c
> @@ -549,58 +549,22 @@ struct rfcomm_dlc *rfcomm_dlc_exists(bdaddr_t *src, bdaddr_t *dst, u8 channel)
>         return dlc;
>  }
>
> -static int rfcomm_dlc_send_frag(struct rfcomm_dlc *d, struct sk_buff *frag)
> -{
> -       int len = frag->len;
> -
> -       BT_DBG("dlc %p mtu %d len %d", d, d->mtu, len);
> -
> -       if (len > d->mtu)
> -               return -EINVAL;
> -
> -       rfcomm_make_uih(frag, d->addr);
> -       __skb_queue_tail(&d->tx_queue, frag);
> -
> -       return len;
> -}
> -
>  int rfcomm_dlc_send(struct rfcomm_dlc *d, struct sk_buff *skb)
>  {
> -       unsigned long flags;
> -       struct sk_buff *frag, *next;
> -       int len;
> +       int len = skb->len;
>
>         if (d->state != BT_CONNECTED)
>                 return -ENOTCONN;
>
> -       frag = skb_shinfo(skb)->frag_list;
> -       skb_shinfo(skb)->frag_list = NULL;
> -
> -       /* Queue all fragments atomically. */
> -       spin_lock_irqsave(&d->tx_queue.lock, flags);
> -
> -       len = rfcomm_dlc_send_frag(d, skb);
> -       if (len < 0 || !frag)
> -               goto unlock;
> -
> -       for (; frag; frag = next) {
> -               int ret;
> -
> -               next = frag->next;
> -
> -               ret = rfcomm_dlc_send_frag(d, frag);
> -               if (ret < 0) {
> -                       kfree_skb(frag);
> -                       goto unlock;
> -               }
> +       BT_DBG("dlc %p mtu %d len %d", d, d->mtu, len);
>
> -               len += ret;
> -       }
> +       if (len > d->mtu)
> +               return -EINVAL;
>
> -unlock:
> -       spin_unlock_irqrestore(&d->tx_queue.lock, flags);
> +       rfcomm_make_uih(skb, d->addr);
> +       skb_queue_tail(&d->tx_queue, skb);
>
> -       if (len > 0 && !test_bit(RFCOMM_TX_THROTTLED, &d->flags))
> +       if (!test_bit(RFCOMM_TX_THROTTLED, &d->flags))
>                 rfcomm_schedule();
>         return len;
>  }
> diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
> index 5938af3e9936..2c95bb58f901 100644
> --- a/net/bluetooth/rfcomm/sock.c
> +++ b/net/bluetooth/rfcomm/sock.c
> @@ -575,20 +575,46 @@ static int rfcomm_sock_sendmsg(struct socket *sock, struct msghdr *msg,
>         lock_sock(sk);
>
>         sent = bt_sock_wait_ready(sk, msg->msg_flags);
> +       if (sent)
> +               goto done;
>
> -       release_sock(sk);
> +       while (len) {
> +               size_t size = min_t(size_t, len, d->mtu);
> +               int err;
>
> -       if (sent)
> -               return sent;
> +               skb = sock_alloc_send_skb(sk, size + RFCOMM_SKB_RESERVE,
> +                               msg->msg_flags & MSG_DONTWAIT, &err);
> +               if (!skb) {
> +                       if (sent == 0)
> +                               sent = err;
> +                       break;
> +               }
> +               skb_reserve(skb, RFCOMM_SKB_HEAD_RESERVE);
> +
> +               err = memcpy_from_msg(skb_put(skb, size), msg, size);
> +               if (err) {
> +                       kfree_skb(skb);
> +                       if (sent == 0)
> +                               sent = err;
> +                       break;
> +               }
> +
> +               skb->priority = sk->sk_priority;
> +
> +               err = rfcomm_dlc_send(d, skb);
> +               if (err < 0) {
> +                       kfree_skb(skb);
> +                       if (sent == 0)
> +                               sent = err;
> +                       break;
> +               }
>
> -       skb = bt_skb_sendmmsg(sk, msg, len, d->mtu, RFCOMM_SKB_HEAD_RESERVE,
> -                             RFCOMM_SKB_TAIL_RESERVE);
> -       if (IS_ERR_OR_NULL(skb))
> -               return PTR_ERR(skb);
> +               sent += size;
> +               len  -= size;
> +       }
>
> -       sent = rfcomm_dlc_send(d, skb);
> -       if (sent < 0)
> -               kfree_skb(skb);
> +done:
> +       release_sock(sk);
>
>         return sent;
>  }
> --
> 2.34.1
>


-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-02-09  2:45 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-08 22:19 [PATCH 1/2] Revert "Bluetooth: Fix passing NULL to PTR_ERR" Paul Menzel
2022-02-08 22:19 ` [PATCH 2/2] Revert "Bluetooth: RFCOMM: Replace use of memcpy_from_msg with bt_skb_sendmmsg" Paul Menzel
2022-02-08 23:39   ` Luiz Augusto von Dentz
2022-02-09  1:06   ` Luiz Augusto von Dentz
2022-02-08 23:02 ` [1/2] Revert "Bluetooth: Fix passing NULL to PTR_ERR" bluez.test.bot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.