From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-oa1-x2e.google.com (mail-oa1-x2e.google.com [IPv6:2001:4860:4864:20::2e]) by mail.toke.dk (Postfix) with ESMTPS id C20689C2D0E for ; Wed, 16 Nov 2022 21:51:36 +0100 (CET) Authentication-Results: mail.toke.dk; dkim=pass (2048-bit key; unprotected) header.d=google.com header.i=@google.com header.a=rsa-sha256 header.s=20210112 header.b=NVjWD1Lk Received: by mail-oa1-x2e.google.com with SMTP id 586e51a60fabf-13ae8117023so21546692fac.9 for ; Wed, 16 Nov 2022 12:51:36 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=lwhVPHIMIomo3BXcO1W7sLlexkx0CTxVPJcQuGHw/8o=; b=NVjWD1LkjwamwK9d6fbufkAMaomEFHxZY+bRcUFXRF6gFj7SQrv4GAlETRVzNmyfOw jtGMZTSDH5QC9pYPkSO1cqt/muNZS2KQj5IyMpJzsbd6TBS6yClc0nz4/MgoKCalmPEq KzMFNbd/JR1pM75UGX/ix3k0c5daCAOBPrYgZghkuhX8sGYh6GhHWp9tQWyYdn/2CJT6 pS+GcW1cM78kXdkUDmzphM+H4MRK/UphSzlVFYgXmFGI1B/IhLBknmpWIeyGrSNPPsC8 F5hlV7f/d2Nq8Q9PS3+QuLuCDACg5k39xQkRdnHdKF+VE61O6kTvgxPnW3bW9JQP4yzt VtlA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=lwhVPHIMIomo3BXcO1W7sLlexkx0CTxVPJcQuGHw/8o=; b=JyZNUUGDDPec2dHk6fESeYeWIM/6GVwBDwGW8MwiPXULa9rzAKJmw1dNW/Ihq1NiFB yD6lvGeyvVSfOTnm7YxvgqVyGNVRU9LY4n0Uo8LTq6IoPyZw96BTip0YxieyPXTgtomz 9EFCyYDF9W0cjWRjH+3/CASPws77IGhEJFR+l0OpZ9+ZPd8C346VBIVQZnjB2/CMnjow 9S9lh5d2h6MP8mwkT7NneEsMTD58CFIoG1xAhtzCyPVYow9YS9Sh19y/Qo9dTgx4JJ2I Fq3anvsUcXxhowCFbNpy4KROdK6roD22y7NZjWU/mWSRSiAMI/HMB2LN222ObJcvkdbr 30dw== X-Gm-Message-State: ANoB5pkD73nulQPKDU4v9T1KQ1BtmlX7wMpRyZq0WaBdViUFqRaJgkwb IDBks6Oms5L0xbYQN+A1lzHcdwRbnGqxYXJDXMo4XQ== X-Google-Smtp-Source: AA0mqf6AnWu6ZOpWwAlz4LRUfKA5FZ2GMyKfkbaeivMwdlj7/dT9WtHZbPViRrSGVJ0NtmfnjH4iq1gH6O5FuKTYrYo= X-Received: by 2002:a05:6870:e9a2:b0:13b:be90:a68a with SMTP id r34-20020a056870e9a200b0013bbe90a68amr2708824oao.181.1668631894779; Wed, 16 Nov 2022 12:51:34 -0800 (PST) MIME-Version: 1.0 References: <20221115030210.3159213-1-sdf@google.com> <20221115030210.3159213-7-sdf@google.com> In-Reply-To: From: Stanislav Fomichev Date: Wed, 16 Nov 2022 12:51:23 -0800 Message-ID: To: Martin KaFai Lau Content-Type: text/plain; charset="UTF-8" Message-ID-Hash: RXBGMCKNFFBZRTKX455J27S7IZBDWHUD X-Message-ID-Hash: RXBGMCKNFFBZRTKX455J27S7IZBDWHUD X-MailFrom: sdf@google.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org, song@kernel.org, yhs@fb.com, john.fastabend@gmail.com, kpsingh@kernel.org, haoluo@google.com, jolsa@kernel.org, David Ahern , Jakub Kicinski , Willem de Bruijn , Jesper Dangaard Brouer , Anatoly Burakov , Alexander Lobakin , Magnus Karlsson , Maryam Tahhan , xdp-hints@xdp-project.net, netdev@vger.kernel.org, bpf@vger.kernel.org X-Mailman-Version: 3.3.6 Precedence: list Subject: [xdp-hints] Re: [PATCH bpf-next 06/11] xdp: Carry over xdp metadata into skb context List-Id: XDP hardware hints design discussion Archived-At: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Tue, Nov 15, 2022 at 11:04 PM Martin KaFai Lau wrote: > > On 11/14/22 7:02 PM, Stanislav Fomichev wrote: > > Implement new bpf_xdp_metadata_export_to_skb kfunc which > > prepares compatible xdp metadata for kernel consumption. > > This kfunc should be called prior to bpf_redirect > > or when XDP_PASS'ing the frame into the kernel (note, the drivers > > have to be updated to enable consuming XDP_PASS'ed metadata). > > > > veth driver is amended to consume this metadata when converting to skb. > > > > Internally, XDP_FLAGS_HAS_SKB_METADATA flag is used to indicate > > whether the frame has skb metadata. The metadata is currently > > stored prior to xdp->data_meta. bpf_xdp_adjust_meta refuses > > to work after a call to bpf_xdp_metadata_export_to_skb (can lift > > this requirement later on if needed, we'd have to memmove > > xdp_skb_metadata). > > It is ok to refuse bpf_xdp_adjust_meta() after bpf_xdp_metadata_export_to_skb() > for now. However, it will also need to refuse bpf_xdp_adjust_head(). Good catch! > [ ... ] > > > +/* For the packets directed to the kernel, this kfunc exports XDP metadata > > + * into skb context. > > + */ > > +noinline int bpf_xdp_metadata_export_to_skb(const struct xdp_md *ctx) > > +{ > > + return 0; > > +} > > + > > I think it is still better to return 'struct xdp_skb_metata *' instead of > true/false. Like: > > noinline struct xdp_skb_metata *bpf_xdp_metadata_export_to_skb(const struct > xdp_md *ctx) > { > return 0; > } > > The KF_RET_NULL has already been set in > BTF_SET8_START_GLOBAL(xdp_metadata_kfunc_ids). There is > "xdp_btf_struct_access()" that can allow write access to 'struct xdp_skb_metata' > What else is missing? We can try to solve it. Ah, that KF_RET_NULL is a left-over from my previous attempt to return pointers :-) I can retry with returning a pointer, I don't have a preference, but I felt like returning simple -errno might be more simple api-wise. (with bpf_xdp_metadata_export_to_skb returning NULL vs return loggable errno - I'd prefer the latter, but not strongly) > Then there is no need for this double check in patch 8 selftest which is not > easy to use: > > + if (bpf_xdp_metadata_export_to_skb(ctx) < 0) { > + bpf_printk("bpf_xdp_metadata_export_to_skb failed"); > + return XDP_DROP; > + } > > [ ... ] > > + skb_metadata = ctx->skb_metadata; > + if (!skb_metadata) { > + bpf_printk("no ctx->skb_metadata"); > + return XDP_DROP; > + } > > [ ... ] > > > > diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h > > index b444b1118c4f..71e3bc7ad839 100644 > > --- a/tools/include/uapi/linux/bpf.h > > +++ b/tools/include/uapi/linux/bpf.h > > @@ -6116,6 +6116,12 @@ enum xdp_action { > > XDP_REDIRECT, > > }; > > > > +/* Subset of XDP metadata exported to skb context. > > + */ > > +struct xdp_skb_metadata { > > + __u64 rx_timestamp; > > +}; > > + > > /* user accessible metadata for XDP packet hook > > * new fields must be added to the end of this structure > > */ > > @@ -6128,6 +6134,7 @@ struct xdp_md { > > __u32 rx_queue_index; /* rxq->queue_index */ > > > > __u32 egress_ifindex; /* txq->dev->ifindex */ > > + __bpf_md_ptr(struct xdp_skb_metadata *, skb_metadata); > > Once the above bpf_xdp_metadata_export_to_skb() returning a pointer works, then > it can be another kfunc 'struct xdp_skb_metata * bpf_xdp_get_skb_metadata(const > struct xdp_md *ctx)' to return the skb_metadata which was a similar point > discussed in the previous RFC. I see. I think you've mentioned it previously somewhere to have a kfunc accessor vs uapi field and I totally forgot. Will try to address it, ty!