* Re: [net PATCH] octeontx2: Fix klockwork and coverity issues
From: Andrew Lunn @ 2023-11-01 12:42 UTC (permalink / raw)
To: Suman Ghosh
Cc: sgoutham, gakula, sbhatta, hkelam, lcherian, jerinj, davem,
edumazet, kuba, pabeni, netdev, linux-kernel, horms,
Ratheesh Kannoth
In-Reply-To: <20231101074919.2614608-1-sumang@marvell.com>
On Wed, Nov 01, 2023 at 01:19:19PM +0530, Suman Ghosh wrote:
> Fix all klockwork and coverity issues reported on AF and PF/VF driver.
>
> Signed-off-by: Suman Ghosh <sumang@marvell.com>
> Signed-off-by: Ratheesh Kannoth <rkannoth@marvell.com>
The subject line is:
[net PATCH] octeontx2: Fix klockwork and coverity issues
So you want these fixes backported to net? If so, you need to provide
Fixes: tags.
This patch is way too big. A fix patch generally fixes one thing, and
it documents what it fixes. Or it could be one class of problems, like
uninitialised variables etc. Its good to include the message from the
static analyser in the commit message.
Andrew
---
pw-bot: cr
^ permalink raw reply
* Re: [PATCH] net: usbnet: Fix potential NULL pointer dereference
From: Ren Mingshuai @ 2023-11-01 12:55 UTC (permalink / raw)
To: renmingshuai
Cc: caowangbao, davem, khlebnikov, liaichun, linux-kernel, netdev,
oneukum, yanan
In-Reply-To: <20231101123559.210756-1-renmingshuai@huawei.com>
>23ba07991dad said SKB can be NULL without describing the triggering
>scenario. Always Check it before dereference to void potential NULL
>pointer dereference.
I've tried to find out the scenarios where SKB is NULL, but failed.
It seems impossible for SKB to be NULL. If SKB can be NULL, please tell
me the reason and I'd be very grateful.
>Fix smatch warning:
>drivers/net/usb/usbnet.c:1380 usbnet_start_xmit() error: we previously assumed 'skb' could be null (see line 1359)
>
>Signed-off-by: Ren Mingshuai <renmingshuai@huawei.com>
>---
> drivers/net/usb/usbnet.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
>diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
>index 64a9a80b2309..386cb1a4ff03 100644
>--- a/drivers/net/usb/usbnet.c
>+++ b/drivers/net/usb/usbnet.c
>@@ -1374,6 +1374,11 @@ netdev_tx_t usbnet_start_xmit (struct sk_buff *skb,
> }
> }
>
>+ if (!skb) {
>+ netif_dbg(dev, tx_err, dev->net, "tx skb is NULL\n");
>+ goto drop;
>+ }
>+
> if (!(urb = usb_alloc_urb (0, GFP_ATOMIC))) {
> netif_dbg(dev, tx_err, dev->net, "no urb\n");
> goto drop;
>--
>2.33.0
^ permalink raw reply
* Re: [net-next PATCH v2 1/2] net: phy: aquantia: add firmware load support
From: Heiner Kallweit @ 2023-11-01 13:01 UTC (permalink / raw)
To: Christian Marangi, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Andrew Lunn, Russell King, netdev, devicetree, linux-kernel
Cc: Robert Marko
In-Reply-To: <20231101123608.11157-1-ansuelsmth@gmail.com>
On 01.11.2023 13:36, Christian Marangi wrote:
> 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 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
>
To make the driver better maintainable: can the firmware handling code
be placed in a separate source code file, similar to what has been done
for the hwmon part?
If yes, then this could also be the right time to move the aquantia
driver to an own subdirectory.
^ permalink raw reply
* [PATCH] [PATCH net] tg3: power down device only on SYSTEM_POWER_OFF
From: George Shuklin @ 2023-11-01 13:04 UTC (permalink / raw)
To: netdev; +Cc: George Shuklin
Dell R650xs servers hangs 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).
This is partial revert of commit 2ca1c94ce0b.
The actual problem is on Dell side, but this fix allow servers
to come back alive after reboot.
---
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: [net-next PATCH v2 1/2] net: phy: aquantia: add firmware load support
From: Christian Marangi @ 2023-11-01 12:57 UTC (permalink / raw)
To: Heiner Kallweit
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Andrew Lunn,
Russell King, netdev, devicetree, linux-kernel, Robert Marko
In-Reply-To: <5af21f93-bb2d-42b1-b4d4-ee4443ffaff9@gmail.com>
On Wed, Nov 01, 2023 at 02:01:33PM +0100, Heiner Kallweit wrote:
> On 01.11.2023 13:36, Christian Marangi wrote:
> > 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 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
> >
>
> To make the driver better maintainable: can the firmware handling code
> be placed in a separate source code file, similar to what has been done
> for the hwmon part?
> If yes, then this could also be the right time to move the aquantia
> driver to an own subdirectory.
>
Sure! Np for me just is it really worth it? hwmod is a bigger one but
this is really a few functions.
Anyway if requested, I will move in v3 the driver to a dedicated
directory and move the function to a separate file!
--
Ansuel
^ permalink raw reply
* Re: [net-next PATCH v2 1/2] net: phy: aquantia: add firmware load support
From: Andrew Lunn @ 2023-11-01 13:13 UTC (permalink / raw)
To: Christian Marangi
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiner Kallweit,
Russell King, netdev, devicetree, linux-kernel, Robert Marko
In-Reply-To: <20231101123608.11157-1-ansuelsmth@gmail.com>
On Wed, Nov 01, 2023 at 01:36:07PM +0100, Christian Marangi wrote:
> 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 v2:
> - Move out of RFC
Actually, since we are in the merge window, RFC would be correct.
> - 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/Kconfig | 1 +
> drivers/net/phy/aquantia_main.c | 304 ++++++++++++++++++++++++++++++++
> 2 files changed, 305 insertions(+)
>
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> index 421d2b62918f..46c7194efcea 100644
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
> @@ -98,6 +98,7 @@ config ADIN1100_PHY
>
> 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_main.c b/drivers/net/phy/aquantia_main.c
> index 334a6904ca5a..0f1b8d75cca0 100644
> --- a/drivers/net/phy/aquantia_main.c
> +++ b/drivers/net/phy/aquantia_main.c
> @@ -12,6 +12,10 @@
> #include <linux/delay.h>
> #include <linux/bitfield.h>
> #include <linux/phy.h>
> +#include <linux/of.h>
> +#include <linux/firmware.h>
> +#include <linux/crc-ccitt.h>
> +#include <linux/nvmem-consumer.h>
>
> #include "aquantia.h"
>
> @@ -92,10 +96,40 @@
> #define MDIO_C22EXT_STAT_SGMII_TX_RUNT_FRAMES 0xd31b
>
> /* 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))
> +
> +#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_GLOBAL_GEN_STAT2 0xc831
> #define VEND1_GLOBAL_GEN_STAT2_OP_IN_PROG BIT(15)
>
> @@ -152,6 +186,30 @@
> #define AQR107_OP_IN_PROG_SLEEP 1000
> #define AQR107_OP_IN_PROG_TIMEOUT 100000
>
> +#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;
> +
> struct aqr107_hw_stat {
> const char *name;
> int reg;
> @@ -677,6 +735,166 @@ static int aqr107_wait_processor_intensive_op(struct phy_device *phydev)
> return 0;
> }
>
> +/* load data into the phy's memory */
> +static int aquantia_load_memory(struct phy_device *phydev, u32 addr,
> + const u8 *data, size_t len)
> +{
> + for (pos = 0; pos < len; pos += min(sizeof(u32), len - pos)) {
> + u32 word = 0;
> +
> + memcpy(&word, data + pos, min(sizeof(u32), len - pos));
Rather than do a memcpy, use the get_unaligned_ macros. They might map
to a memcpy(), but some architectures can do unaligned accesses
without problems.
> +static int aqr_fw_boot(struct phy_device *phydev, const u8 *data, size_t size)
> +{
> + const struct aqr_fw_header *header;
> + u32 iram_offset = 0, iram_size = 0;
> + u32 dram_offset = 0, dram_size = 0;
> + char version[VERSION_STRING_SIZE];
> + u16 calculated_crc, read_crc;
> + u32 primary_offset = 0;
> + int ret;
> +
> + /* extract saved CRC at the end of the fw */
> + memcpy(&read_crc, data + size - 2, sizeof(read_crc));
Say size == 1. You just had a buffer underrun.
> + /* CRC is saved in big-endian as PHY is BE */
> + read_crc = be16_to_cpu(read_crc);
> + calculated_crc = crc_ccitt_false(0, data, size - 2);
> + 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. */
> + memcpy(&primary_offset, data + PRIMARY_OFFSET_OFFSET, sizeof(u16));
What if PRIMARY_OFFSET_OFFSET + sizeof(u16) is greater than size? A
buffer overrun.
Assume the firmware is evil and is trying to hack you. Always test
everything.
I would suggest some helpers, something like
int aqr_fw_get_u16(const u8 *data, size_t size, size_t offset, u16 *value)
Check that offset + sizeof(u16) is within the firmware, and if not return -EINVAL.
Otherwise set *value to the u16 from the firmware and return 0.
This is where Rust would be nice :-)
Andrew
---
pw-bot: cr
^ permalink raw reply
* Re: [PATCH v2] selftests/net: synchronize udpgso_bench rx and tx
From: Lucas Karpinski @ 2023-11-01 13:17 UTC (permalink / raw)
To: Willem de Bruijn
Cc: davem, edumazet, kuba, pabeni, shuah, netdev, linux-kselftest,
linux-kernel
In-Reply-To: <65416d9fc1024_bcdbc29418@willemb.c.googlers.com.notmuch>
On Tue, Oct 31, 2023 at 05:11:59PM -0400, Willem de Bruijn wrote:
>
> The patch subject mentions UDP GSO, but the patch fixes the udpgro
> scripts.
>
> There are separate udpgso testcases. So you probably want to s/gso/gro.
>
The patch synchronizes the connection between the two binaries;
udpgso_bench_rx and udpgso_bench_tx, which are launched by the udpgro
tests. I can remove their names and just specify "synchronize udpgro
tests' tx and rx connection."
>
>
> Might a grep be shorter and more readable?
>
Noted, will change it.
Lucas
^ permalink raw reply
* Re: [net-next PATCH v2 2/2] dt-bindings: Document bindings for Marvell Aquantia PHY
From: Andrew Lunn @ 2023-11-01 13:21 UTC (permalink / raw)
To: Christian Marangi
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiner Kallweit,
Russell King, netdev, devicetree, linux-kernel
In-Reply-To: <20231101123608.11157-2-ansuelsmth@gmail.com>
> + 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 using the sysfs interface. Firmware is
> + loaded using the PHY mailbox.
sysfs is a linux concept. DT bindings should be OS agnostic. It would
be better to say its loaded from a file in the file system.
I'm not sure mailbox is relevant here. All you really are trying to
say is that if there is an SPI flash, the PHY will load the firmware
itself. If not the driver will load the firmware.
Andrew
^ permalink raw reply
* Re: [net-next PATCH v2 2/2] dt-bindings: Document bindings for Marvell Aquantia PHY
From: Rob Herring @ 2023-11-01 13:28 UTC (permalink / raw)
To: Christian Marangi
Cc: Eric Dumazet, David S. Miller, Conor Dooley, netdev, Andrew Lunn,
Russell King, linux-kernel, Rob Herring, Heiner Kallweit,
Krzysztof Kozlowski, Paolo Abeni, Jakub Kicinski, devicetree
In-Reply-To: <20231101123608.11157-2-ansuelsmth@gmail.com>
On Wed, 01 Nov 2023 13:36:08 +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 v2:
> - Add DT patch
>
> .../bindings/net/marvell,aquantia.yaml | 123 ++++++++++++++++++
> 1 file changed, 123 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/marvell,aquantia.yaml
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/mdio-mux-mmioreg.example.dtb: ethernet-phy@4: compatible:0: 'ethernet-phy-ieee802.3-c45' is not one of ['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']
from schema $id: http://devicetree.org/schemas/net/marvell,aquantia.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/mdio-mux-mmioreg.example.dtb: ethernet-phy@4: compatible: ['ethernet-phy-ieee802.3-c45'] is too short
from schema $id: http://devicetree.org/schemas/net/marvell,aquantia.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/mdio-mux-mmioreg.example.dtb: ethernet-phy@4: compatible:0: 'ethernet-phy-ieee802.3-c45' is not one of ['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']
from schema $id: http://devicetree.org/schemas/net/marvell,aquantia.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/mdio-mux-mmioreg.example.dtb: ethernet-phy@4: compatible: ['ethernet-phy-ieee802.3-c45'] is too short
from schema $id: http://devicetree.org/schemas/net/marvell,aquantia.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/ethernet-phy.example.dtb: ethernet-phy@0: compatible:0: 'ethernet-phy-id0141.0e90' is not one of ['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']
from schema $id: http://devicetree.org/schemas/net/marvell,aquantia.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/sff,sfp.example.dtb: ethernet-phy@0: compatible:0: 'ethernet-phy-ieee802.3-c45' is not one of ['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']
from schema $id: http://devicetree.org/schemas/net/marvell,aquantia.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/sff,sfp.example.dtb: ethernet-phy@0: compatible: ['ethernet-phy-ieee802.3-c45'] is too short
from schema $id: http://devicetree.org/schemas/net/marvell,aquantia.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/sff,sfp.example.dtb: ethernet-phy@0: Unevaluated properties are not allowed ('interrupt' was unexpected)
from schema $id: http://devicetree.org/schemas/net/marvell,aquantia.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/nvidia,tegra234-mgbe.example.dtb: phy@0: $nodename:0: 'phy@0' does not match '^ethernet-phy(@[a-f0-9]+)?$'
from schema $id: http://devicetree.org/schemas/net/marvell,aquantia.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/nvidia,tegra234-mgbe.example.dtb: phy@0: compatible:0: 'ethernet-phy-ieee802.3-c45' is not one of ['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']
from schema $id: http://devicetree.org/schemas/net/marvell,aquantia.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/nvidia,tegra234-mgbe.example.dtb: phy@0: compatible: ['ethernet-phy-ieee802.3-c45'] is too short
from schema $id: http://devicetree.org/schemas/net/marvell,aquantia.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/nvidia,tegra234-mgbe.example.dtb: phy@0: Unevaluated properties are not allowed ('#phy-cells', 'compatible' were unexpected)
from schema $id: http://devicetree.org/schemas/net/marvell,aquantia.yaml#
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20231101123608.11157-2-ansuelsmth@gmail.com
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
^ permalink raw reply
* Re: [net-next PATCH v2 2/2] dt-bindings: Document bindings for Marvell Aquantia PHY
From: Rob Herring @ 2023-11-01 13:38 UTC (permalink / raw)
To: Christian Marangi
Cc: Eric Dumazet, David S. Miller, Conor Dooley, netdev, Andrew Lunn,
Russell King, linux-kernel, Heiner Kallweit, Krzysztof Kozlowski,
Paolo Abeni, Jakub Kicinski, devicetree
In-Reply-To: <169884529967.4072013.2362625689707570358.robh@kernel.org>
On Wed, Nov 01, 2023 at 08:28:48AM -0500, Rob Herring wrote:
>
> On Wed, 01 Nov 2023 13:36:08 +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 v2:
> > - Add DT patch
> >
> > .../bindings/net/marvell,aquantia.yaml | 123 ++++++++++++++++++
> > 1 file changed, 123 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/net/marvell,aquantia.yaml
> >
>
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
>
> yamllint warnings/errors:
>
> dtschema/dtc warnings/errors:
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/mdio-mux-mmioreg.example.dtb: ethernet-phy@4: compatible:0: 'ethernet-phy-ieee802.3-c45' is not one of ['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']
> from schema $id: http://devicetree.org/schemas/net/marvell,aquantia.yaml#
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/mdio-mux-mmioreg.example.dtb: ethernet-phy@4: compatible: ['ethernet-phy-ieee802.3-c45'] is too short
> from schema $id: http://devicetree.org/schemas/net/marvell,aquantia.yaml#
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/mdio-mux-mmioreg.example.dtb: ethernet-phy@4: compatible:0: 'ethernet-phy-ieee802.3-c45' is not one of ['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']
> from schema $id: http://devicetree.org/schemas/net/marvell,aquantia.yaml#
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/mdio-mux-mmioreg.example.dtb: ethernet-phy@4: compatible: ['ethernet-phy-ieee802.3-c45'] is too short
> from schema $id: http://devicetree.org/schemas/net/marvell,aquantia.yaml#
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/ethernet-phy.example.dtb: ethernet-phy@0: compatible:0: 'ethernet-phy-id0141.0e90' is not one of ['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']
> from schema $id: http://devicetree.org/schemas/net/marvell,aquantia.yaml#
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/sff,sfp.example.dtb: ethernet-phy@0: compatible:0: 'ethernet-phy-ieee802.3-c45' is not one of ['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']
> from schema $id: http://devicetree.org/schemas/net/marvell,aquantia.yaml#
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/sff,sfp.example.dtb: ethernet-phy@0: compatible: ['ethernet-phy-ieee802.3-c45'] is too short
> from schema $id: http://devicetree.org/schemas/net/marvell,aquantia.yaml#
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/sff,sfp.example.dtb: ethernet-phy@0: Unevaluated properties are not allowed ('interrupt' was unexpected)
> from schema $id: http://devicetree.org/schemas/net/marvell,aquantia.yaml#
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/nvidia,tegra234-mgbe.example.dtb: phy@0: $nodename:0: 'phy@0' does not match '^ethernet-phy(@[a-f0-9]+)?$'
> from schema $id: http://devicetree.org/schemas/net/marvell,aquantia.yaml#
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/nvidia,tegra234-mgbe.example.dtb: phy@0: compatible:0: 'ethernet-phy-ieee802.3-c45' is not one of ['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']
> from schema $id: http://devicetree.org/schemas/net/marvell,aquantia.yaml#
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/nvidia,tegra234-mgbe.example.dtb: phy@0: compatible: ['ethernet-phy-ieee802.3-c45'] is too short
> from schema $id: http://devicetree.org/schemas/net/marvell,aquantia.yaml#
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/nvidia,tegra234-mgbe.example.dtb: phy@0: Unevaluated properties are not allowed ('#phy-cells', 'compatible' were unexpected)
> from schema $id: http://devicetree.org/schemas/net/marvell,aquantia.yaml#
You need a custom 'select' with all the compatibles except
ethernet-phy-ieee802.3-c45 listed.
Rob
^ permalink raw reply
* Re: [PATCH] [PATCH net] tg3: power down device only on SYSTEM_POWER_OFF
From: Pavan Chebbi @ 2023-11-01 15:20 UTC (permalink / raw)
To: George Shuklin; +Cc: netdev, Andrew Gospodarek, Michael Chan
In-Reply-To: <20231101130418.44164-1-george.shuklin@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1183 bytes --]
On Wed, Nov 1, 2023 at 6:34 PM George Shuklin <george.shuklin@gmail.com> wrote:
>
> Dell R650xs servers hangs 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).
>
> This is partial revert of commit 2ca1c94ce0b.
>
> The actual problem is on Dell side, but this fix allow servers
> to come back alive after reboot.
How are you sure that the problem solved by 2ca1c94ce0b is not
reintroduced with this change?
> ---
> 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
>
>
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4209 bytes --]
^ permalink raw reply
* Re: [PATCH] rxrpc_find_service_conn_rcu: use read_seqbegin() rather than read_seqbegin_or_lock()
From: David Howells @ 2023-11-01 15:45 UTC (permalink / raw)
To: Oleg Nesterov
Cc: dhowells, Marc Dionne, Alexander Viro, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Chuck Lever, linux-afs,
netdev, linux-kernel
In-Reply-To: <20231027095842.GA30868@redhat.com>
Oleg Nesterov <oleg@redhat.com> wrote:
> read_seqbegin_or_lock() makes no sense unless you make "seq" odd
> after the lockless access failed.
I think you're wrong.
write_seqlock() turns it odd. For instance, if the read lock is taken first:
sequence seq CPU 1 CPU 2
======= ======= =============================== ===============
0
0 0 seq = 0 // MUST BE EVEN ACCORDING TO DOC
0 0 read_seqbegin_or_lock() [lockless]
...
1 0 write_seqlock()
1 0 need_seqretry() [seq=even; sequence!=seq: retry]
1 1 read_seqbegin_or_lock() [exclusive]
-->spin_lock(lock);
2 1 write_sequnlock()
<--locked
...
2 1 need_seqretry()
However, if the write lock is taken first:
sequence seq CPU 1 CPU 2
======= ======= =============================== ===============
0
1 write_seqlock()
1 0 seq = 0 // MUST BE EVEN ACCORDING TO DOC
1 0 read_seqbegin_or_lock() [lockless]
1 0 __read_seqcount_begin()
while (lock.sequence is odd)
cpu_relax();
2 0 write_sequnlock()
2 2 [loop end]
...
2 2 need_seqretry() [seq=even; sequence==seq; done]
Note that it spins in __read_seqcount_begin() until we get an even seq,
indicating that no write is currently in progress - at which point we can
perform a lockless pass.
> See thread_group_cputime() as an example, note that it does nextseq = 1 for
> the 2nd round.
That's not especially convincing.
David
^ permalink raw reply
* Re: [net-next PATCH v2 1/2] net: phy: aquantia: add firmware load support
From: Christian Marangi @ 2023-11-01 15:51 UTC (permalink / raw)
To: Andrew Lunn
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiner Kallweit,
Russell King, netdev, devicetree, linux-kernel, Robert Marko
In-Reply-To: <4b536ad3-2112-4f28-90e4-586b5745be20@lunn.ch>
On Wed, Nov 01, 2023 at 02:13:05PM +0100, Andrew Lunn wrote:
> On Wed, Nov 01, 2023 at 01:36:07PM +0100, Christian Marangi wrote:
> > 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 v2:
> > - Move out of RFC
>
> Actually, since we are in the merge window, RFC would be correct.
>
My bad!
> > - 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/Kconfig | 1 +
> > drivers/net/phy/aquantia_main.c | 304 ++++++++++++++++++++++++++++++++
> > 2 files changed, 305 insertions(+)
> >
> > diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> > index 421d2b62918f..46c7194efcea 100644
> > --- a/drivers/net/phy/Kconfig
> > +++ b/drivers/net/phy/Kconfig
> > @@ -98,6 +98,7 @@ config ADIN1100_PHY
> >
> > 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_main.c b/drivers/net/phy/aquantia_main.c
> > index 334a6904ca5a..0f1b8d75cca0 100644
> > --- a/drivers/net/phy/aquantia_main.c
> > +++ b/drivers/net/phy/aquantia_main.c
> > @@ -12,6 +12,10 @@
> > #include <linux/delay.h>
> > #include <linux/bitfield.h>
> > #include <linux/phy.h>
> > +#include <linux/of.h>
> > +#include <linux/firmware.h>
> > +#include <linux/crc-ccitt.h>
> > +#include <linux/nvmem-consumer.h>
> >
> > #include "aquantia.h"
> >
> > @@ -92,10 +96,40 @@
> > #define MDIO_C22EXT_STAT_SGMII_TX_RUNT_FRAMES 0xd31b
> >
> > /* 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))
> > +
> > +#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_GLOBAL_GEN_STAT2 0xc831
> > #define VEND1_GLOBAL_GEN_STAT2_OP_IN_PROG BIT(15)
> >
> > @@ -152,6 +186,30 @@
> > #define AQR107_OP_IN_PROG_SLEEP 1000
> > #define AQR107_OP_IN_PROG_TIMEOUT 100000
> >
> > +#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;
> > +
> > struct aqr107_hw_stat {
> > const char *name;
> > int reg;
> > @@ -677,6 +735,166 @@ static int aqr107_wait_processor_intensive_op(struct phy_device *phydev)
> > return 0;
> > }
> >
> > +/* load data into the phy's memory */
> > +static int aquantia_load_memory(struct phy_device *phydev, u32 addr,
> > + const u8 *data, size_t len)
> > +{
>
> > + for (pos = 0; pos < len; pos += min(sizeof(u32), len - pos)) {
> > + u32 word = 0;
> > +
> > + memcpy(&word, data + pos, min(sizeof(u32), len - pos));
>
> Rather than do a memcpy, use the get_unaligned_ macros. They might map
> to a memcpy(), but some architectures can do unaligned accesses
> without problems.
>
I don't think this is doable for this loop, think we would end up in
some funny situation where for the last run we have to copy less than
u32. (get_unaligned would always take u32 of data and that would end up
reading more than requested) Am I wrong?
Aside from this, in the other part of the code I can use the macro and
skip having to convert them.
> > +static int aqr_fw_boot(struct phy_device *phydev, const u8 *data, size_t size)
> > +{
> > + const struct aqr_fw_header *header;
> > + u32 iram_offset = 0, iram_size = 0;
> > + u32 dram_offset = 0, dram_size = 0;
> > + char version[VERSION_STRING_SIZE];
> > + u16 calculated_crc, read_crc;
> > + u32 primary_offset = 0;
> > + int ret;
> > +
> > + /* extract saved CRC at the end of the fw */
> > + memcpy(&read_crc, data + size - 2, sizeof(read_crc));
>
> Say size == 1. You just had a buffer underrun.
>
> > + /* CRC is saved in big-endian as PHY is BE */
> > + read_crc = be16_to_cpu(read_crc);
> > + calculated_crc = crc_ccitt_false(0, data, size - 2);
> > + 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. */
> > + memcpy(&primary_offset, data + PRIMARY_OFFSET_OFFSET, sizeof(u16));
>
> What if PRIMARY_OFFSET_OFFSET + sizeof(u16) is greater than size? A
> buffer overrun.
>
> Assume the firmware is evil and is trying to hack you. Always test
> everything.
>
> I would suggest some helpers, something like
>
> int aqr_fw_get_u16(const u8 *data, size_t size, size_t offset, u16 *value)
>
> Check that offset + sizeof(u16) is within the firmware, and if not return -EINVAL.
> Otherwise set *value to the u16 from the firmware and return 0.
>
> This is where Rust would be nice :-)
>
> Andrew
>
> ---
> pw-bot: cr
--
Ansuel
^ permalink raw reply
* Re: [syzbot] [net?] KASAN: slab-use-after-free Read in ptp_release
From: syzbot @ 2023-11-01 16:03 UTC (permalink / raw)
To: linux-kernel, netdev, richardcochran, syzkaller-bugs
In-Reply-To: <000000000000ffc87a06086172a0@google.com>
syzbot has found a reproducer for the following issue on:
HEAD commit: 89ed67ef126c Merge tag 'net-next-6.7' of git://git.kernel...
git tree: upstream
console+strace: https://syzkaller.appspot.com/x/log.txt?x=150b105f680000
kernel config: https://syzkaller.appspot.com/x/.config?x=6e3b1d98cf5a2cca
dashboard link: https://syzkaller.appspot.com/bug?extid=8a676a50d4eee2f21539
compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=13dd173b680000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=16ce0840e80000
Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/b69c238dd56a/disk-89ed67ef.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/f555d654a8ba/vmlinux-89ed67ef.xz
kernel image: https://storage.googleapis.com/syzbot-assets/335bbfb6c442/bzImage-89ed67ef.xz
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+8a676a50d4eee2f21539@syzkaller.appspotmail.com
list_del corruption. next->prev should be ffff88802a019048, but was ffff88802a401048. (next=ffff88802515e5e8)
------------[ cut here ]------------
kernel BUG at lib/list_debug.c:67!
invalid opcode: 0000 [#1] PREEMPT SMP KASAN
CPU: 1 PID: 6526 Comm: syz-executor155 Not tainted 6.6.0-syzkaller-05843-g89ed67ef126c #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 10/09/2023
RIP: 0010:__list_del_entry_valid_or_report+0x122/0x130 lib/list_debug.c:65
Code: 85 06 0f 0b 48 c7 c7 20 5f 9d 8b 4c 89 fe 48 89 d9 e8 52 db 85 06 0f 0b 48 c7 c7 a0 5f 9d 8b 4c 89 fe 4c 89 f1 e8 3e db 85 06 <0f> 0b 66 2e 0f 1f 84 00 00 00 00 00 66 90 f3 0f 1e fa 80 3d 1d 6e
RSP: 0018:ffffc90009d67b18 EFLAGS: 00010046
RAX: 000000000000006d RBX: ffff88802515e5f0 RCX: 58e87076363f5a00
RDX: 0000000000000000 RSI: 0000000080000001 RDI: 0000000000000000
RBP: ffff88802a019008 R08: ffffffff81717aac R09: 1ffff920013acf04
R10: dffffc0000000000 R11: fffff520013acf05 R12: dffffc0000000000
R13: ffff88802a018000 R14: ffff88802515e5e8 R15: ffff88802a019048
FS: 0000000000000000(0000) GS:ffff8880b9900000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007ff08f2c4110 CR3: 0000000074918000 CR4: 00000000003506f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
<TASK>
__list_del_entry_valid include/linux/list.h:124 [inline]
__list_del_entry include/linux/list.h:215 [inline]
list_del include/linux/list.h:229 [inline]
ptp_release+0xa8/0x1e0 drivers/ptp/ptp_chardev.c:147
posix_clock_release+0x8c/0x100 kernel/time/posix-clock.c:157
__fput+0x3cc/0xa10 fs/file_table.c:394
task_work_run+0x24a/0x300 kernel/task_work.c:180
exit_task_work include/linux/task_work.h:38 [inline]
do_exit+0xa2c/0x2650 kernel/exit.c:874
do_group_exit+0x206/0x2c0 kernel/exit.c:1024
__do_sys_exit_group kernel/exit.c:1035 [inline]
__se_sys_exit_group kernel/exit.c:1033 [inline]
__x64_sys_exit_group+0x3f/0x40 kernel/exit.c:1033
do_syscall_x64 arch/x86/entry/common.c:51 [inline]
do_syscall_64+0x44/0x110 arch/x86/entry/common.c:82
entry_SYSCALL_64_after_hwframe+0x63/0x6b
RIP: 0033:0x7ff08f248bf9
Code: Unable to access opcode bytes at 0x7ff08f248bcf.
RSP: 002b:00007ffc5a8eb438 EFLAGS: 00000246 ORIG_RAX: 00000000000000e7
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007ff08f248bf9
RDX: 000000000000003c RSI: 00000000000000e7 RDI: 0000000000000000
RBP: 00007ff08f2c3290 R08: ffffffffffffffb8 R09: 00000000000000a0
R10: 0000000000000000 R11: 0000000000000246 R12: 00007ff08f2c3290
R13: 0000000000000000 R14: 00007ff08f2c3d00 R15: 00007ff08f219da0
</TASK>
Modules linked in:
---[ end trace 0000000000000000 ]---
RIP: 0010:__list_del_entry_valid_or_report+0x122/0x130 lib/list_debug.c:65
Code: 85 06 0f 0b 48 c7 c7 20 5f 9d 8b 4c 89 fe 48 89 d9 e8 52 db 85 06 0f 0b 48 c7 c7 a0 5f 9d 8b 4c 89 fe 4c 89 f1 e8 3e db 85 06 <0f> 0b 66 2e 0f 1f 84 00 00 00 00 00 66 90 f3 0f 1e fa 80 3d 1d 6e
RSP: 0018:ffffc90009d67b18 EFLAGS: 00010046
RAX: 000000000000006d RBX: ffff88802515e5f0 RCX: 58e87076363f5a00
RDX: 0000000000000000 RSI: 0000000080000001 RDI: 0000000000000000
RBP: ffff88802a019008 R08: ffffffff81717aac R09: 1ffff920013acf04
R10: dffffc0000000000 R11: fffff520013acf05 R12: dffffc0000000000
R13: ffff88802a018000 R14: ffff88802515e5e8 R15: ffff88802a019048
FS: 0000000000000000(0000) GS:ffff8880b9900000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007ff08f2c4110 CR3: 0000000074918000 CR4: 00000000003506f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
---
If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.
^ permalink raw reply
* Re: [net-next PATCH v2 1/2] net: phy: aquantia: add firmware load support
From: Andrew Lunn @ 2023-11-01 16:32 UTC (permalink / raw)
To: Christian Marangi
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiner Kallweit,
Russell King, netdev, devicetree, linux-kernel, Robert Marko
In-Reply-To: <65427400.5d0a0220.41c58.0ded@mx.google.com>
> > > + for (pos = 0; pos < len; pos += min(sizeof(u32), len - pos)) {
> > > + u32 word = 0;
> > > +
> > > + memcpy(&word, data + pos, min(sizeof(u32), len - pos));
> >
> > Rather than do a memcpy, use the get_unaligned_ macros. They might map
> > to a memcpy(), but some architectures can do unaligned accesses
> > without problems.
> >
>
> I don't think this is doable for this loop, think we would end up in
> some funny situation where for the last run we have to copy less than
> u32. (get_unaligned would always take u32 of data and that would end up
> reading more than requested) Am I wrong?
Does it happen in practice that the last chunk is not 4 bytes? Since
this is firmware, its probably produced by some sort of linker, and
they often round segments to words. Could you take a look at the
firmware images you have access to and see if this is true?
It could be we do need to keep with the memcpy, but it would be nice
if we could limit it to words, at least until somebody has a firmware
which is not word aligned.
Andrew
^ permalink raw reply
* Re: [net-next PATCH v2 1/2] net: phy: aquantia: add firmware load support
From: Christian Marangi @ 2023-11-01 16:41 UTC (permalink / raw)
To: Andrew Lunn
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiner Kallweit,
Russell King, netdev, devicetree, linux-kernel, Robert Marko
In-Reply-To: <34a0b76e-aa0e-4148-ba01-c3b4608f17f7@lunn.ch>
On Wed, Nov 01, 2023 at 05:32:29PM +0100, Andrew Lunn wrote:
> > > > + for (pos = 0; pos < len; pos += min(sizeof(u32), len - pos)) {
> > > > + u32 word = 0;
> > > > +
> > > > + memcpy(&word, data + pos, min(sizeof(u32), len - pos));
> > >
> > > Rather than do a memcpy, use the get_unaligned_ macros. They might map
> > > to a memcpy(), but some architectures can do unaligned accesses
> > > without problems.
> > >
> >
> > I don't think this is doable for this loop, think we would end up in
> > some funny situation where for the last run we have to copy less than
> > u32. (get_unaligned would always take u32 of data and that would end up
> > reading more than requested) Am I wrong?
>
> Does it happen in practice that the last chunk is not 4 bytes? Since
> this is firmware, its probably produced by some sort of linker, and
> they often round segments to words. Could you take a look at the
> firmware images you have access to and see if this is true?
>
> It could be we do need to keep with the memcpy, but it would be nice
> if we could limit it to words, at least until somebody has a firmware
> which is not word aligned.
>
There are plenty of firmware around so it can be checked by from what I
have, it looks like they are word aligned... Ok I will use the
get_unaligned and add a comment saying that we assume the iram and dram
section are always word aligned.
Is it ok?
--
Ansuel
^ permalink raw reply
* Re: [net-next PATCH v2 1/2] net: phy: aquantia: add firmware load support
From: Andrew Lunn @ 2023-11-01 16:54 UTC (permalink / raw)
To: Christian Marangi
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiner Kallweit,
Russell King, netdev, devicetree, linux-kernel, Robert Marko
In-Reply-To: <65427fd4.df0a0220.28d26.1955@mx.google.com>
> There are plenty of firmware around so it can be checked by from what I
> have, it looks like they are word aligned... Ok I will use the
> get_unaligned and add a comment saying that we assume the iram and dram
> section are always word aligned.
We probably want to know if there is firmware out there which is not
word aligned. So i would probably do phydev_err() and return -EINVAL.
Andrew
^ permalink raw reply
* Re: [net-next PATCH v2 1/2] net: phy: aquantia: add firmware load support
From: Heiner Kallweit @ 2023-11-01 16:57 UTC (permalink / raw)
To: Christian Marangi
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Andrew Lunn,
Russell King, netdev, devicetree, linux-kernel, Robert Marko
In-Reply-To: <65424cd9.5d0a0220.20d9a.fe0f@mx.google.com>
On 01.11.2023 13:57, Christian Marangi wrote:
> On Wed, Nov 01, 2023 at 02:01:33PM +0100, Heiner Kallweit wrote:
>> On 01.11.2023 13:36, Christian Marangi wrote:
>>> 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 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
>>>
>>
>> To make the driver better maintainable: can the firmware handling code
>> be placed in a separate source code file, similar to what has been done
>> for the hwmon part?
>> If yes, then this could also be the right time to move the aquantia
>> driver to an own subdirectory.
>>
>
> Sure! Np for me just is it really worth it? hwmod is a bigger one but
> this is really a few functions.
>
r8169_firmware.c is even smaller and I've never regretted having it factored
out. Whether it makes sense depends on how much you share with the main module
and how the API is structured that you provide to the main module.
So I don't say you have to do it, I'm just saying it's worth considering it.
> Anyway if requested, I will move in v3 the driver to a dedicated
> directory and move the function to a separate file!
>
^ permalink raw reply
* Re: [net-next PATCH v2 1/2] net: phy: aquantia: add firmware load support
From: Christian Marangi @ 2023-11-01 17:08 UTC (permalink / raw)
To: Andrew Lunn
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiner Kallweit,
Russell King, netdev, devicetree, linux-kernel, Robert Marko
In-Reply-To: <c9dad91a-1de1-4c30-ab7f-414552702009@lunn.ch>
On Wed, Nov 01, 2023 at 05:54:50PM +0100, Andrew Lunn wrote:
> > There are plenty of firmware around so it can be checked by from what I
> > have, it looks like they are word aligned... Ok I will use the
> > get_unaligned and add a comment saying that we assume the iram and dram
> > section are always word aligned.
>
> We probably want to know if there is firmware out there which is not
> word aligned. So i would probably do phydev_err() and return -EINVAL.
>
Do we have API to check this? Or I think I should just check the iram
and dram size and see if iram_size % sizeof(u32) is zero and return
error otherwise.
--
Ansuel
^ permalink raw reply
* Re: [net-next PATCH v2 1/2] net: phy: aquantia: add firmware load support
From: Christian Marangi @ 2023-11-01 17:09 UTC (permalink / raw)
To: Heiner Kallweit
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Andrew Lunn,
Russell King, netdev, devicetree, linux-kernel, Robert Marko
In-Reply-To: <f5f72cc3-0435-4ba0-8291-30d1ec2633a0@gmail.com>
On Wed, Nov 01, 2023 at 05:57:50PM +0100, Heiner Kallweit wrote:
> On 01.11.2023 13:57, Christian Marangi wrote:
> > On Wed, Nov 01, 2023 at 02:01:33PM +0100, Heiner Kallweit wrote:
> >> On 01.11.2023 13:36, Christian Marangi wrote:
> >>> 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 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
> >>>
> >>
> >> To make the driver better maintainable: can the firmware handling code
> >> be placed in a separate source code file, similar to what has been done
> >> for the hwmon part?
> >> If yes, then this could also be the right time to move the aquantia
> >> driver to an own subdirectory.
> >>
> >
> > Sure! Np for me just is it really worth it? hwmod is a bigger one but
> > this is really a few functions.
> >
> r8169_firmware.c is even smaller and I've never regretted having it factored
> out. Whether it makes sense depends on how much you share with the main module
> and how the API is structured that you provide to the main module.
> So I don't say you have to do it, I'm just saying it's worth considering it.
>
Already done! Will be part of this series with v3 :D
> > Anyway if requested, I will move in v3 the driver to a dedicated
> > directory and move the function to a separate file!
> >
>
--
Ansuel
^ permalink raw reply
* Re: [PATCH v2 net] net/tcp: fix possible out-of-bounds reads in tcp_hash_fail()
From: Dmitry Safonov @ 2023-11-01 17:12 UTC (permalink / raw)
To: Eric Dumazet, David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: netdev, eric.dumazet, syzbot, Francesco Ruggeri, David Ahern
In-Reply-To: <20231101045233.3387072-1-edumazet@google.com>
On 11/1/23 04:52, Eric Dumazet 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.
>
> BUG: KASAN: stack-out-of-bounds in string_nocheck lib/vsprintf.c:645 [inline]
> BUG: KASAN: stack-out-of-bounds in string+0x394/0x3d0 lib/vsprintf.c:727
> Read of size 1 at addr ffffc9000397f3f5 by task syz-executor299/5039
>
> CPU: 1 PID: 5039 Comm: syz-executor299 Not tainted 6.6.0-rc7-syzkaller-02075-g55c900477f5b #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 10/09/2023
> Call Trace:
> <TASK>
> __dump_stack lib/dump_stack.c:88 [inline]
> dump_stack_lvl+0xd9/0x1b0 lib/dump_stack.c:106
> print_address_description mm/kasan/report.c:364 [inline]
> print_report+0xc4/0x620 mm/kasan/report.c:475
> kasan_report+0xda/0x110 mm/kasan/report.c:588
> string_nocheck lib/vsprintf.c:645 [inline]
> string+0x394/0x3d0 lib/vsprintf.c:727
> vsnprintf+0xc5f/0x1870 lib/vsprintf.c:2818
> vprintk_store+0x3a0/0xb80 kernel/printk/printk.c:2191
> vprintk_emit+0x14c/0x5f0 kernel/printk/printk.c:2288
> vprintk+0x7b/0x90 kernel/printk/printk_safe.c:45
> _printk+0xc8/0x100 kernel/printk/printk.c:2332
> tcp_inbound_hash.constprop.0+0xdb2/0x10d0 include/net/tcp.h:2760
> tcp_v6_rcv+0x2b31/0x34d0 net/ipv6/tcp_ipv6.c:1882
> ip6_protocol_deliver_rcu+0x33b/0x13d0 net/ipv6/ip6_input.c:438
> ip6_input_finish+0x14f/0x2f0 net/ipv6/ip6_input.c:483
> NF_HOOK include/linux/netfilter.h:314 [inline]
> NF_HOOK include/linux/netfilter.h:308 [inline]
> ip6_input+0xce/0x440 net/ipv6/ip6_input.c:492
> dst_input include/net/dst.h:461 [inline]
> ip6_rcv_finish net/ipv6/ip6_input.c:79 [inline]
> NF_HOOK include/linux/netfilter.h:314 [inline]
> NF_HOOK include/linux/netfilter.h:308 [inline]
> ipv6_rcv+0x563/0x720 net/ipv6/ip6_input.c:310
> __netif_receive_skb_one_core+0x115/0x180 net/core/dev.c:5527
> __netif_receive_skb+0x1f/0x1b0 net/core/dev.c:5641
> netif_receive_skb_internal net/core/dev.c:5727 [inline]
> netif_receive_skb+0x133/0x700 net/core/dev.c:5786
> tun_rx_batched+0x429/0x780 drivers/net/tun.c:1579
> tun_get_user+0x29e7/0x3bc0 drivers/net/tun.c:2002
> tun_chr_write_iter+0xe8/0x210 drivers/net/tun.c:2048
> call_write_iter include/linux/fs.h:1956 [inline]
> new_sync_write fs/read_write.c:491 [inline]
> vfs_write+0x650/0xe40 fs/read_write.c:584
> ksys_write+0x12f/0x250 fs/read_write.c:637
> do_syscall_x64 arch/x86/entry/common.c:50 [inline]
> do_syscall_64+0x38/0xb0 arch/x86/entry/common.c:80
> entry_SYSCALL_64_after_hwframe+0x63/0xcd
>
> Fixes: 2717b5adea9e ("net/tcp: Add tcp_hash_fail() ratelimited logs")
> Reported-by: syzbot <syzkaller@googlegroups.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Dmitry Safonov <dima@arista.com>
> Cc: Francesco Ruggeri <fruggeri@arista.com>
> Cc: David Ahern <dsahern@kernel.org>
LGTM, thanks again!
Reviewed-by: Dmitry Safonov <dima@arista.com>
> ---
> include/net/tcp_ao.h | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/include/net/tcp_ao.h b/include/net/tcp_ao.h
> index a375a171ef3cb37ab1d8246c72c6a3e83f5c9184..b56be10838f09a2cb56ab511242d2b583eb4c33b 100644
> --- a/include/net/tcp_ao.h
> +++ b/include/net/tcp_ao.h
> @@ -124,7 +124,7 @@ struct tcp_ao_info {
> #define tcp_hash_fail(msg, family, skb, fmt, ...) \
> do { \
> const struct tcphdr *th = tcp_hdr(skb); \
> - char hdr_flags[5] = {}; \
> + char hdr_flags[6]; \
> char *f = hdr_flags; \
> \
> if (th->fin) \
> @@ -133,17 +133,18 @@ do { \
> *f++ = 'S'; \
> if (th->rst) \
> *f++ = 'R'; \
> + if (th->psh) \
> + *f++ = 'P'; \
> if (th->ack) \
> - *f++ = 'A'; \
> - if (f != hdr_flags) \
> - *f = ' '; \
> + *f++ = '.'; \
> + *f = 0; \
> if ((family) == AF_INET) { \
> - net_info_ratelimited("%s for (%pI4, %d)->(%pI4, %d) %s" fmt "\n", \
> + net_info_ratelimited("%s for %pI4.%d->%pI4.%d [%s] " fmt "\n", \
> msg, &ip_hdr(skb)->saddr, ntohs(th->source), \
> &ip_hdr(skb)->daddr, ntohs(th->dest), \
> hdr_flags, ##__VA_ARGS__); \
> } else { \
> - net_info_ratelimited("%s for [%pI6c]:%u->[%pI6c]:%u %s" fmt "\n", \
> + net_info_ratelimited("%s for [%pI6c].%d->[%pI6c].%d [%s]" fmt "\n", \
> msg, &ipv6_hdr(skb)->saddr, ntohs(th->source), \
> &ipv6_hdr(skb)->daddr, ntohs(th->dest), \
> hdr_flags, ##__VA_ARGS__); \
--
Dmitry
^ permalink raw reply
* Re: [RFC bpf-next 1/6] bpf: xfrm: Add bpf_xdp_get_xfrm_state() kfunc
From: Daniel Xu @ 2023-11-01 17:51 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Jesper Dangaard Brouer, Steffen Klassert, Alexei Starovoitov,
Paolo Abeni, Daniel Borkmann, Jakub Kicinski, Herbert Xu,
David S. Miller, John Fastabend, Eric Dumazet, antony.antony,
LKML, Network Development, bpf, devel
In-Reply-To: <CAADnVQJkfAGG9_868tLW9m-9V2husAaRK5afnrLL1HqaxN_3vQ@mail.gmail.com>
On Tue, Oct 31, 2023 at 03:38:26PM -0700, Alexei Starovoitov wrote:
> On Sun, Oct 29, 2023 at 3:55 PM Daniel Xu <dxu@dxuuu.xyz> wrote:
> >
> > Hi Alexei,
> >
> > On Sat, Oct 28, 2023 at 04:49:45PM -0700, Alexei Starovoitov wrote:
> > > On Fri, Oct 27, 2023 at 11:46 AM Daniel Xu <dxu@dxuuu.xyz> wrote:
> > > >
> > > > This commit adds an unstable kfunc helper to access internal xfrm_state
> > > > associated with an SA. This is intended to be used for the upcoming
> > > > IPsec pcpu work to assign special pcpu SAs to a particular CPU. In other
> > > > words: for custom software RSS.
> > > >
> > > > That being said, the function that this kfunc wraps is fairly generic
> > > > and used for a lot of xfrm tasks. I'm sure people will find uses
> > > > elsewhere over time.
> > > >
> > > > Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>
> > > > ---
> > > > include/net/xfrm.h | 9 ++++
> > > > net/xfrm/Makefile | 1 +
> > > > net/xfrm/xfrm_policy.c | 2 +
> > > > net/xfrm/xfrm_state_bpf.c | 105 ++++++++++++++++++++++++++++++++++++++
> > > > 4 files changed, 117 insertions(+)
> > > > create mode 100644 net/xfrm/xfrm_state_bpf.c
> > > >
> > > > diff --git a/include/net/xfrm.h b/include/net/xfrm.h
> > > > index 98d7aa78adda..ab4cf66480f3 100644
> > > > --- a/include/net/xfrm.h
> > > > +++ b/include/net/xfrm.h
> > > > @@ -2188,4 +2188,13 @@ static inline int register_xfrm_interface_bpf(void)
> > > >
> > > > #endif
> > > >
> > > > +#if IS_ENABLED(CONFIG_DEBUG_INFO_BTF)
> > > > +int register_xfrm_state_bpf(void);
> > > > +#else
> > > > +static inline int register_xfrm_state_bpf(void)
> > > > +{
> > > > + return 0;
> > > > +}
> > > > +#endif
> > > > +
> > > > #endif /* _NET_XFRM_H */
> > > > diff --git a/net/xfrm/Makefile b/net/xfrm/Makefile
> > > > index cd47f88921f5..547cec77ba03 100644
> > > > --- a/net/xfrm/Makefile
> > > > +++ b/net/xfrm/Makefile
> > > > @@ -21,3 +21,4 @@ obj-$(CONFIG_XFRM_USER_COMPAT) += xfrm_compat.o
> > > > obj-$(CONFIG_XFRM_IPCOMP) += xfrm_ipcomp.o
> > > > obj-$(CONFIG_XFRM_INTERFACE) += xfrm_interface.o
> > > > obj-$(CONFIG_XFRM_ESPINTCP) += espintcp.o
> > > > +obj-$(CONFIG_DEBUG_INFO_BTF) += xfrm_state_bpf.o
> > > > diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
> > > > index 5cdd3bca3637..62e64fa7ae5c 100644
> > > > --- a/net/xfrm/xfrm_policy.c
> > > > +++ b/net/xfrm/xfrm_policy.c
> > > > @@ -4267,6 +4267,8 @@ void __init xfrm_init(void)
> > > > #ifdef CONFIG_XFRM_ESPINTCP
> > > > espintcp_init();
> > > > #endif
> > > > +
> > > > + register_xfrm_state_bpf();
> > > > }
> > > >
> > > > #ifdef CONFIG_AUDITSYSCALL
> > > > diff --git a/net/xfrm/xfrm_state_bpf.c b/net/xfrm/xfrm_state_bpf.c
> > > > new file mode 100644
> > > > index 000000000000..a73a17a6497b
> > > > --- /dev/null
> > > > +++ b/net/xfrm/xfrm_state_bpf.c
> > > > @@ -0,0 +1,105 @@
> > > > +// SPDX-License-Identifier: GPL-2.0-only
> > > > +/* Unstable XFRM state BPF helpers.
> > > > + *
> > > > + * Note that it is allowed to break compatibility for these functions since the
> > > > + * interface they are exposed through to BPF programs is explicitly unstable.
> > > > + */
> > > > +
> > > > +#include <linux/bpf.h>
> > > > +#include <linux/btf_ids.h>
> > > > +#include <net/xdp.h>
> > > > +#include <net/xfrm.h>
> > > > +
> > > > +/* bpf_xfrm_state_opts - Options for XFRM state lookup helpers
> > > > + *
> > > > + * Members:
> > > > + * @error - Out parameter, set for any errors encountered
> > > > + * Values:
> > > > + * -EINVAL - netns_id is less than -1
> > > > + * -EINVAL - Passed NULL for opts
> > > > + * -EINVAL - opts__sz isn't BPF_XFRM_STATE_OPTS_SZ
> > > > + * -ENONET - No network namespace found for netns_id
> > > > + * @netns_id - Specify the network namespace for lookup
> > > > + * Values:
> > > > + * BPF_F_CURRENT_NETNS (-1)
> > > > + * Use namespace associated with ctx
> > > > + * [0, S32_MAX]
> > > > + * Network Namespace ID
> > > > + * @mark - XFRM mark to match on
> > > > + * @daddr - Destination address to match on
> > > > + * @spi - Security parameter index to match on
> > > > + * @proto - L3 protocol to match on
> > > > + * @family - L3 protocol family to match on
> > > > + */
> > > > +struct bpf_xfrm_state_opts {
> > > > + s32 error;
> > > > + s32 netns_id;
> > > > + u32 mark;
> > > > + xfrm_address_t daddr;
> > > > + __be32 spi;
> > > > + u8 proto;
> > > > + u16 family;
> > > > +};
> > > > +
> > > > +enum {
> > > > + BPF_XFRM_STATE_OPTS_SZ = sizeof(struct bpf_xfrm_state_opts),
> > > > +};
> > > > +
> > > > +__diag_push();
> > > > +__diag_ignore_all("-Wmissing-prototypes",
> > > > + "Global functions as their definitions will be in xfrm_state BTF");
> > > > +
> > > > +/* bpf_xdp_get_xfrm_state - Get XFRM state
> > > > + *
> > > > + * Parameters:
> > > > + * @ctx - Pointer to ctx (xdp_md) in XDP program
> > > > + * Cannot be NULL
> > > > + * @opts - Options for lookup (documented above)
> > > > + * Cannot be NULL
> > > > + * @opts__sz - Length of the bpf_xfrm_state_opts structure
> > > > + * Must be BPF_XFRM_STATE_OPTS_SZ
> > > > + */
> > > > +__bpf_kfunc struct xfrm_state *
> > > > +bpf_xdp_get_xfrm_state(struct xdp_md *ctx, struct bpf_xfrm_state_opts *opts, u32 opts__sz)
> > > > +{
> > > > + struct xdp_buff *xdp = (struct xdp_buff *)ctx;
> > > > + struct net *net = dev_net(xdp->rxq->dev);
> > > > +
> > > > + if (!opts || opts__sz != BPF_XFRM_STATE_OPTS_SZ) {
> > > > + opts->error = -EINVAL;
> > > > + return NULL;
> > > > + }
> > > > +
> > > > + if (unlikely(opts->netns_id < BPF_F_CURRENT_NETNS)) {
> > > > + opts->error = -EINVAL;
> > > > + return NULL;
> > > > + }
> > > > +
> > > > + if (opts->netns_id >= 0) {
> > > > + net = get_net_ns_by_id(net, opts->netns_id);
> > > > + if (unlikely(!net)) {
> > > > + opts->error = -ENONET;
> > > > + return NULL;
> > > > + }
> > > > + }
> > > > +
> > > > + return xfrm_state_lookup(net, opts->mark, &opts->daddr, opts->spi,
> > > > + opts->proto, opts->family);
> > > > +}
> > >
> > > Patch 6 example does little to explain how this kfunc can be used.
> > > Cover letter sounds promising, but no code to demonstrate the result.
> >
> > Part of the reason for that is this kfunc is intended to be used with a
> > not-yet-upstreamed xfrm patchset. The other is that the usage is quite
> > trivial. This is the code the experiments were run with:
> >
> > https://github.com/danobi/xdp-tools/blob/e89a1c617aba3b50d990f779357d6ce2863ecb27/xdp-bench/xdp_redirect_cpumap.bpf.c#L385-L406
> >
> > We intend to upstream that cpumap mode to xdp-tools as soon as the xfrm
> > patches are in. (Note the linked code is a little buggy but the
> > main idea is there).
>
> I don't understand how it survives anything, but sanity check.
> To measure perf gains it needs to be under traffic for some time,
> but
> x = bpf_xdp_get_xfrm_state(ctx, &opts, sizeof(opts));
> will keep refcnt++ that state for every packet.
> Minimum -> memory leak or refcnt overflow.
Yeah, I agree the code in this patchset is not correct. I have the fix
(a KF_RELEASE wrapper around xfrm_state_put()) ready to send. I think
Steffen was gonna chat w/ you about this at IETF next week. But I can
send it now if you'd like.
To answer your question why it doesn't blow up immediately:
* The test system only has ~33 inbound SAs and the test doesn't try to
delete any. So leak is not noticed in the test. Oddly enough I recall
`ip x s flush` working correctly... Could be misremembering.
* Refcnt overflow will indeed happen, but some rough math shows it'll
take about 12 hrs receiving at 100Gbps for that to happen. 100Gbps =
12.5 GB/s. 12.5GB / (32 CPUs) / (9000B) = 43k pps for each pcpu SA.
INT_MAX = 2 billion. 2B / 4k = 46k. 46k seconds to hours is ~12 hrs.
And I was only running traffic for ~1 hour.
At least I think that math is right.
Thanks,
Daniel
^ permalink raw reply
* Re: [RFC bpf-next 1/6] bpf: xfrm: Add bpf_xdp_get_xfrm_state() kfunc
From: Alexei Starovoitov @ 2023-11-01 18:51 UTC (permalink / raw)
To: Daniel Xu
Cc: Jesper Dangaard Brouer, Steffen Klassert, Alexei Starovoitov,
Paolo Abeni, Daniel Borkmann, Jakub Kicinski, Herbert Xu,
David S. Miller, John Fastabend, Eric Dumazet, antony.antony,
LKML, Network Development, bpf, devel
In-Reply-To: <fzgysfsfgeqq3tzy2yqrqjibu542qtfi75fcnbxkivsiajaiys@ddd4vftvtwse>
On Wed, Nov 1, 2023 at 10:51 AM Daniel Xu <dxu@dxuuu.xyz> wrote:
>
> Yeah, I agree the code in this patchset is not correct. I have the fix
> (a KF_RELEASE wrapper around xfrm_state_put()) ready to send. I think
> Steffen was gonna chat w/ you about this at IETF next week. But I can
> send it now if you'd like.
I say send a new version with all issues addressed now, since
it might help to frame the discussion at IETF.
>
> To answer your question why it doesn't blow up immediately:
>
> * The test system only has ~33 inbound SAs and the test doesn't try to
> delete any. So leak is not noticed in the test. Oddly enough I recall
> `ip x s flush` working correctly... Could be misremembering.
>
> * Refcnt overflow will indeed happen, but some rough math shows it'll
> take about 12 hrs receiving at 100Gbps for that to happen. 100Gbps =
> 12.5 GB/s. 12.5GB / (32 CPUs) / (9000B) = 43k pps for each pcpu SA.
> INT_MAX = 2 billion. 2B / 4k = 46k. 46k seconds to hours is ~12 hrs.
> And I was only running traffic for ~1 hour.
>
> At least I think that math is right.
Makes sense.
^ permalink raw reply
* Re: [REGRESSION] Userland interface breaks due to hard HFSC_FSC requirement
From: Linux regression tracking #update (Thorsten Leemhuis) @ 2023-11-01 19:05 UTC (permalink / raw)
To: stable, netdev; +Cc: regressions
In-Reply-To: <297D84E3-736E-4AB4-B825-264279E2043C@flyingcircus.io>
[TLDR: This mail in primarily relevant for Linux kernel regression
tracking. See link in footer if these mails annoy you.]
On 06.10.23 10:37, Christian Theune wrote:
>
> I upgraded from 6.1.38 to 6.1.55 this morning and it broke my traffic shaping script, leaving me with a non-functional uplink on a remote router.
> [...]
> #regzbot introduced: a1e820fc7808e42b990d224f40e9b4895503ac40
#regzbot fix: net/sched: sch_hfsc: upgrade rt to sc when it becomes a
inner curve
#regzbot ignore-activity
Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
That page also explains what to do if mails like this annoy you.
^ permalink raw reply
* [PATCH 2/2] tg3: Fix the TX ring stall
From: alexey.pakhunov @ 2023-11-01 19:18 UTC (permalink / raw)
To: siva.kallam
Cc: vincent.wong2, netdev, linux-kernel, prashant, mchan,
Alex Pakhunov
In-Reply-To: <20231101191858.2611154-1-alexey.pakhunov@spacex.com>
From: Alex Pakhunov <alexey.pakhunov@spacex.com>
The TX ring maintained by the tg3 driver can end up in a state, when it
has packets queued for sending but the NIC hardware is not informed, so no
progress is made. This leads to a multi-second interruption in network
traffic followed by dev_watchdog() firing and resetting the queue.
The specific sequence of steps is:
1. tg3_start_xmit() is called at least once and queues packet(s) without
updating tnapi->prodmbox (netdev_xmit_more() returns true)
2. tg3_start_xmit() is called with an SKB which causes tg3_tso_bug() to be
called.
3. tg3_tso_bug() determines that the SKB is too large, ...
if (unlikely(tg3_tx_avail(tnapi) <= frag_cnt_est)) {
... stops the queue, and returns NETDEV_TX_BUSY:
netif_tx_stop_queue(txq);
...
if (tg3_tx_avail(tnapi) <= frag_cnt_est)
return NETDEV_TX_BUSY;
4. Since all tg3_tso_bug() call sites directly return, the code updating
tnapi->prodmbox is skipped.
5. The queue is stuck now. tg3_start_xmit() is not called while the queue
is stopped. The NIC is not processing new packets because
tnapi->prodmbox wasn't updated. tg3_tx() is not called by
tg3_poll_work() because the all TX descriptions that could be freed has
been freed:
/* run TX completion thread */
if (tnapi->hw_status->idx[0].tx_consumer != tnapi->tx_cons) {
tg3_tx(tnapi);
6. Eventually, dev_watchdog() fires triggering a reset of the queue.
This fix makes sure that the tnapi->prodmbox update happens regardless of
the reason tg3_start_xmit() returned.
Signed-off-by: Alex Pakhunov <alexey.pakhunov@spacex.com>
Signed-off-by: Vincent Wong <vincent.wong2@spacex.com>
---
drivers/net/ethernet/broadcom/tg3.c | 46 ++++++++++++++++++++++-------
1 file changed, 35 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index 99638e6c9e16..c3512409434e 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -6603,9 +6603,9 @@ static void tg3_tx(struct tg3_napi *tnapi)
tnapi->tx_cons = sw_idx;
- /* Need to make the tx_cons update visible to tg3_start_xmit()
+ /* Need to make the tx_cons update visible to __tg3_start_xmit()
* before checking for netif_queue_stopped(). Without the
- * memory barrier, there is a small possibility that tg3_start_xmit()
+ * memory barrier, there is a small possibility that __tg3_start_xmit()
* will miss it and cause the queue to be stopped forever.
*/
smp_mb();
@@ -7845,7 +7845,7 @@ static bool tg3_tso_bug_gso_check(struct tg3_napi *tnapi, struct sk_buff *skb)
return skb_shinfo(skb)->gso_segs < tnapi->tx_pending / 3;
}
-static netdev_tx_t tg3_start_xmit(struct sk_buff *, struct net_device *);
+static netdev_tx_t __tg3_start_xmit(struct sk_buff *, struct net_device *);
/* Use GSO to workaround all TSO packets that meet HW bug conditions
* indicated in tg3_tx_frag_set()
@@ -7881,7 +7881,7 @@ static int tg3_tso_bug(struct tg3 *tp, struct tg3_napi *tnapi,
skb_list_walk_safe(segs, seg, next) {
skb_mark_not_on_list(seg);
- tg3_start_xmit(seg, tp->dev);
+ __tg3_start_xmit(seg, tp->dev);
}
tg3_tso_bug_end:
@@ -7891,7 +7891,7 @@ static int tg3_tso_bug(struct tg3 *tp, struct tg3_napi *tnapi,
}
/* hard_start_xmit for all devices */
-static netdev_tx_t tg3_start_xmit(struct sk_buff *skb, struct net_device *dev)
+static netdev_tx_t __tg3_start_xmit(struct sk_buff *skb, struct net_device *dev)
{
struct tg3 *tp = netdev_priv(dev);
u32 len, entry, base_flags, mss, vlan = 0;
@@ -8135,11 +8135,6 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb, struct net_device *dev)
netif_tx_wake_queue(txq);
}
- if (!netdev_xmit_more() || netif_xmit_stopped(txq)) {
- /* Packets are ready, update Tx producer idx on card. */
- tw32_tx_mbox(tnapi->prodmbox, entry);
- }
-
return NETDEV_TX_OK;
dma_error:
@@ -8152,6 +8147,35 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb, struct net_device *dev)
return NETDEV_TX_OK;
}
+static netdev_tx_t tg3_start_xmit(struct sk_buff *skb, struct net_device *dev)
+{
+ u16 skb_queue_mapping = skb_get_queue_mapping(skb);
+ struct netdev_queue *txq = netdev_get_tx_queue(dev, skb_queue_mapping);
+
+ netdev_tx_t ret = __tg3_start_xmit(skb, dev);
+
+ /* Notify the hardware that packets are ready by updating the TX ring
+ * tail pointer. We respect netdev_xmit_more() thus avoiding poking
+ * the hardware for every packet. To guarantee forward progress the TX
+ * ring must be drained when it is full as indicated by
+ * netif_xmit_stopped(). This needs to happen even when the current
+ * skb was dropped or rejected with NETDEV_TX_BUSY. Otherwise packets
+ * queued by previous __tg3_start_xmit() calls might get stuck in
+ * the queue forever.
+ */
+ if (!netdev_xmit_more() || netif_xmit_stopped(txq)) {
+ struct tg3 *tp = netdev_priv(dev);
+ struct tg3_napi *tnapi = &tp->napi[skb_queue_mapping];
+
+ if (tg3_flag(tp, ENABLE_TSS))
+ tnapi++;
+
+ tw32_tx_mbox(tnapi->prodmbox, tnapi->tx_prod);
+ }
+
+ return ret;
+}
+
static void tg3_mac_loopback(struct tg3 *tp, bool enable)
{
if (enable) {
@@ -17682,7 +17706,7 @@ static int tg3_init_one(struct pci_dev *pdev,
* device behind the EPB cannot support DMA addresses > 40-bit.
* On 64-bit systems with IOMMU, use 40-bit dma_mask.
* On 64-bit systems without IOMMU, use 64-bit dma_mask and
- * do DMA address check in tg3_start_xmit().
+ * do DMA address check in __tg3_start_xmit().
*/
if (tg3_flag(tp, IS_5788))
persist_dma_mask = dma_mask = DMA_BIT_MASK(32);
--
2.39.3
^ permalink raw reply related
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