[PATCH bpf-next 0/6] bpftool: support queue and stack

[PATCH bpf-next 0/6] bpftool: support queue and stack

[Stanislav Fomichev]

This patch series add support for queue/stack manipulations.

It goes like this:

#1 and #2 add support for queue/stack in existing 'update' and 'lookup'
commands by permitting empty keys.
#3 make sure we don't print empty keys for queue/stack.
#4 adds peek command, it's essentially an alias for 'lookup'.
#5 adds push/enqueue commands, an alias to 'update'.
#6 adds pop/dequeue commands that use bpf_map_lookup_and_delete_elem.

(Not sure whether it makes sense to have push/enqueue + pop/dequeue or
just have push/pop for both stack/queue, comments are welcome).

Stanislav Fomichev (6):
  bpftool: make key and value optional in update command
  bpftool: make key optional in lookup command
  bpftool: don't print empty key/value for maps
  bpftool: add peek command
  bpftool: add push and enqueue commands
  bpftool: add pop and dequeue commands

 .../bpf/bpftool/Documentation/bpftool-map.rst |  28 ++-
 tools/bpf/bpftool/bash-completion/bpftool     |   7 +-
 tools/bpf/bpftool/map.c                       | 227 ++++++++++++------
 3 files changed, 187 insertions(+), 75 deletions(-)

-- 
2.20.1.97.g81188d93c3-goog

[PATCH bpf-next 1/6] bpftool: make key and value optional in update command

[Stanislav Fomichev]

Bpftool expects both key and value for 'update' operations. For some
map types, key should not be specified. Support updating those map types.

Before:
bpftool map create /sys/fs/bpf/q type queue value 4 entries 10 name q
bpftool map update pinned /sys/fs/bpf/q value 0 1 2 3
Error: did not find key

After:
bpftool map create /sys/fs/bpf/q type queue value 4 entries 10 name q
bpftool map update pinned /sys/fs/bpf/q value 0 1 2 3

Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
 .../bpf/bpftool/Documentation/bpftool-map.rst |  4 +--
 tools/bpf/bpftool/map.c                       | 36 +++++++++++++++----
 2 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/tools/bpf/bpftool/Documentation/bpftool-map.rst b/tools/bpf/bpftool/Documentation/bpftool-map.rst
index 64b001b4f777..1f1dfe06e66d 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-map.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-map.rst
@@ -25,7 +25,7 @@ MAP COMMANDS
 |	**bpftool** **map create**     *FILE* **type** *TYPE* **key** *KEY_SIZE* **value** *VALUE_SIZE* \
 |		**entries** *MAX_ENTRIES* **name** *NAME* [**flags** *FLAGS*] [**dev** *NAME*]
 |	**bpftool** **map dump**       *MAP*
-|	**bpftool** **map update**     *MAP*  **key** *DATA*   **value** *VALUE* [*UPDATE_FLAGS*]
+|	**bpftool** **map update**     *MAP* [**key** *DATA*] [**value** *VALUE*] [*UPDATE_FLAGS*]
 |	**bpftool** **map lookup**     *MAP*  **key** *DATA*
 |	**bpftool** **map getnext**    *MAP* [**key** *DATA*]
 |	**bpftool** **map delete**     *MAP*  **key** *DATA*
@@ -62,7 +62,7 @@ DESCRIPTION
 	**bpftool map dump**    *MAP*
 		  Dump all entries in a given *MAP*.
 
-	**bpftool map update**  *MAP*  **key** *DATA*   **value** *VALUE* [*UPDATE_FLAGS*]
+	**bpftool map update**  *MAP* [**key** *DATA*] [**value** *VALUE*] [*UPDATE_FLAGS*]
 		  Update map entry for a given *KEY*.
 
 		  *UPDATE_FLAGS* can be one of: **any** update existing entry
diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
index 2037e3dc864b..61453a1ba6e1 100644
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@ -779,6 +779,32 @@ static int do_dump(int argc, char **argv)
 	return err;
 }
 
