netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 bpf-next 0/5] bpf: Add support for XDP programs in DEVMAP entries
@ 2020-05-29  5:20 David Ahern
  2020-05-29  5:20 ` [PATCH v3 bpf-next 1/5] devmap: Formalize map value as a named struct David Ahern
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: David Ahern @ 2020-05-29  5:20 UTC (permalink / raw)
  To: netdev
  Cc: bpf, davem, kuba, brouer, toke, lorenzo, daniel, john.fastabend,
	ast, kafai, songliubraving, yhs, andriin, dsahern, David Ahern

Implementation of Daniel's proposal for allowing DEVMAP entries to be
a device index, program fd pair.

Programs are run after XDP_REDIRECT and have access to both Rx device
and Tx device.

v3
- renamed struct to bpf_devmap_val
- used offsetofend to check for expected map size, modification of
  Toke's comment
- check for explicit value sizes
- adjusted switch statement in dev_map_run_prog per Andrii's comment
- changed SEC shortcut to xdp_devmap
- changed selftests to use skeleton and new map declaration

v2
- moved dev_map_ext_val definition to uapi to formalize the API for devmap
  extensions; add bpf_ prefix to the prog_fd and prog_id entries
- changed devmap code to handle struct in a way that it can support future
  extensions
- fixed subject in libbpf patch

v1
- fixed prog put on invalid program - Toke
- changed write value from id to fd per Toke's comments about capabilities
- add test cases

David Ahern (5):
  devmap: Formalize map value as a named struct
  bpf: Add support to attach bpf program to a devmap entry
  xdp: Add xdp_txq_info to xdp_buff
  libbpf: Add SEC name for xdp programs attached to device map
  selftest: Add tests for XDP programs in devmap entries

 include/linux/bpf.h                           |   5 +
 include/net/xdp.h                             |   5 +
 include/uapi/linux/bpf.h                      |  12 ++
 kernel/bpf/devmap.c                           | 121 +++++++++++++++---
 net/core/dev.c                                |  18 +++
 net/core/filter.c                             |  17 +++
 tools/include/uapi/linux/bpf.h                |  12 ++
 tools/lib/bpf/libbpf.c                        |   2 +
 .../bpf/prog_tests/xdp_devmap_attach.c        |  89 +++++++++++++
 .../bpf/progs/test_xdp_devmap_helpers.c       |  22 ++++
 .../bpf/progs/test_xdp_with_devmap_helpers.c  |  43 +++++++
 11 files changed, 328 insertions(+), 18 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/xdp_devmap_attach.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_xdp_devmap_helpers.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_xdp_with_devmap_helpers.c

-- 
2.21.1 (Apple Git-122.3)


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v3 bpf-next 1/5] devmap: Formalize map value as a named struct
  2020-05-29  5:20 [PATCH v3 bpf-next 0/5] bpf: Add support for XDP programs in DEVMAP entries David Ahern
@ 2020-05-29  5:20 ` David Ahern
  2020-05-29  8:22   ` Jesper Dangaard Brouer
  2020-05-29  5:20 ` [PATCH v3 bpf-next 2/5] bpf: Add support to attach bpf program to a devmap entry David Ahern
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: David Ahern @ 2020-05-29  5:20 UTC (permalink / raw)
  To: netdev
  Cc: bpf, davem, kuba, brouer, toke, lorenzo, daniel, john.fastabend,
	ast, kafai, songliubraving, yhs, andriin, dsahern, David Ahern

Add 'struct bpf_devmap_val' to the bpf uapi to formalize the
expected values that can be passed in for a DEVMAP.
Update devmap code to use the struct.

