Hi,
On 9/27/2022 7:24 PM, Quentin Monnet wrote:
> Sat Sep 24 2022 14:36:15 GMT+0100 (British Summer Time) ~ Hou Tao
> <houtao@xxxxxxxxxxxxxxx>
>> From: Hou Tao <houtao1@xxxxxxxxxx>
>>
>> Support lookup/update/delete/iterate/dump operations for qp-trie in
>> bpftool. Mainly add two functions: one function to parse dynptr key and
>> another one to dump dynptr key. The input format of dynptr key is:
>> "key [hex] size BYTES" and the output format of dynptr key is:
>> "size BYTES".
>>
>> The following is the output when using bpftool to manipulate
>> qp-trie:
>>
>> $ bpftool map pin id 724953 /sys/fs/bpf/qp
>> $ bpftool map show pinned /sys/fs/bpf/qp
>> 724953: qp_trie name qp_trie flags 0x1
>> key 16B value 4B max_entries 2 memlock 65536B map_extra 8
>> btf_id 779
>> pids test_qp_trie.bi(109167)
>> $ bpftool map dump pinned /sys/fs/bpf/qp
>> [{
>> "key": {
>> "size": 4,
>> "data": ["0x0","0x0","0x0","0x0"
>> ]
>> },
>> "value": 0
>> },{
>> "key": {
>> "size": 4,
>> "data": ["0x0","0x0","0x0","0x1"
>> ]
>> },
>> "value": 2
>> }
>> ]
>> $ bpftool map lookup pinned /sys/fs/bpf/qp key 4 0 0 0 1
>> {
>> "key": {
>> "size": 4,
>> "data": ["0x0","0x0","0x0","0x1"
>> ]
>> },
>> "value": 2
>> }
> The bpftool patch looks good, thanks! I have one comment on the syntax
> for the keys, I don't find it intuitive to have the size as the first
> BYTE. It makes it awkward to understand what the command does if we read
> it in the wild without knowing the map type. I can see two alternatives,
> either adding a keyword (e.g., "key_size 4 key 0 0 0 1"), or changing
> parse_bytes() to make it able to parse as much as it can then count the
> bytes, when we don't know in advance how many we get.
The suggestion is reasonable, but there is also reason for the current choice (
I should written it down in commit message). For dynptr-typed key, these two
proposed suggestions will work. But for key with embedded dynptrs as show below,
both explict key_size keyword and implicit key_size in BYTEs can not express the
key correctly.
struct map_key {
unsigned int cookie;
struct bpf_dynptr name;
struct bpf_dynptr addr;
unsigned int flags;
};
I also had thought about adding another key word "dynptr_key" (or "dyn_key") to
support dynptr-typed key or key with embedded dynptr, and the format will still
be: "dynptr_key size [BYTES]". But at least we can tell it is different with
"key" which is fixed size. What do you think ?
>
> Thanks,
> Quentin
>
> .
