| Message ID | 20191211223344.165549-6-brianvv@google.com |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | add bpf batch ops to process more than 1 elem | expand |
On 12/11/19 2:33 PM, Brian Vazquez wrote: > This adds the generic batch ops functionality to bpf lpm_trie. > > Signed-off-by: Brian Vazquez <brianvv@google.com> > --- > kernel/bpf/lpm_trie.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/kernel/bpf/lpm_trie.c b/kernel/bpf/lpm_trie.c > index 56e6c75d354d9..92c47b4f03337 100644 > --- a/kernel/bpf/lpm_trie.c > +++ b/kernel/bpf/lpm_trie.c > @@ -743,4 +743,8 @@ const struct bpf_map_ops trie_map_ops = { > .map_update_elem = trie_update_elem, > .map_delete_elem = trie_delete_elem, > .map_check_btf = trie_check_btf, > + .map_lookup_batch = generic_map_lookup_batch, > + .map_lookup_and_delete_batch = generic_map_lookup_and_delete_batch, Not 100% sure whether trie should use generic map lookup/lookup_and_delete or not. If the key is not available, the get_next_key will return the 'leftmost' node which roughly corresponding to the first node in the hash table. > + .map_delete_batch = generic_map_delete_batch, > + .map_update_batch = generic_map_update_batch, > }; >
Hi Yonghong, thanks for reviewing it and sorry for the late reply I had been traveling. On Fri, Dec 13, 2019 at 11:46 AM Yonghong Song <yhs@fb.com> wrote: > > > > On 12/11/19 2:33 PM, Brian Vazquez wrote: > > This adds the generic batch ops functionality to bpf lpm_trie. > > > > Signed-off-by: Brian Vazquez <brianvv@google.com> > > --- > > kernel/bpf/lpm_trie.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/kernel/bpf/lpm_trie.c b/kernel/bpf/lpm_trie.c > > index 56e6c75d354d9..92c47b4f03337 100644 > > --- a/kernel/bpf/lpm_trie.c > > +++ b/kernel/bpf/lpm_trie.c > > @@ -743,4 +743,8 @@ const struct bpf_map_ops trie_map_ops = { > > .map_update_elem = trie_update_elem, > > .map_delete_elem = trie_delete_elem, > > .map_check_btf = trie_check_btf, > > + .map_lookup_batch = generic_map_lookup_batch, > > + .map_lookup_and_delete_batch = generic_map_lookup_and_delete_batch, > > Not 100% sure whether trie should use generic map > lookup/lookup_and_delete or not. If the key is not available, > the get_next_key will return the 'leftmost' node which roughly > corresponding to the first node in the hash table. > I think you're right, we shouldn't use generic lookup/lookup_and_delete for lpm_trie. That being said, would you be ok, if we don't add lpm_trie support in this patch series? Also we can drop the generic_map_lookup_and_delete implementation in this patch series and add it in the future, if needed. What do you think? > > + .map_delete_batch = generic_map_delete_batch, > > + .map_update_batch = generic_map_update_batch,with efault > > }; > >
On 1/6/20 10:39 PM, Brian Vazquez wrote: > Hi Yonghong, thanks for reviewing it and sorry for the late reply I > had been traveling. > > On Fri, Dec 13, 2019 at 11:46 AM Yonghong Song <yhs@fb.com> wrote: >> >> >> >> On 12/11/19 2:33 PM, Brian Vazquez wrote: >>> This adds the generic batch ops functionality to bpf lpm_trie. >>> >>> Signed-off-by: Brian Vazquez <brianvv@google.com> >>> --- >>> kernel/bpf/lpm_trie.c | 4 ++++ >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/kernel/bpf/lpm_trie.c b/kernel/bpf/lpm_trie.c >>> index 56e6c75d354d9..92c47b4f03337 100644 >>> --- a/kernel/bpf/lpm_trie.c >>> +++ b/kernel/bpf/lpm_trie.c >>> @@ -743,4 +743,8 @@ const struct bpf_map_ops trie_map_ops = { >>> .map_update_elem = trie_update_elem, >>> .map_delete_elem = trie_delete_elem, >>> .map_check_btf = trie_check_btf, >>> + .map_lookup_batch = generic_map_lookup_batch, >>> + .map_lookup_and_delete_batch = generic_map_lookup_and_delete_batch, >> >> Not 100% sure whether trie should use generic map >> lookup/lookup_and_delete or not. If the key is not available, >> the get_next_key will return the 'leftmost' node which roughly >> corresponding to the first node in the hash table. >> > > I think you're right, we shouldn't use generic > lookup/lookup_and_delete for lpm_trie. That being said, would you be > ok, if we don't add lpm_trie support in this patch series? Also we can > drop the generic_map_lookup_and_delete implementation in this patch > series and add it in the future, if needed. What do you think? Yes, we can drop generic_map_lookup_and_delete(), it probably will not be used a lot. The normal array map, you cannot delete elements. For fd_array maps, they tend to be small. > >>> + .map_delete_batch = generic_map_delete_batch, >>> + .map_update_batch = generic_map_update_batch,with efault >>> }; >>>
diff --git a/kernel/bpf/lpm_trie.c b/kernel/bpf/lpm_trie.c index 56e6c75d354d9..92c47b4f03337 100644 --- a/kernel/bpf/lpm_trie.c +++ b/kernel/bpf/lpm_trie.c @@ -743,4 +743,8 @@ const struct bpf_map_ops trie_map_ops = { .map_update_elem = trie_update_elem, .map_delete_elem = trie_delete_elem, .map_check_btf = trie_check_btf, + .map_lookup_batch = generic_map_lookup_batch, + .map_lookup_and_delete_batch = generic_map_lookup_and_delete_batch, + .map_delete_batch = generic_map_delete_batch, + .map_update_batch = generic_map_update_batch, };
This adds the generic batch ops functionality to bpf lpm_trie. Signed-off-by: Brian Vazquez <brianvv@google.com> --- kernel/bpf/lpm_trie.c | 4 ++++ 1 file changed, 4 insertions(+)