[PATCH v2 bpf-next 0/6] BPF sk local storage

[PATCH v2 bpf-next 0/6] BPF sk local storage

[Martin KaFai Lau]

v2:
- Add the "test_maps.h" file in patch 5

This series introduces the BPF sk local storage.  The
details is in the patch 1 commit message.

Martin KaFai Lau (6):
  bpf: Introduce bpf sk local storage
  bpf: Sync bpf.h to tools
  bpf: Support BPF_MAP_TYPE_SK_STORAGE in bpf map probing
  bpf: Add verifier tests for the bpf_sk_storage
  bpf: Add BPF_MAP_TYPE_SK_STORAGE test to test_maps
  bpf: Add ene-to-end test for bpf_sk_storage_* helpers

 include/linux/bpf.h                           |   2 +
 include/linux/bpf_types.h                     |   1 +
 include/net/bpf_sk_storage.h                  |  13 +
 include/net/sock.h                            |   5 +
 include/uapi/linux/bpf.h                      |  44 +-
 kernel/bpf/syscall.c                          |   3 +-
 kernel/bpf/verifier.c                         |  27 +-
 net/bpf/test_run.c                            |   2 +
 net/core/Makefile                             |   1 +
 net/core/bpf_sk_storage.c                     | 796 ++++++++++++++++++
 net/core/filter.c                             |  12 +
 net/core/sock.c                               |   5 +
 tools/bpf/bpftool/map.c                       |   1 +
 tools/include/uapi/linux/bpf.h                |  44 +-
 tools/lib/bpf/libbpf_probes.c                 |  74 +-
 tools/testing/selftests/bpf/Makefile          |  25 +-
 tools/testing/selftests/bpf/bpf_helpers.h     |   5 +
 .../selftests/bpf/map_tests/sk_storage_map.c  | 638 ++++++++++++++
 .../bpf/progs/test_sock_fields_kern.c         |  49 ++
 tools/testing/selftests/bpf/test_maps.c       |  18 +-
 tools/testing/selftests/bpf/test_maps.h       |  17 +
 .../testing/selftests/bpf/test_sock_fields.c  | 115 ++-
 tools/testing/selftests/bpf/test_verifier.c   |  42 +-
 tools/testing/selftests/bpf/verifier/sock.c   | 116 +++
 24 files changed, 2015 insertions(+), 40 deletions(-)
 create mode 100644 include/net/bpf_sk_storage.h
 create mode 100644 net/core/bpf_sk_storage.c
 create mode 100644 tools/testing/selftests/bpf/map_tests/sk_storage_map.c
 create mode 100644 tools/testing/selftests/bpf/test_maps.h

-- 
2.17.1

[PATCH v2 bpf-next 1/6] bpf: Introduce bpf sk local storage

[Martin KaFai Lau]

After allowing a bpf prog to
- directly read the skb->sk ptr
- get the fullsock bpf_sock by "bpf_sk_fullsock()"
- get the bpf_tcp_sock by "bpf_tcp_sock()"
- get the listener sock by "bpf_get_listener_sock()"
- avoid duplicating the fields of "(bpf_)sock" and "(bpf_)tcp_sock"
  into different bpf running context.

this patch is another effort to make bpf's network programming
more intuitive to do (together with memory and performance benefit).

When bpf prog needs to store data for a sk, the current practice is to
define a map with the usual 4-tuples (src/dst ip/port) as the key.
If multiple bpf progs require to store different sk data, multiple maps
have to be defined.  Hence, wasting memory to store the duplicated
keys (i.e. 4 tuples here) in each of the bpf map.
[ The smallest key could be the sk pointer itself which requires
  some enhancement in the verifier and it is a separate topic. ]

Also, the bpf prog needs to clean up the elem when sk is freed.
Otherwise, the bpf map will become full and un-usable quickly.
The sk-free tracking currently could be done during sk state
transition (e.g. BPF_SOCK_OPS_STATE_CB).

The size of the map needs to be predefined which then usually ended-up
with an over-provisioned map in production.  Even the map was re-sizable,
while the sk naturally come and go away already, this potential re-size
operation is arguably redundant if the data can be directly connected
to the sk itself instead of proxy-ing through a bpf map.

This patch introduces sk->sk_bpf_storage to provide local storage space
at sk for bpf prog to use.  The space will be allocated when the first bpf
prog has created data for this particular sk.

The design optimizes the bpf prog's lookup (and then optionally followed by
an inline update).  bpf_spin_lock should be used if the inline update needs
to be protected.

BPF_MAP_TYPE_SK_STORAGE:
-----------------------
To define a bpf "sk-local-storage", a BPF_MAP_TYPE_SK_STORAGE map (new in
this patch) needs to be created.  Multiple BPF_MAP_TYPE_SK_STORAGE maps can
be created to fit different bpf progs' needs.  The map enforces
BTF to allow printing the sk-local-storage during a system-wise
sk dump (e.g. "ss -ta") in the future.

The purpose of a BPF_MAP_TYPE_SK_STORAGE map is not for lookup/update/delete
a "sk-local-storage" data from a particular sk.
Think of the map as a meta-data (or "type") of a "sk-local-storage".  This
particular "type" of "sk-local-storage" data can then be stored in any sk.

The main purposes of this map are mostly:
1. Define the size of a "sk-local-storage" type.
2. Provide a similar syscall userspace API as the map (e.g. lookup/update,
   map-id, map-btf...etc.)
3. Keep track of all sk's storages of this "type" and clean them up
   when the map is freed.

sk->sk_bpf_storage:
------------------
The main lookup/update/delete is done on sk->sk_bpf_storage (which
is a "struct bpf_sk_storage").  When doing a lookup,
the "map" pointer is now used as the "key" to search on the
sk_storage->list.  The "map" pointer is actually serving
as the "type" of the "sk-local-storage" that is being
requested.

To allow very fast lookup, it should be as fast as looking up an
array at a stable-offset.  At the same time, it is not ideal to
set a hard limit on the number of sk-local-storage "type" that the
system can have.  Hence, this patch takes a cache approach.
The last search result from sk_storage->list is cached in
sk_storage->cache[] which is a stable sized array.  Each
"sk-local-storage" type has a stable offset to the cache[] array.
In the future, a map's flag could be introduced to do cache
opt-out/enforcement if it became necessary.

All data will be removed from sk->sk_bpf_storage during
sk destruction.

bpf_sk_storage_get() and bpf_sk_storage_delete():
------------------------------------------------
Instead of using bpf_map_(lookup|update|delete)_elem(),
the bpf prog needs to use the new helper bpf_sk_storage_get() and
bpf_sk_storage_delete().  The verifier can then enforce the
ARG_PTR_TO_SOCKET argument.  The bpf_sk_storage_get() also allows to
"create" new elem if one does not exist in the sk.  It is done by
the new BPF_SK_STORAGE_GET_F_CREATE flag.  An optional value can also be
provided as the initial value during BPF_SK_STORAGE_GET_F_CREATE.
The BPF_MAP_TYPE_SK_STORAGE also supports bpf_spin_lock.  Together,
it has eliminated the potential use cases for an equivalent
bpf_map_update_elem() API (for bpf_prog) in this patch.

Misc notes:
----------
1. map_get_next_key is not supported.  From the userspace syscall
   perspective,  the map has the socket fd as the key while the map
   can be shared by pinned-file or map-id.

   Since btf is enforced, the existing "ss" could be enhanced to pretty
   print the local-storage.

   Supporting a kernel defined btf with 4 tuples as the return key could
   be explored later also.

2. The sk->sk_lock cannot be acquired.  Atomic operations is used instead.
   e.g. cmpxchg is done on the sk->sk_bpf_storage ptr.
   Please refer to the source code comments for the details in
   synchronization cases and considerations.

3. The mem is charged to the sk->sk_omem_alloc as the sk filter does.

Benchmark:
---------
Here is the benchmark data collected by turning on
the "kernel.bpf_stats_enabled" sysctl.
Two bpf progs are tested:

One bpf prog with the usual bpf hashmap (max_entries = 8192) with the
sk ptr as the key. (verifier is modified to support sk ptr as the key
That should have shortened the key lookup time.)

Another bpf prog is with the new BPF_MAP_TYPE_SK_STORAGE.

Both are storing a "u32 cnt", do a lookup on "egress_skb/cgroup" for
each egress skb and then bump the cnt.  netperf is used to drive
data with 4096 connected UDP sockets.

BPF_MAP_TYPE_HASH with a modifier verifier (152ns per bpf run)
27: cgroup_skb  name egress_sk_map  tag 74f56e832918070b run_time_ns 58280107540 run_cnt 381347633
    loaded_at 2019-04-15T13:46:39-0700  uid 0
    xlated 344B  jited 258B  memlock 4096B  map_ids 16
    btf_id 5

BPF_MAP_TYPE_SK_STORAGE in this patch (66ns per bpf run)
30: cgroup_skb  name egress_sk_stora  tag d4aa70984cc7bbf6 run_time_ns 25617093319 run_cnt 390989739
    loaded_at 2019-04-15T13:47:54-0700  uid 0
    xlated 168B  jited 156B  memlock 4096B  map_ids 17
    btf_id 6

Here is a high-level picture on how are the objects organized:

       sk
    +------+
    |      |
    |      |
    |      |
    |*sk_bpf_storage-----> bpf_sk_storage
    +------+                 +-------+
                 +-----------+ list  |
                 |           |       |
                 |           |       |
                 |           |       |
                 |           +-------+
                 |
                 |     elem
                 |  +--------+
                 +->| snode  |
                 |  +--------+
                 |  |  data  |          bpf_map
                 |  +--------+        +---------+
                 |  |map_node|<-+-----+  list   |
                 |  +--------+  |     |         |
                 |              |     |         |
                 |     elem     |     |         |
                 |  +--------+  |     +---------+
                 +->| snode  |  |
                    +--------+  |
   bpf_map          |  data  |  |
 +---------+        +--------+  |
 |  list   +------->|map_node|  |
 |         |        +--------+  |
 |         |                    |
 |         |           elem     |
 +---------+        +--------+  |
                 +->| snode  |  |
                 |  +--------+  |
                 |  |  data  |  |
                 |  +--------+  |
                 |  |map_node|<-+
                 |  +--------+
                 |
                 |
                 |          +-------+
     sk          +----------| list  |
  +------+                  |       |
  |      |                  |       |
  |      |                  |       |
  |      |                  +-------+
  |*sk_bpf_storage------->bpf_sk_storage
  +------+

Signed-off-by: Martin KaFai Lau <kafai@fb.com>
---
 include/linux/bpf.h          |   2 +
 include/linux/bpf_types.h    |   1 +
 include/net/bpf_sk_storage.h |  13 +
 include/net/sock.h           |   5 +
 include/uapi/linux/bpf.h     |  44 +-
 kernel/bpf/syscall.c         |   3 +-
 kernel/bpf/verifier.c        |  27 +-
 net/bpf/test_run.c           |   2 +
 net/core/Makefile            |   1 +
 net/core/bpf_sk_storage.c    | 796 +++++++++++++++++++++++++++++++++++
 net/core/filter.c            |  12 +
 net/core/sock.c              |   5 +
 12 files changed, 906 insertions(+), 5 deletions(-)
 create mode 100644 include/net/bpf_sk_storage.h
 create mode 100644 net/core/bpf_sk_storage.c

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index f15432d90728..0d2788beacd2 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -184,6 +184,7 @@ enum bpf_arg_type {
 	ARG_PTR_TO_MAP_KEY,	/* pointer to stack used as map key */
 	ARG_PTR_TO_MAP_VALUE,	/* pointer to stack used as map value */
 	ARG_PTR_TO_UNINIT_MAP_VALUE,	/* pointer to valid memory used to store a map value */
+	ARG_PTR_TO_MAP_VALUE_OR_NULL,	/* pointer to stack used as map value or NULL */
 
 	/* the following constraints used to prototype bpf_memcmp() and other
 	 * functions that access data on eBPF program stack
@@ -204,6 +205,7 @@ enum bpf_arg_type {
 	ARG_PTR_TO_SOCK_COMMON,	/* pointer to sock_common */
 	ARG_PTR_TO_INT,		/* pointer to int */
 	ARG_PTR_TO_LONG,	/* pointer to long */
+	ARG_PTR_TO_SOCKET,	/* pointer to bpf_sock (fullsock) */
 };
 
 /* type of values returned from helper functions */
diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
index d26991a16894..eb1e4f867a5e 100644
--- a/include/linux/bpf_types.h
+++ b/include/linux/bpf_types.h
@@ -60,6 +60,7 @@ BPF_MAP_TYPE(BPF_MAP_TYPE_ARRAY_OF_MAPS, array_of_maps_map_ops)
 BPF_MAP_TYPE(BPF_MAP_TYPE_HASH_OF_MAPS, htab_of_maps_map_ops)
 #ifdef CONFIG_NET
 BPF_MAP_TYPE(BPF_MAP_TYPE_DEVMAP, dev_map_ops)
+BPF_MAP_TYPE(BPF_MAP_TYPE_SK_STORAGE, sk_storage_map_ops)
 #if defined(CONFIG_BPF_STREAM_PARSER)
 BPF_MAP_TYPE(BPF_MAP_TYPE_SOCKMAP, sock_map_ops)
 BPF_MAP_TYPE(BPF_MAP_TYPE_SOCKHASH, sock_hash_ops)
diff --git a/include/net/bpf_sk_storage.h b/include/net/bpf_sk_storage.h
new file mode 100644
index 000000000000..b9dcb02e756b
--- /dev/null
+++ b/include/net/bpf_sk_storage.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (c) 2019 Facebook */
+#ifndef _BPF_SK_STORAGE_H
+#define _BPF_SK_STORAGE_H
+
+struct sock;
+
+void bpf_sk_storage_free(struct sock *sk);
+
+extern const struct bpf_func_proto bpf_sk_storage_get_proto;
+extern const struct bpf_func_proto bpf_sk_storage_delete_proto;
+
+#endif /* _BPF_SK_STORAGE_H */
diff --git a/include/net/sock.h b/include/net/sock.h
index 784cd19d5ff7..4d208c0f9c14 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -236,6 +236,8 @@ struct sock_common {
 	/* public: */
 };
 
+struct bpf_sk_storage;
+
 /**
   *	struct sock - network layer representation of sockets
   *	@__sk_common: shared layout with inet_timewait_sock
@@ -510,6 +512,9 @@ struct sock {
 #endif
 	void                    (*sk_destruct)(struct sock *sk);
 	struct sock_reuseport __rcu	*sk_reuseport_cb;
+#ifdef CONFIG_BPF_SYSCALL
+	struct bpf_sk_storage __rcu	*sk_bpf_storage;
+#endif
 	struct rcu_head		sk_rcu;
 };
 
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index eaf2d3284248..080ff8313ced 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -133,6 +133,7 @@ enum bpf_map_type {
 	BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE,
 	BPF_MAP_TYPE_QUEUE,
 	BPF_MAP_TYPE_STACK,
+	BPF_MAP_TYPE_SK_STORAGE,
 };
 
 /* Note that tracing related programs such as
@@ -2629,6 +2630,42 @@ union bpf_attr {
  *		was provided.
  *
  *		**-ERANGE** if resulting value was out of range.
+ *
+ * void *bpf_sk_storage_get(struct bpf_map *map, struct bpf_sock *sk, void *value, u64 flags)
+ *	Description
+ *		Get a bpf-local-storage from a sk.
+ *
+ *		Logically, it could be thought of getting the value from
+ *		a *map* with *sk* as the **key**.  From this
+ *		perspective,  the usage is not much different from
+ *		**bpf_map_lookup_elem(map, &sk)** except this
+ *		helper enforces the key must be a **bpf_fullsock()**
+ *		and the map must be a BPF_MAP_TYPE_SK_STORAGE also.
+ *
+ *		Underneath, the value is stored locally at *sk* instead of
+ *		the map.  The *map* is used as the bpf-local-storage **type**.
+ *		The bpf-local-storage **type** (i.e. the *map*) is searched
+ *		against all bpf-local-storages residing at sk.
+ *
+ *		An optional *flags* (BPF_SK_STORAGE_GET_F_CREATE) can be
+ *		used such that a new bpf-local-storage will be
+ *		created if one does not exist.  *value* can be used
+ *		together with BPF_SK_STORAGE_GET_F_CREATE to specify
+ *		the initial value of a bpf-local-storage.  If *value* is
+ *		NULL, the new bpf-local-storage will be zero initialized.
+ *	Return
+ *		A bpf-local-storage pointer is returned on success.
+ *
+ *		**NULL** if not found or there was an error in adding
+ *		a new bpf-local-storage.
+ *
+ * int bpf_sk_storage_delete(struct bpf_map *map, struct bpf_sock *sk)
+ *	Description
+ *		Delete a bpf-local-storage from a sk.
+ *	Return
+ *		0 on success.
+ *
+ *		**-ENOENT** if the bpf-local-storage cannot be found.
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -2737,7 +2774,9 @@ union bpf_attr {
 	FN(sysctl_get_new_value),	\
 	FN(sysctl_set_new_value),	\
 	FN(strtol),			\
-	FN(strtoul),
+	FN(strtoul),			\
+	FN(sk_storage_get),		\
+	FN(sk_storage_delete),
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
  * function eBPF program intends to call
@@ -2813,6 +2852,9 @@ enum bpf_func_id {
 /* BPF_FUNC_sysctl_get_name flags. */
 #define BPF_F_SYSCTL_BASE_NAME		(1ULL << 0)
 
