From: Vladimir Oltean <olteanv@gmail.com>
To: Hans Schultz <netdev@kapio-technology.com>
Cc: Ivan Vecera <ivecera@redhat.com>, Andrew Lunn <andrew@lunn.ch>,
Florian Fainelli <f.fainelli@gmail.com>,
Jiri Pirko <jiri@resnulli.us>,
Daniel Borkmann <daniel@iogearbox.net>,
netdev@vger.kernel.org, Nikolay Aleksandrov <razor@blackwall.org>,
bridge@lists.linux-foundation.org, linux-kernel@vger.kernel.org,
Ido Schimmel <idosch@nvidia.com>,
Vivien Didelot <vivien.didelot@gmail.com>,
Eric Dumazet <edumazet@google.com>,
linux-kselftest@vger.kernel.org, Roopa Prabhu <roopa@nvidia.com>,
kuba@kernel.org, Paolo Abeni <pabeni@redhat.com>,
Shuah Khan <shuah@kernel.org>,
davem@davemloft.net
Subject: Re: [Bridge] [PATCH net-next 1/1] net: dsa: mv88e6xxx: allow reading FID when handling ATU violations
Date: Thu, 7 Jul 2022 13:28:36 +0300 [thread overview]
Message-ID: <20220707102836.u7ig6rr2664mcrlf@skbuf> (raw)
In-Reply-To: <20220706122502.1521819-1-netdev@kapio-technology.com>
On Wed, Jul 06, 2022 at 02:25:02PM +0200, Hans Schultz wrote:
> For convenience the function mv88e6xxx_g1_atu_op() has been used to read
> ATU violations, but the function has other purposes and does not enable
> the possibility to read the FID when reading ATU violations.
>
> The FID is needed to get hold of which VID was involved in the violation,
> thus the need for future purposes to be able to read the FID.
Make no mistake, the existing code doesn't disallow reading back the FID
during an ATU Get/Clear Violation operation, and your patch isn't
"allowing" something that wasn't disallowed.
The documentation for the ATU FID register says that its contents is
ignored before the operation starts, and it contains the returned ATU
entry's FID after the operation completes.
So the change simply says: don't bother to write the ATU FID register
with zero, it doesn't matter what this contains. This is probably true,
but the patch needs to do what's written on the box.
Please note that this only even matters at all for switches with
mv88e6xxx_num_databases(chip) > 256, where MV88E6352_G1_ATU_FID is a
dedicated register which this patch avoids writing. For other switches,
the FID is embedded within MV88E6XXX_G1_ATU_CTL or MV88E6XXX_G1_ATU_OP.
So _practically_, for those switches, you are still emitting the
GET_CLR_VIOLATION ATU op with a FID of 0 whether you like it or not, and
this patch introduces a (most likely irrelevant) discrepancy between the
access methods for various switches.
Please note that this observation is relevant for your future changes to
read back the FID too. As I said here:
https://patchwork.kernel.org/project/netdevbpf/patch/20220524152144.40527-4-schultz.hans+netdev@gmail.com/#24912482
you can't just assume that the FID lies within the MV88E6352_G1_ATU_FID
register, just look at the way it is packed within mv88e6xxx_g1_atu_op().
You'll need to unpack it in the same way.
next prev parent reply other threads:[~2022-07-07 10:28 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-06 12:25 [Bridge] [PATCH net-next 1/1] net: dsa: mv88e6xxx: allow reading FID when handling ATU violations Hans Schultz
2022-07-07 10:28 ` Vladimir Oltean [this message]
2022-07-08 0:43 ` Jakub Kicinski
2022-07-08 7:12 ` netdev
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220707102836.u7ig6rr2664mcrlf@skbuf \
--to=olteanv@gmail.com \
--cc=andrew@lunn.ch \
--cc=bridge@lists.linux-foundation.org \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=f.fainelli@gmail.com \
--cc=idosch@nvidia.com \
--cc=ivecera@redhat.com \
--cc=jiri@resnulli.us \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=netdev@kapio-technology.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=razor@blackwall.org \
--cc=roopa@nvidia.com \
--cc=shuah@kernel.org \
--cc=vivien.didelot@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).