bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf v2 0/2] bpf: add _impl suffix for kfuncs with implicit args
@ 2025-11-04 15:29 Mykyta Yatsenko
  2025-11-04 15:29 ` [PATCH bpf v2 1/2] bpf:add _impl suffix for bpf_task_work_schedule* kfuncs Mykyta Yatsenko
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Mykyta Yatsenko @ 2025-11-04 15:29 UTC (permalink / raw)
  To: bpf, ast, andrii, daniel, kafai, kernel-team; +Cc: Mykyta Yatsenko

We’re introducing support for implicit kfunc arguments and need to
rename new kfuncs to comply with the naming convention.
This new feature, will for each kfunc of the form:

`bpf_foo_impl(args..., aux__prog)`

generate a public BTF type:

`bpf_foo(args...)`

and the verifier will resolve calls to bpf_foo() to bpf_foo_impl(),
supplying a valid struct bpf_prog_aux via aux__prog.

Three kfuncs added in 6.18 don’t follow this *_impl convention and
therefore won’t participate in the new mechanism:
 * bpf_task_work_schedule_resume()
 * bpf_task_work_schedule_signal()
 * bpf_stream_vprintk()

Rename them to align with the implicit-arg flow:
bpf_task_work_schedule_resume() -> bpf_task_work_schedule_resume_impl()
bpf_task_work_schedule_signal() -> bpf_task_work_schedule_signal_impl()
bpf_stream_vprintk() -> bpf_stream_vprintk_impl()

The implicit-arg mechanism is not in tree yet, so callers must switch to
the *_impl names for now. Once the new mechanism lands, the plain
names (without _impl) will be reintroduced as BTF-visible entry points
and will resolve to the _impl versions automatically.

Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
---
Changes in v1:
- Split commit into 2
- Rebase on the correct branch
- Link to v1: https://lore.kernel.org/all/20251103232319.122965-1-mykyta.yatsenko5@gmail.com/

---
Mykyta Yatsenko (2):
      bpf:add _impl suffix for bpf_task_work_schedule* kfuncs
      bpf:add _impl suffix for bpf_stream_vprintk() kfunc

 kernel/bpf/helpers.c                               | 26 +++++++++++---------
 kernel/bpf/stream.c                                |  3 ++-
 kernel/bpf/verifier.c                              | 12 +++++-----
 tools/bpf/bpftool/Documentation/bpftool-prog.rst   |  2 +-
 tools/lib/bpf/bpf_helpers.h                        | 28 +++++++++++-----------
 tools/testing/selftests/bpf/progs/stream_fail.c    |  6 ++---
 tools/testing/selftests/bpf/progs/task_work.c      |  6 ++---
 tools/testing/selftests/bpf/progs/task_work_fail.c |  8 +++----
 .../testing/selftests/bpf/progs/task_work_stress.c |  4 ++--
 9 files changed, 50 insertions(+), 45 deletions(-)
---
base-commit: 6146a0f1dfae5d37442a9ddcba012add260bceb0
change-id: 20251104-implv2-d6c4be255026

Best regards,
-- 
Mykyta Yatsenko <yatsenko@meta.com>


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH bpf v2 1/2] bpf:add _impl suffix for bpf_task_work_schedule* kfuncs
  2025-11-04 15:29 [PATCH bpf v2 0/2] bpf: add _impl suffix for kfuncs with implicit args Mykyta Yatsenko
@ 2025-11-04 15:29 ` Mykyta Yatsenko
  2025-11-04 19:21   ` Andrii Nakryiko
  2025-11-04 15:29 ` [PATCH bpf v2 2/2] bpf:add _impl suffix for bpf_stream_vprintk() kfunc Mykyta Yatsenko
  2025-11-04 17:23 ` [PATCH bpf v2 0/2] bpf: add _impl suffix for kfuncs with implicit args Ihor Solodrai
  2 siblings, 1 reply; 9+ messages in thread
From: Mykyta Yatsenko @ 2025-11-04 15:29 UTC (permalink / raw)
  To: bpf, ast, andrii, daniel, kafai, kernel-team; +Cc: Mykyta Yatsenko

From: Mykyta Yatsenko <yatsenko@meta.com>

Rename:
bpf_task_work_schedule_resume()->bpf_task_work_schedule_resume_impl()
bpf_task_work_schedule_signal()->bpf_task_work_schedule_signal_impl()

This aligns task work scheduling kfuncs with the naming scheme required
by the implicit-argument feature.

Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
---
 kernel/bpf/helpers.c                               | 24 +++++++++++++---------
 kernel/bpf/verifier.c                              | 12 +++++------
 tools/testing/selftests/bpf/progs/task_work.c      |  6 +++---
 tools/testing/selftests/bpf/progs/task_work_fail.c |  8 ++++----
 .../testing/selftests/bpf/progs/task_work_stress.c |  4 ++--
 5 files changed, 29 insertions(+), 25 deletions(-)

diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index eb25e70e0bdc0332edd21cde66d9aaadb2090312..33173b027ccf8893ce18aad474b88f8544f7b344 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -4169,7 +4169,8 @@ static int bpf_task_work_schedule(struct task_struct *task, struct bpf_task_work
 }
 
 /**
- * bpf_task_work_schedule_signal - Schedule BPF callback using task_work_add with TWA_SIGNAL mode
+ * bpf_task_work_schedule_signal_impl - Schedule BPF callback using task_work_add with TWA_SIGNAL
+ * mode
  * @task: Task struct for which callback should be scheduled
  * @tw: Pointer to struct bpf_task_work in BPF map value for internal bookkeeping
  * @map__map: bpf_map that embeds struct bpf_task_work in the values
@@ -4178,15 +4179,17 @@ static int bpf_task_work_schedule(struct task_struct *task, struct bpf_task_work
  *
  * Return: 0 if task work has been scheduled successfully, negative error code otherwise
  */
-__bpf_kfunc int bpf_task_work_schedule_signal(struct task_struct *task, struct bpf_task_work *tw,
-					      void *map__map, bpf_task_work_callback_t callback,
-					      void *aux__prog)
+__bpf_kfunc int bpf_task_work_schedule_signal_impl(struct task_struct *task,
+						   struct bpf_task_work *tw, void *map__map,
+						   bpf_task_work_callback_t callback,
+						   void *aux__prog)
 {
 	return bpf_task_work_schedule(task, tw, map__map, callback, aux__prog, TWA_SIGNAL);
 }
 
 /**
- * bpf_task_work_schedule_resume - Schedule BPF callback using task_work_add with TWA_RESUME mode
+ * bpf_task_work_schedule_resume_impl - Schedule BPF callback using task_work_add with TWA_RESUME
+ * mode
  * @task: Task struct for which callback should be scheduled
  * @tw: Pointer to struct bpf_task_work in BPF map value for internal bookkeeping
  * @map__map: bpf_map that embeds struct bpf_task_work in the values
@@ -4195,9 +4198,10 @@ __bpf_kfunc int bpf_task_work_schedule_signal(struct task_struct *task, struct b
  *
  * Return: 0 if task work has been scheduled successfully, negative error code otherwise
  */
-__bpf_kfunc int bpf_task_work_schedule_resume(struct task_struct *task, struct bpf_task_work *tw,
-					      void *map__map, bpf_task_work_callback_t callback,
-					      void *aux__prog)
+__bpf_kfunc int bpf_task_work_schedule_resume_impl(struct task_struct *task,
+						   struct bpf_task_work *tw, void *map__map,
+						   bpf_task_work_callback_t callback,
+						   void *aux__prog)
 {
 	return bpf_task_work_schedule(task, tw, map__map, callback, aux__prog, TWA_RESUME);
 }
@@ -4377,8 +4381,8 @@ BTF_ID_FLAGS(func, bpf_strnstr);
 BTF_ID_FLAGS(func, bpf_cgroup_read_xattr, KF_RCU)
 #endif
 BTF_ID_FLAGS(func, bpf_stream_vprintk, KF_TRUSTED_ARGS)
-BTF_ID_FLAGS(func, bpf_task_work_schedule_signal, KF_TRUSTED_ARGS)
-BTF_ID_FLAGS(func, bpf_task_work_schedule_resume, KF_TRUSTED_ARGS)
+BTF_ID_FLAGS(func, bpf_task_work_schedule_signal_impl, KF_TRUSTED_ARGS)
+BTF_ID_FLAGS(func, bpf_task_work_schedule_resume_impl, KF_TRUSTED_ARGS)
 BTF_KFUNCS_END(common_btf_ids)
 
 static const struct btf_kfunc_id_set common_kfunc_set = {
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index ff40e5e65c435862e9ecd3ed37139cc177a13ea1..8314518c8d93ae16235e6f2fe6c5c28c45cb81d2 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -12259,8 +12259,8 @@ enum special_kfunc_type {
 	KF_bpf_res_spin_lock_irqsave,
 	KF_bpf_res_spin_unlock_irqrestore,
 	KF___bpf_trap,
-	KF_bpf_task_work_schedule_signal,
-	KF_bpf_task_work_schedule_resume,
+	KF_bpf_task_work_schedule_signal_impl,
+	KF_bpf_task_work_schedule_resume_impl,
 };
 
 BTF_ID_LIST(special_kfunc_list)
@@ -12331,13 +12331,13 @@ BTF_ID(func, bpf_res_spin_unlock)
 BTF_ID(func, bpf_res_spin_lock_irqsave)
 BTF_ID(func, bpf_res_spin_unlock_irqrestore)
 BTF_ID(func, __bpf_trap)
-BTF_ID(func, bpf_task_work_schedule_signal)
-BTF_ID(func, bpf_task_work_schedule_resume)
+BTF_ID(func, bpf_task_work_schedule_signal_impl)
+BTF_ID(func, bpf_task_work_schedule_resume_impl)
 
 static bool is_task_work_add_kfunc(u32 func_id)
 {
-	return func_id == special_kfunc_list[KF_bpf_task_work_schedule_signal] ||
-	       func_id == special_kfunc_list[KF_bpf_task_work_schedule_resume];
+	return func_id == special_kfunc_list[KF_bpf_task_work_schedule_signal_impl] ||
+	       func_id == special_kfunc_list[KF_bpf_task_work_schedule_resume_impl];
 }
 
 static bool is_kfunc_ret_null(struct bpf_kfunc_call_arg_meta *meta)
diff --git a/tools/testing/selftests/bpf/progs/task_work.c b/tools/testing/selftests/bpf/progs/task_work.c
index 23217f06a3ece641089c1ea6a470eccc82e0d1fa..663a80990f8f8759f470d2f874d02485b072a5e1 100644
--- a/tools/testing/selftests/bpf/progs/task_work.c
+++ b/tools/testing/selftests/bpf/progs/task_work.c
@@ -66,7 +66,7 @@ int oncpu_hash_map(struct pt_regs *args)
 	if (!work)
 		return 0;
 
-	bpf_task_work_schedule_resume(task, &work->tw, &hmap, process_work, NULL);
+	bpf_task_work_schedule_resume_impl(task, &work->tw, &hmap, process_work, NULL);
 	return 0;
 }
 
@@ -80,7 +80,7 @@ int oncpu_array_map(struct pt_regs *args)
 	work = bpf_map_lookup_elem(&arrmap, &key);
 	if (!work)
 		return 0;
-	bpf_task_work_schedule_signal(task, &work->tw, &arrmap, process_work, NULL);
+	bpf_task_work_schedule_signal_impl(task, &work->tw, &arrmap, process_work, NULL);
 	return 0;
 }
 
@@ -102,6 +102,6 @@ int oncpu_lru_map(struct pt_regs *args)
 	work = bpf_map_lookup_elem(&lrumap, &key);
 	if (!work || work->data[0])
 		return 0;
-	bpf_task_work_schedule_resume(task, &work->tw, &lrumap, process_work, NULL);
+	bpf_task_work_schedule_resume_impl(task, &work->tw, &lrumap, process_work, NULL);
 	return 0;
 }
diff --git a/tools/testing/selftests/bpf/progs/task_work_fail.c b/tools/testing/selftests/bpf/progs/task_work_fail.c
index 77fe8f28facdb60cd69e77b72ea24db8f97004f7..1270953fd0926f83f9ad7112d78e1b86bd1e802c 100644
--- a/tools/testing/selftests/bpf/progs/task_work_fail.c
+++ b/tools/testing/selftests/bpf/progs/task_work_fail.c
@@ -53,7 +53,7 @@ int mismatch_map(struct pt_regs *args)
 	work = bpf_map_lookup_elem(&arrmap, &key);
 	if (!work)
 		return 0;
-	bpf_task_work_schedule_resume(task, &work->tw, &hmap, process_work, NULL);
+	bpf_task_work_schedule_resume_impl(task, &work->tw, &hmap, process_work, NULL);
 	return 0;
 }
 
@@ -65,7 +65,7 @@ int no_map_task_work(struct pt_regs *args)
 	struct bpf_task_work tw;
 
 	task = bpf_get_current_task_btf();
-	bpf_task_work_schedule_resume(task, &tw, &hmap, process_work, NULL);
+	bpf_task_work_schedule_resume_impl(task, &tw, &hmap, process_work, NULL);
 	return 0;
 }
 
@@ -76,7 +76,7 @@ int task_work_null(struct pt_regs *args)
 	struct task_struct *task;
 
 	task = bpf_get_current_task_btf();
-	bpf_task_work_schedule_resume(task, NULL, &hmap, process_work, NULL);
+	bpf_task_work_schedule_resume_impl(task, NULL, &hmap, process_work, NULL);
 	return 0;
 }
 
@@ -91,6 +91,6 @@ int map_null(struct pt_regs *args)
 	work = bpf_map_lookup_elem(&arrmap, &key);
 	if (!work)
 		return 0;
-	bpf_task_work_schedule_resume(task, &work->tw, NULL, process_work, NULL);
+	bpf_task_work_schedule_resume_impl(task, &work->tw, NULL, process_work, NULL);
 	return 0;
 }
diff --git a/tools/testing/selftests/bpf/progs/task_work_stress.c b/tools/testing/selftests/bpf/progs/task_work_stress.c
index 90fca06fff56ca9a03fee028ca1b69db145a227b..55e555f7f41be694f9b3d98f324b402f9355f77d 100644
--- a/tools/testing/selftests/bpf/progs/task_work_stress.c
+++ b/tools/testing/selftests/bpf/progs/task_work_stress.c
@@ -51,8 +51,8 @@ int schedule_task_work(void *ctx)
 		if (!work)
 			return 0;
 	}
-	err = bpf_task_work_schedule_signal(bpf_get_current_task_btf(), &work->tw, &hmap,
-					    process_work, NULL);
+	err = bpf_task_work_schedule_signal_impl(bpf_get_current_task_btf(), &work->tw, &hmap,
+						 process_work, NULL);
 	if (err)
 		__sync_fetch_and_add(&schedule_error, 1);
 	else

-- 
2.51.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH bpf v2 2/2] bpf:add _impl suffix for bpf_stream_vprintk() kfunc
  2025-11-04 15:29 [PATCH bpf v2 0/2] bpf: add _impl suffix for kfuncs with implicit args Mykyta Yatsenko
  2025-11-04 15:29 ` [PATCH bpf v2 1/2] bpf:add _impl suffix for bpf_task_work_schedule* kfuncs Mykyta Yatsenko
