Hi,
On 9/28/2022 5:23 PM, Quentin Monnet wrote:
> Wed Sep 28 2022 10:05:55 GMT+0100 (British Summer Time) ~ Hou Tao
> <houtao@xxxxxxxxxxxxxxx>
>> Hi,
>>
>> On 9/28/2022 4:40 PM, Quentin Monnet wrote:
>>> Wed Sep 28 2022 05:14:45 GMT+0100 (British Summer Time) ~ Hou Tao
>>> <houtao@xxxxxxxxxxxxxxx>
>>>> 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".
>> SNIP
>>>>> 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'm not sure I follow. I don't understand the difference for dealing
>>> internally with the key between "key_size N key BYTES" and "key N BYTES"
>>> (or for parsing then counting). Please could you give an example telling
>>> how you would you express the key from the structure above, with the
>>> syntax you proposed?
>> In my understand, if using "key_size N key BYTES" to represent map_key, it can
>> not tell the exact size of "name" and "addr" and it only can tell the total size
>> of name and addr. If using "key BYTES" to do that, it has the similar problem.
>> But if using "key size BYTES" format, map_key can be expressed as follows:
>>
>> key c c c c [name_size] n n n [addr_size] a a f f f f
> OK thanks I get it now, you can have multiple sizes within the key, one
> for each field. Yes, let's use a new keyword in that case please. Can
> you also provide more details in the man page, and ideally add a new
> example to the list?
Forget to mention that the map key with embedded dynptr is not supported yet and
now only support using dynptr as the map key. So will add a new keyword "dynkey"
in v3 to support operations on qp-trie.
>
> Thanks,
> Quentin
>
> .
