2020-11-04 17:03:48

by Ioana Ciornei

[permalink] [raw]
Subject: [RFC 0/9] staging: dpaa2-switch: add support for CPU terminated traffic

From: Ioana Ciornei <[email protected]>

This patch set adds support for Rx/Tx capabilities on DPAA2 switch port
interfaces as well as fixing up so major blunders in how we take care of
the switching domains. The netdev community considers this as a basic
features, thus it's sent against the staging tree before anything can be
moved out of it.

The control interface is comprised of 3 queues in total: Rx, Rx error
and Tx confirmation. In this patch set we only enable Rx and Tx conf.
All switch ports share the same queues when frames are redirected to the
CPU. Information regarding the ingress switch port is passed through
frame metadata - the flow context field of the descriptor.

NAPI instances are also shared between switch net_devices and are
enabled when at least on one of the switch ports .dev_open() was called
and disabled when no switch port is still up.

Since the last version of this feature was submitted to the list, I
reworked how the switching and flooding domains are taken care of by the
driver, thus the switch is now able to also add the control port (the
queues that the CPU can dequeue from) into the flooding domains of a
port (broadcast, unknown unicast etc). With this, we are able to receive
and sent traffic from the switch interfaces.

Also, the capability to properly partition the DPSW object into multiple
switching domains was added so that when not under a bridge, the ports
are not actually capable to switch between them. This is possible by
adding a private FDB table per switch interface. When multiple switch
interfaces are under the same bridge, they will all use the same FDB
table.

Another thing that is fixed in this patch set is how the driver handles
VLAN awareness. The DPAA2 switch is not capable to run as VLAN unaware
but this was not reflected in how the driver responded to requests to
change the VLAN awareness. In the last patch, this is fixed by
describing the switch interfaces as Rx VLAN filtering on [fixed] and
declining any request to join a VLAN unaware bridge.

I am sending this as a RFC since I would like to receive feedback on the
current capabilities of the switch driver and if they meet or not meet
the expectations, all this before I actually settle on a firmware
interface.

Ioana Ciornei (9):
staging: dpaa2-switch: get control interface attributes
staging: dpaa2-switch: setup buffer pool for control traffic
staging: dpaa2-switch: setup RX path rings
staging: dpaa2-switch: setup dpio
staging: dpaa2-switch: handle Rx path on control interface
staging: dpaa2-switch: add .ndo_start_xmit() callback
staging: dpaa2-switch: enable the control interface
staging: dpaa2-switch: properly setup switching domains
staging: dpaa2-switch: accept only vlan-aware upper devices

drivers/staging/fsl-dpaa2/Kconfig | 1 +
drivers/staging/fsl-dpaa2/ethsw/dpsw-cmd.h | 89 +-
drivers/staging/fsl-dpaa2/ethsw/dpsw.c | 269 +++++
drivers/staging/fsl-dpaa2/ethsw/dpsw.h | 144 +++
drivers/staging/fsl-dpaa2/ethsw/ethsw.c | 1131 ++++++++++++++++++--
drivers/staging/fsl-dpaa2/ethsw/ethsw.h | 75 +-
6 files changed, 1641 insertions(+), 68 deletions(-)

--
2.28.0


2020-11-04 20:13:20

by Ioana Ciornei

[permalink] [raw]
Subject: [RFC 8/9] staging: dpaa2-switch: properly setup switching domains

From: Ioana Ciornei <[email protected]>

Until now, the DPAA2 switch was not capable to properly setup it's
switching domains depending on the existence, or lack thereof, of a
upper bridge device. This meant that all switch ports of a DPSW object
were switching by default even though they were not under the same
bridge device.

Another issue was the inability to actually add the CPU in the flooding
domains (broadcast, unknown unicast etc) of a particular switch port.
This meant that a simple ping on a switch interface was not possible
since no broadcast ARP frame would actually reach the CPU queues.

This patch tries to fix exactly these problems by:

* Creating and managing a FDB table for each flooding domain. This means
that when a switch interface is not bridged it will use it's own FDB
table. While in bridged mode all DPAA2 switch interfaces under the
same upper will use the same FDB table, thus leverage the same FDB
entries.

* Adding a new MC firmware command - dpsw_set_egress_flood() - through
which the driver can setup the flooding domains as needed. For
example, when the switch interface is standalone, thus not in a
bridge with any other DPAA2 switch port, it will setup it's broadcast
and unknown unicast flooding domains to only include the control
interface (the queues that reach the CPU and the driver can dequeue
from). This flooding domain changes when the interface joins a bridge
and is configured to include, beside the control interface, all other
DPAA2 switch interfaces.

Signed-off-by: Ioana Ciornei <[email protected]>
---
drivers/staging/fsl-dpaa2/ethsw/dpsw-cmd.h | 25 ++-
drivers/staging/fsl-dpaa2/ethsw/dpsw.c | 96 +++++++++
drivers/staging/fsl-dpaa2/ethsw/dpsw.h | 41 ++++
drivers/staging/fsl-dpaa2/ethsw/ethsw.c | 220 +++++++++++++++++----
drivers/staging/fsl-dpaa2/ethsw/ethsw.h | 6 +-
5 files changed, 345 insertions(+), 43 deletions(-)

diff --git a/drivers/staging/fsl-dpaa2/ethsw/dpsw-cmd.h b/drivers/staging/fsl-dpaa2/ethsw/dpsw-cmd.h
index c6eb9bffdd3e..9b068b07bbcd 100644
--- a/drivers/staging/fsl-dpaa2/ethsw/dpsw-cmd.h
+++ b/drivers/staging/fsl-dpaa2/ethsw/dpsw-cmd.h
@@ -15,9 +15,11 @@
#define DPSW_VER_MINOR 4

#define DPSW_CMD_BASE_VERSION 1
+#define DPSW_CMD_2ND_VERSION 2
#define DPSW_CMD_ID_OFFSET 4

#define DPSW_CMD_ID(id) (((id) << DPSW_CMD_ID_OFFSET) | DPSW_CMD_BASE_VERSION)
+#define DPSW_CMD_V2(id) (((id) << DPSW_CMD_ID_OFFSET) | DPSW_CMD_2ND_VERSION)

/* Command IDs */
#define DPSW_CMDID_CLOSE DPSW_CMD_ID(0x800)
@@ -58,7 +60,7 @@
#define DPSW_CMDID_IF_SET_LINK_CFG DPSW_CMD_ID(0x04C)

#define DPSW_CMDID_VLAN_ADD DPSW_CMD_ID(0x060)
-#define DPSW_CMDID_VLAN_ADD_IF DPSW_CMD_ID(0x061)
+#define DPSW_CMDID_VLAN_ADD_IF DPSW_CMD_V2(0x061)
#define DPSW_CMDID_VLAN_ADD_IF_UNTAGGED DPSW_CMD_ID(0x062)

