netdev - Re: [PATCH rfc v2 08/10] net: dsa: qca8k: Pass error code from reply decoder to requester

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 24 Sep 2022 14:42:50 +0000
From:   Vladimir Oltean <vladimir.oltean@....com>
To:     Andrew Lunn <andrew@...n.ch>
CC:     netdev <netdev@...r.kernel.org>,
        "mattias.forsblad@...il.com" <mattias.forsblad@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Christian Marangi <ansuelsmth@...il.com>
Subject: Re: [PATCH rfc v2 08/10] net: dsa: qca8k: Pass error code from reply
 decoder to requester

On Sat, Sep 24, 2022 at 04:32:15PM +0200, Andrew Lunn wrote:
> > My understanding of the autocast function (I could be wrong) is that
> > it's essentially one request with 10 (or how many ports there are)
> > responses. At least this is what the code appears to handle.
> 
> The autocast packet handling does not fit the model. I already
> excluded it from parts of the patchset. I might need to exclude it
> from more. It is something i need to understand more. I did find a
> leaked data sheet for the qca8337, but i've not had time to read it
> yet.
> 
> Either the model needs to change a bit, or we don't convert this part
> of the code to use shared functions, or maybe we can do a different
> implementation in the driver for statistics access.

I was thinking, as a complement to your series, maybe we could make the
response processing also generic (so instead of the tagger calling a
driver-provided handler, let it call a dsa_inband_response() instead).
This would look through the list of queued ds->requests, and have a
(*match_cb)() which returns an action, be it "packet doesn't match this
request", "packet matches, please remove request from list", or "packet
matches, but still keep request in list". In addition, the queued
request will also have a (*cb)(), which is the action to execute on
match from a response. The idea is that if we bother to provide a
generic implementation within DSA, at least we could try to make its
core async, and just offer sychronous wrappers if this is what drivers
wish to use (like a generic cb() which calls complete()).

This would also come hand in hand with the requests being allocated on
demand, which I think would simplify a bit the notion that an unexpected
response might trigger a match with an unsolicited request.