Received-SPF: pass (google.com: domain of linux-kernel+bounces-127226-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Received-SPF: Pass (protection.outlook.com: domain of garmin.com designates
 204.77.163.244 as permitted sender) receiver=protection.outlook.com;
 client-ip=204.77.163.244; helo=edgetransport.garmin.com; pr=C
From: Joseph Huang <Joseph.Huang@garmin.com>
To: <netdev@vger.kernel.org>
CC: Joseph Huang <Joseph.Huang@garmin.com>, Andrew Lunn <andrew@lunn.ch>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Vladimir Oltean <olteanv@gmail.com>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>, "Roopa
 Prabhu" <roopa@nvidia.com>,
        Nikolay Aleksandrov <razor@blackwall.org>,
        =?UTF-8?q?Linus=20L=C3=BCssing?= <linus.luessing@c0d3.blue>,
        <linux-kernel@vger.kernel.org>, <bridge@lists.linux.dev>
Subject: [PATCH RFC net-next 10/10] net: dsa: mv88e6xxx: Offload mrouter port
Date: Mon, 1 Apr 2024 20:11:09 -0400
Message-ID: <20240402001137.2980589-11-Joseph.Huang@garmin.com>
In-Reply-To: <20240402001137.2980589-1-Joseph.Huang@garmin.com>
References: <20240402001137.2980589-1-Joseph.Huang@garmin.com>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Content-Type: text/plain
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 02 Apr 2024 00:12:47.0768
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: acdae5a1-2fdc-4f7c-1289-08dc52a9a049
X-MS-Exchange-CrossTenant-Id: 38d0d425-ba52-4c0a-a03e-2a65c8e82e2d
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=38d0d425-ba52-4c0a-a03e-2a65c8e82e2d;Ip=[204.77.163.244];Helo=[edgetransport.garmin.com]
X-MS-Exchange-CrossTenant-AuthSource: 
	CH3PEPF00000017.namprd21.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Anonymous
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR04MB7744
X-Proofpoint-GUID: z5TMBgS6yqwOLReJtOEv4nMEwdA-lVFd
X-Proofpoint-ORIG-GUID: z5TMBgS6yqwOLReJtOEv4nMEwdA-lVFd
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.1011,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2024-04-01_17,2024-04-01_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 clxscore=1015
 lowpriorityscore=0 bulkscore=0 spamscore=0 suspectscore=0 mlxlogscore=999
 malwarescore=0 mlxscore=0 priorityscore=1501 phishscore=0 adultscore=0
 impostorscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.19.0-2403210001 definitions=main-2404020000

Offload mrouter port forwarding to mv88e6xxx.

Currently multicast snooping fails to forward traffic in some cases
where there are multiple hardware-offloading bridges involved.

Consider the following scenario:

                 +--------------------+
                 |                    |
                 |      Snooping   +--|    +------------+
                 |      Bridge 1   |P1|----| Listener 1 |
                 |     (Querier)   +--|    +------------+
                 |                    |
                 +--------------------+
                           |
                           |
                 +--------------------+
                 |    | mrouter |     |
+-----------+    |    +---------+  +--|    +------------+
| MC Source |----|      Snooping   |P2|----| Listener 2 |
+-----------|    |      Bridge 2   +--|    +------------+
                 |    (Non-Querier)   |
                 +--------------------+

In this scenario, Listener 2 is able to receive multicast traffic from
MC Source while Listener 1 is not. The reason is that on Snooping
Bridge 2, when the (soft) bridge attempts to forward a packet to
the mrouter port via br_multicast_flood(), the effort is blocked by
nbp_switchdev_allowed_egress(), since offload_fwd_mark indicates that
the packet should have been handled by the hardware already. Listener 2
would receive the packets without any problem since P2 is programmed unto
the switch chip as a member of the group; however, the mrouter port
would not since the mrouter port would normally not be a member of any
group, and thus will not be added to the address database on the switch
chip of Snooping Bridge 2.

Even if nbp_switchdev_allowed_egress() did not block the forwarding,
it would still be better to offload the forwarding to the switch
rather than letting the bridge handle the forwarding in software.

Before this patch, mv88e6xxx programming matches exactly with mdb:

 +-----+
 | mdb |
 +-----+
    |
 +----------------------------------------------+
 |  |        +--------------------------------+ |
 |  |        | both in mdb and mv88e6xxx      | |
 |  |        | +------+   +------+   +------+ | |
 |  +--------|-| port |---| port |---| port | | |
 |           | +------+   +------+   +------+ | |
 | mv88e6xxx +--------------------------------+ |
 +----------------------------------------------+

After this patch, some entries will only exist in mv88e6xxx and not
in mdb:

 +-----+
 | mdb |
 +-----+
    |
 +---------------------------------------------------------------------+
 |  |        +--------------------------------++---------------------+ |
 |  |        |  both in mdb and mv88e6xxx     ||  only in mv88e6xxx  | |
 |  |        | +------+   +------+   +------+ || +------+   +------+ | |
 |  +--------|-| port |---| port |---| port |-||-|  mr  |---|  mr  | | |
 |           | +------+   +------+   +------+ || +------+   +------+ | |
 | mv88e6xxx +--------------------------------++---------------------+ |
 +---------------------------------------------------------------------+

Signed-off-by: Joseph Huang <Joseph.Huang@garmin.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 104 ++++++++++++++++++++++++++++---
 1 file changed, 94 insertions(+), 10 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 9831aa370921..ab519e4d9e4f 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -2194,13 +2194,10 @@ mv88e6xxx_port_vlan_prepare(struct dsa_switch *ds, int port,
 	return err;
 }
 
-static int mv88e6xxx_port_db_load_purge(struct mv88e6xxx_chip *chip, int port,
-					const unsigned char *addr, u16 vid,
-					u8 state)
+static int mv88e6xxx_fid_from_vid(struct mv88e6xxx_chip *chip, u16 vid,
+				  u16 *fid)
 {
-	struct mv88e6xxx_atu_entry entry;
 	struct mv88e6xxx_vtu_entry vlan;
-	u16 fid;
 	int err;
 
 	/* Ports have two private address databases: one for when the port is
@@ -2211,7 +2208,7 @@ static int mv88e6xxx_port_db_load_purge(struct mv88e6xxx_chip *chip, int port,
 	 * VLAN ID into the port's database used for VLAN-unaware bridging.
 	 */
 	if (vid == 0) {
-		fid = MV88E6XXX_FID_BRIDGED;
+		*fid = MV88E6XXX_FID_BRIDGED;
 	} else {
 		err = mv88e6xxx_vtu_get(chip, vid, &vlan);
 		if (err)
@@ -2221,9 +2218,24 @@ static int mv88e6xxx_port_db_load_purge(struct mv88e6xxx_chip *chip, int port,
 		if (!vlan.valid)
 			return -EOPNOTSUPP;
 
-		fid = vlan.fid;
+		*fid = vlan.fid;
 	}
 
+	return 0;
+}
+
+static int mv88e6xxx_port_db_load_purge(struct mv88e6xxx_chip *chip, int port,
+					const unsigned char *addr, u16 vid,
+					u8 state)
+{
+	struct mv88e6xxx_atu_entry entry;
+	u16 fid;
+	int err;
+
+	err = mv88e6xxx_fid_from_vid(chip, vid, &fid);
+	if (err)
+		return err;
+
 	entry.state = 0;
 	ether_addr_copy(entry.mac, addr);
 	eth_addr_dec(entry.mac);
@@ -2255,6 +2267,30 @@ static int mv88e6xxx_port_db_load_purge(struct mv88e6xxx_chip *chip, int port,
 	return mv88e6xxx_g1_atu_loadpurge(chip, fid, &entry);
 }
 
+static int mv88e6xxx_port_mdb_load_purge(struct mv88e6xxx_chip *chip,
+					 int portvec,
+					 const unsigned char *addr,
+					 u16 vid)
+{
+	struct mv88e6xxx_atu_entry entry;
+	u16 fid;
+	int err;
+
+	err = mv88e6xxx_fid_from_vid(chip, vid, &fid);
+	if (err)
+		return err;
+
+	memset(&entry, 0, sizeof(entry));
+	ether_addr_copy(entry.mac, addr);
+	entry.portvec = portvec;
+	if (!entry.portvec)
+		entry.state = 0;
+	else
+		entry.state = MV88E6XXX_G1_ATU_DATA_STATE_MC_STATIC;
+
+	return mv88e6xxx_g1_atu_loadpurge(chip, fid, &entry);
+}
+
 static int mv88e6xxx_policy_apply(struct mv88e6xxx_chip *chip, int port,
 				  const struct mv88e6xxx_policy *policy)
 {
@@ -6666,6 +6702,12 @@ static void mv88e6xxx_br_mdb_put(struct mv88e6xxx_br_mdb *mv_br_mdb)
 		mv88e6xxx_br_mdb_destroy(mv_br_mdb);
 }
 
+static u16 mv88e6xxx_br_mdb_portvec_fixup(struct mv88e6xxx_bridge *mv_bridge,
+					  u16 portvec)
+{
+	return portvec ? (portvec | mv_bridge->mrouter_ports) : portvec;
+}
+
 static int mv88e6xxx_port_mdb_add(struct dsa_switch *ds, int port,
 				  const struct switchdev_obj_port_mdb *mdb,
 				  struct dsa_db db)
@@ -6675,6 +6717,7 @@ static int mv88e6xxx_port_mdb_add(struct dsa_switch *ds, int port,
 	struct mv88e6xxx_br_mdb *mv_br_mdb;
 	struct net_device *orig_dev;
 	struct net_device *br_dev;
+	u16 portvec;
 	int err;
 
 	orig_dev = mdb->obj.orig_dev;
@@ -6693,9 +6736,11 @@ static int mv88e6xxx_port_mdb_add(struct dsa_switch *ds, int port,
 	if (mv_br_mdb->portvec & BIT(port))
 		return -EEXIST;
 
+	portvec = mv_br_mdb->portvec | BIT(port);
+	portvec = mv88e6xxx_br_mdb_portvec_fixup(mv_bridge, portvec);
+
 	mv88e6xxx_reg_lock(chip);
-	err = mv88e6xxx_port_db_load_purge(chip, port, mdb->addr, mdb->vid,
-					   MV88E6XXX_G1_ATU_DATA_STATE_MC_STATIC);
+	err = mv88e6xxx_port_mdb_load_purge(chip, portvec, mdb->addr, mdb->vid);
 
 	if (err)
 		goto out;
@@ -6717,6 +6762,7 @@ static int mv88e6xxx_port_mdb_del(struct dsa_switch *ds, int port,
 	struct mv88e6xxx_br_mdb *mv_br_mdb;
 	struct net_device *orig_dev;
 	struct net_device *br_dev;
+	u16 portvec;
 	int err;
 
 	orig_dev = mdb->obj.orig_dev;
@@ -6735,8 +6781,11 @@ static int mv88e6xxx_port_mdb_del(struct dsa_switch *ds, int port,
 	if (!(mv_br_mdb->portvec & BIT(port)))
 		return -ENOENT;
 
+	portvec = mv_br_mdb->portvec & ~BIT(port);
+	portvec = mv88e6xxx_br_mdb_portvec_fixup(mv_bridge, portvec);
+
 	mv88e6xxx_reg_lock(chip);
-	err = mv88e6xxx_port_db_load_purge(chip, port, mdb->addr, mdb->vid, 0);
+	err = mv88e6xxx_port_mdb_load_purge(chip, portvec, mdb->addr, mdb->vid);
 	mv_br_mdb->portvec &= ~BIT(port);
 	mv88e6xxx_br_mdb_put(mv_br_mdb);
 	mv88e6xxx_reg_unlock(chip);
@@ -6921,9 +6970,11 @@ static int mv88e6xxx_port_mrouter(struct dsa_switch *ds, int port,
 {
 	struct mv88e6xxx_chip *chip = ds->priv;
 	struct mv88e6xxx_bridge *mv_bridge;
+	struct mv88e6xxx_br_mdb *mv_br_mdb;
 	struct mv88e6xxx_port *p;
 	bool old_mrouter;
 	bool mc_flood;
+	u16 portvec;
 	int err;
 
 	if (!chip->info->ops->port_set_mcast_flood)
@@ -6949,11 +7000,44 @@ static int mv88e6xxx_port_mrouter(struct dsa_switch *ds, int port,
 			goto out;
 	}
 
+	list_for_each_entry(mv_br_mdb, &mv_bridge->br_mdb_list, head) {
+		portvec = mv_br_mdb->portvec;
+		portvec = mv88e6xxx_br_mdb_portvec_fixup(mv_bridge, portvec);
+
+		if (mrouter)
+			portvec |= BIT(port);
+		else
+			portvec &= ~BIT(port);
+
+		err = mv88e6xxx_port_mdb_load_purge(chip, portvec,
+						    mv_br_mdb->addr,
+						    mv_br_mdb->vid);
+		if (err)
+			goto out_port_mdb_load_purge;
+	}
+
 	if (mrouter)
 		mv_bridge->mrouter_ports |= BIT(port);
 	else
 		mv_bridge->mrouter_ports &= ~BIT(port);
 
+	mv88e6xxx_reg_unlock(chip);
+
+	return 0;
+
+out_port_mdb_load_purge:
+	list_for_each_entry_continue_reverse(mv_br_mdb,
+					     &mv_bridge->br_mdb_list,
+					     head) {
+		portvec = mv_br_mdb->portvec;
+		portvec = mv88e6xxx_br_mdb_portvec_fixup(mv_bridge, portvec);
+		mv88e6xxx_port_mdb_load_purge(chip, portvec,
+					      mv_br_mdb->addr,
+					      mv_br_mdb->vid);
+	}
+
+	if (!mc_flood)
+		chip->info->ops->port_set_mcast_flood(chip, port, old_mrouter);
 out:
 	mv88e6xxx_reg_unlock(chip);
 
-- 
2.17.1