* Re: [PATCH] net: stmmac: Use interrupt mode INTM=1 for per channel irq
From: Jakub Kicinski @ 2026-05-01 0:37 UTC (permalink / raw)
To: muhammad.nazim.amirul.nazle.asmade
Cc: Jakub Kicinski, netdev, davem, pabeni, edumazet, andrew+netdev,
linux-kernel
In-Reply-To: <20260429062428.18438-1-muhammad.nazim.amirul.nazle.asmade@altera.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: stmmac: Use interrupt mode INTM=1 for per channel irq
This commit adds programming of the XGMAC DMA_MODE.INTM=1 bit on xgmac2
to deliver per-channel TX/RX interrupts via shared peripheral interrupts
(SPI), mirroring the approach of the earlier MSI-oriented commit
6ccf12ae111e. New fields are added to stmmac_dma_cfg and
plat_stmmacenet_data to drive the new path.
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
> index 51943705a2b0..94cbf24b3118 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
> @@ -320,6 +320,9 @@
> /* DMA Registers */
> #define XGMAC_DMA_MODE 0x00003000
> #define XGMAC_SWR BIT(0)
> +#define DMA_MODE_INTM_MASK GENMASK(13, 12)
> +#define DMA_MODE_INTM_SHIFT 12
> +#define DMA_MODE_INTM_MODE1 0x1
Should these three macros carry the XGMAC_ prefix used by every other
symbol in dwxgmac2.h (XGMAC_DMA_MODE, XGMAC_SWR, XGMAC_DMA_SYSBUS_MODE,
and so on)? The dwmac4 sibling header defines analogous symbols as
DMA_BUS_MODE_INTM_MASK / DMA_BUS_MODE_INTM_MODE1, but within dwxgmac2.h
the established convention appears to be XGMAC_*.
The referenced precedent commit 6ccf12ae111e uses FIELD_PREP() /
u32_replace_bits() rather than a hand-rolled SHIFT constant. Would it
be cleaner to drop DMA_MODE_INTM_SHIFT and use FIELD_PREP() here too?
> #define XGMAC_DMA_SYSBUS_MODE 0x00003004
> #define XGMAC_WR_OSR_LMT GENMASK(29, 24)
> #define XGMAC_RD_OSR_LMT GENMASK(21, 16)
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
> index 03437f1cf3df..3789b62cd2e3 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
> @@ -31,6 +31,13 @@ static void dwxgmac2_dma_init(void __iomem *ioaddr,
> value |= XGMAC_EAME;
>
> writel(value, ioaddr + XGMAC_DMA_SYSBUS_MODE);
> +
> + if (dma_cfg->multi_irq_en) {
> + value = readl(ioaddr + XGMAC_DMA_MODE);
> + value &= ~DMA_MODE_INTM_MASK;
> + value |= (DMA_MODE_INTM_MODE1 << DMA_MODE_INTM_SHIFT);
> + writel(value, ioaddr + XGMAC_DMA_MODE);
> + }
> }
Is this new block reachable on any platform?
A tree-wide search for multi_irq_en returns only three references: this
read site and the two struct declarations added below. Nothing assigns
dma_cfg->multi_irq_en anywhere, so the condition appears to always be
false and INTM=1 would never be programmed.
For comparison, the analogous multi_msi_en is set up in stmmac_main.c:
priv->plat->dma_cfg->multi_msi_en =
(priv->plat->flags & STMMAC_FLAG_MULTI_MSI_EN);
and is consumed from glue drivers such as dwmac-intel.c, dwmac-loongson.c,
and dwmac-s32.c. Should an equivalent assignment and a corresponding
plat flag (or the new plat->multi_irq_en field) be wired up in this
patch so the feature actually takes effect?
The commit message also states:
The TX/RX NORMAL interrupt check is decoupled since the NIS bit is
not asserted for TI/RI events when INTM=1.
but the diff touches only dwxgmac2_dma_init() and does not modify
dwxgmac2_dma_interrupt() or any other ISR/interrupt-status path. The
existing dwxgmac2_dma_interrupt() already tests XGMAC_RI and
XGMAC_TI | XGMAC_TBU independently of XGMAC_NIS:
if (likely(intr_status & XGMAC_RI)) {
...
}
if (likely(intr_status & (XGMAC_TI | XGMAC_TBU))) {
...
}
Is this paragraph carried over from the earlier dwmac4 commit
6ccf12ae111e, or is a companion hunk missing from this patch?
> static void dwxgmac2_dma_init_chan(struct stmmac_priv *priv,
> diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
> index 4430b967abde..5deaa1a797a5 100644
> --- a/include/linux/stmmac.h
> +++ b/include/linux/stmmac.h
> @@ -122,6 +122,7 @@ struct stmmac_dma_cfg {
> bool eame;
> /* multi_msi_en: stmmac core internal */
> bool multi_msi_en;
> + bool multi_irq_en;
> /* atds: stmmac core internal */
> bool atds;
> };
> @@ -349,6 +350,11 @@ struct plat_stmmacenet_data {
> u8 vlan_fail_q;
> bool provide_bus_info;
> int int_snapshot_num;
> + int ext_snapshot_num;
> + bool int_snapshot_en;
> + bool ext_snapshot_en;
> + bool multi_msi_en;
> + bool multi_irq_en;
> int msi_mac_vec;
> int msi_wol_vec;
> int msi_sfty_ce_vec;
Are these five new plat_stmmacenet_data fields used by anything?
A tree-wide search finds no reader or writer for ext_snapshot_num,
int_snapshot_en, ext_snapshot_en, plat->multi_msi_en, or
plat->multi_irq_en, and they are not mentioned in the commit message.
The commit message scope is limited to enabling INTM=1 per-channel SPI
IRQs, which would only require stmmac_dma_cfg::multi_irq_en.
The same concepts already appear to be represented via plat->flags bits
(STMMAC_FLAG_MULTI_MSI_EN, STMMAC_FLAG_INT_SNAPSHOT_EN,
STMMAC_FLAG_EXT_SNAPSHOT_EN) and the existing int_snapshot_num member.
Could the four unrelated fields be dropped from this patch and, if
still needed, be introduced in a separate series that actually wires
them up? Having both a flags bit and a bool for the same concept
risks future drift about which representation is authoritative.
--
pw-bot: cr
^ permalink raw reply
* Re: [PATCH net v5] ipv6: Implement limits on extension header parsing
From: patchwork-bot+netdevbpf @ 2026-05-01 0:40 UTC (permalink / raw)
To: Daniel Borkmann
Cc: kuba, edumazet, dsahern, tom, willemdebruijn.kernel, idosch,
pabeni, justin.iurman, netdev
In-Reply-To: <20260429154648.809751-1-daniel@iogearbox.net>
Hello:
This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Wed, 29 Apr 2026 17:46:48 +0200 you wrote:
> ipv6_{skip_exthdr,find_hdr}() and ip6_{tnl_parse_tlv_enc_lim,
> protocol_deliver_rcu}() iterate over IPv6 extension headers until they
> find a non-extension-header protocol or run out of packet data. The
> loops have no iteration counter, relying solely on the packet length
> to bound them. For a crafted packet with 8-byte extension headers
> filling a 64KB jumbogram, this means a worst case of up to ~8k
> iterations with a skb_header_pointer call each. ipv6_skip_exthdr(),
> for example, is used where it parses the inner quoted packet inside
> an incoming ICMPv6 error:
>
> [...]
Here is the summary with links:
- [net,v5] ipv6: Implement limits on extension header parsing
https://git.kernel.org/netdev/net/c/3744b0964d52
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] net: phy: micrel: fix LAN8814 QSGMII soft reset
From: patchwork-bot+netdevbpf @ 2026-05-01 0:40 UTC (permalink / raw)
To: Robert Marko
Cc: andrew, hkallweit1, linux, davem, edumazet, kuba, pabeni,
Divya.Koppera, horatiu.vultur, netdev, linux-kernel
In-Reply-To: <20260428134138.1741253-1-robert.marko@sartura.hr>
Hello:
This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Tue, 28 Apr 2026 15:41:01 +0200 you wrote:
> LAN8814 QSGMII soft reset was moved into the probe function to avoid
> triggering it for each of 4 PHY-s in the package.
>
> However, that broke QSGMII link between the MAC and PHY on most LAN8814
> PHY-s, specificaly for us on the Microchip LAN969x switch.
> Reading the QSGMII status registers it was visible that lanes were only
> partially synced.
>
> [...]
Here is the summary with links:
- [net] net: phy: micrel: fix LAN8814 QSGMII soft reset
https://git.kernel.org/netdev/net/c/e027c218c482
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] net/smc: cap allocation order for SMC-R physically contiguous buffers
From: patchwork-bot+netdevbpf @ 2026-05-01 0:40 UTC (permalink / raw)
To: D. Wythe
Cc: davem, dust.li, edumazet, kuba, pabeni, sidraya, wenjia, mjambigi,
horms, tonylu, guwen, linux-kernel, linux-rdma, linux-s390,
netdev, oliver.yang, pasic
In-Reply-To: <20260429021637.21815-1-alibuda@linux.alibaba.com>
Hello:
This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Wed, 29 Apr 2026 10:16:37 +0800 you wrote:
> The alloc_pages() cannot satisfy requests exceeding MAX_PAGE_ORDER,
> and attempting such allocations will lead to guaranteed failures
> and potential kernel warnings.
>
> For SMCR_PHYS_CONT_BUFS, cap the allocation order to MAX_PAGE_ORDER.
> This ensures the attempts to allocate the largest possible physically
> contiguous chunk succeed, instead of failing with an invalid order.
> This also avoids redundant "try-fail-degrade" cycles in
> __smc_buf_create().
>
> [...]
Here is the summary with links:
- [net-next,v3] net/smc: cap allocation order for SMC-R physically contiguous buffers
https://git.kernel.org/netdev/net-next/c/4cc5130ee84a
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 5.15.y] tipc: fix kernel warning when sending SYN message
From: Sasha Levin @ 2026-05-01 0:53 UTC (permalink / raw)
To: stable, Tung Nguyen
Cc: Jakub Kicinski, Jon Maloy, David S . Miller, Robert Garcia,
Al Viro, netdev, tipc-discussion, linux-kernel
In-Reply-To: <20260429075033.234885-1-rob_garcia@163.com>
On Wed, Apr 29, 2026 at 03:50:33PM +0800, Robert Garcia wrote:
> From: Tung Nguyen <tung.q.nguyen@dektech.com.au>
>
> [ Upstream commit 11a4d6f67cf55883dc78e31c247d1903ed7feccc ]
>
> When sending a SYN message, this kernel stack trace is observed:
[...]
> It is because commit a41dad905e5a ("iov_iter: saner checks for attempt
> to copy to/from iterator") has introduced sanity check for copying
> from/to iov iterator. Lacking of copy direction from the iterator
> viewpoint would lead to kernel stack trace like above.
Thanks for the submission, but I don't think we need this in 5.15.y.
The WARN this fix silences was added by a41dad905e5a ("iov_iter: saner
checks for attempt to copy to/from iterator"), which landed in v6.1 and
has never been backported to 5.15.y. Without that prerequisite the WARN
isn't reachable in 5.15 — the SYN/ACK sends call copy_from_iter_full()
with dsz=0 on a zero-initialized msghdr, and 5.15's _copy_from_iter()
only WARNs for ITER_PIPE iters, not on data_source.
Your ITER_SOURCE -> WRITE adaptation is correct, but since the bug is
absent in 5.15 the patch isn't needed there.
--
Thanks,
Sasha
^ permalink raw reply
* Re: [PATCH net-next 07/11] net: devmem: support TX over NETMEM_TX_NO_DMA devices
From: Jakub Kicinski @ 2026-05-01 0:57 UTC (permalink / raw)
To: Bobby Eshleman
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni,
Simon Horman, Jonathan Corbet, Shuah Khan, Alex Shi, Yanteng Si,
Dongliang Mu, Michael Chan, Pavan Chebbi, Joshua Washington,
Harshitha Ramamurthy, Saeed Mahameed, Tariq Toukan, Mark Bloch,
Leon Romanovsky, Alexander Duyck, kernel-team, Daniel Borkmann,
Nikolay Aleksandrov, Shuah Khan, netdev, linux-doc, linux-kernel,
linux-rdma, bpf, linux-kselftest, Stanislav Fomichev,
Mina Almasry, Bobby Eshleman
In-Reply-To: <20260428-tcp-dm-netkit-v1-7-719280eba4d2@meta.com>
On Tue, 28 Apr 2026 15:42:04 -0700 Bobby Eshleman wrote:
> shinfo = skb_shinfo(skb);
> + if (shinfo->nr_frags == 0)
> + goto out;
Feels tempting to cover the NETMEM_TX_NO_DMA / NETMEM_TX_NONE
cases here before we even look at the frags?
> - if (shinfo->nr_frags > 0) {
> - niov = netmem_to_net_iov(skb_frag_netmem(&shinfo->frags[0]));
> - if (net_is_devmem_iov(niov) &&
> - READ_ONCE(net_devmem_iov_binding(niov)->dev) != dev)
> + niov = netmem_to_net_iov(skb_frag_netmem(&shinfo->frags[0]));
> + if (!net_is_devmem_iov(niov))
> + goto out;
> +
> + binding = net_devmem_iov_binding(niov);
> +
> + switch (dev->netmem_tx) {
> + case NETMEM_TX_DMA:
> + if (READ_ONCE(binding->dev) != dev)
> goto out_free;
> + break;
> + case NETMEM_TX_NO_DMA:
> + break;
> + default: /* NETMEM_TX_NONE */
> + goto out_free;
> }
^ permalink raw reply
* Re: [PATCH net-next 00/11] net: devmem: support devmem with netkit devices
From: Jakub Kicinski @ 2026-05-01 0:59 UTC (permalink / raw)
To: Bobby Eshleman
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni,
Simon Horman, Jonathan Corbet, Shuah Khan, Alex Shi, Yanteng Si,
Dongliang Mu, Michael Chan, Pavan Chebbi, Joshua Washington,
Harshitha Ramamurthy, Saeed Mahameed, Tariq Toukan, Mark Bloch,
Leon Romanovsky, Alexander Duyck, kernel-team, Daniel Borkmann,
Nikolay Aleksandrov, Shuah Khan, netdev, linux-doc, linux-kernel,
linux-rdma, bpf, linux-kselftest, Stanislav Fomichev,
Mina Almasry, Bobby Eshleman
In-Reply-To: <20260428-tcp-dm-netkit-v1-0-719280eba4d2@meta.com>
On Tue, 28 Apr 2026 15:41:57 -0700 Bobby Eshleman wrote:
> net: add netmem_tx modes that indicate dma capability
> net: bnxt: convert netmem_tx from bool to NETMEM_TX_DMA enum
> gve: convert netmem_tx from bool to NETMEM_TX_DMA enum
> net/mlx5e: convert netmem_tx from bool to NETMEM_TX_DMA enum
> eth: fbnic: convert netmem_tx from bool to NETMEM_TX_DMA enum
> netkit: set NETMEM_TX_NO_DMA for unreadable skb passthrough
> net: devmem: support TX over NETMEM_TX_NO_DMA devices
I think it looks reasonable over all, but the assumption that rx lease
implies tx queue does not seem great. Sounds like Daniel has that part
covered tho :)
When you post v2 - you can squash the driver patches into patch 1.
^ permalink raw reply
* Re: [PATCH net-next 00/11] net: devmem: support devmem with netkit devices
From: Bobby Eshleman @ 2026-05-01 1:04 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni,
Simon Horman, Jonathan Corbet, Shuah Khan, Alex Shi, Yanteng Si,
Dongliang Mu, Michael Chan, Pavan Chebbi, Joshua Washington,
Harshitha Ramamurthy, Saeed Mahameed, Tariq Toukan, Mark Bloch,
Leon Romanovsky, Alexander Duyck, kernel-team, Daniel Borkmann,
Nikolay Aleksandrov, Shuah Khan, netdev, linux-doc, linux-kernel,
linux-rdma, bpf, linux-kselftest, Stanislav Fomichev,
Mina Almasry, Bobby Eshleman
In-Reply-To: <20260430175945.476734ca@kernel.org>
On Thu, Apr 30, 2026 at 05:59:45PM -0700, Jakub Kicinski wrote:
> On Tue, 28 Apr 2026 15:41:57 -0700 Bobby Eshleman wrote:
> > net: add netmem_tx modes that indicate dma capability
> > net: bnxt: convert netmem_tx from bool to NETMEM_TX_DMA enum
> > gve: convert netmem_tx from bool to NETMEM_TX_DMA enum
> > net/mlx5e: convert netmem_tx from bool to NETMEM_TX_DMA enum
> > eth: fbnic: convert netmem_tx from bool to NETMEM_TX_DMA enum
> > netkit: set NETMEM_TX_NO_DMA for unreadable skb passthrough
> > net: devmem: support TX over NETMEM_TX_NO_DMA devices
>
> I think it looks reasonable over all, but the assumption that rx lease
> implies tx queue does not seem great. Sounds like Daniel has that part
> covered tho :)
Indeed, with TX leasing this becomes much nicer.
>
> When you post v2 - you can squash the driver patches into patch 1.
Will do!
Best,
Bobby
^ permalink raw reply
* Re: [PATCH net-next 3/6] pds_core: add PLDM firmware update support via devlink flash
From: Jakub Kicinski @ 2026-05-01 1:05 UTC (permalink / raw)
To: Nikhil P. Rao
Cc: Brett Creeley, Andrew Lunn, David S. Miller, Eric Dumazet,
Paolo Abeni, Kees Cook, Gustavo A. R. Silva, netdev, linux-kernel,
linux-hardening, eric.joyner
In-Reply-To: <20260429-b4-pldm-b4-v1-3-394fafba526f@amd.com>
On Wed, 29 Apr 2026 08:28:19 +0000 Nikhil P. Rao wrote:
> +#define PDS_CORE_FW_COMPONENT_LIST_LEN ((PDS_PAGE_SIZE - \
> + sizeof(struct pds_core_component_list_info)) / \
> + sizeof(struct pds_core_fw_component_info))
> +
> +#if defined(__has_attribute) && !__has_attribute(__counted_by__)
> +#define __counted_by(member)
> +#endif
Please don't redefined kernel-level primitives.
It's a huge pain in the rear to deal with when indexing the code.
This patch also adds a bunch of kdoc warnings.
Last but not least Sashiko points out a number of bugs
^ permalink raw reply
* Re: [PATCH net-next 07/11] net: devmem: support TX over NETMEM_TX_NO_DMA devices
From: Bobby Eshleman @ 2026-05-01 1:07 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni,
Simon Horman, Jonathan Corbet, Shuah Khan, Alex Shi, Yanteng Si,
Dongliang Mu, Michael Chan, Pavan Chebbi, Joshua Washington,
Harshitha Ramamurthy, Saeed Mahameed, Tariq Toukan, Mark Bloch,
Leon Romanovsky, Alexander Duyck, kernel-team, Daniel Borkmann,
Nikolay Aleksandrov, Shuah Khan, netdev, linux-doc, linux-kernel,
linux-rdma, bpf, linux-kselftest, Stanislav Fomichev,
Mina Almasry, Bobby Eshleman
In-Reply-To: <20260430175724.0c134a0d@kernel.org>
On Thu, Apr 30, 2026 at 05:57:24PM -0700, Jakub Kicinski wrote:
> On Tue, 28 Apr 2026 15:42:04 -0700 Bobby Eshleman wrote:
> > shinfo = skb_shinfo(skb);
> > + if (shinfo->nr_frags == 0)
> > + goto out;
>
> Feels tempting to cover the NETMEM_TX_NO_DMA / NETMEM_TX_NONE
> cases here before we even look at the frags?
That sounds good to me (had considered it, but opted out cause I felt it
might look odd with the switch-case that follows). And I'll address the
bug(s) the model called out here/elsewhere too.
Thanks,
Bobby
>
> > - if (shinfo->nr_frags > 0) {
> > - niov = netmem_to_net_iov(skb_frag_netmem(&shinfo->frags[0]));
> > - if (net_is_devmem_iov(niov) &&
> > - READ_ONCE(net_devmem_iov_binding(niov)->dev) != dev)
> > + niov = netmem_to_net_iov(skb_frag_netmem(&shinfo->frags[0]));
> > + if (!net_is_devmem_iov(niov))
> > + goto out;
> > +
> > + binding = net_devmem_iov_binding(niov);
> > +
> > + switch (dev->netmem_tx) {
> > + case NETMEM_TX_DMA:
> > + if (READ_ONCE(binding->dev) != dev)
> > goto out_free;
> > + break;
> > + case NETMEM_TX_NO_DMA:
> > + break;
> > + default: /* NETMEM_TX_NONE */
> > + goto out_free;
> > }
^ permalink raw reply
* Re: [PATCH net 1/2] net: libwx: fix VF illegal register access
From: Jakub Kicinski @ 2026-05-01 1:07 UTC (permalink / raw)
To: Jiawen Wu
Cc: netdev, Mengyuan Lou, Andrew Lunn, David S. Miller, Eric Dumazet,
Paolo Abeni, Simon Horman, Kees Cook, stable
In-Reply-To: <4D1F4452D21DE107+20260429083743.88961-1-jiawenwu@trustnetic.com>
On Wed, 29 Apr 2026 16:37:42 +0800 Jiawen Wu wrote:
> Register WX_CFG_PORT_ST is a PF restricted register. When a VF is
> initialized, attempting to read this register triggers an illegal
> register access, which lead to a system hang.
in the future please make sure that when you submit a series the
patches form a thread
^ permalink raw reply
* [PATCH net 1/1] net/rds: handle zerocopy send cleanup before the message is queued
From: Ren Wei @ 2026-05-01 1:08 UTC (permalink / raw)
To: netdev, linux-rdma, rds-devel
Cc: achender, davem, edumazet, kuba, pabeni, horms, santosh.shilimkar,
sowmini.varadhan, willemb, yuantan098, yifanwucs, tomapufckgml,
bird, lx24, tonanli66, n05ec
In-Reply-To: <cover.1777550074.git.tonanli66@gmail.com>
From: Nan Li <tonanli66@gmail.com>
A zerocopy send can fail after user pages have been pinned but before
the message is attached to the sending socket.
The purge path currently infers zerocopy state from rm->m_rs, so an
unqueued message can be cleaned up as if it owned normal payload pages.
However, zerocopy ownership is really determined by the presence of
op_mmp_znotifier, regardless of whether the message has reached the
socket queue.
Capture op_mmp_znotifier up front in rds_message_purge() and use it as
the cleanup discriminator. If the message is already associated with a
socket, keep the existing completion path. Otherwise, drop the pinned
page accounting directly and release the notifier before putting the
payload pages.
This keeps early send failure cleanup consistent with the zerocopy
lifetime rules without changing the normal queued completion path.
Fixes: 0cebaccef3ac ("rds: zerocopy Tx support.")
Cc: stable@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>
Co-developed-by: Xiao Liu <lx24@stu.ynu.edu.cn>
Signed-off-by: Xiao Liu <lx24@stu.ynu.edu.cn>
Signed-off-by: Nan Li <tonanli66@gmail.com>
Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
---
net/rds/message.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/net/rds/message.c b/net/rds/message.c
index eaa6f22601a4..25fedcb3cd00 100644
--- a/net/rds/message.c
+++ b/net/rds/message.c
@@ -131,24 +131,34 @@ static void rds_rm_zerocopy_callback(struct rds_sock *rs,
*/
static void rds_message_purge(struct rds_message *rm)
{
+ struct rds_znotifier *znotifier;
unsigned long i, flags;
- bool zcopy = false;
+ bool zcopy;
if (unlikely(test_bit(RDS_MSG_PAGEVEC, &rm->m_flags)))
return;
spin_lock_irqsave(&rm->m_rs_lock, flags);
+ znotifier = rm->data.op_mmp_znotifier;
+ rm->data.op_mmp_znotifier = NULL;
+ zcopy = !!znotifier;
+
if (rm->m_rs) {
struct rds_sock *rs = rm->m_rs;
- if (rm->data.op_mmp_znotifier) {
- zcopy = true;
- rds_rm_zerocopy_callback(rs, rm->data.op_mmp_znotifier);
+ if (znotifier) {
+ rds_rm_zerocopy_callback(rs, znotifier);
rds_wake_sk_sleep(rs);
- rm->data.op_mmp_znotifier = NULL;
}
sock_put(rds_rs_to_sk(rs));
rm->m_rs = NULL;
+ } else if (znotifier) {
+ /*
+ * Zerocopy can fail before the message is queued on the
+ * socket, so there is no rs to carry the notification.
+ */
+ mm_unaccount_pinned_pages(&znotifier->z_mmp);
+ kfree(rds_info_from_znotifier(znotifier));
}
spin_unlock_irqrestore(&rm->m_rs_lock, flags);
--
2.43.0
^ permalink raw reply related
* Re: [PATCH net-next 0/2] bridge: Do not suppress ARP probes and DAD NS unconditionally
From: patchwork-bot+netdevbpf @ 2026-05-01 1:10 UTC (permalink / raw)
To: Danielle Ratson
Cc: netdev, razor, idosch, davem, edumazet, kuba, pabeni, horms,
shuah, bridge, linux-kernel, linux-kselftest
In-Reply-To: <20260429062405.1386417-1-danieller@nvidia.com>
Hello:
This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Wed, 29 Apr 2026 09:24:03 +0300 you wrote:
> When using bridge neighbor suppression in EVPN deployments, Duplicate
> Address Detection (DAD) is currently broken for both IPv4 (ARP probes)
> and IPv6 (DAD Neighbor Solicitations). This prevents proper address
> conflict detection across the VXLAN fabric.
>
> The neighbor suppression feature allows the bridge to reply to ARP/NS
> messages on behalf of remote hosts when FDB and neighbor entries exist,
> suppressing unnecessary flooding over the VXLAN overlay. However, the
> current implementation unconditionally suppresses ARP probes and DAD NS,
> which breaks DAD.
>
> [...]
Here is the summary with links:
- [net-next,1/2] bridge: Do not suppress ARP probes and DAD NS unconditionally
https://git.kernel.org/netdev/net-next/c/fee1fc1d5a54
- [net-next,2/2] selftests: net: Add tests for ARP probe and DAD NS handling
https://git.kernel.org/netdev/net-next/c/a3f88d89f698
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 v2] net: airoha: Introduce airoha_fe_get()/airoha_qdma_get() register read helpers
From: Jakub Kicinski @ 2026-05-01 1:11 UTC (permalink / raw)
To: Lorenzo Bianconi
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni,
Simon Horman, linux-arm-kernel, linux-mediatek, netdev
In-Reply-To: <20260429-airoha_fe_get-airoha_qdma_get-v2-1-909e791f1dc0@kernel.org>
On Wed, 29 Apr 2026 12:54:32 +0200 Lorenzo Bianconi wrote:
> Add airoha_fe_get() and airoha_qdma_get() as utility routines for reading
> a masked field from a specified register.
> This is a non-functional refactor, no logical changes are introduced to
> the existing codebase.
Doesn't apply AFAICT.
If this is because of the net -> net-next merge - please avoid posting
patches which will cause a conflict on merge.
^ permalink raw reply
* Re: [Intel-wired-lan] [PATCH iwl-next v3] libie: log more info when virtchnl fails
From: Li Li @ 2026-05-01 1:14 UTC (permalink / raw)
To: Loktionov, Aleksandr
Cc: Nguyen, Anthony L, Kitszel, Przemyslaw, David S. Miller,
Jakub Kicinski, Eric Dumazet, intel-wired-lan@lists.osuosl.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
David Decotigny, Singhai, Anjali, Samudrala, Sridhar,
Brian Vazquez, Tantilov, Emil S
In-Reply-To: <IA3PR11MB8986449FB3714E23C389F6D1E5352@IA3PR11MB8986.namprd11.prod.outlook.com>
On Thu, Apr 30, 2026 at 1:36 AM Loktionov, Aleksandr
<aleksandr.loktionov@intel.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> > Of Li Li via Intel-wired-lan
> > Sent: Wednesday, April 29, 2026 10:41 PM
> > To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel,
> > Przemyslaw <przemyslaw.kitszel@intel.com>; David S. Miller
> > <davem@davemloft.net>; Jakub Kicinski <kuba@kernel.org>; Eric Dumazet
> > <edumazet@google.com>; intel-wired-lan@lists.osuosl.org
> > Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; David
> > Decotigny <decot@google.com>; Singhai, Anjali
> > <anjali.singhai@intel.com>; Samudrala, Sridhar
> > <sridhar.samudrala@intel.com>; Brian Vazquez <brianvv@google.com>; Li
> > Li <boolli@google.com>; Tantilov, Emil S <emil.s.tantilov@intel.com>
> > Subject: [Intel-wired-lan] [PATCH iwl-next v3] libie: log more info
> > when virtchnl fails
> >
> > Virtchnl failures can be hard to debug without logs. Logging the
> > details of virtchnl transactions can be useful for debugging virtchnl-
> > related issues.
> >
> > Tested: Built & booted on a test machine and synthetically produced a
> > virtual failure to produce the following log:
> >
> > idpf 0000:01:00.0: Non-zero virtchnl ret val (msg op: 1, ret val: 6,
> > data_len: 8); xn id: 0, cookie: 0
> > idpf 0000:01:00.0: Transaction failed (op 1, xn state:
> > 3, id: 0, cookie: 0, size: 8)
> >
> > Signed-off-by: Li Li <boolli@google.com>
> > ---
> > v3:
> > - Use dev_err_ratelimited in both logs.
> > - Move log placement to after virtchnl field validation.
> > - Remove redundant op/cookie fields since they were validated.
> > v2:
> > - Use dev_warn_ratelimited instead of dev_notice_ratelimited based on
> > reviewer feedback.
> >
> > drivers/net/ethernet/intel/libie/controlq.c | 13 +++++++++++++
> > 1 file changed, 13 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/intel/libie/controlq.c
> > b/drivers/net/ethernet/intel/libie/controlq.c
> > index ebc05355e39d..ceca8a076d79 100644
> > --- a/drivers/net/ethernet/intel/libie/controlq.c
> > +++ b/drivers/net/ethernet/intel/libie/controlq.c
> > @@ -766,6 +766,14 @@ libie_ctlq_xn_process_recv(struct
> > libie_ctlq_xn_recv_params *params,
> > msg_cookie != xn->cookie)
> > return false;
> >
> > + if (ctlq_msg->chnl_retval) {
> > + dev_err_ratelimited(
> > + params->ctlq->dev,
> > + "Non-zero virtchnl ret val (msg op: %u, ret val:
> > %u, data_len: %u); xn id: %u, cookie: %u\n",
> > + ctlq_msg->chnl_opcode, ctlq_msg->chnl_retval,
> > + ctlq_msg->data_len, xn->index, xn->cookie);
> 'virtchnl ret val' 'ret val:' looks like a duplication in dmesg.
Let me send out v4 to simplify it.
>
>
> > + }
> > +
> > spin_lock(&xn->xn_lock);
> > if (xn->state != LIBIE_CTLQ_XN_ASYNC &&
> > xn->state != LIBIE_CTLQ_XN_WAITING) { @@ -1011,6 +1019,11
> > @@ int libie_ctlq_xn_send(struct libie_ctlq_xn_send_params *params)
> > params->recv_mem = xn->recv_mem;
> > break;
> > default:
> > + dev_err_ratelimited(
> > + params->ctlq->dev,
> > + "Transaction failed (op %u, xn state: %d, id: %u,
> > cookie: %u, size: %zu)\n",
> > + params->chnl_opcode, xn->state, xn->index, xn-
> > >cookie,
> > + xn->recv_mem.iov_len);
> Probably %u fits better for enums than %d, what do you think?
Sure, let me use %u in v4.
>
> > ret = -EBADMSG;
> > break;
> > }
> > --
> > 2.54.0.545.g6539524ca2-goog
>
^ permalink raw reply
* Re: [PATCH net-next v2 1/4] r8152: Add support for 10Gbit Link Speeds and EEE
From: Jakub Kicinski @ 2026-05-01 1:15 UTC (permalink / raw)
To: mail
Cc: Jakub Kicinski, andrew+netdev, davem, edumazet, pabeni, linux-usb,
netdev, linux-kernel, hsu.chih.kai
In-Reply-To: <20260429-rtl8159_net_next-v2-1-bab3cd4e4c66@birger-koblitz.de>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
r8152: Add support for 10Gbit Link Speeds and EEE
This change adds support for 10 Gbit link speeds on RTL8159 via the
ethtool link_ksettings interface and adds 10 Gbit EEE advertisement
handling alongside the existing 2.5G/5G paths.
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index 7337bf1b7d6a..01e65d845f87 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
[ ... ]
> @@ -5773,6 +5779,11 @@ static void r8156_eee_en(struct r8152 *tp, bool enable)
> else
> config &= ~MDIO_EEE_5GT;
>
> + if (enable && (tp->eee_adv2 & MDIO_EEE_10GT))
> + config |= MDIO_EEE_10GT;
> + else
> + config &= ~MDIO_EEE_10GT;
> +
> ocp_reg_write(tp, OCP_EEE_ADV2, config);
> }
Is MDIO_EEE_10GT actually a bit of the EEE advertisement 2 register
(7.62, cap2 / OCP_EEE_ADV2)?
MDIO_EEE_10GT is defined by IEEE 802.3 as a bit of the EEE advertisement
register 7.60 (cap1 / OCP_EEE_ADV), not of register 7.62. The existing
10G EEE advertisement already flows through tp->eee_adv (populated by
linkmode_to_mii_eee_cap1_t() in r8152_set_eee()) and is written to
OCP_EEE_ADV by rtl_eee_enable().
tp->eee_adv2 is populated only by linkmode_to_mii_eee_cap2_t() in
r8152_set_eee(), which covers 2500baseT and 5000baseT and cannot set
MDIO_EEE_10GT, plus the chip-init assignments in rtl_ops_init() which
also do not set this bit.
With that in mind, is the enable && (tp->eee_adv2 & MDIO_EEE_10GT)
condition ever true? The else branch will then unconditionally clear
bit 3 of OCP_EEE_ADV2 on every call, which is a cap1 bit being written
into the cap2 register.
[ ... ]
> @@ -8968,6 +9008,13 @@ static int r8153_get_eee(struct r8152 *tp, struct ethtool_keee *eee)
> linkmode_set_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT, common);
> }
>
> + if (tp->support_10000full) {
> + linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT, eee->supported);
> +
> + if (speed & _10000bps)
> + linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT, common);
> + }
> +
> eee->eee_enabled = tp->eee_en;
Should this hunk also extend the earlier gating inside r8153_get_eee()?
A few lines above, the cap2 register reads are still gated on:
if (tp->support_2500full || tp->support_5000full) {
val = ocp_reg_read(tp, OCP_EEE_ADV2);
mii_eee_cap2_mod_linkmode_adv_t(eee->advertised, val);
val = ocp_reg_read(tp, OCP_EEE_LPABLE2);
mii_eee_cap2_mod_linkmode_adv_t(eee->lp_advertised, val);
}
The equivalent disjunction in rtl8152_set_speed() and
rtl8152_get_link_ksettings() was updated to include support_10000full,
but r8153_get_eee() (and r8152_set_eee() at the analogous spot) still
only checks support_2500full || support_5000full.
A hypothetical chip that sets only support_10000full would bypass the
OCP_EEE_ADV2 / OCP_EEE_LPABLE2 reads and never reflect 10G EEE into
eee->advertised. Should the condition be made consistent with the
other sites?
> @@ -9982,6 +10029,11 @@ static int rtl8152_probe_once(struct usb_interface *intf,
> tp->speed = SPEED_5000;
> tp->advertising |= RTL_ADVERTISED_5000_FULL;
> }
> + if (tp->support_10000full &&
> + tp->udev->speed >= USB_SPEED_SUPER) {
> + tp->speed = SPEED_10000;
> + tp->advertising |= RTL_ADVERTISED_10000_FULL;
> + }
> tp->advertising |= RTL_ADVERTISED_1000_FULL;
> }
> tp->duplex = DUPLEX_FULL;
Is USB_SPEED_SUPER the correct gate for defaulting to a 10 Gbit link?
USB_SPEED_SUPER is USB 3.0 SuperSpeed at 5 Gbit/s signaling, which
cannot carry a 10 Gbit Ethernet link. The minimum bus rate that can
sustain 10GBASE-T is USB_SPEED_SUPER_PLUS (USB 3.1 Gen 2, 10 Gbit/s).
With this check, a 10G-capable RTL8159 plugged into a USB 3.0 host
will still default to tp->speed = SPEED_10000 and advertise
RTL_ADVERTISED_10000_FULL, so the Ethernet link may negotiate at 10G
while the USB bus is bottlenecked well below that, leading to
misleading ethtool speed reporting and back-pressure on the bus.
The same >= USB_SPEED_SUPER gate is reused from the 2.5G and 5G
defaults, which fit inside the USB 3.0 envelope; should the 10G
default instead use >= USB_SPEED_SUPER_PLUS?
--
pw-bot: cr
^ permalink raw reply
* Re: [PATCH net-next v2 2/4] r8152: Add support for the RTL8159 chip
From: Jakub Kicinski @ 2026-05-01 1:15 UTC (permalink / raw)
To: mail
Cc: Jakub Kicinski, andrew+netdev, davem, edumazet, pabeni, linux-usb,
netdev, linux-kernel, hsu.chih.kai
In-Reply-To: <20260429-rtl8159_net_next-v2-2-bab3cd4e4c66@birger-koblitz.de>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
r8152: Add support for the RTL8159 chip
This patch adds support for the Realtek RTL8159 as RTL_VER_17, reusing
large portions of the existing RTL8157 (RTL_VER_16) initialization and
USB power management paths. The bulk of the new code configures the
RTL8159 PHY inside r8157_hw_phy_cfg(), and a new USB product id 0x815a
is added to the device table.
> r8152: Add support for the RTL8159 chip
>
> The RTL8157 re-uses the packet descriptor format introduced with the
> RTL8157 and other hardware features of the RTL8157 (RTL_VER_16) such
> as the SRAM access. The support therefore consists in expanding the
> existing RTL8157 code for initialization and USB power management
> to also be used for the RTL8159 (RTL_VER_17).
The commit subject says RTL8159 is being added, but the body reads
"The RTL8157 re-uses the packet descriptor format introduced with the
RTL8157 ..." and later "Add support for the USB device ID of Realtek
RTL8157-based adapters." Should these occurrences of RTL8157 actually
be RTL8159?
> Most of the addiitonal code is added in r8157_hw_phy_cfg() to configure
^^^^^^^^^^
Typo: addiitonal.
> Add support for the USB device ID of Realtek RTL8157-based adapters. Detect
> the RTL8159 as RTL_VER_17 and set it up.
The new entry in rtl8152_table is 0x815a, which breaks the earlier
0x815N-for-RTL815N naming convention (0x8157 for RTL8157, etc.). Could
the commit message explain why the RTL8159 ships with product id
0x815a?
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index 01e65d845f87..2a07dde289e2 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
[ ... ]
> @@ -6894,7 +6903,7 @@ static void rtl8156_down(struct r8152 *tp)
> PLA_MCU_SPDWN_EN);
>
> r8153b_u1u2en(tp, false);
> - if (tp->version != RTL_VER_16) {
> + if (tp->version < RTL_VER_16) {
> r8153_u2p3en(tp, false);
> r8153b_power_cut_en(tp, false);
> }
rtl8156_down() is converted from "!= RTL_VER_16" to "< RTL_VER_16" so
that RTL_VER_17 takes the same path as RTL_VER_16. rtl8156_up() is
wired as ops->up for RTL_VER_17 in rtl_ops_init() but still uses
"!= RTL_VER_16" / "== RTL_VER_16" checks that were not converted.
In rtl8156_up():
r8153b_u1u2en(tp, false);
if (tp->version != RTL_VER_16)
r8153_u2p3en(tp, false);
...
if (tp->version == RTL_VER_16)
ocp_word_clr_bits(tp, MCU_TYPE_PLA, PLA_RCR1, BIT(3));
...
if (tp->version != RTL_VER_16)
ocp_word_clr_bits(tp, MCU_TYPE_USB, USB_SPEED_OPTION,
RG_PWRDN_EN | ALL_SPEED_OFF);
...
r8153_aldps_en(tp, true);
if (tp->version != RTL_VER_16)
r8153_u2p3en(tp, true);
if (tp->version != RTL_VER_16 && tp->udev->speed >= USB_SPEED_SUPER)
r8153b_u1u2en(tp, true);
Should these checks also be "< RTL_VER_16" for RTL_VER_17?
On RTL_VER_17 the up path will call r8153_u2p3en() (USB_U2P3_CTRL)
while r8156_init() and rtl8157_runtime_enable() route through
r8157_u2p3en() (USB_U2P3_V2_CTRL) - is that intentional, or should
RTL_VER_17 go through r8157_u2p3en() on up/down too?
Similarly, is the PLA_RCR1 BIT(3) clear needed on RTL_VER_17, and is
the re-enabling of r8153b_u1u2en(true) on up intended for the new
chip given the init path deliberately skips it for RTL_VER_16?
[ ... ]
> @@ -8140,6 +8149,190 @@ static void r8157_hw_phy_cfg(struct r8152 *tp)
> sram2_write_w0w1(tp, 0x807c, 0xff00, 0x5000);
> sram2_write_w0w1(tp, 0x809d, 0xff00, 0x5000);
> break;
> +
> + case RTL_VER_17:
[ ... snipped, PHY register table ... ]
> + break;
> +
> default:
> break;
> }
> @@ -8175,6 +8368,18 @@ static void r8157_hw_phy_cfg(struct r8152 *tp)
> set_bit(PHY_RESET, &tp->flags);
> }
>
> +static int r8159_wait_backup_restore(struct r8152 *tp)
> +{
> + u32 ocp_data;
> +
> + ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_MISC_0);
> + if (!(ocp_data & PCUT_STATUS))
> + return 0;
> +
> + return poll_timeout_us(ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_GPHY_CTRL),
> + ocp_data & BACKUP_RESTRORE, 200, 2000, false);
> +}
The rest of the driver organizes helpers by chip family (r8152_,
r8153_, r8156_, r8157_) and the commit message describes the RTL8159
as reusing the RTL8157 scaffolding. Is the r8159_ prefix on this
single helper intentional, or would r8157_wait_backup_restore() (or
similar) fit better?
Also, poll_timeout_us() is invoked with a 200 us poll interval and a
2000 us overall timeout, giving roughly ten iterations before the
error path fires. Other GPHY-related waits in the driver allow up to
the order of seconds. Is 2 ms known to be sufficient for the
backup-restore handshake on RTL8159 across suspend/resume and on
slower hosts?
> @@ -8184,14 +8389,14 @@ static void r8156_init(struct r8152 *tp)
> if (test_bit(RTL8152_INACCESSIBLE, &tp->flags))
> return;
>
> - if (tp->version == RTL_VER_16) {
> + if (tp->version == RTL_VER_16 || tp->version == RTL_VER_17) {
> ocp_byte_set_bits(tp, MCU_TYPE_USB, 0xcffe, BIT(3));
> ocp_byte_clr_bits(tp, MCU_TYPE_USB, 0xd3ca, BIT(0));
> }
>
> ocp_byte_clr_bits(tp, MCU_TYPE_USB, USB_ECM_OP, EN_ALL_SPEED);
>
> - if (tp->version != RTL_VER_16)
> + if (tp->version < RTL_VER_16)
> ocp_write_word(tp, MCU_TYPE_USB, USB_SPEED_OPTION, 0);
Later in the same function:
if (tp->version != RTL_VER_16)
ocp_word_clr_bits(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL3,
PLA_MCU_SPDWN_EN);
Was this site meant to be converted to "< RTL_VER_16" along with the
others? As written, RTL_VER_17 clears PLA_MCU_SPDWN_EN during init,
while RTL_VER_16 does not.
[ ... ]
> @@ -8294,7 +8505,10 @@ static void r8156_init(struct r8152 *tp)
> set_bit(GREEN_ETHERNET, &tp->flags);
>
> /* rx aggregation / 16 bytes Rx descriptor */
> - if (tp->version == RTL_VER_16)
> + if (tp->version == RTL_VER_17)
> + ocp_word_clr_bits(tp, MCU_TYPE_USB, USB_USB_CTRL,
> + RX_AGG_DISABLE | RX_DESC_16B | BIT(11));
> + else if (tp->version == RTL_VER_16)
> ocp_word_clr_bits(tp, MCU_TYPE_USB, USB_USB_CTRL, RX_AGG_DISABLE | RX_DESC_16B);
Every other USB_USB_CTRL bit written in this driver uses a named
macro (CDC_ECM_EN, RX_AGG_DISABLE, RX_ZERO_EN, RX_DESC_16B). Could
BIT(11) be given a symbolic name so future multi-version handling of
this register stays self-describing?
[ ... ]
> @@ -9690,6 +9904,29 @@ static int rtl_ops_init(struct r8152 *tp)
> r8157_desc_init(tp);
> break;
>
> + case RTL_VER_17:
> + tp->eee_en = true;
> + tp->eee_adv = MDIO_EEE_100TX | MDIO_EEE_1000T | MDIO_EEE_10GT;
> + tp->eee_adv2 = MDIO_EEE_2_5GT | MDIO_EEE_5GT;
> + ops->init = r8156_init;
> + ops->enable = rtl8156_enable;
> + ops->disable = rtl8153_disable;
> + ops->up = rtl8156_up;
> + ops->down = rtl8156_down;
ops->up is wired to rtl8156_up() here, but rtl8156_up() still checks
for RTL_VER_16 exclusively at the sites noted above. See the
rtl8156_down() hunk for the pattern that appears to be intended.
> + ops->unload = rtl8153_unload;
> + ops->eee_get = r8153_get_eee;
> + ops->eee_set = r8152_set_eee;
> + ops->in_nway = rtl8153_in_nway;
> + ops->hw_phy_cfg = r8157_hw_phy_cfg;
> + ops->autosuspend_en = rtl8157_runtime_enable;
> + ops->change_mtu = rtl8156_change_mtu;
> + tp->rx_buf_sz = 48 * 1024;
> + tp->support_2500full = 1;
> + tp->support_5000full = 1;
> + tp->support_10000full = 1;
> + r8157_desc_init(tp);
> + break;
RTL_VER_17 sets rx_buf_sz to 48 KiB and enables support_10000full,
but the max_mtu switch in rtl8152_probe() does not include
RTL_VER_17:
case RTL_VER_12:
case RTL_VER_13:
case RTL_VER_15:
case RTL_VER_16:
netdev->max_mtu = size_to_mtu(16 * 1024);
break;
...
default:
netdev->max_mtu = ETH_DATA_LEN;
break;
Should RTL_VER_17 also select the 16 KiB max_mtu case so jumbo frames
are available on the new 10 Gbit chip?
Separately, rtl8156_enable() is set as ops->enable for RTL_VER_17 but
the USB_RX_AGGR_NUM reset is guarded by
"tp->version >= RTL_VER_12 && tp->version <= RTL_VER_16", so
RTL_VER_17 does not execute it. Is that intended given RTL_VER_17
otherwise shares the RTL_VER_16 descriptor/aggregation plumbing
(r8157_desc_init(), and the USB_USB_CTRL aggregation bits cleared in
r8156_init() for RTL_VER_17)?
[ ... ]
^ permalink raw reply
* Re: [PATCH net-next v2 3/4] r8152: Add irq mitigation for RTL8157/9
From: Jakub Kicinski @ 2026-05-01 1:16 UTC (permalink / raw)
To: mail
Cc: Jakub Kicinski, andrew+netdev, davem, edumazet, pabeni, linux-usb,
netdev, linux-kernel, hsu.chih.kai
In-Reply-To: <20260429-rtl8159_net_next-v2-3-bab3cd4e4c66@birger-koblitz.de>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
r8152: Add irq mitigation for RTL8157/9
This commit clears bits 0, 1, 2, and 7 of USB register 0xcf04 in
r8156_init() for RTL_VER_16 and later, with the stated intent of avoiding
intr URB completions with urb->status == -ESHUTDOWN on RTL8157 and
RTL8159.
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index 2a07dde289e2..9fcad3dac76f 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
> @@ -8452,6 +8452,12 @@ static void r8156_init(struct r8152 *tp)
> else
> r8153_u2p3en(tp, false);
>
> + if (tp->version >= RTL_VER_16) {
> + /* Disable Interrupt Mitigation */
> + ocp_byte_clr_bits(tp, MCU_TYPE_USB, 0xcf04,
> + BIT(0) | BIT(1) | BIT(2) | BIT(7));
> + }
> +
The subject and changelog describe this as adding "irq mitigation" /
"interrupt mitigation code", but the in-tree comment says "Disable
Interrupt Mitigation" and the operation is ocp_byte_clr_bits(), which
turns a hardware feature off.
Would it be clearer to phrase the subject along the lines of "r8152:
disable hardware interrupt mitigation on RTL8157/9" so that someone
bisecting or searching the log for when this feature was "added" is not
led in the opposite direction?
The same sequence already exists in rtl8153_unload():
static void rtl8153_unload(struct r8152 *tp)
{
if (test_bit(RTL8152_INACCESSIBLE, &tp->flags))
return;
r8153_power_cut_en(tp, false);
if (tp->version >= RTL_VER_16) {
/* Disable Interrupt Mitigation */
ocp_byte_clr_bits(tp, MCU_TYPE_USB, 0xcf04, BIT(0) | BIT(1) | BIT(2) | BIT(7));
}
}
That copy was introduced by fd3c7d080df5 ("r8152: Add support for the
RTL8157 hardware"). Now that the bits are cleared during init, does the
unload-path copy still serve a purpose, or can it be removed as part of
this change?
The register 0xcf04 and the bit selection BIT(0) | BIT(1) | BIT(2) |
BIT(7) are bare numeric literals, while most of the file uses named
defines (USB_MSC_TIMER, USB_U2P3_CTRL, U2P3_ENABLE, and so on). Could
this change also introduce a symbolic name for the register and for the
individual bits being cleared, so reviewers can check the mask against
the datasheet and tell which bit is the enable versus threshold versus
reserved?
On the causal chain:
The changelog links the register write to intr URB completions with
status -ESHUTDOWN / -108, but does not explain how a MAC-side
interrupt-mitigation setting produces that status.
-ESHUTDOWN on an intr URB is normally posted by the USB core when an
endpoint has been disabled or the device is disconnecting. Could the
commit message describe the mechanism by which clearing these four bits
prevents the host controller from observing endpoint shutdown, and cite
the datasheet naming for bits 0, 1, 2, and 7 of 0xcf04 so it is clear
why exactly those bits (and no others) are the correct set to clear?
^ permalink raw reply
* [PATCH iproute2-next 0/5] netshaper: Extend netshaper support
From: Mohsin Bashir @ 2026-05-01 1:16 UTC (permalink / raw)
To: netdev; +Cc: dsahern, stephen, pabeni, kuba, ernis, mohsin.bashr
From: Mohsin Bashir <hmohsin@meta.com>
This series extends the netshaper CLI with missing parameter support
and adds the group command for building scheduling hierarchies.
The existing netshaper tool only supports setting bw-max on individual
shapers. This series adds the remaining shaper attributes (bw-min,
weight, priority) needed for TX scheduling, and introduces the
group command which ties leaf shapers to a parent node in a single
operation.
Mohsin Bashir (5):
netshaper: Extract parse_scope() and parse_rate() helpers
netshaper: Add bw-min and weight parameter support
netshaper: Extend show output with parent, bw-min and weight
netshaper: Make handle id optional for node scope
netshaper: Add group command for creating scheduling hierarchies
netshaper/netshaper.c | 398 ++++++++++++++++++++++++++++++++++--------
1 file changed, 324 insertions(+), 74 deletions(-)
--
2.52.0
^ permalink raw reply
* [PATCH iproute2-next 1/5] netshaper: Extract parse_scope() and parse_rate() helpers
From: Mohsin Bashir @ 2026-05-01 1:16 UTC (permalink / raw)
To: netdev; +Cc: dsahern, stephen, pabeni, kuba, ernis, mohsin.bashr
In-Reply-To: <20260501011611.3533573-1-mohsin.bashr@gmail.com>
Add parse_scope() and parse_rate() helpers in preparation for
adding the group command which will need scope and rate parsing
at various places in the code. This patch will help to avoid
code duplication.
Verify that the tool works as before:
./netshaper set dev eth2 handle scope queue id 0 bw-max 100kbit
./netshaper delete dev eth2 handle scope queue id 0 bw-max 100kbit
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Mohsin Bashir <mohsin.bashr@gmail.com>
---
netshaper/netshaper.c | 65 +++++++++++++++++++++++--------------------
1 file changed, 35 insertions(+), 30 deletions(-)
diff --git a/netshaper/netshaper.c b/netshaper/netshaper.c
index 47fb805e..1603e6e5 100644
--- a/netshaper/netshaper.c
+++ b/netshaper/netshaper.c
@@ -47,6 +47,28 @@ static const char *net_shaper_scope_names[NET_SHAPER_SCOPE_MAX + 1] = {
"node"
};
+static int parse_scope(const char *str)
+{
+ if (strcmp(str, "netdev") == 0)
+ return NET_SHAPER_SCOPE_NETDEV;
+ if (strcmp(str, "queue") == 0)
+ return NET_SHAPER_SCOPE_QUEUE;
+ if (strcmp(str, "node") == 0)
+ return NET_SHAPER_SCOPE_NODE;
+ return -1;
+}
+
+static int parse_rate(const char *str, __u64 *rate_bps)
+{
+ if (get_rate64(rate_bps, str)) {
+ fprintf(stderr, "Invalid rate value \"%s\"\n", str);
+ return -1;
+ }
+ /* get_rate64 returns bytes/sec, convert to bits/sec */
+ *rate_bps *= 8;
+ return 0;
+}
+
static void print_netshaper_attrs(struct nlmsghdr *answer)
{
struct genlmsghdr *ghdr = NLMSG_DATA(answer);
@@ -117,12 +139,8 @@ static int do_cmd(int argc, char **argv, int cmd)
ifindex = ll_name_to_index(*argv);
} else if (strcmp(*argv, "bw-max") == 0) {
NEXT_ARG();
- if (get_rate64(&bw_max_bps, *argv)) {
- fprintf(stderr, "Invalid bw-max value\n");
+ if (parse_rate(*argv, &bw_max_bps))
return -1;
- }
- /* Convert Bps to bps */
- bw_max_bps *= 8;
} else if (strcmp(*argv, "handle") == 0) {
handle_present = true;
NEXT_ARG();
@@ -134,34 +152,24 @@ static int do_cmd(int argc, char **argv, int cmd)
}
NEXT_ARG();
- if (strcmp(*argv, "netdev") == 0) {
- handle_scope = NET_SHAPER_SCOPE_NETDEV;
- /* For netdev scope, id is optional - check if next arg is "id" */
+ handle_scope = parse_scope(*argv);
+ if (handle_scope < 0) {
+ fprintf(stderr, "Invalid scope \"%s\"\n", *argv);
+ return -1;
+ }
+
+ if (handle_scope == NET_SHAPER_SCOPE_NETDEV) {
+ /* For netdev scope, id is optional */
if (argc > 1 && strcmp(argv[1], "id") == 0) {
- NEXT_ARG(); /* move to "id" */
- NEXT_ARG(); /* move to id value */
+ NEXT_ARG();
+ NEXT_ARG();
if (get_unsigned(&handle_id, *argv, 10)) {
fprintf(stderr, "Invalid handle id\n");
return -1;
}
}
- } else if (strcmp(*argv, "queue") == 0) {
- handle_scope = NET_SHAPER_SCOPE_QUEUE;
- /* For queue scope, id is required */
- NEXT_ARG();
- if (strcmp(*argv, "id") != 0) {
- fprintf(stderr, "What is \"%s\"\n", *argv);
- usage();
- return -1;
- }
- NEXT_ARG();
- if (get_unsigned(&handle_id, *argv, 10)) {
- fprintf(stderr, "Invalid handle id\n");
- return -1;
- }
- } else if (strcmp(*argv, "node") == 0) {
- handle_scope = NET_SHAPER_SCOPE_NODE;
- /* For node scope, id is required */
+ } else {
+ /* For queue/node scope, id is required */
NEXT_ARG();
if (strcmp(*argv, "id") != 0) {
fprintf(stderr, "What is \"%s\"\n", *argv);
@@ -173,9 +181,6 @@ static int do_cmd(int argc, char **argv, int cmd)
fprintf(stderr, "Invalid handle id\n");
return -1;
}
- } else {
- fprintf(stderr, "Invalid scope\n");
- return -1;
}
} else {
fprintf(stderr, "What is \"%s\"\n", *argv);
--
2.52.0
^ permalink raw reply related
* [PATCH iproute2-next 2/5] netshaper: Add bw-min and weight parameter support
From: Mohsin Bashir @ 2026-05-01 1:16 UTC (permalink / raw)
To: netdev; +Cc: dsahern, stephen, pabeni, kuba, ernis, mohsin.bashr
In-Reply-To: <20260501011611.3533573-1-mohsin.bashr@gmail.com>
Add bw-min and weight parameters to the set command. Previously
only bw-max was supported. Update the set validation to accept
any of bw-max, bw-min, or weight. Update usage text to reflect
the new parameters.
Before:
./netshaper set dev eth2 handle scope queue id 0 bw-min 100kbit
What is "bw-min"
Usage: netshaper [ OPTIONS ] { COMMAND | help }
OPTIONS := { -V[ersion] | -c[olor] | -help }
COMMAND := { set | get | delete } dev DEVNAME
handle scope HANDLE_SCOPE [id HANDLE_ID]
[bw-max BW_MAX]
Where: DEVNAME := STRING
HANDLE_SCOPE := { netdev | queue | node }
HANDLE_ID := UINT (required for queue/node, optional for netdev)
BW_MAX := UINT{ kbit | mbit | gbit }
After:
./netshaper set dev eth2 handle scope queue id 0 bw-min 100kbit
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Mohsin Bashir <mohsin.bashr@gmail.com>
---
netshaper/netshaper.c | 38 ++++++++++++++++++++++++++++----------
1 file changed, 28 insertions(+), 10 deletions(-)
diff --git a/netshaper/netshaper.c b/netshaper/netshaper.c
index 1603e6e5..4ee30525 100644
--- a/netshaper/netshaper.c
+++ b/netshaper/netshaper.c
@@ -33,11 +33,12 @@ static void usage(void)
"OPTIONS := { -V[ersion] | -c[olor] | -help }\n"
"COMMAND := { set | get | delete } dev DEVNAME\n"
" handle scope HANDLE_SCOPE [id HANDLE_ID]\n"
- " [bw-max BW_MAX]\n"
+ " [bw-max BW_MAX] [bw-min BW_MIN] [weight WEIGHT]\n"
"Where: DEVNAME := STRING\n"
" HANDLE_SCOPE := { netdev | queue | node }\n"
" HANDLE_ID := UINT (required for queue/node, optional for netdev)\n"
- " BW_MAX := UINT{ kbit | mbit | gbit }\n");
+ " BW_MAX/BW_MIN := UINT{ kbit | mbit | gbit }\n"
+ " WEIGHT := UINT\n");
}
static const char *net_shaper_scope_names[NET_SHAPER_SCOPE_MAX + 1] = {
@@ -125,13 +126,13 @@ static int do_cmd(int argc, char **argv, int cmd)
GENL_REQUEST(req, 1024, genl_family, 0, NET_SHAPER_FAMILY_VERSION, cmd,
NLM_F_REQUEST | NLM_F_ACK);
- struct nlmsghdr *answer;
- __u64 bw_max_bps = 0;
- int ifindex = -1;
+ bool has_bw_max = false, has_bw_min = false, has_weight = false;
int handle_scope = NET_SHAPER_SCOPE_UNSPEC;
- __u32 handle_id = 0;
+ __u64 bw_max_bps = 0, bw_min_bps = 0;
+ __u32 handle_id = 0, weight = 0;
bool handle_present = false;
- int err;
+ struct nlmsghdr *answer;
+ int err, ifindex = -1;
while (argc > 0) {
if (strcmp(*argv, "dev") == 0) {
@@ -141,6 +142,19 @@ static int do_cmd(int argc, char **argv, int cmd)
NEXT_ARG();
if (parse_rate(*argv, &bw_max_bps))
return -1;
+ has_bw_max = true;
+ } else if (strcmp(*argv, "bw-min") == 0) {
+ NEXT_ARG();
+ if (parse_rate(*argv, &bw_min_bps))
+ return -1;
+ has_bw_min = true;
+ } else if (strcmp(*argv, "weight") == 0) {
+ NEXT_ARG();
+ if (get_unsigned(&weight, *argv, 10)) {
+ fprintf(stderr, "Invalid weight value\n");
+ return -1;
+ }
+ has_weight = true;
} else if (strcmp(*argv, "handle") == 0) {
handle_present = true;
NEXT_ARG();
@@ -197,8 +211,8 @@ static int do_cmd(int argc, char **argv, int cmd)
if (!handle_present)
missarg("handle");
- if (cmd == NET_SHAPER_CMD_SET && bw_max_bps == 0)
- missarg("bw-max");
+ if (cmd == NET_SHAPER_CMD_SET && !has_bw_max && !has_bw_min && !has_weight)
+ missarg("bw-max, bw-min, or weight");
addattr32(&req.n, sizeof(req), NET_SHAPER_A_IFINDEX, ifindex);
@@ -208,8 +222,12 @@ static int do_cmd(int argc, char **argv, int cmd)
addattr32(&req.n, sizeof(req), NET_SHAPER_A_HANDLE_ID, handle_id);
addattr_nest_end(&req.n, handle);
- if (cmd == NET_SHAPER_CMD_SET)
+ if (has_bw_max)
addattr64(&req.n, sizeof(req), NET_SHAPER_A_BW_MAX, bw_max_bps);
+ if (has_bw_min)
+ addattr64(&req.n, sizeof(req), NET_SHAPER_A_BW_MIN, bw_min_bps);
+ if (has_weight)
+ addattr32(&req.n, sizeof(req), NET_SHAPER_A_WEIGHT, weight);
err = rtnl_talk(&gen_rth, &req.n, &answer);
if (err < 0) {
--
2.52.0
^ permalink raw reply related
* [PATCH iproute2-next 3/5] netshaper: Extend show output with parent, bw-min and weight
From: Mohsin Bashir @ 2026-05-01 1:16 UTC (permalink / raw)
To: netdev; +Cc: dsahern, stephen, pabeni, kuba, ernis, mohsin.bashr
In-Reply-To: <20260501011611.3533573-1-mohsin.bashr@gmail.com>
Extend print_netshaper_attrs() to display parent scope/id, bw-min,
and weight fields in the show output. Replace the switch-based
iteration with direct attribute checks for cleaner output formatting.
Verify the support by setting the rate for a queue and attempt to read it
back.
Before:
./netshaper set dev eth2 handle scope queue id 0 bw-min 10mbit weight 5
./netshaper show dev eth2 handle scope queue id 0
scope: queue
id: 0
dev: eth2
After:
./netshaper set dev eth2 handle scope queue id 0 bw-min 10mbit weight 5
./netshaper show dev eth2 handle scope queue id 0
dev: eth2 scope queue id 1 parent scope netdev bw-min 10 mbps weight 5
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Mohsin Bashir <mohsin.bashr@gmail.com>
---
netshaper/netshaper.c | 89 ++++++++++++++++++++++++++-----------------
1 file changed, 55 insertions(+), 34 deletions(-)
diff --git a/netshaper/netshaper.c b/netshaper/netshaper.c
index 4ee30525..53a5eae3 100644
--- a/netshaper/netshaper.c
+++ b/netshaper/netshaper.c
@@ -72,53 +72,74 @@ static int parse_rate(const char *str, __u64 *rate_bps)
static void print_netshaper_attrs(struct nlmsghdr *answer)
{
+ struct rtattr *parent_tb[NET_SHAPER_A_HANDLE_MAX + 1] = {};
struct genlmsghdr *ghdr = NLMSG_DATA(answer);
int len = answer->nlmsg_len - NLMSG_LENGTH(GENL_HDRLEN);
struct rtattr *tb[NET_SHAPER_A_MAX + 1] = {};
struct rtattr *handle_tb[NET_SHAPER_A_HANDLE_MAX + 1] = {};
- __u32 bw_max_mbps, scope, id;
- __u64 bw_max_bps;
+ __u32 scope, id;
int ifindex;
parse_rtattr_flags(tb, NET_SHAPER_A_MAX,
(struct rtattr *)((char *)ghdr + GENL_HDRLEN),
len, NLA_F_NESTED);
- for (int i = 1; i <= NET_SHAPER_A_MAX; ++i) {
- if (!tb[i])
- continue;
- switch (i) {
- case NET_SHAPER_A_BW_MAX:
- bw_max_bps = rta_getattr_uint(tb[i]);
- bw_max_mbps = (bw_max_bps / 1000000);
+ if (tb[NET_SHAPER_A_IFINDEX]) {
+ ifindex = rta_getattr_u32(tb[NET_SHAPER_A_IFINDEX]);
+ print_color_string(PRINT_ANY, COLOR_IFNAME, "dev",
+ "dev: %s ", ll_index_to_name(ifindex));
+ }
- print_uint(PRINT_ANY, "bw-max", "bw-max: %u mbps\n",
- bw_max_mbps);
- break;
- case NET_SHAPER_A_IFINDEX:
- ifindex = rta_getattr_u32(tb[i]);
- print_color_string(PRINT_ANY, COLOR_IFNAME, "dev",
- "dev: %s\n",
- ll_index_to_name(ifindex));
- break;
- case NET_SHAPER_A_HANDLE:
- parse_rtattr_nested(handle_tb, NET_SHAPER_A_HANDLE_MAX,
- tb[NET_SHAPER_A_HANDLE]);
- if (handle_tb[NET_SHAPER_A_HANDLE_SCOPE]) {
- scope = rta_getattr_u32(handle_tb[NET_SHAPER_A_HANDLE_SCOPE]);
- print_string(PRINT_ANY, "scope",
- "scope: %s\n",
- net_shaper_scope_names[scope]);
- }
- if (handle_tb[NET_SHAPER_A_HANDLE_ID]) {
- id = rta_getattr_u32(handle_tb[NET_SHAPER_A_HANDLE_ID]);
- print_uint(PRINT_ANY, "id", "id: %u\n", id);
- }
- break;
- default:
- break;
+ if (tb[NET_SHAPER_A_HANDLE]) {
+ parse_rtattr_nested(handle_tb, NET_SHAPER_A_HANDLE_MAX,
+ tb[NET_SHAPER_A_HANDLE]);
+ if (handle_tb[NET_SHAPER_A_HANDLE_SCOPE]) {
+ scope = rta_getattr_u32(handle_tb[NET_SHAPER_A_HANDLE_SCOPE]);
+ print_string(PRINT_ANY, "scope", "scope %s ",
+ net_shaper_scope_names[scope]);
+ }
+ if (handle_tb[NET_SHAPER_A_HANDLE_ID]) {
+ id = rta_getattr_u32(handle_tb[NET_SHAPER_A_HANDLE_ID]);
+ print_uint(PRINT_ANY, "id", "id %u ", id);
+ }
+ }
+
+ if (tb[NET_SHAPER_A_PARENT]) {
+ parse_rtattr_nested(parent_tb, NET_SHAPER_A_HANDLE_MAX,
+ tb[NET_SHAPER_A_PARENT]);
+ if (parent_tb[NET_SHAPER_A_HANDLE_SCOPE]) {
+ scope = rta_getattr_u32(parent_tb[NET_SHAPER_A_HANDLE_SCOPE]);
+ print_string(PRINT_ANY, "parent-scope",
+ "parent scope %s ",
+ net_shaper_scope_names[scope]);
}
+ if (parent_tb[NET_SHAPER_A_HANDLE_ID]) {
+ id = rta_getattr_u32(parent_tb[NET_SHAPER_A_HANDLE_ID]);
+ print_uint(PRINT_ANY, "parent-id", "id %u ", id);
+ }
+ }
+
+ if (tb[NET_SHAPER_A_BW_MAX]) {
+ __u64 bw = rta_getattr_uint(tb[NET_SHAPER_A_BW_MAX]);
+
+ print_uint(PRINT_ANY, "bw-max", "bw-max %u mbps ",
+ (__u32)(bw / 1000000));
}
+
+ if (tb[NET_SHAPER_A_BW_MIN]) {
+ __u64 bw = rta_getattr_uint(tb[NET_SHAPER_A_BW_MIN]);
+
+ print_uint(PRINT_ANY, "bw-min", "bw-min %u mbps ",
+ (__u32)(bw / 1000000));
+ }
+
+ if (tb[NET_SHAPER_A_WEIGHT]) {
+ __u32 weight = rta_getattr_u32(tb[NET_SHAPER_A_WEIGHT]);
+
+ print_uint(PRINT_ANY, "weight", "weight %u ", weight);
+ }
+
+ printf("\n");
}
static int do_cmd(int argc, char **argv, int cmd)
--
2.52.0
^ permalink raw reply related
* RE: [PATCH v3 net] net: enetc: fix VSI mailbox timeout handling and DMA lifecycle
From: Wei Fang @ 2026-05-01 1:16 UTC (permalink / raw)
To: Simon Horman
Cc: Claudiu Manoil, Vladimir Oltean, Clark Wang,
andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com,
kuba@kernel.org, pabeni@redhat.com, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, imx@lists.linux.dev
In-Reply-To: <20260430170922.1729686-1-horms@kernel.org>
> > @@ -259,6 +282,7 @@ static void enetc_vf_remove(struct pci_dev *pdev)
> > {
> > struct enetc_si *si = pci_get_drvdata(pdev);
> > struct enetc_ndev_priv *priv;
> > + struct enetc_msg_swbd msg;
> >
> > priv = netdev_priv(si->ndev);
> > unregister_netdev(si->ndev);
> > @@ -270,7 +294,9 @@ static void enetc_vf_remove(struct pci_dev *pdev)
> >
> > free_netdev(si->ndev);
> >
> > + msg = si->msg;
> > enetc_pci_remove(pdev);
> > + enetc_msg_dma_free(&pdev->dev, &msg);
>
> Is it possible for the mailbox to still be busy here?
>
> The commit addresses the use-after-free in the send path by checking the
> mailbox busy status before freeing the previous buffer. However, during driver
> teardown, enetc_msg_dma_free() is called unconditionally.
>
> If a prior message sent via enetc_msg_vsi_send() times out and the hardware is
> still accessing the buffer, and then the driver is unbound, could this cause the
> hardware to read from freed DMA memory?
>
> Should the teardown path wait for the mailbox to become idle or ensure the
> hardware is safely reset before freeing the buffer?
>
enetc_msg_dma_free() is called after enetc_pci_remove(), enetc_pci_remove()
will disable the device, so it is safe to free the buffer.
^ permalink raw reply
* [PATCH iproute2-next 4/5] netshaper: Make handle id optional for node scope
From: Mohsin Bashir @ 2026-05-01 1:16 UTC (permalink / raw)
To: netdev; +Cc: dsahern, stephen, pabeni, kuba, ernis, mohsin.bashr
In-Reply-To: <20260501011611.3533573-1-mohsin.bashr@gmail.com>
Make handle id optional for node scope so that omitting it tells
the kernel to create a new node and assign an id automatically,
while providing it updates an existing node. Queue scope still
requires an explicit id since it maps to a specific TX queue.
Only send the handle id in the netlink message when the user
explicitly provides one.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Mohsin Bashir <mohsin.bashr@gmail.com>
---
netshaper/netshaper.c | 28 +++++++++++++++-------------
1 file changed, 15 insertions(+), 13 deletions(-)
diff --git a/netshaper/netshaper.c b/netshaper/netshaper.c
index 53a5eae3..fc68e735 100644
--- a/netshaper/netshaper.c
+++ b/netshaper/netshaper.c
@@ -152,6 +152,7 @@ static int do_cmd(int argc, char **argv, int cmd)
__u64 bw_max_bps = 0, bw_min_bps = 0;
__u32 handle_id = 0, weight = 0;
bool handle_present = false;
+ bool has_handle_id = false;
struct nlmsghdr *answer;
int err, ifindex = -1;
@@ -193,18 +194,8 @@ static int do_cmd(int argc, char **argv, int cmd)
return -1;
}
- if (handle_scope == NET_SHAPER_SCOPE_NETDEV) {
- /* For netdev scope, id is optional */
- if (argc > 1 && strcmp(argv[1], "id") == 0) {
- NEXT_ARG();
- NEXT_ARG();
- if (get_unsigned(&handle_id, *argv, 10)) {
- fprintf(stderr, "Invalid handle id\n");
- return -1;
- }
- }
- } else {
- /* For queue/node scope, id is required */
+ if (handle_scope == NET_SHAPER_SCOPE_QUEUE) {
+ /* For queue scope, id is required */
NEXT_ARG();
if (strcmp(*argv, "id") != 0) {
fprintf(stderr, "What is \"%s\"\n", *argv);
@@ -216,6 +207,16 @@ static int do_cmd(int argc, char **argv, int cmd)
fprintf(stderr, "Invalid handle id\n");
return -1;
}
+ has_handle_id = true;
+ } else if (argc > 1 && strcmp(argv[1], "id") == 0) {
+ /* For netdev and node scope, id is optional */
+ NEXT_ARG();
+ NEXT_ARG();
+ if (get_unsigned(&handle_id, *argv, 10)) {
+ fprintf(stderr, "Invalid handle id\n");
+ return -1;
+ }
+ has_handle_id = true;
}
} else {
fprintf(stderr, "What is \"%s\"\n", *argv);
@@ -240,7 +241,8 @@ static int do_cmd(int argc, char **argv, int cmd)
struct rtattr *handle = addattr_nest(&req.n, sizeof(req),
NET_SHAPER_A_HANDLE | NLA_F_NESTED);
addattr32(&req.n, sizeof(req), NET_SHAPER_A_HANDLE_SCOPE, handle_scope);
- addattr32(&req.n, sizeof(req), NET_SHAPER_A_HANDLE_ID, handle_id);
+ if (has_handle_id)
+ addattr32(&req.n, sizeof(req), NET_SHAPER_A_HANDLE_ID, handle_id);
addattr_nest_end(&req.n, handle);
if (has_bw_max)
--
2.52.0
^ permalink raw reply related
* [PATCH iproute2-next 5/5] netshaper: Add group command for creating scheduling hierarchies
From: Mohsin Bashir @ 2026-05-01 1:16 UTC (permalink / raw)
To: netdev; +Cc: dsahern, stephen, pabeni, kuba, ernis, mohsin.bashr
In-Reply-To: <20260501011611.3533573-1-mohsin.bashr@gmail.com>
Add the group command to create and update scheduling groups via the
NET_SHAPER_CMD_GROUP netlink operation. This enables building shaper
hierarchies by specifying a node handle, parent scope, rate parameters,
and a set of leaf shapers (queues or nodes) to attach.
Example usage:
netshaper group dev eth0 handle scope node parent scope netdev \
bw-max 1gbit leaves scope queue id 0 scope queue id 1
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Mohsin Bashir <mohsin.bashr@gmail.com>
---
netshaper/netshaper.c | 206 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 205 insertions(+), 1 deletion(-)
diff --git a/netshaper/netshaper.c b/netshaper/netshaper.c
index fc68e735..7ec8220c 100644
--- a/netshaper/netshaper.c
+++ b/netshaper/netshaper.c
@@ -31,9 +31,15 @@ static void usage(void)
fprintf(stderr,
"Usage: netshaper [ OPTIONS ] { COMMAND | help }\n"
"OPTIONS := { -V[ersion] | -c[olor] | -help }\n"
- "COMMAND := { set | get | delete } dev DEVNAME\n"
+ "COMMAND := { set | get | delete | group } dev DEVNAME\n"
" handle scope HANDLE_SCOPE [id HANDLE_ID]\n"
" [bw-max BW_MAX] [bw-min BW_MIN] [weight WEIGHT]\n"
+ "\n"
+ "netshaper group dev DEVNAME handle scope SCOPE [ id ID ]\n"
+ " parent scope SCOPE [ id ID ]\n"
+ " [ bw-max BW ] [ bw-min BW ] [ weight WEIGHT ]\n"
+ " leaves { scope SCOPE id ID } [ ... ]\n"
+ "\n"
"Where: DEVNAME := STRING\n"
" HANDLE_SCOPE := { netdev | queue | node }\n"
" HANDLE_ID := UINT (required for queue/node, optional for netdev)\n"
@@ -264,6 +270,202 @@ static int do_cmd(int argc, char **argv, int cmd)
return err;
}
+static int do_group(int argc, char **argv)
+{
+ GENL_REQUEST(req, 4096, genl_family, 0, NET_SHAPER_FAMILY_VERSION,
+ NET_SHAPER_CMD_GROUP, NLM_F_REQUEST | NLM_F_ACK);
+
+ bool has_bw_max = false, has_bw_min = false, has_weight = false;
+ bool parsing_leaves = false, has_hid = false, has_pid = false;
+ int parent_scope = -1, ifindex = -1, num_leaves = 0;
+ int err, handle_scope = NET_SHAPER_SCOPE_UNSPEC;
+ __u32 handle_id = 0, parent_id = 0, weight = 0;
+ __u64 bw_max_bps = 0, bw_min_bps = 0;
+ struct nlmsghdr *answer;
+
+ struct {
+ int scope;
+ __u32 id;
+ } leaves[128];
+
+ while (argc > 0) {
+ if (parsing_leaves) {
+ if (strcmp(*argv, "scope") == 0) {
+ int lscope;
+
+ NEXT_ARG();
+ lscope = parse_scope(*argv);
+ if (lscope < 0) {
+ fprintf(stderr, "Invalid leaf scope \"%s\"\n",
+ *argv);
+ return -1;
+ }
+
+ NEXT_ARG();
+ if (strcmp(*argv, "id") != 0) {
+ fprintf(stderr, "Expected \"id\" after leaf scope\n");
+ return -1;
+ }
+
+ NEXT_ARG();
+ leaves[num_leaves].scope = lscope;
+ if (get_unsigned(&leaves[num_leaves].id, *argv, 10)) {
+ fprintf(stderr, "Invalid leaf id\n");
+ return -1;
+ }
+ num_leaves++;
+ argc--;
+ argv++;
+ continue;
+ }
+ parsing_leaves = false;
+ }
+
+ if (strcmp(*argv, "dev") == 0) {
+ NEXT_ARG();
+ ifindex = ll_name_to_index(*argv);
+ if (ifindex == 0) {
+ fprintf(stderr, "Device \"%s\" not found\n", *argv);
+ return -1;
+ }
+ } else if (strcmp(*argv, "bw-max") == 0) {
+ NEXT_ARG();
+ if (parse_rate(*argv, &bw_max_bps))
+ return -1;
+ has_bw_max = true;
+ } else if (strcmp(*argv, "bw-min") == 0) {
+ NEXT_ARG();
+ if (parse_rate(*argv, &bw_min_bps))
+ return -1;
+ has_bw_min = true;
+ } else if (strcmp(*argv, "weight") == 0) {
+ NEXT_ARG();
+ if (get_unsigned(&weight, *argv, 10)) {
+ fprintf(stderr, "Invalid weight value\n");
+ return -1;
+ }
+ has_weight = true;
+ } else if (strcmp(*argv, "handle") == 0) {
+ NEXT_ARG();
+ if (strcmp(*argv, "scope") != 0) {
+ fprintf(stderr, "Expected \"scope\" after \"handle\"\n");
+ return -1;
+ }
+ NEXT_ARG();
+ handle_scope = parse_scope(*argv);
+ if (handle_scope < 0) {
+ fprintf(stderr, "Invalid handle scope \"%s\"\n",
+ *argv);
+ return -1;
+ }
+
+ if (argc > 1 && strcmp(argv[1], "id") == 0) {
+ NEXT_ARG();
+ NEXT_ARG();
+ if (get_unsigned(&handle_id, *argv, 10)) {
+ fprintf(stderr, "Invalid handle id\n");
+ return -1;
+ }
+ has_hid = true;
+ }
+ } else if (strcmp(*argv, "parent") == 0) {
+ NEXT_ARG();
+ if (strcmp(*argv, "scope") != 0) {
+ fprintf(stderr, "Expected \"scope\" after \"parent\"\n");
+ return -1;
+ }
+ NEXT_ARG();
+ parent_scope = parse_scope(*argv);
+ if (parent_scope < 0) {
+ fprintf(stderr, "Invalid parent scope \"%s\"\n",
+ *argv);
+ return -1;
+ }
+
+ if (argc > 1 && strcmp(argv[1], "id") == 0) {
+ NEXT_ARG();
+ NEXT_ARG();
+ if (get_unsigned(&parent_id, *argv, 10)) {
+ fprintf(stderr, "Invalid parent id\n");
+ return -1;
+ }
+ has_pid = true;
+ }
+ } else if (strcmp(*argv, "leaves") == 0) {
+ parsing_leaves = true;
+ argc--;
+ argv++;
+ continue;
+ } else {
+ fprintf(stderr, "What is \"%s\"\n", *argv);
+ usage();
+ return -1;
+ }
+ argc--;
+ argv++;
+ }
+
+ if (ifindex == -1)
+ missarg("dev");
+ if (handle_scope == NET_SHAPER_SCOPE_UNSPEC)
+ missarg("handle");
+ if (parent_scope < 0)
+ missarg("parent");
+ if (num_leaves == 0)
+ missarg("leaves");
+
+ addattr32(&req.n, sizeof(req), NET_SHAPER_A_IFINDEX, ifindex);
+
+ struct rtattr *parent = addattr_nest(&req.n, sizeof(req),
+ NET_SHAPER_A_PARENT | NLA_F_NESTED);
+ addattr32(&req.n, sizeof(req), NET_SHAPER_A_HANDLE_SCOPE, parent_scope);
+ if (has_pid)
+ addattr32(&req.n, sizeof(req), NET_SHAPER_A_HANDLE_ID, parent_id);
+ addattr_nest_end(&req.n, parent);
+
+ struct rtattr *handle = addattr_nest(&req.n, sizeof(req),
+ NET_SHAPER_A_HANDLE | NLA_F_NESTED);
+ addattr32(&req.n, sizeof(req), NET_SHAPER_A_HANDLE_SCOPE, handle_scope);
+ if (has_hid)
+ addattr32(&req.n, sizeof(req), NET_SHAPER_A_HANDLE_ID, handle_id);
+ addattr_nest_end(&req.n, handle);
+
+ if (has_bw_max)
+ addattr64(&req.n, sizeof(req), NET_SHAPER_A_BW_MAX, bw_max_bps);
+ if (has_bw_min)
+ addattr64(&req.n, sizeof(req), NET_SHAPER_A_BW_MIN, bw_min_bps);
+ if (has_weight)
+ addattr32(&req.n, sizeof(req), NET_SHAPER_A_WEIGHT, weight);
+
+ if (has_bw_max || has_bw_min)
+ addattr32(&req.n, sizeof(req), NET_SHAPER_A_METRIC,
+ NET_SHAPER_METRIC_BPS);
+
+ for (int i = 0; i < num_leaves; i++) {
+ struct rtattr *leaf, *leaf_handle;
+
+ leaf = addattr_nest(&req.n, sizeof(req),
+ NET_SHAPER_A_LEAVES | NLA_F_NESTED);
+ leaf_handle = addattr_nest(&req.n, sizeof(req),
+ NET_SHAPER_A_HANDLE | NLA_F_NESTED);
+ addattr32(&req.n, sizeof(req), NET_SHAPER_A_HANDLE_SCOPE,
+ leaves[i].scope);
+ addattr32(&req.n, sizeof(req), NET_SHAPER_A_HANDLE_ID,
+ leaves[i].id);
+ addattr_nest_end(&req.n, leaf_handle);
+ addattr_nest_end(&req.n, leaf);
+ }
+
+ err = rtnl_talk(&gen_rth, &req.n, &answer);
+ if (err < 0) {
+ fprintf(stderr, "Kernel command failed: %d\n", err);
+ return err;
+ }
+
+ print_netshaper_attrs(answer);
+ return 0;
+}
+
int main(int argc, char **argv)
{
int color = default_color_opt();
@@ -308,6 +510,8 @@ int main(int argc, char **argv)
return do_cmd(argc - 1, argv + 1, NET_SHAPER_CMD_DELETE);
if (strcmp(*argv, "show") == 0)
return do_cmd(argc - 1, argv + 1, NET_SHAPER_CMD_GET);
+ if (strcmp(*argv, "group") == 0)
+ return do_group(argc - 1, argv + 1);
if (strcmp(*argv, "help") == 0) {
usage();
return 0;
--
2.52.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox