[v1] net/tap: explain how to compile eBPF C file

  This commit explains how to manually compile the C source file
tap_bpf_program.c into an ELF file using the clang compiler.
The code in tap_bpf_program.c requires definitions found in iproute2
source code. This commit suggests cloning the iproute2 git tree and
include its path in the clang command. It also adds inclusion of file
bpf_api.h (required for eBPF definitions) which is located in iproute2
source tree. For more details refer to TAP documentation.
This commit is related to commits [1] and [2].

[1] commit cdc07e83bb24 ("net/tap: add eBPF program file")
[2] commit aabe70df73a3 ("net/tap: add eBPF bytes code")

Signed-off-by: Ophir Munk <ophirmu@mellanox.com>
---
 doc/guides/nics/tap.rst           | 21 +++++++++++++++++----
 drivers/net/tap/tap_bpf_program.c |  5 +++++
 2 files changed, 22 insertions(+), 4 deletions(-)
  

> On Jun 11, 2018, at 11:06 AM, Ophir Munk <ophirmu@mellanox.com> wrote:
> 
> This commit explains how to manually compile the C source file
> tap_bpf_program.c into an ELF file using the clang compiler.
> The code in tap_bpf_program.c requires definitions found in iproute2
> source code. This commit suggests cloning the iproute2 git tree and
> include its path in the clang command. It also adds inclusion of file
> bpf_api.h (required for eBPF definitions) which is located in iproute2
> source tree. For more details refer to TAP documentation.
> This commit is related to commits [1] and [2].

Normally I would have suggested that eBPF be disable in the TAP driver as it requires external code and programs, but that ship has sailed.

I would like to see building the tap_bpf_program.o as a target in the Makefile, this way the developer can just run the ‘make bpf_program’ target and it would be simpler and less error prone.
> 
> [1] commit cdc07e83bb24 ("net/tap: add eBPF program file")
> [2] commit aabe70df73a3 ("net/tap: add eBPF bytes code")
> 
> Signed-off-by: Ophir Munk <ophirmu@mellanox.com>
> ---
> doc/guides/nics/tap.rst           | 21 +++++++++++++++++----
> drivers/net/tap/tap_bpf_program.c |  5 +++++
> 2 files changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/doc/guides/nics/tap.rst b/doc/guides/nics/tap.rst
> index 2714868..af6c534 100644
> --- a/doc/guides/nics/tap.rst
> +++ b/doc/guides/nics/tap.rst
> @@ -234,13 +234,26 @@ C functions under different ELF sections.
> 
> 2. Install ``LLVM`` library and ``clang`` compiler versions 3.7 and above
> 
> -3. Compile ``tap_bpf_program.c`` via ``LLVM`` into an object file::
> +3. The code in ``tap_bpf_program.c`` requires definitions found in iproute2
> +source code.
> 
> -    clang -O2 -emit-llvm -c tap_bpf_program.c -o - | llc -march=bpf \
> -    -filetype=obj -o <tap_bpf_program.o>
> +Clone the iproute2 git tree and make it accessible to the build environment, say
> +under directory ``<iproute2_root_tree>`` ::
> +
> +    git clone https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/ \
> +    <iproute2_root_tree>
> +
> +The code in ``tap_bpf_program.c`` must include file ``bpf_api.h`` which is
> +located under ``<iproute2_root_tree>`` directory. This file contains eBPF
> +related definitions.
> 
> +4. Compile ``tap_bpf_program.c`` via ``LLVM`` into an object file::
> +
> +    clang -I <iproute2_root_tree>/iproute2/include \
> +    -O2 -emit-llvm -c tap_bpf_program.c -o - | llc -march=bpf \
> +    -filetype=obj -o <tap_bpf_program.o>
> 
> -4. Use a tool that receives two parameters: an eBPF object file and a section
> +5. Use a tool that receives two parameters: an eBPF object file and a section

What ‘tool’ should be used here? objdump?

> name, and prints out the section as a C array of eBPF instructions.
> Embed the C array in your TAP PMD tree.
> 
> diff --git a/drivers/net/tap/tap_bpf_program.c b/drivers/net/tap/tap_bpf_program.c
> index 1cb7382..60b069b 100644
> --- a/drivers/net/tap/tap_bpf_program.c
> +++ b/drivers/net/tap/tap_bpf_program.c
> @@ -17,6 +17,11 @@
> #include <linux/bpf.h>
> 
> #include "tap_rss.h"
> +/*
> + * bpf_api.h file is located under iproute2
> + * tree, see TAP documentation.
> + */
> +#include "bpf_api.h"
> 
> /** Create IPv4 address */
> #define IPv4(a, b, c, d) ((__u32)(((a) & 0xff) << 24) | \
> -- 
> 1.8.3.1
> 

Regards,
Keith
  

11/06/2018 18:35, Wiles, Keith:
> 
> > On Jun 11, 2018, at 11:06 AM, Ophir Munk <ophirmu@mellanox.com> wrote:
> > 
> > This commit explains how to manually compile the C source file
> > tap_bpf_program.c into an ELF file using the clang compiler.
> > The code in tap_bpf_program.c requires definitions found in iproute2
> > source code. This commit suggests cloning the iproute2 git tree and
> > include its path in the clang command. It also adds inclusion of file
> > bpf_api.h (required for eBPF definitions) which is located in iproute2
> > source tree. For more details refer to TAP documentation.
> > This commit is related to commits [1] and [2].
> 
> Normally I would have suggested that eBPF be disable in the TAP driver as it requires external code and programs, but that ship has sailed.

The external programs are required only to generate new instructions,
changing the behaviour of the BPF program.
Currently, the instructions for RSS behaviour are provided.

> I would like to see building the tap_bpf_program.o as a target in the Makefile, this way the developer can just run the ‘make bpf_program’ target and it would be simpler and less error prone.