+static int alloc_key_value(struct bpf_map_info *info, void **key, void **value)
+{
+	*key = NULL;
+	*value = NULL;
+
+	if (info->key_size) {
+		*key = malloc(info->key_size);
+		if (!*key) {
+			p_err("key mem alloc failed");
+			return -1;
+		}
+	}
+
+	if (info->value_size) {
+		*value = alloc_value(info);
+		if (!*value) {
+			p_err("value mem alloc failed");
+			free(*key);
+			*key = NULL;
+			return -1;
+		}
+	}
+
+	return 0;
+}
+
 static int do_update(int argc, char **argv)
 {
 	struct bpf_map_info info = {};
@@ -795,13 +821,9 @@ static int do_update(int argc, char **argv)
 	if (fd < 0)
 		return -1;
 
-	key = malloc(info.key_size);
-	value = alloc_value(&info);
-	if (!key || !value) {
-		p_err("mem alloc failed");
-		err = -1;
+	err = alloc_key_value(&info, &key, &value);
+	if (err)
 		goto exit_free;
-	}
 
 	err = parse_elem(argv, &info, key, value, info.key_size,
 			 info.value_size, &flags, &value_fd);
@@ -1135,7 +1157,7 @@ static int do_help(int argc, char **argv)
 		"                              entries MAX_ENTRIES name NAME [flags FLAGS] \\\n"
 		"                              [dev NAME]\n"
 		"       %s %s dump       MAP\n"
-		"       %s %s update     MAP  key DATA value VALUE [UPDATE_FLAGS]\n"
+		"       %s %s update     MAP [key DATA] [value VALUE] [UPDATE_FLAGS]\n"
 		"       %s %s lookup     MAP  key DATA\n"
 		"       %s %s getnext    MAP [key DATA]\n"
 		"       %s %s delete     MAP  key DATA\n"
-- 
2.20.1.97.g81188d93c3-goog

[PATCH bpf-next 2/6] bpftool: make key optional in lookup command

[Stanislav Fomichev]

Bpftool expects key for 'lookup' operations. For some map types, key should
not be specified. Support looking up those map types.

Before:
bpftool map create /sys/fs/bpf/q type queue value 4 entries 10 name q
bpftool map update pinned /sys/fs/bpf/q value 0 1 2 3
bpftool map lookup pinned /sys/fs/bpf/q
Error: did not find key

After:
bpftool map create /sys/fs/bpf/q type queue value 4 entries 10 name q
bpftool map update pinned /sys/fs/bpf/q value 0 1 2 3
bpftool map lookup pinned /sys/fs/bpf/q
key:   value: 00 01 02 03

Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
 tools/bpf/bpftool/Documentation/bpftool-map.rst |  4 ++--
 tools/bpf/bpftool/map.c                         | 10 +++-------
 2 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/tools/bpf/bpftool/Documentation/bpftool-map.rst b/tools/bpf/bpftool/Documentation/bpftool-map.rst
index 1f1dfe06e66d..f34cace771bd 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-map.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-map.rst
@@ -26,7 +26,7 @@ MAP COMMANDS
 |		**entries** *MAX_ENTRIES* **name** *NAME* [**flags** *FLAGS*] [**dev** *NAME*]
 |	**bpftool** **map dump**       *MAP*
 |	**bpftool** **map update**     *MAP* [**key** *DATA*] [**value** *VALUE*] [*UPDATE_FLAGS*]
-|	**bpftool** **map lookup**     *MAP*  **key** *DATA*
+|	**bpftool** **map lookup**     *MAP* [**key** *DATA*]
 |	**bpftool** **map getnext**    *MAP* [**key** *DATA*]
 |	**bpftool** **map delete**     *MAP*  **key** *DATA*
 |	**bpftool** **map pin**        *MAP*  *FILE*
@@ -75,7 +75,7 @@ DESCRIPTION
 		  the bytes are parsed as decimal values, unless a "0x" prefix
 		  (for hexadecimal) or a "0" prefix (for octal) is provided.
 
-	**bpftool map lookup**  *MAP*  **key** *DATA*
+	**bpftool map lookup**  *MAP* [**key** *DATA*]
 		  Lookup **key** in the map.
 
 	**bpftool map getnext** *MAP* [**key** *DATA*]
diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
index 61453a1ba6e1..4256842f9664 100644
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@ -865,13 +865,9 @@ static int do_lookup(int argc, char **argv)
 	if (fd < 0)
 		return -1;
 
-	key = malloc(info.key_size);
-	value = alloc_value(&info);
-	if (!key || !value) {
-		p_err("mem alloc failed");
-		err = -1;
+	err = alloc_key_value(&info, &key, &value);
+	if (err)
 		goto exit_free;
-	}
 
 	err = parse_elem(argv, &info, key, NULL, info.key_size, 0, NULL, NULL);
 	if (err)
@@ -1158,7 +1154,7 @@ static int do_help(int argc, char **argv)
 		"                              [dev NAME]\n"
 		"       %s %s dump       MAP\n"
 		"       %s %s update     MAP [key DATA] [value VALUE] [UPDATE_FLAGS]\n"
-		"       %s %s lookup     MAP  key DATA\n"
+		"       %s %s lookup     MAP [key DATA]\n"
 		"       %s %s getnext    MAP [key DATA]\n"
 		"       %s %s delete     MAP  key DATA\n"
 		"       %s %s pin        MAP  FILE\n"
-- 
2.20.1.97.g81188d93c3-goog

[PATCH bpf-next 3/6] bpftool: don't print empty key/value for maps

[Stanislav Fomichev]

When doing dump or lookup, don't print key if key_size == 0 or value if
value_size == 0. The initial usecase is queue and stack, where we have
only values.

This is for regular output only, json still has all the fields.

Before:
bpftool map create /sys/fs/bpf/q type queue value 4 entries 10 name q
bpftool map update pinned /sys/fs/bpf/q value 0 1 2 3
bpftool map lookup pinned /sys/fs/bpf/q
key:   value: 00 01 02 03

After:
bpftool map create /sys/fs/bpf/q type queue value 4 entries 10 name q
bpftool map update pinned /sys/fs/bpf/q value 0 1 2 3
bpftool map lookup pinned /sys/fs/bpf/q
value: 00 01 02 03

Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
 tools/bpf/bpftool/map.c | 47 ++++++++++++++++++++++++-----------------
 1 file changed, 28 insertions(+), 19 deletions(-)

diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
index 4256842f9664..3f599399913b 100644
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@ -285,16 +285,21 @@ static void print_entry_plain(struct bpf_map_info *info, unsigned char *key,
 		single_line = info->key_size + info->value_size <= 24 &&
 			!break_names;
 
-		printf("key:%c", break_names ? '\n' : ' ');
-		fprint_hex(stdout, key, info->key_size, " ");
+		if (info->key_size) {
+			printf("key:%c", break_names ? '\n' : ' ');
+			fprint_hex(stdout, key, info->key_size, " ");
 
-		printf(single_line ? "  " : "\n");
+			printf(single_line ? "  " : "\n");
+		}
 
-		printf("value:%c", break_names ? '\n' : ' ');
-		if (value)
-			fprint_hex(stdout, value, info->value_size, " ");
-		else
-			printf("<no entry>");
+		if (info->value_size) {
+			printf("value:%c", break_names ? '\n' : ' ');
+			if (value)
+				fprint_hex(stdout, value, info->value_size,
+					   " ");
+			else
+				printf("<no entry>");
+		}
 
 		printf("\n");
 	} else {
@@ -303,19 +308,23 @@ static void print_entry_plain(struct bpf_map_info *info, unsigned char *key,
 		n = get_possible_cpus();
 		step = round_up(info->value_size, 8);
 
-		printf("key:\n");
-		fprint_hex(stdout, key, info->key_size, " ");
-		printf("\n");
-		for (i = 0; i < n; i++) {
-			printf("value (CPU %02d):%c",
-			       i, info->value_size > 16 ? '\n' : ' ');
-			if (value)
-				fprint_hex(stdout, value + i * step,
-					   info->value_size, " ");
-			else
-				printf("<no entry>");
+		if (info->key_size) {
+			printf("key:\n");
+			fprint_hex(stdout, key, info->key_size, " ");
 			printf("\n");
 		}
+		if (info->value_size) {
+			for (i = 0; i < n; i++) {
+				printf("value (CPU %02d):%c",
+				       i, info->value_size > 16 ? '\n' : ' ');
+				if (value)
+					fprint_hex(stdout, value + i * step,
+						   info->value_size, " ");
+				else
+					printf("<no entry>");
+				printf("\n");
+			}
+		}
 	}
 }
 
-- 
2.20.1.97.g81188d93c3-goog

[PATCH bpf-next 4/6] bpftool: add peek command

[Stanislav Fomichev]

This is intended to be used with queues and stacks and be more
user-friendly than 'lookup' without key/value.

Example:
bpftool map create /sys/fs/bpf/q type queue value 4 entries 10 name q
bpftool map update pinned /sys/fs/bpf/q value 0 1 2 3
bpftool map peek pinned /sys/fs/bpf/q
value: 00 01 02 03

Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
 tools/bpf/bpftool/Documentation/bpftool-map.rst | 4 ++++
 tools/bpf/bpftool/bash-completion/bpftool       | 5 +++--
 tools/bpf/bpftool/map.c                         | 4 +++-
 3 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/tools/bpf/bpftool/Documentation/bpftool-map.rst b/tools/bpf/bpftool/Documentation/bpftool-map.rst
index f34cace771bd..b79da683da88 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-map.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-map.rst
@@ -31,6 +31,7 @@ MAP COMMANDS
 |	**bpftool** **map delete**     *MAP*  **key** *DATA*
 |	**bpftool** **map pin**        *MAP*  *FILE*
 |	**bpftool** **map event_pipe** *MAP* [**cpu** *N* **index** *M*]
+|	**bpftool** **map peek**       *MAP*
 |	**bpftool** **map help**
 |
 |	*MAP* := { **id** *MAP_ID* | **pinned** *FILE* }
@@ -107,6 +108,9 @@ DESCRIPTION
 		  replace any existing ring.  Any other application will stop
 		  receiving events if it installed its rings earlier.
 
+	**bpftool map peek**  *MAP*
+		  Peek next **value** in the queue or stack.
+
 	**bpftool map help**
 		  Print short help message.
 
diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
index e4e4fab1b8c7..fe4880d9bd8d 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool
@@ -382,7 +382,7 @@ _bpftool()
         map)
             local MAP_TYPE='id pinned'
             case $command in
-                show|list|dump)
+                show|list|dump|peek)
                     case $prev in
                         $command)
                             COMPREPLY=( $( compgen -W "$MAP_TYPE" -- "$cur" ) )
