bpf: Add support for XDP programs in DEVMAP entries
Subsystems (9)
v4 cover bpf: Add support for XDP programs in DEVMAP entries
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.
v4
- moved struct bpf_devmap_val from uapi to devmap.c, named the union and dropped the prefix from the elements - Jesper
- fixed 2 bugs in selftests
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 | 3 + kernel/bpf/devmap.c | 130 +++++++++++++++--- net/core/dev.c | 18 +++ net/core/filter.c | 17 +++ tools/include/uapi/linux/bpf.h | 3 + tools/lib/bpf/libbpf.c | 2 + .../bpf/prog_tests/xdp_devmap_attach.c | 97 +++++++++++++ .../bpf/progs/test_xdp_devmap_helpers.c | 22 +++ .../bpf/progs/test_xdp_with_devmap_helpers.c | 44 ++++++ 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
Signature · 1 line
2.21.1 (Apple Git-122.3)
PATCH v4 1/5 devmap: Formalize map value as a named struct
Add 'struct bpf_devmap_val' to formalize the expected values that can be passed in for a DEVMAP. Update devmap code to use the struct.
--- kernel/bpf/devmap.c | 45 ++++++++++++++++++++++++++++----------------- 1 file changed, 28 insertions(+), 17 deletions(-)
kernel/bpf/devmap.c +28 -17
@@ -60,12 +60,18 @@ struct xdp_dev_bulk_queue {unsigned int count; }; +/* DEVMAP values */ +struct bpf_devmap_val { +u32 ifindex;/* device index */ +}; +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 rcu_head rcu;unsigned int idx; +struct bpf_devmap_val val; };struct bpf_dtab {
@@ -472,18 +478,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 +544,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 +554,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 +573,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 +583,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 +617,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 +634,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;
Signature · 1 line
2.21.1 (Apple Git-122.3)
PATCH v4 4/5 libbpf: Add SEC name for xdp programs attached to device map
In reply to David AhernSupport SEC("xdp_devmap*") as a short cut for loading the program with type BPF_PROG_TYPE_XDP and expected attach type BPF_XDP_DEVMAP.
--- tools/lib/bpf/libbpf.c | 2 ++ 1 file changed, 2 insertions(+)
tools/lib/bpf/libbpf.c +2 -0
@@ -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),
Signature · 1 line
2.21.1 (Apple Git-122.3)
PATCH v4 3/5 xdp: Add xdp_txq_info to xdp_buff
In reply to David AhernAdd 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.
--- 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(+)
include/net/xdp.h +5 -0
@@ -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*/ };
include/uapi/linux/bpf.h +2 -0
@@ -3706,6 +3706,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 */ };enum sk_action {
kernel/bpf/devmap.c +3 -0
@@ -476,8 +476,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 :
net/core/filter.c +17 -0
@@ -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;
tools/include/uapi/linux/bpf.h +2 -0
@@ -3706,6 +3706,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 */ };enum sk_action {
Signature · 1 line
2.21.1 (Apple Git-122.3)
PATCH v4 2/5 bpf: Add support to attach bpf program to a devmap entry
In reply to David AhernAdd 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. bpf programs can get access to the struct via vmlinux.h.
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
Quoted 1 line
4-byte values.
Block attach of BPF_XDP_DEVMAP programs to devices.
--- include/linux/bpf.h | 5 ++ include/uapi/linux/bpf.h | 1 + kernel/bpf/devmap.c | 88 ++++++++++++++++++++++++++++++++-- net/core/dev.c | 18 +++++++ tools/include/uapi/linux/bpf.h | 1 + 5 files changed, 109 insertions(+), 4 deletions(-)
include/linux/bpf.h +5 -0
@@ -1250,6 +1250,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 );
@@ -1363,6 +1364,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 ) {
include/uapi/linux/bpf.h +1 -0
@@ -225,6 +225,7 @@ enum bpf_attach_type {BPF_CGROUP_INET6_GETPEERNAME ,BPF_CGROUP_INET4_GETSOCKNAME ,BPF_CGROUP_INET6_GETSOCKNAME , +BPF_XDP_DEVMAP , __MAX_BPF_ATTACH_TYPE };
kernel/bpf/devmap.c +84 -4
183 lines not on this page · raw message
net/core/dev.c +18 -0
@@ -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);
tools/include/uapi/linux/bpf.h +1 -0
@@ -225,6 +225,7 @@ enum bpf_attach_type {BPF_CGROUP_INET6_GETPEERNAME ,BPF_CGROUP_INET4_GETSOCKNAME ,BPF_CGROUP_INET6_GETSOCKNAME , +BPF_XDP_DEVMAP , __MAX_BPF_ATTACH_TYPE };
Signature · 1 line
2.21.1 (Apple Git-122.3)
PATCH v4 5/5 selftest: Add tests for XDP programs in devmap entries
In reply to David AhernAdd 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.
--- .../bpf/prog_tests/xdp_devmap_attach.c | 97 +++++++++++++++++++ .../bpf/progs/test_xdp_devmap_helpers.c | 22 +++++ .../bpf/progs/test_xdp_with_devmap_helpers.c | 44 +++++++++ 3 files changed, 163 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
tools/testing/selftests/bpf/prog_tests/xdp_devmap_attach.c new file +97 -0
@@ -0,0 +1,97 @@ +// 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 + +struct bpf_devmap_val { +u32 ifindex;/* device index */ +union { +int fd;/* prog fd on map write */ +u32 id;/* prog id on map read */ + } bpf_prog; +}; + +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 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, &idx, &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 (); +}
tools/testing/selftests/bpf/progs/test_xdp_devmap_helpers.c new file +22 -0
@@ -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 "vmlinux.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";
tools/testing/selftests/bpf/progs/test_xdp_with_devmap_helpers.c new file +44 -0
44 lines not on this page · raw message
Signature · 1 line
2.21.1 (Apple Git-122.3)
Alexei Starovoitov
AppliedQuoted 11 lines · David Ahern
Implementation of Daniel's proposal for allowing DEVMAP entries to be a device index, program fd pair.
Programs are run after
XDP_REDIRECTand have access to both Rx device and Tx device.v4
- moved structbpf_devmap_valfrom uapi todevmap.c, named the union and dropped the prefix from the elements - Jesper
- fixed 2 bugs in selftests
Applied.
In patch 5 I had to fix:
/data/users/ast/net-next/tools/testing/selftests/bpf/prog_tests/xdp_devmap_attach.c:
In function ‘test_neg_xdp_devmap_helpers’:
/data/users/ast/net-next/tools/testing/selftests/bpf/test_progs.h:106:3:
warning: ‘duration’ may be used uninitialized in this function [-Wmaybe-uninitialized]
106 | fprintf(stdout, "%s:PASS:%s %d nsec\n", \
| ^~~~~~~/data/users/ast/net-next/tools/testing/selftests/bpf/prog_tests/xdp_devmap_attach.c:79:8:
note: ‘duration’ was declared here
79 | __u32 duration;
and that selftest is imo too primitive.
It's only loading progs and not executing them.
Could you please add prog_test_run to it?
Quoted 11 lines · Alexei Starovoitov
In patch 5 I had to fix:
/data/users/ast/net-next/tools/testing/selftests/bpf/prog_tests/xdp_devmap_attach.c:
In function ‘test_neg_xdp_devmap_helpers’:
/data/users/ast/net-next/tools/testing/selftests/bpf/test_progs.h:106:3:
warning: ‘duration’ may be used uninitialized in this function [-Wmaybe-uninitialized]106 | fprintf(stdout, "%s:PASS:%s %d nsec\n", \ | ^~~~~~~/data/users/ast/net-next/tools/testing/selftests/bpf/prog_tests/xdp_devmap_attach.c:79:8:
note: ‘duration’ was declared here
79 |__u32duration;
What compiler version? it compiles cleanly with ubuntu 20.04 and gcc 9.3. The other prog_tests are inconsistent with initializing it.
Quoted 2 lines
and that selftest is imo too primitive.
I focused the selftests on API changes introduced by this set - new attach type, valid accesses to egress_ifindex and not allowing devmap programs with xdp generic.
Quoted 3 lines
It's only loading progs and not executing them.
Could you please addprog_test_runto it?
I will look into it.
Quoted 13 lines · David Ahern
Quoted 11 lines · Alexei Starovoitov
In patch 5 I had to fix:
/data/users/ast/net-next/tools/testing/selftests/bpf/prog_tests/xdp_devmap_attach.c:
In function ‘test_neg_xdp_devmap_helpers’:
/data/users/ast/net-next/tools/testing/selftests/bpf/test_progs.h:106:3:
warning: ‘duration’ may be used uninitialized in this function [-Wmaybe-uninitialized]106 | fprintf(stdout, "%s:PASS:%s %d nsec\n", \ | ^~~~~~~/data/users/ast/net-next/tools/testing/selftests/bpf/prog_tests/xdp_devmap_attach.c:79:8:
note: ‘duration’ was declared here
79 |__u32duration;
Quoted 2 lines
What compiler version? it compiles cleanly with ubuntu 20.04 and gcc 9.3. The other
prog_testsare inconsistent with initializing it.
official rhel's devtoolset-9
gcc version 9.1.1 20190605 (Red Hat 9.1.1-2) (GCC)
Quoted 7 lines
Quoted 2 lines
and that selftest is imo too primitive.
I focused the selftests on API changes introduced by this set - new attach type, valid accesses to
egress_ifindexand not allowing devmap programs with xdp generic.
Quoted 5 lines
Quoted 3 lines
It's only loading progs and not executing them.
Could you please addprog_test_runto it?I will look into it.
Thanks!
Quoted 14 lines · David Ahern
Quoted 11 lines · Alexei Starovoitov
In patch 5 I had to fix:
/data/users/ast/net-next/tools/testing/selftests/bpf/prog_tests/xdp_devmap_attach.c:
In function ‘test_neg_xdp_devmap_helpers’:
/data/users/ast/net-next/tools/testing/selftests/bpf/test_progs.h:106:3:
warning: ‘duration’ may be used uninitialized in this function [-Wmaybe-uninitialized]106 | fprintf(stdout, "%s:PASS:%s %d nsec\n", \ | ^~~~~~~/data/users/ast/net-next/tools/testing/selftests/bpf/prog_tests/xdp_devmap_attach.c:79:8:
note: ‘duration’ was declared here
79 |__u32duration;
Quoted 2 lines
What compiler version? it compiles cleanly with ubuntu 20.04 and gcc 9.3. The other
prog_testsare inconsistent with initializing it.
Do you have specific examples of inconsistencies? Seems like duration is:
1. either static variable, and thus zero-initialized;
2. is initialized explicitly at declaration;
3. is filled out with bpf_prog_test_run().
Quoted 13 lines
Quoted 3 lines
and that selftest is imo too primitive.
I focused the selftests on API changes introduced by this set - new attach type, valid accesses to
egress_ifindexand not allowing devmap programs with xdp generic.Quoted 3 lines
It's only loading progs and not executing them.
Could you please addprog_test_runto it?I will look into it.
Quoted 1 line · Andrii Nakryiko
Do you have specific examples of inconsistencies? Seems like duration is:
nope, just a quick grep trying to understand why it compiled cleanly for me and looking at similar tests.
Quoted 4 lines
1. either static variable, and thus zero-initialized;
2. is initialized explicitly at declaration;
3. is filled out withbpf_prog_test_run().
apparently so.
Quoted 7 lines · David Ahern
Quoted 2 lines
and that selftest is imo too primitive.
I focused the selftests on API changes introduced by this set - new attach type, valid accesses to
egress_ifindexand not allowing devmap programs with xdp generic.
Quoted 6 lines
Quoted 3 lines
It's only loading progs and not executing them.
Could you please addprog_test_runto it?I will look into it.
The test_run infrastructure does not handle XDP_REDIRECT which is needed to step into the devmap code and then run programs tied to devmap entries.