From: Jesper Dangaard Brouer <jbrouer@redhat.com>
To: "Kishen Maloor" <kishen.maloor@intel.com>,
bpf@vger.kernel.org, netdev@vger.kernel.org, hawk@kernel.org,
magnus.karlsson@intel.com, "Björn Töpel" <bjorn@kernel.org>,
"Andrii Nakryiko" <andrii@kernel.org>,
"Jithu Joseph" <jithu.joseph@intel.com>
Cc: brouer@redhat.com
Subject: Re: [RFC bpf-next 2/5] libbpf: SO_TXTIME support in AF_XDP
Date: Wed, 18 Aug 2021 11:49:27 +0200 [thread overview]
Message-ID: <31fb6a84-562e-a41d-0614-061e1f475db3@redhat.com> (raw)
In-Reply-To: <20210803171006.13915-3-kishen.maloor@intel.com>
On 03/08/2021 19.10, Kishen Maloor wrote:
> This change adds userspace support for SO_TXTIME in AF_XDP
> to include a specific TXTIME (aka "Launch Time")
> with XDP frames issued from userspace XDP applications.
>
> The userspace API has been expanded with two helper functons:
>
> - int xsk_socket__enable_so_txtime(struct xsk_socket *xsk, bool enable)
> Sets the SO_TXTIME option on the AF_XDP socket (using setsockopt()).
>
> - void xsk_umem__set_md_txtime(void *umem_area, __u64 chunkAddr,
> __s64 txtime)
> Packages the application supplied TXTIME into struct xdp_user_tx_metadata:
> struct xdp_user_tx_metadata {
Struct name is important and becomes UAPI. I'm not 100% convinced this
is a good name.
For BPF programs libbpf can at load-time lookup the 'btf_id' via:
btf_id = bpf_core_type_id_kernel(struct xdp_user_tx_metadata);
Example see[1]
[1] https://github.com/xdp-project/bpf-examples/commit/2390b4b11079
I know this is AF_XDP userspace, but I hope Andrii can help guide us
howto expose the bpf_core_type_id_kernel() API via libbpf, to be used by
the AF_XDP userspace program.
> __u64 timestamp;
> __u32 md_valid;
> __u32 btf_id;
> };
I assume this struct is intended to be BTF "described".
Struct member *names* are very important for BTF. (E.g. see how
'spinlock' have special meaning and is matched internally by kernel).
The member name 'timestamp' seems too generic. This is a very specific
'LaunchTime' feature, which could be reflected in the name.
Later it looks like you are encoding the "type" in md_valid, which I
guess it is needed as timestamps can have different "types".
E.g. some of the clockid_t types from clock_gettime(2):
CLOCK_REALTIME
CLOCK_TAI
CLOCK_MONOTONIC
CLOCK_BOOTTIME
Which of these timestamp does XDP_METADATA_USER_TX_TIMESTAMP represent?
Or what timestamp type is the expected one?
In principle we could name the member 'Launch_Time_CLOCK_TAI' to encoded
the clockid_t type in the name, but I think that would be too much (and
require too advanced BTF helpers to extract type, having a clock_type
member is easier to understand/consume from C).
> and stores it in the XDP metadata area, which precedes the XDP frame.
>
> Signed-off-by: Kishen Maloor <kishen.maloor@intel.com>
> ---
> tools/include/uapi/linux/if_xdp.h | 2 ++
> tools/include/uapi/linux/xdp_md_std.h | 14 ++++++++++++++
> tools/lib/bpf/xsk.h | 27 ++++++++++++++++++++++++++-
> 3 files changed, 42 insertions(+), 1 deletion(-)
> create mode 100644 tools/include/uapi/linux/xdp_md_std.h
>
> diff --git a/tools/include/uapi/linux/if_xdp.h b/tools/include/uapi/linux/if_xdp.h
> index a78a8096f4ce..31f81f82ed86 100644
> --- a/tools/include/uapi/linux/if_xdp.h
> +++ b/tools/include/uapi/linux/if_xdp.h
> @@ -106,6 +106,8 @@ struct xdp_desc {
> __u32 options;
> };
>
> +#define XDP_DESC_OPTION_METADATA (1 << 0)
> +
> /* UMEM descriptor is __u64 */
>
> #endif /* _LINUX_IF_XDP_H */
> diff --git a/tools/include/uapi/linux/xdp_md_std.h b/tools/include/uapi/linux/xdp_md_std.h
> new file mode 100644
> index 000000000000..f00996a61639
> --- /dev/null
> +++ b/tools/include/uapi/linux/xdp_md_std.h
> @@ -0,0 +1,14 @@
> +#ifndef _UAPI_LINUX_XDP_MD_STD_H
> +#define _UAPI_LINUX_XDP_MD_STD_H
> +
> +#include <linux/types.h>
> +
> +#define XDP_METADATA_USER_TX_TIMESTAMP 0x1
> +
> +struct xdp_user_tx_metadata {
> + __u64 timestamp;
> + __u32 md_valid;
> + __u32 btf_id;
> +};
> +
> +#endif /* _UAPI_LINUX_XDP_MD_STD_H */
> diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h
> index 01c12dca9c10..1b52ffe1c9a3 100644
> --- a/tools/lib/bpf/xsk.h
> +++ b/tools/lib/bpf/xsk.h
> @@ -16,7 +16,8 @@
> #include <stdint.h>
> #include <stdbool.h>
> #include <linux/if_xdp.h>
> -
> +#include <linux/xdp_md_std.h>
> +#include <errno.h>
> #include "libbpf.h"
>
> #ifdef __cplusplus
> @@ -248,6 +249,30 @@ static inline __u64 xsk_umem__add_offset_to_addr(__u64 addr)
> LIBBPF_API int xsk_umem__fd(const struct xsk_umem *umem);
> LIBBPF_API int xsk_socket__fd(const struct xsk_socket *xsk);
>
> +/* Helpers for SO_TXTIME */
> +
> +static inline void xsk_umem__set_md_txtime(void *umem_area, __u64 addr, __s64 txtime)
> +{
> + struct xdp_user_tx_metadata *md;
> +
> + md = (struct xdp_user_tx_metadata *)&((char *)umem_area)[addr];
> +
> + md->timestamp = txtime;
> + md->md_valid |= XDP_METADATA_USER_TX_TIMESTAMP;
Is this encoding the "type" of the timestamp?
I don't see the btf_id being updated. Does that happen in another patch?
As I note above we are current;y lacking an libbpf equivalent
bpf_core_type_id_kernel() lookup function in userspace.
> +}
> +
> +static inline int xsk_socket__enable_so_txtime(struct xsk_socket *xsk, bool enable)
> +{
> + unsigned int val = (enable) ? 1 : 0;
> + int err;
> +
> + err = setsockopt(xsk_socket__fd(xsk), SOL_XDP, SO_TXTIME, &val, sizeof(val));
> +
> + if (err)
> + return -errno;
> + return 0;
> +}
> +
> #define XSK_RING_CONS__DEFAULT_NUM_DESCS 2048
> #define XSK_RING_PROD__DEFAULT_NUM_DESCS 2048
> #define XSK_UMEM__DEFAULT_FRAME_SHIFT 12 /* 4096 bytes */
>
next prev parent reply other threads:[~2021-08-18 9:49 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-03 17:10 [RFC bpf-next 0/5] SO_TXTIME support in AF_XDP Kishen Maloor
2021-08-03 17:10 ` [RFC bpf-next 1/5] net: xdp: " Kishen Maloor
2021-08-03 20:05 ` kernel test robot
2021-08-03 20:56 ` kernel test robot
2021-08-03 17:10 ` [RFC bpf-next 2/5] libbpf: " Kishen Maloor
2021-08-06 23:08 ` Andrii Nakryiko
2021-08-18 9:49 ` Jesper Dangaard Brouer [this message]
2021-08-19 19:32 ` Kishen Maloor
2021-08-03 17:10 ` [RFC bpf-next 3/5] igc: Launchtime support in XDP Tx ZC path Kishen Maloor
2021-08-03 21:41 ` kernel test robot
2021-08-04 4:15 ` kernel test robot
2021-08-05 17:53 ` Kishen Maloor
2021-08-03 17:10 ` [RFC bpf-next 4/5] samples/bpf/xdpsock_user.c: Make get_nsecs() generic Kishen Maloor
2021-08-03 17:10 ` [RFC bpf-next 5/5] samples/bpf/xdpsock_user.c: Launchtime/TXTIME API usage Kishen Maloor
2021-08-18 8:54 ` Jesper Dangaard Brouer
2021-08-19 19:32 ` Kishen Maloor
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=31fb6a84-562e-a41d-0614-061e1f475db3@redhat.com \
--to=jbrouer@redhat.com \
--cc=andrii@kernel.org \
--cc=bjorn@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=brouer@redhat.com \
--cc=hawk@kernel.org \
--cc=jithu.joseph@intel.com \
--cc=kishen.maloor@intel.com \
--cc=magnus.karlsson@intel.com \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.