@ 2025-11-04 15:29 ` Mykyta Yatsenko
  2025-11-04 19:25   ` Andrii Nakryiko
  2025-11-04 17:23 ` [PATCH bpf v2 0/2] bpf: add _impl suffix for kfuncs with implicit args Ihor Solodrai
  2 siblings, 1 reply; 9+ messages in thread
From: Mykyta Yatsenko @ 2025-11-04 15:29 UTC (permalink / raw)
  To: bpf, ast, andrii, daniel, kafai, kernel-team; +Cc: Mykyta Yatsenko

From: Mykyta Yatsenko <yatsenko@meta.com>

Rename bpf_stream_vprintk() to bpf_stream_vprintk_impl().

This aligns this recently added kfunc with the naming scheme required
by the implicit-argument feature.
In future BTF type for bpf_stream_vprintk() will be generated and
aux__prog argument filled by the valid struct implicitly.

Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
---
 kernel/bpf/helpers.c                             |  2 +-
 kernel/bpf/stream.c                              |  3 ++-
 tools/bpf/bpftool/Documentation/bpftool-prog.rst |  2 +-
 tools/lib/bpf/bpf_helpers.h                      | 28 ++++++++++++------------
 tools/testing/selftests/bpf/progs/stream_fail.c  |  6 ++---
 5 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 33173b027ccf8893ce18aad474b88f8544f7b344..e4007fea49091c01c1d23af55a25f5567417e978 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -4380,7 +4380,7 @@ BTF_ID_FLAGS(func, bpf_strnstr);
 #if defined(CONFIG_BPF_LSM) && defined(CONFIG_CGROUPS)
 BTF_ID_FLAGS(func, bpf_cgroup_read_xattr, KF_RCU)
 #endif
-BTF_ID_FLAGS(func, bpf_stream_vprintk, KF_TRUSTED_ARGS)
+BTF_ID_FLAGS(func, bpf_stream_vprintk_impl, KF_TRUSTED_ARGS)
 BTF_ID_FLAGS(func, bpf_task_work_schedule_signal_impl, KF_TRUSTED_ARGS)
 BTF_ID_FLAGS(func, bpf_task_work_schedule_resume_impl, KF_TRUSTED_ARGS)
 BTF_KFUNCS_END(common_btf_ids)
diff --git a/kernel/bpf/stream.c b/kernel/bpf/stream.c
index eb6c5a21c2efee96c41f4c5e43d54062694a4859..ff16c631951bb685e8ecf1707206dad603121a65 100644
--- a/kernel/bpf/stream.c
+++ b/kernel/bpf/stream.c
@@ -355,7 +355,8 @@ __bpf_kfunc_start_defs();
  * Avoid using enum bpf_stream_id so that kfunc users don't have to pull in the
  * enum in headers.
  */
-__bpf_kfunc int bpf_stream_vprintk(int stream_id, const char *fmt__str, const void *args, u32 len__sz, void *aux__prog)
+__bpf_kfunc int bpf_stream_vprintk_impl(int stream_id, const char *fmt__str, const void *args,
+					u32 len__sz, void *aux__prog)
 {
 	struct bpf_bprintf_data data = {
 		.get_bin_args	= true,
diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index 009633294b0934ac282601cf21a0fd03c388de2c..35aeeaf5f71166f0e1e8759da8639c2533d47482 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
@@ -182,7 +182,7 @@ bpftool prog tracelog
 
 bpftool prog tracelog { stdout | stderr } *PROG*
     Dump the BPF stream of the program. BPF programs can write to these streams
-    at runtime with the **bpf_stream_vprintk**\ () kfunc. The kernel may write
+    at runtime with the **bpf_stream_vprintk_impl**\ () kfunc. The kernel may write
     error messages to the standard error stream. This facility should be used
     only for debugging purposes.
 
diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h
index 80c028540656176376909cb796e56de433ef3aab..d4e4e388e625894f8ec27b5a6278dbb46e658720 100644
--- a/tools/lib/bpf/bpf_helpers.h
+++ b/tools/lib/bpf/bpf_helpers.h
@@ -315,20 +315,20 @@ enum libbpf_tristate {
 			  ___param, sizeof(___param));		\
 })
 
-extern int bpf_stream_vprintk(int stream_id, const char *fmt__str, const void *args,
-			      __u32 len__sz, void *aux__prog) __weak __ksym;
-
-#define bpf_stream_printk(stream_id, fmt, args...)				\
-({										\
-	static const char ___fmt[] = fmt;					\
-	unsigned long long ___param[___bpf_narg(args)];				\
-										\
-	_Pragma("GCC diagnostic push")						\
-	_Pragma("GCC diagnostic ignored \"-Wint-conversion\"")			\
-	___bpf_fill(___param, args);						\
-	_Pragma("GCC diagnostic pop")						\
-										\
-	bpf_stream_vprintk(stream_id, ___fmt, ___param, sizeof(___param), NULL);\
+extern int bpf_stream_vprintk_impl(int stream_id, const char *fmt__str, const void *args,
+				   __u32 len__sz, void *aux__prog) __weak __ksym;
+
+#define bpf_stream_printk(stream_id, fmt, args...)					\
+({											\
+	static const char ___fmt[] = fmt;						\
+	unsigned long long ___param[___bpf_narg(args)];					\
+											\
+	_Pragma("GCC diagnostic push")							\
+	_Pragma("GCC diagnostic ignored \"-Wint-conversion\"")				\
+	___bpf_fill(___param, args);							\
+	_Pragma("GCC diagnostic pop")							\
+											\
+	bpf_stream_vprintk_impl(stream_id, ___fmt, ___param, sizeof(___param), NULL);	\
 })
 
 /* Use __bpf_printk when bpf_printk call has 3 or fewer fmt args
diff --git a/tools/testing/selftests/bpf/progs/stream_fail.c b/tools/testing/selftests/bpf/progs/stream_fail.c
index b4a0d0cc8ec8a9483b5967745cd35f8bd940460e..3662515f0107740c147f5a9296b4da06fa508364 100644
--- a/tools/testing/selftests/bpf/progs/stream_fail.c
+++ b/tools/testing/selftests/bpf/progs/stream_fail.c
@@ -10,7 +10,7 @@ SEC("syscall")
 __failure __msg("Possibly NULL pointer passed")
 int stream_vprintk_null_arg(void *ctx)
 {
-	bpf_stream_vprintk(BPF_STDOUT, "", NULL, 0, NULL);
+	bpf_stream_vprintk_impl(BPF_STDOUT, "", NULL, 0, NULL);
 	return 0;
 }
 
@@ -18,7 +18,7 @@ SEC("syscall")
 __failure __msg("R3 type=scalar expected=")
 int stream_vprintk_scalar_arg(void *ctx)
 {
-	bpf_stream_vprintk(BPF_STDOUT, "", (void *)46, 0, NULL);
+	bpf_stream_vprintk_impl(BPF_STDOUT, "", (void *)46, 0, NULL);
 	return 0;
 }
 
@@ -26,7 +26,7 @@ SEC("syscall")
 __failure __msg("arg#1 doesn't point to a const string")
 int stream_vprintk_string_arg(void *ctx)
 {
-	bpf_stream_vprintk(BPF_STDOUT, ctx, NULL, 0, NULL);
+	bpf_stream_vprintk_impl(BPF_STDOUT, ctx, NULL, 0, NULL);
 	return 0;
 }
 

-- 
2.51.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH bpf v2 0/2] bpf: add _impl suffix for kfuncs with implicit args
  2025-11-04 15:29 [PATCH bpf v2 0/2] bpf: add _impl suffix for kfuncs with implicit args Mykyta Yatsenko
  2025-11-04 15:29 ` [PATCH bpf v2 1/2] bpf:add _impl suffix for bpf_task_work_schedule* kfuncs Mykyta Yatsenko
  2025-11-04 15:29 ` [PATCH bpf v2 2/2] bpf:add _impl suffix for bpf_stream_vprintk() kfunc Mykyta Yatsenko
@ 2025-11-04 17:23 ` Ihor Solodrai
  2025-11-04 19:29   ` Andrii Nakryiko
  2 siblings, 1 reply; 9+ messages in thread
From: Ihor Solodrai @ 2025-11-04 17:23 UTC (permalink / raw)
  To: Mykyta Yatsenko, bpf, ast, andrii, daniel, kafai, kernel-team
  Cc: Mykyta Yatsenko

On 11/4/25 7:29 AM, Mykyta Yatsenko wrote:
> We’re introducing support for implicit kfunc arguments and need to
> rename new kfuncs to comply with the naming convention.
> This new feature, will for each kfunc of the form:
> 
> `bpf_foo_impl(args..., aux__prog)`
> 
> generate a public BTF type:
> 
> `bpf_foo(args...)`
> 
> and the verifier will resolve calls to bpf_foo() to bpf_foo_impl(),
> supplying a valid struct bpf_prog_aux via aux__prog.

Hi Mykyta, thank you for submitting this.

The explanation in this cover is inaccurate. There were a few
discussions, and the "implicit" feature is in active development, so
it is confusing... Let me try to elaborate.

Currently if a kfunc needs access to struct bpf_prog_aux data, it must
have an explicit void *aux__prog argument in its declaration. Then on
BPF side the users must pass a dummy value (conventionally NULL).

In the v6.18-rc4 these 4 functions are using aux__prog argument:
  * bpf_wq_set_callback_impl (note existing _impl suffix)
  * bpf_task_work_schedule_signal
  * bpf_task_work_schedule_resume
  * bpf_stream_vprintk

The goal of the KF_IMPLICIT_ARGS feature is to hide this argument from
BPF programs, as it is supplied by the verifier.

With it, the kfuncs still require an explicit argument in the
kernel declaration, for example:

    __bpf_kfunc int bpf_foo(int arg, struct bpf_prog_aux *aux__implicit);

In order to hide it from the BPF users, the following functions will
be produced in BTF from the above declaration:

    /* no aux arg for BPF interface kfunc */
    __bpf_kfunc int bpf_foo(int arg);

    /* no kfunc decl_tag for _impl function */
    int bpf_foo_impl(int arg, struct bpf_prog_aux *aux__implicit);

Now the problem with existing aux__prog users that you're renaming in
this patchset is that because they don't have an _impl suffix, their
prototype will change, breaking binary compatibility with existing BPF
programs. If we simply mark them as KF_IMPLICIT_ARGS, then they lose 
an argument in BTF, for example:

    bpf_task_work_schedule_signal(task, tw, map__map, callback, aux__prog);

becomes

    bpf_task_work_schedule_signal(task, tw, map__map, callback);

However, if we rename it to "bpf_task_work_schedule_signal_impl", then
after KF_IMPLICIT_ARGS feature is implemented, we can *add a new
kfunc* "bpf_task_work_schedule_signal" with an implicit arg.

This way we can avoid breaking BPF progs calling this kfunc, although 
renaming is still a disruption of course.

See links to previous discussions:
* https://lore.kernel.org/bpf/20251029190113.3323406-1-ihor.solodrai@linux.dev/
* https://lore.kernel.org/bpf/20250924211716.1287715-1-ihor.solodrai@linux.dev/
* https://lore.kernel.org/dwarves/20250924211512.1287298-1-ihor.solodrai@linux.dev/

> 
> Three kfuncs added in 6.18 don’t follow this *_impl convention and
> therefore won’t participate in the new mechanism:
>  * bpf_task_work_schedule_resume()
>  * bpf_task_work_schedule_signal()
>  * bpf_stream_vprintk()
> 
> Rename them to align with the implicit-arg flow:
> bpf_task_work_schedule_resume() -> bpf_task_work_schedule_resume_impl()
> bpf_task_work_schedule_signal() -> bpf_task_work_schedule_signal_impl()
> bpf_stream_vprintk() -> bpf_stream_vprintk_impl()
> 
> The implicit-arg mechanism is not in tree yet, so callers must switch to
> the *_impl names for now. Once the new mechanism lands, the plain
> names (without _impl) will be reintroduced as BTF-visible entry points
> and will resolve to the _impl versions automatically.
> 
> Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
> ---
> Changes in v1:
> - Split commit into 2
> - Rebase on the correct branch
> - Link to v1: https://lore.kernel.org/all/20251103232319.122965-1-mykyta.yatsenko5@gmail.com/
> 
> ---
> Mykyta Yatsenko (2):
>       bpf:add _impl suffix for bpf_task_work_schedule* kfuncs
>       bpf:add _impl suffix for bpf_stream_vprintk() kfunc
> 
>  kernel/bpf/helpers.c                               | 26 +++++++++++---------
>  kernel/bpf/stream.c                                |  3 ++-
>  kernel/bpf/verifier.c                              | 12 +++++-----
>  tools/bpf/bpftool/Documentation/bpftool-prog.rst   |  2 +-
>  tools/lib/bpf/bpf_helpers.h                        | 28 +++++++++++-----------
>  tools/testing/selftests/bpf/progs/stream_fail.c    |  6 ++---
>  tools/testing/selftests/bpf/progs/task_work.c      |  6 ++---
>  tools/testing/selftests/bpf/progs/task_work_fail.c |  8 +++----
>  .../testing/selftests/bpf/progs/task_work_stress.c |  4 ++--
>  9 files changed, 50 insertions(+), 45 deletions(-)
> ---
> base-commit: 6146a0f1dfae5d37442a9ddcba012add260bceb0
> change-id: 20251104-implv2-d6c4be255026
> 
> Best regards,


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH bpf v2 1/2] bpf:add _impl suffix for bpf_task_work_schedule* kfuncs
  2025-11-04 15:29 ` [PATCH bpf v2 1/2] bpf:add _impl suffix for bpf_task_work_schedule* kfuncs Mykyta Yatsenko
