Wed Sep 28 2022 11:54:39 GMT+0100 (British Summer Time) ~ Hou Tao <houtao@xxxxxxxxxxxxxxx> > 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. Sounds good thank you, let's do that and ideally mention it in the commit log for context.