* Re: [PATCH net v5 0/8] xsk: fix bugs around xsk skb allocation
From: patchwork-bot+netdevbpf @ 2026-05-06 2:40 UTC (permalink / raw)
To: Jason Xing
Cc: davem, edumazet, kuba, pabeni, bjorn, magnus.karlsson,
maciej.fijalkowski, jonathan.lemon, sdf, ast, daniel, hawk,
john.fastabend, horms, andrew+netdev, bpf, netdev, kernelxing
In-Reply-To: <20260502200722.53960-1-kerneljasonxing@gmail.com>
Hello:
This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Sat, 2 May 2026 23:07:14 +0300 you wrote:
> From: Jason Xing <kernelxing@tencent.com>
>
> There are rare issues around xsk_build_skb(). Some of them
> were founded by Sashiko[1][2].
>
> [1]: https://lore.kernel.org/all/20260415082654.21026-1-kerneljasonxing@gmail.com/
> [2]: https://lore.kernel.org/all/20260418045644.28612-1-kerneljasonxing@gmail.com/
>
> [...]
Here is the summary with links:
- [net,v5,1/8] xsk: reject sw-csum UMEM binding to IFF_TX_SKB_NO_LINEAR devices
https://git.kernel.org/netdev/net/c/d73a9a63f9f7
- [net,v5,2/8] xsk: free the skb when hitting the upper bound MAX_SKB_FRAGS
https://git.kernel.org/netdev/net/c/0bb7a9caf5c1
- [net,v5,3/8] xsk: handle NULL dereference of the skb without frags issue
https://git.kernel.org/netdev/net/c/8cd3c1c6e7d9
- [net,v5,4/8] xsk: fix use-after-free of xs->skb in xsk_build_skb() free_err path
https://git.kernel.org/netdev/net/c/0f3776583d28
- [net,v5,5/8] xsk: prevent CQ desync when freeing half-built skbs in xsk_build_skb()
https://git.kernel.org/netdev/net/c/3dec153ae484
- [net,v5,6/8] xsk: avoid skb leak in XDP_TX_METADATA case
https://git.kernel.org/netdev/net/c/8c2cff50afdd
- [net,v5,7/8] xsk: fix xsk_addrs slab leak on multi-buffer error path
https://git.kernel.org/netdev/net/c/e0f229025a8e
- [net,v5,8/8] xsk: fix u64 descriptor address truncation on 32-bit architectures
https://git.kernel.org/netdev/net/c/203cee647f55
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 net 09/12] tools: ynl: add scope qualifier for definitions
From: Jakub Kicinski @ 2026-05-06 2:32 UTC (permalink / raw)
To: davem
Cc: netdev, edumazet, pabeni, andrew+netdev, horms, shuah,
linux-kselftest, donald.hunter, ast
In-Reply-To: <20260506000628.1501691-10-kuba@kernel.org>
On Tue, 5 May 2026 17:06:25 -0700 Jakub Kicinski wrote:
> + description: |
> + Visibility of this definition. "uapi" (default) renders into
> + the uAPI header, "kernel" renders into the kernel-side
> + generated header, "user" renders into the user-side
> + generated header. When combined with `header:`, the
> + definition is not rendered, and the named header is
> + included only by code matching the scope.
Clashiko says I'm not implementing enum handling even tho
the param is generic. True, but intentional. Since this is
primarily for policies I don't think we'll need an enum.
We tend not to implement things we don't use in the code gen.
I implemented header handling here (even tho it's not used)
because I think that part is tricky (read: I would have forgotten
the plan/thinking if I didn't just implement it right away).
^ permalink raw reply
* Re: [PATCH net-next v3 0/2] net: mana: Avoid queue struct allocation failure under memory fragmentation
From: patchwork-bot+netdevbpf @ 2026-05-06 2:30 UTC (permalink / raw)
To: Aditya Garg
Cc: kys, haiyangz, wei.liu, decui, longli, andrew+netdev, davem,
edumazet, kuba, pabeni, kotaranov, horms, ssengar, jacob.e.keller,
dipayanroy, ernis, shirazsaleem, kees, sbhatta, leitao, netdev,
linux-hyperv, linux-kernel, linux-rdma, bpf, gargaditya
In-Reply-To: <20260502074552.23857-1-gargaditya@linux.microsoft.com>
Hello:
This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Sat, 2 May 2026 00:45:32 -0700 you wrote:
> The MANA driver can fail to load on systems with high memory
> utilization because several allocations in the queue setup paths
> require large physically contiguous blocks via kmalloc. Under memory
> fragmentation these high-order allocations may fail, preventing the
> driver from creating queues when opening the interface or when
> reconfiguring channels, ring parameters or MTU at runtime.
>
> [...]
Here is the summary with links:
- [net-next,v3,1/2] net: mana: Use per-queue allocation for tx_qp to reduce allocation size
https://git.kernel.org/netdev/net-next/c/d07efe5a6e64
- [net-next,v3,2/2] net: mana: Use kvmalloc for large RX queue and buffer allocations
https://git.kernel.org/netdev/net-next/c/3af0820c878e
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 net-next v3 00/10] selftests: rds: Log collection, TAP compliance and cleanups
From: patchwork-bot+netdevbpf @ 2026-05-06 2:30 UTC (permalink / raw)
To: Allison Henderson
Cc: netdev, pabeni, edumazet, kuba, horms, linux-rdma,
linux-kselftest, shuah
In-Reply-To: <20260504054143.4027538-1-achender@kernel.org>
Hello:
This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Sun, 3 May 2026 22:41:33 -0700 you wrote:
> This series is a set of bug fixes and improvements for the rds
> selftests.
>
> Patch 1 bumps the kselftest timeout from 400s to 800s. The original
> limit was developed against a lean config, but the kselftest harness
> counts boot time and gcov log collection against the limit, so a
> default config with gcov enabled needs more headroom.
>
> [...]
Here is the summary with links:
- [net-next,v3,01/10] selftests: rds: Increase selftest timeout
https://git.kernel.org/netdev/net-next/c/94720e01f5bb
- [net-next,v3,02/10] selftests: rds: Update USAGE string for run.sh
https://git.kernel.org/netdev/net-next/c/4781c8b037a8
- [net-next,v3,03/10] selftests: rds: Fix more pylint errors
https://git.kernel.org/netdev/net-next/c/5bdd59f90633
- [net-next,v3,04/10] selftests: rds: Add timeout flag to run.sh
https://git.kernel.org/netdev/net-next/c/7d6a32e7cb45
- [net-next,v3,05/10] selftests: rds: Add RDS_LOG_DIR env variable
https://git.kernel.org/netdev/net-next/c/ad070d903759
- [net-next,v3,06/10] selftests: rds: Add SUDO_USER env variable
https://git.kernel.org/netdev/net-next/c/d601239dcc26
- [net-next,v3,07/10] selftests: rds: Remove tmp pcaps
https://git.kernel.org/netdev/net-next/c/c726bc68fffd
- [net-next,v3,08/10] selftests: rds: Stop tcpdump on timeout
https://git.kernel.org/netdev/net-next/c/ec91483634fe
- [net-next,v3,09/10] selftests: rds: Fix gcov collection
https://git.kernel.org/netdev/net-next/c/c8781c61fd60
- [net-next,v3,10/10] selftests: rds: Make rds selftests TAP compliant
https://git.kernel.org/netdev/net-next/c/06d5c34517e0
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 net-next 10/12] net: stmmac: tc956x: add TC956x/QPS615 support
From: Xilin Wu @ 2026-05-06 2:30 UTC (permalink / raw)
To: Alex Elder, andrew+netdev, davem, edumazet, kuba, pabeni,
maxime.chevallier, rmk+kernel, andersson, konradybcio, robh,
krzk+dt, conor+dt, linusw, brgl, arnd, gregkh
Cc: Daniel Thompson, mohd.anwar, a0987203069, alexandre.torgue, ast,
boon.khai.ng, chenchuangyu, chenhuacai, daniel, hawk, hkallweit1,
inochiama, john.fastabend, julianbraha, livelycarpet87,
matthew.gerlach, mcoquelin.stm32, me, prabhakar.mahadev-lad.rj,
richardcochran, rohan.g.thomas, sdf, siyanteng, weishangjuan,
wens, netdev, bpf, linux-arm-msm, devicetree, linux-gpio,
linux-stm32, linux-arm-kernel, linux-kernel
In-Reply-To: <20260501155421.3329862-11-elder@riscstar.com>
On 5/1/2026 11:54 PM, Alex Elder wrote:
> From: Daniel Thompson <daniel@riscstar.com>
>
> Toshiba TC956x is an Ethernet AVB/TSN bridge and is essentially a
> small and highly-specialized SoC. TC956x includes an "eMAC" subsystem
> that can be accessed, along with several other peripherals, via two
> PCIe endpoint functions. There is a main driver for the endpoint that
> decomposes things and creates auxiliary bus devices to model the SoC.
>
> The eMAC consists of a Designware XGMAC, XPCS and PMA. Each eMAC is
> supported by an MSIGEN that bridges TC956x level interrupts to PCIe
> MSIs.
>
> Add a driver for the eMAC/MSIGEN combination.
>
> Co-developed-by: Alex Elder <elder@riscstar.com>
> Signed-off-by: Alex Elder <elder@riscstar.com>
> Signed-off-by: Daniel Thompson <daniel@riscstar.com>
> ---
> drivers/net/ethernet/stmicro/stmmac/Kconfig | 13 +
> drivers/net/ethernet/stmicro/stmmac/Makefile | 2 +
> .../ethernet/stmicro/stmmac/dwmac-tc956x.c | 791 ++++++++++++++++++
> include/soc/toshiba/tc956x-dwmac.h | 84 ++
> 4 files changed, 890 insertions(+)
> create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-tc956x.c
> create mode 100644 include/soc/toshiba/tc956x-dwmac.h
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> index e3dd5adda5aca..66bcfaccbe21f 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
> +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> @@ -404,6 +404,19 @@ config DWMAC_MOTORCOMM
> This enables glue driver for Motorcomm DWMAC-based PCI Ethernet
> controllers. Currently only YT6801 is supported.
>
> +config DWMAC_TC956X
> + tristate "Toshiba TC956X DWMAC support"
> + depends on PCI
> + depends on COMMON_CLK
> + depends on TOSHIBA_TC956X_PCI
> + default m if TOSHIBA_TC956X_PCI
Hi Alex,
I think GENERIC_IRQ_CHIP should be selected here.
Thank you for the driver.
--
Best regards,
Xilin Wu <sophon@radxa.com>
^ permalink raw reply
* Re: [PATCH net V5 0/4] net/mlx5: Fixes for Socket-Direct
From: patchwork-bot+netdevbpf @ 2026-05-06 2:30 UTC (permalink / raw)
To: Tariq Toukan
Cc: edumazet, kuba, pabeni, andrew+netdev, davem, saeedm, mbloch,
leon, shayd, horms, phaddad, parav, kees, gal, netdev, linux-rdma,
linux-kernel, dtatulea
In-Reply-To: <20260504180206.268568-1-tariqt@nvidia.com>
Hello:
This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Mon, 4 May 2026 21:02:02 +0300 you wrote:
> Hi,
>
> This series fixes several race conditions and bugs in the mlx5
> Socket-Direct (SD) single netdev flow.
>
> Patch 1 serializes mlx5_sd_init()/mlx5_sd_cleanup() with
> mlx5_devcom_comp_lock() and tracks the SD group state on the primary
> device, preventing concurrent or duplicate bring-up/tear-down.
>
> [...]
Here is the summary with links:
- [net,V5,1/4] net/mlx5: SD: Serialize init/cleanup
https://git.kernel.org/netdev/net/c/3abcedfdfd31
- [net,V5,2/4] net/mlx5: SD, Keep multi-pf debugfs entries on primary
https://git.kernel.org/netdev/net/c/05217e4ffbb2
- [net,V5,3/4] net/mlx5e: SD, Fix missing cleanup on probe error
https://git.kernel.org/netdev/net/c/3564222cfdde
- [net,V5,4/4] net/mlx5e: SD, Fix race condition in secondary device probe/remove
https://git.kernel.org/netdev/net/c/d466ddda5500
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 v6] net: wwan: t7xx: validate port_count against message length in t7xx_port_enum_msg_handler
From: patchwork-bot+netdevbpf @ 2026-05-06 2:20 UTC (permalink / raw)
To: Pavitra Jha
Cc: w, pabeni, horms, chandrashekar.devegowda, linux-wwan, netdev,
stable
In-Reply-To: <20260501110713.145563-1-jhapavitra98@gmail.com>
Hello:
This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Fri, 1 May 2026 07:07:12 -0400 you wrote:
> t7xx_port_enum_msg_handler() uses the modem-supplied port_count field as
> a loop bound over port_msg->data[] without checking that the message buffer
> contains sufficient data. A modem sending port_count=65535 in a 12-byte
> buffer triggers a slab-out-of-bounds read of up to 262140 bytes.
>
> Add a sizeof(*port_msg) check before accessing the port message header
> fields to guard against undersized messages.
>
> [...]
Here is the summary with links:
- [v6] net: wwan: t7xx: validate port_count against message length in t7xx_port_enum_msg_handler
https://git.kernel.org/netdev/net/c/0e7c074cfcd9
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 net V3 0/3] net/mlx5e: PSP fixes
From: patchwork-bot+netdevbpf @ 2026-05-06 2:20 UTC (permalink / raw)
To: Tariq Toukan
Cc: edumazet, kuba, pabeni, andrew+netdev, davem, borisp, saeedm,
leon, mbloch, daniel.zahka, willemdebruijn.kernel, cratiu, raeds,
rrameshbabu, dtatulea, kees, netdev, linux-rdma, linux-kernel,
gal
In-Reply-To: <20260504181100.269334-1-tariqt@nvidia.com>
Hello:
This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Mon, 4 May 2026 21:10:57 +0300 you wrote:
> Hi,
>
> This patchset provides bug fixes from Cosmin to the mlx5e PSP feature.
>
> Thanks,
> Tariq.
>
> [...]
Here is the summary with links:
- [net,V3,1/3] net/mlx5e: psp: Fix invalid access on PSP dev registration fail
https://git.kernel.org/netdev/net/c/ae9582cd0b9c
- [net,V3,2/3] net/mlx5e: psp: Expose only a fully initialized priv->psp
https://git.kernel.org/netdev/net/c/50690733db59
- [net,V3,3/3] net/mlx5e: psp: Hook PSP dev reg/unreg to profile enable/disable
https://git.kernel.org/netdev/net/c/c4a5c46199b5
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* [PATCH 2/2] docs: acpi: dsd: add Motorcomm yt8xxx PHY properties
From: chunzhi.lin @ 2026-05-06 2:18 UTC (permalink / raw)
To: netdev
Cc: Frank.Sae, andrew, hkallweit1, linux, kuba, pabeni, edumazet,
davem, linux-acpi, rafael, lenb, chunzhi.lin
In-Reply-To: <20260506021813.3658669-1-linchunzhi0@gmail.com>
Document Motorcomm yt8xxx PHY ACPI _DSD properties consumed by
the motorcomm PHY driver.
Describe property placement, UUID usage, and reference the DT binding
for value constraints and defaults.
Signed-off-by: chunzhi.lin <linchunzhi0@gmail.com>
---
.../acpi/dsd/motorcomm-yt8xxx-phy.rst | 107 ++++++++++++++++++
Documentation/firmware-guide/acpi/index.rst | 1 +
2 files changed, 108 insertions(+)
create mode 100644 Documentation/firmware-guide/acpi/dsd/motorcomm-yt8xxx-phy.rst
diff --git a/Documentation/firmware-guide/acpi/dsd/motorcomm-yt8xxx-phy.rst b/Documentation/firmware-guide/acpi/dsd/motorcomm-yt8xxx-phy.rst
new file mode 100644
index 000000000000..d64a396fac81
--- /dev/null
+++ b/Documentation/firmware-guide/acpi/dsd/motorcomm-yt8xxx-phy.rst
@@ -0,0 +1,107 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+======================================
+Motorcomm yt8xxx PHY properties (_DSD)
+======================================
+
+This document describes ACPI _DSD device properties for Motorcomm yt8xxx
+Ethernet PHYs supported by the in-kernel driver in
+``drivers/net/phy/motorcomm.c``.
+
+The properties are exposed on the PHY device object under the MDIO bus ACPI
+device (the same objects that are registered via
+``fwnode_mdiobus_register_phy()``). MAC-side connection properties such as
+``phy-handle`` and ``phy-mode`` are documented in [acpi-mdio-phy]_.
+
+Property names and semantics are intentionally aligned with the Devicetree
+binding [motorcomm-yt8xxx]_ so that the same driver code path
+(``device_property_*`` on ``&phydev->mdio.dev``) can consume firmware
+described either as Devicetree or ACPI _DSD.
+
+UUID and placement
+==================
+
+Per [acpi-dsd-properties-rules]_ and [acpi-mdio-phy]_, properties must be
+placed in an _DSD package using the standard Device Properties UUID::
+
+ ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301")
+
+Properties
+==========
+
+Unless noted otherwise, integer properties use the same allowed values and
+defaults as [motorcomm-yt8xxx]_.
+
+``rx-internal-delay-ps`` (u32, optional)
+ RGMII RX internal delay in picoseconds. Only meaningful when the link is
+ using RGMII modes with RX internal delay; see [motorcomm-yt8xxx]_.
+
+``tx-internal-delay-ps`` (u32, optional)
+ RGMII TX internal delay in picoseconds. Only meaningful when the link is
+ using RGMII modes with TX internal delay; see [motorcomm-yt8xxx]_.
+
+``motorcomm,clk-out-frequency-hz`` (u32, optional)
+ Clock output frequency on the PHY clock output pin. Allowed values and
+ default are defined in [motorcomm-yt8xxx]_.
+
+``motorcomm,keep-pll-enabled`` (boolean, optional)
+ If true, keep the PLL enabled even when there is no link (useful for using
+ the clock output without an Ethernet link). See [motorcomm-yt8xxx]_.
+
+``motorcomm,auto-sleep-disabled`` (boolean, optional)
+ If true, disable the PHY auto-sleep behavior described in
+ [motorcomm-yt8xxx]_.
+
+``motorcomm,rx-clk-drv-microamp`` (u32, optional)
+ Drive strength for the ``rx_clk`` RGMII pad in microamps. Allowed values
+ depend on the configured RGMII LDO voltage; see [motorcomm-yt8xxx]_.
+
+``motorcomm,rx-data-drv-microamp`` (u32, optional)
+ Drive strength for the ``rx_data`` and ``rx_ctl`` RGMII pads in microamps.
+ See [motorcomm-yt8xxx]_.
+
+``motorcomm,tx-clk-adj-enabled`` (boolean, optional)
+ Enables adjustments related to ``motorcomm,tx-clk-*-inverted`` usage; see
+ [motorcomm-yt8xxx]_.
+
+``motorcomm,tx-clk-10-inverted`` (boolean, optional)
+``motorcomm,tx-clk-100-inverted`` (boolean, optional)
+``motorcomm,tx-clk-1000-inverted`` (boolean, optional)
+ Per-speed TX clock inversion options; see [motorcomm-yt8xxx]_.
+
+ASL example (illustrative)
+==========================
+
+The example below only shows PHY-local _DSD properties. A real platform
+still needs a MAC ``phy-handle`` and ``phy-mode`` package as in
+[acpi-mdio-phy]_.
+
+.. code-block:: none
+
+ Scope (\_SB.MDI0)
+ {
+ Device (PHY4)
+ {
+ Name (_ADR, 0x4)
+
+ Name (_DSD, Package () {
+ ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+ Package () {
+ Package (2) { "rx-internal-delay-ps", 2100 },
+ Package (2) { "tx-internal-delay-ps", 150 },
+ Package (2) { "motorcomm,clk-out-frequency-hz", 0 },
+ Package (2) { "motorcomm,keep-pll-enabled", 1 },
+ Package (2) { "motorcomm,auto-sleep-disabled", 1 },
+ }
+ })
+ }
+ }
+
+References
+==========
+
+.. [acpi-mdio-phy] Documentation/firmware-guide/acpi/dsd/phy.rst
+.. [acpi-dsd-properties-rules]
+ Documentation/firmware-guide/acpi/DSD-properties-rules.rst
+.. [motorcomm-yt8xxx]
+ Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml
diff --git a/Documentation/firmware-guide/acpi/index.rst b/Documentation/firmware-guide/acpi/index.rst
index b246902f523f..3cd38bb4de15 100644
--- a/Documentation/firmware-guide/acpi/index.rst
+++ b/Documentation/firmware-guide/acpi/index.rst
@@ -12,6 +12,7 @@ ACPI Support
dsd/data-node-references
dsd/leds
dsd/phy
+ dsd/motorcomm-yt8xxx-phy
enumeration
osi
method-tracing
--
2.34.1
^ permalink raw reply related
* [PATCH 1/2] net: phy: motorcomm: use device properties for firmware tuning
From: chunzhi.lin @ 2026-05-06 2:18 UTC (permalink / raw)
To: netdev
Cc: Frank.Sae, andrew, hkallweit1, linux, kuba, pabeni, edumazet,
davem, linux-acpi, rafael, lenb, chunzhi.lin
In-Reply-To: <20260506021813.3658669-1-linchunzhi0@gmail.com>
The Motorcomm PHY driver reads optional firmware properties via
of_property_read_*() from phydev->mdio.dev.of_node. This works for
Device Tree based systems, but causes ACPI platforms to ignore the same
properties when they are supplied through _DSD.
As a result, ACPI-described Motorcomm PHY devices fall back to default
settings instead of applying firmware-provided tuning such as
rx/tx internal delay, drive strength, clock output frequency, and
optional boolean controls like auto-sleep-disabled,
keep-pll-enabled, and tx clock inversion.
Switch these lookups to device_property_read_*() so the driver uses the
generic firmware node interface and can consume the same property names
from either Device Tree or ACPI.
This keeps the existing DT behavior unchanged while allowing ACPI
platforms to honor PHY configuration from firmware.
We have completed testing on Sophgo RISC-V architecture server SD3-10.
This server has a 64-core Thead C920 CPU whose DWMAC is connected to
Motorcomm's PHY YT8531. This server supports UEFI boot and it would like
to use the ACPI table.
Signed-off-by: chunzhi.lin <linchunzhi0@gmail.com>
---
drivers/net/phy/motorcomm.c | 41 ++++++++++++++++++-------------------
1 file changed, 20 insertions(+), 21 deletions(-)
diff --git a/drivers/net/phy/motorcomm.c b/drivers/net/phy/motorcomm.c
index 4d62f7b36212..708491bc198a 100644
--- a/drivers/net/phy/motorcomm.c
+++ b/drivers/net/phy/motorcomm.c
@@ -10,7 +10,7 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/phy.h>
-#include <linux/of.h>
+#include <linux/property.h>
#define PHY_ID_YT8511 0x0000010a
#define PHY_ID_YT8521 0x0000011a
@@ -843,12 +843,12 @@ static u32 ytphy_get_delay_reg_value(struct phy_device *phydev,
u16 *rxc_dly_en,
u32 dflt)
{
- struct device_node *node = phydev->mdio.dev.of_node;
+ struct device *dev = &phydev->mdio.dev;
int tb_size_half = tb_size / 2;
u32 val;
int i;
- if (of_property_read_u32(node, prop_name, &val))
+ if (device_property_read_u32(dev, prop_name, &val))
goto err_dts_val;
/* when rxc_dly_en is NULL, it is get the delay for tx, only half of
@@ -996,12 +996,12 @@ static int yt8531_get_ds_map(struct phy_device *phydev, u32 cur)
static int yt8531_set_ds(struct phy_device *phydev)
{
- struct device_node *node = phydev->mdio.dev.of_node;
+ struct device *dev = &phydev->mdio.dev;
u32 ds_field_low, ds_field_hi, val;
int ret, ds;
/* set rgmii rx clk driver strength */
- if (!of_property_read_u32(node, "motorcomm,rx-clk-drv-microamp", &val)) {
+ if (!device_property_read_u32(dev, "motorcomm,rx-clk-drv-microamp", &val)) {
ds = yt8531_get_ds_map(phydev, val);
if (ds < 0)
return dev_err_probe(&phydev->mdio.dev, ds,
@@ -1018,7 +1018,7 @@ static int yt8531_set_ds(struct phy_device *phydev)
return ret;
/* set rgmii rx data driver strength */
- if (!of_property_read_u32(node, "motorcomm,rx-data-drv-microamp", &val)) {
+ if (!device_property_read_u32(dev, "motorcomm,rx-data-drv-microamp", &val)) {
ds = yt8531_get_ds_map(phydev, val);
if (ds < 0)
return dev_err_probe(&phydev->mdio.dev, ds,
@@ -1051,7 +1051,6 @@ static int yt8531_set_ds(struct phy_device *phydev)
*/
static int yt8521_probe(struct phy_device *phydev)
{
- struct device_node *node = phydev->mdio.dev.of_node;
struct device *dev = &phydev->mdio.dev;
struct yt8521_priv *priv;
int chip_config;
@@ -1101,7 +1100,7 @@ static int yt8521_probe(struct phy_device *phydev)
return ret;
}
- if (of_property_read_u32(node, "motorcomm,clk-out-frequency-hz", &freq))
+ if (device_property_read_u32(dev, "motorcomm,clk-out-frequency-hz", &freq))
freq = YTPHY_DTS_OUTPUT_CLK_DIS;
if (phydev->drv->phy_id == PHY_ID_YT8521) {
@@ -1169,11 +1168,11 @@ static int yt8521_probe(struct phy_device *phydev)
static int yt8531_probe(struct phy_device *phydev)
{
- struct device_node *node = phydev->mdio.dev.of_node;
+ struct device *dev = &phydev->mdio.dev;
u16 mask, val;
u32 freq;
- if (of_property_read_u32(node, "motorcomm,clk-out-frequency-hz", &freq))
+ if (device_property_read_u32(dev, "motorcomm,clk-out-frequency-hz", &freq))
freq = YTPHY_DTS_OUTPUT_CLK_DIS;
switch (freq) {
@@ -1665,7 +1664,7 @@ static int yt8521_resume(struct phy_device *phydev)
*/
static int yt8521_config_init(struct phy_device *phydev)
{
- struct device_node *node = phydev->mdio.dev.of_node;
+ struct device *dev = &phydev->mdio.dev;
int old_page;
int ret = 0;
@@ -1680,7 +1679,7 @@ static int yt8521_config_init(struct phy_device *phydev)
goto err_restore_page;
}
- if (of_property_read_bool(node, "motorcomm,auto-sleep-disabled")) {
+ if (device_property_read_bool(dev, "motorcomm,auto-sleep-disabled")) {
/* disable auto sleep */
ret = ytphy_modify_ext(phydev, YT8521_EXTREG_SLEEP_CONTROL1_REG,
YT8521_ESC1R_SLEEP_SW, 0);
@@ -1688,7 +1687,7 @@ static int yt8521_config_init(struct phy_device *phydev)
goto err_restore_page;
}
- if (of_property_read_bool(node, "motorcomm,keep-pll-enabled")) {
+ if (device_property_read_bool(dev, "motorcomm,keep-pll-enabled")) {
/* enable RXC clock when no wire plug */
ret = ytphy_modify_ext(phydev, YT8521_CLOCK_GATING_REG,
YT8521_CGR_RX_CLK_EN, 0);
@@ -1801,14 +1800,14 @@ static int yt8521_led_hw_control_get(struct phy_device *phydev, u8 index,
static int yt8531_config_init(struct phy_device *phydev)
{
- struct device_node *node = phydev->mdio.dev.of_node;
+ struct device *dev = &phydev->mdio.dev;
int ret;
ret = ytphy_rgmii_clk_delay_config_with_lock(phydev);
if (ret < 0)
return ret;
- if (of_property_read_bool(node, "motorcomm,auto-sleep-disabled")) {
+ if (device_property_read_bool(dev, "motorcomm,auto-sleep-disabled")) {
/* disable auto sleep */
ret = ytphy_modify_ext_with_lock(phydev,
YT8521_EXTREG_SLEEP_CONTROL1_REG,
@@ -1817,7 +1816,7 @@ static int yt8531_config_init(struct phy_device *phydev)
return ret;
}
- if (of_property_read_bool(node, "motorcomm,keep-pll-enabled")) {
+ if (device_property_read_bool(dev, "motorcomm,keep-pll-enabled")) {
/* enable RXC clock when no wire plug */
ret = ytphy_modify_ext_with_lock(phydev,
YT8521_CLOCK_GATING_REG,
@@ -1844,7 +1843,7 @@ static int yt8531_config_init(struct phy_device *phydev)
*/
static void yt8531_link_change_notify(struct phy_device *phydev)
{
- struct device_node *node = phydev->mdio.dev.of_node;
+ struct device *dev = &phydev->mdio.dev;
bool tx_clk_1000_inverted = false;
bool tx_clk_100_inverted = false;
bool tx_clk_10_inverted = false;
@@ -1852,17 +1851,17 @@ static void yt8531_link_change_notify(struct phy_device *phydev)
u16 val = 0;
int ret;
- if (of_property_read_bool(node, "motorcomm,tx-clk-adj-enabled"))
+ if (device_property_read_bool(dev, "motorcomm,tx-clk-adj-enabled"))
tx_clk_adj_enabled = true;
if (!tx_clk_adj_enabled)
return;
- if (of_property_read_bool(node, "motorcomm,tx-clk-10-inverted"))
+ if (device_property_read_bool(dev, "motorcomm,tx-clk-10-inverted"))
tx_clk_10_inverted = true;
- if (of_property_read_bool(node, "motorcomm,tx-clk-100-inverted"))
+ if (device_property_read_bool(dev, "motorcomm,tx-clk-100-inverted"))
tx_clk_100_inverted = true;
- if (of_property_read_bool(node, "motorcomm,tx-clk-1000-inverted"))
+ if (device_property_read_bool(dev, "motorcomm,tx-clk-1000-inverted"))
tx_clk_1000_inverted = true;
if (phydev->speed < 0)
--
2.34.1
^ permalink raw reply related
* [PATCH net-next 0/2] net: phy: motorcomm: add ACPI _DSD property support
From: chunzhi.lin @ 2026-05-06 2:18 UTC (permalink / raw)
To: netdev
Cc: Frank.Sae, andrew, hkallweit1, linux, kuba, pabeni, edumazet,
davem, linux-acpi, rafael, lenb, chunzhi.lin
Hi,
This series makes the Motorcomm PHY driver parse firmware properties via
device_property_*() so the same property set can be provided by either
Devicetree or ACPI _DSD.
Patch 1 switches drivers/net/phy/motorcomm.c from of_property_*() to
device_property_*() on &phydev->mdio.dev.
Patch 2 documents Motorcomm yt8xxx PHY ACPI _DSD properties under
Documentation/firmware-guide/acpi/dsd and links the new document from
the ACPI index.
Thanks,
chunzhi.lin
chunzhi.lin (2):
net: phy: motorcomm: use device properties for firmware tuning
docs: acpi: dsd: add Motorcomm yt8xxx PHY properties
.../acpi/dsd/motorcomm-yt8xxx-phy.rst | 107 ++++++++++++++++++
Documentation/firmware-guide/acpi/index.rst | 1 +
drivers/net/phy/motorcomm.c | 41 ++++---
3 files changed, 128 insertions(+), 21 deletions(-)
create mode 100644 Documentation/firmware-guide/acpi/dsd/motorcomm-yt8xxx-phy.rst
--
2.34.1
^ permalink raw reply
* Re: Re: [PATCH net-next v7 2/4] net: stmmac: eic7700: enable clocks before syscon access and correct RX sampling timing
From: 李志 @ 2026-05-06 2:10 UTC (permalink / raw)
To: Jakub Kicinski
Cc: andrew+netdev, devicetree, davem, edumazet, robh, krzk+dt,
conor+dt, netdev, pabeni, mcoquelin.stm32, alexandre.torgue,
rmk+kernel, pjw, palmer, aou, alex, linux-riscv, linux-stm32,
linux-arm-kernel, linux-kernel, maxime.chevallier, ningyu, linmin,
pinkesh.vaghela, pritesh.patel, weishangjuan, horms
In-Reply-To: <20260430163551.7491407a@kernel.org>
> -----原始邮件-----
> 发件人: "Jakub Kicinski" <kuba@kernel.org>
> 发送时间:2026-05-01 07:35:51 (星期五)
> 收件人: 李志 <lizhi2@eswincomputing.com>
> 抄送: andrew+netdev@lunn.ch, devicetree@vger.kernel.org, davem@davemloft.net, edumazet@google.com, robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org, netdev@vger.kernel.org, pabeni@redhat.com, mcoquelin.stm32@gmail.com, alexandre.torgue@foss.st.com, rmk+kernel@armlinux.org.uk, pjw@kernel.org, palmer@dabbelt.com, aou@eecs.berkeley.edu, alex@ghiti.fr, linux-riscv@lists.infradead.org, linux-stm32@st-md-mailman.stormreply.com, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, maxime.chevallier@bootlin.com, ningyu@eswincomputing.com, linmin@eswincomputing.com, pinkesh.vaghela@einfochips.com, pritesh.patel@einfochips.com, weishangjuan@eswincomputing.com, horms@kernel.org
> 主题: Re: [PATCH net-next v7 2/4] net: stmmac: eic7700: enable clocks before syscon access and correct RX sampling timing
>
> On Thu, 30 Apr 2026 14:43:50 +0800 (GMT+08:00) 李志 wrote:
> > > Why Fixes? If eth1 never worked this is not a fix but new functionality
> > > If you want to make this a fix to prevent incompatibility - cut it down
> > > just to the eth0 changes.
> > >
> > Thank you for the suggestion.
> >
> > You're right that eth1 never worked at Gigabit speed, so this should
> > not be treated as a fix.
> >
> > In v8, I will split the changes into two patches within the same series:
> > - Patch 1 will contain only the fixes affecting the existing eth0
> > functionality, and will keep the Fixes tag.
> > - Patch 2 will add the eth1 support (RX clock inversion workaround)
> > as new functionality, without a Fixes tag.
> >
> > Please let me know if you would prefer a different split or ordering.
>
> If you want to consider some part of this commit a fix it has to be
> posted separately to the net tree (rather than net-next).
> Once it's merged and makes it way over to the net-next tree (each
> Thursday) you can post the net-next chnages for eth1
Thanks, understood.
I will split the changes accordingly:
- Send the eth0 fixes as a new v1 series targeting net.
- Send the eth1 enablement as a new v1 series targeting net-next.
Thanks for the guidance.
^ permalink raw reply
* Re: [PATCH net V3 1/3] net/mlx5e: psp: Fix invalid access on PSP dev registration fail
From: Jakub Kicinski @ 2026-05-06 2:11 UTC (permalink / raw)
To: Tariq Toukan
Cc: Eric Dumazet, Paolo Abeni, Andrew Lunn, David S. Miller,
Boris Pismenny, Saeed Mahameed, Leon Romanovsky, Mark Bloch,
Daniel Zahka, Willem de Bruijn, Cosmin Ratiu, Raed Salem,
Rahul Rameshbabu, Dragos Tatulea, Kees Cook, netdev, linux-rdma,
linux-kernel, Gal Pressman
In-Reply-To: <20260504181100.269334-2-tariqt@nvidia.com>
On Mon, 4 May 2026 21:10:58 +0300 Tariq Toukan wrote:
> - if (!priv->psp || !priv->psp->psp)
> + struct mlx5e_psp *psp = priv->psp;
> +
> + if (!psp || !psp->psp)
> return;
>
> - psp_dev_unregister(priv->psp->psp);
> + psp_dev_unregister(psp->psp);
> + psp->psp = NULL;
TBH the pointless churn to add a local variable here was what I was
referring to when talking about unnecessary refactoring. One line
change to clear the pointer and you're turning it to a full rewrite
of the helper.
Whatever. Some things can't be taught I guess :\
^ permalink raw reply
* Re: [PATCH net-next 1/6] bridge: uapi: Add neigh_forward_grat netlink attributes
From: Jakub Kicinski @ 2026-05-06 2:00 UTC (permalink / raw)
To: Danielle Ratson
Cc: netdev, donald.hunter, davem, edumazet, pabeni, horms, razor,
idosch, andrew+netdev, shuah, ast, liuhangbin, daniel, aroulin,
fmaurer, sdf.kernel, sd, kees, nickgarlis, amorenoz, alasdair,
johannes.wiesboeck, petrm, linux-kernel, bridge, linux-kselftest
In-Reply-To: <20260503073532.2138165-2-danieller@nvidia.com>
On Sun, 3 May 2026 10:35:27 +0300 Danielle Ratson wrote:
> --- a/Documentation/netlink/specs/rt-link.yaml
> +++ b/Documentation/netlink/specs/rt-link.yaml
> @@ -1700,6 +1700,9 @@ attribute-sets:
> -
> name: backup-nhid
> type: u32
> + -
> + name: neigh-forward-grat
> + type: flag
I think this should be u8 ? neigh-vlan-suppress looks buggy too
flag is a type without a payload, the presence of the attr is
the entire information
None of the AIs seem to catch this, I think you may have over-split
this submission a little bit. This patch may have been better off
squashed into patch 4 ?
--
pw-bot: cr
^ permalink raw reply
* Re: [PATCH v2] net: stmmac: Add support for TX/RX channel interrupt
From: Nazle Asmade, Muhammad Nazim Amirul @ 2026-05-06 2:00 UTC (permalink / raw)
To: Andrew Lunn
Cc: netdev@vger.kernel.org, davem@davemloft.net, kuba@kernel.org,
pabeni@redhat.com, edumazet@google.com, andrew+netdev@lunn.ch,
linux-kernel@vger.kernel.org
In-Reply-To: <c2996d3f-a8b8-4d70-9b2b-c800aac323c0@lunn.ch>
On 5/5/2026 8:40 pm, Andrew Lunn wrote:
> [You don't often get email from andrew@lunn.ch. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> On Mon, May 04, 2026 at 07:44:59PM -0700, muhammad.nazim.amirul.nazle.asmade@altera.com wrote:
>> From: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>
>>
>> Enable TX/RX channel interrupt registration for MAC that interrupts CPU
>> through shared peripheral interrupt (SPI).
>
> Please take a read on https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html
>
> The Subject line needs to indicate the tree.
>
>> int stmmac_get_platform_resources(struct platform_device *pdev,
>> struct stmmac_resources *stmmac_res)
>> {
>> + char irq_name[9];
>> + int i;
>> + int irq;
>> int ret;
>
> Reverse Christmas Tree.
>
> Andrew
>
> ---
> pw-bot: cr
Thanks Andrew for a quick response! really appreciate it. All comments
have been addressed and updated in v3.
Nazim
^ permalink raw reply
* [PATCH net-next v3] net: stmmac: Add support for TX/RX channel interrupt
From: muhammad.nazim.amirul.nazle.asmade @ 2026-05-06 2:00 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, pabeni, edumazet, andrew+netdev, linux-kernel
From: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>
Enable TX/RX channel interrupt registration for MAC that interrupts CPU
through shared peripheral interrupt (SPI).
Per-channel interrupts and interrupt-names are registered as follows,
e.g. 4 TX and 4 RX channels:
interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 107 IRQ_TYPE_LEVEL_HIGH>;
interrupt-names = "dma_tx0",
"dma_tx1",
"dma_tx2",
"dma_tx3",
"dma_rx0",
"dma_rx1",
"dma_rx2",
"dma_rx3";
Signed-off-by: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>
---
Changes in v3:
- Add net-next tree prefix to subject line.
- Fix variable declarations to follow Reverse Christmas Tree order.
Changes in v2:
- Use -ENXIO to detect when interrupt name is not present,
and return any other negative error code to the caller.
---
.../ethernet/stmicro/stmmac/stmmac_platform.c | 27 +++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 5cae2aa72906..9039e207ddbd 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -732,6 +732,9 @@ static int stmmac_pltfr_get_irq_array(struct platform_device *pdev,
int stmmac_get_platform_resources(struct platform_device *pdev,
struct stmmac_resources *stmmac_res)
{
+ char irq_name[9];
+ int ret;
+ int irq;
+ int i;
memset(stmmac_res, 0, sizeof(*stmmac_res));
@@ -767,6 +770,30 @@ int stmmac_get_platform_resources(struct platform_device *pdev,
dev_info(&pdev->dev, "IRQ sfty not found\n");
}
+ /* For RX Channel */
+ for (i = 0; i < MTL_MAX_RX_QUEUES; i++) {
+ snprintf(irq_name, sizeof(irq_name), "dma_rx%i", i);
+ irq = platform_get_irq_byname_optional(pdev, irq_name);
+ if (irq == -ENXIO)
+ break;
+ else if (irq < 0)
+ return irq;
+
+ stmmac_res->rx_irq[i] = irq;
+ }
+
+ /* For TX Channel */
+ for (i = 0; i < MTL_MAX_TX_QUEUES; i++) {
+ snprintf(irq_name, sizeof(irq_name), "dma_tx%i", i);
+ irq = platform_get_irq_byname_optional(pdev, irq_name);
+ if (irq == -ENXIO)
+ break;
+ else if (irq < 0)
+ return irq;
+
+ stmmac_res->tx_irq[i] = irq;
+ }
+
stmmac_res->addr = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(stmmac_res->addr))
--
2.43.7
^ permalink raw reply related
* Re: [PATCH net-next 0/5] Fixes for mv88e6xxx for 6320/6321 family
From: patchwork-bot+netdevbpf @ 2026-05-06 1:50 UTC (permalink / raw)
To: =?utf-8?q?Marek_Beh=C3=BAn_=3Ckabel=40kernel=2Eorg=3E?=
Cc: andrew, olteanv, rmk+kernel, vivien.didelot, tobias, netdev,
fidan.aliyeva.ext, lev_o
In-Reply-To: <20260504153227.1390546-1-kabel@kernel.org>
Hello:
This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Mon, 4 May 2026 17:32:22 +0200 you wrote:
> Five fixes for mv88e6xxx for 6320/6321 family, for net-next, without
> Fixes tags, as per Andrew's request last year, see
> https://lore.kernel.org/netdev/20250313134146.27087-1-kabel@kernel.org/
>
> Marek Behún (5):
> net: dsa: mv88e6xxx: fix number of g1 interrupts for 6320 family
> net: dsa: mv88e6xxx: allow SPEED_200 for 6320 family on supported
> ports
> net: dsa: mv88e6xxx: define .pot_clear() for 6321
> net: dsa: mv88e6xxx: enable .rmu_disable() for 6320 family
> net: dsa: mv88e6xxx: enable devlink ATU hash param for 6320 family
>
> [...]
Here is the summary with links:
- [net-next,1/5] net: dsa: mv88e6xxx: fix number of g1 interrupts for 6320 family
https://git.kernel.org/netdev/net-next/c/d201c2612e5a
- [net-next,2/5] net: dsa: mv88e6xxx: allow SPEED_200 for 6320 family on supported ports
https://git.kernel.org/netdev/net-next/c/965871277f60
- [net-next,3/5] net: dsa: mv88e6xxx: define .pot_clear() for 6321
https://git.kernel.org/netdev/net-next/c/17826d9708a5
- [net-next,4/5] net: dsa: mv88e6xxx: enable .rmu_disable() for 6320 family
https://git.kernel.org/netdev/net-next/c/e0fdb4157a85
- [net-next,5/5] net: dsa: mv88e6xxx: enable devlink ATU hash param for 6320 family
https://git.kernel.org/netdev/net-next/c/1010deac776a
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 net-next v4 9/9] net: dsa: lan9645x: add port statistics
From: Jakub Kicinski @ 2026-05-06 1:48 UTC (permalink / raw)
To: jensemil.schulzostergaard
Cc: UNGLinuxDriver, andrew, olteanv, davem, edumazet, pabeni, horms,
robh, krzk+dt, conor+dt, woojung.huh, linux, Steen.Hegelund,
daniel.machon, linux-kernel, netdev, devicetree
In-Reply-To: <20260506014618.1616861-1-kuba@kernel.org>
On Tue, 5 May 2026 18:46:18 -0700 Jakub Kicinski wrote:
> > +static const struct lan9645x_ethtool_stat lan9645x_port_ethtool_stats[] = {
> > + { "rx_cat_drop", SCNT_RX_CAT_DROP },
> > + { "rx_red_prio_0", SCNT_RX_RED_PRIO_0 },
>
> Per Documentation/networking/statistics.rst and the netdev stats hierarchy,
> counters that have a standard uAPI must not be duplicated in ethtool -S.
>
> The rx_cat_drop counter is summed into stats->rx_dropped in
> lan9645x_stats_get_stats64():
>
> stats->rx_dropped = c[SCNT_RX_LONG] +
> c[SCNT_DR_LOCAL] +
> c[SCNT_DR_TAIL] +
> c[SCNT_RX_CAT_DROP] +
> ...
>
> Since rtnl_link_stats64.rx_dropped is reachable via "ip -s link show",
> does exposing rx_cat_drop here duplicate a value already covered by the
> standard uAPI?
>
> [ ... ]
>
> > + { "tx_drop", SCNT_TX_DROP },
>
> Similarly, tx_drop feeds stats->tx_dropped:
>
> stats->tx_dropped = c[SCNT_TX_DROP] + c[SCNT_TX_AGED];
>
> Should tx_drop be dropped from the ethtool -S list for the same reason?
Ignore this part, I need to update the AI prompts..
^ permalink raw reply
* Re: [PATCH net-next v4 9/9] net: dsa: lan9645x: add port statistics
From: Jakub Kicinski @ 2026-05-06 1:46 UTC (permalink / raw)
To: jensemil.schulzostergaard
Cc: Jakub Kicinski, UNGLinuxDriver, andrew, olteanv, davem, edumazet,
pabeni, horms, robh, krzk+dt, conor+dt, woojung.huh, linux,
Steen.Hegelund, daniel.machon, linux-kernel, netdev, devicetree
In-Reply-To: <20260430-dsa_lan9645x_switch_driver_base-v4-9-f1b6005fa8b7@microchip.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: dsa: lan9645x: add port statistics
This patch wires up ethtool -S, standard ethtool group stats,
and ndo_get_stats64 for the lan9645x DSA driver. A delayed work
polls the 32-bit hardware counters every 3 seconds and accumulates
them into a 64-bit software cache to handle hardware wraparound.
> diff --git a/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c b/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
> index 15a493c1f215..cea46bfe729e 100644
> --- a/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
> +++ b/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
[ ... ]
> @@ -255,6 +257,12 @@ static int lan9645x_setup(struct dsa_switch *ds)
> if (!lan9645x->owq)
> return -ENOMEM;
>
> + err = lan9645x_stats_init(lan9645x);
> + if (err) {
> + dev_err(dev, "Failed to init stats.\n");
> + goto owq_destroy;
> + }
> +
> ds->mtu_enforcement_ingress = true;
> ds->assisted_learning_on_cpu_port = true;
> ds->fdb_isolation = true;
> @@ -265,6 +273,10 @@ static int lan9645x_setup(struct dsa_switch *ds)
> lan9645x->num_phys_ports - lan9645x->num_port_dis);
>
> return 0;
> +
> +owq_destroy:
> + destroy_workqueue(lan9645x->owq);
> + return err;
> }
Is this owq_destroy error path reachable today?
Looking at lan9645x_stats_init():
int lan9645x_stats_init(struct lan9645x *lan9645x)
{
struct lan9645x_stats *stats = lan9645x->stats;
queue_delayed_work(stats->queue, &stats->work,
LAN9645X_STATS_CHECK_DELAY);
return 0;
}
it unconditionally returns 0, so the goto owq_destroy branch is dead.
If a future change either makes lan9645x_stats_init() return an error, or
adds any fallible step after it in lan9645x_setup(), the cleanup would
fall through owq_destroy without calling lan9645x_stats_deinit(), leaving
the self-requeuing delayed work armed on stats->queue.
Also, dsa_switch_setup() in net/dsa/dsa.c does not invoke ds->ops->teardown
when ops->setup() itself fails, so lan9645x_teardown (which is where
lan9645x_stats_deinit is called today) will not run in that scenario.
Then in lan9645x_remove() -> lan9645x_stats_free() -> destroy_workqueue(),
the queue would be drained with a chained self-requeue still live. Would
adding a dedicated label that calls lan9645x_stats_deinit() before falling
through to owq_destroy be safer for future edits?
[ ... ]
> diff --git a/drivers/net/dsa/microchip/lan9645x/lan9645x_stats.c b/drivers/net/dsa/microchip/lan9645x/lan9645x_stats.c
> new file mode 100644
> index 000000000000..46ac62b4016f
> --- /dev/null
> +++ b/drivers/net/dsa/microchip/lan9645x/lan9645x_stats.c
[ ... ]
> +static const struct lan9645x_ethtool_stat lan9645x_port_ethtool_stats[] = {
> + { "rx_cat_drop", SCNT_RX_CAT_DROP },
> + { "rx_red_prio_0", SCNT_RX_RED_PRIO_0 },
Per Documentation/networking/statistics.rst and the netdev stats hierarchy,
counters that have a standard uAPI must not be duplicated in ethtool -S.
The rx_cat_drop counter is summed into stats->rx_dropped in
lan9645x_stats_get_stats64():
stats->rx_dropped = c[SCNT_RX_LONG] +
c[SCNT_DR_LOCAL] +
c[SCNT_DR_TAIL] +
c[SCNT_RX_CAT_DROP] +
...
Since rtnl_link_stats64.rx_dropped is reachable via "ip -s link show",
does exposing rx_cat_drop here duplicate a value already covered by the
standard uAPI?
[ ... ]
> + { "tx_drop", SCNT_TX_DROP },
Similarly, tx_drop feeds stats->tx_dropped:
stats->tx_dropped = c[SCNT_TX_DROP] + c[SCNT_TX_AGED];
Should tx_drop be dropped from the ethtool -S list for the same reason?
[ ... ]
> + { "tx_aged", SCNT_TX_AGED },
> + { "tx_bufdrop", SCNT_TX_BUFDROP },
tx_aged is also summed into stats->tx_dropped in
lan9645x_stats_get_stats64(), so does it fall under the same duplicate-uAPI
concern as tx_drop?
A related question about tx_bufdrop: if SCNT_TX_BUFDROP represents transmit
frames dropped due to buffer exhaustion, why is it not included in
stats->tx_dropped alongside SCNT_TX_DROP and SCNT_TX_AGED?
stats->tx_dropped = c[SCNT_TX_DROP] + c[SCNT_TX_AGED];
If tx_bufdrop is in fact a transmit-drop cause, would users of
ip -s link show see an under-reported tx_dropped relative to the private
ethtool counter?
> + { "dr_local", SCNT_DR_LOCAL },
> + { "dr_tail", SCNT_DR_TAIL },
dr_local and dr_tail are also fed into stats->rx_dropped:
stats->rx_dropped = c[SCNT_RX_LONG] +
c[SCNT_DR_LOCAL] +
c[SCNT_DR_TAIL] +
...
Would it be appropriate to drop these from the ethtool -S list as well,
given they are already aggregated into the standard rx_dropped counter?
[ ... ]
> +};
[ ... ]
> +static void lan9645x_check_stats_work(struct work_struct *work)
> +{
> + struct delayed_work *del_work = to_delayed_work(work);
> + struct lan9645x_stats *stats;
> +
> + stats = container_of(del_work, struct lan9645x_stats, work);
> +
> + lan9645x_stats_update(stats->lan9645x);
> +
> + queue_delayed_work(stats->queue, &stats->work,
> + LAN9645X_STATS_CHECK_DELAY);
> +}
The commit message says this worker "update[s] it frequently to handle
overflows in hardware."
Looking at lan9645x_stats_add_cnt():
static inline void lan9645x_stats_add_cnt(u64 *cnt, u32 val)
{
if (val < (*cnt & U32_MAX))
*cnt += (u64)1 << 32; /* value has wrapped */
*cnt = (*cnt & ~(u64)U32_MAX) + val;
}
this compensates for at most one 32-bit wrap per polling interval. If two
or more wraps happen between reads, the low-half comparison no longer
detects the missed wrap and the 64-bit counter silently loses 2^32 per
missed wrap.
At the chip's 2.5 Gbps line rate (LAN9645X_SPEED_2500 in lan9645x_main.h),
a 32-bit byte counter wraps roughly every 13.7 s, so two wraps happen in
about 27.5 s. The polling interval is LAN9645X_STATS_CHECK_DELAY = 3 * HZ,
but there is no upper bound on the actual elapsed time between reads:
delayed work can be stretched by system suspend/resume, heavy CPU load, or
regmap/SPI contention.
Would it be worthwhile to either bound the worst-case polling gap
(e.g. a suspend/resume hook that forces a read, or a timestamp-based
sanity check that detects a stretched interval and logs a warning) so
that rx_bytes/tx_bytes reported to ndo_get_stats64 and ethtool cannot
silently undercount by multiples of 2^32?
[ ... ]
^ permalink raw reply
* Re: [PATCH net-next v4 8/9] net: dsa: lan9645x: add mdb management
From: Jakub Kicinski @ 2026-05-06 1:46 UTC (permalink / raw)
To: jensemil.schulzostergaard
Cc: Jakub Kicinski, UNGLinuxDriver, andrew, olteanv, davem, edumazet,
pabeni, horms, robh, krzk+dt, conor+dt, woojung.huh, linux,
Steen.Hegelund, daniel.machon, linux-kernel, netdev, devicetree
In-Reply-To: <20260430-dsa_lan9645x_switch_driver_base-v4-8-f1b6005fa8b7@microchip.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: dsa: lan9645x: add mdb management
This change adds DSA mdb callbacks for lan9645x, with distinct handling
for L2 multicast (PGID-backed) and IP multicast (port mask inline in
the mac table).
> diff --git a/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c b/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
> index b21e1bf25b0c..15a493c1f215 100644
> --- a/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
> +++ b/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
> @@ -70,6 +70,7 @@ static void lan9645x_teardown(struct dsa_switch *ds)
> destroy_workqueue(lan9645x->owq);
> lan9645x_npi_port_deinit(lan9645x, lan9645x->npi);
> lan9645x_mac_deinit(lan9645x);
> + lan9645x_mdb_deinit(lan9645x);
> mutex_destroy(&lan9645x->fwd_domain_lock);
> }
>
> @@ -141,6 +142,7 @@ static int lan9645x_setup(struct dsa_switch *ds)
> if (err)
> return err;
> lan9645x_mac_init(lan9645x);
> + lan9645x_mdb_init(lan9645x);
Is lan9645x_mdb_deinit() reachable when lan9645x_setup() fails after
this point?
lan9645x_setup() continues after lan9645x_mdb_init() with operations
that can fail, including alloc_ordered_workqueue() near the bottom of
the function which returns -ENOMEM. On error from ds->ops->setup(),
dsa_switch_setup() in net/dsa/dsa.c jumps directly to the notifier
cleanup path and does not invoke ds->ops->teardown:
err = ds->ops->setup(ds);
if (err < 0)
goto unregister_notifier;
Since lan9645x_teardown() is where lan9645x_mdb_deinit() runs,
the freshly initialized mdb_lock (and its lockdep map) is never
torn down if setup fails after lan9645x_mdb_init().
Could lan9645x_mdb_init() be moved later in setup (after the steps
that can still fail), or could setup-time error unwinding be added
for mdb_lock, mact_lock and fwd_domain_lock? The same pattern exists
for mact_lock (from lan9645x_mac_init()) and fwd_domain_lock, and this
change adds mdb_lock to the same class of resources.
>
> /* Link Aggregation Mode: NETDEV_LAG_HASH_L2 */
> lan_wr(ANA_AGGR_CFG_AC_SMAC_ENA |
[ ... ]
> diff --git a/drivers/net/dsa/microchip/lan9645x/lan9645x_port.c b/drivers/net/dsa/microchip/lan9645x/lan9645x_port.c
> index 661cd00465e2..5dee92940f89 100644
> --- a/drivers/net/dsa/microchip/lan9645x/lan9645x_port.c
> +++ b/drivers/net/dsa/microchip/lan9645x/lan9645x_port.c
> @@ -189,6 +189,14 @@ int lan9645x_port_setup(struct dsa_switch *ds, int port)
> ANA_PORT_CFG_PORTID_VAL,
> lan9645x, ANA_PORT_CFG(p->chip_port));
>
> + lan_rmw(ANA_CPU_FWD_CFG_IGMP_REDIR_ENA_SET(true) |
> + ANA_CPU_FWD_CFG_MLD_REDIR_ENA_SET(true) |
> + ANA_CPU_FWD_CFG_IPMC_CTRL_COPY_ENA_SET(true),
> + ANA_CPU_FWD_CFG_IGMP_REDIR_ENA |
> + ANA_CPU_FWD_CFG_MLD_REDIR_ENA |
> + ANA_CPU_FWD_CFG_IPMC_CTRL_COPY_ENA,
> + lan9645x, ANA_CPU_FWD_CFG(p->chip_port));
> +
Should this block be guarded for the NPI (CPU-facing) port?
The IGMP/MLD redirect and IPMC control-copy bits are enabled here for
every chip_port that lan9645x_port_setup() is called on, with no
check for whether the port is the NPI. The VLAN host-mode call a few
lines below explicitly excludes the NPI:
if (p->chip_port != lan9645x->npi)
lan9645x_vlan_set_hostmode(p);
In the related driver for the same family, drivers/net/ethernet/mscc/
ocelot_net.c, ocelot_port_attr_mc_set() toggles these same bits only
on user ports in response to SWITCHDEV_ATTR_ID_BRIDGE_MC_DISABLED.
Can CPU-injected IGMP/MLD/IPMC-control frames end up being redirected
or copied back to the CPU with this configuration, or does IFH_BYPASS=1
fully suppress these classifiers on NPI ingress? If the latter, the
bits on the NPI are at least wasteful; if the former, duplicate
delivery or a trap loop seems possible.
> if (p->chip_port != lan9645x->npi)
> lan9645x_vlan_set_hostmode(p);
>
^ permalink raw reply
* Re: [PATCH net-next v4 7/9] net: dsa: lan9645x: add mac table integration
From: Jakub Kicinski @ 2026-05-06 1:46 UTC (permalink / raw)
To: jensemil.schulzostergaard
Cc: Jakub Kicinski, UNGLinuxDriver, andrew, olteanv, davem, edumazet,
pabeni, horms, robh, krzk+dt, conor+dt, woojung.huh, linux,
Steen.Hegelund, daniel.machon, linux-kernel, netdev, devicetree
In-Reply-To: <20260430-dsa_lan9645x_switch_driver_base-v4-7-f1b6005fa8b7@microchip.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: dsa: lan9645x: add mac table integration
This patch adds MAC table support and hooks it into the DSA FDB callbacks
(port_fdb_add, port_fdb_del, port_fdb_dump, port_fast_age), along with a
port-filtered flush path and a GET_NEXT-based dump helper.
> diff --git a/drivers/net/dsa/microchip/lan9645x/lan9645x_mac.c b/drivers/net/dsa/microchip/lan9645x/lan9645x_mac.c
> new file mode 100644
> index 000000000000..7ae23d3464f8
> --- /dev/null
> +++ b/drivers/net/dsa/microchip/lan9645x/lan9645x_mac.c
[ ... ]
> +void lan9645x_mac_init(struct lan9645x *lan9645x)
> +{
> + u32 val;
> +
> + mutex_init(&lan9645x->mact_lock);
> +
> + /* Clear the MAC table */
> + lan_wr(ANA_MACACCESS_MAC_TABLE_CMD_SET(CMD_INIT),
> + lan9645x, ANA_MACACCESS);
> +
> + if (lan9645x_rd_poll_timeout(lan9645x, ANA_MACACCESS, val,
> + ANA_MACACCESS_MAC_TABLE_CMD_GET(val) ==
> + CMD_IDLE))
> + dev_err(lan9645x->dev, "mac init timeout\n");
> +}
Should lan9645x_mac_init() propagate the CMD_INIT timeout instead of
logging and returning?
It is declared void and the only timeout handling is a dev_err(). The
caller in lan9645x_setup() invokes it without a return value and then
proceeds to configure flooding PGIDs, per-port CPU forwarding, and
register the dsa_switch_ops callbacks (.port_fdb_add, .port_fdb_del,
.port_fast_age, .port_fdb_dump) while the MAC table command engine may
still be busy.
For comparison, lan9645x_vlan_init() (called just above in setup) treats
the structurally identical clear-table timeout as fatal:
lan_wr(ANA_VLANACCESS_VLAN_TBL_CMD_SET(VLANACCESS_CMD_INIT),
lan9645x, ANA_VLANACCESS);
err = lan9645x_vlan_wait_for_completion(lan9645x);
if (err) {
dev_err(lan9645x->dev, "Vlan clear table failed\n");
return err;
}
Would returning an int from lan9645x_mac_init() and checking it in
lan9645x_setup() be more consistent?
[ ... ]
> +int lan9645x_mact_dsa_dump(struct lan9645x *lan9645x, int port,
> + dsa_fdb_dump_cb_t *cb, void *data)
> +{
> + u8 mac[ETH_ALEN] __aligned(2);
> + u32 mach, macl, maca;
> + int err = 0;
> + u32 autoage;
> + u64 addr;
> + u16 vid;
> + u8 type;
[ ... ]
> + type = ANA_MACACCESS_ENTRYTYPE_GET(maca);
> + mach = lan_rd(lan9645x, ANA_MACHDATA);
> + macl = lan_rd(lan9645x, ANA_MACLDATA);
> +
> + if (ANA_MACACCESS_DEST_IDX_GET(maca) == port &&
> + type == ENTRYTYPE_NORMAL) {
> + addr = (u64)ANA_MACHDATA_MACHDATA_GET(mach) << 32 |
> + macl;
> + u64_to_ether_addr(addr, mac);
> + vid = ANA_MACHDATA_VID_GET(mach);
> + if (vid > VLAN_MAX)
> + vid = 0;
> +
> + err = cb(mac, vid, false, data);
> + if (err)
> + break;
> + }
> + }
Does this filter cause user-added FDB entries to be invisible to
'bridge fdb show'?
lan9645x_fdb_add() unconditionally programs entries as ENTRYTYPE_LOCKED:
return lan9645x_mact_learn(lan9645x, dest, addr, vid, ENTRYTYPE_LOCKED);
but the dump above only reports entries matching ENTRYTYPE_NORMAL, so any
entry installed via 'bridge fdb add dev swpN <mac> self [static]' is
present in hardware and not reported through port_fdb_dump.
For NTF_SELF entries the DSA core path
(net/dsa/user.c:dsa_user_port_fdb_do_dump) depends on port_fdb_dump to
surface entries that are not tracked in the bridge software FDB.
Separately, the callback hardcodes is_static=false for every reported
entry:
err = cb(mac, vid, false, data);
If the ENTRYTYPE_LOCKED filter is relaxed, would is_static need to be
derived from the entry type, similar to ocelot_fdb_dump() in
drivers/net/ethernet/mscc/ocelot.c which uses
(entry.type == ENTRYTYPE_LOCKED)?
> diff --git a/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c b/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
> index c99189ce586e..b21e1bf25b0c 100644
> --- a/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
> +++ b/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
> @@ -139,6 +140,7 @@ static int lan9645x_setup(struct dsa_switch *ds)
> err = lan9645x_vlan_init(lan9645x);
> if (err)
> return err;
> + lan9645x_mac_init(lan9645x);
Related to the lan9645x_mac_init() comment above, should this call
check a return value and abort setup on failure rather than continuing
to register switch ops with a potentially wedged MAC table engine?
[ ... ]
> +static int lan9645x_fdb_add(struct dsa_switch *ds, int port,
> + const unsigned char *addr, u16 vid,
> + struct dsa_db db)
> +{
> + struct net_device *br = lan9645x_db2bridge(db);
> + struct dsa_port *dp = dsa_to_port(ds, port);
> + struct lan9645x *lan9645x = ds->priv;
> + int dest;
> +
> + if (IS_ERR(br))
> + return PTR_ERR(br);
> +
> + if (dsa_port_is_cpu(dp) && !br &&
> + dsa_fdb_present_in_other_db(ds, port, addr, vid, db))
> + return 0;
> +
> + if (!vid)
> + vid = lan9645x_vlan_unaware_pvid(!!br);
> +
> + dest = dsa_port_is_cpu(dp) ? PGID_CPU : port;
> +
> + return lan9645x_mact_learn(lan9645x, dest, addr, vid, ENTRYTYPE_LOCKED);
> +}
See the question above about lan9645x_mact_dsa_dump() filtering on
ENTRYTYPE_NORMAL: entries created here with ENTRYTYPE_LOCKED do not
appear to be reported by port_fdb_dump.
^ permalink raw reply
* Re: [PATCH net-next v4 6/9] net: dsa: lan9645x: add vlan support
From: Jakub Kicinski @ 2026-05-06 1:46 UTC (permalink / raw)
To: jensemil.schulzostergaard
Cc: Jakub Kicinski, UNGLinuxDriver, andrew, olteanv, davem, edumazet,
pabeni, horms, robh, krzk+dt, conor+dt, woojung.huh, linux,
Steen.Hegelund, daniel.machon, linux-kernel, netdev, devicetree
In-Reply-To: <20260430-dsa_lan9645x_switch_driver_base-v4-6-f1b6005fa8b7@microchip.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: dsa: lan9645x: add vlan support
Adds VLAN-aware bridge support to the lan9645x DSA driver, wiring up
.port_vlan_filtering, .port_vlan_add and .port_vlan_del, along with a
new lan9645x_vlan.c that programs the hardware VLAN table and per-port
ingress/egress tag handling. Reserves PVIDs for standalone and unaware
modes to implement FDB isolation.
> net: dsa: lan9645x: add vlan support
>
> Add support for vlanaware bridge. We reserve vid 4095 for standalone
> mode, to implement fdb-isolation. A vlan-unaware bridge uses vid 0.
Does the commit message accurately describe the reserved VID range?
Looking at drivers/net/dsa/microchip/lan9645x/lan9645x_main.h:
#define UNAWARE_PVID 0
#define HOST_PVID 4095
#define VLAN_HSR_PRP 4094
#define VLAN_MAX (VLAN_HSR_PRP - 1)
VLAN_MAX ends up at 4093, and both the add and delete paths reject any
vid > VLAN_MAX, with an extack that says "VLAN range 4094-4095
reserved.". So in practice both 4094 and 4095 are removed from the
user-visible VID space, not only 4095 as the commit log states. Could
the message be updated to reflect that, and to clarify that "A
vlan-unaware bridge uses vid 0" refers to an internal UNAWARE_PVID
rather than to a VID provisioned by the bridge core (the driver
silently returns 0 for vid == 0)?
> diff --git a/drivers/net/dsa/microchip/lan9645x/lan9645x_vlan.c b/drivers/net/dsa/microchip/lan9645x/lan9645x_vlan.c
> new file mode 100644
> index 000000000000..854a2dde1ecf
> --- /dev/null
> +++ b/drivers/net/dsa/microchip/lan9645x/lan9645x_vlan.c
[ ... ]
> +int lan9645x_vlan_port_add_vlan(struct lan9645x_port *p, u16 vid, bool pvid,
> + bool untagged, struct netlink_ext_ack *extack)
> +{
> + struct lan9645x *lan9645x = p->lan9645x;
> + struct lan9645x_vlan_port_info info;
> + struct lan9645x_vlan old_vlan;
> + struct lan9645x_vlan *v;
> + u16 old_pvid;
> +
> + /* Kernel VLAN core adds vid 0, which collides with our UNAWARE_PVID.
> + * We handle priority tagged frames by other means.
> + */
> + if (!vid)
> + return 0;
> +
> + if (vid > VLAN_MAX) {
> + NL_SET_ERR_MSG_MOD(extack, "VLAN range 4094-4095 reserved.");
> + return -EBUSY;
> + }
Is reserving VID 4094 for HSR/PRP appropriate at this point? Grepping
the series shows VLAN_HSR_PRP has no consumer beyond its use in the
definition of VLAN_MAX, and no HSR/PRP code is present in-tree for
this driver.
The net effect is that 4094, a valid 802.1Q VID, is permanently
carved out of the user-visible VLAN space on first merge. Would it be
better to only reserve 4095 now, and introduce the 4094 reservation
together with the HSR/PRP support that actually consumes it?
On the errno choice: -EBUSY conventionally implies a transient
condition that might succeed on retry, but this rejection is
permanent for any VID above VLAN_MAX. Would -EOPNOTSUPP (or -EINVAL,
for a structurally out-of-range VID) be a better fit here, so that
userspace tooling does not interpret the failure as retryable?
> +int lan9645x_vlan_port_del_vlan(struct lan9645x_port *p, u16 vid)
> +{
> + struct lan9645x *lan9645x = p->lan9645x;
> + struct lan9645x_vlan *v;
> +
> + if (!vid)
> + return 0;
> +
> + if (vid > VLAN_MAX)
> + return -EBUSY;
Same question for the delete path: should this be -EOPNOTSUPP (or
similar) rather than -EBUSY for a permanently reserved VID?
^ permalink raw reply
* Re: [PATCH net-next v4 5/9] net: dsa: lan9645x: add bridge support
From: Jakub Kicinski @ 2026-05-06 1:46 UTC (permalink / raw)
To: jensemil.schulzostergaard
Cc: Jakub Kicinski, UNGLinuxDriver, andrew, olteanv, davem, edumazet,
pabeni, horms, robh, krzk+dt, conor+dt, woojung.huh, linux,
Steen.Hegelund, daniel.machon, linux-kernel, netdev, devicetree
In-Reply-To: <20260430-dsa_lan9645x_switch_driver_base-v4-5-f1b6005fa8b7@microchip.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: dsa: lan9645x: add bridge support
This patch adds hardware offload support for a single bridge device in
the lan9645x DSA driver. It introduces the fwd_domain_lock mutex and an
ordered workqueue, and wires up the bridge-integration switch ops.
> diff --git a/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c b/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
> index 6fd66ea67cfd..70f6a11f0753 100644
> --- a/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
> +++ b/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
> @@ -67,7 +67,9 @@ static void lan9645x_teardown(struct dsa_switch *ds)
> {
> struct lan9645x *lan9645x = ds->priv;
>
> + destroy_workqueue(lan9645x->owq);
> lan9645x_npi_port_deinit(lan9645x, lan9645x->npi);
> + mutex_destroy(&lan9645x->fwd_domain_lock);
> }
[ ... ]
> @@ -133,6 +135,8 @@ static int lan9645x_setup(struct dsa_switch *ds)
> return err;
> }
>
> + mutex_init(&lan9645x->fwd_domain_lock);
> +
> /* Link Aggregation Mode: NETDEV_LAG_HASH_L2 */
> lan_wr(ANA_AGGR_CFG_AC_SMAC_ENA |
> ANA_AGGR_CFG_AC_DMAC_ENA,
[ ... ]
> @@ -239,6 +243,11 @@ static int lan9645x_setup(struct dsa_switch *ds)
>
> lan9645x_port_set_tail_drop_wm(lan9645x);
>
> + lan9645x->owq = alloc_ordered_workqueue("%s-owq", 0,
> + dev_name(lan9645x->dev));
> + if (!lan9645x->owq)
> + return -ENOMEM;
> +
Does this error path leak the fwd_domain_lock initialization and the
NPI port setup?
When alloc_ordered_workqueue() fails, lan9645x_setup() returns -ENOMEM
directly. In DSA, dsa_switch_setup() handles a failed ds->ops->setup()
by jumping past the teardown label:
err = ds->ops->setup(ds);
if (err < 0)
goto unregister_notifier;
So lan9645x_teardown() is never invoked on the setup failure path, and
the mutex_destroy(&lan9645x->fwd_domain_lock) and
lan9645x_npi_port_deinit() calls it contains are skipped.
With CONFIG_DEBUG_MUTEXES or lockdep enabled, the mutex debug state is
left dangling against devm-managed memory, and the NPI port hardware
configuration performed by lan9645x_tag_npi_setup() is left in place
until the next reset_switch().
Should the failure path of lan9645x_setup() unwind the mutex_init()
and lan9645x_tag_npi_setup() it performed earlier?
> ds->mtu_enforcement_ingress = true;
> ds->assisted_learning_on_cpu_port = true;
> ds->fdb_isolation = true;
> @@ -257,6 +266,272 @@ static void lan9645x_port_phylink_get_caps(struct dsa_switch *ds, int port,
> lan9645x_phylink_get_caps(ds->priv, port, config);
> }
>
> +static int lan9645x_set_ageing_time(struct dsa_switch *ds, unsigned int msecs)
> +{
> + u32 age_secs = max(1, msecs / MSEC_PER_SEC / 2);
> + struct lan9645x *lan9645x = ds->priv;
> +
> + /* Entry is must suffer two aging scans before it is removed, so it is
> + * aged after 2*AGE_PERIOD, and the unit is in seconds.
> + * An age period of 0 disables automatic aging.
> + */
> + lan_rmw(ANA_AUTOAGE_AGE_PERIOD_SET(msecs ? age_secs : 0),
> + ANA_AUTOAGE_AGE_PERIOD,
> + lan9645x, ANA_AUTOAGE);
> + return 0;
> +}
Can this silently truncate user-supplied ageing times that exceed the
hardware field?
ANA_AUTOAGE_AGE_PERIOD is GENMASK(20, 1), so the maximum representable
age_secs is 2^20 - 1 = 1048575. ANA_AUTOAGE_AGE_PERIOD_SET() expands
to FIELD_PREP(), whose runtime path in include/linux/bitfield.h is:
((typeof(_mask))(_val) << __bf_shf(_mask)) & (_mask)
which masks the value without any range check or warning.
The driver does not populate ds->ageing_time_min or ds->ageing_time_max,
and dsa_switch_ageing_time() gates its -ERANGE checks on those being
non-zero:
if (ds->ageing_time_min && ageing_time < ds->ageing_time_min)
return -ERANGE;
if (ds->ageing_time_max && ageing_time > ds->ageing_time_max)
return -ERANGE;
so a large ageing time set via "ip link set br0 type bridge ageing_time
<large>" reaches this callback and is silently truncated to a short
period. Should lan9645x set ageing_time_min/max so out-of-range values
are rejected instead?
[ ... ]
> +static void lan9645x_host_flood_work_fn(struct work_struct *work)
> +{
> + struct lan9645x_port *p = container_of(work, struct lan9645x_port,
> + host_flood_work);
> + struct lan9645x *lan9645x = p->lan9645x;
> +
> + mutex_lock(&lan9645x->fwd_domain_lock);
> + __lan9645x_port_mark_host_flood(lan9645x, p->chip_port,
> + p->host_flood_uc, p->host_flood_mc);
> + __lan9645x_port_set_host_flood(lan9645x);
> + mutex_unlock(&lan9645x->fwd_domain_lock);
> +}
> +
> +/* Called in atomic context. */
> +static void lan9645x_port_set_host_flood(struct dsa_switch *ds, int port,
> + bool uc, bool mc)
> +{
> + struct lan9645x *lan9645x = ds->priv;
> + struct lan9645x_port *p;
> +
> + p = lan9645x_to_port(lan9645x, port);
> +
> + p->host_flood_uc = uc;
> + p->host_flood_mc = mc;
> + queue_work(lan9645x->owq, &p->host_flood_work);
> +}
Can the worker observe a mixed (uc, mc) tuple that no caller requested?
The atomic-context writer stores p->host_flood_uc and p->host_flood_mc
as two independent unsynchronized writes and then queues the work. The
worker takes fwd_domain_lock and reads both, but the writer never takes
that lock (it cannot — it is called in atomic context, and fwd_domain_lock
is a mutex).
If a second update arrives between the worker's two reads, the worker
can program PGID_UC/MC/MCIPV4/MCIPV6 on the CPU port with a combination
that was never requested by either call. The state converges on the
next work run triggered by the second queue_work(), but the hardware is
transiently programmed with a tuple nobody asked for.
Would it be safer to bundle both booleans into a single atomic word, or
to use a spinlock that can be taken in atomic context to protect the
pair of stores?
> +
> +static int lan9645x_port_bridge_join(struct dsa_switch *ds, int port,
> + struct dsa_bridge bridge,
> + bool *tx_fwd_offload,
> + struct netlink_ext_ack *extack)
[ ... ]
^ permalink raw reply
* Re: [PATCH net-next v4 4/9] net: dsa: lan9645x: add basic dsa driver for LAN9645X
From: Jakub Kicinski @ 2026-05-06 1:45 UTC (permalink / raw)
To: jensemil.schulzostergaard
Cc: Jakub Kicinski, UNGLinuxDriver, andrew, olteanv, davem, edumazet,
pabeni, horms, robh, krzk+dt, conor+dt, woojung.huh, linux,
Steen.Hegelund, daniel.machon, linux-kernel, netdev, devicetree
In-Reply-To: <20260430-dsa_lan9645x_switch_driver_base-v4-4-f1b6005fa8b7@microchip.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: dsa: lan9645x: add basic dsa driver for LAN9645X
This patch introduces a new DSA driver for the Microchip LAN9645X switch
family, covering probe/setup/teardown, NPI CPU port wiring, per-port
initialisation, and phylink MAC integration. A few questions inline.
> diff --git a/drivers/net/dsa/microchip/lan9645x/Kconfig b/drivers/net/dsa/microchip/lan9645x/Kconfig
> new file mode 100644
> index 000000000000..4d9fdf34104e
> --- /dev/null
> +++ b/drivers/net/dsa/microchip/lan9645x/Kconfig
> @@ -0,0 +1,11 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +config NET_DSA_MICROCHIP_LAN9645X
> + tristate "Microchip Lan9645x switch support"
> + depends on NET_DSA
> + select NET_DSA_TAG_LAN9645X
> + help
> + This driver adds DSA support for Microchip Lan9645x switch chips.
The driver is non-functional without a parent MFD that registers the named
regmaps ("gcb", "qs", "chip_top", "rew", "sys", "hsio", "dev", "dev1" ..
"dev8", "qsys", "afi", "ana") consumed by lan9645x_request_target_regmaps().
Should this Kconfig depend on the parent provider symbol so that selecting
this option without the parent driver does not produce a module whose probe
always fails?
Is there a companion parent-driver patch this patch depends on? If so,
would it be worth naming it in the commit message and documenting the
ordering?
> diff --git a/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c b/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
> new file mode 100644
> index 000000000000..6fd66ea67cfd
> --- /dev/null
> +++ b/drivers/net/dsa/microchip/lan9645x/lan9645x_main.c
> @@ -0,0 +1,423 @@
[ ... ]
> +static const char *lan9645x_resource_names[NUM_TARGETS + 1] = {
> + [TARGET_GCB] = "gcb",
> + [TARGET_QS] = "qs",
> + [TARGET_CHIP_TOP] = "chip_top",
> + [TARGET_REW] = "rew",
> + [TARGET_SYS] = "sys",
> + [TARGET_HSIO] = "hsio",
> + [TARGET_DEV] = "dev",
> + [TARGET_DEV + 1] = "dev1",
> + [TARGET_DEV + 2] = "dev2",
> + [TARGET_DEV + 3] = "dev3",
> + [TARGET_DEV + 4] = "dev4",
> + [TARGET_DEV + 5] = "dev5",
> + [TARGET_DEV + 6] = "dev6",
> + [TARGET_DEV + 7] = "dev7",
> + [TARGET_DEV + 8] = "dev8",
The first DEV instance is named "dev", while the rest are "dev1" through
"dev8". Is this asymmetry intentional, or should the first be "dev0" to
match the natural per-port numbering? Without the parent driver available
in this series, it is hard to tell which naming scheme the parent actually
registers.
If the parent registers "dev0".."dev8" then dev_get_regmap(parent, "dev")
returns NULL and probe fails with -ENODEV.
[ ... ]
> +static void lan9645x_set_feat_dis(struct lan9645x *lan9645x)
> +{
> + u32 feat_dis;
> +
> + /* The features which can be physically disabled on some SKUs are:
> + * 1) Number of ports can be 5, 7 or 9. Any ports can be used, the chip
> + * tracks how many are active.
> + * 2) HSR/PRP. The duplicate discard table can be disabled.
> + * 3) TAS, frame preemption and PSFP can be disabled.
> + */
> + feat_dis = lan_rd(lan9645x, GCB_FEAT_DISABLE);
> +
> + lan9645x->num_port_dis =
> + GCB_FEAT_DISABLE_FEAT_NUM_PORTS_DIS_GET(feat_dis);
> + lan9645x->dd_dis = GCB_FEAT_DISABLE_FEAT_DD_DIS_GET(feat_dis);
> + lan9645x->tsn_dis = GCB_FEAT_DISABLE_FEAT_TSN_DIS_GET(feat_dis);
> +}
Where are num_port_dis, dd_dis and tsn_dis consulted after this read?
ds->num_ports is hardcoded to NUM_PHYS_PORTS (9) in lan9645x_probe(), and
there does not appear to be any rejection of TSN or HSR/PRP configuration
on SKUs where tsn_dis=1 or dd_dis=1 in this patch. On a 5-port SKU
(num_port_dis=4), would the driver still register 9 DSA ports and allow
DT to bring up a fused-off port?
> +static int lan9645x_setup(struct dsa_switch *ds)
> +{
[ ... ]
> + dev_info(lan9645x->dev,
> + "SKU features: tsn_dis=%d hsr_dis=%d max_ports=%d\n",
> + lan9645x->tsn_dis, lan9645x->dd_dis,
> + lan9645x->num_phys_ports - lan9645x->num_port_dis);
The log prints the field called dd_dis in struct lan9645x under the name
hsr_dis. Should the label in the format string match the struct field
name, or should the field be renamed?
[ ... ]
> diff --git a/drivers/net/dsa/microchip/lan9645x/lan9645x_phylink.c b/drivers/net/dsa/microchip/lan9645x/lan9645x_phylink.c
> new file mode 100644
> index 000000000000..9eac32ca342c
> --- /dev/null
> +++ b/drivers/net/dsa/microchip/lan9645x/lan9645x_phylink.c
> @@ -0,0 +1,383 @@
[ ... ]
> +/* Port 4 or 7 is RGMII_0 and port 8 is RGMII_1 */
> +#define RGMII_IDX(port) ((port) == 8 ? 1 : 0)
> +
> +void lan9645x_phylink_get_caps(struct lan9645x *lan9645x, int port,
> + struct phylink_config *c)
> +{
> + c->mac_capabilities = MAC_ASYM_PAUSE | MAC_SYM_PAUSE | MAC_10 |
> + MAC_100 | MAC_1000FD | MAC_2500FD;
> +
> + switch (port) {
> + case 0 ... 3:
> + __set_bit(PHY_INTERFACE_MODE_GMII, c->supported_interfaces);
> + break;
> + case 4:
> + __set_bit(PHY_INTERFACE_MODE_GMII, c->supported_interfaces);
> + phy_interface_set_rgmii(c->supported_interfaces);
> + break;
> + case 5 ... 6:
> + /* SerDes ports: QSGMII/SGMII/1000BASEX/2500BASEX modes
> + * require PCS support which is not yet implemented.
> + */
> + break;
> + case 7 ... 8:
> + /* QSGMII mode on ports 7-8 requires SerDes PCS support,
> + * which is not yet implemented.
> + */
> + phy_interface_set_rgmii(c->supported_interfaces);
> + break;
> + default:
> + break;
> + }
> +}
mac_capabilities advertises MAC_2500FD for every port, but none of the
supported_interfaces entries allow negotiating 2.5 Gb/s. GMII and RGMII
do not carry 2.5 Gb/s, and ports 5-6 have an empty supported_interfaces
bitmap. Should MAC_2500FD be dropped until a PCS-capable interface is
added, or should it be set only on ports that will actually support it?
For ports 5 and 6, supported_interfaces is left empty. Will phylink_create()
succeed for those ports given no interface mode is permitted, or will they
be silently unusable?
The commit message states:
The lan9645x switch is a multi-port Gigabit AVB/TSN Ethernet Switch
with five integrated 10/100/1000Base-T PHYs. In addition to the
integrated PHYs, it supports up to 2 RGMII/RMII, up to 2
BASE-X/SERDES/2.5GBASE-X and one Quad-SGMII/Quad-USGMII interfaces.
Given ports 5-6 have no supported_interfaces and ports 7-8 expose only
RGMII in this patch, would it help to note in the commit message that
SerDes, QSGMII/USGMII and 2.5GBASE-X are not wired up yet?
[ ... ]
> +static int lan9645x_phylink_mac_prepare(struct phylink_config *config,
> + unsigned int mode,
> + phy_interface_t iface)
> +{
> + struct lan9645x_port *p = lan9645x_phylink_config_to_port(config);
> + struct lan9645x *lan9645x = p->lan9645x;
> + int port = p->chip_port;
> + bool is_rgmii;
> + u32 mask;
> +
> + if (port == 5 || port == 6 || port > 8)
> + return -EINVAL;
> +
> + mask = HSIO_HW_CFG_GMII_ENA_SET(BIT(port));
> + lan_rmw(mask, mask, lan9645x, HSIO_HW_CFG);
> +
> + is_rgmii = phy_interface_mode_is_rgmii(iface);
> + if (port == 4)
> + lan_rmw(HSIO_HW_CFG_RGMII_0_CFG_SET(is_rgmii),
> + HSIO_HW_CFG_RGMII_0_CFG,
> + lan9645x, HSIO_HW_CFG);
> +
> + return 0;
> +}
The HSIO_HW_CFG_RGMII_0_CFG bit is only written when port == 4, but the
comment above RGMII_IDX() states:
/* Port 4 or 7 is RGMII_0 and port 8 is RGMII_1 */
#define RGMII_IDX(port) ((port) == 8 ? 1 : 0)
Port 7 also maps to RGMII_0. If port 7 is configured for RGMII, does the
RGMII_0 mux ever get routed to port 7? And if port 4 is later configured
in GMII mode, the write HSIO_HW_CFG_RGMII_0_CFG_SET(0) would run and
appear to reroute RGMII_0 away from port 7.
Is there meant to be mutual-exclusion between port 4 and port 7 in RGMII
mode, and a write path for port 7 as well?
> +static void lan9645x_rgmii_dll_config(struct lan9645x_port *p)
> +{
> + u32 rx_idx, tx_idx;
> +
> + /* DLL register layout:
> + * (N*2): RGMII_N_RX
> + * (N*2)+1: RGMII_N_TX
> + */
> + rx_idx = RGMII_IDX(p->chip_port) * 2;
> + tx_idx = RGMII_IDX(p->chip_port) * 2 + 1;
> +
> + /* Enable DLL in RGMII clock paths, deassert DLL reset, and start the
> + * delay tune FSM.
> + */
> + lan_rmw(HSIO_DLL_CFG_DLL_CLK_ENA_SET(1) |
> + HSIO_DLL_CFG_DLL_RST_SET(0) |
> + HSIO_DLL_CFG_DLL_ENA_SET(p->rx_internal_delay) |
> + HSIO_DLL_CFG_DELAY_ENA_SET(p->rx_internal_delay),
> + HSIO_DLL_CFG_DLL_CLK_ENA |
> + HSIO_DLL_CFG_DLL_RST |
> + HSIO_DLL_CFG_DLL_ENA |
> + HSIO_DLL_CFG_DELAY_ENA,
> + p->lan9645x, HSIO_DLL_CFG(rx_idx));
> +
> + lan_rmw(HSIO_DLL_CFG_DLL_CLK_ENA_SET(1) |
> + HSIO_DLL_CFG_DLL_RST_SET(0) |
> + HSIO_DLL_CFG_DLL_ENA_SET(p->tx_internal_delay) |
> + HSIO_DLL_CFG_DELAY_ENA_SET(p->tx_internal_delay),
> + HSIO_DLL_CFG_DLL_CLK_ENA |
> + HSIO_DLL_CFG_DLL_RST |
> + HSIO_DLL_CFG_DLL_ENA |
> + HSIO_DLL_CFG_DELAY_ENA,
> + p->lan9645x, HSIO_DLL_CFG(tx_idx));
> +}
This function consults only p->rx_internal_delay and p->tx_internal_delay
and never looks at the phy_interface_t. Phylink expects the MAC to apply
internal delay only when the interface is RGMII_ID, RGMII_TXID or
RGMII_RXID, and not when the interface is plain PHY_INTERFACE_MODE_RGMII
(where the PHY provides delay).
With a DT that specifies phy-mode = "rgmii" and nonzero
rx-internal-delay-ps, will both the PHY and the MAC add delay?
And with phy-mode = "rgmii-id" but no {rx,tx}-internal-delay-ps property
in DT, will the MAC fail to add delay on either side?
> diff --git a/drivers/net/dsa/microchip/lan9645x/lan9645x_port.c b/drivers/net/dsa/microchip/lan9645x/lan9645x_port.c
> new file mode 100644
> index 000000000000..394a20ee678f
> --- /dev/null
> +++ b/drivers/net/dsa/microchip/lan9645x/lan9645x_port.c
[ ... ]
> +int lan9645x_port_setup(struct dsa_switch *ds, int port)
> +{
> + struct dsa_port *dp = dsa_to_port(ds, port);
> + struct lan9645x *lan9645x = ds->priv;
> + struct lan9645x_port *p;
> +
> + p = lan9645x_to_port(lan9645x, port);
> +
> + if (dp->dn) {
> + u32 val;
> +
> + if (!of_property_read_u32(dp->dn, "rx-internal-delay-ps", &val))
> + p->rx_internal_delay = val > 0;
> +
> + if (!of_property_read_u32(dp->dn, "tx-internal-delay-ps", &val))
> + p->tx_internal_delay = val > 0;
> + }
The u32 picosecond value from DT is collapsed to a boolean via "val > 0",
discarding the tap selection. Is the discarded precision intentional, and
if so, why read the value as u32 in the first place rather than using
of_property_read_bool() on a differently-named property?
^ permalink raw reply
* Re: [PATCH net-next v4 1/9] net: dsa: add tag driver for LAN9645X
From: Jakub Kicinski @ 2026-05-06 1:45 UTC (permalink / raw)
To: jensemil.schulzostergaard
Cc: Jakub Kicinski, UNGLinuxDriver, andrew, olteanv, davem, edumazet,
pabeni, horms, robh, krzk+dt, conor+dt, woojung.huh, linux,
Steen.Hegelund, daniel.machon, linux-kernel, netdev, devicetree
In-Reply-To: <20260430-dsa_lan9645x_switch_driver_base-v4-1-f1b6005fa8b7@microchip.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: dsa: add tag driver for LAN9645X
Adds a DSA tag driver for Microchip LAN9645x switches operating with a
front port configured as an NPI CPU port. RX uses a 16-byte long prefix
plus a 28-byte internal frame header; TX uses only the IFH.
> diff --git a/net/dsa/tag_lan9645x.c b/net/dsa/tag_lan9645x.c
> new file mode 100644
> index 000000000000..81e7a78e0f81
> --- /dev/null
> +++ b/net/dsa/tag_lan9645x.c
[ ... ]
> +static struct sk_buff *lan9645x_rcv(struct sk_buff *skb,
> + struct net_device *ndev)
> +{
> + u32 src_port, qos_class, vlan_tci, tag_type, popcnt, etype_ofs;
> + struct dsa_port *dp;
> + u32 ifh_gap_len = 0;
> + u16 vlan_tpid;
> + u8 *ifh;
> +
> + /* DSA master already consumed DMAC,SMAC,ETYPE from long prefix. Go back
> + * to beginning of frame.
> + */
> + skb_push(skb, ETH_HLEN);
> +
> + if (unlikely(!pskb_may_pull(skb, LAN9645X_TOTAL_TAG_LEN)))
> + return NULL;
> +
> + /* IFH starts after our long prefix */
> + ifh = skb_pull(skb, LAN9645X_LONG_PREFIX_LEN);
[ ... ]
> + skb_pull(skb, LAN9645X_IFH_LEN);
> +
> + if (unlikely(!pskb_may_pull(skb, ifh_gap_len + ETH_HLEN)))
> + return NULL;
> +
> + skb_pull(skb, ifh_gap_len);
> + skb_reset_mac_header(skb);
> + skb_set_network_header(skb, ETH_HLEN);
> + skb_reset_mac_len(skb);
> +
> + /* Remove the long prefix + IFH + ifh_gap contribution from
> + * skb->csum so the stack sees a checksum consistent with the
> + * real Ethernet frame. skb->data currently points at the real
> + * MAC header.
> + */
> + skb_postpull_rcsum(skb,
> + skb->data - LAN9645X_TOTAL_TAG_LEN - ifh_gap_len,
> + LAN9645X_TOTAL_TAG_LEN + ifh_gap_len);
Does this skb_postpull_rcsum() call subtract the wrong region from
skb->csum for CHECKSUM_COMPLETE frames?
By the time lan9645x_rcv() is entered, the conduit's eth_type_trans()
has already pulled the first ETH_HLEN (14) bytes of the long prefix and
adjusted skb->csum accordingly, so skb->csum covers bytes starting at
offset 14 (the remaining 2 bytes of the long prefix plus IFH plus any
gap plus the real frame).
At the point of the skb_postpull_rcsum() call, skb->data points at the
real MAC header, i.e. at offset LAN9645X_TOTAL_TAG_LEN + ifh_gap_len
from the start of the originally-received frame. So
skb->data - LAN9645X_TOTAL_TAG_LEN - ifh_gap_len points at offset 0,
and the subtracted region is [0, LAN9645X_TOTAL_TAG_LEN + ifh_gap_len).
That region includes the first 14 bytes that eth_type_trans() already
removed from skb->csum, and it stops short of the real MAC header which
also needs to be removed. Shouldn't the subtracted region instead start
at what skb->data was on entry to rcv() (i.e. post-eth_type_trans) and
cover LAN9645X_TOTAL_TAG_LEN + ifh_gap_len bytes from there, so that it
matches what skb->csum actually covers?
For reference, tag_ocelot.c (cited in the commit message as the
architectural model) saves start = skb->data at function entry before
any skb_push()/skb_pull() manipulation:
net/dsa/tag_ocelot.c:ocelot_rcv() {
...
u8 *start = skb->data;
...
skb_push(skb, ETH_HLEN);
skb_pull(skb, OCELOT_SHORT_PREFIX_LEN);
...
}
and later passes that saved pointer to skb_postpull_rcsum() with the
total tag length, so the subtracted region exactly matches what the
NIC's csum_complete covers.
Would saving skb->data on entry (before the initial skb_push(ETH_HLEN))
and passing that as the start argument, with length
LAN9645X_TOTAL_TAG_LEN + ifh_gap_len, produce the correct result here
as well?
^ 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