Message ID | 20190221163926.15221-1-alban@kinvolk.io |
---|---|
State | Changes Requested |
Delegated to: | BPF Maintainers |
Headers | show |
Series | [bpf-next,v1] bpf, lpm: fix lookup bug in map_delete_elem | expand |
On Thu, Feb 21, 2019 at 05:39:26PM +0100, Alban Crequy wrote: > From: Alban Crequy <alban@kinvolk.io> > > trie_delete_elem() was deleting an entry even though it was not matching > if the prefixlen was correct. This patch adds a check on matchlen. > > Reproducer: > > $ sudo bpftool map create /sys/fs/bpf/mylpm type lpm_trie key 8 value 1 entries 128 name mylpm flags 1 > $ sudo bpftool map update pinned /sys/fs/bpf/mylpm key hex 10 00 00 00 aa bb cc dd value hex 01 > $ sudo bpftool map dump pinned /sys/fs/bpf/mylpm > key: 10 00 00 00 aa bb cc dd value: 01 > Found 1 element > $ sudo bpftool map delete pinned /sys/fs/bpf/mylpm key hex 10 00 00 00 ff ff ff ff > $ echo $? > 0 > $ sudo bpftool map dump pinned /sys/fs/bpf/mylpm > Found 0 elements The change makes sense to me. Can you add this reproducer to tools/testing/selftests/bpf/test_lpm_map.c? Bug fix should be for the "bpf" tree instead of "bpf-next" Fixes tag is also required, like Fixes: e454cf595853 ("bpf: Implement map_delete_elem for BPF_MAP_TYPE_LPM_TRIE") Cc: Craig Gallek <kraig@google.com> > > Signed-off-by: Alban Crequy <alban@kinvolk.io> > --- > kernel/bpf/lpm_trie.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/kernel/bpf/lpm_trie.c b/kernel/bpf/lpm_trie.c > index abf1002080df..93a5cbbde421 100644 > --- a/kernel/bpf/lpm_trie.c > +++ b/kernel/bpf/lpm_trie.c > @@ -471,6 +471,7 @@ static int trie_delete_elem(struct bpf_map *map, void *_key) > } > > if (!node || node->prefixlen != key->prefixlen || > + node->prefixlen != matchlen || > (node->flags & LPM_TREE_NODE_FLAG_IM)) { > ret = -ENOENT; > goto out; > -- > 2.20.1 >
On Thu, Feb 21, 2019 at 11:24 PM Martin Lau <kafai@fb.com> wrote: > > On Thu, Feb 21, 2019 at 05:39:26PM +0100, Alban Crequy wrote: > > From: Alban Crequy <alban@kinvolk.io> > > > > trie_delete_elem() was deleting an entry even though it was not matching > > if the prefixlen was correct. This patch adds a check on matchlen. > > > > Reproducer: > > > > $ sudo bpftool map create /sys/fs/bpf/mylpm type lpm_trie key 8 value 1 entries 128 name mylpm flags 1 > > $ sudo bpftool map update pinned /sys/fs/bpf/mylpm key hex 10 00 00 00 aa bb cc dd value hex 01 > > $ sudo bpftool map dump pinned /sys/fs/bpf/mylpm > > key: 10 00 00 00 aa bb cc dd value: 01 > > Found 1 element > > $ sudo bpftool map delete pinned /sys/fs/bpf/mylpm key hex 10 00 00 00 ff ff ff ff > > $ echo $? > > 0 > > $ sudo bpftool map dump pinned /sys/fs/bpf/mylpm > > Found 0 elements > The change makes sense to me. Can you add this reproducer to > tools/testing/selftests/bpf/test_lpm_map.c? > > Bug fix should be for the "bpf" tree instead of "bpf-next" > Fixes tag is also required, like > > Fixes: e454cf595853 ("bpf: Implement map_delete_elem for BPF_MAP_TYPE_LPM_TRIE") > Cc: Craig Gallek <kraig@google.com> Thanks! I'll send a v2 shortly with the selftest and the tags, based on "bpf" tree. Cheers, Alban > > Signed-off-by: Alban Crequy <alban@kinvolk.io> > > --- > > kernel/bpf/lpm_trie.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/kernel/bpf/lpm_trie.c b/kernel/bpf/lpm_trie.c > > index abf1002080df..93a5cbbde421 100644 > > --- a/kernel/bpf/lpm_trie.c > > +++ b/kernel/bpf/lpm_trie.c > > @@ -471,6 +471,7 @@ static int trie_delete_elem(struct bpf_map *map, void *_key) > > } > > > > if (!node || node->prefixlen != key->prefixlen || > > + node->prefixlen != matchlen || > > (node->flags & LPM_TREE_NODE_FLAG_IM)) { > > ret = -ENOENT; > > goto out; > > -- > > 2.20.1 > >
diff --git a/kernel/bpf/lpm_trie.c b/kernel/bpf/lpm_trie.c index abf1002080df..93a5cbbde421 100644 --- a/kernel/bpf/lpm_trie.c +++ b/kernel/bpf/lpm_trie.c @@ -471,6 +471,7 @@ static int trie_delete_elem(struct bpf_map *map, void *_key) } if (!node || node->prefixlen != key->prefixlen || + node->prefixlen != matchlen || (node->flags & LPM_TREE_NODE_FLAG_IM)) { ret = -ENOENT; goto out;