linux-kernel - Re: [PATCH bpf-next] samples/bpf: xdp_redirect

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAGyo_hp2Uunp0_McN3J8MjSeF593thwiODfUaiE-u_NXArEDPg@mail.gmail.com>
Date:   Wed, 4 Aug 2021 10:57:25 -0700
From:   Matt Cover <werekraken@...il.com>
To:     Kumar Kartikeya Dwivedi <memxor@...il.com>
Cc:     Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <jakub.kicinski@...ronome.com>,
        Jesper Dangaard Brouer <hawk@...nel.org>,
        John Fastabend <john.fastabend@...il.com>,
        Andrii Nakryiko <andrii@...nel.org>,
        Martin KaFai Lau <kafai@...com>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        KP Singh <kpsingh@...nel.org>,
        Lorenzo Bianconi <lorenzo@...nel.org>,
        Matthew Cover <matthew.cover@...ckpath.com>,
        netdev <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH bpf-next] samples/bpf: xdp_redirect_cpu: Add mprog-disable
 to optstring.

On Sat, Jul 31, 2021 at 8:25 AM Kumar Kartikeya Dwivedi
<memxor@...il.com> wrote:
>
> On Sat, Jul 31, 2021 at 06:26:32AM IST, Matthew Cover wrote:
> > Commit ce4dade7f12a ("samples/bpf: xdp_redirect_cpu: Load a eBPF program
> > on cpumap") added the following option, but missed adding it to optstring:
> > - mprog-disable: disable loading XDP program on cpumap entries
> >
> > Add the missing option character.
> >
>
> I made some changes in this area in [0], since the support was primarily to do
> redirection from the cpumap prog, so by default we don't install anything now
> and only do so if a redirection interface is specified (and use devmap instead).
> So this option won't be used anyway going forward (since we don't install a
> dummy XDP_PASS program anymore) if it gets accepted.
>
> [0]: https://lore.kernel.org/bpf/20210728165552.435050-1-memxor@gmail.com
>
> PS: I can restore it again if this is something really used beyond redirecting
> to another device (i.e. with custom BPF programs). Any feedback would be helpful.

Hey Kartikeya. I happened to be looking through this code to get a
feel for using CPUMAP for custom steering (e.g. RSS on encapsulated
packets) in XDP and noticed the missing option character. Figured it
was worth doing a quick patch and test.

Unfortunately, I'm not able to say much on your changes as I'm still
getting familiarized with this code. It looks like your submission is
in need of a rebase; v3 has been marked "Changes Requested" in
patchwork [0]. As I see things, It'd be good to get this fix in there
for now, whether or not the code is removed later.

[0]:https://patchwork.kernel.org/project/netdevbpf/patch/20210728165552.435050-9-memxor@gmail.com/

>
> > Fixes: ce4dade7f12a ("samples/bpf: xdp_redirect_cpu: Load a eBPF program on cpumap")
> > Signed-off-by: Matthew Cover <matthew.cover@...ckpath.com>
> > ---
> >  samples/bpf/xdp_redirect_cpu_user.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/samples/bpf/xdp_redirect_cpu_user.c b/samples/bpf/xdp_redirect_cpu_user.c
> > index d3ecdc1..9e225c9 100644
> > --- a/samples/bpf/xdp_redirect_cpu_user.c
> > +++ b/samples/bpf/xdp_redirect_cpu_user.c
> > @@ -841,7 +841,7 @@ int main(int argc, char **argv)
> >       memset(cpu, 0, n_cpus * sizeof(int));
> >
> >       /* Parse commands line args */
> > -     while ((opt = getopt_long(argc, argv, "hSd:s:p:q:c:xzFf:e:r:m:",
> > +     while ((opt = getopt_long(argc, argv, "hSd:s:p:q:c:xzFf:e:r:m:n",
> >                                 long_options, &longindex)) != -1) {
> >               switch (opt) {
> >               case 'd':
> > --
> > 1.8.3.1
> >
>
> --
> Kartikeya