netdev - Re: [PATCH net RFT] bnxt_en: Fix TX timeout during netpoll.

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: <CANn89iLiYE81yDHpfV3UWUVFxrbqJJiTkA6Tg6DAShAwdBeQ6w@mail.gmail.com>
Date:   Tue, 25 Sep 2018 19:25:34 -0700
From:   Eric Dumazet <edumazet@...gle.com>
To:     Michael Chan <michael.chan@...adcom.com>
Cc:     Eric Dumazet <eric.dumazet@...il.com>,
        Song Liu <songliubraving@...com>,
        David Miller <davem@...emloft.net>,
        netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH net RFT] bnxt_en: Fix TX timeout during netpoll.

On Tue, Sep 25, 2018 at 7:15 PM Michael Chan <michael.chan@...adcom.com> wrote:
>
> On Tue, Sep 25, 2018 at 4:11 PM Michael Chan <michael.chan@...adcom.com> wrote:
> >
> > On Tue, Sep 25, 2018 at 3:15 PM Eric Dumazet <eric.dumazet@...il.com> wrote:
> >
> > >
> > > It seems bnx2 should have a similar issue ?
> > >
> >
> > Yes, I think so.  The MSIX mode in bnx2 is also auto-masking, meaning
> > that MSIX will only assert once after it is ARMed.  If we return from
> > ->poll() when budget of 0 is reached without ARMing, we may not get
> > another MSIX.
> >
>
> On second thought, I think bnx2 is ok.  If netpoll is polling on the
> TX packets and reaching budget of 0 and returning, the INT_ACK_CMD
> register is untouched.  bnx2 uses the status block for events and the
> producers/consumers are cumulative.  So there is no need to ACK the
> status block unless ARMing for interrupts.  If there is an IRQ about
> to be fired, it won't be affected by the polling done by netpoll.
>
> In the case of bnxt, a completion ring is used for the events.  The
> polling done by netpoll will cause the completion ring to be ACKed as
> entries are processed.  ACKing the completion ring without ARMing may
> cause future IRQs to be disabled for that ring.

About bnxt : Are you sure it is all about IRQ problems ?

What if the whole ring buffer is is filled, then all entries
are processed from netpoll.

If cp_raw_cons becomes too high without the NIC knowing its (updated)
value, maybe no IRQ can be generated anymore because
of some wrapping issue (based on ring size)

I guess that in order to test this, we would need something bursting
16000 messages while holding napi->poll_owner.
The (single) IRQ would set/grab the SCHED bit but the cpu responsible
to service this (soft)irq would spin for the whole test,
and no more IRQ should be fired really.