RE: [RFCv0 0/3] AMP HCI interface from A2MP

[Marcel Holtmann <marcel@holtmann.org>]

Hi Peter,

> > > > The code adds AMP HCI commands from A2MP protocol. HCI events are handled
> > > > similar way as mgmt interface. amp_pending is a kind of copy of mgmt_pending.
> > >
> > > this is all kernel internal code with no interface to user space. I do
> > > not see the need for such a complex infrastructure. Can not just have
> > > proper callbacks or event callback table like with L2CAP. Or just
> > > something similar.
> > 
> > I see this as a simple callback. amp_pending is just keeping context of HCI
> > command we need to handle. I also included reference counting since we had
> > bad experience with l2cap and rfcomm.
> 
> There does have to be some way to carry the A2MP message context while performing
> local HCI operations to service the message. A2MP Get Remote Assoc requires multiple
> HCI commands with data accumulated between the commands before the response can be
> sent. 

I agree, but this amp_pending seems to be wrong. I talked about changing
the init handling and at the same time we might can apply this to the
A2MP handling.

Why not just set a bit in hdev->dev_flags or maybe even hcon->dev_flags
or something and based on that we do a nice async state machine that
sends the commands and results in calling back into A2MP core when its
finished.

That seems a bit simpler to me and more aligned in a way I wanna redo
the whole HCI command/event handling anyway. In addition we can just
expose these flags via debugfs as text and have a nice way of knowing
current states if something goes wrong.

> > > As far as I see it, we get an A2MP command over L2CAP fixed channel, we
> > > have to issue a HCI command or do some other task based on this and then
> > > respond to it. We only have one user of this first of all. And second of
> > > all, I think we can not really have pending A2MP commands anyway. This
> > > is pretty much one command at a time (per ACL link).
> 
> A2MP commands are serialized by the sender, so A2MP message context could be
> associated with the hci_conn for BR-EDR.

That is what I thought. Thanks for confirming. So using hcon->dev_flags
seems like a possible way to make this a lot simpler.

And if the sender misbehaves, we just reject that commands that way.
Testing a flag is always cheaper then iterating a list.

Regards

Marcel