* 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
* Re: [PATCH net-next v2 0/4] net: convert UDP getsockopt to sockopt_t
From: patchwork-bot+netdevbpf @ 2026-07-02 10:40 UTC (permalink / raw)
To: Breno Leitao
Cc: davem, edumazet, kuba, pabeni, horms, willemdebruijn.kernel,
shuah, sdf.kernel, netdev, linux-kernel, linux-kselftest,
kernel-team
In-Reply-To: <20260630-getsockopt_phase2-v2-0-193335f3d4d1@debian.org>
Hello:
This series was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:
On Tue, 30 Jun 2026 07:01:25 -0700 you wrote:
> The leaf proto_ops getsockopt callbacks have been moving to the new
> getsockopt_iter()/sockopt_t interface.
>
> I was trying to get SMC into getsockop and retire .getsockopt, but,
> I found the best approach is to keep converting other protocols.
>
> This series starts the same conversion one layer down, at the struct proto
> getsockopt path, beginning with UDP.
>
> [...]
Here is the summary with links:
- [net-next,v2,1/4] net: add sockopt_init_user() for getsockopt conversion
https://git.kernel.org/netdev/net-next/c/b5997f911eec
- [net-next,v2,2/4] udp: convert udp_lib_getsockopt to sockopt_t
https://git.kernel.org/netdev/net-next/c/f99d7065a4d4
- [net-next,v2,3/4] ipv4: raw: convert do_raw_getsockopt to sockopt_t
https://git.kernel.org/netdev/net-next/c/9588d5da17ce
- [net-next,v2,4/4] selftests: net: getsockopt_iter: add raw ICMP_FILTER coverage
https://git.kernel.org/netdev/net-next/c/f4d5e3a5c7bc
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 01/15] net: macb: drop "consistent" from alloc/free function names
From: Nicolai Buchwitz @ 2026-07-02 10:41 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-1-00268d5b1502@bootlin.com>
On 1.7.2026 17:59, Théo Lebrun wrote:
> Since commit 4df95131ea80 ("net/macb: change RX path for GEM") those
> functions have not been only allocating or freeing consistent memory
> mappings.
>
> Rename from macb_alloc_consistent() to macb_alloc() and
> from macb_free_consistent() to macb_free().
>
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---
> drivers/net/ethernet/cadence/macb_main.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb_main.c
> b/drivers/net/ethernet/cadence/macb_main.c
> index fd282a1700fb..d677a93b9d72 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -2646,7 +2646,7 @@ static unsigned int
> macb_rx_ring_size_per_queue(struct macb *bp)
> return macb_dma_desc_get_size(bp) * bp->rx_ring_size +
> bp->rx_bd_rd_prefetch;
> }
>
> -static void macb_free_consistent(struct macb *bp)
> +static void macb_free(struct macb *bp)
> {
> struct device *dev = &bp->pdev->dev;
> struct macb_queue *queue;
> @@ -2711,7 +2711,7 @@ static int macb_alloc_rx_buffers(struct macb *bp)
> return 0;
> }
>
> -static int macb_alloc_consistent(struct macb *bp)
> +static int macb_alloc(struct macb *bp)
> {
> struct device *dev = &bp->pdev->dev;
> dma_addr_t tx_dma, rx_dma;
> @@ -2769,7 +2769,7 @@ static int macb_alloc_consistent(struct macb *bp)
> return 0;
>
> out_err:
> - macb_free_consistent(bp);
> + macb_free(bp);
> return -ENOMEM;
> }
>
> @@ -3157,7 +3157,7 @@ static int macb_open(struct net_device *dev)
> /* RX buffers initialization */
> macb_init_rx_buffer_size(bp, bufsz);
>
> - err = macb_alloc_consistent(bp);
> + err = macb_alloc(bp);
> if (err) {
> netdev_err(dev, "Unable to allocate DMA memory (error %d)\n",
> err);
> @@ -3202,7 +3202,7 @@ static int macb_open(struct net_device *dev)
> napi_disable(&queue->napi_rx);
> napi_disable(&queue->napi_tx);
> }
> - macb_free_consistent(bp);
> + macb_free(bp);
> pm_exit:
> pm_runtime_put_sync(&bp->pdev->dev);
> return err;
> @@ -3235,7 +3235,7 @@ static int macb_close(struct net_device *dev)
> netif_carrier_off(dev);
> spin_unlock_irqrestore(&bp->lock, flags);
>
> - macb_free_consistent(bp);
> + macb_free(bp);
>
> if (bp->ptp_info)
> bp->ptp_info->ptp_remove(dev);
Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>
Thanks
Nicolai
^ permalink raw reply
* Re: [PATCH net-next v3 03/15] net: macb: unify variable naming convention in at91ether functions
From: Nicolai Buchwitz @ 2026-07-02 10:42 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-3-00268d5b1502@bootlin.com>
On 1.7.2026 17:59, Théo Lebrun wrote:
> Follow MACB naming convention throughout on two aspects:
> - Always name `struct macb *bp` rather than `lp`.
> - Always name `struct macb_queue *queue` rather than `q`.
>
> The latter is to reserve `q` for queue indexes.
>
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> [...]
Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>
Thanks
Nicolai
^ permalink raw reply
* Re: [PATCH 03/10] net: emac: use DMA-specific and SMP memory barriers
From: Paolo Abeni @ 2026-07-02 10:42 UTC (permalink / raw)
To: Rosen Penev, netdev
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
open list
In-Reply-To: <20260630041634.284127-4-rosenp@gmail.com>
On 6/30/26 6:16 AM, Rosen Penev wrote:
> @@ -1845,7 +1845,7 @@ static int emac_poll_rx(void *param, int budget)
> }
>
> if (unlikely(budget && test_bit(MAL_COMMAC_RX_STOPPED, &dev->commac.flags))) {
> - mb();
> + dma_rmb();
> if (!(dev->rx_desc[slot].ctrl & MAL_RX_CTRL_EMPTY)) {
> DBG2(dev, "rx restart" NL);
> received = 0;
Sashiko geimini thinks the above is not enough for PowerPC:
https://sashiko.dev/#/patchset/20260630041634.284127-1-rosenp%40gmail.com
> diff --git a/drivers/net/ethernet/ibm/emac/zmii.c b/drivers/net/ethernet/ibm/emac/zmii.c
> index a3839cf02ec4..5144ee94a7d2 100644
> --- a/drivers/net/ethernet/ibm/emac/zmii.c
> +++ b/drivers/net/ethernet/ibm/emac/zmii.c
> @@ -258,8 +258,8 @@ static int zmii_probe(struct platform_device *ofdev)
> /* Disable all inputs by default */
> out_be32(&dev->base->fer, 0);
>
> - printk(KERN_INFO "ZMII %pOF initialized\n", ofdev->dev.of_node);
> - wmb();
> + dev_info(&ofdev->dev, "ZMII initialized\n");
The printk change is unrelated and should go in a different patch.
/P
^ permalink raw reply
* Re: [PATCH net-next v3 04/15] net: macb: unify queue index variable naming convention and types
From: Nicolai Buchwitz @ 2026-07-02 10:43 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-4-00268d5b1502@bootlin.com>
On 1.7.2026 17:59, Théo Lebrun wrote:
> Variables are named q or queue_index. Types are int, unsigned int, u32
> and u16. Use `unsigned int q` everywhere.
>
> Skip over taprio functions. They use `u8 queue_id` which fits with the
> `struct macb_queue_enst_config` field. Using `queue_id` everywhere
> would be too verbose.
>
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---
> drivers/net/ethernet/cadence/macb_main.c | 32
> ++++++++++++++++----------------
> 1 file changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb_main.c
> b/drivers/net/ethernet/cadence/macb_main.c
> index 86c0e9ccbfce..3b75797381b6 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -877,7 +877,7 @@ static void gem_shuffle_tx_one_ring(struct
> macb_queue *queue)
> static void gem_shuffle_tx_rings(struct macb *bp)
> {
> struct macb_queue *queue;
> - int q;
> + unsigned int q;
>
> for (q = 0, queue = bp->queues; q < bp->num_queues; q++, queue++)
> gem_shuffle_tx_one_ring(queue);
> @@ -1258,7 +1258,7 @@ static void macb_tx_error_task(struct work_struct
> *work)
> tx_error_task);
> bool halt_timeout = false;
> struct macb *bp = queue->bp;
> - u32 queue_index;
> + unsigned int q;
> u32 packets = 0;
> u32 bytes = 0;
> struct macb_tx_skb *tx_skb;
> @@ -1267,9 +1267,9 @@ static void macb_tx_error_task(struct work_struct
> *work)
> unsigned int tail;
> unsigned long flags;
>
> - queue_index = queue - bp->queues;
> + q = queue - bp->queues;
> netdev_vdbg(bp->netdev, "macb_tx_error_task: q = %u, t = %u, h =
> %u\n",
> - queue_index, queue->tx_tail, queue->tx_head);
> + q, queue->tx_tail, queue->tx_head);
>
> /* Prevent the queue NAPI TX poll from running, as it calls
> * macb_tx_complete(), which in turn may call netif_wake_subqueue().
> @@ -1342,7 +1342,7 @@ static void macb_tx_error_task(struct work_struct
> *work)
> macb_tx_unmap(bp, tx_skb, 0);
> }
>
> - netdev_tx_completed_queue(netdev_get_tx_queue(bp->netdev,
> queue_index),
> + netdev_tx_completed_queue(netdev_get_tx_queue(bp->netdev, q),
> packets, bytes);
>
> /* Set end of TX queue */
> @@ -1407,7 +1407,7 @@ static bool ptp_one_step_sync(struct sk_buff
> *skb)
> static int macb_tx_complete(struct macb_queue *queue, int budget)
> {
> struct macb *bp = queue->bp;
> - u16 queue_index = queue - bp->queues;
> + unsigned int q = queue - bp->queues;
> unsigned long flags;
> unsigned int tail;
> unsigned int head;
> @@ -1469,14 +1469,14 @@ static int macb_tx_complete(struct macb_queue
> *queue, int budget)
> }
> }
>
> - netdev_tx_completed_queue(netdev_get_tx_queue(bp->netdev,
> queue_index),
> + netdev_tx_completed_queue(netdev_get_tx_queue(bp->netdev, q),
> packets, bytes);
>
> queue->tx_tail = tail;
> - if (__netif_subqueue_stopped(bp->netdev, queue_index) &&
> + if (__netif_subqueue_stopped(bp->netdev, q) &&
> CIRC_CNT(queue->tx_head, queue->tx_tail,
> bp->tx_ring_size) <= MACB_TX_WAKEUP_THRESH(bp))
> - netif_wake_subqueue(bp->netdev, queue_index);
> + netif_wake_subqueue(bp->netdev, q);
> spin_unlock_irqrestore(&queue->tx_ptr_lock, flags);
>
> if (packets)
> @@ -2470,10 +2470,10 @@ static int macb_pad_and_fcs(struct sk_buff
> **skb, struct net_device *netdev)
> static netdev_tx_t macb_start_xmit(struct sk_buff *skb,
> struct net_device *netdev)
> {
> - u16 queue_index = skb_get_queue_mapping(skb);
> struct macb *bp = netdev_priv(netdev);
> - struct macb_queue *queue = &bp->queues[queue_index];
> + unsigned int q = skb_get_queue_mapping(skb);
> unsigned int desc_cnt, nr_frags, frag_size, f;
> + struct macb_queue *queue = &bp->queues[q];
> unsigned int hdrlen;
> unsigned long flags;
> bool is_lso;
> @@ -2512,8 +2512,8 @@ static netdev_tx_t macb_start_xmit(struct sk_buff
> *skb,
>
> #if defined(DEBUG) && defined(VERBOSE_DEBUG)
> netdev_vdbg(bp->netdev,
> - "start_xmit: queue %hu len %u head %p data %p tail %p end %p\n",
> - queue_index, skb->len, skb->head, skb->data,
> + "start_xmit: queue %u len %u head %p data %p tail %p end %p\n",
> + q, skb->len, skb->head, skb->data,
> skb_tail_pointer(skb), skb_end_pointer(skb));
> print_hex_dump(KERN_DEBUG, "data: ", DUMP_PREFIX_OFFSET, 16, 1,
> skb->data, 16, true);
> @@ -2539,7 +2539,7 @@ static netdev_tx_t macb_start_xmit(struct sk_buff
> *skb,
> /* This is a hard error, log it. */
> if (CIRC_SPACE(queue->tx_head, queue->tx_tail,
> bp->tx_ring_size) < desc_cnt) {
> - netif_stop_subqueue(netdev, queue_index);
> + netif_stop_subqueue(netdev, q);
> netdev_dbg(netdev, "tx_head = %u, tx_tail = %u\n",
> queue->tx_head, queue->tx_tail);
> ret = NETDEV_TX_BUSY;
> @@ -2555,7 +2555,7 @@ static netdev_tx_t macb_start_xmit(struct sk_buff
> *skb,
> /* Make newly initialized descriptor visible to hardware */
> wmb();
> skb_tx_timestamp(skb);
> - netdev_tx_sent_queue(netdev_get_tx_queue(bp->netdev, queue_index),
> + netdev_tx_sent_queue(netdev_get_tx_queue(bp->netdev, q),
> skb->len);
>
> spin_lock(&bp->lock);
> @@ -2564,7 +2564,7 @@ static netdev_tx_t macb_start_xmit(struct sk_buff
> *skb,
> spin_unlock(&bp->lock);
>
> if (CIRC_SPACE(queue->tx_head, queue->tx_tail, bp->tx_ring_size) < 1)
> - netif_stop_subqueue(netdev, queue_index);
> + netif_stop_subqueue(netdev, q);
>
> unlock:
> spin_unlock_irqrestore(&queue->tx_ptr_lock, flags);
Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>
Thanks
Nicolai
^ permalink raw reply
* Re: [PATCH net v2] net: airoha: Add retry mechanism to airoha_qdma_set_trtcm_param()
From: Leto Liu (刘涛) @ 2026-07-02 10:44 UTC (permalink / raw)
To: Lorenzo Bianconi, Simon Horman
Cc: andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com,
kuba@kernel.org, pabeni@redhat.com,
linux-arm-kernel@lists.infradead.org,
linux-mediatek@lists.infradead.org, netdev@vger.kernel.org,
Brown Huang (黃柏翰)
In-Reply-To: <ajrtd5WA2GFzQyV9@lore-desk>
First, let me descript the detailed mechanism of hardware QDMA register read/write is as follows:
CPU register reads do not have a “write-fail” issue; a CPU read operation does not fail to execute.
A CPU register write may not take effect as expected, i.e., the value written by the CPU may not be preserved. This is because QDMA performs non-atomic read-modify-write (RMW) operations, and a hardware write can overwrite a CPU write, causing the CPU write to be lost.
rw_done is cleared to 0 when the CPU issues a write request, and is set to 1 after completion. The interval is only a few hardware clock cycles (less than 10 ns).
Regarding write contention:
CPU side (driver/software) HW side (rate-limiter engine)
| |
| write cfg_param register | token-bucket refill tick / packet ingress/egress
| (indirectly writes SRAM) | (updates SRAM)
| |
v v
+-----------------------------------------------------------------------------------------------------------------------------------------+
| HW SRAM |
| |
| HW performs Read-Modify-Write (non-atomic) |
| | |
| | 1. HW reads the old value |
| | 2. CPU write happens first ---> contention point |
| | 3. HW write overwrites with old/mod value ---> CPU value overwritten |
| v |
+-----------------------------------------------------------------------------------------------------------------------------------------+
Best Regards, Leto
> > 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
> >
> > Simon says: I'd appreciate it if you could look over the first two issues
> > flagged below in the context of this patch. While the third
> > and final issue looks like it is in possible separate follow-up
> > territory.
> >
> > ---
> > net: airoha: Add retry mechanism to airoha_qdma_set_trtcm_param()
> >
> > This patch introduces a retry loop to airoha_qdma_set_trtcm_param() to
> > mitigate bus timeouts when configuring QDMA parameters. It attempts to
> > verify that the configuration is correctly applied to the hardware.
> >
> > > diff --git a/drivers/net/ethernet/airoha/airoha_eth.c
> > > b/drivers/net/ethernet/airoha/airoha_eth.c
> > > index 3370c3df7c107..bb5c0599a4eed 100644
> > > --- a/drivers/net/ethernet/airoha/airoha_eth.c
> > > +++ b/drivers/net/ethernet/airoha/airoha_eth.c
> > > @@ -2673,14 +2673,30 @@ static int airoha_qdma_set_trtcm_param(struct airoha_qdma *qdma, int channel,
> > > FIELD_PREP(TRTCM_METER_GROUP_MASK, group) |
> > > FIELD_PREP(TRTCM_PARAM_INDEX_MASK, idx) |
> > > FIELD_PREP(TRTCM_PARAM_RATE_TYPE_MASK, mode);
> > > + int i;
> > >
> > > - airoha_qdma_wr(qdma, REG_TRTCM_DATA_LOW(addr), val);
> > > - airoha_qdma_wr(qdma, REG_TRTCM_CFG_PARAM(addr), config);
> > > + for (i = 0; i < 100; i++) {
> > > + u32 data;
> > >
> > > - return read_poll_timeout(airoha_qdma_rr, val,
> > > - val & TRTCM_PARAM_RW_DONE_MASK,
> > > - USEC_PER_MSEC, 10 * USEC_PER_MSEC, true,
> > > - qdma, REG_TRTCM_CFG_PARAM(addr));
> > > + airoha_qdma_wr(qdma, REG_TRTCM_DATA_LOW(addr), val);
> > > + wmb();
> > > + airoha_qdma_wr(qdma, REG_TRTCM_CFG_PARAM(addr), config);
> > > +
> > > + if (read_poll_timeout(airoha_qdma_rr, data,
> > > + data & TRTCM_PARAM_RW_DONE_MASK,
> > > + USEC_PER_MSEC, 10 * USEC_PER_MSEC,
> > > + true, qdma, REG_TRTCM_CFG_PARAM(addr)))
> > > + return -ETIMEDOUT;
> >
> > [Severity: High]
> > If a bus timeout causes the write phase's read_poll_timeout() in
> > airoha_qdma_set_trtcm_param() to fail, this immediately returns
> > -ETIMEDOUT, aborting the 100-iteration retry loop.
> >
> > Doesn't this defeat the mitigation?
Here we are polling the hardware done bit. Based on the hardware behavior described above, a read operation will not fail. In addition, the hardware register write completes within at most 10 ns, while the poll timeout here is configured more than 1 ms. From these two points, we can conclude that this read_poll_timeout will always succeed (i.e., it will return 0). Therefore, adding a return -ETIMEDOUT path here will not cause any issues.
Best Regards, Leto
>
> I agree we just run 'continue' here instead of returing -ETIMEDOUT
No change is needed here, and even if we modify it, it will not cause any issues.
Best Regards, Leto
> >
> > > +
> > > + if (airoha_qdma_get_trtcm_param(qdma, channel, addr, param,
> > > + mode, &data, NULL))
> > > + continue;
> >
> > [Severity: High]
> > If airoha_qdma_get_trtcm_param() times out during the verification
> > phase, it indicates the hardware hasn't finished the read command (DONE bit is still 0).
> >
> > The continue statement then immediately starts the next iteration,
> > writing new commands to DATA_LOW and CFG_PARAM.
> >
> > Can writing to command registers while the hardware state machine is
> > actively busy violate hardware protocols and lead to unrecoverable lockups?
>
> @Brown and @Leto: can you please comment on this case?
>
the hardware register write completes within at most 10 ns, while the poll timeout here is configured more than 1 ms. When airoha_qdma_get_trtcm_param() is called here, the hardware has already completed the previous register write operation. Therefore, the concern mentioned here will definitely not occur, and there will be no issue at this point.
Best Regards, Leto
> >
> > > +
> > > + if (data == val)
> > > + return 0;
> >
> > [Severity: High]
> > This isn't a bug introduced by this patch, but if the bus drops the
> > command write to REG_TRTCM_CFG_PARAM, the hardware never executes the
> > command and the TRTCM_PARAM_RW_DONE_MASK bit remains set from the previous operation.
> >
> > Consequently, read_poll_timeout() returns an immediate false success.
> >
> > If both the set and get command writes are dropped by the bus, the get
> > operation will read back the newly written DATA_LOW register value.
> >
> > Will the data == val check evaluate to true and falsely report success
> > without the hardware ever processing the configuration?
>
> This seems quite a 'pathological' scenario, is it really an issue?
>
> Regards,
> Lorenzo
>
As explained above, the concern here does not apply and there will be no issue.
Best Regards, Leto
> >
> > > + }
> > > +
> > > + return -EBUSY;
> > > }
> > >
> >
> > [ ... ]
> >
> > > static int airoha_qdma_set_trtcm_config(struct airoha_qdma *qdma, int channel,
> > > u32 addr, enum trtcm_mode_type mode,
> > > bool enable, u32 enable_mask)
> > > {
> > > u32 val;
> > >
> > > if (airoha_qdma_get_trtcm_param(qdma, channel, addr, TRTCM_MISC_MODE,
> > > mode, &val, NULL))
> > > return -EINVAL;
> >
> > [Severity: High]
> > Standalone calls to airoha_qdma_get_trtcm_param(), like this one in
> > airoha_qdma_set_trtcm_config(), lack a retry loop entirely.
> >
> > Does this leave read-modify-write cycles fully vulnerable to the same
> > bus timeouts the patch attempts to mitigate?
> >
No change is needed here. Even if the write succeeds but the read-back during verification returns an incorrect value, the verification will fail and the code will proceed to the next retry. A failure would only occur if this happened for 100 consecutive attempts, which has an almost zero probability. Therefore, this code does not affect the final outcome.
That said, your suggestion is a good one, and it is still worthwhile to improve/optimize the read function.
Best Regards, Leto
> > [Severity: High]
> > This isn't a bug introduced by this patch, but the patch mitigates bus
> > timeouts in airoha_qdma_set_trtcm_param() while entirely missing
> > airoha_qdma_set_rl_param().
> >
> > The latter performs the exact same write-and-poll sequence on
> > identical QDMA configuration registers:
> >
> > drivers/net/ethernet/airoha/airoha_eth.c:airoha_qdma_set_rl_param() {
> > ...
> > return read_poll_timeout(..., val & RATE_LIMIT_PARAM_RW_DONE_MASK,
> > ...); }
> >
> > If bus timeouts affect QDMA access broadly as stated in the commit
> > message, are the rate-limiting configuration paths randomly vulnerable to the same failures?
> >
Yes, you are right. Function airoha_qdma_set_rl_param need readback protecting.
Best Regards, Leto
> > [Severity: High]
> > This isn't a bug introduced by this patch, but in
> > airoha_tc_remove_htb_queue(), hardware teardown incorrectly uses queue + 1 instead of queue:
> >
> > drivers/net/ethernet/airoha/airoha_eth.c:airoha_tc_remove_htb_queue() {
> > ...
> > airoha_qdma_set_tx_rate_limit(netdev, queue + 1, 0, 0);
> > ...
> > }
> >
> > Does this leave the removed queue's hardware limits permanently active
> > (resource leak) and inadvertently disable the rate limits for queue +
> > 1, corrupting the QoS state of another active channel?
It has already been fixed, refer to https://github.com/LorenzoBianconi/net-next/commit/d775d542504cbfea2b214149405d97eb00a88dc4
Best Regards, Leto
^ permalink raw reply
* Re: [PATCH 05/10] net: emac: mal: replace busy-wait in mal_poll_disable with wait_event
From: Paolo Abeni @ 2026-07-02 10:46 UTC (permalink / raw)
To: Rosen Penev, netdev
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
open list
In-Reply-To: <20260630041634.284127-6-rosenp@gmail.com>
On 6/30/26 6:16 AM, Rosen Penev wrote:
> @@ -371,8 +373,8 @@ static irqreturn_t mal_int(int irq, void *dev_instance)
> void mal_poll_disable(struct mal_instance *mal, struct mal_commac *commac)
> {
> /* Spinlock-type semantics: only one caller disable poll at a time */
> - while (test_and_set_bit(MAL_COMMAC_POLL_DISABLED, &commac->flags))
> - msleep(1);
> + wait_event(commac->poll_wait,
> + !test_and_set_bit(MAL_COMMAC_POLL_DISABLED, &commac->flags));
The above looks dangerous to me. IDK which is the expected latency here,
but unconditionally spinning without any obvious upper bound and no
reschedule does not look good.
/P
^ permalink raw reply
* Re: [PATCH net-next v3 05/15] net: macb: enforce reverse christmas tree (RCT) convention
From: Nicolai Buchwitz @ 2026-07-02 10:48 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-5-00268d5b1502@bootlin.com>
On 1.7.2026 17:59, Théo Lebrun wrote:
> Enforce the reverse christmas tree convention in those functions:
>
> macb_tx_error_task()
> gem_rx_refill()
> gem_rx()
> macb_rx_frame()
> macb_init_rx_ring()
> macb_rx()
> macb_rx_pending()
> macb_start_xmit()
>
> The goal is to minimise unrelated diff in future patches.
>
> In macb_tx_error_task(), we fold the assignment into the declaration
> statement.
>
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---
> drivers/net/ethernet/cadence/macb_main.c | 61
> ++++++++++++++++----------------
> 1 file changed, 30 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb_main.c
> b/drivers/net/ethernet/cadence/macb_main.c
> index 3b75797381b6..8b52122bc134 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -1254,20 +1254,19 @@ static dma_addr_t macb_get_addr(struct macb
> *bp, struct macb_dma_desc *desc)
>
> static void macb_tx_error_task(struct work_struct *work)
> {
> - struct macb_queue *queue = container_of(work, struct macb_queue,
> - tx_error_task);
> - bool halt_timeout = false;
> - struct macb *bp = queue->bp;
> - unsigned int q;
> - u32 packets = 0;
> - u32 bytes = 0;
> - struct macb_tx_skb *tx_skb;
> - struct macb_dma_desc *desc;
> - struct sk_buff *skb;
> - unsigned int tail;
> - unsigned long flags;
> + struct macb_queue *queue = container_of(work, struct macb_queue,
> + tx_error_task);
> + unsigned int q = queue - queue->bp->queues;
> + struct macb *bp = queue->bp;
> + struct macb_tx_skb *tx_skb;
> + struct macb_dma_desc *desc;
> + bool halt_timeout = false;
> + struct sk_buff *skb;
> + unsigned long flags;
> + unsigned int tail;
> + u32 packets = 0;
> + u32 bytes = 0;
>
> - q = queue - bp->queues;
> netdev_vdbg(bp->netdev, "macb_tx_error_task: q = %u, t = %u, h =
> %u\n",
> q, queue->tx_tail, queue->tx_head);
>
> @@ -1487,11 +1486,11 @@ static int macb_tx_complete(struct macb_queue
> *queue, int budget)
>
> static void gem_rx_refill(struct macb_queue *queue)
> {
> - unsigned int entry;
> - struct sk_buff *skb;
> - dma_addr_t paddr;
> struct macb *bp = queue->bp;
> struct macb_dma_desc *desc;
> + struct sk_buff *skb;
> + unsigned int entry;
> + dma_addr_t paddr;
>
> while (CIRC_SPACE(queue->rx_prepared_head, queue->rx_tail,
> bp->rx_ring_size) > 0) {
> @@ -1584,11 +1583,11 @@ static int gem_rx(struct macb_queue *queue,
> struct napi_struct *napi,
> int budget)
> {
> struct macb *bp = queue->bp;
> - unsigned int len;
> - unsigned int entry;
> - struct sk_buff *skb;
> - struct macb_dma_desc *desc;
> - int count = 0;
> + struct macb_dma_desc *desc;
> + struct sk_buff *skb;
> + unsigned int entry;
> + unsigned int len;
> + int count = 0;
>
> while (count < budget) {
> u32 ctrl;
> @@ -1674,12 +1673,12 @@ static int gem_rx(struct macb_queue *queue,
> struct napi_struct *napi,
> static int macb_rx_frame(struct macb_queue *queue, struct napi_struct
> *napi,
> unsigned int first_frag, unsigned int last_frag)
> {
> - unsigned int len;
> - unsigned int frag;
> + struct macb *bp = queue->bp;
> + struct macb_dma_desc *desc;
> unsigned int offset;
> struct sk_buff *skb;
> - struct macb_dma_desc *desc;
> - struct macb *bp = queue->bp;
> + unsigned int frag;
> + unsigned int len;
>
> desc = macb_rx_desc(queue, last_frag);
> len = desc->ctrl & bp->rx_frm_len_mask;
> @@ -1755,9 +1754,9 @@ static int macb_rx_frame(struct macb_queue
> *queue, struct napi_struct *napi,
>
> static inline void macb_init_rx_ring(struct macb_queue *queue)
> {
> + struct macb_dma_desc *desc = NULL;
> struct macb *bp = queue->bp;
> dma_addr_t addr;
> - struct macb_dma_desc *desc = NULL;
> int i;
>
> addr = queue->rx_buffers_dma;
> @@ -1776,9 +1775,9 @@ static int macb_rx(struct macb_queue *queue,
> struct napi_struct *napi,
> {
> struct macb *bp = queue->bp;
> bool reset_rx_queue = false;
> - int received = 0;
> - unsigned int tail;
> int first_frag = -1;
> + unsigned int tail;
> + int received = 0;
>
> for (tail = queue->rx_tail; budget > 0; tail++) {
> struct macb_dma_desc *desc = macb_rx_desc(queue, tail);
> @@ -1853,8 +1852,8 @@ static int macb_rx(struct macb_queue *queue,
> struct napi_struct *napi,
> static bool macb_rx_pending(struct macb_queue *queue)
> {
> struct macb *bp = queue->bp;
> - unsigned int entry;
> - struct macb_dma_desc *desc;
> + struct macb_dma_desc *desc;
> + unsigned int entry;
>
> entry = macb_rx_ring_wrap(bp, queue->rx_tail);
> desc = macb_rx_desc(queue, entry);
> @@ -2474,10 +2473,10 @@ static netdev_tx_t macb_start_xmit(struct
> sk_buff *skb,
> unsigned int q = skb_get_queue_mapping(skb);
> unsigned int desc_cnt, nr_frags, frag_size, f;
> struct macb_queue *queue = &bp->queues[q];
> + netdev_tx_t ret = NETDEV_TX_OK;
> unsigned int hdrlen;
> unsigned long flags;
> bool is_lso;
> - netdev_tx_t ret = NETDEV_TX_OK;
>
> if (macb_clear_csum(skb)) {
> dev_kfree_skb_any(skb);
Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>
Thanks
Nicolai
^ 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