@ 2025-11-04 19:21   ` Andrii Nakryiko
  0 siblings, 0 replies; 9+ messages in thread
From: Andrii Nakryiko @ 2025-11-04 19:21 UTC (permalink / raw)
  To: Mykyta Yatsenko
  Cc: bpf, ast, andrii, daniel, kafai, kernel-team, Mykyta Yatsenko

On Tue, Nov 4, 2025 at 7:30 AM Mykyta Yatsenko
<mykyta.yatsenko5@gmail.com> wrote:
>
> From: Mykyta Yatsenko <yatsenko@meta.com>
>
> Rename:
> bpf_task_work_schedule_resume()->bpf_task_work_schedule_resume_impl()
> bpf_task_work_schedule_signal()->bpf_task_work_schedule_signal_impl()
>
> This aligns task work scheduling kfuncs with the naming scheme required
> by the implicit-argument feature.
>
> Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
> ---
>  kernel/bpf/helpers.c                               | 24 +++++++++++++---------
>  kernel/bpf/verifier.c                              | 12 +++++------
>  tools/testing/selftests/bpf/progs/task_work.c      |  6 +++---
>  tools/testing/selftests/bpf/progs/task_work_fail.c |  8 ++++----
>  .../testing/selftests/bpf/progs/task_work_stress.c |  4 ++--
>  5 files changed, 29 insertions(+), 25 deletions(-)
>

LGTM, thanks!

Acked-by: Andrii Nakryiko <andrii@kernel.org>


[...]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH bpf v2 2/2] bpf:add _impl suffix for bpf_stream_vprintk() kfunc
  2025-11-04 15:29 ` [PATCH bpf v2 2/2] bpf:add _impl suffix for bpf_stream_vprintk() kfunc Mykyta Yatsenko
