On Mar 05, arthur@xxxxxxxxxxxxxxx wrote:
> From: Arthur Fabre <afabre@xxxxxxxxxxxxxx>
>
[...]
> +static __always_inline void *xdp_buff_traits(const struct xdp_buff *xdp)
> +{
> + return xdp->data_hard_start + _XDP_FRAME_SIZE;
> +}
> +
> static __always_inline void
> xdp_init_buff(struct xdp_buff *xdp, u32 frame_sz, struct xdp_rxq_info *rxq)
> {
> @@ -133,6 +139,13 @@ xdp_prepare_buff(struct xdp_buff *xdp, unsigned char *hard_start,
> xdp->data = data;
> xdp->data_end = data + data_len;
> xdp->data_meta = meta_valid ? data : data + 1;
> +
> + if (meta_valid) {
can we relax this constraint and use xdp->data as end boundary here?
> + /* We assume drivers reserve enough headroom to store xdp_frame
> + * and the traits header.
> + */
> + traits_init(xdp_buff_traits(xdp), xdp->data_meta);
> + }
> }
>
> /* Reserve memory area at end-of data area.
> @@ -267,6 +280,8 @@ struct xdp_frame {
> u32 flags; /* supported values defined in xdp_buff_flags */
> };
>
> +static_assert(sizeof(struct xdp_frame) == _XDP_FRAME_SIZE);
> +
> static __always_inline bool xdp_frame_has_frags(const struct xdp_frame *frame)
> {
> return !!(frame->flags & XDP_FLAGS_HAS_FRAGS);
> @@ -517,6 +532,11 @@ static inline bool xdp_metalen_invalid(unsigned long metalen)
> return !IS_ALIGNED(metalen, sizeof(u32)) || metalen > meta_max;
> }
>
> +static __always_inline void *xdp_meta_hard_start(const struct xdp_buff *xdp)
> +{
> + return xdp_buff_traits(xdp) + traits_size(xdp_buff_traits(xdp));
here we are always consuming sizeof(struct __trait_hdr)), right? We can do
somehing smarter and check if traits are really used? (e.g. adding in the flags
in xdp_buff)?
> +}
> +
> struct xdp_attachment_info {
> struct bpf_prog *prog;
> u32 flags;
> diff --git a/net/core/filter.c b/net/core/filter.c
> index dcc53ac5c5458f67a422453134665d43d466a02e..79b78e7cd57fd78c6cc8443da54ae96408c496b0 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -85,6 +85,7 @@
> #include <linux/un.h>
> #include <net/xdp_sock_drv.h>
> #include <net/inet_dscp.h>
> +#include <net/trait.h>
>
> #include "dev.h"
>
> @@ -3935,9 +3936,8 @@ static unsigned long xdp_get_metalen(const struct xdp_buff *xdp)
>
> BPF_CALL_2(bpf_xdp_adjust_head, struct xdp_buff *, xdp, int, offset)
> {
> - void *xdp_frame_end = xdp->data_hard_start + sizeof(struct xdp_frame);
> unsigned long metalen = xdp_get_metalen(xdp);
> - void *data_start = xdp_frame_end + metalen;
> + void *data_start = xdp_meta_hard_start(xdp) + metalen;
We could waste 16byte here, right?
Regards,
Lorenzo
> void *data = xdp->data + offset;
>
> if (unlikely(data < data_start ||
> @@ -4228,13 +4228,12 @@ static const struct bpf_func_proto bpf_xdp_adjust_tail_proto = {
>
> BPF_CALL_2(bpf_xdp_adjust_meta, struct xdp_buff *, xdp, int, offset)
> {
> - void *xdp_frame_end = xdp->data_hard_start + sizeof(struct xdp_frame);
> void *meta = xdp->data_meta + offset;
> unsigned long metalen = xdp->data - meta;
>
> if (xdp_data_meta_unsupported(xdp))
> return -ENOTSUPP;
> - if (unlikely(meta < xdp_frame_end ||
> + if (unlikely(meta < xdp_meta_hard_start(xdp) ||
> meta > xdp->data))
> return -EINVAL;
> if (unlikely(xdp_metalen_invalid(metalen)))
> diff --git a/net/core/xdp.c b/net/core/xdp.c
> index 2c6ab6fb452f7b90d85125ae17fef96cfc9a8576..2e87f82aa5f835f60295d859a524e40bd47c42ee 100644
> --- a/net/core/xdp.c
> +++ b/net/core/xdp.c
> @@ -1032,3 +1032,53 @@ void xdp_features_clear_redirect_target(struct net_device *dev)
> xdp_set_features_flag(dev, val);
> }
> EXPORT_SYMBOL_GPL(xdp_features_clear_redirect_target);
> +
> +__bpf_kfunc_start_defs();
> +
> +__bpf_kfunc int bpf_xdp_trait_set(const struct xdp_buff *xdp, u64 key,
> + const void *val, u64 val__sz, u64 flags)
> +{
> + if (xdp_data_meta_unsupported(xdp))
> + return -EOPNOTSUPP;
> +
> + return trait_set(xdp_buff_traits(xdp), xdp->data_meta, key,
> + val, val__sz, flags);
> +}
> +
> +__bpf_kfunc int bpf_xdp_trait_get(const struct xdp_buff *xdp, u64 key,
> + void *val, u64 val__sz)
> +{
> + if (xdp_data_meta_unsupported(xdp))
> + return -EOPNOTSUPP;
> +
> + return trait_get(xdp_buff_traits(xdp), key, val, val__sz);
> +}
> +
> +__bpf_kfunc int bpf_xdp_trait_del(const struct xdp_buff *xdp, u64 key)
> +{
> + if (xdp_data_meta_unsupported(xdp))
> + return -EOPNOTSUPP;
> +
> + return trait_del(xdp_buff_traits(xdp), key);
> +}
> +
> +__bpf_kfunc_end_defs();
> +
> +BTF_KFUNCS_START(xdp_trait)
> +// TODO - should we use KF_TRUSTED_ARGS? https://www.kernel.org/doc/html/next/bpf/kfuncs.html#kf-trusted-args-flag
> +BTF_ID_FLAGS(func, bpf_xdp_trait_set)
> +BTF_ID_FLAGS(func, bpf_xdp_trait_get)
> +BTF_ID_FLAGS(func, bpf_xdp_trait_del)
> +BTF_KFUNCS_END(xdp_trait)
> +
> +static const struct btf_kfunc_id_set xdp_trait_kfunc_set = {
> + .owner = THIS_MODULE,
> + .set = &xdp_trait,
> +};
> +
> +static int xdp_trait_init(void)
> +{
> + return register_btf_kfunc_id_set(BPF_PROG_TYPE_XDP,
> + &xdp_trait_kfunc_set);
> +}
> +late_initcall(xdp_trait_init);
>
> --
> 2.43.0
>
>
Attachment:
signature.asc
Description: PGP signature
