netdev - Re: [PATCH net 00/15] netpoll: avoid capture effects for NAPI drivers

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: <CANn89i+rp-fAfLDSBpLN61X9R9hN4JQyoJgP8T=SUJ8PcSva-Q@mail.gmail.com>
Date:   Tue, 25 Sep 2018 07:20:19 -0700
From:   Eric Dumazet <edumazet@...gle.com>
To:     michael.chan@...adcom.com
Cc:     songliubraving@...com, David Miller <davem@...emloft.net>,
        netdev <netdev@...r.kernel.org>,
        Ariel Elior <ariel.elior@...ium.com>,
        Eric Dumazet <eric.dumazet@...il.com>,
        Tariq Toukan <tariqt@...lanox.com>,
        Saeed Mahameed <saeedm@...lanox.com>,
        Jeff Kirsher <jeffrey.t.kirsher@...el.com>,
        jakub.kicinski@...ronome.com, Jay Vosburgh <j.vosburgh@...il.com>,
        Veaceslav Falico <vfalico@...il.com>,
        Andy Gospodarek <andy@...yhouse.net>
Subject: Re: [PATCH net 00/15] netpoll: avoid capture effects for NAPI drivers

On Tue, Sep 25, 2018 at 7:02 AM Michael Chan <michael.chan@...adcom.com> wrote:
>
> On Mon, Sep 24, 2018 at 2:18 PM Song Liu <songliubraving@...com> wrote:
> >
> >
> >
> > > On Sep 24, 2018, at 2:05 PM, Eric Dumazet <edumazet@...gle.com> wrote:
> > >
> > >>
> > >> Interesting, maybe a bnxt specific issue.
> > >>
> > >> It seems their model is to process TX/RX notification in the same queue,
> > >> they throw away RX events if budget == 0
> > >>
> > >> It means commit e7b9569102995ebc26821789628eef45bd9840d8 is wrong and
> > >> must be reverted.
> > >>
> > >> Otherwise, we have a possibility of blocking a queue under netpoll pressure.
> > >
> > > Hmm, actually a revert might not be enough, since code at lines 2030-2031
> > > would fire and we might not call napi_complete_done() anyway.
> > >
> > > Unfortunately this driver logic is quite complex.
> > >
> > > Could you test on other NIC eventually ?
> > >
> >
> > It actually runs OK on ixgbe.
> >
> > @Michael, could you please help us with this?
> >
> I've taken a quick look using today's net tree plus Eric's
> poll_one_napi() patch.  The problem I'm seeing is that netpoll calls
> bnxt_poll() with budget 0.  And since work_done >= budget of 0, we
> return without calling napi_complete_done() and without arming the
> interrupt.  netpoll doesn't always call us back until we call
> napi_complete_done(), right?  So I think if there are in-flight TX
> completions, we'll miss those.

That's the whole point of netpoll :

 We drain the TX queues, without interrupts being involved at all,
by calling ->napi() with a zero budget.

napi_complete(), even if called from ->napi() while budget was zero,
should do nothing but return early.

budget==0 means that ->napi() should process all TX completions.

So it looks like bnxt has a bug, that is showing up after the latest
poll_one_napi() patch.
This latest patch is needed otherwise the cpu attempting the
netpoll-TX-drain might drain nothing at all,
since it does not anymore call ndo_poll_controller() that was grabbing
SCHED bits on all queues (napi_schedule() like calls)