For this to happen, we need to improve the tools.
It is a work in progress.
This is a very first step to use Linux BPF with DPDK.
If there are more interests, we should really streamline its usage
for all parts of DPDK which runs on top of some kernel code.
  

> On Jun 12, 2018, at 7:26 AM, Thomas Monjalon <thomas@monjalon.net> wrote:
> 
> 11/06/2018 18:35, Wiles, Keith:
>> 
>>> On Jun 11, 2018, at 11:06 AM, Ophir Munk <ophirmu@mellanox.com> wrote:
>>> 
>>> This commit explains how to manually compile the C source file
>>> tap_bpf_program.c into an ELF file using the clang compiler.
>>> The code in tap_bpf_program.c requires definitions found in iproute2
>>> source code. This commit suggests cloning the iproute2 git tree and
>>> include its path in the clang command. It also adds inclusion of file
>>> bpf_api.h (required for eBPF definitions) which is located in iproute2
>>> source tree. For more details refer to TAP documentation.
>>> This commit is related to commits [1] and [2].
>> 
>> Normally I would have suggested that eBPF be disable in the TAP driver as it requires external code and programs, but that ship has sailed.
> 
> The external programs are required only to generate new instructions,
> changing the behaviour of the BPF program.
> Currently, the instructions for RSS behaviour are provided.
> 
>> I would like to see building the tap_bpf_program.o as a target in the Makefile, this way the developer can just run the ‘make bpf_program’ target and it would be simpler and less error prone.
> 
> For this to happen, we need to improve the tools.

In what way do we need to improve the tools and which tools are we talking about. Building the .o file below appears to be a simple set of command lines. I have a question in my original email about what tool.

> It is a work in progress.
> This is a very first step to use Linux BPF with DPDK.
> If there are more interests, we should really streamline its usage
> for all parts of DPDK which runs on top of some kernel code.

streamlining other parts of DPDK would be nice, but we are now talking about the tap/eBPF patch.

> 
> 
> 

Regards,
Keith
  

12/06/2018 14:36, Wiles, Keith:
> 
> > On Jun 12, 2018, at 7:26 AM, Thomas Monjalon <thomas@monjalon.net> wrote:
> > 
> > 11/06/2018 18:35, Wiles, Keith:
> >> 
> >>> On Jun 11, 2018, at 11:06 AM, Ophir Munk <ophirmu@mellanox.com> wrote:
> >>> 
> >>> This commit explains how to manually compile the C source file
> >>> tap_bpf_program.c into an ELF file using the clang compiler.
> >>> The code in tap_bpf_program.c requires definitions found in iproute2
> >>> source code. This commit suggests cloning the iproute2 git tree and
> >>> include its path in the clang command. It also adds inclusion of file
> >>> bpf_api.h (required for eBPF definitions) which is located in iproute2
> >>> source tree. For more details refer to TAP documentation.
> >>> This commit is related to commits [1] and [2].
> >> 
> >> Normally I would have suggested that eBPF be disable in the TAP driver as it requires external code and programs, but that ship has sailed.
> > 
> > The external programs are required only to generate new instructions,
> > changing the behaviour of the BPF program.
> > Currently, the instructions for RSS behaviour are provided.
> > 
> >> I would like to see building the tap_bpf_program.o as a target in the Makefile, this way the developer can just run the ‘make bpf_program’ target and it would be simpler and less error prone.

As explained in the documentation, for now there is a dependency on iproute2
for the compilation of this BPF program.
So we cannot make it as simple as a "make command".
Probably that we can rework it to change the dependency.
I heard there are some good BPF libraries available now?

> > For this to happen, we need to improve the tools.
> 
> In what way do we need to improve the tools and which tools are we talking about. Building the .o file below appears to be a simple set of command lines. I have a question in my original email about what tool.

The .o file is only the an intermediate file.
The next step (numbered as 5 in this patch) is to extract the section
of BPF instructions to be uploaded in the kernel.
This step must be done by a "tool". Ophir did it by hacking tc,
but it is not upstreamed yet.
There could be other ways (possibly easier) to achieve the same result.

> > It is a work in progress.

Contributions are welcome.

> > This is a very first step to use Linux BPF with DPDK.
> > If there are more interests, we should really streamline its usage
> > for all parts of DPDK which runs on top of some kernel code.
> 
> streamlining other parts of DPDK would be nice, but we are now talking about the tap/eBPF patch.
  

> On Jun 12, 2018, at 7:58 AM, Thomas Monjalon <thomas@monjalon.net> wrote:
> 
> 12/06/2018 14:36, Wiles, Keith:
>> 
>>> On Jun 12, 2018, at 7:26 AM, Thomas Monjalon <thomas@monjalon.net> wrote:
>>> 
>>> 11/06/2018 18:35, Wiles, Keith:
>>>> 
>>>>> On Jun 11, 2018, at 11:06 AM, Ophir Munk <ophirmu@mellanox.com> wrote:
>>>>> 
>>>>> This commit explains how to manually compile the C source file
>>>>> tap_bpf_program.c into an ELF file using the clang compiler.
>>>>> The code in tap_bpf_program.c requires definitions found in iproute2
>>>>> source code. This commit suggests cloning the iproute2 git tree and
>>>>> include its path in the clang command. It also adds inclusion of file
>>>>> bpf_api.h (required for eBPF definitions) which is located in iproute2
>>>>> source tree. For more details refer to TAP documentation.
>>>>> This commit is related to commits [1] and [2].
>>>> 
>>>> Normally I would have suggested that eBPF be disable in the TAP driver as it requires external code and programs, but that ship has sailed.
>>> 
>>> The external programs are required only to generate new instructions,
>>> changing the behaviour of the BPF program.
>>> Currently, the instructions for RSS behaviour are provided.
>>> 
>>>> I would like to see building the tap_bpf_program.o as a target in the Makefile, this way the developer can just run the ‘make bpf_program’ target and it would be simpler and less error prone.
> 
> As explained in the documentation, for now there is a dependency on iproute2
> for the compilation of this BPF program.
> So we cannot make it as simple as a "make command".
> Probably that we can rework it to change the dependency.
> I heard there are some good BPF libraries available now?