+/* BPF_FUNC_sk_storage_get flags */
+#define BPF_SK_STORAGE_GET_F_CREATE	(1ULL << 0)
+
 /* Mode for BPF_FUNC_skb_adjust_room helper. */
 enum bpf_adj_room_mode {
 	BPF_ADJ_ROOM_NET,
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 92c9b8a32b50..69c9cb7cf5f7 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -526,7 +526,8 @@ static int map_check_btf(struct bpf_map *map, const struct btf *btf,
 			return -EACCES;
 		if (map->map_type != BPF_MAP_TYPE_HASH &&
 		    map->map_type != BPF_MAP_TYPE_ARRAY &&
-		    map->map_type != BPF_MAP_TYPE_CGROUP_STORAGE)
+		    map->map_type != BPF_MAP_TYPE_CGROUP_STORAGE &&
+		    map->map_type != BPF_MAP_TYPE_SK_STORAGE)
 			return -ENOTSUPP;
 		if (map->spin_lock_off + sizeof(struct bpf_spin_lock) >
 		    map->value_size) {
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index dc01abd27849..6ed13a03cdec 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -2525,10 +2525,15 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 regno,
 
 	if (arg_type == ARG_PTR_TO_MAP_KEY ||
 	    arg_type == ARG_PTR_TO_MAP_VALUE ||
-	    arg_type == ARG_PTR_TO_UNINIT_MAP_VALUE) {
+	    arg_type == ARG_PTR_TO_UNINIT_MAP_VALUE ||
+	    arg_type == ARG_PTR_TO_MAP_VALUE_OR_NULL) {
 		expected_type = PTR_TO_STACK;
-		if (!type_is_pkt_pointer(type) && type != PTR_TO_MAP_VALUE &&
-		    type != expected_type)
+		if (register_is_null(reg) &&
+		    arg_type == ARG_PTR_TO_MAP_VALUE_OR_NULL)
+			/* final test in check_stack_boundary() */;
+		else if (!type_is_pkt_pointer(type) &&
+			 type != PTR_TO_MAP_VALUE &&
+			 type != expected_type)
 			goto err_type;
 	} else if (arg_type == ARG_CONST_SIZE ||
 		   arg_type == ARG_CONST_SIZE_OR_ZERO) {
@@ -2560,6 +2565,10 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 regno,
 			}
 			meta->ref_obj_id = reg->ref_obj_id;
 		}
+	} else if (arg_type == ARG_PTR_TO_SOCKET) {
+		expected_type = PTR_TO_SOCKET;
+		if (type != expected_type)
+			goto err_type;
 	} else if (arg_type == ARG_PTR_TO_SPIN_LOCK) {
 		if (meta->func_id == BPF_FUNC_spin_lock) {
 			if (process_spin_lock(env, regno, true))
@@ -2617,6 +2626,8 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 regno,
 					      meta->map_ptr->key_size, false,
 					      NULL);
 	} else if (arg_type == ARG_PTR_TO_MAP_VALUE ||
+		   (arg_type == ARG_PTR_TO_MAP_VALUE_OR_NULL &&
+		    !register_is_null(reg)) ||
 		   arg_type == ARG_PTR_TO_UNINIT_MAP_VALUE) {
 		/* bpf_map_xxx(..., map_ptr, ..., value) call:
 		 * check [value, value + map->value_size) validity
@@ -2766,6 +2777,11 @@ static int check_map_func_compatibility(struct bpf_verifier_env *env,
 		    func_id != BPF_FUNC_map_push_elem)
 			goto error;
 		break;
+	case BPF_MAP_TYPE_SK_STORAGE:
+		if (func_id != BPF_FUNC_sk_storage_get &&
+		    func_id != BPF_FUNC_sk_storage_delete)
+			goto error;
+		break;
 	default:
 		break;
 	}
@@ -2829,6 +2845,11 @@ static int check_map_func_compatibility(struct bpf_verifier_env *env,
 		    map->map_type != BPF_MAP_TYPE_STACK)
 			goto error;
 		break;
+	case BPF_FUNC_sk_storage_get:
+	case BPF_FUNC_sk_storage_delete:
+		if (map->map_type != BPF_MAP_TYPE_SK_STORAGE)
+			goto error;
+		break;
 	default:
 		break;
 	}
diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index 8606e5aef0b6..8c9331913753 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -10,6 +10,7 @@
 #include <linux/etherdevice.h>
 #include <linux/filter.h>
 #include <linux/sched/signal.h>
+#include <net/bpf_sk_storage.h>
 #include <net/sock.h>
 #include <net/tcp.h>
 
@@ -331,6 +332,7 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
 				     sizeof(struct __sk_buff));
 out:
 	kfree_skb(skb);
+	bpf_sk_storage_free(sk);
 	kfree(sk);
 	kfree(ctx);
 	return ret;
diff --git a/net/core/Makefile b/net/core/Makefile
index f97d6254e564..a104dc8faafc 100644
--- a/net/core/Makefile
+++ b/net/core/Makefile
@@ -34,3 +34,4 @@ obj-$(CONFIG_HWBM) += hwbm.o
 obj-$(CONFIG_NET_DEVLINK) += devlink.o
 obj-$(CONFIG_GRO_CELLS) += gro_cells.o
 obj-$(CONFIG_FAILOVER) += failover.o
+obj-$(CONFIG_BPF_SYSCALL) += bpf_sk_storage.o
diff --git a/net/core/bpf_sk_storage.c b/net/core/bpf_sk_storage.c
new file mode 100644
index 000000000000..b6008aed4a03
--- /dev/null
+++ b/net/core/bpf_sk_storage.c
@@ -0,0 +1,796 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2019 Facebook  */
+#include <linux/spinlock_types.h>
+#include <linux/rculist.h>
+#include <linux/list.h>
+#include <linux/hash.h>
+#include <linux/types.h>
+#include <linux/bpf.h>
+#include <net/bpf_sk_storage.h>
+#include <net/sock.h>
+#include <uapi/linux/btf.h>
+
+static atomic_t cache_idx;
+
+struct bucket {
+	struct hlist_head list;
+	raw_spinlock_t lock;
+};
+
+/* Thp map is not the primary owner of a bpf_sk_storage_elem.
+ * Instead, the sk->sk_bpf_storage is.
+ *
+ * The map (bpf_sk_storage_map) is for two purposes
+ * 1. Define the size of the "sk local storage".  It is
+ *    the map's value_size.
+ *
+ * 2. Maintain a list to keep track of all elems such
+ *    that they can be cleaned up during the map destruction.
+ *
+ * When a bpf local storage is being looked up for a
+ * particular sk,  the "bpf_map" pointer is actually used
+ * as the "key" to search in the list of elem in
+ * sk->sk_bpf_storage.
+ *
+ * Hence, consider sk->sk_bpf_storage is the mini-map
+ * with the "bpf_map" pointer as the searching key.
+ */
+struct bpf_sk_storage_map {
+	struct bpf_map map;
+	/* Lookup elem does not require accessing the map.
+	 *
+	 * Updating/Deleting requires a bucket lock to
+	 * link/unlink the elem from the map.  Having
+	 * multiple buckets to improve contention.
+	 */
+	struct bucket *buckets;
+	u32 bucket_log;
+	u16 elem_size;
+	u16 cache_idx;
+};
+
+struct bpf_sk_storage_data {
+	/* smap is used as the searching key when looking up
+	 * from sk->sk_bpf_storage.
+	 *
+	 * Put it in the same cacheline as the data to minimize
+	 * the number of cachelines access during the cache hit case.
+	 */
+	struct bpf_sk_storage_map __rcu *smap;
+	u8 data[0] __aligned(8);
+};
+
+/* Linked to bpf_sk_storage and bpf_sk_storage_map */
+struct bpf_sk_storage_elem {
+	struct hlist_node map_node;	/* Linked to bpf_sk_storage_map */
+	struct hlist_node snode;	/* Linked to bpf_sk_storage */
+	struct bpf_sk_storage __rcu *sk_storage;
+	struct rcu_head rcu;
+	/* 8 bytes hole */
+	/* The data is stored in aother cacheline to minimize
+	 * the number of cachelines access during a cache hit.
+	 */
+	struct bpf_sk_storage_data sdata ____cacheline_aligned;
+};
+
+#define SELEM(_SDATA) container_of((_SDATA), struct bpf_sk_storage_elem, sdata)
+#define SDATA(_SELEM) (&(_SELEM)->sdata)
+#define BPF_SK_STORAGE_CACHE_SIZE	16
+
+struct bpf_sk_storage {
+	struct bpf_sk_storage_data __rcu *cache[BPF_SK_STORAGE_CACHE_SIZE];
+	struct hlist_head list;	/* List of bpf_sk_storage_elem */
+	struct sock *sk;	/* The sk that owns the the above "list" of
+				 * bpf_sk_storage_elem.
+				 */
+	struct rcu_head rcu;
+	raw_spinlock_t lock;	/* Protect adding/removing from the "list" */
+};
+
+static struct bucket *select_bucket(struct bpf_sk_storage_map *smap,
+				    struct bpf_sk_storage_elem *selem)
+{
+	return &smap->buckets[hash_ptr(selem, smap->bucket_log)];
+}
+
+static int omem_charge(struct sock *sk, unsigned int size)
+{
+	/* same check as in sock_kmalloc() */
+	if (size <= sysctl_optmem_max &&
+	    atomic_read(&sk->sk_omem_alloc) + size < sysctl_optmem_max) {
+		atomic_add(size, &sk->sk_omem_alloc);
+		return 0;
+	}
+
+	return -ENOMEM;
+}
+
+static bool selem_linked_to_sk(const struct bpf_sk_storage_elem *selem)
+{
+	return !hlist_unhashed(&selem->snode);
+}
+
+static bool selem_linked_to_map(const struct bpf_sk_storage_elem *selem)
+{
+	return !hlist_unhashed(&selem->map_node);
+}
+
+static struct bpf_sk_storage_elem *selem_alloc(struct bpf_sk_storage_map *smap,
+					       struct sock *sk, void *value,
+					       bool charge_omem)
+{
+	struct bpf_sk_storage_elem *selem;
+
+	if (charge_omem && omem_charge(sk, smap->elem_size))
+		return NULL;
+
+	selem = kzalloc(smap->elem_size, GFP_ATOMIC | __GFP_NOWARN);
+	if (selem) {
+		if (value)
+			memcpy(SDATA(selem)->data, value, smap->map.value_size);
+		return selem;
+	}
+
+	if (charge_omem)
+		atomic_sub(smap->elem_size, &sk->sk_omem_alloc);
+
+	return NULL;
+}
+
+/* sk_storage->lock must be held and selem->sk_storage == sk_storage.
+ * The caller must ensure selem->smap is still valid to be
+ * dereferenced for its smap->elem_size and smap->cache_idx.
+ */
+static bool __selem_unlink_sk(struct bpf_sk_storage *sk_storage,
+			      struct bpf_sk_storage_elem *selem,
+			      bool uncharge_omem)
+{
+	struct bpf_sk_storage_map *smap;
+	bool free_sk_storage;
+	struct sock *sk;
+
+	smap = rcu_dereference(SDATA(selem)->smap);
+	sk = sk_storage->sk;
+
+	/* All uncharging on sk->sk_omem_alloc must be done first.
+	 * sk may be freed once the last selem is unlinked from sk_storage.
+	 */
+	if (uncharge_omem)
+		atomic_sub(smap->elem_size, &sk->sk_omem_alloc);
+
+	free_sk_storage = hlist_is_singular_node(&selem->snode,
+						 &sk_storage->list);
+	if (free_sk_storage) {
+		atomic_sub(sizeof(struct bpf_sk_storage), &sk->sk_omem_alloc);
+		sk_storage->sk = NULL;
+		/* After this RCU_INIT, sk may be freed and cannot be used */
+		RCU_INIT_POINTER(sk->sk_bpf_storage, NULL);
+
+		/* sk_storage is not freed now.  sk_storage->lock is
+		 * still held and raw_spin_unlock_bh(&sk_storage->lock)
+		 * will be done by the caller.
+		 *
+		 * Although the unlock will be done under
+		 * rcu_read_lock(),  it is more intutivie to
+		 * read if kfree_rcu(sk_storage, rcu) is done
+		 * after the raw_spin_unlock_bh(&sk_storage->lock).
+		 *
+		 * Hence, a "bool free_sk_storage" is returned
+		 * to the caller which then calls the kfree_rcu()
+		 * after unlock.
+		 */
+	}
+	hlist_del_init_rcu(&selem->snode);
+	if (rcu_access_pointer(sk_storage->cache[smap->cache_idx]) ==
+	    SDATA(selem))
+		RCU_INIT_POINTER(sk_storage->cache[smap->cache_idx], NULL);
+
+	kfree_rcu(selem, rcu);
+
+	return free_sk_storage;
+}
+
+static void selem_unlink_sk(struct bpf_sk_storage_elem *selem)
+{
+	struct bpf_sk_storage *sk_storage;
+	bool free_sk_storage = false;
+
+	if (unlikely(!selem_linked_to_sk(selem)))
+		/* selem has already been unlinked from sk */
+		return;
+
+	sk_storage = rcu_dereference(selem->sk_storage);
+	raw_spin_lock_bh(&sk_storage->lock);
+	if (likely(selem_linked_to_sk(selem)))
+		free_sk_storage = __selem_unlink_sk(sk_storage, selem, true);
+	raw_spin_unlock_bh(&sk_storage->lock);
+
+	if (free_sk_storage)
+		kfree_rcu(sk_storage, rcu);
+}
+
+/* sk_storage->lock must be held and sk_storage->list cannot be empty */
+static void __selem_link_sk(struct bpf_sk_storage *sk_storage,
+			    struct bpf_sk_storage_elem *selem)
+{
+	RCU_INIT_POINTER(selem->sk_storage, sk_storage);
+	hlist_add_head(&selem->snode, &sk_storage->list);
+}
+
+static void selem_unlink_map(struct bpf_sk_storage_elem *selem)
+{
+	struct bpf_sk_storage_map *smap;
+	struct bucket *b;
+
+	if (unlikely(!selem_linked_to_map(selem)))
+		/* selem has already be unlinked from smap */
+		return;
+
+	smap = rcu_dereference(SDATA(selem)->smap);
+	b = select_bucket(smap, selem);
+	raw_spin_lock_bh(&b->lock);
+	if (likely(selem_linked_to_map(selem)))
+		hlist_del_init_rcu(&selem->map_node);
+	raw_spin_unlock_bh(&b->lock);
+}
+
+static void selem_link_map(struct bpf_sk_storage_map *smap,
+			   struct bpf_sk_storage_elem *selem)
+{
+	struct bucket *b = select_bucket(smap, selem);
+
+	raw_spin_lock_bh(&b->lock);
+	RCU_INIT_POINTER(SDATA(selem)->smap, smap);
+	hlist_add_head_rcu(&selem->map_node, &b->list);
+	raw_spin_unlock_bh(&b->lock);
+}
+
+static void selem_unlink(struct bpf_sk_storage_elem *selem)
+{
+	/* Always unlink from map before unlinking from sk_storage
+	 * because selem will be freed after successfully unlinked from
+	 * the sk_storage.
+	 */
+	selem_unlink_map(selem);
+	selem_unlink_sk(selem);
+}
+
+static struct bpf_sk_storage_data *
+__sk_storage_lookup(struct bpf_sk_storage *sk_storage,
+		    struct bpf_sk_storage_map *smap,
+		    bool cacheit_lockit)
+{
+	struct bpf_sk_storage_data *sdata;
+	struct bpf_sk_storage_elem *selem;
+
+	/* Fast path (cache hit) */
+	sdata = rcu_dereference(sk_storage->cache[smap->cache_idx]);
+	if (sdata && rcu_access_pointer(sdata->smap) == smap)
+		return sdata;
+
+	/* Slow path (cache miss) */
+	hlist_for_each_entry_rcu(selem, &sk_storage->list, snode)
+		if (rcu_access_pointer(SDATA(selem)->smap) == smap)
+			break;
+
+	if (!selem)
+		return NULL;
+
+	sdata = SDATA(selem);
+	if (cacheit_lockit) {
+		/* spinlock is needed to avoid racing with the
+		 * parallel delete.  Otherwise, publishing an already
+		 * deleted sdata to the cache will become a use-after-free
+		 * problem in the next __sk_storage_lookup().
+		 */
+		raw_spin_lock_bh(&sk_storage->lock);
+		if (selem_linked_to_sk(selem))
+			rcu_assign_pointer(sk_storage->cache[smap->cache_idx],
+					   sdata);
+		raw_spin_unlock_bh(&sk_storage->lock);
+	}
+
+	return sdata;
+}
+
+static struct bpf_sk_storage_data *
+sk_storage_lookup(struct sock *sk, struct bpf_map *map, bool cacheit_lockit)
+{
+	struct bpf_sk_storage *sk_storage;
+	struct bpf_sk_storage_map *smap;
+
+	sk_storage = rcu_dereference(sk->sk_bpf_storage);
+	if (!sk_storage)
+		return NULL;
+
+	smap = (struct bpf_sk_storage_map *)map;
+	return __sk_storage_lookup(sk_storage, smap, cacheit_lockit);
+}
+
+static int check_flags(const struct bpf_sk_storage_data *old_sdata,
+		       u64 map_flags)
+{
+	if (old_sdata && (map_flags & ~BPF_F_LOCK) == BPF_NOEXIST)
+		/* elem already exists */
+		return -EEXIST;
+
+	if (!old_sdata && (map_flags & ~BPF_F_LOCK) == BPF_EXIST)
+		/* elem doesn't exist, cannot update it */
+		return -ENOENT;
+
+	return 0;
+}
+
+static int sk_storage_alloc(struct sock *sk,
+			    struct bpf_sk_storage_map *smap,
+			    struct bpf_sk_storage_elem *first_selem)
+{
+	struct bpf_sk_storage *prev_sk_storage, *sk_storage;
+	int err;
+
+	err = omem_charge(sk, sizeof(*sk_storage));
+	if (err)
+		return err;
+
+	sk_storage = kzalloc(sizeof(*sk_storage), GFP_ATOMIC | __GFP_NOWARN);
+	if (!sk_storage) {
+		err = -ENOMEM;
+		goto uncharge;
+	}
+	INIT_HLIST_HEAD(&sk_storage->list);
+	raw_spin_lock_init(&sk_storage->lock);
+	sk_storage->sk = sk;
+
+	__selem_link_sk(sk_storage, first_selem);
+	selem_link_map(smap, first_selem);
+	/* Publish sk_storage to sk.  sk->sk_lock cannot be acquired.
+	 * Hence, atomic ops is used to set sk->sk_bpf_storage
+	 * from NULL to the newly allocated sk_storage ptr.
+	 *
+	 * From now on, the sk->sk_bpf_storage pointer is protected
+	 * by the sk_storage->lock.  Hence,  when freeing
+	 * the sk->sk_bpf_storage, the sk_storage->lock must
+	 * be held before setting sk->sk_bpf_storage to NULL.
+	 */
+	prev_sk_storage = cmpxchg((struct bpf_sk_storage **)&sk->sk_bpf_storage,
+				  NULL, sk_storage);
+	if (unlikely(prev_sk_storage)) {
+		selem_unlink_map(first_selem);
+		err = -EAGAIN;
+		goto uncharge;
+
+		/* Note that even first_selem was linked to smap's
+		 * bucket->list, first_selem can be freed immediately
+		 * (instead of kfree_rcu) because
+		 * bpf_sk_storage_map_free() does a
+		 * synchronize_rcu() before walking the bucket->list.
+		 * Hence, no one is accessing selem from the
+		 * bucket->list under rcu_read_lock().
+		 */
+	}
+
+	return 0;
+
+uncharge:
+	kfree(sk_storage);
+	atomic_sub(sizeof(*sk_storage), &sk->sk_omem_alloc);
+	return err;
+}
+
+/* sk cannot be going away because it is linking new elem
+ * to sk->sk_bpf_storage. (i.e. sk->sk_refcnt cannot be 0).
+ * Otherwise, it will become a leak (and other memory issues
+ * during map destruction).
+ */
+static struct bpf_sk_storage_data *sk_storage_update(struct sock *sk,
+						     struct bpf_map *map,
+						     void *value,
+						     u64 map_flags)
+{
+	struct bpf_sk_storage_data *old_sdata = NULL;
+	struct bpf_sk_storage_elem *selem;
+	struct bpf_sk_storage *sk_storage;
+	struct bpf_sk_storage_map *smap;
+	int err;
+
+	/* BPF_EXIST and BPF_NOEXIST cannot be both set */
+	if (unlikely((map_flags & ~BPF_F_LOCK) > BPF_EXIST) ||
+	    /* BPF_F_LOCK can only be used in a value with spin_lock */
+	    unlikely((map_flags & BPF_F_LOCK) && !map_value_has_spin_lock(map)))
+		return ERR_PTR(-EINVAL);
+
+	smap = (struct bpf_sk_storage_map *)map;
+	sk_storage = rcu_dereference(sk->sk_bpf_storage);
+	if (!sk_storage || hlist_empty(&sk_storage->list)) {
+		/* Very first elem for this sk */
+		err = check_flags(NULL, map_flags);
+		if (err)
+			return ERR_PTR(err);
+
+		selem = selem_alloc(smap, sk, value, true);
+		if (!selem)
+			return ERR_PTR(-ENOMEM);
+
+		err = sk_storage_alloc(sk, smap, selem);
+		if (err) {
+			kfree(selem);
+			atomic_sub(smap->elem_size, &sk->sk_omem_alloc);
+			return ERR_PTR(err);
+		}
+
+		return SDATA(selem);
+	}
+
+	if ((map_flags & BPF_F_LOCK) && !(map_flags & BPF_NOEXIST)) {
+		/* Hoping to find an old_sdata to do inline update
+		 * such that it can avoid taking the sk_storage->lock
+		 * and changing the lists.
+		 */
+		old_sdata = __sk_storage_lookup(sk_storage, smap, false);
+		err = check_flags(old_sdata, map_flags);
+		if (err)
+			return ERR_PTR(err);
+		if (old_sdata && selem_linked_to_sk(SELEM(old_sdata))) {
+			copy_map_value_locked(map, old_sdata->data,
+					      value, false);
+			return old_sdata;
+		}
+	}
+
+	raw_spin_lock_bh(&sk_storage->lock);
+
+	/* Recheck sk_storage->list under sk_storage->lock */
+	if (unlikely(hlist_empty(&sk_storage->list))) {
+		/* A parallel del is happening and sk_storage is going
+		 * away.  It has just been checked before, so very
+		 * unlikely.  Return instead of retry to keep things
+		 * simple.
+		 */
+		err = -EAGAIN;
+		goto unlock_err;
+	}
+
+	old_sdata = __sk_storage_lookup(sk_storage, smap, false);
+	err = check_flags(old_sdata, map_flags);
+	if (err)
+		goto unlock_err;
+
+	if (old_sdata && (map_flags & BPF_F_LOCK)) {
+		copy_map_value_locked(map, old_sdata->data, value, false);
+		selem = SELEM(old_sdata);
+		goto unlock;
+	}
+
+	/* sk_storage->lock is held.  Hence, we are sure
+	 * we can unlink and uncharge the old_sdata successfully
+	 * later.  Hence, instead of charging the new selem now
+	 * and then uncharge the old selem later (which may cause
+	 * a potential but unnecessary charge failure),  avoid taking
+	 * a charge at all here (the "!old_sdata" check) and the
+	 * old_sdata will not be uncharged later during __selem_unlink_sk().
+	 */
+	selem = selem_alloc(smap, sk, value, !old_sdata);
+	if (!selem) {
+		err = -ENOMEM;
+		goto unlock_err;
+	}
+
+	/* First, link the new selem to the map */
+	selem_link_map(smap, selem);
+
+	/* Second, link (and publish) the new selem to sk_storage */
+	__selem_link_sk(sk_storage, selem);
+
+	/* Third, remove old selem, SELEM(old_sdata) */
+	if (old_sdata) {
+		selem_unlink_map(SELEM(old_sdata));
+		__selem_unlink_sk(sk_storage, SELEM(old_sdata), false);
+	}
+
+unlock:
+	raw_spin_unlock_bh(&sk_storage->lock);
+	return SDATA(selem);
+
+unlock_err:
+	raw_spin_unlock_bh(&sk_storage->lock);
+	return ERR_PTR(err);
+}
+
+static int sk_storage_delete(struct sock *sk, struct bpf_map *map)
+{
+	struct bpf_sk_storage_data *sdata;
+
+	sdata = sk_storage_lookup(sk, map, false);
+	if (!sdata)
+		return -ENOENT;
+
+	selem_unlink(SELEM(sdata));
+
+	return 0;
+}
+
+/* Called by __sk_destruct() */
+void bpf_sk_storage_free(struct sock *sk)
+{
+	struct bpf_sk_storage_elem *selem;
+	struct bpf_sk_storage *sk_storage;
+	bool free_sk_storage = false;
+	struct hlist_node *n;
+
+	rcu_read_lock();
+	sk_storage = rcu_dereference(sk->sk_bpf_storage);
+	if (!sk_storage) {
+		rcu_read_unlock();
+		return;
+	}
+
+	/* Netiher the bpf_prog nor the bpf-map's syscall
+	 * could be modifying the sk_storage->list now.
+	 * Thus, no elem can be added-to or deleted-from the
+	 * sk_storage->list by the bpf_prog or by the bpf-map's syscall.
+	 *
+	 * It is racing with bpf_sk_storage_map_free() alone
+	 * when unlinking elem from the sk_storage->list and
+	 * the map's bucket->list.
+	 */
+	raw_spin_lock_bh(&sk_storage->lock);
+	hlist_for_each_entry_safe(selem, n, &sk_storage->list, snode) {
+		/* Always unlink from map before unlinking from
+		 * sk_storage.
+		 */
+		selem_unlink_map(selem);
+		free_sk_storage = __selem_unlink_sk(sk_storage, selem, true);
+	}
+	raw_spin_unlock_bh(&sk_storage->lock);
+	rcu_read_unlock();
+
+	if (free_sk_storage)
+		kfree_rcu(sk_storage, rcu);
+}
+
+static void bpf_sk_storage_map_free(struct bpf_map *map)
+{
+	struct bpf_sk_storage_elem *selem;
+	struct bpf_sk_storage_map *smap;
+	struct bucket *b;
+	unsigned int i;
+
+	smap = (struct bpf_sk_storage_map *)map;
+
+	synchronize_rcu();
+
+	/* bpf prog and the userspace can no longer access this map
+	 * now.  No new selem (of this map) can be added
+	 * to the sk->sk_bpf_storage or to the map bucket's list.
+	 *
+	 * The elem of this map can be cleaned up here
+	 * or
+	 * by bpf_sk_storage_free() during __sk_destruct().
+	 */
+	for (i = 0; i < (1U << smap->bucket_log); i++) {
+		b = &smap->buckets[i];
+
+		rcu_read_lock();
+		/* No one is adding to b->list now */
+		while ((selem = hlist_entry_safe(rcu_dereference_raw(hlist_first_rcu(&b->list)),
+						 struct bpf_sk_storage_elem,
+						 map_node))) {
+			selem_unlink(selem);
+			cond_resched_rcu();
+		}
+		rcu_read_unlock();
+	}
+
+	/* bpf_sk_storage_free() may still need to access the map.
+	 * e.g. bpf_sk_storage_free() has unlinked selem from the map
+	 * which then made the above while((selem = ...)) loop
+	 * exited immediately.
+	 *
+	 * However, the bpf_sk_storage_free() still needs to access
+	 * the smap->elem_size to do the uncharging in
+	 * __selem_unlink_sk().
+	 *
+	 * Hence, wait another rcu grace period for the
+	 * bpf_sk_storage_free() to finish.
+	 */
+	synchronize_rcu();
+
+	kvfree(smap->buckets);
+	kfree(map);
+}
+
+static struct bpf_map *bpf_sk_storage_map_alloc(union bpf_attr *attr)
+{
+	struct bpf_sk_storage_map *smap;
+	unsigned int i;
+	u32 nbuckets;
+	u64 cost;
+
+	if (attr->map_flags != BPF_F_NO_PREALLOC || attr->max_entries ||
+	    attr->key_size != sizeof(int) || !attr->value_size ||
+	    /* Enforce BTF for userspace sk dumping */
+	    !attr->btf_key_type_id || !attr->btf_value_type_id)
+		return ERR_PTR(-EINVAL);
+
+	if (!capable(CAP_SYS_ADMIN))
+		return ERR_PTR(-EPERM);
+
+	if (attr->value_size >= KMALLOC_MAX_SIZE -
+	    MAX_BPF_STACK - sizeof(struct bpf_sk_storage_elem) ||
+	    /* selem->elem_size is a u16 */
+	    attr->value_size > U16_MAX - sizeof(struct bpf_sk_storage_elem))
+		return ERR_PTR(-E2BIG);
+
+	smap = kzalloc(sizeof(*smap), GFP_USER | __GFP_NOWARN);
+	if (!smap)
+		return ERR_PTR(-ENOMEM);
+	bpf_map_init_from_attr(&smap->map, attr);
+
+	smap->bucket_log = ilog2(roundup_pow_of_two(num_possible_cpus()));
+	nbuckets = 1U << smap->bucket_log;
+	smap->buckets = kvcalloc(sizeof(*smap->buckets), nbuckets,
+				 GFP_USER | __GFP_NOWARN);
+	if (!smap->buckets) {
+		kfree(smap);
+		return ERR_PTR(-ENOMEM);
+	}
+	cost = sizeof(*smap->buckets) * nbuckets + sizeof(*smap);
+
+	for (i = 0; i < nbuckets; i++) {
+		INIT_HLIST_HEAD(&smap->buckets[i].list);
+		raw_spin_lock_init(&smap->buckets[i].lock);
+	}
+
+	smap->elem_size = sizeof(struct bpf_sk_storage_elem) + attr->value_size;
+	smap->cache_idx = (unsigned int)atomic_inc_return(&cache_idx) %
+		BPF_SK_STORAGE_CACHE_SIZE;
+	smap->map.pages = round_up(cost, PAGE_SIZE) >> PAGE_SHIFT;
+
+	return &smap->map;
+}
+
+static int notsupp_get_next_key(struct bpf_map *map, void *key,
+				void *next_key)
+{
+	return -ENOTSUPP;
+}
+
+static int bpf_sk_storage_map_check_btf(const struct bpf_map *map,
+					const struct btf *btf,
+					const struct btf_type *key_type,
+					const struct btf_type *value_type)
+{
+	u32 int_data;
+
+	if (BTF_INFO_KIND(key_type->info) != BTF_KIND_INT)
+		return -EINVAL;
+
+	int_data = *(u32 *)(key_type + 1);
+	if (BTF_INT_BITS(int_data) != 32 || BTF_INT_OFFSET(int_data))
+		return -EINVAL;
+
+	return 0;
+}
+
+static void *bpf_fd_sk_storage_lookup_elem(struct bpf_map *map, void *key)
+{
+	struct bpf_sk_storage_data *sdata;
+	struct socket *sock;
+	int fd, err;
+
+	fd = *(int *)key;
+	sock = sockfd_lookup(fd, &err);
+	if (sock) {
+		sdata = sk_storage_lookup(sock->sk, map, true);
+		sockfd_put(sock);
+		return sdata ? sdata->data : NULL;
+	}
+
+	return ERR_PTR(err);
+}
+
+static int bpf_fd_sk_storage_update_elem(struct bpf_map *map, void *key,
+					 void *value, u64 map_flags)
+{
+	struct bpf_sk_storage_data *sdata;
+	struct socket *sock;
+	int fd, err;
+
+	fd = *(int *)key;
+	sock = sockfd_lookup(fd, &err);
+	if (sock) {
+		sdata = sk_storage_update(sock->sk, map, value, map_flags);
+		sockfd_put(sock);
+		return IS_ERR(sdata) ? PTR_ERR(sdata) : 0;
+	}
+
+	return err;
+}
+
+static int bpf_fd_sk_storage_delete_elem(struct bpf_map *map, void *key)
+{
+	struct socket *sock;
+	int fd, err;
+
+	fd = *(int *)key;
+	sock = sockfd_lookup(fd, &err);
+	if (sock) {
+		err = sk_storage_delete(sock->sk, map);
+		sockfd_put(sock);
+		return err;
+	}
+
+	return err;
+}
+
+BPF_CALL_4(bpf_sk_storage_get, struct bpf_map *, map, struct sock *, sk,
+	   void *, value, u64, flags)
+{
+	struct bpf_sk_storage_data *sdata;
+
+	if (flags > BPF_SK_STORAGE_GET_F_CREATE)
+		return (unsigned long)NULL;
+
+	sdata = sk_storage_lookup(sk, map, true);
+	if (sdata)
+		return (unsigned long)sdata->data;
+
+	if (flags == BPF_SK_STORAGE_GET_F_CREATE &&
+	    /* Cannot add new elem to a going away sk.
+	     * Otherwise, the new elem may become a leak
+	     * (and also other memory issues during map
+	     *  destruction).
+	     */
+	    refcount_inc_not_zero(&sk->sk_refcnt)) {
+		sdata = sk_storage_update(sk, map, value, BPF_NOEXIST);
+		/* sk must be a fullsock (guaranteed by verifier),
+		 * so sock_gen_put() is unnecessary.
+		 */
+		sock_put(sk);
+		return IS_ERR(sdata) ?
+			(unsigned long)NULL : (unsigned long)sdata->data;
+	}
+
+	return (unsigned long)NULL;
+}
+
+BPF_CALL_2(bpf_sk_storage_delete, struct bpf_map *, map, struct sock *, sk)
+{
+	if (refcount_inc_not_zero(&sk->sk_refcnt)) {
+		int err;
+
+		err = sk_storage_delete(sk, map);
+		sock_put(sk);
+		return err;
+	}
+
+	return -ENOENT;
+}
+
+const struct bpf_map_ops sk_storage_map_ops = {
+	.map_alloc = bpf_sk_storage_map_alloc,
+	.map_free = bpf_sk_storage_map_free,
+	.map_get_next_key = notsupp_get_next_key,
+	.map_lookup_elem = bpf_fd_sk_storage_lookup_elem,
+	.map_update_elem = bpf_fd_sk_storage_update_elem,
+	.map_delete_elem = bpf_fd_sk_storage_delete_elem,
+	.map_check_btf = bpf_sk_storage_map_check_btf,
+};
+
+const struct bpf_func_proto bpf_sk_storage_get_proto = {
+	.func		= bpf_sk_storage_get,
+	.gpl_only	= false,
+	.ret_type	= RET_PTR_TO_MAP_VALUE_OR_NULL,
+	.arg1_type	= ARG_CONST_MAP_PTR,
+	.arg2_type	= ARG_PTR_TO_SOCKET,
+	.arg3_type	= ARG_PTR_TO_MAP_VALUE_OR_NULL,
+	.arg4_type	= ARG_ANYTHING,
+};
+
+const struct bpf_func_proto bpf_sk_storage_delete_proto = {
+	.func		= bpf_sk_storage_delete,
+	.gpl_only	= false,
+	.ret_type	= RET_INTEGER,
+	.arg1_type	= ARG_CONST_MAP_PTR,
+	.arg2_type	= ARG_PTR_TO_SOCKET,
+};
diff --git a/net/core/filter.c b/net/core/filter.c
index 2f88baf39cc2..27b0dc01dc3f 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -75,6 +75,7 @@
 #include <net/seg6_local.h>
 #include <net/lwtunnel.h>
 #include <net/ipv6_stubs.h>
+#include <net/bpf_sk_storage.h>
 
 /**
  *	sk_filter_trim_cap - run a packet through a socket filter
@@ -5903,6 +5904,9 @@ sk_filter_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 	}
 }
 
+const struct bpf_func_proto bpf_sk_storage_get_proto __weak;
+const struct bpf_func_proto bpf_sk_storage_delete_proto __weak;
+
 static const struct bpf_func_proto *
 cg_skb_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 {
@@ -5911,6 +5915,10 @@ cg_skb_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return &bpf_get_local_storage_proto;
 	case BPF_FUNC_sk_fullsock:
 		return &bpf_sk_fullsock_proto;
+	case BPF_FUNC_sk_storage_get:
+		return &bpf_sk_storage_get_proto;
+	case BPF_FUNC_sk_storage_delete:
+		return &bpf_sk_storage_delete_proto;
 #ifdef CONFIG_INET
 	case BPF_FUNC_tcp_sock:
 		return &bpf_tcp_sock_proto;
@@ -5992,6 +6000,10 @@ tc_cls_act_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return &bpf_skb_fib_lookup_proto;
 	case BPF_FUNC_sk_fullsock:
 		return &bpf_sk_fullsock_proto;
+	case BPF_FUNC_sk_storage_get:
+		return &bpf_sk_storage_get_proto;
+	case BPF_FUNC_sk_storage_delete:
+		return &bpf_sk_storage_delete_proto;
 #ifdef CONFIG_XFRM
 	case BPF_FUNC_skb_get_xfrm_state:
 		return &bpf_skb_get_xfrm_state_proto;
diff --git a/net/core/sock.c b/net/core/sock.c
index 443b98d05f1e..9773be724aa9 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -137,6 +137,7 @@
 
 #include <linux/filter.h>
 #include <net/sock_reuseport.h>
+#include <net/bpf_sk_storage.h>
 
 #include <trace/events/sock.h>
 
@@ -1709,6 +1710,10 @@ static void __sk_destruct(struct rcu_head *head)
 
 	sock_disable_timestamp(sk, SK_FLAGS_TIMESTAMP);
 
+#ifdef CONFIG_BPF_SYSCALL
+	bpf_sk_storage_free(sk);
+#endif
+
 	if (atomic_read(&sk->sk_omem_alloc))
 		pr_debug("%s: optmem leakage (%d bytes) detected\n",
 			 __func__, atomic_read(&sk->sk_omem_alloc));
-- 
2.17.1

Re: [PATCH v2 bpf-next 1/6] bpf: Introduce bpf sk local storage

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3278 bytes --]

Hi Martin,

I love your patch! Yet something to improve:

[auto build test ERROR on bpf-next/master]

url:    https://github.com/0day-ci/linux/commits/Martin-KaFai-Lau/bpf-Introduce-bpf-sk-local-storage/20190426-192951
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.2.0 make.cross ARCH=arm 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from include/linux/spinlock_types.h:13:0,
                    from net//core/bpf_sk_storage.c:3:
>> arch/arm/include/asm/spinlock_types.h:13:3: error: unknown type name 'u32'
      u32 slock;
      ^~~
>> arch/arm/include/asm/spinlock_types.h:16:4: error: unknown type name 'u16'
       u16 next;
       ^~~
   arch/arm/include/asm/spinlock_types.h:17:4: error: unknown type name 'u16'
       u16 owner;
       ^~~
   arch/arm/include/asm/spinlock_types.h:29:2: error: unknown type name 'u32'
     u32 lock;
     ^~~

vim +/u32 +13 arch/arm/include/asm/spinlock_types.h

546c2896 arch/arm/include/asm/spinlock_types.h Will Deacon     2012-07-06  10  
fb1c8f93 include/asm-arm/spinlock_types.h      Ingo Molnar     2005-09-10  11  typedef struct {
546c2896 arch/arm/include/asm/spinlock_types.h Will Deacon     2012-07-06  12  	union {
546c2896 arch/arm/include/asm/spinlock_types.h Will Deacon     2012-07-06 @13  		u32 slock;
546c2896 arch/arm/include/asm/spinlock_types.h Will Deacon     2012-07-06  14  		struct __raw_tickets {
546c2896 arch/arm/include/asm/spinlock_types.h Will Deacon     2012-07-06  15  #ifdef __ARMEB__
546c2896 arch/arm/include/asm/spinlock_types.h Will Deacon     2012-07-06 @16  			u16 next;
546c2896 arch/arm/include/asm/spinlock_types.h Will Deacon     2012-07-06  17  			u16 owner;
546c2896 arch/arm/include/asm/spinlock_types.h Will Deacon     2012-07-06  18  #else
546c2896 arch/arm/include/asm/spinlock_types.h Will Deacon     2012-07-06  19  			u16 owner;
546c2896 arch/arm/include/asm/spinlock_types.h Will Deacon     2012-07-06  20  			u16 next;
546c2896 arch/arm/include/asm/spinlock_types.h Will Deacon     2012-07-06  21  #endif
546c2896 arch/arm/include/asm/spinlock_types.h Will Deacon     2012-07-06  22  		} tickets;
546c2896 arch/arm/include/asm/spinlock_types.h Will Deacon     2012-07-06  23  	};
445c8951 arch/arm/include/asm/spinlock_types.h Thomas Gleixner 2009-12-02  24  } arch_spinlock_t;
fb1c8f93 include/asm-arm/spinlock_types.h      Ingo Molnar     2005-09-10  25  

:::::: The code at line 13 was first introduced by commit
:::::: 546c2896a42202dbc7d02f7c6ec9948ac1bf511b ARM: 7446/1: spinlock: use ticket algorithm for ARMv6+ locking implementation

:::::: TO: Will Deacon <will.deacon@arm.com>
:::::: CC: Russell King <rmk+kernel@arm.linux.org.uk>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 68333 bytes --]

Re: [PATCH v2 bpf-next 1/6] bpf: Introduce bpf sk local storage

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 14565 bytes --]

Hi Martin,

I love your patch! Yet something to improve:

[auto build test ERROR on bpf-next/master]

url:    https://github.com/0day-ci/linux/commits/Martin-KaFai-Lau/bpf-Introduce-bpf-sk-local-storage/20190426-192951
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: arm64-allyesconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.2.0 make.cross ARCH=arm64 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All error/warnings (new ones prefixed by >>):

   In file included from arch/arm64/include/asm/spinlock_types.h:23:0,
                    from include/linux/spinlock_types.h:13,
                    from net//core/bpf_sk_storage.c:3:
>> include/asm-generic/qspinlock_types.h:33:3: error: unknown type name 'atomic_t'
      atomic_t val;
      ^~~~~~~~
>> include/asm-generic/qspinlock_types.h:51:4: error: unknown type name 'u16'
       u16 tail;
       ^~~
   include/asm-generic/qspinlock_types.h:52:4: error: unknown type name 'u16'
       u16 locked_pending;
       ^~~
>> include/asm-generic/qspinlock_types.h:55:4: error: unknown type name 'u8'
       u8 reserved[2];
       ^~
   include/asm-generic/qspinlock_types.h:56:4: error: unknown type name 'u8'
       u8 pending;
       ^~
   include/asm-generic/qspinlock_types.h:57:4: error: unknown type name 'u8'
       u8 locked;
       ^~
   In file included from ./arch/arm64/include/generated/asm/qspinlock.h:1:0,
                    from arch/arm64/include/asm/spinlock.h:20,
                    from include/linux/spinlock.h:88,
                    from include/linux/seqlock.h:36,
                    from include/linux/time.h:6,
                    from include/linux/ktime.h:24,
                    from include/linux/timer.h:6,
                    from include/linux/workqueue.h:9,
                    from include/linux/bpf.h:12,
                    from net//core/bpf_sk_storage.c:8:
   include/asm-generic/qspinlock.h: In function 'queued_spin_is_locked':
>> include/asm-generic/qspinlock.h:35:21: error: passing argument 1 of 'atomic_read' from incompatible pointer type [-Werror=incompatible-pointer-types]
     return atomic_read(&lock->val);
                        ^
   In file included from arch/arm64/include/asm/atomic.h:169:0,
                    from include/linux/atomic.h:7,
                    from include/asm-generic/bitops/atomic.h:5,
                    from arch/arm64/include/asm/bitops.h:37,
                    from include/linux/bitops.h:19,
                    from include/linux/kernel.h:12,
                    from include/linux/list.h:9,
                    from include/linux/lockdep.h:27,
                    from include/linux/spinlock_types.h:18,
                    from net//core/bpf_sk_storage.c:3:
   include/asm-generic/atomic-instrumented.h:24:1: note: expected 'const atomic_t * {aka const struct <anonymous> *}' but argument is of type 'int *'
    atomic_read(const atomic_t *v)
    ^~~~~~~~~~~
   In file included from ./arch/arm64/include/generated/asm/qspinlock.h:1:0,
                    from arch/arm64/include/asm/spinlock.h:20,
                    from include/linux/spinlock.h:88,
                    from include/linux/seqlock.h:36,
                    from include/linux/time.h:6,
                    from include/linux/ktime.h:24,
                    from include/linux/timer.h:6,
                    from include/linux/workqueue.h:9,
                    from include/linux/bpf.h:12,
                    from net//core/bpf_sk_storage.c:8:
   include/asm-generic/qspinlock.h: In function 'queued_spin_value_unlocked':
   include/asm-generic/qspinlock.h:50:22: error: passing argument 1 of 'atomic_read' from incompatible pointer type [-Werror=incompatible-pointer-types]
     return !atomic_read(&lock.val);
                         ^
   In file included from arch/arm64/include/asm/atomic.h:169:0,
                    from include/linux/atomic.h:7,
                    from include/asm-generic/bitops/atomic.h:5,
                    from arch/arm64/include/asm/bitops.h:37,
                    from include/linux/bitops.h:19,
                    from include/linux/kernel.h:12,
                    from include/linux/list.h:9,
                    from include/linux/lockdep.h:27,
                    from include/linux/spinlock_types.h:18,
                    from net//core/bpf_sk_storage.c:3:
   include/asm-generic/atomic-instrumented.h:24:1: note: expected 'const atomic_t * {aka const struct <anonymous> *}' but argument is of type 'int *'
    atomic_read(const atomic_t *v)
    ^~~~~~~~~~~
   In file included from ./arch/arm64/include/generated/asm/qspinlock.h:1:0,
                    from arch/arm64/include/asm/spinlock.h:20,
                    from include/linux/spinlock.h:88,
                    from include/linux/seqlock.h:36,
                    from include/linux/time.h:6,
                    from include/linux/ktime.h:24,
                    from include/linux/timer.h:6,
                    from include/linux/workqueue.h:9,
                    from include/linux/bpf.h:12,
                    from net//core/bpf_sk_storage.c:8:
   include/asm-generic/qspinlock.h: In function 'queued_spin_is_contended':
   include/asm-generic/qspinlock.h:60:21: error: passing argument 1 of 'atomic_read' from incompatible pointer type [-Werror=incompatible-pointer-types]
     return atomic_read(&lock->val) & ~_Q_LOCKED_MASK;
                        ^
   In file included from arch/arm64/include/asm/atomic.h:169:0,
                    from include/linux/atomic.h:7,
                    from include/asm-generic/bitops/atomic.h:5,
                    from arch/arm64/include/asm/bitops.h:37,
                    from include/linux/bitops.h:19,
                    from include/linux/kernel.h:12,
                    from include/linux/list.h:9,
                    from include/linux/lockdep.h:27,
                    from include/linux/spinlock_types.h:18,
                    from net//core/bpf_sk_storage.c:3:
   include/asm-generic/atomic-instrumented.h:24:1: note: expected 'const atomic_t * {aka const struct <anonymous> *}' but argument is of type 'int *'
    atomic_read(const atomic_t *v)
    ^~~~~~~~~~~
   In file included from ./arch/arm64/include/generated/asm/qspinlock.h:1:0,
                    from arch/arm64/include/asm/spinlock.h:20,
                    from include/linux/spinlock.h:88,
                    from include/linux/seqlock.h:36,
                    from include/linux/time.h:6,
                    from include/linux/ktime.h:24,
                    from include/linux/timer.h:6,
                    from include/linux/workqueue.h:9,
                    from include/linux/bpf.h:12,
                    from net//core/bpf_sk_storage.c:8:
   include/asm-generic/qspinlock.h: In function 'queued_spin_trylock':
   include/asm-generic/qspinlock.h:69:24: error: passing argument 1 of 'atomic_read' from incompatible pointer type [-Werror=incompatible-pointer-types]
     u32 val = atomic_read(&lock->val);
                           ^
   In file included from arch/arm64/include/asm/atomic.h:169:0,
                    from include/linux/atomic.h:7,
                    from include/asm-generic/bitops/atomic.h:5,
                    from arch/arm64/include/asm/bitops.h:37,
                    from include/linux/bitops.h:19,
                    from include/linux/kernel.h:12,
                    from include/linux/list.h:9,
                    from include/linux/lockdep.h:27,
                    from include/linux/spinlock_types.h:18,
                    from net//core/bpf_sk_storage.c:3:
   include/asm-generic/atomic-instrumented.h:24:1: note: expected 'const atomic_t * {aka const struct <anonymous> *}' but argument is of type 'int *'
    atomic_read(const atomic_t *v)
    ^~~~~~~~~~~
   In file included from include/uapi/linux/swab.h:6:0,
                    from include/linux/swab.h:5,
                    from include/uapi/linux/byteorder/big_endian.h:13,
                    from include/linux/byteorder/big_endian.h:5,
                    from arch/arm64/include/uapi/asm/byteorder.h:21,
                    from include/asm-generic/qrwlock_types.h:6,
                    from arch/arm64/include/asm/spinlock_types.h:24,
                    from include/linux/spinlock_types.h:13,
                    from net//core/bpf_sk_storage.c:3:
>> include/asm-generic/qspinlock.h:74:43: error: passing argument 1 of 'atomic_try_cmpxchg_acquire' from incompatible pointer type [-Werror=incompatible-pointer-types]
     return likely(atomic_try_cmpxchg_acquire(&lock->val, &val, _Q_LOCKED_VAL));
                                              ^
   include/linux/compiler.h:76:40: note: in definition of macro 'likely'
    # define likely(x) __builtin_expect(!!(x), 1)
                                           ^
   In file included from include/linux/atomic.h:74:0,
                    from include/asm-generic/bitops/atomic.h:5,
                    from arch/arm64/include/asm/bitops.h:37,
                    from include/linux/bitops.h:19,
                    from include/linux/kernel.h:12,
                    from include/linux/list.h:9,
                    from include/linux/lockdep.h:27,
                    from include/linux/spinlock_types.h:18,
                    from net//core/bpf_sk_storage.c:3:
   include/linux/atomic-fallback.h:928:1: note: expected 'atomic_t * {aka struct <anonymous> *}' but argument is of type 'int *'
    atomic_try_cmpxchg_acquire(atomic_t *v, int *old, int new)
    ^~~~~~~~~~~~~~~~~~~~~~~~~~
   In file included from include/uapi/linux/swab.h:6:0,
                    from include/linux/swab.h:5,
                    from include/uapi/linux/byteorder/big_endian.h:13,
                    from include/linux/byteorder/big_endian.h:5,
                    from arch/arm64/include/uapi/asm/byteorder.h:21,
                    from include/asm-generic/qrwlock_types.h:6,
                    from arch/arm64/include/asm/spinlock_types.h:24,
                    from include/linux/spinlock_types.h:13,
                    from net//core/bpf_sk_storage.c:3:
   include/asm-generic/qspinlock.h: In function 'queued_spin_lock':
   include/asm-generic/qspinlock.h:87:40: error: passing argument 1 of 'atomic_try_cmpxchg_acquire' from incompatible pointer type [-Werror=incompatible-pointer-types]
     if (likely(atomic_try_cmpxchg_acquire(&lock->val, &val, _Q_LOCKED_VAL)))
                                           ^
   include/linux/compiler.h:76:40: note: in definition of macro 'likely'
    # define likely(x) __builtin_expect(!!(x), 1)
                                           ^
   In file included from include/linux/atomic.h:74:0,
                    from include/asm-generic/bitops/atomic.h:5,
                    from arch/arm64/include/asm/bitops.h:37,
                    from include/linux/bitops.h:19,
                    from include/linux/kernel.h:12,
                    from include/linux/list.h:9,
                    from include/linux/lockdep.h:27,
                    from include/linux/spinlock_types.h:18,
                    from net//core/bpf_sk_storage.c:3:
   include/linux/atomic-fallback.h:928:1: note: expected 'atomic_t * {aka struct <anonymous> *}' but argument is of type 'int *'
    atomic_try_cmpxchg_acquire(atomic_t *v, int *old, int new)
    ^~~~~~~~~~~~~~~~~~~~~~~~~~
   In file included from include/linux/fs.h:25:0,
                    from include/linux/compat.h:17,
                    from include/linux/ethtool.h:17,
                    from include/linux/netdevice.h:41,
                    from include/net/sock.h:51,
                    from net//core/bpf_sk_storage.c:10:
   include/linux/semaphore.h: In function 'sema_init':
>> include/linux/semaphore.h:35:17: warning: braces around scalar initializer
     *sem = (struct semaphore) __SEMAPHORE_INITIALIZER(*sem, val);
                    ^~~~~~~~~
   include/linux/semaphore.h:35:17: note: (near initialization for '(anonymous).raw_lock.<anonymous>.val')
   cc1: some warnings being treated as errors

vim +/atomic_t +33 include/asm-generic/qspinlock_types.h

a33fda35 Waiman Long 2015-04-24  30  
a33fda35 Waiman Long 2015-04-24  31  typedef struct qspinlock {
625e88be Will Deacon 2018-04-26  32  	union {
a33fda35 Waiman Long 2015-04-24 @33  		atomic_t val;
625e88be Will Deacon 2018-04-26  34  
625e88be Will Deacon 2018-04-26  35  		/*
625e88be Will Deacon 2018-04-26  36  		 * By using the whole 2nd least significant byte for the
625e88be Will Deacon 2018-04-26  37  		 * pending bit, we can allow better optimization of the lock
625e88be Will Deacon 2018-04-26  38  		 * acquisition for the pending bit holder.
625e88be Will Deacon 2018-04-26  39  		 */
625e88be Will Deacon 2018-04-26  40  #ifdef __LITTLE_ENDIAN
625e88be Will Deacon 2018-04-26  41  		struct {
625e88be Will Deacon 2018-04-26  42  			u8	locked;
625e88be Will Deacon 2018-04-26  43  			u8	pending;
625e88be Will Deacon 2018-04-26  44  		};
625e88be Will Deacon 2018-04-26  45  		struct {
625e88be Will Deacon 2018-04-26  46  			u16	locked_pending;
625e88be Will Deacon 2018-04-26  47  			u16	tail;
625e88be Will Deacon 2018-04-26  48  		};
625e88be Will Deacon 2018-04-26  49  #else
625e88be Will Deacon 2018-04-26  50  		struct {
625e88be Will Deacon 2018-04-26 @51  			u16	tail;
625e88be Will Deacon 2018-04-26  52  			u16	locked_pending;
625e88be Will Deacon 2018-04-26  53  		};
625e88be Will Deacon 2018-04-26  54  		struct {
625e88be Will Deacon 2018-04-26 @55  			u8	reserved[2];
625e88be Will Deacon 2018-04-26  56  			u8	pending;
625e88be Will Deacon 2018-04-26  57  			u8	locked;
625e88be Will Deacon 2018-04-26  58  		};
625e88be Will Deacon 2018-04-26  59  #endif
625e88be Will Deacon 2018-04-26  60  	};
a33fda35 Waiman Long 2015-04-24  61  } arch_spinlock_t;
a33fda35 Waiman Long 2015-04-24  62  