@ 2025-11-04 19:25   ` Andrii Nakryiko
  0 siblings, 0 replies; 9+ messages in thread
From: Andrii Nakryiko @ 2025-11-04 19:25 UTC (permalink / raw)
  To: Mykyta Yatsenko
  Cc: bpf, ast, andrii, daniel, kafai, kernel-team, Mykyta Yatsenko

On Tue, Nov 4, 2025 at 7:30 AM Mykyta Yatsenko
<mykyta.yatsenko5@gmail.com> wrote:
>
> From: Mykyta Yatsenko <yatsenko@meta.com>
>
> Rename bpf_stream_vprintk() to bpf_stream_vprintk_impl().
>
> This aligns this recently added kfunc with the naming scheme required
> by the implicit-argument feature.
> In future BTF type for bpf_stream_vprintk() will be generated and
> aux__prog argument filled by the valid struct implicitly.

How about this wording?

This makes bpf_stream_vprintk() follow the already established "_impl"
suffix-based naming convention for kfuncs with the bpf_prog_aux
argument provided by the verifier implicitly. This convention will be
taken advantage of with the upcoming KF_IMPLICIT_ARGS feature to
preserve backwards compatibility to BPF programs.


Other than that, LGTM.

Acked-by: Andrii Nakryiko <andrii@kernel.org>

>
> Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
> ---
>  kernel/bpf/helpers.c                             |  2 +-
>  kernel/bpf/stream.c                              |  3 ++-
>  tools/bpf/bpftool/Documentation/bpftool-prog.rst |  2 +-
>  tools/lib/bpf/bpf_helpers.h                      | 28 ++++++++++++------------
>  tools/testing/selftests/bpf/progs/stream_fail.c  |  6 ++---
>  5 files changed, 21 insertions(+), 20 deletions(-)
>
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index 33173b027ccf8893ce18aad474b88f8544f7b344..e4007fea49091c01c1d23af55a25f5567417e978 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -4380,7 +4380,7 @@ BTF_ID_FLAGS(func, bpf_strnstr);
>  #if defined(CONFIG_BPF_LSM) && defined(CONFIG_CGROUPS)
>  BTF_ID_FLAGS(func, bpf_cgroup_read_xattr, KF_RCU)
>  #endif
> -BTF_ID_FLAGS(func, bpf_stream_vprintk, KF_TRUSTED_ARGS)
> +BTF_ID_FLAGS(func, bpf_stream_vprintk_impl, KF_TRUSTED_ARGS)
>  BTF_ID_FLAGS(func, bpf_task_work_schedule_signal_impl, KF_TRUSTED_ARGS)
>  BTF_ID_FLAGS(func, bpf_task_work_schedule_resume_impl, KF_TRUSTED_ARGS)
>  BTF_KFUNCS_END(common_btf_ids)
> diff --git a/kernel/bpf/stream.c b/kernel/bpf/stream.c
> index eb6c5a21c2efee96c41f4c5e43d54062694a4859..ff16c631951bb685e8ecf1707206dad603121a65 100644
> --- a/kernel/bpf/stream.c
> +++ b/kernel/bpf/stream.c
> @@ -355,7 +355,8 @@ __bpf_kfunc_start_defs();
>   * Avoid using enum bpf_stream_id so that kfunc users don't have to pull in the
>   * enum in headers.
>   */
> -__bpf_kfunc int bpf_stream_vprintk(int stream_id, const char *fmt__str, const void *args, u32 len__sz, void *aux__prog)
> +__bpf_kfunc int bpf_stream_vprintk_impl(int stream_id, const char *fmt__str, const void *args,
> +                                       u32 len__sz, void *aux__prog)
>  {
>         struct bpf_bprintf_data data = {
>                 .get_bin_args   = true,
> diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> index 009633294b0934ac282601cf21a0fd03c388de2c..35aeeaf5f71166f0e1e8759da8639c2533d47482 100644
> --- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> +++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> @@ -182,7 +182,7 @@ bpftool prog tracelog
>
>  bpftool prog tracelog { stdout | stderr } *PROG*
>      Dump the BPF stream of the program. BPF programs can write to these streams
> -    at runtime with the **bpf_stream_vprintk**\ () kfunc. The kernel may write
> +    at runtime with the **bpf_stream_vprintk_impl**\ () kfunc. The kernel may write
>      error messages to the standard error stream. This facility should be used
>      only for debugging purposes.
>
> diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h
> index 80c028540656176376909cb796e56de433ef3aab..d4e4e388e625894f8ec27b5a6278dbb46e658720 100644
> --- a/tools/lib/bpf/bpf_helpers.h
> +++ b/tools/lib/bpf/bpf_helpers.h
> @@ -315,20 +315,20 @@ enum libbpf_tristate {
>                           ___param, sizeof(___param));          \
>  })
>
> -extern int bpf_stream_vprintk(int stream_id, const char *fmt__str, const void *args,
> -                             __u32 len__sz, void *aux__prog) __weak __ksym;
> -
> -#define bpf_stream_printk(stream_id, fmt, args...)                             \
> -({                                                                             \
> -       static const char ___fmt[] = fmt;                                       \
> -       unsigned long long ___param[___bpf_narg(args)];                         \
> -                                                                               \
> -       _Pragma("GCC diagnostic push")                                          \
> -       _Pragma("GCC diagnostic ignored \"-Wint-conversion\"")                  \
> -       ___bpf_fill(___param, args);                                            \
> -       _Pragma("GCC diagnostic pop")                                           \
> -                                                                               \
> -       bpf_stream_vprintk(stream_id, ___fmt, ___param, sizeof(___param), NULL);\
> +extern int bpf_stream_vprintk_impl(int stream_id, const char *fmt__str, const void *args,
> +                                  __u32 len__sz, void *aux__prog) __weak __ksym;
> +
> +#define bpf_stream_printk(stream_id, fmt, args...)                                     \
> +({                                                                                     \
> +       static const char ___fmt[] = fmt;                                               \
> +       unsigned long long ___param[___bpf_narg(args)];                                 \
> +                                                                                       \
> +       _Pragma("GCC diagnostic push")                                                  \
> +       _Pragma("GCC diagnostic ignored \"-Wint-conversion\"")                          \
> +       ___bpf_fill(___param, args);                                                    \
> +       _Pragma("GCC diagnostic pop")                                                   \
> +                                                                                       \
> +       bpf_stream_vprintk_impl(stream_id, ___fmt, ___param, sizeof(___param), NULL);   \
>  })
>
>  /* Use __bpf_printk when bpf_printk call has 3 or fewer fmt args
> diff --git a/tools/testing/selftests/bpf/progs/stream_fail.c b/tools/testing/selftests/bpf/progs/stream_fail.c
> index b4a0d0cc8ec8a9483b5967745cd35f8bd940460e..3662515f0107740c147f5a9296b4da06fa508364 100644
> --- a/tools/testing/selftests/bpf/progs/stream_fail.c
> +++ b/tools/testing/selftests/bpf/progs/stream_fail.c
> @@ -10,7 +10,7 @@ SEC("syscall")
>  __failure __msg("Possibly NULL pointer passed")
>  int stream_vprintk_null_arg(void *ctx)
>  {
> -       bpf_stream_vprintk(BPF_STDOUT, "", NULL, 0, NULL);
> +       bpf_stream_vprintk_impl(BPF_STDOUT, "", NULL, 0, NULL);
>         return 0;
>  }
>
> @@ -18,7 +18,7 @@ SEC("syscall")
>  __failure __msg("R3 type=scalar expected=")
>  int stream_vprintk_scalar_arg(void *ctx)
>  {
> -       bpf_stream_vprintk(BPF_STDOUT, "", (void *)46, 0, NULL);
> +       bpf_stream_vprintk_impl(BPF_STDOUT, "", (void *)46, 0, NULL);
>         return 0;
>  }
>
> @@ -26,7 +26,7 @@ SEC("syscall")
>  __failure __msg("arg#1 doesn't point to a const string")
>  int stream_vprintk_string_arg(void *ctx)
>  {
> -       bpf_stream_vprintk(BPF_STDOUT, ctx, NULL, 0, NULL);
> +       bpf_stream_vprintk_impl(BPF_STDOUT, ctx, NULL, 0, NULL);
>         return 0;
>  }
>
>
> --
> 2.51.1
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH bpf v2 0/2] bpf: add _impl suffix for kfuncs with implicit args
  2025-11-04 17:23 ` [PATCH bpf v2 0/2] bpf: add _impl suffix for kfuncs with implicit args Ihor Solodrai