Well the dependence of iproute2 is really no different then requiring say libnuma, they just have to pull the code first to type the ‘make bpf_program’ right?

If that is the case then a make target make sense to me. If iproute2 is not found then an error, right?

> 
>>> For this to happen, we need to improve the tools.
>> 
>> In what way do we need to improve the tools and which tools are we talking about. Building the .o file below appears to be a simple set of command lines. I have a question in my original email about what tool.
> 
> The .o file is only the an intermediate file.
> The next step (numbered as 5 in this patch) is to extract the section
> of BPF instructions to be uploaded in the kernel.
> This step must be done by a "tool". Ophir did it by hacking tc,
> but it is not upstreamed yet.
> There could be other ways (possibly easier) to achieve the same result.

Please change the doc to reflect the tool is not upstreamed yet and the developer needs to figure out how to extract the data from the binary.

I used objdump -j l3_l4 -s tap_bpf_program.o and got a hex dump of the l3_l4 section

0000 bf160000 00000000 61681000 00000000 <Ascii characters>
...

Someone schooled in the art of Python coding should be able to convert that output to a ‘C’ data array. :-)

> 
>>> It is a work in progress.
> 
> Contributions are welcome.
> 
>>> This is a very first step to use Linux BPF with DPDK.
>>> If there are more interests, we should really streamline its usage
>>> for all parts of DPDK which runs on top of some kernel code.
>> 
>> streamlining other parts of DPDK would be nice, but we are now talking about the tap/eBPF patch.

Regards,
Keith
  

12/06/2018 15:33, Wiles, Keith:
> 
> > On Jun 12, 2018, at 7:58 AM, Thomas Monjalon <thomas@monjalon.net> wrote:
> > 
> > 12/06/2018 14:36, Wiles, Keith:
> >> 
> >>> On Jun 12, 2018, at 7:26 AM, Thomas Monjalon <thomas@monjalon.net> wrote:
> >>> 
> >>> 11/06/2018 18:35, Wiles, Keith:
> >>>> 
> >>>>> On Jun 11, 2018, at 11:06 AM, Ophir Munk <ophirmu@mellanox.com> wrote:
> >>>>> 
> >>>>> This commit explains how to manually compile the C source file
> >>>>> tap_bpf_program.c into an ELF file using the clang compiler.
> >>>>> The code in tap_bpf_program.c requires definitions found in iproute2
> >>>>> source code. This commit suggests cloning the iproute2 git tree and
> >>>>> include its path in the clang command. It also adds inclusion of file
> >>>>> bpf_api.h (required for eBPF definitions) which is located in iproute2
> >>>>> source tree. For more details refer to TAP documentation.
> >>>>> This commit is related to commits [1] and [2].
> >>>> 
> >>>> Normally I would have suggested that eBPF be disable in the TAP driver as it requires external code and programs, but that ship has sailed.
> >>> 
> >>> The external programs are required only to generate new instructions,
> >>> changing the behaviour of the BPF program.
> >>> Currently, the instructions for RSS behaviour are provided.
> >>> 
> >>>> I would like to see building the tap_bpf_program.o as a target in the Makefile, this way the developer can just run the ‘make bpf_program’ target and it would be simpler and less error prone.
> > 
> > As explained in the documentation, for now there is a dependency on iproute2
> > for the compilation of this BPF program.
> > So we cannot make it as simple as a "make command".
> > Probably that we can rework it to change the dependency.
> > I heard there are some good BPF libraries available now?
> 
> Well the dependence of iproute2 is really no different then requiring say libnuma, they just have to pull the code first to type the ‘make bpf_program’ right?

The iproute2 dependency is different because it is not a library.
The .h file is never packaged.
So we need to download the sources and set -I to this directory.


> If that is the case then a make target make sense to me. If iproute2 is not found then an error, right?


> >>> For this to happen, we need to improve the tools.
> >> 
> >> In what way do we need to improve the tools and which tools are we talking about. Building the .o file below appears to be a simple set of command lines. I have a question in my original email about what tool.
> > 
> > The .o file is only the an intermediate file.
> > The next step (numbered as 5 in this patch) is to extract the section
> > of BPF instructions to be uploaded in the kernel.
> > This step must be done by a "tool". Ophir did it by hacking tc,
> > but it is not upstreamed yet.
> > There could be other ways (possibly easier) to achieve the same result.
> 
> Please change the doc to reflect the tool is not upstreamed yet and the developer needs to figure out how to extract the data from the binary.
> 
> I used objdump -j l3_l4 -s tap_bpf_program.o and got a hex dump of the l3_l4 section
> 
> 0000 bf160000 00000000 61681000 00000000 <Ascii characters>
> ...
> 
> Someone schooled in the art of Python coding should be able to convert that output to a ‘C’ data array. :-)
> 
> > 
> >>> It is a work in progress.
> > 
> > Contributions are welcome.
> > 
> >>> This is a very first step to use Linux BPF with DPDK.
> >>> If there are more interests, we should really streamline its usage
> >>> for all parts of DPDK which runs on top of some kernel code.
> >> 
> >> streamlining other parts of DPDK would be nice, but we are now talking about the tap/eBPF patch.
> 
> Regards,
> Keith
> 
>
  