@@ -546,7 +546,8 @@ _bpftool()
                 *)
                     [[ $prev == $object ]] && \
                         COMPREPLY=( $( compgen -W 'delete dump getnext help \
-                            lookup pin event_pipe show list update create' -- \
+                            lookup pin event_pipe show list update create \
+                            peek' -- \
                             "$cur" ) )
                     ;;
             esac
diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
index 3f599399913b..d7344fcd2089 100644
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@ -1168,6 +1168,7 @@ static int do_help(int argc, char **argv)
 		"       %s %s delete     MAP  key DATA\n"
 		"       %s %s pin        MAP  FILE\n"
 		"       %s %s event_pipe MAP [cpu N index M]\n"
+		"       %s %s peek       MAP\n"
 		"       %s %s help\n"
 		"\n"
 		"       " HELP_SPEC_MAP "\n"
@@ -1185,7 +1186,7 @@ static int do_help(int argc, char **argv)
 		bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2],
 		bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2],
 		bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2],
-		bin_name, argv[-2]);
+		bin_name, argv[-2], bin_name, argv[-2]);
 
 	return 0;
 }
@@ -1202,6 +1203,7 @@ static const struct cmd cmds[] = {
 	{ "pin",	do_pin },
 	{ "event_pipe",	do_event_pipe },
 	{ "create",	do_create },
+	{ "peek",	do_lookup },
 	{ 0 }
 };
 
