qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Shreesh Adiga <16567adigashreesh@gmail.com>
To: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: qemu-devel@nongnu.org, jasowang@redhat.com, andrew@daynix.com,
	yuri.benditovich@daynix.com
Subject: Re: [PATCH] ebpf: fix compatibility with libbpf 1.0+
Date: Tue, 20 Dec 2022 19:32:47 +0530	[thread overview]
Message-ID: <Y6HAh5/u9N50wMah@arch.localdomain> (raw)
In-Reply-To: <Y6Br8RW4dwdMohCN@redhat.com>

On Mon, Dec 19, 2022 at 01:49:37PM +0000, Daniel P. Berrangé wrote:
> On Mon, Dec 19, 2022 at 07:14:54PM +0530, Shreesh Adiga wrote:
> > Hi Daniel,
> >
> > On Mon, Dec 19, 2022 at 10:56:57AM +0000, Daniel P. Berrangé wrote:
> > > On Sun, Dec 18, 2022 at 08:09:27PM +0530, Shreesh Adiga wrote:
> > > > The current implementation fails to load on a system with
> > > > libbpf 1.0 and reports that legacy map definitions in 'maps'
> > > > section are not supported by libbpf v1.0+. This commit updates
> > > > the Makefile to add BTF (-g flag) and appropriately updates
> > > > the maps in rss.bpf.c and update the skeleton file in repo.
> > >
> > > Can you split this into two pieces - one updating the build
> > > system for new compiler usage, and one updating the program
> > > to remove the legacy map defs.
> > >
> > If I just update the Makefile first, rss.bpf.c doesn't compile
> > and throws error:
> > rss.bpf.c:80:1: error: variable has incomplete type 'struct bpf_map_def'
> >
> > Similarly if first rss.bpf.c only is updated, then error is thrown:
> > libbpf: BTF is required, but is missing or corrupted
> > Hence, it would seem logical to update both of them together in same
> > commit.
> >
> > Do you mean first commit should update the Makefile and rss.bpf.c
> > together and second commit should be updating the rss.bpf.skeleton.h
> > file? I was under the impression that every commit should result in
> > compilable sources, hence wanted to clarify this.
>
> Yes, it must be compilable. I was under the impression from the
> commit that these were independant changes, but I was wrong.
>
Could you please confirm if the current single patch is good enough for
submission, or do I need to send a v2 series with changes split into
two commits, one for Makefile + rss.bpf.c and second one for skeleton
file update?

Thanks,
Shreesh


  reply	other threads:[~2022-12-20 14:04 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-18 14:39 [PATCH] ebpf: fix compatibility with libbpf 1.0+ Shreesh Adiga
2022-12-18 16:15 ` Philippe Mathieu-Daudé
2022-12-20 16:30   ` Shreesh Adiga
2022-12-28 16:19     ` Andrew Melnichenko
2023-02-14 20:10       ` Andrew Melnichenko
2023-02-16  5:28         ` Jason Wang
2022-12-19 10:56 ` Daniel P. Berrangé
2022-12-19 13:44   ` Shreesh Adiga
2022-12-19 13:49     ` Daniel P. Berrangé
2022-12-20 14:02       ` Shreesh Adiga [this message]
2022-12-20 14:05         ` Daniel P. Berrangé

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y6HAh5/u9N50wMah@arch.localdomain \
    --to=16567adigashreesh@gmail.com \
    --cc=andrew@daynix.com \
    --cc=berrange@redhat.com \
    --cc=jasowang@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=yuri.benditovich@daynix.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).