:::::: The code at line 33 was first introduced by commit
:::::: a33fda35e3a7655fb7df756ed67822afb5ed5e8d locking/qspinlock: Introduce a simple generic 4-byte queued spinlock

:::::: TO: Waiman Long <Waiman.Long@hp.com>
:::::: CC: Ingo Molnar <mingo@kernel.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 63211 bytes --]

Re: [PATCH v2 bpf-next 1/6] bpf: Introduce bpf sk local storage

[Yonghong Song]

On 4/25/19 5:10 PM, Martin KaFai Lau wrote:
> After allowing a bpf prog to
> - directly read the skb->sk ptr
> - get the fullsock bpf_sock by "bpf_sk_fullsock()"
> - get the bpf_tcp_sock by "bpf_tcp_sock()"
> - get the listener sock by "bpf_get_listener_sock()"
> - avoid duplicating the fields of "(bpf_)sock" and "(bpf_)tcp_sock"
>    into different bpf running context.
> 
> this patch is another effort to make bpf's network programming
> more intuitive to do (together with memory and performance benefit).
> 
> When bpf prog needs to store data for a sk, the current practice is to
> define a map with the usual 4-tuples (src/dst ip/port) as the key.
> If multiple bpf progs require to store different sk data, multiple maps
> have to be defined.  Hence, wasting memory to store the duplicated
> keys (i.e. 4 tuples here) in each of the bpf map.
> [ The smallest key could be the sk pointer itself which requires
>    some enhancement in the verifier and it is a separate topic. ]
> 
> Also, the bpf prog needs to clean up the elem when sk is freed.
> Otherwise, the bpf map will become full and un-usable quickly.
> The sk-free tracking currently could be done during sk state
> transition (e.g. BPF_SOCK_OPS_STATE_CB).
> 
> The size of the map needs to be predefined which then usually ended-up
> with an over-provisioned map in production.  Even the map was re-sizable,
> while the sk naturally come and go away already, this potential re-size
> operation is arguably redundant if the data can be directly connected
> to the sk itself instead of proxy-ing through a bpf map.
> 
> This patch introduces sk->sk_bpf_storage to provide local storage space
> at sk for bpf prog to use.  The space will be allocated when the first bpf
> prog has created data for this particular sk.
> 
> The design optimizes the bpf prog's lookup (and then optionally followed by
> an inline update).  bpf_spin_lock should be used if the inline update needs
> to be protected.
> 
> BPF_MAP_TYPE_SK_STORAGE:
> -----------------------
> To define a bpf "sk-local-storage", a BPF_MAP_TYPE_SK_STORAGE map (new in
> this patch) needs to be created.  Multiple BPF_MAP_TYPE_SK_STORAGE maps can
> be created to fit different bpf progs' needs.  The map enforces
> BTF to allow printing the sk-local-storage during a system-wise
> sk dump (e.g. "ss -ta") in the future.
> 
> The purpose of a BPF_MAP_TYPE_SK_STORAGE map is not for lookup/update/delete
> a "sk-local-storage" data from a particular sk.
> Think of the map as a meta-data (or "type") of a "sk-local-storage".  This
> particular "type" of "sk-local-storage" data can then be stored in any sk.
> 
> The main purposes of this map are mostly:
> 1. Define the size of a "sk-local-storage" type.
> 2. Provide a similar syscall userspace API as the map (e.g. lookup/update,
>     map-id, map-btf...etc.)
> 3. Keep track of all sk's storages of this "type" and clean them up
>     when the map is freed.
> 
> sk->sk_bpf_storage:
> ------------------
> The main lookup/update/delete is done on sk->sk_bpf_storage (which
> is a "struct bpf_sk_storage").  When doing a lookup,
> the "map" pointer is now used as the "key" to search on the
> sk_storage->list.  The "map" pointer is actually serving
> as the "type" of the "sk-local-storage" that is being
> requested.
> 
> To allow very fast lookup, it should be as fast as looking up an
> array at a stable-offset.  At the same time, it is not ideal to
> set a hard limit on the number of sk-local-storage "type" that the
> system can have.  Hence, this patch takes a cache approach.
> The last search result from sk_storage->list is cached in
> sk_storage->cache[] which is a stable sized array.  Each
> "sk-local-storage" type has a stable offset to the cache[] array.
> In the future, a map's flag could be introduced to do cache
> opt-out/enforcement if it became necessary.
> 
> All data will be removed from sk->sk_bpf_storage during
> sk destruction.
> 
> bpf_sk_storage_get() and bpf_sk_storage_delete():
> ------------------------------------------------
> Instead of using bpf_map_(lookup|update|delete)_elem(),
> the bpf prog needs to use the new helper bpf_sk_storage_get() and
> bpf_sk_storage_delete().  The verifier can then enforce the
> ARG_PTR_TO_SOCKET argument.  The bpf_sk_storage_get() also allows to
> "create" new elem if one does not exist in the sk.  It is done by
> the new BPF_SK_STORAGE_GET_F_CREATE flag.  An optional value can also be
> provided as the initial value during BPF_SK_STORAGE_GET_F_CREATE.
> The BPF_MAP_TYPE_SK_STORAGE also supports bpf_spin_lock.  Together,
> it has eliminated the potential use cases for an equivalent
> bpf_map_update_elem() API (for bpf_prog) in this patch.
> 
> Misc notes:
> ----------
> 1. map_get_next_key is not supported.  From the userspace syscall
>     perspective,  the map has the socket fd as the key while the map
>     can be shared by pinned-file or map-id.
> 
>     Since btf is enforced, the existing "ss" could be enhanced to pretty
>     print the local-storage.
> 
>     Supporting a kernel defined btf with 4 tuples as the return key could
>     be explored later also.
> 
> 2. The sk->sk_lock cannot be acquired.  Atomic operations is used instead.
>     e.g. cmpxchg is done on the sk->sk_bpf_storage ptr.
>     Please refer to the source code comments for the details in
>     synchronization cases and considerations.
> 
> 3. The mem is charged to the sk->sk_omem_alloc as the sk filter does.
> 
> Benchmark:
> ---------
> Here is the benchmark data collected by turning on
> the "kernel.bpf_stats_enabled" sysctl.
> Two bpf progs are tested:
> 
> One bpf prog with the usual bpf hashmap (max_entries = 8192) with the
> sk ptr as the key. (verifier is modified to support sk ptr as the key
> That should have shortened the key lookup time.)
> 
> Another bpf prog is with the new BPF_MAP_TYPE_SK_STORAGE.
> 
> Both are storing a "u32 cnt", do a lookup on "egress_skb/cgroup" for
> each egress skb and then bump the cnt.  netperf is used to drive
> data with 4096 connected UDP sockets.
> 
> BPF_MAP_TYPE_HASH with a modifier verifier (152ns per bpf run)
> 27: cgroup_skb  name egress_sk_map  tag 74f56e832918070b run_time_ns 58280107540 run_cnt 381347633
>      loaded_at 2019-04-15T13:46:39-0700  uid 0
>      xlated 344B  jited 258B  memlock 4096B  map_ids 16
>      btf_id 5
> 
> BPF_MAP_TYPE_SK_STORAGE in this patch (66ns per bpf run)
> 30: cgroup_skb  name egress_sk_stora  tag d4aa70984cc7bbf6 run_time_ns 25617093319 run_cnt 390989739
>      loaded_at 2019-04-15T13:47:54-0700  uid 0
>      xlated 168B  jited 156B  memlock 4096B  map_ids 17
>      btf_id 6
> 
> Here is a high-level picture on how are the objects organized:
> 
>         sk
>      +------+
>      |      |
>      |      |
>      |      |
>      |*sk_bpf_storage-----> bpf_sk_storage
>      +------+                 +-------+
>                   +-----------+ list  |
>                   |           |       |
>                   |           |       |
>                   |           |       |
>                   |           +-------+
>                   |
>                   |     elem
>                   |  +--------+
>                   +->| snode  |
>                   |  +--------+
>                   |  |  data  |          bpf_map
>                   |  +--------+        +---------+
>                   |  |map_node|<-+-----+  list   |
>                   |  +--------+  |     |         |
>                   |              |     |         |
>                   |     elem     |     |         |
>                   |  +--------+  |     +---------+
>                   +->| snode  |  |
>                      +--------+  |
>     bpf_map          |  data  |  |
>   +---------+        +--------+  |
>   |  list   +------->|map_node|  |
>   |         |        +--------+  |
>   |         |                    |
>   |         |           elem     |
>   +---------+        +--------+  |
>                   +->| snode  |  |
>                   |  +--------+  |
>                   |  |  data  |  |
>                   |  +--------+  |
>                   |  |map_node|<-+
>                   |  +--------+
>                   |
>                   |
>                   |          +-------+
>       sk          +----------| list  |
>    +------+                  |       |
>    |      |                  |       |
>    |      |                  |       |
>    |      |                  +-------+
>    |*sk_bpf_storage------->bpf_sk_storage
>    +------+
> 
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>

Thanks. Generally look good. I have a few comments below.