@ 2025-11-04 19:29   ` Andrii Nakryiko
  2025-11-04 19:39     ` Ihor Solodrai
  2025-11-04 22:13     ` Mykyta Yatsenko
  0 siblings, 2 replies; 9+ messages in thread
From: Andrii Nakryiko @ 2025-11-04 19:29 UTC (permalink / raw)
  To: Ihor Solodrai
  Cc: Mykyta Yatsenko, bpf, ast, andrii, daniel, kafai, kernel-team,
	Mykyta Yatsenko

On Tue, Nov 4, 2025 at 9:23 AM Ihor Solodrai <ihor.solodrai@linux.dev> wrote:
>
> On 11/4/25 7:29 AM, Mykyta Yatsenko wrote:
> > We’re introducing support for implicit kfunc arguments and need to
> > rename new kfuncs to comply with the naming convention.
> > This new feature, will for each kfunc of the form:
> >
> > `bpf_foo_impl(args..., aux__prog)`
> >
> > generate a public BTF type:
> >
> > `bpf_foo(args...)`
> >
> > and the verifier will resolve calls to bpf_foo() to bpf_foo_impl(),
> > supplying a valid struct bpf_prog_aux via aux__prog.
>
> Hi Mykyta, thank you for submitting this.
>
> The explanation in this cover is inaccurate. There were a few
> discussions, and the "implicit" feature is in active development, so
> it is confusing... Let me try to elaborate.
>
> Currently if a kfunc needs access to struct bpf_prog_aux data, it must
> have an explicit void *aux__prog argument in its declaration. Then on
> BPF side the users must pass a dummy value (conventionally NULL).
>
> In the v6.18-rc4 these 4 functions are using aux__prog argument:
>   * bpf_wq_set_callback_impl (note existing _impl suffix)
>   * bpf_task_work_schedule_signal
>   * bpf_task_work_schedule_resume
>   * bpf_stream_vprintk
>
> The goal of the KF_IMPLICIT_ARGS feature is to hide this argument from
> BPF programs, as it is supplied by the verifier.
>
> With it, the kfuncs still require an explicit argument in the
> kernel declaration, for example:
>
>     __bpf_kfunc int bpf_foo(int arg, struct bpf_prog_aux *aux__implicit);
>
> In order to hide it from the BPF users, the following functions will
> be produced in BTF from the above declaration:
>
>     /* no aux arg for BPF interface kfunc */
>     __bpf_kfunc int bpf_foo(int arg);
>
>     /* no kfunc decl_tag for _impl function */
>     int bpf_foo_impl(int arg, struct bpf_prog_aux *aux__implicit);
>
> Now the problem with existing aux__prog users that you're renaming in
> this patchset is that because they don't have an _impl suffix, their
> prototype will change, breaking binary compatibility with existing BPF
> programs. If we simply mark them as KF_IMPLICIT_ARGS, then they lose
> an argument in BTF, for example:
>
>     bpf_task_work_schedule_signal(task, tw, map__map, callback, aux__prog);
>
> becomes
>
>     bpf_task_work_schedule_signal(task, tw, map__map, callback);
>
> However, if we rename it to "bpf_task_work_schedule_signal_impl", then
> after KF_IMPLICIT_ARGS feature is implemented, we can *add a new
> kfunc* "bpf_task_work_schedule_signal" with an implicit arg.
>
> This way we can avoid breaking BPF progs calling this kfunc, although
> renaming is still a disruption of course.
>
> See links to previous discussions:
> * https://lore.kernel.org/bpf/20251029190113.3323406-1-ihor.solodrai@linux.dev/
> * https://lore.kernel.org/bpf/20250924211716.1287715-1-ihor.solodrai@linux.dev/
> * https://lore.kernel.org/dwarves/20250924211512.1287298-1-ihor.solodrai@linux.dev/
>
> >
> > Three kfuncs added in 6.18 don’t follow this *_impl convention and
> > therefore won’t participate in the new mechanism:
> >  * bpf_task_work_schedule_resume()
> >  * bpf_task_work_schedule_signal()
> >  * bpf_stream_vprintk()
> >
> > Rename them to align with the implicit-arg flow:
> > bpf_task_work_schedule_resume() -> bpf_task_work_schedule_resume_impl()
> > bpf_task_work_schedule_signal() -> bpf_task_work_schedule_signal_impl()
> > bpf_stream_vprintk() -> bpf_stream_vprintk_impl()
> >
> > The implicit-arg mechanism is not in tree yet, so callers must switch to
> > the *_impl names for now. Once the new mechanism lands, the plain
> > names (without _impl) will be reintroduced as BTF-visible entry points
> > and will resolve to the _impl versions automatically.
> >

TBH, it looks like both Mykyta's and Ihor's descriptions are a little
bit too detailed and are more concerned with details of the upcoming
feature.

What's important with these fixes is that we are fixing deviation from
the previously established "_impl" suffix naming convention for these
kfuncs that accept verifier-provided bpf_prog_aux arguments. Following
uniform convention will allow for transparent backwards compatibility
with the upcoming KF_IMPLICIT_ARGS feature, so this patch set aims to
fix current deviation from the convention so as to eliminate
unnecessary backwards incompatibility breakage in the future.

WDYT?

> > Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
> > ---
> > Changes in v1:
> > - Split commit into 2
> > - Rebase on the correct branch
> > - Link to v1: https://lore.kernel.org/all/20251103232319.122965-1-mykyta.yatsenko5@gmail.com/
> >
> > ---
> > Mykyta Yatsenko (2):
> >       bpf:add _impl suffix for bpf_task_work_schedule* kfuncs
> >       bpf:add _impl suffix for bpf_stream_vprintk() kfunc
> >
> >  kernel/bpf/helpers.c                               | 26 +++++++++++---------
> >  kernel/bpf/stream.c                                |  3 ++-
> >  kernel/bpf/verifier.c                              | 12 +++++-----
> >  tools/bpf/bpftool/Documentation/bpftool-prog.rst   |  2 +-
> >  tools/lib/bpf/bpf_helpers.h                        | 28 +++++++++++-----------
> >  tools/testing/selftests/bpf/progs/stream_fail.c    |  6 ++---
> >  tools/testing/selftests/bpf/progs/task_work.c      |  6 ++---
> >  tools/testing/selftests/bpf/progs/task_work_fail.c |  8 +++----
> >  .../testing/selftests/bpf/progs/task_work_stress.c |  4 ++--
> >  9 files changed, 50 insertions(+), 45 deletions(-)
> > ---
> > base-commit: 6146a0f1dfae5d37442a9ddcba012add260bceb0
> > change-id: 20251104-implv2-d6c4be255026
> >
> > Best regards,
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH bpf v2 0/2] bpf: add _impl suffix for kfuncs with implicit args
  2025-11-04 19:29   ` Andrii Nakryiko