> On Jun 12, 2018, at 8:44 AM, Thomas Monjalon <thomas@monjalon.net> wrote:
> 
> 12/06/2018 15:33, Wiles, Keith:
>> 
>>> On Jun 12, 2018, at 7:58 AM, Thomas Monjalon <thomas@monjalon.net> wrote:
>>> 
>>> 12/06/2018 14:36, Wiles, Keith:
>>>> 
>>>>> On Jun 12, 2018, at 7:26 AM, Thomas Monjalon <thomas@monjalon.net> wrote:
>>>>> 
>>>>> 11/06/2018 18:35, Wiles, Keith:
>>>>>> 
>>>>>>> On Jun 11, 2018, at 11:06 AM, Ophir Munk <ophirmu@mellanox.com> wrote:
>>>>>>> 
>>>>>>> This commit explains how to manually compile the C source file
>>>>>>> tap_bpf_program.c into an ELF file using the clang compiler.
>>>>>>> The code in tap_bpf_program.c requires definitions found in iproute2
>>>>>>> source code. This commit suggests cloning the iproute2 git tree and
>>>>>>> include its path in the clang command. It also adds inclusion of file
>>>>>>> bpf_api.h (required for eBPF definitions) which is located in iproute2
>>>>>>> source tree. For more details refer to TAP documentation.
>>>>>>> This commit is related to commits [1] and [2].
>>>>>> 
>>>>>> Normally I would have suggested that eBPF be disable in the TAP driver as it requires external code and programs, but that ship has sailed.
>>>>> 
>>>>> The external programs are required only to generate new instructions,
>>>>> changing the behaviour of the BPF program.
>>>>> Currently, the instructions for RSS behaviour are provided.
>>>>> 
>>>>>> I would like to see building the tap_bpf_program.o as a target in the Makefile, this way the developer can just run the ‘make bpf_program’ target and it would be simpler and less error prone.
>>> 
>>> As explained in the documentation, for now there is a dependency on iproute2
>>> for the compilation of this BPF program.
>>> So we cannot make it as simple as a "make command".
>>> Probably that we can rework it to change the dependency.
>>> I heard there are some good BPF libraries available now?
>> 
>> Well the dependence of iproute2 is really no different then requiring say libnuma, they just have to pull the code first to type the ‘make bpf_program’ right?
> 
> The iproute2 dependency is different because it is not a library.
> The .h file is never packaged.
> So we need to download the sources and set -I to this directory.

To eliminate the -I problem the clone could be done inside the tap directory and -I ./iproute2/include used, right?
The make target could even clone the code into the tap directory, which means we can solve these problems you are pointing out.

Go ahead and do what you want here, but making it harder for the developer should not be our normally mode of operation.

> 
> 
>> If that is the case then a make target make sense to me. If iproute2 is not found then an error, right?
> 
> 
>>>>> For this to happen, we need to improve the tools.
>>>> 
>>>> In what way do we need to improve the tools and which tools are we talking about. Building the .o file below appears to be a simple set of command lines. I have a question in my original email about what tool.
>>> 
>>> The .o file is only the an intermediate file.
>>> The next step (numbered as 5 in this patch) is to extract the section
>>> of BPF instructions to be uploaded in the kernel.
>>> This step must be done by a "tool". Ophir did it by hacking tc,
>>> but it is not upstreamed yet.
>>> There could be other ways (possibly easier) to achieve the same result.
>> 
>> Please change the doc to reflect the tool is not upstreamed yet and the developer needs to figure out how to extract the data from the binary.
>> 
>> I used objdump -j l3_l4 -s tap_bpf_program.o and got a hex dump of the l3_l4 section
>> 
>> 0000 bf160000 00000000 61681000 00000000 <Ascii characters>
>> ...
>> 
>> Someone schooled in the art of Python coding should be able to convert that output to a ‘C’ data array. :-)
>> 
>>> 
>>>>> It is a work in progress.
>>> 
>>> Contributions are welcome.
>>> 
>>>>> This is a very first step to use Linux BPF with DPDK.
>>>>> If there are more interests, we should really streamline its usage
>>>>> for all parts of DPDK which runs on top of some kernel code.
>>>> 
>>>> streamlining other parts of DPDK would be nice, but we are now talking about the tap/eBPF patch.
>> 
>> Regards,
>> Keith

Regards,
Keith
  

Please note that other than cloning iproute2 we also need to install clang and llvm tools versions 3.7 and upper.
Not sure there are clang and llvm packages of the required versions for the common distributions. 
I compiled the tools source code and installed them manually.