> ---
>   include/linux/bpf.h          |   2 +
>   include/linux/bpf_types.h    |   1 +
>   include/net/bpf_sk_storage.h |  13 +
>   include/net/sock.h           |   5 +
>   include/uapi/linux/bpf.h     |  44 +-
>   kernel/bpf/syscall.c         |   3 +-
>   kernel/bpf/verifier.c        |  27 +-
>   net/bpf/test_run.c           |   2 +
>   net/core/Makefile            |   1 +
>   net/core/bpf_sk_storage.c    | 796 +++++++++++++++++++++++++++++++++++
>   net/core/filter.c            |  12 +
>   net/core/sock.c              |   5 +
>   12 files changed, 906 insertions(+), 5 deletions(-)
>   create mode 100644 include/net/bpf_sk_storage.h
>   create mode 100644 net/core/bpf_sk_storage.c
> 
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index f15432d90728..0d2788beacd2 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -184,6 +184,7 @@ enum bpf_arg_type {
>   	ARG_PTR_TO_MAP_KEY,	/* pointer to stack used as map key */
>   	ARG_PTR_TO_MAP_VALUE,	/* pointer to stack used as map value */
>   	ARG_PTR_TO_UNINIT_MAP_VALUE,	/* pointer to valid memory used to store a map value */
> +	ARG_PTR_TO_MAP_VALUE_OR_NULL,	/* pointer to stack used as map value or NULL */
>   
>   	/* the following constraints used to prototype bpf_memcmp() and other
>   	 * functions that access data on eBPF program stack
> @@ -204,6 +205,7 @@ enum bpf_arg_type {
>   	ARG_PTR_TO_SOCK_COMMON,	/* pointer to sock_common */
>   	ARG_PTR_TO_INT,		/* pointer to int */
>   	ARG_PTR_TO_LONG,	/* pointer to long */
> +	ARG_PTR_TO_SOCKET,	/* pointer to bpf_sock (fullsock) */
>   };
>   
>   /* type of values returned from helper functions */
> diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
> index d26991a16894..eb1e4f867a5e 100644
> --- a/include/linux/bpf_types.h
> +++ b/include/linux/bpf_types.h
> @@ -60,6 +60,7 @@ BPF_MAP_TYPE(BPF_MAP_TYPE_ARRAY_OF_MAPS, array_of_maps_map_ops)
>   BPF_MAP_TYPE(BPF_MAP_TYPE_HASH_OF_MAPS, htab_of_maps_map_ops)
>   #ifdef CONFIG_NET
>   BPF_MAP_TYPE(BPF_MAP_TYPE_DEVMAP, dev_map_ops)
> +BPF_MAP_TYPE(BPF_MAP_TYPE_SK_STORAGE, sk_storage_map_ops)
>   #if defined(CONFIG_BPF_STREAM_PARSER)
>   BPF_MAP_TYPE(BPF_MAP_TYPE_SOCKMAP, sock_map_ops)
>   BPF_MAP_TYPE(BPF_MAP_TYPE_SOCKHASH, sock_hash_ops)
> diff --git a/include/net/bpf_sk_storage.h b/include/net/bpf_sk_storage.h
> new file mode 100644
> index 000000000000..b9dcb02e756b
> --- /dev/null
> +++ b/include/net/bpf_sk_storage.h
> @@ -0,0 +1,13 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (c) 2019 Facebook */
> +#ifndef _BPF_SK_STORAGE_H
> +#define _BPF_SK_STORAGE_H
> +
> +struct sock;
> +
> +void bpf_sk_storage_free(struct sock *sk);
> +
> +extern const struct bpf_func_proto bpf_sk_storage_get_proto;
> +extern const struct bpf_func_proto bpf_sk_storage_delete_proto;
> +
> +#endif /* _BPF_SK_STORAGE_H */
[...]
> +
> +static struct bpf_sk_storage_data *
> +__sk_storage_lookup(struct bpf_sk_storage *sk_storage,
> +		    struct bpf_sk_storage_map *smap,
> +		    bool cacheit_lockit)
> +{
> +	struct bpf_sk_storage_data *sdata;
> +	struct bpf_sk_storage_elem *selem;
> +
> +	/* Fast path (cache hit) */
> +	sdata = rcu_dereference(sk_storage->cache[smap->cache_idx]);
> +	if (sdata && rcu_access_pointer(sdata->smap) == smap)
> +		return sdata;
> +
> +	/* Slow path (cache miss) */
> +	hlist_for_each_entry_rcu(selem, &sk_storage->list, snode)
> +		if (rcu_access_pointer(SDATA(selem)->smap) == smap)
> +			break;
> +
> +	if (!selem)
> +		return NULL;
> +
> +	sdata = SDATA(selem);
> +	if (cacheit_lockit) {
> +		/* spinlock is needed to avoid racing with the
> +		 * parallel delete.  Otherwise, publishing an already
> +		 * deleted sdata to the cache will become a use-after-free
> +		 * problem in the next __sk_storage_lookup().
> +		 */
> +		raw_spin_lock_bh(&sk_storage->lock);
> +		if (selem_linked_to_sk(selem))
> +			rcu_assign_pointer(sk_storage->cache[smap->cache_idx],
> +					   sdata);
> +		raw_spin_unlock_bh(&sk_storage->lock);
> +	}
> +
> +	return sdata;
> +}
> +
> +static struct bpf_sk_storage_data *
> +sk_storage_lookup(struct sock *sk, struct bpf_map *map, bool cacheit_lockit)
> +{
> +	struct bpf_sk_storage *sk_storage;
> +	struct bpf_sk_storage_map *smap;
> +
> +	sk_storage = rcu_dereference(sk->sk_bpf_storage);
> +	if (!sk_storage)
> +		return NULL;
> +
> +	smap = (struct bpf_sk_storage_map *)map;
> +	return __sk_storage_lookup(sk_storage, smap, cacheit_lockit);
> +}
> +
> +static int check_flags(const struct bpf_sk_storage_data *old_sdata,
> +		       u64 map_flags)
> +{
> +	if (old_sdata && (map_flags & ~BPF_F_LOCK) == BPF_NOEXIST)
> +		/* elem already exists */
> +		return -EEXIST;
> +
> +	if (!old_sdata && (map_flags & ~BPF_F_LOCK) == BPF_EXIST)
> +		/* elem doesn't exist, cannot update it */
> +		return -ENOENT;
> +
> +	return 0;
> +}
> +
> +static int sk_storage_alloc(struct sock *sk,
> +			    struct bpf_sk_storage_map *smap,
> +			    struct bpf_sk_storage_elem *first_selem)
> +{
> +	struct bpf_sk_storage *prev_sk_storage, *sk_storage;
> +	int err;
> +
> +	err = omem_charge(sk, sizeof(*sk_storage));
> +	if (err)
> +		return err;
> +
> +	sk_storage = kzalloc(sizeof(*sk_storage), GFP_ATOMIC | __GFP_NOWARN);
> +	if (!sk_storage) {
> +		err = -ENOMEM;
> +		goto uncharge;
> +	}
> +	INIT_HLIST_HEAD(&sk_storage->list);
> +	raw_spin_lock_init(&sk_storage->lock);
> +	sk_storage->sk = sk;
> +
> +	__selem_link_sk(sk_storage, first_selem);
> +	selem_link_map(smap, first_selem);
> +	/* Publish sk_storage to sk.  sk->sk_lock cannot be acquired.
> +	 * Hence, atomic ops is used to set sk->sk_bpf_storage
> +	 * from NULL to the newly allocated sk_storage ptr.
> +	 *
> +	 * From now on, the sk->sk_bpf_storage pointer is protected
> +	 * by the sk_storage->lock.  Hence,  when freeing
> +	 * the sk->sk_bpf_storage, the sk_storage->lock must
> +	 * be held before setting sk->sk_bpf_storage to NULL.
> +	 */
> +	prev_sk_storage = cmpxchg((struct bpf_sk_storage **)&sk->sk_bpf_storage,
> +				  NULL, sk_storage);
> +	if (unlikely(prev_sk_storage)) {
> +		selem_unlink_map(first_selem);
> +		err = -EAGAIN;
> +		goto uncharge;
> +
> +		/* Note that even first_selem was linked to smap's
> +		 * bucket->list, first_selem can be freed immediately
> +		 * (instead of kfree_rcu) because
> +		 * bpf_sk_storage_map_free() does a
> +		 * synchronize_rcu() before walking the bucket->list.
> +		 * Hence, no one is accessing selem from the
> +		 * bucket->list under rcu_read_lock().
> +		 */
> +	}
> +
> +	return 0;
> +
> +uncharge:
> +	kfree(sk_storage);
> +	atomic_sub(sizeof(*sk_storage), &sk->sk_omem_alloc);
> +	return err;
> +}
> +
> +/* sk cannot be going away because it is linking new elem
> + * to sk->sk_bpf_storage. (i.e. sk->sk_refcnt cannot be 0).
> + * Otherwise, it will become a leak (and other memory issues
> + * during map destruction).
> + */
> +static struct bpf_sk_storage_data *sk_storage_update(struct sock *sk,
> +						     struct bpf_map *map,
> +						     void *value,
> +						     u64 map_flags)
> +{
> +	struct bpf_sk_storage_data *old_sdata = NULL;
> +	struct bpf_sk_storage_elem *selem;
> +	struct bpf_sk_storage *sk_storage;
> +	struct bpf_sk_storage_map *smap;
> +	int err;
> +
> +	/* BPF_EXIST and BPF_NOEXIST cannot be both set */
> +	if (unlikely((map_flags & ~BPF_F_LOCK) > BPF_EXIST) ||
> +	    /* BPF_F_LOCK can only be used in a value with spin_lock */
> +	    unlikely((map_flags & BPF_F_LOCK) && !map_value_has_spin_lock(map)))
> +		return ERR_PTR(-EINVAL);
> +
> +	smap = (struct bpf_sk_storage_map *)map;
> +	sk_storage = rcu_dereference(sk->sk_bpf_storage);
> +	if (!sk_storage || hlist_empty(&sk_storage->list)) {

Is it possible sk_storage is not NULL and hlist_empty(&sk_storage->list) 
is true? Is this case, should we avoid allocation of new sk_storage at all?

> +		/* Very first elem for this sk */
> +		err = check_flags(NULL, map_flags);
> +		if (err)
> +			return ERR_PTR(err);
> +
> +		selem = selem_alloc(smap, sk, value, true);
> +		if (!selem)
> +			return ERR_PTR(-ENOMEM);
> +
> +		err = sk_storage_alloc(sk, smap, selem);
> +		if (err) {
> +			kfree(selem);
> +			atomic_sub(smap->elem_size, &sk->sk_omem_alloc);

discharding sk->sk_omem_alloc already happens in sk_storage_alloc?

> +			return ERR_PTR(err);
> +		}
> +
> +		return SDATA(selem);
> +	}
> +
> +	if ((map_flags & BPF_F_LOCK) && !(map_flags & BPF_NOEXIST)) {
> +		/* Hoping to find an old_sdata to do inline update
> +		 * such that it can avoid taking the sk_storage->lock
> +		 * and changing the lists.
> +		 */
> +		old_sdata = __sk_storage_lookup(sk_storage, smap, false);
> +		err = check_flags(old_sdata, map_flags);
> +		if (err)
> +			return ERR_PTR(err);
> +		if (old_sdata && selem_linked_to_sk(SELEM(old_sdata))) {
> +			copy_map_value_locked(map, old_sdata->data,
> +					      value, false);
> +			return old_sdata;
> +		}
> +	}
> +
> +	raw_spin_lock_bh(&sk_storage->lock);
> +
> +	/* Recheck sk_storage->list under sk_storage->lock */
> +	if (unlikely(hlist_empty(&sk_storage->list))) {
> +		/* A parallel del is happening and sk_storage is going
> +		 * away.  It has just been checked before, so very
> +		 * unlikely.  Return instead of retry to keep things
> +		 * simple.
> +		 */
> +		err = -EAGAIN;
> +		goto unlock_err;
> +	}
> +
> +	old_sdata = __sk_storage_lookup(sk_storage, smap, false);
> +	err = check_flags(old_sdata, map_flags);
> +	if (err)
> +		goto unlock_err;
> +
> +	if (old_sdata && (map_flags & BPF_F_LOCK)) {
> +		copy_map_value_locked(map, old_sdata->data, value, false);
> +		selem = SELEM(old_sdata);
> +		goto unlock;
> +	}
> +
> +	/* sk_storage->lock is held.  Hence, we are sure
> +	 * we can unlink and uncharge the old_sdata successfully
> +	 * later.  Hence, instead of charging the new selem now
> +	 * and then uncharge the old selem later (which may cause
> +	 * a potential but unnecessary charge failure),  avoid taking
> +	 * a charge at all here (the "!old_sdata" check) and the
> +	 * old_sdata will not be uncharged later during __selem_unlink_sk().
> +	 */
> +	selem = selem_alloc(smap, sk, value, !old_sdata);
> +	if (!selem) {
> +		err = -ENOMEM;
> +		goto unlock_err;
> +	}
> +
> +	/* First, link the new selem to the map */
> +	selem_link_map(smap, selem);
> +
> +	/* Second, link (and publish) the new selem to sk_storage */
> +	__selem_link_sk(sk_storage, selem);
> +
> +	/* Third, remove old selem, SELEM(old_sdata) */
> +	if (old_sdata) {
> +		selem_unlink_map(SELEM(old_sdata));
> +		__selem_unlink_sk(sk_storage, SELEM(old_sdata), false);
> +	}
> +
> +unlock:
> +	raw_spin_unlock_bh(&sk_storage->lock);
> +	return SDATA(selem);
> +
> +unlock_err:
> +	raw_spin_unlock_bh(&sk_storage->lock);
> +	return ERR_PTR(err);
> +}
> +
> +static int sk_storage_delete(struct sock *sk, struct bpf_map *map)
> +{
> +	struct bpf_sk_storage_data *sdata;
> +
> +	sdata = sk_storage_lookup(sk, map, false);
> +	if (!sdata)
> +		return -ENOENT;
> +
> +	selem_unlink(SELEM(sdata));
> +
> +	return 0;
> +}
> +
[...]
> +static struct bpf_map *bpf_sk_storage_map_alloc(union bpf_attr *attr)
> +{
> +	struct bpf_sk_storage_map *smap;
> +	unsigned int i;
> +	u32 nbuckets;
> +	u64 cost;
> +
> +	if (attr->map_flags != BPF_F_NO_PREALLOC || attr->max_entries ||
> +	    attr->key_size != sizeof(int) || !attr->value_size ||
> +	    /* Enforce BTF for userspace sk dumping */
> +	    !attr->btf_key_type_id || !attr->btf_value_type_id)
> +		return ERR_PTR(-EINVAL);
> +
> +	if (!capable(CAP_SYS_ADMIN))
> +		return ERR_PTR(-EPERM);
> +
> +	if (attr->value_size >= KMALLOC_MAX_SIZE -
> +	    MAX_BPF_STACK - sizeof(struct bpf_sk_storage_elem) ||
> +	    /* selem->elem_size is a u16 */
> +	    attr->value_size > U16_MAX - sizeof(struct bpf_sk_storage_elem))
> +		return ERR_PTR(-E2BIG);
> +
> +	smap = kzalloc(sizeof(*smap), GFP_USER | __GFP_NOWARN);
> +	if (!smap)
> +		return ERR_PTR(-ENOMEM);
> +	bpf_map_init_from_attr(&smap->map, attr);
> +
> +	smap->bucket_log = ilog2(roundup_pow_of_two(num_possible_cpus()));
> +	nbuckets = 1U << smap->bucket_log;
> +	smap->buckets = kvcalloc(sizeof(*smap->buckets), nbuckets,
> +				 GFP_USER | __GFP_NOWARN);
> +	if (!smap->buckets) {
> +		kfree(smap);
> +		return ERR_PTR(-ENOMEM);
> +	}
> +	cost = sizeof(*smap->buckets) * nbuckets + sizeof(*smap);
> +
> +	for (i = 0; i < nbuckets; i++) {
> +		INIT_HLIST_HEAD(&smap->buckets[i].list);
> +		raw_spin_lock_init(&smap->buckets[i].lock);
> +	}
> +
> +	smap->elem_size = sizeof(struct bpf_sk_storage_elem) + attr->value_size;
> +	smap->cache_idx = (unsigned int)atomic_inc_return(&cache_idx) %
> +		BPF_SK_STORAGE_CACHE_SIZE;
> +	smap->map.pages = round_up(cost, PAGE_SIZE) >> PAGE_SHIFT;

should we test bpf_map_precharge_memlock() here?

> +
> +	return &smap->map;
> +}
> +
> +static int notsupp_get_next_key(struct bpf_map *map, void *key,
> +				void *next_key)
> +{
> +	return -ENOTSUPP;
> +}
> +
> +static int bpf_sk_storage_map_check_btf(const struct bpf_map *map,
> +					const struct btf *btf,
> +					const struct btf_type *key_type,
> +					const struct btf_type *value_type)
> +{
> +	u32 int_data;
> +
> +	if (BTF_INFO_KIND(key_type->info) != BTF_KIND_INT)
> +		return -EINVAL;
> +
> +	int_data = *(u32 *)(key_type + 1);
> +	if (BTF_INT_BITS(int_data) != 32 || BTF_INT_OFFSET(int_data))
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
[...]> diff --git a/net/core/sock.c b/net/core/sock.c
> index 443b98d05f1e..9773be724aa9 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -137,6 +137,7 @@
>   
>   #include <linux/filter.h>
>   #include <net/sock_reuseport.h>
> +#include <net/bpf_sk_storage.h>
>   
>   #include <trace/events/sock.h>
>   
> @@ -1709,6 +1710,10 @@ static void __sk_destruct(struct rcu_head *head)
>   
>   	sock_disable_timestamp(sk, SK_FLAGS_TIMESTAMP);
>   
> +#ifdef CONFIG_BPF_SYSCALL
> +	bpf_sk_storage_free(sk);
> +#endif
> +
>   	if (atomic_read(&sk->sk_omem_alloc))
>   		pr_debug("%s: optmem leakage (%d bytes) detected\n",
>   			 __func__, atomic_read(&sk->sk_omem_alloc));
> 

Re: [PATCH v2 bpf-next 1/6] bpf: Introduce bpf sk local storage

[Martin Lau]

On Fri, Apr 26, 2019 at 10:27:16AM -0700, Yonghong Song wrote:
> 
> 
> On 4/25/19 5:10 PM, Martin KaFai Lau wrote:
> > After allowing a bpf prog to
> > - directly read the skb->sk ptr
> > - get the fullsock bpf_sock by "bpf_sk_fullsock()"
> > - get the bpf_tcp_sock by "bpf_tcp_sock()"
> > - get the listener sock by "bpf_get_listener_sock()"
> > - avoid duplicating the fields of "(bpf_)sock" and "(bpf_)tcp_sock"
> >    into different bpf running context.
> > 
> > this patch is another effort to make bpf's network programming
> > more intuitive to do (together with memory and performance benefit).
> > 
> > When bpf prog needs to store data for a sk, the current practice is to
> > define a map with the usual 4-tuples (src/dst ip/port) as the key.
> > If multiple bpf progs require to store different sk data, multiple maps
> > have to be defined.  Hence, wasting memory to store the duplicated
> > keys (i.e. 4 tuples here) in each of the bpf map.
> > [ The smallest key could be the sk pointer itself which requires
> >    some enhancement in the verifier and it is a separate topic. ]
> > 
> > Also, the bpf prog needs to clean up the elem when sk is freed.
> > Otherwise, the bpf map will become full and un-usable quickly.
> > The sk-free tracking currently could be done during sk state
> > transition (e.g. BPF_SOCK_OPS_STATE_CB).
> > 
> > The size of the map needs to be predefined which then usually ended-up
> > with an over-provisioned map in production.  Even the map was re-sizable,
> > while the sk naturally come and go away already, this potential re-size
> > operation is arguably redundant if the data can be directly connected
> > to the sk itself instead of proxy-ing through a bpf map.
> > 
> > This patch introduces sk->sk_bpf_storage to provide local storage space
> > at sk for bpf prog to use.  The space will be allocated when the first bpf
> > prog has created data for this particular sk.
> > 
> > The design optimizes the bpf prog's lookup (and then optionally followed by
> > an inline update).  bpf_spin_lock should be used if the inline update needs
> > to be protected.
> > 
> > BPF_MAP_TYPE_SK_STORAGE:
> > -----------------------
> > To define a bpf "sk-local-storage", a BPF_MAP_TYPE_SK_STORAGE map (new in
> > this patch) needs to be created.  Multiple BPF_MAP_TYPE_SK_STORAGE maps can
> > be created to fit different bpf progs' needs.  The map enforces
> > BTF to allow printing the sk-local-storage during a system-wise
> > sk dump (e.g. "ss -ta") in the future.
> > 
> > The purpose of a BPF_MAP_TYPE_SK_STORAGE map is not for lookup/update/delete
> > a "sk-local-storage" data from a particular sk.
> > Think of the map as a meta-data (or "type") of a "sk-local-storage".  This
> > particular "type" of "sk-local-storage" data can then be stored in any sk.
> > 
> > The main purposes of this map are mostly:
> > 1. Define the size of a "sk-local-storage" type.
> > 2. Provide a similar syscall userspace API as the map (e.g. lookup/update,
> >     map-id, map-btf...etc.)
> > 3. Keep track of all sk's storages of this "type" and clean them up
> >     when the map is freed.
> > 
> > sk->sk_bpf_storage:
> > ------------------
> > The main lookup/update/delete is done on sk->sk_bpf_storage (which
> > is a "struct bpf_sk_storage").  When doing a lookup,
> > the "map" pointer is now used as the "key" to search on the
> > sk_storage->list.  The "map" pointer is actually serving
> > as the "type" of the "sk-local-storage" that is being
> > requested.
> > 
> > To allow very fast lookup, it should be as fast as looking up an
> > array at a stable-offset.  At the same time, it is not ideal to
> > set a hard limit on the number of sk-local-storage "type" that the
> > system can have.  Hence, this patch takes a cache approach.
> > The last search result from sk_storage->list is cached in
> > sk_storage->cache[] which is a stable sized array.  Each
> > "sk-local-storage" type has a stable offset to the cache[] array.
> > In the future, a map's flag could be introduced to do cache
> > opt-out/enforcement if it became necessary.
> > 
> > All data will be removed from sk->sk_bpf_storage during
> > sk destruction.
> > 
> > bpf_sk_storage_get() and bpf_sk_storage_delete():
> > ------------------------------------------------
> > Instead of using bpf_map_(lookup|update|delete)_elem(),
> > the bpf prog needs to use the new helper bpf_sk_storage_get() and
> > bpf_sk_storage_delete().  The verifier can then enforce the
> > ARG_PTR_TO_SOCKET argument.  The bpf_sk_storage_get() also allows to
> > "create" new elem if one does not exist in the sk.  It is done by
> > the new BPF_SK_STORAGE_GET_F_CREATE flag.  An optional value can also be
> > provided as the initial value during BPF_SK_STORAGE_GET_F_CREATE.
> > The BPF_MAP_TYPE_SK_STORAGE also supports bpf_spin_lock.  Together,
> > it has eliminated the potential use cases for an equivalent
> > bpf_map_update_elem() API (for bpf_prog) in this patch.
> > 
> > Misc notes:
> > ----------
> > 1. map_get_next_key is not supported.  From the userspace syscall
> >     perspective,  the map has the socket fd as the key while the map
> >     can be shared by pinned-file or map-id.
> > 
> >     Since btf is enforced, the existing "ss" could be enhanced to pretty
> >     print the local-storage.
> > 
> >     Supporting a kernel defined btf with 4 tuples as the return key could
> >     be explored later also.
> > 
> > 2. The sk->sk_lock cannot be acquired.  Atomic operations is used instead.
> >     e.g. cmpxchg is done on the sk->sk_bpf_storage ptr.
> >     Please refer to the source code comments for the details in
> >     synchronization cases and considerations.
> > 
> > 3. The mem is charged to the sk->sk_omem_alloc as the sk filter does.
> > 
> > Benchmark:
> > ---------
> > Here is the benchmark data collected by turning on
> > the "kernel.bpf_stats_enabled" sysctl.
> > Two bpf progs are tested:
> > 
> > One bpf prog with the usual bpf hashmap (max_entries = 8192) with the
> > sk ptr as the key. (verifier is modified to support sk ptr as the key
> > That should have shortened the key lookup time.)
> > 
> > Another bpf prog is with the new BPF_MAP_TYPE_SK_STORAGE.
> > 
> > Both are storing a "u32 cnt", do a lookup on "egress_skb/cgroup" for
> > each egress skb and then bump the cnt.  netperf is used to drive
> > data with 4096 connected UDP sockets.
> > 
> > BPF_MAP_TYPE_HASH with a modifier verifier (152ns per bpf run)
> > 27: cgroup_skb  name egress_sk_map  tag 74f56e832918070b run_time_ns 58280107540 run_cnt 381347633
> >      loaded_at 2019-04-15T13:46:39-0700  uid 0
> >      xlated 344B  jited 258B  memlock 4096B  map_ids 16
> >      btf_id 5
> > 
> > BPF_MAP_TYPE_SK_STORAGE in this patch (66ns per bpf run)
> > 30: cgroup_skb  name egress_sk_stora  tag d4aa70984cc7bbf6 run_time_ns 25617093319 run_cnt 390989739
> >      loaded_at 2019-04-15T13:47:54-0700  uid 0
> >      xlated 168B  jited 156B  memlock 4096B  map_ids 17
> >      btf_id 6
> > 
> > Here is a high-level picture on how are the objects organized:
> > 
> >         sk
> >      +------+
> >      |      |
> >      |      |
> >      |      |
> >      |*sk_bpf_storage-----> bpf_sk_storage
> >      +------+                 +-------+
> >                   +-----------+ list  |
> >                   |           |       |
> >                   |           |       |
> >                   |           |       |
> >                   |           +-------+
> >                   |
> >                   |     elem
> >                   |  +--------+
> >                   +->| snode  |
> >                   |  +--------+
> >                   |  |  data  |          bpf_map
> >                   |  +--------+        +---------+
> >                   |  |map_node|<-+-----+  list   |
> >                   |  +--------+  |     |         |
> >                   |              |     |         |
> >                   |     elem     |     |         |
> >                   |  +--------+  |     +---------+
> >                   +->| snode  |  |
> >                      +--------+  |
> >     bpf_map          |  data  |  |
> >   +---------+        +--------+  |
> >   |  list   +------->|map_node|  |
> >   |         |        +--------+  |
> >   |         |                    |
> >   |         |           elem     |
> >   +---------+        +--------+  |
> >                   +->| snode  |  |
> >                   |  +--------+  |
> >                   |  |  data  |  |
> >                   |  +--------+  |
> >                   |  |map_node|<-+
> >                   |  +--------+
> >                   |
> >                   |
> >                   |          +-------+
> >       sk          +----------| list  |
> >    +------+                  |       |
> >    |      |                  |       |
> >    |      |                  |       |
> >    |      |                  +-------+
> >    |*sk_bpf_storage------->bpf_sk_storage
> >    +------+
> > 
> > Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> 
> Thanks. Generally look good. I have a few comments below.
> 
> > ---
> >   include/linux/bpf.h          |   2 +
> >   include/linux/bpf_types.h    |   1 +
> >   include/net/bpf_sk_storage.h |  13 +
> >   include/net/sock.h           |   5 +
> >   include/uapi/linux/bpf.h     |  44 +-
> >   kernel/bpf/syscall.c         |   3 +-
> >   kernel/bpf/verifier.c        |  27 +-
> >   net/bpf/test_run.c           |   2 +
> >   net/core/Makefile            |   1 +
> >   net/core/bpf_sk_storage.c    | 796 +++++++++++++++++++++++++++++++++++
> >   net/core/filter.c            |  12 +
> >   net/core/sock.c              |   5 +
> >   12 files changed, 906 insertions(+), 5 deletions(-)
> >   create mode 100644 include/net/bpf_sk_storage.h
> >   create mode 100644 net/core/bpf_sk_storage.c
> > 
> > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > index f15432d90728..0d2788beacd2 100644
> > --- a/include/linux/bpf.h
> > +++ b/include/linux/bpf.h
> > @@ -184,6 +184,7 @@ enum bpf_arg_type {
> >   	ARG_PTR_TO_MAP_KEY,	/* pointer to stack used as map key */
> >   	ARG_PTR_TO_MAP_VALUE,	/* pointer to stack used as map value */
> >   	ARG_PTR_TO_UNINIT_MAP_VALUE,	/* pointer to valid memory used to store a map value */
> > +	ARG_PTR_TO_MAP_VALUE_OR_NULL,	/* pointer to stack used as map value or NULL */
> >   
> >   	/* the following constraints used to prototype bpf_memcmp() and other
> >   	 * functions that access data on eBPF program stack
> > @@ -204,6 +205,7 @@ enum bpf_arg_type {
> >   	ARG_PTR_TO_SOCK_COMMON,	/* pointer to sock_common */
> >   	ARG_PTR_TO_INT,		/* pointer to int */
> >   	ARG_PTR_TO_LONG,	/* pointer to long */
> > +	ARG_PTR_TO_SOCKET,	/* pointer to bpf_sock (fullsock) */
> >   };
> >   
> >   /* type of values returned from helper functions */
> > diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
> > index d26991a16894..eb1e4f867a5e 100644
> > --- a/include/linux/bpf_types.h
> > +++ b/include/linux/bpf_types.h
> > @@ -60,6 +60,7 @@ BPF_MAP_TYPE(BPF_MAP_TYPE_ARRAY_OF_MAPS, array_of_maps_map_ops)
> >   BPF_MAP_TYPE(BPF_MAP_TYPE_HASH_OF_MAPS, htab_of_maps_map_ops)
> >   #ifdef CONFIG_NET
> >   BPF_MAP_TYPE(BPF_MAP_TYPE_DEVMAP, dev_map_ops)
> > +BPF_MAP_TYPE(BPF_MAP_TYPE_SK_STORAGE, sk_storage_map_ops)
> >   #if defined(CONFIG_BPF_STREAM_PARSER)
> >   BPF_MAP_TYPE(BPF_MAP_TYPE_SOCKMAP, sock_map_ops)
> >   BPF_MAP_TYPE(BPF_MAP_TYPE_SOCKHASH, sock_hash_ops)
> > diff --git a/include/net/bpf_sk_storage.h b/include/net/bpf_sk_storage.h
> > new file mode 100644
> > index 000000000000..b9dcb02e756b
> > --- /dev/null
> > +++ b/include/net/bpf_sk_storage.h
> > @@ -0,0 +1,13 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/* Copyright (c) 2019 Facebook */
> > +#ifndef _BPF_SK_STORAGE_H
> > +#define _BPF_SK_STORAGE_H
> > +
> > +struct sock;
> > +
> > +void bpf_sk_storage_free(struct sock *sk);
> > +
> > +extern const struct bpf_func_proto bpf_sk_storage_get_proto;
> > +extern const struct bpf_func_proto bpf_sk_storage_delete_proto;
> > +
> > +#endif /* _BPF_SK_STORAGE_H */
> [...]
> > +
> > +static struct bpf_sk_storage_data *
> > +__sk_storage_lookup(struct bpf_sk_storage *sk_storage,
> > +		    struct bpf_sk_storage_map *smap,
> > +		    bool cacheit_lockit)
> > +{
> > +	struct bpf_sk_storage_data *sdata;
> > +	struct bpf_sk_storage_elem *selem;
> > +
> > +	/* Fast path (cache hit) */
> > +	sdata = rcu_dereference(sk_storage->cache[smap->cache_idx]);
> > +	if (sdata && rcu_access_pointer(sdata->smap) == smap)
> > +		return sdata;
> > +
> > +	/* Slow path (cache miss) */
> > +	hlist_for_each_entry_rcu(selem, &sk_storage->list, snode)
> > +		if (rcu_access_pointer(SDATA(selem)->smap) == smap)
> > +			break;
> > +
> > +	if (!selem)
> > +		return NULL;
> > +
> > +	sdata = SDATA(selem);
> > +	if (cacheit_lockit) {
> > +		/* spinlock is needed to avoid racing with the
> > +		 * parallel delete.  Otherwise, publishing an already
> > +		 * deleted sdata to the cache will become a use-after-free
> > +		 * problem in the next __sk_storage_lookup().
> > +		 */
> > +		raw_spin_lock_bh(&sk_storage->lock);
> > +		if (selem_linked_to_sk(selem))
> > +			rcu_assign_pointer(sk_storage->cache[smap->cache_idx],
> > +					   sdata);
> > +		raw_spin_unlock_bh(&sk_storage->lock);
> > +	}
> > +
> > +	return sdata;
> > +}
> > +
> > +static struct bpf_sk_storage_data *
> > +sk_storage_lookup(struct sock *sk, struct bpf_map *map, bool cacheit_lockit)
> > +{
> > +	struct bpf_sk_storage *sk_storage;
> > +	struct bpf_sk_storage_map *smap;
> > +
> > +	sk_storage = rcu_dereference(sk->sk_bpf_storage);
> > +	if (!sk_storage)
> > +		return NULL;
> > +
> > +	smap = (struct bpf_sk_storage_map *)map;
> > +	return __sk_storage_lookup(sk_storage, smap, cacheit_lockit);
> > +}
> > +
> > +static int check_flags(const struct bpf_sk_storage_data *old_sdata,
> > +		       u64 map_flags)
> > +{
> > +	if (old_sdata && (map_flags & ~BPF_F_LOCK) == BPF_NOEXIST)
> > +		/* elem already exists */
> > +		return -EEXIST;
> > +
> > +	if (!old_sdata && (map_flags & ~BPF_F_LOCK) == BPF_EXIST)
> > +		/* elem doesn't exist, cannot update it */
> > +		return -ENOENT;
> > +
> > +	return 0;
> > +}
> > +
> > +static int sk_storage_alloc(struct sock *sk,
> > +			    struct bpf_sk_storage_map *smap,
> > +			    struct bpf_sk_storage_elem *first_selem)
> > +{
> > +	struct bpf_sk_storage *prev_sk_storage, *sk_storage;
> > +	int err;
> > +
> > +	err = omem_charge(sk, sizeof(*sk_storage));
> > +	if (err)
> > +		return err;
> > +
> > +	sk_storage = kzalloc(sizeof(*sk_storage), GFP_ATOMIC | __GFP_NOWARN);
> > +	if (!sk_storage) {
> > +		err = -ENOMEM;
> > +		goto uncharge;
> > +	}
> > +	INIT_HLIST_HEAD(&sk_storage->list);
> > +	raw_spin_lock_init(&sk_storage->lock);
> > +	sk_storage->sk = sk;
> > +
> > +	__selem_link_sk(sk_storage, first_selem);
> > +	selem_link_map(smap, first_selem);
> > +	/* Publish sk_storage to sk.  sk->sk_lock cannot be acquired.
> > +	 * Hence, atomic ops is used to set sk->sk_bpf_storage
> > +	 * from NULL to the newly allocated sk_storage ptr.
> > +	 *
> > +	 * From now on, the sk->sk_bpf_storage pointer is protected
> > +	 * by the sk_storage->lock.  Hence,  when freeing
> > +	 * the sk->sk_bpf_storage, the sk_storage->lock must
> > +	 * be held before setting sk->sk_bpf_storage to NULL.
> > +	 */
> > +	prev_sk_storage = cmpxchg((struct bpf_sk_storage **)&sk->sk_bpf_storage,
> > +				  NULL, sk_storage);
> > +	if (unlikely(prev_sk_storage)) {
> > +		selem_unlink_map(first_selem);
> > +		err = -EAGAIN;
> > +		goto uncharge;
> > +
> > +		/* Note that even first_selem was linked to smap's
> > +		 * bucket->list, first_selem can be freed immediately
> > +		 * (instead of kfree_rcu) because
> > +		 * bpf_sk_storage_map_free() does a
> > +		 * synchronize_rcu() before walking the bucket->list.
> > +		 * Hence, no one is accessing selem from the
> > +		 * bucket->list under rcu_read_lock().
> > +		 */
> > +	}
> > +
> > +	return 0;
> > +
> > +uncharge:
> > +	kfree(sk_storage);
> > +	atomic_sub(sizeof(*sk_storage), &sk->sk_omem_alloc);
> > +	return err;
> > +}
> > +
> > +/* sk cannot be going away because it is linking new elem
> > + * to sk->sk_bpf_storage. (i.e. sk->sk_refcnt cannot be 0).
> > + * Otherwise, it will become a leak (and other memory issues
> > + * during map destruction).
> > + */
> > +static struct bpf_sk_storage_data *sk_storage_update(struct sock *sk,
> > +						     struct bpf_map *map,
> > +						     void *value,
> > +						     u64 map_flags)
> > +{
> > +	struct bpf_sk_storage_data *old_sdata = NULL;
> > +	struct bpf_sk_storage_elem *selem;
> > +	struct bpf_sk_storage *sk_storage;
> > +	struct bpf_sk_storage_map *smap;
> > +	int err;
> > +
> > +	/* BPF_EXIST and BPF_NOEXIST cannot be both set */
> > +	if (unlikely((map_flags & ~BPF_F_LOCK) > BPF_EXIST) ||
> > +	    /* BPF_F_LOCK can only be used in a value with spin_lock */
> > +	    unlikely((map_flags & BPF_F_LOCK) && !map_value_has_spin_lock(map)))
> > +		return ERR_PTR(-EINVAL);
> > +
> > +	smap = (struct bpf_sk_storage_map *)map;
> > +	sk_storage = rcu_dereference(sk->sk_bpf_storage);
> > +	if (!sk_storage || hlist_empty(&sk_storage->list)) {
> 
> Is it possible sk_storage is not NULL and hlist_empty(&sk_storage->list) 
> is true?
Very unlikely but possible.  Note that the sk->sk_bpf_storage ptr is protected
by atomic cmpxchg().

> Is this case, should we avoid allocation of new sk_storage at all?
It should still try which has a good chance that the cmpxchg(..., NULL, ...)
in sk_storage_alloc() will succeed.

> 
> > +		/* Very first elem for this sk */
> > +		err = check_flags(NULL, map_flags);
> > +		if (err)
> > +			return ERR_PTR(err);
> > +
> > +		selem = selem_alloc(smap, sk, value, true);
> > +		if (!selem)
> > +			return ERR_PTR(-ENOMEM);
> > +
> > +		err = sk_storage_alloc(sk, smap, selem);
> > +		if (err) {
> > +			kfree(selem);
> > +			atomic_sub(smap->elem_size, &sk->sk_omem_alloc);
> 
> discharding sk->sk_omem_alloc already happens in sk_storage_alloc?
They are discharging two different things.
sk_storage_alloc() discharges the sk_storage.
Here it dischages the selem.

> 
> > +			return ERR_PTR(err);
> > +		}
> > +
> > +		return SDATA(selem);
> > +	}
> > +
> > +	if ((map_flags & BPF_F_LOCK) && !(map_flags & BPF_NOEXIST)) {
> > +		/* Hoping to find an old_sdata to do inline update
> > +		 * such that it can avoid taking the sk_storage->lock
> > +		 * and changing the lists.
> > +		 */
> > +		old_sdata = __sk_storage_lookup(sk_storage, smap, false);
> > +		err = check_flags(old_sdata, map_flags);
> > +		if (err)
> > +			return ERR_PTR(err);
> > +		if (old_sdata && selem_linked_to_sk(SELEM(old_sdata))) {
> > +			copy_map_value_locked(map, old_sdata->data,
> > +					      value, false);
> > +			return old_sdata;
> > +		}
> > +	}
> > +
> > +	raw_spin_lock_bh(&sk_storage->lock);
> > +
> > +	/* Recheck sk_storage->list under sk_storage->lock */
> > +	if (unlikely(hlist_empty(&sk_storage->list))) {
> > +		/* A parallel del is happening and sk_storage is going
> > +		 * away.  It has just been checked before, so very
> > +		 * unlikely.  Return instead of retry to keep things
> > +		 * simple.
> > +		 */
> > +		err = -EAGAIN;
> > +		goto unlock_err;
> > +	}
> > +
> > +	old_sdata = __sk_storage_lookup(sk_storage, smap, false);
> > +	err = check_flags(old_sdata, map_flags);
> > +	if (err)
> > +		goto unlock_err;
> > +
> > +	if (old_sdata && (map_flags & BPF_F_LOCK)) {
> > +		copy_map_value_locked(map, old_sdata->data, value, false);
> > +		selem = SELEM(old_sdata);
> > +		goto unlock;
> > +	}
> > +
> > +	/* sk_storage->lock is held.  Hence, we are sure
> > +	 * we can unlink and uncharge the old_sdata successfully
> > +	 * later.  Hence, instead of charging the new selem now
> > +	 * and then uncharge the old selem later (which may cause
> > +	 * a potential but unnecessary charge failure),  avoid taking
> > +	 * a charge at all here (the "!old_sdata" check) and the
> > +	 * old_sdata will not be uncharged later during __selem_unlink_sk().
> > +	 */
> > +	selem = selem_alloc(smap, sk, value, !old_sdata);
> > +	if (!selem) {
> > +		err = -ENOMEM;
> > +		goto unlock_err;
> > +	}
> > +
> > +	/* First, link the new selem to the map */
> > +	selem_link_map(smap, selem);
> > +
> > +	/* Second, link (and publish) the new selem to sk_storage */
> > +	__selem_link_sk(sk_storage, selem);
> > +
> > +	/* Third, remove old selem, SELEM(old_sdata) */
> > +	if (old_sdata) {
> > +		selem_unlink_map(SELEM(old_sdata));
> > +		__selem_unlink_sk(sk_storage, SELEM(old_sdata), false);
> > +	}
> > +
> > +unlock:
> > +	raw_spin_unlock_bh(&sk_storage->lock);
> > +	return SDATA(selem);
> > +
> > +unlock_err:
> > +	raw_spin_unlock_bh(&sk_storage->lock);
> > +	return ERR_PTR(err);
> > +}
> > +
> > +static int sk_storage_delete(struct sock *sk, struct bpf_map *map)
> > +{
> > +	struct bpf_sk_storage_data *sdata;
> > +
> > +	sdata = sk_storage_lookup(sk, map, false);
> > +	if (!sdata)
> > +		return -ENOENT;
> > +
> > +	selem_unlink(SELEM(sdata));
> > +
> > +	return 0;
> > +}
> > +
> [...]
> > +static struct bpf_map *bpf_sk_storage_map_alloc(union bpf_attr *attr)
> > +{
> > +	struct bpf_sk_storage_map *smap;
> > +	unsigned int i;
> > +	u32 nbuckets;
> > +	u64 cost;
> > +
> > +	if (attr->map_flags != BPF_F_NO_PREALLOC || attr->max_entries ||
> > +	    attr->key_size != sizeof(int) || !attr->value_size ||
> > +	    /* Enforce BTF for userspace sk dumping */
> > +	    !attr->btf_key_type_id || !attr->btf_value_type_id)
> > +		return ERR_PTR(-EINVAL);
> > +
> > +	if (!capable(CAP_SYS_ADMIN))
> > +		return ERR_PTR(-EPERM);
> > +
> > +	if (attr->value_size >= KMALLOC_MAX_SIZE -
> > +	    MAX_BPF_STACK - sizeof(struct bpf_sk_storage_elem) ||
> > +	    /* selem->elem_size is a u16 */
> > +	    attr->value_size > U16_MAX - sizeof(struct bpf_sk_storage_elem))
> > +		return ERR_PTR(-E2BIG);
> > +
> > +	smap = kzalloc(sizeof(*smap), GFP_USER | __GFP_NOWARN);
> > +	if (!smap)
> > +		return ERR_PTR(-ENOMEM);
> > +	bpf_map_init_from_attr(&smap->map, attr);
> > +
> > +	smap->bucket_log = ilog2(roundup_pow_of_two(num_possible_cpus()));
> > +	nbuckets = 1U << smap->bucket_log;
> > +	smap->buckets = kvcalloc(sizeof(*smap->buckets), nbuckets,
> > +				 GFP_USER | __GFP_NOWARN);
> > +	if (!smap->buckets) {
> > +		kfree(smap);
> > +		return ERR_PTR(-ENOMEM);
> > +	}
> > +	cost = sizeof(*smap->buckets) * nbuckets + sizeof(*smap);
> > +
> > +	for (i = 0; i < nbuckets; i++) {
> > +		INIT_HLIST_HEAD(&smap->buckets[i].list);
> > +		raw_spin_lock_init(&smap->buckets[i].lock);
> > +	}
> > +
> > +	smap->elem_size = sizeof(struct bpf_sk_storage_elem) + attr->value_size;
> > +	smap->cache_idx = (unsigned int)atomic_inc_return(&cache_idx) %
> > +		BPF_SK_STORAGE_CACHE_SIZE;
> > +	smap->map.pages = round_up(cost, PAGE_SIZE) >> PAGE_SHIFT;
> 
> should we test bpf_map_precharge_memlock() here?
I did not do it here because the size of this map does not depend on
"map.max_entries".  Meaning,
this map merely needs the memory for the buckets which is less likely to
fail (and destroying in case of final charge failure is not that expensive
either) which then makes this precheck not as useful as other maps.

Thanks for reviewing!

> 
> > +
> > +	return &smap->map;
> > +}
> > +
> > +static int notsupp_get_next_key(struct bpf_map *map, void *key,
> > +				void *next_key)
> > +{
> > +	return -ENOTSUPP;
> > +}
> > +
> > +static int bpf_sk_storage_map_check_btf(const struct bpf_map *map,
> > +					const struct btf *btf,
> > +					const struct btf_type *key_type,
> > +					const struct btf_type *value_type)
> > +{
> > +	u32 int_data;
> > +
> > +	if (BTF_INFO_KIND(key_type->info) != BTF_KIND_INT)
> > +		return -EINVAL;
> > +
> > +	int_data = *(u32 *)(key_type + 1);
> > +	if (BTF_INT_BITS(int_data) != 32 || BTF_INT_OFFSET(int_data))
> > +		return -EINVAL;
> > +
> > +	return 0;
> > +}
> > +
> [...]> diff --git a/net/core/sock.c b/net/core/sock.c
> > index 443b98d05f1e..9773be724aa9 100644
> > --- a/net/core/sock.c
> > +++ b/net/core/sock.c
> > @@ -137,6 +137,7 @@
> >   
> >   #include <linux/filter.h>
> >   #include <net/sock_reuseport.h>
> > +#include <net/bpf_sk_storage.h>
> >   
> >   #include <trace/events/sock.h>
> >   
> > @@ -1709,6 +1710,10 @@ static void __sk_destruct(struct rcu_head *head)
> >   
> >   	sock_disable_timestamp(sk, SK_FLAGS_TIMESTAMP);
> >   
> > +#ifdef CONFIG_BPF_SYSCALL
> > +	bpf_sk_storage_free(sk);
> > +#endif
> > +
> >   	if (atomic_read(&sk->sk_omem_alloc))
> >   		pr_debug("%s: optmem leakage (%d bytes) detected\n",
> >   			 __func__, atomic_read(&sk->sk_omem_alloc));
> > 

