Re: [PATCH v6 bpf-next 6/9] bpf: Add BTF_KIND_FLOAT support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 2/25/21 2:40 AM, Ilya Leoshkevich wrote:
On Wed, 2021-02-24 at 23:10 -0800, Yonghong Song wrote:
On 2/24/21 3:45 PM, Ilya Leoshkevich wrote:
On the kernel side, introduce a new btf_kind_operations. It is
similar to that of BTF_KIND_INT, however, it does not need to
handle encodings and bit offsets. Do not implement printing, since
the kernel does not know how to format floating-point values.

Signed-off-by: Ilya Leoshkevich <iii@xxxxxxxxxxxxx>
---
   kernel/bpf/btf.c | 79
++++++++++++++++++++++++++++++++++++++++++++++--
   1 file changed, 77 insertions(+), 2 deletions(-)

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 2efeb5f4b343..c405edc8e615 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c

[...]

@@ -1849,7 +1852,7 @@ static int btf_df_check_kflag_member(struct
btf_verifier_env *env,
         return -EINVAL;
   }
-/* Used for ptr, array and struct/union type members.
+/* Used for ptr, array struct/union and float type members.
    * int, enum and modifier types have their specific callback
functions.
    */
   static int btf_generic_check_kflag_member(struct btf_verifier_env
*env,
@@ -3675,6 +3678,77 @@ static const struct btf_kind_operations
datasec_ops = {
         .show                   = btf_datasec_show,
   };
+static s32 btf_float_check_meta(struct btf_verifier_env *env,
+                               const struct btf_type *t,
+                               u32 meta_left)
+{
+       if (btf_type_vlen(t)) {
+               btf_verifier_log_type(env, t, "vlen != 0");
+               return -EINVAL;
+       }
+
+       if (btf_type_kflag(t)) {
+               btf_verifier_log_type(env, t, "Invalid btf_info
kind_flag");
+               return -EINVAL;
+       }
+
+       if (t->size != 2 && t->size != 4 && t->size != 8 && t->size
!= 12 &&
+           t->size != 16) {
+               btf_verifier_log_type(env, t, "Invalid type_size");
+               return -EINVAL;
+       }
+
+       btf_verifier_log_type(env, t, NULL);
+
+       return 0;
+}
+
+static int btf_float_check_member(struct btf_verifier_env *env,
+                                 const struct btf_type
*struct_type,
+                                 const struct btf_member *member,
+                                 const struct btf_type
*member_type)
+{
+       u64 start_offset_bytes;
+       u64 end_offset_bytes;
+       u64 misalign_bits;
+       u64 align_bytes;
+       u64 align_bits;
+
+       align_bytes = min_t(u64, sizeof(void *), member_type-
size);

I listed the following possible (size, align) pairs:
      size     x86_32 align_bytes   x86_64 align bytes
       2        2                    2
       4        4                    4
       8        4                    8
       12       4                    8
       16       4                    8

A few observations.
    1. I don't know, just want to confirm, for double, the alignment
could be 4 (for a member) on 32bit system, is that right?
    2. for size 12, alignment will be 8 for x86_64 system, this is
strange, not sure whether it is true or not. Or size 12 cannot be
on x86_64 and we should error out if sizeof(void *) is 8.

1 - Yes.

2 - On x86_64 long double is 16 bytes and the required alignment is 16
bytes too. However, on other architectures all this might be different.
For example, for us long double is 16 bytes too, but the alignment can
be 8. So can we be somewhat lax here and just allow smaller alignments,
instead of trying to figure out what exactly each supported
architecture does?

Maybe this is fine. I think, "float" is also the first BTF type whose
validation may have a dependence on underlying architecture. For example, member offset 4, type size 8, will be okay on x86_32,
but not okay on x84_64. That means BTF cannot be independently
validated without considering underlying architecture.

I am not against this patch. Maybe float is special. Maybe it is
okay since float is rarely used. I would like to see other people's
opinion.


[...]





[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux