[PATCH bpf-next v3] libbpf: fix compatibility for kernels without need_wakeup

[PATCH bpf-next v3] libbpf: fix compatibility for kernels without need_wakeup

[Magnus Karlsson]

When the need_wakeup flag was added to AF_XDP, the format of the
XDP_MMAP_OFFSETS getsockopt was extended. Code was added to the
kernel to take care of compatibility issues arrising from running
applications using any of the two formats. However, libbpf was
not extended to take care of the case when the application/libbpf
uses the new format but the kernel only supports the old
format. This patch adds support in libbpf for parsing the old
format, before the need_wakeup flag was added, and emulating a
set of static need_wakeup flags that will always work for the
application.

v2 -> v3:
* Incorporated code improvements suggested by Jonathan Lemon

v1 -> v2:
* Rebased to bpf-next
* Rewrote the code as the previous version made you blind

Fixes: a4500432c2587cb2a ("libbpf: add support for need_wakeup flag in AF_XDP part")
Reported-by: Eloy Degen <degeneloy@gmail.com>
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
 tools/lib/bpf/xsk.c | 83 +++++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 71 insertions(+), 12 deletions(-)

diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c
index 9a2af44..280010c 100644
--- a/tools/lib/bpf/xsk.c
+++ b/tools/lib/bpf/xsk.c
@@ -73,6 +73,21 @@ struct xsk_nl_info {
 	int fd;
 };
 