-- 
2.20.1.97.g81188d93c3-goog

[PATCH bpf-next 5/6] bpftool: add push and enqueue commands

[Stanislav Fomichev]

This is intended to be used with queues and stacks and be more
user-friendly than 'update' without the key.

Example:
bpftool map create /sys/fs/bpf/q type queue value 4 entries 10 name q
bpftool map push pinned /sys/fs/bpf/q value 0 1 2 3
bpftool map peek pinned /sys/fs/bpf/q
value: 00 01 02 03

bpftool map create /sys/fs/bpf/s type stack value 4 entries 10 name s
bpftool map enqueue pinned /sys/fs/bpf/s value 0 1 2 3
bpftool map peek pinned /sys/fs/bpf/s
value: 00 01 02 03

Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
 tools/bpf/bpftool/Documentation/bpftool-map.rst | 8 ++++++++
 tools/bpf/bpftool/bash-completion/bpftool       | 4 ++--
 tools/bpf/bpftool/map.c                         | 7 ++++++-
 3 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/tools/bpf/bpftool/Documentation/bpftool-map.rst b/tools/bpf/bpftool/Documentation/bpftool-map.rst
index b79da683da88..435a79d2eed5 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-map.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-map.rst
@@ -32,6 +32,8 @@ MAP COMMANDS
 |	**bpftool** **map pin**        *MAP*  *FILE*
 |	**bpftool** **map event_pipe** *MAP* [**cpu** *N* **index** *M*]
 |	**bpftool** **map peek**       *MAP*
