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
| ||
|
Date: Sat, 24 Sep 2022 17:27:59 +0200 From: Andrew Lunn <andrew@...n.ch> To: Vladimir Oltean <vladimir.oltean@....com> 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 02:42:50PM +0000, Vladimir Oltean wrote: > 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). I agree i've mostly looked at the dsa_inband_request side. Not having any hardware means i've been trying to keep the patches simple, obviously, and bug free. It seems i've failed at this last part. I hope we can iterate the dsa_inband_response() afterwards. My patchset is a good size on its own, so maybe we should get that merged first, before starting on dsa_inband_response(). I would also like to get some basic mv88e6xxx code merged, so it gives me a test system. > 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()). The autocast is just different. What i don't know is, does any other switch have anything similar? I guess every switch with inband is going to have Request/reply, so i think we should concentrate on that. Another thing we need to decide is, do we want to allow multiple in flight request/reply. If we say No, all need is to be able to determine is if a reply is late reply we should discard, or the reply to the current request. That is much simpler than a generic match the reply to a list of requests. For the moment, i would prefer KISS, lets get something working for two devices. We can make it more complex later. Andrew
Powered by blists - more mailing lists