bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Leon Hwang <leon.hwang@linux.dev>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: bpf <bpf@vger.kernel.org>, Alexei Starovoitov <ast@kernel.org>,
	Andrii Nakryiko <andrii@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Martin KaFai Lau <martin.lau@linux.dev>,
	Eduard <eddyz87@gmail.com>, Song Liu <song@kernel.org>,
	Yonghong Song <yonghong.song@linux.dev>,
	kernel-patches-bot@fb.com
Subject: Re: [PATCH bpf-next v2 1/2] bpf: Introduce bpf_in_interrupt kfunc
Date: Tue, 26 Aug 2025 11:00:03 +0800	[thread overview]
Message-ID: <d7ca66b9-c8a5-47c4-9feb-d7814efcce0a@linux.dev> (raw)
In-Reply-To: <CAADnVQLdmjApwAbrGca2VLQ-SK-3EdQTyd0prEy0BQGrW4Fr6A@mail.gmail.com>



On 25/8/25 23:17, Alexei Starovoitov wrote:
> On Mon, Aug 25, 2025 at 6:15 AM Leon Hwang <leon.hwang@linux.dev> wrote:
>>
>> Filtering pid_tgid is meaningless when the current task is preempted by
>> an interrupt.
>>
>> To address this, introduce the bpf_in_interrupt kfunc, which allows BPF
>> programs to determine whether they are executing in interrupt context.
>>
>> This enables programs to avoid applying pid_tgid filtering when running
>> in such contexts.
>>
>> Signed-off-by: Leon Hwang <leon.hwang@linux.dev>
>> ---
>>  kernel/bpf/helpers.c  |  9 +++++++++
>>  kernel/bpf/verifier.c | 11 +++++++++++
>>  2 files changed, 20 insertions(+)
>>
>> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
>> index 401b4932cc49f..38991b7b4a9e9 100644
>> --- a/kernel/bpf/helpers.c
>> +++ b/kernel/bpf/helpers.c
>> @@ -3711,6 +3711,14 @@ __bpf_kfunc int bpf_strstr(const char *s1__ign, const char *s2__ign)
>>         return bpf_strnstr(s1__ign, s2__ign, XATTR_SIZE_MAX);
>>  }
>>
>> +/**
>> + * bpf_in_interrupt - Check whether it's in interrupt context
>> + */
>> +__bpf_kfunc int bpf_in_interrupt(void)
>> +{
>> +       return in_interrupt();
>> +}
> 
> It doesn't scale. Next thing people will ask for hard vs soft irq.
> 

How about adding a 'flags'?

Here are the values for 'flags':

* 0: return in_interrupt();
* 1(NMI): return in_nmi();
* 2(HARDIRQ): return in_hardirq();
* 3(SOFTIRQ): return in_softirq();

>> +
>>  __bpf_kfunc_end_defs();
>>
>>  BTF_KFUNCS_START(generic_btf_ids)
>> @@ -3751,6 +3759,7 @@ BTF_ID_FLAGS(func, bpf_throw)
>>  #ifdef CONFIG_BPF_EVENTS
>>  BTF_ID_FLAGS(func, bpf_send_signal_task, KF_TRUSTED_ARGS)
>>  #endif
>> +BTF_ID_FLAGS(func, bpf_in_interrupt, KF_FASTCALL)
>>  BTF_KFUNCS_END(generic_btf_ids)
>>
>>  static const struct btf_kfunc_id_set generic_kfunc_set = {
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index 5c9dd16b2c56b..e30ecbfc29dad 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -12259,6 +12259,7 @@ enum special_kfunc_type {
>>         KF_bpf_res_spin_lock_irqsave,
>>         KF_bpf_res_spin_unlock_irqrestore,
>>         KF___bpf_trap,
>> +       KF_bpf_in_interrupt,
>>  };
>>
>>  BTF_ID_LIST(special_kfunc_list)
>> @@ -12327,6 +12328,7 @@ 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_in_interrupt)
>>
>>  static bool is_kfunc_ret_null(struct bpf_kfunc_call_arg_meta *meta)
>>  {
>> @@ -21977,6 +21979,15 @@ static int fixup_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
>>                    desc->func_id == special_kfunc_list[KF_bpf_rdonly_cast]) {
>>                 insn_buf[0] = BPF_MOV64_REG(BPF_REG_0, BPF_REG_1);
>>                 *cnt = 1;
>> +       } else if (desc->func_id == special_kfunc_list[KF_bpf_in_interrupt]) {
>> +#if defined(CONFIG_X86_64) && !defined(CONFIG_UML)
>> +               insn_buf[0] = BPF_MOV64_IMM(BPF_REG_0, (u32)(unsigned long)&__preempt_count);
> 
> I think bpf_per_cpu_ptr() should already be able to read that per cpu var.
> 

Correct. bpf_per_cpu_ptr() and bpf_this_cpu_ptr() are helpful to read it.

So, this patch seems useless.

>> +               insn_buf[1] = BPF_MOV64_PERCPU_REG(BPF_REG_0, BPF_REG_0);
>> +               insn_buf[2] = BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_0, 0);
>> +               insn_buf[3] = BPF_ALU32_IMM(BPF_AND, BPF_REG_0, NMI_MASK | HARDIRQ_MASK |
>> +                                           (IS_ENABLED(CONFIG_PREEMPT_RT) ? 0 : SOFTIRQ_MASK));
> 
> This is still wrong in PREEMPT_RT.
> 

You are right.

Double-check the following macros:

#ifdef CONFIG_PREEMPT_RT
# define softirq_count()        (current->softirq_disable_cnt &
SOFTIRQ_MASK)
# define irq_count()            ((preempt_count() & (NMI_MASK |
HARDIRQ_MASK)) | softirq_count())
#else
# define softirq_count()        (preempt_count() & SOFTIRQ_MASK)
# define irq_count()            (preempt_count() & (NMI_MASK |
HARDIRQ_MASK | SOFTIRQ_MASK))
#endif
#define in_interrupt()          (irq_count())

If PREEMPT_RT, 'softirq_count()' is missing here.

Thanks,
Leon


  reply	other threads:[~2025-08-26  3:00 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-25 13:14 [PATCH bpf-next v2 0/2] bpf: Introduce bpf_in_interrupt kfunc Leon Hwang
2025-08-25 13:15 ` [PATCH bpf-next v2 1/2] " Leon Hwang
2025-08-25 15:17   ` Alexei Starovoitov
2025-08-26  3:00     ` Leon Hwang [this message]
2025-08-26 22:18       ` Alexei Starovoitov
2025-09-01 15:12         ` Leon Hwang
2025-09-02  2:29           ` Alexei Starovoitov
2025-09-03  5:22             ` Leon Hwang
2025-08-25 13:15 ` [PATCH bpf-next v2 2/2] selftests/bpf: Add case to test " Leon Hwang
2025-08-25 17:26   ` Eduard Zingerman
2025-08-26  3:05     ` Leon Hwang
2025-08-26 22:31       ` Eduard Zingerman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d7ca66b9-c8a5-47c4-9feb-d7814efcce0a@linux.dev \
    --to=leon.hwang@linux.dev \
    --cc=alexei.starovoitov@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=kernel-patches-bot@fb.com \
    --cc=martin.lau@linux.dev \
    --cc=song@kernel.org \
    --cc=yonghong.song@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).