> -----Original Message-----
> From: Wiles, Keith [mailto:keith.wiles@intel.com]
> Sent: Tuesday, June 12, 2018 4:53 PM
> To: Thomas Monjalon <thomas@monjalon.net>
> Cc: dev@dpdk.org; Ophir Munk <ophirmu@mellanox.com>; Pascal Mazon
> <pascal.mazon@6wind.com>; Olga Shern <olgas@mellanox.com>
> Subject: Re: [dpdk-dev] [PATCH v1] net/tap: explain how to compile eBPF C
> file
> 
> 
> 
> > On Jun 12, 2018, at 8:44 AM, Thomas Monjalon <thomas@monjalon.net>
> wrote:
> >
> > 12/06/2018 15:33, Wiles, Keith:
> >>
> >>> On Jun 12, 2018, at 7:58 AM, Thomas Monjalon
> <thomas@monjalon.net> wrote:
> >>>
> >>> 12/06/2018 14:36, Wiles, Keith:
> >>>>
> >>>>> On Jun 12, 2018, at 7:26 AM, Thomas Monjalon
> <thomas@monjalon.net> wrote:
> >>>>>
> >>>>> 11/06/2018 18:35, Wiles, Keith:
> >>>>>>
> >>>>>>> On Jun 11, 2018, at 11:06 AM, Ophir Munk
> <ophirmu@mellanox.com> wrote:
> >>>>>>>
> >>>>>>> This commit explains how to manually compile the C source file
> >>>>>>> tap_bpf_program.c into an ELF file using the clang compiler.
> >>>>>>> The code in tap_bpf_program.c requires definitions found in
> >>>>>>> iproute2 source code. This commit suggests cloning the iproute2
> >>>>>>> git tree and include its path in the clang command. It also adds
> >>>>>>> inclusion of file bpf_api.h (required for eBPF definitions)
> >>>>>>> which is located in iproute2 source tree. For more details refer to
> TAP documentation.
> >>>>>>> This commit is related to commits [1] and [2].
> >>>>>>
> >>>>>> Normally I would have suggested that eBPF be disable in the TAP
> driver as it requires external code and programs, but that ship has sailed.
> >>>>>
> >>>>> The external programs are required only to generate new
> >>>>> instructions, changing the behaviour of the BPF program.
> >>>>> Currently, the instructions for RSS behaviour are provided.
> >>>>>
> >>>>>> I would like to see building the tap_bpf_program.o as a target in the
> Makefile, this way the developer can just run the ‘make bpf_program’ target
> and it would be simpler and less error prone.
> >>>
> >>> As explained in the documentation, for now there is a dependency on
> >>> iproute2 for the compilation of this BPF program.
> >>> So we cannot make it as simple as a "make command".
> >>> Probably that we can rework it to change the dependency.
> >>> I heard there are some good BPF libraries available now?
> >>
> >> Well the dependence of iproute2 is really no different then requiring say
> libnuma, they just have to pull the code first to type the ‘make bpf_program’
> right?
> >
> > The iproute2 dependency is different because it is not a library.
> > The .h file is never packaged.
> > So we need to download the sources and set -I to this directory.
> 
> To eliminate the -I problem the clone could be done inside the tap directory
> and -I ./iproute2/include used, right?
> The make target could even clone the code into the tap directory, which
> means we can solve these problems you are pointing out.
> 
> Go ahead and do what you want here, but making it harder for the developer
> should not be our normally mode of operation.
> 
> >
> >
> >> If that is the case then a make target make sense to me. If iproute2 is not
> found then an error, right?
> >
> >
> >>>>> For this to happen, we need to improve the tools.
> >>>>
> >>>> In what way do we need to improve the tools and which tools are we
> talking about. Building the .o file below appears to be a simple set of
> command lines. I have a question in my original email about what tool.
> >>>
> >>> The .o file is only the an intermediate file.
> >>> The next step (numbered as 5 in this patch) is to extract the
> >>> section of BPF instructions to be uploaded in the kernel.
> >>> This step must be done by a "tool". Ophir did it by hacking tc, but
> >>> it is not upstreamed yet.
> >>> There could be other ways (possibly easier) to achieve the same result.
> >>
> >> Please change the doc to reflect the tool is not upstreamed yet and the
> developer needs to figure out how to extract the data from the binary.
> >>
> >> I used objdump -j l3_l4 -s tap_bpf_program.o and got a hex dump of
> >> the l3_l4 section
> >>
> >> 0000 bf160000 00000000 61681000 00000000 <Ascii characters> ...
> >>
> >> Someone schooled in the art of Python coding should be able to
> >> convert that output to a ‘C’ data array. :-)
> >>
> >>>
> >>>>> It is a work in progress.
> >>>
> >>> Contributions are welcome.
> >>>
> >>>>> This is a very first step to use Linux BPF with DPDK.
> >>>>> If there are more interests, we should really streamline its usage
> >>>>> for all parts of DPDK which runs on top of some kernel code.
> >>>>
> >>>> streamlining other parts of DPDK would be nice, but we are now talking
> about the tap/eBPF patch.
> >>
> >> Regards,
> >> Keith
> 
> Regards,
> Keith
  

On 6/12/2018 3:02 PM, Ophir Munk wrote:
> Please note that other than cloning iproute2 we also need to install clang and llvm tools versions 3.7 and upper.
> Not sure there are clang and llvm packages of the required versions for the common distributions. 
> I compiled the tools source code and installed them manually.

Hi Keith, Thomas,

What do you suggest on this patch?

The "tap_bpf_program.c" is already withing the tap pmd this patch improves the
doc about how to compile it, although it may not be so user friendly as Keith
pointed, I believe better to get doc improvement here.

And there is a code update "+#include "bpf_api.h", which includes a iproute2
header, I am not sure about this one and how to manage this dependency.