#define DPSW_CMDID_VLAN_REMOVE_IF DPSW_CMD_ID(0x064)
@@ -66,6 +68,8 @@
#define DPSW_CMDID_VLAN_REMOVE_IF_FLOODING DPSW_CMD_ID(0x066)
#define DPSW_CMDID_VLAN_REMOVE DPSW_CMD_ID(0x067)

+#define DPSW_CMDID_FDB_ADD DPSW_CMD_ID(0x082)
+#define DPSW_CMDID_FDB_REMOVE DPSW_CMD_ID(0x083)
#define DPSW_CMDID_FDB_ADD_UNICAST DPSW_CMD_ID(0x084)
#define DPSW_CMDID_FDB_REMOVE_UNICAST DPSW_CMD_ID(0x085)
#define DPSW_CMDID_FDB_ADD_MULTICAST DPSW_CMD_ID(0x086)
@@ -83,6 +87,8 @@
#define DPSW_CMDID_CTRL_IF_DISABLE DPSW_CMD_ID(0x0A3)
#define DPSW_CMDID_CTRL_IF_SET_QUEUE DPSW_CMD_ID(0x0A6)

+#define DPSW_CMDID_SET_EGRESS_FLOOD DPSW_CMD_ID(0x0AC)
+
/* Macros for accessing command fields smaller than 1byte */
#define DPSW_MASK(field) \
GENMASK(DPSW_##field##_SHIFT + DPSW_##field##_SIZE - 1, \
@@ -324,6 +330,16 @@ struct dpsw_vlan_add {
__le16 vlan_id;
};

+struct dpsw_cmd_vlan_add_if {
+ /* cmd word 0 */
+ __le16 options;
+ __le16 vlan_id;
+ __le16 fdb_id;
+ __le16 pad0;
+ /* cmd word 1-4 */
+ __le64 if_id;
+};
+
struct dpsw_cmd_vlan_manage_if {
/* cmd word 0 */
__le16 pad0;
@@ -445,4 +461,11 @@ struct dpsw_cmd_if_set_mac_addr {
u8 mac_addr[6];
};

+struct dpsw_cmd_set_egress_flood {
+ __le16 fdb_id;
+ u8 flood_type;
+ u8 pad[5];
+ __le64 if_id;
+};
+
#endif /* __FSL_DPSW_CMD_H */
diff --git a/drivers/staging/fsl-dpaa2/ethsw/dpsw.c b/drivers/staging/fsl-dpaa2/ethsw/dpsw.c
index f5cfe61498b8..936984e7af50 100644
--- a/drivers/staging/fsl-dpaa2/ethsw/dpsw.c
+++ b/drivers/staging/fsl-dpaa2/ethsw/dpsw.c
@@ -986,6 +986,76 @@ int dpsw_vlan_remove(struct fsl_mc_io *mc_io,
return mc_send_command(mc_io, &cmd);
}

+/**
+ * dpsw_fdb_add() - Add FDB to switch and Returns handle to FDB table for
+ * the reference
+ * @mc_io: Pointer to MC portal's I/O object
+ * @cmd_flags: Command flags; one or more of 'MC_CMD_FLAG_'
+ * @token: Token of DPSW object
+ * @fdb_id: Returned Forwarding Database Identifier
+ * @cfg: FDB Configuration
+ *
+ * Return: Completion status. '0' on Success; Error code otherwise.
+ */
+int dpsw_fdb_add(struct fsl_mc_io *mc_io,
+ u32 cmd_flags,
+ u16 token,
+ u16 *fdb_id,
+ const struct dpsw_fdb_cfg *cfg)
+{
+ struct dpsw_cmd_fdb_add *cmd_params;
+ struct dpsw_rsp_fdb_add *rsp_params;
+ struct fsl_mc_command cmd = { 0 };
+ int err;
+
+ /* prepare command */
+ cmd.header = mc_encode_cmd_header(DPSW_CMDID_FDB_ADD,
+ cmd_flags,
+ token);
+ cmd_params = (struct dpsw_cmd_fdb_add *)cmd.params;
+ cmd_params->fdb_aging_time = cpu_to_le16(cfg->fdb_aging_time);
+ cmd_params->num_fdb_entries = cpu_to_le16(cfg->num_fdb_entries);
+
+ /* send command to mc*/
+ err = mc_send_command(mc_io, &cmd);
+ if (err)
+ return err;
+
+ /* retrieve response parameters */
+ rsp_params = (struct dpsw_rsp_fdb_add *)cmd.params;
+ *fdb_id = le16_to_cpu(rsp_params->fdb_id);
+
+ return 0;
+}
+
+/**
+ * dpsw_fdb_remove() - Remove FDB from switch
+ * @mc_io: Pointer to MC portal's I/O object
+ * @cmd_flags: Command flags; one or more of 'MC_CMD_FLAG_'
+ * @token: Token of DPSW object
+ * @fdb_id: Forwarding Database Identifier
+ *
+ * Return: Completion status. '0' on Success; Error code otherwise.
+ */
+int dpsw_fdb_remove(struct fsl_mc_io *mc_io,
+ u32 cmd_flags,
+ u16 token,
+ u16 fdb_id)
+{
+ struct dpsw_cmd_fdb_remove *cmd_params;
+ struct fsl_mc_command cmd = { 0 };
+
+ /* prepare command */
+ cmd.header = mc_encode_cmd_header(DPSW_CMDID_FDB_REMOVE,
+ cmd_flags,
+ token);
+ cmd_params = (struct dpsw_cmd_fdb_remove *)cmd.params;
+ cmd_params->fdb_id = cpu_to_le16(fdb_id);
+
+ /* send command to mc*/
+ return mc_send_command(mc_io, &cmd);
+}
+
/**
* dpsw_fdb_add_unicast() - Function adds an unicast entry into MAC lookup table
* @mc_io: Pointer to MC portal's I/O object
@@ -1493,3 +1563,29 @@ int dpsw_ctrl_if_disable(struct fsl_mc_io *mc_io, u32 cmd_flags, u16 token)

return mc_send_command(mc_io, &cmd);
}
+
+/**
+ * dpsw_set_egress_flood() - Set egress parameters associated with an FDB ID
+ * @mc_io: Pointer to MC portal's I/O object
+ * @cmd_flags: Command flags; one or more of 'MC_CMD_FLAG_'
+ * @token: Token of DPSW object
+ * @cfg: Egress flooding configuration
+ *
+ * Return: '0' on Success; Error code otherwise.
+ */
+int dpsw_set_egress_flood(struct fsl_mc_io *mc_io, u32 cmd_flags, u16 token,
+ const struct dpsw_egress_flood_cfg *cfg)
+{
+ struct dpsw_cmd_set_egress_flood *cmd_params;
+ struct fsl_mc_command cmd = { 0 };
+
+ cmd.header = mc_encode_cmd_header(DPSW_CMDID_SET_EGRESS_FLOOD,
+ cmd_flags,
+ token);
+ cmd_params = (struct dpsw_cmd_set_egress_flood *)cmd.params;
+ cmd_params->fdb_id = cpu_to_le16(cfg->fdb_id);
+ cmd_params->flood_type = cfg->flood_type;
+ build_if_id_bitmap(&cmd_params->if_id, cfg->if_id, cfg->num_ifs);
+
+ return mc_send_command(mc_io, &cmd);
+}
diff --git a/drivers/staging/fsl-dpaa2/ethsw/dpsw.h b/drivers/staging/fsl-dpaa2/ethsw/dpsw.h
index 9e30adad4ffa..8fe76c7227ea 100644
--- a/drivers/staging/fsl-dpaa2/ethsw/dpsw.h
+++ b/drivers/staging/fsl-dpaa2/ethsw/dpsw.h
@@ -493,6 +493,8 @@ int dpsw_vlan_add(struct fsl_mc_io *mc_io,
u16 vlan_id,
const struct dpsw_vlan_cfg *cfg);

+#define DPSW_VLAN_ADD_IF_OPT_FDB_ID 0x0001
+
/**
* struct dpsw_vlan_if_cfg - Set of VLAN Interfaces
* @num_ifs: The number of interfaces that are assigned to the egress
@@ -502,7 +504,9 @@ int dpsw_vlan_add(struct fsl_mc_io *mc_io,
*/
struct dpsw_vlan_if_cfg {
u16 num_ifs;
+ u16 options;
u16 if_id[DPSW_MAX_IF];
+ u16 fdb_id;
};

int dpsw_vlan_add_if(struct fsl_mc_io *mc_io,
@@ -692,4 +696,41 @@ int dpsw_if_get_primary_mac_addr(struct fsl_mc_io *mc_io, u32 cmd_flags,
int dpsw_if_set_primary_mac_addr(struct fsl_mc_io *mc_io, u32 cmd_flags,
u16 token, u16 if_id, u8 mac_addr[6]);

+/**
+ * struct dpsw_fdb_cfg - FDB Configuration
+ * @num_fdb_entries: Number of FDB entries
+ * @fdb_aging_time: Aging time in seconds
+ */
+struct dpsw_fdb_cfg {
+ u16 num_fdb_entries;
+ u16 fdb_aging_time;
+};
+
+int dpsw_fdb_add(struct fsl_mc_io *mc_io,
+ u32 cmd_flags,
+ u16 token,
+ u16 *fdb_id,
+ const struct dpsw_fdb_cfg *cfg);
+
+int dpsw_fdb_remove(struct fsl_mc_io *mc_io,
+ u32 cmd_flags,
+ u16 token,
+ u16 fdb_id);
+
+enum dpsw_flood_type {
+ DPSW_BROADCAST_FLOOD = 0,
+ DPSW_UNICAST_FLOOD,
+ DPSW_MULTICAST_FLOOD,
+};
+
+struct dpsw_egress_flood_cfg {
+ u16 fdb_id;
+ enum dpsw_flood_type flood_type;
+ u16 num_ifs;
+ u16 if_id[DPSW_MAX_IF];
+};
+
+int dpsw_set_egress_flood(struct fsl_mc_io *mc_io, u32 cmd_flags, u16 token,
+ const struct dpsw_egress_flood_cfg *cfg);
+
#endif /* __FSL_DPSW_H */
diff --git a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
index 24bdac6d6005..7a0d9a178cdc 100644
--- a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
+++ b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
@@ -25,6 +25,36 @@

#define DEFAULT_VLAN_ID 1

+static u16 dpaa2_switch_port_get_fdb_id(struct ethsw_port_priv *port_priv)
+{
+ struct ethsw_port_priv *other_port_priv = NULL;
+ struct net_device *other_dev;
+ struct list_head *iter;
+
+ /* If not part of a bridge, just use the private FDB */
+ if (!port_priv->bridge_dev)
+ return port_priv->fdb_id;
+
+ /* If part of a bridge, use the FDB of the first dpaa2 switch interface
+ * to be present in that bridge
+ */
+ netdev_for_each_lower_dev(port_priv->bridge_dev, other_dev, iter) {
+ if (!dpaa2_switch_port_dev_check(other_dev, NULL))
+ continue;
+
+ other_port_priv = netdev_priv(other_dev);
+ break;
+ }
+
+ /* We are the first dpaa2 switch interface to join the bridge, just use
+ * our own FDB
+ */
+ if (!other_port_priv)
+ other_port_priv = port_priv;
+
+ return other_port_priv->fdb_id;
+}
+
static void *dpaa2_iova_to_virt(struct iommu_domain *domain,
dma_addr_t iova_addr)
{
@@ -133,7 +163,7 @@ static int dpaa2_switch_port_add_vlan(struct ethsw_port_priv *port_priv,
{
struct ethsw_core *ethsw = port_priv->ethsw_data;
struct net_device *netdev = port_priv->netdev;
- struct dpsw_vlan_if_cfg vcfg;
+ struct dpsw_vlan_if_cfg vcfg = {0};
int err;

if (port_priv->vlans[vid]) {
@@ -141,8 +171,13 @@ static int dpaa2_switch_port_add_vlan(struct ethsw_port_priv *port_priv,
return -EEXIST;
}

+ /* If hit, this VLAN rule will lead the packet into the FDB table
+ * specified in the vlan configuration below
+ */
vcfg.num_ifs = 1;
vcfg.if_id[0] = port_priv->idx;
+ vcfg.fdb_id = dpaa2_switch_port_get_fdb_id(port_priv);
+ vcfg.options |= DPSW_VLAN_ADD_IF_OPT_FDB_ID;
err = dpsw_vlan_add_if(ethsw->mc_io, 0, ethsw->dpsw_handle, vid, &vcfg);
if (err) {
netdev_err(netdev, "dpsw_vlan_add_if err %d\n", err);
@@ -172,8 +207,10 @@ static int dpaa2_switch_port_add_vlan(struct ethsw_port_priv *port_priv,
return 0;
}

-static int dpaa2_switch_set_learning(struct ethsw_core *ethsw, bool enable)
+static int dpaa2_switch_port_set_learning(struct ethsw_port_priv *port_priv, bool enable)
{
+ u16 fdb_id = dpaa2_switch_port_get_fdb_id(port_priv);
+ struct ethsw_core *ethsw = port_priv->ethsw_data;
enum dpsw_fdb_learning_mode learn_mode;
int err;

@@ -182,13 +219,12 @@ static int dpaa2_switch_set_learning(struct ethsw_core *ethsw, bool enable)
else
learn_mode = DPSW_FDB_LEARNING_MODE_DIS;

- err = dpsw_fdb_set_learning_mode(ethsw->mc_io, 0, ethsw->dpsw_handle, 0,
+ err = dpsw_fdb_set_learning_mode(ethsw->mc_io, 0, ethsw->dpsw_handle, fdb_id,
learn_mode);
if (err) {
dev_err(ethsw->dev, "dpsw_fdb_set_learning_mode err %d\n", err);
return err;
}
- ethsw->learning = enable;

return 0;
}
@@ -267,15 +303,17 @@ static int dpaa2_switch_port_fdb_add_uc(struct ethsw_port_priv *port_priv,
const unsigned char *addr)
{
struct dpsw_fdb_unicast_cfg entry = {0};
+ u16 fdb_id;
int err;

entry.if_egress = port_priv->idx;
entry.type = DPSW_FDB_ENTRY_STATIC;
ether_addr_copy(entry.mac_addr, addr);

+ fdb_id = dpaa2_switch_port_get_fdb_id(port_priv);
err = dpsw_fdb_add_unicast(port_priv->ethsw_data->mc_io, 0,
port_priv->ethsw_data->dpsw_handle,
- 0, &entry);
+ fdb_id, &entry);
if (err)
netdev_err(port_priv->netdev,
"dpsw_fdb_add_unicast err %d\n", err);
@@ -306,6 +344,7 @@ static int dpaa2_switch_port_fdb_add_mc(struct ethsw_port_priv *port_priv,
const unsigned char *addr)
{
struct dpsw_fdb_multicast_cfg entry = {0};
+ u16 fdb_id;
int err;

ether_addr_copy(entry.mac_addr, addr);
@@ -313,9 +352,10 @@ static int dpaa2_switch_port_fdb_add_mc(struct ethsw_port_priv *port_priv,
entry.num_ifs = 1;
entry.if_id[0] = port_priv->idx;

+ fdb_id = dpaa2_switch_port_get_fdb_id(port_priv);
err = dpsw_fdb_add_multicast(port_priv->ethsw_data->mc_io, 0,
port_priv->ethsw_data->dpsw_handle,
- 0, &entry);
+ fdb_id, &entry);
/* Silently discard error for calling multiple times the add command */
if (err && err != -ENXIO)
netdev_err(port_priv->netdev, "dpsw_fdb_add_multicast err %d\n",
@@ -723,6 +763,7 @@ static int dpaa2_switch_port_fdb_dump(struct sk_buff *skb, struct netlink_callba
u32 fdb_dump_size;
int err = 0, i;
u8 *dma_mem;
+ u16 fdb_id;

fdb_dump_size = ethsw->sw_attr.max_fdb_entries * sizeof(fdb_entry);
dma_mem = kzalloc(fdb_dump_size, GFP_KERNEL);
@@ -737,7 +778,8 @@ static int dpaa2_switch_port_fdb_dump(struct sk_buff *skb, struct netlink_callba
goto err_map;
}

- err = dpsw_fdb_dump(ethsw->mc_io, 0, ethsw->dpsw_handle, 0,
+ fdb_id = dpaa2_switch_port_get_fdb_id(port_priv);
+ err = dpsw_fdb_dump(ethsw->mc_io, 0, ethsw->dpsw_handle, fdb_id,
fdb_dump_iova, fdb_dump_size, &num_fdb_entries);
if (err) {
netdev_err(net_dev, "dpsw_fdb_dump() = %d\n", err);
@@ -960,8 +1002,8 @@ static const struct net_device_ops dpaa2_switch_port_ops = {
.ndo_get_phys_port_name = dpaa2_switch_port_get_phys_name,
};

-static bool dpaa2_switch_port_dev_check(const struct net_device *netdev,
- struct notifier_block *nb)
+bool dpaa2_switch_port_dev_check(const struct net_device *netdev,
+ struct notifier_block *nb)
{
struct ethsw_port_priv *port_priv = netdev_priv(netdev);

@@ -1119,9 +1161,11 @@ static int dpaa2_switch_port_attr_br_flags_set(struct net_device *netdev,
if (switchdev_trans_ph_prepare(trans))
return 0;

- /* Learning is enabled per switch */
- err = dpaa2_switch_set_learning(port_priv->ethsw_data,
- !!(flags & BR_LEARNING));
+ /* Learning is enabled per swiching domain, thus all dpaa2 switch
+ * interfaces under the same bridge will have their flooding state
+ * changed also
+ */
+ err = dpaa2_switch_port_set_learning(port_priv, !!(flags & BR_LEARNING));
if (err)
goto exit;

@@ -1403,24 +1447,69 @@ static int dpaa2_switch_port_attr_set_event(struct net_device *netdev,
return notifier_from_errno(err);
}

-/* For the moment, only flood setting needs to be updated */
-static int dpaa2_switch_port_bridge_join(struct net_device *netdev,
- struct net_device *upper_dev)
+static int dpaa2_switch_port_set_egress_flood(struct ethsw_port_priv *port_priv)
{
- struct ethsw_port_priv *port_priv = netdev_priv(netdev);
struct ethsw_core *ethsw = port_priv->ethsw_data;
+ struct net_device *netdev = port_priv->netdev;
struct ethsw_port_priv *other_port_priv;
+ struct dpsw_egress_flood_cfg flood_cfg;
struct net_device *other_dev;
struct list_head *iter;
- int i, err;
+ int i = 0;
+ int err;

- for (i = 0; i < ethsw->sw_attr.num_ifs; i++)
- if (ethsw->ports[i]->bridge_dev &&
- (ethsw->ports[i]->bridge_dev != upper_dev)) {
- netdev_err(netdev,
- "Only one bridge supported per DPSW object!\n");
- return -EINVAL;
+ if (port_priv->bridge_dev) {
+ /* Add all the DPAA2 switch ports found under the same bridge to the
+ * egress flooding domain
+ */
+ netdev_for_each_lower_dev(port_priv->bridge_dev, other_dev, iter) {
+ if (!dpaa2_switch_port_dev_check(other_dev, NULL))
+ continue;
+
+ other_port_priv = netdev_priv(other_dev);
+ flood_cfg.if_id[i++] = other_port_priv->idx;
}
+ } else {
+ flood_cfg.if_id[i++] = port_priv->idx;
+ }
+
+ /* Add the CTRL interface to the egress flooding domain */
+ flood_cfg.if_id[i++] = ethsw->sw_attr.num_ifs;
+
+ /* Use the FDB of the first dpaa2 switch port added to the bridge */
+ flood_cfg.fdb_id = dpaa2_switch_port_get_fdb_id(port_priv);
+
+ /* Setup broadcast flooding domain */
+ flood_cfg.flood_type = DPSW_BROADCAST_FLOOD;
+ flood_cfg.num_ifs = i;
+ err = dpsw_set_egress_flood(ethsw->mc_io, 0, ethsw->dpsw_handle,
+ &flood_cfg);
+ if (err) {
+ netdev_err(netdev, "dpsw_set_egress_flood() = %d\n", err);
+ return err;
+ }
+
+ /* Setup unknown unicast flooding domain */
+ flood_cfg.flood_type = DPSW_UNICAST_FLOOD;
+ flood_cfg.num_ifs = i;
+ err = dpsw_set_egress_flood(ethsw->mc_io, 0, ethsw->dpsw_handle,
+ &flood_cfg);
+ if (err) {
+ netdev_err(netdev, "dpsw_set_egress_flood() = %d\n", err);
+ return err;
+ }
+
+ return 0;
+}
+
+static int dpaa2_switch_port_bridge_join(struct net_device *netdev,
+ struct net_device *upper_dev)
+{
+ struct ethsw_port_priv *port_priv = netdev_priv(netdev);
+ struct ethsw_port_priv *other_port_priv;
+ struct net_device *other_dev;
+ struct list_head *iter;
+ int err;

netdev_for_each_lower_dev(upper_dev, other_dev, iter) {
if (!dpaa2_switch_port_dev_check(other_dev, NULL))
@@ -1434,11 +1523,23 @@ static int dpaa2_switch_port_bridge_join(struct net_device *netdev,
}
}

- /* Enable flooding */
- err = dpaa2_switch_port_set_flood(port_priv, 1);
- if (!err)
- port_priv->bridge_dev = upper_dev;
+ /* Delete the previously manually installed VLAN 1 */
+ err = dpaa2_switch_port_del_vlan(port_priv, 1);
+ if (err)
+ return err;
+
+ /* Keep track of the upper bridge device */
+ port_priv->bridge_dev = upper_dev;
+
+ /* Setup the egress flood policy (broadcast, unknown unicast) */
+ err = dpaa2_switch_port_set_egress_flood(port_priv);
+ if (err)
+ goto err_egress_flood;
+
+ return 0;

+err_egress_flood:
+ port_priv->bridge_dev = NULL;
return err;
}

@@ -1447,10 +1548,24 @@ static int dpaa2_switch_port_bridge_leave(struct net_device *netdev)
struct ethsw_port_priv *port_priv = netdev_priv(netdev);
int err;

- /* Disable flooding */
- err = dpaa2_switch_port_set_flood(port_priv, 0);
- if (!err)
- port_priv->bridge_dev = NULL;
+ /* Port is not part of a bridge anymore */
+ port_priv->bridge_dev = NULL;
+
+ /* Setup the egress flood policy (broadcast, unknown unicast).
+ * When the port is not under a bridge, only the CTRL interface is part
+ * of the flooding domain besides the actual port
+ */
+ err = dpaa2_switch_port_set_egress_flood(port_priv);
+ if (err)
+ return err;
+
+ /* Add the VLAN 1 as PVID when not under a bridge. We need this since
+ * the dpaa2 switch interfaces are not capable to be VLAN unaware
+ */
+ err = dpaa2_switch_port_add_vlan(port_priv, 1,
+ BRIDGE_VLAN_INFO_UNTAGGED | BRIDGE_VLAN_INFO_PVID);
+ if (err)
+ return err;

return err;
}
@@ -2291,13 +2406,6 @@ static int dpaa2_switch_init(struct fsl_mc_device *sw_dev)
goto err_close;
}

- err = dpsw_fdb_set_learning_mode(ethsw->mc_io, 0, ethsw->dpsw_handle, 0,
- DPSW_FDB_LEARNING_MODE_HW);
- if (err) {
- dev_err(dev, "dpsw_fdb_set_learning_mode err %d\n", err);
- goto err_close;
- }
-
stp_cfg.vlan_id = DEFAULT_VLAN_ID;
stp_cfg.state = DPSW_STP_STATE_FORWARDING;

@@ -2352,6 +2460,7 @@ static int dpaa2_switch_port_init(struct ethsw_port_priv *port_priv, u16 port)
{
struct net_device *netdev = port_priv->netdev;
struct ethsw_core *ethsw = port_priv->ethsw_data;
+ struct dpsw_fdb_cfg fdb_cfg = {0};
struct dpsw_if_attr dpsw_if_attr;
struct dpsw_vlan_if_cfg vcfg;
int err;
@@ -2387,6 +2496,38 @@ static int dpaa2_switch_port_init(struct ethsw_port_priv *port_priv, u16 port)
}
port_priv->tx_qdid = dpsw_if_attr.qdid;

+ /* Create a FDB table for this particular switch port */
+ fdb_cfg.num_fdb_entries = ethsw->sw_attr.max_fdb_entries / ethsw->sw_attr.num_ifs;
+ err = dpsw_fdb_add(ethsw->mc_io, 0, ethsw->dpsw_handle,
+ &port_priv->fdb_id, &fdb_cfg);
+ if (err) {
+ netdev_err(netdev, "dpsw_fdb_add err %d\n", err);
+ return err;
+ }
+
+ /* Setup the default learning mode to he HW learning enabled */
+ err = dpsw_fdb_set_learning_mode(ethsw->mc_io, 0, ethsw->dpsw_handle,
+ port_priv->fdb_id,
+ DPSW_FDB_LEARNING_MODE_HW);
+ if (err) {
+ netdev_err(netdev, "dpsw_fdb_set_learning_mode err %d\n", err);
+ return err;
+ }
+
+ /* We need to add VLAN 1 as the PVID on this port until it is under a
+ * bridge since the DPAA2 switch is not able to handle the traffic in a
+ * VLAN unaware fashion
+ */
+ err = dpaa2_switch_port_add_vlan(port_priv, 1,
+ BRIDGE_VLAN_INFO_UNTAGGED | BRIDGE_VLAN_INFO_PVID);
+ if (err)
+ return err;
+
+ /* Setup the egress flooding domains (broadcast, unknown unicast */
+ err = dpaa2_switch_port_set_egress_flood(port_priv);
+ if (err)
+ return err;
+
return 0;
}

@@ -2562,9 +2703,6 @@ static int dpaa2_switch_probe(struct fsl_mc_device *sw_dev)
/* DEFAULT_VLAN_ID is implicitly configured on the switch */
ethsw->vlans[DEFAULT_VLAN_ID] = ETHSW_VLAN_MEMBER;

- /* Learning is implicitly enabled */
- ethsw->learning = true;
-
ethsw->ports = kcalloc(ethsw->sw_attr.num_ifs, sizeof(*ethsw->ports),
GFP_KERNEL);
if (!(ethsw->ports)) {
diff --git a/drivers/staging/fsl-dpaa2/ethsw/ethsw.h b/drivers/staging/fsl-dpaa2/ethsw/ethsw.h
index b267c04e2008..f905acd18c67 100644
--- a/drivers/staging/fsl-dpaa2/ethsw/ethsw.h
+++ b/drivers/staging/fsl-dpaa2/ethsw/ethsw.h
@@ -106,6 +106,7 @@ struct ethsw_port_priv {
u16 pvid;
struct net_device *bridge_dev;
u16 tx_qdid;
+ u16 fdb_id;
};

/* Switch data */
@@ -121,7 +122,6 @@ struct ethsw_core {
struct iommu_domain *iommu_domain;

u8 vlans[VLAN_VID_MASK + 1];
- bool learning;

struct notifier_block port_nb;
struct notifier_block port_switchdev_nb;
@@ -139,4 +139,8 @@ static inline bool dpaa2_switch_has_ctrl_if(struct ethsw_core *ethsw)
{
return !(ethsw->sw_attr.options & DPSW_OPT_CTRL_IF_DIS);
}
+
+bool dpaa2_switch_port_dev_check(const struct net_device *netdev,
+ struct notifier_block *nb);
+
#endif /* __ETHSW_H */
--
2.28.0

2020-11-05 02:10:26

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [RFC 8/9] staging: dpaa2-switch: properly setup switching domains

On Wed, Nov 04, 2020 at 06:57:19PM +0200, Ioana Ciornei wrote:
> From: Ioana Ciornei <[email protected]>
>
> Until now, the DPAA2 switch was not capable to properly setup it's
> switching domains depending on the existence, or lack thereof, of a
> upper bridge device. This meant that all switch ports of a DPSW object
> were switching by default even though they were not under the same
> bridge device.
>
> Another issue was the inability to actually add the CPU in the flooding
> domains (broadcast, unknown unicast etc) of a particular switch port.
> This meant that a simple ping on a switch interface was not possible
> since no broadcast ARP frame would actually reach the CPU queues.
>
> This patch tries to fix exactly these problems by:
>
> * Creating and managing a FDB table for each flooding domain. This means
> that when a switch interface is not bridged it will use it's own FDB
> table. While in bridged mode all DPAA2 switch interfaces under the
> same upper will use the same FDB table, thus leverage the same FDB
> entries.
>
> * Adding a new MC firmware command - dpsw_set_egress_flood() - through
> which the driver can setup the flooding domains as needed. For
> example, when the switch interface is standalone, thus not in a
> bridge with any other DPAA2 switch port, it will setup it's broadcast
> and unknown unicast flooding domains to only include the control
> interface (the queues that reach the CPU and the driver can dequeue
> from). This flooding domain changes when the interface joins a bridge
> and is configured to include, beside the control interface, all other
> DPAA2 switch interfaces.
>
> Signed-off-by: Ioana Ciornei <[email protected]>
> ---

None of the occurrences of "it's" in the commit message is grammatically
correct. So please s/it's/its/g.

> diff --git a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
> index 24bdac6d6005..7a0d9a178cdc 100644
> --- a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
> +++ b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
> @@ -25,6 +25,36 @@
>
> #define DEFAULT_VLAN_ID 1
>
> +static u16 dpaa2_switch_port_get_fdb_id(struct ethsw_port_priv *port_priv)
> +{
> + struct ethsw_port_priv *other_port_priv = NULL;
> + struct net_device *other_dev;
> + struct list_head *iter;
> +
> + /* If not part of a bridge, just use the private FDB */
> + if (!port_priv->bridge_dev)
> + return port_priv->fdb_id;
> +
> + /* If part of a bridge, use the FDB of the first dpaa2 switch interface
> + * to be present in that bridge
> + */
> + netdev_for_each_lower_dev(port_priv->bridge_dev, other_dev, iter) {

netdev_for_each_lower_dev calls netdev_lower_get_next which has this in
the comments:

* The caller must hold RTNL lock or
* its own locking that guarantees that the neighbour lower
* list will remain unchanged.

Does that hold true for all callers, if you put ASSERT_RTNL() here?

> + if (!dpaa2_switch_port_dev_check(other_dev, NULL))
> + continue;
> +
> + other_port_priv = netdev_priv(other_dev);
> + break;
> + }
> +
> + /* We are the first dpaa2 switch interface to join the bridge, just use
> + * our own FDB
> + */
> + if (!other_port_priv)
> + other_port_priv = port_priv;
> +
> + return other_port_priv->fdb_id;
> +}
> +
> static void *dpaa2_iova_to_virt(struct iommu_domain *domain,
> dma_addr_t iova_addr)
> {
> @@ -133,7 +163,7 @@ static int dpaa2_switch_port_add_vlan(struct ethsw_port_priv *port_priv,
> {
> struct ethsw_core *ethsw = port_priv->ethsw_data;
> struct net_device *netdev = port_priv->netdev;
> - struct dpsw_vlan_if_cfg vcfg;
> + struct dpsw_vlan_if_cfg vcfg = {0};
> int err;
>
> if (port_priv->vlans[vid]) {
> @@ -141,8 +171,13 @@ static int dpaa2_switch_port_add_vlan(struct ethsw_port_priv *port_priv,
> return -EEXIST;
> }
>
> + /* If hit, this VLAN rule will lead the packet into the FDB table
> + * specified in the vlan configuration below
> + */

And this is the reason why VLAN-unaware mode is unsupported, right? No
hit on any VLAN rule => no FDB table selected for the packet. What is
the default action for misses on VLAN rules? Drop or some default FDB
ID?

> vcfg.num_ifs = 1;
> vcfg.if_id[0] = port_priv->idx;
> + vcfg.fdb_id = dpaa2_switch_port_get_fdb_id(port_priv);
> + vcfg.options |= DPSW_VLAN_ADD_IF_OPT_FDB_ID;
> err = dpsw_vlan_add_if(ethsw->mc_io, 0, ethsw->dpsw_handle, vid, &vcfg);
> if (err) {
> netdev_err(netdev, "dpsw_vlan_add_if err %d\n", err);
> @@ -172,8 +207,10 @@ static int dpaa2_switch_port_add_vlan(struct ethsw_port_priv *port_priv,
> return 0;
> }
>
> -static int dpaa2_switch_set_learning(struct ethsw_core *ethsw, bool enable)
> +static int dpaa2_switch_port_set_learning(struct ethsw_port_priv *port_priv, bool enable)

The commit message says nothing about changes to the learning
configuration.

> {
> + u16 fdb_id = dpaa2_switch_port_get_fdb_id(port_priv);
> + struct ethsw_core *ethsw = port_priv->ethsw_data;
> enum dpsw_fdb_learning_mode learn_mode;
> int err;
>
> @@ -182,13 +219,12 @@ static int dpaa2_switch_set_learning(struct ethsw_core *ethsw, bool enable)
> else
> learn_mode = DPSW_FDB_LEARNING_MODE_DIS;
>
> - err = dpsw_fdb_set_learning_mode(ethsw->mc_io, 0, ethsw->dpsw_handle, 0,
> + err = dpsw_fdb_set_learning_mode(ethsw->mc_io, 0, ethsw->dpsw_handle, fdb_id,
> learn_mode);
> if (err) {
> dev_err(ethsw->dev, "dpsw_fdb_set_learning_mode err %d\n", err);
> return err;
> }
> - ethsw->learning = enable;
>
> return 0;
> }
> @@ -267,15 +303,17 @@ static int dpaa2_switch_port_fdb_add_uc(struct ethsw_port_priv *port_priv,
> const unsigned char *addr)
> {
> struct dpsw_fdb_unicast_cfg entry = {0};
> + u16 fdb_id;
> int err;
>
> entry.if_egress = port_priv->idx;
> entry.type = DPSW_FDB_ENTRY_STATIC;
> ether_addr_copy(entry.mac_addr, addr);
>
> + fdb_id = dpaa2_switch_port_get_fdb_id(port_priv);
> err = dpsw_fdb_add_unicast(port_priv->ethsw_data->mc_io, 0,
> port_priv->ethsw_data->dpsw_handle,
> - 0, &entry);
> + fdb_id, &entry);

Hmmm, so in dpaa2_switch_port_get_fdb_id you say:

/* If part of a bridge, use the FDB of the first dpaa2 switch interface
* to be present in that bridge
*/

So let's say there is a br0 with swp3 and swp2, and a br1 with swp4.
IIUC, br0 interfaces (swp3 and swp2) will have an fdb_id of 3 (due to
swp3 being added first) and br1 will have an fdb_id of 4 (due to swp4).

When swp3 leaves br0, will this cause the fdb_id of swp2 to change?
I expect the answer is yes, since otherwise swp2 and swp3 would keep
forwarding packets to one another. Is this change graceful?

For example, if you add a static FDB entry to swp2 prior to removing
swp3, I would expect the fdb_id of swp2 to preserve that static FDB
entry, even if swp2 now gets moved to a different fdb_id. Similarly,
flooding settings, everything is preserved when the fdb_id changes?

The flip side of that is: what happens if you add an FDB entry to swp2,
then you remove swp2 from br0 and move it to br1? Will swp4 (which was
already in br1) see that static FDB entry in hardware, even if the
software bridge br1 hasn't notified you about it?

Basically, my question boils down to: why is there so little activity in
dpaa2_switch_port_bridge_leave.

> if (err)
> netdev_err(port_priv->netdev,
> "dpsw_fdb_add_unicast err %d\n", err);

2020-11-05 11:00:37

by Ioana Ciornei

[permalink] [raw]
Subject: Re: [RFC 8/9] staging: dpaa2-switch: properly setup switching domains

On Thu, Nov 05, 2020 at 12:08:10AM +0200, Vladimir Oltean wrote:
> On Wed, Nov 04, 2020 at 06:57:19PM +0200, Ioana Ciornei wrote:
> > From: Ioana Ciornei <[email protected]>
> >
> > Until now, the DPAA2 switch was not capable to properly setup it's
> > switching domains depending on the existence, or lack thereof, of a
> > upper bridge device. This meant that all switch ports of a DPSW object
> > were switching by default even though they were not under the same
> > bridge device.
> >
> > Another issue was the inability to actually add the CPU in the flooding
> > domains (broadcast, unknown unicast etc) of a particular switch port.
> > This meant that a simple ping on a switch interface was not possible
> > since no broadcast ARP frame would actually reach the CPU queues.
> >
> > This patch tries to fix exactly these problems by:
> >
> > * Creating and managing a FDB table for each flooding domain. This means
> > that when a switch interface is not bridged it will use it's own FDB
> > table. While in bridged mode all DPAA2 switch interfaces under the
> > same upper will use the same FDB table, thus leverage the same FDB
> > entries.
> >
> > * Adding a new MC firmware command - dpsw_set_egress_flood() - through
> > which the driver can setup the flooding domains as needed. For
> > example, when the switch interface is standalone, thus not in a
> > bridge with any other DPAA2 switch port, it will setup it's broadcast
> > and unknown unicast flooding domains to only include the control
> > interface (the queues that reach the CPU and the driver can dequeue
> > from). This flooding domain changes when the interface joins a bridge
> > and is configured to include, beside the control interface, all other
> > DPAA2 switch interfaces.
> >
> > Signed-off-by: Ioana Ciornei <[email protected]>
> > ---
>
> None of the occurrences of "it's" in the commit message is grammatically
> correct. So please s/it's/its/g.
>
> > diff --git a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
> > index 24bdac6d6005..7a0d9a178cdc 100644
> > --- a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
> > +++ b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
> > @@ -25,6 +25,36 @@
> >
> > #define DEFAULT_VLAN_ID 1
> >
> > +static u16 dpaa2_switch_port_get_fdb_id(struct ethsw_port_priv *port_priv)
> > +{
> > + struct ethsw_port_priv *other_port_priv = NULL;
> > + struct net_device *other_dev;
> > + struct list_head *iter;
> > +
> > + /* If not part of a bridge, just use the private FDB */
> > + if (!port_priv->bridge_dev)
> > + return port_priv->fdb_id;
> > +
> > + /* If part of a bridge, use the FDB of the first dpaa2 switch interface
> > + * to be present in that bridge
> > + */
> > + netdev_for_each_lower_dev(port_priv->bridge_dev, other_dev, iter) {
>
> netdev_for_each_lower_dev calls netdev_lower_get_next which has this in
> the comments:
>
> * The caller must hold RTNL lock or
> * its own locking that guarantees that the neighbour lower
> * list will remain unchanged.
>
> Does that hold true for all callers, if you put ASSERT_RTNL() here?

No, not for all. The probe path uses this as well and is not protected
by the rtnl lock.

Good point, I'll add an explicit lock/unlock. Thanks.

>
> > + if (!dpaa2_switch_port_dev_check(other_dev, NULL))
> > + continue;
> > +
> > + other_port_priv = netdev_priv(other_dev);
> > + break;
> > + }
> > +
> > + /* We are the first dpaa2 switch interface to join the bridge, just use
> > + * our own FDB
> > + */
> > + if (!other_port_priv)
> > + other_port_priv = port_priv;
> > +
> > + return other_port_priv->fdb_id;
> > +}
> > +
> > static void *dpaa2_iova_to_virt(struct iommu_domain *domain,
> > dma_addr_t iova_addr)
> > {
> > @@ -133,7 +163,7 @@ static int dpaa2_switch_port_add_vlan(struct ethsw_port_priv *port_priv,
> > {
> > struct ethsw_core *ethsw = port_priv->ethsw_data;
> > struct net_device *netdev = port_priv->netdev;
> > - struct dpsw_vlan_if_cfg vcfg;
> > + struct dpsw_vlan_if_cfg vcfg = {0};
> > int err;
> >
> > if (port_priv->vlans[vid]) {
> > @@ -141,8 +171,13 @@ static int dpaa2_switch_port_add_vlan(struct ethsw_port_priv *port_priv,
> > return -EEXIST;
> > }
> >
> > + /* If hit, this VLAN rule will lead the packet into the FDB table
> > + * specified in the vlan configuration below
> > + */
>
> And this is the reason why VLAN-unaware mode is unsupported, right?

Yes, exactly.

> No
> hit on any VLAN rule => no FDB table selected for the packet. What is
> the default action for misses on VLAN rules? Drop or some default FDB
> ID?

The default action for misses on the VLAN table is drop. For example, if
a VLAN tagged packet is received on a switch interface which does not
have an upper VLAN interface with that VLAN id (thus the
.ndo_vlan_rx_add_vid() is not called) , the packet will get dropped
immediately.

>
> > vcfg.num_ifs = 1;
> > vcfg.if_id[0] = port_priv->idx;
> > + vcfg.fdb_id = dpaa2_switch_port_get_fdb_id(port_priv);
> > + vcfg.options |= DPSW_VLAN_ADD_IF_OPT_FDB_ID;
> > err = dpsw_vlan_add_if(ethsw->mc_io, 0, ethsw->dpsw_handle, vid, &vcfg);
> > if (err) {
> > netdev_err(netdev, "dpsw_vlan_add_if err %d\n", err);
> > @@ -172,8 +207,10 @@ static int dpaa2_switch_port_add_vlan(struct ethsw_port_priv *port_priv,
> > return 0;
> > }
> >
> > -static int dpaa2_switch_set_learning(struct ethsw_core *ethsw, bool enable)
> > +static int dpaa2_switch_port_set_learning(struct ethsw_port_priv *port_priv, bool enable)
>
> The commit message says nothing about changes to the learning
> configuration.

The learning flag is per FDB table, thus it's configuration now has to
take into account the corresponding FDB of an interface.

Actually, being able to configure the learning flag is somewhat of an
inconvenience since this would also change the learning behavior of all
the other switch ports under the same bridge, all this without the
bridge actually learning of this change.

I think I will just remove the code that handles changing the learning
status at the moment, until I can make changes in the MC firmware so
that this flag is indeed per port.

>
> > {
> > + u16 fdb_id = dpaa2_switch_port_get_fdb_id(port_priv);
> > + struct ethsw_core *ethsw = port_priv->ethsw_data;
> > enum dpsw_fdb_learning_mode learn_mode;
> > int err;
> >
> > @@ -182,13 +219,12 @@ static int dpaa2_switch_set_learning(struct ethsw_core *ethsw, bool enable)
> > else
> > learn_mode = DPSW_FDB_LEARNING_MODE_DIS;
> >
> > - err = dpsw_fdb_set_learning_mode(ethsw->mc_io, 0, ethsw->dpsw_handle, 0,
> > + err = dpsw_fdb_set_learning_mode(ethsw->mc_io, 0, ethsw->dpsw_handle, fdb_id,
> > learn_mode);
> > if (err) {
> > dev_err(ethsw->dev, "dpsw_fdb_set_learning_mode err %d\n", err);
> > return err;
> > }
> > - ethsw->learning = enable;
> >
> > return 0;
> > }
> > @@ -267,15 +303,17 @@ static int dpaa2_switch_port_fdb_add_uc(struct ethsw_port_priv *port_priv,
> > const unsigned char *addr)
> > {
> > struct dpsw_fdb_unicast_cfg entry = {0};
> > + u16 fdb_id;
> > int err;
> >
> > entry.if_egress = port_priv->idx;
> > entry.type = DPSW_FDB_ENTRY_STATIC;
> > ether_addr_copy(entry.mac_addr, addr);
> >
> > + fdb_id = dpaa2_switch_port_get_fdb_id(port_priv);
> > err = dpsw_fdb_add_unicast(port_priv->ethsw_data->mc_io, 0,
> > port_priv->ethsw_data->dpsw_handle,
> > - 0, &entry);
> > + fdb_id, &entry);
>
> Hmmm, so in dpaa2_switch_port_get_fdb_id you say:
>
> /* If part of a bridge, use the FDB of the first dpaa2 switch interface
> * to be present in that bridge
> */
>
> So let's say there is a br0 with swp3 and swp2, and a br1 with swp4.
> IIUC, br0 interfaces (swp3 and swp2) will have an fdb_id of 3 (due to
> swp3 being added first) and br1 will have an fdb_id of 4 (due to swp4).
>
> When swp3 leaves br0, will this cause the fdb_id of swp2 to change?
> I expect the answer is yes, since otherwise swp2 and swp3 would keep
> forwarding packets to one another. Is this change graceful?
>

Yes, the fdb_id of swp2 will change but, as the code is now, it will
only change for new FDB static entries added or any new VLANs
installed.

> For example, if you add a static FDB entry to swp2 prior to removing
> swp3, I would expect the fdb_id of swp2 to preserve that static FDB
> entry, even if swp2 now gets moved to a different fdb_id. Similarly,
> flooding settings, everything is preserved when the fdb_id changes?
>

No, moving static FDB entries on a bridge leave/join does not happen
now.

> The flip side of that is: what happens if you add an FDB entry to swp2,
> then you remove swp2 from br0 and move it to br1? Will swp4 (which was
> already in br1) see that static FDB entry in hardware, even if the
> software bridge br1 hasn't notified you about it?
>

As you said, moving any FDB entry from one FDB table to another one
would be a problem since the software bridge br1 would not be notified
of any of these new entries.

Taking all the above into account, what I think the code should do if
swp2, for example, leaves a bridge is to:
- update all VLAN entries installed for swp2 so that on a hit it would
lead the packet into the new FDB table.
- all static FDB entries already installed on swp2 should be removed
from the FDB table corresponding to the previous bridge that the port
was under.

> Basically, my question boils down to: why is there so little activity in
> dpaa2_switch_port_bridge_leave.
>
> > if (err)
> > netdev_err(port_priv->netdev,
> > "dpsw_fdb_add_unicast err %d\n", err);