Re: [PATCH v2 bpf-next 1/6] bpf: Introduce bpf sk local storage

[Yonghong Song]

On 4/26/19 11:04 AM, Martin Lau wrote:
> On Fri, Apr 26, 2019 at 10:27:16AM -0700, Yonghong Song wrote:
>>
>>
>> On 4/25/19 5:10 PM, Martin KaFai Lau wrote:
>>> After allowing a bpf prog to
>>> - directly read the skb->sk ptr
>>> - get the fullsock bpf_sock by "bpf_sk_fullsock()"
>>> - get the bpf_tcp_sock by "bpf_tcp_sock()"
>>> - get the listener sock by "bpf_get_listener_sock()"
>>> - avoid duplicating the fields of "(bpf_)sock" and "(bpf_)tcp_sock"
>>>     into different bpf running context.
>>>
>>> this patch is another effort to make bpf's network programming
>>> more intuitive to do (together with memory and performance benefit).
>>>
>>> When bpf prog needs to store data for a sk, the current practice is to
>>> define a map with the usual 4-tuples (src/dst ip/port) as the key.
>>> If multiple bpf progs require to store different sk data, multiple maps
>>> have to be defined.  Hence, wasting memory to store the duplicated
>>> keys (i.e. 4 tuples here) in each of the bpf map.
>>> [ The smallest key could be the sk pointer itself which requires
>>>     some enhancement in the verifier and it is a separate topic. ]
>>>
>>> Also, the bpf prog needs to clean up the elem when sk is freed.
>>> Otherwise, the bpf map will become full and un-usable quickly.
>>> The sk-free tracking currently could be done during sk state
>>> transition (e.g. BPF_SOCK_OPS_STATE_CB).
>>>
>>> The size of the map needs to be predefined which then usually ended-up
>>> with an over-provisioned map in production.  Even the map was re-sizable,
>>> while the sk naturally come and go away already, this potential re-size
>>> operation is arguably redundant if the data can be directly connected
>>> to the sk itself instead of proxy-ing through a bpf map.
>>>
>>> This patch introduces sk->sk_bpf_storage to provide local storage space
>>> at sk for bpf prog to use.  The space will be allocated when the first bpf
>>> prog has created data for this particular sk.
>>>
>>> The design optimizes the bpf prog's lookup (and then optionally followed by
>>> an inline update).  bpf_spin_lock should be used if the inline update needs
>>> to be protected.
>>>
>>> BPF_MAP_TYPE_SK_STORAGE:
>>> -----------------------
>>> To define a bpf "sk-local-storage", a BPF_MAP_TYPE_SK_STORAGE map (new in
>>> this patch) needs to be created.  Multiple BPF_MAP_TYPE_SK_STORAGE maps can
>>> be created to fit different bpf progs' needs.  The map enforces
>>> BTF to allow printing the sk-local-storage during a system-wise
>>> sk dump (e.g. "ss -ta") in the future.
>>>
>>> The purpose of a BPF_MAP_TYPE_SK_STORAGE map is not for lookup/update/delete
>>> a "sk-local-storage" data from a particular sk.
>>> Think of the map as a meta-data (or "type") of a "sk-local-storage".  This
>>> particular "type" of "sk-local-storage" data can then be stored in any sk.
>>>
>>> The main purposes of this map are mostly:
>>> 1. Define the size of a "sk-local-storage" type.
>>> 2. Provide a similar syscall userspace API as the map (e.g. lookup/update,
>>>      map-id, map-btf...etc.)
>>> 3. Keep track of all sk's storages of this "type" and clean them up
>>>      when the map is freed.
>>>
>>> sk->sk_bpf_storage:
>>> ------------------
>>> The main lookup/update/delete is done on sk->sk_bpf_storage (which
>>> is a "struct bpf_sk_storage").  When doing a lookup,
>>> the "map" pointer is now used as the "key" to search on the
>>> sk_storage->list.  The "map" pointer is actually serving
>>> as the "type" of the "sk-local-storage" that is being
>>> requested.
>>>
>>> To allow very fast lookup, it should be as fast as looking up an
>>> array at a stable-offset.  At the same time, it is not ideal to
>>> set a hard limit on the number of sk-local-storage "type" that the
>>> system can have.  Hence, this patch takes a cache approach.
>>> The last search result from sk_storage->list is cached in
>>> sk_storage->cache[] which is a stable sized array.  Each
>>> "sk-local-storage" type has a stable offset to the cache[] array.
>>> In the future, a map's flag could be introduced to do cache
>>> opt-out/enforcement if it became necessary.
>>>
>>> All data will be removed from sk->sk_bpf_storage during
>>> sk destruction.
>>>
>>> bpf_sk_storage_get() and bpf_sk_storage_delete():
>>> ------------------------------------------------
>>> Instead of using bpf_map_(lookup|update|delete)_elem(),
>>> the bpf prog needs to use the new helper bpf_sk_storage_get() and
>>> bpf_sk_storage_delete().  The verifier can then enforce the
>>> ARG_PTR_TO_SOCKET argument.  The bpf_sk_storage_get() also allows to
>>> "create" new elem if one does not exist in the sk.  It is done by
>>> the new BPF_SK_STORAGE_GET_F_CREATE flag.  An optional value can also be
>>> provided as the initial value during BPF_SK_STORAGE_GET_F_CREATE.
>>> The BPF_MAP_TYPE_SK_STORAGE also supports bpf_spin_lock.  Together,
>>> it has eliminated the potential use cases for an equivalent
>>> bpf_map_update_elem() API (for bpf_prog) in this patch.
>>>
>>> Misc notes:
>>> ----------
>>> 1. map_get_next_key is not supported.  From the userspace syscall
>>>      perspective,  the map has the socket fd as the key while the map
>>>      can be shared by pinned-file or map-id.
>>>
>>>      Since btf is enforced, the existing "ss" could be enhanced to pretty
>>>      print the local-storage.
>>>
>>>      Supporting a kernel defined btf with 4 tuples as the return key could
>>>      be explored later also.
>>>
>>> 2. The sk->sk_lock cannot be acquired.  Atomic operations is used instead.
>>>      e.g. cmpxchg is done on the sk->sk_bpf_storage ptr.
>>>      Please refer to the source code comments for the details in
>>>      synchronization cases and considerations.
>>>
>>> 3. The mem is charged to the sk->sk_omem_alloc as the sk filter does.
>>>
>>> Benchmark:
>>> ---------
>>> Here is the benchmark data collected by turning on
>>> the "kernel.bpf_stats_enabled" sysctl.
>>> Two bpf progs are tested:
>>>
>>> One bpf prog with the usual bpf hashmap (max_entries = 8192) with the
>>> sk ptr as the key. (verifier is modified to support sk ptr as the key
>>> That should have shortened the key lookup time.)
>>>
>>> Another bpf prog is with the new BPF_MAP_TYPE_SK_STORAGE.
>>>
>>> Both are storing a "u32 cnt", do a lookup on "egress_skb/cgroup" for
>>> each egress skb and then bump the cnt.  netperf is used to drive
>>> data with 4096 connected UDP sockets.
>>>
>>> BPF_MAP_TYPE_HASH with a modifier verifier (152ns per bpf run)
>>> 27: cgroup_skb  name egress_sk_map  tag 74f56e832918070b run_time_ns 58280107540 run_cnt 381347633
>>>       loaded_at 2019-04-15T13:46:39-0700  uid 0
>>>       xlated 344B  jited 258B  memlock 4096B  map_ids 16
>>>       btf_id 5
>>>
>>> BPF_MAP_TYPE_SK_STORAGE in this patch (66ns per bpf run)
>>> 30: cgroup_skb  name egress_sk_stora  tag d4aa70984cc7bbf6 run_time_ns 25617093319 run_cnt 390989739
>>>       loaded_at 2019-04-15T13:47:54-0700  uid 0
>>>       xlated 168B  jited 156B  memlock 4096B  map_ids 17
>>>       btf_id 6
>>>
>>> Here is a high-level picture on how are the objects organized:
>>>
>>>          sk
>>>       +------+
>>>       |      |
>>>       |      |
>>>       |      |
>>>       |*sk_bpf_storage-----> bpf_sk_storage
>>>       +------+                 +-------+
>>>                    +-----------+ list  |
>>>                    |           |       |
>>>                    |           |       |
>>>                    |           |       |
>>>                    |           +-------+
>>>                    |
>>>                    |     elem
>>>                    |  +--------+
>>>                    +->| snode  |
>>>                    |  +--------+
>>>                    |  |  data  |          bpf_map
>>>                    |  +--------+        +---------+
>>>                    |  |map_node|<-+-----+  list   |
>>>                    |  +--------+  |     |         |
>>>                    |              |     |         |
>>>                    |     elem     |     |         |
>>>                    |  +--------+  |     +---------+
>>>                    +->| snode  |  |
>>>                       +--------+  |
>>>      bpf_map          |  data  |  |
>>>    +---------+        +--------+  |
>>>    |  list   +------->|map_node|  |
>>>    |         |        +--------+  |
>>>    |         |                    |
>>>    |         |           elem     |
>>>    +---------+        +--------+  |
>>>                    +->| snode  |  |
>>>                    |  +--------+  |
>>>                    |  |  data  |  |
>>>                    |  +--------+  |
>>>                    |  |map_node|<-+
>>>                    |  +--------+
>>>                    |
>>>                    |
>>>                    |          +-------+
>>>        sk          +----------| list  |
>>>     +------+                  |       |
>>>     |      |                  |       |
>>>     |      |                  |       |
>>>     |      |                  +-------+
>>>     |*sk_bpf_storage------->bpf_sk_storage
>>>     +------+
>>>
>>> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
>>
>> Thanks. Generally look good. I have a few comments below.
>>
>>> ---
>>>    include/linux/bpf.h          |   2 +
>>>    include/linux/bpf_types.h    |   1 +
>>>    include/net/bpf_sk_storage.h |  13 +
>>>    include/net/sock.h           |   5 +
>>>    include/uapi/linux/bpf.h     |  44 +-
>>>    kernel/bpf/syscall.c         |   3 +-
>>>    kernel/bpf/verifier.c        |  27 +-
>>>    net/bpf/test_run.c           |   2 +
>>>    net/core/Makefile            |   1 +
>>>    net/core/bpf_sk_storage.c    | 796 +++++++++++++++++++++++++++++++++++
>>>    net/core/filter.c            |  12 +
>>>    net/core/sock.c              |   5 +
>>>    12 files changed, 906 insertions(+), 5 deletions(-)
>>>    create mode 100644 include/net/bpf_sk_storage.h
>>>    create mode 100644 net/core/bpf_sk_storage.c
>>>
>>> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
>>> index f15432d90728..0d2788beacd2 100644
>>> --- a/include/linux/bpf.h
>>> +++ b/include/linux/bpf.h
>>> @@ -184,6 +184,7 @@ enum bpf_arg_type {
>>>    	ARG_PTR_TO_MAP_KEY,	/* pointer to stack used as map key */
>>>    	ARG_PTR_TO_MAP_VALUE,	/* pointer to stack used as map value */
>>>    	ARG_PTR_TO_UNINIT_MAP_VALUE,	/* pointer to valid memory used to store a map value */
>>> +	ARG_PTR_TO_MAP_VALUE_OR_NULL,	/* pointer to stack used as map value or NULL */
>>>    
>>>    	/* the following constraints used to prototype bpf_memcmp() and other
>>>    	 * functions that access data on eBPF program stack
>>> @@ -204,6 +205,7 @@ enum bpf_arg_type {
>>>    	ARG_PTR_TO_SOCK_COMMON,	/* pointer to sock_common */
>>>    	ARG_PTR_TO_INT,		/* pointer to int */
>>>    	ARG_PTR_TO_LONG,	/* pointer to long */
>>> +	ARG_PTR_TO_SOCKET,	/* pointer to bpf_sock (fullsock) */
>>>    };
>>>    
>>>    /* type of values returned from helper functions */
>>> diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
>>> index d26991a16894..eb1e4f867a5e 100644
>>> --- a/include/linux/bpf_types.h
>>> +++ b/include/linux/bpf_types.h
>>> @@ -60,6 +60,7 @@ BPF_MAP_TYPE(BPF_MAP_TYPE_ARRAY_OF_MAPS, array_of_maps_map_ops)
>>>    BPF_MAP_TYPE(BPF_MAP_TYPE_HASH_OF_MAPS, htab_of_maps_map_ops)
>>>    #ifdef CONFIG_NET
>>>    BPF_MAP_TYPE(BPF_MAP_TYPE_DEVMAP, dev_map_ops)
>>> +BPF_MAP_TYPE(BPF_MAP_TYPE_SK_STORAGE, sk_storage_map_ops)
>>>    #if defined(CONFIG_BPF_STREAM_PARSER)
>>>    BPF_MAP_TYPE(BPF_MAP_TYPE_SOCKMAP, sock_map_ops)
>>>    BPF_MAP_TYPE(BPF_MAP_TYPE_SOCKHASH, sock_hash_ops)
>>> diff --git a/include/net/bpf_sk_storage.h b/include/net/bpf_sk_storage.h
>>> new file mode 100644
>>> index 000000000000..b9dcb02e756b
>>> --- /dev/null
>>> +++ b/include/net/bpf_sk_storage.h
>>> @@ -0,0 +1,13 @@
>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>> +/* Copyright (c) 2019 Facebook */
>>> +#ifndef _BPF_SK_STORAGE_H
>>> +#define _BPF_SK_STORAGE_H
>>> +
>>> +struct sock;
>>> +
>>> +void bpf_sk_storage_free(struct sock *sk);
>>> +
>>> +extern const struct bpf_func_proto bpf_sk_storage_get_proto;
>>> +extern const struct bpf_func_proto bpf_sk_storage_delete_proto;
>>> +
>>> +#endif /* _BPF_SK_STORAGE_H */
>> [...]
>>> +
>>> +static struct bpf_sk_storage_data *
>>> +__sk_storage_lookup(struct bpf_sk_storage *sk_storage,
>>> +		    struct bpf_sk_storage_map *smap,
>>> +		    bool cacheit_lockit)
>>> +{
>>> +	struct bpf_sk_storage_data *sdata;
>>> +	struct bpf_sk_storage_elem *selem;
>>> +
>>> +	/* Fast path (cache hit) */
>>> +	sdata = rcu_dereference(sk_storage->cache[smap->cache_idx]);
>>> +	if (sdata && rcu_access_pointer(sdata->smap) == smap)
>>> +		return sdata;
>>> +
>>> +	/* Slow path (cache miss) */
>>> +	hlist_for_each_entry_rcu(selem, &sk_storage->list, snode)
>>> +		if (rcu_access_pointer(SDATA(selem)->smap) == smap)
>>> +			break;
>>> +
>>> +	if (!selem)
>>> +		return NULL;
>>> +
>>> +	sdata = SDATA(selem);
>>> +	if (cacheit_lockit) {
>>> +		/* spinlock is needed to avoid racing with the
>>> +		 * parallel delete.  Otherwise, publishing an already
>>> +		 * deleted sdata to the cache will become a use-after-free
>>> +		 * problem in the next __sk_storage_lookup().
>>> +		 */
>>> +		raw_spin_lock_bh(&sk_storage->lock);
>>> +		if (selem_linked_to_sk(selem))
>>> +			rcu_assign_pointer(sk_storage->cache[smap->cache_idx],
>>> +					   sdata);
>>> +		raw_spin_unlock_bh(&sk_storage->lock);
>>> +	}
>>> +
>>> +	return sdata;
>>> +}
>>> +
>>> +static struct bpf_sk_storage_data *
>>> +sk_storage_lookup(struct sock *sk, struct bpf_map *map, bool cacheit_lockit)
>>> +{
>>> +	struct bpf_sk_storage *sk_storage;
>>> +	struct bpf_sk_storage_map *smap;
>>> +
>>> +	sk_storage = rcu_dereference(sk->sk_bpf_storage);
>>> +	if (!sk_storage)
>>> +		return NULL;
>>> +
>>> +	smap = (struct bpf_sk_storage_map *)map;
>>> +	return __sk_storage_lookup(sk_storage, smap, cacheit_lockit);
>>> +}
>>> +
>>> +static int check_flags(const struct bpf_sk_storage_data *old_sdata,
>>> +		       u64 map_flags)
>>> +{
>>> +	if (old_sdata && (map_flags & ~BPF_F_LOCK) == BPF_NOEXIST)
>>> +		/* elem already exists */
>>> +		return -EEXIST;
>>> +
>>> +	if (!old_sdata && (map_flags & ~BPF_F_LOCK) == BPF_EXIST)
>>> +		/* elem doesn't exist, cannot update it */
>>> +		return -ENOENT;
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static int sk_storage_alloc(struct sock *sk,
>>> +			    struct bpf_sk_storage_map *smap,
>>> +			    struct bpf_sk_storage_elem *first_selem)
>>> +{
>>> +	struct bpf_sk_storage *prev_sk_storage, *sk_storage;
>>> +	int err;
>>> +
>>> +	err = omem_charge(sk, sizeof(*sk_storage));
>>> +	if (err)
>>> +		return err;
>>> +
>>> +	sk_storage = kzalloc(sizeof(*sk_storage), GFP_ATOMIC | __GFP_NOWARN);
>>> +	if (!sk_storage) {
>>> +		err = -ENOMEM;
>>> +		goto uncharge;
>>> +	}
>>> +	INIT_HLIST_HEAD(&sk_storage->list);
>>> +	raw_spin_lock_init(&sk_storage->lock);
>>> +	sk_storage->sk = sk;
>>> +
>>> +	__selem_link_sk(sk_storage, first_selem);
>>> +	selem_link_map(smap, first_selem);
>>> +	/* Publish sk_storage to sk.  sk->sk_lock cannot be acquired.
>>> +	 * Hence, atomic ops is used to set sk->sk_bpf_storage
>>> +	 * from NULL to the newly allocated sk_storage ptr.
>>> +	 *
>>> +	 * From now on, the sk->sk_bpf_storage pointer is protected
>>> +	 * by the sk_storage->lock.  Hence,  when freeing
>>> +	 * the sk->sk_bpf_storage, the sk_storage->lock must
>>> +	 * be held before setting sk->sk_bpf_storage to NULL.
>>> +	 */
>>> +	prev_sk_storage = cmpxchg((struct bpf_sk_storage **)&sk->sk_bpf_storage,
>>> +				  NULL, sk_storage);
>>> +	if (unlikely(prev_sk_storage)) {
>>> +		selem_unlink_map(first_selem);
>>> +		err = -EAGAIN;
>>> +		goto uncharge;
>>> +
>>> +		/* Note that even first_selem was linked to smap's
>>> +		 * bucket->list, first_selem can be freed immediately
>>> +		 * (instead of kfree_rcu) because
>>> +		 * bpf_sk_storage_map_free() does a
>>> +		 * synchronize_rcu() before walking the bucket->list.
>>> +		 * Hence, no one is accessing selem from the
>>> +		 * bucket->list under rcu_read_lock().
>>> +		 */
>>> +	}
>>> +
>>> +	return 0;
>>> +
>>> +uncharge:
>>> +	kfree(sk_storage);
>>> +	atomic_sub(sizeof(*sk_storage), &sk->sk_omem_alloc);
>>> +	return err;
>>> +}
>>> +
>>> +/* sk cannot be going away because it is linking new elem
>>> + * to sk->sk_bpf_storage. (i.e. sk->sk_refcnt cannot be 0).
>>> + * Otherwise, it will become a leak (and other memory issues
>>> + * during map destruction).
>>> + */
>>> +static struct bpf_sk_storage_data *sk_storage_update(struct sock *sk,
>>> +						     struct bpf_map *map,
>>> +						     void *value,
>>> +						     u64 map_flags)
>>> +{
>>> +	struct bpf_sk_storage_data *old_sdata = NULL;
>>> +	struct bpf_sk_storage_elem *selem;
>>> +	struct bpf_sk_storage *sk_storage;
>>> +	struct bpf_sk_storage_map *smap;
>>> +	int err;
>>> +
>>> +	/* BPF_EXIST and BPF_NOEXIST cannot be both set */
>>> +	if (unlikely((map_flags & ~BPF_F_LOCK) > BPF_EXIST) ||
>>> +	    /* BPF_F_LOCK can only be used in a value with spin_lock */
>>> +	    unlikely((map_flags & BPF_F_LOCK) && !map_value_has_spin_lock(map)))
>>> +		return ERR_PTR(-EINVAL);
>>> +
>>> +	smap = (struct bpf_sk_storage_map *)map;
>>> +	sk_storage = rcu_dereference(sk->sk_bpf_storage);
>>> +	if (!sk_storage || hlist_empty(&sk_storage->list)) {
>>
>> Is it possible sk_storage is not NULL and hlist_empty(&sk_storage->list)
>> is true?
> Very unlikely but possible.  Note that the sk->sk_bpf_storage ptr is protected
> by atomic cmpxchg().
> 
>> Is this case, should we avoid allocation of new sk_storage at all?
> It should still try which has a good chance that the cmpxchg(..., NULL, ...)
> in sk_storage_alloc() will succeed.

Right. This is a rare situation. I guess there is no need to optimize 
for it.

> 
>>
>>> +		/* Very first elem for this sk */
>>> +		err = check_flags(NULL, map_flags);
>>> +		if (err)
>>> +			return ERR_PTR(err);
>>> +
>>> +		selem = selem_alloc(smap, sk, value, true);
>>> +		if (!selem)
>>> +			return ERR_PTR(-ENOMEM);
>>> +
>>> +		err = sk_storage_alloc(sk, smap, selem);
>>> +		if (err) {
>>> +			kfree(selem);
>>> +			atomic_sub(smap->elem_size, &sk->sk_omem_alloc);
>>
>> discharding sk->sk_omem_alloc already happens in sk_storage_alloc?
> They are discharging two different things.
> sk_storage_alloc() discharges the sk_storage.
> Here it dischages the selem.

Yes, make sense.

> 
>>
>>> +			return ERR_PTR(err);
>>> +		}
>>> +
>>> +		return SDATA(selem);
>>> +	}
>>> +
>>> +	if ((map_flags & BPF_F_LOCK) && !(map_flags & BPF_NOEXIST)) {
>>> +		/* Hoping to find an old_sdata to do inline update
>>> +		 * such that it can avoid taking the sk_storage->lock
>>> +		 * and changing the lists.
>>> +		 */
>>> +		old_sdata = __sk_storage_lookup(sk_storage, smap, false);
>>> +		err = check_flags(old_sdata, map_flags);
>>> +		if (err)
>>> +			return ERR_PTR(err);
>>> +		if (old_sdata && selem_linked_to_sk(SELEM(old_sdata))) {
>>> +			copy_map_value_locked(map, old_sdata->data,
>>> +					      value, false);
>>> +			return old_sdata;
>>> +		}
>>> +	}
>>> +
>>> +	raw_spin_lock_bh(&sk_storage->lock);
>>> +
>>> +	/* Recheck sk_storage->list under sk_storage->lock */
>>> +	if (unlikely(hlist_empty(&sk_storage->list))) {
>>> +		/* A parallel del is happening and sk_storage is going
>>> +		 * away.  It has just been checked before, so very
>>> +		 * unlikely.  Return instead of retry to keep things
>>> +		 * simple.
>>> +		 */
>>> +		err = -EAGAIN;
>>> +		goto unlock_err;
>>> +	}
>>> +
>>> +	old_sdata = __sk_storage_lookup(sk_storage, smap, false);
>>> +	err = check_flags(old_sdata, map_flags);
>>> +	if (err)
>>> +		goto unlock_err;
>>> +
>>> +	if (old_sdata && (map_flags & BPF_F_LOCK)) {
>>> +		copy_map_value_locked(map, old_sdata->data, value, false);
>>> +		selem = SELEM(old_sdata);
>>> +		goto unlock;
>>> +	}
>>> +
>>> +	/* sk_storage->lock is held.  Hence, we are sure
>>> +	 * we can unlink and uncharge the old_sdata successfully
>>> +	 * later.  Hence, instead of charging the new selem now
>>> +	 * and then uncharge the old selem later (which may cause
>>> +	 * a potential but unnecessary charge failure),  avoid taking
>>> +	 * a charge at all here (the "!old_sdata" check) and the
>>> +	 * old_sdata will not be uncharged later during __selem_unlink_sk().
>>> +	 */
>>> +	selem = selem_alloc(smap, sk, value, !old_sdata);
>>> +	if (!selem) {
>>> +		err = -ENOMEM;
>>> +		goto unlock_err;
>>> +	}
>>> +
>>> +	/* First, link the new selem to the map */
>>> +	selem_link_map(smap, selem);
>>> +
>>> +	/* Second, link (and publish) the new selem to sk_storage */
>>> +	__selem_link_sk(sk_storage, selem);
>>> +
>>> +	/* Third, remove old selem, SELEM(old_sdata) */
>>> +	if (old_sdata) {
>>> +		selem_unlink_map(SELEM(old_sdata));
>>> +		__selem_unlink_sk(sk_storage, SELEM(old_sdata), false);
>>> +	}
>>> +
>>> +unlock:
>>> +	raw_spin_unlock_bh(&sk_storage->lock);
>>> +	return SDATA(selem);
>>> +
>>> +unlock_err:
>>> +	raw_spin_unlock_bh(&sk_storage->lock);
>>> +	return ERR_PTR(err);
>>> +}
>>> +
>>> +static int sk_storage_delete(struct sock *sk, struct bpf_map *map)
>>> +{
>>> +	struct bpf_sk_storage_data *sdata;
>>> +
>>> +	sdata = sk_storage_lookup(sk, map, false);
>>> +	if (!sdata)
>>> +		return -ENOENT;
>>> +
>>> +	selem_unlink(SELEM(sdata));
>>> +
>>> +	return 0;
>>> +}
>>> +
>> [...]
>>> +static struct bpf_map *bpf_sk_storage_map_alloc(union bpf_attr *attr)
>>> +{
>>> +	struct bpf_sk_storage_map *smap;
>>> +	unsigned int i;
>>> +	u32 nbuckets;
>>> +	u64 cost;
>>> +
>>> +	if (attr->map_flags != BPF_F_NO_PREALLOC || attr->max_entries ||
>>> +	    attr->key_size != sizeof(int) || !attr->value_size ||
>>> +	    /* Enforce BTF for userspace sk dumping */
>>> +	    !attr->btf_key_type_id || !attr->btf_value_type_id)
>>> +		return ERR_PTR(-EINVAL);
>>> +
>>> +	if (!capable(CAP_SYS_ADMIN))
>>> +		return ERR_PTR(-EPERM);
>>> +
>>> +	if (attr->value_size >= KMALLOC_MAX_SIZE -
>>> +	    MAX_BPF_STACK - sizeof(struct bpf_sk_storage_elem) ||
>>> +	    /* selem->elem_size is a u16 */
>>> +	    attr->value_size > U16_MAX - sizeof(struct bpf_sk_storage_elem))
>>> +		return ERR_PTR(-E2BIG);
>>> +
>>> +	smap = kzalloc(sizeof(*smap), GFP_USER | __GFP_NOWARN);
>>> +	if (!smap)
>>> +		return ERR_PTR(-ENOMEM);
>>> +	bpf_map_init_from_attr(&smap->map, attr);
>>> +
>>> +	smap->bucket_log = ilog2(roundup_pow_of_two(num_possible_cpus()));
>>> +	nbuckets = 1U << smap->bucket_log;
>>> +	smap->buckets = kvcalloc(sizeof(*smap->buckets), nbuckets,
>>> +				 GFP_USER | __GFP_NOWARN);
>>> +	if (!smap->buckets) {
>>> +		kfree(smap);
>>> +		return ERR_PTR(-ENOMEM);
>>> +	}
>>> +	cost = sizeof(*smap->buckets) * nbuckets + sizeof(*smap);
>>> +
>>> +	for (i = 0; i < nbuckets; i++) {
>>> +		INIT_HLIST_HEAD(&smap->buckets[i].list);
>>> +		raw_spin_lock_init(&smap->buckets[i].lock);
>>> +	}
>>> +
>>> +	smap->elem_size = sizeof(struct bpf_sk_storage_elem) + attr->value_size;
>>> +	smap->cache_idx = (unsigned int)atomic_inc_return(&cache_idx) %
>>> +		BPF_SK_STORAGE_CACHE_SIZE;
>>> +	smap->map.pages = round_up(cost, PAGE_SIZE) >> PAGE_SHIFT;
>>
>> should we test bpf_map_precharge_memlock() here?
> I did not do it here because the size of this map does not depend on
> "map.max_entries".  Meaning,
> this map merely needs the memory for the buckets which is less likely to
> fail (and destroying in case of final charge failure is not that expensive
> either) which then makes this precheck not as useful as other maps.

I think it is okay as is as you described in the above.
Later on, a real charging will happen and if indeed running out of 
locked memory, it will fail then. But this should a rare thing.

> 
> Thanks for reviewing!
> 
>>
>>> +
>>> +	return &smap->map;
>>> +}
>>> +
>>> +static int notsupp_get_next_key(struct bpf_map *map, void *key,
>>> +				void *next_key)
>>> +{
>>> +	return -ENOTSUPP;
>>> +}
>>> +
>>> +static int bpf_sk_storage_map_check_btf(const struct bpf_map *map,
>>> +					const struct btf *btf,
>>> +					const struct btf_type *key_type,
>>> +					const struct btf_type *value_type)
>>> +{
>>> +	u32 int_data;
>>> +
>>> +	if (BTF_INFO_KIND(key_type->info) != BTF_KIND_INT)
>>> +		return -EINVAL;
>>> +
>>> +	int_data = *(u32 *)(key_type + 1);
>>> +	if (BTF_INT_BITS(int_data) != 32 || BTF_INT_OFFSET(int_data))
>>> +		return -EINVAL;
>>> +
>>> +	return 0;
>>> +}
>>> +
>> [...]> diff --git a/net/core/sock.c b/net/core/sock.c
>>> index 443b98d05f1e..9773be724aa9 100644
>>> --- a/net/core/sock.c
>>> +++ b/net/core/sock.c
>>> @@ -137,6 +137,7 @@
>>>    
>>>    #include <linux/filter.h>
>>>    #include <net/sock_reuseport.h>
>>> +#include <net/bpf_sk_storage.h>
>>>    
>>>    #include <trace/events/sock.h>
>>>    
>>> @@ -1709,6 +1710,10 @@ static void __sk_destruct(struct rcu_head *head)
>>>    
>>>    	sock_disable_timestamp(sk, SK_FLAGS_TIMESTAMP);
>>>    
>>> +#ifdef CONFIG_BPF_SYSCALL
>>> +	bpf_sk_storage_free(sk);
>>> +#endif
>>> +
>>>    	if (atomic_read(&sk->sk_omem_alloc))
>>>    		pr_debug("%s: optmem leakage (%d bytes) detected\n",
>>>    			 __func__, atomic_read(&sk->sk_omem_alloc));
>>>

[PATCH v2 bpf-next 2/6] bpf: Sync bpf.h to tools

[Martin KaFai Lau]

This patch sync the bpf.h to tools/.

Signed-off-by: Martin KaFai Lau <kafai@fb.com>
---
 tools/include/uapi/linux/bpf.h | 44 +++++++++++++++++++++++++++++++++-
 1 file changed, 43 insertions(+), 1 deletion(-)

diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 704bb69514a2..be08ef00b2c8 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -133,6 +133,7 @@ enum bpf_map_type {
 	BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE,
 	BPF_MAP_TYPE_QUEUE,
 	BPF_MAP_TYPE_STACK,
+	BPF_MAP_TYPE_SK_STORAGE,
 };
 
 /* Note that tracing related programs such as
@@ -2622,6 +2623,42 @@ union bpf_attr {
  *		was provided.
  *
  *		**-ERANGE** if resulting value was out of range.
+ *
+ * void *bpf_sk_storage_get(struct bpf_map *map, struct bpf_sock *sk, void *value, u64 flags)
+ *	Description
+ *		Get a bpf-local-storage from a sk.
+ *
+ *		Logically, it could be thought of getting the value from
+ *		a *map* with *sk* as the **key**.  From this
+ *		perspective,  the usage is not much different from
+ *		**bpf_map_lookup_elem(map, &sk)** except this
+ *		helper enforces the key must be a **bpf_fullsock()**
+ *		and the map must be a BPF_MAP_TYPE_SK_STORAGE also.
+ *
+ *		Underneath, the value is stored locally at *sk* instead of
+ *		the map.  The *map* is used as the bpf-local-storage **type**.
+ *		The bpf-local-storage **type** (i.e. the *map*) is searched
+ *		against all bpf-local-storages residing at sk.
+ *
+ *		An optional *flags* (BPF_SK_STORAGE_GET_F_CREATE) can be
+ *		used such that a new bpf-local-storage will be
+ *		created if one does not exist.  *value* can be used
+ *		together with BPF_SK_STORAGE_GET_F_CREATE to specify
+ *		the initial value of a bpf-local-storage.  If *value* is
+ *		NULL, the new bpf-local-storage will be zero initialized.
+ *	Return
+ *		A bpf-local-storage pointer is returned on success.
+ *
+ *		**NULL** if not found or there was an error in adding
+ *		a new bpf-local-storage.
+ *
+ * int bpf_sk_storage_delete(struct bpf_map *map, struct bpf_sock *sk)
+ *	Description
+ *		Delete a bpf-local-storage from a sk.
+ *	Return
+ *		0 on success.
+ *
+ *		**-ENOENT** if the bpf-local-storage cannot be found.
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -2730,7 +2767,9 @@ union bpf_attr {
 	FN(sysctl_get_new_value),	\
 	FN(sysctl_set_new_value),	\
 	FN(strtol),			\
-	FN(strtoul),
+	FN(strtoul),			\
+	FN(sk_storage_get),		\
+	FN(sk_storage_delete),
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
  * function eBPF program intends to call
@@ -2806,6 +2845,9 @@ enum bpf_func_id {
 /* BPF_FUNC_sysctl_get_name flags. */
 #define BPF_F_SYSCTL_BASE_NAME		(1ULL << 0)
 
+/* BPF_FUNC_sk_storage_get flags */
+#define BPF_SK_STORAGE_GET_F_CREATE	(1ULL << 0)
+
 /* Mode for BPF_FUNC_skb_adjust_room helper. */
 enum bpf_adj_room_mode {
 	BPF_ADJ_ROOM_NET,
-- 
2.17.1

[PATCH v2 bpf-next 3/6] bpf: Support BPF_MAP_TYPE_SK_STORAGE in bpf map probing

[Martin KaFai Lau]

This patch supports probing for the new BPF_MAP_TYPE_SK_STORAGE.
BPF_MAP_TYPE_SK_STORAGE enforces BTF usage, so the new probe
requires to create and load a BTF also.

Signed-off-by: Martin KaFai Lau <kafai@fb.com>
---
 tools/bpf/bpftool/map.c       |  1 +
 tools/lib/bpf/libbpf_probes.c | 74 ++++++++++++++++++++++++++++++++++-
 2 files changed, 74 insertions(+), 1 deletion(-)

diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
index 10b6c9d3e525..693ab3da37de 100644
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@ -46,6 +46,7 @@ const char * const map_type_name[] = {
 	[BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE]	= "percpu_cgroup_storage",
 	[BPF_MAP_TYPE_QUEUE]			= "queue",
 	[BPF_MAP_TYPE_STACK]			= "stack",
+	[BPF_MAP_TYPE_SK_STORAGE]		= "sk_storage",
 };
 
 const size_t map_type_name_size = ARRAY_SIZE(map_type_name);
diff --git a/tools/lib/bpf/libbpf_probes.c b/tools/lib/bpf/libbpf_probes.c
index 0f25541632e3..2e26ddb005e4 100644
--- a/tools/lib/bpf/libbpf_probes.c
+++ b/tools/lib/bpf/libbpf_probes.c
@@ -9,6 +9,7 @@
 #include <net/if.h>
 #include <sys/utsname.h>
 
+#include <linux/btf.h>
 #include <linux/filter.h>
 #include <linux/kernel.h>
 
@@ -130,11 +131,65 @@ bool bpf_probe_prog_type(enum bpf_prog_type prog_type, __u32 ifindex)
 	return errno != EINVAL && errno != EOPNOTSUPP;
 }
 
+static int load_btf(void)
+{
+#define BTF_INFO_ENC(kind, kind_flag, vlen) \
+	((!!(kind_flag) << 31) | ((kind) << 24) | ((vlen) & BTF_MAX_VLEN))
+#define BTF_TYPE_ENC(name, info, size_or_type) \
+	(name), (info), (size_or_type)
+#define BTF_INT_ENC(encoding, bits_offset, nr_bits) \
+	((encoding) << 24 | (bits_offset) << 16 | (nr_bits))
+#define BTF_TYPE_INT_ENC(name, encoding, bits_offset, bits, sz) \
+	BTF_TYPE_ENC(name, BTF_INFO_ENC(BTF_KIND_INT, 0, 0), sz), \
+	BTF_INT_ENC(encoding, bits_offset, bits)
+#define BTF_MEMBER_ENC(name, type, bits_offset) \
+	(name), (type), (bits_offset)
+
+	const char btf_str_sec[] = "\0bpf_spin_lock\0val\0cnt\0l";
+	/* struct bpf_spin_lock {
+	 *   int val;
+	 * };
+	 * struct val {
+	 *   int cnt;
+	 *   struct bpf_spin_lock l;
+	 * };
+	 */
+	__u32 btf_raw_types[] = {
+		/* int */
+		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),  /* [1] */
+		/* struct bpf_spin_lock */                      /* [2] */
+		BTF_TYPE_ENC(1, BTF_INFO_ENC(BTF_KIND_STRUCT, 0, 1), 4),
+		BTF_MEMBER_ENC(15, 1, 0), /* int val; */
+		/* struct val */                                /* [3] */
+		BTF_TYPE_ENC(15, BTF_INFO_ENC(BTF_KIND_STRUCT, 0, 2), 8),
+		BTF_MEMBER_ENC(19, 1, 0), /* int cnt; */
+		BTF_MEMBER_ENC(23, 2, 32),/* struct bpf_spin_lock l; */
+	};
+	struct btf_header btf_hdr = {
+		.magic = BTF_MAGIC,
+		.version = BTF_VERSION,
+		.hdr_len = sizeof(struct btf_header),
+		.type_len = sizeof(btf_raw_types),
+		.str_off = sizeof(btf_raw_types),
+		.str_len = sizeof(btf_str_sec),
+	};
+	__u8 raw_btf[sizeof(struct btf_header) + sizeof(btf_raw_types) +
+		     sizeof(btf_str_sec)];
+
+	memcpy(raw_btf, &btf_hdr, sizeof(btf_hdr));
+	memcpy(raw_btf + sizeof(btf_hdr), btf_raw_types, sizeof(btf_raw_types));
+	memcpy(raw_btf + sizeof(btf_hdr) + sizeof(btf_raw_types),
+	       btf_str_sec, sizeof(btf_str_sec));
+
+	return bpf_load_btf(raw_btf, sizeof(raw_btf), 0, 0, 0);
+}
+
 bool bpf_probe_map_type(enum bpf_map_type map_type, __u32 ifindex)
 {
 	int key_size, value_size, max_entries, map_flags;
+	__u32 btf_key_type_id = 0, btf_value_type_id = 0;
 	struct bpf_create_map_attr attr = {};
-	int fd = -1, fd_inner;
+	int fd = -1, btf_fd = -1, fd_inner;
 
 	key_size	= sizeof(__u32);
 	value_size	= sizeof(__u32);
@@ -160,6 +215,16 @@ bool bpf_probe_map_type(enum bpf_map_type map_type, __u32 ifindex)
 	case BPF_MAP_TYPE_STACK:
 		key_size	= 0;
 		break;
+	case BPF_MAP_TYPE_SK_STORAGE:
+		btf_key_type_id = 1;
+		btf_value_type_id = 3;
+		value_size = 8;
+		max_entries = 0;
+		map_flags = BPF_F_NO_PREALLOC;
+		btf_fd = load_btf();
+		if (btf_fd < 0)
+			return false;
+		break;
 	case BPF_MAP_TYPE_UNSPEC:
 	case BPF_MAP_TYPE_HASH:
 	case BPF_MAP_TYPE_ARRAY:
@@ -205,11 +270,18 @@ bool bpf_probe_map_type(enum bpf_map_type map_type, __u32 ifindex)
 		attr.max_entries = max_entries;
 		attr.map_flags = map_flags;
 		attr.map_ifindex = ifindex;
+		if (btf_fd >= 0) {
+			attr.btf_fd = btf_fd;
+			attr.btf_key_type_id = btf_key_type_id;
+			attr.btf_value_type_id = btf_value_type_id;
+		}
 
 		fd = bpf_create_map_xattr(&attr);
 	}
 	if (fd >= 0)
 		close(fd);
