Subject: Re: [xdp-hints] [PATCH bpf-next 05/11] veth: Support rx timestamp metadata for xdp

From: Toke Høiland-Jørgensen <[email-masked]>
Date: Tue, 15 Nov 2022 17:17:42 +0100

Stanislav Fomichev <[email-masked]> writes:

> The goal is to enable end-to-end testing of the metadata
> for AF_XDP. Current rx_timestamp kfunc returns current
> time which should be enough to exercise this new functionality.
>
> Cc: John Fastabend <[email-masked]>
> Cc: David Ahern <[email-masked]>
> Cc: Martin KaFai Lau <[email-masked]>
> Cc: Jakub Kicinski <[email-masked]>
> Cc: Willem de Bruijn <[email-masked]>
> Cc: Jesper Dangaard Brouer <[email-masked]>
> Cc: Anatoly Burakov <[email-masked]>
> Cc: Alexander Lobakin <[email-masked]>
> Cc: Magnus Karlsson <[email-masked]>
> Cc: Maryam Tahhan <[email-masked]>
> Cc: [email-masked]
> Cc: [email-masked]
> Signed-off-by: Stanislav Fomichev <[email-masked]>
> ---
>  drivers/net/veth.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/drivers/net/veth.c b/drivers/net/veth.c
> index 2a4592780141..c626580a2294 100644
> --- a/drivers/net/veth.c
> +++ b/drivers/net/veth.c
> @@ -25,6 +25,7 @@
>  #include <linux/filter.h>
>  #include <linux/ptr_ring.h>
>  #include <linux/bpf_trace.h>
> +#include <linux/bpf_patch.h>
>  #include <linux/net_tstamp.h>
>  
>  #define DRV_NAME	"veth"
> @@ -1659,6 +1660,18 @@ static int veth_xdp(struct net_device *dev, struct netdev_bpf *xdp)
>  	}
>  }
>  
> +static void veth_unroll_kfunc(const struct bpf_prog *prog, u32 func_id,
> +			      struct bpf_patch *patch)
> +{
> +	if (func_id == xdp_metadata_kfunc_id(XDP_METADATA_KFUNC_RX_TIMESTAMP_SUPPORTED)) {
> +		/* return true; */
> +		bpf_patch_append(patch, BPF_MOV64_IMM(BPF_REG_0, 1));
> +	} else if (func_id == xdp_metadata_kfunc_id(XDP_METADATA_KFUNC_RX_TIMESTAMP)) {
> +		/* return ktime_get_mono_fast_ns(); */
> +		bpf_patch_append(patch, BPF_EMIT_CALL(ktime_get_mono_fast_ns));
> +	}
> +}

So these look reasonable enough, but would be good to see some examples
of kfunc implementations that don't just BPF_CALL to a kernel function
(with those helper wrappers we were discussing before).

-Toke