ebpf/rss.bpf.skeleton.h generated by what?

ebpf/rss.bpf.skeleton.h generated by what?

[Markus Armbruster]

ebpf/rss.bpf.skeleton.h contains

    /* THIS FILE IS AUTOGENERATED! */

If it was generated by something in the tree, it should not be committed
to git.  Doesn't look like it is.

Perhaps it was copied from elsewhere, but the commit message offers no
clue.

Please advise.

Re: ebpf/rss.bpf.skeleton.h generated by what?

[Jason Wang]

On Fri, May 6, 2022 at 8:26 PM Markus Armbruster <armbru@redhat.com> wrote:
>
> ebpf/rss.bpf.skeleton.h contains
>
>     /* THIS FILE IS AUTOGENERATED! */
>
> If it was generated by something in the tree, it should not be committed
> to git.  Doesn't look like it is.

Andrew may know more.

I remember it was generated by libbpf.

Thanks

>
> Perhaps it was copied from elsewhere, but the commit message offers no
> clue.
>
> Please advise.
>

Re: ebpf/rss.bpf.skeleton.h generated by what?

[Paolo Bonzini]

On 5/7/22 07:05, Jason Wang wrote:
>> If it was generated by something in the tree, it should not be committed
>> to git.  Doesn't look like it is.
> Andrew may know more.
> 
> I remember it was generated by libbpf.

What is the source?

Paolo

Re: ebpf/rss.bpf.skeleton.h generated by what?

[Jason Wang]

On Sat, May 7, 2022 at 2:46 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 5/7/22 07:05, Jason Wang wrote:
> >> If it was generated by something in the tree, it should not be committed
> >> to git.  Doesn't look like it is.
> > Andrew may know more.
> >
> > I remember it was generated by libbpf.
>
> What is the source?

It's tools/ebpf/rss.bpf.c.

Thanks

>
> Paolo
>

Re: ebpf/rss.bpf.skeleton.h generated by what?

[Paolo Bonzini]

On 5/7/22 08:53, Jason Wang wrote:
> On Sat, May 7, 2022 at 2:46 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>> On 5/7/22 07:05, Jason Wang wrote:
>>>> If it was generated by something in the tree, it should not be committed
>>>> to git.  Doesn't look like it is.
>>> Andrew may know more.
>>>
>>> I remember it was generated by libbpf.
>>
>> What is the source?
> 
> It's tools/ebpf/rss.bpf.c.

Thanks, maybe we can reuse some of the support for cross compilation 
that will be introduced soon.

Paolo

Re: ebpf/rss.bpf.skeleton.h generated by what?

[Jason Wang]

On Sat, May 7, 2022 at 2:58 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 5/7/22 08:53, Jason Wang wrote:
> > On Sat, May 7, 2022 at 2:46 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
> >>
> >> On 5/7/22 07:05, Jason Wang wrote:
> >>>> If it was generated by something in the tree, it should not be committed
> >>>> to git.  Doesn't look like it is.
> >>> Andrew may know more.
> >>>
> >>> I remember it was generated by libbpf.
> >>
> >> What is the source?
> >
> > It's tools/ebpf/rss.bpf.c.
>
> Thanks, maybe we can reuse some of the support for cross compilation
> that will be introduced soon.

That would be better. The reason we don't auto generate during make is
to avoid adding new dependencies (where libbpf/bpftool is not popular
at that time).

Thanks

>
> Paolo
>

Re: ebpf/rss.bpf.skeleton.h generated by what?

[Markus Armbruster]

Jason Wang <jasowang@redhat.com> writes:

> On Sat, May 7, 2022 at 2:58 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>> On 5/7/22 08:53, Jason Wang wrote:
>> > On Sat, May 7, 2022 at 2:46 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>> >>
>> >> On 5/7/22 07:05, Jason Wang wrote:
>> >>>> If it was generated by something in the tree, it should not be committed
>> >>>> to git.  Doesn't look like it is.
>> >>> Andrew may know more.
>> >>>
>> >>> I remember it was generated by libbpf.
>> >>
>> >> What is the source?
>> >
>> > It's tools/ebpf/rss.bpf.c.

Aha:

    $ cat tools/ebpf/Makefile.ebpf
    [...]
    $(OBJS):  %.o:%.c
            $(CLANG) $(INC_FLAGS) \
                    -D__KERNEL__ -D__ASM_SYSREG_H \
                    -I../include $(LINUXINCLUDE) \
                    $(EXTRA_CFLAGS) -c $< -o -| $(LLC) -march=bpf -filetype=obj -o $@
            bpftool gen skeleton rss.bpf.o > rss.bpf.skeleton.h
--->        cp rss.bpf.skeleton.h ../../ebpf/

and

    $ cat docs/devel/ebpf_rss.rst
    [...]
    RSS eBPF program
    ----------------

    RSS program located in ebpf/rss.bpf.skeleton.h generated by bpftool.
    So the program is part of the qemu binary.
    Initially, the eBPF program was compiled by clang and source code located at tools/ebpf/rss.bpf.c.
    Prerequisites to recompile the eBPF program (regenerate ebpf/rss.bpf.skeleton.h):

            llvm, clang, kernel source tree, bpftool
            Adjust Makefile.ebpf to reflect the location of the kernel source tree

            $ cd tools/ebpf
            $ make -f Makefile.ebpf
    [...]

