> On Mar 14, 2018, at 9:07 AM, Daniel Borkmann <dan...@iogearbox.net> wrote:
>
> Just a minor question below, the rest seems fine to me as far as I
> can tell.
>
> On 03/13/2018 10:47 PM, Song Liu wrote:
> [...]
>> +enum bpf_stack_build_id_status {
>> + /* user space need an empty entry to identify end of a trace */
>> + BPF_STACK_BUILD_ID_EMPTY = 0,
>> + /* with valid build_id and offset */
>> + BPF_STACK_BUILD_ID_VALID = 1,
>> + /* couldn't get build_id, fallback to ip */
>> + BPF_STACK_BUILD_ID_IP = 2,
>> +};
>> +
>> +#define BPF_BUILD_ID_SIZE 20
>> +struct bpf_stack_build_id {
>> + __s32 status;
>> + unsigned char build_id[BPF_BUILD_ID_SIZE];
>> + union {
>> + __u64 offset;
>> + __u64 ip;
>> + };
>> +};
> [...]> BPF_CALL_3(bpf_get_stackid, struct pt_regs *, regs, struct bpf_map *,
> map,
>> u64, flags)
>> {
>> struct bpf_stack_map *smap = container_of(map, struct bpf_stack_map,
>> map);
>> struct perf_callchain_entry *trace;
>> struct stack_map_bucket *bucket, *new_bucket, *old_bucket;
>> - u32 max_depth = map->value_size / 8;
>> + u32 max_depth = map->value_size / stack_map_data_size(map);
>> /* stack_map_alloc() checks that max_depth <=
>> sysctl_perf_event_max_stack */
>> u32 init_nr = sysctl_perf_event_max_stack - max_depth;
>> u32 skip = flags & BPF_F_SKIP_FIELD_MASK;
>> @@ -128,11 +318,16 @@ BPF_CALL_3(bpf_get_stackid, struct pt_regs *, regs,
>> struct bpf_map *, map,
>> bool user = flags & BPF_F_USER_STACK;
>> bool kernel = !user;
>> u64 *ips;
>> + bool hash_matches;
>>
>> if (unlikely(flags & ~(BPF_F_SKIP_FIELD_MASK | BPF_F_USER_STACK |
>> BPF_F_FAST_STACK_CMP | BPF_F_REUSE_STACKID)))
>> return -EINVAL;
>>
>> + /* build_id+offset stack map only supports user stack */
>> + if (stack_map_use_build_id(map) && !user)
>> + return -EINVAL;
>
> Instead of bailing out here, wouldn't it make sense to just reuse the
> BPF_STACK_BUILD_ID_IP status and use this 'fallback' for kernel similar
> to what we do anyway in stack_map_get_build_id_offset() when we cannot
> get the build id so that map can be used for both cases?
This a great idea! Let me implement it.
Thanks,
Song
