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

Message-ID: <1480602274.18162.285.camel@edumazet-glaptop3.roam.corp.google.com>
Date:   Thu, 01 Dec 2016 06:24:34 -0800
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     Jesper Dangaard Brouer <brouer@...hat.com>
Cc:     Saeed Mahameed <saeedm@....mellanox.co.il>,
        Rick Jones <rick.jones2@....com>,
        Linux Netdev List <netdev@...r.kernel.org>,
        Saeed Mahameed <saeedm@...lanox.com>,
        Tariq Toukan <tariqt@...lanox.com>
Subject: Re: [WIP] net+mlx4: auto doorbell

On Thu, 2016-12-01 at 13:05 +0100, Jesper Dangaard Brouer wrote:
> On Wed, 30 Nov 2016 18:27:45 +0200
> Saeed Mahameed <saeedm@....mellanox.co.il> wrote:
> 
> > >> All in all, this is risky business :),  the right way to go is to
> > >> force the upper layer to use xmit-more and delay doorbells/use bulking
> > >> but from the same context (xmit routine).  For example see
> > >> Achiad's suggestion (attached in Jesper's response), he used stop
> > >> queue to force the stack to queue up packets (TX bulking)
> > >> which would set xmit-more and will use the next completion to
> > >> release the "stopped" ring TXQ rather than hit the doorbell on
> > >> behalf of it.  
> > >
> > > Well, you depend on having a higher level queue like a qdisc.
> > >
> > > Some users do not use a qdisc.
> > > If you stop the queue, they no longer can send anything -> drops.
> > >
> 
> You do have a point that stopping the device might not be the best way
> to create a push-back (to allow stack queue packets).
> 
>  netif_tx_stop_queue() / __QUEUE_STATE_DRV_XOFF
> 
> 
> > In this case, i think they should implement their own bulking (pktgen
> > is not a good example) but XDP can predict if it has more packets to
> > xmit  as long as all of them fall in the same NAPI cycle.
> > Others should try and do the same.
> 
> I actually agree with Saeed here.
> 
> Maybe we can come up with another __QUEUE_STATE_xxx that informs the
> upper layer what the driver is doing.  Then users not using a qdisc can
> use this indication (like the qdisc could).  (qdisc-bypass users already
> check the QUEUE_STATE flags e.g. via netif_xmit_frozen_or_drv_stopped).

Can you explain how this is going to help trafgen using AF_PACKET with
Qdisc bypass ?

Say trafgen wants to send 10 or 1000 packets back to back (as fast as
possible)

With my proposal, only the first is triggering a doorbell from
ndo_start_xmit(). Following ones are driven by TX completion logic, or
BQL if we can push packets faster than TX interrupt can be
delivered/handled.

If you stop the queue (with yet another atomic operations to stop/unstop
btw), packet_direct_xmit() will have to drop trafgen packets on the
floor.

We already have BQL stopping the queue at a fine granularity.
I suspect that Saeed proposal will interfere with BQL logic.