+	if (btf_fd >= 0)
+		close(btf_fd);
 
 	return fd >= 0;
 }
-- 
2.17.1

[PATCH v2 bpf-next 4/6] bpf: Add verifier tests for the bpf_sk_storage

[Martin KaFai Lau]

This patch adds verifier tests for the bpf_sk_storage:
1. ARG_PTR_TO_MAP_VALUE_OR_NULL
2. Map and helper compatibility (e.g. disallow bpf_map_loookup_elem)

It also takes this chance to remove the unused struct btf_raw_data.

Signed-off-by: Martin KaFai Lau <kafai@fb.com>
---
 tools/testing/selftests/bpf/test_verifier.c |  42 +++++--
 tools/testing/selftests/bpf/verifier/sock.c | 116 ++++++++++++++++++++
 2 files changed, 151 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index ed9e894afef3..bff28b68f387 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -52,7 +52,7 @@
 #define MAX_INSNS	BPF_MAXINSNS
 #define MAX_TEST_INSNS	1000000
 #define MAX_FIXUPS	8
-#define MAX_NR_MAPS	17
+#define MAX_NR_MAPS	18
 #define MAX_TEST_RUNS	8
 #define POINTER_VALUE	0xcafe4all
 #define TEST_DATA_LEN	64
@@ -85,6 +85,7 @@ struct bpf_test {
 	int fixup_map_array_ro[MAX_FIXUPS];
 	int fixup_map_array_wo[MAX_FIXUPS];
 	int fixup_map_array_small[MAX_FIXUPS];
+	int fixup_sk_storage_map[MAX_FIXUPS];
 	const char *errstr;
 	const char *errstr_unpriv;
 	uint32_t retval, retval_unpriv, insn_processed;
@@ -509,12 +510,6 @@ static int create_cgroup_storage(bool percpu)
 #define BTF_MEMBER_ENC(name, type, bits_offset) \
 	(name), (type), (bits_offset)
 
-struct btf_raw_data {
-	__u32 raw_types[64];
-	const char *str_sec;
-	__u32 str_sec_size;
-};
-
 /* struct bpf_spin_lock {
  *   int val;
  * };
@@ -589,6 +584,31 @@ static int create_map_spin_lock(void)
 	return fd;
 }
 
+static int create_sk_storage_map(void)
+{
+	struct bpf_create_map_attr attr = {
+		.name = "test_map",
+		.map_type = BPF_MAP_TYPE_SK_STORAGE,
+		.key_size = 4,
+		.value_size = 8,
+		.max_entries = 0,
+		.map_flags = BPF_F_NO_PREALLOC,
+		.btf_key_type_id = 1,
+		.btf_value_type_id = 3,
+	};
+	int fd, btf_fd;
+
+	btf_fd = load_btf();
+	if (btf_fd < 0)
+		return -1;
+	attr.btf_fd = btf_fd;
+	fd = bpf_create_map_xattr(&attr);
+	close(attr.btf_fd);
+	if (fd < 0)
+		printf("Failed to create sk_storage_map\n");
+	return fd;
+}
+
 static char bpf_vlog[UINT_MAX >> 8];
 
 static void do_test_fixup(struct bpf_test *test, enum bpf_prog_type prog_type,
@@ -611,6 +631,7 @@ static void do_test_fixup(struct bpf_test *test, enum bpf_prog_type prog_type,
 	int *fixup_map_array_ro = test->fixup_map_array_ro;
 	int *fixup_map_array_wo = test->fixup_map_array_wo;
 	int *fixup_map_array_small = test->fixup_map_array_small;
+	int *fixup_sk_storage_map = test->fixup_sk_storage_map;
 
 	if (test->fill_helper) {
 		test->fill_insns = calloc(MAX_TEST_INSNS, sizeof(struct bpf_insn));
@@ -765,6 +786,13 @@ static void do_test_fixup(struct bpf_test *test, enum bpf_prog_type prog_type,
 			fixup_map_array_small++;
 		} while (*fixup_map_array_small);
 	}
+	if (*fixup_sk_storage_map) {
+		map_fds[17] = create_sk_storage_map();
+		do {
+			prog[*fixup_sk_storage_map].imm = map_fds[17];
+			fixup_sk_storage_map++;
+		} while (*fixup_sk_storage_map);
+	}
 }
 
 static int set_admin(bool admin)
diff --git a/tools/testing/selftests/bpf/verifier/sock.c b/tools/testing/selftests/bpf/verifier/sock.c
index 416436231fab..b31cd2cf50d0 100644
--- a/tools/testing/selftests/bpf/verifier/sock.c
+++ b/tools/testing/selftests/bpf/verifier/sock.c
@@ -382,3 +382,119 @@
 	.result = REJECT,
 	.errstr = "reference has not been acquired before",
 },
+{
+	"sk_storage_get(map, skb->sk, NULL, 0): value == NULL",
+	.insns = {
+	BPF_LDX_MEM(BPF_DW, BPF_REG_1, BPF_REG_1, offsetof(struct __sk_buff, sk)),
+	BPF_JMP_IMM(BPF_JNE, BPF_REG_1, 0, 2),
+	BPF_MOV64_IMM(BPF_REG_0, 0),
+	BPF_EXIT_INSN(),
+	BPF_EMIT_CALL(BPF_FUNC_sk_fullsock),
+	BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, 2),
+	BPF_MOV64_IMM(BPF_REG_0, 0),
+	BPF_EXIT_INSN(),
+	BPF_MOV64_IMM(BPF_REG_4, 0),
+	BPF_MOV64_IMM(BPF_REG_3, 0),
+	BPF_MOV64_REG(BPF_REG_2, BPF_REG_0),
+	BPF_LD_MAP_FD(BPF_REG_1, 0),
+	BPF_EMIT_CALL(BPF_FUNC_sk_storage_get),
+	BPF_MOV64_IMM(BPF_REG_0, 0),
+	BPF_EXIT_INSN(),
+	},
+	.fixup_sk_storage_map = { 11 },
+	.prog_type = BPF_PROG_TYPE_SCHED_CLS,
+	.result = ACCEPT,
+},
+{
+	"sk_storage_get(map, skb->sk, 1, 1): value == 1",
+	.insns = {
+	BPF_LDX_MEM(BPF_DW, BPF_REG_1, BPF_REG_1, offsetof(struct __sk_buff, sk)),
+	BPF_JMP_IMM(BPF_JNE, BPF_REG_1, 0, 2),
+	BPF_MOV64_IMM(BPF_REG_0, 0),
+	BPF_EXIT_INSN(),
+	BPF_EMIT_CALL(BPF_FUNC_sk_fullsock),
+	BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, 2),
+	BPF_MOV64_IMM(BPF_REG_0, 0),
+	BPF_EXIT_INSN(),
+	BPF_MOV64_IMM(BPF_REG_4, 1),
+	BPF_MOV64_IMM(BPF_REG_3, 1),
+	BPF_MOV64_REG(BPF_REG_2, BPF_REG_0),
+	BPF_LD_MAP_FD(BPF_REG_1, 0),
+	BPF_EMIT_CALL(BPF_FUNC_sk_storage_get),
+	BPF_MOV64_IMM(BPF_REG_0, 0),
+	BPF_EXIT_INSN(),
+	},
+	.fixup_sk_storage_map = { 11 },
+	.prog_type = BPF_PROG_TYPE_SCHED_CLS,
+	.result = REJECT,
+	.errstr = "R3 type=inv expected=fp",
+},
+{
+	"sk_storage_get(map, skb->sk, &stack_value, 1): stack_value",
+	.insns = {
+	BPF_MOV64_IMM(BPF_REG_2, 0),
+	BPF_STX_MEM(BPF_DW, BPF_REG_10, BPF_REG_2, -8),
+	BPF_LDX_MEM(BPF_DW, BPF_REG_1, BPF_REG_1, offsetof(struct __sk_buff, sk)),
+	BPF_JMP_IMM(BPF_JNE, BPF_REG_1, 0, 2),
+	BPF_MOV64_IMM(BPF_REG_0, 0),
+	BPF_EXIT_INSN(),
+	BPF_EMIT_CALL(BPF_FUNC_sk_fullsock),
+	BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, 2),
+	BPF_MOV64_IMM(BPF_REG_0, 0),
+	BPF_EXIT_INSN(),
+	BPF_MOV64_IMM(BPF_REG_4, 1),
+	BPF_MOV64_REG(BPF_REG_3, BPF_REG_10),
+	BPF_ALU64_IMM(BPF_ADD, BPF_REG_3, -8),
+	BPF_MOV64_REG(BPF_REG_2, BPF_REG_0),
+	BPF_LD_MAP_FD(BPF_REG_1, 0),
+	BPF_EMIT_CALL(BPF_FUNC_sk_storage_get),
+	BPF_MOV64_IMM(BPF_REG_0, 0),
+	BPF_EXIT_INSN(),
+	},
+	.fixup_sk_storage_map = { 14 },
+	.prog_type = BPF_PROG_TYPE_SCHED_CLS,
+	.result = ACCEPT,
+},
+{
+	"sk_storage_get(map, skb->sk, &stack_value, 1): partially init stack_value",
+	.insns = {
+	BPF_MOV64_IMM(BPF_REG_2, 0),
+	BPF_STX_MEM(BPF_W, BPF_REG_10, BPF_REG_2, -8),
+	BPF_LDX_MEM(BPF_DW, BPF_REG_1, BPF_REG_1, offsetof(struct __sk_buff, sk)),
+	BPF_JMP_IMM(BPF_JNE, BPF_REG_1, 0, 2),
+	BPF_MOV64_IMM(BPF_REG_0, 0),
+	BPF_EXIT_INSN(),
+	BPF_EMIT_CALL(BPF_FUNC_sk_fullsock),
+	BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, 2),
+	BPF_MOV64_IMM(BPF_REG_0, 0),
+	BPF_EXIT_INSN(),
+	BPF_MOV64_IMM(BPF_REG_4, 1),
+	BPF_MOV64_REG(BPF_REG_3, BPF_REG_10),
+	BPF_ALU64_IMM(BPF_ADD, BPF_REG_3, -8),
+	BPF_MOV64_REG(BPF_REG_2, BPF_REG_0),
+	BPF_LD_MAP_FD(BPF_REG_1, 0),
+	BPF_EMIT_CALL(BPF_FUNC_sk_storage_get),
+	BPF_MOV64_IMM(BPF_REG_0, 0),
+	BPF_EXIT_INSN(),
+	},
+	.fixup_sk_storage_map = { 14 },
+	.prog_type = BPF_PROG_TYPE_SCHED_CLS,
+	.result = REJECT,
+	.errstr = "invalid indirect read from stack",
+},
+{
+	"bpf_map_lookup_elem(smap, &key)",
+	.insns = {
+	BPF_ST_MEM(BPF_W, BPF_REG_10, -4, 0),
+	BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+	BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -4),
+	BPF_LD_MAP_FD(BPF_REG_1, 0),
+	BPF_EMIT_CALL(BPF_FUNC_map_lookup_elem),
+	BPF_MOV64_IMM(BPF_REG_0, 0),
+	BPF_EXIT_INSN(),
+	},
+	.fixup_sk_storage_map = { 3 },
+	.prog_type = BPF_PROG_TYPE_SCHED_CLS,
+	.result = REJECT,
+	.errstr = "cannot pass map_type 24 into func bpf_map_lookup_elem",
+},
-- 
2.17.1

[PATCH v2 bpf-next 5/6] bpf: Add BPF_MAP_TYPE_SK_STORAGE test to test_maps

[Martin KaFai Lau]

This patch adds BPF_MAP_TYPE_SK_STORAGE test to test_maps.
The src file is rather long, so it is put into another dir map_tests/
and compile like the current prog_tests/ does.  Other existing
tests in test_maps can also be re-factored into map_tests/ in the
future.

Signed-off-by: Martin KaFai Lau <kafai@fb.com>
---
 tools/testing/selftests/bpf/Makefile          |  25 +-
 .../selftests/bpf/map_tests/sk_storage_map.c  | 638 ++++++++++++++++++
 tools/testing/selftests/bpf/test_maps.c       |  18 +-
 tools/testing/selftests/bpf/test_maps.h       |  17 +
 4 files changed, 688 insertions(+), 10 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/map_tests/sk_storage_map.c
 create mode 100644 tools/testing/selftests/bpf/test_maps.h

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index f9d83ba7843e..66f2dca1dee1 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -74,6 +74,8 @@ all: $(TEST_CUSTOM_PROGS)
 $(OUTPUT)/urandom_read: $(OUTPUT)/%: %.c
 	$(CC) -o $@ $< -Wl,--build-id
 
+$(OUTPUT)/test_maps: map_tests/*.c
+
 BPFOBJ := $(OUTPUT)/libbpf.a
 
 $(TEST_GEN_PROGS): $(BPFOBJ)
@@ -232,6 +234,27 @@ $(PROG_TESTS_H): $(PROG_TESTS_DIR) $(PROG_TESTS_FILES)
 		  echo '#endif' \
 		 ) > $(PROG_TESTS_H))
 
+TEST_MAPS_CFLAGS := -I. -I$(OUTPUT)
+MAP_TESTS_DIR = $(OUTPUT)/map_tests
+$(MAP_TESTS_DIR):
+	mkdir -p $@
+MAP_TESTS_H := $(MAP_TESTS_DIR)/tests.h
+test_maps.c: $(MAP_TESTS_H)
+$(OUTPUT)/test_maps: CFLAGS += $(TEST_MAPS_CFLAGS)
+MAP_TESTS_FILES := $(wildcard map_tests/*.c)
+$(MAP_TESTS_H): $(MAP_TESTS_DIR) $(MAP_TESTS_FILES)
+	$(shell ( cd map_tests/; \
+		  echo '/* Generated header, do not edit */'; \
+		  echo '#ifdef DECLARE'; \
+		  ls *.c 2> /dev/null | \
+			sed -e 's@\([^\.]*\)\.c@extern void test_\1(void);@'; \
+		  echo '#endif'; \
+		  echo '#ifdef CALL'; \
+		  ls *.c 2> /dev/null | \
+			sed -e 's@\([^\.]*\)\.c@test_\1();@'; \
+		  echo '#endif' \
+		 ) > $(MAP_TESTS_H))
+
 VERIFIER_TESTS_H := $(OUTPUT)/verifier/tests.h
 test_verifier.c: $(VERIFIER_TESTS_H)
 $(OUTPUT)/test_verifier: CFLAGS += $(TEST_VERIFIER_CFLAGS)
