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: Wed, 21 Feb 2024 09:31:56 +0000
From: "Madalin Bucur (OSS)" <madalin.bucur@....nxp.com>
To: Vladimir Oltean <vladimir.oltean@....com>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>
CC: "David S. Miller" <davem@...emloft.net>, Eric Dumazet
	<edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
	<pabeni@...hat.com>, Sean Anderson <sean.anderson@...o.com>, Russell King
	<linux@...linux.org.uk>, Zachary Goldstein
	<zachary.goldstein@...current-rt.com>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>
Subject: RE: [PATCH net] net: dpaa: fman_memac: accept phy-interface-type =
 "10gbase-r" in the device tree

> -----Original Message-----
> From: Vladimir Oltean <vladimir.oltean@....com>
> Sent: Wednesday, February 21, 2024 12:35 AM
> To: netdev@...r.kernel.org
> Cc: David S. Miller <davem@...emloft.net>; Eric Dumazet
> <edumazet@...gle.com>; Jakub Kicinski <kuba@...nel.org>; Paolo Abeni
> <pabeni@...hat.com>; Madalin Bucur <madalin.bucur@....com>; Sean
> Anderson <sean.anderson@...o.com>; Russell King <linux@...linux.org.uk>;
> Zachary Goldstein <zachary.goldstein@...current-rt.com>; linux-
> kernel@...r.kernel.org
> Subject: [PATCH net] net: dpaa: fman_memac: accept phy-interface-type =
> "10gbase-r" in the device tree
> 
> Since commit 5d93cfcf7360 ("net: dpaa: Convert to phylink"), we support
> the "10gbase-r" phy-mode through a driver-based conversion of "xgmii",
> but we still don't actually support it when the device tree specifies
> "10gbase-r" proper.
> 
> This is because boards such as LS1046A-RDB do not define pcs-handle-names
> (for whatever reason) in the ethernet@...00 device tree node, and the
> code enters through this code path:
> 
> 	err = of_property_match_string(mac_node, "pcs-handle-names", "xfi");
> 	// code takes neither branch and falls through
> 	if (err >= 0) {
> 		(...)
> 	} else if (err != -EINVAL && err != -ENODATA) {
> 		goto _return_fm_mac_free;
> 	}
> 
> 	(...)
> 
> 	/* For compatibility, if pcs-handle-names is missing, we assume this
> 	 * phy is the first one in pcsphy-handle
> 	 */
> 	err = of_property_match_string(mac_node, "pcs-handle-names",
> "sgmii");
> 	if (err == -EINVAL || err == -ENODATA)
> 		pcs = memac_pcs_create(mac_node, 0); // code takes this
> branch
> 	else if (err < 0)
> 		goto _return_fm_mac_free;
> 	else
> 		pcs = memac_pcs_create(mac_node, err);
> 
> 	// A default PCS is created and saved in "pcs"
> 
> 	// This determination fails and mistakenly saves the default PCS
> 	// memac->sgmii_pcs instead of memac->xfi_pcs, because at this
> 	// stage, mac_dev->phy_if == PHY_INTERFACE_MODE_10GBASER.
> 	if (err && mac_dev->phy_if == PHY_INTERFACE_MODE_XGMII)
> 		memac->xfi_pcs = pcs;
> 	else
> 		memac->sgmii_pcs = pcs;
> 
> In other words, in the absence of pcs-handle-names, the default
> xfi_pcs assignment logic only works when in the device tree we have
> PHY_INTERFACE_MODE_XGMII.
> 
> By reversing the order between the fallback xfi_pcs assignment and the
> "xgmii" overwrite with "10gbase-r", we are able to support both values
> in the device tree, with identical behavior.
> 
> Currently, it is impossible to make the s/xgmii/10gbase-r/ device tree
> conversion, because it would break forward compatibility (new device
> tree with old kernel). The only way to modify existing device trees to
> phy-interface-mode = "10gbase-r" is to fix stable kernels to accept this
> value and handle it properly.
> 
> One reason why the conversion is desirable is because with pre-phylink
> kernels, the Aquantia PHY driver used to warn about the improper use
> of PHY_INTERFACE_MODE_XGMII [1]. It is best to have a single (latest)
> device tree that works with all supported stable kernel versions.
> 
> Note that the blamed commit does not constitute a regression per se.
> Older stable kernels like 6.1 still do not work with "10gbase-r", but
> for a different reason. That is a battle for another time.
> 
> [1] https://lore.kernel.org/netdev/20240214-ls1046-dts-use-10gbase-r-v1-1-
> 8c2d68547393@...current-rt.com/
> 
> Fixes: 5d93cfcf7360 ("net: dpaa: Convert to phylink")
> Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>
> ---
>  .../net/ethernet/freescale/fman/fman_memac.c   | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/fman/fman_memac.c
> b/drivers/net/ethernet/freescale/fman/fman_memac.c
> index e30bf75b1d48..0996759907a8 100644
> --- a/drivers/net/ethernet/freescale/fman/fman_memac.c
> +++ b/drivers/net/ethernet/freescale/fman/fman_memac.c
> @@ -1076,6 +1076,14 @@ int memac_initialization(struct mac_device
> *mac_dev,
>  	unsigned long		 capabilities;
>  	unsigned long		*supported;
> 
> +	/* The internal connection to the serdes is XGMII, but this isn't
> +	 * really correct for the phy mode (which is the external connection).
> +	 * However, this is how all older device trees say that they want
> +	 * 10GBASE-R (aka XFI), so just convert it for them.
> +	 */
> +	if (mac_dev->phy_if == PHY_INTERFACE_MODE_XGMII)
> +		mac_dev->phy_if = PHY_INTERFACE_MODE_10GBASER;
> +
>  	mac_dev->phylink_ops		= &memac_mac_ops;
>  	mac_dev->set_promisc		= memac_set_promiscuous;
>  	mac_dev->change_addr		= memac_modify_mac_address;
> @@ -1142,7 +1150,7 @@ int memac_initialization(struct mac_device *mac_dev,
>  	 * (and therefore that xfi_pcs cannot be set). If we are defaulting to
>  	 * XGMII, assume this is for XFI. Otherwise, assume it is for SGMII.
>  	 */
> -	if (err && mac_dev->phy_if == PHY_INTERFACE_MODE_XGMII)
> +	if (err && mac_dev->phy_if == PHY_INTERFACE_MODE_10GBASER)
>  		memac->xfi_pcs = pcs;
>  	else
>  		memac->sgmii_pcs = pcs;
> @@ -1156,14 +1164,6 @@ int memac_initialization(struct mac_device
> *mac_dev,
>  		goto _return_fm_mac_free;
>  	}
> 
> -	/* The internal connection to the serdes is XGMII, but this isn't
> -	 * really correct for the phy mode (which is the external connection).
> -	 * However, this is how all older device trees say that they want
> -	 * 10GBASE-R (aka XFI), so just convert it for them.
> -	 */
> -	if (mac_dev->phy_if == PHY_INTERFACE_MODE_XGMII)
> -		mac_dev->phy_if = PHY_INTERFACE_MODE_10GBASER;
> -
>  	/* TODO: The following interface modes are supported by (some)
> hardware
>  	 * but not by this driver:
>  	 * - 1000BASE-KX
> --
> 2.34.1

Thank you for the detailed description of the situation.

Acked-by: Madalin Bucur <madalin.bucur@....nxp.com>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