> 
>> -----Original Message-----
>> From: Wiles, Keith [mailto:keith.wiles@intel.com]
>> Sent: Tuesday, June 12, 2018 4:53 PM
>> To: Thomas Monjalon <thomas@monjalon.net>
>> Cc: dev@dpdk.org; Ophir Munk <ophirmu@mellanox.com>; Pascal Mazon
>> <pascal.mazon@6wind.com>; Olga Shern <olgas@mellanox.com>
>> Subject: Re: [dpdk-dev] [PATCH v1] net/tap: explain how to compile eBPF C
>> file
>>
>>
>>
>>> On Jun 12, 2018, at 8:44 AM, Thomas Monjalon <thomas@monjalon.net>
>> wrote:
>>>
>>> 12/06/2018 15:33, Wiles, Keith:
>>>>
>>>>> On Jun 12, 2018, at 7:58 AM, Thomas Monjalon
>> <thomas@monjalon.net> wrote:
>>>>>
>>>>> 12/06/2018 14:36, Wiles, Keith:
>>>>>>
>>>>>>> On Jun 12, 2018, at 7:26 AM, Thomas Monjalon
>> <thomas@monjalon.net> wrote:
>>>>>>>
>>>>>>> 11/06/2018 18:35, Wiles, Keith:
>>>>>>>>
>>>>>>>>> On Jun 11, 2018, at 11:06 AM, Ophir Munk
>> <ophirmu@mellanox.com> wrote:
>>>>>>>>>
>>>>>>>>> This commit explains how to manually compile the C source file
>>>>>>>>> tap_bpf_program.c into an ELF file using the clang compiler.
>>>>>>>>> The code in tap_bpf_program.c requires definitions found in
>>>>>>>>> iproute2 source code. This commit suggests cloning the iproute2
>>>>>>>>> git tree and include its path in the clang command. It also adds
>>>>>>>>> inclusion of file bpf_api.h (required for eBPF definitions)
>>>>>>>>> which is located in iproute2 source tree. For more details refer to
>> TAP documentation.
>>>>>>>>> This commit is related to commits [1] and [2].
>>>>>>>>
>>>>>>>> Normally I would have suggested that eBPF be disable in the TAP
>> driver as it requires external code and programs, but that ship has sailed.
>>>>>>>
>>>>>>> The external programs are required only to generate new
>>>>>>> instructions, changing the behaviour of the BPF program.
>>>>>>> Currently, the instructions for RSS behaviour are provided.
>>>>>>>
>>>>>>>> I would like to see building the tap_bpf_program.o as a target in the
>> Makefile, this way the developer can just run the ‘make bpf_program’ target
>> and it would be simpler and less error prone.
>>>>>
>>>>> As explained in the documentation, for now there is a dependency on
>>>>> iproute2 for the compilation of this BPF program.
>>>>> So we cannot make it as simple as a "make command".
>>>>> Probably that we can rework it to change the dependency.
>>>>> I heard there are some good BPF libraries available now?
>>>>
>>>> Well the dependence of iproute2 is really no different then requiring say
>> libnuma, they just have to pull the code first to type the ‘make bpf_program’
>> right?
>>>
>>> The iproute2 dependency is different because it is not a library.
>>> The .h file is never packaged.
>>> So we need to download the sources and set -I to this directory.
>>
>> To eliminate the -I problem the clone could be done inside the tap directory
>> and -I ./iproute2/include used, right?
>> The make target could even clone the code into the tap directory, which
>> means we can solve these problems you are pointing out.
>>
>> Go ahead and do what you want here, but making it harder for the developer
>> should not be our normally mode of operation.
>>
>>>
>>>
>>>> If that is the case then a make target make sense to me. If iproute2 is not
>> found then an error, right?
>>>
>>>
>>>>>>> For this to happen, we need to improve the tools.
>>>>>>
>>>>>> In what way do we need to improve the tools and which tools are we
>> talking about. Building the .o file below appears to be a simple set of
>> command lines. I have a question in my original email about what tool.
>>>>>
>>>>> The .o file is only the an intermediate file.
>>>>> The next step (numbered as 5 in this patch) is to extract the
>>>>> section of BPF instructions to be uploaded in the kernel.
>>>>> This step must be done by a "tool". Ophir did it by hacking tc, but
>>>>> it is not upstreamed yet.
>>>>> There could be other ways (possibly easier) to achieve the same result.
>>>>
>>>> Please change the doc to reflect the tool is not upstreamed yet and the
>> developer needs to figure out how to extract the data from the binary.
>>>>
>>>> I used objdump -j l3_l4 -s tap_bpf_program.o and got a hex dump of
>>>> the l3_l4 section
>>>>
>>>> 0000 bf160000 00000000 61681000 00000000 <Ascii characters> ...
>>>>
>>>> Someone schooled in the art of Python coding should be able to
>>>> convert that output to a ‘C’ data array. :-)
>>>>
>>>>>
>>>>>>> It is a work in progress.
>>>>>
>>>>> Contributions are welcome.
>>>>>
>>>>>>> This is a very first step to use Linux BPF with DPDK.
>>>>>>> If there are more interests, we should really streamline its usage
>>>>>>> for all parts of DPDK which runs on top of some kernel code.
>>>>>>
>>>>>> streamlining other parts of DPDK would be nice, but we are now talking
>> about the tap/eBPF patch.
>>>>
>>>> Regards,
>>>> Keith
>>
>> Regards,
>> Keith
>
  

04/07/2018 21:47, Ferruh Yigit:
> On 6/12/2018 3:02 PM, Ophir Munk wrote:
> > Please note that other than cloning iproute2 we also need to install clang and llvm tools versions 3.7 and upper.
> > Not sure there are clang and llvm packages of the required versions for the common distributions. 
> > I compiled the tools source code and installed them manually.
> 
> Hi Keith, Thomas,
> 
> What do you suggest on this patch?
> 
> The "tap_bpf_program.c" is already withing the tap pmd this patch improves the
> doc about how to compile it, although it may not be so user friendly as Keith
> pointed, I believe better to get doc improvement here.
> 
> And there is a code update "+#include "bpf_api.h", which includes a iproute2
> header, I am not sure about this one and how to manage this dependency.

If you feel it needs some improvement, we can postpone it for 18.11.
The most important is to have a patch to reference when somebody asks.

It can be improved and merged later, no pressure.