@ 2025-11-04 19:39     ` Ihor Solodrai
  2025-11-04 22:13     ` Mykyta Yatsenko
  1 sibling, 0 replies; 9+ messages in thread
From: Ihor Solodrai @ 2025-11-04 19:39 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Mykyta Yatsenko, bpf, ast, andrii, daniel, kafai, kernel-team,
	Mykyta Yatsenko

On 11/4/25 11:29 AM, Andrii Nakryiko wrote:
> On Tue, Nov 4, 2025 at 9:23 AM Ihor Solodrai <ihor.solodrai@linux.dev> wrote:
>>
>> On 11/4/25 7:29 AM, Mykyta Yatsenko wrote:
>>> We’re introducing support for implicit kfunc arguments and need to
>>> rename new kfuncs to comply with the naming convention.
>>> This new feature, will for each kfunc of the form:
>>>
>>> `bpf_foo_impl(args..., aux__prog)`
>>>
>>> generate a public BTF type:
>>>
>>> `bpf_foo(args...)`
>>>
>>> and the verifier will resolve calls to bpf_foo() to bpf_foo_impl(),
>>> supplying a valid struct bpf_prog_aux via aux__prog.
>>
>> Hi Mykyta, thank you for submitting this.
>>
>> The explanation in this cover is inaccurate. There were a few
>> discussions, and the "implicit" feature is in active development, so
>> it is confusing... Let me try to elaborate.
>>
>> [...]
>> 
>>> The implicit-arg mechanism is not in tree yet, so callers must switch to
>>> the *_impl names for now. Once the new mechanism lands, the plain
>>> names (without _impl) will be reintroduced as BTF-visible entry points
>>> and will resolve to the _impl versions automatically.
>>>
> 
> TBH, it looks like both Mykyta's and Ihor's descriptions are a little
> bit too detailed and are more concerned with details of the upcoming
> feature.
> 
> What's important with these fixes is that we are fixing deviation from
> the previously established "_impl" suffix naming convention for these
> kfuncs that accept verifier-provided bpf_prog_aux arguments. Following
> uniform convention will allow for transparent backwards compatibility
> with the upcoming KF_IMPLICIT_ARGS feature, so this patch set aims to
> fix current deviation from the convention so as to eliminate
> unnecessary backwards incompatibility breakage in the future.
> 
> WDYT?