+|	**bpftool** **map push**       *MAP* **value** *VALUE*
+|	**bpftool** **map enqueue**    *MAP* **value** *VALUE*
 |	**bpftool** **map help**
 |
 |	*MAP* := { **id** *MAP_ID* | **pinned** *FILE* }
@@ -111,6 +113,12 @@ DESCRIPTION
 	**bpftool map peek**  *MAP*
 		  Peek next **value** in the queue or stack.
 
+	**bpftool map push**  *MAP* **value** *VALUE*
+		  Push **value** onto the stack.
+
+	**bpftool map enqueue**  *MAP* **value** *VALUE*
+		  Enqueue **value** into the queue.
+
 	**bpftool map help**
 		  Print short help message.
 
diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
index fe4880d9bd8d..69096e058308 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool
@@ -452,7 +452,7 @@ _bpftool()
                             ;;
                     esac
                     ;;
-                update)
+                update|push|enqueue)
                     case $prev in
                         $command)
                             COMPREPLY=( $( compgen -W "$MAP_TYPE" -- "$cur" ) )
@@ -547,7 +547,7 @@ _bpftool()
                     [[ $prev == $object ]] && \
                         COMPREPLY=( $( compgen -W 'delete dump getnext help \
                             lookup pin event_pipe show list update create \
-                            peek' -- \
+                            peek push enqueue' -- \
                             "$cur" ) )
                     ;;
             esac
diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
index d7344fcd2089..6b5fcbe2d9d4 100644
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@ -1169,6 +1169,8 @@ static int do_help(int argc, char **argv)
 		"       %s %s pin        MAP  FILE\n"
 		"       %s %s event_pipe MAP [cpu N index M]\n"
 		"       %s %s peek       MAP\n"
+		"       %s %s push       MAP value VALUE\n"
+		"       %s %s enqueue    MAP value VALUE\n"
 		"       %s %s help\n"
 		"\n"
 		"       " HELP_SPEC_MAP "\n"
@@ -1186,7 +1188,8 @@ static int do_help(int argc, char **argv)
 		bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2],
 		bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2],
 		bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2],
-		bin_name, argv[-2], bin_name, argv[-2]);
+		bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2],
+		bin_name, argv[-2]);
 
 	return 0;
 }
@@ -1204,6 +1207,8 @@ static const struct cmd cmds[] = {
 	{ "event_pipe",	do_event_pipe },
 	{ "create",	do_create },
 	{ "peek",	do_lookup },
+	{ "push",	do_update },
+	{ "enqueue",	do_update },
 	{ 0 }
 };
 
-- 
2.20.1.97.g81188d93c3-goog

[PATCH bpf-next 6/6] bpftool: add pop and dequeue commands

[Stanislav Fomichev]

This is intended to be used with queues and stacks, it pops and prints
the last element via bpf_map_lookup_and_delete_elem.

Example:

bpftool map create /sys/fs/bpf/q type queue value 4 entries 10 name q
bpftool map push pinned /sys/fs/bpf/q value 0 1 2 3
bpftool map pop pinned /sys/fs/bpf/q
value: 00 01 02 03
bpftool map pop pinned /sys/fs/bpf/q
Error: empty map

bpftool map create /sys/fs/bpf/s type stack value 4 entries 10 name s
bpftool map enqueue pinned /sys/fs/bpf/s value 0 1 2 3
bpftool map dequeue pinned /sys/fs/bpf/s
value: 00 01 02 03
bpftool map dequeue pinned /sys/fs/bpf/s
Error: empty map

Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
 .../bpf/bpftool/Documentation/bpftool-map.rst |   8 ++
 tools/bpf/bpftool/bash-completion/bpftool     |   4 +-
 tools/bpf/bpftool/map.c                       | 127 +++++++++++++-----
 3 files changed, 102 insertions(+), 37 deletions(-)

