diff mbox series

[v3,bpf-next,1/2] bpf: Allow bpf_map_lookup_elem() on an xskmap

Message ID 20190531185705.2629959-2-jonathan.lemon@gmail.com
State Changes Requested
Delegated to: BPF Maintainers
Headers show
Series Better handling of xskmap entries | expand

Commit Message

Jonathan Lemon May 31, 2019, 6:57 p.m. UTC
Currently, the AF_XDP code uses a separate map in order to
determine if an xsk is bound to a queue.  Instead of doing this,
have bpf_map_lookup_elem() return the queue_id, as a way of
indicating that there is a valid entry at the map index.

Rearrange some xdp_sock members to eliminate structure holes.

Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
Acked-by: Björn Töpel <bjorn.topel@intel.com>
---
 include/net/xdp_sock.h                            |  6 +++---
 kernel/bpf/verifier.c                             |  6 +++++-
 kernel/bpf/xskmap.c                               |  4 +++-
 .../selftests/bpf/verifier/prevent_map_lookup.c   | 15 ---------------
 4 files changed, 11 insertions(+), 20 deletions(-)

Comments

Song Liu June 1, 2019, 11:06 p.m. UTC | #1
> On May 31, 2019, at 11:57 AM, Jonathan Lemon <jonathan.lemon@gmail.com> wrote:
> 
> Currently, the AF_XDP code uses a separate map in order to
> determine if an xsk is bound to a queue.  Instead of doing this,
> have bpf_map_lookup_elem() return the queue_id, as a way of
> indicating that there is a valid entry at the map index.
> 
> Rearrange some xdp_sock members to eliminate structure holes.
> 
> Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
> Acked-by: Björn Töpel <bjorn.topel@intel.com>

Acked-by: Song Liu <songliubraving@fb.com>