Together, this answers my "what" question.  It doesn't answer my next
question: why?  That one gets answered ...

>> Thanks, maybe we can reuse some of the support for cross compilation
>> that will be introduced soon.
>
> That would be better.

... here:

>                       The reason we don't auto generate during make is
> to avoid adding new dependencies (where libbpf/bpftool is not popular
> at that time).

Always, always, *always* document your reasons for doing stuff right in
the commit message, unless they are blindingly obvious.  I understand
reasons can be obvious enough to the author.  Document them anyway if
there is any chance they are not obvious to others.

Re: ebpf/rss.bpf.skeleton.h generated by what?

[Jason Wang]

On Mon, May 9, 2022 at 1:27 PM Markus Armbruster <armbru@redhat.com> wrote:
>
> Jason Wang <jasowang@redhat.com> writes:
>
> > On Sat, May 7, 2022 at 2:58 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
> >>
> >> On 5/7/22 08:53, Jason Wang wrote:
> >> > On Sat, May 7, 2022 at 2:46 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
> >> >>
> >> >> On 5/7/22 07:05, Jason Wang wrote:
> >> >>>> If it was generated by something in the tree, it should not be committed
> >> >>>> to git.  Doesn't look like it is.
> >> >>> Andrew may know more.
> >> >>>
> >> >>> I remember it was generated by libbpf.
> >> >>
> >> >> What is the source?
> >> >
> >> > It's tools/ebpf/rss.bpf.c.
>
> Aha:
>
>     $ cat tools/ebpf/Makefile.ebpf
>     [...]
>     $(OBJS):  %.o:%.c
>             $(CLANG) $(INC_FLAGS) \
>                     -D__KERNEL__ -D__ASM_SYSREG_H \
>                     -I../include $(LINUXINCLUDE) \
>                     $(EXTRA_CFLAGS) -c $< -o -| $(LLC) -march=bpf -filetype=obj -o $@
>             bpftool gen skeleton rss.bpf.o > rss.bpf.skeleton.h
> --->        cp rss.bpf.skeleton.h ../../ebpf/
>
> and
>
>     $ cat docs/devel/ebpf_rss.rst
>     [...]
>     RSS eBPF program
>     ----------------
>
>     RSS program located in ebpf/rss.bpf.skeleton.h generated by bpftool.
>     So the program is part of the qemu binary.
>     Initially, the eBPF program was compiled by clang and source code located at tools/ebpf/rss.bpf.c.
>     Prerequisites to recompile the eBPF program (regenerate ebpf/rss.bpf.skeleton.h):
>
>             llvm, clang, kernel source tree, bpftool
>             Adjust Makefile.ebpf to reflect the location of the kernel source tree
>
>             $ cd tools/ebpf
>             $ make -f Makefile.ebpf
>     [...]
>
> Together, this answers my "what" question.  It doesn't answer my next
> question: why?  That one gets answered ...
>
> >> Thanks, maybe we can reuse some of the support for cross compilation
> >> that will be introduced soon.
> >
> > That would be better.
>
> ... here:
>
> >                       The reason we don't auto generate during make is
> > to avoid adding new dependencies (where libbpf/bpftool is not popular
> > at that time).
>
> Always, always, *always* document your reasons for doing stuff right in
> the commit message, unless they are blindingly obvious.  I understand
> reasons can be obvious enough to the author.  Document them anyway if
> there is any chance they are not obvious to others.
>

Ok.

Andrew, want to post a patch to explain this?

Thanks

Re: ebpf/rss.bpf.skeleton.h generated by what?

[Peter Maydell]

On Mon, 9 May 2022 at 06:30, Markus Armbruster <armbru@redhat.com> wrote:
> Always, always, *always* document your reasons for doing stuff right in
> the commit message, unless they are blindingly obvious.  I understand
> reasons can be obvious enough to the author.  Document them anyway if
> there is any chance they are not obvious to others.

It's also nice for code-generators to say who they are
in this kind of "this file is autogenerated" comment.
For instance our own decodetree script's comments read
  /* This file is autogenerated by scripts/decodetree.py.  */

-- PMM

Re: ebpf/rss.bpf.skeleton.h generated by what?

[Jason Wang]

On Mon, May 9, 2022 at 4:42 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Mon, 9 May 2022 at 06:30, Markus Armbruster <armbru@redhat.com> wrote:
> > Always, always, *always* document your reasons for doing stuff right in
> > the commit message, unless they are blindingly obvious.  I understand
> > reasons can be obvious enough to the author.  Document them anyway if
> > there is any chance they are not obvious to others.
>
> It's also nice for code-generators to say who they are
> in this kind of "this file is autogenerated" comment.
> For instance our own decodetree script's comments read
>   /* This file is autogenerated by scripts/decodetree.py.  */

Unfortunately, this is not what bpftool did right now.

Have posted a patch (with Markus and you cced).

Thanks

>
> -- PMM
>

