> On Sep 25, 2018, at 7:43 AM, Michael Chan <michael.c...@broadcom.com> wrote:
>
> On Tue, Sep 25, 2018 at 7:20 AM Eric Dumazet <eduma...@google.com> wrote:
>>
>> On Tue, Sep 25, 2018 at 7:02 AM Michael Chan <michael.c...@broadcom.com>
>> wrote:
>>>
>>> On Mon, Sep 24, 2018 at 2:18 PM Song Liu <songliubrav...@fb.com> wrote:
>>>>
>>>>
>>>>
>>>>> On Sep 24, 2018, at 2:05 PM, Eric Dumazet <eduma...@google.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.
>
> All TX completions that we can see. We cannot see the in-flight ones.
>
> If budget is exceeded, I think the assumption is that poll will always
> be called again.
>
>>
>> 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)
>
> I think the latest patch is preventing the normal interrupt -> NAPI
> path from coming in and cleaning the remaining TX completions and
> arming the interrupt.
Hi Michael,
This may not be related. But I am looking at this:
bnxt_poll_work() {
while (1) {
....
if (rx_pkts == budget)
return
}
}
With budget of 0, the loop will terminate after processing one packet.
But I think the expectation is to finish all tx packets. So it doesn't
feel right. Could you please confirm?
Thanks,
Song
