| Message ID | 20190912072840.20947-1-kevin.laatz@intel.com |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | [bpf-next] libbpf: add xsk_umem__adjust_offset | expand |
On Thu, 12 Sep 2019 at 17:47, Kevin Laatz <kevin.laatz@intel.com> wrote: > > Currently, xsk_umem_adjust_offset exists as a kernel internal function. > This patch adds xsk_umem__adjust_offset to libbpf so that it can be used > from userspace. This will take the responsibility of properly storing the > offset away from the application, making it less error prone. > > Since xsk_umem__adjust_offset is called on a per-packet basis, we need to > inline the function to avoid any performance regressions. In order to > inline xsk_umem__adjust_offset, we need to add it to xsk.h. Unfortunately > this means that we can't dereference the xsk_umem_config struct directly > since it is defined only in xsk.c. We therefore add an extra API to return > the flags field to the user from the structure, and have the inline > function use this flags field directly. > Can you expand this to a series, with an additional patch where these functions are used in XDP socket sample application, so it's more clear for users? Thanks, Björn > Signed-off-by: Kevin Laatz <kevin.laatz@intel.com> > --- > tools/lib/bpf/libbpf.map | 1 + > tools/lib/bpf/xsk.c | 5 +++++ > tools/lib/bpf/xsk.h | 14 ++++++++++++++ > 3 files changed, 20 insertions(+) > > diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map > index d04c7cb623ed..760350c9b81c 100644 > --- a/tools/lib/bpf/libbpf.map > +++ b/tools/lib/bpf/libbpf.map > @@ -189,4 +189,5 @@ LIBBPF_0.0.4 { > LIBBPF_0.0.5 { > global: > bpf_btf_get_next_id; > + xsk_umem__get_flags; > } LIBBPF_0.0.4; > diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c > index 842c4fd55859..a4250a721ea6 100644 > --- a/tools/lib/bpf/xsk.c > +++ b/tools/lib/bpf/xsk.c > @@ -84,6 +84,11 @@ int xsk_socket__fd(const struct xsk_socket *xsk) > return xsk ? xsk->fd : -EINVAL; > } > > +__u32 xsk_umem__get_flags(struct xsk_umem *umem) > +{ > + return umem->config.flags; > +} > + > static bool xsk_page_aligned(void *buffer) > { > unsigned long addr = (unsigned long)buffer; > diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h > index 584f6820a639..bf782facb274 100644 > --- a/tools/lib/bpf/xsk.h > +++ b/tools/lib/bpf/xsk.h > @@ -183,8 +183,22 @@ static inline __u64 xsk_umem__add_offset_to_addr(__u64 addr) > return xsk_umem__extract_addr(addr) + xsk_umem__extract_offset(addr); > } > > +/* Handle the offset appropriately depending on aligned or unaligned mode. > + * For unaligned mode, we store the offset in the upper 16-bits of the address. > + * For aligned mode, we simply add the offset to the address. > + */ > +static inline __u64 xsk_umem__adjust_offset(__u32 umem_flags, __u64 addr, > + __u64 offset) > +{ > + if (umem_flags & XDP_UMEM_UNALIGNED_CHUNK_FLAG) > + return addr + (offset << XSK_UNALIGNED_BUF_OFFSET_SHIFT); > + else > + return addr + offset; > +} > + > LIBBPF_API int xsk_umem__fd(const struct xsk_umem *umem); > LIBBPF_API int xsk_socket__fd(const struct xsk_socket *xsk); > +LIBBPF_API __u32 xsk_umem__get_flags(struct xsk_umem *umem); > > #define XSK_RING_CONS__DEFAULT_NUM_DESCS 2048 > #define XSK_RING_PROD__DEFAULT_NUM_DESCS 2048 > -- > 2.17.1 >
On 13/09/2019 05:59, Björn Töpel wrote: > On Thu, 12 Sep 2019 at 17:47, Kevin Laatz <kevin.laatz@intel.com> wrote: >> Currently, xsk_umem_adjust_offset exists as a kernel internal function. >> This patch adds xsk_umem__adjust_offset to libbpf so that it can be used >> from userspace. This will take the responsibility of properly storing the >> offset away from the application, making it less error prone. >> >> Since xsk_umem__adjust_offset is called on a per-packet basis, we need to >> inline the function to avoid any performance regressions. In order to >> inline xsk_umem__adjust_offset, we need to add it to xsk.h. Unfortunately >> this means that we can't dereference the xsk_umem_config struct directly >> since it is defined only in xsk.c. We therefore add an extra API to return >> the flags field to the user from the structure, and have the inline >> function use this flags field directly. >> > Can you expand this to a series, with an additional patch where these > functions are used in XDP socket sample application, so it's more > clear for users? These functions are currently not required in the xdpsock application and I think forcing them in would be messy :-). However, an example of the use of these functions could be seen in the DPDK AF_XDP PMD. There is a patch herehttp://patches.dpdk.org/patch/58624/ where we currently do the offset adjustment to the handle manually, but with this patch we could replace it with xsk_umem__adjust_offset and have a real use example of the functions being used. Would this be enough for an example? Thanks, Kevin
On 2019-09-13 12:21, Laatz, Kevin wrote: > On 13/09/2019 05:59, Björn Töpel wrote: >> On Thu, 12 Sep 2019 at 17:47, Kevin Laatz <kevin.laatz@intel.com> wrote: >>> Currently, xsk_umem_adjust_offset exists as a kernel internal function. >>> This patch adds xsk_umem__adjust_offset to libbpf so that it can be used >>> from userspace. This will take the responsibility of properly storing >>> the >>> offset away from the application, making it less error prone. >>> >>> Since xsk_umem__adjust_offset is called on a per-packet basis, we >>> need to >>> inline the function to avoid any performance regressions. In order to >>> inline xsk_umem__adjust_offset, we need to add it to xsk.h. >>> Unfortunately >>> this means that we can't dereference the xsk_umem_config struct directly >>> since it is defined only in xsk.c. We therefore add an extra API to >>> return >>> the flags field to the user from the structure, and have the inline >>> function use this flags field directly. >>> >> Can you expand this to a series, with an additional patch where these >> functions are used in XDP socket sample application, so it's more >> clear for users? > > These functions are currently not required in the xdpsock application > and I think forcing them in would be messy :-). However, an example of > the use of these functions could be seen in the DPDK AF_XDP PMD. There > is a patch herehttp://patches.dpdk.org/patch/58624/ where we currently > do the offset adjustment to the handle manually, but with this patch we > could replace it with xsk_umem__adjust_offset and have a real use > example of the functions being used. > > Would this be enough for an example? > Fair enough! :-) Acked-by: Björn Töpel <bjorn.topel@intel.com> > Thanks, > Kevin >
On 9/12/19 8:28 AM, Kevin Laatz wrote: > Currently, xsk_umem_adjust_offset exists as a kernel internal function. > This patch adds xsk_umem__adjust_offset to libbpf so that it can be used > from userspace. This will take the responsibility of properly storing the > offset away from the application, making it less error prone. > > Since xsk_umem__adjust_offset is called on a per-packet basis, we need to > inline the function to avoid any performance regressions. In order to > inline xsk_umem__adjust_offset, we need to add it to xsk.h. Unfortunately > this means that we can't dereference the xsk_umem_config struct directly > since it is defined only in xsk.c. We therefore add an extra API to return > the flags field to the user from the structure, and have the inline > function use this flags field directly. Could you add an example (either in samples/bpf or in tools/testing/selftests/bpf) to use xsk_umem__adjust_offset()? This will make it easier to check whether the interface is appropriate or not and what is the performance implication as you stated in the above. > > Signed-off-by: Kevin Laatz <kevin.laatz@intel.com> > --- > tools/lib/bpf/libbpf.map | 1 + > tools/lib/bpf/xsk.c | 5 +++++ > tools/lib/bpf/xsk.h | 14 ++++++++++++++ > 3 files changed, 20 insertions(+) > > diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map > index d04c7cb623ed..760350c9b81c 100644 > --- a/tools/lib/bpf/libbpf.map > +++ b/tools/lib/bpf/libbpf.map > @@ -189,4 +189,5 @@ LIBBPF_0.0.4 { > LIBBPF_0.0.5 { > global: > bpf_btf_get_next_id; > + xsk_umem__get_flags; > } LIBBPF_0.0.4; > diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c > index 842c4fd55859..a4250a721ea6 100644 > --- a/tools/lib/bpf/xsk.c > +++ b/tools/lib/bpf/xsk.c > @@ -84,6 +84,11 @@ int xsk_socket__fd(const struct xsk_socket *xsk) > return xsk ? xsk->fd : -EINVAL; > } > > +__u32 xsk_umem__get_flags(struct xsk_umem *umem) > +{ > + return umem->config.flags; > +} > + > static bool xsk_page_aligned(void *buffer) > { > unsigned long addr = (unsigned long)buffer; > diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h > index 584f6820a639..bf782facb274 100644 > --- a/tools/lib/bpf/xsk.h > +++ b/tools/lib/bpf/xsk.h > @@ -183,8 +183,22 @@ static inline __u64 xsk_umem__add_offset_to_addr(__u64 addr) > return xsk_umem__extract_addr(addr) + xsk_umem__extract_offset(addr); > } > > +/* Handle the offset appropriately depending on aligned or unaligned mode. > + * For unaligned mode, we store the offset in the upper 16-bits of the address. > + * For aligned mode, we simply add the offset to the address. > + */ > +static inline __u64 xsk_umem__adjust_offset(__u32 umem_flags, __u64 addr, > + __u64 offset) > +{ > + if (umem_flags & XDP_UMEM_UNALIGNED_CHUNK_FLAG) > + return addr + (offset << XSK_UNALIGNED_BUF_OFFSET_SHIFT); > + else > + return addr + offset; > +} > + > LIBBPF_API int xsk_umem__fd(const struct xsk_umem *umem); > LIBBPF_API int xsk_socket__fd(const struct xsk_socket *xsk); > +LIBBPF_API __u32 xsk_umem__get_flags(struct xsk_umem *umem); > > #define XSK_RING_CONS__DEFAULT_NUM_DESCS 2048 > #define XSK_RING_PROD__DEFAULT_NUM_DESCS 2048 >
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map index d04c7cb623ed..760350c9b81c 100644 --- a/tools/lib/bpf/libbpf.map +++ b/tools/lib/bpf/libbpf.map @@ -189,4 +189,5 @@ LIBBPF_0.0.4 { LIBBPF_0.0.5 { global: bpf_btf_get_next_id; + xsk_umem__get_flags; } LIBBPF_0.0.4; diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c index 842c4fd55859..a4250a721ea6 100644 --- a/tools/lib/bpf/xsk.c +++ b/tools/lib/bpf/xsk.c @@ -84,6 +84,11 @@ int xsk_socket__fd(const struct xsk_socket *xsk) return xsk ? xsk->fd : -EINVAL; } +__u32 xsk_umem__get_flags(struct xsk_umem *umem) +{ + return umem->config.flags; +} + static bool xsk_page_aligned(void *buffer) { unsigned long addr = (unsigned long)buffer; diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h index 584f6820a639..bf782facb274 100644 --- a/tools/lib/bpf/xsk.h +++ b/tools/lib/bpf/xsk.h @@ -183,8 +183,22 @@ static inline __u64 xsk_umem__add_offset_to_addr(__u64 addr) return xsk_umem__extract_addr(addr) + xsk_umem__extract_offset(addr); } +/* Handle the offset appropriately depending on aligned or unaligned mode. + * For unaligned mode, we store the offset in the upper 16-bits of the address. + * For aligned mode, we simply add the offset to the address. + */ +static inline __u64 xsk_umem__adjust_offset(__u32 umem_flags, __u64 addr, + __u64 offset) +{ + if (umem_flags & XDP_UMEM_UNALIGNED_CHUNK_FLAG) + return addr + (offset << XSK_UNALIGNED_BUF_OFFSET_SHIFT); + else + return addr + offset; +} + LIBBPF_API int xsk_umem__fd(const struct xsk_umem *umem); LIBBPF_API int xsk_socket__fd(const struct xsk_socket *xsk); +LIBBPF_API __u32 xsk_umem__get_flags(struct xsk_umem *umem); #define XSK_RING_CONS__DEFAULT_NUM_DESCS 2048 #define XSK_RING_PROD__DEFAULT_NUM_DESCS 2048
Currently, xsk_umem_adjust_offset exists as a kernel internal function. This patch adds xsk_umem__adjust_offset to libbpf so that it can be used from userspace. This will take the responsibility of properly storing the offset away from the application, making it less error prone. Since xsk_umem__adjust_offset is called on a per-packet basis, we need to inline the function to avoid any performance regressions. In order to inline xsk_umem__adjust_offset, we need to add it to xsk.h. Unfortunately this means that we can't dereference the xsk_umem_config struct directly since it is defined only in xsk.c. We therefore add an extra API to return the flags field to the user from the structure, and have the inline function use this flags field directly. Signed-off-by: Kevin Laatz <kevin.laatz@intel.com> --- tools/lib/bpf/libbpf.map | 1 + tools/lib/bpf/xsk.c | 5 +++++ tools/lib/bpf/xsk.h | 14 ++++++++++++++ 3 files changed, 20 insertions(+)