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.karls...@intel.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" ?

Reply via email to