Re: ebpf/rss.bpf.skeleton.h generated by what?

[Andrew Melnichenko]

Hi all,
The ebpf/rss.bpf.skeleton.h is generetad by bpftool from tools/ebpf/rss.bpf.c.
To generate the skeleton - would require dependencies of llvm and bpftool.
RSS eBPF solution, for now, is only for Linux hosts.

> Andrew, want to post a patch to explain this?
I am all in for clarification. And will prepare a new patch if required.
Please advise, what those patches should be? Documentation update?


On Mon, May 9, 2022 at 12:35 PM Jason Wang <jasowang@redhat.com> wrote:
>
> On Mon, May 9, 2022 at 4:42 PM Peter Maydell <peter.maydell@linaro.org> wrote:
> >
> > On Mon, 9 May 2022 at 06:30, Markus Armbruster <armbru@redhat.com> wrote:
> > > Always, always, *always* document your reasons for doing stuff right in
> > > the commit message, unless they are blindingly obvious.  I understand
> > > reasons can be obvious enough to the author.  Document them anyway if
> > > there is any chance they are not obvious to others.
> >
> > It's also nice for code-generators to say who they are
> > in this kind of "this file is autogenerated" comment.
> > For instance our own decodetree script's comments read
> >   /* This file is autogenerated by scripts/decodetree.py.  */
>
> Unfortunately, this is not what bpftool did right now.
>
> Have posted a patch (with Markus and you cced).
>
> Thanks
>
> >
> > -- PMM
> >
>

Re: ebpf/rss.bpf.skeleton.h generated by what?

[Jason Wang]

On Tue, May 10, 2022 at 8:24 PM Andrew Melnichenko <andrew@daynix.com> wrote:
>
> Hi all,
> The ebpf/rss.bpf.skeleton.h is generetad by bpftool from tools/ebpf/rss.bpf.c.
> To generate the skeleton - would require dependencies of llvm and bpftool.
> RSS eBPF solution, for now, is only for Linux hosts.
>
> > Andrew, want to post a patch to explain this?
> I am all in for clarification. And will prepare a new patch if required.
> Please advise, what those patches should be? Documentation update?

It looks to me the doc are fine:

"""
RSS eBPF program
----------------

RSS program located in ebpf/rss.bpf.skeleton.h generated by bpftool.
So the program is part of the qemu binary.
Initially, the eBPF program was compiled by clang and source code
located at tools/ebpf/rss.bpf.c.
"""

It was too late to fix the changelog but we probably don't have anything to do.

Or we can update ebpf/rss.bpf.skeleton.h by using the most recent
bpftool where I've added the BPFTOOL as the generator name[1].

Thanks

[1] https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/commit/?id=56c3e749d08a

>
>
> On Mon, May 9, 2022 at 12:35 PM Jason Wang <jasowang@redhat.com> wrote:
> >
> > On Mon, May 9, 2022 at 4:42 PM Peter Maydell <peter.maydell@linaro.org> wrote:
> > >
> > > On Mon, 9 May 2022 at 06:30, Markus Armbruster <armbru@redhat.com> wrote:
> > > > Always, always, *always* document your reasons for doing stuff right in
> > > > the commit message, unless they are blindingly obvious.  I understand
> > > > reasons can be obvious enough to the author.  Document them anyway if
> > > > there is any chance they are not obvious to others.
> > >
> > > It's also nice for code-generators to say who they are
> > > in this kind of "this file is autogenerated" comment.
> > > For instance our own decodetree script's comments read
> > >   /* This file is autogenerated by scripts/decodetree.py.  */
> >
> > Unfortunately, this is not what bpftool did right now.
> >
> > Have posted a patch (with Markus and you cced).
> >
> > Thanks
> >
> > >
> > > -- PMM
> > >
> >
>

Re: ebpf/rss.bpf.skeleton.h generated by what?

[Markus Armbruster]

Jason Wang <jasowang@redhat.com> writes:

> On Tue, May 10, 2022 at 8:24 PM Andrew Melnichenko <andrew@daynix.com> wrote:
>>
>> Hi all,
>> The ebpf/rss.bpf.skeleton.h is generetad by bpftool from tools/ebpf/rss.bpf.c.
>> To generate the skeleton - would require dependencies of llvm and bpftool.
>> RSS eBPF solution, for now, is only for Linux hosts.
>>
>> > Andrew, want to post a patch to explain this?
>> I am all in for clarification. And will prepare a new patch if required.
>> Please advise, what those patches should be? Documentation update?
>
> It looks to me the doc are fine:
>
> """
> RSS eBPF program
> ----------------
>
> RSS program located in ebpf/rss.bpf.skeleton.h generated by bpftool.
> So the program is part of the qemu binary.
> Initially, the eBPF program was compiled by clang and source code
> located at tools/ebpf/rss.bpf.c.
> """

Agreed.

> It was too late to fix the changelog but we probably don't have anything to do.

Next time :)

> Or we can update ebpf/rss.bpf.skeleton.h by using the most recent
> bpftool where I've added the BPFTOOL as the generator name[1].

Won't hurt, but no rush.

> Thanks
>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/commit/?id=56c3e749d08a