> ---
> include/net/xdp_sock.h                            |  6 +++---
> kernel/bpf/verifier.c                             |  6 +++++-
> kernel/bpf/xskmap.c                               |  4 +++-
> .../selftests/bpf/verifier/prevent_map_lookup.c   | 15 ---------------
> 4 files changed, 11 insertions(+), 20 deletions(-)
> 
> diff --git a/include/net/xdp_sock.h b/include/net/xdp_sock.h
> index d074b6d60f8a..7d84b1da43d2 100644
> --- a/include/net/xdp_sock.h
> +++ b/include/net/xdp_sock.h
> @@ -57,12 +57,12 @@ struct xdp_sock {
> 	struct net_device *dev;
> 	struct xdp_umem *umem;
> 	struct list_head flush_node;
> -	u16 queue_id;
> -	struct xsk_queue *tx ____cacheline_aligned_in_smp;
> -	struct list_head list;
> +	u32 queue_id;
> 	bool zc;
> 	/* Protects multiple processes in the control path */
> 	struct mutex mutex;
> +	struct xsk_queue *tx ____cacheline_aligned_in_smp;
> +	struct list_head list;
> 	/* Mutual exclusion of NAPI TX thread and sendmsg error paths
> 	 * in the SKB destructor callback.
> 	 */
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 2778417e6e0c..91c730f85e92 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -2905,10 +2905,14 @@ static int check_map_func_compatibility(struct bpf_verifier_env *env,
> 	 * appear.
> 	 */
> 	case BPF_MAP_TYPE_CPUMAP:
> -	case BPF_MAP_TYPE_XSKMAP:
> 		if (func_id != BPF_FUNC_redirect_map)
> 			goto error;
> 		break;
> +	case BPF_MAP_TYPE_XSKMAP:
> +		if (func_id != BPF_FUNC_redirect_map &&
> +		    func_id != BPF_FUNC_map_lookup_elem)
> +			goto error;
> +		break;
> 	case BPF_MAP_TYPE_ARRAY_OF_MAPS:
> 	case BPF_MAP_TYPE_HASH_OF_MAPS:
> 		if (func_id != BPF_FUNC_map_lookup_elem)
> diff --git a/kernel/bpf/xskmap.c b/kernel/bpf/xskmap.c
> index 686d244e798d..249b22089014 100644
> --- a/kernel/bpf/xskmap.c
> +++ b/kernel/bpf/xskmap.c
> @@ -154,7 +154,9 @@ void __xsk_map_flush(struct bpf_map *map)
> 
> static void *xsk_map_lookup_elem(struct bpf_map *map, void *key)
> {
> -	return ERR_PTR(-EOPNOTSUPP);
> +	struct xdp_sock *xs = __xsk_map_lookup_elem(map, *(u32 *)key);
> +
> +	return xs ? &xs->queue_id : NULL;
> }
> 
> static int xsk_map_update_elem(struct bpf_map *map, void *key, void *value,
> diff --git a/tools/testing/selftests/bpf/verifier/prevent_map_lookup.c b/tools/testing/selftests/bpf/verifier/prevent_map_lookup.c
> index bbdba990fefb..da7a4b37cb98 100644
> --- a/tools/testing/selftests/bpf/verifier/prevent_map_lookup.c
> +++ b/tools/testing/selftests/bpf/verifier/prevent_map_lookup.c
> @@ -28,21 +28,6 @@
> 	.errstr = "cannot pass map_type 18 into func bpf_map_lookup_elem",
> 	.prog_type = BPF_PROG_TYPE_SOCK_OPS,
> },
> -{
> -	"prevent map lookup in xskmap",
> -	.insns = {
> -	BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
> -	BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
> -	BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
> -	BPF_LD_MAP_FD(BPF_REG_1, 0),
> -	BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem),
> -	BPF_EXIT_INSN(),
> -	},
> -	.fixup_map_xskmap = { 3 },
> -	.result = REJECT,
> -	.errstr = "cannot pass map_type 17 into func bpf_map_lookup_elem",
> -	.prog_type = BPF_PROG_TYPE_XDP,
> -},
> {
> 	"prevent map lookup in stack trace",
> 	.insns = {
> -- 
> 2.17.1
>
diff mbox series

Patch

diff --git a/include/net/xdp_sock.h b/include/net/xdp_sock.h
index d074b6d60f8a..7d84b1da43d2 100644
--- a/include/net/xdp_sock.h
+++ b/include/net/xdp_sock.h
@@ -57,12 +57,12 @@  struct xdp_sock {
 	struct net_device *dev;
 	struct xdp_umem *umem;
 	struct list_head flush_node;
-	u16 queue_id;
-	struct xsk_queue *tx ____cacheline_aligned_in_smp;
-	struct list_head list;
+	u32 queue_id;
 	bool zc;
 	/* Protects multiple processes in the control path */
 	struct mutex mutex;
+	struct xsk_queue *tx ____cacheline_aligned_in_smp;
+	struct list_head list;
 	/* Mutual exclusion of NAPI TX thread and sendmsg error paths
 	 * in the SKB destructor callback.
 	 */
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 2778417e6e0c..91c730f85e92 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -2905,10 +2905,14 @@  static int check_map_func_compatibility(struct bpf_verifier_env *env,
 	 * appear.
 	 */
 	case BPF_MAP_TYPE_CPUMAP:
-	case BPF_MAP_TYPE_XSKMAP:
 		if (func_id != BPF_FUNC_redirect_map)
 			goto error;
 		break;
+	case BPF_MAP_TYPE_XSKMAP:
+		if (func_id != BPF_FUNC_redirect_map &&
+		    func_id != BPF_FUNC_map_lookup_elem)
+			goto error;
+		break;
 	case BPF_MAP_TYPE_ARRAY_OF_MAPS:
 	case BPF_MAP_TYPE_HASH_OF_MAPS:
 		if (func_id != BPF_FUNC_map_lookup_elem)
diff --git a/kernel/bpf/xskmap.c b/kernel/bpf/xskmap.c
index 686d244e798d..249b22089014 100644
--- a/kernel/bpf/xskmap.c
+++ b/kernel/bpf/xskmap.c
@@ -154,7 +154,9 @@  void __xsk_map_flush(struct bpf_map *map)
 
 static void *xsk_map_lookup_elem(struct bpf_map *map, void *key)
 {
-	return ERR_PTR(-EOPNOTSUPP);
+	struct xdp_sock *xs = __xsk_map_lookup_elem(map, *(u32 *)key);
+
+	return xs ? &xs->queue_id : NULL;
 }
 
 static int xsk_map_update_elem(struct bpf_map *map, void *key, void *value,
diff --git a/tools/testing/selftests/bpf/verifier/prevent_map_lookup.c b/tools/testing/selftests/bpf/verifier/prevent_map_lookup.c
index bbdba990fefb..da7a4b37cb98 100644
--- a/tools/testing/selftests/bpf/verifier/prevent_map_lookup.c
+++ b/tools/testing/selftests/bpf/verifier/prevent_map_lookup.c
@@ -28,21 +28,6 @@ 
 	.errstr = "cannot pass map_type 18 into func bpf_map_lookup_elem",
 	.prog_type = BPF_PROG_TYPE_SOCK_OPS,
 },
-{
-	"prevent map lookup in xskmap",
-	.insns = {
-	BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
-	BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
-	BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
-	BPF_LD_MAP_FD(BPF_REG_1, 0),
-	BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem),
-	BPF_EXIT_INSN(),
-	},
-	.fixup_map_xskmap = { 3 },
-	.result = REJECT,
-	.errstr = "cannot pass map_type 17 into func bpf_map_lookup_elem",
-	.prog_type = BPF_PROG_TYPE_XDP,
-},
 {
 	"prevent map lookup in stack trace",
 	.insns = {