Message ID | 1564416712-16946-1-git-send-email-ioana.ciornei@nxp.com |
---|---|
Headers | show |
Series | staging: fsl-dpaa2/ethsw: add the .ndo_fdb_dump callback | expand |
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. > 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? Andrew
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