netdev - Re: [WIP] net+mlx4: auto doorbell

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] [day] [month] [year] [list]

Message-ID: <1480521021.18162.183.camel@edumazet-glaptop3.roam.corp.google.com>
Date:   Wed, 30 Nov 2016 07:50:21 -0800
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc:     Jesper Dangaard Brouer <brouer@...hat.com>,
        Rick Jones <rick.jones2@....com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Saeed Mahameed <saeedm@...lanox.com>,
        Tariq Toukan <tariqt@...lanox.com>
Subject: Re: [WIP] net+mlx4: auto doorbell

On Tue, 2016-11-29 at 23:28 -0800, Alexei Starovoitov wrote:
> On Mon, Nov 28, 2016 at 10:58 PM, Eric Dumazet <eric.dumazet@...il.com> wrote:
> >  {
> > @@ -496,8 +531,13 @@ static bool mlx4_en_process_tx_cq(struct net_device *dev,
> >         wmb();
> >
> >         /* we want to dirty this cache line once */
> > -       ACCESS_ONCE(ring->last_nr_txbb) = last_nr_txbb;
> > -       ACCESS_ONCE(ring->cons) = ring_cons + txbbs_skipped;
> > +       WRITE_ONCE(ring->last_nr_txbb, last_nr_txbb);
> > +       ring_cons += txbbs_skipped;
> > +       WRITE_ONCE(ring->cons, ring_cons);
> > +       WRITE_ONCE(ring->ncons, ring_cons + last_nr_txbb);
> > +
> > +       if (dev->doorbell_opt)
> > +               mlx4_en_xmit_doorbell(dev, ring);
> ...
> > +       /* Doorbell avoidance : We can omit doorbell if we know a TX completion
> > +        * will happen shortly.
> > +        */
> > +       if (send_doorbell &&
> > +           dev->doorbell_opt &&
> > +           (s32)(READ_ONCE(ring->prod_bell) - READ_ONCE(ring->ncons)) > 0)
> > +               send_doorbell = false;
> 
> this is awesome idea!
> I'm missing though why you need all these read_once/write_once.
> It's a tx ring that is already protected by tx queue lock
> or completion is happening without grabbing the lock?

Yes, TX completion runs lockless, on any good/recent NIC driver.


> Then how do we make sure that we don't race here
> and indeed above prod_bell - ncons > 0 condition is safe?

That is why I have a cmpxchg()

> Good description of algorithm would certainly help ;)

Sure, when it is ready ;)