* 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 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 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
* [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 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 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
* [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-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
* 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
* 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 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 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 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 05/10] net/nebula-matrix: add common 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-6-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 resource implementation
The patch adds the common part of the resource layer for the Nebula Matrix
driver, including conversion relationships among vsi_id, func_id, eth_id,
and pf_id.
> diff --git a/drivers/net/ethernet/nebula-matrix/nbl/nbl_hw/nbl_resource.c b/drivers/net/ethernet/nebula-matrix/nbl/nbl_hw/nbl_resource.c
> new file mode 100644
> index 0000000000000..6742da7c66376
> --- /dev/null
> +++ b/drivers/net/ethernet/nebula-matrix/nbl/nbl_hw/nbl_resource.c
[ ... ]
> +int nbl_res_func_id_to_vsi_id(struct nbl_resource_mgt *res_mgt, u16 func_id,
> + u16 type, u16 *vsi_id)
> +{
[ ... ]
> + nbl_res_pf_dev_vsi_type_to_hw_vsi_type(type, &dst_type);
> + *vsi_id = vsi_info->serv_info[diff][dst_type].base_id;
> + return 0;
> +}
[ ... ]
> +void nbl_res_pf_dev_vsi_type_to_hw_vsi_type(u16 src_type,
> + enum nbl_vsi_serv_type *dst_type)
> +{
> + switch (src_type) {
> + case NBL_VSI_DATA:
> + *dst_type = NBL_VSI_SERV_PF_DATA_TYPE;
> + break;
> + default:
> + WARN_ON_ONCE(1);
[Severity: High]
Can a malicious or buggy Virtual Function (VF) trigger this warning by
passing an invalid type?
Because nbl_res_pf_dev_vsi_type_to_hw_vsi_type() returns void, it cannot
propagate an error back to the caller. When an invalid type is processed,
nbl_res_func_id_to_vsi_id() silently defaults to NBL_VSI_SERV_PF_DATA_TYPE
and returns 0 for success.
Should this function return an error code so that nbl_res_func_id_to_vsi_id()
can properly reject the invalid type instead of reporting a false success?
> + break;
> + }
> +}
--
This is an AI-generated review.
^ permalink raw reply
* Re: [PATCH v20 net-next 04/10] net/nebula-matrix: add channel layer
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-5-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/
---
Subject: net/nebula-matrix: add channel layer
This patch introduces a channel management layer for structured communication
between components and drivers. It implements message registration, sending,
and acknowledgment via hardware mailbox queues.
> diff --git a/drivers/net/ethernet/nebula-matrix/nbl/nbl_channel/nbl_channel.c b/drivers/net/ethernet/nebula-matrix/nbl/nbl_channel/nbl_channel.c
> index c7689f0e40293..eca9495449938 100644
> --- a/drivers/net/ethernet/nebula-matrix/nbl/nbl_channel/nbl_channel.c
> +++ b/drivers/net/ethernet/nebula-matrix/nbl/nbl_channel/nbl_channel.c
[ ... ]
> @@ -2,12 +2,997 @@
[ ... ]
> +static int nbl_chan_teardown_queue(struct nbl_channel_mgt *chan_mgt,
> + u8 chan_type)
> +{
[ ... ]
> + /*
> + * Lock and unlock to ensure all in-flight callers have released the
> + * lock. After unlock returns, we're guaranteed no other thread is
> + * using the lock, so it's safe to destroy it.
> + */
> + mutex_lock(&chan_info->txq_lock);
> + mutex_unlock(&chan_info->txq_lock);
> + wait_for_completion(&chan_info->teardown_done);
> + mutex_destroy(&chan_info->txq_lock);
> +
> + return 0;
> +}
[Severity: High]
Does this code deadlock or fail to synchronize?
Because nbl_chan_send_msg() calls complete(&chan_info->teardown_done) on
every exit path, it increments the completion counter for every message sent.
If no messages were ever sent, the counter is 0 and this wait_for_completion()
will block forever.
If messages were sent, the counter is greater than 0, so this returns
immediately without waiting for active senders. It then destroys txq_lock
while other threads might still be using it, which can cause a use-after-free
or lock corruption.
[ ... ]
> +static int nbl_chan_update_txqueue(struct nbl_channel_mgt *chan_mgt,
> + struct nbl_chan_info *chan_info,
> + struct nbl_chan_tx_param *param)
> +{
[ ... ]
> + if (param->arg_len > NBL_CHAN_TX_DESC_EMBEDDED_DATA_LEN) {
> + memcpy(tx_buf->va, param->arg, param->arg_len);
> + tx_desc->buf_addr = cpu_to_le64(tx_buf->pa);
> + tx_desc->buf_len = cpu_to_le16(param->arg_len);
> + tx_desc->data_len = 0;
> + memset(tx_desc->data, 0, sizeof(tx_desc->data));
> + } else {
> + memset(tx_desc->data, 0, sizeof(tx_desc->data));
> + memcpy(tx_desc->data, param->arg, param->arg_len);
> + tx_desc->buf_len = 0;
> + tx_desc->data_len = cpu_to_le16(param->arg_len);
> + }
[Severity: High]
Does this code leak host physical memory addresses to the receiving endpoint?
In the else branch (when embedding data), tx_desc->buf_addr is not cleared.
Since the TX descriptor ring is reused circularly, if this slot previously held
a large message, buf_addr still contains the physical DMA address of the host's
buffer.
This leaked physical address is written to memory shared with the receiving
endpoint, potentially crossing a security boundary.
[ ... ]
> +static void nbl_chan_clean_queue(struct nbl_channel_mgt *chan_mgt,
> + struct nbl_chan_info *chan_info)
> +{
[ ... ]
> + while (le16_to_cpu(rx_desc->flags) & BIT(NBL_CHAN_RX_DESC_USED)) {
[ ... ]
> + if (--budget == 0)
> + break;
> + }
> + rxq->next_to_clean = next_to_clean;
> +}
[Severity: High]
Can this permanently stall the RX queue?
The loop exits when the budget is exhausted, but nbl_chan_clean_queue()
returns void and doesn't indicate that more packets remain.
The workqueue handler does not check if the RX queue was completely drained
and won't reschedule the work item. If the ring wasn't completely drained,
edge-triggered MSI-X interrupts won't fire again until a new packet arrives,
leading to a stalled queue and missed messages.
[ ... ]
> +static int nbl_chan_send_msg(struct nbl_channel_mgt *chan_mgt,
> + struct nbl_chan_send_info *chan_send)
> +{
[ ... ]
> + if (!chan_send->ack) {
> + complete(&chan_info->teardown_done);
> + return 0;
> + }
[Severity: High]
Is it intentional to call complete(&chan_info->teardown_done) here and on
all other exit paths of nbl_chan_send_msg()?
This increments the completion counter on every message sent, which breaks the
synchronization in nbl_chan_teardown_queue() (as mentioned above).
[ ... ]
--
This is an AI-generated review.
^ permalink raw reply
* Re: [PATCH v3] net/sched: cake: reject overhead values that underflow length
From: Toke Høiland-Jørgensen @ 2026-07-02 9:40 UTC (permalink / raw)
To: Samuel Moelius
Cc: Samuel Moelius, Jamal Hadi Salim, Jiri Pirko, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
moderated list:CAKE QDISC, open list:TC subsystem, open list
In-Reply-To: <20260702000758.297407.e5c888d9d99d.cake-overhead-underflow@trailofbits.com>
Samuel Moelius <sam.moelius@trailofbits.com> writes:
> CAKE accepts signed overhead values and stores them in an s16, but the
> adjusted packet length calculation uses unsigned arithmetic. A negative
> effective length can therefore wrap to a large value.
>
> Such configurations make rate accounting depend on integer wraparound
> rather than on the packet size userspace intended to model. A static
> netlink lower bound is not enough because packets reaching CAKE can be
> smaller than any reasonable manual-overhead allowance.
>
> Fold the signed overhead adjustment into the existing datapath MPU clamp
> so negative adjusted lengths are clamped before link-layer framing
> adjustments.
>
> Fixes: a729b7f0bd5b ("sch_cake: Add overhead compensation support to the rate shaper")
> Assisted-by: Codex:gpt-5.5-cyber-preview
> Signed-off-by: Samuel Moelius <sam.moelius@trailofbits.com>
Acked-by: Toke Høiland-Jørgensen <toke@toke.dk>
^ permalink raw reply
* Re: [PATCH net-next v2 0/5] net: dsa: realtek: rtl8366rb: Use generic RTL83xx code
From: patchwork-bot+netdevbpf @ 2026-07-02 9:40 UTC (permalink / raw)
To: Linus Walleij
Cc: luizluca, alsi, andrew, olteanv, davem, edumazet, kuba, pabeni,
netdev
In-Reply-To: <20260630-rtl8366rb-improvements-v2-0-05eb9d6a37f5@kernel.org>
Hello:
This series was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:
On Tue, 30 Jun 2026 13:19:40 +0200 you wrote:
> As a follow-up to Luiz's and Alvin's series improvining the
> generic handling of the Realtek DSA switches, this small
> series brings the RTL8366RB closer to the way things are done
> in the RTL8365MB driver.
>
> This patch series switches over to using the generic helpers
> for:
>
> [...]
Here is the summary with links:
- [net-next,v2,1/5] net: dsa: realtek: rtl83xx: Make learning optional in join/leave
https://git.kernel.org/netdev/net-next/c/69bf497cfa79
- [net-next,v2,2/5] net: dsa: realtek: rtl8366rb: Switch to generic port_bridge* handlers
https://git.kernel.org/netdev/net-next/c/82ddf181448b
- [net-next,v2,3/5] net: dsa: realtek: rtl8366rb: Use DSA port iterators
https://git.kernel.org/netdev/net-next/c/cc61d5d7c205
- [net-next,v2,4/5] net: dsa: realtek: rtl8366rb: Disable STP learning on all ports in setup
https://git.kernel.org/netdev/net-next/c/e058ab0c4616
- [net-next,v2,5/5] net: dsa: realtek: rtl8366rb: Switch to generic learning enablement
https://git.kernel.org/netdev/net-next/c/b269a0596191
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] usb: atm: ueagle: fix use-after-free in uea_upload_pre_firmware()
From: Stanislaw Gruszka @ 2026-07-02 9:37 UTC (permalink / raw)
To: Deepanshu Kartikey
Cc: castet.matthieu, 3chas3, gregkh, linux-atm-general, netdev,
linux-usb, linux-kernel, syzbot+3d45d763d18796f97412,
Mauricio Faria de Oliveira
In-Reply-To: <20260630041716.97102-1-kartikey406@gmail.com>
Hi, thanks for working at this,
On Tue, Jun 30, 2026 at 09:47:16AM +0530, Deepanshu Kartikey wrote:
> uea_load_firmware() calls request_firmware_nowait() passing a raw
> struct usb_device pointer as context, without holding a reference
> to it.
>
> If the USB device is disconnected before the firmware workqueue
> fires, the usb_device and its usb_interface objects are freed while
> uea_upload_pre_firmware() is still pending on the workqueue. When
> the callback eventually runs, it accesses the freed memory causing
> a slab-use-after-free:
>
> BUG: KASAN: slab-use-after-free in __intf_to_usbdev
> include/linux/usb.h:752 [inline]
> BUG: KASAN: slab-use-after-free in uea_upload_pre_firmware+0x8d/0x640
> drivers/usb/atm/ueagle-atm.c:598
> Read of size 8 at addr ffff88802b0710b8 by task kworker/0:2/1664
>
> Fix by calling usb_get_dev() before queuing the firmware request to
> pin the usb_device in memory for the lifetime of the async operation,
> and usb_put_dev() in the callback once it is finished with the
> pointer. On the error path where request_firmware_nowait() itself
> fails, drop the reference immediately since the callback will never
> fire.
> Reported-by: syzbot+3d45d763d18796f97412@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=3d45d763d18796f97412
I think the problem is not lack of usb device reference.
request_firmware_nowait() does get_device() and after fw work
finish - put_device().
I suspect the issue is that syskaller corrupt descriptor such
the below condition:
else if (usb->config->desc.bNumInterfaces == 1)
is not met for pre-firmware device.
Adding Mauricio, who has setup for reproducing syskaller bugs on ueagle.
Hopefully he can confirm the diagnostic. If it's correct, we could
either save flag to recognize pre-firmware device, or separate driver
probe/disconnect for pre-firmware and post-firmware, to fix the issue.
Regards
Stanislaw
> Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
> ---
> drivers/usb/atm/ueagle-atm.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/atm/ueagle-atm.c b/drivers/usb/atm/ueagle-atm.c
> index d610cdcef7d0..686cc58fb89f 100644
> --- a/drivers/usb/atm/ueagle-atm.c
> +++ b/drivers/usb/atm/ueagle-atm.c
> @@ -663,6 +663,7 @@ static void uea_upload_pre_firmware(const struct firmware *fw_entry,
> uea_err(usb, "firmware is corrupted\n");
> err:
> release_firmware(fw_entry);
> + usb_put_dev(usb);
> }
>
> /*
> @@ -693,12 +694,14 @@ static int uea_load_firmware(struct usb_device *usb, unsigned int ver)
> break;
> }
>
> + usb_get_dev(usb);
> ret = request_firmware_nowait(THIS_MODULE, 1, fw_name, &usb->dev,
> GFP_KERNEL, usb,
> uea_upload_pre_firmware);
> - if (ret)
> + if (ret) {
> uea_err(usb, "firmware %s is not available\n", fw_name);
> - else
> + usb_put_dev(usb);
> + } else
> uea_info(usb, "loading firmware %s\n", fw_name);
>
> return ret;
> --
> 2.43.0
>
^ permalink raw reply
* [PATCH iwl-next v1 6/6] ixgbe: take rtnl lock before ixgbe_reset() is called
From: Jedrzej Jagielski @ 2026-07-02 9:15 UTC (permalink / raw)
To: intel-wired-lan; +Cc: anthony.l.nguyen, netdev, Jedrzej Jagielski
In-Reply-To: <20260702091553.57112-1-jedrzej.jagielski@intel.com>
Previous commit introduced ixgbe_mac_addr_refresh which touches netdev
struct by updating mac addr. It should operate after taking rtnl lock.
One of the callers is ixgbe_reset(). Most of scenarios when ixgbe_reset()
is called met taking lock requirement, but there is a ixgbe_resume() path
which calls ixgbe_reset() -> ixgbe_mac_addr_refresh() without taking
the lock. So there is a risk of race.
Move rtnl_lock() before ixgbe_reset() is called.
Signed-off-by: Jedrzej Jagielski <jedrzej.jagielski@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index ce2b1e208c0f..c7261eb0e9b0 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -7574,11 +7574,11 @@ static int ixgbe_resume(struct device *dev_d)
device_wakeup_disable(dev_d);
+ rtnl_lock();
ixgbe_reset(adapter);
IXGBE_WRITE_REG(&adapter->hw, IXGBE_WUS, ~0);
- rtnl_lock();
err = ixgbe_init_interrupt_scheme(adapter);
if (!err && netif_running(netdev))
err = ixgbe_open(netdev);
--
2.31.1
^ permalink raw reply related
* [PATCH iwl-next v1 5/6] ixgbe: E610: add MAC address runtime refresh
From: Jedrzej Jagielski @ 2026-07-02 9:15 UTC (permalink / raw)
To: intel-wired-lan; +Cc: anthony.l.nguyen, netdev, Jedrzej Jagielski
In-Reply-To: <20260702091553.57112-1-jedrzej.jagielski@intel.com>
Whenever MGMT requests MAC addr change and FW does it, driver does not
get aligned to that change. Current - legacy approach doesn't handle
such scenario.
Poll RAR0 (Receive Address Register) each service task cycle and update
driver and netdev structs once new MAC addr is detected. It may happen
that FW updates MAC address stored in RAR0 during runtime so SW shall fetch
the new address.
Refresh addr also during reset path to ensure the address survives RAR0
clearing during init_hw().
Signed-off-by: Jedrzej Jagielski <jedrzej.jagielski@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 37 +++++++++++++++++++
1 file changed, 37 insertions(+)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index d1cfe913081f..ce2b1e208c0f 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -6499,6 +6499,36 @@ void ixgbe_disable_tx(struct ixgbe_adapter *adapter)
}
}
+static void ixgbe_mac_addr_refresh(struct ixgbe_adapter *adapter)
+{
+ struct net_device *netdev = adapter->netdev;
+ struct ixgbe_hw *hw = &adapter->hw;
+ int err;
+
+ if (hw->mac.type != ixgbe_mac_e610)
+ return;
+
+ /* fetch address stored currently in RAR0 in case the addr has been
+ * altered by FW; if so, use it as the default one
+ */
+ err = hw->mac.ops.get_mac_addr(hw, hw->mac.addr);
+ if (err) {
+ e_dev_warn("Cannot get MAC address\n");
+ return;
+ }
+
+ if (ether_addr_equal(netdev->dev_addr, hw->mac.addr) ||
+ !is_valid_ether_addr(hw->mac.addr))
+ return;
+
+ ASSERT_RTNL();
+
+ eth_hw_addr_set(netdev, hw->mac.addr);
+ ether_addr_copy(adapter->mac_table[0].addr, hw->mac.addr);
+
+ call_netdevice_notifiers(NETDEV_CHANGEADDR, netdev);
+}
+
void ixgbe_reset(struct ixgbe_adapter *adapter)
{
struct ixgbe_hw *hw = &adapter->hw;
@@ -6516,6 +6546,8 @@ void ixgbe_reset(struct ixgbe_adapter *adapter)
IXGBE_FLAG2_SFP_NEEDS_RESET);
adapter->flags &= ~IXGBE_FLAG_NEED_LINK_CONFIG;
+ ixgbe_mac_addr_refresh(adapter);
+
err = hw->mac.ops.init_hw(hw);
switch (err) {
case 0:
@@ -8701,6 +8733,11 @@ static void ixgbe_service_task(struct work_struct *work)
ixgbe_handle_fw_event(adapter);
ixgbe_check_media_subtask(adapter);
}
+
+ rtnl_lock();
+ ixgbe_mac_addr_refresh(adapter);
+ rtnl_unlock();
+
ixgbe_reset_subtask(adapter);
ixgbe_phy_interrupt_subtask(adapter);
ixgbe_sfp_detection_subtask(adapter);
--
2.31.1
^ permalink raw reply related
* [PATCH iwl-next v1 4/6] ixgbe: E610: re-enable LSE unconditionally
From: Jedrzej Jagielski @ 2026-07-02 9:15 UTC (permalink / raw)
To: intel-wired-lan
Cc: anthony.l.nguyen, netdev, Jedrzej Jagielski, Aleksandr Loktionov
In-Reply-To: <20260702091553.57112-1-jedrzej.jagielski@intel.com>
ixgbe_aci_get_link_info() currently allows to [en/dis]able LSE by @ena_lse
param. This isn't proper approach as this function objective is completely
different than toggling LSE feature. Creating such parameter was probably
dictated by the fact the flag corresponding for LSE enablement was placed
within the ACI command used by the function.
LSE should not be switched off while the ixgbe driver is running.
Current driver behavior doesn't provide such possibility for the user.
Enable LSE by default whenever 0x0607 ACI command is sent. Such change
corresponds to the fact that FW disables LSE whenever new event is
triggered. ixgbe_aci_get_link_info() is a part of routine handling LSE
events, so it ensures that LSE remain enabled.
Remove wrappers utilizing @ena_lse param to [en/dis]able LSE as they
are no longer valid.
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Signed-off-by: Jedrzej Jagielski <jedrzej.jagielski@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c | 25 ++++---------------
drivers/net/ethernet/intel/ixgbe/ixgbe_e610.h | 3 +--
.../net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 2 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 23 +----------------
4 files changed, 8 insertions(+), 45 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
index 54cc0f116e88..ec7d8073efe6 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
@@ -1391,7 +1391,7 @@ int ixgbe_update_link_info(struct ixgbe_hw *hw)
li = &hw->link.link_info;
- err = ixgbe_aci_get_link_info(hw, true, NULL);
+ err = ixgbe_aci_get_link_info(hw, NULL);
if (err)
return err;
@@ -1445,7 +1445,6 @@ int ixgbe_get_link_status(struct ixgbe_hw *hw, bool *link_up)
/**
* ixgbe_aci_get_link_info - get the link status
* @hw: pointer to the HW struct
- * @ena_lse: enable/disable LinkStatusEvent reporting
* @link: pointer to link status structure - optional
*
* Get the current Link Status using ACI command (0x607).
@@ -1454,8 +1453,7 @@ int ixgbe_get_link_status(struct ixgbe_hw *hw, bool *link_up)
*
* Return: the link status of the adapter.
*/
-int ixgbe_aci_get_link_info(struct ixgbe_hw *hw, bool ena_lse,
- struct ixgbe_link_status *link)
+int ixgbe_aci_get_link_info(struct ixgbe_hw *hw, struct ixgbe_link_status *link)
{
struct ixgbe_aci_cmd_get_link_status_data link_data = {};
struct ixgbe_aci_cmd_get_link_status *resp;
@@ -1474,7 +1472,7 @@ int ixgbe_aci_get_link_info(struct ixgbe_hw *hw, bool ena_lse,
hw_fc_info = &hw->fc;
ixgbe_fill_dflt_direct_cmd_desc(&desc, ixgbe_aci_opc_get_link_status);
- cmd_flags = (ena_lse) ? IXGBE_ACI_LSE_ENA : IXGBE_ACI_LSE_DIS;
+ cmd_flags = IXGBE_ACI_LSE_ENA;
resp = libie_aq_raw(&desc);
resp->cmd_flags = cpu_to_le16(cmd_flags);
resp->lport_num = hw->bus.func;
@@ -1561,19 +1559,6 @@ static int ixgbe_aci_init_event_mask(struct ixgbe_hw *hw)
return ixgbe_aci_set_event_mask(hw, (u8)hw->bus.func, mask);
}
-/**
- * ixgbe_configure_lse - enable/disable link status events
- * @hw: pointer to the HW struct
- * @activate: true for enable lse, false otherwise
- *
- * Return: the exit code of the operation.
- */
-int ixgbe_configure_lse(struct ixgbe_hw *hw, bool activate)
-{
- /* Enabling link status events generation by fw. */
- return ixgbe_aci_get_link_info(hw, activate, NULL);
-}
-
/**
* ixgbe_start_hw_e610 - Prepare hardware for Tx/Rx
* @hw: pointer to hardware structure
@@ -1913,7 +1898,7 @@ void ixgbe_fc_autoneg_e610(struct ixgbe_hw *hw)
/* Get current link err.
* Current FC mode will be stored in the hw context.
*/
- err = ixgbe_aci_get_link_info(hw, true, NULL);
+ err = ixgbe_aci_get_link_info(hw, NULL);
if (err)
goto no_autoneg;
@@ -2219,7 +2204,7 @@ int ixgbe_setup_phy_link_e610(struct ixgbe_hw *hw)
u64 phy_type_low = 0, phy_type_high = 0;
int err;
- err = ixgbe_aci_get_link_info(hw, true, NULL);
+ err = ixgbe_aci_get_link_info(hw, NULL);
if (err)
return err;
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.h
index ecda35a77adb..4fb4ab99d458 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.h
@@ -32,9 +32,8 @@ int ixgbe_aci_set_phy_cfg(struct ixgbe_hw *hw,
int ixgbe_aci_set_link_restart_an(struct ixgbe_hw *hw, bool ena_link);
int ixgbe_update_link_info(struct ixgbe_hw *hw);
int ixgbe_get_link_status(struct ixgbe_hw *hw, bool *link_up);
-int ixgbe_aci_get_link_info(struct ixgbe_hw *hw, bool ena_lse,
+int ixgbe_aci_get_link_info(struct ixgbe_hw *hw,
struct ixgbe_link_status *link);
-int ixgbe_configure_lse(struct ixgbe_hw *hw, bool activate);
int ixgbe_aci_set_port_id_led(struct ixgbe_hw *hw, bool orig_mode);
enum ixgbe_media_type ixgbe_get_media_type_e610(struct ixgbe_hw *hw);
int ixgbe_setup_link_e610(struct ixgbe_hw *hw, ixgbe_link_speed speed,
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
index b8e85bc91a27..5d71c1011cf4 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
@@ -3647,7 +3647,7 @@ static int ixgbe_get_eee_e610(struct net_device *netdev,
linkmode_zero(kedata->supported);
linkmode_zero(kedata->advertised);
- err = ixgbe_aci_get_link_info(hw, true, &link);
+ err = ixgbe_aci_get_link_info(hw, &link);
if (err)
return err;
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index f33a534490b5..d1cfe913081f 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -5966,19 +5966,6 @@ static void ixgbe_configure(struct ixgbe_adapter *adapter)
ixgbe_configure_dfwd(adapter);
}
-/**
- * ixgbe_enable_link_status_events - enable link status events
- * @adapter: pointer to the adapter structure
- *
- * Enables link status events by invoking ixgbe_configure_lse()
- *
- * Return: the exit code of the operation.
- */
-static int ixgbe_enable_link_status_events(struct ixgbe_adapter *adapter)
-{
- return ixgbe_configure_lse(&adapter->hw, true);
-}
-
/**
* ixgbe_sfp_link_config - set up SFP+ link
* @adapter: pointer to private adapter struct
@@ -6008,8 +5995,6 @@ static void ixgbe_sfp_link_config(struct ixgbe_adapter *adapter)
**/
static int ixgbe_non_sfp_link_config(struct ixgbe_hw *hw)
{
- struct ixgbe_adapter *adapter = container_of(hw, struct ixgbe_adapter,
- hw);
bool autoneg, link_up = false;
int ret = -EIO;
u32 speed;
@@ -6035,14 +6020,8 @@ static int ixgbe_non_sfp_link_config(struct ixgbe_hw *hw)
if (ret)
return ret;
- if (hw->mac.ops.setup_link) {
- if (adapter->hw.mac.type == ixgbe_mac_e610) {
- ret = ixgbe_enable_link_status_events(adapter);
- if (ret)
- return ret;
- }
+ if (hw->mac.ops.setup_link)
ret = hw->mac.ops.setup_link(hw, speed, link_up);
- }
return ret;
}
--
2.31.1
^ permalink raw reply related
* [PATCH iwl-next v1 3/6] ixgbe: E610: do not disable LSE on driver down/remove
From: Jedrzej Jagielski @ 2026-07-02 9:15 UTC (permalink / raw)
To: intel-wired-lan
Cc: anthony.l.nguyen, netdev, Jedrzej Jagielski, Aleksandr Loktionov
In-Reply-To: <20260702091553.57112-1-jedrzej.jagielski@intel.com>
For the first case:
LSE especially shall remain enabled when interface is down and link watchdog
cannot track status of the link. Then the only way for link update is by
handling LSE.
For the second case:
There's no value in disabling LSE when the driver's unloading is about to be
terminated.
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Signed-off-by: Jedrzej Jagielski <jedrzej.jagielski@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 18 ------------------
1 file changed, 18 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 2c8968b9b86c..f33a534490b5 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -5979,19 +5979,6 @@ static int ixgbe_enable_link_status_events(struct ixgbe_adapter *adapter)
return ixgbe_configure_lse(&adapter->hw, true);
}
-/**
- * ixgbe_disable_link_status_events - disable link status events
- * @adapter: pointer to the adapter structure
- *
- * Disables link status events by invoking ixgbe_configure_lse()
- *
- * Return: the exit code of the operation.
- */
-static int ixgbe_disable_link_status_events(struct ixgbe_adapter *adapter)
-{
- return ixgbe_configure_lse(&adapter->hw, false);
-}
-
/**
* ixgbe_sfp_link_config - set up SFP+ link
* @adapter: pointer to private adapter struct
@@ -6775,8 +6762,6 @@ void ixgbe_down(struct ixgbe_adapter *adapter)
ixgbe_clean_all_tx_rings(adapter);
ixgbe_clean_all_rx_rings(adapter);
- if (adapter->hw.mac.type == ixgbe_mac_e610)
- ixgbe_disable_link_status_events(adapter);
}
/**
@@ -12143,9 +12128,6 @@ static void ixgbe_remove(struct pci_dev *pdev)
set_bit(__IXGBE_REMOVING, &adapter->state);
cancel_work_sync(&adapter->service_task);
- if (adapter->hw.mac.type == ixgbe_mac_e610)
- ixgbe_disable_link_status_events(adapter);
-
if (adapter->mii_bus)
mdiobus_unregister(adapter->mii_bus);
--
2.31.1
^ permalink raw reply related
* [PATCH iwl-next v1 2/6] ixgbe: E610: prevent from disabling LSE
From: Jedrzej Jagielski @ 2026-07-02 9:15 UTC (permalink / raw)
To: intel-wired-lan
Cc: anthony.l.nguyen, netdev, Jedrzej Jagielski, Aleksandr Loktionov
In-Reply-To: <20260702091553.57112-1-jedrzej.jagielski@intel.com>
LSE is not allowed to be switched off while the ixgbe driver is running.
Some of the ixgbe_aci_get_link_info() callers called it with @ena_lse
equal to false what was causing issues.
There was a known issue when FW after being called by Redfish MGMT request
to disable an interface wasn't able to notify the driver about link status
change due to disabled LSE reception on the driver side. As a result driver
exposed incorrect link status.
There's no any requirement forcing to disable LSE either during configuring
FC, either during setting PHY params.
Force calling ixgbe_aci_get_link_info() with @ena_lse == true.
Fixes: 4600cdf9f5ac ("ixgbe: Enable link management in E610 device")
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Signed-off-by: Jedrzej Jagielski <jedrzej.jagielski@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
index d451da68fd6d..54cc0f116e88 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
@@ -1913,7 +1913,7 @@ void ixgbe_fc_autoneg_e610(struct ixgbe_hw *hw)
/* Get current link err.
* Current FC mode will be stored in the hw context.
*/
- err = ixgbe_aci_get_link_info(hw, false, NULL);
+ err = ixgbe_aci_get_link_info(hw, true, NULL);
if (err)
goto no_autoneg;
@@ -2219,7 +2219,7 @@ int ixgbe_setup_phy_link_e610(struct ixgbe_hw *hw)
u64 phy_type_low = 0, phy_type_high = 0;
int err;
- err = ixgbe_aci_get_link_info(hw, false, NULL);
+ err = ixgbe_aci_get_link_info(hw, true, NULL);
if (err)
return err;
--
2.31.1
^ permalink raw reply related
* [PATCH iwl-next v1 1/6] ixgbe: E610: init Link Status Events mask just once
From: Jedrzej Jagielski @ 2026-07-02 9:15 UTC (permalink / raw)
To: intel-wired-lan
Cc: anthony.l.nguyen, netdev, Jedrzej Jagielski, Aleksandr Loktionov
In-Reply-To: <20260702091553.57112-1-jedrzej.jagielski@intel.com>
Current approach is that LSE mask is configured each time the LSE feature
itself is toggled. The set of bits corresponding to LSE trigger causes
remain the same for the whole ixgbe lifecycle. There's no support for
enabling or disabling specific bits within LSE mask.
This is redundant; there's no need to configure LSE mask over and over.
The mask persists in hardware and only needs to be set once so should
be separated from the LSE toggling logic.
Do it just once at the init phase.
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Signed-off-by: Jedrzej Jagielski <jedrzej.jagielski@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe.h | 1 -
drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c | 34 ++++++++++++-------
drivers/net/ethernet/intel/ixgbe/ixgbe_e610.h | 3 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 28 +++------------
4 files changed, 27 insertions(+), 39 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index 30f62174acf2..17e0a9824777 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -821,7 +821,6 @@ struct ixgbe_adapter {
struct ixgbe_mac_addr *mac_table;
u8 tx_hang_count[IXGBE_MAX_TX_QUEUES];
struct kobject *info_kobj;
- u16 lse_mask;
#ifdef CONFIG_IXGBE_HWMON
struct hwmon_buff *ixgbe_hwmon_buff;
#endif /* CONFIG_IXGBE_HWMON */
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
index 831cfe9a4697..d451da68fd6d 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c
@@ -1536,7 +1536,7 @@ int ixgbe_aci_get_link_info(struct ixgbe_hw *hw, bool ena_lse,
*
* Return: the exit code of the operation.
*/
-int ixgbe_aci_set_event_mask(struct ixgbe_hw *hw, u8 port_num, u16 mask)
+static int ixgbe_aci_set_event_mask(struct ixgbe_hw *hw, u8 port_num, u16 mask)
{
struct ixgbe_aci_cmd_set_event_mask *cmd;
struct libie_aq_desc desc;
@@ -1551,25 +1551,25 @@ int ixgbe_aci_set_event_mask(struct ixgbe_hw *hw, u8 port_num, u16 mask)
return ixgbe_aci_send_cmd(hw, &desc, NULL, 0);
}
+static int ixgbe_aci_init_event_mask(struct ixgbe_hw *hw)
+{
+ u16 mask = ~((u16)(IXGBE_ACI_LINK_EVENT_UPDOWN |
+ IXGBE_ACI_LINK_EVENT_MEDIA_NA |
+ IXGBE_ACI_LINK_EVENT_MODULE_QUAL_FAIL |
+ IXGBE_ACI_LINK_EVENT_PHY_FW_LOAD_FAIL));
+
+ return ixgbe_aci_set_event_mask(hw, (u8)hw->bus.func, mask);
+}
+
/**
* ixgbe_configure_lse - enable/disable link status events
* @hw: pointer to the HW struct
* @activate: true for enable lse, false otherwise
- * @mask: event mask to be set; a set bit means deactivation of the
- * corresponding event
- *
- * Set the event mask and then enable or disable link status events
*
* Return: the exit code of the operation.
*/
-int ixgbe_configure_lse(struct ixgbe_hw *hw, bool activate, u16 mask)
+int ixgbe_configure_lse(struct ixgbe_hw *hw, bool activate)
{
- int err;
-
- err = ixgbe_aci_set_event_mask(hw, (u8)hw->bus.func, mask);
- if (err)
- return err;
-
/* Enabling link status events generation by fw. */
return ixgbe_aci_get_link_info(hw, activate, NULL);
}
@@ -1597,6 +1597,16 @@ static int ixgbe_start_hw_e610(struct ixgbe_hw *hw)
ixgbe_start_hw_gen2(hw);
+ err = ixgbe_aci_init_event_mask(hw);
+ /* In case of error just log it, don't fail the whole init */
+ if (err) {
+ struct ixgbe_adapter *adapter =
+ container_of(hw, struct ixgbe_adapter, hw);
+
+ e_dev_err("Cannot configure Link Status Event mask, err = %d\n",
+ err);
+ }
+
return 0;
}
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.h
index 2cb76a3d30ae..ecda35a77adb 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.h
@@ -34,8 +34,7 @@ int ixgbe_update_link_info(struct ixgbe_hw *hw);
int ixgbe_get_link_status(struct ixgbe_hw *hw, bool *link_up);
int ixgbe_aci_get_link_info(struct ixgbe_hw *hw, bool ena_lse,
struct ixgbe_link_status *link);
-int ixgbe_aci_set_event_mask(struct ixgbe_hw *hw, u8 port_num, u16 mask);
-int ixgbe_configure_lse(struct ixgbe_hw *hw, bool activate, u16 mask);
+int ixgbe_configure_lse(struct ixgbe_hw *hw, bool activate);
int ixgbe_aci_set_port_id_led(struct ixgbe_hw *hw, bool orig_mode);
enum ixgbe_media_type ixgbe_get_media_type_e610(struct ixgbe_hw *hw);
int ixgbe_setup_link_e610(struct ixgbe_hw *hw, ixgbe_link_speed speed,
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 7a0783c1abc1..2c8968b9b86c 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -5969,23 +5969,14 @@ static void ixgbe_configure(struct ixgbe_adapter *adapter)
/**
* ixgbe_enable_link_status_events - enable link status events
* @adapter: pointer to the adapter structure
- * @mask: event mask to be set
*
* Enables link status events by invoking ixgbe_configure_lse()
*
* Return: the exit code of the operation.
*/
-static int ixgbe_enable_link_status_events(struct ixgbe_adapter *adapter,
- u16 mask)
+static int ixgbe_enable_link_status_events(struct ixgbe_adapter *adapter)
{
- int err;
-
- err = ixgbe_configure_lse(&adapter->hw, true, mask);
- if (err)
- return err;
-
- adapter->lse_mask = mask;
- return 0;
+ return ixgbe_configure_lse(&adapter->hw, true);
}
/**
@@ -5998,14 +5989,7 @@ static int ixgbe_enable_link_status_events(struct ixgbe_adapter *adapter,
*/
static int ixgbe_disable_link_status_events(struct ixgbe_adapter *adapter)
{
- int err;
-
- err = ixgbe_configure_lse(&adapter->hw, false, adapter->lse_mask);
- if (err)
- return err;
-
- adapter->lse_mask = 0;
- return 0;
+ return ixgbe_configure_lse(&adapter->hw, false);
}
/**
@@ -6039,10 +6023,6 @@ static int ixgbe_non_sfp_link_config(struct ixgbe_hw *hw)
{
struct ixgbe_adapter *adapter = container_of(hw, struct ixgbe_adapter,
hw);
- u16 mask = ~((u16)(IXGBE_ACI_LINK_EVENT_UPDOWN |
- IXGBE_ACI_LINK_EVENT_MEDIA_NA |
- IXGBE_ACI_LINK_EVENT_MODULE_QUAL_FAIL |
- IXGBE_ACI_LINK_EVENT_PHY_FW_LOAD_FAIL));
bool autoneg, link_up = false;
int ret = -EIO;
u32 speed;
@@ -6070,7 +6050,7 @@ static int ixgbe_non_sfp_link_config(struct ixgbe_hw *hw)
if (hw->mac.ops.setup_link) {
if (adapter->hw.mac.type == ixgbe_mac_e610) {
- ret = ixgbe_enable_link_status_events(adapter, mask);
+ ret = ixgbe_enable_link_status_events(adapter);
if (ret)
return ret;
}
--
2.31.1
^ permalink raw reply related
* [PATCH iwl-next v1 0/6] ixgbe: improve FW/SW data synchronization
From: Jedrzej Jagielski @ 2026-07-02 9:15 UTC (permalink / raw)
To: intel-wired-lan; +Cc: anthony.l.nguyen, netdev, Jedrzej Jagielski
There are some possible discrepancies between data shown by the driver
with the actual NIC/FW state.
First 4 patches address problem with link state correctness. In some
scenarios FW may not be able to notify the driver about link change.
Along with fix, refactor the whole approach to LSE (Link Status Events)
enablement. As there's no any reasonable argument for disabling LSE
during driver life cycle, make them constantly enabled.
5th and 6th patches address problem occurring when FW changes MAC address.
There's no any kind of notify/event accompanying that change. Therefore let the
driver to poll for the new address.
Jedrzej Jagielski (6):
ixgbe: E610: init Link Status Events mask just once
ixgbe: E610: prevent from disabling LSE
ixgbe: E610: do not disable LSE on driver down/remove
ixgbe: E610: re-enable LSE unconditionally
ixgbe: E610: add MAC address runtime refresh
ixgbe: take rtnl lock before ixgbe_reset() is called
drivers/net/ethernet/intel/ixgbe/ixgbe.h | 1 -
drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c | 49 ++++-----
drivers/net/ethernet/intel/ixgbe/ixgbe_e610.h | 4 +-
.../net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 2 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 100 +++++++-----------
5 files changed, 63 insertions(+), 93 deletions(-)
--
2.31.1
^ 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