lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <d2236430-0303-b74c-2b35-99bef4ac30a1@arinc9.com>
Date: Thu, 18 May 2023 13:36:42 +0300
From: Arınç ÜNAL <arinc.unal@...nc9.com>
To: Vladimir Oltean <olteanv@...il.com>
Cc: Frank Wunderlich <frank-w@...lic-files.de>, Felix Fietkau <nbd@....name>,
 netdev <netdev@...r.kernel.org>, erkin.bozoglu@...ont.com,
 Andrew Lunn <andrew@...n.ch>, Florian Fainelli <f.fainelli@...il.com>,
 John Crispin <john@...ozen.org>, Mark Lee <Mark-MC.Lee@...iatek.com>,
 Lorenzo Bianconi <lorenzo@...nel.org>,
 Matthias Brugger <matthias.bgg@...il.com>,
 Landen Chao <Landen.Chao@...iatek.com>, Sean Wang <sean.wang@...iatek.com>,
 DENG Qingfang <dqfext@...il.com>, mithat.guner@...ont.com
Subject: Re: Choose a default DSA CPU port

On 17.05.2023 19:16, Vladimir Oltean wrote:
> On Wed, May 17, 2023 at 07:14:01PM +0300, Arınç ÜNAL wrote:
>> On 17.05.2023 19:10, Vladimir Oltean wrote:
>>> On Tue, May 16, 2023 at 10:29:27PM +0300, Arınç ÜNAL wrote:
>>>> For MT7530, the port to trap the frames to is fixed since CPU_PORT is only
>>>> of 3 bits so only one CPU port can be defined. This means that, in case of
>>>> multiple ports used as CPU ports, any frames set for trapping to CPU port
>>>> will be trapped to the numerically greatest CPU port.
>>>
>>> *that is up
>>
>> Yes, the DSA conduit interface of the CPU port must be up. Otherwise, these
>> frames won't appear anywhere. I should mention this on my patch, thanks.
> 
> Well, mentioning it in the patch or in a comment is likely not going to
> be enough. You likely have to implement ds->ops->master_state_change()
> and update the MT7530_MFC register to a value that is always valid when
> the conduit interfaces come and go.

Thanks for pointing this out. I have done this below and confirm frames are
trapped as expected.

The frames won't necessarily be trapped to the CPU port the user port is
connected to. This operation is only there to make sure the trapped frames
always reach the CPU.

I don't (know how to) check for other conduits being up when changing the
trap port. So if a conduit is set down which results in both conduits being
down, the trap port will still be changed to the other port which is
unnecessary but it doesn't break anything.

Looking forward to your comments.

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index b5c8fdd381e5..55c11633f96f 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -961,11 +961,6 @@ mt753x_cpu_port_enable(struct dsa_switch *ds, int port)
  	mt7530_set(priv, MT753X_MFC, MT753X_BC_FFP(BIT(port)) |
  		   MT753X_UNM_FFP(BIT(port)) | MT753X_UNU_FFP(BIT(port)));
  
-	/* Set CPU port number */
-	if (priv->id == ID_MT7621)
-		mt7530_rmw(priv, MT753X_MFC, MT7530_CPU_MASK, MT7530_CPU_EN |
-			   MT7530_CPU_PORT(port));
-
  	/* Add the CPU port to the CPU port bitmap for MT7531 and switch on
  	 * MT7988 SoC. Any frames set for trapping to CPU port will be trapped
  	 * to the CPU port the user port is connected to.
@@ -2258,6 +2253,10 @@ mt7530_setup(struct dsa_switch *ds)
  			   PVC_EG_TAG(MT7530_VLAN_EG_CONSISTENT));
  	}
  
+	/* Trap BPDUs to the CPU port */
+	mt7530_rmw(priv, MT753X_BPC, MT753X_BPDU_PORT_FW_MASK,
+		   MT753X_BPDU_CPU_ONLY);
+
  	/* Setup VLAN ID 0 for VLAN-unaware bridges */
  	ret = mt7530_setup_vlan0(priv);
  	if (ret)
