Re: [PATCH bpf-next 2/3] bpf: Add size arg to build_id_parse function

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

 





On 1/26/21 12:52 PM, Jiri Olsa wrote:
On Thu, Jan 14, 2021 at 07:47:20PM -0800, Alexei Starovoitov wrote:
On Thu, Jan 14, 2021 at 3:44 PM Yonghong Song <yhs@xxxxxx> wrote:



On 1/14/21 2:02 PM, Jiri Olsa wrote:
On Thu, Jan 14, 2021 at 01:05:33PM -0800, Yonghong Song wrote:


On 1/14/21 12:01 PM, Jiri Olsa wrote:
On Thu, Jan 14, 2021 at 10:56:33AM -0800, Yonghong Song wrote:


On 1/14/21 5:40 AM, Jiri Olsa wrote:
It's possible to have other build id types (other than default SHA1).
Currently there's also ld support for MD5 build id.

Currently, bpf build_id based stackmap does not returns the size of
the build_id. Did you see an issue here? I guess user space can check
the length of non-zero bits of the build id to decide what kind of
type it is, right?

you can have zero bytes in the build id hash, so you need to get the size

I never saw MD5 being used in practise just SHA1, but we added the
size to be complete and make sure we'll fit with build id, because
there's only limited space in mmap2 event

I am asking to check whether we should extend uapi struct
bpf_stack_build_id to include build_id_size as well. I guess
we can delay this until a real use case.

right, we can try make some MD5 build id binaries and check if it
explodes with some bcc tools, but I don't expect that.. I'll try
to find some time for that

Thanks. We may have issues on bcc side. For build_id collected in
kernel, bcc always generates a length-20 string. But for user
binaries, the build_id string length is equal to actual size of
the build_id. They may not match (MD5 length is 16).
The fix is probably to append '0's (up to length 20) for user
binary build_id's.

I guess MD5 is very seldom used. I will wait if you can reproduce
the issue and then we might fix it.

Indeed.
Jiri, please check whether md5 is really an issue.
Sounds like we have to do something on the kernel side.
Hopefully zero padding will be enough.
I would prefer to avoid extending uapi struct to cover rare case.

build_id_parse is already doing the zero padding, so we are ok

I tried several bcc tools over perf bench with md5 buildid and
the results looked ok

Great. Thanks for confirmation!


jirka




[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