* Re: [PATCH v20 net-next 06/10] net/nebula-matrix: add intr resource implementation
From: Paolo Abeni @ 2026-07-02 9:42 UTC (permalink / raw)
To: illusion.wang
Cc: dimon.zhao, alvin.wang, sam.chen, netdev, andrew+netdev, corbet,
kuba, horms, linux-doc, pabeni, vadim.fedorenko, lukas.bulwahn,
edumazet, enelsonmoore, skhan, hkallweit1, linux-kernel
In-Reply-To: <20260630010718.4346-7-illusion.wang@nebula-matrix.com>
From: AI Reviewer <ai@example.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.
Full review at: https://sashiko.dev/
---
net/nebula-matrix: add intr resource implementation
This patch adds the nbl_interrupt module for management of driver-private
MSI-X indices. It adds hw_ops callbacks to program the function MSI-X mapping
registers, configure MSI-X info, and enable mailbox interrupts.
> diff --git a/drivers/net/ethernet/nebula-matrix/nbl/nbl_hw/nbl_interrupt.c b/drivers/net/ethernet/nebula-matrix/nbl/nbl_hw/nbl_interrupt.c
> new file mode 100644
> index 0000000000000..53a657aef9ca0
> --- /dev/null
> +++ b/drivers/net/ethernet/nebula-matrix/nbl/nbl_hw/nbl_interrupt.c
> @@ -0,0 +1,267 @@
[ ... ]
> +/*
> + * This function tears down the MSI-X mapping for a function by clearing the
> + * interrupt bitmaps, disabling MSI-X via HW callbacks, and resetting the
> + * bookkeeping state. The DMA buffer and interrupts[] array are intentionally
> + * left allocated for potential reuse on reset/hot-add paths. Caller must
> + * ensure this is only invoked when the HW cannot issue in-flight DMA to
> + * the region (IRQs already freed or not yet configured).
> + */
> +int nbl_res_intr_destroy_msix_map(struct nbl_resource_mgt *res_mgt,
> + u16 func_id)
> +{
[ ... ]
> + /* use ctrl dev bdf */
> + hw_ops->configure_msix_map(res_mgt->hw_ops_tbl->priv, func_id, false, 0,
> + 0, 0, 0);
> +
> + intr_mgt->func_intr_res[func_id].interrupts = NULL;
> + intr_mgt->func_intr_res[func_id].num_interrupts = 0;
> + intr_mgt->func_intr_res[func_id].msix_map_table.base_addr = NULL;
> + intr_mgt->func_intr_res[func_id].msix_map_table.size = 0;
> + hw_ops->flush_write(res_mgt->hw_ops_tbl->priv);
> +
> + return 0;
> +}
[Severity: High]
Does this code leak the devm-managed resources? The comment mentions that
the DMA buffer and interrupts array are left allocated for potential reuse,
but the pointers are set to NULL here. This causes the pointers to be lost
so they cannot be reused or explicitly freed later.
Because nbl_res_intr_configure_msix_map() unconditionally allocates new devres
memory on every call, won't this cause the devres list to accumulate leaked
allocations (dmam_alloc_coherent and devm_kcalloc) each time the device is
reset or a VF is reconfigured?
[ ... ]
> +int nbl_res_intr_configure_msix_map(struct nbl_resource_mgt *res_mgt,
> + u16 func_id, u16 num_net_msix,
> + u16 num_others_msix,
> + bool net_msix_mask_en)
> +{
[ ... ]
> + nbl_res_intr_destroy_msix_map(res_mgt, func_id);
> +
> + msix_map_table = &intr_mgt->func_intr_res[func_id].msix_map_table;
> + WARN_ON(msix_map_table->base_addr);
> + msix_map_table->size =
> + sizeof(struct nbl_msix_map) * NBL_MSIX_MAP_TABLE_MAX_ENTRIES;
> + /* HW indexes the MSI-X map table with a fixed stride of 1024 entries,
> + * so we must allocate the full table even if fewer vectors are used.
> + */
> + msix_map_table->base_addr = dmam_alloc_coherent(dev,
> + msix_map_table->size,
> + &msix_map_table->dma,
> + GFP_KERNEL);
> + if (!msix_map_table->base_addr) {
> + dev_err(dev, "Allocate DMA memory for function msix map table failed\n");
> + msix_map_table->size = 0;
> + return -ENOMEM;
> + }
> +
> + interrupts = devm_kcalloc(dev, requested, sizeof(interrupts[0]),
> + GFP_KERNEL);
> + if (!interrupts) {
> + ret = -ENOMEM;
> + goto alloc_interrupts_err;
> + }
[Severity: High]
If devm_kcalloc() fails, the goto alloc_interrupts_err path bypasses
clearing msix_map_table->base_addr and size.
While nbl_res_intr_destroy_msix_map() clears it unconditionally on the next
call, doesn't this leave the internal state inconsistent by keeping base_addr
populated when interrupts is NULL?
[ ... ]
> +get_interrupt_err:
> + while (i--) {
> + intr_index = interrupts[i];
> + if (intr_index >= NBL_NET_INTR_BASE)
> + clear_bit(intr_index - NBL_NET_INTR_BASE,
> + intr_mgt->interrupt_net_bitmap);
> + else
> + clear_bit(intr_index,
> + intr_mgt->interrupt_others_bitmap);
> + }
> +
> + intr_mgt->func_intr_res[func_id].num_interrupts = 0;
> + intr_mgt->func_intr_res[func_id].interrupts = NULL;
> + intr_mgt->func_intr_res[func_id].num_net_interrupts = 0;
> + intr_mgt->func_intr_res[func_id].msix_map_table.base_addr = NULL;
> + intr_mgt->func_intr_res[func_id].msix_map_table.size = 0;
> +alloc_interrupts_err:
> + return ret;
> +}
--
This is an AI-generated review.
^ permalink raw reply
* Re: [PATCH v20 net-next 08/10] net/nebula-matrix: add Dispatch layer implementation
From: Paolo Abeni @ 2026-07-02 9:43 UTC (permalink / raw)
To: illusion.wang
Cc: dimon.zhao, alvin.wang, sam.chen, netdev, andrew+netdev, corbet,
kuba, horms, linux-doc, pabeni, vadim.fedorenko, lukas.bulwahn,
edumazet, enelsonmoore, skhan, hkallweit1, linux-kernel
In-Reply-To: <20260630010718.4346-9-illusion.wang@nebula-matrix.com>
From: AI Reviewer <ai@example.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.
Full review at: https://sashiko.dev/
---
net/nebula-matrix: add Dispatch layer implementation
This patch introduces a control-level routing mechanism for the Dispatch layer,
supporting direct and channel routing paths based on declared control levels.
It establishes the message handling framework and sets up the resource layer
locking strategy for operations like MSI-X configuration.
> diff --git a/drivers/net/ethernet/nebula-matrix/nbl/nbl_core/nbl_dispatch.c b/drivers/net/ethernet/nebula-matrix/nbl/nbl_core/nbl_dispatch.c
> index f0b4406ca5601..ca565c6c4819f 100644
> --- a/drivers/net/ethernet/nebula-matrix/nbl/nbl_core/nbl_dispatch.c
> +++ b/drivers/net/ethernet/nebula-matrix/nbl/nbl_core/nbl_dispatch.c
[ ... ]
> +static void nbl_disp_chan_configure_msix_map_resp(void *priv, u16 src_id,
> + u16 msg_id, void *data,
> + u32 data_len)
> +{
> + struct nbl_dispatch_mgt *disp_mgt = (struct nbl_dispatch_mgt *)priv;
> + struct nbl_channel_ops *chan_ops = disp_mgt->chan_ops_tbl->ops;
> + struct nbl_resource_ops *res_ops = disp_mgt->res_ops_tbl->ops;
> + struct nbl_resource_mgt *p = disp_mgt->res_ops_tbl->priv;
> + struct device *dev = disp_mgt->common->dev;
> + struct nbl_chan_param_cfg_msix_map param = { 0 };
> + struct nbl_chan_ack_info chan_ack;
> + int err = NBL_CHAN_RESP_OK;
> + int copy_len;
> + int ret;
> +
> + copy_len = data_len < sizeof(param) ? data_len : sizeof(param);
> + memcpy(¶m, data, copy_len);
> + ret = NBL_OPS_CALL_LOCK_RET(disp_mgt, res_ops->configure_msix_map, p,
> + src_id, le16_to_cpu(param.num_net_msix),
> + le16_to_cpu(param.num_others_msix),
> + le16_to_cpu(param.msix_mask_en));
[Severity: High]
This is a pre-existing issue, but does exposing this configuration via
channel messages allow a VM to trigger an unbounded memory leak?
Looking at the underlying nbl_res_intr_configure_msix_map() implementation,
it allocates device-managed memory for the MSI-X map table:
nbl_res_intr_configure_msix_map() {
...
msix_map_table->base_addr = dmam_alloc_coherent(dev,
msix_map_table->size,
&msix_map_table->dma,
GFP_KERNEL);
...
}
However, prior to this allocation, it appears to call
nbl_res_intr_destroy_msix_map(), which merely clears the pointer without
freeing the previously allocated memory:
nbl_res_intr_destroy_msix_map() {
...
intr_mgt->func_intr_res[func_id].msix_map_table.base_addr = NULL;
...
}
Since this patch wires up nbl_disp_chan_configure_msix_map_resp() to
process NBL_CHAN_MSG_CONFIGURE_MSIX_MAP channel messages from untrusted VMs,
could a compromised VM repeatedly send these messages to exhaust host memory
and DMA mapping space?
> + if (ret)
> + err = NBL_CHAN_RESP_ERR;
> +
> + NBL_CHAN_ACK(chan_ack, src_id, NBL_CHAN_MSG_CONFIGURE_MSIX_MAP, msg_id,
> + err, NULL, 0);
> + ret = chan_ops->send_ack(disp_mgt->chan_ops_tbl->priv, &chan_ack);
> + if (ret)
> + dev_err(dev,
> + "channel send ack failed with ret: %d, msg_type: %d\n",
> + ret, NBL_CHAN_MSG_CONFIGURE_MSIX_MAP);
> +}
--
This is an AI-generated review.
^ permalink raw reply
* Re: [PATCH v20 net-next 09/10] net/nebula-matrix: add common/ctrl dev init/remove operation
From: Paolo Abeni @ 2026-07-02 9:43 UTC (permalink / raw)
To: illusion.wang
Cc: dimon.zhao, alvin.wang, sam.chen, netdev, andrew+netdev, corbet,
kuba, horms, linux-doc, pabeni, vadim.fedorenko, lukas.bulwahn,
edumazet, enelsonmoore, skhan, hkallweit1, linux-kernel
In-Reply-To: <20260630010718.4346-10-illusion.wang@nebula-matrix.com>
From: AI Reviewer <ai@example.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.
Full review at: https://sashiko.dev/
---
net/nebula-matrix: add common/ctrl dev init/remove operation
This patch adds nbl_dev_setup_common_dev and nbl_dev_setup_ctrl_dev helpers,
plus matching teardown paths, and wires them into nbl_dev_init and
nbl_dev_remove.
> diff --git a/drivers/net/ethernet/nebula-matrix/nbl/nbl_core/nbl_dev.c b/drivers/net/ethernet/nebula-matrix/nbl/nbl_core/nbl_dev.c
> index 5deb21e35f8ef..8feae561d0f13 100644
> --- a/drivers/net/ethernet/nebula-matrix/nbl/nbl_core/nbl_dev.c
> +++ b/drivers/net/ethernet/nebula-matrix/nbl/nbl_core/nbl_dev.c
[ ... ]
> @@ -38,11 +190,29 @@ int nbl_dev_init(struct nbl_adapter *adapter)
> dev_mgt->chan_ops_tbl = chan_ops_tbl;
> adapter->core.dev_mgt = dev_mgt;
>
[Severity: High]
Does this initialize the common device components before the chip is actually
initialized?
If nbl_dev_setup_common_dev() configures queues (which accesses hardware)
before the hardware is brought up via nbl_dev_setup_ctrl_dev(), could writing
to those queue registers cause hardware faults?
> + ret = nbl_dev_setup_common_dev(adapter);
> + if (ret)
> + return ret;
> +
> + if (common->has_ctrl) {
> + ret = nbl_dev_setup_ctrl_dev(adapter);
> + if (ret)
> + goto setup_ctrl_dev_fail;
> + }
>
> return 0;
> +setup_ctrl_dev_fail:
> + nbl_dev_remove_common_dev(adapter);
> + return ret;
> }
>
> void nbl_dev_remove(struct nbl_adapter *adapter)
> {
[Severity: High]
Are the teardown operations in the correct order here?
The comment in nbl_dev_remove_ctrl_dev() mentions that setting the driver
status to false triggers the firmware to clean up the qinfo registers.
If nbl_dev_remove_ctrl_dev() deinitializes the chip first, will the
subsequent call to nbl_dev_remove_common_dev() attempt to tear down the
queues by writing directly to those deactivated qinfo registers?
Could this lead to PCIe master aborts or panics during driver unbind?
> + struct nbl_common_info *common = &adapter->common;
> +
> + if (common->has_ctrl)
> + nbl_dev_remove_ctrl_dev(adapter);
> + nbl_dev_remove_common_dev(adapter);
> }
>
> /* ---------- Dev start process ---------- */
--
This is an AI-generated review.
^ permalink raw reply
* Re: [PATCH v20 net-next 10/10] net/nebula-matrix: add common dev start/stop operation
From: Paolo Abeni @ 2026-07-02 9:43 UTC (permalink / raw)
To: illusion.wang
Cc: dimon.zhao, alvin.wang, sam.chen, netdev, andrew+netdev, corbet,
kuba, horms, linux-doc, pabeni, vadim.fedorenko, lukas.bulwahn,
edumazet, enelsonmoore, skhan, hkallweit1, linux-kernel
In-Reply-To: <20260630010718.4346-11-illusion.wang@nebula-matrix.com>
From: AI Reviewer <ai@example.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.
Full review at: https://sashiko.dev/
---
net/nebula-matrix: add common dev start/stop operation
This patch wires up nbl_dev_start() and nbl_dev_stop() to configure
the MSI-X map, allocate vectors, and request/enable the mailbox IRQ.
It also adds a workqueue in nbl_dev_init() for the new IRQ handler.
(Note: Due to recitation restrictions, the standard inline diff quoting format
has been replaced with a free-form summary of the findings below.)
[Severity: High]
In drivers/net/ethernet/nebula-matrix/nbl/nbl_core/nbl_dev.c, looking at
nbl_dev_configure_msix_map(), the loop that calculates msix_not_net_num
explicitly starts at NBL_MSIX_MAILBOX_TYPE.
Does this calculation omit NBL_MSIX_VIRTIO_TYPE entirely?
If the virtio type is skipped, will the hardware map table lack sufficient
entries, potentially leading to memory corruption or an inability to map
VIRTIO interrupts since disp_ops->configure_msix_map() is called with an
undersized sum?
[Severity: High]
This is a pre-existing issue, but it is exposed by wiring up the new dynamic
start and stop paths in this patch.
In drivers/net/ethernet/nebula-matrix/nbl/nbl_hw/nbl_interrupt.c, looking at
nbl_res_intr_destroy_msix_map(), it sets msix_map_table->base_addr and the
interrupts pointers to NULL, but does not appear to free the device-managed
allocations created by dmam_alloc_coherent() and devm_kcalloc().
When nbl_dev_start() subsequently calls nbl_res_intr_configure_msix_map(),
it will allocate entirely new buffers because the old pointers were lost.
Could this introduce a persistent memory and DMA space leak on every
start/stop cycle, such as when bringing the network interface up and down?
--
This is an AI-generated review.
^ permalink raw reply
* Re: [PATCH net-next v11 1/7] dt-bindings: phy: document the serdes PHY on sa8255p
From: Bartosz Golaszewski @ 2026-07-02 9:44 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Vinod Koul, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin,
Alexandre Torgue, Giuseppe Cavallaro, Chen-Yu Tsai,
Jernej Skrabec, Neil Armstrong, Kevin Hilman, Jerome Brunet,
Shawn Guo, Fabio Estevam, Jan Petrous, s32, Mohd Ayaan Anwar,
Romain Gantois, Magnus Damm, Maxime Ripard, Christophe Roullier,
Radu Rendec, linux-arm-msm, devicetree, linux-kernel, netdev,
linux-stm32, linux-arm-kernel, Drew Fustini, linux-sunxi,
linux-amlogic, linux-mips, imx, linux-renesas-soc, linux-rockchip,
sophgo, linux-riscv, Bartosz Golaszewski, Bartosz Golaszewski,
Bartosz Golaszewski
In-Reply-To: <CAMuHMdXNG=C=XcioQUEN1M7cQgKhO0AxUyg5X+TWb2rQ3-H3fw@mail.gmail.com>
On Thu, 2 Jul 2026 11:16:22 +0200, Geert Uytterhoeven
<geert@linux-m68k.org> said:
> Hi Bartosz,
>
> On Thu, 2 Jul 2026 at 11:12, Bartosz Golaszewski <brgl@kernel.org> wrote:
>> On Tue, 30 Jun 2026 12:23:16 +0200, Vinod Koul <vkoul@kernel.org> said:
>> > On 29-06-26, 16:51, Geert Uytterhoeven wrote:
>> >> > Russell King asked me to put the PHY logic for SCMI pm domains into the PHY
>> >> > driver instead of the MAC driver where it was previously. Instead of cramming
>> >> > both HLOS and firmware handling into the same driver, I figured it makes more
>> >> > sense to have a dedicated, cleaner driver as the two share very little code (if
>> >> > any).
>> >>
>> >> I think you are mixing up DT bindings and driver implementation?
>> >
>> > Should the bindings change if we have different driver and firmware
>> > implementations? Isn't binding supposed to be agnostic of
>> > implementations..?
>>
>> I've thought about it some more and I believe this question is philosophical in
>> nature.
>>
>> sa8775p and sa8255p are *the same* hardware. I can flash different firmware on
>> the same Lemans Ride board and it becomes one or the other. Yet they are not
>> described by the same DTS and the bindings differ as well. I don't see why we
>> wouldn't allow the same approach for the this PHY.
>>
>> We treat it as different HW variant when it's managed by firmware - just like
>> we do with the rest of the SoC.
>
> DT describes hardware, not software policy.
>
I'll defer to DT maintainers then for that particular case because it affects
more than just this platform. For instance: Qualcomm Nord[1] is already being
upstreamed with a similar split into common parts and then sources specific to
the SCMI and non-SCMI variants - even though it's the same SoC.
Bartosz
[1] https://lore.kernel.org/all/20260526051300.1669201-1-shengchao.guo@oss.qualcomm.com/
^ permalink raw reply
* [PATCH net-next 1/2] net: ethernet: qualcomm: Unconstify function arguments passed by value
From: Krzysztof Kozlowski @ 2026-07-02 9:49 UTC (permalink / raw)
To: Luo Jie, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, netdev, linux-kernel
Cc: Krzysztof Kozlowski
There is no benefit in marking "const" a pass-by-value (not a pointer)
function argument, because it is passed as a copy on the stack. No code
readability improvements, no additional compiler-time safety for misuse.
Drop such redundant "const".
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
---
drivers/net/ethernet/qualcomm/ppe/ppe_config.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/qualcomm/ppe/ppe_config.c b/drivers/net/ethernet/qualcomm/ppe/ppe_config.c
index e9a0e22907a6..94f69c077949 100644
--- a/drivers/net/ethernet/qualcomm/ppe/ppe_config.c
+++ b/drivers/net/ethernet/qualcomm/ppe/ppe_config.c
@@ -1380,7 +1380,7 @@ int ppe_ring_queue_map_set(struct ppe_device *ppe_dev, int ring_id, u32 *queue_m
}
static int ppe_config_bm_threshold(struct ppe_device *ppe_dev, int bm_port_id,
- const struct ppe_bm_port_config port_cfg)
+ struct ppe_bm_port_config port_cfg)
{
u32 reg, val, bm_fc_val[2];
int ret;
@@ -1586,7 +1586,7 @@ static int ppe_config_qm(struct ppe_device *ppe_dev)
}
static int ppe_node_scheduler_config(struct ppe_device *ppe_dev,
- const struct ppe_scheduler_port_config config)
+ struct ppe_scheduler_port_config config)
{
struct ppe_scheduler_cfg sch_cfg;
int ret, i;
--
2.53.0
^ permalink raw reply related
* [PATCH net-next 2/2] net: ethernet: qualcomm: Constify "queue_map" in ppe_ring_queue_map_set()
From: Krzysztof Kozlowski @ 2026-07-02 9:49 UTC (permalink / raw)
To: Luo Jie, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, netdev, linux-kernel
Cc: Krzysztof Kozlowski
In-Reply-To: <20260702094908.79859-3-krzysztof.kozlowski@oss.qualcomm.com>
"queue_map" is a pointer to "u32" and is not modified by the
ppe_ring_queue_map_set() function, thus can be made a pointer to const to
indicate that function is treating the pointed value read-only. This in
general makes the code easier to follow and a bit safer.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
---
drivers/net/ethernet/qualcomm/ppe/ppe_config.c | 3 ++-
drivers/net/ethernet/qualcomm/ppe/ppe_config.h | 2 +-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/qualcomm/ppe/ppe_config.c b/drivers/net/ethernet/qualcomm/ppe/ppe_config.c
index 94f69c077949..125b73be92b1 100644
--- a/drivers/net/ethernet/qualcomm/ppe/ppe_config.c
+++ b/drivers/net/ethernet/qualcomm/ppe/ppe_config.c
@@ -1367,7 +1367,8 @@ int ppe_rss_hash_config_set(struct ppe_device *ppe_dev, int mode,
*
* Return: 0 on success, negative error code on failure.
*/
-int ppe_ring_queue_map_set(struct ppe_device *ppe_dev, int ring_id, u32 *queue_map)
+int ppe_ring_queue_map_set(struct ppe_device *ppe_dev, int ring_id,
+ const u32 *queue_map)
{
u32 reg, queue_bitmap_val[PPE_RING_TO_QUEUE_BITMAP_WORD_CNT];
diff --git a/drivers/net/ethernet/qualcomm/ppe/ppe_config.h b/drivers/net/ethernet/qualcomm/ppe/ppe_config.h
index 4bb45ca40144..60493e51e0a4 100644
--- a/drivers/net/ethernet/qualcomm/ppe/ppe_config.h
+++ b/drivers/net/ethernet/qualcomm/ppe/ppe_config.h
@@ -313,5 +313,5 @@ int ppe_rss_hash_config_set(struct ppe_device *ppe_dev, int mode,
struct ppe_rss_hash_cfg hash_cfg);
int ppe_ring_queue_map_set(struct ppe_device *ppe_dev,
int ring_id,
- u32 *queue_map);
+ const u32 *queue_map);
#endif
--
2.53.0
^ permalink raw reply related
* [PATCH net v3 1/1] tcp: bound SYN-ACK timers to reqsk timeout range
From: Ren Wei @ 2026-07-02 9:52 UTC (permalink / raw)
To: netdev
Cc: edumazet, ncardwell, kuniyu, davem, pabeni, horms, chia-yu.chang,
ij, idosch, fmancera, bronzed_45_vested, yuuchihsu, yuantan098,
yifanwucs, tomapufckgml, bird, roxy520tt, n05ec
From: Zhiling Zou <roxy520tt@gmail.com>
tcp_synack_retries supplies the SYN-ACK retry limit used by request
socket timers. The same effective limit can also come from TCP_SYNCNT
through icsk_syn_retries, while TCP_DEFER_ACCEPT can keep an ACKed
request alive until rskq_defer_accept is reached.
The request socket timeout counter is incremented before it is used to
compute the next timeout. tcp_reqsk_timeout() and the Fast Open SYN-ACK
timer shift req->timeout by req->num_timeout. Excessive retry or
defer-accept limits can therefore drive these timer paths into invalid
shift counts before the request expires.
Limit tcp_synack_retries to the request socket timer range, clamp the
effective retry and defer-accept limits in the regular request socket
timer path, clamp the Fast Open retry limit, and make the request
socket timeout helper saturate before shifting.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Reported-by: Yuan Tan <yuantan098@gmail.com>
Reported-by: Yifan Wu <yifanwucs@gmail.com>
Reported-by: Juefei Pu <tomapufckgml@gmail.com>
Reported-by: Xin Liu <bird@lzu.edu.cn>
Assisted-by: Codex:gpt-5.4
Signed-off-by: Zhiling Zou <roxy520tt@gmail.com>
Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
---
Changes in v3:
- Order local variables in tcp_reqsk_timeout_sk() by reverse Christmas tree.
- v2 Link: https://lore.kernel.org/all/20260630035009.55201-1-n05ec@lzu.edu.cn/
Changes in v2:
- Keep the existing max_retries calculation in tcp_fastopen_synack_timer()
and only add the clamp, avoiding code churn.
- v1 Link: https://lore.kernel.org/all/02e24eb83639e9d7ecc623f000c60254bb5c40a5.1782643946.git.roxy520tt@gmail.com/
include/net/tcp.h | 19 +++++++++++++++----
net/ipv4/inet_connection_sock.c | 6 +++++-
net/ipv4/sysctl_net_ipv4.c | 2 ++
net/ipv4/tcp_timer.c | 3 ++-
4 files changed, 24 insertions(+), 6 deletions(-)
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 6d376ea4d1c0..ee78436b8964 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -183,6 +183,7 @@ static_assert((1 << ATO_BITS) > TCP_DELACK_MAX);
#define MAX_TCP_KEEPINTVL 32767
#define MAX_TCP_KEEPCNT 127
#define MAX_TCP_SYNCNT 127
+#define MAX_TCP_SYNACK_RETRIES 63
/* Ensure that TCP PAWS checks are relaxed after ~2147 seconds
* to avoid overflows. This assumes a clock smaller than 1 Mhz.
@@ -882,12 +883,22 @@ static inline u32 __tcp_set_rto(const struct tcp_sock *tp)
return usecs_to_jiffies((tp->srtt_us >> 3) + tp->rttvar_us);
}
-static inline unsigned long tcp_reqsk_timeout(struct request_sock *req)
+static inline unsigned long tcp_reqsk_timeout_sk(const struct sock *sk,
+ struct request_sock *req)
{
- u64 timeout = (u64)req->timeout << req->num_timeout;
+ u32 rto_max = tcp_rto_max(sk);
+ u64 timeout = req->timeout;
+
+ if (req->num_timeout >= BITS_PER_TYPE(u64) ||
+ timeout > U64_MAX >> req->num_timeout)
+ return rto_max;
+
+ return (unsigned long)min_t(u64, timeout << req->num_timeout, rto_max);
+}
- return (unsigned long)min_t(u64, timeout,
- tcp_rto_max(req->rsk_listener));
+static inline unsigned long tcp_reqsk_timeout(struct request_sock *req)
+{
+ return tcp_reqsk_timeout_sk(req->rsk_listener, req);
}
u32 tcp_delack_max(const struct sock *sk);
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index 56902bba5483..b74212bae3dd 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -1056,6 +1056,8 @@ static void reqsk_timer_handler(struct timer_list *t)
net = sock_net(sk_listener);
max_syn_ack_retries = READ_ONCE(icsk->icsk_syn_retries) ? :
READ_ONCE(net->ipv4.sysctl_tcp_synack_retries);
+ max_syn_ack_retries = min_t(int, max_syn_ack_retries,
+ MAX_TCP_SYNACK_RETRIES);
/* Normally all the openreqs are young and become mature
* (i.e. converted to established socket) for first timeout.
* If synack was not acknowledged for 1 second, it means
@@ -1086,7 +1088,9 @@ static void reqsk_timer_handler(struct timer_list *t)
}
}
- syn_ack_recalc(req, max_syn_ack_retries, READ_ONCE(queue->rskq_defer_accept),
+ syn_ack_recalc(req, max_syn_ack_retries,
+ min_t(u8, READ_ONCE(queue->rskq_defer_accept),
+ MAX_TCP_SYNACK_RETRIES),
&expire, &resend);
tcp_syn_ack_timeout(req);
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index ca1180dba1de..f9d233b98bbc 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -35,6 +35,7 @@ static int ip_ttl_min = 1;
static int ip_ttl_max = 255;
static int tcp_syn_retries_min = 1;
static int tcp_syn_retries_max = MAX_TCP_SYNCNT;
+static int tcp_synack_retries_max = MAX_TCP_SYNACK_RETRIES;
static int tcp_syn_linear_timeouts_max = MAX_TCP_SYNCNT;
static unsigned long ip_ping_group_range_min[] = { 0, 0 };
static unsigned long ip_ping_group_range_max[] = { GID_T_MAX, GID_T_MAX };
@@ -1034,6 +1035,7 @@ static struct ctl_table ipv4_net_table[] = {
.maxlen = sizeof(u8),
.mode = 0644,
.proc_handler = proc_dou8vec_minmax,
+ .extra2 = &tcp_synack_retries_max
},
#ifdef CONFIG_SYN_COOKIES
{
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index bf171b5e1eb3..bbedf2b9e1bc 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -467,6 +467,7 @@ static void tcp_fastopen_synack_timer(struct sock *sk, struct request_sock *req)
*/
max_retries = READ_ONCE(icsk->icsk_syn_retries) ? :
READ_ONCE(sock_net(sk)->ipv4.sysctl_tcp_synack_retries) + 1;
+ max_retries = min_t(int, max_retries, MAX_TCP_SYNACK_RETRIES);
if (req->num_timeout >= max_retries) {
tcp_write_err(sk);
@@ -488,7 +489,7 @@ static void tcp_fastopen_synack_timer(struct sock *sk, struct request_sock *req)
if (!tp->retrans_stamp)
tp->retrans_stamp = tcp_time_stamp_ts(tp);
tcp_reset_xmit_timer(sk, ICSK_TIME_RETRANS,
- req->timeout << req->num_timeout, false);
+ tcp_reqsk_timeout_sk(sk, req), false);
}
static bool tcp_rtx_probe0_timed_out(const struct sock *sk,
--
2.43.0
^ permalink raw reply related
* Re: [PATCH net-next] octeontx2-pf: link RQ page pools to netdev for Netlink stats
From: patchwork-bot+netdevbpf @ 2026-07-02 9:50 UTC (permalink / raw)
To: Ratheesh Kannoth
Cc: linux-kernel, netdev, andrew+netdev, davem, edumazet, kuba,
pabeni, sgoutham
In-Reply-To: <20260630013814.3657831-1-rkannoth@marvell.com>
Hello:
This patch was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:
On Tue, 30 Jun 2026 07:08:14 +0530 you wrote:
> page_pool_create() only registers pools with the netdev Netlink
> interface when pp_params.netdev is set. Set netdev in page pool
> params.
>
> Signed-off-by: Ratheesh Kannoth <rkannoth@marvell.com>
> ---
> drivers/net/ethernet/marvell/octeontx2/nic/cn20k.c | 1 +
> drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c | 2 +-
> 2 files changed, 2 insertions(+), 1 deletion(-)
Here is the summary with links:
- [net-next] octeontx2-pf: link RQ page pools to netdev for Netlink stats
https://git.kernel.org/netdev/net-next/c/49c5ad3cd518
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* [PATCH v4 1/1] bus: mhi: pci_generic: fix Rolling Wireless RW151 MBIM channel ring size
From: zwq2226404116 @ 2026-07-02 10:00 UTC (permalink / raw)
To: mhi, linux-arm-msm, netdev
Cc: mani, loic.poulain, ryazanov.s.a, andrew+netdev, davem, kuba,
Wanquan Zhong
From: Wanquan Zhong <wanquan.zhong@fibocom.com>
bus: mhi: pci_generic: fix Rolling Wireless RW151 MBIM channel ring size
Increase RW151 MBIM channel ring size from 4 to 32 to match the device
firmware channel configuration.
Signed-off-by: Wanquan Zhong <wanquan.zhong@fibocom.com>
---
v3 -> v4: Drop no_m3 quirk per review; keep RW151 MBIM ring size fix only
drivers/bus/mhi/host/pci_generic.c | 2 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
index d598bb3b3981..d686aef6e158 100644
--- a/drivers/bus/mhi/host/pci_generic.c
+++ b/drivers/bus/mhi/host/pci_generic.c
@@ -949,8 +949,8 @@ static const struct mhi_pci_dev_info mhi_rolling_rw135r_info = {
static const struct mhi_channel_config mhi_rolling_rw151_channels[] = {
MHI_CHANNEL_CONFIG_UL(4, "DIAG", 16, 1),
MHI_CHANNEL_CONFIG_DL(5, "DIAG", 16, 1),
- MHI_CHANNEL_CONFIG_UL(12, "MBIM", 4, 0),
- MHI_CHANNEL_CONFIG_DL(13, "MBIM", 4, 0),
+ MHI_CHANNEL_CONFIG_UL(12, "MBIM", 32, 0),
+ MHI_CHANNEL_CONFIG_DL(13, "MBIM", 32, 0),
MHI_CHANNEL_CONFIG_UL(14, "NMEA", 32, 0),
MHI_CHANNEL_CONFIG_DL(15, "NMEA", 32, 0),
MHI_CHANNEL_CONFIG_UL(32, "DUN", 32, 0),
--
2.50.0
^ permalink raw reply related
* Re: [PATCH v3] Subject: [PATCH] net: gro: fix double aggregation of flush-marked skbs
From: Paolo Abeni @ 2026-07-02 10:02 UTC (permalink / raw)
To: Shiming Cheng, davem, edumazet, kuba, horms, matthias.bgg,
angelogioacchino.delregno, willemb, daniel.zahka, alice, sd,
eilaimemedsnaimel, imv4bel, nbd, dsahern, netdev, linux-kernel,
linux-arm-kernel, linux-mediatek
Cc: stable, lena.wang
In-Reply-To: <20260630023512.26927-1-shiming.cheng@mediatek.com>
Note: the patch subject is quite uncorrected
On 6/30/26 4:35 AM, Shiming Cheng wrote:
> The new skb_gro_receive_list() function is missing a critical safety check
> present in the legacy skb_gro_receive() path. Specifically, it does not
> validate NAPI_GRO_CB(skb)->flush before allowing packet aggregation.
skb_gro_receive_list() is not very "new" and definitely
skb_gro_receive() is not legacy.
> This allows already-GRO'd packets with existing frag_list to be
> re-aggregated into a new GRO session, corrupting the frag_list chain
> structure. When skb_segment() attempts to unpack these malformed packets,
> it encounters invalid state and triggers a kernel panic.
>
> Scenario (Tethering/Device forwarding):
> 1. Driver: Generated aggregated packet P1 via LRO with frag_list
> 2. Dev A: Receives aggregated fraglist packet and flush flag set
> 3. Dev A: Re-enters GRO, skb_gro_receive_list() is called
> 4. Missing flush check allows re-aggregation despite flush flag
> 5. Frag_list chain becomes corrupted (loops or dangling refs)
> 6. Dev B: TX path calls skb_segment(), crashes on corrupted frag_list
I can't parse the above. Is this something that can happen with in-tree
drivers or do you need OoT module to trigger it? In any case please
clarify the actual order and the involved driver. Possibly a stack
strace leading to the critical aggregation could help.
> Fix: Add NAPI_GRO_CB(skb)->flush validation to the early-return check in
> skb_gro_receive_list(), matching the defensive programming pattern of
> skb_gro_receive().
>
> Fixes: 8928756d53d5 ("net: add fraglist GRO/GSO support")
The fix tag is wrong, should be:
Fixes: 3a1296a38d0c ('net: Support GRO/GSO fraglist chaining.')
/P
^ permalink raw reply
* Re: [PATCH net-next 03/12] net: mctp: usblib: Move RX transfer processing to a new mctp-usblib
From: Paolo Abeni @ 2026-07-02 10:09 UTC (permalink / raw)
To: jk
Cc: matt, andrew+netdev, davem, edumazet, kuba, pabeni, gregkh,
netdev, linux-usb
In-Reply-To: <20260630-dev-mctp-usb-1-1-v1-3-86a311fc67b7@codeconstruct.com.au>
From: AI Reviewer <ai@example.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.
Full review at: https://sashiko.dev/
---
net: mctp: usblib: Move RX transfer processing to a new mctp-usblib
This commit extracts common USB receive transfer processing for MCTP into a
new library file (mctp-usblib.c). This is intended to support both the
current host-side driver and a future gadget driver.
> diff --git a/drivers/net/mctp/mctp-usblib.c b/drivers/net/mctp/mctp-usblib.c
> new file mode 100644
> index 0000000000000..9b86eb4310ce3
> --- /dev/null
> +++ b/drivers/net/mctp/mctp-usblib.c
[ ... ]
> +/*
> + * Prepare a transfer buffer for future completion; *bufp and *lenp will
> + * be populated on success.
> + */
> +int mctp_usblib_rx_prepare(struct net_device *netdev,
> + struct mctp_usblib_rx *rx,
> + void **bufp, size_t *lenp, gfp_t gfp)
> +{
> + const unsigned int len = MCTP_USB_1_0_XFER_SIZE;
> + struct sk_buff *skb;
> +
> + skb = __netdev_alloc_skb(netdev, len, gfp);
> + if (!skb)
> + return -ENOMEM;
> +
> + rx->skb = skb;
> +
> + *bufp = skb_tail_pointer(skb);
> + *lenp = skb_tailroom(skb);
[Severity: High]
Does assigning the full skb_tailroom() here create a risk of a USB host
controller hang?
Because of slab alignment and padding, skb_tailroom() often exceeds the
exact requested allocation length (e.g., returning 640 bytes instead of
512). If the MCTP device sends exactly a full USB High-Speed packet
(512 bytes) and does not append a Zero Length Packet (ZLP), the USB
host controller might not terminate the transfer since 512 is a multiple
of wMaxPacketSize and the requested 640 bytes haven't been met.
Could this instead use the exact protocol max transfer size (e.g., passing
the len variable instead of skb_tailroom(skb)) to ensure the controller
completes the transfer correctly?
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(mctp_usblib_rx_prepare);
--
This is an AI-generated review.
^ permalink raw reply
* Re: [PATCH net-next 12/12] net: mctp: usb: Allow multiple urbs in flight
From: Paolo Abeni @ 2026-07-02 10:09 UTC (permalink / raw)
To: jk
Cc: matt, andrew+netdev, davem, edumazet, kuba, pabeni, gregkh,
netdev, linux-usb
In-Reply-To: <20260630-dev-mctp-usb-1-1-v1-12-86a311fc67b7@codeconstruct.com.au>
From: AI Reviewer <ai@example.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.
Full review at: https://sashiko.dev/
---
net: mctp: usb: Allow multiple urbs in flight
Currently, we stop tx queues when we have one urb submitted. This means
we will immediately hit dev_hard_start_xmit's tx-queues-off ->
NETDEV_TX_BUSY case, and revert to the requeue -> gso_skb single-dequeue
path, and no longer be able to pack skbs without an xmit_more
indication.
Instead, allow a few urbs to be in-flight, with a limit of 16kB of data
outstanding (after which we will disable queues). With this, the tx path
will cause fewer requeues (and therefore non-packed transfers) under
normal loads.
> diff --git a/drivers/net/mctp/mctp-usb.c b/drivers/net/mctp/mctp-usb.c
> index 5739c87da1095..28aeb5b258723 100644
> --- a/drivers/net/mctp/mctp-usb.c
> +++ b/drivers/net/mctp/mctp-usb.c
[ ... ]
> @@ -48,22 +51,38 @@ enum {
> MCTP_USB_SUBCLASS_SPAN = 0x02,
> };
>
> +/* We use a total-size limit for outstanding URBs, as the transfer counts
> + * may vary a lot between spanning- and non-spanning modes. In spanning mode,
> + * this will allow for a couple of max-sized transfers to be in flight. In
> + * non-spanning mode, 32.
> + *
> + * We want to avoid disabling the tx queue if possible; doing so will end up
> + * requeueing to gso_skb, and we only dequeue from that one skb at a time,
> + * so can no longer perform transfer packing.
> + */
> +static const unsigned int TX_QMEM_MAX = 16384;
> +
> static void mctp_usb_out_complete(struct urb *urb)
> {
> struct mctp_usblib_tx_ctx *tx_ctx = urb->context;
> struct mctp_usb *mctp_usb = mctp_usblib_tx_ctx_priv(tx_ctx);
> struct net_device *netdev = mctp_usb->netdev;
> unsigned long flags;
> + bool wake = false;
>
> mctp_usblib_tx_send_complete(tx_ctx, netdev, urb->status == 0);
>
> spin_lock_irqsave(&mctp_usb->tx_lock, flags);
> + mctp_usb->tx_qmem -= urb->transfer_buffer_length;
[Severity: High]
Does this code underflow tx_qmem if called concurrently with or after
mctp_usb_stop() zeroes it?
If a user downs the interface while TX URBs are in flight, the USB core
(__usb_hcd_giveback_urb) unanchors the URB before invoking this completion
handler.
Because the URB is unanchored, usb_kill_anchored_urbs() in mctp_usb_stop()
can see an empty anchor list and return immediately, without waiting for
the completion handler.
> + if (mctp_usb->tx_qmem < TX_QMEM_MAX)
> + wake = true;
> usb_unanchor_urb(urb);
> spin_unlock_irqrestore(&mctp_usb->tx_lock, flags);
>
> usb_free_urb(urb);
>
> - netif_wake_queue(netdev);
> + if (wake)
> + netif_wake_queue(netdev);
> }
[ ... ]
> @@ -221,12 +241,15 @@ static int mctp_usb_stop(struct net_device *dev)
> flush_delayed_work(&mctp_usb->rx_retry_work);
>
> usb_kill_urb(mctp_usb->rx_urb);
> - /* we have stopped queues, the anchor's own lock will serialise
> - * access from the urb completion.
> +
> + /* We have stopped queues, the anchor's own lock will serialise
> + * access from the urb completion. We are then guaranteed that no
> + * further completions can occur, so can clear tx_qmem without locking.
> */
> usb_kill_anchored_urbs(&mctp_usb->tx_anchor);
>
> mctp_usblib_tx_cancel(&mctp_usb->tx, dev);
> + mctp_usb->tx_qmem = 0;
[Severity: High]
Is the assumption in the comment correct?
Since __usb_hcd_giveback_urb unanchors the URB before calling the completion
handler, usb_kill_anchored_urbs() doesn't guarantee that all completion
handlers have finished.
If tx_qmem is zeroed here before the completion handler finishes, the
subsequent subtraction in mctp_usb_out_complete() will underflow tx_qmem to a
huge unsigned value, causing the TX queue to be permanently stalled on the
next device open.
>
> return 0;
> }
--
This is an AI-generated review.
^ permalink raw reply
* Re: [PATCH net-next 04/12] net: mctp: usblib: Move TX transfer processing to mctp-usblib
From: Paolo Abeni @ 2026-07-02 10:10 UTC (permalink / raw)
To: jk
Cc: matt, andrew+netdev, davem, edumazet, kuba, pabeni, gregkh,
netdev, linux-usb
In-Reply-To: <20260630-dev-mctp-usb-1-1-v1-4-86a311fc67b7@codeconstruct.com.au>
From: AI Reviewer <ai@example.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.
Full review at: https://netdev-ai.bots.linux.dev/sashiko/
---
net: mctp: usblib: Move TX transfer processing to mctp-usblib
This change moves the TX path from mctp-usb into the mctp-usblib helper
by splitting it into push, send, and complete steps so future packed
transfers can carry multiple MCTP packets per USB transfer. The current
implementation keeps a 1:1 mapping between packets and transfers, with no
tx context persisting beyond a single send.
> diff --git a/drivers/net/mctp/mctp-usb.c b/drivers/net/mctp/mctp-usb.c
> index 531b7c994afb..385350792dd4 100644
> --- a/drivers/net/mctp/mctp-usb.c
> +++ b/drivers/net/mctp/mctp-usb.c
> @@ -29,90 +29,82 @@ struct mctp_usb {
[ ... ]
> @@ -220,7 +212,12 @@ static int mctp_usb_stop(struct net_device *dev)
> flush_delayed_work(&mctp_usb->rx_retry_work);
>
> usb_kill_urb(mctp_usb->rx_urb);
> +
> + spin_lock_irqsave(&mctp_usb->tx_lock, flags);
> usb_kill_urb(mctp_usb->tx_urb);
> + spin_unlock_irqrestore(&mctp_usb->tx_lock, flags);
[Critical]
Can usb_kill_urb() safely be called from under spin_lock_irqsave()?
usb_kill_urb() starts with might_sleep() and then blocks on
wait_event(usb_kill_urb_queue, urb->use_count == 0):
drivers/usb/core/urb.c:usb_kill_urb() {
might_sleep();
...
wait_event(usb_kill_urb_queue, atomic_read(&urb->use_count) == 0);
...
}
With CONFIG_DEBUG_ATOMIC_SLEEP, would this trigger a
"sleeping function called from invalid context" splat because tx_lock
is held with IRQs disabled?
There also appears to be a nested-lock concern with the URB completion
handler taking the same tx_lock:
mctp_usb_out_complete() {
...
spin_lock_irqsave(&mctp_usb->tx_lock, flags);
mctp_usb->tx_urb = NULL;
spin_unlock_irqrestore(&mctp_usb->tx_lock, flags);
...
}
The HCD giveback path needs to run the completion before urb->use_count
drops to zero. If the completion is waiting for tx_lock held by the
caller of usb_kill_urb(), and usb_kill_urb() is itself waiting for
use_count to drop, can this deadlock?
The next patch in the series ("net: mctp: usb: Allow for multiple urb
submissions from a packet tx", 5cf1f88ce49a) appears to remove this
lock-wrapped usb_kill_urb() in favour of usb_kill_anchored_urbs()
called outside the lock — should the fix be folded into this patch
instead?
> +
> + mctp_usblib_tx_cancel(&mctp_usb->tx, dev);
>
> return 0;
> }
[ ... ]
> diff --git a/drivers/net/mctp/mctp-usblib.c b/drivers/net/mctp/mctp-usblib.c
> index 9b86eb4310ce..56eca496bbe4 100644
> --- a/drivers/net/mctp/mctp-usblib.c
> +++ b/drivers/net/mctp/mctp-usblib.c
> @@ -162,6 +162,186 @@ void mctp_usblib_rx_cancel(struct mctp_usblib_rx *rx)
[ ... ]
> +/*
> + * Completion for the ->send() op. This will update netdev stats and
> + * free the tx context.
> + *
> + * Likely called from (atomic) URB completion context.
> + */
> +void mctp_usblib_tx_send_complete(struct mctp_usblib_tx_ctx *tx_ctx,
> + struct net_device *dev, bool ok)
> +{
> + mctp_usblib_tx_ctx_free(tx_ctx);
> +}
> +EXPORT_SYMBOL_GPL(mctp_usblib_tx_send_complete);
[High]
Does the body of mctp_usblib_tx_send_complete() match the comment
above it?
The comment says "This will update netdev stats and free the tx
context", and the function takes dev and ok parameters, but the body
only calls mctp_usblib_tx_ctx_free(tx_ctx) and never invokes
mctp_usblib_tx_stats_update().
Prior to this patch, mctp_usb_out_complete() updated counters for both
success and failure:
case 0:
dev_dstats_tx_add(netdev, skb->len);
...
case -ENOENT:
case -ECONNRESET:
case -ESHUTDOWN:
case -EPROTO:
dev_dstats_tx_dropped(netdev);
...
After this patch, mctp_usb_out_complete() defers stats to
mctp_usblib_tx_send_complete(), which does not touch them. The only
caller of mctp_usblib_tx_stats_update() is the synchronous push()
failure path. Does this mean tx_packets, tx_bytes and the URB-failure
tx_drops counters are no longer incremented for any successful or
asynchronously-failed transmit?
Was the intent to call mctp_usblib_tx_stats_update(tx_ctx, dev, ok)
here before freeing the context?
> +
> +/* Prepare a skb for push() */
> +static int mctp_usblib_tx_skb_prepare(struct sk_buff *skb)
[ ... ]
--
This is an AI-generated review.
^ permalink raw reply
* [PATCH v2 nf-next] ipvs: Move defense_work and est_reload_work to system_dfl_long_wq
From: Ismael Luceno @ 2026-07-02 10:10 UTC (permalink / raw)
To: linux-kernel
Cc: Ismael Luceno, Marco Crivellari, Tejun Heo, Simon Horman,
Julian Anastasov, Pablo Neira Ayuso, Florian Westphal,
Phil Sutter, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, open list:IPVS, open list:IPVS, open list:NETFILTER,
open list:NETFILTER
Under synflood conditions binding these handlers to system_long_wq may
pin them to a saturated CPU.
We've observed improved throughtput on a DPDK/VPP application with this
change, which we attribute to the reduced context switching.
Neither handler has per-CPU data dependencies nor cache locality
requirements that would prevent this change.
Signed-off-by: Ismael Luceno <iluceno@suse.de>
---
CC: Marco Crivellari <marco.crivellari@suse.com>
CC: Tejun Heo <tj@kernel.org>
Changes since v1:
* Rebased on nf-next
* Reworded commit message
net/netfilter/ipvs/ip_vs_ctl.c | 6 +++---
net/netfilter/ipvs/ip_vs_est.c | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index bcf40b8c41cf..d7e669efab4d 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -235,7 +235,7 @@ #define DEFENSE_TIMER_PERIOD 1*HZ
update_defense_level(ipvs);
if (atomic_read(&ipvs->dropentry))
ip_vs_random_dropentry(ipvs);
- queue_delayed_work(system_long_wq, &ipvs->defense_work,
+ queue_delayed_work(system_dfl_long_wq, &ipvs->defense_work,
DEFENSE_TIMER_PERIOD);
}
#endif
@@ -290,7 +290,7 @@ #define DEFENSE_TIMER_PERIOD 1*HZ
atomic_set(&ipvs->est_genid_done, genid);
if (repeat)
- queue_delayed_work(system_long_wq, &ipvs->est_reload_work,
+ queue_delayed_work(system_dfl_long_wq, &ipvs->est_reload_work,
delay);
unlock:
@@ -5126,7 +5126,7 @@ do_ip_vs_get_ctl(struct sock *sk, int cmd, void __user *user, int *len)
goto err;
/* Schedule defense work */
- queue_delayed_work(system_long_wq, &ipvs->defense_work,
+ queue_delayed_work(system_dfl_long_wq, &ipvs->defense_work,
DEFENSE_TIMER_PERIOD);
return 0;
diff --git a/net/netfilter/ipvs/ip_vs_est.c b/net/netfilter/ipvs/ip_vs_est.c
index ab09f5182951..78964aa861e9 100644
--- a/net/netfilter/ipvs/ip_vs_est.c
+++ b/net/netfilter/ipvs/ip_vs_est.c
@@ -243,7 +243,7 @@ #define pr_fmt(fmt) "IPVS: " fmt
/* Bump the kthread configuration genid if stopping is requested */
if (restart)
atomic_inc(&ipvs->est_genid);
- queue_delayed_work(system_long_wq, &ipvs->est_reload_work, 0);
+ queue_delayed_work(system_dfl_long_wq, &ipvs->est_reload_work, 0);
}
/* Start kthread task with current configuration */
^ permalink raw reply related
* Re: [PATCH net-next 10/12] net: mctp: usblib: Add initial kunit tests
From: Paolo Abeni @ 2026-07-02 10:10 UTC (permalink / raw)
To: jk
Cc: matt, andrew+netdev, davem, edumazet, kuba, pabeni, gregkh,
netdev, linux-usb
In-Reply-To: <20260630-dev-mctp-usb-1-1-v1-10-86a311fc67b7@codeconstruct.com.au>
From: AI Reviewer <ai@example.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.
Full review at: https://netdev-ai.bots.linux.dev/sashiko/
---
net: mctp: usblib: Add initial kunit tests
This commit introduces an initial set of kunit tests covering the
mctp-usblib receive path, exercising packet extraction from incoming
USB transfer data.
> diff --git a/drivers/net/mctp/mctp-usblib-test.c b/drivers/net/mctp/mctp-usblib-test.c
> new file mode 100644
> index 000000000000..ac837cbb4436
> --- /dev/null
> +++ b/drivers/net/mctp/mctp-usblib-test.c
> @@ -0,0 +1,330 @@
[ ... ]
> +static struct mctp_usblib_test_dev *mctp_usblib_test_create_dev(void)
> +{
> + struct mctp_usblib_test_dev *dev;
> + struct net_device *ndev;
> + int rc;
> +
> + ndev = alloc_netdev(sizeof(*dev), "mctptest%d", NET_NAME_ENUM,
> + mctp_usblib_dev_setup);
> + if (!ndev)
> + return NULL;
> +
> + dev = netdev_priv(ndev);
> + dev->ndev = ndev;
> + skb_queue_head_init(&dev->rx_pkts);
> +
> + rc = register_netdev(ndev);
> + if (rc) {
> + free_netdev(ndev);
> + return NULL;
> + }
> +
> + rcu_read_lock();
> + dev->mdev = __mctp_dev_get(ndev);
> + dev->mdev->net = mctp_default_net(dev_net(ndev));
> + rcu_read_unlock();
[Low]
Can __mctp_dev_get() return NULL here? Looking at net/mctp/device.c:
struct mctp_dev *__mctp_dev_get(const struct net_device *dev)
{
struct mctp_dev *mdev = rcu_dereference(dev->mctp_ptr);
if (mdev)
if (!refcount_inc_not_zero(&mdev->refs))
return NULL;
return mdev;
}
If mctp_add_dev() fails inside the NETDEV_REGISTER notifier path, mctp_ptr
will not be installed and __mctp_dev_get() returns NULL, making the
subsequent dev->mdev->net = ... a NULL dereference in the test kthread.
> +
> + rtnl_lock();
> + dev_open(ndev, NULL);
> + rtnl_unlock();
[Low]
Is it intentional to ignore the return value of dev_open() here? If
dev_open() fails the netdev is registered but never brought up, and the
test then proceeds against an inconsistently-initialized device.
> +
> + return dev;
> +}
> +
> +static int mctp_usblib_test_dst_output(struct mctp_dst *dst,
> + struct sk_buff *skb)
> +{
> + struct mctp_usblib_test_dev *dev = netdev_priv(skb->dev);
> +
> + skb_queue_tail(&dev->rx_pkts, skb);
> +
> + return 0;
> +}
> +
> +static void mctp_usblib_test_init(struct kunit *test,
> + struct mctp_usblib_test_ctx *ctx)
> +{
> + struct mctp_route *rt = &ctx->rt;
> +
> + ctx->dev = mctp_usblib_test_create_dev();
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx->dev);
> +
> + memset(rt, 0, sizeof(*rt));
> + rt->min = local_eid;
> + rt->max = local_eid;
> + rt->dst_type = MCTP_ROUTE_DIRECT;
> + rt->type = RTN_LOCAL;
> + rt->dev = ctx->dev->mdev;
> + rt->output = mctp_usblib_test_dst_output;
> +
> + list_add_rcu(&ctx->rt.list, &init_net.mctp.routes);
> +}
[High]
What happens to ctx.rt on a KUNIT_ASSERT_* failure later in the test?
The ctx variable is a stack local in mctp_usblib_test_rx_single() and
mctp_usblib_test_rx_pkt_span(), and mctp_usblib_test_init() inserts
&ctx->rt.list into the global RCU-protected list init_net.mctp.routes:
list_add_rcu(&ctx->rt.list, &init_net.mctp.routes);
KUNIT_ASSERT_* macros invoke __kunit_abort() which calls
kunit_try_catch_throw() and ultimately kthread_exit(0), so control never
returns to the test function:
lib/kunit/test.c: __kunit_abort()
-> kunit_try_catch_throw()
-> kthread_exit(0)
When the test kthread exits, its stack is gone, leaving a list_head in
init_net.mctp.routes that points at freed/reused stack memory.
The next RCU traversal of init_net.mctp.routes (subsequent mctp test,
mctp routing code, another kunit run) would then dereference that
dangling pointer.
The same abort path also bypasses unregister_netdev() and the
mctp_dev_put() done by mctp_usblib_test_fini().
Would using kunit_add_action() / kunit_add_action_or_reset() (or suite
.init/.exit hooks together with kunit_kmalloc()) ensure list_del_rcu(),
synchronize_rcu() and the netdev teardown always run, regardless of
assertion outcome?
> +
[ ... ]
> +static void mctp_usblib_test_rx_single(struct kunit *test)
> +{
> + struct mctp_usblib_test_dev *dev;
> + struct mctp_usblib_test_ctx ctx;
> + struct mctp_usblib_rx rx;
> + struct sk_buff *skb;
> + size_t len;
> + void *buf;
> + int rc;
> +
> + mctp_usblib_test_init(test, &ctx);
> + dev = ctx.dev;
> +
> + mctp_usblib_rx_init(&rx, true);
> +
> + rc = mctp_usblib_rx_prepare(dev->ndev, &rx, &buf, &len, GFP_KERNEL);
> + KUNIT_ASSERT_EQ(test, rc, 0);
> +
> + mctp_usblib_test_init_pkt(buf, 8, 8);
> +
> + rc = mctp_usblib_rx_complete(dev->ndev, &rx, 8);
> + KUNIT_ASSERT_EQ(test, rc, 0);
> +
> + skb = __skb_dequeue(&dev->rx_pkts);
> + KUNIT_EXPECT_NOT_ERR_OR_NULL(test, skb);
> + KUNIT_EXPECT_EQ(test, skb->len, 4);
> + kfree_skb(skb);
> +
> + mctp_usblib_rx_fini(&rx);
> + mctp_usblib_test_fini(test, &ctx);
> +}
> +
[ ... ]
> +static void mctp_usblib_test_rx_pkt_span(struct kunit *test)
> +{
> + const struct mctp_usblib_test_pkt_span *pkt_span = test->param_value;
> + size_t len, xfer_len, off, xfer_off;
> + struct mctp_usblib_test_dev *dev;
> + struct mctp_usblib_test_ctx ctx;
> + struct mctp_usblib_rx rx;
> + unsigned int i;
> + u8 *pktbuf;
> + void *buf;
> + int rc;
> +
> + mctp_usblib_test_pkt_span_validate(test, pkt_span, &len);
> + pktbuf = kmalloc_array(1, len, GFP_KERNEL);
> +
> + /* lay out packets */
> + for (off = 0, i = 0; i < pkt_span->n_pkts; i++) {
> + len = pkt_span->pkts[i];
> + mctp_usblib_test_init_pkt(pktbuf + off, len, len);
> + off += len;
> + }
[Low]
Is the kmalloc_array() return value missing a NULL check? pktbuf is
dereferenced immediately via mctp_usblib_test_init_pkt(pktbuf + off, ...),
so an allocation failure (including under kunit fault injection) becomes
a NULL deref. Would kunit_kmalloc_array() or an explicit
KUNIT_ASSERT_NOT_NULL(test, pktbuf) be more appropriate here?
> +
[ ... ]
> + /* check received packets */
> + KUNIT_EXPECT_EQ(test, dev->rx_pkts.qlen, pkt_span->n_pkts);
> + for (i = 0; ; i++) {
> + struct sk_buff *skb = __skb_dequeue(&dev->rx_pkts);
> +
> + if (!skb)
> + break;
> +
> + if (i <= pkt_span->n_pkts)
> + KUNIT_EXPECT_EQ(test, skb->len, pkt_span->pkts[i] - 4);
> +
> + kfree_skb(skb);
> + }
[Low]
Should this comparison be i < pkt_span->n_pkts rather than
i <= pkt_span->n_pkts?
With <=, when i == n_pkts the test reads pkts[n_pkts], which is a
zero-initialized tail slot of pkts[6]; the expected length then
underflows to (size_t)0 - 4 and produces a confusing diagnostic instead
of a clean failure if the queue ever holds more SKBs than expected.
> +
[ ... ]
--
This is an AI-generated review.
^ permalink raw reply
* Re: [Intel-wired-lan] [PATCH iwl-net 1/2] ice: move ice_vsi_realloc_stat_arrays() up
From: Marcin Szycik @ 2026-07-02 10:11 UTC (permalink / raw)
To: Przemek Kitszel, intel-wired-lan, Michal Schmidt, Jakub Kicinski
Cc: netdev, Tony Nguyen, Aleksandr Loktionov, Andrew Lunn,
David S. Miller, Eric Dumazet, Paolo Abeni, Jedrzej Jagielski,
Piotr Kwapulinski
In-Reply-To: <20260701104141.9740-1-przemyslaw.kitszel@intel.com>
On 01.07.2026 12:41, Przemek Kitszel wrote:
> Move ice_vsi_realloc_stat_arrays() up, to allow calling it from
> ice_vsi_cfg_def() by the next commit.
>
> Fix kdoc for touched code. One line break removed, "int i" scope
> minimized to the loop, no changes otherwise.
>
> Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Reviewed-by: Marcin Szycik <marcin.szycik@linux.intel.com>
> ---
> drivers/net/ethernet/intel/ice/ice_lib.c | 119 +++++++++++------------
> 1 file changed, 59 insertions(+), 60 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
> index 8cdc4fda89e9..e48ee5940f17 100644
> --- a/drivers/net/ethernet/intel/ice/ice_lib.c
> +++ b/drivers/net/ethernet/intel/ice/ice_lib.c
> @@ -2303,6 +2303,65 @@ static int ice_vsi_cfg_tc_lan(struct ice_pf *pf, struct ice_vsi *vsi)
> return 0;
> }
>
> +/**
> + * ice_vsi_realloc_stat_arrays - Frees unused stat structures or alloc new ones
> + * @vsi: VSI pointer
> + * Return: 0 on success or -ENOMEM on allocation failure.
> + */
> +static int ice_vsi_realloc_stat_arrays(struct ice_vsi *vsi)
> +{
> + u16 req_txq = vsi->req_txq ? vsi->req_txq : vsi->alloc_txq;
> + u16 req_rxq = vsi->req_rxq ? vsi->req_rxq : vsi->alloc_rxq;
> + struct ice_ring_stats **tx_ring_stats;
> + struct ice_ring_stats **rx_ring_stats;
> + struct ice_vsi_stats *vsi_stat;
> + struct ice_pf *pf = vsi->back;
> + u16 prev_txq = vsi->alloc_txq;
> + u16 prev_rxq = vsi->alloc_rxq;
> +
> + vsi_stat = pf->vsi_stats[vsi->idx];
> +
> + if (req_txq < prev_txq) {
> + for (int i = req_txq; i < prev_txq; i++) {
> + if (vsi_stat->tx_ring_stats[i]) {
> + kfree_rcu(vsi_stat->tx_ring_stats[i], rcu);
> + WRITE_ONCE(vsi_stat->tx_ring_stats[i], NULL);
> + }
> + }
> + }
> +
> + tx_ring_stats = vsi_stat->tx_ring_stats;
> + vsi_stat->tx_ring_stats =
> + krealloc_array(vsi_stat->tx_ring_stats, req_txq,
> + sizeof(*vsi_stat->tx_ring_stats),
> + GFP_KERNEL | __GFP_ZERO);
> + if (!vsi_stat->tx_ring_stats) {
> + vsi_stat->tx_ring_stats = tx_ring_stats;
> + return -ENOMEM;
> + }
> +
> + if (req_rxq < prev_rxq) {
> + for (int i = req_rxq; i < prev_rxq; i++) {
> + if (vsi_stat->rx_ring_stats[i]) {
> + kfree_rcu(vsi_stat->rx_ring_stats[i], rcu);
> + WRITE_ONCE(vsi_stat->rx_ring_stats[i], NULL);
> + }
> + }
> + }
> +
> + rx_ring_stats = vsi_stat->rx_ring_stats;
> + vsi_stat->rx_ring_stats =
> + krealloc_array(vsi_stat->rx_ring_stats, req_rxq,
> + sizeof(*vsi_stat->rx_ring_stats),
> + GFP_KERNEL | __GFP_ZERO);
> + if (!vsi_stat->rx_ring_stats) {
> + vsi_stat->rx_ring_stats = rx_ring_stats;
> + return -ENOMEM;
> + }
> +
> + return 0;
> +}
> +
> /**
> * ice_vsi_cfg_def - configure default VSI based on the type
> * @vsi: pointer to VSI
> @@ -3011,66 +3070,6 @@ ice_vsi_rebuild_set_coalesce(struct ice_vsi *vsi,
> }
> }
>
> -/**
> - * ice_vsi_realloc_stat_arrays - Frees unused stat structures or alloc new ones
> - * @vsi: VSI pointer
> - */
> -static int
> -ice_vsi_realloc_stat_arrays(struct ice_vsi *vsi)
> -{
> - u16 req_txq = vsi->req_txq ? vsi->req_txq : vsi->alloc_txq;
> - u16 req_rxq = vsi->req_rxq ? vsi->req_rxq : vsi->alloc_rxq;
> - struct ice_ring_stats **tx_ring_stats;
> - struct ice_ring_stats **rx_ring_stats;
> - struct ice_vsi_stats *vsi_stat;
> - struct ice_pf *pf = vsi->back;
> - u16 prev_txq = vsi->alloc_txq;
> - u16 prev_rxq = vsi->alloc_rxq;
> - int i;
> -
> - vsi_stat = pf->vsi_stats[vsi->idx];
> -
> - if (req_txq < prev_txq) {
> - for (i = req_txq; i < prev_txq; i++) {
> - if (vsi_stat->tx_ring_stats[i]) {
> - kfree_rcu(vsi_stat->tx_ring_stats[i], rcu);
> - WRITE_ONCE(vsi_stat->tx_ring_stats[i], NULL);
> - }
> - }
> - }
> -
> - tx_ring_stats = vsi_stat->tx_ring_stats;
> - vsi_stat->tx_ring_stats =
> - krealloc_array(vsi_stat->tx_ring_stats, req_txq,
> - sizeof(*vsi_stat->tx_ring_stats),
> - GFP_KERNEL | __GFP_ZERO);
> - if (!vsi_stat->tx_ring_stats) {
> - vsi_stat->tx_ring_stats = tx_ring_stats;
> - return -ENOMEM;
> - }
> -
> - if (req_rxq < prev_rxq) {
> - for (i = req_rxq; i < prev_rxq; i++) {
> - if (vsi_stat->rx_ring_stats[i]) {
> - kfree_rcu(vsi_stat->rx_ring_stats[i], rcu);
> - WRITE_ONCE(vsi_stat->rx_ring_stats[i], NULL);
> - }
> - }
> - }
> -
> - rx_ring_stats = vsi_stat->rx_ring_stats;
> - vsi_stat->rx_ring_stats =
> - krealloc_array(vsi_stat->rx_ring_stats, req_rxq,
> - sizeof(*vsi_stat->rx_ring_stats),
> - GFP_KERNEL | __GFP_ZERO);
> - if (!vsi_stat->rx_ring_stats) {
> - vsi_stat->rx_ring_stats = rx_ring_stats;
> - return -ENOMEM;
> - }
> -
> - return 0;
> -}
> -
> /**
> * ice_vsi_rebuild - Rebuild VSI after reset
> * @vsi: VSI to be rebuild
^ permalink raw reply
* [RFC net-next] net: sparx5: configure TAS port link speed
From: Robert Marko @ 2026-07-02 10:15 UTC (permalink / raw)
To: daniel.machon, UNGLinuxDriver, andrew+netdev, davem, edumazet,
kuba, pabeni, Steen.Hegelund, horms, netdev, linux-arm-kernel,
linux-kernel
Cc: luka.perkov, Robert Marko
On the TSN and RED variants of LAN969x and SparX-5i TAS (Time-Aware Shaper)
is present in the silicon.
Currently, the driver does not use configure it at all, which means that
the TAS_PROFILE_CONFIG.LINK_SPEED[1] value is left at the default of 3
which means that its configured for 1 Gbps.
So, running iperf between two 10G switch ports will result in only 940-ish
Mbps while we should be getting around 9.3 Gbps.
Correctly populating the TAS_PROFILE_CONFIG.LINK_SPEED[1] with the current
port speed fixes this issue and we achieve around 9.4 Gbps between two 10G
switch ports.
So, port the TAS port link speed setting from the vendor BSP 6.18 kernel[2]
[1] https://microchip-ung.github.io/lan969x-industrial_reginfo/reginfo_LAN969x-Industrial.html?select=hsch,tas_profile_cfg,tas_profile_config,link_speed
[2] https://github.com/microchip-ung/linux/tree/bsp-6.18-2026
Signed-off-by: Robert Marko <robert.marko@sartura.hr>
---
.../microchip/sparx5/lan969x/lan969x_regs.c | 3 ++
.../microchip/sparx5/sparx5_main_regs.h | 12 +++++
.../ethernet/microchip/sparx5/sparx5_port.c | 4 ++
.../ethernet/microchip/sparx5/sparx5_qos.c | 49 +++++++++++++++++++
.../ethernet/microchip/sparx5/sparx5_qos.h | 1 +
.../ethernet/microchip/sparx5/sparx5_regs.c | 3 ++
.../ethernet/microchip/sparx5/sparx5_regs.h | 3 ++
7 files changed, 75 insertions(+)
diff --git a/drivers/net/ethernet/microchip/sparx5/lan969x/lan969x_regs.c b/drivers/net/ethernet/microchip/sparx5/lan969x/lan969x_regs.c
index ace4ba21eec4..3fc2c006ba12 100644
--- a/drivers/net/ethernet/microchip/sparx5/lan969x/lan969x_regs.c
+++ b/drivers/net/ethernet/microchip/sparx5/lan969x/lan969x_regs.c
@@ -95,6 +95,7 @@ const unsigned int lan969x_gaddr[GADDR_LAST] = {
[GA_HSCH_SYSTEM] = 37384,
[GA_HSCH_MMGT] = 36260,
[GA_HSCH_TAS_CONFIG] = 37696,
+ [GA_HSCH_TAS_PROFILE_CFG] = 37712,
[GA_PTP_PTP_CFG] = 512,
[GA_PTP_PTP_TOD_DOMAINS] = 528,
[GA_PTP_PHASE_DETECTOR_CTRL] = 628,
@@ -129,6 +130,7 @@ const unsigned int lan969x_gcnt[GCNT_LAST] = {
[GC_GCB_SIO_CTRL] = 1,
[GC_HSCH_HSCH_CFG] = 1120,
[GC_HSCH_HSCH_DWRR] = 32,
+ [GC_HSCH_TAS_PROFILE_CFG] = 30,
[GC_PTP_PTP_PINS] = 8,
[GC_PTP_PHASE_DETECTOR_CTRL] = 8,
[GC_REW_PORT] = 35,
@@ -144,6 +146,7 @@ const unsigned int lan969x_gsize[GSIZE_LAST] = {
[GW_FDMA_FDMA] = 448,
[GW_GCB_CHIP_REGS] = 180,
[GW_HSCH_TAS_CONFIG] = 16,
+ [GW_HSCH_TAS_PROFILE_CFG] = 68,
[GW_PTP_PHASE_DETECTOR_CTRL] = 12,
[GW_QSYS_PAUSE_CFG] = 988,
};
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main_regs.h b/drivers/net/ethernet/microchip/sparx5/sparx5_main_regs.h
index 27d02eea7ce1..15fbfa68bc75 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_main_regs.h
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main_regs.h
@@ -5369,6 +5369,18 @@ extern const struct sparx5_regs *regs;
#define HSCH_TAS_STATEMACHINE_CFG_REVISIT_DLY_GET(x)\
FIELD_GET(HSCH_TAS_STATEMACHINE_CFG_REVISIT_DLY, x)
+/* HSCH:TAS_PROFILE_CFG:TAS_PROFILE_CONFIG */
+#define HSCH_TAS_PROFILE_CONFIG(g) \
+ __REG(TARGET_HSCH, 0, 1, regs->gaddr[GA_HSCH_TAS_PROFILE_CFG], g, \
+ regs->gcnt[GC_HSCH_TAS_PROFILE_CFG], \
+ regs->gsize[GW_HSCH_TAS_PROFILE_CFG], 32, 0, 1, 4)
+
+#define HSCH_TAS_PROFILE_CONFIG_LINK_SPEED GENMASK(10, 8)
+#define HSCH_TAS_PROFILE_CONFIG_LINK_SPEED_SET(x)\
+ FIELD_PREP(HSCH_TAS_PROFILE_CONFIG_LINK_SPEED, x)
+#define HSCH_TAS_PROFILE_CONFIG_LINK_SPEED_GET(x)\
+ FIELD_GET(HSCH_TAS_PROFILE_CONFIG_LINK_SPEED, x)
+
/* LAN969X ONLY */
/* HSIOWRAP:XMII_CFG:XMII_CFG */
#define HSIO_WRAP_XMII_CFG(g) \
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_port.c b/drivers/net/ethernet/microchip/sparx5/sparx5_port.c
index 62c49893de3c..ef06bed3a9cc 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_port.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_port.c
@@ -11,6 +11,7 @@
#include "sparx5_main_regs.h"
#include "sparx5_main.h"
#include "sparx5_port.h"
+#include "sparx5_qos.h"
#define SPX5_ETYPE_TAG_C 0x8100
#define SPX5_ETYPE_TAG_S 0x88a8
@@ -1050,6 +1051,9 @@ int sparx5_port_config(struct sparx5 *sparx5,
sparx5,
QFWD_SWITCH_PORT_MODE(port->portno));
+ /* Notify TAS about the speed. */
+ sparx5_tas_speed(port, conf->speed);
+
/* Save the new values */
port->conf = *conf;
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_qos.c b/drivers/net/ethernet/microchip/sparx5/sparx5_qos.c
index e580670f3992..972da8a71f5a 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_qos.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_qos.c
@@ -9,6 +9,17 @@
#include "sparx5_main.h"
#include "sparx5_qos.h"
+enum sparx5_tas_link_speed {
+ TAS_SPEED_NO_GB,
+ TAS_SPEED_10,
+ TAS_SPEED_100,
+ TAS_SPEED_1000,
+ TAS_SPEED_2500,
+ TAS_SPEED_5000,
+ TAS_SPEED_10000,
+ TAS_SPEED_25000,
+};
+
/* Calculate new base_time based on cycle_time.
*
* The hardware requires a base_time that is always in the future.
@@ -581,3 +592,41 @@ int sparx5_tc_ets_del(struct sparx5_port *port)
return sparx5_dwrr_conf_set(port, &dwrr);
}
+
+void sparx5_tas_speed(struct sparx5_port *port, int speed)
+{
+ struct sparx5 *sparx5 = port->sparx5;
+ u8 spd;
+
+ switch (speed) {
+ case SPEED_10:
+ spd = TAS_SPEED_10;
+ break;
+ case SPEED_100:
+ spd = TAS_SPEED_100;
+ break;
+ case SPEED_1000:
+ spd = TAS_SPEED_1000;
+ break;
+ case SPEED_2500:
+ spd = TAS_SPEED_2500;
+ break;
+ case SPEED_5000:
+ spd = TAS_SPEED_5000;
+ break;
+ case SPEED_10000:
+ spd = TAS_SPEED_10000;
+ break;
+ case SPEED_25000:
+ spd = TAS_SPEED_25000;
+ break;
+ default:
+ netdev_err(port->ndev, "TAS: Unsupported speed: %d\n", speed);
+ return;
+ }
+
+ spx5_rmw(HSCH_TAS_PROFILE_CONFIG_LINK_SPEED_SET(spd),
+ HSCH_TAS_PROFILE_CONFIG_LINK_SPEED,
+ sparx5,
+ HSCH_TAS_PROFILE_CONFIG(port->portno));
+}
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_qos.h b/drivers/net/ethernet/microchip/sparx5/sparx5_qos.h
index 04f76f1e23f6..a92a699c551f 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_qos.h
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_qos.h
@@ -60,6 +60,7 @@ struct sparx5_dwrr {
};
int sparx5_qos_init(struct sparx5 *sparx5);
+void sparx5_tas_speed(struct sparx5_port *port, int speed);
/* Multi-Queue Priority */
int sparx5_tc_mqprio_add(struct net_device *ndev, u8 num_tc);
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_regs.c b/drivers/net/ethernet/microchip/sparx5/sparx5_regs.c
index 220e81b714d4..3863f954bd83 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_regs.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_regs.c
@@ -95,6 +95,7 @@ const unsigned int sparx5_gaddr[GADDR_LAST] = {
[GA_HSCH_SYSTEM] = 184000,
[GA_HSCH_MMGT] = 162368,
[GA_HSCH_TAS_CONFIG] = 162384,
+ [GA_HSCH_TAS_PROFILE_CFG] = 188416,
[GA_PTP_PTP_CFG] = 320,
[GA_PTP_PTP_TOD_DOMAINS] = 336,
[GA_PTP_PHASE_DETECTOR_CTRL] = 420,
@@ -129,6 +130,7 @@ const unsigned int sparx5_gcnt[GCNT_LAST] = {
[GC_GCB_SIO_CTRL] = 3,
[GC_HSCH_HSCH_CFG] = 5040,
[GC_HSCH_HSCH_DWRR] = 72,
+ [GC_HSCH_TAS_PROFILE_CFG] = 100,
[GC_PTP_PTP_PINS] = 5,
[GC_PTP_PHASE_DETECTOR_CTRL] = 5,
[GC_REW_PORT] = 70,
@@ -144,6 +146,7 @@ const unsigned int sparx5_gsize[GSIZE_LAST] = {
[GW_FDMA_FDMA] = 428,
[GW_GCB_CHIP_REGS] = 424,
[GW_HSCH_TAS_CONFIG] = 12,
+ [GW_HSCH_TAS_PROFILE_CFG] = 64,
[GW_PTP_PHASE_DETECTOR_CTRL] = 8,
[GW_QSYS_PAUSE_CFG] = 1128,
};
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_regs.h b/drivers/net/ethernet/microchip/sparx5/sparx5_regs.h
index ea28130c2341..585589a31e90 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_regs.h
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_regs.h
@@ -104,6 +104,7 @@ enum sparx5_gaddr_enum {
GA_HSCH_SYSTEM,
GA_HSCH_MMGT,
GA_HSCH_TAS_CONFIG,
+ GA_HSCH_TAS_PROFILE_CFG,
GA_PTP_PTP_CFG,
GA_PTP_PTP_TOD_DOMAINS,
GA_PTP_PHASE_DETECTOR_CTRL,
@@ -139,6 +140,7 @@ enum sparx5_gcnt_enum {
GC_GCB_SIO_CTRL,
GC_HSCH_HSCH_CFG,
GC_HSCH_HSCH_DWRR,
+ GC_HSCH_TAS_PROFILE_CFG,
GC_PTP_PTP_PINS,
GC_PTP_PHASE_DETECTOR_CTRL,
GC_REW_PORT,
@@ -155,6 +157,7 @@ enum sparx5_gsize_enum {
GW_FDMA_FDMA,
GW_GCB_CHIP_REGS,
GW_HSCH_TAS_CONFIG,
+ GW_HSCH_TAS_PROFILE_CFG,
GW_PTP_PHASE_DETECTOR_CTRL,
GW_QSYS_PAUSE_CFG,
GSIZE_LAST,
--
2.55.0
^ permalink raw reply related
* Re: [PATCH net] net/stmmac: Set Rx queue page_pool to NULL when freeing DMA resources
From: Jakub Raczynski @ 2026-07-02 10:17 UTC (permalink / raw)
To: Maxime Chevallier
Cc: netdev, andrew+netdev, davem, edumazet, kuba, pabeni,
mcoquelin.stm32, linux-kernel, k.tegowski, k.domagalski,
Yashwant Varur
In-Reply-To: <1e4cedb5-fc16-4007-9a11-8c819f2c5731@bootlin.com>
[-- Attachment #1: Type: text/plain, Size: 572 bytes --]
On Thu, Jul 02, 2026 at 11:24:41AM +0200, Maxime Chevallier wrote:
> >
> > kfree(rx_q->buf_pool);
> > - if (rx_q->page_pool)
> > + if (rx_q->page_pool) {
> > page_pool_destroy(rx_q->page_pool);
> > + rx_q->page_pool = NULL;
> > + }
>
> page_pool_destroy can be passed NULL, as you're fixing this I think you might as well
> drop the if(rx_q->page_pool) at the same time :) I agree with the change otherwise.
>
> with that changed,
>
> Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
>
> Maxime
>
Agreed, will send V2 soon.
BR
Jakub Raczynski
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply
* Re: [Intel-wired-lan] [PATCH iwl-net 2/2] ice: fix stats array overflow via proper realloc
From: Marcin Szycik @ 2026-07-02 10:25 UTC (permalink / raw)
To: Przemek Kitszel, intel-wired-lan, Michal Schmidt, Jakub Kicinski
Cc: netdev, Tony Nguyen, Aleksandr Loktionov, Andrew Lunn,
David S. Miller, Eric Dumazet, Paolo Abeni, Jedrzej Jagielski,
Piotr Kwapulinski
In-Reply-To: <20260701104141.9740-2-przemyslaw.kitszel@intel.com>
On 01.07.2026 12:41, Przemek Kitszel wrote:
> Integrate ice_vsi_alloc_stat_arrays() with realloc variant.
>
> Instead of keeping two functions for stat arrays allocation, change the
> ice_vsi_realloc_stat_arrays() to handle initial condition (no vsi_stat
> entry) and replace ice_vsi_alloc_stat_arrays() by the more generic
> ice_vsi_realloc_stat_arrays().
>
> Note that VSIs of ICE_VSI_CHNL type are ignored in realloc variant as they
> were in the replaced ice_vsi_alloc_stat_arrays().
>
> This is a fix for stats array overflow that occurs when VF is given more
> queues (an operation that will be more frequent, and by bigger increase,
> when we will merge my "XLVF" series).
>
> Splat for increasing number of queues thanks to Michal Schmidt:
> KASAN detects the bug:
> ==================================================================
> BUG: KASAN: slab-out-of-bounds in ice_vsi_alloc_ring_stats+0x385/0x4a0 [ice]
> Read of size 8 at addr ffff88810affea60 by task kworker/u131:7/221
>
> CPU: 24 UID: 0 PID: 221 Comm: kworker/u131:7 Not tainted 7.1.0-rc1+ #1 PREEMPT(lazy)
> ...
> Workqueue: ice ice_service_task [ice]
> Call Trace:
> <TASK>
> ...
> kasan_report+0xd7/0x120
> ice_vsi_alloc_ring_stats+0x385/0x4a0 [ice]
> ice_vsi_cfg_def+0x12e2/0x2060 [ice]
> ice_vsi_cfg+0xb5/0x3c0 [ice]
> ice_reset_vf+0x858/0xf80 [ice]
> ice_vc_request_qs_msg+0x1da/0x290 [ice]
> ice_vc_process_vf_msg+0xb15/0x1430 [ice]
> __ice_clean_ctrlq+0x70d/0x9d0 [ice]
> ice_service_task+0x840/0xf20 [ice]
> process_one_work+0x690/0xff0
> worker_thread+0x4d9/0xd20
> kthread+0x322/0x410
> ret_from_fork+0x332/0x660
> ret_from_fork_asm+0x1a/0x30
> </TASK>
>
> Allocated by task 2439:
> kasan_save_stack+0x1c/0x40
> kasan_save_track+0x10/0x30
> __kasan_kmalloc+0x96/0xb0
> __kmalloc_noprof+0x1d8/0x580
> ice_vsi_cfg_def+0x115c/0x2060 [ice]
> ice_vsi_cfg+0xb5/0x3c0 [ice]
> ice_vsi_setup+0x180/0x320 [ice]
> ice_start_vfs+0x1f3/0x590 [ice]
> ice_ena_vfs+0x66d/0x798 [ice]
> ice_sriov_configure.cold+0xe4/0x121 [ice]
> sriov_numvfs_store+0x279/0x480
> kernfs_fop_write_iter+0x331/0x4f0
> vfs_write+0x4c4/0xe40
> ksys_write+0x10c/0x240
> do_syscall_64+0xd9/0x650
> entry_SYSCALL_64_after_hwframe+0x76/0x7e
>
> The buggy address belongs to the object at ffff88810affea40
> which belongs to the cache kmalloc-32 of size 32
> The buggy address is located 0 bytes to the right of
> allocated 32-byte region [ffff88810affea40, ffff88810affea60)
>
> Fixes: 2a2cb4c6c181 ("ice: replace ice_vf_recreate_vsi() with ice_vf_reconfig_vsi()")
> Closes: https://redhat.atlassian.net/browse/RHEL-164321
Is there a simpler reproducer than the script attached in the ticket?
> Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Reviewed-by: Marcin Szycik <marcin.szycik@linux.intel.com>
> ---
> This is an alternative to the fix [1] by Michal Schmidt, which were
> blocked due to AI feedback. My fix was already developed before Michal's,
> just not public back then. We have agreed to go on with my version.
>
> [1] https://lore.kernel.org/netdev/20260520183501.3360810-3-anthony.l.nguyen@intel.com
> ---
> drivers/net/ethernet/intel/ice/ice_lib.c | 57 +++++-------------------
> 1 file changed, 11 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
> index e48ee5940f17..ae167b42c558 100644
> --- a/drivers/net/ethernet/intel/ice/ice_lib.c
> +++ b/drivers/net/ethernet/intel/ice/ice_lib.c
> @@ -513,51 +513,6 @@ static irqreturn_t ice_msix_clean_rings(int __always_unused irq, void *data)
> return IRQ_HANDLED;
> }
>
> -/**
> - * ice_vsi_alloc_stat_arrays - Allocate statistics arrays
> - * @vsi: VSI pointer
> - */
> -static int ice_vsi_alloc_stat_arrays(struct ice_vsi *vsi)
> -{
> - struct ice_vsi_stats *vsi_stat;
> - struct ice_pf *pf = vsi->back;
> -
> - if (vsi->type == ICE_VSI_CHNL)
> - return 0;
> - if (!pf->vsi_stats)
> - return -ENOENT;
> -
> - if (pf->vsi_stats[vsi->idx])
> - /* realloc will happen in rebuild path */
> - return 0;
> -
> - vsi_stat = kzalloc_obj(*vsi_stat);
> - if (!vsi_stat)
> - return -ENOMEM;
> -
> - vsi_stat->tx_ring_stats =
> - kzalloc_objs(*vsi_stat->tx_ring_stats, vsi->alloc_txq);
> - if (!vsi_stat->tx_ring_stats)
> - goto err_alloc_tx;
> -
> - vsi_stat->rx_ring_stats =
> - kzalloc_objs(*vsi_stat->rx_ring_stats, vsi->alloc_rxq);
> - if (!vsi_stat->rx_ring_stats)
> - goto err_alloc_rx;
> -
> - pf->vsi_stats[vsi->idx] = vsi_stat;
> -
> - return 0;
> -
> -err_alloc_rx:
> - kfree(vsi_stat->rx_ring_stats);
> -err_alloc_tx:
> - kfree(vsi_stat->tx_ring_stats);
> - kfree(vsi_stat);
> - pf->vsi_stats[vsi->idx] = NULL;
> - return -ENOMEM;
> -}
> -
> /**
> * ice_vsi_alloc_def - set default values for already allocated VSI
> * @vsi: ptr to VSI
> @@ -2319,7 +2274,17 @@ static int ice_vsi_realloc_stat_arrays(struct ice_vsi *vsi)
> u16 prev_txq = vsi->alloc_txq;
> u16 prev_rxq = vsi->alloc_rxq;
>
> + if (vsi->type == ICE_VSI_CHNL)
> + return 0;
> +
> vsi_stat = pf->vsi_stats[vsi->idx];
> + if (!vsi_stat) {
> + vsi_stat = kzalloc_obj(*vsi_stat);
> + if (!vsi_stat)
> + return -ENOMEM;
> +
> + pf->vsi_stats[vsi->idx] = vsi_stat;
> + }
>
> if (req_txq < prev_txq) {
> for (int i = req_txq; i < prev_txq; i++) {
> @@ -2379,7 +2344,7 @@ static int ice_vsi_cfg_def(struct ice_vsi *vsi)
> return ret;
>
> /* allocate memory for Tx/Rx ring stat pointers */
> - ret = ice_vsi_alloc_stat_arrays(vsi);
> + ret = ice_vsi_realloc_stat_arrays(vsi);
> if (ret)
> goto unroll_vsi_alloc;
>
^ permalink raw reply
* Re: [PATCH v2 nf-next] ipvs: Move defense_work and est_reload_work to system_dfl_long_wq
From: Ismael Luceno @ 2026-07-02 10:31 UTC (permalink / raw)
To: linux-kernel
Cc: open list:IPVS, open list:IPVS, open list:NETFILTER,
open list:NETFILTER
In-Reply-To: <20260702101100.24256-2-iluceno@suse.de>
Link: https://lore.kernel.org/all/20260317140100.24993-2-iluceno@suse.de/
^ permalink raw reply
* Re: [PATCH bpf v3 1/4] selftests/bpf: Ensure UDP sockets are bound
From: Jakub Sitnicki @ 2026-07-02 10:35 UTC (permalink / raw)
To: Michal Luczaj
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Eduard Zingerman, Kumar Kartikeya Dwivedi, Martin KaFai Lau,
Song Liu, Yonghong Song, Jiri Olsa, Emil Tsalapatis, Shuah Khan,
John Fastabend, Jiayuan Chen, Eric Dumazet, Kuniyuki Iwashima,
Paolo Abeni, Willem de Bruijn, David S. Miller, Jakub Kicinski,
Simon Horman, Cong Wang, bpf, linux-kselftest, linux-kernel,
netdev
In-Reply-To: <20260702-sockmap-lookup-udp-leak-v3-1-ff8de8782468@rbox.co>
On Thu, Jul 02, 2026 at 01:28 AM +02, Michal Luczaj wrote:
> Update sockmap_basic tests to bind sockets before they are used. This
> accommodates the recent change in sockmap that rejects unbound UDP sockets.
>
> Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>
> Signed-off-by: Michal Luczaj <mhal@rbox.co>
> ---
Reviewed-by: Jakub Sitnicki <jakub@cloudflare.com>
^ permalink raw reply
* Re: [PATCH 02/10] net: emac: fix sparse __iomem warnings in IAHT register access
From: Paolo Abeni @ 2026-07-02 10:37 UTC (permalink / raw)
To: Rosen Penev, netdev
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
open list
In-Reply-To: <20260630041634.284127-3-rosenp@gmail.com>
On 6/30/26 6:16 AM, Rosen Penev wrote:
> Annotate iaht1/iaht2 in the EMAC4 register union with __iomem so
> sparse does not warn about address-space mismatches, and simplify
> emac_xaht_base() to return &p->u1.emac4sync.iaht1 (or the EMAC4
> variant) directly instead of computing the offset by hand.
This patch does not include the actual annotation.
/P
^ permalink raw reply
* Re: [PATCH net-next v3 14/15] net: macb: use context swapping in .set_ringparam()
From: Nicolai Buchwitz @ 2026-07-02 10:37 UTC (permalink / raw)
To: Théo Lebrun
Cc: Conor Dooley, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Richard Cochran, Russell King,
netdev, linux-kernel, Nicolas Ferre, Claudiu Beznea,
Paolo Valerio, Vladimir Kondratiev, Gregory CLEMENT,
Benoît Monin, Tawfik Bayouk, Thomas Petazzoni,
Maxime Chevallier
In-Reply-To: <20260701-macb-context-v3-14-00268d5b1502@bootlin.com>
Hi Théo
On 1.7.2026 17:59, Théo Lebrun wrote:
> ethtool_ops.set_ringparam() is implemented using the primitive close /
> update ring size / reopen sequence. Under memory pressure this does not
> fly: we free our buffers at close and cannot reallocate new ones at
> open. Also, it triggers a slow PHY reinit.
>
> Instead, exploit the new context mechanism and improve our sequence to:
> - allocate a new context (including buffers) first
> - if it fails, early return without any impact to the interface
> - stop interface
> - update global state (bp, netdev, etc)
> - pass buffer pointers to the hardware
> - start interface
> - free old context.
>
> The HW disable sequence is inspired by macb_reset_hw() but avoids
> (1) setting NCR bit CLRSTAT and (2) clearing register PBUFRXCUT.
>
> The HW re-enable sequence is inspired by macb_mac_link_up(), skipping
> over register writes which would be redundant (because values have not
> changed).
>
> The generic context swapping parts are isolated into helper functions
> macb_context_swap_start|end(), reusable by other operations
> (change_mtu,
> set_channels, etc).
>
> Introduce a new locking primitive (mac_cfg_lock mutex) to serialise
> swap
> with phylink MAC callbacks. Avoid stopping phylink to avoid a slow PHY
> retrain. Those callbacks grab phydev->lock if it exists so we could
> imagine grabbing that from the swap op, but phydev->lock doesn't exist
> in the SFP case.
>
> AT91 EMAC is handled differently as their buffer management is separate
> and they don't do NAPI. We refuse them (-EBUSY) to avoid implementing
> context swapping for them.
>
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---
> drivers/net/ethernet/cadence/macb.h | 2 +
> drivers/net/ethernet/cadence/macb_main.c | 142
> +++++++++++++++++++++++++++++--
> [...]
> +static void macb_context_swap_start(struct macb *bp)
> +{
> + struct macb_queue *queue;
> + unsigned long flags;
> + unsigned int q;
> + u32 ctrl;
> +
> + mutex_lock(&bp->mac_cfg_lock);
> +
> + /* Mask interrupts before disabling BH features. */
> + spin_lock_irqsave(&bp->lock, flags);
> + for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
> + queue_writel(queue, IDR, -1);
> + queue_readl(queue, ISR);
> + macb_queue_isr_clear(bp, queue, -1);
> + }
> + spin_unlock_irqrestore(&bp->lock, flags);
> +
> + /* Drain BH features. HW is still active and usable at this point. */
> +
> + cancel_work_sync(&bp->hresp_err_bh_work);
> + cancel_delayed_work_sync(&bp->tx_lpi_work);
> +
> + for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
> + napi_disable(&queue->napi_rx);
> + napi_disable(&queue->napi_tx);
> + cancel_work_sync(&queue->tx_error_task);
> + netdev_tx_reset_queue(netdev_get_tx_queue(bp->netdev, q));
> + }
Can this deadlock against a pending tx_error_task?
AFAIU macb_tx_error_task() does napi_disable(&queue->napi_tx) and later
napi_enable() on the same napi, and it can already be queued
(macb_interrupt()
schedules it on a TX error) by the time the swap runs:
swap_start: napi_disable(napi_tx) /* sets SCHED, returns */
worker: tx_error_task: napi_disable(napi_tx) /* spins on SCHED */
swap_start: cancel_work_sync(tx_error_task) /* waits on worker
*/
napi_disable() spins until napi_enable() clears SCHED, but here the swap
won't
re-enable until macb_context_swap_end(), and cancel_work_sync() is
what's
holding it up. Nothing clears it.
Maybe cancel_work_sync() before the napi_disable() calls would work
instead? IRQs
are masked just above, so AFAICT nothing can reschedule tx_error_task by
then.
(Same path via .ndo_change_mtu() in the next patch.)
> [...]
Thanks
Nicolai
^ permalink raw reply
* Re: [PATCH 07/10] net: emac: fix DMA API mapping and unmapping correctness
From: Paolo Abeni @ 2026-07-02 10:39 UTC (permalink / raw)
To: rosenp; +Cc: netdev, andrew+netdev, davem, edumazet, kuba, pabeni,
linux-kernel
In-Reply-To: <20260630041634.284127-8-rosenp@gmail.com>
From: AI Reviewer <ai@example.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.
Full review at: https://netdev-ai.bots.linux.dev/sashiko/
---
net: emac: fix DMA API mapping and unmapping correctness
This patch adds dma_mapping_error() checks after every dma_map_single()
and skb_frag_dma_map() call in the ibm emac driver, introduces a shadow
rx_dma[] array in struct emac_instance, and replaces the map-and-leak
pattern in emac_recycle_rx_skb() with dma_sync_single_for_device().
A few questions follow inline.
> net: emac: fix DMA API mapping and unmapping correctness
[Low]
The subject says the fix is for DMA API mapping and unmapping
correctness, but the in-tree comment added by this patch states:
+ * TX buffers still lack dma_unmap calls for the reasons explained in the
+ * original note below (a single skb may be split across multiple BDs on
+ * TAH-equipped EMACs, making per-fragment tracking complex).
Should the subject be scoped to the RX path (for example, "fix DMA API
correctness on the RX path") since the TX-side DMA API violation is
intentionally preserved?
[Low]
The commit message bullet says:
- emac_recycle_rx_skb: map first, check error, then unmap old;
use dma_sync_single_for_device() with SKB_DATA_ALIGN(len + NET_IP_ALIGN)
But emac_recycle_rx_skb() only calls dma_sync_single_for_device() and
performs no map/check/unmap sequence; the "map first, check, unmap old"
sequence is in the emac_poll_rx() MAL_RX_CTRL_FIRST branch. Would it
help to split this bullet across the two functions?
[Low]
The changelog ends with:
For whatever reason after this patch, ath9k stopped throwing DMA
errors with CONFIG_DMA_API_DEBUG.
Is this perhaps an artifact of CONFIG_DMA_API_DEBUG hash exhaustion
caused by the leaked mappings in the pre-patch driver? It might be
worth investigating and stating the mechanism rather than leaving it
as an unexplained side effect.
[Low]
The emac_recycle_rx_skb() change (discarding the returned DMA address
from dma_map_single()) looks like a long-standing functional bug on
systems with IOMMU/SWIOTLB. Would a Fixes: tag and stable consideration
be appropriate for that hunk?
> diff --git a/drivers/net/ethernet/ibm/emac/core.c b/drivers/net/ethernet/ibm/emac/core.c
> index ced9690cddc3..aed1ad21e2ea 100644
> --- a/drivers/net/ethernet/ibm/emac/core.c
> +++ b/drivers/net/ethernet/ibm/emac/core.c
[ ... ]
> @@ -1058,6 +1066,7 @@ static int emac_resize_rx_ring(struct emac_instance *dev, int new_mtu)
> /* Second pass, allocate new skbs */
> for (i = 0; i < NUM_RX_BUFF; ++i) {
> struct sk_buff *skb;
> + dma_addr_t dma;
>
> skb = netdev_alloc_skb_ip_align(dev->ndev, rx_skb_size);
> if (!skb) {
> @@ -1066,12 +1075,24 @@ static int emac_resize_rx_ring(struct emac_instance *dev, int new_mtu)
> }
>
> BUG_ON(!dev->rx_skb[i]);
> + dma_unmap_single(&dev->ofdev->dev,
> + dev->rx_dma[i],
> + dev->rx_sync_size, DMA_FROM_DEVICE);
> dev_kfree_skb(dev->rx_skb[i]);
>
> - dev->rx_desc[i].data_ptr =
> - dma_map_single(&dev->ofdev->dev, skb->data - NET_IP_ALIGN,
> - rx_sync_size, DMA_FROM_DEVICE)
> - + NET_IP_ALIGN;
> + dma = dma_map_single(&dev->ofdev->dev, skb->data - NET_IP_ALIGN,
> + rx_sync_size, DMA_FROM_DEVICE);
> + if (dma_mapping_error(&dev->ofdev->dev, dma)) {
> + dev_kfree_skb(skb);
> + dev->rx_skb[i] = NULL;
> + dev->rx_dma[i] = 0;
> + dev->rx_desc[i].data_ptr = 0;
> + dev->rx_desc[i].ctrl = 0;
> + ret = -ENOMEM;
> + goto oom;
> + }
[High]
Can this error path leave the descriptor ring in a state that crashes
the next NAPI poll?
The first pass earlier in emac_resize_rx_ring() initialises every
slot's ctrl with:
dev->rx_desc[i].ctrl = MAL_RX_CTRL_EMPTY |
(i == (NUM_RX_BUFF - 1) ? MAL_RX_CTRL_WRAP : 0);
so software (which polls with "if (ctrl & MAL_RX_CTRL_EMPTY) break;")
treats those slots as owned by hardware and skips them. The new error
path here sets ctrl = 0 instead, which clears MAL_RX_CTRL_EMPTY (and
MAL_RX_CTRL_WRAP for the last slot).
The oom: label below unconditionally re-enables RX:
oom:
clear_bit(MAL_COMMAC_RX_STOPPED, &dev->commac.flags);
dev->rx_slot = 0;
mal_enable_rx_channel(dev->mal, dev->mal_rx_chan);
emac_rx_enable(dev);
emac_netif_start(dev);
On the next emac_poll_rx() call, the loop reads ctrl = 0, finds the
EMPTY bit clear, and proceeds with skb = dev->rx_skb[slot] = NULL,
followed by memcpy()/skb_put()/eth_type_trans() on that NULL skb.
Additionally, the slot's rx_skb[i] is now NULL but the loop above
starts each iteration with BUG_ON(!dev->rx_skb[i]). Would a subsequent
MTU change deterministically panic on that BUG_ON()?
Should this error path mirror the first pass and use MAL_RX_CTRL_EMPTY
(plus MAL_RX_CTRL_WRAP for the last slot) rather than zero?
[ ... ]
> @@ -1176,15 +1201,23 @@ static void emac_clear_mal_desc(struct mal_descriptor *desc, int count)
> static int
> __emac_prepare_rx_skb(struct sk_buff *skb, struct emac_instance *dev, int slot)
> {
> + dma_addr_t dma;
> +
> if (unlikely(!skb))
> return -ENOMEM;
>
> dev->rx_skb[slot] = skb;
> dev->rx_desc[slot].data_len = 0;
>
> - dev->rx_desc[slot].data_ptr =
> - dma_map_single(&dev->ofdev->dev, skb->data - NET_IP_ALIGN,
> - dev->rx_sync_size, DMA_FROM_DEVICE) + NET_IP_ALIGN;
> + dma = dma_map_single(&dev->ofdev->dev, skb->data - NET_IP_ALIGN,
> + dev->rx_sync_size, DMA_FROM_DEVICE);
> + if (dma_mapping_error(&dev->ofdev->dev, dma)) {
> + dev->rx_skb[slot] = NULL;
> + dev_kfree_skb(skb);
> + return -ENOMEM;
> + }
> + dev->rx_desc[slot].data_ptr = dma + NET_IP_ALIGN;
> + dev->rx_dma[slot] = dma;
[Critical]
Can this corrupt the slot state on a DMA mapping failure?
The new skb is installed into the slot before the mapping is performed:
dev->rx_skb[slot] = skb;
dev->rx_desc[slot].data_len = 0;
dma = dma_map_single(...);
if (dma_mapping_error(...)) {
dev->rx_skb[slot] = NULL;
dev_kfree_skb(skb);
return -ENOMEM;
}
On failure, the new skb is freed and rx_skb[slot] is set to NULL, but
dev->rx_desc[slot].data_ptr and dev->rx_dma[slot] still hold the old
mapping, and the previous skb pointer that the caller saved (the
emac_poll_rx() local "skb") is also lost.
The emac_poll_rx() caller paths handle the failure as:
} else if (unlikely(emac_alloc_rx_skb_napi(dev, slot)))
goto oom;
...
oom:
++dev->estats.rx_dropped_oom;
emac_recycle_rx_skb(dev, slot, 0);
emac_recycle_rx_skb() then re-arms the descriptor with
MAL_RX_CTRL_EMPTY, handing the still-mapped old buffer back to
hardware. On the next received packet, dev->rx_skb[slot] is NULL and:
skb = dev->rx_skb[slot];
...
memcpy(copy_skb->data - NET_IP_ALIGN,
skb->data - NET_IP_ALIGN,
len + NET_IP_ALIGN);
would dereference NULL in NAPI context. The MAL_RX_CTRL_FIRST branch
that calls emac_alloc_rx_skb_napi() has the same shape.
Would it be safer to perform the dma_map_single() and error check
before installing the new skb into rx_skb[slot], so the slot still
points at a valid skb and mapping on failure?
[ ... ]
> @@ -1808,12 +1863,17 @@ static int emac_poll_rx(void *param, int budget)
> continue;
> sg:
> if (ctrl & MAL_RX_CTRL_FIRST) {
> + dma_addr_t old_dma = dev->rx_dma[slot];
> +
> BUG_ON(dev->rx_sg_skb);
> if (unlikely(emac_alloc_rx_skb_napi(dev, slot))) {
> DBG(dev, "rx OOM %d" NL, slot);
> ++dev->estats.rx_dropped_oom;
> emac_recycle_rx_skb(dev, slot, 0);
> } else {
> + dma_unmap_single(&dev->ofdev->dev, old_dma,
> + dev->rx_sync_size,
> + DMA_FROM_DEVICE);
> dev->rx_sg_skb = skb;
> skb_put(skb, len);
> }
[High]
The commit message bullet says:
- emac_poll_rx: use rx_dma[] shadow array, unmap old skb before
passing to napi_gro_receive
but the new dma_unmap_single() above only covers the SG
MAL_RX_CTRL_FIRST branch. The non-SG large-packet path
(len >= EMAC_RX_COPY_THRESH) is in emac_poll_rx():
skb = dev->rx_skb[slot];
...
if (len && len < EMAC_RX_COPY_THRESH) {
...
} else if (unlikely(emac_alloc_rx_skb_napi(dev, slot)))
goto oom;
skb_put(skb, len);
push_packet:
skb->protocol = eth_type_trans(skb, dev->ndev);
emac_rx_csum(dev, skb, ctrl);
napi_gro_receive(&dev->mal->napi, skb);
emac_alloc_rx_skb_napi() calls __emac_prepare_rx_skb() which
overwrites dev->rx_dma[slot] with the new mapping, so the previous
mapping address is lost. The original "skb" is then handed to
napi_gro_receive() while still mapped DMA_FROM_DEVICE.
Does this leak one DMA mapping per large-packet RX, and on the
non-coherent PowerPC platforms this patch targets, hand the stack a
buffer that has not been dma_unmap_single()'d?
Should the non-SG large-packet branch also save old_dma before
emac_alloc_rx_skb_napi() and call dma_unmap_single() before
napi_gro_receive(), mirroring the SG-first branch?
[ ... ]
--
This is an AI-generated review.
^ 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