Received-SPF: pass (google.com: domain of linux-kernel+bounces-31246-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Date: Fri, 19 Jan 2024 15:12:04 +0100
From: Andrew Lunn <andrew@lunn.ch>
To: MD Danish Anwar <danishanwar@ti.com>
Cc: Rob Herring <robh@kernel.org>, Dan Carpenter <dan.carpenter@linaro.org>,
	Jan Kiszka <jan.kiszka@siemens.com>,
	Vladimir Oltean <vladimir.oltean@nxp.com>,
	Wolfram Sang <wsa+renesas@sang-engineering.com>,
	Arnd Bergmann <arnd@arndb.de>,
	Grygorii Strashko <grygorii.strashko@ti.com>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Roger Quadros <rogerq@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Jakub Kicinski <kuba@kernel.org>,
	Eric Dumazet <edumazet@google.com>,
	"David S. Miller" <davem@davemloft.net>,
	linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, srk@ti.com, r-gunasekaran@ti.com
Subject: Re: [RFC PATCH v2 2/3] net: ti: icssg-switch: Add switchdev based
 driver for ethernet switch support
Message-ID: <3ea8934e-c41f-4366-a0c4-1894ed4e3d7e@lunn.ch>
References: <20240118071005.1514498-1-danishanwar@ti.com>
 <20240118071005.1514498-3-danishanwar@ti.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240118071005.1514498-3-danishanwar@ti.com>

> +static int prueth_switchdev_stp_state_set(struct prueth_emac *emac,
> +					  u8 state)
> +{
> +	enum icssg_port_state_cmd emac_state;
> +	int ret = 0;
> +
> +	switch (state) {
> +	case BR_STATE_FORWARDING:
> +		emac_state = ICSSG_EMAC_PORT_FORWARD;
> +		break;
> +	case BR_STATE_DISABLED:
> +		emac_state = ICSSG_EMAC_PORT_DISABLE;
> +		break;
> +	case BR_STATE_LEARNING:
> +	case BR_STATE_LISTENING:
> +	case BR_STATE_BLOCKING:
> +		emac_state = ICSSG_EMAC_PORT_BLOCK;
> +		break;

That is unusual. Does it still learn while in BLOCK? It might be you
need to flush the FDB for this port when it changes from BLOCKING to
LISTENING or LEARNING?

> +static void prueth_switchdev_event_work(struct work_struct *work)
> +{
> +	struct prueth_switchdev_event_work *switchdev_work =
> +		container_of(work, struct prueth_switchdev_event_work, work);
> +	struct prueth_emac *emac = switchdev_work->emac;
> +	struct switchdev_notifier_fdb_info *fdb;
> +	int port_id = emac->port_id;
> +	int ret;
> +
> +	rtnl_lock();
> +	switch (switchdev_work->event) {
> +	case SWITCHDEV_FDB_ADD_TO_DEVICE:
> +		fdb = &switchdev_work->fdb_info;
> +
> +		netdev_dbg(emac->ndev, "prueth_fdb_add: MACID = %pM vid = %u flags = %u %u -- port %d\n",
> +			   fdb->addr, fdb->vid, fdb->added_by_user,
> +			   fdb->offloaded, port_id);
> +
> +		if (!fdb->added_by_user)
> +			break;
> +		if (memcmp(emac->mac_addr, (u8 *)fdb->addr, ETH_ALEN) == 0)
> +			break;

ether_addr_equal(). Please review all the code and use these helpers
when possible.

So you don't add an FDB for the interfaces own MAC address? Does the
switch know the interfaces MAC address?

       Andrew