diff --git a/tools/bpf/bpftool/Documentation/bpftool-map.rst b/tools/bpf/bpftool/Documentation/bpftool-map.rst
index 435a79d2eed5..0584c31d3fe2 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-map.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-map.rst
@@ -33,7 +33,9 @@ MAP COMMANDS
 |	**bpftool** **map event_pipe** *MAP* [**cpu** *N* **index** *M*]
 |	**bpftool** **map peek**       *MAP*
 |	**bpftool** **map push**       *MAP* **value** *VALUE*
+|	**bpftool** **map pop**        *MAP*
 |	**bpftool** **map enqueue**    *MAP* **value** *VALUE*
+|	**bpftool** **map dequeue**    *MAP*
 |	**bpftool** **map help**
 |
 |	*MAP* := { **id** *MAP_ID* | **pinned** *FILE* }
@@ -116,9 +118,15 @@ DESCRIPTION
 	**bpftool map push**  *MAP* **value** *VALUE*
 		  Push **value** onto the stack.
 
+	**bpftool map pop**  *MAP*
+		  Pop and print **value** from the stack.
+
 	**bpftool map enqueue**  *MAP* **value** *VALUE*
 		  Enqueue **value** into the queue.
 
+	**bpftool map dequeue**  *MAP*
+		  Dequeue and print **value** from the queue.
+
 	**bpftool map help**
 		  Print short help message.
 
diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
index 69096e058308..7a015aaa487e 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool
@@ -382,7 +382,7 @@ _bpftool()
         map)
             local MAP_TYPE='id pinned'
             case $command in
-                show|list|dump|peek)
+                show|list|dump|peek|pop|dequeue)
                     case $prev in
                         $command)
                             COMPREPLY=( $( compgen -W "$MAP_TYPE" -- "$cur" ) )
@@ -547,7 +547,7 @@ _bpftool()
                     [[ $prev == $object ]] && \
                         COMPREPLY=( $( compgen -W 'delete dump getnext help \
                             lookup pin event_pipe show list update create \
-                            peek push enqueue' -- \
+                            peek push enqueue pop dequeue' -- \
                             "$cur" ) )
                     ;;
             esac
diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
index 6b5fcbe2d9d4..2187ba02185e 100644
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@ -857,12 +857,51 @@ static int do_update(int argc, char **argv)
 	return err;
 }
 
+static void print_key_value(struct bpf_map_info *info, void *key,
+			    void *value)
+{
+	json_writer_t *btf_wtr;
+	struct btf *btf = NULL;
+	int err;
+
+	err = btf__get_from_id(info->btf_id, &btf);
+	if (err) {
+		p_err("failed to get btf");
+		return;
+	}
+
+	if (json_output) {
+		print_entry_json(info, key, value, btf);
+	} else if (btf) {
+		/* if here json_wtr wouldn't have been initialised,
+		 * so let's create separate writer for btf
+		 */
+		btf_wtr = get_btf_writer();
+		if (!btf_wtr) {
+			p_info("failed to create json writer for btf. falling back to plain output");
+			btf__free(btf);
+			btf = NULL;
+			print_entry_plain(info, key, value);
+		} else {
+			struct btf_dumper d = {
+				.btf = btf,
+				.jw = btf_wtr,
+				.is_plain_text = true,
+			};
+
+			do_dump_btf(&d, info, key, value);
+			jsonw_destroy(&btf_wtr);
+		}
+	} else {
+		print_entry_plain(info, key, value);
+	}
+	btf__free(btf);
+}
+
 static int do_lookup(int argc, char **argv)
 {
 	struct bpf_map_info info = {};
 	__u32 len = sizeof(info);
-	json_writer_t *btf_wtr;
-	struct btf *btf = NULL;
 	void *key, *value;
 	int err;
 	int fd;
@@ -900,43 +939,12 @@ static int do_lookup(int argc, char **argv)
 	}
 
 	/* here means bpf_map_lookup_elem() succeeded */
-	err = btf__get_from_id(info.btf_id, &btf);
-	if (err) {
-		p_err("failed to get btf");
-		goto exit_free;
-	}
-
-	if (json_output) {
-		print_entry_json(&info, key, value, btf);
-	} else if (btf) {
-		/* if here json_wtr wouldn't have been initialised,
-		 * so let's create separate writer for btf
-		 */
-		btf_wtr = get_btf_writer();
-		if (!btf_wtr) {
-			p_info("failed to create json writer for btf. falling back to plain output");
-			btf__free(btf);
-			btf = NULL;
-			print_entry_plain(&info, key, value);
-		} else {
-			struct btf_dumper d = {
-				.btf = btf,
-				.jw = btf_wtr,
-				.is_plain_text = true,
-			};
-
-			do_dump_btf(&d, &info, key, value);
-			jsonw_destroy(&btf_wtr);
-		}
-	} else {
-		print_entry_plain(&info, key, value);
-	}
+	print_key_value(&info, key, value);
 
 exit_free:
 	free(key);
 	free(value);
 	close(fd);
-	btf__free(btf);
 
 	return err;
 }
