diff mbox series

[ovs-dev] docs: Add header install command for afxdp.

Message ID 1579628908-23493-1-git-send-email-u9012063@gmail.com
State Superseded
Headers show
Series [ovs-dev] docs: Add header install command for afxdp. | expand

Commit Message

William Tu Jan. 21, 2020, 5:48 p.m. UTC
The 'XDP_RING_NEED_WAKEUP' and related flags are defined if_xdp.h, so after
installing newer kernel, users have to update the kernel's header files,
by doing:
  $ make headers_install INSTALL_HDR_PATH=/usr

Otherwise the following error shows:
/usr/local/include/bpf/xsk.h: In function 'xsk_ring_prod__needs_wakeup':
/usr/local/include/bpf/xsk.h:82:21: error: 'XDP_RING_NEED_WAKEUP' undeclared \
  (first use in this function)
  return *r->flags & XDP_RING_NEED_WAKEUP;

Reported-by: Tomek Osinski <osinstom@gmail.com>
Reported-at: https://osinstom.github.io/en/tutorial/ovs-afxdp-installation/
Signed-off-by: William Tu <u9012063@gmail.com>
---
 Documentation/intro/install/afxdp.rst | 1 +
 1 file changed, 1 insertion(+)

Comments

Ben Pfaff Jan. 21, 2020, 6:49 p.m. UTC | #1
On Tue, Jan 21, 2020 at 09:48:28AM -0800, William Tu wrote:
> The 'XDP_RING_NEED_WAKEUP' and related flags are defined if_xdp.h, so after
> installing newer kernel, users have to update the kernel's header files,
> by doing:
>   $ make headers_install INSTALL_HDR_PATH=/usr
> 
> Otherwise the following error shows:
> /usr/local/include/bpf/xsk.h: In function 'xsk_ring_prod__needs_wakeup':
> /usr/local/include/bpf/xsk.h:82:21: error: 'XDP_RING_NEED_WAKEUP' undeclared \
>   (first use in this function)
>   return *r->flags & XDP_RING_NEED_WAKEUP;
> 
> Reported-by: Tomek Osinski <osinstom@gmail.com>
> Reported-at: https://osinstom.github.io/en/tutorial/ovs-afxdp-installation/
> Signed-off-by: William Tu <u9012063@gmail.com>
> ---
>  Documentation/intro/install/afxdp.rst | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/intro/install/afxdp.rst b/Documentation/intro/install/afxdp.rst
> index c4685fa7ebac..2683d8301bb7 100644
> --- a/Documentation/intro/install/afxdp.rst
> +++ b/Documentation/intro/install/afxdp.rst
> @@ -125,6 +125,7 @@ Second, go into the Linux source directory and build libbpf in the tools
>  directory::
>  
>    cd bpf-next/
> +  make headers_install INSTALL_HDR_PATH=/usr
>    cd tools/lib/bpf/
>    make && make install
>    make install_headers

This seems reasonable.

Acked-by: Ben Pfaff <blp@ovn.org>
Ilya Maximets Jan. 23, 2020, 1:14 p.m. UTC | #2
> The 'XDP_RING_NEED_WAKEUP' and related flags are defined if_xdp.h, so after
> installing newer kernel, users have to update the kernel's header files,
> by doing:
>   $ make headers_install INSTALL_HDR_PATH=/usr

> Otherwise the following error shows:
> /usr/local/include/bpf/xsk.h: In function 'xsk_ring_prod__needs_wakeup':
> /usr/local/include/bpf/xsk.h:82:21: error: 'XDP_RING_NEED_WAKEUP' undeclared \
>   (first use in this function)
>   return *r->flags & XDP_RING_NEED_WAKEUP;

> Reported-by: Tomek Osinski <osinstom@gmail.com>
> Reported-at: https://osinstom.github.io/en/tutorial/ovs-afxdp-installation/
> Signed-off-by: William Tu <u9012063@gmail.com>
> ---
>  Documentation/intro/install/afxdp.rst | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/intro/install/afxdp.rst b/Documentation/intro/install/afxdp.rst
> index c4685fa7ebac..2683d8301bb7 100644
> --- a/Documentation/intro/install/afxdp.rst
> +++ b/Documentation/intro/install/afxdp.rst
> @@ -125,6 +125,7 @@ Second, go into the Linux source directory and build libbpf in the tools
>  directory::
>  
>    cd bpf-next/
> +  make headers_install INSTALL_HDR_PATH=/usr
>    cd tools/lib/bpf/
>    make && make install
>    make install_headers

