* Re: [PATCH v9 02/10] x86/bhi: Make clear_bhb_loop() effective on newer CPUs
From: Pawan Gupta @ 2026-04-03 23:33 UTC (permalink / raw)
To: Jim Mattson
Cc: x86, Jon Kohler, Nikolay Borisov, H. Peter Anvin, Josh Poimboeuf,
David Kaplan, Sean Christopherson, Borislav Petkov, Dave Hansen,
Peter Zijlstra, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, KP Singh, Jiri Olsa, David S. Miller,
David Laight, Andy Lutomirski, Thomas Gleixner, Ingo Molnar,
David Ahern, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, John Fastabend, Stanislav Fomichev, Hao Luo,
Paolo Bonzini, Jonathan Corbet, linux-kernel, kvm, Asit Mallick,
Tao Zhang, bpf, netdev, linux-doc, chao.gao
In-Reply-To: <CALMp9eT2vJBdLPY2uBYrPgVrhS_aYmfGfdXe6MZXG_gyryLHVA@mail.gmail.com>
On Fri, Apr 03, 2026 at 04:22:28PM -0700, Jim Mattson wrote:
> On Fri, Apr 3, 2026 at 4:16 PM Pawan Gupta
> <pawan.kumar.gupta@linux.intel.com> wrote:
> >
> > On Fri, Apr 03, 2026 at 02:59:33PM -0700, Jim Mattson wrote:
> > > On Fri, Apr 3, 2026 at 2:34 PM Pawan Gupta
> > > <pawan.kumar.gupta@linux.intel.com> wrote:
> > > >
> > > > On Fri, Apr 03, 2026 at 01:19:17PM -0700, Jim Mattson wrote:
> > > > > On Fri, Apr 3, 2026 at 11:52 AM Pawan Gupta
> > > > > <pawan.kumar.gupta@linux.intel.com> wrote:
> > > > > >
> > > > > > On Fri, Apr 03, 2026 at 11:10:08AM -0700, Jim Mattson wrote:
> > > > > > > On Thu, Apr 2, 2026 at 5:32 PM Pawan Gupta
> > > > > > > <pawan.kumar.gupta@linux.intel.com> wrote:
> > > > > > > >
> > > > > > > > As a mitigation for BHI, clear_bhb_loop() executes branches that overwrite
> > > > > > > > the Branch History Buffer (BHB). On Alder Lake and newer parts this
> > > > > > > > sequence is not sufficient because it doesn't clear enough entries. This
> > > > > > > > was not an issue because these CPUs use the BHI_DIS_S hardware mitigation
> > > > > > > > in the kernel.
> > > > > > > >
> > > > > > > > Now with VMSCAPE (BHI variant) it is also required to isolate branch
> > > > > > > > history between guests and userspace. Since BHI_DIS_S only protects the
> > > > > > > > kernel, the newer CPUs also use IBPB.
> > > > > > > >
> > > > > > > > A cheaper alternative to the current IBPB mitigation is clear_bhb_loop().
> > > > > > > > But it currently does not clear enough BHB entries to be effective on newer
> > > > > > > > CPUs with larger BHB. At boot, dynamically set the loop count of
> > > > > > > > clear_bhb_loop() such that it is effective on newer CPUs too. Use the
> > > > > > > > X86_FEATURE_BHI_CTRL feature flag to select the appropriate loop count.
> > > > > > > >
> > > > > > > > Suggested-by: Dave Hansen <dave.hansen@linux.intel.com>
> > > > > > > > Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
> > > > > > > > ---
> > > > > > > > arch/x86/entry/entry_64.S | 8 +++++---
> > > > > > > > arch/x86/include/asm/nospec-branch.h | 2 ++
> > > > > > > > arch/x86/kernel/cpu/bugs.c | 13 +++++++++++++
> > > > > > > > 3 files changed, 20 insertions(+), 3 deletions(-)
> > > > > > > >
> > > > > > > > diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
> > > > > > > > index 3a180a36ca0e..bbd4b1c7ec04 100644
> > > > > > > > --- a/arch/x86/entry/entry_64.S
> > > > > > > > +++ b/arch/x86/entry/entry_64.S
> > > > > > > > @@ -1536,7 +1536,9 @@ SYM_FUNC_START(clear_bhb_loop)
> > > > > > > > ANNOTATE_NOENDBR
> > > > > > > > push %rbp
> > > > > > > > mov %rsp, %rbp
> > > > > > > > - movl $5, %ecx
> > > > > > > > +
> > > > > > > > + movzbl bhb_seq_outer_loop(%rip), %ecx
> > > > > > > > +
> > > > > > > > ANNOTATE_INTRA_FUNCTION_CALL
> > > > > > > > call 1f
> > > > > > > > jmp 5f
> > > > > > > > @@ -1556,8 +1558,8 @@ SYM_FUNC_START(clear_bhb_loop)
> > > > > > > > * This should be ideally be: .skip 32 - (.Lret2 - 2f), 0xcc
> > > > > > > > * but some Clang versions (e.g. 18) don't like this.
> > > > > > > > */
> > > > > > > > - .skip 32 - 18, 0xcc
> > > > > > > > -2: movl $5, %eax
> > > > > > > > + .skip 32 - 20, 0xcc
> > > > > > > > +2: movzbl bhb_seq_inner_loop(%rip), %eax
> > > > > > > > 3: jmp 4f
> > > > > > > > nop
> > > > > > > > 4: sub $1, %eax
> > > > > > > > diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
> > > > > > > > index 70b377fcbc1c..87b83ae7c97f 100644
> > > > > > > > --- a/arch/x86/include/asm/nospec-branch.h
> > > > > > > > +++ b/arch/x86/include/asm/nospec-branch.h
> > > > > > > > @@ -548,6 +548,8 @@ DECLARE_PER_CPU(u64, x86_spec_ctrl_current);
> > > > > > > > extern void update_spec_ctrl_cond(u64 val);
> > > > > > > > extern u64 spec_ctrl_current(void);
> > > > > > > >
> > > > > > > > +extern u8 bhb_seq_inner_loop, bhb_seq_outer_loop;
> > > > > > > > +
> > > > > > > > /*
> > > > > > > > * With retpoline, we must use IBRS to restrict branch prediction
> > > > > > > > * before calling into firmware.
> > > > > > > > diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> > > > > > > > index 83f51cab0b1e..2cb4a96247d8 100644
> > > > > > > > --- a/arch/x86/kernel/cpu/bugs.c
> > > > > > > > +++ b/arch/x86/kernel/cpu/bugs.c
> > > > > > > > @@ -2047,6 +2047,10 @@ enum bhi_mitigations {
> > > > > > > > static enum bhi_mitigations bhi_mitigation __ro_after_init =
> > > > > > > > IS_ENABLED(CONFIG_MITIGATION_SPECTRE_BHI) ? BHI_MITIGATION_AUTO : BHI_MITIGATION_OFF;
> > > > > > > >
> > > > > > > > +/* Default to short BHB sequence values */
> > > > > > > > +u8 bhb_seq_outer_loop __ro_after_init = 5;
> > > > > > > > +u8 bhb_seq_inner_loop __ro_after_init = 5;
> > > > > > > > +
> > > > > > > > static int __init spectre_bhi_parse_cmdline(char *str)
> > > > > > > > {
> > > > > > > > if (!str)
> > > > > > > > @@ -3242,6 +3246,15 @@ void __init cpu_select_mitigations(void)
> > > > > > > > x86_spec_ctrl_base &= ~SPEC_CTRL_MITIGATIONS_MASK;
> > > > > > > > }
> > > > > > > >
> > > > > > > > + /*
> > > > > > > > + * Switch to long BHB clear sequence on newer CPUs (with BHI_CTRL
> > > > > > > > + * support), see Intel's BHI guidance.
> > > > > > > > + */
> > > > > > > > + if (cpu_feature_enabled(X86_FEATURE_BHI_CTRL)) {
> > > > > > > > + bhb_seq_outer_loop = 12;
> > > > > > > > + bhb_seq_inner_loop = 7;
> > > > > > > > + }
> > > > > > > > +
> > > > > > >
> > > > > > > How does this work for VMs in a heterogeneous migration pool that
> > > > > > > spans the Alder Lake boundary? They can't advertise BHI_CTRL, because
> > > > > > > it isn't available on all hosts in the migration pool, but they need
> > > > > > > the long sequence when running on Alder Lake or newer.
> > > > > >
> > > > > > As we discussed elsewhere, support for migration pool is much more
> > > > > > involved. It should be dealt in a separate QEMU/KVM focused series.
> > > > > >
> > > > > > A quickfix could be adding support for spectre_bhi=long that guests in a
> > > > > > migration pool can use?
> > > > >
> > > > > The simplest solution is to add "|
> > > > > cpu_feature_enabled(X86_FEATURE_HYPERVISOR)" to the condition above.
> > > > > If that is unacceptable for the performance of pre-Alder Lake
> > > >
> > > > Yes, that would be unnecessary overhead.
> > > >
> > > > > migration pools, you could define a CPUID or MSR bit that says
> > > > > explicitly, "long BHB flush sequence needed," rather than trying to
> > > > > intuit that property from the presence of BHI_CTRL. Like
> > > > > IA32_ARCH_CAPABILITIES.SKIP_L1DFL_VMENTRY, the bit would only be set
> > > > > by a hypervisor.
> > > >
> > > > I will think about this more.
> > > >
> > > > > I am still skeptical of the need for MSR_VIRTUAL_ENUMERATION and
> > > > > friends, unless there is a major guest OS out there that relies on
> > > > > them.
> > > >
> > > > If we forget about MSR_VIRTUAL_ENUMERATION for a moment, userspace VMM is
> > > > in the best position to decide whether a guest needs
> > > > virtual.SPEC_CTRL[BHI_DIS_S]. Via a KVM interface userspace VMM can get
> > > > BHI_DIS_S for the guests that are in migration pool?
> > >
> > > That is not possible today, since KVM does not implement Intel's
> > > IA32_SPEC_CTRL virtualization, and cedes the hardware IA32_SPEC_CTRL
> > > to the guest after the first non-zero write to the guest's MSR.
> >
> > Yes, KVM doesn't support it yet. But, adding that support to give more
> > control to userspace VMM helps this case, and probably many other in
> > the future.
>
> But didn't you tell me that Windows doesn't want the hypervisor to set
> BHI_DIS_S behind their back?
Since cloud providers have greater control over userspace, the decision to
use BHI_DIS_S or not can be left to them. KVM would simply follow what it
is asked to do by the userspace.
> > I will check with Chao if he can prepare the next version of virtual
> > SPEC_CTRL series (leaving out virtual mitigation MSRs).
>
> Excellent.
^ permalink raw reply
* Re: [PATCH v9 02/10] x86/bhi: Make clear_bhb_loop() effective on newer CPUs
From: Jim Mattson @ 2026-04-03 23:39 UTC (permalink / raw)
To: Pawan Gupta
Cc: x86, Jon Kohler, Nikolay Borisov, H. Peter Anvin, Josh Poimboeuf,
David Kaplan, Sean Christopherson, Borislav Petkov, Dave Hansen,
Peter Zijlstra, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, KP Singh, Jiri Olsa, David S. Miller,
David Laight, Andy Lutomirski, Thomas Gleixner, Ingo Molnar,
David Ahern, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, John Fastabend, Stanislav Fomichev, Hao Luo,
Paolo Bonzini, Jonathan Corbet, linux-kernel, kvm, Asit Mallick,
Tao Zhang, bpf, netdev, linux-doc, chao.gao
In-Reply-To: <20260403233329.fb2ppifgwm3um6ny@desk>
On Fri, Apr 3, 2026 at 4:33 PM Pawan Gupta
<pawan.kumar.gupta@linux.intel.com> wrote:
>
> On Fri, Apr 03, 2026 at 04:22:28PM -0700, Jim Mattson wrote:
> > On Fri, Apr 3, 2026 at 4:16 PM Pawan Gupta
> > <pawan.kumar.gupta@linux.intel.com> wrote:
> > >
> > > On Fri, Apr 03, 2026 at 02:59:33PM -0700, Jim Mattson wrote:
> > > > On Fri, Apr 3, 2026 at 2:34 PM Pawan Gupta
> > > > <pawan.kumar.gupta@linux.intel.com> wrote:
> > > > >
> > > > > On Fri, Apr 03, 2026 at 01:19:17PM -0700, Jim Mattson wrote:
> > > > > > On Fri, Apr 3, 2026 at 11:52 AM Pawan Gupta
> > > > > > <pawan.kumar.gupta@linux.intel.com> wrote:
> > > > > > >
> > > > > > > On Fri, Apr 03, 2026 at 11:10:08AM -0700, Jim Mattson wrote:
> > > > > > > > On Thu, Apr 2, 2026 at 5:32 PM Pawan Gupta
> > > > > > > > <pawan.kumar.gupta@linux.intel.com> wrote:
> > > > > > > > >
> > > > > > > > > As a mitigation for BHI, clear_bhb_loop() executes branches that overwrite
> > > > > > > > > the Branch History Buffer (BHB). On Alder Lake and newer parts this
> > > > > > > > > sequence is not sufficient because it doesn't clear enough entries. This
> > > > > > > > > was not an issue because these CPUs use the BHI_DIS_S hardware mitigation
> > > > > > > > > in the kernel.
> > > > > > > > >
> > > > > > > > > Now with VMSCAPE (BHI variant) it is also required to isolate branch
> > > > > > > > > history between guests and userspace. Since BHI_DIS_S only protects the
> > > > > > > > > kernel, the newer CPUs also use IBPB.
> > > > > > > > >
> > > > > > > > > A cheaper alternative to the current IBPB mitigation is clear_bhb_loop().
> > > > > > > > > But it currently does not clear enough BHB entries to be effective on newer
> > > > > > > > > CPUs with larger BHB. At boot, dynamically set the loop count of
> > > > > > > > > clear_bhb_loop() such that it is effective on newer CPUs too. Use the
> > > > > > > > > X86_FEATURE_BHI_CTRL feature flag to select the appropriate loop count.
> > > > > > > > >
> > > > > > > > > Suggested-by: Dave Hansen <dave.hansen@linux.intel.com>
> > > > > > > > > Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
> > > > > > > > > ---
> > > > > > > > > arch/x86/entry/entry_64.S | 8 +++++---
> > > > > > > > > arch/x86/include/asm/nospec-branch.h | 2 ++
> > > > > > > > > arch/x86/kernel/cpu/bugs.c | 13 +++++++++++++
> > > > > > > > > 3 files changed, 20 insertions(+), 3 deletions(-)
> > > > > > > > >
> > > > > > > > > diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
> > > > > > > > > index 3a180a36ca0e..bbd4b1c7ec04 100644
> > > > > > > > > --- a/arch/x86/entry/entry_64.S
> > > > > > > > > +++ b/arch/x86/entry/entry_64.S
> > > > > > > > > @@ -1536,7 +1536,9 @@ SYM_FUNC_START(clear_bhb_loop)
> > > > > > > > > ANNOTATE_NOENDBR
> > > > > > > > > push %rbp
> > > > > > > > > mov %rsp, %rbp
> > > > > > > > > - movl $5, %ecx
> > > > > > > > > +
> > > > > > > > > + movzbl bhb_seq_outer_loop(%rip), %ecx
> > > > > > > > > +
> > > > > > > > > ANNOTATE_INTRA_FUNCTION_CALL
> > > > > > > > > call 1f
> > > > > > > > > jmp 5f
> > > > > > > > > @@ -1556,8 +1558,8 @@ SYM_FUNC_START(clear_bhb_loop)
> > > > > > > > > * This should be ideally be: .skip 32 - (.Lret2 - 2f), 0xcc
> > > > > > > > > * but some Clang versions (e.g. 18) don't like this.
> > > > > > > > > */
> > > > > > > > > - .skip 32 - 18, 0xcc
> > > > > > > > > -2: movl $5, %eax
> > > > > > > > > + .skip 32 - 20, 0xcc
> > > > > > > > > +2: movzbl bhb_seq_inner_loop(%rip), %eax
> > > > > > > > > 3: jmp 4f
> > > > > > > > > nop
> > > > > > > > > 4: sub $1, %eax
> > > > > > > > > diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
> > > > > > > > > index 70b377fcbc1c..87b83ae7c97f 100644
> > > > > > > > > --- a/arch/x86/include/asm/nospec-branch.h
> > > > > > > > > +++ b/arch/x86/include/asm/nospec-branch.h
> > > > > > > > > @@ -548,6 +548,8 @@ DECLARE_PER_CPU(u64, x86_spec_ctrl_current);
> > > > > > > > > extern void update_spec_ctrl_cond(u64 val);
> > > > > > > > > extern u64 spec_ctrl_current(void);
> > > > > > > > >
> > > > > > > > > +extern u8 bhb_seq_inner_loop, bhb_seq_outer_loop;
> > > > > > > > > +
> > > > > > > > > /*
> > > > > > > > > * With retpoline, we must use IBRS to restrict branch prediction
> > > > > > > > > * before calling into firmware.
> > > > > > > > > diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> > > > > > > > > index 83f51cab0b1e..2cb4a96247d8 100644
> > > > > > > > > --- a/arch/x86/kernel/cpu/bugs.c
> > > > > > > > > +++ b/arch/x86/kernel/cpu/bugs.c
> > > > > > > > > @@ -2047,6 +2047,10 @@ enum bhi_mitigations {
> > > > > > > > > static enum bhi_mitigations bhi_mitigation __ro_after_init =
> > > > > > > > > IS_ENABLED(CONFIG_MITIGATION_SPECTRE_BHI) ? BHI_MITIGATION_AUTO : BHI_MITIGATION_OFF;
> > > > > > > > >
> > > > > > > > > +/* Default to short BHB sequence values */
> > > > > > > > > +u8 bhb_seq_outer_loop __ro_after_init = 5;
> > > > > > > > > +u8 bhb_seq_inner_loop __ro_after_init = 5;
> > > > > > > > > +
> > > > > > > > > static int __init spectre_bhi_parse_cmdline(char *str)
> > > > > > > > > {
> > > > > > > > > if (!str)
> > > > > > > > > @@ -3242,6 +3246,15 @@ void __init cpu_select_mitigations(void)
> > > > > > > > > x86_spec_ctrl_base &= ~SPEC_CTRL_MITIGATIONS_MASK;
> > > > > > > > > }
> > > > > > > > >
> > > > > > > > > + /*
> > > > > > > > > + * Switch to long BHB clear sequence on newer CPUs (with BHI_CTRL
> > > > > > > > > + * support), see Intel's BHI guidance.
> > > > > > > > > + */
> > > > > > > > > + if (cpu_feature_enabled(X86_FEATURE_BHI_CTRL)) {
> > > > > > > > > + bhb_seq_outer_loop = 12;
> > > > > > > > > + bhb_seq_inner_loop = 7;
> > > > > > > > > + }
> > > > > > > > > +
> > > > > > > >
> > > > > > > > How does this work for VMs in a heterogeneous migration pool that
> > > > > > > > spans the Alder Lake boundary? They can't advertise BHI_CTRL, because
> > > > > > > > it isn't available on all hosts in the migration pool, but they need
> > > > > > > > the long sequence when running on Alder Lake or newer.
> > > > > > >
> > > > > > > As we discussed elsewhere, support for migration pool is much more
> > > > > > > involved. It should be dealt in a separate QEMU/KVM focused series.
> > > > > > >
> > > > > > > A quickfix could be adding support for spectre_bhi=long that guests in a
> > > > > > > migration pool can use?
> > > > > >
> > > > > > The simplest solution is to add "|
> > > > > > cpu_feature_enabled(X86_FEATURE_HYPERVISOR)" to the condition above.
> > > > > > If that is unacceptable for the performance of pre-Alder Lake
> > > > >
> > > > > Yes, that would be unnecessary overhead.
> > > > >
> > > > > > migration pools, you could define a CPUID or MSR bit that says
> > > > > > explicitly, "long BHB flush sequence needed," rather than trying to
> > > > > > intuit that property from the presence of BHI_CTRL. Like
> > > > > > IA32_ARCH_CAPABILITIES.SKIP_L1DFL_VMENTRY, the bit would only be set
> > > > > > by a hypervisor.
> > > > >
> > > > > I will think about this more.
> > > > >
> > > > > > I am still skeptical of the need for MSR_VIRTUAL_ENUMERATION and
> > > > > > friends, unless there is a major guest OS out there that relies on
> > > > > > them.
> > > > >
> > > > > If we forget about MSR_VIRTUAL_ENUMERATION for a moment, userspace VMM is
> > > > > in the best position to decide whether a guest needs
> > > > > virtual.SPEC_CTRL[BHI_DIS_S]. Via a KVM interface userspace VMM can get
> > > > > BHI_DIS_S for the guests that are in migration pool?
> > > >
> > > > That is not possible today, since KVM does not implement Intel's
> > > > IA32_SPEC_CTRL virtualization, and cedes the hardware IA32_SPEC_CTRL
> > > > to the guest after the first non-zero write to the guest's MSR.
> > >
> > > Yes, KVM doesn't support it yet. But, adding that support to give more
> > > control to userspace VMM helps this case, and probably many other in
> > > the future.
> >
> > But didn't you tell me that Windows doesn't want the hypervisor to set
> > BHI_DIS_S behind their back?
>
> Since cloud providers have greater control over userspace, the decision to
> use BHI_DIS_S or not can be left to them. KVM would simply follow what it
> is asked to do by the userspace.
I feel like we've gone over this before, but if userspace tells KVM
not to enable BHI_DIS_S, how do we inform Windows that it needs to do
the longer clearing sequence, despite the fact that the virtual CPU is
masquerading as Ice Lake?
I don't think the virtual mitigation MSRs address that issue.
> > > I will check with Chao if he can prepare the next version of virtual
> > > SPEC_CTRL series (leaving out virtual mitigation MSRs).
> >
> > Excellent.
^ permalink raw reply
* Re: [PATCH net-next] selftests/net: convert so_txtime to drv-net
From: Willem de Bruijn @ 2026-04-03 23:54 UTC (permalink / raw)
To: Jakub Kicinski, Willem de Bruijn
Cc: netdev, davem, edumazet, pabeni, horms, Willem de Bruijn
In-Reply-To: <20260403132855.7616daee@kernel.org>
Jakub Kicinski wrote:
> On Fri, 3 Apr 2026 13:50:12 -0400 Willem de Bruijn wrote:
> > From: Willem de Bruijn <willemb@google.com>
> >
> > In preparation for extending to pacing hardware offload, convert the
> > so_txtime.sh test to a drv-net test that can be run against netdevsim
> > and real hardware.
> >
> > Also move so_txtime.c to lib so that it is easily accessible for the
> > new drv-net test, similar to gro.c in commit 8888bf4fb980 ("selftests:
> > net: move gro to lib for HW vs SW reuse").
> >
> > Also update so_txtime.c to not exit on first failure, but run to
> > completion and report exit code there. This helps with debugging
> > unexpected results, especially when processing multiple packets,
> > as in the "reverse_order" testcase.
>
> > diff --git a/tools/testing/selftests/drivers/net/config b/tools/testing/selftests/drivers/net/config
> > index 77ccf83d87e0..a3301d4da540 100644
> > --- a/tools/testing/selftests/drivers/net/config
> > +++ b/tools/testing/selftests/drivers/net/config
> > @@ -3,6 +3,8 @@ CONFIG_DEBUG_INFO_BTF=y
> > CONFIG_DEBUG_INFO_BTF_MODULES=n
> > CONFIG_INET_PSP=y
> > CONFIG_IPV6=y
> > +CONFIG_NET_SCH_ETF=m
> > +CONFIG_NET_SCH_FQ=m
>
> I think our sort order puts _ after characters
>
> > CONFIG_NETCONSOLE=m
> > CONFIG_NETCONSOLE_DYNAMIC=y
> > CONFIG_NETCONSOLE_EXTENDED_LOG=y
> > diff --git a/tools/testing/selftests/drivers/net/so_txtime.py b/tools/testing/selftests/drivers/net/so_txtime.py
> > new file mode 100755
> > index 000000000000..565cecf307dd
> > --- /dev/null
> > +++ b/tools/testing/selftests/drivers/net/so_txtime.py
> > @@ -0,0 +1,87 @@
> > +#!/usr/bin/env python3
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +"""Regression tests for the SO_TXTIME interface.
> > +
> > +Test delivery time in FQ and ETF qdiscs.
> > +"""
>
> > +import time
> > +
> > +from lib.py import ksft_exit, ksft_run
> > +from lib.py import KsftNamedVariant, KsftSkipEx, ksft_variants
> > +from lib.py import NetDrvEpEnv, bkg, cmd
> > +
> > +
> > +def test_so_txtime(cfg, clockid, ipver, args_tx, args_rx, expect_fail):
> > + bin_path = cfg.net_lib_dir / "so_txtime"
> > +
> > + tstart = time.time_ns() + 100_000_000
> > +
> > + cmd_addr = f"-S {cfg.addr_v[ipver]} -D {cfg.remote_addr_v[ipver]}"
> > + cmd_base = f"{bin_path} -{ipver} -c {clockid} -t {tstart} {cmd_addr}"
> > + cmd_rx = f"{cmd_base} {args_rx} -r"
> > + cmd_tx = f"{cmd_base} {args_tx}"
> > +
> > + try:
> > + with bkg(cmd_rx, host=cfg.remote, exit_wait=True):
> > + cmd(cmd_tx)
> > + except:
> > + if not expect_fail:
> > + raise
>
> ruff check says:
> tools/testing/selftests/drivers/net/so_txtime.py:29: [E722] Do not use bare `except`
>
> Why not pass fail=(not expect_fail) to bkg ?
That's indeed a lot simpler.
Thanks for the feedback! Will include all these changes in v2.
^ permalink raw reply
* Re: [PATCH v6 net 3/8] xsk: fix XDP_UMEM_SG_FLAG issues
From: Jakub Kicinski @ 2026-04-03 23:56 UTC (permalink / raw)
To: Maciej Fijalkowski
Cc: netdev, bpf, magnus.karlsson, stfomichev, pabeni, horms,
larysa.zaremba, aleksander.lobakin, bjorn
In-Reply-To: <20260402154958.562179-4-maciej.fijalkowski@intel.com>
On Thu, 2 Apr 2026 17:49:53 +0200 Maciej Fijalkowski wrote:
> Currently xp_assign_dev_shared() is missing XDP_USE_SG being propagated
> to flags so set it in order to preserve mtu check that is supposed to be
> done only when no multi-buffer setup is in picture.
>
> Also, this flag has the same value as XDP_UMEM_TX_SW_CSUM so we could
> get unexpected SG setups for software Tx checksums. Since csum flag is
> UAPI, modify value of XDP_UMEM_SG_FLAG.
should we maybe add:
BUILD_BUG_ON(XDP_UMEM_SG_FLAG & XDP_UMEM_FLAGS_VALID)
somewhere or use a hack like this ?
#define XDP_UMEM_SG_FLAG (XDP_UMEM_FLAGS_VALID + 1)
> diff --git a/include/net/xdp_sock.h b/include/net/xdp_sock.h
> index 23e8861e8b25..ebac60a3d8a1 100644
> --- a/include/net/xdp_sock.h
> +++ b/include/net/xdp_sock.h
> @@ -14,7 +14,7 @@
> #include <linux/mm.h>
> #include <net/sock.h>
>
> -#define XDP_UMEM_SG_FLAG (1 << 1)
> +#define XDP_UMEM_SG_FLAG BIT(3)
>
> struct net_device;
> struct xsk_queue;
> diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
> index 37b7a68b89b3..729602a3cec0 100644
> --- a/net/xdp/xsk_buff_pool.c
> +++ b/net/xdp/xsk_buff_pool.c
> @@ -247,6 +247,10 @@ int xp_assign_dev_shared(struct xsk_buff_pool *pool, struct xdp_sock *umem_xs,
> struct xdp_umem *umem = umem_xs->umem;
>
> flags = umem->zc ? XDP_ZEROCOPY : XDP_COPY;
> +
> + if (umem->flags & XDP_UMEM_SG_FLAG)
> + flags |= XDP_USE_SG;
> +
> if (umem_xs->pool->uses_need_wakeup)
> flags |= XDP_USE_NEED_WAKEUP;
>
^ permalink raw reply
* Re: [PATCH v6 net 1/8] xsk: tighten UMEM headroom validation to account for tailroom and min frame
From: Jakub Kicinski @ 2026-04-04 0:01 UTC (permalink / raw)
To: Maciej Fijalkowski
Cc: netdev, bpf, magnus.karlsson, stfomichev, pabeni, horms,
larysa.zaremba, aleksander.lobakin, bjorn, Stanislav Fomichev
In-Reply-To: <20260402154958.562179-2-maciej.fijalkowski@intel.com>
On Thu, 2 Apr 2026 17:49:51 +0200 Maciej Fijalkowski wrote:
> The current headroom validation in xdp_umem_reg() could leave us with
> insufficient space dedicated to even receive minimum-sized ethernet
> frame. Furthermore if multi-buffer would come to play then
> skb_shared_info stored at the end of XSK frame would be corrupted.
>
> HW typically works with 128-aligned sizes so let us provide this value
> as bare minimum.
>
> Multi-buffer setting is known later in the configuration process so
> besides accounting for 128 bytes, let us also take care of tailroom space
> upfront.
I guess it's a bit late to ask this, but am I the only one that has
weird feelings about shinfo living in user-mapped memory?
^ permalink raw reply
* Re: [PATCH v6 net 4/8] xsk: validate MTU against usable frame size on bind
From: Jakub Kicinski @ 2026-04-04 0:03 UTC (permalink / raw)
To: Maciej Fijalkowski
Cc: netdev, bpf, magnus.karlsson, stfomichev, pabeni, horms,
larysa.zaremba, aleksander.lobakin, bjorn
In-Reply-To: <20260402154958.562179-5-maciej.fijalkowski@intel.com>
On Thu, 2 Apr 2026 17:49:54 +0200 Maciej Fijalkowski wrote:
> AF_XDP bind currently accepts zero-copy pool configurations without
> verifying that the device MTU fits into the usable frame space provided
> by the UMEM chunk.
>
> This becomes a problem since we started to respect tailroom which is
> subtracted from chunk_size (among with headroom). 2k chunk size might
> not provide enough space for standard 1500 MTU, so let us catch such
> settings at bind time. Furthermore, validate whether underlying HW will
> be able to satisfy configured MTU wrt XSK's frame size multiplied by
> supported Rx buffer chain length (that is exposed via
> net_device::xdp_zc_max_segs).
For multi-buf - I suppose it's fine, but wouldn't this be potentially
a regression for setups which use single buf? Suddenly we expect the
user to provide space for shinfo which was not part of the initial
AF_XDP design?
^ permalink raw reply
* Re: [PATCH net-next v5 01/14] net: add address list snapshot and reconciliation infrastructure
From: Jakub Kicinski @ 2026-04-04 0:06 UTC (permalink / raw)
To: sdf; +Cc: Jakub Kicinski, netdev, davem, edumazet, pabeni, jiri, jv
In-Reply-To: <20260402225535.4124525-2-sdf@fomichev.me>
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: add address list snapshot and reconciliation infrastructure
This commit introduces address list snapshot and reconciliation routines
to support the async rx_mode path. It provides functions to snapshot the
lists, and to reconcile the hardware sync state back to the original lists
after driver processing.
> diff --git a/net/core/dev_addr_lists.c b/net/core/dev_addr_lists.c
> --- a/net/core/dev_addr_lists.c
> +++ b/net/core/dev_addr_lists.c
[ ... ]
> @@ -501,6 +503,114 @@ void __hw_addr_init(struct netdev_hw_addr_list *list)
[ ... ]
> +void __hw_addr_list_reconcile(struct netdev_hw_addr_list *real_list,
> + struct netdev_hw_addr_list *work,
> + struct netdev_hw_addr_list *ref, int addr_len)
> +{
[ ... ]
> + real_ha = __hw_addr_lookup(real_list, ref_ha->addr, addr_len,
> + ref_ha->type);
> + if (!real_ha) {
> + /* The real entry was concurrently removed. If the
> + * driver synced this addr to hardware (delta > 0),
> + * re-insert it as a stale entry so the next work
> + * run unsyncs it from hardware.
> + */
> + if (delta > 0) {
> + real_ha = __hw_addr_create(ref_ha->addr,
> + addr_len,
> + ref_ha->type, false,
> + false);
Does this code silently leak a hardware MAC filter if the GFP_ATOMIC allocation
fails? If the software loses track of this address, it appears future sync runs
will never unsync it from the hardware.
Could this failure be avoided by unlinking ref_ha from the ref list (which is
about to be unconditionally freed via __hw_addr_flush) and moving it directly
into real_list?
> + if (real_ha) {
> + real_ha->sync_cnt = 1;
> + real_ha->refcount = 1;
Does this code break compatibility with reference-counted sync APIs?
While __hw_addr_sync_dev() increments by 1, drivers processing snapshots with
__hw_addr_ref_sync_dev() might increment sync_cnt by the total number of
references. If such an entry is concurrently removed, recreation with a
hardcoded value of 1 could instruct the next async run to unsync only 1
reference, permanently leaking the remaining hardware references.
Should these be dynamically initialized to real_ha->sync_cnt = delta and
real_ha->refcount = delta?
> + list_add_tail_rcu(&real_ha->list,
> + &real_list->list);
> + __hw_addr_insert(real_list, real_ha,
> + addr_len);
> + real_list->count++;
> + }
> + }
> + continue;
> + }
[ ... ]
> diff --git a/net/core/dev_addr_lists_test.c b/net/core/dev_addr_lists_test.c
> --- a/net/core/dev_addr_lists_test.c
> +++ b/net/core/dev_addr_lists_test.c
[ ... ]
> @@ -225,6 +245,339 @@ static void dev_addr_test_add_excl(struct kunit *test)
[ ... ]
> +static void dev_addr_test_snapshot_sync(struct kunit *test)
> +{
[ ... ]
> + netif_addr_lock_bh(netdev);
> + __hw_addr_init(&snap);
> + __hw_addr_init(&ref);
> + KUNIT_ASSERT_EQ(test, 0,
> + __hw_addr_list_snapshot(&snap, &netdev->uc, ETH_ALEN));
> + KUNIT_ASSERT_EQ(test, 0,
> + __hw_addr_list_snapshot(&ref, &netdev->uc, ETH_ALEN));
> + netif_addr_unlock_bh(netdev);
Will this cause a system deadlock if the assertion fails?
If KUNIT_ASSERT_EQ triggers (e.g. from an out-of-memory condition in
__hw_addr_list_snapshot), it immediately aborts the test case thread and skips
the rest of the function, leaving the netif_addr_lock_bh spinlock permanently
held and bottom-halves disabled.
Could this be replaced with KUNIT_EXPECT_EQ and explicit goto error handling
to ensure netif_addr_unlock_bh is always reached?
[ ... ]
> +static void dev_addr_test_snapshot_benchmark(struct kunit *test)
> +{
[ ... ]
> + for (iter = 0; iter < 1000; iter++) {
> + netif_addr_lock_bh(netdev);
> + __hw_addr_init(&snap);
> +
> + start = ktime_get();
> + KUNIT_ASSERT_EQ(test, 0,
> + __hw_addr_list_snapshot(&snap, &netdev->uc,
> + ETH_ALEN));
> + duration += ktime_to_ns(ktime_sub(ktime_get(), start));
> +
> + netif_addr_unlock_bh(netdev);
Will this also cause a deadlock if the assertion fails while the lock is held?
--
pw-bot: cr
^ permalink raw reply
* Re: [PATCH net-next v5 02/14] net: introduce ndo_set_rx_mode_async and netdev_rx_mode_work
From: Jakub Kicinski @ 2026-04-04 0:06 UTC (permalink / raw)
To: sdf; +Cc: Jakub Kicinski, netdev, davem, edumazet, pabeni, jiri, jv
In-Reply-To: <20260402225535.4124525-3-sdf@fomichev.me>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
Subject: net: introduce ndo_set_rx_mode_async and netdev_rx_mode_work
This commit introduces an async variant of ndo_set_rx_mode that runs from a
workqueue, allowing drivers to sleep during RX mode changes. It also adds
netif_rx_mode_sync() to opportunistically flush pending updates before
returning to userspace.
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
[ ... ]
> @@ -12217,6 +12178,9 @@ void free_netdev(struct net_device *dev)
>
> kfree(rcu_dereference_protected(dev->ingress_queue, 1));
>
> + if (netif_rx_mode_clean(dev))
> + dev_put(dev);
> +
> /* Flush device addresses */
> dev_addr_flush(dev);
Is this cleanup reachable during normal shutdown?
When a device is queued to rx_mode_list, netif_rx_mode_queue() elevates its
reference count via dev_hold(). During unregistration, netdev_wait_allrefs()
blocks the system until all references are dropped.
Because of this wait, the system shouldn't be able to reach free_netdev()
until the worker thread has already dequeued the device and called
dev_put(). It seems the device is guaranteed to not be on the list by the
time this code executes.
> --- a/net/core/dev_addr_lists.c
> +++ b/net/core/dev_addr_lists.c
[ ... ]
> +static void netdev_rx_mode_work(struct work_struct *work)
> +{
> + struct net_device *dev;
> +
> + rtnl_lock();
> +
> + while (true) {
> + spin_lock_bh(&rx_mode_lock);
> + if (list_empty(&rx_mode_list)) {
> + spin_unlock_bh(&rx_mode_lock);
> + break;
> + }
> + dev = list_first_entry(&rx_mode_list, struct net_device,
> + rx_mode_node);
> + list_del_init(&dev->rx_mode_node);
> + spin_unlock_bh(&rx_mode_lock);
> +
> + netdev_lock_ops(dev);
Can this while loop starve the system of the rtnl lock?
Since userspace can trigger RX mode updates via ioctls like SIOCADDMULTI
while holding only netdev_lock_ops() and not the rtnl lock, a continuous
stream of these operations could repeatedly add devices to rx_mode_list.
Because the loop releases rx_mode_lock while processing but retains the rtnl
lock, could it run indefinitely if the list is continually replenished?
Additionally, does the order of list_del_init() and netdev_lock_ops() create
a race condition with netif_rx_mode_sync()?
If the worker thread dequeues the device and drops the spinlock, it might
then block waiting for netdev_lock_ops(). Meanwhile, an ioctl thread
holding netdev_lock_ops() might call netif_rx_mode_sync().
The ioctl thread would call netif_rx_mode_clean(), which checks
list_empty(). Since the worker already removed the device from the list,
clean returns false. This causes netif_rx_mode_sync() to act as a no-op,
allowing the ioctl to return to userspace before the hardware update
actually completes.
> --- a/net/core/dev_api.c
> +++ b/net/core/dev_api.c
[ ... ]
> @@ -311,6 +313,7 @@ int dev_set_allmulti(struct net_device *dev, int inc)
>
> netdev_lock_ops(dev);
> ret = netif_set_allmulti(dev, inc, true);
> + netif_rx_mode_sync(dev);
> netdev_unlock_ops(dev);
>
> return ret;
Does this correctly handle cascaded updates for stacked devices?
When operating on a stacked device like a VLAN, the RX mode update cascades
to the underlying physical device, queuing the physical device to the
rx_mode_list via __dev_set_rx_mode().
Since netif_rx_mode_sync() is only called on the stacked device here, and
the stacked device itself is not on the rx_mode_list, will the physical
device's update be left on the workqueue? This seems to bypass the
synchronous update guarantee.
^ permalink raw reply
* Re: [PATCH net-next v5 03/14] net: move promiscuity handling into netdev_rx_mode_work
From: Jakub Kicinski @ 2026-04-04 0:06 UTC (permalink / raw)
To: sdf; +Cc: Jakub Kicinski, netdev, davem, edumazet, pabeni, jiri, jv
In-Reply-To: <20260402225535.4124525-4-sdf@fomichev.me>
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: move promiscuity handling into netdev_rx_mode_work
This commit moves unicast promiscuity tracking into netdev_rx_mode_work so
it runs under netdev_ops_lock instead of the addr_lock spinlock. It also
changes ASSERT_RTNL() to netdev_ops_assert_locked() in several functions
called from the work queue.
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 0efac0c730077..08dac9c759996 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -9586,7 +9586,7 @@ int __dev_set_promiscuity(struct net_device *dev, int inc, bool notify)
> kuid_t uid;
> kgid_t gid;
>
> - ASSERT_RTNL();
> + netdev_ops_assert_locked(dev);
Can this cause a lockdep assertion failure for unconverted ops-locked devices
using the legacy RX mode path?
Replacing ASSERT_RTNL() with netdev_ops_assert_locked(dev) asserts that
dev->lock is held for devices where netdev_need_ops_lock(dev) is true.
[ ... ]
> diff --git a/net/core/dev_addr_lists.c b/net/core/dev_addr_lists.c
> index a05db3ac43ea9..61589537b2d3d 100644
> --- a/net/core/dev_addr_lists.c
> +++ b/net/core/dev_addr_lists.c
[ ... ]
> @@ -1295,20 +1337,16 @@ void __dev_set_rx_mode(struct net_device *dev)
> if (!netif_device_present(dev))
> return;
>
> - if (ops->ndo_set_rx_mode_async) {
> + if (ops->ndo_set_rx_mode_async || ops->ndo_change_rx_flags) {
> netif_rx_mode_queue(dev);
> return;
> }
>
> - if (!(dev->priv_flags & IFF_UNICAST_FLT)) {
> - if (!netdev_uc_empty(dev) && !dev->uc_promisc) {
> - __dev_set_promiscuity(dev, 1, false);
> - dev->uc_promisc = true;
> - } else if (netdev_uc_empty(dev) && dev->uc_promisc) {
> - __dev_set_promiscuity(dev, -1, false);
> - dev->uc_promisc = false;
> - }
> - }
> + /* Legacy path for non-ops-locked HW devices. */
> +
> + promisc_inc = netif_uc_promisc_update(dev);
> + if (promisc_inc)
> + __dev_set_promiscuity(dev, promisc_inc, false);
If an ops-locked device has not yet been converted to implement the new
ndo_set_rx_mode_async or ndo_change_rx_flags callbacks, this legacy path
will be executed.
When a secondary unicast address is added to such a device (e.g., by
bringing up a macvlan interface over it) and the device lacks
IFF_UNICAST_FLT, netif_uc_promisc_update() returns 1. This triggers a
synchronous inline call to __dev_set_promiscuity(dev, 1, false).
Since __dev_set_rx_mode() is invoked from address list helpers like
dev_uc_add() which only hold rtnl_lock() and netif_addr_lock_bh(dev),
dev->lock is not held, which would trigger the new assertion in
__dev_set_promiscuity().
^ permalink raw reply
* Re: [PATCH net-next v5 05/14] fbnic: convert to ndo_set_rx_mode_async
From: Jakub Kicinski @ 2026-04-04 0:06 UTC (permalink / raw)
To: sdf; +Cc: Jakub Kicinski, netdev, davem, edumazet, pabeni, jiri, jv
In-Reply-To: <20260402225535.4124525-6-sdf@fomichev.me>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
fbnic: convert to ndo_set_rx_mode_async
Convert fbnic from ndo_set_rx_mode to ndo_set_rx_mode_async, updating
the driver to take explicit uc/mc lists and using __hw_addr_sync_dev()
on the snapshots instead of __dev_uc_sync() on the netdev directly.
> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
> index b4b396ca9bce3..c406a3b56b37f 100644
> --- a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
[ ... ]
> @@ -213,10 +215,10 @@ void __fbnic_set_rx_mode(struct fbnic_dev *fbd)
> }
>
> /* Synchronize unicast and multicast address lists */
> - err = __dev_uc_sync(netdev, fbnic_uc_sync, fbnic_uc_unsync);
> + err = __hw_addr_sync_dev(uc, netdev, fbnic_uc_sync, fbnic_uc_unsync);
> if (err == -ENOSPC)
> uc_promisc = true;
> - err = __dev_mc_sync(netdev, fbnic_mc_sync, fbnic_mc_unsync);
> + err = __hw_addr_sync_dev(mc, netdev, fbnic_mc_sync, fbnic_mc_unsync);
Since __fbnic_set_rx_mode() performs read-modify-write operations on the
internal fbd->mac_addr array and updates the device TCAM, is there a need
for internal synchronization?
It appears that concurrent out-of-band callers like fbnic_bmc_rpc_check()
(from an unlocked workqueue), RTNL paths like fbnic_set_mac(), and the async
RX mode workqueue can race on the internal fbd->mac_addr array and interleave
writes to the hardware, potentially breaking the RX filtering rules.
> if (err == -ENOSPC)
> mc_promisc = true;
[ ... ]
> @@ -257,7 +262,8 @@ static int fbnic_set_mac(struct net_device *netdev, void *p)
>
> eth_hw_addr_set(netdev, addr->sa_data);
>
> - fbnic_set_rx_mode(netdev);
> + if (netif_running(netdev))
> + __fbnic_set_rx_mode(fbn->fbd, &netdev->uc, &netdev->mc);
Is it safe to pass the live &netdev->uc and &netdev->mc address lists to
__hw_addr_sync_dev() without holding netif_addr_lock_bh() or rtnl_lock()?
This bypasses the core network stack's lock, which could lead to list
corruption and use-after-free if the network stack concurrently modifies
the address lists (e.g., via dev_mc_add()).
Additionally, doesn't manually mutating sync_cnt on the live lists bypass
the core's snapshot reconciliation state machine?
>
> return 0;
> }
> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_pci.c b/drivers/net/ethernet/meta/fbnic/fbnic_pci.c
> index e3aebbe3656d5..6b139cf542567 100644
> --- a/drivers/net/ethernet/meta/fbnic/fbnic_pci.c
> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_pci.c
[ ... ]
> @@ -180,7 +180,7 @@ static int fbnic_fw_config_after_crash(struct fbnic_dev *fbd)
> }
>
> fbnic_rpc_reset_valid_entries(fbd);
> - __fbnic_set_rx_mode(fbd);
> + __fbnic_set_rx_mode(fbd, &fbd->netdev->uc, &fbd->netdev->mc);
Similarly here, fbnic_fw_config_after_crash() appears to be executed from the
unlocked fbnic_service_task workqueue. Could this cause lockless data races on
the live network stack lists?
>
> return 0;
> }
> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_rpc.c b/drivers/net/ethernet/meta/fbnic/fbnic_rpc.c
> index 42a186db43ea9..fe95b6f696463 100644
> --- a/drivers/net/ethernet/meta/fbnic/fbnic_rpc.c
> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_rpc.c
[ ... ]
> @@ -244,7 +244,7 @@ void fbnic_bmc_rpc_check(struct fbnic_dev *fbd)
>
> if (fbd->fw_cap.need_bmc_tcam_reinit) {
> fbnic_bmc_rpc_init(fbd);
> - __fbnic_set_rx_mode(fbd);
> + __fbnic_set_rx_mode(fbd, &fbd->netdev->uc, &fbd->netdev->mc);
And here as well, passing live address lists without synchronization.
^ permalink raw reply
* Re: [PATCH net-next v5 06/14] mlx5: convert to ndo_set_rx_mode_async
From: Jakub Kicinski @ 2026-04-04 0:06 UTC (permalink / raw)
To: sdf; +Cc: Jakub Kicinski, netdev, davem, edumazet, pabeni, jiri, jv
In-Reply-To: <20260402225535.4124525-7-sdf@fomichev.me>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
mlx5: convert to ndo_set_rx_mode_async
This commit converts the mlx5 driver to use ndo_set_rx_mode_async. It passes
the uc and mc snapshots directly to the internal work handlers instead of
queueing work and manually iterating the address lists.
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_fs.c b/drivers/net/ethernet/mellanox/mlx5/core/en_fs.c
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_fs.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_fs.c
> @@ -609,20 +609,26 @@ static void mlx5e_execute_l2_action(struct mlx5e_flow_steering *fs,
> }
>
> static void mlx5e_sync_netdev_addr(struct mlx5e_flow_steering *fs,
> - struct net_device *netdev)
> + struct net_device *netdev,
> + struct netdev_hw_addr_list *uc,
> + struct netdev_hw_addr_list *mc)
> {
> struct netdev_hw_addr *ha;
>
> - netif_addr_lock_bh(netdev);
> + if (!uc || !mc) {
> + netif_addr_lock_bh(netdev);
> + mlx5e_sync_netdev_addr(fs, netdev, &netdev->uc, &netdev->mc);
> + netif_addr_unlock_bh(netdev);
> + return;
> + }
>
> mlx5e_add_l2_to_hash(fs->l2.netdev_uc, netdev->dev_addr);
Can this lead to a torn read of the MAC address?
When uc and mc are provided, this path now reads netdev->dev_addr without
holding netif_addr_lock_bh().
Because ndo_set_rx_mode_async runs asynchronously in a workqueue, this read
could happen concurrently with a MAC address change in mlx5e_set_mac().
Since mlx5e_set_mac() updates the address using eth_hw_addr_set() under
netif_addr_lock_bh(), a concurrent read here might be torn and program a
corrupted MAC address into the hardware filters.
[ ... ]
^ permalink raw reply
* Re: [PATCH net-next v5 07/14] bnxt: convert to ndo_set_rx_mode_async
From: Jakub Kicinski @ 2026-04-04 0:06 UTC (permalink / raw)
To: sdf; +Cc: Jakub Kicinski, netdev, davem, edumazet, pabeni, jiri, jv
In-Reply-To: <20260402225535.4124525-8-sdf@fomichev.me>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
bnxt: convert to ndo_set_rx_mode_async
This commit converts the bnxt driver to use ndo_set_rx_mode_async, updating
the rx mode callbacks to take explicit list parameters and iterate over them.
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> index fe8b886ff82ea..569c7de7c5e44 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
[ ... ]
> @@ -13523,17 +13524,17 @@ void bnxt_get_ring_drv_stats(struct bnxt *bp,
> bnxt_get_one_ring_drv_stats(bp, stats, &bp->bnapi[i]->cp_ring);
> }
>
> -static bool bnxt_mc_list_updated(struct bnxt *bp, u32 *rx_mask)
> +static bool bnxt_mc_list_updated(struct bnxt *bp, u32 *rx_mask,
> + const struct netdev_hw_addr_list *mc)
> {
> struct bnxt_vnic_info *vnic = &bp->vnic_info[BNXT_VNIC_DEFAULT];
> - struct net_device *dev = bp->dev;
> struct netdev_hw_addr *ha;
> u8 *haddr;
> int mc_count = 0;
> bool update = false;
> int off = 0;
>
> - netdev_for_each_mc_addr(ha, dev) {
> + netdev_hw_addr_list_for_each(ha, mc) {
When iterating over the list, bnxt_mc_list_updated() performs a memcpy()
into vnic->mc_list.
If this executes asynchronously while bnxt_cfg_rx_mode() is mapping
vnic->mc_list for DMA to the firmware via bnxt_hwrm_cfa_l2_set_rx_mask(),
could the NIC hardware read torn or corrupted multicast MAC addresses?
Even if a later patch addresses the explicit list TOCTOU issue, is a
driver-level spinlock needed here to serialize vnic state access between
the two concurrent workqueues?
[ ... ]
> @@ -15881,7 +15884,7 @@ static const struct net_device_ops bnxt_netdev_ops = {
> .ndo_start_xmit = bnxt_start_xmit,
> .ndo_stop = bnxt_close,
> .ndo_get_stats64 = bnxt_get_stats64,
> - .ndo_set_rx_mode = bnxt_set_rx_mode,
> + .ndo_set_rx_mode_async = bnxt_set_rx_mode,
Since this callback now runs asynchronously without the netif_addr_lock_bh()
that the core networking stack previously held, does this introduce data races
on the driver state?
With bnxt_set_rx_mode() executing locklessly in the netdev_rx_mode_work
workqueue, and bnxt_cfg_rx_mode() concurrently modifying vnic->uc_list,
vnic->uc_filter_count, and vnic->rx_mask in the driver's bnxt_sp_task
workqueue, can they race when accessing the vnic state?
For example, could lockless assignments to vnic->rx_mask in bnxt_set_rx_mode()
temporarily drop hardware fallback states (like the PROMISCUOUS bit) set by
bnxt_cfg_rx_mode()?
> .ndo_eth_ioctl = bnxt_ioctl,
> .ndo_validate_addr = eth_validate_addr,
> .ndo_set_mac_address = bnxt_change_mac_addr,
^ permalink raw reply
* Re: [PATCH net-next v5 08/14] bnxt: use snapshot in bnxt_cfg_rx_mode
From: Jakub Kicinski @ 2026-04-04 0:06 UTC (permalink / raw)
To: sdf; +Cc: Jakub Kicinski, netdev, davem, edumazet, pabeni, jiri, jv
In-Reply-To: <20260402225535.4124525-9-sdf@fomichev.me>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
bnxt: use snapshot in bnxt_cfg_rx_mode
With the introduction of ndo_set_rx_mode_async we can call bnxt_cfg_rx_mode
directly. Convert bnxt_cfg_rx_mode to use uc/mc snapshots and move its
call in bnxt_sp_task to the section that resets BNXT_STATE_IN_SP_TASK.
Link: https://lore.kernel.org/netdev/CACKFLi=5vj8hPqEUKDd8RTw3au5G+zRgQEqjF+6NZnyoNm90KA@mail.gmail.com/
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
[ ... ]
> -static int bnxt_cfg_rx_mode(struct bnxt *bp)
> +static int bnxt_cfg_rx_mode(struct bnxt *bp, struct netdev_hw_addr_list *uc,
> + struct netdev_hw_addr_list *mc)
This isn't a bug, but is the mc parameter used in this function? The
multicast configuration appears to be handled synchronously prior to this call
by bnxt_mc_list_updated().
> {
> struct net_device *dev = bp->dev;
> struct bnxt_vnic_info *vnic = &bp->vnic_info[BNXT_VNIC_DEFAULT];
> struct netdev_hw_addr *ha;
> int i, off = 0, rc;
> - bool uc_update;
> -
> - netif_addr_lock_bh(dev);
> - uc_update = bnxt_uc_list_updated(bp, &dev->uc);
> - netif_addr_unlock_bh(dev);
> -
> - if (!uc_update)
> - goto skip_uc;
By removing this check, will the driver now unconditionally delete and recreate
all secondary hardware unicast MAC filters via HWRM commands on any RX mode
change?
For example, if an unprivileged user joins a multicast group via
IP_ADD_MEMBERSHIP, does this create a time window where the hardware drops
valid unicast traffic destined for secondary MACs?
Also, if any of the firmware commands to re-add a filter fail, will the
function return early and skip programming the RX mask entirely?
[ ... ]
> @@ -14684,6 +14675,12 @@ static void bnxt_sp_task(struct work_struct *work)
> /* These functions below will clear BNXT_STATE_IN_SP_TASK. They
> * must be the last functions to be called before exiting.
> */
> + if (test_and_clear_bit(BNXT_RX_MASK_SP_EVENT, &bp->sp_event)) {
> + bnxt_lock_sp(bp);
Could this introduce a race condition with bnxt_close()?
bnxt_lock_sp() temporarily clears the BNXT_STATE_IN_SP_TASK bit before
blocking on netdev_lock(bp->dev). If bnxt_close() is called concurrently, it
holds netdev_lock() and loops waiting for BNXT_STATE_IN_SP_TASK to clear.
When bnxt_lock_sp() clears the bit, bnxt_close() could unblock, shut down the
device, and free memory including bp->vnic_info via bnxt_free_vnics().
After bnxt_close() completes and releases the lock, bnxt_sp_task() would
acquire it and unconditionally call bnxt_cfg_rx_mode(), which dereferences the
now-freed bp->vnic_info.
Would it be safer to check test_bit(BNXT_STATE_OPEN, &bp->state) after
acquiring the lock, similar to what neighboring handlers do?
> + bnxt_cfg_rx_mode(bp, &dev->uc, &dev->mc);
> + bnxt_unlock_sp(bp);
> + }
^ permalink raw reply
* Re: [PATCH net-next v5 09/14] iavf: convert to ndo_set_rx_mode_async
From: Jakub Kicinski @ 2026-04-04 0:06 UTC (permalink / raw)
To: sdf; +Cc: Jakub Kicinski, netdev, davem, edumazet, pabeni, jiri, jv
In-Reply-To: <20260402225535.4124525-10-sdf@fomichev.me>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
iavf: convert to ndo_set_rx_mode_async
This commit converts the iavf driver from using ndo_set_rx_mode to
ndo_set_rx_mode_async. It updates iavf_set_rx_mode to take explicit
unicast and multicast list parameters and uses __hw_addr_sync_dev on
the snapshots instead of __dev_uc_sync and __dev_mc_sync.
> diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c
> --- a/drivers/net/ethernet/intel/iavf/iavf_main.c
> +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
> @@ -1150,14 +1150,18 @@ bool iavf_promiscuous_mode_changed(struct iavf_adapter *adapter)
> -static void iavf_set_rx_mode(struct net_device *netdev)
> +static void iavf_set_rx_mode(struct net_device *netdev,
> + struct netdev_hw_addr_list *uc,
> + struct netdev_hw_addr_list *mc)
> {
> struct iavf_adapter *adapter = netdev_priv(netdev);
>
> spin_lock_bh(&adapter->mac_vlan_list_lock);
> - __dev_uc_sync(netdev, iavf_addr_sync, iavf_addr_unsync);
> - __dev_mc_sync(netdev, iavf_addr_sync, iavf_addr_unsync);
> + __hw_addr_sync_dev(uc, netdev, iavf_addr_sync, iavf_addr_unsync);
> + __hw_addr_sync_dev(mc, netdev, iavf_addr_sync, iavf_addr_unsync);
> spin_unlock_bh(&adapter->mac_vlan_list_lock);
[ ... ]
> @@ -1210,7 +1214,7 @@ static void iavf_configure(struct iavf_adapter *adapter)
> struct net_device *netdev = adapter->netdev;
> int i;
>
> - iavf_set_rx_mode(netdev);
> + iavf_set_rx_mode(netdev, &netdev->uc, &netdev->mc);
Does this code iterate and mutate the live netdev address lists without
holding the required locks?
Previously, iavf_set_rx_mode() called __dev_uc_sync() and __dev_mc_sync(),
which internally acquire netif_addr_lock_bh() to protect the live netdev
address lists.
Now, iavf_configure() passes the live lists (&netdev->uc and &netdev->mc)
directly to iavf_set_rx_mode(), which calls __hw_addr_sync_dev() on them.
__hw_addr_sync_dev() iterates and modifies the lists by updating sync_cnt,
refcount, and removing stale entries.
Since iavf_configure() is called from contexts like iavf_open() and the
driver's watchdog task, it appears netif_addr_lock_bh() is not held here.
Could this cause list or RB-tree corruption if there are concurrent
modifications, such as IGMP/MLD timers adding multicast addresses in softirq
while holding netif_addr_lock_bh()?
^ permalink raw reply
* [PATCH v4 0/9] driver core: Fix some race conditions
From: Douglas Anderson @ 2026-04-04 0:04 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J . Wysocki, Danilo Krummrich,
Alan Stern
Cc: Saravana Kannan, Christoph Hellwig, Eric Dumazet, Johan Hovold,
Leon Romanovsky, Alexander Lobakin, Alexey Kardashevskiy,
Robin Murphy, Douglas Anderson, Andrew Morton, Frank.Li,
Jason Gunthorpe, alex, alexander.stein, andre.przywara, andrew,
andrew, andriy.shevchenko, aou, ardb, bhelgaas, brgl, broonie,
catalin.marinas, chleroy, davem, david, devicetree, dmaengine,
driver-core, gbatra, gregory.clement, hkallweit1, iommu,
jirislaby, joel, joro, kees, kevin.brodsky, kuba, lenb, lgirdwood,
linux-acpi, linux-arm-kernel, linux-aspeed, linux-cxl,
linux-kernel, linux-mips, linux-mm, linux-pci, linux-riscv,
linux-serial, linux-snps-arc, linux-usb, linux, linuxppc-dev,
m.szyprowski, maddy, mani, maz, miko.lenczewski, mpe, netdev,
npiggin, osalvador, oupton, pabeni, palmer, peter.ujfalusi,
peterz, pjw, robh, sebastian.hesselbarth, tglx, tsbogend, vgupta,
vkoul, will, willy, yangyicong, yeoreum.yun
The main goal of this series is to fix the observed bug talked about
in the first patch ("driver core: Don't let a device probe until it's
ready"). That patch fixes a problem that has been observed in the real
world and could land even if the rest of the patches are found
unacceptable or need to be spun.
That said, during patch review Danilo correctly pointed out that many
of the bitfield accesses in "struct device" are unsafe. I added a
bunch of patches in the series to address each one.
Danilo said he's most worried about "can_match", so I put that one
first. After that, I tried to transition bitfields to flags in reverse
order to when the bitfield was added.
Even if transitioning from bitfields to flags isn't truly needed for
correctness, it seems silly (and wasteful of space in struct device)
to have some in bitfields and some as flags. Thus I didn't spend time
for each bitfield showing that it's truly needed for correctness.
Transition was done semi manually. Presumably someone skilled at
coccinelle could do a better job, but I just used sed in a heavy-
handed manner and then reviewed/fixed the results, undoing anything my
script got wrong. My terrible/ugly script was:
var=can_match
caps="${var^^}"
for f in $(git grep -l "[>\.]${var}[^1-9_a-zA-Z\[]"); do
echo $f
sed -i~ -e "s/\([a-zA-Z_0-9\.>()-][a-zA-Z_0-9\.>()-]*\)->${var} = true/set_bit(DEV_FLAG_${caps}, \&\\1->flags)/" "$f"
sed -i~ -e "s/\([a-zA-Z_0-9\.>()-][a-zA-Z_0-9\.>()-]*\)\.${var} = true/dev_set_${caps}(\&\\1)/" "$f"
sed -i~ -e "s/\([a-zA-Z_0-9\.>()-][a-zA-Z_0-9\.>()-]*\)->${var} = false/clear_bit(DEV_FLAG_${caps}, \&\\1->flags)/" "$f"
sed -i~ -e "s/\([a-zA-Z_0-9\.>()-][a-zA-Z_0-9\.>()-]*\)\.${var} = false/dev_clear_${caps}(\&\\1)/" "$f"
sed -i~ -e "s/\([a-zA-Z_0-9\.>()-][a-zA-Z_0-9\.>()-]*\)->${var} = \([^;]*\)/assign_bit(DEV_FLAG_${caps}, \&\\1->flags, \\2)/" "$f"
sed -i~ -e "s/\([a-zA-Z_0-9\.>()-][a-zA-Z_0-9\.>()-]*\)\.${var} = \([^;]*\)/dev_assign_${caps}(\&\\1, \\2)/" "$f"
sed -i~ -e "s/\([a-zA-Z_0-9\.>()-][a-zA-Z_0-9\.>()-]*\)->${var}\([^1-9_a-zA-Z\[]\)/test_bit(DEV_FLAG_${caps}, \&\\1->flags)\\2/" "$f"
sed -i~ -e "s/\([a-zA-Z_0-9\.>()-][a-zA-Z_0-9\.>()-]*\)\.${var}\([^1-9_a-zA-Z\[]\)/dev_${caps}(\&\\1)\\2/" "$f"
done
From v3 to v4, I transitioned to accessor functions with another ugly
sed script. I had git format the old patches, then transformed them
with:
for f in *.patch; do
echo $f
sed -i~ -e "s/test_and_set_bit(DEV_FLAG_\([^,]*\), \&\(.*\)->flags)/dev_test_and_set_\\L\\1(\\2)/" "$f"
sed -i~ -e "s/test_and_set_bit(DEV_FLAG_\([^,]*\), \(.*\)\.flags)/dev_test_and_set_\\L\\1(\\2)/" "$f"
sed -i~ -e "s/test_bit(DEV_FLAG_\([^,]*\), \&\(.*\)->flags)/dev_\\L\\1(\\2)/" "$f"
sed -i~ -e "s/test_bit(DEV_FLAG_\([^,]*\), \(.*\)\.flags)/dev_\\L\\1(\\2)/" "$f"
sed -i~ -e "s/set_bit(DEV_FLAG_\([^,]*\), \&\(.*\)->flags)/dev_set_\\L\\1(\\2)/" "$f"
sed -i~ -e "s/set_bit(DEV_FLAG_\([^,]*\), \(.*\)\.flags)/dev_set_\\L\\1(\\2)/" "$f"
sed -i~ -e "s/clear_bit(DEV_FLAG_\([^,]*\), \&\(.*\)->flags)/dev_clear_\\L\\1(\\2)/" "$f"
sed -i~ -e "s/clear_bit(DEV_FLAG_\([^,]*\), \(.*\)\.flags)/dev_clear_\\L\\1(\\2)/" "$f"
sed -i~ -e "s/assign_bit(DEV_FLAG_\([^,]*\), \&\(.*\)->flags, \(.*\))/dev_assign_\\L\\1(\\2, \\3)/" "$f"
sed -i~ -e "s/assign_bit(DEV_FLAG_\([^,]*\), \(.*\)\.flags, \(.*\))/dev_assign_\\L\\1(\\2, \\3)/" "$f"
done
...and then did a few manual touchups for spacing.
NOTE: one potentially "controversial" choice I made in some patches
was to always reserve a flag ID even if a flag is only used under
certain CONFIG_ settings. This is a change from how things were
before. Keeping the numbering consistent and allowing easy
compile-testing of both CONFIG settings seemed worth it, especially
since it won't take up any extra space until we've added a lot more
flags.
I only marked the first patch as a "Fix" since it is the only one
fixing observed problems. Other patches could be considered fixes too
if folks want.
I tested the first patch in the series backported to kernel 6.6 on the
Pixel phone that was experiencing the race. I added extra printouts to
make sure that the problem was hitting / addressed. The rest of the
patches are tested with allmodconfig with arm32, arm64, ppc, and
x86. I boot tested on an arm64 Chromebook running mainline.
Changes in v4:
- Use accessor functions for flags
Changes in v3:
- Use a new "flags" bitfield
- Add missing \n in probe error message
Changes in v2:
- Instead of adjusting the ordering, use "ready_to_probe" flag
Douglas Anderson (9):
driver core: Don't let a device probe until it's ready
driver core: Replace dev->can_match with dev_can_match()
driver core: Replace dev->dma_iommu with dev_dma_iommu()
driver core: Replace dev->dma_skip_sync with dev_dma_skip_sync()
driver core: Replace dev->dma_ops_bypass with dev_dma_ops_bypass()
driver core: Replace dev->state_synced with dev_state_synced()
driver core: Replace dev->dma_coherent with dev_dma_coherent()
driver core: Replace dev->of_node_reused with dev_of_node_reused()
driver core: Replace dev->offline + ->offline_disabled with accessors
arch/arc/mm/dma.c | 4 +-
arch/arm/mach-highbank/highbank.c | 2 +-
arch/arm/mach-mvebu/coherency.c | 2 +-
arch/arm/mm/dma-mapping-nommu.c | 4 +-
arch/arm/mm/dma-mapping.c | 28 ++--
arch/arm64/kernel/cpufeature.c | 2 +-
arch/arm64/mm/dma-mapping.c | 2 +-
arch/mips/mm/dma-noncoherent.c | 2 +-
arch/powerpc/kernel/dma-iommu.c | 8 +-
.../platforms/pseries/hotplug-memory.c | 4 +-
arch/riscv/mm/dma-noncoherent.c | 2 +-
drivers/acpi/scan.c | 2 +-
drivers/base/core.c | 53 +++++---
drivers/base/cpu.c | 4 +-
drivers/base/dd.c | 28 ++--
drivers/base/memory.c | 2 +-
drivers/base/pinctrl.c | 2 +-
drivers/base/platform.c | 2 +-
drivers/dma/ti/k3-udma-glue.c | 6 +-
drivers/dma/ti/k3-udma.c | 6 +-
drivers/iommu/dma-iommu.c | 9 +-
drivers/iommu/iommu.c | 5 +-
drivers/net/pcs/pcs-xpcs-plat.c | 2 +-
drivers/of/device.c | 6 +-
drivers/pci/of.c | 2 +-
drivers/pci/pwrctrl/core.c | 2 +-
drivers/regulator/bq257xx-regulator.c | 2 +-
drivers/regulator/rk808-regulator.c | 2 +-
drivers/tty/serial/serial_base_bus.c | 2 +-
drivers/usb/gadget/udc/aspeed-vhub/dev.c | 2 +-
include/linux/device.h | 120 ++++++++++++------
include/linux/dma-map-ops.h | 6 +-
include/linux/dma-mapping.h | 2 +-
include/linux/iommu-dma.h | 3 +-
kernel/cpu.c | 4 +-
kernel/dma/mapping.c | 12 +-
mm/hmm.c | 2 +-
37 files changed, 206 insertions(+), 142 deletions(-)
--
2.53.0.1213.gd9a14994de-goog
^ permalink raw reply
* [PATCH v4 8/9] driver core: Replace dev->of_node_reused with dev_of_node_reused()
From: Douglas Anderson @ 2026-04-04 0:05 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J . Wysocki, Danilo Krummrich,
Alan Stern
Cc: Saravana Kannan, Christoph Hellwig, Eric Dumazet, Johan Hovold,
Leon Romanovsky, Alexander Lobakin, Alexey Kardashevskiy,
Robin Murphy, Douglas Anderson, Mark Brown, alexander.stein,
andrew, andrew, andriy.shevchenko, bhelgaas, brgl, davem,
devicetree, driver-core, hkallweit1, jirislaby, joel, kees, kuba,
lgirdwood, linux-arm-kernel, linux-aspeed, linux-kernel,
linux-pci, linux-serial, linux-usb, linux, mani, netdev, pabeni,
robh
In-Reply-To: <20260404000644.522677-1-dianders@chromium.org>
In C, bitfields are not necessarily safe to modify from multiple
threads without locking. Switch "of_node_reused" over to the "flags"
field so modifications are safe.
Cc: Johan Hovold <johan@kernel.org>
Acked-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
Not fixing any known bugs; problem is theoretical and found by code
inspection. Change is done somewhat manually and only lightly tested
(mostly compile-time tested).
Changes in v4:
- Use accessor functions for flags
Changes in v3:
- New
drivers/base/core.c | 2 +-
drivers/base/pinctrl.c | 2 +-
drivers/base/platform.c | 2 +-
drivers/net/pcs/pcs-xpcs-plat.c | 2 +-
drivers/of/device.c | 6 +++---
drivers/pci/of.c | 2 +-
drivers/pci/pwrctrl/core.c | 2 +-
drivers/regulator/bq257xx-regulator.c | 2 +-
drivers/regulator/rk808-regulator.c | 2 +-
drivers/tty/serial/serial_base_bus.c | 2 +-
drivers/usb/gadget/udc/aspeed-vhub/dev.c | 2 +-
include/linux/device.h | 7 ++++---
12 files changed, 17 insertions(+), 16 deletions(-)
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 531f02a5469a..f12f3b53b4d0 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -5281,7 +5281,7 @@ void device_set_of_node_from_dev(struct device *dev, const struct device *dev2)
{
of_node_put(dev->of_node);
dev->of_node = of_node_get(dev2->of_node);
- dev->of_node_reused = true;
+ dev_set_of_node_reused(dev);
}
EXPORT_SYMBOL_GPL(device_set_of_node_from_dev);
diff --git a/drivers/base/pinctrl.c b/drivers/base/pinctrl.c
index 6e250272c843..0bbc83231234 100644
--- a/drivers/base/pinctrl.c
+++ b/drivers/base/pinctrl.c
@@ -24,7 +24,7 @@ int pinctrl_bind_pins(struct device *dev)
{
int ret;
- if (dev->of_node_reused)
+ if (dev_of_node_reused(dev))
return 0;
dev->pins = devm_kzalloc(dev, sizeof(*(dev->pins)), GFP_KERNEL);
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index d44591d52e36..199e6fb25770 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -856,7 +856,7 @@ struct platform_device *platform_device_register_full(
pdev->dev.parent = pdevinfo->parent;
pdev->dev.fwnode = pdevinfo->fwnode;
pdev->dev.of_node = of_node_get(to_of_node(pdev->dev.fwnode));
- pdev->dev.of_node_reused = pdevinfo->of_node_reused;
+ dev_assign_of_node_reused(&pdev->dev, pdevinfo->of_node_reused);
if (pdevinfo->dma_mask) {
pdev->platform_dma_mask = pdevinfo->dma_mask;
diff --git a/drivers/net/pcs/pcs-xpcs-plat.c b/drivers/net/pcs/pcs-xpcs-plat.c
index b8c48f9effbf..f4b1b8246ce9 100644
--- a/drivers/net/pcs/pcs-xpcs-plat.c
+++ b/drivers/net/pcs/pcs-xpcs-plat.c
@@ -349,7 +349,7 @@ static int xpcs_plat_init_dev(struct dw_xpcs_plat *pxpcs)
* up later. Make sure DD-core is aware of the OF-node being re-used.
*/
device_set_node(&mdiodev->dev, fwnode_handle_get(dev_fwnode(dev)));
- mdiodev->dev.of_node_reused = true;
+ dev_set_of_node_reused(&mdiodev->dev);
/* Pass the data further so the DW XPCS driver core could use it */
mdiodev->dev.platform_data = (void *)device_get_match_data(dev);
diff --git a/drivers/of/device.c b/drivers/of/device.c
index f7e75e527667..be4e1584e0af 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -26,7 +26,7 @@
const struct of_device_id *of_match_device(const struct of_device_id *matches,
const struct device *dev)
{
- if (!matches || !dev->of_node || dev->of_node_reused)
+ if (!matches || !dev->of_node || dev_of_node_reused(dev))
return NULL;
return of_match_node(matches, dev->of_node);
}
@@ -192,7 +192,7 @@ ssize_t of_device_modalias(struct device *dev, char *str, ssize_t len)
{
ssize_t sl;
- if (!dev || !dev->of_node || dev->of_node_reused)
+ if (!dev || !dev->of_node || dev_of_node_reused(dev))
return -ENODEV;
sl = of_modalias(dev->of_node, str, len - 2);
@@ -254,7 +254,7 @@ int of_device_uevent_modalias(const struct device *dev, struct kobj_uevent_env *
{
int sl;
- if ((!dev) || (!dev->of_node) || dev->of_node_reused)
+ if ((!dev) || (!dev->of_node) || dev_of_node_reused(dev))
return -ENODEV;
/* Devicetree modalias is tricky, we add it in 2 steps */
diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index 9f8eb5df279e..1f9b669abdb0 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -38,7 +38,7 @@ int pci_set_of_node(struct pci_dev *dev)
struct device *pdev __free(put_device) =
bus_find_device_by_of_node(&platform_bus_type, node);
if (pdev)
- dev->bus->dev.of_node_reused = true;
+ dev_set_of_node_reused(&dev->bus->dev);
device_set_node(&dev->dev, of_fwnode_handle(no_free_ptr(node)));
return 0;
diff --git a/drivers/pci/pwrctrl/core.c b/drivers/pci/pwrctrl/core.c
index 7754baed67f2..72963a92362a 100644
--- a/drivers/pci/pwrctrl/core.c
+++ b/drivers/pci/pwrctrl/core.c
@@ -39,7 +39,7 @@ static int pci_pwrctrl_notify(struct notifier_block *nb, unsigned long action,
* If we got here then the PCI device is the second after the
* power control platform device. Mark its OF node as reused.
*/
- dev->of_node_reused = true;
+ dev_set_of_node_reused(dev);
break;
}
diff --git a/drivers/regulator/bq257xx-regulator.c b/drivers/regulator/bq257xx-regulator.c
index dab8f1ab4450..40e0f1a7ae81 100644
--- a/drivers/regulator/bq257xx-regulator.c
+++ b/drivers/regulator/bq257xx-regulator.c
@@ -143,7 +143,7 @@ static int bq257xx_regulator_probe(struct platform_device *pdev)
struct regulator_config cfg = {};
pdev->dev.of_node = pdev->dev.parent->of_node;
- pdev->dev.of_node_reused = true;
+ dev_set_of_node_reused(&pdev->dev);
pdata = devm_kzalloc(&pdev->dev, sizeof(struct bq257xx_reg_data), GFP_KERNEL);
if (!pdata)
diff --git a/drivers/regulator/rk808-regulator.c b/drivers/regulator/rk808-regulator.c
index e66408f23bb6..8297d31cde9f 100644
--- a/drivers/regulator/rk808-regulator.c
+++ b/drivers/regulator/rk808-regulator.c
@@ -2115,7 +2115,7 @@ static int rk808_regulator_probe(struct platform_device *pdev)
int ret, i, nregulators;
pdev->dev.of_node = pdev->dev.parent->of_node;
- pdev->dev.of_node_reused = true;
+ dev_set_of_node_reused(&pdev->dev);
regmap = dev_get_regmap(pdev->dev.parent, NULL);
if (!regmap)
diff --git a/drivers/tty/serial/serial_base_bus.c b/drivers/tty/serial/serial_base_bus.c
index a12935f6b992..5f23284a8778 100644
--- a/drivers/tty/serial/serial_base_bus.c
+++ b/drivers/tty/serial/serial_base_bus.c
@@ -74,7 +74,7 @@ static int serial_base_device_init(struct uart_port *port,
dev->parent = parent_dev;
dev->bus = &serial_base_bus_type;
dev->release = release;
- dev->of_node_reused = true;
+ dev_set_of_node_reused(dev);
device_set_node(dev, fwnode_handle_get(dev_fwnode(parent_dev)));
diff --git a/drivers/usb/gadget/udc/aspeed-vhub/dev.c b/drivers/usb/gadget/udc/aspeed-vhub/dev.c
index 2ecd049dacc2..8b9449d16324 100644
--- a/drivers/usb/gadget/udc/aspeed-vhub/dev.c
+++ b/drivers/usb/gadget/udc/aspeed-vhub/dev.c
@@ -593,7 +593,7 @@ int ast_vhub_init_dev(struct ast_vhub *vhub, unsigned int idx)
d->gadget.max_speed = USB_SPEED_HIGH;
d->gadget.speed = USB_SPEED_UNKNOWN;
d->gadget.dev.of_node = vhub->pdev->dev.of_node;
- d->gadget.dev.of_node_reused = true;
+ dev_set_of_node_reused(&d->gadget.dev);
rc = usb_add_gadget_udc(d->port_dev, &d->gadget);
if (rc != 0)
diff --git a/include/linux/device.h b/include/linux/device.h
index fca986cef2ed..8132aab17e04 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -483,6 +483,8 @@ struct device_physical_location {
* driver/bus sync_state() callback.
* @DEV_FLAG_DMA_COHERENT: This particular device is dma coherent, even if the
* architecture supports non-coherent devices.
+ * @DEV_FLAG_OF_NODE_REUSED: Set if the device-tree node is shared with an
+ * ancestor device.
*/
enum struct_device_flags {
DEV_FLAG_READY_TO_PROBE = 0,
@@ -492,6 +494,7 @@ enum struct_device_flags {
DEV_FLAG_DMA_OPS_BYPASS = 4,
DEV_FLAG_STATE_SYNCED = 5,
DEV_FLAG_DMA_COHERENT = 6,
+ DEV_FLAG_OF_NODE_REUSED = 7,
DEV_FLAG_COUNT
};
@@ -573,8 +576,6 @@ enum struct_device_flags {
*
* @offline_disabled: If set, the device is permanently online.
* @offline: Set after successful invocation of bus type's .offline().
- * @of_node_reused: Set if the device-tree node is shared with an ancestor
- * device.
* @flags: DEV_FLAG_XXX flags. Use atomic bitfield operations to modify.
*
* At the lowest level, every device in a Linux system is represented by an
@@ -681,7 +682,6 @@ struct device {
bool offline_disabled:1;
bool offline:1;
- bool of_node_reused:1;
DECLARE_BITMAP(flags, DEV_FLAG_COUNT);
};
@@ -715,6 +715,7 @@ __create_dev_flag_accessors(dma_skip_sync, DEV_FLAG_DMA_SKIP_SYNC);
__create_dev_flag_accessors(dma_ops_bypass, DEV_FLAG_DMA_OPS_BYPASS);
__create_dev_flag_accessors(state_synced, DEV_FLAG_STATE_SYNCED);
__create_dev_flag_accessors(dma_coherent, DEV_FLAG_DMA_COHERENT);
+__create_dev_flag_accessors(of_node_reused, DEV_FLAG_OF_NODE_REUSED);
/**
* struct device_link - Device link representation.
--
2.53.0.1213.gd9a14994de-goog
^ permalink raw reply related
* [PATCH net] net: avoid nul-deref trying to bind mp to incapable device
From: Jakub Kicinski @ 2026-04-04 0:19 UTC (permalink / raw)
To: davem
Cc: netdev, edumazet, pabeni, andrew+netdev, horms, Jakub Kicinski,
sdf, almasrymina, daniel
Sashiko points out that we use qops in __net_mp_open_rxq()
but never validate they are null. This was introduced when
check was moved from netdev_rx_queue_restart().
Look at ops directly instead of the locking config.
qops imply netdev_need_ops_lock(). We used netdev_need_ops_lock()
initially to signify that the real_num_rx_queues check below
is safe without rtnl_lock, but I'm not sure if this is actually
clear to most people, anyway.
Fixes: da7772a2b4ad ("net: move mp->rx_page_size validation to __net_mp_open_rxq()")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
CC: sdf@fomichev.me
CC: almasrymina@google.com
CC: daniel@iogearbox.net
---
net/core/netdev_rx_queue.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/core/netdev_rx_queue.c b/net/core/netdev_rx_queue.c
index 668a90658f25..05fd2875d725 100644
--- a/net/core/netdev_rx_queue.c
+++ b/net/core/netdev_rx_queue.c
@@ -117,7 +117,7 @@ int __net_mp_open_rxq(struct net_device *dev, unsigned int rxq_idx,
struct netdev_rx_queue *rxq;
int ret;
- if (!netdev_need_ops_lock(dev))
+ if (!qops)
return -EOPNOTSUPP;
if (rxq_idx >= dev->real_num_rx_queues) {
--
2.53.0
^ permalink raw reply related
* Re: [PATCH v9 02/10] x86/bhi: Make clear_bhb_loop() effective on newer CPUs
From: Pawan Gupta @ 2026-04-04 0:21 UTC (permalink / raw)
To: Jim Mattson
Cc: x86, Jon Kohler, Nikolay Borisov, H. Peter Anvin, Josh Poimboeuf,
David Kaplan, Sean Christopherson, Borislav Petkov, Dave Hansen,
Peter Zijlstra, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, KP Singh, Jiri Olsa, David S. Miller,
David Laight, Andy Lutomirski, Thomas Gleixner, Ingo Molnar,
David Ahern, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, John Fastabend, Stanislav Fomichev, Hao Luo,
Paolo Bonzini, Jonathan Corbet, linux-kernel, kvm, Asit Mallick,
Tao Zhang, bpf, netdev, linux-doc, chao.gao
In-Reply-To: <CALMp9eTpsenqsWjzmpXLEubn9uNjgZgzgrMwtZ72HDuV_2xgfg@mail.gmail.com>
On Fri, Apr 03, 2026 at 04:39:54PM -0700, Jim Mattson wrote:
> > Since cloud providers have greater control over userspace, the decision to
> > use BHI_DIS_S or not can be left to them. KVM would simply follow what it
> > is asked to do by the userspace.
>
> I feel like we've gone over this before, but if userspace tells KVM
> not to enable BHI_DIS_S, how do we inform Windows that it needs to do
> the longer clearing sequence, despite the fact that the virtual CPU is
> masquerading as Ice Lake?
IMO, if an OS is allergic to a hardware mitigation, and is also aware that
it is virtualized, it should default to a sw mitigation that works everywhere.
> I don't think the virtual mitigation MSRs address that issue.
Virtual mitigation MSRs are meant to inform the VMM about the guest
mitigation. Even if there was a way to tell the guest that it needs to use
a different mitigation, it seems unrealistic for a guest to change its
mitigation post-migration.
^ permalink raw reply
* nfc: nci: fix OOB heap read in nci_core_init_rsp_packet_v1()
From: Main Frame @ 2026-04-04 0:26 UTC (permalink / raw)
To: netdev@vger.kernel.org; +Cc: Jakub Kicinski
nci_core_init_rsp_packet_v1() uses the raw chip-supplied
num_supported_rf_interfaces byte to compute the rsp_2 pointer, but
the preceding min() already stores the capped value in
ndev->num_supported_rf_interfaces. When a hostile chip returns
num_supported_rf_interfaces > 4 the memcpy is safe (capped) but rsp_2
lands past the end of the skb, and the fields copied out of it corrupt
nci_dev with data from adjacent kernel heap.
Use the already-capped ndev->num_supported_rf_interfaces for both the
length check and the pointer, making the relationship between the two
explicit.
v2: drop intermediate offset variable, check skb->len directly against
ndev->num_supported_rf_interfaces + sizeof(*rsp_2) (@Kuba)
Fixes: e8c0dacd9836 ("NFC: Update names and structs to NCI spec 1.0 d18")
Signed-off-by: Lekë Hapçiu <framemain@outlook.com>
---
net/nfc/nci/rsp.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/net/nfc/nci/rsp.c b/net/nfc/nci/rsp.c
index 9eeb86282..4aaf362b9 100644
--- a/net/nfc/nci/rsp.c
+++ b/net/nfc/nci/rsp.c
@@ -66,7 +66,16 @@ static u8 nci_core_init_rsp_packet_v1(struct nci_dev *ndev,
rsp_1->supported_rf_interfaces,
ndev->num_supported_rf_interfaces);
- rsp_2 = (void *) (skb->data + 6 + rsp_1->num_supported_rf_interfaces);
+ if (skb->len < sizeof(*rsp_1) + ndev->num_supported_rf_interfaces +
+ sizeof(*rsp_2)) {
+ pr_err("CORE_INIT_RSP too short: len=%u need=%zu\n",
+ skb->len,
+ sizeof(*rsp_1) + ndev->num_supported_rf_interfaces +
+ sizeof(*rsp_2));
+ return NCI_STATUS_SYNTAX_ERROR;
+ }
+ rsp_2 = (void *)(skb->data + sizeof(*rsp_1) +
+ ndev->num_supported_rf_interfaces);
ndev->max_logical_connections = rsp_2->max_logical_connections;
ndev->max_routing_table_size =
--
2.51.0
^ permalink raw reply
* Re: [PATCH net-next v5 02/14] net: introduce ndo_set_rx_mode_async and netdev_rx_mode_work
From: Jakub Kicinski @ 2026-04-04 0:27 UTC (permalink / raw)
To: Stanislav Fomichev; +Cc: netdev, davem, edumazet, pabeni
In-Reply-To: <20260402225535.4124525-3-sdf@fomichev.me>
On Thu, 2 Apr 2026 15:55:23 -0700 Stanislav Fomichev wrote:
> + if (netif_rx_mode_clean(dev))
> + dev_put(dev);
let's toss a netdev_tracker into this reference counting
but overall the work stealing is looking cleaner than I expected,
nicely done!
^ permalink raw reply
* Re: [PATCH net] bridge: mrp: reject zero test interval to avoid OOM panic
From: Xiang Mei @ 2026-04-04 0:31 UTC (permalink / raw)
To: Simon Horman
Cc: Nikolay Aleksandrov, netdev, bridge, idosch, davem, edumazet,
pabeni, bestswngs
In-Reply-To: <20260403083415.GC11973@horms.kernel.org>
On Fri, Apr 03, 2026 at 09:34:15AM +0100, Simon Horman wrote:
> On Fri, Mar 27, 2026 at 01:46:39PM +0200, Nikolay Aleksandrov wrote:
> > On 27/03/2026 13:34, Simon Horman wrote:
> > > On Wed, Mar 25, 2026 at 08:24:38PM -0700, Xiang Mei wrote:
> > > > br_mrp_start_test() and br_mrp_start_in_test() accept the user-supplied
> > > > interval value from netlink without validation. When interval is 0,
> > > > usecs_to_jiffies(0) yields 0, causing the delayed work
> > > > (br_mrp_test_work_expired / br_mrp_in_test_work_expired) to reschedule
> > > > itself with zero delay. This creates a tight loop on system_percpu_wq
> > > > that allocates and transmits MRP test frames at maximum rate, exhausting
> > > > all system memory and causing a kernel panic via OOM deadlock.
> > >
> > > I would suspect the primary outcome of this problem is high CPU consumption
> > > rather than memory exhaustion. Is there a reason to expect that
> > > the transmitted fames can't be consumed as fast as they are created?
> > >
> >
> > +1
> > More so with CAP_NET_ADMIN you can cause all sorts of OOM and high-cpu usage
> > conditions. This is a configuration error and OOM doesn't lead to panic unless
> > instructed to. I don't think this is worth changing at all.
>
> Right, I was getting to think that too.
>
> ...
Thank you for the review.
Regarding the privilege argument: this path is reachable from an
unprivileged user namespace, which checks CAP_NET_ADMIN against the
network namespace's user_ns. An unprivileged user can create a
user+net namespace, obtain CAP_NET_ADMIN within it, set up a bridge
with MRP, and trigger the zero-interval loop. So this crosses a
security boundary.
Regarding the OOM behavior: this does lead to a kernel panic without
panic_on_oom or oops=panic. The zero-delay workqueue loop allocates
memory from kernel context continuously. The OOM killer fires and kills
all related userspace processes, but the memory pressure originates
from a kernel workqueue which the OOM killer cannot target. Once no
killable processes remain, the kernel hits "System is deadlocked on
memory" and panics. Below is the relevant dmesg from a test without
oops=panic:
[ 5.254108] Kernel panic - not syncing: System is deadlocked on memory
[ 5.254470] CPU: 0 UID: 0 PID: 1 Comm: init Not tainted 7.0.0-rc4+ #6
[ 5.255524] Call Trace:
[ 5.255796] vpanic+0x694/0x780
[ 5.256607] out_of_memory+0x124e/0x1350
[ 5.257281] __alloc_pages_slowpath.constprop.0+0x2325/0x2dd0
[ 5.258777] __alloc_frozen_pages_noprof+0x4f8/0x800
Best,
Xiang
^ permalink raw reply
* Re: [PATCH v4 0/3] dpll: add frequency monitoring feature
From: patchwork-bot+netdevbpf @ 2026-04-04 0:40 UTC (permalink / raw)
To: Ivan Vecera
Cc: netdev, arkadiusz.kubalewski, davem, donald.hunter, edumazet,
kuba, jiri, corbet, mschmidt, pabeni, poros, Prathosh.Satish,
skhan, horms, vadim.fedorenko, linux-doc, linux-kernel
In-Reply-To: <20260402184057.1890514-1-ivecera@redhat.com>
Hello:
This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Thu, 2 Apr 2026 20:40:54 +0200 you wrote:
> This series adds support for monitoring the measured input frequency
> of DPLL input pins via the DPLL netlink interface.
>
> Some DPLL devices can measure the actual frequency being received on
> input pins. The approach mirrors the existing phase-offset-monitor
> feature: a device-level attribute (DPLL_A_FREQUENCY_MONITOR) enables
> or disables monitoring, and a per-pin attribute
> (DPLL_A_PIN_MEASURED_FREQUENCY) exposes the measured frequency in
> millihertz (mHz) when monitoring is enabled.
>
> [...]
Here is the summary with links:
- [v4,1/3] dpll: add frequency monitoring to netlink spec
https://git.kernel.org/netdev/net-next/c/3fdea79c09d1
- [v4,2/3] dpll: add frequency monitoring callback ops
https://git.kernel.org/netdev/net-next/c/15ed91aa84ea
- [v4,3/3] dpll: zl3073x: implement frequency monitoring
https://git.kernel.org/netdev/net-next/c/bfc923b64287
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 0/2] seg6: fix dst_cache sharing in seg6 lwtunnel
From: Andrea Mayer @ 2026-04-04 0:44 UTC (permalink / raw)
To: netdev
Cc: davem, edumazet, kuba, pabeni, horms, dsahern, david.lebrun,
stefano.salsano, paolo.lungaroni, nicolas.dichtel, justin.iurman,
linux-kernel, Andrea Mayer
The seg6 lwtunnel encap uses a single per-route dst_cache shared
between seg6_input_core() and seg6_output_core(). These two paths
can perform the post-encap SID lookup in different routing contexts
(e.g., ip rules matching on the ingress interface, or VRF table
separation). Whichever path runs first populates the cache, and the
other reuses it blindly, bypassing its own lookup.
Patch 1 fixes this by splitting the cache into cache_input and
cache_output. Patch 2 adds a selftest that validates the isolation.
Changes v2 -> v3:
- Patch 2: add root privilege check (Justin)
- Patch 2: extend prerequisite tool checks and use ping instead
of ping6 for consistency with other srv6 selftests
Changes v1 -> v2:
- Patch 2: fix SKIP message wording (Nicolas)
Andrea Mayer (2):
seg6: separate dst_cache for input and output paths in seg6 lwtunnel
selftests: seg6: add test for dst_cache isolation in seg6 lwtunnel
net/ipv6/seg6_iptunnel.c | 34 ++-
tools/testing/selftests/net/Makefile | 1 +
.../selftests/net/srv6_iptunnel_cache.sh | 197 ++++++++++++++++++
3 files changed, 221 insertions(+), 11 deletions(-)
create mode 100755 tools/testing/selftests/net/srv6_iptunnel_cache.sh
--
2.43.0
^ permalink raw reply
* [PATCH net v3 2/2] selftests: seg6: add test for dst_cache isolation in seg6 lwtunnel
From: Andrea Mayer @ 2026-04-04 0:44 UTC (permalink / raw)
To: netdev
Cc: davem, edumazet, kuba, pabeni, horms, dsahern, david.lebrun,
stefano.salsano, paolo.lungaroni, nicolas.dichtel, justin.iurman,
linux-kernel, Andrea Mayer, Shuah Khan, linux-kselftest
In-Reply-To: <20260404004405.4057-1-andrea.mayer@uniroma2.it>
Add a selftest that verifies the dst_cache in seg6 lwtunnel is not
shared between the input (forwarding) and output (locally generated)
paths.
The test creates three namespaces (ns_src, ns_router, ns_dst)
connected in a line. An SRv6 encap route on ns_router encapsulates
traffic destined to cafe::1 with SID fc00::100. The SID is
reachable only for forwarded traffic (from ns_src) via an ip rule
matching the ingress interface (iif veth-r0 lookup 100), and
blackholed in the main table.
The test verifies that:
1. A packet generated locally on ns_router does not reach
ns_dst with an empty cache, since the SID is blackholed;
2. A forwarded packet from ns_src populates the input cache
from table 100 and reaches ns_dst;
3. A packet generated locally on ns_router still does not
reach ns_dst after the input cache is populated,
confirming the output path does not reuse the input
cache entry.
Both the forwarded and local packets are pinned to the same CPU
with taskset, since dst_cache is per-cpu.
Cc: Shuah Khan <shuah@kernel.org>
Cc: linux-kselftest@vger.kernel.org
Signed-off-by: Andrea Mayer <andrea.mayer@uniroma2.it>
Reviewed-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Reviewed-by: Justin Iurman <justin.iurman@gmail.com>
---
Changes v2 -> v3:
- add root privilege check (Justin)
- extend prerequisite tool checks and use ping instead of ping6
for consistency with other srv6 selftests
Changes v1 -> v2:
- fix SKIP message wording (Nicolas)
---
tools/testing/selftests/net/Makefile | 1 +
.../selftests/net/srv6_iptunnel_cache.sh | 197 ++++++++++++++++++
2 files changed, 198 insertions(+)
create mode 100755 tools/testing/selftests/net/srv6_iptunnel_cache.sh
diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index 605c54c0e8a3..c709523c99c6 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -89,6 +89,7 @@ TEST_PROGS := \
srv6_end_x_next_csid_l3vpn_test.sh \
srv6_hencap_red_l3vpn_test.sh \
srv6_hl2encap_red_l2vpn_test.sh \
+ srv6_iptunnel_cache.sh \
stress_reuseport_listen.sh \
tcp_fastopen_backup_key.sh \
test_bpf.sh \
diff --git a/tools/testing/selftests/net/srv6_iptunnel_cache.sh b/tools/testing/selftests/net/srv6_iptunnel_cache.sh
new file mode 100755
index 000000000000..62638ab679d9
--- /dev/null
+++ b/tools/testing/selftests/net/srv6_iptunnel_cache.sh
@@ -0,0 +1,197 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+#
+# author: Andrea Mayer <andrea.mayer@uniroma2.it>
+
+# This test verifies that the seg6 lwtunnel does not share the dst_cache
+# between the input (forwarding) and output (locally generated) paths.
+#
+# A shared dst_cache allows a forwarded packet to populate the cache and a
+# subsequent locally generated packet to silently reuse that entry, bypassing
+# its own route lookup. To expose this, the SID is made reachable only for
+# forwarded traffic (via an ip rule matching iif) and blackholed for everything
+# else. A local ping on ns_router must always hit the blackhole;
+# if it succeeds after a forwarded packet has populated the
+# cache, the bug is confirmed.
+#
+# Both forwarded and local packets are pinned to the same CPU with taskset,
+# since dst_cache is per-cpu.
+#
+#
+# +--------------------+ +--------------------+
+# | ns_src | | ns_dst |
+# | | | |
+# | veth-s0 | | veth-d0 |
+# | fd00::1/64 | | fd01::2/64 |
+# +-------+------------+ +----------+---------+
+# | |
+# | +--------------------+ |
+# | | ns_router | |
+# | | | |
+# +------------+ veth-r0 veth-r1 +--------------+
+# | fd00::2 fd01::1 |
+# +--------------------+
+#
+#
+# ns_router: encap (main table)
+# +---------+---------------------------------------+
+# | dst | action |
+# +---------+---------------------------------------+
+# | cafe::1 | encap seg6 mode encap segs fc00::100 |
+# +---------+---------------------------------------+
+#
+# ns_router: post-encap SID resolution
+# +-------+------------+----------------------------+
+# | table | dst | action |
+# +-------+------------+----------------------------+
+# | 100 | fc00::100 | via fd01::2 dev veth-r1 |
+# +-------+------------+----------------------------+
+# | main | fc00::100 | blackhole |
+# +-------+------------+----------------------------+
+#
+# ns_router: ip rule
+# +------------------+------------------------------+
+# | match | action |
+# +------------------+------------------------------+
+# | iif veth-r0 | lookup 100 |
+# +------------------+------------------------------+
+#
+# ns_dst: SRv6 decap (main table)
+# +--------------+----------------------------------+
+# | SID | action |
+# +--------------+----------------------------------+
+# | fc00::100 | End.DT6 table 255 (local) |
+# +--------------+----------------------------------+
+
+source lib.sh
+
+readonly SID="fc00::100"
+readonly DEST="cafe::1"
+
+readonly SRC_MAC="02:00:00:00:00:01"
+readonly RTR_R0_MAC="02:00:00:00:00:02"
+readonly RTR_R1_MAC="02:00:00:00:00:03"
+readonly DST_MAC="02:00:00:00:00:04"
+
+cleanup()
+{
+ cleanup_ns "${NS_SRC}" "${NS_RTR}" "${NS_DST}"
+}
+
+check_prerequisites()
+{
+ if ! command -v ip &>/dev/null; then
+ echo "SKIP: ip tool not found"
+ exit "${ksft_skip}"
+ fi
+
+ if ! command -v ping &>/dev/null; then
+ echo "SKIP: ping not found"
+ exit "${ksft_skip}"
+ fi
+
+ if ! command -v sysctl &>/dev/null; then
+ echo "SKIP: sysctl not found"
+ exit "${ksft_skip}"
+ fi
+
+ if ! command -v taskset &>/dev/null; then
+ echo "SKIP: taskset not found"
+ exit "${ksft_skip}"
+ fi
+}
+
+setup()
+{
+ setup_ns NS_SRC NS_RTR NS_DST
+
+ ip link add veth-s0 netns "${NS_SRC}" type veth \
+ peer name veth-r0 netns "${NS_RTR}"
+ ip link add veth-r1 netns "${NS_RTR}" type veth \
+ peer name veth-d0 netns "${NS_DST}"
+
+ ip -n "${NS_SRC}" link set veth-s0 address "${SRC_MAC}"
+ ip -n "${NS_RTR}" link set veth-r0 address "${RTR_R0_MAC}"
+ ip -n "${NS_RTR}" link set veth-r1 address "${RTR_R1_MAC}"
+ ip -n "${NS_DST}" link set veth-d0 address "${DST_MAC}"
+
+ # ns_src
+ ip -n "${NS_SRC}" link set veth-s0 up
+ ip -n "${NS_SRC}" addr add fd00::1/64 dev veth-s0 nodad
+ ip -n "${NS_SRC}" -6 route add "${DEST}"/128 via fd00::2
+
+ # ns_router
+ ip -n "${NS_RTR}" link set veth-r0 up
+ ip -n "${NS_RTR}" addr add fd00::2/64 dev veth-r0 nodad
+ ip -n "${NS_RTR}" link set veth-r1 up
+ ip -n "${NS_RTR}" addr add fd01::1/64 dev veth-r1 nodad
+ ip netns exec "${NS_RTR}" sysctl -qw net.ipv6.conf.all.forwarding=1
+
+ ip -n "${NS_RTR}" -6 route add "${DEST}"/128 \
+ encap seg6 mode encap segs "${SID}" dev veth-r0
+ ip -n "${NS_RTR}" -6 route add "${SID}"/128 table 100 \
+ via fd01::2 dev veth-r1
+ ip -n "${NS_RTR}" -6 route add blackhole "${SID}"/128
+ ip -n "${NS_RTR}" -6 rule add iif veth-r0 lookup 100
+
+ # ns_dst
+ ip -n "${NS_DST}" link set veth-d0 up
+ ip -n "${NS_DST}" addr add fd01::2/64 dev veth-d0 nodad
+ ip -n "${NS_DST}" addr add "${DEST}"/128 dev lo nodad
+ ip -n "${NS_DST}" -6 route add "${SID}"/128 \
+ encap seg6local action End.DT6 table 255 dev veth-d0
+ ip -n "${NS_DST}" -6 route add fd00::/64 via fd01::1
+
+ # static neighbors
+ ip -n "${NS_SRC}" -6 neigh add fd00::2 dev veth-s0 \
+ lladdr "${RTR_R0_MAC}" nud permanent
+ ip -n "${NS_RTR}" -6 neigh add fd00::1 dev veth-r0 \
+ lladdr "${SRC_MAC}" nud permanent
+ ip -n "${NS_RTR}" -6 neigh add fd01::2 dev veth-r1 \
+ lladdr "${DST_MAC}" nud permanent
+ ip -n "${NS_DST}" -6 neigh add fd01::1 dev veth-d0 \
+ lladdr "${RTR_R1_MAC}" nud permanent
+}
+
+test_cache_isolation()
+{
+ RET=0
+
+ # local ping with empty cache: must fail (SID is blackholed)
+ if ip netns exec "${NS_RTR}" taskset -c 0 \
+ ping -c 1 -W 2 "${DEST}" &>/dev/null; then
+ echo "SKIP: local ping succeeded, topology broken"
+ exit "${ksft_skip}"
+ fi
+
+ # forward from ns_src to populate the input cache
+ if ! ip netns exec "${NS_SRC}" taskset -c 0 \
+ ping -c 1 -W 2 "${DEST}" &>/dev/null; then
+ echo "SKIP: forwarded ping failed, topology broken"
+ exit "${ksft_skip}"
+ fi
+
+ # local ping again: must still fail; if the output path reuses
+ # the input cache, it bypasses the blackhole and the ping succeeds
+ if ip netns exec "${NS_RTR}" taskset -c 0 \
+ ping -c 1 -W 2 "${DEST}" &>/dev/null; then
+ echo "FAIL: output path used dst cached by input path"
+ RET="${ksft_fail}"
+ else
+ echo "PASS: output path dst_cache is independent"
+ fi
+
+ return "${RET}"
+}
+
+if [ "$(id -u)" -ne 0 ]; then
+ echo "SKIP: Need root privileges"
+ exit "${ksft_skip}"
+fi
+
+trap cleanup EXIT
+
+check_prerequisites
+setup
+test_cache_isolation
+exit "${RET}"
--
2.43.0
^ permalink raw reply related
* [PATCH net v3 1/2] seg6: separate dst_cache for input and output paths in seg6 lwtunnel
From: Andrea Mayer @ 2026-04-04 0:44 UTC (permalink / raw)
To: netdev
Cc: davem, edumazet, kuba, pabeni, horms, dsahern, david.lebrun,
stefano.salsano, paolo.lungaroni, nicolas.dichtel, justin.iurman,
linux-kernel, Andrea Mayer, stable
In-Reply-To: <20260404004405.4057-1-andrea.mayer@uniroma2.it>
The seg6 lwtunnel uses a single dst_cache per encap route, shared
between seg6_input_core() and seg6_output_core(). These two paths
can perform the post-encap SID lookup in different routing contexts
(e.g., ip rules matching on the ingress interface, or VRF table
separation). Whichever path runs first populates the cache, and the
other reuses it blindly, bypassing its own lookup.
Fix this by splitting the cache into cache_input and cache_output,
so each path maintains its own cached dst independently.
Fixes: 6c8702c60b88 ("ipv6: sr: add support for SRH encapsulation and injection with lwtunnels")
Cc: stable@vger.kernel.org
Signed-off-by: Andrea Mayer <andrea.mayer@uniroma2.it>
Reviewed-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Reviewed-by: Justin Iurman <justin.iurman@gmail.com>
---
net/ipv6/seg6_iptunnel.c | 34 +++++++++++++++++++++++-----------
1 file changed, 23 insertions(+), 11 deletions(-)
diff --git a/net/ipv6/seg6_iptunnel.c b/net/ipv6/seg6_iptunnel.c
index 3e1b9991131a..d6a0f7df9080 100644
--- a/net/ipv6/seg6_iptunnel.c
+++ b/net/ipv6/seg6_iptunnel.c
@@ -48,7 +48,8 @@ static size_t seg6_lwt_headroom(struct seg6_iptunnel_encap *tuninfo)
}
struct seg6_lwt {
- struct dst_cache cache;
+ struct dst_cache cache_input;
+ struct dst_cache cache_output;
struct seg6_iptunnel_encap tuninfo[];
};
@@ -488,7 +489,7 @@ static int seg6_input_core(struct net *net, struct sock *sk,
slwt = seg6_lwt_lwtunnel(lwtst);
local_bh_disable();
- dst = dst_cache_get(&slwt->cache);
+ dst = dst_cache_get(&slwt->cache_input);
local_bh_enable();
err = seg6_do_srh(skb, dst);
@@ -504,7 +505,7 @@ static int seg6_input_core(struct net *net, struct sock *sk,
/* cache only if we don't create a dst reference loop */
if (!dst->error && lwtst != dst->lwtstate) {
local_bh_disable();
- dst_cache_set_ip6(&slwt->cache, dst,
+ dst_cache_set_ip6(&slwt->cache_input, dst,
&ipv6_hdr(skb)->saddr);
local_bh_enable();
}
@@ -564,7 +565,7 @@ static int seg6_output_core(struct net *net, struct sock *sk,
slwt = seg6_lwt_lwtunnel(orig_dst->lwtstate);
local_bh_disable();
- dst = dst_cache_get(&slwt->cache);
+ dst = dst_cache_get(&slwt->cache_output);
local_bh_enable();
err = seg6_do_srh(skb, dst);
@@ -591,7 +592,7 @@ static int seg6_output_core(struct net *net, struct sock *sk,
/* cache only if we don't create a dst reference loop */
if (orig_dst->lwtstate != dst->lwtstate) {
local_bh_disable();
- dst_cache_set_ip6(&slwt->cache, dst, &fl6.saddr);
+ dst_cache_set_ip6(&slwt->cache_output, dst, &fl6.saddr);
local_bh_enable();
}
@@ -701,11 +702,13 @@ static int seg6_build_state(struct net *net, struct nlattr *nla,
slwt = seg6_lwt_lwtunnel(newts);
- err = dst_cache_init(&slwt->cache, GFP_ATOMIC);
- if (err) {
- kfree(newts);
- return err;
- }
+ err = dst_cache_init(&slwt->cache_input, GFP_ATOMIC);
+ if (err)
+ goto err_free_newts;
+
+ err = dst_cache_init(&slwt->cache_output, GFP_ATOMIC);
+ if (err)
+ goto err_destroy_input;
memcpy(&slwt->tuninfo, tuninfo, tuninfo_len);
@@ -720,11 +723,20 @@ static int seg6_build_state(struct net *net, struct nlattr *nla,
*ts = newts;
return 0;
+
+err_destroy_input:
+ dst_cache_destroy(&slwt->cache_input);
+err_free_newts:
+ kfree(newts);
+ return err;
}
static void seg6_destroy_state(struct lwtunnel_state *lwt)
{
- dst_cache_destroy(&seg6_lwt_lwtunnel(lwt)->cache);
+ struct seg6_lwt *slwt = seg6_lwt_lwtunnel(lwt);
+
+ dst_cache_destroy(&slwt->cache_input);
+ dst_cache_destroy(&slwt->cache_output);
}
static int seg6_fill_encap_info(struct sk_buff *skb,
--
2.43.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