[bpf-next,4/6] bpf: Move the declaration of __bpf_obj_drop_impl() to internal.h

Commit Message

Hou Tao Oct. 7, 2023, 1:51 p.m. UTC

From: Hou Tao <[e-mail]>

both syscall.c and helpers.c have the declaration of
__bpf_obj_drop_impl(), so just move it to a common header file.

Signed-off-by: Hou Tao <[e-mail]>
---
 kernel/bpf/helpers.c  |  3 +--
 kernel/bpf/internal.h | 11 +++++++++++
 kernel/bpf/syscall.c  |  4 ++--
 3 files changed, 14 insertions(+), 4 deletions(-)
 create mode 100644 kernel/bpf/internal.h

Comments

Stanislav Fomichev Oct. 9, 2023, 4:28 p.m. UTC | #1

On 10/07, Hou Tao wrote:
> From: Hou Tao <[e-mail]>
> 
> both syscall.c and helpers.c have the declaration of
> __bpf_obj_drop_impl(), so just move it to a common header file.
> 
> Signed-off-by: Hou Tao <[e-mail]>
> ---
>  kernel/bpf/helpers.c  |  3 +--
>  kernel/bpf/internal.h | 11 +++++++++++
>  kernel/bpf/syscall.c  |  4 ++--
>  3 files changed, 14 insertions(+), 4 deletions(-)
>  create mode 100644 kernel/bpf/internal.h
> 
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index dd1c69ee3375..07f49f8831c0 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -24,6 +24,7 @@
>  #include <linux/bpf_mem_alloc.h>
>  #include <linux/kasan.h>
>  
> +#include "internal.h"
>  #include "../../lib/kstrtox.h"
>  
>  /* If kernel subsystem is allowing eBPF programs to call this function,
> @@ -1808,8 +1809,6 @@ bpf_base_func_proto(enum bpf_func_id func_id)
>  	}
>  }
>  
> -void __bpf_obj_drop_impl(void *p, const struct btf_record *rec);
> -
>  void bpf_list_head_free(const struct btf_field *field, void *list_head,
>  			struct bpf_spin_lock *spin_lock)
>  {
> diff --git a/kernel/bpf/internal.h b/kernel/bpf/internal.h
> new file mode 100644
> index 000000000000..e233ea83eb0a
> --- /dev/null
> +++ b/kernel/bpf/internal.h
> @@ -0,0 +1,11 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/* Copyright (C) 2023. Huawei Technologies Co., Ltd
> + */

Don't think copyright works this way? You can't move the code and
claim authorship.

In general, git tracks authors and contributors, so not sure
why we still keep putting these explicit notices..

Alexei Starovoitov Oct. 9, 2023, 4:56 p.m. UTC | #2

On Sat, Oct 07, 2023 at 09:51:04PM +0800, Hou Tao wrote:
> From: Hou Tao <[e-mail]>
> 
> both syscall.c and helpers.c have the declaration of
> __bpf_obj_drop_impl(), so just move it to a common header file.
> 
> Signed-off-by: Hou Tao <[e-mail]>
> ---
>  kernel/bpf/helpers.c  |  3 +--
>  kernel/bpf/internal.h | 11 +++++++++++
>  kernel/bpf/syscall.c  |  4 ++--
>  3 files changed, 14 insertions(+), 4 deletions(-)
>  create mode 100644 kernel/bpf/internal.h
> 
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index dd1c69ee3375..07f49f8831c0 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -24,6 +24,7 @@
>  #include <linux/bpf_mem_alloc.h>
>  #include <linux/kasan.h>
>  
> +#include "internal.h"

Pls use one of the existing headers. No need for new one.

Hou Tao Oct. 11, 2023, 6:31 a.m. UTC | #3

Hi,

On 10/10/2023 12:28 AM, Stanislav Fomichev wrote:
> On 10/07, Hou Tao wrote:
>> From: Hou Tao <[e-mail]>
>>
>> both syscall.c and helpers.c have the declaration of
>> __bpf_obj_drop_impl(), so just move it to a common header file.
>>
>> Signed-off-by: Hou Tao <[e-mail]>
>> ---
>>  kernel/bpf/helpers.c  |  3 +--
>>  kernel/bpf/internal.h | 11 +++++++++++
>>  kernel/bpf/syscall.c  |  4 ++--
>>  3 files changed, 14 insertions(+), 4 deletions(-)
>>  create mode 100644 kernel/bpf/internal.h
>>
>> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
>> index dd1c69ee3375..07f49f8831c0 100644
>> --- a/kernel/bpf/helpers.c
>> +++ b/kernel/bpf/helpers.c
>> @@ -24,6 +24,7 @@
>>  #include <linux/bpf_mem_alloc.h>
>>  #include <linux/kasan.h>
>>  
>> +#include "internal.h"
>>  #include "../../lib/kstrtox.h"
>>  
>>  /* If kernel subsystem is allowing eBPF programs to call this function,
>> @@ -1808,8 +1809,6 @@ bpf_base_func_proto(enum bpf_func_id func_id)
>>  	}
>>  }
>>  
>> -void __bpf_obj_drop_impl(void *p, const struct btf_record *rec);
>> -
>>  void bpf_list_head_free(const struct btf_field *field, void *list_head,
>>  			struct bpf_spin_lock *spin_lock)
>>  {
>> diff --git a/kernel/bpf/internal.h b/kernel/bpf/internal.h
>> new file mode 100644
>> index 000000000000..e233ea83eb0a
>> --- /dev/null
>> +++ b/kernel/bpf/internal.h
>> @@ -0,0 +1,11 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/* Copyright (C) 2023. Huawei Technologies Co., Ltd
>> + */
> Don't think copyright works this way? You can't move the code and
> claim authorship.
>
> In general, git tracks authors and contributors, so not sure
> why we still keep putting these explicit notices..
My bad. Thanks for the remainder. Will fix in v2.

