[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20160720173338.GA16467@gmail.com>
Date: Wed, 20 Jul 2016 10:33:39 -0700
From: Brenden Blanco <bblanco@...mgrid.com>
To: Daniel Borkmann <daniel@...earbox.net>
Cc: davem@...emloft.net, netdev@...r.kernel.org,
Jamal Hadi Salim <jhs@...atatu.com>,
Saeed Mahameed <saeedm@....mellanox.co.il>,
Martin KaFai Lau <kafai@...com>,
Jesper Dangaard Brouer <brouer@...hat.com>,
Ari Saha <as754m@....com>,
Alexei Starovoitov <alexei.starovoitov@...il.com>,
Or Gerlitz <gerlitz.or@...il.com>, john.fastabend@...il.com,
hannes@...essinduktion.org, Thomas Graf <tgraf@...g.ch>,
Tom Herbert <tom@...bertland.com>,
Tariq Toukan <ttoukan.linux@...il.com>
Subject: Re: [PATCH v10 05/12] net/mlx4_en: add support for fast rx drop bpf
program
On Wed, Jul 20, 2016 at 11:07:57AM +0200, Daniel Borkmann wrote:
> On 07/19/2016 09:16 PM, Brenden Blanco wrote:
[...]
> >+ if (ring->xdp_prog)
> >+ bpf_prog_put(ring->xdp_prog);
>
> Would be good if you also make this a READ_ONCE() here. I believe this is the
> only other spot in your set that has this 'direct' access (besides xchg() and
> READ_ONCE() from mlx4_en_process_rx_cq()). It would be mostly for consistency
> and to indicate that there's a more complex synchronization behind it. I'm mostly
> worried that if it's not consistently used, people might copy this and not use
> the READ_ONCE() also in other spots where it matters, and thus add hard to find
> bugs.
I can do that. My thinking was just that this is the cleanup path so the
code would have been superfluous. I think there were a few nits so I'll
collect those and clean them up.
>
[...]
Powered by blists - more mailing lists