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