@@ -1149,6 +1157,51 @@ static int do_create(int argc, char **argv)
 	return 0;
 }
 
+static int do_pop_dequeue(int argc, char **argv)
+{
+	struct bpf_map_info info = {};
+	__u32 len = sizeof(info);
+	void *key, *value;
+	int err;
+	int fd;
+
+	if (argc < 2)
+		usage();
+
+	fd = map_parse_fd_and_info(&argc, &argv, &info, &len);
+	if (fd < 0)
+		return -1;
+
+	err = alloc_key_value(&info, &key, &value);
+	if (err)
+		goto exit_free;
+
+	err = bpf_map_lookup_and_delete_elem(fd, key, value);
+	if (err) {
+		if (errno == ENOENT) {
+			if (json_output)
+				jsonw_null(json_wtr);
+			else
+				printf("Error: empty map\n");
+		} else {
+			p_err("pop failed: %s", strerror(errno));
+		}
+
+		goto exit_free;
+	}
+
+	print_key_value(&info, key, value);
+
+exit_free:
+	free(key);
+	free(value);
+	close(fd);
+
+	if (!err && json_output)
+		jsonw_null(json_wtr);
+	return err;
+}
+
 static int do_help(int argc, char **argv)
 {
 	if (json_output) {
@@ -1170,7 +1223,9 @@ static int do_help(int argc, char **argv)
 		"       %s %s event_pipe MAP [cpu N index M]\n"
 		"       %s %s peek       MAP\n"
 		"       %s %s push       MAP value VALUE\n"
+		"       %s %s pop        MAP\n"
 		"       %s %s enqueue    MAP value VALUE\n"
+		"       %s %s dequeue    MAP\n"
 		"       %s %s help\n"
 		"\n"
 		"       " HELP_SPEC_MAP "\n"
@@ -1189,7 +1244,7 @@ static int do_help(int argc, char **argv)
 		bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2],
 		bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2],
 		bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2],
-		bin_name, argv[-2]);
+		bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2]);
 
 	return 0;
 }
@@ -1209,6 +1264,8 @@ static const struct cmd cmds[] = {
 	{ "peek",	do_lookup },
 	{ "push",	do_update },
 	{ "enqueue",	do_update },
+	{ "pop",	do_pop_dequeue },
+	{ "dequeue",	do_pop_dequeue },
 	{ 0 }
 };
 
-- 
2.20.1.97.g81188d93c3-goog

Re: [PATCH bpf-next 6/6] bpftool: add pop and dequeue commands

[Jakub Kicinski]

On Tue, 15 Jan 2019 15:22:52 -0800, Stanislav Fomichev wrote:
> +static int do_pop_dequeue(int argc, char **argv)
> +{
> +	struct bpf_map_info info = {};
> +	__u32 len = sizeof(info);
> +	void *key, *value;
> +	int err;
> +	int fd;
> +
> +	if (argc < 2)
> +		usage();
> +
> +	fd = map_parse_fd_and_info(&argc, &argv, &info, &len);
> +	if (fd < 0)
> +		return -1;
> +
> +	err = alloc_key_value(&info, &key, &value);
> +	if (err)
> +		goto exit_free;
> +
> +	err = bpf_map_lookup_and_delete_elem(fd, key, value);
> +	if (err) {
> +		if (errno == ENOENT) {
> +			if (json_output)
> +				jsonw_null(json_wtr);
> +			else
> +				printf("Error: empty map\n");
> +		} else {
> +			p_err("pop failed: %s", strerror(errno));
> +		}
> +
> +		goto exit_free;
> +	}
> +
> +	print_key_value(&info, key, value);
> +
> +exit_free:
> +	free(key);
> +	free(value);
> +	close(fd);
> +
> +	if (!err && json_output)
> +		jsonw_null(json_wtr);

These two lines look out of place?

> +	return err;
> +}

Re: [PATCH bpf-next 0/6] bpftool: support queue and stack

