Content-Type: text/plain;
        charset=us-ascii
Mime-Version: 1.0 (Mac OS X Mail 12.2 \(3445.102.3\))
Subject: Re: [PATCH] Bluetooth: Fix flow bugs in H5 so the protocol doesn't
 stall
From:   Marcel Holtmann <marcel@holtmann.org>
In-Reply-To: <CAO1O6sf8G8VPJLoctB4mNC842NUE4FA0KKi8Hb-cVQq+rfuTag@mail.gmail.com>
Date:   Fri, 18 Jan 2019 11:56:04 +0100
Cc:     Bluez mailing list <linux-bluetooth@vger.kernel.org>
Content-Transfer-Encoding: 8BIT
Message-Id: <091E1C5C-18A8-4100-BD41-F930957B09DD@holtmann.org>
References: <1527256763-13474-1-git-send-email-emil.lenngren@gmail.com>
 <A73D99C8-7E66-49CF-8B85-E8879308A5F8@holtmann.org>
 <CAO1O6sf8G8VPJLoctB4mNC842NUE4FA0KKi8Hb-cVQq+rfuTag@mail.gmail.com>
To:     Emil Lenngren <emil.lenngren@gmail.com>
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk

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