* [PATCH v2 4/9] iio: magnetometer: yas530: switch to using FIELD_GET_SIGNED()
From: Yury Norov @ 2026-04-27 21:41 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
H. Peter Anvin, Andy Lutomirski, Peter Zijlstra, Jonathan Cameron,
David Lechner, Johannes Berg, David Laight, Nuno Sá,
Andy Shevchenko, Ping-Ke Shih, Richard Cochran, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Alexandre Belloni, Yury Norov, Rasmus Villemoes, Hans de Goede,
Linus Walleij, Sakari Ailus, Salah Triki, Achim Gratz,
Ben Collins, x86, linux-kernel, linux-iio, linux-wireless, netdev,
linux-rtc
Cc: Yury Norov
In-Reply-To: <20260427214127.406067-1-ynorov@nvidia.com>
Switch from sign_extend32(FIELD_GET()) to the dedicated
FIELD_GET_SIGNED() and don't calculate the fields length explicitly.
Reviewed-by: Linus Walleij <linusw@kernel.org>
Acked-by: Jonathan Cameron <jic23@kernel.org>
Signed-off-by: Yury Norov <ynorov@nvidia.com>
---
drivers/iio/magnetometer/yamaha-yas530.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/iio/magnetometer/yamaha-yas530.c b/drivers/iio/magnetometer/yamaha-yas530.c
index d49e37edcbed..6a80042602c6 100644
--- a/drivers/iio/magnetometer/yamaha-yas530.c
+++ b/drivers/iio/magnetometer/yamaha-yas530.c
@@ -859,9 +859,9 @@ static int yas530_get_calibration_data(struct yas5xx *yas5xx)
c->f[0] = FIELD_GET(GENMASK(22, 21), val);
c->f[1] = FIELD_GET(GENMASK(14, 13), val);
c->f[2] = FIELD_GET(GENMASK(6, 5), val);
- c->r[0] = sign_extend32(FIELD_GET(GENMASK(28, 23), val), 5);
- c->r[1] = sign_extend32(FIELD_GET(GENMASK(20, 15), val), 5);
- c->r[2] = sign_extend32(FIELD_GET(GENMASK(12, 7), val), 5);
+ c->r[0] = FIELD_GET_SIGNED(GENMASK(28, 23), val);
+ c->r[1] = FIELD_GET_SIGNED(GENMASK(20, 15), val);
+ c->r[2] = FIELD_GET_SIGNED(GENMASK(12, 7), val);
return 0;
}
@@ -914,9 +914,9 @@ static int yas532_get_calibration_data(struct yas5xx *yas5xx)
c->f[0] = FIELD_GET(GENMASK(24, 23), val);
c->f[1] = FIELD_GET(GENMASK(16, 15), val);
c->f[2] = FIELD_GET(GENMASK(8, 7), val);
- c->r[0] = sign_extend32(FIELD_GET(GENMASK(30, 25), val), 5);
- c->r[1] = sign_extend32(FIELD_GET(GENMASK(22, 17), val), 5);
- c->r[2] = sign_extend32(FIELD_GET(GENMASK(14, 7), val), 5);
+ c->r[0] = FIELD_GET_SIGNED(GENMASK(30, 25), val);
+ c->r[1] = FIELD_GET_SIGNED(GENMASK(22, 17), val);
+ c->r[2] = FIELD_GET_SIGNED(GENMASK(14, 7), val);
return 0;
}
--
2.51.0
^ permalink raw reply related
* [PATCH v2 3/9] iio: intel_dc_ti_adc: switch to using FIELD_GET_SIGNED()
From: Yury Norov @ 2026-04-27 21:41 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
H. Peter Anvin, Andy Lutomirski, Peter Zijlstra, Jonathan Cameron,
David Lechner, Johannes Berg, David Laight, Nuno Sá,
Andy Shevchenko, Ping-Ke Shih, Richard Cochran, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Alexandre Belloni, Yury Norov, Rasmus Villemoes, Hans de Goede,
Linus Walleij, Sakari Ailus, Salah Triki, Achim Gratz,
Ben Collins, x86, linux-kernel, linux-iio, linux-wireless, netdev,
linux-rtc
Cc: Yury Norov
In-Reply-To: <20260427214127.406067-1-ynorov@nvidia.com>
Switch from sign_extend32(FIELD_GET()) to the dedicated
FIELD_GET_SIGNED() and don't provide the fields length explicitly.
Acked-by: Jonathan Cameron <jic23@kernel.org>
Signed-off-by: Yury Norov <ynorov@nvidia.com>
---
drivers/iio/adc/intel_dc_ti_adc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/adc/intel_dc_ti_adc.c b/drivers/iio/adc/intel_dc_ti_adc.c
index 0fe34f1c338e..b5afad713e2d 100644
--- a/drivers/iio/adc/intel_dc_ti_adc.c
+++ b/drivers/iio/adc/intel_dc_ti_adc.c
@@ -290,8 +290,8 @@ static int dc_ti_adc_probe(struct platform_device *pdev)
if (ret)
return ret;
- info->vbat_zse = sign_extend32(FIELD_GET(DC_TI_VBAT_ZSE, val), 3);
- info->vbat_ge = sign_extend32(FIELD_GET(DC_TI_VBAT_GE, val), 3);
+ info->vbat_zse = FIELD_GET_SIGNED(DC_TI_VBAT_ZSE, val);
+ info->vbat_ge = FIELD_GET_SIGNED(DC_TI_VBAT_GE, val);
dev_dbg(dev, "vbat-zse %d vbat-ge %d\n", info->vbat_zse, info->vbat_ge);
--
2.51.0
^ permalink raw reply related
* [PATCH v2 2/9] x86/extable: switch to using FIELD_GET_SIGNED()
From: Yury Norov @ 2026-04-27 21:41 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
H. Peter Anvin, Andy Lutomirski, Peter Zijlstra, Jonathan Cameron,
David Lechner, Johannes Berg, David Laight, Nuno Sá,
Andy Shevchenko, Ping-Ke Shih, Richard Cochran, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Alexandre Belloni, Yury Norov, Rasmus Villemoes, Hans de Goede,
Linus Walleij, Sakari Ailus, Salah Triki, Achim Gratz,
Ben Collins, x86, linux-kernel, linux-iio, linux-wireless, netdev,
linux-rtc
Cc: Yury Norov
In-Reply-To: <20260427214127.406067-1-ynorov@nvidia.com>
The EX_DATA register is laid out such that EX_DATA_IMM occupied MSB.
It's done to make sure that FIELD_GET() will sign-extend the IMM
field during extraction.
To enforce that, all EX_DATA masks are made signed integers. This
works, but relies on the particular implementation of FIELD_GET(),
i.e. masking then shifting, not vice versa; and the particular
placement of the fields in the register.
Switch to using the dedicated FIELD_GET_SIGNED(), and relax those
limitations.
Signed-off-by: Yury Norov <ynorov@nvidia.com>
---
arch/x86/include/asm/extable_fixup_types.h | 13 ++++---------
arch/x86/mm/extable.c | 2 +-
2 files changed, 5 insertions(+), 10 deletions(-)
diff --git a/arch/x86/include/asm/extable_fixup_types.h b/arch/x86/include/asm/extable_fixup_types.h
index 906b0d5541e8..fd0cfb472103 100644
--- a/arch/x86/include/asm/extable_fixup_types.h
+++ b/arch/x86/include/asm/extable_fixup_types.h
@@ -2,15 +2,10 @@
#ifndef _ASM_X86_EXTABLE_FIXUP_TYPES_H
#define _ASM_X86_EXTABLE_FIXUP_TYPES_H
-/*
- * Our IMM is signed, as such it must live at the top end of the word. Also,
- * since C99 hex constants are of ambiguous type, force cast the mask to 'int'
- * so that FIELD_GET() will DTRT and sign extend the value when it extracts it.
- */
-#define EX_DATA_TYPE_MASK ((int)0x000000FF)
-#define EX_DATA_REG_MASK ((int)0x00000F00)
-#define EX_DATA_FLAG_MASK ((int)0x0000F000)
-#define EX_DATA_IMM_MASK ((int)0xFFFF0000)
+#define EX_DATA_TYPE_MASK (0x000000FF)
+#define EX_DATA_REG_MASK (0x00000F00)
+#define EX_DATA_FLAG_MASK (0x0000F000)
+#define EX_DATA_IMM_MASK (0xFFFF0000)
#define EX_DATA_REG_SHIFT 8
#define EX_DATA_FLAG_SHIFT 12
diff --git a/arch/x86/mm/extable.c b/arch/x86/mm/extable.c
index 6b9ff1c6cafa..ceb8d03191ab 100644
--- a/arch/x86/mm/extable.c
+++ b/arch/x86/mm/extable.c
@@ -322,7 +322,7 @@ int fixup_exception(struct pt_regs *regs, int trapnr, unsigned long error_code,
type = FIELD_GET(EX_DATA_TYPE_MASK, e->data);
reg = FIELD_GET(EX_DATA_REG_MASK, e->data);
- imm = FIELD_GET(EX_DATA_IMM_MASK, e->data);
+ imm = FIELD_GET_SIGNED(EX_DATA_IMM_MASK, e->data);
switch (type) {
case EX_TYPE_DEFAULT:
--
2.51.0
^ permalink raw reply related
* [PATCH v2 1/9] bitfield: add FIELD_GET_SIGNED()
From: Yury Norov @ 2026-04-27 21:41 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
H. Peter Anvin, Andy Lutomirski, Peter Zijlstra, Jonathan Cameron,
David Lechner, Johannes Berg, David Laight, Nuno Sá,
Andy Shevchenko, Ping-Ke Shih, Richard Cochran, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Alexandre Belloni, Yury Norov, Rasmus Villemoes, Hans de Goede,
Linus Walleij, Sakari Ailus, Salah Triki, Achim Gratz,
Ben Collins, x86, linux-kernel, linux-iio, linux-wireless, netdev,
linux-rtc
Cc: Yury Norov
In-Reply-To: <20260427214127.406067-1-ynorov@nvidia.com>
The bitfields are designed in assumption that fields contain unsigned
integer values, thus extracting the values from the field implies
zero-extending.
Some drivers need to sign-extend their fields, and currently do it like:
dc_re += sign_extend32(FIELD_GET(0xfff000, tmp), 11);
dc_im += sign_extend32(FIELD_GET(0xfff, tmp), 11);
It's error-prone because it relies on user to provide the correct
index of the most significant bit and proper 32 vs 64 function flavor.
Thus, introduce a FIELD_GET_SIGNED(). With the new API, the above
snippet turns into the more convenient:
dc_re += FIELD_GET_SIGNED(0xfff000, tmp);
dc_im += FIELD_GET_SIGNED(0xfff, tmp);
It compiles (on x86_64) into just a couple instructions: shl and sar.
When the mask includes MSB, the '<< __builtin_clzll(mask)' part becomes
a NOP, and the compiler only emits a single sar:
long long foo(long long reg)
{
10: f3 0f 1e fa endbr64
return FIELD_GET_SIGNED(GENMASK_ULL(63, 60), reg);
14: 48 89 f8 mov %rdi,%rax
17: 48 c1 f8 3c sar $0x3c,%rax
}
32-bit code generation is equally well. On arm32:
long long foo(long long reg)
{
return FIELD_GET_SIGNED(0x00f00000ULL, reg);
}
generates:
foo(long long):
lsls r1, r0, #8
asrs r0, r1, #28
asrs r1, r1, #31
bx lr
Signed-off-by: Yury Norov <ynorov@nvidia.com>
---
include/linux/bitfield.h | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
index 54aeeef1f0ec..cd44013281c7 100644
--- a/include/linux/bitfield.h
+++ b/include/linux/bitfield.h
@@ -178,6 +178,22 @@
__FIELD_GET(_mask, _reg, "FIELD_GET: "); \
})
+/**
+ * FIELD_GET_SIGNED() - extract a signed bitfield element
+ * @mask: shifted mask defining the field's length and position
+ * @reg: value of entire bitfield
+ *
+ * Returns the sign-extended field specified by @_mask from the
+ * bitfield passed in as @_reg by masking and shifting it down.
+ */
+#define FIELD_GET_SIGNED(mask, reg) \
+ ({ \
+ __BF_FIELD_CHECK(mask, reg, 0U, "FIELD_GET_SIGNED: "); \
+ ((__signed_scalar_typeof(mask)) \
+ (((long long)(reg) << __builtin_clzll(mask)) >> \
+ (__builtin_clzll(mask) + __builtin_ctzll(mask)))); \
+ })
+
/**
* FIELD_MODIFY() - modify a bitfield element
* @_mask: shifted mask defining the field's length and position
--
2.51.0
^ permalink raw reply related
* [PATCH v2 0/9] bitfield: add FIELD_GET_SIGNED()
From: Yury Norov @ 2026-04-27 21:41 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
H. Peter Anvin, Andy Lutomirski, Peter Zijlstra, Jonathan Cameron,
David Lechner, Johannes Berg, David Laight, Nuno Sá,
Andy Shevchenko, Ping-Ke Shih, Richard Cochran, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Alexandre Belloni, Yury Norov, Rasmus Villemoes, Hans de Goede,
Linus Walleij, Sakari Ailus, Salah Triki, Achim Gratz,
Ben Collins, x86, linux-kernel, linux-iio, linux-wireless, netdev,
linux-rtc
Cc: Yury Norov
The bitfields are designed in assumption that fields contain unsigned
integer values, thus extracting the values from the field implies
zero-extending.
Some drivers need to sign-extend their fields, and currently do it like:
dc_re += sign_extend32(FIELD_GET(0xfff000, tmp), 11);
dc_im += sign_extend32(FIELD_GET(0xfff, tmp), 11);
It's error-prone because it relies on user to provide the correct
index of the most significant bit and proper 32 vs 64 function flavor.
Thus, introduce a FIELD_GET_SIGNED(). With the new API, the above
snippet turns into the more convenient:
dc_re += FIELD_GET_SIGNED(0xfff000, tmp);
dc_im += FIELD_GET_SIGNED(0xfff, tmp);
It compiles (on x86_64) into just a couple instructions: shl and sar.
When the mask includes MSB, the '<< __builtin_clzll(mask)' part becomes
a NOP, and the compiler only emits a single sar:
long long foo(long long reg)
{
10: f3 0f 1e fa endbr64
return FIELD_GET_SIGNED(GENMASK_ULL(63, 60), reg);
14: 48 89 f8 mov %rdi,%rax
17: 48 c1 f8 3c sar $0x3c,%rax
}
32-bit code generation is equally well. On arm32:
long long foo(long long reg)
{
return FIELD_GET_SIGNED(0x00f00000ULL, reg);
}
generates:
foo(long long):
lsls r1, r0, #8
asrs r0, r1, #28
asrs r1, r1, #31
bx lr
Immutable branch:
https://github.com/norov/linux/pull/new/fgsv2
v1: https://lore.kernel.org/all/20260417173621.368914-1-ynorov@nvidia.com/
v2:
- more examples of the new API and code generation (Andy, David);
- fix #7 FIELD_GET() / FIELD_GET_SIGNED() typo (Ping-Ke);
- re-indent the macro (Andy, Peter);
Yury Norov (9):
bitfield: add FIELD_GET_SIGNED()
x86/extable: switch to using FIELD_GET_SIGNED()
iio: intel_dc_ti_adc: switch to using FIELD_GET_SIGNED()
iio: magnetometer: yas530: switch to using FIELD_GET_SIGNED()
iio: pressure: bmp280: switch to using FIELD_GET_SIGNED()
iio: mcp9600: switch to using FIELD_GET_SIGNED()
wifi: rtw89: switch to using FIELD_GET_SIGNED()
rtc: rv3032: switch to using FIELD_GET_SIGNED()
ptp: switch to using FIELD_GET_SIGNED()
arch/x86/include/asm/extable_fixup_types.h | 13 ++++---------
arch/x86/mm/extable.c | 2 +-
drivers/iio/adc/intel_dc_ti_adc.c | 4 ++--
drivers/iio/magnetometer/yamaha-yas530.c | 12 ++++++------
drivers/iio/pressure/bmp280-core.c | 2 +-
drivers/iio/temperature/mcp9600.c | 2 +-
.../net/wireless/realtek/rtw89/rtw8852a_rfk.c | 4 ++--
.../net/wireless/realtek/rtw89/rtw8852b_common.c | 4 ++--
.../net/wireless/realtek/rtw89/rtw8852b_rfk.c | 4 ++--
drivers/net/wireless/realtek/rtw89/rtw8852c.c | 4 ++--
drivers/ptp/ptp_fc3.c | 4 ++--
drivers/rtc/rtc-rv3032.c | 2 +-
include/linux/bitfield.h | 16 ++++++++++++++++
13 files changed, 42 insertions(+), 31 deletions(-)
--
2.51.0
^ permalink raw reply
* Re: [RFC PATCH v1 5/9] uaccess: Switch to copy_{to/from}_user_partial() when relevant
From: Linus Torvalds @ 2026-04-27 21:39 UTC (permalink / raw)
To: David Laight
Cc: Christophe Leroy (CS GROUP), Yury Norov, Andrew Morton,
Thomas Gleixner, linux-alpha, linux-kernel, linux-snps-arc,
linux-arm-kernel, linux-mips, linuxppc-dev, kvm, linux-riscv,
linux-s390, sparclinux, linux-um, dmaengine, linux-efi, linux-fsi,
amd-gfx, dri-devel, intel-gfx, linux-wpan, netdev, linux-wireless,
linux-spi, linux-media, linux-staging, linux-serial, linux-usb,
xen-devel, linux-fsdevel, ocfs2-devel, bpf, kasan-dev, linux-mm,
linux-x25, rust-for-linux, linux-sound, sound-open-firmware,
linux-csky, linux-hexagon, loongarch, linux-m68k, linux-openrisc,
linux-parisc, linux-sh, linux-arch
In-Reply-To: <20260427222914.1cb2dd3b@pumpkin>
On Mon, 27 Apr 2026 at 14:29, David Laight <david.laight.linux@gmail.com> wrote:
>
> I think there is a slight difference in that the normal copy_to_user()
> will determine the exact offset of the error by retrying with byte copies.
I have this dim memory that we decided that you can't reply on byte
exactness anyway, because not all architectures gave that guarantee
for the user copies.
But that thing came up many years ago, I might mis-remember.
Linus
^ permalink raw reply
* Re: [PATCH net-next v2] r8169: expose software counters through netdev qstats
From: Gustavo Arantes @ 2026-04-27 21:35 UTC (permalink / raw)
To: Heiner Kallweit
Cc: nic_swsd, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Javen Xu, netdev, linux-kernel
In-Reply-To: <a7b8a496-baf9-4e0e-b1c0-510e24d0bbaa@gmail.com>
On Mon, Apr 27, 2026 at 11:21:44PM +0200, Heiner Kallweit wrote:
> What do you mean with RTNL here?
I meant the rtnetlink link statistics exposed through ndo_get_stats64(),
i.e. struct rtnl_link_stats64.
> Counters are exposed to userspace already. So what's the benefit of
> duplicating this functionality? To me the patch looks like AI slop.
The benefit I had in mind was coverage for userspace using the netdev
qstats generic-netlink interface, so that it could query r8169 through
the same qstats path as drivers which already implement netdev_stat_ops.
That said, it does not expose additional driver-specific information.
> This is generic, driver-independent code. If exposing dev->tstats this
> way makes sense, then this should go to net core.
Agreed, I'll drop this version. Thanks for the review.
^ permalink raw reply
* Re: [RFC PATCH v1 5/9] uaccess: Switch to copy_{to/from}_user_partial() when relevant
From: David Laight @ 2026-04-27 21:29 UTC (permalink / raw)
To: Linus Torvalds
Cc: Christophe Leroy (CS GROUP), Yury Norov, Andrew Morton,
Thomas Gleixner, linux-alpha, linux-kernel, linux-snps-arc,
linux-arm-kernel, linux-mips, linuxppc-dev, kvm, linux-riscv,
linux-s390, sparclinux, linux-um, dmaengine, linux-efi, linux-fsi,
amd-gfx, dri-devel, intel-gfx, linux-wpan, netdev, linux-wireless,
linux-spi, linux-media, linux-staging, linux-serial, linux-usb,
xen-devel, linux-fsdevel, ocfs2-devel, bpf, kasan-dev, linux-mm,
linux-x25, rust-for-linux, linux-sound, sound-open-firmware,
linux-csky, linux-hexagon, loongarch, linux-m68k, linux-openrisc,
linux-parisc, linux-sh, linux-arch
In-Reply-To: <CAHk-=whC1DZojwdMB1=sJWG2=dsCdfyU8N6tDE1qx50HRZ-WJQ@mail.gmail.com>
On Mon, 27 Apr 2026 12:01:23 -0700
Linus Torvalds <torvalds@linux-foundation.org> wrote:
> On Mon, 27 Apr 2026 at 10:18, Christophe Leroy (CS GROUP)
> <chleroy@kernel.org> wrote:
> >
> > In a subsequent patch, copy_{to/from}_user() will be modified to
> > return -EFAULT when copy fails.
>
> Please don't do this.
>
> This is a maintenance nightmare, and changes pretty much three decades
> of semantics, and will cause *very* subtle backporting issues if
> somebody happens to rely on the old / new behavior.
>
> I understand the reasoning for the change, but I really don't think
> the pain of creating yet another user copy interface is worth it.
>
> We already have a lot of different versions of user copies for
> different reasons, and while they all tend to have a good reason (and
> some not-so-good, but historical reasons) for existing, this one
> doesn't seem worth it.
>
> The main - perhaps only - reason for this "partial" version is that
> you want to do that "automatically inlined and optimized fixed-sized
> case".
>
> But here's the thing: I think you can already do that. Yes, it
> requires some improvements to unsafe_copy_from_user(), but *that*
> interface doesn't have three decades of history associated with it,
> _and_ you're extending on that one anyway in this series.
>
> "unsafe_copy_from_user()" is very odd, is meant only for small simple
> copies that can be inlined and it's special-cased for 'objtool' anyway
> (because objtool would have complained about an out-of-line call,
> although it could have been special-cased other ways).
>
> In other words: unsafe_copy_from_user() is *very* close to what you
> want for that "Oh, I noticed that it's a small fixed-size copy, so I
> want to special-case copy-from-user for that".
>
> The _only_ issue with unsafe_copy_from_user() is that you can't see
> that there were partial successes. But if *that* was fixed, then this
> whole "create a new copy_from_user interface" issue would just go
> away.
>
> So please - let's just change unsafe_copy_from_user() to be usable for
> the partial case.
>
> And the thing is, all the existing unsafe_copy_from_user()
> implementations already effectively *have* the "how much did I not
> copy" internally, and they actually do extra work to hide it, ie they
> have things like that
>
> int _i;
>
> that is "how many bytes have I copied" in the powerpc implementation,
> or the x86 code does
>
> size_t __ucu_len = (_len);
>
> where that "ucu_len" is updated as you go along and is literally the
> "how many bytes are left to copy" return value that is missing from
> this interface.
>
> So what I would suggest is
>
> - introduce a new user accessor helper that is used for *both*
> unsafe_copy_to/from_user() *and* the "inline small constant-sized
> normal copy_to/from_user()" calls
>
> - it's the same thing as the existing unsafe_copy_to/from_user()
> implementation, except it exposes how many bytes are left to be copied
> to the exception label.
I think there is a slight difference in that the normal copy_to_user()
will determine the exact offset of the error by retrying with byte copies.
There is also the issue of misaligned copies.
Then there is the 'bugbear' of hardened user copies.
Chasing down the stack to find whether the kernel buffer crosses
a stack frame is probably more expensive than the copy for the typically
small copies that will use on-stack buffers.
David
>
> IOW, it would look something like
>
> #define unsafe_copy_to_user_outlen(_dst,_src,_len,label)...
>
> which is exactly the same as the current unsafe_copy_to_user(),
> *except* it changes "_len" as it does along.
>
> And then you use that for both the "real" unsafe_copy_user and for the
> "small constant values" case.
>
> Just as an example, attached is a completely stupid rough draft of a
> patch that does this for x86 and only for unsafe_copy_to_user().
>
> And I made a very very hacky change to kernel/sys.c to see what the
> code generation looks like.
>
> This is what it results in on x86 with clang (with all the magic
> .section data edited out):
>
> ... edited out the code to generate the times
> ... this is the actual user copy:
> # HERE!
> movabsq $81985529216486895, %rcx # imm = 0x123456789ABCDEF
> cmpq %rcx, %rbx
> cmovaq %rcx, %rbx
> stac
> movq %r13, (%rbx) # exception to .LBB45_8
> movq %r14, 8(%rbx) # exception to .LBB45_8
> movq %r15, 16(%rbx) # exception to .LBB45_8
> movq %rax, 24(%rbx) # exception to .LBB45_8
> clac
> .LBB45_6:
> movq jiffies(%rip), %rdi
> callq jiffies_64_to_clock_t
> .LBB45_7:
> addq $16, %rsp
> popq %rbx
> popq %r12
> popq %r13
> popq %r14
> popq %r15
> retq
> .LBB45_8:
> clac
> movq $-14, %rax
> jmp .LBB45_7
>
> and notice how the compiler noticed that the 'outlen' isn't actually
> used, and turned the exception label into just a "return -EFAULT" and
> never actually generated any code for updating remaining lengths?
>
> That actually looks pretty much optimal for a 32-byte user copy.
>
> And it didn't involve changing the semantics at all.
>
> Just to check, I changed that "times()" system call to return the
> number of bytes uncopied instead (to emulate the "I actually want to
> know what's left" case), and it generated this:
>
> # HERE!
> movabsq $81985529216486895, %rcx # imm = 0x123456789ABCDEF
> cmpq %rcx, %rbx
> cmovaq %rcx, %rbx
> stac
> movl $32, %ecx
> movq %r13, (%rbx) # exception to .LBB45_7
> movl $24, %ecx
> movq %r15, 8(%rbx) # exception to .LBB45_7
> movl $16, %ecx
> movq %r14, 16(%rbx) # exception to .LBB45_7
> movl $8, %ecx
> movq %rax, 24(%rbx) # exception to .LBB45_7
> clac
> xorl %ecx, %ecx
> .LBB45_8:
> movq %rcx, %rax
> addq $16, %rsp
> popq %rbx
> popq %r12
> popq %r13
> popq %r14
> popq %r15
> retq
> .LBB45_6:
> movq jiffies(%rip), %rdi
> jmp jiffies_64_to_clock_t # TAILCALL
> .LBB45_7:
> clac
> jmp .LBB45_8
>
> so it all seems to work - although obviously the above is *not* the normal case.
>
> NOTE NOTE NOTE! The attached patch is entirely untested. I obviously
> did some "test code generation" with it, but I only *looked* at the
> result, and maybe it has some fundamental problem that I just didn't
> notice. So treat this as a "how about this approach" patch, not as
> anything more serious than that.
>
> And the kerrnel/sys.c hack is very obviously just that: a complate
> hack for testing.
>
> A real patch would do that "for small constant-sized copies, turn
> copy_to_user() automatically into "_small_copy_to_user()".
>
> The attached is *not* a real patch. Treat it with the contempt it deserves.
>
> Linus
^ permalink raw reply
* Re: [PATCH net-next v2] r8169: expose software counters through netdev qstats
From: Heiner Kallweit @ 2026-04-27 21:21 UTC (permalink / raw)
To: Gustavo Arantes, nic_swsd
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Javen Xu, netdev, linux-kernel
In-Reply-To: <20260427205931.5884-1-dev.gustavoa@gmail.com>
On 27.04.2026 22:59, Gustavo Arantes wrote:
> r8169 maintains synchronized per-CPU software counters for packet and
> byte accounting and exposes them through ndo_get_stats64(), but
> userspace using the structured netdev qstats API cannot retrieve them from
> this driver.
>
> Expose the same counters through netdev_stat_ops. r8169 has a single Rx
> and Tx queue, so report the accumulated counters on queue 0. Use zero
> base stats so device-scope qstats are derived from the queue counters
> and match the packet and byte values reported through RTNL.
What do you mean with RTNL here?
>
> This does not add new accounting and does not touch the data path. It
> only makes the existing counters available through the common qstats
> interface, which lets generic userspace tooling query r8169 the same way
> it queries other drivers with qstats support.
>
Counters are exposed to userspace already. So what's the benefit of
duplicating this functionality? To me the patch looks like AI slop.
> Signed-off-by: Gustavo Arantes <dev.gustavoa@gmail.com>
> ---
> v2:
> - Submit again now that net-next has reopened.
> - Expand commit message to explain the qstats userspace benefit.
>
> Tested on a Realtek RTL8111/8168/8211/8411 PCIe Gigabit Ethernet
> controller using the r8169 driver.
>
> v1: https://lore.kernel.org/netdev/20260418021232.5425-1-dev.gustavoa@gmail.com/
>
> drivers/net/ethernet/realtek/r8169_main.c | 70 +++++++++++++++++++++++
> 1 file changed, 70 insertions(+)
>
> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
> index 791277e750ba..9d833b446383 100644
> --- a/drivers/net/ethernet/realtek/r8169_main.c
> +++ b/drivers/net/ethernet/realtek/r8169_main.c
> @@ -5175,6 +5175,75 @@ rtl8169_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *stats)
> pm_runtime_put_noidle(&pdev->dev);
> }
>
> +static void rtl8169_fetch_sw_stats(struct net_device *dev,
> + struct netdev_queue_stats_rx *rx,
> + struct netdev_queue_stats_tx *tx)
> +{
> + const struct pcpu_sw_netstats *stats;
> + unsigned int start;
> + int cpu;
> +
> + for_each_possible_cpu(cpu) {
> + u64 rx_packets, rx_bytes, tx_packets, tx_bytes;
> +
> + stats = per_cpu_ptr(dev->tstats, cpu);
> + do {
> + start = u64_stats_fetch_begin(&stats->syncp);
> + rx_packets = u64_stats_read(&stats->rx_packets);
> + rx_bytes = u64_stats_read(&stats->rx_bytes);
> + tx_packets = u64_stats_read(&stats->tx_packets);
> + tx_bytes = u64_stats_read(&stats->tx_bytes);
> + } while (u64_stats_fetch_retry(&stats->syncp, start));
> +
> + rx->packets += rx_packets;
> + rx->bytes += rx_bytes;
> + tx->packets += tx_packets;
> + tx->bytes += tx_bytes;
> + }
> +}
This is generic, driver-independent code. If exposing dev->tstats this
way makes sense, then this should go to net core.
> +
> +static void rtl8169_get_queue_stats_rx(struct net_device *dev, int idx,
> + struct netdev_queue_stats_rx *rx)
> +{
> + struct netdev_queue_stats_tx tx = {};
> +
> + if (idx)
> + return;
> +
> + rx->packets = 0;
> + rx->bytes = 0;
> + rtl8169_fetch_sw_stats(dev, rx, &tx);
> +}
> +
> +static void rtl8169_get_queue_stats_tx(struct net_device *dev, int idx,
> + struct netdev_queue_stats_tx *tx)
> +{
> + struct netdev_queue_stats_rx rx = {};
> +
> + if (idx)
> + return;
> +
> + tx->packets = 0;
> + tx->bytes = 0;
> + rtl8169_fetch_sw_stats(dev, &rx, tx);
> +}
> +
> +static void rtl8169_get_base_stats(struct net_device *dev,
> + struct netdev_queue_stats_rx *rx,
> + struct netdev_queue_stats_tx *tx)
> +{
> + rx->packets = 0;
> + rx->bytes = 0;
> + tx->packets = 0;
> + tx->bytes = 0;
> +}
> +
> +static const struct netdev_stat_ops rtl8169_stat_ops = {
> + .get_queue_stats_rx = rtl8169_get_queue_stats_rx,
> + .get_queue_stats_tx = rtl8169_get_queue_stats_tx,
> + .get_base_stats = rtl8169_get_base_stats,
> +};
> +
> static void rtl8169_net_suspend(struct rtl8169_private *tp)
> {
> netif_device_detach(tp->dev);
> @@ -5615,6 +5684,7 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>
> SET_NETDEV_DEV(dev, &pdev->dev);
> dev->netdev_ops = &rtl_netdev_ops;
> + dev->stat_ops = &rtl8169_stat_ops;
> tp = netdev_priv(dev);
> tp->dev = dev;
> tp->pci_dev = pdev;
> --
> 2.54.0
>
^ permalink raw reply
* Re: [BUG] mlx5: VLAN-aware bridge drops all traffic in legacy eswitch mode without promiscuous
From: bryan @ 2026-04-27 21:10 UTC (permalink / raw)
To: Dragos Tatulea, netdev; +Cc: saeedm, tariqt
In-Reply-To: <1126aa35-1924-492f-8d7f-072c0dec9bde@nvidia.com>
Here would be one example config (sanitized). The promisc on link is
what allows traffic to pass - I disable promisc, and traffic stops.
These are single-port CX4Lx cards. nic0 is the physical interface, no
VFs configured, SrIOV has been disabled as part of testing and
troubleshooting, Kernel 6.17 currently:
auto lo
iface lo inet loopback
auto nic0
iface nic0 inet manual
up ip link set nic0 promisc on
auto vmbr0
iface vmbr0 inet manual
bridge-ports nic0
bridge-stp off
bridge-fd 0
bridge-vlan-aware yes
bridge-vids 2-10 555
auto vmbr0.555
iface vmbr0.555 inet static
address 192.168.1.123/24
gateway 192.168.1.1
iface nic3 inet manual
iface nic1 inet manual
iface nic2 inet manual
This was ported over to use the new nic# bindings, before this it was
the standard enps0np0 naming. No difference in behvaiour.
>Is this even with one vlan? I ran a flow on a CX4LX pair with one vlan
>and vlan_filtering set and traffic seems to be flowing normally.
I have not checked with literally only one VLAN, as that is not at all
the use case. I can absolutely test that if it would help! Would you
like me to remove every VLAN but 555 from the interface, and leave the
rest of the config as-is?
>This last link seems the only one that provides some extra data. From
it I can see that the amount of VLAN ids > what the FW supports. This
could result in loss of traffic for the vlan ids > 512. Do you also see
in your dmesg these kinds of errors:
>mlx5_core 0000:19:00.1: mlx5e_vport_context_update_vlans:179:(pid
13470): netdev vlans list size (4080) > (512) max vport list size, some
vlans will be dropped
>This is not a bug, simply a limit being reached.
Considering I am only operating with 10 VLANs, as can be seen in my
config, I do not think that is my issue. I am aware that there is also
quite a bit of noise in these threads - they are just forum posts - But
that does not appear to me to be related to my issue, or the issue of
others here. I get no such messages or warnings. Additionally reports
about that bug were about some some VLANs being dropped. In my case
(and as others have reported) all VLANs are being dropped.
>eth2 is a PF in legacy switchdev mode.
It was my understanding that Legacy mode and Switchdev mode were two
independent modes, with Legacy done in-software and Switchdev using the
eSwitch on the NIC itself. Please excuse my ignorance if that is not
the case. Would you be able to specify if you used Switchdev mode or
Legacy mode? because Switchdev mode DOES function as a workaround and
passes traffic (but in my case results in system instability after a
time).
Thank You,
Bryan
On Mon, 2026-04-27 at 15:55 +0200, Dragos Tatulea wrote:
> Hi,
>
> On 24.04.26 13:07, bryan wrote:
> > Good day,
> >
> > I wanted to check whether there is an open bug report or known fix
> > in
> > progress for an issue that has been affecting mlx5 users
> > (specifically
> > ConnectX-4 Lx, but likely broader from what I have seen other
> > reporting) since at least 2021:
> >
> > When an mlx5 interface is added as a port to a VLAN-aware Linux
> > bridge
> > (bridge-vlan-aware yes / vlan_filtering 1) in legacy eswitch mode,
> > all
> > traffic stops passing through the bridge. Both tagged and untagged
> > traffic is affected. The same configuration works correctly with
> > non-
> > mlx5 NICs (tested Intel, Chelsio cards).
> >
> Is this even with one vlan? I ran a flow on a CX4LX pair with one
> vlan
> and vlan_filtering set and traffic seems to be flowing normally.
> Something like:
>
> # IFACE=eth2
> # VID=100
> # ip link add br0 type bridge vlan_filtering 1
> # ip link set "$IFACE" master br0
> # bridge vlan add vid "$VID" dev "$IFACE"
> # bridge vlan add vid "$VID" dev br0 self
> # ip link add link br0 name "br0.$VID" type vlan id "$VID"
> # ip addr add 10.0.0.1/24 dev br0
> # ip addr add "10.0.$VID.1/24" dev "br0.$VID"
> # ip link set "$IFACE" up
> # ip link set br0 up
> # ip link set "br0.$VID" up
>
> From the other side where I have a similar setup I can ping
> br0.100.
>
> Tested on a CX4LX with FW version 28.48.1000 and kernel 6.18.
> eth2 is a PF in legacy switchdev mode.
>
> > [...]
> > This is well documented in community forums but does not appear to
> > have
> > been formally reported to netdev that I have been able to find. My
> > apologies in advance if this has been reported and I wasn't able to
> > locate it. Here are a couple of forum examples where this is
> > discussed
> > among other affected users:
> >
> > - NVIDIA Developer Forum (opened 2021, unresolved):
> >
> > https://forums.developer.nvidia.com/t/vlan-aware-linux-bridging-is-not-functional-on-connectx4lx-card-unless-manually-put-in-promiscuous-mode/206083
> >
> > - Proxmox Forum thread (2023, ongoing):
> >
> > https://forum.proxmox.com/threads/mellanox-connectx-4-lx-and-brigde-vlan-aware-on-proxmox-8-0-1.130902/
> >
> > - Community writeup with analysis:
> > https://www.apalrd.net/posts/2023/tip_mellanox/
> >
> This last link seems the only one that provides some extra data. From
> it
> I can see that the amount of VLAN ids > what the FW supports. This
> could
> result in loss of traffic for the vlan ids > 512. Do you also see in
> your dmesg these kinds of errors:
>
> mlx5_core 0000:19:00.1: mlx5e_vport_context_update_vlans:179:(pid
> 13470): netdev vlans list size (4080) > (512) max vport list size,
> some vlans will be dropped
>
> This is not a bug, simply a limit being reached.
>
> > Has anyone bisected this or is there a fix already in progress that
> > I
> > did not find? This affects a fairly common hypervisor configuration
> > (VLAN-aware bridge for VM networking) and the workarounds are not
> > conducive to production use.
> >
> Could you provide a short repro script for this. Not being able to
> reproduce the issue makes it hard to check :).
>
> Thanks,
> Dragos
^ permalink raw reply
* Re: [PATCH net] net: psp: require admin permission for dev-set and key-rotate
From: Daniel Zahka @ 2026-04-27 21:04 UTC (permalink / raw)
To: Jakub Kicinski, davem
Cc: netdev, edumazet, pabeni, andrew+netdev, horms,
willemdebruijn.kernel, donald.hunter
In-Reply-To: <20260427195856.401223-1-kuba@kernel.org>
On 4/27/26 3:58 PM, Jakub Kicinski wrote:
> The dev-set and key-rotate netlink operations modify shared device
> state (PSP version configuration and cryptographic key material,
> respectively) but do not require CAP_NET_ADMIN. The only access
> control is psp_dev_check_access() which merely verifies netns
> membership.
>
> Fixes: 00c94ca2b99e ("psp: base PSP device support")
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Reviewed-by: Daniel Zahka <daniel.zahka@gmail.com>
^ permalink raw reply
* [PATCH net-next v2] r8169: expose software counters through netdev qstats
From: Gustavo Arantes @ 2026-04-27 20:59 UTC (permalink / raw)
To: Heiner Kallweit, nic_swsd
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Javen Xu, netdev, linux-kernel, Gustavo Arantes
r8169 maintains synchronized per-CPU software counters for packet and
byte accounting and exposes them through ndo_get_stats64(), but
userspace using the structured netdev qstats API cannot retrieve them from
this driver.
Expose the same counters through netdev_stat_ops. r8169 has a single Rx
and Tx queue, so report the accumulated counters on queue 0. Use zero
base stats so device-scope qstats are derived from the queue counters
and match the packet and byte values reported through RTNL.
This does not add new accounting and does not touch the data path. It
only makes the existing counters available through the common qstats
interface, which lets generic userspace tooling query r8169 the same way
it queries other drivers with qstats support.
Signed-off-by: Gustavo Arantes <dev.gustavoa@gmail.com>
---
v2:
- Submit again now that net-next has reopened.
- Expand commit message to explain the qstats userspace benefit.
Tested on a Realtek RTL8111/8168/8211/8411 PCIe Gigabit Ethernet
controller using the r8169 driver.
v1: https://lore.kernel.org/netdev/20260418021232.5425-1-dev.gustavoa@gmail.com/
drivers/net/ethernet/realtek/r8169_main.c | 70 +++++++++++++++++++++++
1 file changed, 70 insertions(+)
diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 791277e750ba..9d833b446383 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -5175,6 +5175,75 @@ rtl8169_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *stats)
pm_runtime_put_noidle(&pdev->dev);
}
+static void rtl8169_fetch_sw_stats(struct net_device *dev,
+ struct netdev_queue_stats_rx *rx,
+ struct netdev_queue_stats_tx *tx)
+{
+ const struct pcpu_sw_netstats *stats;
+ unsigned int start;
+ int cpu;
+
+ for_each_possible_cpu(cpu) {
+ u64 rx_packets, rx_bytes, tx_packets, tx_bytes;
+
+ stats = per_cpu_ptr(dev->tstats, cpu);
+ do {
+ start = u64_stats_fetch_begin(&stats->syncp);
+ rx_packets = u64_stats_read(&stats->rx_packets);
+ rx_bytes = u64_stats_read(&stats->rx_bytes);
+ tx_packets = u64_stats_read(&stats->tx_packets);
+ tx_bytes = u64_stats_read(&stats->tx_bytes);
+ } while (u64_stats_fetch_retry(&stats->syncp, start));
+
+ rx->packets += rx_packets;
+ rx->bytes += rx_bytes;
+ tx->packets += tx_packets;
+ tx->bytes += tx_bytes;
+ }
+}
+
+static void rtl8169_get_queue_stats_rx(struct net_device *dev, int idx,
+ struct netdev_queue_stats_rx *rx)
+{
+ struct netdev_queue_stats_tx tx = {};
+
+ if (idx)
+ return;
+
+ rx->packets = 0;
+ rx->bytes = 0;
+ rtl8169_fetch_sw_stats(dev, rx, &tx);
+}
+
+static void rtl8169_get_queue_stats_tx(struct net_device *dev, int idx,
+ struct netdev_queue_stats_tx *tx)
+{
+ struct netdev_queue_stats_rx rx = {};
+
+ if (idx)
+ return;
+
+ tx->packets = 0;
+ tx->bytes = 0;
+ rtl8169_fetch_sw_stats(dev, &rx, tx);
+}
+
+static void rtl8169_get_base_stats(struct net_device *dev,
+ struct netdev_queue_stats_rx *rx,
+ struct netdev_queue_stats_tx *tx)
+{
+ rx->packets = 0;
+ rx->bytes = 0;
+ tx->packets = 0;
+ tx->bytes = 0;
+}
+
+static const struct netdev_stat_ops rtl8169_stat_ops = {
+ .get_queue_stats_rx = rtl8169_get_queue_stats_rx,
+ .get_queue_stats_tx = rtl8169_get_queue_stats_tx,
+ .get_base_stats = rtl8169_get_base_stats,
+};
+
static void rtl8169_net_suspend(struct rtl8169_private *tp)
{
netif_device_detach(tp->dev);
@@ -5615,6 +5684,7 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
SET_NETDEV_DEV(dev, &pdev->dev);
dev->netdev_ops = &rtl_netdev_ops;
+ dev->stat_ops = &rtl8169_stat_ops;
tp = netdev_priv(dev);
tp->dev = dev;
tp->pci_dev = pdev;
--
2.54.0
^ permalink raw reply related
* Re: [RFC PATCH v1 2/9] uaccess: Convert INLINE_COPY_{TO/FROM}_USER to kconfig and reduce ifdefery
From: Yury Norov @ 2026-04-27 20:47 UTC (permalink / raw)
To: Andrew Cooper
Cc: Christophe Leroy (CS GROUP), Andrew Morton, Linus Torvalds,
David Laight, Thomas Gleixner, linux-alpha, Yury Norov,
linux-kernel, linux-snps-arc, linux-arm-kernel, linux-mips,
linuxppc-dev, kvm, linux-riscv, linux-s390, sparclinux, linux-um,
dmaengine, linux-efi, linux-fsi, amd-gfx, dri-devel, intel-gfx,
linux-wpan, netdev, linux-wireless, linux-spi, linux-media,
linux-staging, linux-serial, linux-usb, xen-devel, linux-fsdevel,
ocfs2-devel, bpf, kasan-dev, linux-mm, linux-x25, rust-for-linux,
linux-sound, sound-open-firmware, linux-csky, linux-hexagon,
loongarch, linux-m68k, linux-openrisc, linux-parisc, linux-sh,
linux-arch
In-Reply-To: <f54c3c2b-33da-42a0-80b7-0f6615d930ce@citrix.com>
On Mon, Apr 27, 2026 at 09:39:33PM +0100, Andrew Cooper wrote:
> On 27/04/2026 7:39 pm, Yury Norov wrote:
> > On Mon, Apr 27, 2026 at 07:13:43PM +0200, Christophe Leroy (CS GROUP) wrote:
> >> Among the 21 architectures supported by the kernel, 16 define both
> >> INLINE_COPY_TO_USER and INLINE_COPY_FROM_USER while the 5 other ones
> >> don't define any of the two.
> >>
> >> To simplify and reduce risk of mistakes, convert them to a single
> >> kconfig item named CONFIG_ARCH_WANTS_NOINLINE_COPY which will be
> > We've got a special word for it: outline. Can you name it
> > CONFIG_OUTLINE_USERCOPY, or similar?
>
> You can't swap the "in" for "out" like this. "out of line" is the
> opposite of "inline" in this context, while "outline" means something
> different and unrelated.
Check KASAN_OUTLINE vs KASAN_INLINE for example
^ permalink raw reply
* Re: [RFC PATCH v1 2/9] uaccess: Convert INLINE_COPY_{TO/FROM}_USER to kconfig and reduce ifdefery
From: Andrew Cooper @ 2026-04-27 20:39 UTC (permalink / raw)
To: Yury Norov, Christophe Leroy (CS GROUP)
Cc: Andrew Cooper, Andrew Morton, Linus Torvalds, David Laight,
Thomas Gleixner, linux-alpha, Yury Norov, linux-kernel,
linux-snps-arc, linux-arm-kernel, linux-mips, linuxppc-dev, kvm,
linux-riscv, linux-s390, sparclinux, linux-um, dmaengine,
linux-efi, linux-fsi, amd-gfx, dri-devel, intel-gfx, linux-wpan,
netdev, linux-wireless, linux-spi, linux-media, linux-staging,
linux-serial, linux-usb, xen-devel, linux-fsdevel, ocfs2-devel,
bpf, kasan-dev, linux-mm, linux-x25, rust-for-linux, linux-sound,
sound-open-firmware, linux-csky, linux-hexagon, loongarch,
linux-m68k, linux-openrisc, linux-parisc, linux-sh, linux-arch
In-Reply-To: <ae-tVFVfx72oCC_i@yury>
On 27/04/2026 7:39 pm, Yury Norov wrote:
> On Mon, Apr 27, 2026 at 07:13:43PM +0200, Christophe Leroy (CS GROUP) wrote:
>> Among the 21 architectures supported by the kernel, 16 define both
>> INLINE_COPY_TO_USER and INLINE_COPY_FROM_USER while the 5 other ones
>> don't define any of the two.
>>
>> To simplify and reduce risk of mistakes, convert them to a single
>> kconfig item named CONFIG_ARCH_WANTS_NOINLINE_COPY which will be
> We've got a special word for it: outline. Can you name it
> CONFIG_OUTLINE_USERCOPY, or similar?
You can't swap the "in" for "out" like this. "out of line" is the
opposite of "inline" in this context, while "outline" means something
different and unrelated.
~Andrew
^ permalink raw reply
* Re: [PATCH net v2 4/4] gve: Make ethtool config changes synchronous
From: Pin-yen Lin @ 2026-04-27 20:34 UTC (permalink / raw)
To: Harshitha Ramamurthy
Cc: netdev, joshwash, andrew+netdev, davem, edumazet, kuba, pabeni,
willemb, maolson, nktgrg, jfraker, ziweixiao, jacob.e.keller,
pkaligineedi, shailend, jordanrhee, stable, linux-kernel
In-Reply-To: <20260425002450.163421-5-hramamurthy@google.com>
On Fri, Apr 24, 2026 at 5:25 PM Harshitha Ramamurthy
<hramamurthy@google.com> wrote:
>
> From: Pin-yen Lin <treapking@google.com>
>
> When modifying device features via ethtool, the driver queues the
> carrier status update to its workqueue (gve_wq). This leads to a
> short link-down state after running the ethtool command.
>
> Use `gve_turnup_and_check_status()` instead of `gve_turnup()` in
> `gve_queues_start()` to update the carrier status before returning to
> the userspace.
>
> This was discovered by drivers/net/ping.py selftest. The test calls
> ping command right after an ethtool configuration, but the interface
> could be down without this fix.
>
> Cc: stable@vger.kernel.org
> Fixes: 5f08cd3d6423 ("gve: Alloc before freeing when adjusting queues")
> Signed-off-by: Pin-yen Lin <treapking@google.com>
> Reviewed-by: Joshua Washington <joshwash@google.com>
> Signed-off-by: Harshitha Ramamurthy <hramamurthy@google.com>
> ---
> drivers/net/ethernet/google/gve/gve_main.c | 56 +++++++++++-----------
> 1 file changed, 28 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/net/ethernet/google/gve/gve_main.c b/drivers/net/ethernet/google/gve/gve_main.c
> index 1fec8e1e4821..2c461be12a75 100644
> --- a/drivers/net/ethernet/google/gve/gve_main.c
> +++ b/drivers/net/ethernet/google/gve/gve_main.c
> @@ -1424,6 +1424,33 @@ static void gve_queues_mem_remove(struct gve_priv *priv)
> priv->rx = NULL;
> }
Sashiko says:
This isn't a bug introduced by this patch, but is there a pre-existing
use-after-free risk here involving stats_report_task?
Looking at gve_stats_report_task(), it iterates over priv->tx and priv->rx
to collect statistics without holding the rtnl_lock. During teardown,
gve_queues_stop() deletes the timer via timer_delete_sync(), but it does
not appear to explicitly cancel the work item with
cancel_work_sync(&priv->stats_report_task).
If the timer queues the work item just before timer_delete_sync() executes,
could the task remain pending on the workqueue?
When gve_close() then calls gve_queues_mem_remove(), the memory for
priv->tx and priv->rx is freed and the pointers are set to NULL. If the
pending stats_report_task runs concurrently and evaluates if (priv->tx)
right before the memory is freed, could it dereference stale pointers and
cause a use-after-free?
Additionally, during module removal, gve_teardown_priv_resources() frees
priv->stats_report and sets it to NULL. Since destroy_workqueue() is
called immediately after, could this flush and execute the pending task,
leading to a NULL pointer dereference?
Sashiko pointed out the same issue at patch 1/4. We will address this in v3.
>
> +static void gve_handle_link_status(struct gve_priv *priv, bool link_status)
> +{
> + if (!gve_get_napi_enabled(priv))
> + return;
> +
> + if (link_status == netif_carrier_ok(priv->dev))
> + return;
> +
> + if (link_status) {
> + netdev_info(priv->dev, "Device link is up.\n");
> + netif_carrier_on(priv->dev);
> + } else {
> + netdev_info(priv->dev, "Device link is down.\n");
> + netif_carrier_off(priv->dev);
> + }
> +}
> +
> +static void gve_turnup_and_check_status(struct gve_priv *priv)
> +{
> + u32 status;
> +
> + gve_turnup(priv);
> + status = ioread32be(&priv->reg_bar0->device_status);
> + gve_handle_link_status(priv,
> + GVE_DEVICE_STATUS_LINK_STATUS_MASK & status);
> +}
> +
> /* The passed-in queue memory is stored into priv and the queues are made live.
> * No memory is allocated. Passed-in memory is freed on errors.
> */
> @@ -1486,8 +1513,7 @@ static int gve_queues_start(struct gve_priv *priv,
> round_jiffies(jiffies +
> msecs_to_jiffies(priv->stats_report_timer_period)));
>
> - gve_turnup(priv);
> - queue_work(priv->gve_wq, &priv->service_task);
> + gve_turnup_and_check_status(priv);
> priv->interface_up_cnt++;
> return 0;
>
> @@ -1608,23 +1634,6 @@ static int gve_close(struct net_device *dev)
> return 0;
> }
>
> -static void gve_handle_link_status(struct gve_priv *priv, bool link_status)
> -{
> - if (!gve_get_napi_enabled(priv))
> - return;
> -
> - if (link_status == netif_carrier_ok(priv->dev))
> - return;
> -
> - if (link_status) {
> - netdev_info(priv->dev, "Device link is up.\n");
> - netif_carrier_on(priv->dev);
> - } else {
> - netdev_info(priv->dev, "Device link is down.\n");
> - netif_carrier_off(priv->dev);
> - }
> -}
> -
> static int gve_configure_rings_xdp(struct gve_priv *priv,
> u16 num_xdp_rings)
> {
> @@ -2099,15 +2108,6 @@ static void gve_turnup(struct gve_priv *priv)
> gve_set_napi_enabled(priv);
> }
>
> -static void gve_turnup_and_check_status(struct gve_priv *priv)
> -{
> - u32 status;
> -
> - gve_turnup(priv);
> - status = ioread32be(&priv->reg_bar0->device_status);
> - gve_handle_link_status(priv, GVE_DEVICE_STATUS_LINK_STATUS_MASK & status);
> -}
> -
> static struct gve_notify_block *gve_get_tx_notify_block(struct gve_priv *priv,
> unsigned int txqueue)
> {
> --
> 2.54.0.545.g6539524ca2-goog
>
Regards,
Pin-yen
^ permalink raw reply
* Re: [PATCH net v2 3/4] gve: Use default min ring size when device option values are 0
From: Pin-yen Lin @ 2026-04-27 20:33 UTC (permalink / raw)
To: Harshitha Ramamurthy
Cc: netdev, joshwash, andrew+netdev, davem, edumazet, kuba, pabeni,
willemb, maolson, nktgrg, jfraker, ziweixiao, jacob.e.keller,
pkaligineedi, shailend, jordanrhee, stable, linux-kernel
In-Reply-To: <20260425002450.163421-4-hramamurthy@google.com>
On Fri, Apr 24, 2026 at 5:25 PM Harshitha Ramamurthy
<hramamurthy@google.com> wrote:
>
> From: Pin-yen Lin <treapking@google.com>
>
> On gvnic devices that support reporting minimum ring sizes, the device
> option always includes the min_(rx|tx)_ring_size fields, and the values
> will be 0 if they are not configured to be exposed. This makes the
> driver allow unexpected small ring size configurations from the
> userspace.
>
> Use the default ring size in the driver if the min ring sizes from the
> device option are 0.
>
> This was discovered by drivers/net/ring_reconfig.py selftest.
>
> Cc: stable@vger.kernel.org
> Fixes: ed4fb326947d ("gve: add support to read ring size ranges from the device")
> Reviewed-by: Joshua Washington <joshwash@google.com>
> Reviewed-by: Jordan Rhee <jordanrhee@google.com>
> Signed-off-by: Pin-yen Lin <treapking@google.com>
> Signed-off-by: Harshitha Ramamurthy <hramamurthy@google.com>
> ---
> drivers/net/ethernet/google/gve/gve_adminq.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/google/gve/gve_adminq.c b/drivers/net/ethernet/google/gve/gve_adminq.c
> index 08587bf40ed4..2cd0dd6ced94 100644
> --- a/drivers/net/ethernet/google/gve/gve_adminq.c
> +++ b/drivers/net/ethernet/google/gve/gve_adminq.c
> @@ -189,7 +189,9 @@ void gve_parse_device_option(struct gve_priv *priv,
> *dev_op_modify_ring = (void *)(option + 1);
>
> /* device has not provided min ring size */
> - if (option_length == GVE_DEVICE_OPTION_NO_MIN_RING_SIZE)
> + if (option_length == GVE_DEVICE_OPTION_NO_MIN_RING_SIZE ||
> + be16_to_cpu((*dev_op_modify_ring)->min_rx_ring_size) == 0 ||
> + be16_to_cpu((*dev_op_modify_ring)->min_tx_ring_size) == 0)
Sashiko says:
Could this cause an out-of-bounds memory read when processing malformed
device option lengths?
Looking at the earlier validation in gve_parse_device_option() for
GVE_DEV_OPT_ID_MODIFY_RING, the length is only verified to be at least
GVE_DEVICE_OPTION_NO_MIN_RING_SIZE (8 bytes), while the full
struct gve_device_option_modify_ring is 12 bytes:
if (option_length < GVE_DEVICE_OPTION_NO_MIN_RING_SIZE ||
req_feat_mask != GVE_DEV_OPT_REQ_FEAT_MASK_MODIFY_RING) {
If the device provides an option_length of 9, 10, or 11, the initial
option_length == 8 check will evaluate to false. The code will then
evaluate the next conditions and read min_rx_ring_size and
min_tx_ring_size.
Since these fields are located at offsets 8-11, this reads past the end
of the provided option payload.
Because the descriptor buffer is allocated via a page-aligned dma_pool_alloc
of exactly GVE_ADMINQ_BUFFER_SIZE, if the malformed option ends exactly at
the page boundary, could this read cross into adjacent, potentially unmapped
memory and result in a page fault?
Would it be safer to verify that option_length is large enough to contain
the minimum ring size fields before dereferencing them?
GVNIC will never return an option length of 9, 10, or 11, so this
shouldn't be an issue.
> priv->default_min_ring_size = true;
> break;
> case GVE_DEV_OPT_ID_FLOW_STEERING:
> --
> 2.54.0.545.g6539524ca2-goog
>
Regards,
Pin-yen
^ permalink raw reply
* Re: [PATCH net v2 2/4] gve: Fix backward stats when interface goes down or configuration is adjusted
From: Pin-yen Lin @ 2026-04-27 20:30 UTC (permalink / raw)
To: Harshitha Ramamurthy
Cc: netdev, joshwash, andrew+netdev, davem, edumazet, kuba, pabeni,
willemb, maolson, nktgrg, jfraker, ziweixiao, jacob.e.keller,
pkaligineedi, shailend, jordanrhee, stable, linux-kernel,
Debarghya Kundu
In-Reply-To: <20260425002450.163421-3-hramamurthy@google.com>
On Fri, Apr 24, 2026 at 5:24 PM Harshitha Ramamurthy
<hramamurthy@google.com> wrote:
>
> From: Debarghya Kundu <debarghyak@google.com>
>
> gve_get_base_stats() sets all the stats to 0, so the stats go backwards
> when interface goes down or configuration is adjusted.
>
> Fix this by persisting baseline stats across interface down.
>
> Cc: stable@vger.kernel.org
> Fixes: 2e5e0932dff5 ("gve: add support for basic queue stats")
> Signed-off-by: Debarghya Kundu <debarghyak@google.com>
> Co-developed-by: Pin-yen Lin <treapking@google.com>
> Signed-off-by: Pin-yen Lin <treapking@google.com>
> Signed-off-by: Harshitha Ramamurthy <hramamurthy@google.com>
> ---
> Changes in v2:
> - Add a NULL pointer check in gve_get_ring_err_stats() (Sashiko)
> - Use local variable to prevent inflates from u64_stats_fetch_retry()
> (Sashiko)
> - Add u64_stats_fetch/begin to protect base stats (Sashiko)
>
> drivers/net/ethernet/google/gve/gve.h | 7 ++
> drivers/net/ethernet/google/gve/gve_main.c | 88 ++++++++++++++++++++--
> 2 files changed, 88 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/ethernet/google/gve/gve.h b/drivers/net/ethernet/google/gve/gve.h
> index 1d66d3834f7e..702b1641d984 100644
> --- a/drivers/net/ethernet/google/gve/gve.h
> +++ b/drivers/net/ethernet/google/gve/gve.h
> @@ -794,6 +794,10 @@ struct gve_ptp {
> struct gve_priv *priv;
> };
>
> +struct gve_ring_err_stats {
> + u64 rx_alloc_fails;
> +};
> +
> struct gve_priv {
> struct net_device *dev;
> struct gve_tx_ring *tx; /* array of tx_cfg.num_queues */
> @@ -883,6 +887,9 @@ struct gve_priv {
> unsigned long service_task_flags;
> unsigned long state_flags;
>
> + struct gve_ring_err_stats base_ring_err_stats;
> + struct rtnl_link_stats64 base_net_stats;
> + struct u64_stats_sync base_statss; /* sync stats for 32bit archs */
Sashiko says:
Is u64_stats_init(&priv->base_statss) ever explicitly called during driver
probe?
netdev_alloc_priv() zero-allocates the memory, but on 32-bit architectures
or when lockdep is enabled, the underlying seqcount_t requires explicit
initialization to set up its lockdep class key.
Could failing to initialize it trigger a lockdep splat when the interface
goes down and u64_stats_update_begin() is invoked?
Yes, this is missing in this series. We will include this in v3.
> struct gve_stats_report *stats_report;
> u64 stats_report_len;
> dma_addr_t stats_report_bus; /* dma address for the stats report */
> diff --git a/drivers/net/ethernet/google/gve/gve_main.c b/drivers/net/ethernet/google/gve/gve_main.c
> index ef00d9ca1643..1fec8e1e4821 100644
> --- a/drivers/net/ethernet/google/gve/gve_main.c
> +++ b/drivers/net/ethernet/google/gve/gve_main.c
> @@ -106,9 +106,34 @@ static netdev_tx_t gve_start_xmit(struct sk_buff *skb, struct net_device *dev)
> return gve_tx_dqo(skb, dev);
> }
>
> -static void gve_get_stats(struct net_device *dev, struct rtnl_link_stats64 *s)
> +static void gve_add_base_stats(struct gve_priv *priv,
> + struct rtnl_link_stats64 *s)
> +{
> + struct rtnl_link_stats64 *base_stats = &priv->base_net_stats;
> + unsigned int start;
> + u64 rx_packets, rx_bytes, rx_dropped, tx_packets, tx_bytes, tx_dropped;
> +
> + do {
> + start = u64_stats_fetch_begin(&priv->base_statss);
> + rx_packets = base_stats->rx_packets;
> + rx_bytes = base_stats->rx_bytes;
> + rx_dropped = base_stats->rx_dropped;
> + tx_packets = base_stats->tx_packets;
> + tx_bytes = base_stats->tx_bytes;
> + tx_dropped = base_stats->tx_dropped;
> + } while (u64_stats_fetch_retry(&priv->base_statss, start));
> +
> + s->rx_packets += rx_packets;
> + s->rx_bytes += rx_bytes;
> + s->rx_dropped += rx_dropped;
> + s->tx_packets += tx_packets;
> + s->tx_bytes += tx_bytes;
> + s->tx_dropped += tx_dropped;
> +}
> +
> +static void gve_get_ring_stats(struct gve_priv *priv,
> + struct rtnl_link_stats64 *s)
> {
> - struct gve_priv *priv = netdev_priv(dev);
> unsigned int start;
> u64 packets, bytes;
> int num_tx_queues;
> @@ -143,6 +168,14 @@ static void gve_get_stats(struct net_device *dev, struct rtnl_link_stats64 *s)
> }
> }
>
> +static void gve_get_stats(struct net_device *dev, struct rtnl_link_stats64 *s)
> +{
> + struct gve_priv *priv = netdev_priv(dev);
> +
> + gve_get_ring_stats(priv, s);
> + gve_add_base_stats(priv, s);
> +}
> +
> static int gve_alloc_flow_rule_caches(struct gve_priv *priv)
> {
> struct gve_flow_rules_cache *flow_rules_cache = &priv->flow_rules_cache;
> @@ -1533,6 +1566,29 @@ static int gve_queues_stop(struct gve_priv *priv)
> return gve_reset_recovery(priv, false);
> }
>
> +static void gve_get_ring_err_stats(struct gve_priv *priv,
> + struct gve_ring_err_stats *err_stats)
> +{
> + int ring;
> +
> + if (!priv->rx)
> + return;
> +
> + for (ring = 0; ring < priv->rx_cfg.num_queues; ring++) {
> + struct gve_rx_ring *rx = &priv->rx[ring];
> + unsigned int start;
> + u64 rx_alloc_fails;
> +
> + do {
> + start = u64_stats_fetch_begin(&rx->statss);
> + rx_alloc_fails = rx->rx_skb_alloc_fail +
> + rx->rx_buf_alloc_fail;
> + } while (u64_stats_fetch_retry(&rx->statss, start));
> +
> + err_stats->rx_alloc_fails += rx_alloc_fails;
> + }
> +}
> +
> static int gve_close(struct net_device *dev)
> {
> struct gve_priv *priv = netdev_priv(dev);
> @@ -1542,6 +1598,12 @@ static int gve_close(struct net_device *dev)
> if (err)
> return err;
>
> + /* Save ring queue and err stats before closing the interface */
> + u64_stats_update_begin(&priv->base_statss);
Sashiko says:
Is there a risk of a deadlock here on 32-bit platforms?
ndo_get_stats64 (which calls gve_get_stats()) can execute concurrently in
softirq context. u64_stats_update_begin() on 32-bit platforms acquires a
seqcount lock but does not disable bottom halves.
If a softirq interrupts this update on the same CPU, could the lockless
reader spin infinitely waiting for the sequence counter to become even,
causing a hard deadlock?
Should this update be wrapped in local_bh_disable() and local_bh_enable()?
We will add this in v3.
> + gve_get_ring_stats(priv, &priv->base_net_stats);
> + gve_get_ring_err_stats(priv, &priv->base_ring_err_stats);
> + u64_stats_update_end(&priv->base_statss);
> +
> gve_queues_mem_remove(priv);
Sashiko says:
Will this sequence cause a transient double-counting of statistics?
The active ring stats are accumulated into base_net_stats, but the ring
pointers (priv->rx, priv->tx) remain active and are not cleared until
gve_queues_mem_remove() executes later.
If gve_get_stats() executes concurrently during this window, does it sum both
the active ring stats and the newly updated base_net_stats (which now
incorporates those same ring stats)?
One additional pre-existing issue related to this code path: does
gve_queues_mem_remove() leave a window for a use-after-free?
ndo_get_stats64 executes locklessly under RCU and checks if (priv->rx)
before reading ring stats. gve_queues_mem_remove() frees priv->rx using
kvfree(), but sets the pointer to NULL only after the memory is freed.
If ndo_get_stats64 checks priv->rx, gets preempted, and resumes after
gve_close() frees the array, could it dereference freed memory?
Both point to a pre-existing race between stats and gve_close(). We
will address this in v3.
> return 0;
> }
> @@ -2784,12 +2846,24 @@ static void gve_get_base_stats(struct net_device *dev,
> struct netdev_queue_stats_rx *rx,
> struct netdev_queue_stats_tx *tx)
> {
> - rx->packets = 0;
> - rx->bytes = 0;
> - rx->alloc_fail = 0;
> + const struct gve_ring_err_stats *base_err_stats;
> + const struct rtnl_link_stats64 *base_stats;
> + struct gve_priv *priv;
> + unsigned int start;
>
> - tx->packets = 0;
> - tx->bytes = 0;
> + priv = netdev_priv(dev);
> + base_stats = &priv->base_net_stats;
> + base_err_stats = &priv->base_ring_err_stats;
> +
> + do {
> + start = u64_stats_fetch_begin(&priv->base_statss);
> + rx->packets = base_stats->rx_packets;
> + rx->bytes = base_stats->rx_bytes;
> + rx->alloc_fail = base_err_stats->rx_alloc_fails;
> +
> + tx->packets = base_stats->tx_packets;
> + tx->bytes = base_stats->tx_bytes;
> + } while (u64_stats_fetch_retry(&priv->base_statss, start));
> }
>
> static const struct netdev_stat_ops gve_stat_ops = {
> --
> 2.54.0.545.g6539524ca2-goog
>
Regards,
Pin-yen
^ permalink raw reply
* Re: [syzbot] [wireless?] divide error in mac80211_hwsim_link_info_changed (3)
From: Arjan van de Ven @ 2026-04-27 20:29 UTC (permalink / raw)
To: linux-wireless
Cc: syzbot+ca7a2759caaa6cd4e3db, johannes, linux-kernel, netdev,
syzkaller-bugs
In-Reply-To: <69efb8dd.050a0220.18b4f.0006.GAE@google.com>
This email is created by automation to help kernel developers
deal with a large volume of AI generated bug reports by decoding
oopses into more actionable information.
Decoded Backtrace
1. mac80211_hwsim_link_info_changed -- crash site
(drivers/net/wireless/virtual/mac80211_hwsim.c:2734)
2701 static void mac80211_hwsim_link_info_changed(struct ieee80211_hw *hw,
2702 struct ieee80211_vif *vif,
2703 struct ieee80211_bss_conf *info,
2704 u64 changed)
2705 {
2706 struct hwsim_vif_priv *vp = (void *)vif->drv_priv;
2707 struct mac80211_hwsim_data *data = hw->priv;
2708 unsigned int link_id = info->link_id;
2709 struct mac80211_hwsim_link_data *link_data = &data->link_data[link_id];
...
2722 if (changed & BSS_CHANGED_BEACON_ENABLED) {
2723 wiphy_dbg(hw->wiphy, " BCN EN: %d (BI=%u)\n",
2724 info->enable_beacon, info->beacon_int);
2725 vp->bcn_en = info->enable_beacon;
2726 if (data->started &&
2727 !hrtimer_active(&link_data->beacon_timer) &&
2728 info->enable_beacon) {
2729 u64 tsf, until_tbtt;
2730 u32 bcn_int;
2731 link_data->beacon_int = info->beacon_int * 1024;
2732 tsf = mac80211_hwsim_get_tsf(hw, vif);
2733 bcn_int = link_data->beacon_int;
-> 2734 until_tbtt = bcn_int - do_div(tsf, bcn_int); // <- bcn_int=0; RSI=0 -> #DE
2735
2736 hrtimer_start(&link_data->beacon_timer,
2737 ns_to_ktime(until_tbtt * NSEC_PER_USEC),
2738 HRTIMER_MODE_REL_SOFT);
2739 } else if (!info->enable_beacon) {
...
2748 link_data->beacon_int = 0;
...
2750 }
2751 }
2. drv_link_info_changed -- (net/mac80211/driver-ops.c:497)
460 void drv_link_info_changed(struct ieee80211_local *local,
461 struct ieee80211_sub_if_data *sdata,
462 struct ieee80211_bss_conf *info,
463 int link_id, u64 changed)
464 {
...
493 trace_drv_link_info_changed(local, sdata, info, changed);
494 if (local->ops->link_info_changed)
495 local->ops->link_info_changed(&local->hw, &sdata->vif,
496 info, changed);
-> 497 else if (local->ops->bss_info_changed)
498 local->ops->bss_info_changed(&local->hw, &sdata->vif,
499 info, changed);
500 trace_drv_return_void(local);
501 }
3. ieee80211_offchannel_return -- (net/mac80211/offchannel.c:160)
133 void ieee80211_offchannel_return(struct ieee80211_local *local)
134 {
135 struct ieee80211_sub_if_data *sdata;
136
137 lockdep_assert_wiphy(local->hw.wiphy);
...
142 list_for_each_entry(sdata, &local->interfaces, list) {
...
157 if (test_and_clear_bit(SDATA_STATE_OFFCHANNEL_BEACON_STOPPED,
158 &sdata->state)) {
159 sdata->vif.bss_conf.enable_beacon = true;
-> 160 ieee80211_link_info_change_notify(
161 sdata, &sdata->deflink,
162 BSS_CHANGED_BEACON_ENABLED);
163 }
164 }
4. __ieee80211_scan_completed -- (net/mac80211/scan.c:519)
516 if (!hw_scan && was_scanning) {
517 ieee80211_configure_filter(local);
518 drv_sw_scan_complete(local, scan_sdata);
-> 519 ieee80211_offchannel_return(local);
520 }
5. cfg80211_wiphy_work -- (net/wireless/core.c:513)
(workqueue dispatch point for scan completion)
Tentative Analysis
When a software scan completes, __ieee80211_scan_completed calls
ieee80211_offchannel_return, which iterates active interfaces and
re-enables beaconing for any interface that had its beacon stopped
during the scan. It sets bss_conf.enable_beacon = true and issues
a BSS_CHANGED_BEACON_ENABLED notification, but does NOT update
bss_conf.beacon_int.
This notification reaches mac80211_hwsim_link_info_changed. The
condition at lines 2726-2728 (data->started, timer not active,
enable_beacon) is satisfied, so the function enters the beacon
timer setup block at line 2731. It sets link_data->beacon_int =
info->beacon_int * 1024. When info->beacon_int is zero -- which
syzbot achieved by creating an AP interface with beacon_int=0 --
this produces link_data->beacon_int = 0 and bcn_int = 0 (line
2733). The subsequent do_div(tsf, bcn_int) with a zero divisor
triggers the x86 #DE (divide error) exception (RSI = 0, as
confirmed by the register dump).
An identical do_div(tsf, bcn_int) expression in
mac80211_hwsim_config (mac80211_hwsim.c:2635) is protected by a
guard: "if (!data->started || !link_data->beacon_int)" -- this
guard is absent from the BSS_CHANGED_BEACON_ENABLED path.
Potential Solution
Add a zero check for bcn_int immediately after it is loaded from
link_data->beacon_int (line 2733), before the do_div call. A
beacon interval of zero is invalid; when encountered, return
without starting the timer:
bcn_int = link_data->beacon_int;
if (!bcn_int)
return;
until_tbtt = bcn_int - do_div(tsf, bcn_int);
This mirrors the guard already present in mac80211_hwsim_config
for the same expression. The Fixes tag for the commit would be:
Fixes: c51f878379b1 ("mac80211_hwsim: fix beacon timing")
More information
Oops-Analysis: http://oops.fenrus.org/reports/lkml/69efb8dd.050a0220.18b4f.0006.GAE_google.com/
Assisted-by: Copilot:claude-sonnet-4.6 linux-kernel-oops-x86.
^ permalink raw reply
* Re: [PATCH] dsa: b53: hide legacy gpiolib usage on non-mips
From: Linus Walleij @ 2026-04-27 20:25 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Florian Fainelli, Jonas Gorski, Andrew Lunn, Vladimir Oltean,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Bartosz Golaszewski, Arnd Bergmann,
Álvaro Fernández Rojas, Kyle Hendry, netdev,
linux-kernel, linux-gpio
In-Reply-To: <20260427143020.2800317-1-arnd@kernel.org>
On Mon, Apr 27, 2026 at 4:30 PM Arnd Bergmann <arnd@kernel.org> wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> The MIPS bcm53xx platform still uses the legacy gpiolib interfaces based
> on gpio numbers, but other platforms do not.
>
> Hide these interfaces inside of the existing #ifdef block and use the
> modern interfaces in the common parts of the driver to allow building
> it when the gpio_set_value() is left out of the kernel.
>
> Reviewed-by: Jonas Gorski <jonas.gorski@gmail.com>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Linus Walleij <linusw@kernel.org>
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH bpf-next v3 4/9] bpf: Refactor object relationship tracking and fix dynptr UAF bug
From: Amery Hung @ 2026-04-27 20:23 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Mykyta Yatsenko, bpf, netdev, alexei.starovoitov, andrii, daniel,
eddyz87, memxor, martin.lau, kernel-team
In-Reply-To: <CAEf4BzYuaA0xDm9WXv0vnorOgzmzsD-Rf6R3w5kus5Uo37SBFg@mail.gmail.com>
On Thu, Apr 23, 2026 at 5:04 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Thu, Apr 23, 2026 at 11:44 AM Amery Hung <ameryhung@gmail.com> wrote:
> >
> > On Thu, Apr 23, 2026 at 11:19 AM Mykyta Yatsenko
> > <mykyta.yatsenko5@gmail.com> wrote:
> > >
> > >
> > >
> > > On 4/21/26 11:10 PM, Amery Hung wrote:
> > > > Refactor object relationship tracking in the verifier and fix a dynptr
> > > > use-after-free bug where file/skb dynptrs are not invalidated when the
> > > > parent referenced object is freed.
> > > >
> > > > Add parent_id to bpf_reg_state to precisely track child-parent
> > > > relationships. A child object's parent_id points to the parent object's
> > > > id. This replaces the PTR_TO_MEM-specific dynptr_id and does not
> > > > increase the size of bpf_reg_state on 64-bit machines as there is
> > > > existing padding.
> > > >
> > > > When calling dynptr constructors (i.e., process_dynptr_func() with
> > > > MEM_UNINIT argument), track the parent's id if the parent is a
> > > > referenced object. This only applies to file dynptr and skb dynptr,
> > > > so only pass parent reg->id to kfunc constructors.
> > > >
> > > > For release_reference(), invalidating an object now also invalidates
> > > > all descendants by traversing the object tree. This is done using
> > > > stack-based DFS to avoid recursive call chains of release_reference() ->
> > > > unmark_stack_slots_dynptr() -> release_reference(). Referenced objects
> > > > encountered during tree traversal cannot be indirectly released. They
> > > > require an explicit helper/kfunc call to release the acquired resources.
> > > >
> > > > While the new design changes how object relationships are tracked in
> > > > the verifier, it does not change the verifier's behavior. Here is the
> > > > implication for dynptr, pointer casting, and owning/non-owning
> > > > references:
> > > >
> > > > Dynptr:
> > > >
> > > > When initializing a dynptr, referenced dynptrs acquire a reference for
> > > > ref_obj_id. If the dynptr has a referenced parent, parent_id tracks the
> > > > parent's id. When cloning, ref_obj_id and parent_id are copied from the
> > > > original. Releasing a referenced dynptr via release_reference(ref_obj_id)
> > > > invalidates all clones and derived slices. For non-referenced dynptrs,
> > > > only the specific dynptr and its children are invalidated.
> > > >
> > > > Pointer casting:
> > > >
> > > > Referenced socket pointers and their casted counterparts share the same
> > > > lifetime but have different nullness — they have different id but the
> > > > same ref_obj_id.
> > > >
> > > > Owning to non-owning reference conversion:
> > > >
> > > > After converting owning to non-owning by clearing ref_obj_id (e.g.,
> > > > object(id=1, ref_obj_id=1) -> object(id=1, ref_obj_id=0)), the
> > > > verifier only needs to release the reference state, so it calls
> > > > release_reference_nomark() instead of release_reference().
> > > >
> > > > Note that the error message "reference has not been acquired before" in
> > > > the helper and kfunc release paths is removed. This message was already
> > > > unreachable. The verifier only calls release_reference() after
> > > > confirming meta.ref_obj_id is valid, so the condition could never
> > > > trigger in practice (no selftest exercises it either). With the
> > > > refactor, release_reference() can now be called with non-acquired ids
> > > > and have different error conditions. Report directly in
> > > > release_reference() instead.
> > > >
> > > > Fixes: 870c28588afa ("bpf: net_sched: Add basic bpf qdisc kfuncs")
> > > > Signed-off-by: Amery Hung <ameryhung@gmail.com>
> > > > ---
> > > > include/linux/bpf_verifier.h | 22 ++-
> > > > kernel/bpf/log.c | 4 +-
> > > > kernel/bpf/states.c | 9 +-
> > > > kernel/bpf/verifier.c | 264 +++++++++++++++++------------------
> > > > 4 files changed, 152 insertions(+), 147 deletions(-)
> > > >
>
> [...]
>
> > > > @@ -673,82 +675,56 @@ static int mark_stack_slots_dynptr(struct bpf_verifier_env *env, struct bpf_reg_
> > > > mark_dynptr_stack_regs(env, &state->stack[spi].spilled_ptr,
> > > > &state->stack[spi - 1].spilled_ptr, type);
> > > >
> > > > - if (dynptr_type_refcounted(type)) {
> > > > - /* The id is used to track proper releasing */
> > > > - int id;
> > > > -
> > > > - if (clone_ref_obj_id)
> > > > - id = clone_ref_obj_id;
> > > > - else
> > > > - id = acquire_reference(env, insn_idx);
> > > > -
> > > > - if (id < 0)
> > > > - return id;
> > > > -
> > > > - state->stack[spi].spilled_ptr.ref_obj_id = id;
> > > > - state->stack[spi - 1].spilled_ptr.ref_obj_id = id;
>
> is id assigned somewhere else now? where?
>
I figured that I missed this question. The existing
mark_dynptr_stack_regs() above already assigned an id.
^ permalink raw reply
* Re: [PATCH net v2 1/4] gve: Add NULL pointer checks for per-queue statistics
From: Pin-yen Lin @ 2026-04-27 20:22 UTC (permalink / raw)
To: Harshitha Ramamurthy
Cc: netdev, joshwash, andrew+netdev, davem, edumazet, kuba, pabeni,
willemb, maolson, nktgrg, jfraker, ziweixiao, jacob.e.keller,
pkaligineedi, shailend, jordanrhee, stable, linux-kernel,
Debarghya Kundu
In-Reply-To: <20260425002450.163421-2-hramamurthy@google.com>
On Fri, Apr 24, 2026 at 5:24 PM Harshitha Ramamurthy
<hramamurthy@google.com> wrote:
>
> From: Debarghya Kundu <debarghyak@google.com>
>
> gve_get_[tx/rx]_queue_stats references the [tx/rx] null rings when the
> link is down. Add NULL pointer checks to guard this
>
> This was discovered by drivers/net/stats.py selftest.
>
> Cc: stable@vger.kernel.org
> Fixes: 2e5e0932dff5 ("gve: add support for basic queue stats")
> Signed-off-by: Debarghya Kundu <debarghyak@google.com>
> Signed-off-by: Pin-yen Lin <treapking@google.com>
> Signed-off-by: Harshitha Ramamurthy <hramamurthy@google.com>
> ---
> drivers/net/ethernet/google/gve/gve_main.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/google/gve/gve_main.c b/drivers/net/ethernet/google/gve/gve_main.c
> index 424d973c97f2..ef00d9ca1643 100644
> --- a/drivers/net/ethernet/google/gve/gve_main.c
> +++ b/drivers/net/ethernet/google/gve/gve_main.c
> @@ -2746,9 +2746,13 @@ static void gve_get_rx_queue_stats(struct net_device *dev, int idx,
> struct netdev_queue_stats_rx *rx_stats)
> {
> struct gve_priv *priv = netdev_priv(dev);
> - struct gve_rx_ring *rx = &priv->rx[idx];
> + struct gve_rx_ring *rx;
> unsigned int start;
>
> + if (!priv->rx)
> + return;
Sashiko says:
Does this NULL check safely prevent a use-after-free regression in lockless
paths?
Queue statistics can be accessed locklessly from the core network stack
(e.g., via ndo_get_stats64 under rcu_read_lock). If gve_close() is called
concurrently, gve_queues_mem_free() frees priv->rx and priv->tx using
kvfree() without an RCU grace period (e.g., synchronize_net()).
A concurrent reader could pass this NULL check, stall briefly, and then
dereference the freed array memory.
Sashiko also made a similar comment about the tx stats.
While this race between stats and gve_close() is a pre-existing issue,
we will address it in a separate patch in v3.
> + rx = &priv->rx[idx];
> +
> do {
> start = u64_stats_fetch_begin(&rx->statss);
> rx_stats->packets = rx->rpackets;
> @@ -2762,9 +2766,13 @@ static void gve_get_tx_queue_stats(struct net_device *dev, int idx,
> struct netdev_queue_stats_tx *tx_stats)
> {
> struct gve_priv *priv = netdev_priv(dev);
> - struct gve_tx_ring *tx = &priv->tx[idx];
> + struct gve_tx_ring *tx;
> unsigned int start;
>
> + if (!priv->tx)
> + return;
> + tx = &priv->tx[idx];
> +
> do {
> start = u64_stats_fetch_begin(&tx->statss);
> tx_stats->packets = tx->pkt_done;
> --
> 2.54.0.545.g6539524ca2-goog
>
Regards,
Pin-yen
^ permalink raw reply
* Re: [PATCH net 1/1] ovpn: fix race between deleting interface and adding new peer
From: Jakub Kicinski @ 2026-04-27 20:22 UTC (permalink / raw)
To: Sabrina Dubroca
Cc: Antonio Quartulli, netdev, Ralf Lici, Paolo Abeni, Andrew Lunn,
David S. Miller, Eric Dumazet, Hyunwoo Kim
In-Reply-To: <aeqcvgufY0F50rkU@krikkit>
On Fri, 24 Apr 2026 00:27:10 +0200 Sabrina Dubroca wrote:
> 2026-04-23, 10:36:28 -0700, Jakub Kicinski wrote:
> > On Thu, 23 Apr 2026 18:37:49 +0200 Sabrina Dubroca wrote:
> > > We could also wrap all of ovpn_peer_add (including the new reg_state
> > > check) in netdev_lock, since we can't move to NETREG_UNREGISTERING
> > > until netdev_lock is released (then I think we wouldn't need the
> > > READ_ONCE(reg_state)). But I don't know if that's an acceptable use of
> > > netdev_lock.
> >
> > Yup, fwiw it's a perfectly legitimate use case.
>
> Ok. There's still a worrying bit in nesting lock_sock under
> netdev_lock (ovpn_peer_add -> netdev_lock -> ovpn_peer_add_p2p ->
> unlock_ovpn -> ovpn_socket_release -> lock_sock).
No opinion on socket lock, TBH. It does indeed increase the potential
for hard to trace deadlocks. But I'd think the ordering is intuitive -
netdev lock "feels" heavier than socket lock.
^ permalink raw reply
* Re: [PATCH bpf-next v3 4/9] bpf: Refactor object relationship tracking and fix dynptr UAF bug
From: Amery Hung @ 2026-04-27 20:21 UTC (permalink / raw)
To: Eduard Zingerman
Cc: bpf, netdev, alexei.starovoitov, andrii, daniel, memxor,
martin.lau, mykyta.yatsenko5, kernel-team
In-Reply-To: <02dbf08511c72ea0c1a5caa33ed400f4a3e3c3c9.camel@gmail.com>
On Fri, Apr 24, 2026 at 3:48 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> On Tue, 2026-04-21 at 15:10 -0700, Amery Hung wrote:
>
> Tbh, I find current state of affairs with id/ref_obj_id/parent_id hard
> to follow. The release_reference() is an improvement, but the means by
> which the fields are propagated to bpf_reg_state objects are convoluted.
> I wonder if having a separate "object table" in bpf_verifier_state and
> having bpf_reg_state->id refer to objects within this table would make
> things more straight forward.
I think this is a good idea in the long term. First, we need to make
id a stable and unique object identifier (i.e., always assign id to
objects; different objects have different ids). Then, we can create
the object table indexed by id and each entry contains the ref_obj_id
and parent_id moved from bpf_reg_state. Then, there will be helpers
managing the lifetime, relationship (e.g., bpf_obj_create,
bpf_obj_release, bpf_obj_clone) with clear semantics to centralize how
id,parent_id,ref_obj_id are manipulated.
If this makes sense, I can send this as a follow up patchset.
>
> A few nits below.
>
> [...]
>
> > diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
>
> [...]
>
> > @@ -1323,6 +1335,7 @@ struct bpf_dynptr_desc {
> > enum bpf_dynptr_type type;
> > u32 id;
> > u32 ref_obj_id;
> > + u32 parent_id;
> > };
>
> Nit: would be nice to have a comment describing when the above fields
> are populated.
Ack
>
> >
> > struct bpf_kfunc_call_arg_meta {
> > @@ -1334,6 +1347,7 @@ struct bpf_kfunc_call_arg_meta {
> > const char *func_name;
> > /* Out parameters */
> > u32 ref_obj_id;
> > + u32 id;
>
> Nit: would be nice to have a comment here too.
Ack
>
> [...]
>
> > diff --git a/kernel/bpf/states.c b/kernel/bpf/states.c
> > index 8478d2c6ed5b..72bd3bcda5fb 100644
> > --- a/kernel/bpf/states.c
> > +++ b/kernel/bpf/states.c
> > @@ -494,7 +494,8 @@ static bool regs_exact(const struct bpf_reg_state *rold,
> > {
> > return memcmp(rold, rcur, offsetof(struct bpf_reg_state, id)) == 0 &&
> > check_ids(rold->id, rcur->id, idmap) &&
> > - check_ids(rold->ref_obj_id, rcur->ref_obj_id, idmap);
> > + check_ids(rold->ref_obj_id, rcur->ref_obj_id, idmap) &&
> > + check_ids(rold->parent_id, rcur->parent_id, idmap);
>
> Nit: these check_ids() become repetitive, maybe add a utility function
> checking id/ref_obj_id/parent_id?
Will add a utility function.
>
> > }
> >
> > enum exact_level {
> > @@ -619,7 +620,8 @@ static bool regsafe(struct bpf_verifier_env *env, struct bpf_reg_state *rold,
> > range_within(rold, rcur) &&
> > tnum_in(rold->var_off, rcur->var_off) &&
> > check_ids(rold->id, rcur->id, idmap) &&
> > - check_ids(rold->ref_obj_id, rcur->ref_obj_id, idmap);
> > + check_ids(rold->ref_obj_id, rcur->ref_obj_id, idmap) &&
> > + check_ids(rold->parent_id, rcur->parent_id, idmap);
> > case PTR_TO_PACKET_META:
> > case PTR_TO_PACKET:
> > /* We must have at least as much range as the old ptr
> > @@ -799,7 +801,8 @@ static bool stacksafe(struct bpf_verifier_env *env, struct bpf_func_state *old,
> > cur_reg = &cur->stack[spi].spilled_ptr;
> > if (old_reg->dynptr.type != cur_reg->dynptr.type ||
> > old_reg->dynptr.first_slot != cur_reg->dynptr.first_slot ||
> > - !check_ids(old_reg->ref_obj_id, cur_reg->ref_obj_id, idmap))
> > + !check_ids(old_reg->ref_obj_id, cur_reg->ref_obj_id, idmap) ||
> > + !check_ids(old_reg->parent_id, cur_reg->parent_id, idmap))
>
> Not something changed by the current patch, but still a question:
> this path ignores old_reg->id, is it a bug?
Yes. I will also check id.
>
> > return false;
> > break;
> > case STACK_ITER:
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index 0313b7d5f6c9..908a3af0e7c4 100644
>
> [...]
>
> > @@ -241,6 +241,7 @@ struct bpf_call_arg_meta {
> > int mem_size;
> > u64 msize_max_value;
> > int ref_obj_id;
> > + u32 id;
>
> Nit: please add a comment here as well.
Ack
>
> [...]
>
> > @@ -635,11 +636,12 @@ static int destroy_if_dynptr_stack_slot(struct bpf_verifier_env *env,
> > struct bpf_func_state *state, int spi);
> >
> > static int mark_stack_slots_dynptr(struct bpf_verifier_env *env, struct bpf_reg_state *reg,
> > - enum bpf_arg_type arg_type, int insn_idx, int clone_ref_obj_id)
> > + enum bpf_arg_type arg_type, int insn_idx, int parent_id,
> > + struct bpf_dynptr_desc *dynptr)
>
> Having both parent_id and dynptr->parent_id as parameters of this
> function is very confusing, but I don't have a suggestion on how to
> better deal with it.
Does tweaking the argument name and reorder the if-else block make it
more obvious how these two arguments are used?
if (clone_dynptr->type != BPF_DYNPTR_TYPE_INVALID) { /* bpf_dynptr_clone() */
ref_obj_id = clone_dynptr->ref_obj_id;
parent_id = clone_dynptr->parent_id;
} else { /* dynptr constructors */
if (dynptr_type_referenced(type)) {
ref_obj_id = acquire_reference(env, insn_idx);
if (ref_obj_id < 0)
return ref_obj_id;
}
}
>
> [...]
>
> > @@ -8489,6 +8444,7 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
> > return -EACCES;
> > }
> > meta->ref_obj_id = reg->ref_obj_id;
> > + meta->id = reg->id;
>
> Could you please leave a comment here explaining when e.g.
> meta->ref_obj_id and meta->id would have same or different ids here?
> Maybe a few lines of BPF C code.
Ack
>
> > }
> >
> > switch (base_type(arg_type)) {
> > @@ -9111,26 +9067,75 @@ static int release_reference_nomark(struct bpf_verifier_state *state, int ref_ob
>
> [...]
>
> > +/* Release id and objects referencing the id iteratively in a DFS manner */
> > +static int release_reference(struct bpf_verifier_env *env, int id)
> > +{
> > + u32 mask = (1 << STACK_SPILL) | (1 << STACK_DYNPTR);
> > struct bpf_verifier_state *vstate = env->cur_state;
> > + struct bpf_idmap *idstack = &env->idmap_scratch;
> > + struct bpf_stack_state *stack;
> > struct bpf_func_state *state;
> > struct bpf_reg_state *reg;
> > - int err;
> > + int root_id = id, err;
> >
> > - err = release_reference_nomark(vstate, ref_obj_id);
> > - if (err)
> > - return err;
> > + idstack->cnt = 0;
> > + idstack_push(idstack, id);
> >
> > - bpf_for_each_reg_in_vstate(vstate, state, reg, ({
> > - if (reg->ref_obj_id == ref_obj_id)
> > - mark_reg_invalid(env, reg);
> > - }));
> > + if (find_reference_state(vstate, id))
> > + WARN_ON_ONCE(release_reference_nomark(vstate, id));
> > +
> > + while ((id = idstack_pop(idstack))) {
> > + bpf_for_each_reg_in_vstate_mask(vstate, state, reg, stack, mask, ({
> > + if (reg->id != id && reg->parent_id != id && reg->ref_obj_id != id)
> > + continue;
> > +
> > + if (reg->ref_obj_id && id != root_id) {
> > + struct bpf_reference_state *ref_state;
> > +
> > + ref_state = find_reference_state(env->cur_state, reg->ref_obj_id);
> > + verbose(env, "Unreleased reference id=%d alloc_insn=%d when releasing id=%d\n",
> > + ref_state->id, ref_state->insn_idx, root_id);
> > + return -EINVAL;
> > + }
> > +
> > + if (reg->id != id) {
> > + err = idstack_push(idstack, reg->id);
> > + if (err)
> > + return err;
> > + }
> > +
> > + if (!stack || stack->slot_type[BPF_REG_SIZE - 1] == STACK_SPILL)
> > + mark_reg_invalid(env, reg);
> > + else if (stack->slot_type[BPF_REG_SIZE - 1] == STACK_DYNPTR)
> > + invalidate_dynptr(env, state, stack);
>
> invalidate_dynptr() rewrites to stack slots, can it be the case that
> this body of bpf_for_each_reg_in_vstate_mask() is computed for first
> and second dynptr stack slots, hence triggering invalidate_dynptr() to
> rewrite three slots instead of two?
invalidate_dynptr() will mark the two stack slots as STACK_INVALID so
I didn't bother to check whether it is the first slot or not. Am I
missing anything?
>
> > + }));
> > + }
> >
> > return 0;
> > }
>
> [...]
>
> > @@ -12009,6 +12009,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
> > return -EFAULT;
> > }
> > meta->ref_obj_id = reg->ref_obj_id;
> > + meta->id = reg->id;
>
> And here a comment describing when this happens would be helpful.
>
Ack
> [...]
>
> > @@ -12171,15 +12171,10 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
> > }
> >
> > dynptr_arg_type |= (unsigned int)get_dynptr_type_flag(parent_type);
> > - clone_ref_obj_id = meta->dynptr.ref_obj_id;
> > - if (dynptr_type_refcounted(parent_type) && !clone_ref_obj_id) {
> > - verifier_bug(env, "missing ref obj id for parent of clone");
> > - return -EFAULT;
> > - }
> > }
> >
> > - ret = process_dynptr_func(env, regno, insn_idx, dynptr_arg_type, clone_ref_obj_id,
> > - &meta->dynptr);
> > + ret = process_dynptr_func(env, regno, insn_idx, dynptr_arg_type,
> > + meta->ref_obj_id ? meta->id : 0, &meta->dynptr);
>
> And an example here as well.
Ack
>
> > if (ret < 0)
> > return ret;
> > break;
>
> [...]
^ permalink raw reply
* Re: [PATCH net-next v6 0/2] net: mana: add ethtool private flag for full-page RX buffers
From: Jakub Kicinski @ 2026-04-27 20:17 UTC (permalink / raw)
To: Dipayaan Roy
Cc: David Wei, kys, haiyangz, wei.liu, decui, andrew+netdev, davem,
edumazet, pabeni, leon, longli, kotaranov, horms, shradhagupta,
ssengar, ernis, shirazsaleem, linux-hyperv, netdev, linux-kernel,
linux-rdma, stephen, jacob.e.keller, leitao, kees, john.fastabend,
hawk, bpf, daniel, ast, sdf, dipayanroy
In-Reply-To: <aex119OtL8CEGXkb@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
On Sat, 25 Apr 2026 01:05:43 -0700 Dipayaan Roy wrote:
> Hi Jakub,
> with this new data from David, is it convincing enough for a mana driver
> specific private flag, which can be set from user space by a udev rule
> by detecting the underlying platform? If not then I will send the next
> version with the other rxbuflen approach.
I think so, thank you both for the testing.
Please look out for the net-next opening up and repost the patches.
(The reopening is delayed, it was supposed to happen already but I
can't get a clean run out of our CI, sigh)
^ permalink raw reply
* [PATCH net-next v5 0/3] selftests: drv-net: convert so_txtime to drv-net
From: Willem de Bruijn @ 2026-04-27 20:14 UTC (permalink / raw)
To: netdev
Cc: davem, kuba, edumazet, pabeni, horms, linux-kselftest, shuah,
Willem de Bruijn
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.
Two preparatory patches
1. support negative tests, where tests are expected to fail
2. add a tc helper
See individual patches for details.
Willem de Bruijn (3):
selftests: net: py: support cmd verifying expected failure
selftests: net: py: add tc utility
selftests: drv-net: convert so_txtime to drv-net
.../testing/selftests/drivers/net/.gitignore | 1 +
tools/testing/selftests/drivers/net/Makefile | 2 +
tools/testing/selftests/drivers/net/config | 2 +
.../selftests/drivers/net/lib/py/__init__.py | 5 +-
.../selftests/{ => drivers}/net/so_txtime.c | 25 +++-
.../selftests/drivers/net/so_txtime.py | 95 +++++++++++++++
tools/testing/selftests/net/.gitignore | 1 -
tools/testing/selftests/net/Makefile | 2 -
.../testing/selftests/net/lib/py/__init__.py | 4 +-
tools/testing/selftests/net/lib/py/utils.py | 18 ++-
tools/testing/selftests/net/so_txtime.sh | 110 ------------------
11 files changed, 141 insertions(+), 124 deletions(-)
rename tools/testing/selftests/{ => drivers}/net/so_txtime.c (96%)
create mode 100755 tools/testing/selftests/drivers/net/so_txtime.py
delete mode 100755 tools/testing/selftests/net/so_txtime.sh
--
2.54.0.545.g6539524ca2-goog
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox