Patchwork [RFC,bpf-next,4/4] tools/bpf: handle EOPNOTSUPP when map lookup is failed

login
register
mail settings
Submitter Prashant Bhole
Date Sept. 19, 2018, 7:51 a.m.
Message ID <20180919075143.9308-5-bhole_prashant_q7@lab.ntt.co.jp>
Download mbox | patch
Permalink /patch/615587/
State New
Headers show

Comments

Prashant Bhole - Sept. 19, 2018, 7:51 a.m.
Let's add a check for EOPNOTSUPP error when map lookup is failed.
Also in case map doesn't support lookup, the output of map dump is
changed from "can't lookup element" to "lookup not supported for
this map".

Patch adds function print_entry_error() function to print the error
value.

Following example dumps a map which does not support lookup.

Output before:
root# bpftool map -jp dump id 40
[
    "key": ["0x0a","0x00","0x00","0x00"
    ],
    "value": {
        "error": "can\'t lookup element"
    },
    "key": ["0x0b","0x00","0x00","0x00"
    ],
    "value": {
        "error": "can\'t lookup element"
    }
]

root# bpftool map dump id 40
can't lookup element with key:
0a 00 00 00
can't lookup element with key:
0b 00 00 00
Found 0 elements

Output after changes:
root# bpftool map dump -jp  id 45
[
    "key": ["0x0a","0x00","0x00","0x00"
    ],
    "value": {
        "error": "lookup not supported for this map"
    },
    "key": ["0x0b","0x00","0x00","0x00"
    ],
    "value": {
        "error": "lookup not supported for this map"
    }
]

root# bpftool map dump id 45
key:
0a 00 00 00
value:
lookup not supported for this map
key:
0b 00 00 00
value:
lookup not supported for this map
Found 0 elements

Signed-off-by: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
---
 tools/bpf/bpftool/main.h |  5 +++++
 tools/bpf/bpftool/map.c  | 35 ++++++++++++++++++++++++++++++-----
 2 files changed, 35 insertions(+), 5 deletions(-)
Jakub Kicinski - Sept. 19, 2018, 3:29 p.m.
On Wed, 19 Sep 2018 16:51:43 +0900, Prashant Bhole wrote:
> Let's add a check for EOPNOTSUPP error when map lookup is failed.
> Also in case map doesn't support lookup, the output of map dump is
> changed from "can't lookup element" to "lookup not supported for
> this map".
> 
> Patch adds function print_entry_error() function to print the error
> value.
> 
> Following example dumps a map which does not support lookup.
> 
> Output before:
> root# bpftool map -jp dump id 40
> [
>     "key": ["0x0a","0x00","0x00","0x00"
>     ],
>     "value": {
>         "error": "can\'t lookup element"
>     },
>     "key": ["0x0b","0x00","0x00","0x00"
>     ],
>     "value": {
>         "error": "can\'t lookup element"
>     }
> ]
> 
> root# bpftool map dump id 40
> can't lookup element with key:
> 0a 00 00 00
> can't lookup element with key:
> 0b 00 00 00
> Found 0 elements
> 
> Output after changes:
> root# bpftool map dump -jp  id 45
> [
>     "key": ["0x0a","0x00","0x00","0x00"
>     ],
>     "value": {
>         "error": "lookup not supported for this map"
>     },
>     "key": ["0x0b","0x00","0x00","0x00"
>     ],
>     "value": {
>         "error": "lookup not supported for this map"
>     }
> ]
> 
> root# bpftool map dump id 45
> key:
> 0a 00 00 00
> value:
> lookup not supported for this map
> key:
> 0b 00 00 00
> value:
> lookup not supported for this map
> Found 0 elements

Nice improvement, thanks for the changes!  I wonder what your thoughts
would be on just printing some form of "lookup not supported for this
map" only once?  It seems slightly like repeated information - if
lookup is not supported for one key it likely won't be for other keys
too, so we could shorten the output.  Would that make sense?

> Signed-off-by: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
> ---
>  tools/bpf/bpftool/main.h |  5 +++++
>  tools/bpf/bpftool/map.c  | 35 ++++++++++++++++++++++++++++++-----
>  2 files changed, 35 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
> index 40492cdc4e53..1a8c683f949b 100644
> --- a/tools/bpf/bpftool/main.h
> +++ b/tools/bpf/bpftool/main.h
> @@ -46,6 +46,11 @@
>  
>  #include "json_writer.h"
>  
> +#define ERR_CANNOT_LOOKUP \
> +	"can't lookup element"
> +#define ERR_LOOKUP_NOT_SUPPORTED \
> +	"lookup not supported for this map"

Do we need these?  Are we going to reused them in more parts of the
code?

>  #define ptr_to_u64(ptr)	((__u64)(unsigned long)(ptr))
>  
>  #define NEXT_ARG()	({ argc--; argv++; if (argc < 0) usage(); })
> diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
> index 284e12a289c0..2faccd2098c9 100644
> --- a/tools/bpf/bpftool/map.c
> +++ b/tools/bpf/bpftool/map.c
> @@ -333,6 +333,25 @@ static void print_entry_json(struct bpf_map_info *info, unsigned char *key,
>  	jsonw_end_object(json_wtr);
>  }
>  
> +static void print_entry_error(struct bpf_map_info *info, unsigned char *key,
> +			      char *value)
> +{
> +	bool single_line, break_names;
> +	int value_size = strlen(value);

nit: order variables declaration lines to shortest, please.

> +
> +	break_names = info->key_size > 16 || value_size > 16;
> +	single_line = info->key_size + value_size <= 24 && !break_names;
> +
> +	printf("key:%c", break_names ? '\n' : ' ');
> +	fprint_hex(stdout, key, info->key_size, " ");
> +
> +	printf(single_line ? "  " : "\n");
> +
> +	printf("value:%c%s", break_names ? '\n' : ' ', value);
> +
> +	printf("\n");
> +}
> +
>  static void print_entry_plain(struct bpf_map_info *info, unsigned char *key,
>  			      unsigned char *value)
>  {
> @@ -660,6 +679,8 @@ static int dump_map_elem(int fd, void *key, void *value,
>  			 json_writer_t *btf_wtr)
>  {
>  	int num_elems = 0;
> +	int lookup_errno;
> +	char *errstr;

nit: const char?

>  
>  	if (!bpf_map_lookup_elem(fd, key, value)) {
>  		if (json_output) {
Prashant Bhole - Sept. 20, 2018, 2:58 a.m.
On 9/20/2018 12:29 AM, Jakub Kicinski wrote:
> On Wed, 19 Sep 2018 16:51:43 +0900, Prashant Bhole wrote:
>> Let's add a check for EOPNOTSUPP error when map lookup is failed.
>> Also in case map doesn't support lookup, the output of map dump is
>> changed from "can't lookup element" to "lookup not supported for
>> this map".
>>
>> Patch adds function print_entry_error() function to print the error
>> value.
>>
>> Following example dumps a map which does not support lookup.
>>
>> Output before:
>> root# bpftool map -jp dump id 40
>> [
>>      "key": ["0x0a","0x00","0x00","0x00"
>>      ],
>>      "value": {
>>          "error": "can\'t lookup element"
>>      },
>>      "key": ["0x0b","0x00","0x00","0x00"
>>      ],
>>      "value": {
>>          "error": "can\'t lookup element"
>>      }
>> ]
>>
>> root# bpftool map dump id 40
>> can't lookup element with key:
>> 0a 00 00 00
>> can't lookup element with key:
>> 0b 00 00 00
>> Found 0 elements
>>
>> Output after changes:
>> root# bpftool map dump -jp  id 45
>> [
>>      "key": ["0x0a","0x00","0x00","0x00"
>>      ],
>>      "value": {
>>          "error": "lookup not supported for this map"
>>      },
>>      "key": ["0x0b","0x00","0x00","0x00"
>>      ],
>>      "value": {
>>          "error": "lookup not supported for this map"
>>      }
>> ]
>>
>> root# bpftool map dump id 45
>> key:
>> 0a 00 00 00
>> value:
>> lookup not supported for this map
>> key:
>> 0b 00 00 00
>> value:
>> lookup not supported for this map
>> Found 0 elements
> 
> Nice improvement, thanks for the changes!  I wonder what your thoughts
> would be on just printing some form of "lookup not supported for this
> map" only once?  It seems slightly like repeated information - if
> lookup is not supported for one key it likely won't be for other keys
> too, so we could shorten the output.  Would that make sense?

I too thought that the message is repeated information. One idea was as 
you said, stop iterating once we get EOPNOTSUPP. But only reason for 
keeping this way is that user will at least see dump of keys along with 
it. Also it will be consistent with Json output. What is your opinion on it?

I will fix other things that you pointed out. Thanks!

-Prashant

> 
>> Signed-off-by: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
>> ---
>>   tools/bpf/bpftool/main.h |  5 +++++
>>   tools/bpf/bpftool/map.c  | 35 ++++++++++++++++++++++++++++++-----
>>   2 files changed, 35 insertions(+), 5 deletions(-)
>>
>> diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
>> index 40492cdc4e53..1a8c683f949b 100644
>> --- a/tools/bpf/bpftool/main.h
>> +++ b/tools/bpf/bpftool/main.h
>> @@ -46,6 +46,11 @@
>>   
>>   #include "json_writer.h"
>>   
>> +#define ERR_CANNOT_LOOKUP \
>> +	"can't lookup element"
>> +#define ERR_LOOKUP_NOT_SUPPORTED \
>> +	"lookup not supported for this map"
> 
> Do we need these?  Are we going to reused them in more parts of the
> code?
> 
>>   #define ptr_to_u64(ptr)	((__u64)(unsigned long)(ptr))
>>   
>>   #define NEXT_ARG()	({ argc--; argv++; if (argc < 0) usage(); })
>> diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
>> index 284e12a289c0..2faccd2098c9 100644
>> --- a/tools/bpf/bpftool/map.c
>> +++ b/tools/bpf/bpftool/map.c
>> @@ -333,6 +333,25 @@ static void print_entry_json(struct bpf_map_info *info, unsigned char *key,
>>   	jsonw_end_object(json_wtr);
>>   }
>>   
>> +static void print_entry_error(struct bpf_map_info *info, unsigned char *key,
>> +			      char *value)
>> +{
>> +	bool single_line, break_names;
>> +	int value_size = strlen(value);
> 
> nit: order variables declaration lines to shortest, please.
> 
>> +
>> +	break_names = info->key_size > 16 || value_size > 16;
>> +	single_line = info->key_size + value_size <= 24 && !break_names;
>> +
>> +	printf("key:%c", break_names ? '\n' : ' ');
>> +	fprint_hex(stdout, key, info->key_size, " ");
>> +
>> +	printf(single_line ? "  " : "\n");
>> +
>> +	printf("value:%c%s", break_names ? '\n' : ' ', value);
>> +
>> +	printf("\n");
>> +}
>> +
>>   static void print_entry_plain(struct bpf_map_info *info, unsigned char *key,
>>   			      unsigned char *value)
>>   {
>> @@ -660,6 +679,8 @@ static int dump_map_elem(int fd, void *key, void *value,
>>   			 json_writer_t *btf_wtr)
>>   {
>>   	int num_elems = 0;
>> +	int lookup_errno;
>> +	char *errstr;
> 
> nit: const char?
> 
>>   
>>   	if (!bpf_map_lookup_elem(fd, key, value)) {
>>   		if (json_output) {
> 
> 
>
Prashant Bhole - Sept. 20, 2018, 5:04 a.m.
On 9/20/2018 12:29 AM, Jakub Kicinski wrote:
> On Wed, 19 Sep 2018 16:51:43 +0900, Prashant Bhole wrote:
>> Let's add a check for EOPNOTSUPP error when map lookup is failed.
>> Also in case map doesn't support lookup, the output of map dump is
>> changed from "can't lookup element" to "lookup not supported for
>> this map".
>>
>> Patch adds function print_entry_error() function to print the error
>> value.
>>
>> Following example dumps a map which does not support lookup.
>>
>> Output before:
>> root# bpftool map -jp dump id 40
>> [
>>      "key": ["0x0a","0x00","0x00","0x00"
>>      ],
>>      "value": {
>>          "error": "can\'t lookup element"
>>      },
>>      "key": ["0x0b","0x00","0x00","0x00"
>>      ],
>>      "value": {
>>          "error": "can\'t lookup element"
>>      }
>> ]
>>
>> root# bpftool map dump id 40
>> can't lookup element with key:
>> 0a 00 00 00
>> can't lookup element with key:
>> 0b 00 00 00
>> Found 0 elements
>>
>> Output after changes:
>> root# bpftool map dump -jp  id 45
>> [
>>      "key": ["0x0a","0x00","0x00","0x00"
>>      ],
>>      "value": {
>>          "error": "lookup not supported for this map"
>>      },
>>      "key": ["0x0b","0x00","0x00","0x00"
>>      ],
>>      "value": {
>>          "error": "lookup not supported for this map"
>>      }
>> ]
>>
>> root# bpftool map dump id 45
>> key:
>> 0a 00 00 00
>> value:
>> lookup not supported for this map
>> key:
>> 0b 00 00 00
>> value:
>> lookup not supported for this map
>> Found 0 elements
> 
> Nice improvement, thanks for the changes!  I wonder what your thoughts
> would be on just printing some form of "lookup not supported for this
> map" only once?  It seems slightly like repeated information - if
> lookup is not supported for one key it likely won't be for other keys
> too, so we could shorten the output.  Would that make sense?
> 
>> Signed-off-by: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
>> ---
>>   tools/bpf/bpftool/main.h |  5 +++++
>>   tools/bpf/bpftool/map.c  | 35 ++++++++++++++++++++++++++++++-----
>>   2 files changed, 35 insertions(+), 5 deletions(-)
>>
>> diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
>> index 40492cdc4e53..1a8c683f949b 100644
>> --- a/tools/bpf/bpftool/main.h
>> +++ b/tools/bpf/bpftool/main.h
>> @@ -46,6 +46,11 @@
>>   
>>   #include "json_writer.h"
>>   
>> +#define ERR_CANNOT_LOOKUP \
>> +	"can't lookup element"
>> +#define ERR_LOOKUP_NOT_SUPPORTED \
>> +	"lookup not supported for this map"
> 
> Do we need these?  Are we going to reused them in more parts of the
> code?

These are used only once. These can be used in do_lookup(). Currently 
do_lookup() prints strerror(errno) when lookup is failed. Shall I change 
that do_lookup() output?

-Prashant
Jakub Kicinski - Sept. 20, 2018, 3:59 p.m.
On Thu, 20 Sep 2018 14:04:19 +0900, Prashant Bhole wrote:
> On 9/20/2018 12:29 AM, Jakub Kicinski wrote:
> > On Wed, 19 Sep 2018 16:51:43 +0900, Prashant Bhole wrote:  
> >> Let's add a check for EOPNOTSUPP error when map lookup is failed.
> >> Also in case map doesn't support lookup, the output of map dump is
> >> changed from "can't lookup element" to "lookup not supported for
> >> this map".
> >>
> >> Patch adds function print_entry_error() function to print the error
> >> value.
> >>
> >> Following example dumps a map which does not support lookup.
> >>
> >> Output before:
> >> root# bpftool map -jp dump id 40
> >> [
> >>      "key": ["0x0a","0x00","0x00","0x00"
> >>      ],
> >>      "value": {
> >>          "error": "can\'t lookup element"
> >>      },
> >>      "key": ["0x0b","0x00","0x00","0x00"
> >>      ],
> >>      "value": {
> >>          "error": "can\'t lookup element"
> >>      }
> >> ]
> >>
> >> root# bpftool map dump id 40
> >> can't lookup element with key:
> >> 0a 00 00 00
> >> can't lookup element with key:
> >> 0b 00 00 00
> >> Found 0 elements
> >>
> >> Output after changes:
> >> root# bpftool map dump -jp  id 45
> >> [
> >>      "key": ["0x0a","0x00","0x00","0x00"
> >>      ],
> >>      "value": {
> >>          "error": "lookup not supported for this map"
> >>      },
> >>      "key": ["0x0b","0x00","0x00","0x00"
> >>      ],
> >>      "value": {
> >>          "error": "lookup not supported for this map"
> >>      }
> >> ]
> >>
> >> root# bpftool map dump id 45
> >> key:
> >> 0a 00 00 00
> >> value:
> >> lookup not supported for this map
> >> key:
> >> 0b 00 00 00
> >> value:
> >> lookup not supported for this map
> >> Found 0 elements  
> > 
> > Nice improvement, thanks for the changes!  I wonder what your thoughts
> > would be on just printing some form of "lookup not supported for this
> > map" only once?  It seems slightly like repeated information - if
> > lookup is not supported for one key it likely won't be for other keys
> > too, so we could shorten the output.  Would that make sense?
> >   
> >> Signed-off-by: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
> >> ---
> >>   tools/bpf/bpftool/main.h |  5 +++++
> >>   tools/bpf/bpftool/map.c  | 35 ++++++++++++++++++++++++++++++-----
> >>   2 files changed, 35 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
> >> index 40492cdc4e53..1a8c683f949b 100644
> >> --- a/tools/bpf/bpftool/main.h
> >> +++ b/tools/bpf/bpftool/main.h
> >> @@ -46,6 +46,11 @@
> >>   
> >>   #include "json_writer.h"
> >>   
> >> +#define ERR_CANNOT_LOOKUP \
> >> +	"can't lookup element"
> >> +#define ERR_LOOKUP_NOT_SUPPORTED \
> >> +	"lookup not supported for this map"  
> > 
> > Do we need these?  Are we going to reused them in more parts of the
> > code?  
> 
> These are used only once. These can be used in do_lookup(). Currently 
> do_lookup() prints strerror(errno) when lookup is failed. Shall I change 
> that do_lookup() output?

I actually prefer to stick to strerror(), the standard errors more
clearly correlate with what happened in my mind (i.e. "Operation not
supported" == kernel sent EOPNOTSUPP).   strerror() may also print in
local language if translation/localization matters.

We could even use strerr() in dump_map_elem() but up to you.  The one
in do_lookup() I'd prefer to leave be ;)
Prashant Bhole - Oct. 2, 2018, 5:33 a.m.
On 9/21/2018 12:59 AM, Jakub Kicinski wrote:
> On Thu, 20 Sep 2018 14:04:19 +0900, Prashant Bhole wrote:
>> On 9/20/2018 12:29 AM, Jakub Kicinski wrote:
>>> On Wed, 19 Sep 2018 16:51:43 +0900, Prashant Bhole wrote:
>>>> Let's add a check for EOPNOTSUPP error when map lookup is failed.
>>>> Also in case map doesn't support lookup, the output of map dump is
>>>> changed from "can't lookup element" to "lookup not supported for
>>>> this map".
>>>>
>>>> Patch adds function print_entry_error() function to print the error
>>>> value.
>>>>
>>>> Following example dumps a map which does not support lookup.
>>>>
>>>> Output before:
>>>> root# bpftool map -jp dump id 40
>>>> [
>>>>       "key": ["0x0a","0x00","0x00","0x00"
>>>>       ],
>>>>       "value": {
>>>>           "error": "can\'t lookup element"
>>>>       },
>>>>       "key": ["0x0b","0x00","0x00","0x00"
>>>>       ],
>>>>       "value": {
>>>>           "error": "can\'t lookup element"
>>>>       }
>>>> ]
>>>>
>>>> root# bpftool map dump id 40
>>>> can't lookup element with key:
>>>> 0a 00 00 00
>>>> can't lookup element with key:
>>>> 0b 00 00 00
>>>> Found 0 elements
>>>>
>>>> Output after changes:
>>>> root# bpftool map dump -jp  id 45
>>>> [
>>>>       "key": ["0x0a","0x00","0x00","0x00"
>>>>       ],
>>>>       "value": {
>>>>           "error": "lookup not supported for this map"
>>>>       },
>>>>       "key": ["0x0b","0x00","0x00","0x00"
>>>>       ],
>>>>       "value": {
>>>>           "error": "lookup not supported for this map"
>>>>       }
>>>> ]
>>>>
>>>> root# bpftool map dump id 45
>>>> key:
>>>> 0a 00 00 00
>>>> value:
>>>> lookup not supported for this map
>>>> key:
>>>> 0b 00 00 00
>>>> value:
>>>> lookup not supported for this map
>>>> Found 0 elements
>>>
>>> Nice improvement, thanks for the changes!  I wonder what your thoughts
>>> would be on just printing some form of "lookup not supported for this
>>> map" only once?  It seems slightly like repeated information - if
>>> lookup is not supported for one key it likely won't be for other keys
>>> too, so we could shorten the output.  Would that make sense?
>>>    
>>>> Signed-off-by: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
>>>> ---
>>>>    tools/bpf/bpftool/main.h |  5 +++++
>>>>    tools/bpf/bpftool/map.c  | 35 ++++++++++++++++++++++++++++++-----
>>>>    2 files changed, 35 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
>>>> index 40492cdc4e53..1a8c683f949b 100644
>>>> --- a/tools/bpf/bpftool/main.h
>>>> +++ b/tools/bpf/bpftool/main.h
>>>> @@ -46,6 +46,11 @@
>>>>    
>>>>    #include "json_writer.h"
>>>>    
>>>> +#define ERR_CANNOT_LOOKUP \
>>>> +	"can't lookup element"
>>>> +#define ERR_LOOKUP_NOT_SUPPORTED \
>>>> +	"lookup not supported for this map"
>>>
>>> Do we need these?  Are we going to reused them in more parts of the
>>> code?
>>
>> These are used only once. These can be used in do_lookup(). Currently
>> do_lookup() prints strerror(errno) when lookup is failed. Shall I change
>> that do_lookup() output?
> 
> I actually prefer to stick to strerror(), the standard errors more
> clearly correlate with what happened in my mind (i.e. "Operation not
> supported" == kernel sent EOPNOTSUPP).   strerror() may also print in
> local language if translation/localization matters.
> 
> We could even use strerr() in dump_map_elem() but up to you.  The one
> in do_lookup() I'd prefer to leave be ;)

Sorry for the late reply.
In v2 I have removed the error strings altogether. As you suggested 
output will be strerror(). Also added verifier tests as Alexei 
suggested. I am sending the RFC-v2 series soon.
Thanks.

-Prashant

Patch

diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index 40492cdc4e53..1a8c683f949b 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -46,6 +46,11 @@ 
 
 #include "json_writer.h"
 
+#define ERR_CANNOT_LOOKUP \
+	"can't lookup element"
+#define ERR_LOOKUP_NOT_SUPPORTED \
+	"lookup not supported for this map"
+
 #define ptr_to_u64(ptr)	((__u64)(unsigned long)(ptr))
 
 #define NEXT_ARG()	({ argc--; argv++; if (argc < 0) usage(); })
diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
index 284e12a289c0..2faccd2098c9 100644
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@ -333,6 +333,25 @@  static void print_entry_json(struct bpf_map_info *info, unsigned char *key,
 	jsonw_end_object(json_wtr);
 }
 
+static void print_entry_error(struct bpf_map_info *info, unsigned char *key,
+			      char *value)
+{
+	bool single_line, break_names;
+	int value_size = strlen(value);
+
+	break_names = info->key_size > 16 || value_size > 16;
+	single_line = info->key_size + value_size <= 24 && !break_names;
+
+	printf("key:%c", break_names ? '\n' : ' ');
+	fprint_hex(stdout, key, info->key_size, " ");
+
+	printf(single_line ? "  " : "\n");
+
+	printf("value:%c%s", break_names ? '\n' : ' ', value);
+
+	printf("\n");
+}
+
 static void print_entry_plain(struct bpf_map_info *info, unsigned char *key,
 			      unsigned char *value)
 {
@@ -660,6 +679,8 @@  static int dump_map_elem(int fd, void *key, void *value,
 			 json_writer_t *btf_wtr)
 {
 	int num_elems = 0;
+	int lookup_errno;
+	char *errstr;
 
 	if (!bpf_map_lookup_elem(fd, key, value)) {
 		if (json_output) {
@@ -682,22 +703,26 @@  static int dump_map_elem(int fd, void *key, void *value,
 	}
 
 	/* lookup error handling */
+	lookup_errno = errno;
+
 	if (map_is_map_of_maps(map_info->type) ||
 	    map_is_map_of_progs(map_info->type))
 		goto out;
 
+	if (lookup_errno == EOPNOTSUPP)
+		errstr = ERR_LOOKUP_NOT_SUPPORTED;
+	else
+		errstr = ERR_CANNOT_LOOKUP;
+
 	if (json_output) {
 		jsonw_name(json_wtr, "key");
 		print_hex_data_json(key, map_info->key_size);
 		jsonw_name(json_wtr, "value");
 		jsonw_start_object(json_wtr);
-		jsonw_string_field(json_wtr, "error",
-				   "can't lookup element");
+		jsonw_string_field(json_wtr, "error", errstr);
 		jsonw_end_object(json_wtr);
 	} else {
-		p_info("can't lookup element with key: ");
-		fprint_hex(stderr, key, map_info->key_size, " ");
-		fprintf(stderr, "\n");
+		print_entry_error(map_info, key, errstr);
 	}
 out:
 	return num_elems;