On 08/09/2018 05:55 AM, Yonghong Song wrote:
> On 8/8/18 7:25 PM, Alexei Starovoitov wrote:
>> On Wed, Aug 08, 2018 at 06:25:19PM -0700, Yonghong Song wrote:
>>> In function map_seq_next() of kernel/bpf/inode.c,
>>> the first key will be the "0" regardless of the map type.
>>> This works for array. But for hash type, if it happens
>>> key "0" is in the map, the bpffs map show will miss
>>> some items if the key "0" is not the first element of
>>> the first bucket.
>>>
>>> This patch fixed the issue by guaranteeing to get
>>> the first element, if the seq_show is just started,
>>> by passing NULL pointer key to map_get_next_key() callback.
>>> This way, no missing elements will occur for
>>> bpffs hash table show even if key "0" is in the map.
> 
> Currently, map_seq_show_elem callback is only implemented
> for arraymap. So the problem actually is not exposed.
> 
> The issue is discovered when I tried to implement
> map_seq_show_elem for hash maps, and I will have followup
> patches for it.
> 
> So this patch probably should apply to bpf-next or
> I can include this patch in my later patch set
> which implements map_seq_show_elem for hash map
> which can demonstrate the problem.
> 
> Please let me know.
> 
>>> Fixes: a26ca7c982cb5 ("bpf: btf: Add pretty print support to the basic 
>>> arraymap")
>>> Signed-off-by: Yonghong Song <y...@fb.com>
>>
>> Acked-by: Alexei Starovoitov <a...@kernel.org>

Given this doesn't affect any current code, I think bpf-next
would be fine.

Anyway, this cannot be used as-is, results in following compile
warning ...

# make -j4 kernel/bpf/
  DESCEND  objtool
  CALL    scripts/checksyscalls.sh
  CC      kernel/bpf/verifier.o
  CC      kernel/bpf/inode.o
kernel/bpf/inode.c: In function ‘map_seq_next’:
kernel/bpf/inode.c:214:1: warning: label ‘done’ defined but not used 
[-Wunused-label]
 done:
 ^~~~
  AR      kernel/bpf/built-in.a

Reply via email to