On 2016/04/27 11:16AM, Jesper Dangaard Brouer wrote: > On Wed, 27 Apr 2016 14:05:22 +0530 > "Naveen N. Rao" <naveen.n.rao@xxxxxxxxxxxxxxxxxx> wrote: > > > On 2016/04/27 09:30AM, Jesper Dangaard Brouer wrote: > > > Getting started with using examples in samples/bpf/ is not > > > straightforward. There are several dependencies, and specific > > > versions of these dependencies. > > > > > > Just compiling the example tool is also slightly obscure, e.g. one > > > need to call make like: > > > > > > make samples/bpf/ > > > > > > Do notice the "/" slash after the directory name. > > > > > > Signed-off-by: Jesper Dangaard Brouer <brouer@xxxxxxxxxx> > > > --- > > > samples/bpf/README.rst | 75 ++++++++++++++++++++++++++++++++++++++++++++++++ > > > 1 file changed, 75 insertions(+) > > > create mode 100644 samples/bpf/README.rst > > > > Thanks for adding this! A few nits... > > I would prefer if we could apply this patchset and you could followup > with a patch with your nits... ... and have another patch just for that? Regardless, I thought the reason we review is so the patch that goes in is already in a good shape. > > > > > > > diff --git a/samples/bpf/README.rst b/samples/bpf/README.rst > > > new file mode 100644 > > > index 000000000000..1fa157db905b > > > --- /dev/null > > > +++ b/samples/bpf/README.rst > > > @@ -0,0 +1,75 @@ > > > +eBPF sample programs > > > +==================== > > > + > > > +This kernel samples/bpf directory contains a mini eBPF library, test > > ^^^^^^^^^^^^^^^^^^ > > 'This directory contains' should suffice. > > The reason I formulated it like this, was that people will often hit > this kind of documentation when searching google. That doesn't make sense - shouldn't they be looking at a README file in the local samples/bpf directory first before going to google? > > > > > +stubs, verifier test-suite and examples for using eBPF. > > > + > > > +Build dependencies > > > +================== > > > + > > > +Compiling requires having installed: > > > + * clang >= version 3.4.0 > > > + * llvm >= version 3.7.1 > > > + > > > +Note that LLVM's tool 'llc' must support target 'bpf', list with command:: > > > + > > > + $ llc --version > > > > 'llc --version | grep bpf' is probably simpler? > > I wanted to give people the impression of how the output looks like. But, that won't help someone trying to check if their installed llc has bpf support or not. > > > > + LLVM (http://llvm.org/): > > > + LLVM version 3.x.y > > > + [...] > > > + Host CPU: xxx For instance, is the above output something the user needs to see to ensure BPF support for llc? > > > + > > > + Registered Targets: > > > + [...] > > > + bpf - BPF (host endian) > > > + bpfeb - BPF (big endian) > > > + bpfel - BPF (little endian) The above is what really matters. Adding 'grep bpf' makes it explicit on what the user needs to look for. > > > + [...] > > > + > > > +Kernel headers > > > +-------------- > > > + > > > +There are usually dependencies to header files of the current kernel. > > > +To avoid installing devel kernel headers system wide, as a normal > > > +user, simply call:: > > > + > > > + make headers_install > > > + > > > +This will creates a local "usr/include" directory in the git/build top > > > +level directory, that the make system automatically pickup first. > > > + > > > +Compiling > > > +========= > > > + > > > +For compiling goto kernel top level build directory and run make like:: > > > > For building the BPF samples, issue the below command from the kernel > > root directory: > > I like your formulation better, but it it worth a respin of the entire > patchset? > > Notice you need the extra "::" ending of the paragraph, to make this > document format nicely with RST (ReStructuredText). > > The a README with a .rst suffix will be picked up by github and > displayed as the doc for the directory. Thus I also made sure it > "compiles" with the rst tools. E.g see how samples/pktgen gets auto > documented and nicely formatted via github (scroll down): > https://github.com/torvalds/linux/tree/master/samples/pktgen Looks nice, though I wasn't aware we had any text in the kernel tree adhering to this formatting. > > > > + > > > + make samples/bpf/ > > > + > > > +Do notice the "/" slash after the directory name. > > > + > > > +Manually compiling LLVM with 'bpf' support > > > +------------------------------------------ > > > + > > > +Since version 3.7.0, LLVM adds a proper LLVM backend target for the > > > +BPF bytecode architecture. > > > + > > > +By default llvm will build all non-experimental backends including bpf. > > > +To generate a smaller llc binary one can use:: > > > + > > > + -DLLVM_TARGETS_TO_BUILD="BPF;X86" > > > > Is the X86 target really needed? > > I'm not sure, but if you want to use clang/llc for something else it is > useful, and the example usage of the ";" separator syntax makes it > worth including as an example. Ok. The reason I asked is if users need to include the appropriate arch target depending on where they build this. It doesn't look like X86 or other architecture targets are necessary though. - Naveen -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