@@ -2886,6 +2885,50 @@ static const struct phylink_pcs_ops mt7530_pcs_ops = {
  	.pcs_an_restart = mt7530_pcs_an_restart,
  };
  
+static void
+mt753x_master_state_change(struct dsa_switch *ds,
+			   const struct net_device *master,
+			   bool operational)
+{
+	struct mt7530_priv *priv = ds->priv;
+	struct dsa_port *cpu_dp = master->dsa_ptr;
+	unsigned int trap_port;
+
+	/* Set the CPU port to trap frames to for MT7530. There can be only one
+	 * CPU port due to MT7530_CPU_PORT having only 3 bits. Any frames set
+	 * for trapping to CPU port will be trapped to the CPU port connected to
+	 * the most recently set up DSA conduit. If the most recently set up DSA
+	 * conduit is set down, frames will be trapped to the CPU port connected
+	 * to the other DSA conduit.
+	 */
+	if (priv->id == ID_MT7530 || priv->id == ID_MT7621) {
+		trap_port = (mt7530_read(priv, MT753X_MFC) & MT7530_CPU_PORT_MASK) >> 4;
+		dev_info(priv->dev, "trap_port is %d\n", trap_port);
+		if (operational) {
+			dev_info(priv->dev, "the conduit for cpu port %d is up\n", cpu_dp->index);
+
+			/* This check will be unnecessary if we find a way to
+			 * not change the trap port to the other port when a
+			 * conduit is set down which results in both conduits
+			 * being down.
+			 */
+			if (!(cpu_dp->index == trap_port)) {
+				dev_info(priv->dev, "trap to cpu port %d\n", cpu_dp->index);
+				mt7530_set(priv, MT753X_MFC, MT7530_CPU_EN);
+				mt7530_rmw(priv, MT753X_MFC, MT7530_CPU_PORT_MASK, MT7530_CPU_PORT(cpu_dp->index));
+			}
+		} else {
+			if (cpu_dp->index == 5 && trap_port == 5) {
+				dev_info(priv->dev, "the conduit for cpu port 5 is down, trap frames to port 6\n");
+				mt7530_rmw(priv, MT753X_MFC, MT7530_CPU_PORT_MASK, MT7530_CPU_PORT(6));
+			} else if (cpu_dp->index == 6 && trap_port == 6) {
+				dev_info(priv->dev, "the conduit for cpu port 6 is down, trap frames to port 5\n");
+				mt7530_rmw(priv, MT753X_MFC, MT7530_CPU_PORT_MASK, MT7530_CPU_PORT(5));
+			}
+		}
+	}
+}
+
  static int
  mt753x_setup(struct dsa_switch *ds)
  {
@@ -2999,6 +3042,7 @@ const struct dsa_switch_ops mt7530_switch_ops = {
  	.phylink_mac_link_up	= mt753x_phylink_mac_link_up,
  	.get_mac_eee		= mt753x_get_mac_eee,
  	.set_mac_eee		= mt753x_set_mac_eee,
+	.master_state_change	= mt753x_master_state_change,
  };
  EXPORT_SYMBOL_GPL(mt7530_switch_ops);
  
diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index fd2a2f726b8a..2abd3c5ce05a 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -41,8 +41,8 @@ enum mt753x_id {
  #define  MT753X_UNU_FFP(x)		(((x) & 0xff) << 8)
  #define  MT753X_UNU_FFP_MASK		MT753X_UNU_FFP(~0)
  #define  MT7530_CPU_EN			BIT(7)
-#define  MT7530_CPU_PORT(x)		((x) << 4)
-#define  MT7530_CPU_MASK		(0xf << 4)
+#define  MT7530_CPU_PORT(x)		(((x) & 0x7) << 4)
+#define  MT7530_CPU_PORT_MASK		MT7530_CPU_PORT(~0)
  #define  MT7530_MIRROR_EN		BIT(3)
  #define  MT7530_MIRROR_PORT(x)		((x) & 0x7)
  #define  MT7530_MIRROR_MASK		0x7

Arınç

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