Re: [PATCH v2 bpf-next 5/5] selftest: Add tests for XDP programs in devmap entries

[David Ahern <dsahern@gmail.com>]

On 5/28/20 1:08 AM, Andrii Nakryiko wrote:
>> 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..d81b2b366f39
>> --- /dev/null
>> +++ b/tools/testing/selftests/bpf/prog_tests/xdp_devmap_attach.c
>> @@ -0,0 +1,94 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +#include <uapi/linux/bpf.h>
>> +#include <linux/if_link.h>
>> +#include <test_progs.h>
>> +
>> +#define IFINDEX_LO 1
>> +
>> +void test_xdp_devmap_attach(void)
>> +{
>> +       struct bpf_prog_load_attr attr = {
>> +               .prog_type = BPF_PROG_TYPE_XDP,
>> +       };
>> +       struct bpf_object *obj, *dm_obj = NULL;
>> +       int err, dm_fd = -1, fd = -1, map_fd;
>> +       struct bpf_prog_info info = {};
>> +       struct devmap_val val = {
>> +               .ifindex = IFINDEX_LO,
>> +       };
>> +       __u32 id, len = sizeof(info);
>> +       __u32 duration = 0, idx = 0;
>> +
>> +       attr.file = "./test_xdp_with_devmap.o",
>> +       err = bpf_prog_load_xattr(&attr, &obj, &fd);
> 
> please use skeletons instead of loading .o files.

I will look into it.

> 
>> +       if (CHECK(err, "load of xdp program with 8-byte devmap",
>> +                 "err %d errno %d\n", err, errno))
>> +               return;
>> +
> 
> [...]
> 

>> diff --git a/tools/testing/selftests/bpf/progs/test_xdp_devmap2.c b/tools/testing/selftests/bpf/progs/test_xdp_devmap2.c
>> new file mode 100644
>> index 000000000000..64fc2c3cae01
>> --- /dev/null
>> +++ b/tools/testing/selftests/bpf/progs/test_xdp_devmap2.c
>> @@ -0,0 +1,19 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/* program inserted into devmap entry */
>> +#include <linux/bpf.h>
>> +#include <bpf/bpf_helpers.h>
>> +
>> +SEC("xdp_devmap_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);
> 
> instead of just printing ifindexes, why not return them through global
> variable and validate in a test?

The point of this program to access the egress_ifindex with the expected
attached type NOT set to BPF_XDP_DEVMAP to FAIL the verifier checks.