Date:   Fri, 3 Feb 2023 23:14:51 +0200
From:   Vladimir Oltean <olteanv@gmail.com>
To:     Ido Schimmel <idosch@idosch.org>
Cc:     netdev@kapio-technology.com, davem@davemloft.net, kuba@kernel.org,
        netdev@vger.kernel.org, Florian Fainelli <f.fainelli@gmail.com>,
        Andrew Lunn <andrew@lunn.ch>,
        Eric Dumazet <edumazet@google.com>,
        Paolo Abeni <pabeni@redhat.com>,
        Kurt Kanzenbach <kurt@linutronix.de>,
        Hauke Mehrtens <hauke@hauke-m.de>,
        Woojung Huh <woojung.huh@microchip.com>,
        "maintainer:MICROCHIP KSZ SERIES ETHERNET SWITCH DRIVER" 
        <UNGLinuxDriver@microchip.com>, Sean Wang <sean.wang@mediatek.com>,
        Landen Chao <Landen.Chao@mediatek.com>,
        DENG Qingfang <dqfext@gmail.com>,
        Matthias Brugger <matthias.bgg@gmail.com>,
        Claudiu Manoil <claudiu.manoil@nxp.com>,
        Alexandre Belloni <alexandre.belloni@bootlin.com>,
        =?utf-8?B?Q2zDqW1lbnQgTMOpZ2Vy?= <clement.leger@bootlin.com>,
        Jiri Pirko <jiri@resnulli.us>,
        Ivan Vecera <ivecera@redhat.com>,
        Roopa Prabhu <roopa@nvidia.com>,
        Nikolay Aleksandrov <razor@blackwall.org>,
        Russell King <linux@armlinux.org.uk>,
        Christian Marangi <ansuelsmth@gmail.com>,
        open list <linux-kernel@vger.kernel.org>,
        "moderated list:ARM/Mediatek SoC support" 
        <linux-arm-kernel@lists.infradead.org>,
        "moderated list:ARM/Mediatek SoC support" 
        <linux-mediatek@lists.infradead.org>,
        "open list:RENESAS RZ/N1 A5PSW SWITCH DRIVER" 
        <linux-renesas-soc@vger.kernel.org>,
        "moderated list:ETHERNET BRIDGE" <bridge@lists.linux-foundation.org>
Subject: Re: [PATCH net-next 0/5] ATU and FDB synchronization on locked ports
Message-ID: <20230203211451.3rhg2kg6tjmmhfmp@skbuf>
References: <20230130173429.3577450-1-netdev@kapio-technology.com>
 <Y9lrIWMnWLqGreZL@shredder>
 <e2535b002be9044958ab0003d8bd6966@kapio-technology.com>
 <Y9vaIOefIf/gI0BR@shredder>
 <3cecf4425b0e6f38646e25e40fd8f0fd@kapio-technology.com>
 <Y9vmfoaFxPdKvgxt@shredder>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <Y9vmfoaFxPdKvgxt@shredder>
Precedence: bulk

On Thu, Feb 02, 2023 at 06:36:14PM +0200, Ido Schimmel wrote:
> On Thu, Feb 02, 2023 at 05:19:07PM +0100, netdev@kapio-technology.com wrote:
> > On 2023-02-02 16:43, Ido Schimmel wrote:
> > > On Thu, Feb 02, 2023 at 08:37:08AM +0100, netdev@kapio-technology.com wrote:
> > > > On 2023-01-31 20:25, Ido Schimmel wrote:
> > > > >
> > > > > Will try to review tomorrow, but it looks like this set is missing
> > > > > selftests. What about extending bridge_locked_port.sh?
> > > > 
> > > > I knew you would take this up. :-)
> > > > But I am not sure that it's so easy to have selftests here as it is timing
> > > > based and it would take the 5+ minutes just waiting to test in the stadard
> > > > case, and there is opnly support for mv88e6xxx driver with this
> > > > patch set.
> > > 
> > > The ageing time is configurable: See commit 081197591769 ("selftests:
> > > net: bridge: Parameterize ageing timeout"). Please add test cases in the
> > > next version.
> > 
> > When I was looking at configuring the ageing time last time, my finding was
> > that the ageing time could not be set very low as there was some part in the
> > DSA layer etc, and confusion wrt units. I think the minimum secured was like
> > around 2 min. (not validated), which is not that much of an improvement for
> > fast testing. If you know what would be a good low timeout to set, I would
> > like to know.
> 
> My point is that the ageing time is parametrized via 'LOW_AGEING_TIME'
> in forwarding.config so just use '$LOW_AGEING_TIME' in the selftest and
> set it as high as it needs to be for mv88e6xxx in your own
> forwarding.config.

FWIW, we have a forwarding.config file in tools/testing/selftests/drivers/net/dsa/.
So you could cd to that folder, edit the file with your variable, and run the symlinked
script from there.

> as there was some part in the DSA layer etc

	if (ds->ageing_time_min && ageing_time < ds->ageing_time_min)
		return -ERANGE;

High tech, advanced software.....

You could print the ds->ageing_time_min variable. For mv88e6xxx, my 6390
and 6190 report 3750. I have to admit the ageing time units are confusing,
but Tobias Waldekranz kindly explained in one of those commit messages
that Ido linked to that these represent "centiseconds" (or 37.5 seconds).
And I think we discussed the units with you before. And in general, it's
not hard to find the answer if you search for it, I know I could find it.

Please stop trying to find silly excuses to always go through the path
of minimal resistance.