All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jesper Dangaard Brouer <brouer@redhat.com>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: netdev@vger.kernel.org, brouer@redhat.com
Subject: Re: [PATCH net] samples/bpf: bpf_load.c order of prog_fd[] should correspond with ELF order
Date: Wed, 31 May 2017 16:05:32 +0200	[thread overview]
Message-ID: <20170531160532.4e4738b8@redhat.com> (raw)
In-Reply-To: <20170530163410.mhag3k5nssohcofd@ast-mbp.dhcp.thefacebook.com>

On Tue, 30 May 2017 09:34:12 -0700
Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:

> On Tue, May 30, 2017 at 02:37:51PM +0200, Jesper Dangaard Brouer wrote:
> > An eBPF ELF file generated with LLVM can contain several program
> > section, which can be used for bpf tail calls.  The bpf prog file
> > descriptors are accessible via array prog_fd[].
> > 
> > At-least XDP samples assume ordering, and uses prog_fd[0] is the main
> > XDP program to attach.  The actual order of array prog_fd[] depend on
> > whether or not a bpf program section is referencing any maps or not.
> > Not using a map result in being loaded/processed after all other
> > prog section.  Thus, this can lead to some very strange and hard to
> > debug situation, as the user can only see a FD and cannot correlated
> > that with the ELF section name.
> > 
> > The fix is rather simple, and even removes duplicate memcmp code.
> > Simply load program sections as the last step, instead of
> > load_and_attach while processing the relocation section.
> > 
> > When working with tail calls, it become even more essential that the
> > order of prog_fd[] is consistant, like the current dependency of the
> > map_fd[] order.
> > 
> > Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>  
> 
> Looks fine, but imo net-next is better, since it's not a bugfix
> to anything in the net tree, but a general improvement.

Okay, I'm fine with this going into net-next. I'm running with own
fix[1] in my git-tree, so I can get my tail call example[2] working.

[1] https://github.com/netoptimizer/prototype-kernel/commit/e785522d84d5bf
[2] https://github.com/netoptimizer/prototype-kernel/tree/master/kernel/samples/bpf

> Acked-by: Alexei Starovoitov <ast@kernel.org>

Thanks!

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

  reply	other threads:[~2017-05-31 14:05 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-30 12:37 [PATCH net] samples/bpf: bpf_load.c order of prog_fd[] should correspond with ELF order Jesper Dangaard Brouer
2017-05-30 16:34 ` Alexei Starovoitov
2017-05-31 14:05   ` Jesper Dangaard Brouer [this message]
2017-05-31 16:59 ` David Miller

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=20170531160532.4e4738b8@redhat.com \
    --to=brouer@redhat.com \
    --cc=alexei.starovoitov@gmail.com \
    --cc=netdev@vger.kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.