> >> -----Original Message-----
> >> From: Wiles, Keith [mailto:keith.wiles@intel.com]
> >>
> >>> On Jun 12, 2018, at 8:44 AM, Thomas Monjalon <thomas@monjalon.net>
> >> wrote:
> >>>
> >>> 12/06/2018 15:33, Wiles, Keith:
> >>>>
> >>>>> On Jun 12, 2018, at 7:58 AM, Thomas Monjalon
> >> <thomas@monjalon.net> wrote:
> >>>>>
> >>>>> 12/06/2018 14:36, Wiles, Keith:
> >>>>>>
> >>>>>>> On Jun 12, 2018, at 7:26 AM, Thomas Monjalon
> >> <thomas@monjalon.net> wrote:
> >>>>>>>
> >>>>>>> 11/06/2018 18:35, Wiles, Keith:
> >>>>>>>>
> >>>>>>>>> On Jun 11, 2018, at 11:06 AM, Ophir Munk
> >> <ophirmu@mellanox.com> wrote:
> >>>>>>>>>
> >>>>>>>>> This commit explains how to manually compile the C source file
> >>>>>>>>> tap_bpf_program.c into an ELF file using the clang compiler.
> >>>>>>>>> The code in tap_bpf_program.c requires definitions found in
> >>>>>>>>> iproute2 source code. This commit suggests cloning the iproute2
> >>>>>>>>> git tree and include its path in the clang command. It also adds
> >>>>>>>>> inclusion of file bpf_api.h (required for eBPF definitions)
> >>>>>>>>> which is located in iproute2 source tree. For more details refer to
> >> TAP documentation.
> >>>>>>>>> This commit is related to commits [1] and [2].
> >>>>>>>>
> >>>>>>>> Normally I would have suggested that eBPF be disable in the TAP
> >> driver as it requires external code and programs, but that ship has sailed.
> >>>>>>>
> >>>>>>> The external programs are required only to generate new
> >>>>>>> instructions, changing the behaviour of the BPF program.
> >>>>>>> Currently, the instructions for RSS behaviour are provided.
> >>>>>>>
> >>>>>>>> I would like to see building the tap_bpf_program.o as a target in the
> >> Makefile, this way the developer can just run the ‘make bpf_program’ target
> >> and it would be simpler and less error prone.
> >>>>>
> >>>>> As explained in the documentation, for now there is a dependency on
> >>>>> iproute2 for the compilation of this BPF program.
> >>>>> So we cannot make it as simple as a "make command".
> >>>>> Probably that we can rework it to change the dependency.
> >>>>> I heard there are some good BPF libraries available now?
> >>>>
> >>>> Well the dependence of iproute2 is really no different then requiring say
> >> libnuma, they just have to pull the code first to type the ‘make bpf_program’
> >> right?
> >>>
> >>> The iproute2 dependency is different because it is not a library.
> >>> The .h file is never packaged.
> >>> So we need to download the sources and set -I to this directory.
> >>
> >> To eliminate the -I problem the clone could be done inside the tap directory
> >> and -I ./iproute2/include used, right?
> >> The make target could even clone the code into the tap directory, which
> >> means we can solve these problems you are pointing out.
> >>
> >> Go ahead and do what you want here, but making it harder for the developer
> >> should not be our normally mode of operation.
> >>
> >>>
> >>>
> >>>> If that is the case then a make target make sense to me. If iproute2 is not
> >> found then an error, right?
> >>>
> >>>
> >>>>>>> For this to happen, we need to improve the tools.
> >>>>>>
> >>>>>> In what way do we need to improve the tools and which tools are we
> >> talking about. Building the .o file below appears to be a simple set of
> >> command lines. I have a question in my original email about what tool.
> >>>>>
> >>>>> The .o file is only the an intermediate file.
> >>>>> The next step (numbered as 5 in this patch) is to extract the
> >>>>> section of BPF instructions to be uploaded in the kernel.
> >>>>> This step must be done by a "tool". Ophir did it by hacking tc, but
> >>>>> it is not upstreamed yet.
> >>>>> There could be other ways (possibly easier) to achieve the same result.
> >>>>
> >>>> Please change the doc to reflect the tool is not upstreamed yet and the
> >> developer needs to figure out how to extract the data from the binary.
> >>>>
> >>>> I used objdump -j l3_l4 -s tap_bpf_program.o and got a hex dump of
> >>>> the l3_l4 section
> >>>>
> >>>> 0000 bf160000 00000000 61681000 00000000 <Ascii characters> ...
> >>>>
> >>>> Someone schooled in the art of Python coding should be able to
> >>>> convert that output to a ‘C’ data array. :-)
> >>>>
> >>>>>
> >>>>>>> It is a work in progress.
> >>>>>
> >>>>> Contributions are welcome.
> >>>>>
> >>>>>>> This is a very first step to use Linux BPF with DPDK.
> >>>>>>> If there are more interests, we should really streamline its usage
> >>>>>>> for all parts of DPDK which runs on top of some kernel code.
> >>>>>>
> >>>>>> streamlining other parts of DPDK would be nice, but we are now talking
> >> about the tap/eBPF patch.
> >>>>
> >>>> Regards,
> >>>> Keith
> >>
> >> Regards,
> >> Keith
> > 
> 
>
  

> On Jul 4, 2018, at 3:11 PM, Thomas Monjalon <thomas@monjalon.net> wrote:
> 
> 04/07/2018 21:47, Ferruh Yigit:
>> On 6/12/2018 3:02 PM, Ophir Munk wrote:
>>> Please note that other than cloning iproute2 we also need to install clang and llvm tools versions 3.7 and upper.
>>> Not sure there are clang and llvm packages of the required versions for the common distributions. 
>>> I compiled the tools source code and installed them manually.
>> 
>> Hi Keith, Thomas,
>> 
>> What do you suggest on this patch?
>> 
>> The "tap_bpf_program.c" is already withing the tap pmd this patch improves the
>> doc about how to compile it, although it may not be so user friendly as Keith
>> pointed, I believe better to get doc improvement here.
>> 
>> And there is a code update "+#include "bpf_api.h", which includes a iproute2
>> header, I am not sure about this one and how to manage this dependency.
> 
> If you feel it needs some improvement, we can postpone it for 18.11.
> The most important is to have a patch to reference when somebody asks.
> 
> It can be improved and merged later, no pressure.

I agree.