I agree. For this patches we didn't have to go into the details, but that
has already happened :)

> 
>>> Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
>>> ---
>>> Changes in v1:
>>> - Split commit into 2
>>> - Rebase on the correct branch
>>> - Link to v1: https://lore.kernel.org/all/20251103232319.122965-1-mykyta.yatsenko5@gmail.com/
>>>
>>> ---
>>> Mykyta Yatsenko (2):
>>>       bpf:add _impl suffix for bpf_task_work_schedule* kfuncs
>>>       bpf:add _impl suffix for bpf_stream_vprintk() kfunc
>>>
>>>  kernel/bpf/helpers.c                               | 26 +++++++++++---------
>>>  kernel/bpf/stream.c                                |  3 ++-
>>>  kernel/bpf/verifier.c                              | 12 +++++-----
>>>  tools/bpf/bpftool/Documentation/bpftool-prog.rst   |  2 +-
>>>  tools/lib/bpf/bpf_helpers.h                        | 28 +++++++++++-----------
>>>  tools/testing/selftests/bpf/progs/stream_fail.c    |  6 ++---
>>>  tools/testing/selftests/bpf/progs/task_work.c      |  6 ++---
>>>  tools/testing/selftests/bpf/progs/task_work_fail.c |  8 +++----
>>>  .../testing/selftests/bpf/progs/task_work_stress.c |  4 ++--
>>>  9 files changed, 50 insertions(+), 45 deletions(-)
>>> ---
>>> base-commit: 6146a0f1dfae5d37442a9ddcba012add260bceb0
>>> change-id: 20251104-implv2-d6c4be255026
>>>
>>> Best regards,
>>


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH bpf v2 0/2] bpf: add _impl suffix for kfuncs with implicit args
  2025-11-04 19:29   ` Andrii Nakryiko
  2025-11-04 19:39     ` Ihor Solodrai
