netdev - Re: [PATCH v6 04/12] net/mlx4_en: add support for fast rx drop bpf program

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: <20160710154029.GA6657@gmail.com>
Date:	Sun, 10 Jul 2016 08:40:31 -0700
From:	Brenden Blanco <bblanco@...mgrid.com>
To:	Or Gerlitz <gerlitz.or@...il.com>
Cc:	David Miller <davem@...emloft.net>,
	Linux Netdev List <netdev@...r.kernel.org>,
	Martin KaFai Lau <kafai@...com>,
	Jesper Dangaard Brouer <brouer@...hat.com>,
	Ari Saha <as754m@....com>,
	Alexei Starovoitov <alexei.starovoitov@...il.com>,
	john fastabend <john.fastabend@...il.com>,
	Hannes Frederic Sowa <hannes@...essinduktion.org>,
	Thomas Graf <tgraf@...g.ch>, Tom Herbert <tom@...bertland.com>,
	Daniel Borkmann <daniel@...earbox.net>
Subject: Re: [PATCH v6 04/12] net/mlx4_en: add support for fast rx drop bpf
 program

On Sat, Jul 09, 2016 at 05:07:36PM +0300, Or Gerlitz wrote:
> On Fri, Jul 8, 2016 at 5:15 AM, Brenden Blanco <bblanco@...mgrid.com> wrote:
> > Add support for the BPF_PROG_TYPE_XDP hook in mlx4 driver.
> >
> > In tc/socket bpf programs, helpers linearize skb fragments as needed
> > when the program touchs the packet data. However, in the pursuit of
> 
> nit, for the next version touchs --> touches
Will fix.
> 
[...]
> > +                       switch (act) {
> > +                       case XDP_PASS:
> > +                               break;
> > +                       default:
> > +                               bpf_warn_invalid_xdp_action(act);
> > +                       case XDP_DROP:
> > +                               goto next;
> > +                       }
> > +               }
> 
> 
> (probably a nit too, but wanted to make sure we don't miss something
> here) is the default case preceding the DROP one in purpose? any
> special reason to do that?
This is intentional, and legal though unconventional C. Without this
order, the later patches end up with a bit too much copy/paste for my
liking, as in:

case XDP_DROP:
        if (mlx4_en_rx_recycle(ring, frags))
                goto consumed;
        goto next;
default:
        bpf_warn_invalid_xdp_action(act);
        if (mlx4_en_rx_recycle(ring, frags))
                goto consumed;
        goto next;