> 
> 
>>>> -----Original Message-----
>>>> From: Wiles, Keith [mailto:keith.wiles@intel.com]
>>>> 
>>>>> On Jun 12, 2018, at 8:44 AM, Thomas Monjalon <thomas@monjalon.net>
>>>> wrote:
>>>>> 
>>>>> 12/06/2018 15:33, Wiles, Keith:
>>>>>> 
>>>>>>> On Jun 12, 2018, at 7:58 AM, Thomas Monjalon
>>>> <thomas@monjalon.net> wrote:
>>>>>>> 
>>>>>>> 12/06/2018 14:36, Wiles, Keith:
>>>>>>>> 
>>>>>>>>> On Jun 12, 2018, at 7:26 AM, Thomas Monjalon
>>>> <thomas@monjalon.net> wrote:
>>>>>>>>> 
>>>>>>>>> 11/06/2018 18:35, Wiles, Keith:
>>>>>>>>>> 
>>>>>>>>>>> On Jun 11, 2018, at 11:06 AM, Ophir Munk
>>>> <ophirmu@mellanox.com> wrote:
>>>>>>>>>>> 
>>>>>>>>>>> This commit explains how to manually compile the C source file
>>>>>>>>>>> tap_bpf_program.c into an ELF file using the clang compiler.
>>>>>>>>>>> The code in tap_bpf_program.c requires definitions found in
>>>>>>>>>>> iproute2 source code. This commit suggests cloning the iproute2
>>>>>>>>>>> git tree and include its path in the clang command. It also adds
>>>>>>>>>>> inclusion of file bpf_api.h (required for eBPF definitions)
>>>>>>>>>>> which is located in iproute2 source tree. For more details refer to
>>>> TAP documentation.
>>>>>>>>>>> This commit is related to commits [1] and [2].
>>>>>>>>>> 
>>>>>>>>>> Normally I would have suggested that eBPF be disable in the TAP
>>>> driver as it requires external code and programs, but that ship has sailed.
>>>>>>>>> 
>>>>>>>>> The external programs are required only to generate new
>>>>>>>>> instructions, changing the behaviour of the BPF program.
>>>>>>>>> Currently, the instructions for RSS behaviour are provided.
>>>>>>>>> 
>>>>>>>>>> I would like to see building the tap_bpf_program.o as a target in the
>>>> Makefile, this way the developer can just run the ‘make bpf_program’ target
>>>> and it would be simpler and less error prone.
>>>>>>> 
>>>>>>> As explained in the documentation, for now there is a dependency on
>>>>>>> iproute2 for the compilation of this BPF program.
>>>>>>> So we cannot make it as simple as a "make command".
>>>>>>> Probably that we can rework it to change the dependency.
>>>>>>> I heard there are some good BPF libraries available now?
>>>>>> 
>>>>>> Well the dependence of iproute2 is really no different then requiring say
>>>> libnuma, they just have to pull the code first to type the ‘make bpf_program’
>>>> right?
>>>>> 
>>>>> The iproute2 dependency is different because it is not a library.
>>>>> The .h file is never packaged.
>>>>> So we need to download the sources and set -I to this directory.
>>>> 
>>>> To eliminate the -I problem the clone could be done inside the tap directory
>>>> and -I ./iproute2/include used, right?
>>>> The make target could even clone the code into the tap directory, which
>>>> means we can solve these problems you are pointing out.
>>>> 
>>>> Go ahead and do what you want here, but making it harder for the developer
>>>> should not be our normally mode of operation.
>>>> 
>>>>> 
>>>>> 
>>>>>> If that is the case then a make target make sense to me. If iproute2 is not
>>>> found then an error, right?
>>>>> 
>>>>> 
>>>>>>>>> For this to happen, we need to improve the tools.
>>>>>>>> 
>>>>>>>> In what way do we need to improve the tools and which tools are we
>>>> talking about. Building the .o file below appears to be a simple set of
>>>> command lines. I have a question in my original email about what tool.
>>>>>>> 
>>>>>>> The .o file is only the an intermediate file.
>>>>>>> The next step (numbered as 5 in this patch) is to extract the
>>>>>>> section of BPF instructions to be uploaded in the kernel.
>>>>>>> This step must be done by a "tool". Ophir did it by hacking tc, but
>>>>>>> it is not upstreamed yet.
>>>>>>> There could be other ways (possibly easier) to achieve the same result.
>>>>>> 
>>>>>> Please change the doc to reflect the tool is not upstreamed yet and the
>>>> developer needs to figure out how to extract the data from the binary.
>>>>>> 
>>>>>> I used objdump -j l3_l4 -s tap_bpf_program.o and got a hex dump of
>>>>>> the l3_l4 section
>>>>>> 
>>>>>> 0000 bf160000 00000000 61681000 00000000 <Ascii characters> ...
>>>>>> 
>>>>>> Someone schooled in the art of Python coding should be able to
>>>>>> convert that output to a ‘C’ data array. :-)
>>>>>> 
>>>>>>> 
>>>>>>>>> It is a work in progress.
>>>>>>> 
>>>>>>> Contributions are welcome.
>>>>>>> 
>>>>>>>>> This is a very first step to use Linux BPF with DPDK.
>>>>>>>>> If there are more interests, we should really streamline its usage
>>>>>>>>> for all parts of DPDK which runs on top of some kernel code.
>>>>>>>> 
>>>>>>>> streamlining other parts of DPDK would be nice, but we are now talking
>>>> about the tap/eBPF patch.
>>>>>> 
>>>>>> Regards,
>>>>>> Keith
>>>> 
>>>> Regards,
>>>> Keith

Regards,
Keith
  

On 7/5/2018 1:34 PM, Wiles, Keith wrote:
> 
> 
>> On Jul 4, 2018, at 3:11 PM, Thomas Monjalon <thomas@monjalon.net> wrote:
>>
>> 04/07/2018 21:47, Ferruh Yigit:
>>> On 6/12/2018 3:02 PM, Ophir Munk wrote:
>>>> Please note that other than cloning iproute2 we also need to install clang and llvm tools versions 3.7 and upper.
>>>> Not sure there are clang and llvm packages of the required versions for the common distributions. 
>>>> I compiled the tools source code and installed them manually.
>>>
>>> Hi Keith, Thomas,
>>>
>>> What do you suggest on this patch?
>>>
>>> The "tap_bpf_program.c" is already withing the tap pmd this patch improves the
>>> doc about how to compile it, although it may not be so user friendly as Keith
>>> pointed, I believe better to get doc improvement here.
>>>
>>> And there is a code update "+#include "bpf_api.h", which includes a iproute2
>>> header, I am not sure about this one and how to manage this dependency.
>>
>> If you feel it needs some improvement, we can postpone it for 18.11.
>> The most important is to have a patch to reference when somebody asks.
>>
>> It can be improved and merged later, no pressure.
> 
> I agree.

Hi Ophir,

Are you planning a new version of patch for this release?

Thanks,
ferruh