netdev - RE: [PATCH net v5 4/4] net: dsa: microchip: monitor potential faults in half-duplex mode

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]

Date: Tue, 4 Jun 2024 16:48:33 +0000
From: <Woojung.Huh@...rochip.com>
To: <enguerrand.de-ribaucourt@...oirfairelinux.com>, <netdev@...r.kernel.org>
CC: <andrew@...n.ch>, <hkallweit1@...il.com>, <linux@...linux.org.uk>,
	<UNGLinuxDriver@...rochip.com>, <horms@...nel.org>,
	<Tristram.Ha@...rochip.com>, <Arun.Ramadoss@...rochip.com>
Subject: RE: [PATCH net v5 4/4] net: dsa: microchip: monitor potential faults
 in half-duplex mode

Hi Enguerrand,

Looks you are covering Module 17 & 23 in Errata[1].

[1] https://ww1.microchip.com/downloads/aemDocuments/documents/UNG/ProductDocuments/Errata/KSZ9477S-Errata-DS80000754.pdf

> The errata DS80000754 recommends monitoring potential faults in
> half-duplex mode for the KSZ9477 family.
> 
> half-duplex is not very common so I just added a critical message
> when the fault conditions are detected. The switch can be expected
> to be unable to communicate anymore in these states and a software
> reset of the switch would be required which I did not implement.
> 
> Fixes: b987e98e50ab ("dsa: add DSA switch driver for Microchip KSZ9477")
> Signed-off-by: Enguerrand de Ribaucourt <enguerrand.de-
> ribaucourt@...oirfairelinux.com>
...
> +int ksz9477_errata_monitor(struct ksz_device *dev, int port,
> +                          u64 tx_late_col)
> +{
> +       u32 pmavbc;
> +       u8 status;
> +       u16 pqm;
> +       int ret;
> +
> +       ret = ksz_pread8(dev, port, REG_PORT_STATUS_0, &status);
> +       if (ret)
> +               return ret;
> +       if (!((status & PORT_INTF_SPEED_MASK) == PORT_INTF_SPEED_MASK) &&
> +           !(status & PORT_INTF_FULL_DUPLEX)) {
> +               dev_warn_once(dev->dev,
> +                             "Half-duplex detected on port %d, transmission
> halt may occur\n",
> +                             port);
> +               /* Errata DS80000754 recommends monitoring potential faults
> in
> +                * half-duplex mode. The switch might not be able to
> communicate anymore
> +                * in these states.
> +                */
> +               if (tx_late_col != 0) {
> +                       /* Transmission halt with late collisions */
> +                       dev_crit_ratelimited(dev->dev,
> +                                            "TX late collisions detected,
> transmission may be halted on port %d\n",
> +                                            port);
> +               }

This covers method 1 of Module 18.

Even though MIB read is not high bandwidth (every 30sec) work,
however, still looping over all ports.
Can you tighten condition for Method 2 because it happens
when both half-duplex and VLAN are enabled?

Adding condition such as 
	ret = ksz_read8(dev, REG_SW_LUE_CTRL_0, &status)
	if (status & SW_VLAN_ENABLE) {
	....
	}

> +               ret = ksz_pread16(dev, port, REG_PORT_QM_TX_CNT_0__4, &pqm);
> +               if (ret)
> +                       return ret;
> +               ret = ksz_read32(dev, REG_PMAVBC, &pmavbc);
> +               if (ret)
> +                       return ret;
> +               if ((FIELD_GET(PMAVBC_MASK, pmavbc) <= PMAVBC_MIN) ||
> +                   (FIELD_GET(PORT_QM_TX_CNT_M, pqm) >= PORT_QM_TX_CNT_MAX))
> {
> +                       /* Transmission halt with Half-Duplex and VLAN */
> +                       dev_crit_ratelimited(dev->dev,
> +                                            "resources out of limits,
> transmission may be halted\n");
> +               }
> +       }
> +       return ret;
> +}
> +

Thanks
Woojung