+/* Up until and including Linux 5.3 */
+struct xdp_ring_offset_v1 {
+	__u64 producer;
+	__u64 consumer;
+	__u64 desc;
+};
+
+/* Up until and including Linux 5.3 */
+struct xdp_mmap_offsets_v1 {
+	struct xdp_ring_offset_v1 rx;
+	struct xdp_ring_offset_v1 tx;
+	struct xdp_ring_offset_v1 fr;
+	struct xdp_ring_offset_v1 cr;
+};
+
 int xsk_umem__fd(const struct xsk_umem *umem)
 {
 	return umem ? umem->fd : -EINVAL;
@@ -133,6 +148,58 @@ static int xsk_set_xdp_socket_config(struct xsk_socket_config *cfg,
 	return 0;
 }
 
+static void xsk_mmap_offsets_v1(struct xdp_mmap_offsets *off)
+{
+	struct xdp_mmap_offsets_v1 off_v1;
+
+	/* getsockopt on a kernel <= 5.3 has no flags fields.
+	 * Copy over the offsets to the correct places in the >=5.4 format
+	 * and put the flags where they would have been on that kernel.
+	 */
+	memcpy(&off_v1, off, sizeof(off_v1));
+
+	off->rx.producer = off_v1.rx.producer;
+	off->rx.consumer = off_v1.rx.consumer;
+	off->rx.desc = off_v1.rx.desc;
+	off->rx.flags = off_v1.rx.consumer + sizeof(u32);
+
+	off->tx.producer = off_v1.tx.producer;
+	off->tx.consumer = off_v1.tx.consumer;
+	off->tx.desc = off_v1.tx.desc;
+	off->tx.flags = off_v1.tx.consumer + sizeof(u32);
+
+	off->fr.producer = off_v1.fr.producer;
+	off->fr.consumer = off_v1.fr.consumer;
+	off->fr.desc = off_v1.fr.desc;
+	off->fr.flags = off_v1.fr.consumer + sizeof(u32);
+
+	off->cr.producer = off_v1.cr.producer;
+	off->cr.consumer = off_v1.cr.consumer;
+	off->cr.desc = off_v1.cr.desc;
+	off->cr.flags = off_v1.cr.consumer + sizeof(u32);
+}
+
+static int xsk_get_mmap_offsets(int fd, struct xdp_mmap_offsets *off)
+{
+	socklen_t optlen;
+	int err;
+
+	optlen = sizeof(*off);
+	err = getsockopt(fd, SOL_XDP, XDP_MMAP_OFFSETS, off, &optlen);
+	if (err)
+		return err;
+
+	if (optlen == sizeof(*off))
+		return 0;
+
+	if (optlen == sizeof(struct xdp_mmap_offsets_v1)) {
+		xsk_mmap_offsets_v1(off);
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
 int xsk_umem__create_v0_0_4(struct xsk_umem **umem_ptr, void *umem_area,
 			    __u64 size, struct xsk_ring_prod *fill,
 			    struct xsk_ring_cons *comp,
@@ -141,7 +208,6 @@ int xsk_umem__create_v0_0_4(struct xsk_umem **umem_ptr, void *umem_area,
 	struct xdp_mmap_offsets off;
 	struct xdp_umem_reg mr;
 	struct xsk_umem *umem;
-	socklen_t optlen;
 	void *map;
 	int err;
 
@@ -190,8 +256,7 @@ int xsk_umem__create_v0_0_4(struct xsk_umem **umem_ptr, void *umem_area,
 		goto out_socket;
 	}
 
-	optlen = sizeof(off);
-	err = getsockopt(umem->fd, SOL_XDP, XDP_MMAP_OFFSETS, &off, &optlen);
+	err = xsk_get_mmap_offsets(umem->fd, &off);
 	if (err) {
 		err = -errno;
 		goto out_socket;
@@ -514,7 +579,6 @@ int xsk_socket__create(struct xsk_socket **xsk_ptr, const char *ifname,
 	struct sockaddr_xdp sxdp = {};
 	struct xdp_mmap_offsets off;
 	struct xsk_socket *xsk;
-	socklen_t optlen;
 	int err;
 
 	if (!umem || !xsk_ptr || !rx || !tx)
@@ -573,8 +637,7 @@ int xsk_socket__create(struct xsk_socket **xsk_ptr, const char *ifname,
 		}
 	}
 
-	optlen = sizeof(off);
-	err = getsockopt(xsk->fd, SOL_XDP, XDP_MMAP_OFFSETS, &off, &optlen);
+	err = xsk_get_mmap_offsets(xsk->fd, &off);
 	if (err) {
 		err = -errno;
 		goto out_socket;
@@ -660,7 +723,6 @@ int xsk_socket__create(struct xsk_socket **xsk_ptr, const char *ifname,
 int xsk_umem__delete(struct xsk_umem *umem)
 {
 	struct xdp_mmap_offsets off;
-	socklen_t optlen;
 	int err;
 
 	if (!umem)
@@ -669,8 +731,7 @@ int xsk_umem__delete(struct xsk_umem *umem)
 	if (umem->refcount)
 		return -EBUSY;
 
-	optlen = sizeof(off);
-	err = getsockopt(umem->fd, SOL_XDP, XDP_MMAP_OFFSETS, &off, &optlen);
+	err = xsk_get_mmap_offsets(umem->fd, &off);
 	if (!err) {
 		munmap(umem->fill->ring - off.fr.desc,
 		       off.fr.desc + umem->config.fill_size * sizeof(__u64));
@@ -688,7 +749,6 @@ void xsk_socket__delete(struct xsk_socket *xsk)
 {
 	size_t desc_sz = sizeof(struct xdp_desc);
 	struct xdp_mmap_offsets off;
-	socklen_t optlen;
 	int err;
 
 	if (!xsk)
@@ -699,8 +759,7 @@ void xsk_socket__delete(struct xsk_socket *xsk)
 		close(xsk->prog_fd);
 	}
 
-	optlen = sizeof(off);
-	err = getsockopt(xsk->fd, SOL_XDP, XDP_MMAP_OFFSETS, &off, &optlen);
+	err = xsk_get_mmap_offsets(xsk->fd, &off);
 	if (!err) {
 		if (xsk->rx) {
 			munmap(xsk->rx->ring - off.rx.desc,
-- 
2.7.4

Re: [PATCH bpf-next v3] libbpf: fix compatibility for kernels without need_wakeup

[Jonathan Lemon]

On 25 Oct 2019, at 2:17, Magnus Karlsson wrote:

> When the need_wakeup flag was added to AF_XDP, the format of the
> XDP_MMAP_OFFSETS getsockopt was extended. Code was added to the
> kernel to take care of compatibility issues arrising from running
> applications using any of the two formats. However, libbpf was
> not extended to take care of the case when the application/libbpf
> uses the new format but the kernel only supports the old
> format. This patch adds support in libbpf for parsing the old
> format, before the need_wakeup flag was added, and emulating a
> set of static need_wakeup flags that will always work for the
> application.
>
> v2 -> v3:
> * Incorporated code improvements suggested by Jonathan Lemon

Thanks for doing this, Magnus!  It looks much simpler now.


>
> v1 -> v2:
> * Rebased to bpf-next
> * Rewrote the code as the previous version made you blind
>
> Fixes: a4500432c2587cb2a ("libbpf: add support for need_wakeup flag in 
> AF_XDP part")
> Reported-by: Eloy Degen <degeneloy@gmail.com>
> Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>

Acked-by: Jonathan Lemon <jonathan.lemon@gmail.com>

Re: [PATCH bpf-next v3] libbpf: fix compatibility for kernels without need_wakeup

[Alexei Starovoitov]

On Fri, Oct 25, 2019 at 2:17 AM Magnus Karlsson
<magnus.karlsson@intel.com> wrote:
>
> When the need_wakeup flag was added to AF_XDP, the format of the
> XDP_MMAP_OFFSETS getsockopt was extended. Code was added to the
> kernel to take care of compatibility issues arrising from running
> applications using any of the two formats. However, libbpf was
> not extended to take care of the case when the application/libbpf
> uses the new format but the kernel only supports the old
> format. This patch adds support in libbpf for parsing the old
> format, before the need_wakeup flag was added, and emulating a
> set of static need_wakeup flags that will always work for the
> application.
>
> v2 -> v3:
> * Incorporated code improvements suggested by Jonathan Lemon
>
> v1 -> v2:
> * Rebased to bpf-next
> * Rewrote the code as the previous version made you blind
>
> Fixes: a4500432c2587cb2a ("libbpf: add support for need_wakeup flag in AF_XDP part")
> Reported-by: Eloy Degen <degeneloy@gmail.com>
> Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>

Applied. Thanks

Re: [PATCH bpf-next v3] libbpf: fix compatibility for kernels without need_wakeup

[Toke Høiland-Jørgensen]

Magnus Karlsson <magnus.karlsson@intel.com> writes:

> When the need_wakeup flag was added to AF_XDP, the format of the
> XDP_MMAP_OFFSETS getsockopt was extended. Code was added to the
> kernel to take care of compatibility issues arrising from running
> applications using any of the two formats. However, libbpf was
> not extended to take care of the case when the application/libbpf
> uses the new format but the kernel only supports the old
> format. This patch adds support in libbpf for parsing the old
> format, before the need_wakeup flag was added, and emulating a
> set of static need_wakeup flags that will always work for the
> application.

Hi Magnus

While you're looking at backwards compatibility issues with xsk: libbpf
currently fails to compile on a system that has old kernel headers
installed (this is with kernel-headers 5.3):

$ echo "#include <bpf/xsk.h>" | gcc -x c - 
In file included from <stdin>:1:
/usr/include/bpf/xsk.h: In function ‘xsk_ring_prod__needs_wakeup’:
/usr/include/bpf/xsk.h:82:21: error: ‘XDP_RING_NEED_WAKEUP’ undeclared (first use in this function)
   82 |  return *r->flags & XDP_RING_NEED_WAKEUP;
      |                     ^~~~~~~~~~~~~~~~~~~~
/usr/include/bpf/xsk.h:82:21: note: each undeclared identifier is reported only once for each function it appears in
/usr/include/bpf/xsk.h: In function ‘xsk_umem__extract_addr’:
/usr/include/bpf/xsk.h:173:16: error: ‘XSK_UNALIGNED_BUF_ADDR_MASK’ undeclared (first use in this function)
  173 |  return addr & XSK_UNALIGNED_BUF_ADDR_MASK;
      |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/bpf/xsk.h: In function ‘xsk_umem__extract_offset’:
/usr/include/bpf/xsk.h:178:17: error: ‘XSK_UNALIGNED_BUF_OFFSET_SHIFT’ undeclared (first use in this function)
  178 |  return addr >> XSK_UNALIGNED_BUF_OFFSET_SHIFT;
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~



How would you prefer to handle this? A patch like the one below will fix
the compile errors, but I'm not sure it makes sense semantically?

-Toke

diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h
index 584f6820a639..954d66e85208 100644
--- a/tools/lib/bpf/xsk.h
+++ b/tools/lib/bpf/xsk.h
@@ -79,7 +79,11 @@ xsk_ring_cons__rx_desc(const struct xsk_ring_cons *rx, __u32 idx)
 
 static inline int xsk_ring_prod__needs_wakeup(const struct xsk_ring_prod *r)
 {
+#ifdef XDP_RING_NEED_WAKEUP
        return *r->flags & XDP_RING_NEED_WAKEUP;
+#else
+       return 0;
+#endif
 }
 
 static inline __u32 xsk_prod_nb_free(struct xsk_ring_prod *r, __u32 nb)
@@ -170,12 +174,20 @@ static inline void *xsk_umem__get_data(void *umem_area, __u64 addr)
 
 static inline __u64 xsk_umem__extract_addr(__u64 addr)
 {
+#ifdef XSK_UNALIGNED_BUF_ADDR_MASK
        return addr & XSK_UNALIGNED_BUF_ADDR_MASK;
+#else
+       return addr;
+#endif
 }
 
 static inline __u64 xsk_umem__extract_offset(__u64 addr)
 {
+#ifdef XSK_UNALIGNED_BUF_OFFSET_SHIFT
        return addr >> XSK_UNALIGNED_BUF_OFFSET_SHIFT;
+#else
+       return 0;
+#endif
 }
 
 static inline __u64 xsk_umem__add_offset_to_addr(__u64 addr)

Re: [PATCH bpf-next v3] libbpf: fix compatibility for kernels without need_wakeup

[Magnus Karlsson]

On Wed, Oct 30, 2019 at 2:36 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>
> Magnus Karlsson <magnus.karlsson@intel.com> writes:
>
> > When the need_wakeup flag was added to AF_XDP, the format of the
> > XDP_MMAP_OFFSETS getsockopt was extended. Code was added to the
> > kernel to take care of compatibility issues arrising from running
> > applications using any of the two formats. However, libbpf was
> > not extended to take care of the case when the application/libbpf
> > uses the new format but the kernel only supports the old
> > format. This patch adds support in libbpf for parsing the old
> > format, before the need_wakeup flag was added, and emulating a
> > set of static need_wakeup flags that will always work for the
> > application.
>
> Hi Magnus
>
> While you're looking at backwards compatibility issues with xsk: libbpf
> currently fails to compile on a system that has old kernel headers
> installed (this is with kernel-headers 5.3):
>
> $ echo "#include <bpf/xsk.h>" | gcc -x c -
> In file included from <stdin>:1:
> /usr/include/bpf/xsk.h: In function ‘xsk_ring_prod__needs_wakeup’:
> /usr/include/bpf/xsk.h:82:21: error: ‘XDP_RING_NEED_WAKEUP’ undeclared (first use in this function)
>    82 |  return *r->flags & XDP_RING_NEED_WAKEUP;
>       |                     ^~~~~~~~~~~~~~~~~~~~
> /usr/include/bpf/xsk.h:82:21: note: each undeclared identifier is reported only once for each function it appears in
> /usr/include/bpf/xsk.h: In function ‘xsk_umem__extract_addr’:
> /usr/include/bpf/xsk.h:173:16: error: ‘XSK_UNALIGNED_BUF_ADDR_MASK’ undeclared (first use in this function)
>   173 |  return addr & XSK_UNALIGNED_BUF_ADDR_MASK;
>       |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> /usr/include/bpf/xsk.h: In function ‘xsk_umem__extract_offset’:
> /usr/include/bpf/xsk.h:178:17: error: ‘XSK_UNALIGNED_BUF_OFFSET_SHIFT’ undeclared (first use in this function)
>   178 |  return addr >> XSK_UNALIGNED_BUF_OFFSET_SHIFT;
>       |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
>
>
> How would you prefer to handle this? A patch like the one below will fix
> the compile errors, but I'm not sure it makes sense semantically?

Thanks Toke for finding this. Of course it should be possible to
compile this on an older kernel, but without getting any of the newer
functionality that is not present in that older kernel. My preference
is if we just remove these functions completely if you compile it with
a kernel that does not have support for it. For the need_wakeup
feature, we cannot provide semantics that make sense in the function
above. If we return 0, Tx will break. If we return 1, Rx will become
really slow. And we cannot detect /without an ugly hack) if it is the
fill queue or the Tx queue that was provided to the function. So what
do you think of just removing these functions if the kernel does not
have the corresponding defines in its kernel header? The user should
not use them in that case.

We should also think about the case when someone takes the new libbpf
source, compiles it with an older kernel then runs the binary on the
newer kernel. It will for sure happen :-). We should add some code in
the xsk_socket__create call that checks so that users do not try to
use a bind flag that did not exist on the system libbpf was compiled
for. In that case, we should return an error. We need the same code
that we have in the kernel for checking against illegal flags to be
present in the xsk part of libbpf.

/Magnus

> -Toke
>
> diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h
> index 584f6820a639..954d66e85208 100644
> --- a/tools/lib/bpf/xsk.h
> +++ b/tools/lib/bpf/xsk.h
> @@ -79,7 +79,11 @@ xsk_ring_cons__rx_desc(const struct xsk_ring_cons *rx, __u32 idx)
>
>  static inline int xsk_ring_prod__needs_wakeup(const struct xsk_ring_prod *r)
>  {
> +#ifdef XDP_RING_NEED_WAKEUP
>         return *r->flags & XDP_RING_NEED_WAKEUP;
> +#else
> +       return 0;
> +#endif
>  }
>
>  static inline __u32 xsk_prod_nb_free(struct xsk_ring_prod *r, __u32 nb)
> @@ -170,12 +174,20 @@ static inline void *xsk_umem__get_data(void *umem_area, __u64 addr)
>
>  static inline __u64 xsk_umem__extract_addr(__u64 addr)
>  {
> +#ifdef XSK_UNALIGNED_BUF_ADDR_MASK
>         return addr & XSK_UNALIGNED_BUF_ADDR_MASK;
> +#else
> +       return addr;
> +#endif
>  }
>
>  static inline __u64 xsk_umem__extract_offset(__u64 addr)
>  {
> +#ifdef XSK_UNALIGNED_BUF_OFFSET_SHIFT
>         return addr >> XSK_UNALIGNED_BUF_OFFSET_SHIFT;
> +#else
> +       return 0;
> +#endif
>  }
>
>  static inline __u64 xsk_umem__add_offset_to_addr(__u64 addr)
>

Re: [PATCH bpf-next v3] libbpf: fix compatibility for kernels without need_wakeup

[Björn Töpel]

On Thu, 31 Oct 2019 at 08:17, Magnus Karlsson <magnus.karlsson@gmail.com> wrote:
>
> On Wed, Oct 30, 2019 at 2:36 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> >
> > Magnus Karlsson <magnus.karlsson@intel.com> writes:
> >
> > > When the need_wakeup flag was added to AF_XDP, the format of the
> > > XDP_MMAP_OFFSETS getsockopt was extended. Code was added to the
> > > kernel to take care of compatibility issues arrising from running
> > > applications using any of the two formats. However, libbpf was
> > > not extended to take care of the case when the application/libbpf
> > > uses the new format but the kernel only supports the old
> > > format. This patch adds support in libbpf for parsing the old
> > > format, before the need_wakeup flag was added, and emulating a
> > > set of static need_wakeup flags that will always work for the
> > > application.
> >
> > Hi Magnus
> >
> > While you're looking at backwards compatibility issues with xsk: libbpf
> > currently fails to compile on a system that has old kernel headers
> > installed (this is with kernel-headers 5.3):
> >
> > $ echo "#include <bpf/xsk.h>" | gcc -x c -
> > In file included from <stdin>:1:
> > /usr/include/bpf/xsk.h: In function ‘xsk_ring_prod__needs_wakeup’:
> > /usr/include/bpf/xsk.h:82:21: error: ‘XDP_RING_NEED_WAKEUP’ undeclared (first use in this function)
> >    82 |  return *r->flags & XDP_RING_NEED_WAKEUP;
> >       |                     ^~~~~~~~~~~~~~~~~~~~
> > /usr/include/bpf/xsk.h:82:21: note: each undeclared identifier is reported only once for each function it appears in
> > /usr/include/bpf/xsk.h: In function ‘xsk_umem__extract_addr’:
> > /usr/include/bpf/xsk.h:173:16: error: ‘XSK_UNALIGNED_BUF_ADDR_MASK’ undeclared (first use in this function)
> >   173 |  return addr & XSK_UNALIGNED_BUF_ADDR_MASK;
> >       |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> > /usr/include/bpf/xsk.h: In function ‘xsk_umem__extract_offset’:
> > /usr/include/bpf/xsk.h:178:17: error: ‘XSK_UNALIGNED_BUF_OFFSET_SHIFT’ undeclared (first use in this function)
> >   178 |  return addr >> XSK_UNALIGNED_BUF_OFFSET_SHIFT;
> >       |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> >
> >
> > How would you prefer to handle this? A patch like the one below will fix
> > the compile errors, but I'm not sure it makes sense semantically?
>
> Thanks Toke for finding this. Of course it should be possible to
> compile this on an older kernel, but without getting any of the newer
> functionality that is not present in that older kernel.

Is the plan to support source compatibility for the headers only, or
the whole the libbpf itself? Is the usecase here, that you've built
libbpf.so with system headers X, and then would like to use the
library on a system with older system headers X~10? XDP sockets? BTF?

Re: [PATCH bpf-next v3] libbpf: fix compatibility for kernels without need_wakeup

[Magnus Karlsson]

On Thu, Oct 31, 2019 at 9:03 AM Björn Töpel <bjorn.topel@gmail.com> wrote:
>
> On Thu, 31 Oct 2019 at 08:17, Magnus Karlsson <magnus.karlsson@gmail.com> wrote:
> >
> > On Wed, Oct 30, 2019 at 2:36 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> > >
> > > Magnus Karlsson <magnus.karlsson@intel.com> writes:
> > >
> > > > When the need_wakeup flag was added to AF_XDP, the format of the
> > > > XDP_MMAP_OFFSETS getsockopt was extended. Code was added to the
> > > > kernel to take care of compatibility issues arrising from running
> > > > applications using any of the two formats. However, libbpf was
> > > > not extended to take care of the case when the application/libbpf
> > > > uses the new format but the kernel only supports the old
> > > > format. This patch adds support in libbpf for parsing the old
> > > > format, before the need_wakeup flag was added, and emulating a
> > > > set of static need_wakeup flags that will always work for the
> > > > application.
> > >
> > > Hi Magnus
> > >
> > > While you're looking at backwards compatibility issues with xsk: libbpf
> > > currently fails to compile on a system that has old kernel headers
> > > installed (this is with kernel-headers 5.3):
> > >
> > > $ echo "#include <bpf/xsk.h>" | gcc -x c -
> > > In file included from <stdin>:1:
> > > /usr/include/bpf/xsk.h: In function ‘xsk_ring_prod__needs_wakeup’:
> > > /usr/include/bpf/xsk.h:82:21: error: ‘XDP_RING_NEED_WAKEUP’ undeclared (first use in this function)
> > >    82 |  return *r->flags & XDP_RING_NEED_WAKEUP;
> > >       |                     ^~~~~~~~~~~~~~~~~~~~
> > > /usr/include/bpf/xsk.h:82:21: note: each undeclared identifier is reported only once for each function it appears in
> > > /usr/include/bpf/xsk.h: In function ‘xsk_umem__extract_addr’:
> > > /usr/include/bpf/xsk.h:173:16: error: ‘XSK_UNALIGNED_BUF_ADDR_MASK’ undeclared (first use in this function)
> > >   173 |  return addr & XSK_UNALIGNED_BUF_ADDR_MASK;
> > >       |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > /usr/include/bpf/xsk.h: In function ‘xsk_umem__extract_offset’:
> > > /usr/include/bpf/xsk.h:178:17: error: ‘XSK_UNALIGNED_BUF_OFFSET_SHIFT’ undeclared (first use in this function)
> > >   178 |  return addr >> XSK_UNALIGNED_BUF_OFFSET_SHIFT;
> > >       |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > >
> > >
> > >
> > > How would you prefer to handle this? A patch like the one below will fix
> > > the compile errors, but I'm not sure it makes sense semantically?
> >
> > Thanks Toke for finding this. Of course it should be possible to
> > compile this on an older kernel, but without getting any of the newer
> > functionality that is not present in that older kernel.
>
> Is the plan to support source compatibility for the headers only, or
> the whole the libbpf itself? Is the usecase here, that you've built
> libbpf.so with system headers X, and then would like to use the
> library on a system with older system headers X~10? XDP sockets? BTF?

Good question. I let someone with more insight answer this. Providing
the support Toke wants does make the header files less pleasant to
look at for sure. But in any case, I think we should provide an error
when you try to enable a new kernel feature using an old libbpf that
has no support for it. Just in case someone mixes things up.

Re: [PATCH bpf-next v3] libbpf: fix compatibility for kernels without need_wakeup

[Toke Høiland-Jørgensen]

Magnus Karlsson <magnus.karlsson@gmail.com> writes:

> On Thu, Oct 31, 2019 at 9:03 AM Björn Töpel <bjorn.topel@gmail.com> wrote:
>>
>> On Thu, 31 Oct 2019 at 08:17, Magnus Karlsson <magnus.karlsson@gmail.com> wrote:
>> >
>> > On Wed, Oct 30, 2019 at 2:36 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>> > >
>> > > Magnus Karlsson <magnus.karlsson@intel.com> writes:
>> > >
>> > > > When the need_wakeup flag was added to AF_XDP, the format of the
>> > > > XDP_MMAP_OFFSETS getsockopt was extended. Code was added to the
>> > > > kernel to take care of compatibility issues arrising from running
>> > > > applications using any of the two formats. However, libbpf was
>> > > > not extended to take care of the case when the application/libbpf
>> > > > uses the new format but the kernel only supports the old
>> > > > format. This patch adds support in libbpf for parsing the old
>> > > > format, before the need_wakeup flag was added, and emulating a
>> > > > set of static need_wakeup flags that will always work for the
>> > > > application.
>> > >
>> > > Hi Magnus
>> > >
>> > > While you're looking at backwards compatibility issues with xsk: libbpf
>> > > currently fails to compile on a system that has old kernel headers
>> > > installed (this is with kernel-headers 5.3):
>> > >
>> > > $ echo "#include <bpf/xsk.h>" | gcc -x c -
>> > > In file included from <stdin>:1:
>> > > /usr/include/bpf/xsk.h: In function ‘xsk_ring_prod__needs_wakeup’:
>> > > /usr/include/bpf/xsk.h:82:21: error: ‘XDP_RING_NEED_WAKEUP’ undeclared (first use in this function)
>> > >    82 |  return *r->flags & XDP_RING_NEED_WAKEUP;
>> > >       |                     ^~~~~~~~~~~~~~~~~~~~
>> > > /usr/include/bpf/xsk.h:82:21: note: each undeclared identifier is reported only once for each function it appears in
>> > > /usr/include/bpf/xsk.h: In function ‘xsk_umem__extract_addr’:
>> > > /usr/include/bpf/xsk.h:173:16: error: ‘XSK_UNALIGNED_BUF_ADDR_MASK’ undeclared (first use in this function)
>> > >   173 |  return addr & XSK_UNALIGNED_BUF_ADDR_MASK;
>> > >       |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>> > > /usr/include/bpf/xsk.h: In function ‘xsk_umem__extract_offset’:
>> > > /usr/include/bpf/xsk.h:178:17: error: ‘XSK_UNALIGNED_BUF_OFFSET_SHIFT’ undeclared (first use in this function)
>> > >   178 |  return addr >> XSK_UNALIGNED_BUF_OFFSET_SHIFT;
>> > >       |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> > >
>> > >
>> > >
>> > > How would you prefer to handle this? A patch like the one below will fix
>> > > the compile errors, but I'm not sure it makes sense semantically?
>> >
>> > Thanks Toke for finding this. Of course it should be possible to
>> > compile this on an older kernel, but without getting any of the newer
>> > functionality that is not present in that older kernel.
>>
>> Is the plan to support source compatibility for the headers only, or
>> the whole the libbpf itself? Is the usecase here, that you've built
>> libbpf.so with system headers X, and then would like to use the
>> library on a system with older system headers X~10? XDP sockets? BTF?
>
> Good question. I let someone with more insight answer this. Providing
> the support Toke wants does make the header files less pleasant to
> look at for sure. But in any case, I think we should provide an error
> when you try to enable a new kernel feature using an old libbpf that
> has no support for it. Just in case someone mixes things up.

Yup, I agree. Removing the functions completely is fine with me. As for
the flags, I agree that having a check in libbpf would make sense; I can
see someone upgrading their kernel, but still using the distro-specified
libbpf and running into weird errors otherwise.

Maybe we should define XDP_FLAGS_ALL in if_xdp.h and use that for the
check in both libbpf and the kernel? We'd still need conditional defines
for backwards compatibility, but at least we wouldn't need to keep
updating that as new flags are added?

-Toke

Re: [PATCH bpf-next v3] libbpf: fix compatibility for kernels without need_wakeup

[Alexei Starovoitov]

On Thu, Oct 31, 2019 at 1:03 AM Björn Töpel <bjorn.topel@gmail.com> wrote:
>
> On Thu, 31 Oct 2019 at 08:17, Magnus Karlsson <magnus.karlsson@gmail.com> wrote:
> >
> > On Wed, Oct 30, 2019 at 2:36 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> > >
> > > Magnus Karlsson <magnus.karlsson@intel.com> writes:
> > >
> > > > When the need_wakeup flag was added to AF_XDP, the format of the
> > > > XDP_MMAP_OFFSETS getsockopt was extended. Code was added to the
> > > > kernel to take care of compatibility issues arrising from running
> > > > applications using any of the two formats. However, libbpf was
> > > > not extended to take care of the case when the application/libbpf
> > > > uses the new format but the kernel only supports the old
> > > > format. This patch adds support in libbpf for parsing the old
> > > > format, before the need_wakeup flag was added, and emulating a
> > > > set of static need_wakeup flags that will always work for the
> > > > application.
> > >
> > > Hi Magnus
> > >
> > > While you're looking at backwards compatibility issues with xsk: libbpf
> > > currently fails to compile on a system that has old kernel headers
> > > installed (this is with kernel-headers 5.3):
> > >
> > > $ echo "#include <bpf/xsk.h>" | gcc -x c -
> > > In file included from <stdin>:1:
> > > /usr/include/bpf/xsk.h: In function ‘xsk_ring_prod__needs_wakeup’:
> > > /usr/include/bpf/xsk.h:82:21: error: ‘XDP_RING_NEED_WAKEUP’ undeclared (first use in this function)
> > >    82 |  return *r->flags & XDP_RING_NEED_WAKEUP;
> > >       |                     ^~~~~~~~~~~~~~~~~~~~
> > > /usr/include/bpf/xsk.h:82:21: note: each undeclared identifier is reported only once for each function it appears in
> > > /usr/include/bpf/xsk.h: In function ‘xsk_umem__extract_addr’:
> > > /usr/include/bpf/xsk.h:173:16: error: ‘XSK_UNALIGNED_BUF_ADDR_MASK’ undeclared (first use in this function)
> > >   173 |  return addr & XSK_UNALIGNED_BUF_ADDR_MASK;
> > >       |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > /usr/include/bpf/xsk.h: In function ‘xsk_umem__extract_offset’:
> > > /usr/include/bpf/xsk.h:178:17: error: ‘XSK_UNALIGNED_BUF_OFFSET_SHIFT’ undeclared (first use in this function)
> > >   178 |  return addr >> XSK_UNALIGNED_BUF_OFFSET_SHIFT;
> > >       |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > >
> > >
> > >
> > > How would you prefer to handle this? A patch like the one below will fix
> > > the compile errors, but I'm not sure it makes sense semantically?
> >
> > Thanks Toke for finding this. Of course it should be possible to
> > compile this on an older kernel, but without getting any of the newer
> > functionality that is not present in that older kernel.
>
> Is the plan to support source compatibility for the headers only, or
> the whole the libbpf itself? Is the usecase here, that you've built
> libbpf.so with system headers X, and then would like to use the
> library on a system with older system headers X~10? XDP sockets? BTF?

libbpf has to be backward and forward compatible.
Once compiled it has to run on older and newer kernels.
Conditional compilation is not an option obviously.

Re: [PATCH bpf-next v3] libbpf: fix compatibility for kernels without need_wakeup

[Toke Høiland-Jørgensen]

Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:

> On Thu, Oct 31, 2019 at 1:03 AM Björn Töpel <bjorn.topel@gmail.com> wrote:
>>
>> On Thu, 31 Oct 2019 at 08:17, Magnus Karlsson <magnus.karlsson@gmail.com> wrote:
>> >
>> > On Wed, Oct 30, 2019 at 2:36 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>> > >
>> > > Magnus Karlsson <magnus.karlsson@intel.com> writes:
>> > >
>> > > > When the need_wakeup flag was added to AF_XDP, the format of the
>> > > > XDP_MMAP_OFFSETS getsockopt was extended. Code was added to the
>> > > > kernel to take care of compatibility issues arrising from running
>> > > > applications using any of the two formats. However, libbpf was
>> > > > not extended to take care of the case when the application/libbpf
>> > > > uses the new format but the kernel only supports the old
>> > > > format. This patch adds support in libbpf for parsing the old
>> > > > format, before the need_wakeup flag was added, and emulating a
>> > > > set of static need_wakeup flags that will always work for the
>> > > > application.
>> > >
>> > > Hi Magnus
>> > >
>> > > While you're looking at backwards compatibility issues with xsk: libbpf
>> > > currently fails to compile on a system that has old kernel headers
>> > > installed (this is with kernel-headers 5.3):
>> > >
>> > > $ echo "#include <bpf/xsk.h>" | gcc -x c -
>> > > In file included from <stdin>:1:
>> > > /usr/include/bpf/xsk.h: In function ‘xsk_ring_prod__needs_wakeup’:
>> > > /usr/include/bpf/xsk.h:82:21: error: ‘XDP_RING_NEED_WAKEUP’ undeclared (first use in this function)
>> > >    82 |  return *r->flags & XDP_RING_NEED_WAKEUP;
>> > >       |                     ^~~~~~~~~~~~~~~~~~~~
>> > > /usr/include/bpf/xsk.h:82:21: note: each undeclared identifier is reported only once for each function it appears in
>> > > /usr/include/bpf/xsk.h: In function ‘xsk_umem__extract_addr’:
>> > > /usr/include/bpf/xsk.h:173:16: error: ‘XSK_UNALIGNED_BUF_ADDR_MASK’ undeclared (first use in this function)
>> > >   173 |  return addr & XSK_UNALIGNED_BUF_ADDR_MASK;
>> > >       |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>> > > /usr/include/bpf/xsk.h: In function ‘xsk_umem__extract_offset’:
>> > > /usr/include/bpf/xsk.h:178:17: error: ‘XSK_UNALIGNED_BUF_OFFSET_SHIFT’ undeclared (first use in this function)
>> > >   178 |  return addr >> XSK_UNALIGNED_BUF_OFFSET_SHIFT;
>> > >       |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> > >
>> > >
>> > >
>> > > How would you prefer to handle this? A patch like the one below will fix
>> > > the compile errors, but I'm not sure it makes sense semantically?
>> >
>> > Thanks Toke for finding this. Of course it should be possible to
>> > compile this on an older kernel, but without getting any of the newer
>> > functionality that is not present in that older kernel.
>>
>> Is the plan to support source compatibility for the headers only, or
>> the whole the libbpf itself? Is the usecase here, that you've built
>> libbpf.so with system headers X, and then would like to use the
>> library on a system with older system headers X~10? XDP sockets? BTF?
>
> libbpf has to be backward and forward compatible.
> Once compiled it has to run on older and newer kernels.
> Conditional compilation is not an option obviously.

So what do we do, then? Redefine the constants in libbpf/xsh.h if
they're not in the kernel header file?

-Toke

Re: [PATCH bpf-next v3] libbpf: fix compatibility for kernels without need_wakeup

[Alexei Starovoitov]

On Thu, Oct 31, 2019 at 7:13 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>
> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
>
> > On Thu, Oct 31, 2019 at 1:03 AM Björn Töpel <bjorn.topel@gmail.com> wrote:
> >>
> >> On Thu, 31 Oct 2019 at 08:17, Magnus Karlsson <magnus.karlsson@gmail.com> wrote:
> >> >
> >> > On Wed, Oct 30, 2019 at 2:36 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> >> > >
> >> > > Magnus Karlsson <magnus.karlsson@intel.com> writes:
> >> > >
> >> > > > When the need_wakeup flag was added to AF_XDP, the format of the
> >> > > > XDP_MMAP_OFFSETS getsockopt was extended. Code was added to the
> >> > > > kernel to take care of compatibility issues arrising from running
> >> > > > applications using any of the two formats. However, libbpf was
> >> > > > not extended to take care of the case when the application/libbpf
> >> > > > uses the new format but the kernel only supports the old
> >> > > > format. This patch adds support in libbpf for parsing the old
> >> > > > format, before the need_wakeup flag was added, and emulating a
> >> > > > set of static need_wakeup flags that will always work for the
> >> > > > application.
> >> > >
> >> > > Hi Magnus
> >> > >
> >> > > While you're looking at backwards compatibility issues with xsk: libbpf
> >> > > currently fails to compile on a system that has old kernel headers
> >> > > installed (this is with kernel-headers 5.3):
> >> > >
> >> > > $ echo "#include <bpf/xsk.h>" | gcc -x c -
> >> > > In file included from <stdin>:1:
> >> > > /usr/include/bpf/xsk.h: In function ‘xsk_ring_prod__needs_wakeup’:
> >> > > /usr/include/bpf/xsk.h:82:21: error: ‘XDP_RING_NEED_WAKEUP’ undeclared (first use in this function)
> >> > >    82 |  return *r->flags & XDP_RING_NEED_WAKEUP;
> >> > >       |                     ^~~~~~~~~~~~~~~~~~~~
> >> > > /usr/include/bpf/xsk.h:82:21: note: each undeclared identifier is reported only once for each function it appears in
> >> > > /usr/include/bpf/xsk.h: In function ‘xsk_umem__extract_addr’:
> >> > > /usr/include/bpf/xsk.h:173:16: error: ‘XSK_UNALIGNED_BUF_ADDR_MASK’ undeclared (first use in this function)
> >> > >   173 |  return addr & XSK_UNALIGNED_BUF_ADDR_MASK;
> >> > >       |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> >> > > /usr/include/bpf/xsk.h: In function ‘xsk_umem__extract_offset’:
> >> > > /usr/include/bpf/xsk.h:178:17: error: ‘XSK_UNALIGNED_BUF_OFFSET_SHIFT’ undeclared (first use in this function)
> >> > >   178 |  return addr >> XSK_UNALIGNED_BUF_OFFSET_SHIFT;
> >> > >       |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >> > >
> >> > >
> >> > >
> >> > > How would you prefer to handle this? A patch like the one below will fix
> >> > > the compile errors, but I'm not sure it makes sense semantically?
> >> >
> >> > Thanks Toke for finding this. Of course it should be possible to
> >> > compile this on an older kernel, but without getting any of the newer
> >> > functionality that is not present in that older kernel.
> >>
> >> Is the plan to support source compatibility for the headers only, or
> >> the whole the libbpf itself? Is the usecase here, that you've built
> >> libbpf.so with system headers X, and then would like to use the
> >> library on a system with older system headers X~10? XDP sockets? BTF?
> >
> > libbpf has to be backward and forward compatible.
> > Once compiled it has to run on older and newer kernels.
> > Conditional compilation is not an option obviously.
>
> So what do we do, then? Redefine the constants in libbpf/xsh.h if
> they're not in the kernel header file?

why? How and whom it will help?
To libbpf.rpm creating person or to end user?

Re: [PATCH bpf-next v3] libbpf: fix compatibility for kernels without need_wakeup

[Toke Høiland-Jørgensen]

Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:

> On Thu, Oct 31, 2019 at 7:13 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>>
>> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
>>
>> > On Thu, Oct 31, 2019 at 1:03 AM Björn Töpel <bjorn.topel@gmail.com> wrote:
>> >>
>> >> On Thu, 31 Oct 2019 at 08:17, Magnus Karlsson <magnus.karlsson@gmail.com> wrote:
>> >> >
>> >> > On Wed, Oct 30, 2019 at 2:36 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>> >> > >
>> >> > > Magnus Karlsson <magnus.karlsson@intel.com> writes:
>> >> > >
>> >> > > > When the need_wakeup flag was added to AF_XDP, the format of the
>> >> > > > XDP_MMAP_OFFSETS getsockopt was extended. Code was added to the
>> >> > > > kernel to take care of compatibility issues arrising from running
>> >> > > > applications using any of the two formats. However, libbpf was
>> >> > > > not extended to take care of the case when the application/libbpf
>> >> > > > uses the new format but the kernel only supports the old
>> >> > > > format. This patch adds support in libbpf for parsing the old
>> >> > > > format, before the need_wakeup flag was added, and emulating a
>> >> > > > set of static need_wakeup flags that will always work for the
>> >> > > > application.
>> >> > >
>> >> > > Hi Magnus
>> >> > >
>> >> > > While you're looking at backwards compatibility issues with xsk: libbpf
>> >> > > currently fails to compile on a system that has old kernel headers
>> >> > > installed (this is with kernel-headers 5.3):
>> >> > >
>> >> > > $ echo "#include <bpf/xsk.h>" | gcc -x c -
>> >> > > In file included from <stdin>:1:
>> >> > > /usr/include/bpf/xsk.h: In function ‘xsk_ring_prod__needs_wakeup’:
>> >> > > /usr/include/bpf/xsk.h:82:21: error: ‘XDP_RING_NEED_WAKEUP’ undeclared (first use in this function)
>> >> > >    82 |  return *r->flags & XDP_RING_NEED_WAKEUP;
>> >> > >       |                     ^~~~~~~~~~~~~~~~~~~~
>> >> > > /usr/include/bpf/xsk.h:82:21: note: each undeclared identifier is reported only once for each function it appears in
>> >> > > /usr/include/bpf/xsk.h: In function ‘xsk_umem__extract_addr’:
>> >> > > /usr/include/bpf/xsk.h:173:16: error: ‘XSK_UNALIGNED_BUF_ADDR_MASK’ undeclared (first use in this function)
>> >> > >   173 |  return addr & XSK_UNALIGNED_BUF_ADDR_MASK;
>> >> > >       |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>> >> > > /usr/include/bpf/xsk.h: In function ‘xsk_umem__extract_offset’:
>> >> > > /usr/include/bpf/xsk.h:178:17: error: ‘XSK_UNALIGNED_BUF_OFFSET_SHIFT’ undeclared (first use in this function)
>> >> > >   178 |  return addr >> XSK_UNALIGNED_BUF_OFFSET_SHIFT;
>> >> > >       |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> >> > >
>> >> > >
>> >> > >
>> >> > > How would you prefer to handle this? A patch like the one below will fix
>> >> > > the compile errors, but I'm not sure it makes sense semantically?
>> >> >
>> >> > Thanks Toke for finding this. Of course it should be possible to
>> >> > compile this on an older kernel, but without getting any of the newer
>> >> > functionality that is not present in that older kernel.
>> >>
>> >> Is the plan to support source compatibility for the headers only, or
>> >> the whole the libbpf itself? Is the usecase here, that you've built
>> >> libbpf.so with system headers X, and then would like to use the
>> >> library on a system with older system headers X~10? XDP sockets? BTF?
>> >
>> > libbpf has to be backward and forward compatible.
>> > Once compiled it has to run on older and newer kernels.
>> > Conditional compilation is not an option obviously.
>>
>> So what do we do, then? Redefine the constants in libbpf/xsh.h if
>> they're not in the kernel header file?
>
> why? How and whom it will help?
> To libbpf.rpm creating person or to end user?

Anyone who tries to compile a new libbpf against an older kernel. You're
saying yourself that "libbpf has to be backward and forward compatible".
Surely that extends to compile time as well as runtime?

-Toke

Re: [PATCH bpf-next v3] libbpf: fix compatibility for kernels without need_wakeup

[Alexei Starovoitov]

On Thu, Oct 31, 2019 at 7:26 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>
> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
>
> > On Thu, Oct 31, 2019 at 7:13 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> >>
> >> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
> >>
> >> > On Thu, Oct 31, 2019 at 1:03 AM Björn Töpel <bjorn.topel@gmail.com> wrote:
> >> >>
> >> >> On Thu, 31 Oct 2019 at 08:17, Magnus Karlsson <magnus.karlsson@gmail.com> wrote:
> >> >> >
> >> >> > On Wed, Oct 30, 2019 at 2:36 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> >> >> > >
> >> >> > > Magnus Karlsson <magnus.karlsson@intel.com> writes:
> >> >> > >
> >> >> > > > When the need_wakeup flag was added to AF_XDP, the format of the
> >> >> > > > XDP_MMAP_OFFSETS getsockopt was extended. Code was added to the
> >> >> > > > kernel to take care of compatibility issues arrising from running
> >> >> > > > applications using any of the two formats. However, libbpf was
> >> >> > > > not extended to take care of the case when the application/libbpf
> >> >> > > > uses the new format but the kernel only supports the old
> >> >> > > > format. This patch adds support in libbpf for parsing the old
> >> >> > > > format, before the need_wakeup flag was added, and emulating a
> >> >> > > > set of static need_wakeup flags that will always work for the
> >> >> > > > application.
> >> >> > >
> >> >> > > Hi Magnus
> >> >> > >
> >> >> > > While you're looking at backwards compatibility issues with xsk: libbpf
> >> >> > > currently fails to compile on a system that has old kernel headers
> >> >> > > installed (this is with kernel-headers 5.3):
> >> >> > >
> >> >> > > $ echo "#include <bpf/xsk.h>" | gcc -x c -
> >> >> > > In file included from <stdin>:1:
> >> >> > > /usr/include/bpf/xsk.h: In function ‘xsk_ring_prod__needs_wakeup’:
> >> >> > > /usr/include/bpf/xsk.h:82:21: error: ‘XDP_RING_NEED_WAKEUP’ undeclared (first use in this function)
> >> >> > >    82 |  return *r->flags & XDP_RING_NEED_WAKEUP;
> >> >> > >       |                     ^~~~~~~~~~~~~~~~~~~~
> >> >> > > /usr/include/bpf/xsk.h:82:21: note: each undeclared identifier is reported only once for each function it appears in
> >> >> > > /usr/include/bpf/xsk.h: In function ‘xsk_umem__extract_addr’:
> >> >> > > /usr/include/bpf/xsk.h:173:16: error: ‘XSK_UNALIGNED_BUF_ADDR_MASK’ undeclared (first use in this function)
> >> >> > >   173 |  return addr & XSK_UNALIGNED_BUF_ADDR_MASK;
> >> >> > >       |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> >> >> > > /usr/include/bpf/xsk.h: In function ‘xsk_umem__extract_offset’:
> >> >> > > /usr/include/bpf/xsk.h:178:17: error: ‘XSK_UNALIGNED_BUF_OFFSET_SHIFT’ undeclared (first use in this function)
> >> >> > >   178 |  return addr >> XSK_UNALIGNED_BUF_OFFSET_SHIFT;
> >> >> > >       |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >> >> > >
> >> >> > >
> >> >> > >
> >> >> > > How would you prefer to handle this? A patch like the one below will fix
> >> >> > > the compile errors, but I'm not sure it makes sense semantically?
> >> >> >
> >> >> > Thanks Toke for finding this. Of course it should be possible to
> >> >> > compile this on an older kernel, but without getting any of the newer
> >> >> > functionality that is not present in that older kernel.
> >> >>
> >> >> Is the plan to support source compatibility for the headers only, or
> >> >> the whole the libbpf itself? Is the usecase here, that you've built
> >> >> libbpf.so with system headers X, and then would like to use the
> >> >> library on a system with older system headers X~10? XDP sockets? BTF?
> >> >
> >> > libbpf has to be backward and forward compatible.
> >> > Once compiled it has to run on older and newer kernels.
> >> > Conditional compilation is not an option obviously.
> >>
> >> So what do we do, then? Redefine the constants in libbpf/xsh.h if
> >> they're not in the kernel header file?
> >
> > why? How and whom it will help?
> > To libbpf.rpm creating person or to end user?
>
> Anyone who tries to compile a new libbpf against an older kernel. You're
> saying yourself that "libbpf has to be backward and forward compatible".
> Surely that extends to compile time as well as runtime?

how old that older kernel?
Does it have up-to-date bpf.h in /usr/include ?
Also consider that running kernel is often not the same
thing as installed in /usr/include
vmlinux and /usr/include are different packages.

Re: [PATCH bpf-next v3] libbpf: fix compatibility for kernels without need_wakeup

[Toke Høiland-Jørgensen]

Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:

> On Thu, Oct 31, 2019 at 7:26 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>>
>> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
>>
>> > On Thu, Oct 31, 2019 at 7:13 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>> >>
>> >> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
>> >>
>> >> > On Thu, Oct 31, 2019 at 1:03 AM Björn Töpel <bjorn.topel@gmail.com> wrote:
>> >> >>
>> >> >> On Thu, 31 Oct 2019 at 08:17, Magnus Karlsson <magnus.karlsson@gmail.com> wrote:
>> >> >> >
>> >> >> > On Wed, Oct 30, 2019 at 2:36 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>> >> >> > >
>> >> >> > > Magnus Karlsson <magnus.karlsson@intel.com> writes:
>> >> >> > >
>> >> >> > > > When the need_wakeup flag was added to AF_XDP, the format of the
>> >> >> > > > XDP_MMAP_OFFSETS getsockopt was extended. Code was added to the
>> >> >> > > > kernel to take care of compatibility issues arrising from running
>> >> >> > > > applications using any of the two formats. However, libbpf was
>> >> >> > > > not extended to take care of the case when the application/libbpf
>> >> >> > > > uses the new format but the kernel only supports the old
>> >> >> > > > format. This patch adds support in libbpf for parsing the old
>> >> >> > > > format, before the need_wakeup flag was added, and emulating a
>> >> >> > > > set of static need_wakeup flags that will always work for the
>> >> >> > > > application.
>> >> >> > >
>> >> >> > > Hi Magnus
>> >> >> > >
>> >> >> > > While you're looking at backwards compatibility issues with xsk: libbpf
>> >> >> > > currently fails to compile on a system that has old kernel headers
>> >> >> > > installed (this is with kernel-headers 5.3):
>> >> >> > >
>> >> >> > > $ echo "#include <bpf/xsk.h>" | gcc -x c -
>> >> >> > > In file included from <stdin>:1:
>> >> >> > > /usr/include/bpf/xsk.h: In function ‘xsk_ring_prod__needs_wakeup’:
>> >> >> > > /usr/include/bpf/xsk.h:82:21: error: ‘XDP_RING_NEED_WAKEUP’ undeclared (first use in this function)
>> >> >> > >    82 |  return *r->flags & XDP_RING_NEED_WAKEUP;
>> >> >> > >       |                     ^~~~~~~~~~~~~~~~~~~~
>> >> >> > > /usr/include/bpf/xsk.h:82:21: note: each undeclared identifier is reported only once for each function it appears in
>> >> >> > > /usr/include/bpf/xsk.h: In function ‘xsk_umem__extract_addr’:
>> >> >> > > /usr/include/bpf/xsk.h:173:16: error: ‘XSK_UNALIGNED_BUF_ADDR_MASK’ undeclared (first use in this function)
>> >> >> > >   173 |  return addr & XSK_UNALIGNED_BUF_ADDR_MASK;
>> >> >> > >       |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>> >> >> > > /usr/include/bpf/xsk.h: In function ‘xsk_umem__extract_offset’:
>> >> >> > > /usr/include/bpf/xsk.h:178:17: error: ‘XSK_UNALIGNED_BUF_OFFSET_SHIFT’ undeclared (first use in this function)
>> >> >> > >   178 |  return addr >> XSK_UNALIGNED_BUF_OFFSET_SHIFT;
>> >> >> > >       |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> >> >> > >
>> >> >> > >
>> >> >> > >
>> >> >> > > How would you prefer to handle this? A patch like the one below will fix
>> >> >> > > the compile errors, but I'm not sure it makes sense semantically?
>> >> >> >
>> >> >> > Thanks Toke for finding this. Of course it should be possible to
>> >> >> > compile this on an older kernel, but without getting any of the newer
>> >> >> > functionality that is not present in that older kernel.
>> >> >>
>> >> >> Is the plan to support source compatibility for the headers only, or
>> >> >> the whole the libbpf itself? Is the usecase here, that you've built
>> >> >> libbpf.so with system headers X, and then would like to use the
>> >> >> library on a system with older system headers X~10? XDP sockets? BTF?
>> >> >
>> >> > libbpf has to be backward and forward compatible.
>> >> > Once compiled it has to run on older and newer kernels.
>> >> > Conditional compilation is not an option obviously.
>> >>
>> >> So what do we do, then? Redefine the constants in libbpf/xsh.h if
>> >> they're not in the kernel header file?
>> >
>> > why? How and whom it will help?
>> > To libbpf.rpm creating person or to end user?
>>
>> Anyone who tries to compile a new libbpf against an older kernel. You're
>> saying yourself that "libbpf has to be backward and forward compatible".
>> Surely that extends to compile time as well as runtime?
>
> how old that older kernel?
> Does it have up-to-date bpf.h in /usr/include ?
> Also consider that running kernel is often not the same
> thing as installed in /usr/include
> vmlinux and /usr/include are different packages.

In this case, it's a constant introduced in the kernel in the current
(5.4) cycle; so currently, you can't compile libbpf with
kernel-headers-5.3. And we're discussing how to handle this in a
backwards compatible way in libbpf...

-Toke

Re: [PATCH bpf-next v3] libbpf: fix compatibility for kernels without need_wakeup

[Alexei Starovoitov]

On Thu, Oct 31, 2019 at 7:52 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>
> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
>
> > On Thu, Oct 31, 2019 at 7:26 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> >>
> >> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
> >>
> >> > On Thu, Oct 31, 2019 at 7:13 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> >> >>
> >> >> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
> >> >>
> >> >> > On Thu, Oct 31, 2019 at 1:03 AM Björn Töpel <bjorn.topel@gmail.com> wrote:
> >> >> >>
> >> >> >> On Thu, 31 Oct 2019 at 08:17, Magnus Karlsson <magnus.karlsson@gmail.com> wrote:
> >> >> >> >
> >> >> >> > On Wed, Oct 30, 2019 at 2:36 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> >> >> >> > >
> >> >> >> > > Magnus Karlsson <magnus.karlsson@intel.com> writes:
> >> >> >> > >
> >> >> >> > > > When the need_wakeup flag was added to AF_XDP, the format of the
> >> >> >> > > > XDP_MMAP_OFFSETS getsockopt was extended. Code was added to the
> >> >> >> > > > kernel to take care of compatibility issues arrising from running
> >> >> >> > > > applications using any of the two formats. However, libbpf was
> >> >> >> > > > not extended to take care of the case when the application/libbpf
> >> >> >> > > > uses the new format but the kernel only supports the old
> >> >> >> > > > format. This patch adds support in libbpf for parsing the old
> >> >> >> > > > format, before the need_wakeup flag was added, and emulating a
> >> >> >> > > > set of static need_wakeup flags that will always work for the
> >> >> >> > > > application.
> >> >> >> > >
> >> >> >> > > Hi Magnus
> >> >> >> > >
> >> >> >> > > While you're looking at backwards compatibility issues with xsk: libbpf
> >> >> >> > > currently fails to compile on a system that has old kernel headers
> >> >> >> > > installed (this is with kernel-headers 5.3):
> >> >> >> > >
> >> >> >> > > $ echo "#include <bpf/xsk.h>" | gcc -x c -
> >> >> >> > > In file included from <stdin>:1:
> >> >> >> > > /usr/include/bpf/xsk.h: In function ‘xsk_ring_prod__needs_wakeup’:
> >> >> >> > > /usr/include/bpf/xsk.h:82:21: error: ‘XDP_RING_NEED_WAKEUP’ undeclared (first use in this function)
> >> >> >> > >    82 |  return *r->flags & XDP_RING_NEED_WAKEUP;
> >> >> >> > >       |                     ^~~~~~~~~~~~~~~~~~~~
> >> >> >> > > /usr/include/bpf/xsk.h:82:21: note: each undeclared identifier is reported only once for each function it appears in
> >> >> >> > > /usr/include/bpf/xsk.h: In function ‘xsk_umem__extract_addr’:
> >> >> >> > > /usr/include/bpf/xsk.h:173:16: error: ‘XSK_UNALIGNED_BUF_ADDR_MASK’ undeclared (first use in this function)
> >> >> >> > >   173 |  return addr & XSK_UNALIGNED_BUF_ADDR_MASK;
> >> >> >> > >       |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> >> >> >> > > /usr/include/bpf/xsk.h: In function ‘xsk_umem__extract_offset’:
> >> >> >> > > /usr/include/bpf/xsk.h:178:17: error: ‘XSK_UNALIGNED_BUF_OFFSET_SHIFT’ undeclared (first use in this function)
> >> >> >> > >   178 |  return addr >> XSK_UNALIGNED_BUF_OFFSET_SHIFT;
> >> >> >> > >       |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >> >> >> > >
> >> >> >> > >
> >> >> >> > >
> >> >> >> > > How would you prefer to handle this? A patch like the one below will fix
> >> >> >> > > the compile errors, but I'm not sure it makes sense semantically?
> >> >> >> >
> >> >> >> > Thanks Toke for finding this. Of course it should be possible to
> >> >> >> > compile this on an older kernel, but without getting any of the newer
> >> >> >> > functionality that is not present in that older kernel.
> >> >> >>
> >> >> >> Is the plan to support source compatibility for the headers only, or
> >> >> >> the whole the libbpf itself? Is the usecase here, that you've built
> >> >> >> libbpf.so with system headers X, and then would like to use the
> >> >> >> library on a system with older system headers X~10? XDP sockets? BTF?
> >> >> >
> >> >> > libbpf has to be backward and forward compatible.
> >> >> > Once compiled it has to run on older and newer kernels.
> >> >> > Conditional compilation is not an option obviously.
> >> >>
> >> >> So what do we do, then? Redefine the constants in libbpf/xsh.h if
> >> >> they're not in the kernel header file?
> >> >
> >> > why? How and whom it will help?
> >> > To libbpf.rpm creating person or to end user?
> >>
> >> Anyone who tries to compile a new libbpf against an older kernel. You're
> >> saying yourself that "libbpf has to be backward and forward compatible".
> >> Surely that extends to compile time as well as runtime?
> >
> > how old that older kernel?
> > Does it have up-to-date bpf.h in /usr/include ?
> > Also consider that running kernel is often not the same
> > thing as installed in /usr/include
> > vmlinux and /usr/include are different packages.
>
> In this case, it's a constant introduced in the kernel in the current
> (5.4) cycle; so currently, you can't compile libbpf with
> kernel-headers-5.3. And we're discussing how to handle this in a
> backwards compatible way in libbpf...

you simply don't.
It's not a problem to begin with.

Re: [PATCH bpf-next v3] libbpf: fix compatibility for kernels without need_wakeup

[Jiri Olsa]

On Thu, Oct 31, 2019 at 08:17:43AM -0700, Alexei Starovoitov wrote:
> On Thu, Oct 31, 2019 at 7:52 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> >
> > Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
> >
> > > On Thu, Oct 31, 2019 at 7:26 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> > >>
> > >> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
> > >>
> > >> > On Thu, Oct 31, 2019 at 7:13 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> > >> >>
> > >> >> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
> > >> >>
> > >> >> > On Thu, Oct 31, 2019 at 1:03 AM Björn Töpel <bjorn.topel@gmail.com> wrote:
> > >> >> >>
> > >> >> >> On Thu, 31 Oct 2019 at 08:17, Magnus Karlsson <magnus.karlsson@gmail.com> wrote:
> > >> >> >> >
> > >> >> >> > On Wed, Oct 30, 2019 at 2:36 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> > >> >> >> > >
> > >> >> >> > > Magnus Karlsson <magnus.karlsson@intel.com> writes:
> > >> >> >> > >
> > >> >> >> > > > When the need_wakeup flag was added to AF_XDP, the format of the
> > >> >> >> > > > XDP_MMAP_OFFSETS getsockopt was extended. Code was added to the
> > >> >> >> > > > kernel to take care of compatibility issues arrising from running
> > >> >> >> > > > applications using any of the two formats. However, libbpf was
> > >> >> >> > > > not extended to take care of the case when the application/libbpf
> > >> >> >> > > > uses the new format but the kernel only supports the old
> > >> >> >> > > > format. This patch adds support in libbpf for parsing the old
> > >> >> >> > > > format, before the need_wakeup flag was added, and emulating a
> > >> >> >> > > > set of static need_wakeup flags that will always work for the
> > >> >> >> > > > application.
> > >> >> >> > >
> > >> >> >> > > Hi Magnus
> > >> >> >> > >
> > >> >> >> > > While you're looking at backwards compatibility issues with xsk: libbpf
> > >> >> >> > > currently fails to compile on a system that has old kernel headers
> > >> >> >> > > installed (this is with kernel-headers 5.3):
> > >> >> >> > >
> > >> >> >> > > $ echo "#include <bpf/xsk.h>" | gcc -x c -
> > >> >> >> > > In file included from <stdin>:1:
> > >> >> >> > > /usr/include/bpf/xsk.h: In function ‘xsk_ring_prod__needs_wakeup’:
> > >> >> >> > > /usr/include/bpf/xsk.h:82:21: error: ‘XDP_RING_NEED_WAKEUP’ undeclared (first use in this function)
> > >> >> >> > >    82 |  return *r->flags & XDP_RING_NEED_WAKEUP;
> > >> >> >> > >       |                     ^~~~~~~~~~~~~~~~~~~~
> > >> >> >> > > /usr/include/bpf/xsk.h:82:21: note: each undeclared identifier is reported only once for each function it appears in
> > >> >> >> > > /usr/include/bpf/xsk.h: In function ‘xsk_umem__extract_addr’:
> > >> >> >> > > /usr/include/bpf/xsk.h:173:16: error: ‘XSK_UNALIGNED_BUF_ADDR_MASK’ undeclared (first use in this function)
> > >> >> >> > >   173 |  return addr & XSK_UNALIGNED_BUF_ADDR_MASK;
> > >> >> >> > >       |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> > >> >> >> > > /usr/include/bpf/xsk.h: In function ‘xsk_umem__extract_offset’:
> > >> >> >> > > /usr/include/bpf/xsk.h:178:17: error: ‘XSK_UNALIGNED_BUF_OFFSET_SHIFT’ undeclared (first use in this function)
> > >> >> >> > >   178 |  return addr >> XSK_UNALIGNED_BUF_OFFSET_SHIFT;
> > >> >> >> > >       |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > >> >> >> > >
> > >> >> >> > >
> > >> >> >> > >
> > >> >> >> > > How would you prefer to handle this? A patch like the one below will fix
> > >> >> >> > > the compile errors, but I'm not sure it makes sense semantically?
> > >> >> >> >
> > >> >> >> > Thanks Toke for finding this. Of course it should be possible to
> > >> >> >> > compile this on an older kernel, but without getting any of the newer
> > >> >> >> > functionality that is not present in that older kernel.
> > >> >> >>
> > >> >> >> Is the plan to support source compatibility for the headers only, or
> > >> >> >> the whole the libbpf itself? Is the usecase here, that you've built
> > >> >> >> libbpf.so with system headers X, and then would like to use the
> > >> >> >> library on a system with older system headers X~10? XDP sockets? BTF?
> > >> >> >
> > >> >> > libbpf has to be backward and forward compatible.
> > >> >> > Once compiled it has to run on older and newer kernels.
> > >> >> > Conditional compilation is not an option obviously.
> > >> >>
> > >> >> So what do we do, then? Redefine the constants in libbpf/xsh.h if
> > >> >> they're not in the kernel header file?
> > >> >
> > >> > why? How and whom it will help?
> > >> > To libbpf.rpm creating person or to end user?
> > >>
> > >> Anyone who tries to compile a new libbpf against an older kernel. You're
> > >> saying yourself that "libbpf has to be backward and forward compatible".
> > >> Surely that extends to compile time as well as runtime?
> > >
> > > how old that older kernel?
> > > Does it have up-to-date bpf.h in /usr/include ?
> > > Also consider that running kernel is often not the same
> > > thing as installed in /usr/include
> > > vmlinux and /usr/include are different packages.
> >
> > In this case, it's a constant introduced in the kernel in the current
> > (5.4) cycle; so currently, you can't compile libbpf with
> > kernel-headers-5.3. And we're discussing how to handle this in a
> > backwards compatible way in libbpf...
> 
> you simply don't.
> It's not a problem to begin with.

hum, that's possible case for distro users.. older kernel, newer libbpf

jirka

Re: [PATCH bpf-next v3] libbpf: fix compatibility for kernels without need_wakeup

[Alexei Starovoitov]

On Thu, Oct 31, 2019 at 10:42 AM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Thu, Oct 31, 2019 at 08:17:43AM -0700, Alexei Starovoitov wrote:
> > On Thu, Oct 31, 2019 at 7:52 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> > >
> > > Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
> > >
> > > > On Thu, Oct 31, 2019 at 7:26 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> > > >>
> > > >> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
> > > >>
> > > >> > On Thu, Oct 31, 2019 at 7:13 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> > > >> >>
> > > >> >> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
> > > >> >>
> > > >> >> > On Thu, Oct 31, 2019 at 1:03 AM Björn Töpel <bjorn.topel@gmail.com> wrote:
> > > >> >> >>
> > > >> >> >> On Thu, 31 Oct 2019 at 08:17, Magnus Karlsson <magnus.karlsson@gmail.com> wrote:
> > > >> >> >> >
> > > >> >> >> > On Wed, Oct 30, 2019 at 2:36 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> > > >> >> >> > >
> > > >> >> >> > > Magnus Karlsson <magnus.karlsson@intel.com> writes:
> > > >> >> >> > >
> > > >> >> >> > > > When the need_wakeup flag was added to AF_XDP, the format of the
> > > >> >> >> > > > XDP_MMAP_OFFSETS getsockopt was extended. Code was added to the
> > > >> >> >> > > > kernel to take care of compatibility issues arrising from running
> > > >> >> >> > > > applications using any of the two formats. However, libbpf was
> > > >> >> >> > > > not extended to take care of the case when the application/libbpf
> > > >> >> >> > > > uses the new format but the kernel only supports the old
> > > >> >> >> > > > format. This patch adds support in libbpf for parsing the old
> > > >> >> >> > > > format, before the need_wakeup flag was added, and emulating a
> > > >> >> >> > > > set of static need_wakeup flags that will always work for the
> > > >> >> >> > > > application.
> > > >> >> >> > >
> > > >> >> >> > > Hi Magnus
> > > >> >> >> > >
> > > >> >> >> > > While you're looking at backwards compatibility issues with xsk: libbpf
> > > >> >> >> > > currently fails to compile on a system that has old kernel headers
> > > >> >> >> > > installed (this is with kernel-headers 5.3):
> > > >> >> >> > >
> > > >> >> >> > > $ echo "#include <bpf/xsk.h>" | gcc -x c -
> > > >> >> >> > > In file included from <stdin>:1:
> > > >> >> >> > > /usr/include/bpf/xsk.h: In function ‘xsk_ring_prod__needs_wakeup’:
> > > >> >> >> > > /usr/include/bpf/xsk.h:82:21: error: ‘XDP_RING_NEED_WAKEUP’ undeclared (first use in this function)
> > > >> >> >> > >    82 |  return *r->flags & XDP_RING_NEED_WAKEUP;
> > > >> >> >> > >       |                     ^~~~~~~~~~~~~~~~~~~~
> > > >> >> >> > > /usr/include/bpf/xsk.h:82:21: note: each undeclared identifier is reported only once for each function it appears in
> > > >> >> >> > > /usr/include/bpf/xsk.h: In function ‘xsk_umem__extract_addr’:
> > > >> >> >> > > /usr/include/bpf/xsk.h:173:16: error: ‘XSK_UNALIGNED_BUF_ADDR_MASK’ undeclared (first use in this function)
> > > >> >> >> > >   173 |  return addr & XSK_UNALIGNED_BUF_ADDR_MASK;
> > > >> >> >> > >       |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > >> >> >> > > /usr/include/bpf/xsk.h: In function ‘xsk_umem__extract_offset’:
> > > >> >> >> > > /usr/include/bpf/xsk.h:178:17: error: ‘XSK_UNALIGNED_BUF_OFFSET_SHIFT’ undeclared (first use in this function)
> > > >> >> >> > >   178 |  return addr >> XSK_UNALIGNED_BUF_OFFSET_SHIFT;
> > > >> >> >> > >       |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > >> >> >> > >
> > > >> >> >> > >
> > > >> >> >> > >
> > > >> >> >> > > How would you prefer to handle this? A patch like the one below will fix
> > > >> >> >> > > the compile errors, but I'm not sure it makes sense semantically?
> > > >> >> >> >
> > > >> >> >> > Thanks Toke for finding this. Of course it should be possible to
> > > >> >> >> > compile this on an older kernel, but without getting any of the newer
> > > >> >> >> > functionality that is not present in that older kernel.
> > > >> >> >>
> > > >> >> >> Is the plan to support source compatibility for the headers only, or
> > > >> >> >> the whole the libbpf itself? Is the usecase here, that you've built
> > > >> >> >> libbpf.so with system headers X, and then would like to use the
> > > >> >> >> library on a system with older system headers X~10? XDP sockets? BTF?
> > > >> >> >
> > > >> >> > libbpf has to be backward and forward compatible.
> > > >> >> > Once compiled it has to run on older and newer kernels.
> > > >> >> > Conditional compilation is not an option obviously.
> > > >> >>
> > > >> >> So what do we do, then? Redefine the constants in libbpf/xsh.h if
> > > >> >> they're not in the kernel header file?
> > > >> >
> > > >> > why? How and whom it will help?
> > > >> > To libbpf.rpm creating person or to end user?
> > > >>
> > > >> Anyone who tries to compile a new libbpf against an older kernel. You're
> > > >> saying yourself that "libbpf has to be backward and forward compatible".
> > > >> Surely that extends to compile time as well as runtime?
> > > >
> > > > how old that older kernel?
> > > > Does it have up-to-date bpf.h in /usr/include ?
> > > > Also consider that running kernel is often not the same
> > > > thing as installed in /usr/include
> > > > vmlinux and /usr/include are different packages.
> > >
> > > In this case, it's a constant introduced in the kernel in the current
> > > (5.4) cycle; so currently, you can't compile libbpf with
> > > kernel-headers-5.3. And we're discussing how to handle this in a
> > > backwards compatible way in libbpf...
> >
> > you simply don't.
> > It's not a problem to begin with.
>
> hum, that's possible case for distro users.. older kernel, newer libbpf

yes. older vmlinux and newer installed libbpf.so
or any version of libbpf.a that is statically linked into apps
is something that libbpf code has to support.
The server can be rebooted into older than libbpf kernel and
into newer than libbpf kernel. libbpf has to recognize all these
combinations and work appropriately.
That's what backward and forward compatibility is.
That's what makes libbpf so difficult to test, develop and code review.
What that particular server has in /usr/include is irrelevant.

Re: [PATCH bpf-next v3] libbpf: fix compatibility for kernels without need_wakeup

[Jiri Olsa]

On Thu, Oct 31, 2019 at 11:19:21AM -0700, Alexei Starovoitov wrote:
> On Thu, Oct 31, 2019 at 10:42 AM Jiri Olsa <jolsa@redhat.com> wrote:
> >
> > On Thu, Oct 31, 2019 at 08:17:43AM -0700, Alexei Starovoitov wrote:
> > > On Thu, Oct 31, 2019 at 7:52 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> > > >
> > > > Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
> > > >
> > > > > On Thu, Oct 31, 2019 at 7:26 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> > > > >>
> > > > >> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
> > > > >>
> > > > >> > On Thu, Oct 31, 2019 at 7:13 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> > > > >> >>
> > > > >> >> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
> > > > >> >>
> > > > >> >> > On Thu, Oct 31, 2019 at 1:03 AM Björn Töpel <bjorn.topel@gmail.com> wrote:
> > > > >> >> >>
> > > > >> >> >> On Thu, 31 Oct 2019 at 08:17, Magnus Karlsson <magnus.karlsson@gmail.com> wrote:
> > > > >> >> >> >
> > > > >> >> >> > On Wed, Oct 30, 2019 at 2:36 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> > > > >> >> >> > >
> > > > >> >> >> > > Magnus Karlsson <magnus.karlsson@intel.com> writes:
> > > > >> >> >> > >
> > > > >> >> >> > > > When the need_wakeup flag was added to AF_XDP, the format of the
> > > > >> >> >> > > > XDP_MMAP_OFFSETS getsockopt was extended. Code was added to the
> > > > >> >> >> > > > kernel to take care of compatibility issues arrising from running
> > > > >> >> >> > > > applications using any of the two formats. However, libbpf was
> > > > >> >> >> > > > not extended to take care of the case when the application/libbpf
> > > > >> >> >> > > > uses the new format but the kernel only supports the old
> > > > >> >> >> > > > format. This patch adds support in libbpf for parsing the old
> > > > >> >> >> > > > format, before the need_wakeup flag was added, and emulating a
> > > > >> >> >> > > > set of static need_wakeup flags that will always work for the
> > > > >> >> >> > > > application.
> > > > >> >> >> > >
> > > > >> >> >> > > Hi Magnus
> > > > >> >> >> > >
> > > > >> >> >> > > While you're looking at backwards compatibility issues with xsk: libbpf
> > > > >> >> >> > > currently fails to compile on a system that has old kernel headers
> > > > >> >> >> > > installed (this is with kernel-headers 5.3):
> > > > >> >> >> > >
> > > > >> >> >> > > $ echo "#include <bpf/xsk.h>" | gcc -x c -
> > > > >> >> >> > > In file included from <stdin>:1:
> > > > >> >> >> > > /usr/include/bpf/xsk.h: In function ‘xsk_ring_prod__needs_wakeup’:
> > > > >> >> >> > > /usr/include/bpf/xsk.h:82:21: error: ‘XDP_RING_NEED_WAKEUP’ undeclared (first use in this function)
> > > > >> >> >> > >    82 |  return *r->flags & XDP_RING_NEED_WAKEUP;
> > > > >> >> >> > >       |                     ^~~~~~~~~~~~~~~~~~~~
> > > > >> >> >> > > /usr/include/bpf/xsk.h:82:21: note: each undeclared identifier is reported only once for each function it appears in
> > > > >> >> >> > > /usr/include/bpf/xsk.h: In function ‘xsk_umem__extract_addr’:
> > > > >> >> >> > > /usr/include/bpf/xsk.h:173:16: error: ‘XSK_UNALIGNED_BUF_ADDR_MASK’ undeclared (first use in this function)
> > > > >> >> >> > >   173 |  return addr & XSK_UNALIGNED_BUF_ADDR_MASK;
> > > > >> >> >> > >       |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > >> >> >> > > /usr/include/bpf/xsk.h: In function ‘xsk_umem__extract_offset’:
> > > > >> >> >> > > /usr/include/bpf/xsk.h:178:17: error: ‘XSK_UNALIGNED_BUF_OFFSET_SHIFT’ undeclared (first use in this function)
> > > > >> >> >> > >   178 |  return addr >> XSK_UNALIGNED_BUF_OFFSET_SHIFT;
> > > > >> >> >> > >       |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > >> >> >> > >
> > > > >> >> >> > >
> > > > >> >> >> > >
> > > > >> >> >> > > How would you prefer to handle this? A patch like the one below will fix
> > > > >> >> >> > > the compile errors, but I'm not sure it makes sense semantically?
> > > > >> >> >> >
> > > > >> >> >> > Thanks Toke for finding this. Of course it should be possible to
> > > > >> >> >> > compile this on an older kernel, but without getting any of the newer
> > > > >> >> >> > functionality that is not present in that older kernel.
> > > > >> >> >>
> > > > >> >> >> Is the plan to support source compatibility for the headers only, or
> > > > >> >> >> the whole the libbpf itself? Is the usecase here, that you've built
> > > > >> >> >> libbpf.so with system headers X, and then would like to use the
> > > > >> >> >> library on a system with older system headers X~10? XDP sockets? BTF?
> > > > >> >> >
> > > > >> >> > libbpf has to be backward and forward compatible.
> > > > >> >> > Once compiled it has to run on older and newer kernels.
> > > > >> >> > Conditional compilation is not an option obviously.
> > > > >> >>
> > > > >> >> So what do we do, then? Redefine the constants in libbpf/xsh.h if
> > > > >> >> they're not in the kernel header file?
> > > > >> >
> > > > >> > why? How and whom it will help?
> > > > >> > To libbpf.rpm creating person or to end user?
> > > > >>
> > > > >> Anyone who tries to compile a new libbpf against an older kernel. You're
> > > > >> saying yourself that "libbpf has to be backward and forward compatible".
> > > > >> Surely that extends to compile time as well as runtime?
> > > > >
> > > > > how old that older kernel?
> > > > > Does it have up-to-date bpf.h in /usr/include ?
> > > > > Also consider that running kernel is often not the same
> > > > > thing as installed in /usr/include
> > > > > vmlinux and /usr/include are different packages.
> > > >
> > > > In this case, it's a constant introduced in the kernel in the current
> > > > (5.4) cycle; so currently, you can't compile libbpf with
> > > > kernel-headers-5.3. And we're discussing how to handle this in a
> > > > backwards compatible way in libbpf...
> > >
> > > you simply don't.
> > > It's not a problem to begin with.
> >
> > hum, that's possible case for distro users.. older kernel, newer libbpf
> 
> yes. older vmlinux and newer installed libbpf.so
> or any version of libbpf.a that is statically linked into apps
> is something that libbpf code has to support.
> The server can be rebooted into older than libbpf kernel and
> into newer than libbpf kernel. libbpf has to recognize all these
> combinations and work appropriately.
> That's what backward and forward compatibility is.
> That's what makes libbpf so difficult to test, develop and code review.
> What that particular server has in /usr/include is irrelevant.

sure, anyway we can't compile following:

	tredaell@aldebaran ~ $ echo "#include <bpf/xsk.h>" | gcc -x c - 
	In file included from <stdin>:1:
	/usr/include/bpf/xsk.h: In function ‘xsk_ring_prod__needs_wakeup’:
	/usr/include/bpf/xsk.h:82:21: error: ‘XDP_RING_NEED_WAKEUP’ undeclared (first use in this function)
	   82 |  return *r->flags & XDP_RING_NEED_WAKEUP;
	...

	XDP_RING_NEED_WAKEUP is defined in kernel v5.4-rc1 (77cd0d7b3f257fd0e3096b4fdcff1a7d38e99e10).
	XSK_UNALIGNED_BUF_ADDR_MASK and XSK_UNALIGNED_BUF_OFFSET_SHIFT are defined in kernel v5.4-rc1 (c05cd3645814724bdeb32a2b4d953b12bdea5f8c).

with:
  kernel-headers-5.3.6-300.fc31.x86_64
  libbpf-0.0.5-1.fc31.x86_64

if you're saying this is not supported, I guess we could be postponing
libbpf rpm releases until we have the related fedora kernel released

or how about inluding uapi headers in libbpf-devel.. but that might
actualy cause more confusion

jirka

Re: [PATCH bpf-next v3] libbpf: fix compatibility for kernels without need_wakeup

[Andrii Nakryiko]

On Thu, Oct 31, 2019 at 8:18 AM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Thu, Oct 31, 2019 at 7:52 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> >
> > Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
> >
> > > On Thu, Oct 31, 2019 at 7:26 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> > >>
> > >> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
> > >>
> > >> > On Thu, Oct 31, 2019 at 7:13 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> > >> >>
> > >> >> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
> > >> >>
> > >> >> > On Thu, Oct 31, 2019 at 1:03 AM Björn Töpel <bjorn.topel@gmail.com> wrote:
> > >> >> >>
> > >> >> >> On Thu, 31 Oct 2019 at 08:17, Magnus Karlsson <magnus.karlsson@gmail.com> wrote:
> > >> >> >> >
> > >> >> >> > On Wed, Oct 30, 2019 at 2:36 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> > >> >> >> > >
> > >> >> >> > > Magnus Karlsson <magnus.karlsson@intel.com> writes:
> > >> >> >> > >
> > >> >> >> > > > When the need_wakeup flag was added to AF_XDP, the format of the
> > >> >> >> > > > XDP_MMAP_OFFSETS getsockopt was extended. Code was added to the
> > >> >> >> > > > kernel to take care of compatibility issues arrising from running
> > >> >> >> > > > applications using any of the two formats. However, libbpf was
> > >> >> >> > > > not extended to take care of the case when the application/libbpf
> > >> >> >> > > > uses the new format but the kernel only supports the old
> > >> >> >> > > > format. This patch adds support in libbpf for parsing the old
> > >> >> >> > > > format, before the need_wakeup flag was added, and emulating a
> > >> >> >> > > > set of static need_wakeup flags that will always work for the
> > >> >> >> > > > application.
> > >> >> >> > >
> > >> >> >> > > Hi Magnus
> > >> >> >> > >
> > >> >> >> > > While you're looking at backwards compatibility issues with xsk: libbpf
> > >> >> >> > > currently fails to compile on a system that has old kernel headers
> > >> >> >> > > installed (this is with kernel-headers 5.3):
> > >> >> >> > >
> > >> >> >> > > $ echo "#include <bpf/xsk.h>" | gcc -x c -
> > >> >> >> > > In file included from <stdin>:1:
> > >> >> >> > > /usr/include/bpf/xsk.h: In function ‘xsk_ring_prod__needs_wakeup’:
> > >> >> >> > > /usr/include/bpf/xsk.h:82:21: error: ‘XDP_RING_NEED_WAKEUP’ undeclared (first use in this function)
> > >> >> >> > >    82 |  return *r->flags & XDP_RING_NEED_WAKEUP;
> > >> >> >> > >       |                     ^~~~~~~~~~~~~~~~~~~~
> > >> >> >> > > /usr/include/bpf/xsk.h:82:21: note: each undeclared identifier is reported only once for each function it appears in
> > >> >> >> > > /usr/include/bpf/xsk.h: In function ‘xsk_umem__extract_addr’:
> > >> >> >> > > /usr/include/bpf/xsk.h:173:16: error: ‘XSK_UNALIGNED_BUF_ADDR_MASK’ undeclared (first use in this function)
> > >> >> >> > >   173 |  return addr & XSK_UNALIGNED_BUF_ADDR_MASK;
> > >> >> >> > >       |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> > >> >> >> > > /usr/include/bpf/xsk.h: In function ‘xsk_umem__extract_offset’:
> > >> >> >> > > /usr/include/bpf/xsk.h:178:17: error: ‘XSK_UNALIGNED_BUF_OFFSET_SHIFT’ undeclared (first use in this function)
> > >> >> >> > >   178 |  return addr >> XSK_UNALIGNED_BUF_OFFSET_SHIFT;
> > >> >> >> > >       |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > >> >> >> > >
> > >> >> >> > >
> > >> >> >> > >
> > >> >> >> > > How would you prefer to handle this? A patch like the one below will fix
> > >> >> >> > > the compile errors, but I'm not sure it makes sense semantically?
> > >> >> >> >
> > >> >> >> > Thanks Toke for finding this. Of course it should be possible to
> > >> >> >> > compile this on an older kernel, but without getting any of the newer
> > >> >> >> > functionality that is not present in that older kernel.
> > >> >> >>
> > >> >> >> Is the plan to support source compatibility for the headers only, or
> > >> >> >> the whole the libbpf itself? Is the usecase here, that you've built
> > >> >> >> libbpf.so with system headers X, and then would like to use the
> > >> >> >> library on a system with older system headers X~10? XDP sockets? BTF?
> > >> >> >
> > >> >> > libbpf has to be backward and forward compatible.
> > >> >> > Once compiled it has to run on older and newer kernels.
> > >> >> > Conditional compilation is not an option obviously.
> > >> >>
> > >> >> So what do we do, then? Redefine the constants in libbpf/xsh.h if
> > >> >> they're not in the kernel header file?
> > >> >
> > >> > why? How and whom it will help?
> > >> > To libbpf.rpm creating person or to end user?
> > >>
> > >> Anyone who tries to compile a new libbpf against an older kernel. You're
> > >> saying yourself that "libbpf has to be backward and forward compatible".
> > >> Surely that extends to compile time as well as runtime?
> > >
> > > how old that older kernel?
> > > Does it have up-to-date bpf.h in /usr/include ?
> > > Also consider that running kernel is often not the same
> > > thing as installed in /usr/include
> > > vmlinux and /usr/include are different packages.
> >
> > In this case, it's a constant introduced in the kernel in the current
> > (5.4) cycle; so currently, you can't compile libbpf with
> > kernel-headers-5.3. And we're discussing how to handle this in a
> > backwards compatible way in libbpf...

If someone is compiling libbpf from sources, he has to make sure to
use libbpf's include directory (see
https://github.com/libbpf/libbpf/blob/master/include/uapi/linux/if_xdp.h),
it has all the latest XDP stuff, including XDP_RING_NEED_WAKEUP.

>
> you simply don't.
> It's not a problem to begin with.

Re: [PATCH bpf-next v3] libbpf: fix compatibility for kernels without need_wakeup

[Alexei Starovoitov]

On Thu, Oct 31, 2019 at 12:18 PM Jiri Olsa <jolsa@redhat.com> wrote:
> >
> > yes. older vmlinux and newer installed libbpf.so
> > or any version of libbpf.a that is statically linked into apps
> > is something that libbpf code has to support.
> > The server can be rebooted into older than libbpf kernel and
> > into newer than libbpf kernel. libbpf has to recognize all these
> > combinations and work appropriately.
> > That's what backward and forward compatibility is.
> > That's what makes libbpf so difficult to test, develop and code review.
> > What that particular server has in /usr/include is irrelevant.
>
> sure, anyway we can't compile following:
>
>         tredaell@aldebaran ~ $ echo "#include <bpf/xsk.h>" | gcc -x c -
>         In file included from <stdin>:1:
>         /usr/include/bpf/xsk.h: In function ‘xsk_ring_prod__needs_wakeup’:
>         /usr/include/bpf/xsk.h:82:21: error: ‘XDP_RING_NEED_WAKEUP’ undeclared (first use in this function)
>            82 |  return *r->flags & XDP_RING_NEED_WAKEUP;
>         ...
>
>         XDP_RING_NEED_WAKEUP is defined in kernel v5.4-rc1 (77cd0d7b3f257fd0e3096b4fdcff1a7d38e99e10).
>         XSK_UNALIGNED_BUF_ADDR_MASK and XSK_UNALIGNED_BUF_OFFSET_SHIFT are defined in kernel v5.4-rc1 (c05cd3645814724bdeb32a2b4d953b12bdea5f8c).
>
> with:
>   kernel-headers-5.3.6-300.fc31.x86_64
>   libbpf-0.0.5-1.fc31.x86_64
>
> if you're saying this is not supported, I guess we could be postponing
> libbpf rpm releases until we have the related fedora kernel released

why? github/libbpf is the source of truth for building packages
and afaik it builds fine.

> or how about inluding uapi headers in libbpf-devel.. but that might
> actualy cause more confusion

Libraries (libbpf or any other) should not install headers that
typically go into /usr/include/
if_xdp.h case is not unique.
We'll surely add another #define, enum, etc to uapi/linux/bpf.h tomorrow.
And we will not copy paste these constants and types into tools/lib/bpf/.
In kernel tree libbpf development is using kernel tree headers.
No problem there for libbpf developers.
Packages are built out of github/libbpf that has a copy of uapi headers
necessary to create packages.
No problem there for package builders either.
But libbpf package is not going to install those uapi headers.
libbpf package installs only libbpf own headers (like libbpf.h)
The users that want to build against the latest libbpf package need
to install corresponding uapi headers package.
I don't think such dependency is specified in rpm scripts.
May be it is something to fix? Or may be not.
Some folks might not want to update all of /usr/include to bring libbpf-devel.
Then it would be their responsibility to get fresh /usr/include headers.

Re: [PATCH bpf-next v3] libbpf: fix compatibility for kernels without need_wakeup

[Jiri Olsa]

On Thu, Oct 31, 2019 at 01:39:12PM -0700, Alexei Starovoitov wrote:
> On Thu, Oct 31, 2019 at 12:18 PM Jiri Olsa <jolsa@redhat.com> wrote:
> > >
> > > yes. older vmlinux and newer installed libbpf.so
> > > or any version of libbpf.a that is statically linked into apps
> > > is something that libbpf code has to support.
> > > The server can be rebooted into older than libbpf kernel and
> > > into newer than libbpf kernel. libbpf has to recognize all these
> > > combinations and work appropriately.
> > > That's what backward and forward compatibility is.
> > > That's what makes libbpf so difficult to test, develop and code review.
> > > What that particular server has in /usr/include is irrelevant.
> >
> > sure, anyway we can't compile following:
> >
> >         tredaell@aldebaran ~ $ echo "#include <bpf/xsk.h>" | gcc -x c -
> >         In file included from <stdin>:1:
> >         /usr/include/bpf/xsk.h: In function ‘xsk_ring_prod__needs_wakeup’:
> >         /usr/include/bpf/xsk.h:82:21: error: ‘XDP_RING_NEED_WAKEUP’ undeclared (first use in this function)
> >            82 |  return *r->flags & XDP_RING_NEED_WAKEUP;
> >         ...
> >
> >         XDP_RING_NEED_WAKEUP is defined in kernel v5.4-rc1 (77cd0d7b3f257fd0e3096b4fdcff1a7d38e99e10).
> >         XSK_UNALIGNED_BUF_ADDR_MASK and XSK_UNALIGNED_BUF_OFFSET_SHIFT are defined in kernel v5.4-rc1 (c05cd3645814724bdeb32a2b4d953b12bdea5f8c).
> >
> > with:
> >   kernel-headers-5.3.6-300.fc31.x86_64
> >   libbpf-0.0.5-1.fc31.x86_64
> >
> > if you're saying this is not supported, I guess we could be postponing
> > libbpf rpm releases until we have the related fedora kernel released
> 
> why? github/libbpf is the source of truth for building packages
> and afaik it builds fine.

because we will get issues like above if there's no kernel
avilable that we could compile libbpf against

> 
> > or how about inluding uapi headers in libbpf-devel.. but that might
> > actualy cause more confusion
> 
> Libraries (libbpf or any other) should not install headers that
> typically go into /usr/include/
> if_xdp.h case is not unique.
> We'll surely add another #define, enum, etc to uapi/linux/bpf.h tomorrow.
> And we will not copy paste these constants and types into tools/lib/bpf/.
> In kernel tree libbpf development is using kernel tree headers.
> No problem there for libbpf developers.
> Packages are built out of github/libbpf that has a copy of uapi headers
> necessary to create packages.
> No problem there for package builders either.
> But libbpf package is not going to install those uapi headers.
> libbpf package installs only libbpf own headers (like libbpf.h)
> The users that want to build against the latest libbpf package need
> to install corresponding uapi headers package.
> I don't think such dependency is specified in rpm scripts.
> May be it is something to fix? Or may be not.

I'll check if we can add some kernel version/package dependency

> Some folks might not want to update all of /usr/include to bring libbpf-devel.
> Then it would be their responsibility to get fresh /usr/include headers.

jirka

Re: [PATCH bpf-next v3] libbpf: fix compatibility for kernels without need_wakeup

[Toke Høiland-Jørgensen]

Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:

> On Thu, Oct 31, 2019 at 12:18 PM Jiri Olsa <jolsa@redhat.com> wrote:
>> >
>> > yes. older vmlinux and newer installed libbpf.so
>> > or any version of libbpf.a that is statically linked into apps
>> > is something that libbpf code has to support.
>> > The server can be rebooted into older than libbpf kernel and
>> > into newer than libbpf kernel. libbpf has to recognize all these
>> > combinations and work appropriately.
>> > That's what backward and forward compatibility is.
>> > That's what makes libbpf so difficult to test, develop and code review.
>> > What that particular server has in /usr/include is irrelevant.
>>
>> sure, anyway we can't compile following:
>>
>>         tredaell@aldebaran ~ $ echo "#include <bpf/xsk.h>" | gcc -x c -
>>         In file included from <stdin>:1:
>>         /usr/include/bpf/xsk.h: In function ‘xsk_ring_prod__needs_wakeup’:
>>         /usr/include/bpf/xsk.h:82:21: error: ‘XDP_RING_NEED_WAKEUP’ undeclared (first use in this function)
>>            82 |  return *r->flags & XDP_RING_NEED_WAKEUP;
>>         ...
>>
>>         XDP_RING_NEED_WAKEUP is defined in kernel v5.4-rc1 (77cd0d7b3f257fd0e3096b4fdcff1a7d38e99e10).
>>         XSK_UNALIGNED_BUF_ADDR_MASK and XSK_UNALIGNED_BUF_OFFSET_SHIFT are defined in kernel v5.4-rc1 (c05cd3645814724bdeb32a2b4d953b12bdea5f8c).
>>
>> with:
>>   kernel-headers-5.3.6-300.fc31.x86_64
>>   libbpf-0.0.5-1.fc31.x86_64
>>
>> if you're saying this is not supported, I guess we could be postponing
>> libbpf rpm releases until we have the related fedora kernel released
>
> why? github/libbpf is the source of truth for building packages
> and afaik it builds fine.
>
>> or how about inluding uapi headers in libbpf-devel.. but that might
>> actualy cause more confusion
>
> Libraries (libbpf or any other) should not install headers that
> typically go into /usr/include/
> if_xdp.h case is not unique.
> We'll surely add another #define, enum, etc to uapi/linux/bpf.h tomorrow.
> And we will not copy paste these constants and types into tools/lib/bpf/.
> In kernel tree libbpf development is using kernel tree headers.
> No problem there for libbpf developers.
> Packages are built out of github/libbpf that has a copy of uapi headers
> necessary to create packages.
> No problem there for package builders either.
> But libbpf package is not going to install those uapi headers.
> libbpf package installs only libbpf own headers (like libbpf.h)
> The users that want to build against the latest libbpf package need
> to install corresponding uapi headers package.
> I don't think such dependency is specified in rpm scripts.
> May be it is something to fix? Or may be not.
> Some folks might not want to update all of /usr/include to bring libbpf-devel.
> Then it would be their responsibility to get fresh /usr/include headers.

We can certainly tie libbpf to the kernel version. The obvious way to do
that is to just ship the version of libbpf that's in the kernel tree of
whatever kernel version the distro ships. But how will we handle
bugfixes, then? You've explicitly stated that libbpf gets no bugfixes
outside of bpf-next...

-Toke

Re: [PATCH bpf-next v3] libbpf: fix compatibility for kernels without need_wakeup

[John Fastabend]

Toke Høiland-Jørgensen wrote:
> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
> 
> > On Thu, Oct 31, 2019 at 12:18 PM Jiri Olsa <jolsa@redhat.com> wrote:
> >> >
> >> > yes. older vmlinux and newer installed libbpf.so
> >> > or any version of libbpf.a that is statically linked into apps
> >> > is something that libbpf code has to support.
> >> > The server can be rebooted into older than libbpf kernel and
> >> > into newer than libbpf kernel. libbpf has to recognize all these
> >> > combinations and work appropriately.
> >> > That's what backward and forward compatibility is.
> >> > That's what makes libbpf so difficult to test, develop and code review.
> >> > What that particular server has in /usr/include is irrelevant.
> >>
> >> sure, anyway we can't compile following:
> >>
> >>         tredaell@aldebaran ~ $ echo "#include <bpf/xsk.h>" | gcc -x c -
> >>         In file included from <stdin>:1:
> >>         /usr/include/bpf/xsk.h: In function ‘xsk_ring_prod__needs_wakeup’:
> >>         /usr/include/bpf/xsk.h:82:21: error: ‘XDP_RING_NEED_WAKEUP’ undeclared (first use in this function)
> >>            82 |  return *r->flags & XDP_RING_NEED_WAKEUP;
> >>         ...
> >>
> >>         XDP_RING_NEED_WAKEUP is defined in kernel v5.4-rc1 (77cd0d7b3f257fd0e3096b4fdcff1a7d38e99e10).
> >>         XSK_UNALIGNED_BUF_ADDR_MASK and XSK_UNALIGNED_BUF_OFFSET_SHIFT are defined in kernel v5.4-rc1 (c05cd3645814724bdeb32a2b4d953b12bdea5f8c).
> >>
> >> with:
> >>   kernel-headers-5.3.6-300.fc31.x86_64
> >>   libbpf-0.0.5-1.fc31.x86_64
> >>
> >> if you're saying this is not supported, I guess we could be postponing
> >> libbpf rpm releases until we have the related fedora kernel released
> >
> > why? github/libbpf is the source of truth for building packages
> > and afaik it builds fine.
> >
> >> or how about inluding uapi headers in libbpf-devel.. but that might
> >> actualy cause more confusion
> >
> > Libraries (libbpf or any other) should not install headers that
> > typically go into /usr/include/
> > if_xdp.h case is not unique.
> > We'll surely add another #define, enum, etc to uapi/linux/bpf.h tomorrow.
> > And we will not copy paste these constants and types into tools/lib/bpf/.
> > In kernel tree libbpf development is using kernel tree headers.
> > No problem there for libbpf developers.
> > Packages are built out of github/libbpf that has a copy of uapi headers
> > necessary to create packages.
> > No problem there for package builders either.
> > But libbpf package is not going to install those uapi headers.
> > libbpf package installs only libbpf own headers (like libbpf.h)
> > The users that want to build against the latest libbpf package need
> > to install corresponding uapi headers package.
> > I don't think such dependency is specified in rpm scripts.
> > May be it is something to fix? Or may be not.
> > Some folks might not want to update all of /usr/include to bring libbpf-devel.
> > Then it would be their responsibility to get fresh /usr/include headers.
> 
> We can certainly tie libbpf to the kernel version. The obvious way to do
> that is to just ship the version of libbpf that's in the kernel tree of
> whatever kernel version the distro ships. But how will we handle
> bugfixes, then? You've explicitly stated that libbpf gets no bugfixes
> outside of bpf-next...
> 
> -Toke

We use libbpf and build for a wide variety of kernels so I don't think we
want to make libbpf kernel version specific. I always want the latest libbpf
features even when building on older kernels. I generally use the bpf-next
version though so maybe I'm not the target user.

.John

Re: [PATCH bpf-next v3] libbpf: fix compatibility for kernels without need_wakeup

[Alexei Starovoitov]

On Fri, Nov 1, 2019 at 12:27 AM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Thu, Oct 31, 2019 at 01:39:12PM -0700, Alexei Starovoitov wrote:
> > On Thu, Oct 31, 2019 at 12:18 PM Jiri Olsa <jolsa@redhat.com> wrote:
> > > >
> > > > yes. older vmlinux and newer installed libbpf.so
> > > > or any version of libbpf.a that is statically linked into apps
> > > > is something that libbpf code has to support.
> > > > The server can be rebooted into older than libbpf kernel and
> > > > into newer than libbpf kernel. libbpf has to recognize all these
> > > > combinations and work appropriately.
> > > > That's what backward and forward compatibility is.
> > > > That's what makes libbpf so difficult to test, develop and code review.
> > > > What that particular server has in /usr/include is irrelevant.
> > >
> > > sure, anyway we can't compile following:
> > >
> > >         tredaell@aldebaran ~ $ echo "#include <bpf/xsk.h>" | gcc -x c -
> > >         In file included from <stdin>:1:
> > >         /usr/include/bpf/xsk.h: In function ‘xsk_ring_prod__needs_wakeup’:
> > >         /usr/include/bpf/xsk.h:82:21: error: ‘XDP_RING_NEED_WAKEUP’ undeclared (first use in this function)
> > >            82 |  return *r->flags & XDP_RING_NEED_WAKEUP;
> > >         ...
> > >
> > >         XDP_RING_NEED_WAKEUP is defined in kernel v5.4-rc1 (77cd0d7b3f257fd0e3096b4fdcff1a7d38e99e10).
> > >         XSK_UNALIGNED_BUF_ADDR_MASK and XSK_UNALIGNED_BUF_OFFSET_SHIFT are defined in kernel v5.4-rc1 (c05cd3645814724bdeb32a2b4d953b12bdea5f8c).
> > >
> > > with:
> > >   kernel-headers-5.3.6-300.fc31.x86_64
> > >   libbpf-0.0.5-1.fc31.x86_64
> > >
> > > if you're saying this is not supported, I guess we could be postponing
> > > libbpf rpm releases until we have the related fedora kernel released
> >
> > why? github/libbpf is the source of truth for building packages
> > and afaik it builds fine.
>
> because we will get issues like above if there's no kernel
> avilable that we could compile libbpf against

what is the issue again?
bpf-next builds fine. github/libbpf builds fine.
If distro is doing something else it's distro's mistake.

Re: [PATCH bpf-next v3] libbpf: fix compatibility for kernels without need_wakeup

[Toke Høiland-Jørgensen]

Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:

> On Fri, Nov 1, 2019 at 12:27 AM Jiri Olsa <jolsa@redhat.com> wrote:
>>
>> On Thu, Oct 31, 2019 at 01:39:12PM -0700, Alexei Starovoitov wrote:
>> > On Thu, Oct 31, 2019 at 12:18 PM Jiri Olsa <jolsa@redhat.com> wrote:
>> > > >
>> > > > yes. older vmlinux and newer installed libbpf.so
>> > > > or any version of libbpf.a that is statically linked into apps
>> > > > is something that libbpf code has to support.
>> > > > The server can be rebooted into older than libbpf kernel and
>> > > > into newer than libbpf kernel. libbpf has to recognize all these
>> > > > combinations and work appropriately.
>> > > > That's what backward and forward compatibility is.
>> > > > That's what makes libbpf so difficult to test, develop and code review.
>> > > > What that particular server has in /usr/include is irrelevant.
>> > >
>> > > sure, anyway we can't compile following:
>> > >
>> > >         tredaell@aldebaran ~ $ echo "#include <bpf/xsk.h>" | gcc -x c -
>> > >         In file included from <stdin>:1:
>> > >         /usr/include/bpf/xsk.h: In function ‘xsk_ring_prod__needs_wakeup’:
>> > >         /usr/include/bpf/xsk.h:82:21: error: ‘XDP_RING_NEED_WAKEUP’ undeclared (first use in this function)
>> > >            82 |  return *r->flags & XDP_RING_NEED_WAKEUP;
>> > >         ...
>> > >
>> > >         XDP_RING_NEED_WAKEUP is defined in kernel v5.4-rc1 (77cd0d7b3f257fd0e3096b4fdcff1a7d38e99e10).
>> > >         XSK_UNALIGNED_BUF_ADDR_MASK and XSK_UNALIGNED_BUF_OFFSET_SHIFT are defined in kernel v5.4-rc1 (c05cd3645814724bdeb32a2b4d953b12bdea5f8c).
>> > >
>> > > with:
>> > >   kernel-headers-5.3.6-300.fc31.x86_64
>> > >   libbpf-0.0.5-1.fc31.x86_64
>> > >
>> > > if you're saying this is not supported, I guess we could be postponing
>> > > libbpf rpm releases until we have the related fedora kernel released
>> >
>> > why? github/libbpf is the source of truth for building packages
>> > and afaik it builds fine.
>>
>> because we will get issues like above if there's no kernel
>> avilable that we could compile libbpf against
>
> what is the issue again?
> bpf-next builds fine. github/libbpf builds fine.
> If distro is doing something else it's distro's mistake.

With that you're saying that distros should always keep their kernel
headers and libbpf version in sync. Which is fine in itself; they can
certainly do that.

The only concern with this is that without a flow of bugfixes into the
'bpf' tree (and stable), users may end up with buggy versions of libbpf.
Which is in no one's interest. So how do we avoid that?

-Toke

Re: [PATCH bpf-next v3] libbpf: fix compatibility for kernels without need_wakeup

[Alexei Starovoitov]

On Fri, Nov 1, 2019 at 12:36 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>
> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
>
> > On Fri, Nov 1, 2019 at 12:27 AM Jiri Olsa <jolsa@redhat.com> wrote:
> >>
> >> On Thu, Oct 31, 2019 at 01:39:12PM -0700, Alexei Starovoitov wrote:
> >> > On Thu, Oct 31, 2019 at 12:18 PM Jiri Olsa <jolsa@redhat.com> wrote:
> >> > > >
> >> > > > yes. older vmlinux and newer installed libbpf.so
> >> > > > or any version of libbpf.a that is statically linked into apps
> >> > > > is something that libbpf code has to support.
> >> > > > The server can be rebooted into older than libbpf kernel and
> >> > > > into newer than libbpf kernel. libbpf has to recognize all these
> >> > > > combinations and work appropriately.
> >> > > > That's what backward and forward compatibility is.
> >> > > > That's what makes libbpf so difficult to test, develop and code review.
> >> > > > What that particular server has in /usr/include is irrelevant.
> >> > >
> >> > > sure, anyway we can't compile following:
> >> > >
> >> > >         tredaell@aldebaran ~ $ echo "#include <bpf/xsk.h>" | gcc -x c -
> >> > >         In file included from <stdin>:1:
> >> > >         /usr/include/bpf/xsk.h: In function ‘xsk_ring_prod__needs_wakeup’:
> >> > >         /usr/include/bpf/xsk.h:82:21: error: ‘XDP_RING_NEED_WAKEUP’ undeclared (first use in this function)
> >> > >            82 |  return *r->flags & XDP_RING_NEED_WAKEUP;
> >> > >         ...
> >> > >
> >> > >         XDP_RING_NEED_WAKEUP is defined in kernel v5.4-rc1 (77cd0d7b3f257fd0e3096b4fdcff1a7d38e99e10).
> >> > >         XSK_UNALIGNED_BUF_ADDR_MASK and XSK_UNALIGNED_BUF_OFFSET_SHIFT are defined in kernel v5.4-rc1 (c05cd3645814724bdeb32a2b4d953b12bdea5f8c).
> >> > >
> >> > > with:
> >> > >   kernel-headers-5.3.6-300.fc31.x86_64
> >> > >   libbpf-0.0.5-1.fc31.x86_64
> >> > >
> >> > > if you're saying this is not supported, I guess we could be postponing
> >> > > libbpf rpm releases until we have the related fedora kernel released
> >> >
> >> > why? github/libbpf is the source of truth for building packages
> >> > and afaik it builds fine.
> >>
> >> because we will get issues like above if there's no kernel
> >> avilable that we could compile libbpf against
> >
> > what is the issue again?
> > bpf-next builds fine. github/libbpf builds fine.
> > If distro is doing something else it's distro's mistake.
>
> With that you're saying that distros should always keep their kernel
> headers and libbpf version in sync. Which is fine in itself; they can
> certainly do that.

No. I'm not suggesting that.
distro is free to package whatever /usr/include headers.
kernel version is often != /usr/include headers

> The only concern with this is that without a flow of bugfixes into the
> 'bpf' tree (and stable), users may end up with buggy versions of libbpf.
> Which is in no one's interest. So how do we avoid that?

As I explained earlier. There is no 'bpf' tree for libbpf. It always
moves forward.

Re: [PATCH bpf-next v3] libbpf: fix compatibility for kernels without need_wakeup

[Toke Høiland-Jørgensen]

Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:

> On Fri, Nov 1, 2019 at 12:36 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>>
>> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
>>
>> > On Fri, Nov 1, 2019 at 12:27 AM Jiri Olsa <jolsa@redhat.com> wrote:
>> >>
>> >> On Thu, Oct 31, 2019 at 01:39:12PM -0700, Alexei Starovoitov wrote:
>> >> > On Thu, Oct 31, 2019 at 12:18 PM Jiri Olsa <jolsa@redhat.com> wrote:
>> >> > > >
>> >> > > > yes. older vmlinux and newer installed libbpf.so
>> >> > > > or any version of libbpf.a that is statically linked into apps
>> >> > > > is something that libbpf code has to support.
>> >> > > > The server can be rebooted into older than libbpf kernel and
>> >> > > > into newer than libbpf kernel. libbpf has to recognize all these
>> >> > > > combinations and work appropriately.
>> >> > > > That's what backward and forward compatibility is.
>> >> > > > That's what makes libbpf so difficult to test, develop and code review.
>> >> > > > What that particular server has in /usr/include is irrelevant.
>> >> > >
>> >> > > sure, anyway we can't compile following:
>> >> > >
>> >> > >         tredaell@aldebaran ~ $ echo "#include <bpf/xsk.h>" | gcc -x c -
>> >> > >         In file included from <stdin>:1:
>> >> > >         /usr/include/bpf/xsk.h: In function ‘xsk_ring_prod__needs_wakeup’:
>> >> > >         /usr/include/bpf/xsk.h:82:21: error: ‘XDP_RING_NEED_WAKEUP’ undeclared (first use in this function)
>> >> > >            82 |  return *r->flags & XDP_RING_NEED_WAKEUP;
>> >> > >         ...
>> >> > >
>> >> > >         XDP_RING_NEED_WAKEUP is defined in kernel v5.4-rc1 (77cd0d7b3f257fd0e3096b4fdcff1a7d38e99e10).
>> >> > >         XSK_UNALIGNED_BUF_ADDR_MASK and XSK_UNALIGNED_BUF_OFFSET_SHIFT are defined in kernel v5.4-rc1 (c05cd3645814724bdeb32a2b4d953b12bdea5f8c).
>> >> > >
>> >> > > with:
>> >> > >   kernel-headers-5.3.6-300.fc31.x86_64
>> >> > >   libbpf-0.0.5-1.fc31.x86_64
>> >> > >
>> >> > > if you're saying this is not supported, I guess we could be postponing
>> >> > > libbpf rpm releases until we have the related fedora kernel released
>> >> >
>> >> > why? github/libbpf is the source of truth for building packages
>> >> > and afaik it builds fine.
>> >>
>> >> because we will get issues like above if there's no kernel
>> >> avilable that we could compile libbpf against
>> >
>> > what is the issue again?
>> > bpf-next builds fine. github/libbpf builds fine.
>> > If distro is doing something else it's distro's mistake.
>>
>> With that you're saying that distros should always keep their kernel
>> headers and libbpf version in sync. Which is fine in itself; they can
>> certainly do that.
>
> No. I'm not suggesting that.
> distro is free to package whatever /usr/include headers.
> kernel version is often != /usr/include headers

I did say kernel *headers*. By which I mean the files in /usr/include.
E.g., on my machine:

$ pacman -Qo /usr/include/linux/if_xdp.h                                                                /usr/include/linux/if_xdp.h is owned by linux-api-headers 5.3.1-1


>> The only concern with this is that without a flow of bugfixes into the
>> 'bpf' tree (and stable), users may end up with buggy versions of libbpf.
>> Which is in no one's interest. So how do we avoid that?
>
> As I explained earlier. There is no 'bpf' tree for libbpf. It always
> moves forward.

Yes, you did. And I was just pointing out that this means that there
will be no bug fixes in older versions. So the only way to update is to
move to an entirely new version of libbpf, including updating all the
headers in /usr/include. And when that is not feasible, then the only
choice left is to ship a buggy libbpf... Unless you have a third option
I'm missing?

-Toke

Re: [PATCH bpf-next v3] libbpf: fix compatibility for kernels without need_wakeup

[Alexei Starovoitov]

On Fri, Nov 1, 2019 at 2:41 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>
> >> The only concern with this is that without a flow of bugfixes into the
> >> 'bpf' tree (and stable), users may end up with buggy versions of libbpf.
> >> Which is in no one's interest. So how do we avoid that?
> >
> > As I explained earlier. There is no 'bpf' tree for libbpf. It always
> > moves forward.
>
> Yes, you did. And I was just pointing out that this means that there
> will be no bug fixes in older versions. So the only way to update is to
> move to an entirely new version of libbpf, including updating all the
> headers in /usr/include. And when that is not feasible, then the only
> choice left is to ship a buggy libbpf... Unless you have a third option
> I'm missing?

I'm not seeing the problem still.
Say there is a bug in installed libbpf package.
A bunch of services are using it as libbpf.so
The admin upgrades libbpf package. New libbpf.so is installed
and services should continue work as-is, since we're preserving
api binary compatibility.
Now if you're a developer you install libbpf-devel and the first thing
you'll hit is that /usr/include is so old that it doesn't have basic things
to start writing bpf progs. So recent linux-api-headers package
would have to be installed.
You keep developing stuff. Few month passes by. New libbpf is released.
At the current pace of the development one release contains
a bunch of features and a bunch of fixes.
If you're upgrading it you'd need to refresh api-headers too.
So where is the problem?

Of course there is an option to branch existing libbpf releases, but let's
cross that bridge when we actually need to do that.