| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20181214202325.pxwjl6cqzyf4a4f7@ast-mbp.dhcp.thefacebook.com>
Date: Fri, 14 Dec 2018 12:23:27 -0800
From: Alexei Starovoitov <alexei.starovoitov@...il.com>
To: Magnus Karlsson <magnus.karlsson@...el.com>
Cc: bjorn.topel@...el.com, ast@...nel.org, daniel@...earbox.net,
netdev@...r.kernel.org, jakub.kicinski@...ronome.com,
bjorn.topel@...il.com, qi.z.zhang@...el.com, brouer@...hat.com
Subject: Re: [PATCH bpf-next v2 1/2] libbpf: add support for using AF_XDP
sockets
On Wed, Dec 12, 2018 at 02:09:48PM +0100, Magnus Karlsson wrote:
> This commit adds AF_XDP support to libbpf. The main reason for
> this is to facilitate writing applications that use AF_XDP by offering
> higher-level APIs that hide many of the details of the AF_XDP
> uapi. This is in the same vein as libbpf facilitates XDP adoption by
> offering easy-to-use higher level interfaces of XDP
> functionality. Hopefully this will facilitate adoption of AF_XDP, make
> applications using it simpler and smaller, and finally also make it
> possible for applications to benefit from optimizations in the AF_XDP
> user space access code. Previously, people just copied and pasted the
> code from the sample application into their application, which is not
> desirable.
>
> The interface is composed of two parts:
>
> * Low-level access interface to the four rings and the packet
> * High-level control plane interface for creating and setting
> up umems and af_xdp sockets.
>
> Signed-off-by: Magnus Karlsson <magnus.karlsson@...el.com>
...
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index 5f68d7b..da99203 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
may be instead of lib/bpf/libbpf.h the xsk stuff should go to lib/bpf/xsk.h ?
> @@ -15,6 +15,7 @@
> #include <stdbool.h>
> #include <sys/types.h> // for size_t
> #include <linux/bpf.h>
> +#include <linux/if_xdp.h>
>
> #ifdef __cplusplus
> extern "C" {
> @@ -355,6 +356,98 @@ LIBBPF_API const struct bpf_line_info *
> bpf_prog_linfo__lfind(const struct bpf_prog_linfo *prog_linfo,
> __u32 insn_off, __u32 nr_skip);
>
> +/* Do not access these members directly. Use the functions below. */
> +struct xsk_prod_ring {
> + __u32 cached_prod;
> + __u32 cached_cons;
> + __u32 mask;
> + __u32 size;
> + __u32 *producer;
> + __u32 *consumer;
> + void *ring;
> +};
> +
> +/* Do not access these members directly. Use the functions below. */
> +struct xsk_cons_ring {
> + __u32 cached_prod;
> + __u32 cached_cons;
> + __u32 mask;
> + __u32 size;
> + __u32 *producer;
> + __u32 *consumer;
> + void *ring;
> +};
xsk_prod_ring and xsk_cons_ring have exactly the same members,
but they're two different structs? why?
May be have one 'struct xsk_ring' ?
> +
> +static inline __u64 *xsk__get_fill_desc(struct xsk_prod_ring *fill,
> + __u64 idx)
see tools/lib/bpf/README.rst
the main idea is for "objects" use __ to separate class vs method.
In this case 'struct xsk_ring' would be an object and
the name of the method would be:
static inline __u64 *xsk_ring__get_fill_desc(struct xsk_ring *fill, __u64 idx)
> +{
> + __u64 *descs = (__u64 *)fill->ring;
> +
> + return &descs[idx & fill->mask];
> +}
> +
> +static inline __u64 *xsk__get_completion_desc(struct xsk_cons_ring *comp,
> + __u64 idx)
> +{
> + __u64 *descs = (__u64 *)comp->ring;
> +
> + return &descs[idx & comp->mask];
> +}
> +
> +static inline struct xdp_desc *xsk__get_tx_desc(struct xsk_prod_ring *tx,
> + __u64 idx)
> +{
> + struct xdp_desc *descs = (struct xdp_desc *)tx->ring;
> +
> + return &descs[idx & tx->mask];
> +}
> +
> +static inline struct xdp_desc *xsk__get_rx_desc(struct xsk_cons_ring *rx,
> + __u64 idx)
> +{
> + struct xdp_desc *descs = (struct xdp_desc *)rx->ring;
> +
> + return &descs[idx & rx->mask];
> +}
> +
> +LIBBPF_API size_t xsk__peek_cons(struct xsk_cons_ring *ring, size_t nb,
> + __u32 *idx);
> +LIBBPF_API void xsk__release_cons(struct xsk_cons_ring *ring);
> +LIBBPF_API size_t xsk__reserve_prod(struct xsk_prod_ring *ring, size_t nb,
> + __u32 *idx);
> +LIBBPF_API void xsk__submit_prod(struct xsk_prod_ring *ring);
if we combine the struct names then above could be:
LIBBPF_API size_t xsk_ring__reserve(struct xsk_ring *ring, size_t nb, __u32 *idx);
LIBBPF_API void xsk_ring__submit(struct xsk_ring *ring);
?
> +
> +LIBBPF_API void *xsk__get_data(void *umem_area, __u64 addr);
> +
> +#define XSK__DEFAULT_NUM_DESCS 2048
> +#define XSK__DEFAULT_FRAME_SHIFT 11 /* 2048 bytes */
> +#define XSK__DEFAULT_FRAME_SIZE (1 << XSK__DEFAULT_FRAME_SHIFT)
> +#define XSK__DEFAULT_FRAME_HEADROOM 0
> +
> +struct xsk_umem_config {
> + __u32 fq_size;
> + __u32 cq_size;
> + __u32 frame_size;
> + __u32 frame_headroom;
> +};
> +
> +struct xsk_xdp_socket_config {
> + __u32 rx_size;
> + __u32 tx_size;
> +};
> +
> +/* Set config to XSK_DEFAULT_CONFIG to get the default configuration. */
> +LIBBPF_API int xsk__create_umem(void *umem_area, __u64 size,
> + struct xsk_prod_ring *fq,
> + struct xsk_cons_ring *cq,
> + struct xsk_umem_config *config);
this one looks too low level.
espcially considering it's usage:
umem->fd = xsk__create_umem(buffer, size, &umem->fq, &umem->cq, NULL);
may be create an object "struct xsk_umem" ?
then api will be:
err = xsk_umem__create(buffer, size, NULL) ?
> +LIBBPF_API int xsk__create_xdp_socket(int umem_fd, struct xsk_cons_ring *rx,
> + struct xsk_prod_ring *tx,
> + struct xsk_xdp_socket_config *config);
similar concern here. feels that implementation details are leaking into api.
The usage of it is:
xsk->fd = xsk__create_xdp_socket(umem->fd, &xsk->rx, &xsk->tx, NULL);
may be create an object "struct xsk_socket" ?
Powered by blists - more mailing lists