@@ -251,4 +274,4 @@ $(OUTPUT)/verifier/tests.h: $(VERIFIER_TESTS_DIR) $(VERIFIER_TEST_FILES)
 		 ) > $(VERIFIER_TESTS_H))
 
 EXTRA_CLEAN := $(TEST_CUSTOM_PROGS) $(ALU32_BUILD_DIR) \
-	$(VERIFIER_TESTS_H) $(PROG_TESTS_H)
+	$(VERIFIER_TESTS_H) $(PROG_TESTS_H) $(MAP_TESTS_H)
diff --git a/tools/testing/selftests/bpf/map_tests/sk_storage_map.c b/tools/testing/selftests/bpf/map_tests/sk_storage_map.c
new file mode 100644
index 000000000000..f66e4a3ee6e4
--- /dev/null
+++ b/tools/testing/selftests/bpf/map_tests/sk_storage_map.c
@@ -0,0 +1,638 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2019 Facebook  */
+#include <linux/compiler.h>
+#include <linux/err.h>
+
+#include <sys/resource.h>
+#include <sys/socket.h>
+#include <sys/types.h>
+#include <linux/btf.h>
+#include <unistd.h>
+#include <signal.h>
+#include <errno.h>
+#include <string.h>
+#include <pthread.h>
+
+#include <bpf/bpf.h>
+#include <bpf/libbpf.h>
+
+#include <test_maps.h>
+
+static struct bpf_create_map_attr xattr = {
+	.name = "sk_storage_map",
+	.map_type = BPF_MAP_TYPE_SK_STORAGE,
+	.map_flags = BPF_F_NO_PREALLOC,
+	.max_entries = 0,
+	.key_size = 4,
+	.value_size = 8,
+	.btf_key_type_id = 1,
+	.btf_value_type_id = 3,
+	.btf_fd = -1,
+};
+
+static unsigned int nr_sk_threads_done;
+static unsigned int nr_sk_threads_err;
+static unsigned int nr_sk_per_thread = 4096;
+static unsigned int nr_sk_threads = 4;
+static int sk_storage_map = -1;
+static unsigned int stop;
+static int runtime_s = 5;
+
+static bool is_stopped(void)
+{
+	return READ_ONCE(stop);
+}
+
+static unsigned int threads_err(void)
+{
+	return READ_ONCE(nr_sk_threads_err);
+}
+
+static void notify_thread_err(void)
+{
+	__sync_add_and_fetch(&nr_sk_threads_err, 1);
+}
+
+static bool wait_for_threads_err(void)
+{
+	while (!is_stopped() && !threads_err())
+		usleep(500);
+
+	return !is_stopped();
+}
+
+static unsigned int threads_done(void)
+{
+	return READ_ONCE(nr_sk_threads_done);
+}
+
+static void notify_thread_done(void)
+{
+	__sync_add_and_fetch(&nr_sk_threads_done, 1);
+}
+
+static void notify_thread_redo(void)
+{
+	__sync_sub_and_fetch(&nr_sk_threads_done, 1);
+}
+
+static bool wait_for_threads_done(void)
+{
+	while (threads_done() != nr_sk_threads && !is_stopped() &&
+	       !threads_err())
+		usleep(50);
+
+	return !is_stopped() && !threads_err();
+}
+
+static bool wait_for_threads_redo(void)
+{
+	while (threads_done() && !is_stopped() && !threads_err())
+		usleep(50);
+
+	return !is_stopped() && !threads_err();
+}
+
+static bool wait_for_map(void)
+{
+	while (READ_ONCE(sk_storage_map) == -1 && !is_stopped())
+		usleep(50);
+
+	return !is_stopped();
+}
+
+static bool wait_for_map_close(void)
+{
+	while (READ_ONCE(sk_storage_map) != -1 && !is_stopped())
+		;
+
+	return !is_stopped();
+}
+
+static int load_btf(void)
+{
+#define BTF_INFO_ENC(kind, kind_flag, vlen) \
+	((!!(kind_flag) << 31) | ((kind) << 24) | ((vlen) & BTF_MAX_VLEN))
+#define BTF_TYPE_ENC(name, info, size_or_type) \
+	(name), (info), (size_or_type)
+#define BTF_INT_ENC(encoding, bits_offset, nr_bits) \
+	((encoding) << 24 | (bits_offset) << 16 | (nr_bits))
+#define BTF_TYPE_INT_ENC(name, encoding, bits_offset, bits, sz) \
+	BTF_TYPE_ENC(name, BTF_INFO_ENC(BTF_KIND_INT, 0, 0), sz),	\
+	BTF_INT_ENC(encoding, bits_offset, bits)
+#define BTF_MEMBER_ENC(name, type, bits_offset) \
+	(name), (type), (bits_offset)
+
+	const char btf_str_sec[] = "\0bpf_spin_lock\0val\0cnt\0l";
+	__u32 btf_raw_types[] = {
+		/* int */
+		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),  /* [1] */
+		/* struct bpf_spin_lock */                      /* [2] */
+		BTF_TYPE_ENC(1, BTF_INFO_ENC(BTF_KIND_STRUCT, 0, 1), 4),
+		BTF_MEMBER_ENC(15, 1, 0), /* int val; */
+		/* struct val */                                /* [3] */
+		BTF_TYPE_ENC(15, BTF_INFO_ENC(BTF_KIND_STRUCT, 0, 2), 8),
+		BTF_MEMBER_ENC(19, 1, 0), /* int cnt; */
+		BTF_MEMBER_ENC(23, 2, 32),/* struct bpf_spin_lock l; */
+	};
+	struct btf_header btf_hdr = {
+		.magic = BTF_MAGIC,
+		.version = BTF_VERSION,
+		.hdr_len = sizeof(struct btf_header),
+		.type_len = sizeof(btf_raw_types),
+		.str_off = sizeof(btf_raw_types),
+		.str_len = sizeof(btf_str_sec),
+	};
+	__u8 raw_btf[sizeof(struct btf_header) + sizeof(btf_raw_types) +
+		     sizeof(btf_str_sec)];
+
+	memcpy(raw_btf, &btf_hdr, sizeof(btf_hdr));
+	memcpy(raw_btf + sizeof(btf_hdr), btf_raw_types, sizeof(btf_raw_types));
+	memcpy(raw_btf + sizeof(btf_hdr) + sizeof(btf_raw_types),
+	       btf_str_sec, sizeof(btf_str_sec));
+
+	return bpf_load_btf(raw_btf, sizeof(raw_btf), 0, 0, 0);
+}
+
+static int create_sk_storage_map(void)
+{
+	int btf_fd, map_fd;
+
+	btf_fd = load_btf();
+	CHECK(btf_fd == -1, "bpf_load_btf", "btf_fd:%d errno:%d\n",
+	      btf_fd, errno);
+	xattr.btf_fd = btf_fd;
+
+	map_fd = bpf_create_map_xattr(&xattr);
+	xattr.btf_fd = -1;
+	close(btf_fd);
+	CHECK(map_fd == -1,
+	      "bpf_create_map_xattr()", "errno:%d\n", errno);
+
+	return map_fd;
+}
+
+static void *insert_close_thread(void *arg)
+{
+	struct {
+		int cnt;
+		int lock;
+	} value = { .cnt = 0xeB9F, .lock = 0, };
+	int i, map_fd, err, *sk_fds;
+
+	sk_fds = malloc(sizeof(*sk_fds) * nr_sk_per_thread);
+	if (!sk_fds) {
+		notify_thread_err();
+		return ERR_PTR(-ENOMEM);
+	}
+
+	for (i = 0; i < nr_sk_per_thread; i++)
+		sk_fds[i] = -1;
+
+	while (!is_stopped()) {
+		if (!wait_for_map())
+			goto close_all;
+
+		map_fd = READ_ONCE(sk_storage_map);
+		for (i = 0; i < nr_sk_per_thread && !is_stopped(); i++) {
+			sk_fds[i] = socket(AF_INET6, SOCK_STREAM, 0);
+			if (sk_fds[i] == -1) {
+				err = -errno;
+				fprintf(stderr, "socket(): errno:%d\n", errno);
+				goto errout;
+			}
+			err = bpf_map_update_elem(map_fd, &sk_fds[i], &value,
+						  BPF_NOEXIST);
+			if (err) {
+				err = -errno;
+				fprintf(stderr,
+					"bpf_map_update_elem(): errno:%d\n",
+					errno);
+				goto errout;
+			}
+		}
+
+		notify_thread_done();
+		wait_for_map_close();
+
+close_all:
+		for (i = 0; i < nr_sk_per_thread; i++) {
+			close(sk_fds[i]);
+			sk_fds[i] = -1;
+		}
+
+		notify_thread_redo();
+	}
+
+	free(sk_fds);
+	return NULL;
+
+errout:
+	for (i = 0; i < nr_sk_per_thread && sk_fds[i] != -1; i++)
+		close(sk_fds[i]);
+	free(sk_fds);
+	notify_thread_err();
+	return ERR_PTR(err);
+}
+
+static int do_sk_storage_map_stress_free(void)
+{
+	int i, map_fd = -1, err = 0, nr_threads_created = 0;
+	pthread_t *sk_thread_ids;
+	void *thread_ret;
+
+	sk_thread_ids = malloc(sizeof(pthread_t) * nr_sk_threads);
+	if (!sk_thread_ids) {
+		fprintf(stderr, "malloc(sk_threads): NULL\n");
+		return -ENOMEM;
+	}
+
+	for (i = 0; i < nr_sk_threads; i++) {
+		err = pthread_create(&sk_thread_ids[i], NULL,
+				     insert_close_thread, NULL);
+		if (err) {
+			err = -errno;
+			goto done;
+		}
+		nr_threads_created++;
+	}
+
+	while (!is_stopped()) {
+		map_fd = create_sk_storage_map();
+		WRITE_ONCE(sk_storage_map, map_fd);
+
+		if (!wait_for_threads_done())
+			break;
+
+		WRITE_ONCE(sk_storage_map, -1);
+		close(map_fd);
+		map_fd = -1;
+
+		if (!wait_for_threads_redo())
+			break;
+	}
+
+done:
+	WRITE_ONCE(stop, 1);
+	for (i = 0; i < nr_threads_created; i++) {
+		pthread_join(sk_thread_ids[i], &thread_ret);
+		if (IS_ERR(thread_ret) && !err) {
+			err = PTR_ERR(thread_ret);
+			fprintf(stderr, "threads#%u: err:%d\n", i, err);
+		}
+	}
+	free(sk_thread_ids);
+
+	if (map_fd != -1)
+		close(map_fd);
+
+	return err;
+}
+
+static void *update_thread(void *arg)
+{
+	struct {
+		int cnt;
+		int lock;
+	} value = { .cnt = 0xeB9F, .lock = 0, };
+	int map_fd = READ_ONCE(sk_storage_map);
+	int sk_fd = *(int *)arg;
+	int err = 0; /* Suppress compiler false alarm */
+
+	while (!is_stopped()) {
+		err = bpf_map_update_elem(map_fd, &sk_fd, &value, 0);
+		if (err && errno != EAGAIN) {
+			err = -errno;
+			fprintf(stderr, "bpf_map_update_elem: %d %d\n",
+				err, errno);
+			break;
+		}
+	}
+
+	if (!is_stopped()) {
+		notify_thread_err();
+		return ERR_PTR(err);
+	}
+
+	return NULL;
+}
+
+static void *delete_thread(void *arg)
+{
+	int map_fd = READ_ONCE(sk_storage_map);
+	int sk_fd = *(int *)arg;
+	int err = 0; /* Suppress compiler false alarm */
+
+	while (!is_stopped()) {
+		err = bpf_map_delete_elem(map_fd, &sk_fd);
+		if (err && errno != ENOENT) {
+			err = -errno;
+			fprintf(stderr, "bpf_map_delete_elem: %d %d\n",
+				err, errno);
+			break;
+		}
+	}
+
+	if (!is_stopped()) {
+		notify_thread_err();
+		return ERR_PTR(err);
+	}
+
+	return NULL;
+}
+
+static int do_sk_storage_map_stress_change(void)
+{
+	int i, sk_fd, map_fd = -1, err = 0, nr_threads_created = 0;
+	pthread_t *sk_thread_ids;
+	void *thread_ret;
+
+	sk_thread_ids = malloc(sizeof(pthread_t) * nr_sk_threads);
+	if (!sk_thread_ids) {
+		fprintf(stderr, "malloc(sk_threads): NULL\n");
+		return -ENOMEM;
+	}
+
+	sk_fd = socket(AF_INET6, SOCK_STREAM, 0);
+	if (sk_fd == -1) {
+		err = -errno;
+		goto done;
+	}
+
+	map_fd = create_sk_storage_map();
+	WRITE_ONCE(sk_storage_map, map_fd);
+
+	for (i = 0; i < nr_sk_threads; i++) {
+		if (i & 0x1)
+			err = pthread_create(&sk_thread_ids[i], NULL,
+					     update_thread, &sk_fd);
+		else
+			err = pthread_create(&sk_thread_ids[i], NULL,
+					     delete_thread, &sk_fd);
+		if (err) {
+			err = -errno;
+			goto done;
+		}
+		nr_threads_created++;
+	}
+
+	wait_for_threads_err();
+
+done:
+	WRITE_ONCE(stop, 1);
+	for (i = 0; i < nr_threads_created; i++) {
+		pthread_join(sk_thread_ids[i], &thread_ret);
+		if (IS_ERR(thread_ret) && !err) {
+			err = PTR_ERR(thread_ret);
+			fprintf(stderr, "threads#%u: err:%d\n", i, err);
+		}
+	}
+	free(sk_thread_ids);
+
+	if (sk_fd != -1)
+		close(sk_fd);
+	close(map_fd);
+
+	return err;
+}
+
+static void stop_handler(int signum)
+{
+	if (signum != SIGALRM)
+		printf("stopping...\n");
+	WRITE_ONCE(stop, 1);
+}
+
+#define BPF_SK_STORAGE_MAP_TEST_NR_THREADS "BPF_SK_STORAGE_MAP_TEST_NR_THREADS"
+#define BPF_SK_STORAGE_MAP_TEST_SK_PER_THREAD "BPF_SK_STORAGE_MAP_TEST_SK_PER_THREAD"
+#define BPF_SK_STORAGE_MAP_TEST_RUNTIME_S "BPF_SK_STORAGE_MAP_TEST_RUNTIME_S"
+#define BPF_SK_STORAGE_MAP_TEST_NAME "BPF_SK_STORAGE_MAP_TEST_NAME"
+
+static void test_sk_storage_map_stress_free(void)
+{
+	struct rlimit rlim_old, rlim_new = {};
+	const char *env_opt;
+	int err;
+
+	getrlimit(RLIMIT_NOFILE, &rlim_old);
+
+	env_opt = getenv(BPF_SK_STORAGE_MAP_TEST_NR_THREADS);
+	if (env_opt)
+		nr_sk_threads = atoi(env_opt);
+
+	env_opt = getenv(BPF_SK_STORAGE_MAP_TEST_SK_PER_THREAD);
+	if (env_opt)
+		nr_sk_per_thread = atoi(env_opt);
+
+	env_opt = getenv(BPF_SK_STORAGE_MAP_TEST_RUNTIME_S);
+	if (env_opt)
+		runtime_s = atoi(env_opt);
+
+	signal(SIGTERM, stop_handler);
+	signal(SIGINT, stop_handler);
+	if (runtime_s > 0) {
+		signal(SIGALRM, stop_handler);
+		alarm(runtime_s);
+	}
+
+	if (rlim_old.rlim_cur < nr_sk_threads * nr_sk_per_thread) {
+		rlim_new.rlim_cur = nr_sk_threads * nr_sk_per_thread + 128;
+		rlim_new.rlim_max = rlim_new.rlim_cur + 128;
+		err = setrlimit(RLIMIT_NOFILE, &rlim_new);
+		CHECK(err, "setrlimit(RLIMIT_NOFILE)", "rlim_new:%lu errno:%d",
+		      rlim_new.rlim_cur, errno);
+	}
+
+	err = do_sk_storage_map_stress_free();
+
+	signal(SIGTERM, SIG_DFL);
+	signal(SIGINT, SIG_DFL);
+	if (runtime_s > 0) {
+		signal(SIGALRM, SIG_DFL);
+		alarm(0);
+	}
+
+	if (rlim_new.rlim_cur)
+		setrlimit(RLIMIT_NOFILE, &rlim_old);
+
+	CHECK(err, "test_sk_storage_map_stress_free", "err:%d\n", err);
+}
+
+static void test_sk_storage_map_stress_change(void)
+{
+	const char *env_opt;
+	int err;
+
+	env_opt = getenv(BPF_SK_STORAGE_MAP_TEST_NR_THREADS);
+	if (env_opt)
+		nr_sk_threads = atoi(env_opt);
+
+	env_opt = getenv(BPF_SK_STORAGE_MAP_TEST_RUNTIME_S);
+	if (env_opt)
+		runtime_s = atoi(env_opt);
+
+	signal(SIGTERM, stop_handler);
+	signal(SIGINT, stop_handler);
+	if (runtime_s > 0) {
+		signal(SIGALRM, stop_handler);
+		alarm(runtime_s);
+	}
+
+	err = do_sk_storage_map_stress_change();
+
+	signal(SIGTERM, SIG_DFL);
+	signal(SIGINT, SIG_DFL);
+	if (runtime_s > 0) {
+		signal(SIGALRM, SIG_DFL);
+		alarm(0);
+	}
+
+	CHECK(err, "test_sk_storage_map_stress_change", "err:%d\n", err);
+}
+
+static void test_sk_storage_map_basic(void)
+{
+	struct {
+		int cnt;
+		int lock;
+	} value = { .cnt = 0xeB9f, .lock = 0, }, lookup_value;
+	struct bpf_create_map_attr bad_xattr;
+	int btf_fd, map_fd, sk_fd, err;
+
+	btf_fd = load_btf();
+	CHECK(btf_fd == -1, "bpf_load_btf", "btf_fd:%d errno:%d\n",
+	      btf_fd, errno);
+	xattr.btf_fd = btf_fd;
+
+	sk_fd = socket(AF_INET6, SOCK_STREAM, 0);
+	CHECK(sk_fd == -1, "socket()", "sk_fd:%d errno:%d\n",
+	      sk_fd, errno);
+
+	map_fd = bpf_create_map_xattr(&xattr);
+	CHECK(map_fd == -1, "bpf_create_map_xattr(good_xattr)",
+	      "map_fd:%d errno:%d\n", map_fd, errno);
+
+	/* Add new elem */
+	memcpy(&lookup_value, &value, sizeof(value));
+	err = bpf_map_update_elem(map_fd, &sk_fd, &value,
+				  BPF_NOEXIST | BPF_F_LOCK);
+	CHECK(err, "bpf_map_update_elem(BPF_NOEXIST|BPF_F_LOCK)",
+	      "err:%d errno:%d\n", err, errno);
+	err = bpf_map_lookup_elem_flags(map_fd, &sk_fd, &lookup_value,
+					BPF_F_LOCK);
+	CHECK(err || lookup_value.cnt != value.cnt,
+	      "bpf_map_lookup_elem_flags(BPF_F_LOCK)",
+	      "err:%d errno:%d cnt:%x(%x)\n",
+	      err, errno, lookup_value.cnt, value.cnt);
+
+	/* Bump the cnt and update with BPF_EXIST | BPF_F_LOCK */
+	value.cnt += 1;
+	err = bpf_map_update_elem(map_fd, &sk_fd, &value,
+				  BPF_EXIST | BPF_F_LOCK);
+	CHECK(err, "bpf_map_update_elem(BPF_EXIST|BPF_F_LOCK)",
+	      "err:%d errno:%d\n", err, errno);
+	err = bpf_map_lookup_elem_flags(map_fd, &sk_fd, &lookup_value,
+					BPF_F_LOCK);
+	CHECK(err || lookup_value.cnt != value.cnt,
+	      "bpf_map_lookup_elem_flags(BPF_F_LOCK)",
+	      "err:%d errno:%d cnt:%x(%x)\n",
+	      err, errno, lookup_value.cnt, value.cnt);
+
+	/* Bump the cnt and update with BPF_EXIST */
+	value.cnt += 1;
+	err = bpf_map_update_elem(map_fd, &sk_fd, &value, BPF_EXIST);
+	CHECK(err, "bpf_map_update_elem(BPF_EXIST)",
+	      "err:%d errno:%d\n", err, errno);
+	err = bpf_map_lookup_elem_flags(map_fd, &sk_fd, &lookup_value,
+					BPF_F_LOCK);
+	CHECK(err || lookup_value.cnt != value.cnt,
+	      "bpf_map_lookup_elem_flags(BPF_F_LOCK)",
+	      "err:%d errno:%d cnt:%x(%x)\n",
+	      err, errno, lookup_value.cnt, value.cnt);
+
+	/* Update with BPF_NOEXIST */
+	value.cnt += 1;
+	err = bpf_map_update_elem(map_fd, &sk_fd, &value,
+				  BPF_NOEXIST | BPF_F_LOCK);
+	CHECK(!err || errno != EEXIST,
+	      "bpf_map_update_elem(BPF_NOEXIST|BPF_F_LOCK)",
+	      "err:%d errno:%d\n", err, errno);
+	err = bpf_map_update_elem(map_fd, &sk_fd, &value, BPF_NOEXIST);
+	CHECK(!err || errno != EEXIST, "bpf_map_update_elem(BPF_NOEXIST)",
+	      "err:%d errno:%d\n", err, errno);
+	value.cnt -= 1;
+	err = bpf_map_lookup_elem_flags(map_fd, &sk_fd, &lookup_value,
+					BPF_F_LOCK);
+	CHECK(err || lookup_value.cnt != value.cnt,
+	      "bpf_map_lookup_elem_flags(BPF_F_LOCK)",
+	      "err:%d errno:%d cnt:%x(%x)\n",
+	      err, errno, lookup_value.cnt, value.cnt);
+
+	/* Bump the cnt again and update with map_flags == 0 */
+	value.cnt += 1;
+	err = bpf_map_update_elem(map_fd, &sk_fd, &value, 0);
+	CHECK(err, "bpf_map_update_elem()", "err:%d errno:%d\n",
+	      err, errno);
+	err = bpf_map_lookup_elem_flags(map_fd, &sk_fd, &lookup_value,
+					BPF_F_LOCK);
+	CHECK(err || lookup_value.cnt != value.cnt,
+	      "bpf_map_lookup_elem_flags(BPF_F_LOCK)",
+	      "err:%d errno:%d cnt:%x(%x)\n",
+	      err, errno, lookup_value.cnt, value.cnt);
+
+	/* Test delete elem */
+	err = bpf_map_delete_elem(map_fd, &sk_fd);
+	CHECK(err, "bpf_map_delete_elem()", "err:%d errno:%d\n",
+	      err, errno);
+	err = bpf_map_lookup_elem_flags(map_fd, &sk_fd, &lookup_value,
+					BPF_F_LOCK);
+	CHECK(!err || errno != ENOENT,
+	      "bpf_map_lookup_elem_flags(BPF_F_LOCK)",
+	      "err:%d errno:%d\n", err, errno);
+	err = bpf_map_delete_elem(map_fd, &sk_fd);
+	CHECK(!err || errno != ENOENT, "bpf_map_delete_elem()",
+	      "err:%d errno:%d\n", err, errno);
+
+	memcpy(&bad_xattr, &xattr, sizeof(xattr));
+	bad_xattr.btf_key_type_id = 0;
+	err = bpf_create_map_xattr(&bad_xattr);
+	CHECK(!err || errno != EINVAL, "bap_create_map_xattr(bad_xattr)",
+	      "err:%d errno:%d\n", err, errno);
+
+	memcpy(&bad_xattr, &xattr, sizeof(xattr));
+	bad_xattr.btf_key_type_id = 3;
+	err = bpf_create_map_xattr(&bad_xattr);
+	CHECK(!err || errno != EINVAL, "bap_create_map_xattr(bad_xattr)",
+	      "err:%d errno:%d\n", err, errno);
+
+	memcpy(&bad_xattr, &xattr, sizeof(xattr));
+	bad_xattr.max_entries = 1;
+	err = bpf_create_map_xattr(&bad_xattr);
+	CHECK(!err || errno != EINVAL, "bap_create_map_xattr(bad_xattr)",
+	      "err:%d errno:%d\n", err, errno);
+
+	memcpy(&bad_xattr, &xattr, sizeof(xattr));
+	bad_xattr.map_flags = 0;
+	err = bpf_create_map_xattr(&bad_xattr);
+	CHECK(!err || errno != EINVAL, "bap_create_map_xattr(bad_xattr)",
+	      "err:%d errno:%d\n", err, errno);
+
+	xattr.btf_fd = -1;
+	close(btf_fd);
+	close(map_fd);
+	close(sk_fd);
+}
+
+void test_sk_storage_map(void)
+{
+	const char *test_name;
+
+	test_name = getenv(BPF_SK_STORAGE_MAP_TEST_NAME);
+
+	if (!test_name || !strcmp(test_name, "basic"))
+		test_sk_storage_map_basic();
+	if (!test_name || !strcmp(test_name, "stress_free"))
+		test_sk_storage_map_stress_free();
+	if (!test_name || !strcmp(test_name, "stress_change"))
+		test_sk_storage_map_stress_change();
+}
diff --git a/tools/testing/selftests/bpf/test_maps.c b/tools/testing/selftests/bpf/test_maps.c
index 3c627771f965..246f745cb006 100644
--- a/tools/testing/selftests/bpf/test_maps.c
+++ b/tools/testing/selftests/bpf/test_maps.c
@@ -27,6 +27,7 @@
 
 #include "bpf_util.h"
 #include "bpf_rlimit.h"