Hou Tao Oct. 11, 2023, 6:40 a.m. UTC | #4

Hi,

On 10/10/2023 12:56 AM, Alexei Starovoitov wrote:
> On Sat, Oct 07, 2023 at 09:51:04PM +0800, Hou Tao wrote:
>> From: Hou Tao <[e-mail]>
>>
>> both syscall.c and helpers.c have the declaration of
>> __bpf_obj_drop_impl(), so just move it to a common header file.
>>
>> Signed-off-by: Hou Tao <[e-mail]>
>> ---
>>  kernel/bpf/helpers.c  |  3 +--
>>  kernel/bpf/internal.h | 11 +++++++++++
>>  kernel/bpf/syscall.c  |  4 ++--
>>  3 files changed, 14 insertions(+), 4 deletions(-)
>>  create mode 100644 kernel/bpf/internal.h
>>
>> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
>> index dd1c69ee3375..07f49f8831c0 100644
>> --- a/kernel/bpf/helpers.c
>> +++ b/kernel/bpf/helpers.c
>> @@ -24,6 +24,7 @@
>>  #include <linux/bpf_mem_alloc.h>
>>  #include <linux/kasan.h>
>>  
>> +#include "internal.h"
> Pls use one of the existing headers. No need for new one.

Do you mean one of header files in include/linux, right ? Because there
is no proper header files under kernel/bpf. The best fit is
include/linux/bpf.h, but after modify bpf.h, multiple files need to be
rebuild: (e.g. under net/, kernel, drivers/net, block/, fs/). Maybe it
is time to break bpf.h into multiple independent files ?

Patch

diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index dd1c69ee3375..07f49f8831c0 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -24,6 +24,7 @@ 
 #include <linux/bpf_mem_alloc.h>
 #include <linux/kasan.h>
 
+#include "internal.h"
 #include "../../lib/kstrtox.h"
 
 /* If kernel subsystem is allowing eBPF programs to call this function,
@@ -1808,8 +1809,6 @@  bpf_base_func_proto(enum bpf_func_id func_id)
 	}
 }
 
-void __bpf_obj_drop_impl(void *p, const struct btf_record *rec);
-
 void bpf_list_head_free(const struct btf_field *field, void *list_head,
 			struct bpf_spin_lock *spin_lock)
 {
diff --git a/kernel/bpf/internal.h b/kernel/bpf/internal.h
new file mode 100644
index 000000000000..e233ea83eb0a
--- /dev/null
+++ b/kernel/bpf/internal.h
@@ -0,0 +1,11 @@ 
+/* SPDX-License-Identifier: GPL-2.0-only */
+/* Copyright (C) 2023. Huawei Technologies Co., Ltd
+ */
+#ifndef __BPF_INTERNAL_H_
+#define __BPF_INTERNAL_H_
+
+struct btf_record;
+
+void __bpf_obj_drop_impl(void *p, const struct btf_record *rec);
+
+#endif /* __BPF_INTERNAL_H_ */
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 6b5280f14a53..7de4b9f97c8f 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -39,6 +39,8 @@ 
 
 #include <net/tcx.h>
 
+#include "internal.h"
+
 #define IS_FD_ARRAY(map) ((map)->map_type == BPF_MAP_TYPE_PERF_EVENT_ARRAY || \
 			  (map)->map_type == BPF_MAP_TYPE_CGROUP_ARRAY || \
 			  (map)->map_type == BPF_MAP_TYPE_ARRAY_OF_MAPS)
@@ -626,8 +628,6 @@  void bpf_obj_free_timer(const struct btf_record *rec, void *obj)
 	bpf_timer_cancel_and_free(obj + rec->timer_off);
 }
 
-extern void __bpf_obj_drop_impl(void *p, const struct btf_record *rec);
-
 void bpf_obj_free_fields(const struct btf_record *rec, void *obj)
 {
 	const struct btf_field *fields;