[Jakub Kicinski]

On Tue, 15 Jan 2019 15:22:46 -0800, Stanislav Fomichev wrote:
> This patch series add support for queue/stack manipulations.
> 
> It goes like this:
> 
> #1 and #2 add support for queue/stack in existing 'update' and 'lookup'
> commands by permitting empty keys.
> #3 make sure we don't print empty keys for queue/stack.
> #4 adds peek command, it's essentially an alias for 'lookup'.
> #5 adds push/enqueue commands, an alias to 'update'.
> #6 adds pop/dequeue commands that use bpf_map_lookup_and_delete_elem.
> 
> (Not sure whether it makes sense to have push/enqueue + pop/dequeue or
> just have push/pop for both stack/queue, comments are welcome).

Other than one part of last patch looks good!

I'd personally be tempted to only allow the peek/push/pop commands for
stacks/queues, i.e. for maps which don't have a key.  Right now even
though the help says:

	**bpftool** **map peek**       *MAP*

it's just an alias to lookup, so key can be specified..

Could bash completions also only suggest them when the map type is
right (and perhaps vice versa - not suggest lookup etc.)?

Re: [PATCH bpf-next 6/6] bpftool: add pop and dequeue commands

[Stanislav Fomichev]

On 01/15, Jakub Kicinski wrote:
> On Tue, 15 Jan 2019 15:22:52 -0800, Stanislav Fomichev wrote:
> > +static int do_pop_dequeue(int argc, char **argv)
> > +{
> > +	struct bpf_map_info info = {};
> > +	__u32 len = sizeof(info);
> > +	void *key, *value;
> > +	int err;
> > +	int fd;
> > +
> > +	if (argc < 2)
> > +		usage();
> > +
> > +	fd = map_parse_fd_and_info(&argc, &argv, &info, &len);
> > +	if (fd < 0)
> > +		return -1;
> > +
> > +	err = alloc_key_value(&info, &key, &value);
> > +	if (err)
> > +		goto exit_free;
> > +
> > +	err = bpf_map_lookup_and_delete_elem(fd, key, value);
> > +	if (err) {
> > +		if (errno == ENOENT) {
> > +			if (json_output)
> > +				jsonw_null(json_wtr);
> > +			else
> > +				printf("Error: empty map\n");
> > +		} else {
> > +			p_err("pop failed: %s", strerror(errno));
> > +		}
> > +
> > +		goto exit_free;
> > +	}
> > +
> > +	print_key_value(&info, key, value);
> > +
> > +exit_free:
> > +	free(key);
> > +	free(value);
> > +	close(fd);
> > +
> > +	if (!err && json_output)
> > +		jsonw_null(json_wtr);
> 
> These two lines look out of place?
Ah, indeed, sorry about that. Will remove in v2.
> 
> > +	return err;
> > +}

Re: [PATCH bpf-next 0/6] bpftool: support queue and stack

[Stanislav Fomichev]

On 01/15, Jakub Kicinski wrote:
> On Tue, 15 Jan 2019 15:22:46 -0800, Stanislav Fomichev wrote:
> > This patch series add support for queue/stack manipulations.
> > 
> > It goes like this:
> > 
> > #1 and #2 add support for queue/stack in existing 'update' and 'lookup'
> > commands by permitting empty keys.
> > #3 make sure we don't print empty keys for queue/stack.
> > #4 adds peek command, it's essentially an alias for 'lookup'.
> > #5 adds push/enqueue commands, an alias to 'update'.
> > #6 adds pop/dequeue commands that use bpf_map_lookup_and_delete_elem.
> > 
> > (Not sure whether it makes sense to have push/enqueue + pop/dequeue or
> > just have push/pop for both stack/queue, comments are welcome).
> 
> Other than one part of last patch looks good!
> 
> I'd personally be tempted to only allow the peek/push/pop commands for
> stacks/queues, i.e. for maps which don't have a key.  Right now even
> though the help says:
> 
> 	**bpftool** **map peek**       *MAP*
> 
> it's just an alias to lookup, so key can be specified..
> 
> Could bash completions also only suggest them when the map type is
> right (and perhaps vice versa - not suggest lookup etc.)?
I can look into bash completion. I'm not at all familiar with it.

Also, let me maybe add some checks in the do_lookup/do_update which
print an error in case key is specified when info->key_size is zero (and
the same for value).