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.
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.
Ioana Ciornei (5):
staging: fsl-dpaa2/ethsw: remove unused structure
staging: fsl-dpaa2/ethsw: notify switchdev of offloaded entry
staging: fsl-dpaa2/ethsw: add .ndo_fdb_dump callback
staging: fsl-dpaa2/ethsw: check added_by_user flag
staging: fsl-dpaa2/ethsw: add .ndo_fdb[add|del] callbacks
drivers/staging/fsl-dpaa2/ethsw/TODO | 1 -
drivers/staging/fsl-dpaa2/ethsw/dpsw-cmd.h | 15 ++-
drivers/staging/fsl-dpaa2/ethsw/dpsw.c | 51 +++++++++
drivers/staging/fsl-dpaa2/ethsw/dpsw.h | 56 ++++-----
drivers/staging/fsl-dpaa2/ethsw/ethsw.c | 178 ++++++++++++++++++++++++++++-
5 files changed, 265 insertions(+), 36 deletions(-)
--
1.9.1
Add the .ndo_fdb_[add|del] callbacks so that FDB entries not associated
with a master device still end up offloaded.
Signed-off-by: Ioana Ciornei <[email protected]>
---
drivers/staging/fsl-dpaa2/ethsw/ethsw.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
index 2d3179c6bad8..4b94a01513a7 100644
--- a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
+++ b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
@@ -316,6 +316,31 @@ static int ethsw_port_fdb_del_mc(struct ethsw_port_priv *port_priv,
return err;
}
+static int port_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
+ struct net_device *dev, const unsigned char *addr,
+ u16 vid, u16 flags,
+ struct netlink_ext_ack *extack)
+{
+ if (is_unicast_ether_addr(addr))
+ return ethsw_port_fdb_add_uc(netdev_priv(dev),
+ addr);
+ else
+ return ethsw_port_fdb_add_mc(netdev_priv(dev),
+ addr);
+}
+
+static int port_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
+ struct net_device *dev,
+ const unsigned char *addr, u16 vid)
+{
+ if (is_unicast_ether_addr(addr))
+ return ethsw_port_fdb_del_uc(netdev_priv(dev),
+ addr);
+ else
+ return ethsw_port_fdb_del_mc(netdev_priv(dev),
+ addr);
+}
+
static void port_get_stats(struct net_device *netdev,
struct rtnl_link_stats64 *stats)
{
@@ -670,6 +695,8 @@ static int port_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb,
.ndo_change_mtu = port_change_mtu,
.ndo_has_offload_stats = port_has_offload_stats,
.ndo_get_offload_stats = port_get_offload_stats,
+ .ndo_fdb_add = port_fdb_add,
+ .ndo_fdb_del = port_fdb_del,
.ndo_fdb_dump = port_fdb_dump,
.ndo_start_xmit = port_dropframe,
--
1.9.1
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