I'm not sure about this change.  Especially in this section.
This will work only if the running kernel is bpf-next kernel and if it's not
this will cause unexpected runtime issues that will be really hard to debug and
understand.  User should have its own kernel headers installed, not the headers
from bpf-next.

If you really want this information in the docs, I'd suggest to put another
item to previous "Build requirements" section that will say something like this:

"""
- If you're building your own kernel, be sure that you're installing kernel
  headers too.  For example, with the following command::

    make headers_install INSTALL_HDR_PATH=/usr

- If you're using kernel from the distribution, be sure that corresponding
  kernel headers package installed.
"""

Best regards, Ilya Maximets.
William Tu Jan. 23, 2020, 4:58 p.m. UTC | #3
On Thu, Jan 23, 2020 at 02:14:03PM +0100, Ilya Maximets wrote:
> > The 'XDP_RING_NEED_WAKEUP' and related flags are defined if_xdp.h, so after
> > installing newer kernel, users have to update the kernel's header files,
> > by doing:
> >   $ make headers_install INSTALL_HDR_PATH=/usr
> 
> > Otherwise the following error shows:
> > /usr/local/include/bpf/xsk.h: In function 'xsk_ring_prod__needs_wakeup':
> > /usr/local/include/bpf/xsk.h:82:21: error: 'XDP_RING_NEED_WAKEUP' undeclared \
> >   (first use in this function)
> >   return *r->flags & XDP_RING_NEED_WAKEUP;
> 
> > Reported-by: Tomek Osinski <osinstom@gmail.com>
> > Reported-at: https://osinstom.github.io/en/tutorial/ovs-afxdp-installation/
> > Signed-off-by: William Tu <u9012063@gmail.com>
> > ---
> >  Documentation/intro/install/afxdp.rst | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/Documentation/intro/install/afxdp.rst b/Documentation/intro/install/afxdp.rst
> > index c4685fa7ebac..2683d8301bb7 100644
> > --- a/Documentation/intro/install/afxdp.rst
> > +++ b/Documentation/intro/install/afxdp.rst
> > @@ -125,6 +125,7 @@ Second, go into the Linux source directory and build libbpf in the tools
> >  directory::
> >  
> >    cd bpf-next/
> > +  make headers_install INSTALL_HDR_PATH=/usr
> >    cd tools/lib/bpf/
> >    make && make install
> >    make install_headers
> 
> I'm not sure about this change.  Especially in this section.
> This will work only if the running kernel is bpf-next kernel and if it's not
> this will cause unexpected runtime issues that will be really hard to debug and
> understand.  User should have its own kernel headers installed, not the headers
> from bpf-next.
> 
> If you really want this information in the docs, I'd suggest to put another
> item to previous "Build requirements" section that will say something like this:
> 
> """
> - If you're building your own kernel, be sure that you're installing kernel
>   headers too.  For example, with the following command::
> 
>     make headers_install INSTALL_HDR_PATH=/usr
> 
> - If you're using kernel from the distribution, be sure that corresponding
>   kernel headers package installed.
> """
> 
> Best regards, Ilya Maximets.

Hi Ilya,

That makes sense. I will send v2

William
diff mbox series

Patch

diff --git a/Documentation/intro/install/afxdp.rst b/Documentation/intro/install/afxdp.rst
index c4685fa7ebac..2683d8301bb7 100644
--- a/Documentation/intro/install/afxdp.rst
+++ b/Documentation/intro/install/afxdp.rst
@@ -125,6 +125,7 @@  Second, go into the Linux source directory and build libbpf in the tools
 directory::
 
   cd bpf-next/
+  make headers_install INSTALL_HDR_PATH=/usr
   cd tools/lib/bpf/
   make && make install
   make install_headers