All of lore.kernel.org
 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 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.