Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:1 as permitted sender) client-ip=2620:137:e000::3:1;
Date:   Tue, 12 Sep 2023 17:26:44 +0300
From:   Vladimir Oltean <olteanv@gmail.com>
To:     Lukasz Majewski <lukma@denx.de>
Cc:     Tristram.Ha@microchip.com, Eric Dumazet <edumazet@google.com>,
        Andrew Lunn <andrew@lunn.ch>, davem@davemloft.net,
        Woojung Huh <woojung.huh@microchip.com>,
        Oleksij Rempel <o.rempel@pengutronix.de>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>, UNGLinuxDriver@microchip.com,
        Oleksij Rempel <linux@rempel-privat.de>,
        netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [[RFC PATCH v4 net-next] 0/2] net: dsa: hsr: Enable HSR HW
 offloading for KSZ9477
Message-ID: <20230912142644.u4sdkveei3e5hwaf@skbuf>
References: <20230911160501.5vc4nttz6fnww56h@skbuf>
 <20230912101748.0ca4eec8@wsk>
 <20230912092909.4yj4b2b4xrhzdztu@skbuf>
 <20230906152801.921664-1-lukma@denx.de>
 <20230911165848.0741c03c@wsk>
 <20230911160501.5vc4nttz6fnww56h@skbuf>
 <20230912101748.0ca4eec8@wsk>
 <20230912092909.4yj4b2b4xrhzdztu@skbuf>
 <20230912160326.188e1d13@wsk>
 <20230912160326.188e1d13@wsk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230912160326.188e1d13@wsk>
 <20230912160326.188e1d13@wsk>
Precedence: bulk

On Tue, Sep 12, 2023 at 04:03:26PM +0200, Lukasz Majewski wrote:
> > In any case, as long as it's the DSA master's address that we program
> > to hardware, then I see it as inevitable to add a new struct
> > dsa_switch_ops :: master_addr_change() function
> 
> Please correct my understanding. The above change would affect the
> whole DSA subsystem. It would require to have the core DSA modified and
> would affect its operation?

Uhm, yes, that would be the idea. If we were going to track changes to
the DSA master's MAC address, we should do it from the DSA framework
which has the existing netdev notifier listener infrastructure in place.

> > Or you can argue that dragging the DSA master into the discussion
> > about how we should program REG_SW_MAC_ADDR_0 is a complication.
> 
> Yes, it is IMHO the complication.

Ok, it's a point of view.

> git grep -n "REG_SW_MAC_ADDR_0"
> drivers/net/dsa/microchip/ksz8795_reg.h:326:#define REG_SW_MAC_ADDR_0
>         0x68 
> drivers/net/dsa/microchip/ksz9477.c:1194:
>      ksz_write8(dev, REG_SW_MAC_ADDR_0 + i,
> 
> drivers/net/dsa/microchip/ksz9477_reg.h:169:#define REG_SW_MAC_ADDR_0
> 0x0302
> 
> In the current net-next the REG_SW_MAC_ADDR_0 is altered used (the only
> usage are now with mine patches on ksz9477).
> 
> To sum up:
> 
> 1. Up till now in (net-next) REG_SW_MAC_ADDR_0 is ONLY declared for
> Microchip switches. No risk for access - other than HSR patches.

I know (except for Oleksij's WoL patches, which will eventually be
resubmitted).

> 2. The MAC address alteration of DSA master and propagation to slaves
> is a generic DSA bug.

Which can be/will be fixed. The diff I've included in the question to
Jakub closes it, in fact.

> Considering the above - the HSR implementation is safe (to the extend
> to the whole DSA subsystem current operation). Am I correct?

If we exclude the aforementioned bug (which won't be a bug forever),
there still exists the case where the MAC address of a DSA user port can
be changed. The HSR driver has a NETDEV_CHANGEADDR handler for this as
well, and updates its own MAC address to follow the port. If that is
allowed to happen after the offload, currently it will break the offload.