Re: Trying the bpf trace a bpf xdp program

["Eelco Chaudron" <echaudro@xxxxxxxxxx>]

On 5 Dec 2019, at 18:35, Y Song wrote:

> On Thu, Dec 5, 2019 at 4:41 AM Eelco Chaudron <echaudro@xxxxxxxxxx> 
> wrote:

> It is a little tricky. The below change can make verifier happy. I did
> not test it so not sure whether produces correct result or not.
>
> ==========
> struct xdp_rxq_info {
>         __u32 queue_index;
> } __attribute__((preserve_access_index));
>
> struct xdp_buff {
>         struct xdp_rxq_info *rxq;
> } __attribute__((preserve_access_index));
>
> BPF_TRACE_2("fexit/xdp_prog_simple", trace_on_exit,
>             struct xdp_buff *, ctx, int, ret)
> {
>    __u32 rx_queue;
>
>    rx_queue = ctx->rxq->queue_index;
>    bpf_debug("fexit: queue = %u, ret = %d\n", rx_queue, ret);
>
>    return 0;
> }
> ==========
>
> In the above, I am using newly added clang attribute 
> "__preserve_access_index"
> (in llvm-project trunk since Nov. 13) to make code
> a little bit cleaner. The old way as in selftests fexit_bpf2bpf.c
> should work too.
>
> Basically, the argument for fexit function should be types actually
> passing to the jited function.
> For user visible 'xdp_md`. the jited function will receive `xdp_buff`.
> The access for each field
> sometimes is not one-to-one mapping. You need to check kernel code to
> find the correct
> way. We probably should make this part better to improve user 
> experience.

Thanks the hint that it should be the jitted arguments solved it… And 
you quick example worked, just in case some one else is playing with it, 
here is my working example:

// SPDX-License-Identifier: GPL-2.0
#include <linux/bpf.h>
#include "bpf_helpers.h"
#include "bpf_trace_helpers.h"

#define bpf_debug(fmt, ...)                \
{                                          \
    char __fmt[] = fmt;                    \
    bpf_trace_printk(__fmt, sizeof(__fmt), \
                     ##__VA_ARGS__);       \
}

struct net_device {
    /* Structure does not need to contain all entries,
     * as "preserve_access_index" will use BTF to fix this... */
    int                    ifindex;
} __attribute__((preserve_access_index));

struct xdp_rxq_info {
    /* Structure does not need to contain all entries,
     * as "preserve_access_index" will use BTF to fix this... */
    struct net_device *dev;
    __u32 queue_index;
} __attribute__((preserve_access_index));

struct xdp_buff {
    void *data;
    void *data_end;
    void *data_meta;
    void *data_hard_start;
    unsigned long handle;
    struct xdp_rxq_info *rxq;
} __attribute__((preserve_access_index));


BPF_TRACE_1("fentry/xdp_prog_simple", trace_on_entry,
            struct xdp_buff *, xdp)
{
    bpf_debug("fentry: [ifindex = %u, queue =  %u]\n",
              xdp->rxq->dev->ifindex, xdp->rxq->queue_index);
    return 0;
}


BPF_TRACE_2("fexit/xdp_prog_simple", trace_on_exit,
            struct xdp_buff*, xdp, int, ret)
{
    bpf_debug("fexit: [ifindex = %u, queue =  %u, ret = %d]\n",
              xdp->rxq->dev->ifindex, xdp->rxq->queue_index, ret);

    return 0;
}

char _license[] SEC("license") = "GPL";