netdev - Re: [net-next 01/14] net/mlx5e: RX, Add a prefetch command for small L1_CACHE

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: <CAKgT0UdLCS3j6SxES9MMudYv5JhX4bdZRmvXiDy4K8tG4FehdQ@mail.gmail.com>
Date:   Tue, 23 Apr 2019 08:21:23 -0700
From:   Alexander Duyck <alexander.duyck@...il.com>
To:     Jesper Dangaard Brouer <brouer@...hat.com>
Cc:     Jakub Kicinski <jakub.kicinski@...ronome.com>,
        Saeed Mahameed <saeedm@...lanox.com>,
        Tariq Toukan <tariqt@...lanox.com>,
        "David S. Miller" <davem@...emloft.net>,
        Netdev <netdev@...r.kernel.org>, Jonathan Lemon <bsd@...com>
Subject: Re: [net-next 01/14] net/mlx5e: RX, Add a prefetch command for small L1_CACHE_BYTES

On Tue, Apr 23, 2019 at 6:23 AM Jesper Dangaard Brouer
<brouer@...hat.com> wrote:
>
> On Mon, 22 Apr 2019 19:46:47 -0700
> Jakub Kicinski <jakub.kicinski@...ronome.com> wrote:
>
> > On Mon, 22 Apr 2019 15:32:53 -0700, Saeed Mahameed wrote:
> > > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h
> > > index 51e109fdeec1..6147be23a9b9 100644
> > > --- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
> > > +++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
> > > @@ -50,6 +50,7 @@
> > >  #include <net/xdp.h>
> > >  #include <linux/net_dim.h>
> > >  #include <linux/bits.h>
> > > +#include <linux/prefetch.h>
> > >  #include "wq.h"
> > >  #include "mlx5_core.h"
> > >  #include "en_stats.h"
> > > @@ -986,6 +987,22 @@ static inline void mlx5e_cq_arm(struct mlx5e_cq *cq)
> > >     mlx5_cq_arm(mcq, MLX5_CQ_DB_REQ_NOT, mcq->uar->map, cq->wq.cc);
> > >  }
> > >
> > > +static inline void mlx5e_prefetch(void *p)
> > > +{
> > > +   prefetch(p);
> > > +#if L1_CACHE_BYTES < 128
> > > +   prefetch(p + L1_CACHE_BYTES);
> > > +#endif
> > > +}
> > > +
> > > +static inline void mlx5e_prefetchw(void *p)
> > > +{
> > > +   prefetchw(p);
> > > +#if L1_CACHE_BYTES < 128
> > > +   prefetchw(p + L1_CACHE_BYTES);
> > > +#endif
> > > +}
> >
> > All Intel drivers do the exact same thing, perhaps it's time to add a
> > helper fot this?
> >
> > net_prefetch_headers()
> >
> > or some such?
>
> I wonder if Tariq measured any effect from doing this?
>
> Because Intel CPUs will usually already prefetch the next cache-line,
> as described in [1], you can even read (and modify) this MSR 0x1A4
> e.g. via tools in [2].  Maybe Intel guys added it before this was done
> in HW, and never cleaned it up?
>
> [1] https://software.intel.com/en-us/articles/disclosure-of-hw-prefetcher-control-on-some-intel-processors

The issue is the adjacent cache line prefetcher can be on or off and a
network driver shouldn't really be going through and twiddling those
sort of bits. In some cases having it on can result in more memory
being consumed then is needed. The reason why I enabled the additional
cacheline prefetch for the Intel NICs is because most TCP packets are
at a minimum 68 bytes for just the headers so there was an advantage
for TCP traffic to make certain we prefetched at least enough for us
to process the headers.

As far as Jakub comment about combining the functions I would be okay
with that. We just need to make it a static inline function available
to all the network drivers.

Thanks.

- Alex