Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Ioana Ciornei <ioana.ciornei@nxp.com>
To:     Andrew Lunn <andrew@lunn.ch>
CC:     "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "davem@davemloft.net" <davem@davemloft.net>,
        "f.fainelli@gmail.com" <f.fainelli@gmail.com>,
        "jiri@mellanox.com" <jiri@mellanox.com>
Subject: Re: [PATCH 0/5] staging: fsl-dpaa2/ethsw: add the .ndo_fdb_dump
 callback
Thread-Topic: [PATCH 0/5] staging: fsl-dpaa2/ethsw: add the .ndo_fdb_dump
 callback
Thread-Index: AQHVRihjfsknlOPS30mjM+S1Pbz/Qw==
Date:   Tue, 30 Jul 2019 09:13:01 +0000
Message-ID: <VI1PR0402MB28000AEB0C3AE6E45AC2AAC8E0DC0@VI1PR0402MB2800.eurprd04.prod.outlook.com>
References: <1564416712-16946-1-git-send-email-ioana.ciornei@nxp.com>
 <20190729163506.GJ4110@lunn.ch>
Accept-Language: en-US
Content-Language: en-US
received-spf: None (protection.outlook.com: nxp.com does not designate
 permitted sender hosts)
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 15b0c382-ac18-418c-32ab-08d714ce1f7a
X-MS-Exchange-CrossTenant-originalarrivaltime: 30 Jul 2019 09:13:01.8343
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: ioana.ciornei@nxp.com
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR0402MB2750
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 7/29/19 7:35 PM, Andrew Lunn wrote:=0A=
> On Mon, Jul 29, 2019 at 07:11:47PM +0300, Ioana Ciornei wrote:=0A=
>> This patch set adds some features and small fixes in the=0A=
>> FDB table manipulation area.=0A=
>>=0A=
>> First of all, we implement the .ndo_fdb_dump netdev callback so that all=
=0A=
>> offloaded FDB entries, either static or learnt, are available to the use=
r.=0A=
>> This is necessary because the DPAA2 switch does not emit interrupts when=
 a=0A=
>> new FDB is learnt or deleted, thus we are not able to keep the software=
=0A=
>> bridge state and the HW in sync by calling the switchdev notifiers.=0A=
>>=0A=
>> The patch set also adds the .ndo_fdb_[add|del] callbacks in order to=0A=
>> facilitate adding FDB entries not associated with any master device.=0A=
>>=0A=
>> One interesting thing that I observed is that when adding an FDB entry=
=0A=
>> associated with a bridge (ie using the 'master' keywork appended to the=
=0A=
>> bridge command) and then dumping the FDB entries, there will be duplicat=
es=0A=
>> of the same entry: one listed by the bridge device and one by the=0A=
>> driver's .ndo_fdb_dump).=0A=
>> It raises the question whether this is the expected behavior or not.=0A=
> =0A=
> DSA devices are the same, they don't provide an interrupt when a new=0A=
> entry is added by the hardware. So we can have two entries, or just=0A=
> the SW bridge entry, or just the HW entry, depending on ageing.=0A=
>=0A=
=0A=
This also happens when dealing with static entries (not just dynamic =0A=
ones that can be affected by ageing). All in all, the basic actions of =0A=
adding/deleting entries and then dumping them works. It was just a =0A=
question about switchdev's architecture.=0A=
=0A=
=0A=
>> Another concern is regarding the correct/desired machanism for drivers t=
o=0A=
>> signal errors back to switchdev on adding or deleting an FDB entry.=0A=
>> In the switchdev documentation, there is a TODO in the place of this top=
ic.=0A=
> =0A=
> It used to be a two state prepare/commit transaction, but that was=0A=
> changed a while back.=0A=
> =0A=
> Maybe the DSA core code can give you ideas?=0A=
>=0A=
=0A=
I looked in the DSA core before sending these patches out and it's doing =
=0A=
the exact same thing as ethsw - even though it notifies switchdev if the =
=0A=
entry could be offloaded (ie no error) all entries will still be present =
=0A=
in the 'bridge fdb' output. In the SWITCHDEV_FDB_DEL_TO_DEVICE case, it =0A=
seems that it just closes the netdev without any further action.=0A=
=0A=
On the other hand, the mlxsw_spectrum also calls the notifiers when an =0A=
offloaded entry is deleted (on SWITCHDEV_FDB_DEL_TO_DEVICE). This seems =0A=
like a reasonable thing to do, maybe in another patch set.=0A=
=0A=
Ioana C=0A=
=0A=