On Tue, Sep 25, 2018 at 7:25 PM Eric Dumazet <eduma...@google.com> wrote: > > On Tue, Sep 25, 2018 at 7:15 PM Michael Chan <michael.c...@broadcom.com> > wrote: > > > > On Tue, Sep 25, 2018 at 4:11 PM Michael Chan <michael.c...@broadcom.com> > > wrote: > > > > > > On Tue, Sep 25, 2018 at 3:15 PM Eric Dumazet <eric.duma...@gmail.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 ?
I'm pretty sure, because FB first reported TX timeouts followed by ring reset failures when running netconsole. These ring reset failures are caused by IRQs no longer working on some rings. > > 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) Good point. We have logic to handle that. We will ACK the ring at least once every tp->tx_wake_thresh TX packets. But this logic fails when the budget is 0, so I need to send a revised patch take care of this one case. > > 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. Right, not easy to hit. But it should be handled by my v2 patch. Thanks.
