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: <70150252-5989-63b7-9224-9dd4a572099a@gmail.com>
Date:	Sun, 10 Jul 2016 19:38:42 +0300
From:	Tariq Toukan <ttoukan.linux@...il.com>
To:	Brenden Blanco <bblanco@...mgrid.com>,
	Or Gerlitz <gerlitz.or@...il.com>,
	Jesper Dangaard Brouer <brouer@...hat.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


>>> +                       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;
The more critical issue here is the default action. All packets that get 
unknown/unsupported filter classification will be dropped.
I think the XDP_PASS is the better choice here as default action, there 
are good reasons why it should be, as Jesper already explained in 
replies to other patch in the series.

Regards,
Tariq