| Message ID | 20200423033314.49205-2-maowenan@huawei.com |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | Change return code if failed to load object | expand |
On Wed, Apr 22, 2020 at 8:31 PM Mao Wenan <maowenan@huawei.com> wrote: > > There is one error printed when use BPF_MAP_TYPE_SOCKMAP to create map: > libbpf: failed to create map (name: 'sock_map'): Invalid argument(-22) > > This is because CONFIG_BPF_STREAM_PARSER is not set, and > bpf_map_types[type] return invalid ops. It is not clear to show the > cause of config missing with return code -EINVAL, so add pr_warn() and > change error code to describe the reason. > > Signed-off-by: Mao Wenan <maowenan@huawei.com> > --- > kernel/bpf/syscall.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c > index d85f37239540..7686778457c7 100644 > --- a/kernel/bpf/syscall.c > +++ b/kernel/bpf/syscall.c > @@ -112,9 +112,10 @@ static struct bpf_map *find_and_alloc_map(union bpf_attr *attr) > return ERR_PTR(-EINVAL); > type = array_index_nospec(type, ARRAY_SIZE(bpf_map_types)); > ops = bpf_map_types[type]; > - if (!ops) > - return ERR_PTR(-EINVAL); > - > + if (!ops) { > + pr_warn("map type %d not supported or kernel config not opened\n", type); > + return ERR_PTR(-EOPNOTSUPP); > + } I don't think users will like it when kernel spams dmesg. If you need this level of verbosity please teach consumer of libbpf to print them. It's not a job of libbpf either.
On 2020/4/23 13:43, Alexei Starovoitov wrote: > On Wed, Apr 22, 2020 at 8:31 PM Mao Wenan <maowenan@huawei.com> wrote: >> >> There is one error printed when use BPF_MAP_TYPE_SOCKMAP to create map: >> libbpf: failed to create map (name: 'sock_map'): Invalid argument(-22) >> >> This is because CONFIG_BPF_STREAM_PARSER is not set, and >> bpf_map_types[type] return invalid ops. It is not clear to show the >> cause of config missing with return code -EINVAL, so add pr_warn() and >> change error code to describe the reason. >> >> Signed-off-by: Mao Wenan <maowenan@huawei.com> >> --- >> kernel/bpf/syscall.c | 7 ++++--- >> 1 file changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c >> index d85f37239540..7686778457c7 100644 >> --- a/kernel/bpf/syscall.c >> +++ b/kernel/bpf/syscall.c >> @@ -112,9 +112,10 @@ static struct bpf_map *find_and_alloc_map(union bpf_attr *attr) >> return ERR_PTR(-EINVAL); >> type = array_index_nospec(type, ARRAY_SIZE(bpf_map_types)); >> ops = bpf_map_types[type]; >> - if (!ops) >> - return ERR_PTR(-EINVAL); >> - >> + if (!ops) { >> + pr_warn("map type %d not supported or kernel config not opened\n", type); >> + return ERR_PTR(-EOPNOTSUPP); >> + } > > I don't think users will like it when kernel spams dmesg. > If you need this level of verbosity please teach consumer of libbpf to > print them. > It's not a job of libbpf either. thanks for reviw, so is it better to delete redundant pr_warn()? > > . >
On Wed, Apr 22, 2020 at 11:25 PM maowenan <maowenan@huawei.com> wrote: > > On 2020/4/23 13:43, Alexei Starovoitov wrote: > > On Wed, Apr 22, 2020 at 8:31 PM Mao Wenan <maowenan@huawei.com> wrote: > >> > >> There is one error printed when use BPF_MAP_TYPE_SOCKMAP to create map: > >> libbpf: failed to create map (name: 'sock_map'): Invalid argument(-22) > >> > >> This is because CONFIG_BPF_STREAM_PARSER is not set, and > >> bpf_map_types[type] return invalid ops. It is not clear to show the > >> cause of config missing with return code -EINVAL, so add pr_warn() and > >> change error code to describe the reason. > >> > >> Signed-off-by: Mao Wenan <maowenan@huawei.com> > >> --- > >> kernel/bpf/syscall.c | 7 ++++--- > >> 1 file changed, 4 insertions(+), 3 deletions(-) > >> > >> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c > >> index d85f37239540..7686778457c7 100644 > >> --- a/kernel/bpf/syscall.c > >> +++ b/kernel/bpf/syscall.c > >> @@ -112,9 +112,10 @@ static struct bpf_map *find_and_alloc_map(union bpf_attr *attr) > >> return ERR_PTR(-EINVAL); > >> type = array_index_nospec(type, ARRAY_SIZE(bpf_map_types)); > >> ops = bpf_map_types[type]; > >> - if (!ops) > >> - return ERR_PTR(-EINVAL); > >> - > >> + if (!ops) { > >> + pr_warn("map type %d not supported or kernel config not opened\n", type); > >> + return ERR_PTR(-EOPNOTSUPP); > >> + } > > > > I don't think users will like it when kernel spams dmesg. > > If you need this level of verbosity please teach consumer of libbpf to > > print them. > > It's not a job of libbpf either. > thanks for reviw, so is it better to delete redundant pr_warn()? which one?
On 2020/4/24 0:07, Alexei Starovoitov wrote: > On Wed, Apr 22, 2020 at 11:25 PM maowenan <maowenan@huawei.com> wrote: >> >> On 2020/4/23 13:43, Alexei Starovoitov wrote: >>> On Wed, Apr 22, 2020 at 8:31 PM Mao Wenan <maowenan@huawei.com> wrote: >>>> >>>> There is one error printed when use BPF_MAP_TYPE_SOCKMAP to create map: >>>> libbpf: failed to create map (name: 'sock_map'): Invalid argument(-22) >>>> >>>> This is because CONFIG_BPF_STREAM_PARSER is not set, and >>>> bpf_map_types[type] return invalid ops. It is not clear to show the >>>> cause of config missing with return code -EINVAL, so add pr_warn() and >>>> change error code to describe the reason. >>>> >>>> Signed-off-by: Mao Wenan <maowenan@huawei.com> >>>> --- >>>> kernel/bpf/syscall.c | 7 ++++--- >>>> 1 file changed, 4 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c >>>> index d85f37239540..7686778457c7 100644 >>>> --- a/kernel/bpf/syscall.c >>>> +++ b/kernel/bpf/syscall.c >>>> @@ -112,9 +112,10 @@ static struct bpf_map *find_and_alloc_map(union bpf_attr *attr) >>>> return ERR_PTR(-EINVAL); >>>> type = array_index_nospec(type, ARRAY_SIZE(bpf_map_types)); >>>> ops = bpf_map_types[type]; >>>> - if (!ops) >>>> - return ERR_PTR(-EINVAL); >>>> - >>>> + if (!ops) { >>>> + pr_warn("map type %d not supported or kernel config not opened\n", type); >>>> + return ERR_PTR(-EOPNOTSUPP); >>>> + } >>> >>> I don't think users will like it when kernel spams dmesg. >>> If you need this level of verbosity please teach consumer of libbpf to >>> print them. >>> It's not a job of libbpf either. >> thanks for reviw, so is it better to delete redundant pr_warn()? > > which one? I mean pr_warn is no need, this patch just change the return code ERR_PTR(-EOPNOTSUPP); + pr_warn("map type %d not supported or kernel config not opened\n", type); > > . >
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index d85f37239540..7686778457c7 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -112,9 +112,10 @@ static struct bpf_map *find_and_alloc_map(union bpf_attr *attr) return ERR_PTR(-EINVAL); type = array_index_nospec(type, ARRAY_SIZE(bpf_map_types)); ops = bpf_map_types[type]; - if (!ops) - return ERR_PTR(-EINVAL); - + if (!ops) { + pr_warn("map type %d not supported or kernel config not opened\n", type); + return ERR_PTR(-EOPNOTSUPP); + } if (ops->map_alloc_check) { err = ops->map_alloc_check(attr); if (err)
There is one error printed when use BPF_MAP_TYPE_SOCKMAP to create map: libbpf: failed to create map (name: 'sock_map'): Invalid argument(-22) This is because CONFIG_BPF_STREAM_PARSER is not set, and bpf_map_types[type] return invalid ops. It is not clear to show the cause of config missing with return code -EINVAL, so add pr_warn() and change error code to describe the reason. Signed-off-by: Mao Wenan <maowenan@huawei.com> --- kernel/bpf/syscall.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)