@ 2025-11-04 22:13     ` Mykyta Yatsenko
  1 sibling, 0 replies; 9+ messages in thread
From: Mykyta Yatsenko @ 2025-11-04 22:13 UTC (permalink / raw)
  To: Andrii Nakryiko, Ihor Solodrai
  Cc: bpf, ast, andrii, daniel, kafai, kernel-team, Mykyta Yatsenko

On 11/4/25 19:29, Andrii Nakryiko wrote:
> On Tue, Nov 4, 2025 at 9:23 AM Ihor Solodrai <ihor.solodrai@linux.dev> wrote:
>> On 11/4/25 7:29 AM, Mykyta Yatsenko wrote:
>>> We’re introducing support for implicit kfunc arguments and need to
>>> rename new kfuncs to comply with the naming convention.
>>> This new feature, will for each kfunc of the form:
>>>
>>> `bpf_foo_impl(args..., aux__prog)`
>>>
>>> generate a public BTF type:
>>>
>>> `bpf_foo(args...)`
>>>
>>> and the verifier will resolve calls to bpf_foo() to bpf_foo_impl(),
>>> supplying a valid struct bpf_prog_aux via aux__prog.
>> Hi Mykyta, thank you for submitting this.
>>
>> The explanation in this cover is inaccurate. There were a few
>> discussions, and the "implicit" feature is in active development, so
>> it is confusing... Let me try to elaborate.
>>
>> Currently if a kfunc needs access to struct bpf_prog_aux data, it must
>> have an explicit void *aux__prog argument in its declaration. Then on
>> BPF side the users must pass a dummy value (conventionally NULL).
>>
>> In the v6.18-rc4 these 4 functions are using aux__prog argument:
>>    * bpf_wq_set_callback_impl (note existing _impl suffix)
>>    * bpf_task_work_schedule_signal
>>    * bpf_task_work_schedule_resume
>>    * bpf_stream_vprintk
>>
>> The goal of the KF_IMPLICIT_ARGS feature is to hide this argument from
>> BPF programs, as it is supplied by the verifier.
>>
>> With it, the kfuncs still require an explicit argument in the
>> kernel declaration, for example:
>>
>>      __bpf_kfunc int bpf_foo(int arg, struct bpf_prog_aux *aux__implicit);
>>
>> In order to hide it from the BPF users, the following functions will
>> be produced in BTF from the above declaration:
>>
>>      /* no aux arg for BPF interface kfunc */
>>      __bpf_kfunc int bpf_foo(int arg);
>>
>>      /* no kfunc decl_tag for _impl function */
>>      int bpf_foo_impl(int arg, struct bpf_prog_aux *aux__implicit);
>>
>> Now the problem with existing aux__prog users that you're renaming in
>> this patchset is that because they don't have an _impl suffix, their
>> prototype will change, breaking binary compatibility with existing BPF
>> programs. If we simply mark them as KF_IMPLICIT_ARGS, then they lose
>> an argument in BTF, for example:
>>
>>      bpf_task_work_schedule_signal(task, tw, map__map, callback, aux__prog);
>>
>> becomes
>>
>>      bpf_task_work_schedule_signal(task, tw, map__map, callback);
>>
>> However, if we rename it to "bpf_task_work_schedule_signal_impl", then
>> after KF_IMPLICIT_ARGS feature is implemented, we can *add a new
>> kfunc* "bpf_task_work_schedule_signal" with an implicit arg.
>>
>> This way we can avoid breaking BPF progs calling this kfunc, although
>> renaming is still a disruption of course.
>>
>> See links to previous discussions:
>> * https://lore.kernel.org/bpf/20251029190113.3323406-1-ihor.solodrai@linux.dev/
>> * https://lore.kernel.org/bpf/20250924211716.1287715-1-ihor.solodrai@linux.dev/
>> * https://lore.kernel.org/dwarves/20250924211512.1287298-1-ihor.solodrai@linux.dev/
>>
>>> Three kfuncs added in 6.18 don’t follow this *_impl convention and
>>> therefore won’t participate in the new mechanism:
>>>   * bpf_task_work_schedule_resume()
>>>   * bpf_task_work_schedule_signal()
>>>   * bpf_stream_vprintk()
>>>
>>> Rename them to align with the implicit-arg flow:
>>> bpf_task_work_schedule_resume() -> bpf_task_work_schedule_resume_impl()
>>> bpf_task_work_schedule_signal() -> bpf_task_work_schedule_signal_impl()
>>> bpf_stream_vprintk() -> bpf_stream_vprintk_impl()
>>>
>>> The implicit-arg mechanism is not in tree yet, so callers must switch to
>>> the *_impl names for now. Once the new mechanism lands, the plain
>>> names (without _impl) will be reintroduced as BTF-visible entry points
>>> and will resolve to the _impl versions automatically.
>>>
> TBH, it looks like both Mykyta's and Ihor's descriptions are a little
> bit too detailed and are more concerned with details of the upcoming
> feature.
>
> What's important with these fixes is that we are fixing deviation from
> the previously established "_impl" suffix naming convention for these
> kfuncs that accept verifier-provided bpf_prog_aux arguments. Following
> uniform convention will allow for transparent backwards compatibility
> with the upcoming KF_IMPLICIT_ARGS feature, so this patch set aims to
> fix current deviation from the convention so as to eliminate
> unnecessary backwards incompatibility breakage in the future.
>
> WDYT?
Thanks, this message makes more sense, let me update commit descriptions 
and send v3.
>>> Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
>>> ---
>>> Changes in v1:
>>> - Split commit into 2
>>> - Rebase on the correct branch
>>> - Link to v1: https://lore.kernel.org/all/20251103232319.122965-1-mykyta.yatsenko5@gmail.com/
>>>
>>> ---
>>> Mykyta Yatsenko (2):
>>>        bpf:add _impl suffix for bpf_task_work_schedule* kfuncs
>>>        bpf:add _impl suffix for bpf_stream_vprintk() kfunc
>>>
>>>   kernel/bpf/helpers.c                               | 26 +++++++++++---------
>>>   kernel/bpf/stream.c                                |  3 ++-
>>>   kernel/bpf/verifier.c                              | 12 +++++-----
>>>   tools/bpf/bpftool/Documentation/bpftool-prog.rst   |  2 +-
>>>   tools/lib/bpf/bpf_helpers.h                        | 28 +++++++++++-----------
>>>   tools/testing/selftests/bpf/progs/stream_fail.c    |  6 ++---
>>>   tools/testing/selftests/bpf/progs/task_work.c      |  6 ++---
>>>   tools/testing/selftests/bpf/progs/task_work_fail.c |  8 +++----
>>>   .../testing/selftests/bpf/progs/task_work_stress.c |  4 ++--
>>>   9 files changed, 50 insertions(+), 45 deletions(-)
>>> ---
>>> base-commit: 6146a0f1dfae5d37442a9ddcba012add260bceb0
>>> change-id: 20251104-implv2-d6c4be255026
>>>
>>> Best regards,


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2025-11-04 22:13 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-04 15:29 [PATCH bpf v2 0/2] bpf: add _impl suffix for kfuncs with implicit args Mykyta Yatsenko
2025-11-04 15:29 ` [PATCH bpf v2 1/2] bpf:add _impl suffix for bpf_task_work_schedule* kfuncs Mykyta Yatsenko
2025-11-04 19:21   ` Andrii Nakryiko
2025-11-04 15:29 ` [PATCH bpf v2 2/2] bpf:add _impl suffix for bpf_stream_vprintk() kfunc Mykyta Yatsenko
2025-11-04 19:25   ` Andrii Nakryiko
2025-11-04 17:23 ` [PATCH bpf v2 0/2] bpf: add _impl suffix for kfuncs with implicit args Ihor Solodrai
2025-11-04 19:29   ` Andrii Nakryiko
2025-11-04 19:39     ` Ihor Solodrai
2025-11-04 22:13     ` Mykyta Yatsenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).