Re: [PATCH 10/21] offchannel: add support to issue multiple offchannel requests

[Denis Kenzior <denkenz@gmail.com>]

Hi James,

On 10/12/23 15:01, James Prestwood wrote:
> There was nothing which prevented this, but due to the behavior of
> some drivers multiple offchannel requests on the same channel

Isn't the point of offchannel API to serialize all such requests so this does 
not happen?

> resulted in the second request never starting and eventually timing
> out. This is because some drivers combine offchannel requests if
> they are on the same channel and this ultimately results in the
> netlink ACK coming after the ROC started event. This patch fixes
> some logic to allow for this case.

Kernel ROC APIs have no enforcement of semantics at all.  Different drivers just 
do whatever.  Have you tested that this works on brcmfmac for example?  It might 
be better to explicitly wait for the ROC event to be ended before starting a new 
one.

> 
> The motivation to support this is so modules can start offchannel
> work items for short durations and wait for a response, if a frame
> is received the offchannel request can be canceled/restarted for
> a longer duration.
> 
> This could also be done instead by using a long duration initially
> and an extra timer to cancel, but its more convenient if offchannel
> supports this natively. In addition, this driver quirk should be
> supported regardless (e.g. if two IWD modules happen to issue ROC's
> on the same channel).

Then shouldn't offchannel serialize such requests?

> 
> Furthermore, the offchannel module was only looking up requests by
> wdev_id which could result in the wrong request being found.
> Instead the request should be looked up by both wdev_id and cookie
> (when possible), or the ID in the case of canceling.

This part makes sense and probably belongs in a separate patch.

> ---
>   src/offchannel.c | 55 ++++++++++++++++++++++++++++++++++++++++++------
>   1 file changed, 48 insertions(+), 7 deletions(-)
> 

Regards,
-Denis