+#include "test_maps.h"
 
 #ifndef ENOTSUPP
 #define ENOTSUPP 524
@@ -36,15 +37,6 @@ static int skips;
 
 static int map_flags;
 
-#define CHECK(condition, tag, format...) ({				\
-	int __ret = !!(condition);					\
-	if (__ret) {							\
-		printf("%s(%d):FAIL:%s ", __func__, __LINE__, tag);	\
-		printf(format);						\
-		exit(-1);						\
-	}								\
-})
-
 static void test_hashmap(unsigned int task, void *data)
 {
 	long long key, next_key, first_key, value;
@@ -1703,6 +1695,10 @@ static void run_all_tests(void)
 	test_map_in_map();
 }
 
+#define DECLARE
+#include <map_tests/tests.h>
+#undef DECLARE
+
 int main(void)
 {
 	srand(time(NULL));
@@ -1713,6 +1709,10 @@ int main(void)
 	map_flags = BPF_F_NO_PREALLOC;
 	run_all_tests();
 
+#define CALL
+#include <map_tests/tests.h>
+#undef CALL
+
 	printf("test_maps: OK, %d SKIPPED\n", skips);
 	return 0;
 }
diff --git a/tools/testing/selftests/bpf/test_maps.h b/tools/testing/selftests/bpf/test_maps.h
new file mode 100644
index 000000000000..77d8587ac4ed
--- /dev/null
+++ b/tools/testing/selftests/bpf/test_maps.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _TEST_MAPS_H
+#define _TEST_MAPS_H
+
+#include <stdio.h>
+#include <stdlib.h>
+
+#define CHECK(condition, tag, format...) ({				\
+	int __ret = !!(condition);					\
+	if (__ret) {							\
+		printf("%s(%d):FAIL:%s ", __func__, __LINE__, tag);	\
+		printf(format);						\
+		exit(-1);						\
+	}								\
+})
+
+#endif
-- 
2.17.1

[PATCH v2 bpf-next 6/6] bpf: Add ene-to-end test for bpf_sk_storage_* helpers

[Martin KaFai Lau]

This patch rides on an existing BPF_PROG_TYPE_CGROUP_SKB test
(test_sock_fields.c) to do a TCP end-to-end test on the new
bpf_sk_storage_* helpers.

Signed-off-by: Martin KaFai Lau <kafai@fb.com>
---
 tools/testing/selftests/bpf/bpf_helpers.h     |   5 +
 .../bpf/progs/test_sock_fields_kern.c         |  49 ++++++++
 .../testing/selftests/bpf/test_sock_fields.c  | 115 +++++++++++++++---
 3 files changed, 153 insertions(+), 16 deletions(-)

diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
index 59e221586cf7..6e80b66d7fb1 100644
--- a/tools/testing/selftests/bpf/bpf_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_helpers.h
@@ -211,6 +211,11 @@ static int (*bpf_strtol)(const char *buf, unsigned long long buf_len,
 static int (*bpf_strtoul)(const char *buf, unsigned long long buf_len,
 			  unsigned long long flags, unsigned long *res) =
 	(void *) BPF_FUNC_strtoul;
+static void *(*bpf_sk_storage_get)(void *map, struct bpf_sock *sk,
+				   void *value, __u64 flags) =
+	(void *) BPF_FUNC_sk_storage_get;
+static int (*bpf_sk_storage_delete)(void *map, struct bpf_sock *sk) =
+	(void *)BPF_FUNC_sk_storage_delete;
 
 /* llvm builtin functions that eBPF C program may use to
  * emit BPF_LD_ABS and BPF_LD_IND instructions
diff --git a/tools/testing/selftests/bpf/progs/test_sock_fields_kern.c b/tools/testing/selftests/bpf/progs/test_sock_fields_kern.c
index 37328f148538..93ef3e039da3 100644
--- a/tools/testing/selftests/bpf/progs/test_sock_fields_kern.c
+++ b/tools/testing/selftests/bpf/progs/test_sock_fields_kern.c
@@ -55,6 +55,31 @@ struct bpf_map_def SEC("maps") linum_map = {
 	.max_entries = __NR_BPF_LINUM_ARRAY_IDX,
 };
 
+struct bpf_spinlock_cnt {
+	struct bpf_spin_lock lock;
+	__u32 cnt;
+};
+
+struct bpf_map_def SEC("maps") sk_pkt_out_cnt = {
+	.type = BPF_MAP_TYPE_SK_STORAGE,
+	.key_size = sizeof(int),
+	.value_size = sizeof(struct bpf_spinlock_cnt),
+	.max_entries = 0,
+	.map_flags = BPF_F_NO_PREALLOC,
+};
+
+BPF_ANNOTATE_KV_PAIR(sk_pkt_out_cnt, int, struct bpf_spinlock_cnt);
+
+struct bpf_map_def SEC("maps") sk_pkt_out_cnt10 = {
+	.type = BPF_MAP_TYPE_SK_STORAGE,
+	.key_size = sizeof(int),
+	.value_size = sizeof(struct bpf_spinlock_cnt),
+	.max_entries = 0,
+	.map_flags = BPF_F_NO_PREALLOC,
+};
+
+BPF_ANNOTATE_KV_PAIR(sk_pkt_out_cnt10, int, struct bpf_spinlock_cnt);
+
 static bool is_loopback6(__u32 *a6)
 {
 	return !a6[0] && !a6[1] && !a6[2] && a6[3] == bpf_htonl(1);
@@ -120,7 +145,9 @@ static void tpcpy(struct bpf_tcp_sock *dst,
 SEC("cgroup_skb/egress")
 int egress_read_sock_fields(struct __sk_buff *skb)
 {
+	struct bpf_spinlock_cnt cli_cnt_init = { .lock = 0, .cnt = 0xeB9F };
 	__u32 srv_idx = ADDR_SRV_IDX, cli_idx = ADDR_CLI_IDX, result_idx;
+	struct bpf_spinlock_cnt *pkt_out_cnt, *pkt_out_cnt10;
 	struct sockaddr_in6 *srv_sa6, *cli_sa6;
 	struct bpf_tcp_sock *tp, *tp_ret;
 	struct bpf_sock *sk, *sk_ret;
@@ -161,6 +188,28 @@ int egress_read_sock_fields(struct __sk_buff *skb)
 	skcpy(sk_ret, sk);
 	tpcpy(tp_ret, tp);
 
+	if (result_idx == EGRESS_SRV_IDX) {
+		/* The userspace has created it for srv sk */
+		pkt_out_cnt = bpf_sk_storage_get(&sk_pkt_out_cnt, sk, 0, 0);
+		pkt_out_cnt10 = bpf_sk_storage_get(&sk_pkt_out_cnt10, sk,
+						   0, 0);
+	} else {
+		pkt_out_cnt = bpf_sk_storage_get(&sk_pkt_out_cnt, sk,
+						 &cli_cnt_init,
+						 BPF_SK_STORAGE_GET_F_CREATE);
+		pkt_out_cnt10 = bpf_sk_storage_get(&sk_pkt_out_cnt10,
+						   sk, &cli_cnt_init,
+						   BPF_SK_STORAGE_GET_F_CREATE);
+	}
+
+	if (!pkt_out_cnt || !pkt_out_cnt10)
+		RETURN;
+
+	pkt_out_cnt->cnt += 1;
+	bpf_spin_lock(&pkt_out_cnt10->lock);
+	pkt_out_cnt10->cnt += 10;
+	bpf_spin_unlock(&pkt_out_cnt10->lock);
+
 	RETURN;
 }
 
diff --git a/tools/testing/selftests/bpf/test_sock_fields.c b/tools/testing/selftests/bpf/test_sock_fields.c
index dcae7f664dce..550baf8aa1ca 100644
--- a/tools/testing/selftests/bpf/test_sock_fields.c
+++ b/tools/testing/selftests/bpf/test_sock_fields.c
@@ -35,6 +35,11 @@ enum bpf_linum_array_idx {
 	__NR_BPF_LINUM_ARRAY_IDX,
 };
 
+struct bpf_spinlock_cnt {
+	struct bpf_spin_lock lock;
+	__u32 cnt;
+};
+
 #define CHECK(condition, tag, format...) ({				\
 	int __ret = !!(condition);					\
 	if (__ret) {							\
@@ -50,6 +55,8 @@ enum bpf_linum_array_idx {
 #define DATA_LEN sizeof(DATA)
 
 static struct sockaddr_in6 srv_sa6, cli_sa6;
+static int sk_pkt_out_cnt10_fd;
+static int sk_pkt_out_cnt_fd;
 static int linum_map_fd;
 static int addr_map_fd;
 static int tp_map_fd;
@@ -220,28 +227,90 @@ static void check_result(void)
 	      "Unexpected listen_tp", "Check listen_tp output. ingress_linum:%u",
 	      ingress_linum);
 
-	CHECK(srv_tp.data_segs_out != 1 ||
+	CHECK(srv_tp.data_segs_out != 2 ||
 	      srv_tp.data_segs_in ||
 	      srv_tp.snd_cwnd != 10 ||
 	      srv_tp.total_retrans ||
-	      srv_tp.bytes_acked != DATA_LEN,
+	      srv_tp.bytes_acked != 2 * DATA_LEN,
 	      "Unexpected srv_tp", "Check srv_tp output. egress_linum:%u",
 	      egress_linum);
 
 	CHECK(cli_tp.data_segs_out ||
-	      cli_tp.data_segs_in != 1 ||
+	      cli_tp.data_segs_in != 2 ||
 	      cli_tp.snd_cwnd != 10 ||
 	      cli_tp.total_retrans ||
-	      cli_tp.bytes_received != DATA_LEN,
+	      cli_tp.bytes_received != 2 * DATA_LEN,
 	      "Unexpected cli_tp", "Check cli_tp output. egress_linum:%u",
 	      egress_linum);
 }
 
+static void check_sk_pkt_out_cnt(int accept_fd, int cli_fd)
+{
+	struct bpf_spinlock_cnt pkt_out_cnt = {} , pkt_out_cnt10 = {};
+	int err;
+
+	pkt_out_cnt.cnt = ~0;
+	pkt_out_cnt10.cnt = ~0;
+	err = bpf_map_lookup_elem(sk_pkt_out_cnt_fd, &accept_fd, &pkt_out_cnt);
+	if (!err)
+		err = bpf_map_lookup_elem(sk_pkt_out_cnt10_fd, &accept_fd,
+					  &pkt_out_cnt10);
+
+	/* The bpf prog only counts for fullsock and
+	 * passive conneciton did not become fullsock until 3WHS
+	 * had been finished.
+	 * The bpf prog only counted two data packet out but we
+	 * specially init accept_fd's pkt_out_cnt by 2 in
+	 * init_sk_storage().  Hence, 4 here.
+	 */
+	CHECK(err || pkt_out_cnt.cnt != 4 || pkt_out_cnt10.cnt != 40,
+	      "bpf_map_lookup_elem(sk_pkt_out_cnt, &accept_fd)",
+	      "err:%d errno:%d pkt_out_cnt:%u pkt_out_cnt10:%u",
+	      err, errno, pkt_out_cnt.cnt, pkt_out_cnt10.cnt);
+
+	pkt_out_cnt.cnt = ~0;
+	pkt_out_cnt10.cnt = ~0;
+	err = bpf_map_lookup_elem(sk_pkt_out_cnt_fd, &cli_fd, &pkt_out_cnt);
+	if (!err)
+		err = bpf_map_lookup_elem(sk_pkt_out_cnt10_fd, &cli_fd,
+					  &pkt_out_cnt10);
+	/* Active connection is fullsock from the beginning.
+	 * 1 SYN and 1 ACK during 3WHS
+	 * 2 Acks on data packet.
+	 *
+	 * The bpf_prog initialized it to 0xeB9F.
+	 */
+	CHECK(err || pkt_out_cnt.cnt != 0xeB9F + 4 ||
+	      pkt_out_cnt10.cnt != 0xeB9F + 40,
+	      "bpf_map_lookup_elem(sk_pkt_out_cnt, &cli_fd)",
+	      "err:%d errno:%d pkt_out_cnt:%u pkt_out_cnt10:%u",
+	      err, errno, pkt_out_cnt.cnt, pkt_out_cnt10.cnt);
+}
+
+static void init_sk_storage(int sk_fd, __u32 pkt_out_cnt)
+{
+	struct bpf_spinlock_cnt scnt = {};
+	int err;
+
+	scnt.cnt = pkt_out_cnt;
+	err = bpf_map_update_elem(sk_pkt_out_cnt_fd, &sk_fd, &scnt,
+				  BPF_NOEXIST);
+	CHECK(err, "bpf_map_update_elem(sk_pkt_out_cnt_fd)",
+	      "err:%d errno:%d", err, errno);
+
+	scnt.cnt *= 10;
+	err = bpf_map_update_elem(sk_pkt_out_cnt10_fd, &sk_fd, &scnt,
+				  BPF_NOEXIST);
+	CHECK(err, "bpf_map_update_elem(sk_pkt_out_cnt10_fd)",
+	      "err:%d errno:%d", err, errno);
+}
+
 static void test(void)
 {
 	int listen_fd, cli_fd, accept_fd, epfd, err;
 	struct epoll_event ev;
 	socklen_t addrlen;
+	int i;
 
 	addrlen = sizeof(struct sockaddr_in6);
 	ev.events = EPOLLIN;
@@ -308,24 +377,30 @@ static void test(void)
 	      accept_fd, errno);
 	close(listen_fd);
 
-	/* Send some data from accept_fd to cli_fd */
-	err = send(accept_fd, DATA, DATA_LEN, 0);
-	CHECK(err != DATA_LEN, "send(accept_fd)", "err:%d errno:%d",
-	      err, errno);
-
-	/* Have some timeout in recv(cli_fd). Just in case. */
 	ev.data.fd = cli_fd;
 	err = epoll_ctl(epfd, EPOLL_CTL_ADD, cli_fd, &ev);
 	CHECK(err, "epoll_ctl(EPOLL_CTL_ADD, cli_fd)", "err:%d errno:%d",
 	      err, errno);
 
-	err = epoll_wait(epfd, &ev, 1, 1000);
-	CHECK(err != 1 || ev.data.fd != cli_fd,
-	      "epoll_wait(cli_fd)", "err:%d errno:%d ev.data.fd:%d cli_fd:%d",
-	      err, errno, ev.data.fd, cli_fd);
+	init_sk_storage(accept_fd, 2);
 
-	err = recv(cli_fd, NULL, 0, MSG_TRUNC);
-	CHECK(err, "recv(cli_fd)", "err:%d errno:%d", err, errno);
+	for (i = 0; i < 2; i++) {
+		/* Send some data from accept_fd to cli_fd */
+		err = send(accept_fd, DATA, DATA_LEN, 0);
+		CHECK(err != DATA_LEN, "send(accept_fd)", "err:%d errno:%d",
+		      err, errno);
+
+		/* Have some timeout in recv(cli_fd). Just in case. */
+		err = epoll_wait(epfd, &ev, 1, 1000);
+		CHECK(err != 1 || ev.data.fd != cli_fd,
+		      "epoll_wait(cli_fd)", "err:%d errno:%d ev.data.fd:%d cli_fd:%d",
+		      err, errno, ev.data.fd, cli_fd);
+
+		err = recv(cli_fd, NULL, 0, MSG_TRUNC);
+		CHECK(err, "recv(cli_fd)", "err:%d errno:%d", err, errno);
+	}
+
+	check_sk_pkt_out_cnt(accept_fd, cli_fd);
 
 	close(epfd);
 	close(accept_fd);
@@ -395,6 +470,14 @@ int main(int argc, char **argv)
 	CHECK(!map, "cannot find linum_map", "(null)");
 	linum_map_fd = bpf_map__fd(map);
 
+	map = bpf_object__find_map_by_name(obj, "sk_pkt_out_cnt");
+	CHECK(!map, "cannot find sk_pkt_out_cnt", "(null)");
+	sk_pkt_out_cnt_fd = bpf_map__fd(map);
+
+	map = bpf_object__find_map_by_name(obj, "sk_pkt_out_cnt10");
+	CHECK(!map, "cannot find sk_pkt_out_cnt10", "(null)");
+	sk_pkt_out_cnt10_fd = bpf_map__fd(map);
+
 	test();
 
 	bpf_object__close(obj);
-- 
2.17.1