Re: [PATCH 1/5] firewire: fix use of multiple AV/C devices, allowmultiple FCP listeners

[Stefan Richter]

Pieter Palmers wrote:
> Stefan Richter wrote:
>> Date: Thu, 24 Dec 2009 12:05:58 +0100
>> From: Clemens Ladisch <cladisch@xxxxxxxxxxxx>
>>
>> Control of more than one AV/C device at once --- e.g. camcorders, tape
>> decks, audio devices, TV tuners --- failed or worked only unreliably,
>> depending on driver implementation.  This affected kernelspace and
>> userspace drivers alike and was caused by firewire-core's inability to
>> accept multiple registrations of FCP listeners.
>>
>> The fix allows multiple address handlers to be registered for the FCP
>> command and response registers.  When a request for these registers is
>> received, all handlers are invoked, and the Firewire response is
>> generated by the core and not by any handler.
> 
> I'm not convinced that this patch makes sense.

Oh yes, it makes a lot of sense because it simply fixes firewire-core
for the status quo.

To remind, the status quo is in the old stack as well as in the new
stack:  It is the responsibility of applications (high-level drivers)
  - to sort out whether an FCP request or FCP response is destined to
    them,
  - to serialize their own FCP transactions.
Neither ieee1394 core nor firewire-core cares for any of the above yet,
nor do they serialize FCP transactions to particular nodes or units or
subunits.

With this status quo in mind, the patch does nothing more or less than
bringing firewire-core's FCP related functionality to the same level as
ieee1394 core's.  The design of FCP handling in firewire-core and
ieee1394 is the same, only firewire-core's implementation of it was
lacking until now.

But what you are criticizing is the design, not the implementation:

> I can't come up with a
> scenario where multiple listeners would want access to an FCP transaction.
> 
> If the application is the originator of the request, it's also the only
> one that can make sense out of the response as in the general case you
> need to know what you asked in order to know what the response means. So
> there is little use in having multiple receivers for a response.

True.  We only have multiple receivers now because the kernel is not
(yet) in a position to sort out who the receiver is.

> I case the application is the FCP target, there is also little use in
> multiplexing as it discards any responses generated by the application,
> rendering it useless as an FCP target.

This is a misunderstandig of the term "Firewire response" in the
changelog.  The patch only takes care to supersede FCP related IEEE 1394
write response packets from applications (high-level drivers) by its own
write response packet.  This is in contrast to _FCP responses_ which are
of course not discarded.

On the terminology:  One FCP transaction is encapsulated into two IEEE
1394 transactions (in case of the typical immediate FCP transaction,
three IEEE 1394 transactions in case of deferred FCP transactions).
Typical case:
  - FCP request from A to B ==
    #1 IEEE 1394 block write request from A to B's FCP_Request CSR,
    #2 IEEE 1394 block write response from B to A,
  - FCP response from B to A ==
    #3 IEEE 1394 block write request from B to A's FCP_Response CSR,
    #4 IEEE 1394 block write response from A to B.
A is a controller, B is a target.  The IEEE 1394 write response can be
either a write response packet or an ack_complete packet; in case of
OHCI 1394 based controllers or targets, it is the former.

A Linux kernel on B generates packet #2 on behalf of 0...n FCP target
drivers running on B, and a Linux kernel on A generates packet #4 on
behalf of 0...n FCP controller drivers running on A.  Packets #1 and #3
are generated by one of the FCP drivers and passed through by a Linux
kernel without looking at them (yet).

> IMHO it is impossible to reliably match an FCP response to its request
> without relying on ordering. There is no information in the FCP frame to
> allow other mechanisms. If two applications send the same AV/C command,
> but with slightly different parameters, there is no way to tell what
> response is meant for what application.

True.  Though it has not been a practical problem yet.  Well, it has not
been a practical problem for video yet (camcorders, set top boxes etc.).
I am saying this based on linux1394-devel and linux1394-user traffic
over many years now and based on occasional querying of distribution
bugzillas.

If it is a practical problem for audio, then I have not heard of it
until now.  I guess you may want to run several independent processes
which concurrently control the same FCP target, do you?  (Streaming
driver == jack backend or ALSA backend, routing driver == mixer panel, ...)

> Furthermore, some (most?) of the devices don't support incoming FCP
> commands while another one is still being processed. In any case they
> don't have to, and can send a 'REJECTED' response to this second
> request. Where does this response end up?

Like with other responses, all FCP listeners get this response and match
it with their respective pending requests.  As you describe, this
becomes ambiguous if there is more than one pending request to the same
subunit, carrying the same command.

> After re-reading section 8 of the AV/C Digital Interface Command Set
> General Specification v4 (TA1999026), I think that the only way to
> reliably implement this is by moving the FCP and AV/C transaction logic
> into the kernel.
> 
> I don't see how you can reliably implement the following scenario using
> the current API/ABI:
> 
> """
> APP1 sends AVC_cmd_1 to device
> device responds with INTERIM
> 
> APP2 sends AVC_cmd_1 to device
> device responds with REJECTED as it doesn't support two AVC_cmd_1's
> this response should not end up at APP1, as that has a correct request
> pending.
> 
> device sends response to INTERIM, which should end up at APP1.
> """
> 
> I think this can only be implemented by tracking which AVC commands have
> been sent to what device, regardless of the userspace application (or
> kernel driver) that sent them, and disallowing any transactions that
> result in scenarios such as the above.

You are right about all this, but it does not devalue the discussed fix
patch.

If or when somebody designs a programming interface and implements the
infrastructure with the features that you described, the functionality
which this patch merely fixes can be removed, but not before.

Of course a hard requirement of such an interface is that the FCP
related parts of present libraw1394 v2 API can be layered on top of it.
It is a dated and deficient API, but it is here to stay.  (I.e. the
three FCP listening related functions and the generic IEEE 1394 write
transaction functions.)  A soft requirement is that present libraw1394
implementations remain functional.  (Without having it thought through
yet, I think both requirements can be meat without disproportional effort.)

Until then, the immediately necessary task is to fix the bug that
firewire-core does not allow more than one FCP listener at a time.  And
this is what Clemens' patch is doing perfectly.

---------------
PS:  I noted your request for kernel-assisted serialized FCP
transactions at
http://ieee1394.wiki.kernel.org/index.php/To_Do#firewire-core .

Of course such kernel infrastructure does not cover the case of several
controllers running on different nodes, controlling the same target.  In
this case, whether it all works still depends on the capabilities and
correctness of the controllers and the target.
-- 
Stefan Richter
-=====-==--= ==-- ==-=-
http://arcgraph.de/sr/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/