Signed-off-by: David Ahern <dsahern@kernel.org>
---
 include/uapi/linux/bpf.h       |  5 +++++
 kernel/bpf/devmap.c            | 40 +++++++++++++++++++---------------
 tools/include/uapi/linux/bpf.h |  5 +++++
 3 files changed, 33 insertions(+), 17 deletions(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 54b93f8b49b8..42c884dfbad9 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -3625,6 +3625,11 @@ struct xdp_md {
 	__u32 rx_queue_index;  /* rxq->queue_index  */
 };
 
+/* DEVMAP values */
+struct bpf_devmap_val {
+	__u32 ifindex;   /* device index */
+};
+
 enum sk_action {
 	SK_DROP = 0,
 	SK_PASS,
diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
index a51d9fb7a359..2887fe6d1e1d 100644
--- a/kernel/bpf/devmap.c
+++ b/kernel/bpf/devmap.c
@@ -66,6 +66,7 @@ struct bpf_dtab_netdev {
 	struct bpf_dtab *dtab;
 	struct rcu_head rcu;
 	unsigned int idx;
+	struct bpf_devmap_val val;
 };
 
 struct bpf_dtab {
@@ -472,18 +473,15 @@ int dev_map_generic_redirect(struct bpf_dtab_netdev *dst, struct sk_buff *skb,
 static void *dev_map_lookup_elem(struct bpf_map *map, void *key)
 {
 	struct bpf_dtab_netdev *obj = __dev_map_lookup_elem(map, *(u32 *)key);
-	struct net_device *dev = obj ? obj->dev : NULL;
 
-	return dev ? &dev->ifindex : NULL;
+	return obj ? &obj->val : NULL;
 }
 
 static void *dev_map_hash_lookup_elem(struct bpf_map *map, void *key)
 {
 	struct bpf_dtab_netdev *obj = __dev_map_hash_lookup_elem(map,
 								*(u32 *)key);
-	struct net_device *dev = obj ? obj->dev : NULL;
-
-	return dev ? &dev->ifindex : NULL;
+	return obj ? &obj->val : NULL;
 }
 
 static void __dev_map_entry_free(struct rcu_head *rcu)
@@ -541,7 +539,7 @@ static int dev_map_hash_delete_elem(struct bpf_map *map, void *key)
 
 static struct bpf_dtab_netdev *__dev_map_alloc_node(struct net *net,
 						    struct bpf_dtab *dtab,
-						    u32 ifindex,
+						    struct bpf_devmap_val *val,
 						    unsigned int idx)
 {
 	struct bpf_dtab_netdev *dev;
@@ -551,16 +549,18 @@ static struct bpf_dtab_netdev *__dev_map_alloc_node(struct net *net,
 	if (!dev)
 		return ERR_PTR(-ENOMEM);
 
-	dev->dev = dev_get_by_index(net, ifindex);
-	if (!dev->dev) {
-		kfree(dev);
-		return ERR_PTR(-EINVAL);
-	}
+	dev->dev = dev_get_by_index(net, val->ifindex);
+	if (!dev->dev)
+		goto err_out;
 
 	dev->idx = idx;
 	dev->dtab = dtab;
+	dev->val.ifindex = val->ifindex;
 
 	return dev;
+err_out:
+	kfree(dev);
+	return ERR_PTR(-EINVAL);
 }
 
 static int __dev_map_update_elem(struct net *net, struct bpf_map *map,
@@ -568,7 +568,7 @@ static int __dev_map_update_elem(struct net *net, struct bpf_map *map,
 {
 	struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map);
 	struct bpf_dtab_netdev *dev, *old_dev;
-	u32 ifindex = *(u32 *)value;
+	struct bpf_devmap_val val = { };
 	u32 i = *(u32 *)key;
 
 	if (unlikely(map_flags > BPF_EXIST))
@@ -578,10 +578,13 @@ static int __dev_map_update_elem(struct net *net, struct bpf_map *map,
 	if (unlikely(map_flags == BPF_NOEXIST))
 		return -EEXIST;
 
-	if (!ifindex) {
+	/* already verified value_size <= sizeof val */
+	memcpy(&val, value, map->value_size);
+
+	if (!val.ifindex) {
 		dev = NULL;
 	} else {
-		dev = __dev_map_alloc_node(net, dtab, ifindex, i);
+		dev = __dev_map_alloc_node(net, dtab, &val, i);
 		if (IS_ERR(dev))
 			return PTR_ERR(dev);
 	}
@@ -609,12 +612,15 @@ static int __dev_map_hash_update_elem(struct net *net, struct bpf_map *map,
 {
 	struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map);
 	struct bpf_dtab_netdev *dev, *old_dev;
-	u32 ifindex = *(u32 *)value;
+	struct bpf_devmap_val val = { };
 	u32 idx = *(u32 *)key;
 	unsigned long flags;
 	int err = -EEXIST;
 
-	if (unlikely(map_flags > BPF_EXIST || !ifindex))
+	/* already verified value_size <= sizeof val */
+	memcpy(&val, value, map->value_size);
+
+	if (unlikely(map_flags > BPF_EXIST || !val.ifindex))
 		return -EINVAL;
 
 	spin_lock_irqsave(&dtab->index_lock, flags);
@@ -623,7 +629,7 @@ static int __dev_map_hash_update_elem(struct net *net, struct bpf_map *map,
 	if (old_dev && (map_flags & BPF_NOEXIST))
 		goto out_err;
 
-	dev = __dev_map_alloc_node(net, dtab, ifindex, idx);
+	dev = __dev_map_alloc_node(net, dtab, &val, idx);
 	if (IS_ERR(dev)) {
 		err = PTR_ERR(dev);
 		goto out_err;
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 54b93f8b49b8..42c884dfbad9 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -3625,6 +3625,11 @@ struct xdp_md {
 	__u32 rx_queue_index;  /* rxq->queue_index  */
 };
 
+/* DEVMAP values */
+struct bpf_devmap_val {
+	__u32 ifindex;   /* device index */
+};
+
 enum sk_action {
 	SK_DROP = 0,
 	SK_PASS,
-- 
2.21.1 (Apple Git-122.3)


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v3 bpf-next 2/5] bpf: Add support to attach bpf program to a devmap entry
  2020-05-29  5:20 [PATCH v3 bpf-next 0/5] bpf: Add support for XDP programs in DEVMAP entries David Ahern
  2020-05-29  5:20 ` [PATCH v3 bpf-next 1/5] devmap: Formalize map value as a named struct David Ahern
@ 2020-05-29  5:20 ` David Ahern
  2020-05-29  5:20 ` [PATCH v3 bpf-next 3/5] xdp: Add xdp_txq_info to xdp_buff David Ahern
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: David Ahern @ 2020-05-29  5:20 UTC (permalink / raw)
  To: netdev
  Cc: bpf, davem, kuba, brouer, toke, lorenzo, daniel, john.fastabend,
	ast, kafai, songliubraving, yhs, andriin, dsahern, David Ahern

Add BPF_XDP_DEVMAP attach type for use with programs associated with a
DEVMAP entry.

Allow DEVMAPs to associate a program with a device entry by adding
a bpf_prog_fd to 'struct bpf_devmap_val'. Values read show the program
id, so the fd and id are a union.

The program associated with the fd must have type XDP with expected
attach type BPF_XDP_DEVMAP. When a program is associated with a device
index, the program is run on an XDP_REDIRECT and before the buffer is
added to the per-cpu queue. At this point rxq data is still valid; the
next patch adds tx device information allowing the prorgam to see both
ingress and egress device indices.

XDP generic is skb based and XDP programs do not work with skb's. Block
the use case by walking maps used by a program that is to be attached
via xdpgeneric and fail if any of them are DEVMAP / DEVMAP_HASH with
> 4-byte values.

Block attach of BPF_XDP_DEVMAP programs to devices.

Signed-off-by: David Ahern <dsahern@kernel.org>
---
 include/linux/bpf.h            |  5 ++
 include/uapi/linux/bpf.h       |  5 ++
 kernel/bpf/devmap.c            | 84 ++++++++++++++++++++++++++++++++--
 net/core/dev.c                 | 18 ++++++++
 tools/include/uapi/linux/bpf.h |  5 ++
 5 files changed, 113 insertions(+), 4 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index efe8836b5c48..088751bc09aa 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1242,6 +1242,7 @@ int dev_map_enqueue(struct bpf_dtab_netdev *dst, struct xdp_buff *xdp,
 		    struct net_device *dev_rx);
 int dev_map_generic_redirect(struct bpf_dtab_netdev *dst, struct sk_buff *skb,
 			     struct bpf_prog *xdp_prog);
+bool dev_map_can_have_prog(struct bpf_map *map);
 
 struct bpf_cpu_map_entry *__cpu_map_lookup_elem(struct bpf_map *map, u32 key);
 void __cpu_map_flush(void);
@@ -1355,6 +1356,10 @@ static inline struct net_device  *__dev_map_hash_lookup_elem(struct bpf_map *map
 {
 	return NULL;
 }
+static inline bool dev_map_can_have_prog(struct bpf_map *map)
+{
+	return false;
+}
 
 static inline void __dev_flush(void)
 {
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 42c884dfbad9..02177049cf66 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -224,6 +224,7 @@ enum bpf_attach_type {
 	BPF_CGROUP_INET6_GETPEERNAME,
 	BPF_CGROUP_INET4_GETSOCKNAME,
 	BPF_CGROUP_INET6_GETSOCKNAME,
+	BPF_XDP_DEVMAP,
 	__MAX_BPF_ATTACH_TYPE
 };
 
@@ -3628,6 +3629,10 @@ struct xdp_md {
 /* DEVMAP values */
 struct bpf_devmap_val {
 	__u32 ifindex;   /* device index */
+	union {
+		int   bpf_prog_fd;  /* prog fd on map write */
+		__u32 bpf_prog_id;  /* prog id on map read */
+	};
 };
 
 enum sk_action {
diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
index 2887fe6d1e1d..3152151d3bb8 100644
--- a/kernel/bpf/devmap.c
+++ b/kernel/bpf/devmap.c
@@ -64,6 +64,7 @@ struct bpf_dtab_netdev {
 	struct net_device *dev; /* must be first member, due to tracepoint */
 	struct hlist_node index_hlist;
 	struct bpf_dtab *dtab;
+	struct bpf_prog *xdp_prog;
 	struct rcu_head rcu;
 	unsigned int idx;
 	struct bpf_devmap_val val;
@@ -106,12 +107,18 @@ static inline struct hlist_head *dev_map_index_hash(struct bpf_dtab *dtab,
 
 static int dev_map_init_map(struct bpf_dtab *dtab, union bpf_attr *attr)
 {
+	__u32 valsize = attr->value_size;
 	u64 cost = 0;
 	int err;
 
-	/* check sanity of attributes */
+	/* check sanity of attributes. 2 value sizes supported:
+	 * 4 bytes: ifindex
+	 * 8 bytes: ifindex + prog fd
+	 */
 	if (attr->max_entries == 0 || attr->key_size != 4 ||
-	    attr->value_size != 4 || attr->map_flags & ~DEV_CREATE_FLAG_MASK)
+	    (valsize != offsetofend(struct bpf_devmap_val, ifindex) &&
+	     valsize != offsetofend(struct bpf_devmap_val, bpf_prog_fd)) ||
+	    attr->map_flags & ~DEV_CREATE_FLAG_MASK)
 		return -EINVAL;
 
 	/* Lookup returns a pointer straight to dev->ifindex, so make sure the
@@ -218,6 +225,8 @@ static void dev_map_free(struct bpf_map *map)
 
 			hlist_for_each_entry_safe(dev, next, head, index_hlist) {
 				hlist_del_rcu(&dev->index_hlist);
+				if (dev->xdp_prog)
+					bpf_prog_put(dev->xdp_prog);
 				dev_put(dev->dev);
 				kfree(dev);
 			}
@@ -232,6 +241,8 @@ static void dev_map_free(struct bpf_map *map)
 			if (!dev)
 				continue;
 
+			if (dev->xdp_prog)
+				bpf_prog_put(dev->xdp_prog);
 			dev_put(dev->dev);
 			kfree(dev);
 		}
@@ -318,6 +329,16 @@ static int dev_map_hash_get_next_key(struct bpf_map *map, void *key,
 	return -ENOENT;
 }
 
+bool dev_map_can_have_prog(struct bpf_map *map)
+{
+	if ((map->map_type == BPF_MAP_TYPE_DEVMAP ||
+	     map->map_type == BPF_MAP_TYPE_DEVMAP_HASH) &&
+	    map->value_size != offsetofend(struct bpf_devmap_val, ifindex))
+		return true;
+
+	return false;
+}
+
 static int bq_xmit_all(struct xdp_dev_bulk_queue *bq, u32 flags)
 {
 	struct net_device *dev = bq->dev;
@@ -442,6 +463,30 @@ static inline int __xdp_enqueue(struct net_device *dev, struct xdp_buff *xdp,
 	return bq_enqueue(dev, xdpf, dev_rx);
 }
 
+static struct xdp_buff *dev_map_run_prog(struct net_device *dev,
+					 struct xdp_buff *xdp,
+					 struct bpf_prog *xdp_prog)
+{
+	u32 act;
+
+	act = bpf_prog_run_xdp(xdp_prog, xdp);
+	switch (act) {
+	case XDP_PASS:
+		return xdp;
+	case XDP_DROP:
+		break;
+	default:
+		bpf_warn_invalid_xdp_action(act);
+		fallthrough;
+	case XDP_ABORTED:
+		trace_xdp_exception(dev, xdp_prog, act);
+		break;
+	}
+
+	xdp_return_buff(xdp);
+	return NULL;
+}
+
 int dev_xdp_enqueue(struct net_device *dev, struct xdp_buff *xdp,
 		    struct net_device *dev_rx)
 {
@@ -453,6 +498,11 @@ int dev_map_enqueue(struct bpf_dtab_netdev *dst, struct xdp_buff *xdp,
 {
 	struct net_device *dev = dst->dev;
 
+	if (dst->xdp_prog) {
+		xdp = dev_map_run_prog(dev, xdp, dst->xdp_prog);
+		if (!xdp)
+			return 0;
+	}
 	return __xdp_enqueue(dev, xdp, dev_rx);
 }
 
@@ -489,6 +539,8 @@ static void __dev_map_entry_free(struct rcu_head *rcu)
 	struct bpf_dtab_netdev *dev;
 
 	dev = container_of(rcu, struct bpf_dtab_netdev, rcu);
+	if (dev->xdp_prog)
+		bpf_prog_put(dev->xdp_prog);
 	dev_put(dev->dev);
 	kfree(dev);
 }
@@ -542,6 +594,7 @@ static struct bpf_dtab_netdev *__dev_map_alloc_node(struct net *net,
 						    struct bpf_devmap_val *val,
 						    unsigned int idx)
 {
+	struct bpf_prog *prog = NULL;
 	struct bpf_dtab_netdev *dev;
 
 	dev = kmalloc_node(sizeof(*dev), GFP_ATOMIC | __GFP_NOWARN,
@@ -553,11 +606,31 @@ static struct bpf_dtab_netdev *__dev_map_alloc_node(struct net *net,
 	if (!dev->dev)
 		goto err_out;
 
+	if (val->bpf_prog_fd >= 0) {
+		prog = bpf_prog_get_type_dev(val->bpf_prog_fd,
+					     BPF_PROG_TYPE_XDP, false);
+		if (IS_ERR(prog))
+			goto err_put_dev;
+		if (prog->expected_attach_type != BPF_XDP_DEVMAP)
+			goto err_put_prog;
+	}
+
 	dev->idx = idx;
 	dev->dtab = dtab;
+	if (prog) {
+		dev->xdp_prog = prog;
+		dev->val.bpf_prog_id = prog->aux->id;
+	} else {
+		dev->xdp_prog = NULL;
+		dev->val.bpf_prog_id = 0;
+	}
 	dev->val.ifindex = val->ifindex;
 
 	return dev;
+err_put_prog:
+	bpf_prog_put(prog);
+err_put_dev:
+	dev_put(dev->dev);
 err_out:
 	kfree(dev);
 	return ERR_PTR(-EINVAL);
@@ -567,8 +640,8 @@ static int __dev_map_update_elem(struct net *net, struct bpf_map *map,
 				 void *key, void *value, u64 map_flags)
 {
 	struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map);
+	struct bpf_devmap_val val = { .bpf_prog_fd = -1 };
 	struct bpf_dtab_netdev *dev, *old_dev;
-	struct bpf_devmap_val val = { };
 	u32 i = *(u32 *)key;
 
 	if (unlikely(map_flags > BPF_EXIST))
@@ -583,6 +656,9 @@ static int __dev_map_update_elem(struct net *net, struct bpf_map *map,
 
 	if (!val.ifindex) {
 		dev = NULL;
+		/* can not specify fd if ifindex is 0 */
+		if (val.bpf_prog_fd != -1)
+			return -EINVAL;
 	} else {
 		dev = __dev_map_alloc_node(net, dtab, &val, i);
 		if (IS_ERR(dev))
@@ -611,8 +687,8 @@ static int __dev_map_hash_update_elem(struct net *net, struct bpf_map *map,
 				     void *key, void *value, u64 map_flags)
 {
 	struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map);
+	struct bpf_devmap_val val = { .bpf_prog_fd = -1 };
 	struct bpf_dtab_netdev *dev, *old_dev;
-	struct bpf_devmap_val val = { };
 	u32 idx = *(u32 *)key;
 	unsigned long flags;
 	int err = -EEXIST;
diff --git a/net/core/dev.c b/net/core/dev.c
index ae37586f6ee8..10684833f864 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5420,6 +5420,18 @@ static int generic_xdp_install(struct net_device *dev, struct netdev_bpf *xdp)
 	struct bpf_prog *new = xdp->prog;
 	int ret = 0;
 
+	if (new) {
+		u32 i;
+
+		/* generic XDP does not work with DEVMAPs that can
+		 * have a bpf_prog installed on an entry
+		 */
+		for (i = 0; i < new->aux->used_map_cnt; i++) {
+			if (dev_map_can_have_prog(new->aux->used_maps[i]))
+				return -EINVAL;
+		}
+	}
+
 	switch (xdp->command) {
 	case XDP_SETUP_PROG:
 		rcu_assign_pointer(dev->xdp_prog, new);
@@ -8835,6 +8847,12 @@ int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
 			return -EINVAL;
 		}
 
+		if (prog->expected_attach_type == BPF_XDP_DEVMAP) {
+			NL_SET_ERR_MSG(extack, "BPF_XDP_DEVMAP programs can not be attached to a device");
+			bpf_prog_put(prog);
+			return -EINVAL;
+		}
+
 		/* prog->aux->id may be 0 for orphaned device-bound progs */
 		if (prog->aux->id && prog->aux->id == prog_id) {
 			bpf_prog_put(prog);
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 42c884dfbad9..02177049cf66 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -224,6 +224,7 @@ enum bpf_attach_type {
 	BPF_CGROUP_INET6_GETPEERNAME,
 	BPF_CGROUP_INET4_GETSOCKNAME,
 	BPF_CGROUP_INET6_GETSOCKNAME,
+	BPF_XDP_DEVMAP,
 	__MAX_BPF_ATTACH_TYPE
 };
 
@@ -3628,6 +3629,10 @@ struct xdp_md {
 /* DEVMAP values */
 struct bpf_devmap_val {
 	__u32 ifindex;   /* device index */
+	union {
+		int   bpf_prog_fd;  /* prog fd on map write */
+		__u32 bpf_prog_id;  /* prog id on map read */
+	};
 };
 
 enum sk_action {
-- 
2.21.1 (Apple Git-122.3)


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v3 bpf-next 3/5] xdp: Add xdp_txq_info to xdp_buff
  2020-05-29  5:20 [PATCH v3 bpf-next 0/5] bpf: Add support for XDP programs in DEVMAP entries David Ahern
  2020-05-29  5:20 ` [PATCH v3 bpf-next 1/5] devmap: Formalize map value as a named struct David Ahern
  2020-05-29  5:20 ` [PATCH v3 bpf-next 2/5] bpf: Add support to attach bpf program to a devmap entry David Ahern
@ 2020-05-29  5:20 ` David Ahern
  2020-05-29  5:20 ` [PATCH v3 bpf-next 4/5] libbpf: Add SEC name for xdp programs attached to device map David Ahern
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: David Ahern @ 2020-05-29  5:20 UTC (permalink / raw)
  To: netdev
  Cc: bpf, davem, kuba, brouer, toke, lorenzo, daniel, john.fastabend,
	ast, kafai, songliubraving, yhs, andriin, dsahern, David Ahern

Add xdp_txq_info as the Tx counterpart to xdp_rxq_info. At the
moment only the device is added. Other fields (queue_index)
can be added as use cases arise.

From a UAPI perspective, add egress_ifindex to xdp context for
bpf programs to see the Tx device.

Update the verifier to only allow accesses to egress_ifindex by
XDP programs with BPF_XDP_DEVMAP expected attach type.

Signed-off-by: David Ahern <dsahern@kernel.org>
---
 include/net/xdp.h              |  5 +++++
 include/uapi/linux/bpf.h       |  2 ++
 kernel/bpf/devmap.c            |  3 +++
 net/core/filter.c              | 17 +++++++++++++++++
 tools/include/uapi/linux/bpf.h |  2 ++
 5 files changed, 29 insertions(+)

diff --git a/include/net/xdp.h b/include/net/xdp.h
index 90f11760bd12..d54022959491 100644
--- a/include/net/xdp.h
+++ b/include/net/xdp.h
@@ -61,12 +61,17 @@ struct xdp_rxq_info {
 	struct xdp_mem_info mem;
 } ____cacheline_aligned; /* perf critical, avoid false-sharing */
 
+struct xdp_txq_info {
+	struct net_device *dev;
+};
+
 struct xdp_buff {
 	void *data;
 	void *data_end;
 	void *data_meta;
 	void *data_hard_start;
 	struct xdp_rxq_info *rxq;
+	struct xdp_txq_info *txq;
 	u32 frame_sz; /* frame size to deduce data_hard_end/reserved tailroom*/
 };
 
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 02177049cf66..61ae81bf67de 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -3624,6 +3624,8 @@ struct xdp_md {
 	/* Below access go through struct xdp_rxq_info */
 	__u32 ingress_ifindex; /* rxq->dev->ifindex */
 	__u32 rx_queue_index;  /* rxq->queue_index  */
+
+	__u32 egress_ifindex;  /* txq->dev->ifindex */
 };
 
 /* DEVMAP values */
diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
index 3152151d3bb8..defdd22caa4b 100644
--- a/kernel/bpf/devmap.c
+++ b/kernel/bpf/devmap.c
@@ -467,8 +467,11 @@ static struct xdp_buff *dev_map_run_prog(struct net_device *dev,
 					 struct xdp_buff *xdp,
 					 struct bpf_prog *xdp_prog)
 {
+	struct xdp_txq_info txq = { .dev = dev };
 	u32 act;
 
+	xdp->txq = &txq;
+
 	act = bpf_prog_run_xdp(xdp_prog, xdp);
 	switch (act) {
 	case XDP_PASS:
diff --git a/net/core/filter.c b/net/core/filter.c
index a6fc23447f12..2e9dbfd8e60c 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -7014,6 +7014,13 @@ static bool xdp_is_valid_access(int off, int size,
 				const struct bpf_prog *prog,
 				struct bpf_insn_access_aux *info)
 {
+	if (prog->expected_attach_type != BPF_XDP_DEVMAP) {
+		switch (off) {
+		case offsetof(struct xdp_md, egress_ifindex):
+			return false;
+		}
+	}
+
 	if (type == BPF_WRITE) {
 		if (bpf_prog_is_dev_bound(prog->aux)) {
 			switch (off) {
@@ -7967,6 +7974,16 @@ static u32 xdp_convert_ctx_access(enum bpf_access_type type,
 				      offsetof(struct xdp_rxq_info,
 					       queue_index));
 		break;
+	case offsetof(struct xdp_md, egress_ifindex):
+		*insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(struct xdp_buff, txq),
+				      si->dst_reg, si->src_reg,
+				      offsetof(struct xdp_buff, txq));
+		*insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(struct xdp_txq_info, dev),
+				      si->dst_reg, si->dst_reg,
+				      offsetof(struct xdp_txq_info, dev));
+		*insn++ = BPF_LDX_MEM(BPF_W, si->dst_reg, si->dst_reg,
+				      offsetof(struct net_device, ifindex));
+		break;
 	}
 
 	return insn - insn_buf;
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 02177049cf66..61ae81bf67de 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -3624,6 +3624,8 @@ struct xdp_md {
 	/* Below access go through struct xdp_rxq_info */
 	__u32 ingress_ifindex; /* rxq->dev->ifindex */
 	__u32 rx_queue_index;  /* rxq->queue_index  */
+
+	__u32 egress_ifindex;  /* txq->dev->ifindex */
 };
 
 /* DEVMAP values */
-- 
2.21.1 (Apple Git-122.3)


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v3 bpf-next 4/5] libbpf: Add SEC name for xdp programs attached to device map
  2020-05-29  5:20 [PATCH v3 bpf-next 0/5] bpf: Add support for XDP programs in DEVMAP entries David Ahern
                   ` (2 preceding siblings ...)
  2020-05-29  5:20 ` [PATCH v3 bpf-next 3/5] xdp: Add xdp_txq_info to xdp_buff David Ahern
@ 2020-05-29  5:20 ` David Ahern
  2020-05-29  5:20 ` [PATCH v3 bpf-next 5/5] selftest: Add tests for XDP programs in devmap entries David Ahern
  2020-05-29 16:46 ` [PATCH v3 bpf-next 0/5] bpf: Add support for XDP programs in DEVMAP entries Toke Høiland-Jørgensen
  5 siblings, 0 replies; 13+ messages in thread
From: David Ahern @ 2020-05-29  5:20 UTC (permalink / raw)
  To: netdev
  Cc: bpf, davem, kuba, brouer, toke, lorenzo, daniel, john.fastabend,
	ast, kafai, songliubraving, yhs, andriin, dsahern, David Ahern

Support SEC("xdp_devmap*") as a short cut for loading the program with
type BPF_PROG_TYPE_XDP and expected attach type BPF_XDP_DEVMAP.

Signed-off-by: David Ahern <dsahern@kernel.org>
---
 tools/lib/bpf/libbpf.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 74d967619dcf..85d4f1c5fc52 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -6657,6 +6657,8 @@ static const struct bpf_sec_def section_defs[] = {
 		.expected_attach_type = BPF_TRACE_ITER,
 		.is_attach_btf = true,
 		.attach_fn = attach_iter),
+	BPF_EAPROG_SEC("xdp_devmap",		BPF_PROG_TYPE_XDP,
+						BPF_XDP_DEVMAP),
 	BPF_PROG_SEC("xdp",			BPF_PROG_TYPE_XDP),
 	BPF_PROG_SEC("perf_event",		BPF_PROG_TYPE_PERF_EVENT),
 	BPF_PROG_SEC("lwt_in",			BPF_PROG_TYPE_LWT_IN),
-- 
2.21.1 (Apple Git-122.3)


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v3 bpf-next 5/5] selftest: Add tests for XDP programs in devmap entries
  2020-05-29  5:20 [PATCH v3 bpf-next 0/5] bpf: Add support for XDP programs in DEVMAP entries David Ahern
                   ` (3 preceding siblings ...)
  2020-05-29  5:20 ` [PATCH v3 bpf-next 4/5] libbpf: Add SEC name for xdp programs attached to device map David Ahern
@ 2020-05-29  5:20 ` David Ahern
  2020-05-29 16:45   ` Toke Høiland-Jørgensen
  2020-05-29 16:46 ` [PATCH v3 bpf-next 0/5] bpf: Add support for XDP programs in DEVMAP entries Toke Høiland-Jørgensen
  5 siblings, 1 reply; 13+ messages in thread
From: David Ahern @ 2020-05-29  5:20 UTC (permalink / raw)
  To: netdev
  Cc: bpf, davem, kuba, brouer, toke, lorenzo, daniel, john.fastabend,
	ast, kafai, songliubraving, yhs, andriin, dsahern, David Ahern

Add tests to verify ability to add an XDP program to a
entry in a DEVMAP.

Add negative tests to show DEVMAP programs can not be
attached to devices as a normal XDP program, and accesses
to egress_ifindex require BPF_XDP_DEVMAP attach type.

Signed-off-by: David Ahern <dsahern@kernel.org>
---
 .../bpf/prog_tests/xdp_devmap_attach.c        | 89 +++++++++++++++++++
 .../bpf/progs/test_xdp_devmap_helpers.c       | 22 +++++
 .../bpf/progs/test_xdp_with_devmap_helpers.c  | 43 +++++++++
 3 files changed, 154 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/xdp_devmap_attach.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_xdp_devmap_helpers.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_xdp_with_devmap_helpers.c

diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_devmap_attach.c b/tools/testing/selftests/bpf/prog_tests/xdp_devmap_attach.c
new file mode 100644
index 000000000000..caeea19f2772
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/xdp_devmap_attach.c
@@ -0,0 +1,89 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <uapi/linux/bpf.h>
+#include <linux/if_link.h>
+#include <test_progs.h>
+
+#include "test_xdp_devmap_helpers.skel.h"
+#include "test_xdp_with_devmap_helpers.skel.h"
+
+#define IFINDEX_LO 1
+
+void test_xdp_with_devmap_helpers(void)
+{
+	struct test_xdp_with_devmap_helpers *skel;
+	struct bpf_prog_info info = {};
+	struct bpf_devmap_val val = {
+		.ifindex = IFINDEX_LO,
+	};
+	__u32 id, len = sizeof(info);
+	__u32 duration, idx = 0;
+	int err, dm_fd, map_fd;
+
+
+	skel = test_xdp_with_devmap_helpers__open_and_load();
+	if (CHECK_FAIL(!skel)) {
+		perror("test_xdp_with_devmap_helpers__open_and_load");
+		return;
+	}
+
+	/* can not attach program with DEVMAPs that allow programs
+	 * as xdp generic
+	 */
+	dm_fd = bpf_program__fd(skel->progs.xdp_redir_prog);
+	err = bpf_set_link_xdp_fd(IFINDEX_LO, dm_fd, XDP_FLAGS_SKB_MODE);
+	CHECK(err == 0, "Generic attach of program with 8-byte devmap",
+	      "should have failed\n");
+
+	dm_fd = bpf_program__fd(skel->progs.xdp_dummy_dm);
+	map_fd = bpf_map__fd(skel->maps.dm_ports);
+	err = bpf_obj_get_info_by_fd(dm_fd, &info, &len);
+	if (CHECK_FAIL(err))
+		goto out_close;
+
+	val.bpf_prog_fd = dm_fd;
+	err = bpf_map_update_elem(map_fd, &idx, &val, 0);
+	CHECK(err, "Add program to devmap entry",
+	      "err %d errno %d\n", err, errno);
+
+	err = bpf_map_lookup_elem(map_fd, &id, &val);
+	CHECK(err, "Read devmap entry", "err %d errno %d\n", err, errno);
+	CHECK(info.id != val.bpf_prog_id, "Expected program id in devmap entry",
+	      "expected %u read %u\n", info.id, val.bpf_prog_id);
+
+	/* can not attach BPF_XDP_DEVMAP program to a device */
+	err = bpf_set_link_xdp_fd(IFINDEX_LO, dm_fd, XDP_FLAGS_SKB_MODE);
+	CHECK(err == 0, "Attach of BPF_XDP_DEVMAP program",
+	      "should have failed\n");
+
+	val.ifindex = 1;
+	val.bpf_prog_fd = bpf_program__fd(skel->progs.xdp_dummy_prog);
+	err = bpf_map_update_elem(map_fd, &idx, &val, 0);
+	CHECK(err == 0, "Add non-BPF_XDP_DEVMAP program to devmap entry",
+	      "should have failed\n");
+
+out_close:
+	test_xdp_with_devmap_helpers__destroy(skel);
+}
+
+void test_neg_xdp_devmap_helpers(void)
+{
+	struct test_xdp_devmap_helpers *skel;
+	__u32 duration;
+
+	skel = test_xdp_devmap_helpers__open_and_load();
+	if (CHECK(skel,
+		  "Load of XDP program accessing egress ifindex without attach type",
+		  "should have failed\n")) {
+		test_xdp_devmap_helpers__destroy(skel);
+	}
+}
+
+
+void test_xdp_devmap_attach(void)
+{
+	if (test__start_subtest("DEVMAP with programs in entries"))
+		test_xdp_with_devmap_helpers();
+
+	if (test__start_subtest("Verifier check of DEVMAP programs"))
+		test_neg_xdp_devmap_helpers();
+}
diff --git a/tools/testing/selftests/bpf/progs/test_xdp_devmap_helpers.c b/tools/testing/selftests/bpf/progs/test_xdp_devmap_helpers.c
new file mode 100644
index 000000000000..b360ba2bd441
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_xdp_devmap_helpers.c
@@ -0,0 +1,22 @@
+// SPDX-License-Identifier: GPL-2.0
+/* fails to load without expected_attach_type = BPF_XDP_DEVMAP
+ * because of access to egress_ifindex
+ */
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+
+SEC("xdp_dm_log")
+int xdpdm_devlog(struct xdp_md *ctx)
+{
+	char fmt[] = "devmap redirect: dev %u -> dev %u len %u\n";
+	void *data_end = (void *)(long)ctx->data_end;
+	void *data = (void *)(long)ctx->data;
+	unsigned int len = data_end - data;
+
+	bpf_trace_printk(fmt, sizeof(fmt),
+			 ctx->ingress_ifindex, ctx->egress_ifindex, len);
+
+	return XDP_PASS;
+}
+
+char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/progs/test_xdp_with_devmap_helpers.c b/tools/testing/selftests/bpf/progs/test_xdp_with_devmap_helpers.c
new file mode 100644
index 000000000000..645f7f415857
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_xdp_with_devmap_helpers.c
@@ -0,0 +1,43 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+
+struct {
+	__uint(type, BPF_MAP_TYPE_DEVMAP);
+	__uint(key_size, sizeof(__u32));
+	__uint(value_size, sizeof(struct bpf_devmap_val));
+	__uint(max_entries, 4);
+} dm_ports SEC(".maps");
+
+SEC("xdp_redir")
+int xdp_redir_prog(struct xdp_md *ctx)
+{
+	return bpf_redirect_map(&dm_ports, 1, 0);
+}
+
+/* invalid program on DEVMAP entry;
+ * SEC name means expected attach type not set
+ */
+SEC("xdp_dummy")
+int xdp_dummy_prog(struct xdp_md *ctx)
+{
+	return XDP_PASS;
+}
+
+/* valid program on DEVMAP entry via SEC name;
+ * has access to egress and ingress ifindex
+ */
+SEC("xdp_devmap")
+int xdp_dummy_dm(struct xdp_md *ctx)
+{
+	char fmt[] = "devmap redirect: dev %u -> dev %u len %u\n";
+	void *data_end = (void *)(long)ctx->data_end;
+	void *data = (void *)(long)ctx->data;
+	unsigned int len = data_end - data;
+
+	bpf_trace_printk(fmt, sizeof(fmt),
+			 ctx->ingress_ifindex, ctx->egress_ifindex, len);
+
+	return XDP_PASS;
+}
-- 
2.21.1 (Apple Git-122.3)


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 bpf-next 1/5] devmap: Formalize map value as a named struct
  2020-05-29  5:20 ` [PATCH v3 bpf-next 1/5] devmap: Formalize map value as a named struct David Ahern
@ 2020-05-29  8:22   ` Jesper Dangaard Brouer
  2020-05-29 15:36     ` David Ahern
  0 siblings, 1 reply; 13+ messages in thread
From: Jesper Dangaard Brouer @ 2020-05-29  8:22 UTC (permalink / raw)
  To: David Ahern
  Cc: netdev, bpf, davem, kuba, toke, lorenzo, daniel, john.fastabend,
	ast, kafai, songliubraving, yhs, andriin, dsahern, brouer

On Thu, 28 May 2020 23:20:53 -0600
David Ahern <dsahern@kernel.org> wrote:

> Add 'struct bpf_devmap_val' to the bpf uapi to formalize the
> expected values that can be passed in for a DEVMAP.
> Update devmap code to use the struct.
> 
> Signed-off-by: David Ahern <dsahern@kernel.org>
> ---
>  include/uapi/linux/bpf.h       |  5 +++++
>  kernel/bpf/devmap.c            | 40 +++++++++++++++++++---------------
>  tools/include/uapi/linux/bpf.h |  5 +++++
>  3 files changed, 33 insertions(+), 17 deletions(-)
> 
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 54b93f8b49b8..42c884dfbad9 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -3625,6 +3625,11 @@ struct xdp_md {
>  	__u32 rx_queue_index;  /* rxq->queue_index  */
>  };
>  
> +/* DEVMAP values */
> +struct bpf_devmap_val {
> +	__u32 ifindex;   /* device index */
> +};

We do need this struct bpf_devmap_val, but I think it is wrong to make this UAPI.

A BPF-prog can get this via:  #include "vmlinux.h"

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 bpf-next 1/5] devmap: Formalize map value as a named struct
  2020-05-29  8:22   ` Jesper Dangaard Brouer
@ 2020-05-29 15:36     ` David Ahern
  2020-05-29 16:02       ` Jesper Dangaard Brouer
  0 siblings, 1 reply; 13+ messages in thread
From: David Ahern @ 2020-05-29 15:36 UTC (permalink / raw)
  To: Jesper Dangaard Brouer, David Ahern
  Cc: netdev, bpf, davem, kuba, toke, lorenzo, daniel, john.fastabend,
	ast, kafai, songliubraving, yhs, andriin

On 5/29/20 2:22 AM, Jesper Dangaard Brouer wrote:
> We do need this struct bpf_devmap_val, but I think it is wrong to make this UAPI.
> 
> A BPF-prog can get this via:  #include "vmlinux.h"

sure. I see that now.

I forgot to fold in a small update to the selftests, so I need to send a
v4 anyways. I will wait until later in the day in case there are other
comments.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 bpf-next 1/5] devmap: Formalize map value as a named struct
  2020-05-29 15:36     ` David Ahern
@ 2020-05-29 16:02       ` Jesper Dangaard Brouer
  0 siblings, 0 replies; 13+ messages in thread
From: Jesper Dangaard Brouer @ 2020-05-29 16:02 UTC (permalink / raw)
  To: David Ahern
  Cc: David Ahern, netdev, bpf, davem, kuba, toke, lorenzo, daniel,
	john.fastabend, ast, kafai, songliubraving, yhs, andriin, brouer

On Fri, 29 May 2020 09:36:14 -0600
David Ahern <dsahern@gmail.com> wrote:

> On 5/29/20 2:22 AM, Jesper Dangaard Brouer wrote:
> > We do need this struct bpf_devmap_val, but I think it is wrong to make this UAPI.
> > 
> > A BPF-prog can get this via:  #include "vmlinux.h"  
> 
> sure. I see that now.
> 
> I forgot to fold in a small update to the selftests, so I need to send a
> v4 anyways. I will wait until later in the day in case there are other
> comments.

I've just posted a patchset on top of this V3, that moves struct
bpf_devmap_val, and that demonstrate via code that I mean by
leveraging BTF for dynamic config API.

https://lore.kernel.org/netdev/159076794319.1387573.8722376887638960093.stgit@firesoul/

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 bpf-next 5/5] selftest: Add tests for XDP programs in devmap entries
  2020-05-29  5:20 ` [PATCH v3 bpf-next 5/5] selftest: Add tests for XDP programs in devmap entries David Ahern
@ 2020-05-29 16:45   ` Toke Høiland-Jørgensen
  2020-05-29 16:48     ` David Ahern
  0 siblings, 1 reply; 13+ messages in thread
From: Toke Høiland-Jørgensen @ 2020-05-29 16:45 UTC (permalink / raw)
  To: David Ahern, netdev
  Cc: bpf, davem, kuba, brouer, lorenzo, daniel, john.fastabend, ast,
	kafai, songliubraving, yhs, andriin, dsahern, David Ahern

David Ahern <dsahern@kernel.org> writes:

> Add tests to verify ability to add an XDP program to a
> entry in a DEVMAP.
>
> Add negative tests to show DEVMAP programs can not be
> attached to devices as a normal XDP program, and accesses
> to egress_ifindex require BPF_XDP_DEVMAP attach type.
>
> Signed-off-by: David Ahern <dsahern@kernel.org>
> ---
>  .../bpf/prog_tests/xdp_devmap_attach.c        | 89 +++++++++++++++++++
>  .../bpf/progs/test_xdp_devmap_helpers.c       | 22 +++++
>  .../bpf/progs/test_xdp_with_devmap_helpers.c  | 43 +++++++++
>  3 files changed, 154 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/xdp_devmap_attach.c
>  create mode 100644 tools/testing/selftests/bpf/progs/test_xdp_devmap_helpers.c
>  create mode 100644 tools/testing/selftests/bpf/progs/test_xdp_with_devmap_helpers.c
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_devmap_attach.c b/tools/testing/selftests/bpf/prog_tests/xdp_devmap_attach.c
> new file mode 100644
> index 000000000000..caeea19f2772
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/xdp_devmap_attach.c
> @@ -0,0 +1,89 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <uapi/linux/bpf.h>
> +#include <linux/if_link.h>
> +#include <test_progs.h>
> +
> +#include "test_xdp_devmap_helpers.skel.h"
> +#include "test_xdp_with_devmap_helpers.skel.h"
> +
> +#define IFINDEX_LO 1
> +
> +void test_xdp_with_devmap_helpers(void)
> +{
> +	struct test_xdp_with_devmap_helpers *skel;
> +	struct bpf_prog_info info = {};
> +	struct bpf_devmap_val val = {
> +		.ifindex = IFINDEX_LO,
> +	};
> +	__u32 id, len = sizeof(info);
> +	__u32 duration, idx = 0;
> +	int err, dm_fd, map_fd;
> +
> +
> +	skel = test_xdp_with_devmap_helpers__open_and_load();
> +	if (CHECK_FAIL(!skel)) {
> +		perror("test_xdp_with_devmap_helpers__open_and_load");
> +		return;
> +	}
> +
> +	/* can not attach program with DEVMAPs that allow programs
> +	 * as xdp generic
> +	 */
> +	dm_fd = bpf_program__fd(skel->progs.xdp_redir_prog);
> +	err = bpf_set_link_xdp_fd(IFINDEX_LO, dm_fd, XDP_FLAGS_SKB_MODE);
> +	CHECK(err == 0, "Generic attach of program with 8-byte devmap",
> +	      "should have failed\n");
> +
> +	dm_fd = bpf_program__fd(skel->progs.xdp_dummy_dm);
> +	map_fd = bpf_map__fd(skel->maps.dm_ports);
> +	err = bpf_obj_get_info_by_fd(dm_fd, &info, &len);
> +	if (CHECK_FAIL(err))
> +		goto out_close;
> +
> +	val.bpf_prog_fd = dm_fd;
> +	err = bpf_map_update_elem(map_fd, &idx, &val, 0);
> +	CHECK(err, "Add program to devmap entry",
> +	      "err %d errno %d\n", err, errno);
> +
> +	err = bpf_map_lookup_elem(map_fd, &id, &val);
> +	CHECK(err, "Read devmap entry", "err %d errno %d\n", err, errno);
> +	CHECK(info.id != val.bpf_prog_id, "Expected program id in devmap entry",
> +	      "expected %u read %u\n", info.id, val.bpf_prog_id);
> +
> +	/* can not attach BPF_XDP_DEVMAP program to a device */
> +	err = bpf_set_link_xdp_fd(IFINDEX_LO, dm_fd, XDP_FLAGS_SKB_MODE);
> +	CHECK(err == 0, "Attach of BPF_XDP_DEVMAP program",
> +	      "should have failed\n");
> +
> +	val.ifindex = 1;
> +	val.bpf_prog_fd = bpf_program__fd(skel->progs.xdp_dummy_prog);
> +	err = bpf_map_update_elem(map_fd, &idx, &val, 0);
> +	CHECK(err == 0, "Add non-BPF_XDP_DEVMAP program to devmap entry",
> +	      "should have failed\n");
> +
> +out_close:
> +	test_xdp_with_devmap_helpers__destroy(skel);
> +}
> +
> +void test_neg_xdp_devmap_helpers(void)
> +{
> +	struct test_xdp_devmap_helpers *skel;
> +	__u32 duration;
> +
> +	skel = test_xdp_devmap_helpers__open_and_load();
> +	if (CHECK(skel,
> +		  "Load of XDP program accessing egress ifindex without attach type",
> +		  "should have failed\n")) {
> +		test_xdp_devmap_helpers__destroy(skel);
> +	}
> +}
> +
> +
> +void test_xdp_devmap_attach(void)
> +{
> +	if (test__start_subtest("DEVMAP with programs in entries"))
> +		test_xdp_with_devmap_helpers();
> +
> +	if (test__start_subtest("Verifier check of DEVMAP programs"))
> +		test_neg_xdp_devmap_helpers();
> +}
> diff --git a/tools/testing/selftests/bpf/progs/test_xdp_devmap_helpers.c b/tools/testing/selftests/bpf/progs/test_xdp_devmap_helpers.c
> new file mode 100644
> index 000000000000..b360ba2bd441
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/test_xdp_devmap_helpers.c
> @@ -0,0 +1,22 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* fails to load without expected_attach_type = BPF_XDP_DEVMAP
> + * because of access to egress_ifindex
> + */
> +#include <linux/bpf.h>
> +#include <bpf/bpf_helpers.h>
> +
> +SEC("xdp_dm_log")

Guess this should be xdp_devmap_log now?

-Toke


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 bpf-next 0/5] bpf: Add support for XDP programs in DEVMAP entries
  2020-05-29  5:20 [PATCH v3 bpf-next 0/5] bpf: Add support for XDP programs in DEVMAP entries David Ahern
                   ` (4 preceding siblings ...)
  2020-05-29  5:20 ` [PATCH v3 bpf-next 5/5] selftest: Add tests for XDP programs in devmap entries David Ahern
@ 2020-05-29 16:46 ` Toke Høiland-Jørgensen
  5 siblings, 0 replies; 13+ messages in thread
From: Toke Høiland-Jørgensen @ 2020-05-29 16:46 UTC (permalink / raw)
  To: David Ahern, netdev
  Cc: bpf, davem, kuba, brouer, lorenzo, daniel, john.fastabend, ast,
	kafai, songliubraving, yhs, andriin, dsahern, David Ahern

David Ahern <dsahern@kernel.org> writes:

> Implementation of Daniel's proposal for allowing DEVMAP entries to be
> a device index, program fd pair.
>
> Programs are run after XDP_REDIRECT and have access to both Rx device
> and Tx device.

Found one more nit apart from the changes you already said you'd do for
v4. So with those fixes, feel free to add my:

Acked-by: Toke Høiland-Jørgensen <toke@redhat.com>


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 bpf-next 5/5] selftest: Add tests for XDP programs in devmap entries
  2020-05-29 16:45   ` Toke Høiland-Jørgensen
@ 2020-05-29 16:48     ` David Ahern
  2020-05-29 16:58       ` Toke Høiland-Jørgensen
  0 siblings, 1 reply; 13+ messages in thread
From: David Ahern @ 2020-05-29 16:48 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen, David Ahern, netdev
  Cc: bpf, davem, kuba, brouer, lorenzo, daniel, john.fastabend, ast,
	kafai, songliubraving, yhs, andriin

On 5/29/20 10:45 AM, Toke Høiland-Jørgensen wrote:
>> diff --git a/tools/testing/selftests/bpf/progs/test_xdp_devmap_helpers.c b/tools/testing/selftests/bpf/progs/test_xdp_devmap_helpers.c
>> new file mode 100644
>> index 000000000000..b360ba2bd441
>> --- /dev/null
>> +++ b/tools/testing/selftests/bpf/progs/test_xdp_devmap_helpers.c
>> @@ -0,0 +1,22 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/* fails to load without expected_attach_type = BPF_XDP_DEVMAP
>> + * because of access to egress_ifindex
>> + */
>> +#include <linux/bpf.h>
>> +#include <bpf/bpf_helpers.h>
>> +
>> +SEC("xdp_dm_log")
> Guess this should be xdp_devmap_log now?
> 
no. this program is for negative testing - it should load as an XDP
program without the expected_attach_type set. See the comment at the top
of the file.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 bpf-next 5/5] selftest: Add tests for XDP programs in devmap entries
  2020-05-29 16:48     ` David Ahern
@ 2020-05-29 16:58       ` Toke Høiland-Jørgensen
  0 siblings, 0 replies; 13+ messages in thread
From: Toke Høiland-Jørgensen @ 2020-05-29 16:58 UTC (permalink / raw)
  To: David Ahern, David Ahern, netdev
  Cc: bpf, davem, kuba, brouer, lorenzo, daniel, john.fastabend, ast,
	kafai, songliubraving, yhs, andriin

David Ahern <dsahern@gmail.com> writes:

> On 5/29/20 10:45 AM, Toke Høiland-Jørgensen wrote:
>>> diff --git a/tools/testing/selftests/bpf/progs/test_xdp_devmap_helpers.c b/tools/testing/selftests/bpf/progs/test_xdp_devmap_helpers.c
>>> new file mode 100644
>>> index 000000000000..b360ba2bd441
>>> --- /dev/null
>>> +++ b/tools/testing/selftests/bpf/progs/test_xdp_devmap_helpers.c
>>> @@ -0,0 +1,22 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/* fails to load without expected_attach_type = BPF_XDP_DEVMAP
>>> + * because of access to egress_ifindex
>>> + */
>>> +#include <linux/bpf.h>
>>> +#include <bpf/bpf_helpers.h>
>>> +
>>> +SEC("xdp_dm_log")
>> Guess this should be xdp_devmap_log now?
>> 
> no. this program is for negative testing - it should load as an XDP
> program without the expected_attach_type set. See the comment at the top
> of the file.

Ah, right, sorry - missed that (obviously) :)

-Toke


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2020-05-29 16:58 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-29  5:20 [PATCH v3 bpf-next 0/5] bpf: Add support for XDP programs in DEVMAP entries David Ahern
2020-05-29  5:20 ` [PATCH v3 bpf-next 1/5] devmap: Formalize map value as a named struct David Ahern
2020-05-29  8:22   ` Jesper Dangaard Brouer
2020-05-29 15:36     ` David Ahern
2020-05-29 16:02       ` Jesper Dangaard Brouer
2020-05-29  5:20 ` [PATCH v3 bpf-next 2/5] bpf: Add support to attach bpf program to a devmap entry David Ahern
2020-05-29  5:20 ` [PATCH v3 bpf-next 3/5] xdp: Add xdp_txq_info to xdp_buff David Ahern
2020-05-29  5:20 ` [PATCH v3 bpf-next 4/5] libbpf: Add SEC name for xdp programs attached to device map David Ahern
2020-05-29  5:20 ` [PATCH v3 bpf-next 5/5] selftest: Add tests for XDP programs in devmap entries David Ahern
2020-05-29 16:45   ` Toke Høiland-Jørgensen
2020-05-29 16:48     ` David Ahern
2020-05-29 16:58       ` Toke Høiland-Jørgensen
2020-05-29 16:46 ` [PATCH v3 bpf-next 0/5] bpf: Add support for XDP programs in DEVMAP entries Toke Høiland-Jørgensen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).