linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Marcel Holtmann <marcel@holtmann.org>
To: Emil Lenngren <emil.lenngren@gmail.com>
Cc: Bluez mailing list <linux-bluetooth@vger.kernel.org>
Subject: Re: [PATCH] Bluetooth: Fix flow bugs in H5 so the protocol doesn't stall
Date: Fri, 18 Jan 2019 11:56:04 +0100	[thread overview]
Message-ID: <091E1C5C-18A8-4100-BD41-F930957B09DD@holtmann.org> (raw)
In-Reply-To: <CAO1O6sf8G8VPJLoctB4mNC842NUE4FA0KKi8Hb-cVQq+rfuTag@mail.gmail.com>

Hi Emil,

>>> 1. If more than tx_win packets are enqueued, so that the unack queue
>>> gets full, then when packets are later acked, uart tx is not woken up,
>>> meaning that the flow will be stalled unless uart tx is not later
>>> woken up for some other reason (e.g. packet is received so an ack
>>> needs to be sent).
>>> 
>>> 2. If remote peer sends tx_win packets to us and our ack(s) are
>>> incorrectly received by the remote device, it will first resend the
>>> tx_win packets and wait for their ack before it can send the next
>>> packets. However, we only send ack if a NEW packet (not a resent packet)
>>> is arrived. Therefore, we will never send ack and the remote device
>>> will keep resend the packets (and wait for the acks) forever, until
>>> we send a new tx packet.
>> 
>> do you have interest in working on the bt3wire.c driver that is a pure serdev driver and make it fully H:5 compliant. I think it would be good to move away from hci_h5.c since it is too much entangled with the line discipline.
> 
> I can take a look at it but can't promise anything. I found the email
> from 2018-03-18. Is that the latest version? Are there any known
> issues with it expect that it misses important features?
> 
> Anyway, the current hci_h5.c driver should still be fixed since it's
> still in use and probably will for some time...
> Yeah the protocol gets pretty complicated in practice and there are
> quite many "edge" cases that can be a bit tricky to cover. I'm not
> sure if a few comments spread out at different places would make
> someone follow the code. I would rather see them as a compliment to a
> bigger description at the top of the file or so, discussing various
> flows and cases. What do you think?

we can fix it in hci_h5.c since it will help current users. Please send patches separated out and I take a look.

Regards

Marcel


      reply	other threads:[~2019-01-18 10:56 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-25 13:59 [PATCH] Bluetooth: Fix flow bugs in H5 so the protocol doesn't stall Emil Lenngren
2018-12-29 15:18 ` Emil Lenngren
2018-12-30 10:29 ` Marcel Holtmann
2018-12-30 14:01   ` Emil Lenngren
2019-01-18 10:56     ` Marcel Holtmann [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=091E1C5C-18A8-4100-BD41-F930957B09DD@holtmann.org \
    --to=marcel@holtmann.org \
    --cc=emil.lenngren@gmail.com \
    --cc=linux-bluetooth@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).