Check for Rx ring state to prevent from stall in the ENA driver
ClosedPublic
Actions

Authored by mw on Oct 31 2017, 1:33 PM.

Details

Reviewers

Commits

Summary

In case when Rx ring is full and driver will fail to allocate Rx mbufs,
the ring could be stalled.

Keep alive is checking every second for Rx ring state, and if it is full
for two cycles, then trigger rx_cleanup routine in another thread.

Repository

rS FreeBSD src repository - subversion

Lint

Lint Skipped

Unit

Tests Skipped

sys/dev/ena/ena.c
857	Suggest making this a M_WAITOK flag and remove lines 879-884, 899-900. This will keep it consistent with the earlier malloc calls in this routine.
901	nit. Make this while (taskqueue_cancel(rx_ring->cmpl_tq, &rx_ring->cmpl_task, NULL) != 0) to match the style in this file.
3432	Is 2 sec picked because this value must be < DEFAULT_KEEP_ALIVE_TO which 6 sec?

sys/dev/ena/ena.c
3432	It is not unit of time, this is indicating how many times in the row we must detect that rx_ring hadn't available descriptors before we will trigger deferred rx cleanup (in another thread, because it is highly possible that interrupts are stalled and that's why the ring wasn't cleaned up). Please take a look at the function check_for_empty_rx_ring() and how we are using it there.

byenduri_gmail.com added inline comments.

sys/dev/ena/ena.c
3432	OK. I see check_for_empty_rx_ring() is called from the ena_timer_service() routine which is scheduled every second. Hence we will wait a minimum of 2 seconds before triggering deferred cleanup. I was curious as to why 2 seconds and not some other number (say 30 seconds).

This revision is now accepted and ready to land.Nov 8 2017, 5:43 PM

mw commandeered this revision.Nov 9 2017, 11:47 AM

mw edited reviewers, added: mk_semihalf.com; removed: mw.

This revision was automatically updated to reflect the committed changes.