* Re: [net-next RFC PATCH v3 4/4] dt-bindings: Document bindings for Marvell Aquantia PHY
From: Conor Dooley @ 2023-11-03 13:08 UTC (permalink / raw)
To: Christian Marangi
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Andrew Lunn,
Heiner Kallweit, Russell King, Robert Marko, Vladimir Oltean,
netdev, devicetree, linux-kernel
In-Reply-To: <20231102150032.10740-4-ansuelsmth@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2726 bytes --]
Yo,
On Thu, Nov 02, 2023 at 04:00:32PM +0100, Christian Marangi wrote:
> Document bindings for Marvell Aquantia PHY.
>
> The Marvell Aquantia PHY require a firmware to work correctly and there
> at least 3 way to load this firmware.
>
> Describe all the different way and document the binding "firmware-name"
> to load the PHY firmware from userspace.
>
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
> Changes v3:
> - Make DT description more OS agnostic
> - Use custom select to fix dtbs checks
> Changes v2:
> - Add DT patch
>
> .../bindings/net/marvell,aquantia.yaml | 126 ++++++++++++++++++
> 1 file changed, 126 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/marvell,aquantia.yaml
>
> diff --git a/Documentation/devicetree/bindings/net/marvell,aquantia.yaml b/Documentation/devicetree/bindings/net/marvell,aquantia.yaml
> new file mode 100644
> index 000000000000..d43cf28a4d61
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/marvell,aquantia.yaml
> @@ -0,0 +1,126 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/marvell,aquantia.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Marvell Aquantia Ethernet PHY
> +
> +maintainers:
> + - Christian Marangi <ansuelsmth@gmail.com>
> +
> +description: |
> + Marvell Aquantia Ethernet PHY require a firmware to be loaded to actually
> + work.
> +
> + This can be done and is implemented by OEM in 3 different way:
> + - Attached SPI directly to the PHY with the firmware. The PHY will
You a word here? Should that not be "SPI flash"?
> + self load the firmware in the presence of this configuration.
> + - Dedicated partition on system NAND with firmware in it. NVMEM
> + subsystem will be used and the declared NVMEM cell will load
> + the firmware to the PHY using the PHY mailbox interface.
> + - Manually provided firmware loaded from a file in the filesystem.
> +
> + If declared, NVMEM will always take priority over filesystem provided
> + firmware.
This section here reads entirely like "software policy". The first
bullet in your list is fine - as that is what the PHY will do itself.
The second and third bullets here seem like two different ways that
someone could integrate their system, and I am not objecting to either
of those ways of doing things.
The priority system that you mention however I don't think is suitable
for a description of the hardware - the PHY itself doesn't require that
an external-to-it flash device take priority over something in the
filesystem, right?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH] net: phy: at803x: add QCA8084 ethernet phy support
From: Andrew Lunn @ 2023-11-03 13:01 UTC (permalink / raw)
To: Luo Jie
Cc: hkallweit1, linux, davem, edumazet, kuba, pabeni, netdev,
linux-kernel
In-Reply-To: <20231103123538.15735-1-quic_luoj@quicinc.com>
> #define QCA8081_PHY_ID 0x004dd101
> +#define QCA8081_PHY_MASK 0xffffff00
That is an unusual mask. Please check it is correct. All you should
need its PHY_ID_MATCH_EXACT, PHY_ID_MATCH_MODEL, PHY_ID_MATCH_VENDOR.
> @@ -1767,6 +1781,20 @@ static int qca808x_config_init(struct phy_device *phydev)
> {
> int ret;
>
> + if (phydev->phy_id == QCA8084_PHY_ID) {
> + /* Invert ADC clock edge */
> + ret = at803x_debug_reg_mask(phydev, QCA8084_ADC_CLK_SEL,
> + QCA8084_ADC_CLK_SEL_ACLK,
> + FIELD_PREP(QCA8084_ADC_CLK_SEL_ACLK,
> + QCA8084_ADC_CLK_SEL_ACLK_FALL));
> + if (ret < 0)
> + return ret;
> +
> + /* Adjust MSE threshold value to avoid link issue with some link partner */
> + return phy_write_mmd(phydev, MDIO_MMD_PMAPMD,
> + QCA8084_MSE_THRESHOLD, QCA8084_MSE_THRESHOLD_2P5G_VAL);
> + }
> +
Please add a qca8084_config_init() and use that from the phy_driver
structure.
> /* Active adc&vga on 802.3az for the link 1000M and 100M */
> ret = phy_modify_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_ADDR_CLD_CTRL7,
> QCA808X_8023AZ_AFE_CTRL_MASK, QCA808X_8023AZ_AFE_EN);
> @@ -1958,6 +1986,11 @@ static int qca808x_cable_test_start(struct phy_device *phydev)
> phy_write_mmd(phydev, MDIO_MMD_PCS, 0x807a, 0xc060);
> phy_write_mmd(phydev, MDIO_MMD_PCS, 0x807e, 0xb060);
>
> + if (phydev->phy_id == QCA8084_PHY_ID) {
> + phy_write_mmd(phydev, MDIO_MMD_PCS, 0x8075, 0xa060);
> + phy_write_mmd(phydev, MDIO_MMD_PCS, 0x807f, 0x1eb0);
> + }
> +
Please add a comment what this is doing.
> }, {
> /* Qualcomm QCA8081 */
> - PHY_ID_MATCH_EXACT(QCA8081_PHY_ID),
> - .name = "Qualcomm QCA8081",
> + .phy_id = QCA8081_PHY_ID,
> + .phy_id_mask = QCA8081_PHY_MASK,
> + .name = "Qualcomm QCA808X",
Please add a new entry for the 8084.
Andrew
^ permalink raw reply
* Re: [PATCH] Prevent out-of-bounds read/write in bcmasp_netfilt_rd and bcmasp_netfilt_wr
From: Greg KH @ 2023-11-03 12:57 UTC (permalink / raw)
To: Yuran Pereira
Cc: davem, netdev, florian.fainelli, linux-kernel, justin.chen,
edumazet, bcm-kernel-feedback-list, kuba, pabeni,
linux-kernel-mentees
In-Reply-To: <DB3PR10MB6835E073F668AD24F57AE64AE8A5A@DB3PR10MB6835.EURPRD10.PROD.OUTLOOK.COM>
On Fri, Nov 03, 2023 at 05:57:48PM +0530, Yuran Pereira wrote:
> The functions `bcmasp_netfilt_rd` and `bcmasp_netfilt_wr` both call
> `bcmasp_netfilt_get_reg_offset` which, when it fails, returns `-EINVAL`.
> This could lead to an out-of-bounds read or write when `rx_filter_core_rl`
> or `rx_filter_core_wl` is called.
>
> This patch adds a check in both functions to return immediately if
> `bcmasp_netfilt_get_reg_offset` fails. This prevents potential out-of-bounds read
> or writes, and ensures that no undefined or buggy behavior would originate from
> the failure of `bcmasp_netfilt_get_reg_offset`.
>
> Addresses-Coverity-IDs: 1544536 ("Out-of-bounds access")
> Signed-off-by: Yuran Pereira <yuran.pereira@hotmail.com>
> ---
> drivers/net/ethernet/broadcom/asp2/bcmasp.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/net/ethernet/broadcom/asp2/bcmasp.c b/drivers/net/ethernet/broadcom/asp2/bcmasp.c
> index 29b04a274d07..8b90b761bdec 100644
> --- a/drivers/net/ethernet/broadcom/asp2/bcmasp.c
> +++ b/drivers/net/ethernet/broadcom/asp2/bcmasp.c
> @@ -227,6 +227,8 @@ static void bcmasp_netfilt_wr(struct bcmasp_priv *priv,
>
> reg_offset = bcmasp_netfilt_get_reg_offset(priv, nfilt, reg_type,
> offset);
> + if (reg_offset < 0)
> + return;
>
> rx_filter_core_wl(priv, val, reg_offset);
> }
> @@ -244,6 +246,8 @@ static u32 bcmasp_netfilt_rd(struct bcmasp_priv *priv,
>
> reg_offset = bcmasp_netfilt_get_reg_offset(priv, nfilt, reg_type,
> offset);
> + if (reg_offset < 0)
> + return 0;
Shouldn't you return an error here?
thanks
greg k-h
^ permalink raw reply
* [PATCH] net: phy: at803x: add QCA8084 ethernet phy support
From: Luo Jie @ 2023-11-03 12:35 UTC (permalink / raw)
To: andrew, hkallweit1, linux, davem, edumazet, kuba, pabeni
Cc: netdev, linux-kernel
Add qca8084 PHY support, which is four-port PHY with maximum
link capability 2.5G, the features of each port is almost same
as QCA8081 and slave seed config is not needed.
There are some initialization configurations needed.
1. Configuring qca8084 related initializations including
MSE detect threshold and ADC clock edge invert.
2. Add the additional configurations for the CDT feature.
Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
---
drivers/net/phy/at803x.c | 40 +++++++++++++++++++++++++++++++++++++---
1 file changed, 37 insertions(+), 3 deletions(-)
diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index 37fb033e1c29..4124eb76d835 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -176,6 +176,8 @@
#define AT8030_PHY_ID_MASK 0xffffffef
#define QCA8081_PHY_ID 0x004dd101
+#define QCA8081_PHY_MASK 0xffffff00
+#define QCA8084_PHY_ID 0x004dd180
#define QCA8327_A_PHY_ID 0x004dd033
#define QCA8327_B_PHY_ID 0x004dd034
@@ -279,6 +281,15 @@
#define QCA8081_PHY_SERDES_MMD1_FIFO_CTRL 0x9072
#define QCA8081_PHY_FIFO_RSTN BIT(11)
+/* QCA8084 ADC clock edge */
+#define QCA8084_ADC_CLK_SEL 0x8b80
+#define QCA8084_ADC_CLK_SEL_ACLK GENMASK(7, 4)
+#define QCA8084_ADC_CLK_SEL_ACLK_FALL 0xf
+#define QCA8084_ADC_CLK_SEL_ACLK_RISE 0x0
+
+#define QCA8084_MSE_THRESHOLD 0x800a
+#define QCA8084_MSE_THRESHOLD_2P5G_VAL 0x51c6
+
MODULE_DESCRIPTION("Qualcomm Atheros AR803x and QCA808X PHY driver");
MODULE_AUTHOR("Matus Ujhelyi");
MODULE_LICENSE("GPL");
@@ -1760,6 +1771,9 @@ static bool qca808x_is_prefer_master(struct phy_device *phydev)
static bool qca808x_has_fast_retrain_or_slave_seed(struct phy_device *phydev)
{
+ if (phydev->phy_id == QCA8084_PHY_ID)
+ return false;
+
return linkmode_test_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, phydev->supported);
}
@@ -1767,6 +1781,20 @@ static int qca808x_config_init(struct phy_device *phydev)
{
int ret;
+ if (phydev->phy_id == QCA8084_PHY_ID) {
+ /* Invert ADC clock edge */
+ ret = at803x_debug_reg_mask(phydev, QCA8084_ADC_CLK_SEL,
+ QCA8084_ADC_CLK_SEL_ACLK,
+ FIELD_PREP(QCA8084_ADC_CLK_SEL_ACLK,
+ QCA8084_ADC_CLK_SEL_ACLK_FALL));
+ if (ret < 0)
+ return ret;
+
+ /* Adjust MSE threshold value to avoid link issue with some link partner */
+ return phy_write_mmd(phydev, MDIO_MMD_PMAPMD,
+ QCA8084_MSE_THRESHOLD, QCA8084_MSE_THRESHOLD_2P5G_VAL);
+ }
+
/* Active adc&vga on 802.3az for the link 1000M and 100M */
ret = phy_modify_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_ADDR_CLD_CTRL7,
QCA808X_8023AZ_AFE_CTRL_MASK, QCA808X_8023AZ_AFE_EN);
@@ -1958,6 +1986,11 @@ static int qca808x_cable_test_start(struct phy_device *phydev)
phy_write_mmd(phydev, MDIO_MMD_PCS, 0x807a, 0xc060);
phy_write_mmd(phydev, MDIO_MMD_PCS, 0x807e, 0xb060);
+ if (phydev->phy_id == QCA8084_PHY_ID) {
+ phy_write_mmd(phydev, MDIO_MMD_PCS, 0x8075, 0xa060);
+ phy_write_mmd(phydev, MDIO_MMD_PCS, 0x807f, 0x1eb0);
+ }
+
return 0;
}
@@ -2207,8 +2240,9 @@ static struct phy_driver at803x_driver[] = {
.resume = qca83xx_resume,
}, {
/* Qualcomm QCA8081 */
- PHY_ID_MATCH_EXACT(QCA8081_PHY_ID),
- .name = "Qualcomm QCA8081",
+ .phy_id = QCA8081_PHY_ID,
+ .phy_id_mask = QCA8081_PHY_MASK,
+ .name = "Qualcomm QCA808X",
.flags = PHY_POLL_CABLE_TEST,
.probe = at803x_probe,
.config_intr = at803x_config_intr,
@@ -2241,7 +2275,7 @@ static struct mdio_device_id __maybe_unused atheros_tbl[] = {
{ PHY_ID_MATCH_EXACT(QCA8327_A_PHY_ID) },
{ PHY_ID_MATCH_EXACT(QCA8327_B_PHY_ID) },
{ PHY_ID_MATCH_EXACT(QCA9561_PHY_ID) },
- { PHY_ID_MATCH_EXACT(QCA8081_PHY_ID) },
+ { QCA8081_PHY_ID, QCA8081_PHY_MASK},
{ }
};
--
2.42.0
^ permalink raw reply related
* [net-next RFC PATCH v4 4/4] dt-bindings: Document bindings for Marvell Aquantia PHY
From: Christian Marangi @ 2023-11-03 12:35 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Andrew Lunn,
Heiner Kallweit, Russell King, Christian Marangi, Robert Marko,
Vladimir Oltean, netdev, devicetree, linux-kernel
In-Reply-To: <20231103123532.687-1-ansuelsmth@gmail.com>
Document bindings for Marvell Aquantia PHY.
The Marvell Aquantia PHY require a firmware to work correctly and there
at least 3 way to load this firmware.
Describe all the different way and document the binding "firmware-name"
to load the PHY firmware from userspace.
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
Changes v3:
- Make DT description more OS agnostic
- Use custom select to fix dtbs checks
Changes v2:
- Add DT patch
.../bindings/net/marvell,aquantia.yaml | 126 ++++++++++++++++++
1 file changed, 126 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/marvell,aquantia.yaml
diff --git a/Documentation/devicetree/bindings/net/marvell,aquantia.yaml b/Documentation/devicetree/bindings/net/marvell,aquantia.yaml
new file mode 100644
index 000000000000..d43cf28a4d61
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/marvell,aquantia.yaml
@@ -0,0 +1,126 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/marvell,aquantia.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Marvell Aquantia Ethernet PHY
+
+maintainers:
+ - Christian Marangi <ansuelsmth@gmail.com>
+
+description: |
+ Marvell Aquantia Ethernet PHY require a firmware to be loaded to actually
+ work.
+
+ This can be done and is implemented by OEM in 3 different way:
+ - Attached SPI directly to the PHY with the firmware. The PHY will
+ self load the firmware in the presence of this configuration.
+ - Dedicated partition on system NAND with firmware in it. NVMEM
+ subsystem will be used and the declared NVMEM cell will load
+ the firmware to the PHY using the PHY mailbox interface.
+ - Manually provided firmware loaded from a file in the filesystem.
+
+ If declared, NVMEM will always take priority over filesystem provided
+ firmware.
+
+allOf:
+ - $ref: ethernet-phy.yaml#
+
+select:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - ethernet-phy-id03a1.b445
+ - ethernet-phy-id03a1.b460
+ - ethernet-phy-id03a1.b4a2
+ - ethernet-phy-id03a1.b4d0
+ - ethernet-phy-id03a1.b4e0
+ - ethernet-phy-id03a1.b5c2
+ - ethernet-phy-id03a1.b4b0
+ - ethernet-phy-id03a1.b662
+ - ethernet-phy-id03a1.b712
+ - ethernet-phy-id31c3.1c12
+ required:
+ - compatible
+
+properties:
+ reg:
+ maxItems: 1
+
+ firmware-name:
+ description: specify the name of PHY firmware to load
+
+ nvmem-cells:
+ description: phandle to the firmware nvmem cell
+ maxItems: 1
+
+ nvmem-cell-names:
+ const: firmware
+
+required:
+ - compatible
+ - reg
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ mdio {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ethernet-phy@0 {
+ /* Only needed to make DT lint tools work. Do not copy/paste
+ * into real DTS files.
+ */
+ compatible = "ethernet-phy-id31c3.1c12",
+ "ethernet-phy-ieee802.3-c45";
+
+ reg = <0>;
+ firmware-name = "AQR-G4_v5.4.C-AQR_CIG_WF-1945_0x8_ID44776_VER1630.cld";
+ };
+
+ ethernet-phy@1 {
+ /* Only needed to make DT lint tools work. Do not copy/paste
+ * into real DTS files.
+ */
+ compatible = "ethernet-phy-id31c3.1c12",
+ "ethernet-phy-ieee802.3-c45";
+
+ reg = <0>;
+ nvmem-cells = <&aqr_fw>;
+ nvmem-cell-names = "firmware";
+ };
+ };
+
+ flash {
+ compatible = "jedec,spi-nor";
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ partitions {
+ compatible = "fixed-partitions";
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ /* ... */
+
+ partition@650000 {
+ compatible = "nvmem-cells";
+ label = "0:ethphyfw";
+ reg = <0x650000 0x80000>;
+ read-only;
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ aqr_fw: aqr_fw@0 {
+ reg = <0x0 0x5f42a>;
+ };
+ };
+
+ /* ... */
+
+ };
+ };
--
2.40.1
^ permalink raw reply related
* [net-next RFC PATCH v4 3/4] net: phy: aquantia: add firmware load support
From: Christian Marangi @ 2023-11-03 12:35 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Andrew Lunn,
Heiner Kallweit, Russell King, Christian Marangi, Robert Marko,
Vladimir Oltean, netdev, devicetree, linux-kernel
In-Reply-To: <20231103123532.687-1-ansuelsmth@gmail.com>
From: Robert Marko <robimarko@gmail.com>
Aquantia PHY-s require firmware to be loaded before they start operating.
It can be automatically loaded in case when there is a SPI-NOR connected
to Aquantia PHY-s or can be loaded from the host via MDIO.
This patch adds support for loading the firmware via MDIO as in most cases
there is no SPI-NOR being used to save on cost.
Firmware loading code itself is ported from mainline U-boot with cleanups.
The firmware has mixed values both in big and little endian.
PHY core itself is big-endian but it expects values to be in little-endian.
The firmware is little-endian but CRC-16 value for it is stored at the end
of firmware in big-endian.
It seems the PHY does the conversion internally from firmware that is
little-endian to the PHY that is big-endian on using the mailbox
but mailbox returns a big-endian CRC-16 to verify the written data
integrity.
Co-developed-by: Christian Marangi <ansuelsmth@gmail.com>
Signed-off-by: Robert Marko <robimarko@gmail.com>
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
Changes v4:
- Drop inline
- Drop extra impossible check for negative values
Changes v3:
- Back to RFC due to merge window
- Use unaligned macro instead of memcpy
- Spam sanity check as firmware is evil
- Add print to signal the user the source of the fw (FS or NVMEM)
Changes v2:
- Move out of RFC
- Address sanity check for offsets
- Add additional comments on firmware load check
- Fix some typo
- Capitalize CRC in comments
- Rename load_sysfs to load_fs
drivers/net/phy/aquantia/Kconfig | 1 +
drivers/net/phy/aquantia/Makefile | 2 +-
drivers/net/phy/aquantia/aquantia.h | 32 ++
drivers/net/phy/aquantia/aquantia_firmware.c | 367 +++++++++++++++++++
drivers/net/phy/aquantia/aquantia_main.c | 6 +
5 files changed, 407 insertions(+), 1 deletion(-)
create mode 100644 drivers/net/phy/aquantia/aquantia_firmware.c
diff --git a/drivers/net/phy/aquantia/Kconfig b/drivers/net/phy/aquantia/Kconfig
index 226146417a6a..a35de4b9b554 100644
--- a/drivers/net/phy/aquantia/Kconfig
+++ b/drivers/net/phy/aquantia/Kconfig
@@ -1,5 +1,6 @@
# SPDX-License-Identifier: GPL-2.0-only
config AQUANTIA_PHY
tristate "Aquantia PHYs"
+ select CRC_CCITT
help
Currently supports the Aquantia AQ1202, AQ2104, AQR105, AQR405
diff --git a/drivers/net/phy/aquantia/Makefile b/drivers/net/phy/aquantia/Makefile
index 346f350bc084..aa77fb63c8ec 100644
--- a/drivers/net/phy/aquantia/Makefile
+++ b/drivers/net/phy/aquantia/Makefile
@@ -1,5 +1,5 @@
# SPDX-License-Identifier: GPL-2.0
-aquantia-objs += aquantia_main.o
+aquantia-objs += aquantia_main.o aquantia_firmware.o
ifdef CONFIG_HWMON
aquantia-objs += aquantia_hwmon.o
endif
diff --git a/drivers/net/phy/aquantia/aquantia.h b/drivers/net/phy/aquantia/aquantia.h
index f0c767c4fad1..9ed38972abdb 100644
--- a/drivers/net/phy/aquantia/aquantia.h
+++ b/drivers/net/phy/aquantia/aquantia.h
@@ -10,10 +10,35 @@
#include <linux/phy.h>
/* Vendor specific 1, MDIO_MMD_VEND1 */
+#define VEND1_GLOBAL_SC 0x0
+#define VEND1_GLOBAL_SC_SOFT_RESET BIT(15)
+#define VEND1_GLOBAL_SC_LOW_POWER BIT(11)
+
#define VEND1_GLOBAL_FW_ID 0x0020
#define VEND1_GLOBAL_FW_ID_MAJOR GENMASK(15, 8)
#define VEND1_GLOBAL_FW_ID_MINOR GENMASK(7, 0)
+#define VEND1_GLOBAL_MAILBOX_INTERFACE1 0x0200
+#define VEND1_GLOBAL_MAILBOX_INTERFACE1_EXECUTE BIT(15)
+#define VEND1_GLOBAL_MAILBOX_INTERFACE1_WRITE BIT(14)
+#define VEND1_GLOBAL_MAILBOX_INTERFACE1_CRC_RESET BIT(12)
+#define VEND1_GLOBAL_MAILBOX_INTERFACE1_BUSY BIT(8)
+
+#define VEND1_GLOBAL_MAILBOX_INTERFACE2 0x0201
+#define VEND1_GLOBAL_MAILBOX_INTERFACE3 0x0202
+#define VEND1_GLOBAL_MAILBOX_INTERFACE3_MSW_ADDR_MASK GENMASK(15, 0)
+#define VEND1_GLOBAL_MAILBOX_INTERFACE3_MSW_ADDR(x) FIELD_PREP(VEND1_GLOBAL_MAILBOX_INTERFACE3_MSW_ADDR_MASK, (u16)((x) >> 16))
+#define VEND1_GLOBAL_MAILBOX_INTERFACE4 0x0203
+#define VEND1_GLOBAL_MAILBOX_INTERFACE4_LSW_ADDR_MASK GENMASK(15, 2)
+#define VEND1_GLOBAL_MAILBOX_INTERFACE4_LSW_ADDR(x) FIELD_PREP(VEND1_GLOBAL_MAILBOX_INTERFACE4_LSW_ADDR_MASK, (u16)(x))
+
+#define VEND1_GLOBAL_MAILBOX_INTERFACE5 0x0204
+#define VEND1_GLOBAL_MAILBOX_INTERFACE5_MSW_DATA_MASK GENMASK(15, 0)
+#define VEND1_GLOBAL_MAILBOX_INTERFACE5_MSW_DATA(x) FIELD_PREP(VEND1_GLOBAL_MAILBOX_INTERFACE5_MSW_DATA_MASK, (u16)((x) >> 16))
+#define VEND1_GLOBAL_MAILBOX_INTERFACE6 0x0205
+#define VEND1_GLOBAL_MAILBOX_INTERFACE6_LSW_DATA_MASK GENMASK(15, 0)
+#define VEND1_GLOBAL_MAILBOX_INTERFACE6_LSW_DATA(x) FIELD_PREP(VEND1_GLOBAL_MAILBOX_INTERFACE6_LSW_DATA_MASK, (u16)(x))
+
/* The following registers all have similar layouts; first the registers... */
#define VEND1_GLOBAL_CFG_10M 0x0310
#define VEND1_GLOBAL_CFG_100M 0x031b
@@ -28,6 +53,11 @@
#define VEND1_GLOBAL_CFG_RATE_ADAPT_PAUSE 2
/* Vendor specific 1, MDIO_MMD_VEND2 */
+#define VEND1_GLOBAL_CONTROL2 0xc001
+#define VEND1_GLOBAL_CONTROL2_UP_RUN_STALL_RST BIT(15)
+#define VEND1_GLOBAL_CONTROL2_UP_RUN_STALL_OVD BIT(6)
+#define VEND1_GLOBAL_CONTROL2_UP_RUN_STALL BIT(0)
+
#define VEND1_THERMAL_PROV_HIGH_TEMP_FAIL 0xc421
#define VEND1_THERMAL_PROV_LOW_TEMP_FAIL 0xc422
#define VEND1_THERMAL_PROV_HIGH_TEMP_WARN 0xc423
@@ -83,3 +113,5 @@ int aqr_hwmon_probe(struct phy_device *phydev);
#else
static inline int aqr_hwmon_probe(struct phy_device *phydev) { return 0; }
#endif
+
+int aqr_firmware_load(struct phy_device *phydev);
diff --git a/drivers/net/phy/aquantia/aquantia_firmware.c b/drivers/net/phy/aquantia/aquantia_firmware.c
new file mode 100644
index 000000000000..0267ef2a231a
--- /dev/null
+++ b/drivers/net/phy/aquantia/aquantia_firmware.c
@@ -0,0 +1,367 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/bitfield.h>
+#include <linux/of.h>
+#include <linux/firmware.h>
+#include <linux/crc-ccitt.h>
+#include <linux/nvmem-consumer.h>
+
+#include <asm/unaligned.h>
+
+#include "aquantia.h"
+
+#define UP_RESET_SLEEP 100
+
+/* addresses of memory segments in the phy */
+#define DRAM_BASE_ADDR 0x3FFE0000
+#define IRAM_BASE_ADDR 0x40000000
+
+/* firmware image format constants */
+#define VERSION_STRING_SIZE 0x40
+#define VERSION_STRING_OFFSET 0x0200
+/* primary offset is written at an offset from the start of the fw blob */
+#define PRIMARY_OFFSET_OFFSET 0x8
+/* primary offset needs to be then added to a base offset */
+#define PRIMARY_OFFSET_SHIFT 12
+#define PRIMARY_OFFSET(x) ((x) << PRIMARY_OFFSET_SHIFT)
+#define HEADER_OFFSET 0x300
+
+struct aqr_fw_header {
+ u32 padding;
+ u8 iram_offset[3];
+ u8 iram_size[3];
+ u8 dram_offset[3];
+ u8 dram_size[3];
+} __packed;
+
+enum aqr_fw_src {
+ AQR_FW_SRC_NVMEM = 0,
+ AQR_FW_SRC_FS,
+};
+
+static const char * const aqr_fw_src_string[] = {
+ [AQR_FW_SRC_NVMEM] = "NVMEM",
+ [AQR_FW_SRC_FS] = "FS",
+};
+
+/* AQR firmware doesn't have fixed offsets for iram and dram section
+ * but instead provide an header with the offset to use on reading
+ * and parsing the firmware.
+ *
+ * AQR firmware can't be trusted and each offset is validated to be
+ * not negative and be in the size of the firmware itself.
+ */
+static bool aqr_fw_validate_get(size_t size, size_t offset, size_t get_size)
+{
+ return offset + get_size <= size;
+}
+
+static int aqr_fw_get_be16(const u8 *data, size_t offset, size_t size, u16 *value)
+{
+ if (!aqr_fw_validate_get(size, offset, sizeof(u16)))
+ return -EINVAL;
+
+ *value = get_unaligned_be16(data + offset);
+
+ return 0;
+}
+
+static int aqr_fw_get_le16(const u8 *data, size_t offset, size_t size, u16 *value)
+{
+ if (!aqr_fw_validate_get(size, offset, sizeof(u16)))
+ return -EINVAL;
+
+ *value = get_unaligned_le16(data + offset);
+
+ return 0;
+}
+
+static int aqr_fw_get_le24(const u8 *data, size_t offset, size_t size, u32 *value)
+{
+ if (!aqr_fw_validate_get(size, offset, sizeof(u8) * 3))
+ return -EINVAL;
+
+ *value = get_unaligned_le24(data + offset);
+
+ return 0;
+}
+
+/* load data into the phy's memory */
+static int aqr_fw_load_memory(struct phy_device *phydev, u32 addr,
+ const u8 *data, size_t len)
+{
+ u16 crc = 0, up_crc;
+ size_t pos;
+
+ /* PHY expect addr in LE */
+ addr = cpu_to_le32(addr);
+
+ phy_write_mmd(phydev, MDIO_MMD_VEND1,
+ VEND1_GLOBAL_MAILBOX_INTERFACE1,
+ VEND1_GLOBAL_MAILBOX_INTERFACE1_CRC_RESET);
+ phy_write_mmd(phydev, MDIO_MMD_VEND1,
+ VEND1_GLOBAL_MAILBOX_INTERFACE3,
+ VEND1_GLOBAL_MAILBOX_INTERFACE3_MSW_ADDR(addr));
+ phy_write_mmd(phydev, MDIO_MMD_VEND1,
+ VEND1_GLOBAL_MAILBOX_INTERFACE4,
+ VEND1_GLOBAL_MAILBOX_INTERFACE4_LSW_ADDR(addr));
+
+ /* We assume and enforce the size to be word aligned.
+ * If a firmware that is not word aligned is found, please report upstream.
+ */
+ for (pos = 0; pos < len; pos += sizeof(u32)) {
+ u32 word = get_unaligned((const u32 *)(data + pos));
+
+ phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLOBAL_MAILBOX_INTERFACE5,
+ VEND1_GLOBAL_MAILBOX_INTERFACE5_MSW_DATA(word));
+ phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLOBAL_MAILBOX_INTERFACE6,
+ VEND1_GLOBAL_MAILBOX_INTERFACE6_LSW_DATA(word));
+
+ phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLOBAL_MAILBOX_INTERFACE1,
+ VEND1_GLOBAL_MAILBOX_INTERFACE1_EXECUTE |
+ VEND1_GLOBAL_MAILBOX_INTERFACE1_WRITE);
+
+ /* calculate CRC as we load data to the mailbox.
+ * We convert word to big-endiang as PHY is BE and mailbox will
+ * return a BE CRC.
+ */
+ word = cpu_to_be32(word);
+ crc = crc_ccitt_false(crc, (u8 *)&word, sizeof(word));
+ }
+
+ up_crc = phy_read_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLOBAL_MAILBOX_INTERFACE2);
+ if (crc != up_crc) {
+ phydev_err(phydev, "CRC mismatch: calculated 0x%04x PHY 0x%04x\n",
+ crc, up_crc);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int aqr_fw_boot(struct phy_device *phydev, const u8 *data, size_t size,
+ enum aqr_fw_src fw_src)
+{
+ u16 calculated_crc, read_crc, read_primary_offset;
+ u32 iram_offset = 0, iram_size = 0;
+ u32 dram_offset = 0, dram_size = 0;
+ char version[VERSION_STRING_SIZE];
+ u32 primary_offset = 0;
+ int ret;
+
+ /* extract saved CRC at the end of the fw
+ * CRC is saved in big-endian as PHY is BE
+ */
+ ret = aqr_fw_get_be16(data, size - sizeof(u16), size, &read_crc);
+ if (ret) {
+ phydev_err(phydev, "bad firmware CRC in firmware\n");
+ return ret;
+ }
+ calculated_crc = crc_ccitt_false(0, data, size - sizeof(u16));
+ if (read_crc != calculated_crc) {
+ phydev_err(phydev, "bad firmware CRC: file 0x%04x calculated 0x%04x\n",
+ read_crc, calculated_crc);
+ return -EINVAL;
+ }
+
+ /* Get the primary offset to extract DRAM and IRAM sections. */
+ ret = aqr_fw_get_le16(data, PRIMARY_OFFSET_OFFSET, size, &read_primary_offset);
+ if (ret) {
+ phydev_err(phydev, "bad primary offset in firmware\n");
+ return ret;
+ }
+ primary_offset = PRIMARY_OFFSET(read_primary_offset);
+
+ /* Find the DRAM and IRAM sections within the firmware file.
+ * Make sure the fw_header is correctly in the firmware.
+ */
+ if (!aqr_fw_validate_get(size, primary_offset + HEADER_OFFSET,
+ sizeof(struct aqr_fw_header))) {
+ phydev_err(phydev, "bad fw_header in firmware\n");
+ return -EINVAL;
+ }
+
+ /* offset are in LE and values needs to be converted to cpu endian */
+ ret = aqr_fw_get_le24(data, primary_offset + HEADER_OFFSET +
+ offsetof(struct aqr_fw_header, iram_offset),
+ size, &iram_offset);
+ if (ret) {
+ phydev_err(phydev, "bad iram offset in firmware\n");
+ return ret;
+ }
+ ret = aqr_fw_get_le24(data, primary_offset + HEADER_OFFSET +
+ offsetof(struct aqr_fw_header, iram_size),
+ size, &iram_size);
+ if (ret) {
+ phydev_err(phydev, "invalid iram size in firmware\n");
+ return ret;
+ }
+ ret = aqr_fw_get_le24(data, primary_offset + HEADER_OFFSET +
+ offsetof(struct aqr_fw_header, dram_offset),
+ size, &dram_offset);
+ if (ret) {
+ phydev_err(phydev, "bad dram offset in firmware\n");
+ return ret;
+ }
+ ret = aqr_fw_get_le24(data, primary_offset + HEADER_OFFSET +
+ offsetof(struct aqr_fw_header, dram_size),
+ size, &dram_size);
+ if (ret) {
+ phydev_err(phydev, "invalid dram size in firmware\n");
+ return ret;
+ }
+
+ /* Increment the offset with the primary offset.
+ * Validate iram/dram offset and size.
+ */
+ iram_offset += primary_offset;
+ if (iram_size % sizeof(u32)) {
+ phydev_err(phydev, "iram size if not aligned to word size. Please report this upstream!\n");
+ return -EINVAL;
+ }
+ if (!aqr_fw_validate_get(size, iram_offset, iram_size)) {
+ phydev_err(phydev, "invalid iram offset for iram size\n");
+ return -EINVAL;
+ }
+
+ dram_offset += primary_offset;
+ if (dram_size % sizeof(u32)) {
+ phydev_err(phydev, "dram size if not aligned to word size. Please report this upstream!\n");
+ return -EINVAL;
+ }
+ if (!aqr_fw_validate_get(size, dram_offset, dram_size)) {
+ phydev_err(phydev, "invalid iram offset for iram size\n");
+ return -EINVAL;
+ }
+
+ phydev_dbg(phydev, "primary %d IRAM offset=%d size=%d DRAM offset=%d size=%d\n",
+ primary_offset, iram_offset, iram_size, dram_offset, dram_size);
+
+ if (!aqr_fw_validate_get(size, dram_offset + VERSION_STRING_OFFSET,
+ VERSION_STRING_SIZE)) {
+ phydev_err(phydev, "invalid version in firmware\n");
+ return -EINVAL;
+ }
+ strscpy(version, (char *)data + dram_offset + VERSION_STRING_OFFSET,
+ VERSION_STRING_SIZE);
+ if (version[0] == '\0') {
+ phydev_err(phydev, "invalid version in firmware\n");
+ return -EINVAL;
+ }
+ phydev_info(phydev, "loading firmware version '%s' from '%s'\n", version,
+ aqr_fw_src_string[fw_src]);
+
+ /* stall the microcprocessor */
+ phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLOBAL_CONTROL2,
+ VEND1_GLOBAL_CONTROL2_UP_RUN_STALL | VEND1_GLOBAL_CONTROL2_UP_RUN_STALL_OVD);
+
+ phydev_dbg(phydev, "loading DRAM 0x%08x from offset=%d size=%d\n",
+ DRAM_BASE_ADDR, dram_offset, dram_size);
+ ret = aqr_fw_load_memory(phydev, DRAM_BASE_ADDR, data + dram_offset,
+ dram_size);
+ if (ret)
+ return ret;
+
+ phydev_dbg(phydev, "loading IRAM 0x%08x from offset=%d size=%d\n",
+ IRAM_BASE_ADDR, iram_offset, iram_size);
+ ret = aqr_fw_load_memory(phydev, IRAM_BASE_ADDR, data + iram_offset,
+ iram_size);
+ if (ret)
+ return ret;
+
+ /* make sure soft reset and low power mode are clear */
+ phy_clear_bits_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLOBAL_SC,
+ VEND1_GLOBAL_SC_SOFT_RESET | VEND1_GLOBAL_SC_LOW_POWER);
+
+ /* Release the microprocessor. UP_RESET must be held for 100 usec. */
+ phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLOBAL_CONTROL2,
+ VEND1_GLOBAL_CONTROL2_UP_RUN_STALL |
+ VEND1_GLOBAL_CONTROL2_UP_RUN_STALL_OVD |
+ VEND1_GLOBAL_CONTROL2_UP_RUN_STALL_RST);
+ usleep_range(UP_RESET_SLEEP, UP_RESET_SLEEP * 2);
+
+ phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLOBAL_CONTROL2,
+ VEND1_GLOBAL_CONTROL2_UP_RUN_STALL_OVD);
+
+ return 0;
+}
+
+static int aqr_firmware_load_nvmem(struct phy_device *phydev)
+{
+ struct nvmem_cell *cell;
+ size_t size;
+ u8 *buf;
+ int ret;
+
+ cell = nvmem_cell_get(&phydev->mdio.dev, "firmware");
+ if (IS_ERR(cell))
+ return PTR_ERR(cell);
+
+ buf = nvmem_cell_read(cell, &size);
+ if (IS_ERR(buf)) {
+ ret = PTR_ERR(buf);
+ goto exit;
+ }
+
+ ret = aqr_fw_boot(phydev, buf, size, AQR_FW_SRC_NVMEM);
+ if (ret)
+ phydev_err(phydev, "firmware loading failed: %d\n", ret);
+
+exit:
+ nvmem_cell_put(cell);
+
+ return ret;
+}
+
+static int aqr_firmware_load_fs(struct phy_device *phydev)
+{
+ struct device *dev = &phydev->mdio.dev;
+ const struct firmware *fw;
+ const char *fw_name;
+ int ret;
+
+ ret = of_property_read_string(dev->of_node, "firmware-name",
+ &fw_name);
+ if (ret)
+ return ret;
+
+ ret = request_firmware(&fw, fw_name, dev);
+ if (ret) {
+ phydev_err(phydev, "failed to find FW file %s (%d)\n",
+ fw_name, ret);
+ goto exit;
+ }
+
+ ret = aqr_fw_boot(phydev, fw->data, fw->size, AQR_FW_SRC_FS);
+ if (ret)
+ phydev_err(phydev, "firmware loading failed: %d\n", ret);
+
+exit:
+ release_firmware(fw);
+
+ return ret;
+}
+
+int aqr_firmware_load(struct phy_device *phydev)
+{
+ int ret;
+
+ /* Check if the firmware is not already loaded by pooling
+ * the current version returned by the PHY. If 0 is returned,
+ * no firmware is loaded.
+ */
+ ret = phy_read_mmd(phydev, MDIO_MMD_VEND1, VEND1_GLOBAL_FW_ID);
+ if (ret > 0)
+ goto exit;
+
+ ret = aqr_firmware_load_nvmem(phydev);
+ if (!ret)
+ goto exit;
+
+ ret = aqr_firmware_load_fs(phydev);
+ if (ret)
+ return ret;
+
+exit:
+ return 0;
+}
diff --git a/drivers/net/phy/aquantia/aquantia_main.c b/drivers/net/phy/aquantia/aquantia_main.c
index 4498426e9a52..cc4a97741c4a 100644
--- a/drivers/net/phy/aquantia/aquantia_main.c
+++ b/drivers/net/phy/aquantia/aquantia_main.c
@@ -658,11 +658,17 @@ static int aqr107_resume(struct phy_device *phydev)
static int aqr107_probe(struct phy_device *phydev)
{
+ int ret;
+
phydev->priv = devm_kzalloc(&phydev->mdio.dev,
sizeof(struct aqr107_priv), GFP_KERNEL);
if (!phydev->priv)
return -ENOMEM;
+ ret = aqr_firmware_load(phydev);
+ if (ret)
+ return ret;
+
return aqr_hwmon_probe(phydev);
}
--
2.40.1
^ permalink raw reply related
* [net-next RFC PATCH v4 2/4] net: phy: aquantia: move MMD_VEND define to header
From: Christian Marangi @ 2023-11-03 12:35 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Andrew Lunn,
Heiner Kallweit, Russell King, Christian Marangi, Robert Marko,
Vladimir Oltean, netdev, devicetree, linux-kernel
In-Reply-To: <20231103123532.687-1-ansuelsmth@gmail.com>
Move MMD_VEND define to header to clean things up and in preparation for
firmware loading support that require some define placed in
aquantia_main.
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
Changes v4:
- Add Reviewed-by tag
Changes v3:
- Add this patch
drivers/net/phy/aquantia/aquantia.h | 69 +++++++++++++++++++++++
drivers/net/phy/aquantia/aquantia_hwmon.c | 14 -----
drivers/net/phy/aquantia/aquantia_main.c | 55 ------------------
3 files changed, 69 insertions(+), 69 deletions(-)
diff --git a/drivers/net/phy/aquantia/aquantia.h b/drivers/net/phy/aquantia/aquantia.h
index c684b65c642c..f0c767c4fad1 100644
--- a/drivers/net/phy/aquantia/aquantia.h
+++ b/drivers/net/phy/aquantia/aquantia.h
@@ -9,6 +9,75 @@
#include <linux/device.h>
#include <linux/phy.h>
+/* Vendor specific 1, MDIO_MMD_VEND1 */
+#define VEND1_GLOBAL_FW_ID 0x0020
+#define VEND1_GLOBAL_FW_ID_MAJOR GENMASK(15, 8)
+#define VEND1_GLOBAL_FW_ID_MINOR GENMASK(7, 0)
+
+/* The following registers all have similar layouts; first the registers... */
+#define VEND1_GLOBAL_CFG_10M 0x0310
+#define VEND1_GLOBAL_CFG_100M 0x031b
+#define VEND1_GLOBAL_CFG_1G 0x031c
+#define VEND1_GLOBAL_CFG_2_5G 0x031d
+#define VEND1_GLOBAL_CFG_5G 0x031e
+#define VEND1_GLOBAL_CFG_10G 0x031f
+/* ...and now the fields */
+#define VEND1_GLOBAL_CFG_RATE_ADAPT GENMASK(8, 7)
+#define VEND1_GLOBAL_CFG_RATE_ADAPT_NONE 0
+#define VEND1_GLOBAL_CFG_RATE_ADAPT_USX 1
+#define VEND1_GLOBAL_CFG_RATE_ADAPT_PAUSE 2
+
+/* Vendor specific 1, MDIO_MMD_VEND2 */
+#define VEND1_THERMAL_PROV_HIGH_TEMP_FAIL 0xc421
+#define VEND1_THERMAL_PROV_LOW_TEMP_FAIL 0xc422
+#define VEND1_THERMAL_PROV_HIGH_TEMP_WARN 0xc423
+#define VEND1_THERMAL_PROV_LOW_TEMP_WARN 0xc424
+#define VEND1_THERMAL_STAT1 0xc820
+#define VEND1_THERMAL_STAT2 0xc821
+#define VEND1_THERMAL_STAT2_VALID BIT(0)
+#define VEND1_GENERAL_STAT1 0xc830
+#define VEND1_GENERAL_STAT1_HIGH_TEMP_FAIL BIT(14)
+#define VEND1_GENERAL_STAT1_LOW_TEMP_FAIL BIT(13)
+#define VEND1_GENERAL_STAT1_HIGH_TEMP_WARN BIT(12)
+#define VEND1_GENERAL_STAT1_LOW_TEMP_WARN BIT(11)
+
+#define VEND1_GLOBAL_GEN_STAT2 0xc831
+#define VEND1_GLOBAL_GEN_STAT2_OP_IN_PROG BIT(15)
+
+#define VEND1_GLOBAL_RSVD_STAT1 0xc885
+#define VEND1_GLOBAL_RSVD_STAT1_FW_BUILD_ID GENMASK(7, 4)
+#define VEND1_GLOBAL_RSVD_STAT1_PROV_ID GENMASK(3, 0)
+
+#define VEND1_GLOBAL_RSVD_STAT9 0xc88d
+#define VEND1_GLOBAL_RSVD_STAT9_MODE GENMASK(7, 0)
+#define VEND1_GLOBAL_RSVD_STAT9_1000BT2 0x23
+
+#define VEND1_GLOBAL_INT_STD_STATUS 0xfc00
+#define VEND1_GLOBAL_INT_VEND_STATUS 0xfc01
+
+#define VEND1_GLOBAL_INT_STD_MASK 0xff00
+#define VEND1_GLOBAL_INT_STD_MASK_PMA1 BIT(15)
+#define VEND1_GLOBAL_INT_STD_MASK_PMA2 BIT(14)
+#define VEND1_GLOBAL_INT_STD_MASK_PCS1 BIT(13)
+#define VEND1_GLOBAL_INT_STD_MASK_PCS2 BIT(12)
+#define VEND1_GLOBAL_INT_STD_MASK_PCS3 BIT(11)
+#define VEND1_GLOBAL_INT_STD_MASK_PHY_XS1 BIT(10)
+#define VEND1_GLOBAL_INT_STD_MASK_PHY_XS2 BIT(9)
+#define VEND1_GLOBAL_INT_STD_MASK_AN1 BIT(8)
+#define VEND1_GLOBAL_INT_STD_MASK_AN2 BIT(7)
+#define VEND1_GLOBAL_INT_STD_MASK_GBE BIT(6)
+#define VEND1_GLOBAL_INT_STD_MASK_ALL BIT(0)
+
+#define VEND1_GLOBAL_INT_VEND_MASK 0xff01
+#define VEND1_GLOBAL_INT_VEND_MASK_PMA BIT(15)
+#define VEND1_GLOBAL_INT_VEND_MASK_PCS BIT(14)
+#define VEND1_GLOBAL_INT_VEND_MASK_PHY_XS BIT(13)
+#define VEND1_GLOBAL_INT_VEND_MASK_AN BIT(12)
+#define VEND1_GLOBAL_INT_VEND_MASK_GBE BIT(11)
+#define VEND1_GLOBAL_INT_VEND_MASK_GLOBAL1 BIT(2)
+#define VEND1_GLOBAL_INT_VEND_MASK_GLOBAL2 BIT(1)
+#define VEND1_GLOBAL_INT_VEND_MASK_GLOBAL3 BIT(0)
+
#if IS_REACHABLE(CONFIG_HWMON)
int aqr_hwmon_probe(struct phy_device *phydev);
#else
diff --git a/drivers/net/phy/aquantia/aquantia_hwmon.c b/drivers/net/phy/aquantia/aquantia_hwmon.c
index 0da451e46f69..7b3c49c3bf49 100644
--- a/drivers/net/phy/aquantia/aquantia_hwmon.c
+++ b/drivers/net/phy/aquantia/aquantia_hwmon.c
@@ -13,20 +13,6 @@
#include "aquantia.h"
-/* Vendor specific 1, MDIO_MMD_VEND2 */
-#define VEND1_THERMAL_PROV_HIGH_TEMP_FAIL 0xc421
-#define VEND1_THERMAL_PROV_LOW_TEMP_FAIL 0xc422
-#define VEND1_THERMAL_PROV_HIGH_TEMP_WARN 0xc423
-#define VEND1_THERMAL_PROV_LOW_TEMP_WARN 0xc424
-#define VEND1_THERMAL_STAT1 0xc820
-#define VEND1_THERMAL_STAT2 0xc821
-#define VEND1_THERMAL_STAT2_VALID BIT(0)
-#define VEND1_GENERAL_STAT1 0xc830
-#define VEND1_GENERAL_STAT1_HIGH_TEMP_FAIL BIT(14)
-#define VEND1_GENERAL_STAT1_LOW_TEMP_FAIL BIT(13)
-#define VEND1_GENERAL_STAT1_HIGH_TEMP_WARN BIT(12)
-#define VEND1_GENERAL_STAT1_LOW_TEMP_WARN BIT(11)
-
#if IS_REACHABLE(CONFIG_HWMON)
static umode_t aqr_hwmon_is_visible(const void *data,
diff --git a/drivers/net/phy/aquantia/aquantia_main.c b/drivers/net/phy/aquantia/aquantia_main.c
index 334a6904ca5a..4498426e9a52 100644
--- a/drivers/net/phy/aquantia/aquantia_main.c
+++ b/drivers/net/phy/aquantia/aquantia_main.c
@@ -91,61 +91,6 @@
#define MDIO_C22EXT_STAT_SGMII_TX_FRAME_ALIGN_ERR 0xd31a
#define MDIO_C22EXT_STAT_SGMII_TX_RUNT_FRAMES 0xd31b
-/* Vendor specific 1, MDIO_MMD_VEND1 */
-#define VEND1_GLOBAL_FW_ID 0x0020
-#define VEND1_GLOBAL_FW_ID_MAJOR GENMASK(15, 8)
-#define VEND1_GLOBAL_FW_ID_MINOR GENMASK(7, 0)
-
-#define VEND1_GLOBAL_GEN_STAT2 0xc831
-#define VEND1_GLOBAL_GEN_STAT2_OP_IN_PROG BIT(15)
-
-/* The following registers all have similar layouts; first the registers... */
-#define VEND1_GLOBAL_CFG_10M 0x0310
-#define VEND1_GLOBAL_CFG_100M 0x031b
-#define VEND1_GLOBAL_CFG_1G 0x031c
-#define VEND1_GLOBAL_CFG_2_5G 0x031d
-#define VEND1_GLOBAL_CFG_5G 0x031e
-#define VEND1_GLOBAL_CFG_10G 0x031f
-/* ...and now the fields */
-#define VEND1_GLOBAL_CFG_RATE_ADAPT GENMASK(8, 7)
-#define VEND1_GLOBAL_CFG_RATE_ADAPT_NONE 0
-#define VEND1_GLOBAL_CFG_RATE_ADAPT_USX 1
-#define VEND1_GLOBAL_CFG_RATE_ADAPT_PAUSE 2
-
-#define VEND1_GLOBAL_RSVD_STAT1 0xc885
-#define VEND1_GLOBAL_RSVD_STAT1_FW_BUILD_ID GENMASK(7, 4)
-#define VEND1_GLOBAL_RSVD_STAT1_PROV_ID GENMASK(3, 0)
-
-#define VEND1_GLOBAL_RSVD_STAT9 0xc88d
-#define VEND1_GLOBAL_RSVD_STAT9_MODE GENMASK(7, 0)
-#define VEND1_GLOBAL_RSVD_STAT9_1000BT2 0x23
-
-#define VEND1_GLOBAL_INT_STD_STATUS 0xfc00
-#define VEND1_GLOBAL_INT_VEND_STATUS 0xfc01
-
-#define VEND1_GLOBAL_INT_STD_MASK 0xff00
-#define VEND1_GLOBAL_INT_STD_MASK_PMA1 BIT(15)
-#define VEND1_GLOBAL_INT_STD_MASK_PMA2 BIT(14)
-#define VEND1_GLOBAL_INT_STD_MASK_PCS1 BIT(13)
-#define VEND1_GLOBAL_INT_STD_MASK_PCS2 BIT(12)
-#define VEND1_GLOBAL_INT_STD_MASK_PCS3 BIT(11)
-#define VEND1_GLOBAL_INT_STD_MASK_PHY_XS1 BIT(10)
-#define VEND1_GLOBAL_INT_STD_MASK_PHY_XS2 BIT(9)
-#define VEND1_GLOBAL_INT_STD_MASK_AN1 BIT(8)
-#define VEND1_GLOBAL_INT_STD_MASK_AN2 BIT(7)
-#define VEND1_GLOBAL_INT_STD_MASK_GBE BIT(6)
-#define VEND1_GLOBAL_INT_STD_MASK_ALL BIT(0)
-
-#define VEND1_GLOBAL_INT_VEND_MASK 0xff01
-#define VEND1_GLOBAL_INT_VEND_MASK_PMA BIT(15)
-#define VEND1_GLOBAL_INT_VEND_MASK_PCS BIT(14)
-#define VEND1_GLOBAL_INT_VEND_MASK_PHY_XS BIT(13)
-#define VEND1_GLOBAL_INT_VEND_MASK_AN BIT(12)
-#define VEND1_GLOBAL_INT_VEND_MASK_GBE BIT(11)
-#define VEND1_GLOBAL_INT_VEND_MASK_GLOBAL1 BIT(2)
-#define VEND1_GLOBAL_INT_VEND_MASK_GLOBAL2 BIT(1)
-#define VEND1_GLOBAL_INT_VEND_MASK_GLOBAL3 BIT(0)
-
/* Sleep and timeout for checking if the Processor-Intensive
* MDIO operation is finished
*/
--
2.40.1
^ permalink raw reply related
* [net-next RFC PATCH v4 1/4] net: phy: aquantia: move to separate directory
From: Christian Marangi @ 2023-11-03 12:35 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Andrew Lunn,
Heiner Kallweit, Russell King, Christian Marangi, Robert Marko,
Vladimir Oltean, netdev, devicetree, linux-kernel
Move aquantia PHY driver to separate driectory in preparation for
firmware loading support to keep things tidy.
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
Changes v4:
- Keep order for kconfig config
Changes v3:
- Add this patch
drivers/net/phy/Kconfig | 5 +----
drivers/net/phy/Makefile | 6 +-----
drivers/net/phy/aquantia/Kconfig | 5 +++++
drivers/net/phy/aquantia/Makefile | 6 ++++++
drivers/net/phy/{ => aquantia}/aquantia.h | 0
drivers/net/phy/{ => aquantia}/aquantia_hwmon.c | 0
drivers/net/phy/{ => aquantia}/aquantia_main.c | 0
7 files changed, 13 insertions(+), 9 deletions(-)
create mode 100644 drivers/net/phy/aquantia/Kconfig
create mode 100644 drivers/net/phy/aquantia/Makefile
rename drivers/net/phy/{ => aquantia}/aquantia.h (100%)
rename drivers/net/phy/{ => aquantia}/aquantia_hwmon.c (100%)
rename drivers/net/phy/{ => aquantia}/aquantia_main.c (100%)
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 421d2b62918f..25cfc5ded1da 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -96,10 +96,7 @@ config ADIN1100_PHY
Currently supports the:
- ADIN1100 - Robust,Industrial, Low Power 10BASE-T1L Ethernet PHY
-config AQUANTIA_PHY
- tristate "Aquantia PHYs"
- help
- Currently supports the Aquantia AQ1202, AQ2104, AQR105, AQR405
+source "drivers/net/phy/aquantia/Kconfig"
config AX88796B_PHY
tristate "Asix PHYs"
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index c945ed9bd14b..f65e85c91fc1 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -35,11 +35,7 @@ obj-y += $(sfp-obj-y) $(sfp-obj-m)
obj-$(CONFIG_ADIN_PHY) += adin.o
obj-$(CONFIG_ADIN1100_PHY) += adin1100.o
obj-$(CONFIG_AMD_PHY) += amd.o
-aquantia-objs += aquantia_main.o
-ifdef CONFIG_HWMON
-aquantia-objs += aquantia_hwmon.o
-endif
-obj-$(CONFIG_AQUANTIA_PHY) += aquantia.o
+obj-$(CONFIG_AQUANTIA_PHY) += aquantia/
obj-$(CONFIG_AT803X_PHY) += at803x.o
obj-$(CONFIG_AX88796B_PHY) += ax88796b.o
obj-$(CONFIG_BCM54140_PHY) += bcm54140.o
diff --git a/drivers/net/phy/aquantia/Kconfig b/drivers/net/phy/aquantia/Kconfig
new file mode 100644
index 000000000000..226146417a6a
--- /dev/null
+++ b/drivers/net/phy/aquantia/Kconfig
@@ -0,0 +1,5 @@
+# SPDX-License-Identifier: GPL-2.0-only
+config AQUANTIA_PHY
+ tristate "Aquantia PHYs"
+ help
+ Currently supports the Aquantia AQ1202, AQ2104, AQR105, AQR405
diff --git a/drivers/net/phy/aquantia/Makefile b/drivers/net/phy/aquantia/Makefile
new file mode 100644
index 000000000000..346f350bc084
--- /dev/null
+++ b/drivers/net/phy/aquantia/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0
+aquantia-objs += aquantia_main.o
+ifdef CONFIG_HWMON
+aquantia-objs += aquantia_hwmon.o
+endif
+obj-$(CONFIG_AQUANTIA_PHY) += aquantia.o
diff --git a/drivers/net/phy/aquantia.h b/drivers/net/phy/aquantia/aquantia.h
similarity index 100%
rename from drivers/net/phy/aquantia.h
rename to drivers/net/phy/aquantia/aquantia.h
diff --git a/drivers/net/phy/aquantia_hwmon.c b/drivers/net/phy/aquantia/aquantia_hwmon.c
similarity index 100%
rename from drivers/net/phy/aquantia_hwmon.c
rename to drivers/net/phy/aquantia/aquantia_hwmon.c
diff --git a/drivers/net/phy/aquantia_main.c b/drivers/net/phy/aquantia/aquantia_main.c
similarity index 100%
rename from drivers/net/phy/aquantia_main.c
rename to drivers/net/phy/aquantia/aquantia_main.c
--
2.40.1
^ permalink raw reply related
* [PATCH] Prevent out-of-bounds read/write in bcmasp_netfilt_rd and bcmasp_netfilt_wr
From: Yuran Pereira @ 2023-11-03 12:27 UTC (permalink / raw)
To: davem, netdev
Cc: Yuran Pereira, justin.chen, florian.fainelli, edumazet, kuba,
pabeni, bcm-kernel-feedback-list, linux-kernel,
linux-kernel-mentees
The functions `bcmasp_netfilt_rd` and `bcmasp_netfilt_wr` both call
`bcmasp_netfilt_get_reg_offset` which, when it fails, returns `-EINVAL`.
This could lead to an out-of-bounds read or write when `rx_filter_core_rl`
or `rx_filter_core_wl` is called.
This patch adds a check in both functions to return immediately if
`bcmasp_netfilt_get_reg_offset` fails. This prevents potential out-of-bounds read
or writes, and ensures that no undefined or buggy behavior would originate from
the failure of `bcmasp_netfilt_get_reg_offset`.
Addresses-Coverity-IDs: 1544536 ("Out-of-bounds access")
Signed-off-by: Yuran Pereira <yuran.pereira@hotmail.com>
---
drivers/net/ethernet/broadcom/asp2/bcmasp.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/ethernet/broadcom/asp2/bcmasp.c b/drivers/net/ethernet/broadcom/asp2/bcmasp.c
index 29b04a274d07..8b90b761bdec 100644
--- a/drivers/net/ethernet/broadcom/asp2/bcmasp.c
+++ b/drivers/net/ethernet/broadcom/asp2/bcmasp.c
@@ -227,6 +227,8 @@ static void bcmasp_netfilt_wr(struct bcmasp_priv *priv,
reg_offset = bcmasp_netfilt_get_reg_offset(priv, nfilt, reg_type,
offset);
+ if (reg_offset < 0)
+ return;
rx_filter_core_wl(priv, val, reg_offset);
}
@@ -244,6 +246,8 @@ static u32 bcmasp_netfilt_rd(struct bcmasp_priv *priv,
reg_offset = bcmasp_netfilt_get_reg_offset(priv, nfilt, reg_type,
offset);
+ if (reg_offset < 0)
+ return 0;
return rx_filter_core_rl(priv, reg_offset);
}
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v2] tg3: power down device only on SYSTEM_POWER_OFF
From: Pavan Chebbi @ 2023-11-03 12:19 UTC (permalink / raw)
To: George Shuklin; +Cc: netdev, kai.heng.feng
In-Reply-To: <20231103115029.83273-1-george.shuklin@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1269 bytes --]
On Fri, Nov 3, 2023 at 5:21 PM George Shuklin <george.shuklin@gmail.com> wrote:
>
> Dell R650xs servers hangs on reboot if tg3 driver calls
> tg3_power_down.
>
> This happens only if network adapters (BCM5720 for R650xs) were
> initialized using SNP (e.g. by booting ipxe.efi).
>
> The actual problem is on Dell side, but this fix allows servers
> to come back alive after reboot.
>
> Signed-off-by: George Shuklin <george.shuklin@gmail.com>
> Fixes: 2ca1c94ce0b6 ("tg3: Disable tg3 device on system reboot to avoid triggering AER")
> ---
> drivers/net/ethernet/broadcom/tg3.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
> index 14b311196b8f..22b00912f7ac 100644
> --- a/drivers/net/ethernet/broadcom/tg3.c
> +++ b/drivers/net/ethernet/broadcom/tg3.c
> @@ -18078,7 +18078,8 @@ static void tg3_shutdown(struct pci_dev *pdev)
> if (netif_running(dev))
> dev_close(dev);
>
> - tg3_power_down(tp);
> + if (system_state == SYSTEM_POWER_OFF)
> + tg3_power_down(tp);
>
> rtnl_unlock();
>
> --
> 2.42.0
>
>
Reviewed-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
Thank you.
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4209 bytes --]
^ permalink raw reply
* Re: [PATCH] net: phy: broadcom: Wire suspend/resume for BCM54612E
From: Andrew Lunn @ 2023-11-03 12:13 UTC (permalink / raw)
To: Marco von Rosenberg
Cc: Florian Fainelli, Broadcom internal kernel review list,
Heiner Kallweit, Russell King, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, netdev, linux-kernel
In-Reply-To: <4890615.31r3eYUQgx@5cd116mnfx>
On Fri, Nov 03, 2023 at 02:47:38AM +0100, Marco von Rosenberg wrote:
> On Wednesday, November 1, 2023 11:06:56 PM CET Andrew Lunn wrote:
> > On Wed, Nov 01, 2023 at 10:42:52PM +0100, Marco von Rosenberg wrote:
> > > On Tuesday, October 31, 2023 1:31:11 AM CET Andrew Lunn wrote:
> > > > Are we talking about a device which as been suspended? The PHY has
> > > > been left running because there is no suspend callback? Something then
> > > > triggers a resume. The bootloader then suspends the active PHY? Linux
> > > > then boots, detects its a resume, so does not touch the hardware
> > > > because there is no resume callback? The suspended PHY is then
> > > > useless.
> > >
> > > Hi Andrew,
> > >
> > > thanks for your feedback. I guess a bit of context is missing here. The
> > > issue has nothing to do with an ordinary suspension of the OS. The main
> > > point is that on initial power-up, the bootloader suspends the PHY before
> > > booting Linux. With a resume callback defined, Linux would call it on
> > > boot and make the PHY usable.
> >
> > Ah, so you rely on phy_attach_direct() calling phy_resume(phydev).
> >
> > This seems an odd way to solve the problem. It was not Linux which
> > suspend the PHY, so using resume is asymmetric.
> >
> > I think soft_reset() or config_init() should be taking the PHY out of
> > suspend.
>
> I agree with all of your points. This is just one way which happens to solve
> this specific problem. Of course it might be asymmetric to see the patch as
> a solution to my problem. However is there anything fundamentally wrong with
> adding suspend/resume callbacks?
No, there is nothing wrong with that at all, if you want to support
suspend/resume. I do however see that as a different use case to what
you describe as your problem. It fixing your problem is more of a side
effect.
We can go with this fix, but please change your justification in the
commit message. Also, its unlikely, but resume could be made
conditional in phy_attach_direct(), and you would then be back to a
broken PHY on boot. Fixing this in config_init() is the correct way
for your use case.
Andrew
^ permalink raw reply
* [PATCH v2] tg3: power down device only on SYSTEM_POWER_OFF
From: George Shuklin @ 2023-11-03 11:50 UTC (permalink / raw)
To: netdev; +Cc: kai.heng.feng, George Shuklin
Dell R650xs servers hangs on reboot if tg3 driver calls
tg3_power_down.
This happens only if network adapters (BCM5720 for R650xs) were
initialized using SNP (e.g. by booting ipxe.efi).
The actual problem is on Dell side, but this fix allows servers
to come back alive after reboot.
Signed-off-by: George Shuklin <george.shuklin@gmail.com>
Fixes: 2ca1c94ce0b6 ("tg3: Disable tg3 device on system reboot to avoid triggering AER")
---
drivers/net/ethernet/broadcom/tg3.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index 14b311196b8f..22b00912f7ac 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -18078,7 +18078,8 @@ static void tg3_shutdown(struct pci_dev *pdev)
if (netif_running(dev))
dev_close(dev);
- tg3_power_down(tp);
+ if (system_state == SYSTEM_POWER_OFF)
+ tg3_power_down(tp);
rtnl_unlock();
--
2.42.0
^ permalink raw reply related
* Re: [PATCH net] netlink: fill in missing MODULE_DESCRIPTION()
From: patchwork-bot+netdevbpf @ 2023-11-03 11:50 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: davem, netdev, edumazet, pabeni
In-Reply-To: <20231102045724.2516647-1-kuba@kernel.org>
Hello:
This patch was applied to netdev/net.git (main)
by David S. Miller <davem@davemloft.net>:
On Wed, 1 Nov 2023 21:57:24 -0700 you wrote:
> W=1 builds now warn if a module is built without
> a MODULE_DESCRIPTION(). Fill it in for sock_diag.
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> net/netlink/diag.c | 1 +
> 1 file changed, 1 insertion(+)
Here is the summary with links:
- [net] netlink: fill in missing MODULE_DESCRIPTION()
https://git.kernel.org/netdev/net/c/016b9332a334
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH v2 net] net/tcp: fix possible out-of-bounds reads in tcp_hash_fail()
From: patchwork-bot+netdevbpf @ 2023-11-03 11:40 UTC (permalink / raw)
To: Eric Dumazet
Cc: davem, kuba, pabeni, netdev, eric.dumazet, syzkaller, dima,
fruggeri, dsahern
In-Reply-To: <20231101045233.3387072-1-edumazet@google.com>
Hello:
This patch was applied to netdev/net.git (main)
by David S. Miller <davem@davemloft.net>:
On Wed, 1 Nov 2023 04:52:33 +0000 you wrote:
> syzbot managed to trigger a fault by sending TCP packets
> with all flags being set.
>
> v2:
> - While fixing this bug, add PSH flag handling and represent
> flags the way tcpdump does : [S], [S.], [P.]
> - Print 4-tuples more consistently between families.
>
> [...]
Here is the summary with links:
- [v2,net] net/tcp: fix possible out-of-bounds reads in tcp_hash_fail()
https://git.kernel.org/netdev/net/c/02f0717e9835
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* [PATCH] selftests: bpf: xskxceiver: ksft_print_msg: fix format type error
From: Anders Roxell @ 2023-11-03 11:22 UTC (permalink / raw)
To: bjorn, magnus.karlsson, maciej.fijalkowski
Cc: netdev, bpf, linux-kernel, Anders Roxell
Crossbuilding selftests/bpf for architecture arm64, format specifies
type error show up like.
xskxceiver.c:912:34: error: format specifies type 'int' but the argument
has type '__u64' (aka 'unsigned long long') [-Werror,-Wformat]
ksft_print_msg("[%s] expected meta_count [%d], got meta_count [%d]\n",
~~
%llu
__func__, pkt->pkt_nb, meta->count);
^~~~~~~~~~~
xskxceiver.c:929:55: error: format specifies type 'unsigned long long' but
the argument has type 'u64' (aka 'unsigned long') [-Werror,-Wformat]
ksft_print_msg("Frag invalid addr: %llx len: %u\n", addr, len);
~~~~ ^~~~
Fixing the issues by using the proposed format specifiers by the
compilor.
Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
---
tools/testing/selftests/bpf/xskxceiver.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
index 591ca9637b23..dc03692f34d8 100644
--- a/tools/testing/selftests/bpf/xskxceiver.c
+++ b/tools/testing/selftests/bpf/xskxceiver.c
@@ -908,7 +908,7 @@ static bool is_metadata_correct(struct pkt *pkt, void *buffer, u64 addr)
struct xdp_info *meta = data - sizeof(struct xdp_info);
if (meta->count != pkt->pkt_nb) {
- ksft_print_msg("[%s] expected meta_count [%d], got meta_count [%d]\n",
+ ksft_print_msg("[%s] expected meta_count [%d], got meta_count [%llu]\n",
__func__, pkt->pkt_nb, meta->count);
return false;
}
@@ -926,11 +926,11 @@ static bool is_frag_valid(struct xsk_umem_info *umem, u64 addr, u32 len, u32 exp
if (addr >= umem->num_frames * umem->frame_size ||
addr + len > umem->num_frames * umem->frame_size) {
- ksft_print_msg("Frag invalid addr: %llx len: %u\n", addr, len);
+ ksft_print_msg("Frag invalid addr: %lx len: %u\n", addr, len);
return false;
}
if (!umem->unaligned_mode && addr % umem->frame_size + len > umem->frame_size) {
- ksft_print_msg("Frag crosses frame boundary addr: %llx len: %u\n", addr, len);
+ ksft_print_msg("Frag crosses frame boundary addr: %lx len: %u\n", addr, len);
return false;
}
@@ -1029,7 +1029,7 @@ static int complete_pkts(struct xsk_socket_info *xsk, int batch_size)
u64 addr = *xsk_ring_cons__comp_addr(&xsk->umem->cq, idx + rcvd - 1);
ksft_print_msg("[%s] Too many packets completed\n", __func__);
- ksft_print_msg("Last completion address: %llx\n", addr);
+ ksft_print_msg("Last completion address: %lx\n", addr);
return TEST_FAILURE;
}
@@ -1513,7 +1513,7 @@ static int validate_tx_invalid_descs(struct ifobject *ifobject)
}
if (stats.tx_invalid_descs != ifobject->xsk->pkt_stream->nb_pkts / 2) {
- ksft_print_msg("[%s] tx_invalid_descs incorrect. Got [%u] expected [%u]\n",
+ ksft_print_msg("[%s] tx_invalid_descs incorrect. Got [%llu] expected [%u]\n",
__func__, stats.tx_invalid_descs,
ifobject->xsk->pkt_stream->nb_pkts);
return TEST_FAILURE;
--
2.42.0
^ permalink raw reply related
* Re: [PATCH net-next] tcp: get rid of sysctl_tcp_adv_win_scale
From: Jiri Slaby @ 2023-11-03 11:07 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, netdev,
Soheil Hassas Yeganeh, Neal Cardwell, Yuchung Cheng, eric.dumazet
In-Reply-To: <CANn89i+7FWCX6+-puMj7L9f+=Ji6mV-Oca63k-c7OsEikmFPHQ@mail.gmail.com>
On 03. 11. 23, 11:27, Eric Dumazet wrote:
> On Fri, Nov 3, 2023 at 11:14 AM Jiri Slaby <jirislaby@kernel.org> wrote:
>>
>> On 03. 11. 23, 9:17, Eric Dumazet wrote:
>>> What happens if you double /proc/sys/net/ipv4/tcp_wmem and/or
>>> /proc/sys/net/ipv4/tcp_rmem first value ?
>>> (4096 -> 8192)
>>
>> No change:
>> # cat /proc/sys/net/ipv4/tcp_wmem
>> 8192 16384 4194304
>>
>>
>> But this:
>> # cat /proc/sys/net/ipv4/tcp_rmem
>> 8192 16384 4194304
>>
>> results in test to run 4.7 s, so even 3 more times slower!
>
>
> You might try setting tcp_shrink_window sysctl to one, with some side
> effects for normal TCP flows.
That results in 3 seconds.
In anyway, it looks like they need to fix the test, right? Created:
https://github.com/eventlet/eventlet/issues/821
thanks,
--
js
suse labs
^ permalink raw reply
* Re: [PATCH] net: ipmr_base: Check iif when returning a (*, G) MFC
From: Yang Sun @ 2023-11-03 11:05 UTC (permalink / raw)
To: nicolas.dichtel
Cc: Ido Schimmel, davem, dsahern, edumazet, kuba, pabeni, netdev
In-Reply-To: <fc356b9d-d7fc-4db8-b26c-8c786758d3e5@6wind.com>
On Thu, Nov 2, 2023 at 10:19 PM Nicolas Dichtel
<nicolas.dichtel@6wind.com> wrote:
>
> Le 02/11/2023 à 12:48, Yang Sun a écrit :
> >> Is this a regression (doesn't seem that way)? If not, the change should
> >> be targeted at net-next which is closed right now:
> >
> >> https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html
> >
> > I see.
> >
> >>> - if (c->mfc_un.res.ttls[vifi] < 255)
> >>> + if (c->mfc_parent == vifi && c->mfc_un.res.ttls[vifi] < 255)
> >
> >> What happens if the route doesn't have an iif (-1)? It won't match
> >> anymore?
> >
> > Looks like the mfc_parent can't be -1? There is the check:
> > if (mfc->mf6cc_parent >= MAXMIFS)
> > return -ENFILE;
> > before setting the parent:
> > c->_c.mfc_parent = mfc->mf6cc_parent;
> >
> > I wrote this patch thinking (*, G) MFCs could be per iif, similar to the
> > (S, G) MFCs, like we can add the following MFCs to forward packets from
> > any address with group destination ff05::aa from if1 to if2, and forward
> > packets from any address with group destination ff05::aa from if2 to
> > both if1 and if3.
> >
> > (::, ff05::aa) Iif: if1 Oifs: if1 if2 State: resolved
> > (::, ff05::aa) Iif: if2 Oifs: if1 if2 if3 State: resolved
> >
> > But reading Nicolas's initial commit message again, it seems to me that
> > (*, G) has to be used together with (*, *) and there should be only one
> > (*, G) entry per group address and include all relevant interfaces in
> > the oifs? Like the following:
> >
> > (::, ::) Iif: if1 Oifs: if1 if2 if3 State: resolved
> > (::, ff05::aa) Iif: if1 Oifs: if1 if2 if3 State: resolved
> >
> > Is this how the (*, *|G) MFCs are intended to be used? which means packets
> > to ff05::aa are forwarded from any one of the interfaces to all the other
> > interfaces? If this is the intended way it works then my patch would break
> > things and should be rejected.
> Yes, this was the intend. Only one (*, G) entry was expected (per G).
>
> >
> > Is there a way to achieve the use case I described above? Like having
> > different oifs for different iif?
> Instead of being too strict, maybe you could try to return the 'best' entry.
>
> #1 (::, ff05::aa) Iif: if1 Oifs: if1 if2 State: resolved
> #2 (::, ff05::aa) Iif: if2 Oifs: if1 if2 if3 State: resolved
>
> If a packet comes from if2, returns #2, but if a packet comes from if3, returns
> the first matching entry, ie #1 here.
>
Thanks for your reply Nicolas!
Here if it returns the first matching then it depends on which entry
is returned first
by the hash table lookup, the forwarding behavior may be indeterminate
in that case
it seems.
If a packet has no matching (*, G) entry, then it will use the (*, *)
entry to be forwarded
to the upstream interface in (*, *). And with the (*, *) it means we
won't get any nocache upcall
for interfaces included in the static tree, right? So the (S, G) MFC
and the static proxy MFCs
are not meant to be used together?
I wonder how a real use case with (*, G|*) would look like, what
interface could be an
upstream interface. Is there an example?
Thanks,
Yang
>
> Regards,
> Nicolas
^ permalink raw reply
* Re: [PATCH] net: stmmac: Wait a bit for the reset to take effect
From: Bernd Edlinger @ 2023-11-03 10:45 UTC (permalink / raw)
To: Paolo Abeni, Alexandre Torgue, Jose Abreu, David S. Miller,
Eric Dumazet, Jakub Kicinski, Maxime Coquelin, netdev,
linux-stm32, linux-arm-kernel, linux-kernel@vger.kernel.org
In-Reply-To: <5a265e5541ab1de2258a6e61bd672ef5fb6be65f.camel@redhat.com>
On 11/2/23 12:25, Paolo Abeni wrote:
> On Mon, 2023-10-30 at 07:01 +0100, Bernd Edlinger wrote:
>> otherwise the synopsys_id value may be read out wrong,
>> because the GMAC_VERSION register might still be in reset
>> state, for at least 1 us after the reset is de-asserted.
>>
>> Add a wait for 10 us before continuing to be on the safe side.
>
> This looks like a bugfix: you should target explicitly the 'net' tree,
I dont understand, did you mean to add "net:" to the subject?
It is already "net: stmmac: ..." or did you mean to send this message to
another mailing list?
> adding such tag into the subj. More importantly you should include a
> suitable 'Fixes' tag.
>
You mean an informal description of what this fixes?
like
Fixes: Randomly occurring "Version ID not available" messages.
Or do mean to pick a commit where the error was introduced? I doubt
I am able to do the necessary bisection steps, as this seems like an
issue that was introduced by the initial design. And I also doubt that
it can only affect the GMAC_VERSION register.
To make that clear, the issue is totally harmless, maybe some performance
degradation but I became aware of it only because I was trying to solve
a totally different issue, and therefore I have looked very closely at the
printk messages, so I spotted the anomaly to tracked it down the reason for
this flaw.
Thanks
Bernd.
> Please send a new revision with the above changes. You can retain the
> already collected reviewed tags.
>
> Thanks,
>
> Paolo
>
^ permalink raw reply
* Re: [net-next v4 0/5] net: wwan: t7xx: fw flashing & coredump support
From: Jiri Pirko @ 2023-11-03 10:41 UTC (permalink / raw)
To: SongJinJian@hotmail.com
Cc: davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
pabeni@redhat.com, corbet@lwn.net, loic.poulain@linaro.org,
ryazanov.s.a@gmail.com, johannes@sipsolutions.net,
chandrashekar.devegowda@intel.com, linuxwwan@intel.com,
chiranjeevi.rapolu@linux.intel.com, haijun.liu@mediatek.com,
m.chetan.kumar@linux.intel.com, ricardo.martinez@linux.intel.com,
netdev@vger.kernel.org, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, nmarupaka@google.com,
vsankar@lenovo.com, danielwinkler@google.com
In-Reply-To: <MEYP282MB2697B33940B6E9F3BA802729BBFBA@MEYP282MB2697.AUSP282.PROD.OUTLOOK.COM>
Mon, Sep 18, 2023 at 08:56:26AM CEST, SongJinJian@hotmail.com wrote:
>Tue, Sep 12, 2023 at 11:48:40AM CEST, songjinjian@hotmail.com wrote:
>>>Adds support for t7xx wwan device firmware flashing & coredump
>>>collection using devlink.
>
>>I don't believe that use of devlink is correct here. It seems like a misfit. IIUC, what you need is to communicate with the modem. Basically a communication channel to modem. The other wwan drivers implement these channels in _ctrl.c files, using multiple protocols. Why can't you do something similar and let devlink out of this please?
>
>>Until you put in arguments why you really need devlink and why is it a good fit, I'm against this. Please don't send any other versions of this patchset that use devlink.
>
> Yes, t7xx driver need communicate with modem with a communication channel to modem.
> I took a look at the _ctrl.c files under wwan directory, it seemed the implementation can be well integrated with QualCommon's modem, if we do like this, I think we need modem firmware change, maybe not be suitable for current MTK modem directly.
> Except for Qualcomm modem driver, there is also an Intel wwan driver 'iosm' and it use devlink to implement firmware flash(https://www.kernel.org/doc/html/latest/networking/devlink/iosm.html), Intel and MTK design and use devlink to do this work on
If that exists, I made a mistake as a gatekeeper. That usage looks
wrong.
> 'mtk_t7xx' driver and I continue to do this work.
>
> I think devlink framework can support this scene and if we use devlink we don't need to develop other flash tools or other user space applications, use upstream devlink commands directly.
Please don't.
>
> Thanks.
>>NACK.
^ permalink raw reply
* Re: [net-next v4 0/5] net: wwan: t7xx: fw flashing & coredump support
From: Jiri Pirko @ 2023-11-03 10:40 UTC (permalink / raw)
To: Loic Poulain
Cc: Jinjian Song, davem, edumazet, kuba, pabeni, corbet, ryazanov.s.a,
johannes, chandrashekar.devegowda, linuxwwan, chiranjeevi.rapolu,
haijun.liu, m.chetan.kumar, ricardo.martinez, netdev, linux-doc,
linux-kernel, nmarupaka, vsankar, danielwinkler
In-Reply-To: <CAMZdPi-qZ3JjZmEAtEmJETNzKd+k6UcLnLkM0MZoSZ1hKaOXuA@mail.gmail.com>
Thu, Sep 21, 2023 at 11:36:26AM CEST, loic.poulain@linaro.org wrote:
>On Wed, 13 Sept 2023 at 11:17, Jiri Pirko <jiri@resnulli.us> wrote:
>>
>> Tue, Sep 12, 2023 at 11:48:40AM CEST, songjinjian@hotmail.com wrote:
>> >Adds support for t7xx wwan device firmware flashing & coredump collection
>> >using devlink.
>>
>> I don't believe that use of devlink is correct here. It seems like a
>> misfit. IIUC, what you need is to communicate with the modem. Basically
>> a communication channel to modem. The other wwan drivers implement these
>> channels in _ctrl.c files, using multiple protocols. Why can't you do
>> something similar and let devlink out of this please?
>>
>> Until you put in arguments why you really need devlink and why is it a
>> good fit, I'm against this. Please don't send any other versions of this
>> patchset that use devlink.
>
>The t7xx driver already has regular wwan data and control interfaces
>registered with the wwan framework, making it functional. Here the
>exposed low level resources are not really wwan/class specific as it
>is for firmware upgrade and coredump, so I think that is why Jinjian
>chose the 'feature agnostic' devlink framework. IMHO I think it makes
>sense to rely on such a framework, or maybe on the devcoredump class.
>
>That said, I see the protocol for flashing and doing the coreboot is
>fastboot, which is already supported on the user side with the
>fastboot tool, so I'm not sure abstracting it here makes sense. If the
>protocol is really fasboot compliant, Wouldn't it be simpler to
>directly expose it as a new device/channel? and rely on a userspace
>tool for regular fastboot operations (flash, boot, dump). This may
>require slightly modifying the fastboot tool to detect and support
>that new transport (in addition to the existing usb and ethernet
>support).
Sounds sane. Please let devlink out of this.
>
>Regards,
>Loic
^ permalink raw reply
* Re: [PATCH net-next] tcp: get rid of sysctl_tcp_adv_win_scale
From: Eric Dumazet @ 2023-11-03 10:27 UTC (permalink / raw)
To: Jiri Slaby
Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, netdev,
Soheil Hassas Yeganeh, Neal Cardwell, Yuchung Cheng, eric.dumazet
In-Reply-To: <28e2bd4a-3233-471c-a1ae-57a445173401@kernel.org>
On Fri, Nov 3, 2023 at 11:14 AM Jiri Slaby <jirislaby@kernel.org> wrote:
>
> On 03. 11. 23, 9:17, Eric Dumazet wrote:
> > What happens if you double /proc/sys/net/ipv4/tcp_wmem and/or
> > /proc/sys/net/ipv4/tcp_rmem first value ?
> > (4096 -> 8192)
>
> No change:
> # cat /proc/sys/net/ipv4/tcp_wmem
> 8192 16384 4194304
>
>
> But this:
> # cat /proc/sys/net/ipv4/tcp_rmem
> 8192 16384 4194304
>
> results in test to run 4.7 s, so even 3 more times slower!
You might try setting tcp_shrink_window sysctl to one, with some side
effects for normal TCP flows.
^ permalink raw reply
* Re: [PATCH net] netfilter: nf_tables: fix pointer math issue in nft_byteorder_eval()
From: Florian Westphal @ 2023-11-03 10:25 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: Florian Westphal, Dan Carpenter, Jozsef Kadlecsik,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
netfilter-devel, coreteam, netdev, linux-kernel, kernel-janitors
In-Reply-To: <ZUTBNcA7ApLu5DMA@calendula>
Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Fri, Nov 03, 2023 at 10:18:01AM +0100, Florian Westphal wrote:
> > Dan Carpenter <dan.carpenter@linaro.org> wrote:
> > > The problem is in nft_byteorder_eval() where we are iterating through a
> > > loop and writing to dst[0], dst[1], dst[2] and so on... On each
> > > iteration we are writing 8 bytes. But dst[] is an array of u32 so each
> > > element only has space for 4 bytes. That means that every iteration
> > > overwrites part of the previous element.
> > >
> > > I spotted this bug while reviewing commit caf3ef7468f7 ("netfilter:
> > > nf_tables: prevent OOB access in nft_byteorder_eval") which is a related
> > > issue. I think that the reason we have not detected this bug in testing
> > > is that most of time we only write one element.
> >
> > LGTM, thanks Dan. We will route this via nf.git.
>
> Thanks for your patch.
>
> One question, is this update really required?
I think so, yes. Part of this bug here is that this helper-niceness
masks whats really happening in the caller (advancing in strides of
'u32', rather than 'u64').
^ permalink raw reply
* Re: [PATCH net-next] tcp: get rid of sysctl_tcp_adv_win_scale
From: Jiri Slaby @ 2023-11-03 10:14 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, netdev,
Soheil Hassas Yeganeh, Neal Cardwell, Yuchung Cheng, eric.dumazet
In-Reply-To: <CANn89iJOwQUwAVcofW+X_8srFcPnaWKyqOoM005L6Zgh8=OvpA@mail.gmail.com>
On 03. 11. 23, 9:17, Eric Dumazet wrote:
> What happens if you double /proc/sys/net/ipv4/tcp_wmem and/or
> /proc/sys/net/ipv4/tcp_rmem first value ?
> (4096 -> 8192)
No change:
# cat /proc/sys/net/ipv4/tcp_wmem
8192 16384 4194304
But this:
# cat /proc/sys/net/ipv4/tcp_rmem
8192 16384 4194304
results in test to run 4.7 s, so even 3 more times slower!
thanks,
--
js
suse labs
^ permalink raw reply
* Re: [PATCH net-next] tcp: get rid of sysctl_tcp_adv_win_scale
From: Eric Dumazet @ 2023-11-03 9:53 UTC (permalink / raw)
To: Michal Kubecek
Cc: Jiri Slaby, David S . Miller, Jakub Kicinski, Paolo Abeni, netdev,
Soheil Hassas Yeganeh, Neal Cardwell, Yuchung Cheng, eric.dumazet
In-Reply-To: <20231103092706.6rw2ehuigxfdvhlc@lion.mk-sys.cz>
On Fri, Nov 3, 2023 at 10:27 AM Michal Kubecek <mkubecek@suse.cz> wrote:
>
> On Fri, Nov 03, 2023 at 09:17:27AM +0100, Eric Dumazet wrote:
> >
> > It seems the test had some expectations.
> >
> > Setting a small (1 byte) RCVBUF/SNDBUF, and yet expecting to send
> > 46080 bytes fast enough was not reasonable.
> > It might have relied on the fact that tcp sendmsg() can cook large GSO
> > packets, even if sk->sk_sndbuf is small.
> >
> > With tight memory settings, it is possible TCP has to resort on RTO
> > timers (200ms by default) to recover from dropped packets.
>
> There seems to be one drop but somehow the sender does not recover from
> it, even if the retransmit and following packets are acked quickly:
>
> 09:15:29.424017 IP 127.0.0.1.40386 > 127.0.0.1.42483: Flags [S], seq 104649613, win 33280, options [mss 65495,sackOK,TS val 1319295278 ecr 0,nop,wscale 7], length 0
> 09:15:29.424024 IP 127.0.0.1.42483 > 127.0.0.1.40386: Flags [S.], seq 1343383818, ack 104649614, win 585, options [mss 65495,sackOK,TS val 1319295278 ecr 1319295278,nop,wscale 0], length 0
> 09:15:29.424031 IP 127.0.0.1.40386 > 127.0.0.1.42483: Flags [.], ack 1, win 260, options [nop,nop,TS val 1319295278 ecr 1319295278], length 0
> 09:15:29.424155 IP 127.0.0.1.42483 > 127.0.0.1.40386: Flags [.], seq 1:16641, ack 1, win 585, options [nop,nop,TS val 1319295279 ecr 1319295278], length 16640
> 09:15:29.424160 IP 127.0.0.1.40386 > 127.0.0.1.42483: Flags [.], ack 16641, win 130, options [nop,nop,TS val 1319295279 ecr 1319295279], length 0
> 09:15:29.424179 IP 127.0.0.1.42483 > 127.0.0.1.40386: Flags [P.], seq 16641:33281, ack 1, win 585, options [nop,nop,TS val 1319295279 ecr 1319295279], length 16640
> 09:15:29.424183 IP 127.0.0.1.40386 > 127.0.0.1.42483: Flags [.], ack 16641, win 0, options [nop,nop,TS val 1319295279 ecr 1319295279], length 0
> 09:15:29.424280 IP 127.0.0.1.40386 > 127.0.0.1.42483: Flags [P.], seq 1:12, ack 16641, win 16640, options [nop,nop,TS val 1319295279 ecr 1319295279], length 11
> 09:15:29.424284 IP 127.0.0.1.42483 > 127.0.0.1.40386: Flags [.], ack 12, win 574, options [nop,nop,TS val 1319295279 ecr 1319295279], length 0
> 09:15:29.630272 IP 127.0.0.1.42483 > 127.0.0.1.40386: Flags [P.], seq 16641:33281, ack 12, win 574, options [nop,nop,TS val 1319295485 ecr 1319295279], length 16640
> 09:15:29.630334 IP 127.0.0.1.40386 > 127.0.0.1.42483: Flags [.], ack 33281, win 2304, options [nop,nop,TS val 1319295485 ecr 1319295485], length 0
> 09:15:29.836938 IP 127.0.0.1.42483 > 127.0.0.1.40386: Flags [P.], seq 33281:35585, ack 12, win 574, options [nop,nop,TS val 1319295691 ecr 1319295485], length 2304
> 09:15:29.836984 IP 127.0.0.1.40386 > 127.0.0.1.42483: Flags [.], ack 35585, win 2304, options [nop,nop,TS val 1319295691 ecr 1319295691], length 0
> 09:15:30.043606 IP 127.0.0.1.42483 > 127.0.0.1.40386: Flags [P.], seq 35585:37889, ack 12, win 574, options [nop,nop,TS val 1319295898 ecr 1319295691], length 2304
> 09:15:30.043653 IP 127.0.0.1.40386 > 127.0.0.1.42483: Flags [.], ack 37889, win 2304, options [nop,nop,TS val 1319295898 ecr 1319295898], length 0
> 09:15:30.250270 IP 127.0.0.1.42483 > 127.0.0.1.40386: Flags [P.], seq 37889:40193, ack 12, win 574, options [nop,nop,TS val 1319296105 ecr 1319295898], length 2304
> 09:15:30.250316 IP 127.0.0.1.40386 > 127.0.0.1.42483: Flags [.], ack 40193, win 2304, options [nop,nop,TS val 1319296105 ecr 1319296105], length 0
> 09:15:30.456932 IP 127.0.0.1.42483 > 127.0.0.1.40386: Flags [P.], seq 40193:42497, ack 12, win 574, options [nop,nop,TS val 1319296311 ecr 1319296105], length 2304
> 09:15:30.456975 IP 127.0.0.1.40386 > 127.0.0.1.42483: Flags [.], ack 42497, win 2304, options [nop,nop,TS val 1319296311 ecr 1319296311], length 0
> 09:15:30.663598 IP 127.0.0.1.42483 > 127.0.0.1.40386: Flags [P.], seq 42497:44801, ack 12, win 574, options [nop,nop,TS val 1319296518 ecr 1319296311], length 2304
> 09:15:30.663638 IP 127.0.0.1.40386 > 127.0.0.1.42483: Flags [.], ack 44801, win 2304, options [nop,nop,TS val 1319296518 ecr 1319296518], length 0
> 09:15:30.663646 IP 127.0.0.1.42483 > 127.0.0.1.40386: Flags [FP.], seq 44801:46081, ack 12, win 574, options [nop,nop,TS val 1319296518 ecr 1319296518], length 1280
> 09:15:30.663712 IP 127.0.0.1.40386 > 127.0.0.1.42483: Flags [F.], seq 12, ack 46082, win 2304, options [nop,nop,TS val 1319296518 ecr 1319296518], length 0
> 09:15:30.663724 IP 127.0.0.1.42483 > 127.0.0.1.40386: Flags [.], ack 13, win 573, options [nop,nop,TS val 1319296518 ecr 1319296518], length 0
>
> (window size values are scaled here). Part of the problem is that the
> receiver side sets SO_RCVBUF after connect() so that the window shrinks
> after sender already sent more data; when I move the bufsized() calls
> in the python script before listen() and connect(), the test runs
> quickly.
This makes sense.
Old kernels would have instead dropped a packet, without changing test status:
09:49:49.390066 IP localhost.39710 > localhost.44173: Flags [S], seq
1464131415, win 65495, options [mss 65495,sackOK,TS val 578664891 ecr
0,nop,wscale 7], length 0
09:49:49.390078 IP localhost.44173 > localhost.39710: Flags [S.], seq
2322612108, ack 1464131416, win 1152, options [mss 65495,sackOK,TS val
578664891 ecr 578664891,nop,wscale 0], length 0
09:49:49.390088 IP localhost.39710 > localhost.44173: Flags [.], ack
1, win 512, options [nop,nop,TS val 578664891 ecr 578664891], length 0
09:49:49.390319 IP localhost.44173 > localhost.39710: Flags [.], seq
1:32769, ack 1, win 1152, options [nop,nop,TS val 578664892 ecr
578664891], length 32768
09:49:49.390325 IP localhost.39710 > localhost.44173: Flags [.], ack
32769, win 256, options [nop,nop,TS val 578664892 ecr 578664892],
length 0
09:49:49.390355 IP localhost.44173 > localhost.39710: Flags [P.], seq
32769:46081, ack 1, win 1152, options [nop,nop,TS val 578664892 ecr
578664892], length 13312
<prior packet has been dropped by receiver>
09:49:49.390479 IP localhost.39710 > localhost.44173: Flags [P.], seq
1:12, ack 32769, win 256, options [nop,nop,TS val 578664892 ecr
578664892], length 11
09:49:49.390483 IP localhost.44173 > localhost.39710: Flags [.], ack
12, win 1141, options [nop,nop,TS val 578664892 ecr 578664892], length
0
09:49:49.390547 IP localhost.44173 > localhost.39710: Flags [F.], seq
46081, ack 12, win 1141, options [nop,nop,TS val 578664892 ecr
578664892], length 0
09:49:49.390552 IP localhost.39710 > localhost.44173: Flags [.], ack
32769, win 256, options [nop,nop,TS val 578664892 ecr
578664892,nop,nop,sack 1 {46081:46082}], length 0
<packet retransmit>
09:49:49.390562 IP localhost.44173 > localhost.39710: Flags [P.], seq
32769:46081, ack 12, win 1141, options [nop,nop,TS val 578664892 ecr
578664892], length 13312
09:49:49.390567 IP localhost.39710 > localhost.44173: Flags [.], ack
46082, win 152, options [nop,nop,TS val 578664892 ecr 578664892],
length 0
09:49:49.390677 IP localhost.39710 > localhost.44173: Flags [F.], seq
12, ack 46082, win 152, options [nop,nop,TS val 578664892 ecr
578664892], length 0
09:49:49.390685 IP localhost.44173 > localhost.39710: Flags [.], ack
13, win 1141, options [nop,nop,TS val 578664892 ecr 578664892], length
0
Retracting TCP windows has always been problematic.
If we really want to be very gentle, this could add more logic,
shorter timer events for pathological cases like that,
I am not sure this is really worth it, especially if dealing with one
million TCP sockets in this state.
^ permalink raw reply
* Re: [PATCH net] net/mlx5: Fix a NULL vs IS_ERR() check
From: Wojciech Drewek @ 2023-11-03 9:51 UTC (permalink / raw)
To: Dan Carpenter, Saeed Mahameed
Cc: Leon Romanovsky, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Eli Cohen, netdev, linux-rdma, kernel-janitors
In-Reply-To: <4ee5fbea-7807-42dd-a9b8-738ac23249d0@moroto.mountain>
On 03.11.2023 07:36, Dan Carpenter wrote:
> The mlx5_esw_offloads_devlink_port() function returns error pointers, not
> NULL.
>
> Fixes: 7bef147a6ab6 ("net/mlx5: Don't skip vport check")
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> ---
> I *think* these normally go through the mellanox tree and not net.
Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>
>
> drivers/net/ethernet/mellanox/mlx5/core/en_rep.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
> index 693e55b010d9..5c569d4bfd00 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
> @@ -1493,7 +1493,7 @@ mlx5e_vport_vf_rep_load(struct mlx5_core_dev *dev, struct mlx5_eswitch_rep *rep)
>
> dl_port = mlx5_esw_offloads_devlink_port(dev->priv.eswitch,
> rpriv->rep->vport);
> - if (dl_port) {
> + if (!IS_ERR(dl_port)) {
> SET_NETDEV_DEVLINK_PORT(netdev, dl_port);
> mlx5e_rep_vnic_reporter_create(priv, dl_port);
> }
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox