[PATCH bpf-next 0/2] bpf: fix couple of netdevsim issues

[PATCH bpf-next 0/2] bpf: fix couple of netdevsim issues

[Stanislav Fomichev]

The first one is found by the syzkaller and needs to make a proper
distinction between offloaded and dev-bound cases.

The second one restores behavior where the offloaded/dev-bound programs
with a dead netdev were not correctly skipped during BPF_PROG_GET_NEXT_ID.

Stanislav Fomichev (2):
  netdevsim: don't accept device bound programs
  bpf: bring back removal of dev-bound id from idr

 drivers/net/netdevsim/bpf.c |  4 ++--
 include/linux/bpf.h         |  2 ++
 kernel/bpf/offload.c        |  3 +++
 kernel/bpf/syscall.c        | 15 +++++++++++----
 4 files changed, 18 insertions(+), 6 deletions(-)

-- 
2.42.0.869.gea05f2083d-goog

[PATCH bpf-next 1/2] netdevsim: don't accept device bound programs

[Stanislav Fomichev]

Commit 2b3486bc2d23 ("bpf: Introduce device-bound XDP programs")
introduced device-bound programs by largely reusing existing
offloading infrastructure. This changed the semantics of
'prog->aux->offload' a bit. Now, it's non-null for both
offloaded and device-bound programs.

Instead of looking at 'prog->aux->offload' let's call
bpf_prog_is_offloaded which should be true iff the program
is offloaded and not merely device-bound.

Cc: Dipendra Khadka <kdipendra88@gmail.com>
Reported-by: syzbot+44c2416196b7c607f226@syzkaller.appspotmail.com
Fixes: 2b3486bc2d23 ("bpf: Introduce device-bound XDP programs")
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
 drivers/net/netdevsim/bpf.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/netdevsim/bpf.c b/drivers/net/netdevsim/bpf.c
index f60eb97e3a62..608953d4f98d 100644
--- a/drivers/net/netdevsim/bpf.c
+++ b/drivers/net/netdevsim/bpf.c
@@ -93,7 +93,7 @@ static void nsim_prog_set_loaded(struct bpf_prog *prog, bool loaded)
 {
 	struct nsim_bpf_bound_prog *state;
 
-	if (!prog || !prog->aux->offload)
+	if (!prog || !bpf_prog_is_offloaded(prog->aux))
 		return;
 
 	state = prog->aux->offload->dev_priv;
@@ -311,7 +311,7 @@ nsim_setup_prog_hw_checks(struct netdevsim *ns, struct netdev_bpf *bpf)
 	if (!bpf->prog)
 		return 0;
 
-	if (!bpf->prog->aux->offload) {
+	if (!bpf_prog_is_offloaded(bpf->prog->aux)) {
 		NSIM_EA(bpf->extack, "xdpoffload of non-bound program");
 		return -EINVAL;
 	}
-- 
2.42.0.869.gea05f2083d-goog

Re: [PATCH bpf-next 1/2] netdevsim: don't accept device bound programs

[Stanislav Fomichev]

On 11/13, Stanislav Fomichev wrote:
> Commit 2b3486bc2d23 ("bpf: Introduce device-bound XDP programs")
> introduced device-bound programs by largely reusing existing
> offloading infrastructure. This changed the semantics of
> 'prog->aux->offload' a bit. Now, it's non-null for both
> offloaded and device-bound programs.
> 
> Instead of looking at 'prog->aux->offload' let's call
> bpf_prog_is_offloaded which should be true iff the program
> is offloaded and not merely device-bound.
> 
> Cc: Dipendra Khadka <kdipendra88@gmail.com>
> Reported-by: syzbot+44c2416196b7c607f226@syzkaller.appspotmail.com
> Fixes: 2b3486bc2d23 ("bpf: Introduce device-bound XDP programs")
> Signed-off-by: Stanislav Fomichev <sdf@google.com>
> ---
>  drivers/net/netdevsim/bpf.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/netdevsim/bpf.c b/drivers/net/netdevsim/bpf.c
> index f60eb97e3a62..608953d4f98d 100644
> --- a/drivers/net/netdevsim/bpf.c
> +++ b/drivers/net/netdevsim/bpf.c
> @@ -93,7 +93,7 @@ static void nsim_prog_set_loaded(struct bpf_prog *prog, bool loaded)
>  {
>  	struct nsim_bpf_bound_prog *state;
>  
> -	if (!prog || !prog->aux->offload)
> +	if (!prog || !bpf_prog_is_offloaded(prog->aux))
>  		return;
>  
>  	state = prog->aux->offload->dev_priv;
> @@ -311,7 +311,7 @@ nsim_setup_prog_hw_checks(struct netdevsim *ns, struct netdev_bpf *bpf)
>  	if (!bpf->prog)
>  		return 0;
>  
> -	if (!bpf->prog->aux->offload) {
> +	if (!bpf_prog_is_offloaded(bpf->prog->aux)) {
>  		NSIM_EA(bpf->extack, "xdpoffload of non-bound program");
>  		return -EINVAL;
>  	}
> -- 
> 2.42.0.869.gea05f2083d-goog
> 

Forgot to CC netdev of these..

Re: [PATCH bpf-next 1/2] netdevsim: don't accept device bound programs

[Jakub Kicinski]

On Mon, 13 Nov 2023 20:54:52 -0800 Stanislav Fomichev wrote:
> Commit 2b3486bc2d23 ("bpf: Introduce device-bound XDP programs")
> introduced device-bound programs by largely reusing existing
> offloading infrastructure. This changed the semantics of
> 'prog->aux->offload' a bit. Now, it's non-null for both
> offloaded and device-bound programs.
> 
> Instead of looking at 'prog->aux->offload' let's call
> bpf_prog_is_offloaded which should be true iff the program
> is offloaded and not merely device-bound.

Reviewed-by: Jakub Kicinski <kuba@kernel.org>

[PATCH bpf-next 2/2] bpf: bring back removal of dev-bound id from idr

[Stanislav Fomichev]

Commit ef01f4e25c17 ("bpf: restore the ebpf program ID for BPF_AUDIT_UNLOAD
and PERF_BPF_EVENT_PROG_UNLOAD") stopped removing program's id from
idr when the offloaded/bound netdev goes away. I was supposed to
take a look and check in [0], but apparently I did not.

The purpose of idr removal is to avoid BPF_PROG_GET_NEXT_ID returning
stale ids for the programs that have a dead netdev. This functionality
is verified by test_offload.py, but we don't run this test in the CI.

Introduce new bpf_prog_remove_from_idr which takes care of correctly
dealing with potential double idr_remove() via separate skip_idr_remove
flag in the aux.

Verified by running the test manually:
test_offload.py: OK

0: https://lore.kernel.org/all/CAKH8qBtyR20ZWAc11z1-6pGb3Hd47AQUTbE_cfoktG59TqaJ7Q@mail.gmail.com/

Fixes: ef01f4e25c17 ("bpf: restore the ebpf program ID for BPF_AUDIT_UNLOAD and PERF_BPF_EVENT_PROG_UNLOAD")
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
 include/linux/bpf.h  |  2 ++
 kernel/bpf/offload.c |  3 +++
 kernel/bpf/syscall.c | 15 +++++++++++----
 3 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 4001d11be151..d2aa4b59bf1e 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1414,6 +1414,7 @@ struct bpf_prog_aux {
 	bool xdp_has_frags;
 	bool exception_cb;
 	bool exception_boundary;
+	bool skip_idr_remove;
 	/* BTF_KIND_FUNC_PROTO for valid attach_btf_id */
 	const struct btf_type *attach_func_proto;
 	/* function name for valid attach_btf_id */
@@ -2049,6 +2050,7 @@ void bpf_prog_inc(struct bpf_prog *prog);
 struct bpf_prog * __must_check bpf_prog_inc_not_zero(struct bpf_prog *prog);
 void bpf_prog_put(struct bpf_prog *prog);
 
+void bpf_prog_remove_from_idr(struct bpf_prog *prog);
 void bpf_prog_free_id(struct bpf_prog *prog);
 void bpf_map_free_id(struct bpf_map *map);
 
diff --git a/kernel/bpf/offload.c b/kernel/bpf/offload.c
index 1a4fec330eaa..6f4fe492ee2a 100644
--- a/kernel/bpf/offload.c
+++ b/kernel/bpf/offload.c
@@ -112,6 +112,9 @@ static void __bpf_prog_offload_destroy(struct bpf_prog *prog)
 	if (offload->dev_state)
 		offload->offdev->ops->destroy(prog);
 
+	/* Make sure BPF_PROG_GET_NEXT_ID can't find this dead program */
+	bpf_prog_remove_from_idr(prog);
+
 	list_del_init(&offload->offloads);
 	kfree(offload);
 	prog->aux->offload = NULL;
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 0ed286b8a0f0..bc813e03e2cf 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -2083,10 +2083,19 @@ static int bpf_prog_alloc_id(struct bpf_prog *prog)
 	return id > 0 ? 0 : id;
 }
 
-void bpf_prog_free_id(struct bpf_prog *prog)
+void bpf_prog_remove_from_idr(struct bpf_prog *prog)
 {
 	unsigned long flags;
 
+	spin_lock_irqsave(&prog_idr_lock, flags);
+	if (!prog->aux->skip_idr_remove)
+		idr_remove(&prog_idr, prog->aux->id);
+	prog->aux->skip_idr_remove = 1;
+	spin_unlock_irqrestore(&prog_idr_lock, flags);
+}
+
+void bpf_prog_free_id(struct bpf_prog *prog)
+{
 	/* cBPF to eBPF migrations are currently not in the idr store.
 	 * Offloaded programs are removed from the store when their device
 	 * disappears - even if someone grabs an fd to them they are unusable,
@@ -2095,10 +2104,8 @@ void bpf_prog_free_id(struct bpf_prog *prog)
 	if (!prog->aux->id)
 		return;
 
-	spin_lock_irqsave(&prog_idr_lock, flags);
-	idr_remove(&prog_idr, prog->aux->id);
+	bpf_prog_remove_from_idr(prog);
 	prog->aux->id = 0;
-	spin_unlock_irqrestore(&prog_idr_lock, flags);
 }
 
 static void __bpf_prog_put_rcu(struct rcu_head *rcu)
-- 
2.42.0.869.gea05f2083d-goog

Re: [PATCH bpf-next 2/2] bpf: bring back removal of dev-bound id from idr

[Stanislav Fomichev]

On 11/13, Stanislav Fomichev wrote:
> Commit ef01f4e25c17 ("bpf: restore the ebpf program ID for BPF_AUDIT_UNLOAD
> and PERF_BPF_EVENT_PROG_UNLOAD") stopped removing program's id from
> idr when the offloaded/bound netdev goes away. I was supposed to
> take a look and check in [0], but apparently I did not.
> 
> The purpose of idr removal is to avoid BPF_PROG_GET_NEXT_ID returning
> stale ids for the programs that have a dead netdev. This functionality
> is verified by test_offload.py, but we don't run this test in the CI.
> 
> Introduce new bpf_prog_remove_from_idr which takes care of correctly
> dealing with potential double idr_remove() via separate skip_idr_remove
> flag in the aux.
> 
> Verified by running the test manually:
> test_offload.py: OK
> 
> 0: https://lore.kernel.org/all/CAKH8qBtyR20ZWAc11z1-6pGb3Hd47AQUTbE_cfoktG59TqaJ7Q@mail.gmail.com/
> 
> Fixes: ef01f4e25c17 ("bpf: restore the ebpf program ID for BPF_AUDIT_UNLOAD and PERF_BPF_EVENT_PROG_UNLOAD")
> Signed-off-by: Stanislav Fomichev <sdf@google.com>
> ---
>  include/linux/bpf.h  |  2 ++
>  kernel/bpf/offload.c |  3 +++
>  kernel/bpf/syscall.c | 15 +++++++++++----
>  3 files changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 4001d11be151..d2aa4b59bf1e 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -1414,6 +1414,7 @@ struct bpf_prog_aux {
>  	bool xdp_has_frags;
>  	bool exception_cb;
>  	bool exception_boundary;
> +	bool skip_idr_remove;
>  	/* BTF_KIND_FUNC_PROTO for valid attach_btf_id */
>  	const struct btf_type *attach_func_proto;
>  	/* function name for valid attach_btf_id */
> @@ -2049,6 +2050,7 @@ void bpf_prog_inc(struct bpf_prog *prog);
>  struct bpf_prog * __must_check bpf_prog_inc_not_zero(struct bpf_prog *prog);
>  void bpf_prog_put(struct bpf_prog *prog);
>  
> +void bpf_prog_remove_from_idr(struct bpf_prog *prog);
>  void bpf_prog_free_id(struct bpf_prog *prog);
>  void bpf_map_free_id(struct bpf_map *map);
>  
> diff --git a/kernel/bpf/offload.c b/kernel/bpf/offload.c
> index 1a4fec330eaa..6f4fe492ee2a 100644
> --- a/kernel/bpf/offload.c
> +++ b/kernel/bpf/offload.c
> @@ -112,6 +112,9 @@ static void __bpf_prog_offload_destroy(struct bpf_prog *prog)
>  	if (offload->dev_state)
>  		offload->offdev->ops->destroy(prog);
>  
> +	/* Make sure BPF_PROG_GET_NEXT_ID can't find this dead program */
> +	bpf_prog_remove_from_idr(prog);
> +
>  	list_del_init(&offload->offloads);
>  	kfree(offload);
>  	prog->aux->offload = NULL;
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 0ed286b8a0f0..bc813e03e2cf 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -2083,10 +2083,19 @@ static int bpf_prog_alloc_id(struct bpf_prog *prog)
>  	return id > 0 ? 0 : id;
>  }
>  
> -void bpf_prog_free_id(struct bpf_prog *prog)
> +void bpf_prog_remove_from_idr(struct bpf_prog *prog)
>  {
>  	unsigned long flags;
>  
> +	spin_lock_irqsave(&prog_idr_lock, flags);
> +	if (!prog->aux->skip_idr_remove)
> +		idr_remove(&prog_idr, prog->aux->id);
> +	prog->aux->skip_idr_remove = 1;
> +	spin_unlock_irqrestore(&prog_idr_lock, flags);
> +}
> +
> +void bpf_prog_free_id(struct bpf_prog *prog)
> +{
>  	/* cBPF to eBPF migrations are currently not in the idr store.
>  	 * Offloaded programs are removed from the store when their device
>  	 * disappears - even if someone grabs an fd to them they are unusable,
> @@ -2095,10 +2104,8 @@ void bpf_prog_free_id(struct bpf_prog *prog)
>  	if (!prog->aux->id)
>  		return;
>  
> -	spin_lock_irqsave(&prog_idr_lock, flags);
> -	idr_remove(&prog_idr, prog->aux->id);
> +	bpf_prog_remove_from_idr(prog);
>  	prog->aux->id = 0;
> -	spin_unlock_irqrestore(&prog_idr_lock, flags);
>  }
>  
>  static void __bpf_prog_put_rcu(struct rcu_head *rcu)
> -- 
> 2.42.0.869.gea05f2083d-goog
> 

Same here, should have been CC'ed to netdev.

Re: [PATCH bpf-next 2/2] bpf: bring back removal of dev-bound id from idr

[Martin KaFai Lau]

On 11/13/23 8:54 PM, Stanislav Fomichev wrote:
> Commit ef01f4e25c17 ("bpf: restore the ebpf program ID for BPF_AUDIT_UNLOAD
> and PERF_BPF_EVENT_PROG_UNLOAD") stopped removing program's id from
> idr when the offloaded/bound netdev goes away. I was supposed to
> take a look and check in [0], but apparently I did not.
> 
> The purpose of idr removal is to avoid BPF_PROG_GET_NEXT_ID returning
> stale ids for the programs that have a dead netdev. This functionality

What may be wrong if BPF_PROG_GET_NEXT_ID returns the id?
e.g. If the prog is pinned somewhere, it may be useful to know a prog is still 
loaded in the system.

Does the fixes mean to be for the bpf tree instead?

> is verified by test_offload.py, but we don't run this test in the CI.
> 
> Introduce new bpf_prog_remove_from_idr which takes care of correctly
> dealing with potential double idr_remove() via separate skip_idr_remove
> flag in the aux.
> 
> Verified by running the test manually:
> test_offload.py: OK
> 
> 0: https://lore.kernel.org/all/CAKH8qBtyR20ZWAc11z1-6pGb3Hd47AQUTbE_cfoktG59TqaJ7Q@mail.gmail.com/
> 
> Fixes: ef01f4e25c17 ("bpf: restore the ebpf program ID for BPF_AUDIT_UNLOAD and PERF_BPF_EVENT_PROG_UNLOAD")
> Signed-off-by: Stanislav Fomichev <sdf@google.com>
> ---
>   include/linux/bpf.h  |  2 ++
>   kernel/bpf/offload.c |  3 +++
>   kernel/bpf/syscall.c | 15 +++++++++++----
>   3 files changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 4001d11be151..d2aa4b59bf1e 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -1414,6 +1414,7 @@ struct bpf_prog_aux {
>   	bool xdp_has_frags;
>   	bool exception_cb;
>   	bool exception_boundary;
> +	bool skip_idr_remove;
>   	/* BTF_KIND_FUNC_PROTO for valid attach_btf_id */
>   	const struct btf_type *attach_func_proto;
>   	/* function name for valid attach_btf_id */
> @@ -2049,6 +2050,7 @@ void bpf_prog_inc(struct bpf_prog *prog);
>   struct bpf_prog * __must_check bpf_prog_inc_not_zero(struct bpf_prog *prog);
>   void bpf_prog_put(struct bpf_prog *prog);
>   
> +void bpf_prog_remove_from_idr(struct bpf_prog *prog);
>   void bpf_prog_free_id(struct bpf_prog *prog);
>   void bpf_map_free_id(struct bpf_map *map);
>   

Re: [PATCH bpf-next 2/2] bpf: bring back removal of dev-bound id from idr

[Daniel Borkmann]

On 11/21/23 10:03 PM, Martin KaFai Lau wrote:
> On 11/13/23 8:54 PM, Stanislav Fomichev wrote:
>> Commit ef01f4e25c17 ("bpf: restore the ebpf program ID for BPF_AUDIT_UNLOAD
>> and PERF_BPF_EVENT_PROG_UNLOAD") stopped removing program's id from
>> idr when the offloaded/bound netdev goes away. I was supposed to
>> take a look and check in [0], but apparently I did not.
>>
>> The purpose of idr removal is to avoid BPF_PROG_GET_NEXT_ID returning
>> stale ids for the programs that have a dead netdev. This functionality
> 
> What may be wrong if BPF_PROG_GET_NEXT_ID returns the id?
> e.g. If the prog is pinned somewhere, it may be useful to know a prog is still loaded in the system.

Wouldn't this strictly speaking provide an invalid id (== 0) upon unload
back to audit - see the bpf_audit_prog(prog, BPF_AUDIT_UNLOAD) call location?

> Does the fixes mean to be for the bpf tree instead?

+1 given syzbot report, I'll take the first one in for now.

>> is verified by test_offload.py, but we don't run this test in the CI.
>>
>> Introduce new bpf_prog_remove_from_idr which takes care of correctly
>> dealing with potential double idr_remove() via separate skip_idr_remove
>> flag in the aux.
>>
>> Verified by running the test manually:
>> test_offload.py: OK
>>
>> 0: https://lore.kernel.org/all/CAKH8qBtyR20ZWAc11z1-6pGb3Hd47AQUTbE_cfoktG59TqaJ7Q@mail.gmail.com/
>>
>> Fixes: ef01f4e25c17 ("bpf: restore the ebpf program ID for BPF_AUDIT_UNLOAD and PERF_BPF_EVENT_PROG_UNLOAD")
>> Signed-off-by: Stanislav Fomichev <sdf@google.com>
>> ---
>>   include/linux/bpf.h  |  2 ++
>>   kernel/bpf/offload.c |  3 +++
>>   kernel/bpf/syscall.c | 15 +++++++++++----
>>   3 files changed, 16 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
>> index 4001d11be151..d2aa4b59bf1e 100644
>> --- a/include/linux/bpf.h
>> +++ b/include/linux/bpf.h
>> @@ -1414,6 +1414,7 @@ struct bpf_prog_aux {
>>       bool xdp_has_frags;
>>       bool exception_cb;
>>       bool exception_boundary;
>> +    bool skip_idr_remove;
>>       /* BTF_KIND_FUNC_PROTO for valid attach_btf_id */
>>       const struct btf_type *attach_func_proto;
>>       /* function name for valid attach_btf_id */
>> @@ -2049,6 +2050,7 @@ void bpf_prog_inc(struct bpf_prog *prog);
>>   struct bpf_prog * __must_check bpf_prog_inc_not_zero(struct bpf_prog *prog);
>>   void bpf_prog_put(struct bpf_prog *prog);
>> +void bpf_prog_remove_from_idr(struct bpf_prog *prog);
>>   void bpf_prog_free_id(struct bpf_prog *prog);
>>   void bpf_map_free_id(struct bpf_map *map);
> 
> 

Re: [PATCH bpf-next 2/2] bpf: bring back removal of dev-bound id from idr

[Stanislav Fomichev]

On 11/22, Daniel Borkmann wrote:
> On 11/21/23 10:03 PM, Martin KaFai Lau wrote:
> > On 11/13/23 8:54 PM, Stanislav Fomichev wrote:
> > > Commit ef01f4e25c17 ("bpf: restore the ebpf program ID for BPF_AUDIT_UNLOAD
> > > and PERF_BPF_EVENT_PROG_UNLOAD") stopped removing program's id from
> > > idr when the offloaded/bound netdev goes away. I was supposed to
> > > take a look and check in [0], but apparently I did not.
> > > 
> > > The purpose of idr removal is to avoid BPF_PROG_GET_NEXT_ID returning
> > > stale ids for the programs that have a dead netdev. This functionality
> > 
> > What may be wrong if BPF_PROG_GET_NEXT_ID returns the id?
> > e.g. If the prog is pinned somewhere, it may be useful to know a prog is still loaded in the system.

bpftool is a bit spooked by those prog ids currently: calling GET_INFO_BY_ID
on those programs returns ENODEV. So we can keep those ids around, but
need some tweaks on the bpftool in this case. LMK if any of you prefer
this option.

> Wouldn't this strictly speaking provide an invalid id (== 0) upon unload
> back to audit - see the bpf_audit_prog(prog, BPF_AUDIT_UNLOAD) call location?

Removing from idr shouldn't affect bpf_audit_prog, right? bpf_audit_prog
is using prog->aux->id for its purposes, so as long as we are not resetting
this value - we're good.

Re: [PATCH bpf-next 2/2] bpf: bring back removal of dev-bound id from idr

[Martin KaFai Lau]

On 11/22/23 10:05 AM, Stanislav Fomichev wrote:
>>>> Commit ef01f4e25c17 ("bpf: restore the ebpf program ID for BPF_AUDIT_UNLOAD
>>>> and PERF_BPF_EVENT_PROG_UNLOAD") stopped removing program's id from
>>>> idr when the offloaded/bound netdev goes away. I was supposed to
>>>> take a look and check in [0], but apparently I did not.
>>>>
>>>> The purpose of idr removal is to avoid BPF_PROG_GET_NEXT_ID returning
>>>> stale ids for the programs that have a dead netdev. This functionality
>>>
>>> What may be wrong if BPF_PROG_GET_NEXT_ID returns the id?
>>> e.g. If the prog is pinned somewhere, it may be useful to know a prog is still loaded in the system.
> 
> bpftool is a bit spooked by those prog ids currently: calling GET_INFO_BY_ID
> on those programs returns ENODEV. So we can keep those ids around, but
> need some tweaks on the bpftool in this case. LMK if any of you prefer
> this option.

I think it is in general useful to improve 'bpftool prog show' to keep going for 
the next prog id if possible. May be print an error message after the prog id 
and then keep going for the next prog id?

Re: [PATCH bpf-next 2/2] bpf: bring back removal of dev-bound id from idr

[Stanislav Fomichev]

On Wed, Nov 22, 2023 at 10:40 AM Martin KaFai Lau <martin.lau@linux.dev> wrote:
>
> On 11/22/23 10:05 AM, Stanislav Fomichev wrote:
> >>>> Commit ef01f4e25c17 ("bpf: restore the ebpf program ID for BPF_AUDIT_UNLOAD
> >>>> and PERF_BPF_EVENT_PROG_UNLOAD") stopped removing program's id from
> >>>> idr when the offloaded/bound netdev goes away. I was supposed to
> >>>> take a look and check in [0], but apparently I did not.
> >>>>
> >>>> The purpose of idr removal is to avoid BPF_PROG_GET_NEXT_ID returning
> >>>> stale ids for the programs that have a dead netdev. This functionality
> >>>
> >>> What may be wrong if BPF_PROG_GET_NEXT_ID returns the id?
> >>> e.g. If the prog is pinned somewhere, it may be useful to know a prog is still loaded in the system.
> >
> > bpftool is a bit spooked by those prog ids currently: calling GET_INFO_BY_ID
> > on those programs returns ENODEV. So we can keep those ids around, but
> > need some tweaks on the bpftool in this case. LMK if any of you prefer
> > this option.
>
> I think it is in general useful to improve 'bpftool prog show' to keep going for
> the next prog id if possible. May be print an error message after the prog id
> and then keep going for the next prog id?

Replied with a v2 where I mark those progs as 'orphaned'!

Re: [PATCH bpf-next 2/2] bpf: bring back removal of dev-bound id from idr

[Daniel Borkmann]

On 11/22/23 11:41 PM, Stanislav Fomichev wrote:
> On Wed, Nov 22, 2023 at 10:40 AM Martin KaFai Lau <martin.lau@linux.dev> wrote:
>> On 11/22/23 10:05 AM, Stanislav Fomichev wrote:
>>>>>> Commit ef01f4e25c17 ("bpf: restore the ebpf program ID for BPF_AUDIT_UNLOAD
>>>>>> and PERF_BPF_EVENT_PROG_UNLOAD") stopped removing program's id from
>>>>>> idr when the offloaded/bound netdev goes away. I was supposed to
>>>>>> take a look and check in [0], but apparently I did not.
>>>>>>
>>>>>> The purpose of idr removal is to avoid BPF_PROG_GET_NEXT_ID returning
>>>>>> stale ids for the programs that have a dead netdev. This functionality
>>>>>
>>>>> What may be wrong if BPF_PROG_GET_NEXT_ID returns the id?
>>>>> e.g. If the prog is pinned somewhere, it may be useful to know a prog is still loaded in the system.
>>>
>>> bpftool is a bit spooked by those prog ids currently: calling GET_INFO_BY_ID
>>> on those programs returns ENODEV. So we can keep those ids around, but
>>> need some tweaks on the bpftool in this case. LMK if any of you prefer
>>> this option.
>>
>> I think it is in general useful to improve 'bpftool prog show' to keep going for
>> the next prog id if possible. May be print an error message after the prog id
>> and then keep going for the next prog id?
> 
> Replied with a v2 where I mark those progs as 'orphaned'!

Sg, we could perhaps do something similar for netdev detached links.