Changeset View
Standalone View
sys/net/iflib.c
Show First 20 Lines • Show All 328 Lines • ▼ Show 20 Lines | struct iflib_txq { | ||||
uint64_t ift_cleaned; | uint64_t ift_cleaned; | ||||
uint64_t ift_cleaned_prev; | uint64_t ift_cleaned_prev; | ||||
#if MEMORY_LOGGING | #if MEMORY_LOGGING | ||||
uint64_t ift_enqueued; | uint64_t ift_enqueued; | ||||
uint64_t ift_dequeued; | uint64_t ift_dequeued; | ||||
#endif | #endif | ||||
uint64_t ift_no_tx_dma_setup; | uint64_t ift_no_tx_dma_setup; | ||||
uint64_t ift_no_desc_avail; | uint64_t ift_no_desc_avail; | ||||
uint64_t ift_mbuf_collapse_failed; | |||||
uint64_t ift_mbuf_defrag_failed; | uint64_t ift_mbuf_defrag_failed; | ||||
uint64_t ift_mbuf_defrag; | uint64_t ift_mbuf_defrag; | ||||
uint64_t ift_map_failed; | uint64_t ift_map_failed; | ||||
uint64_t ift_txd_encap_efbig; | uint64_t ift_txd_encap_efbig; | ||||
uint64_t ift_pullups; | uint64_t ift_pullups; | ||||
struct mtx ift_mtx; | struct mtx ift_mtx; | ||||
struct mtx ift_db_mtx; | struct mtx ift_db_mtx; | ||||
▲ Show 20 Lines • Show All 1,907 Lines • ▼ Show 20 Lines | for (i = 0; i < scctx->isc_ntxqsets; i++, txq++) { | ||||
/* Free any existing tx buffers. */ | /* Free any existing tx buffers. */ | ||||
for (j = 0; j < txq->ift_size; j++) { | for (j = 0; j < txq->ift_size; j++) { | ||||
iflib_txsd_free(ctx, txq, j); | iflib_txsd_free(ctx, txq, j); | ||||
} | } | ||||
txq->ift_processed = txq->ift_cleaned = txq->ift_cidx_processed = 0; | txq->ift_processed = txq->ift_cleaned = txq->ift_cidx_processed = 0; | ||||
txq->ift_in_use = txq->ift_gen = txq->ift_cidx = txq->ift_pidx = txq->ift_no_desc_avail = 0; | txq->ift_in_use = txq->ift_gen = txq->ift_cidx = txq->ift_pidx = txq->ift_no_desc_avail = 0; | ||||
txq->ift_closed = txq->ift_mbuf_defrag = txq->ift_mbuf_defrag_failed = 0; | txq->ift_closed = txq->ift_mbuf_defrag = txq->ift_mbuf_defrag_failed = 0; | ||||
txq->ift_no_tx_dma_setup = txq->ift_txd_encap_efbig = txq->ift_map_failed = 0; | txq->ift_no_tx_dma_setup = txq->ift_txd_encap_efbig = txq->ift_map_failed = 0; | ||||
txq->ift_pullups = 0; | txq->ift_pullups = txq->ift_mbuf_collapse_failed = 0; | ||||
ifmp_ring_reset_stats(txq->ift_br); | ifmp_ring_reset_stats(txq->ift_br); | ||||
for (j = 0, di = txq->ift_ifdi; j < ctx->ifc_nhwtxqs; j++, di++) | for (j = 0, di = txq->ift_ifdi; j < ctx->ifc_nhwtxqs; j++, di++) | ||||
bzero((void *)di->idi_vaddr, di->idi_size); | bzero((void *)di->idi_vaddr, di->idi_size); | ||||
} | } | ||||
for (i = 0; i < scctx->isc_nrxqsets; i++, rxq++) { | for (i = 0; i < scctx->isc_nrxqsets; i++, rxq++) { | ||||
/* make sure all transmitters have completed before proceeding XXX */ | /* make sure all transmitters have completed before proceeding XXX */ | ||||
for (j = 0, di = txq->ift_ifdi; j < ctx->ifc_nhwrxqs; j++, di++) | for (j = 0, di = txq->ift_ifdi; j < ctx->ifc_nhwrxqs; j++, di++) | ||||
▲ Show 20 Lines • Show All 703 Lines • ▼ Show 20 Lines | #endif | ||||
sgsize = PAGE_SIZE - (curaddr & PAGE_MASK); | sgsize = PAGE_SIZE - (curaddr & PAGE_MASK); | ||||
sgsize = MIN(sgsize, max_sgsize); | sgsize = MIN(sgsize, max_sgsize); | ||||
segs[i].ds_addr = curaddr; | segs[i].ds_addr = curaddr; | ||||
segs[i].ds_len = sgsize; | segs[i].ds_len = sgsize; | ||||
vaddr += sgsize; | vaddr += sgsize; | ||||
buflen -= sgsize; | buflen -= sgsize; | ||||
i++; | i++; | ||||
if (i >= max_segs) | if (i >= max_segs) | ||||
goto err; | goto err; | ||||
cramerj_intel.com: iflib_busdma_load_mbuf_sg() doesn't fail (EFBIG) because of a hardware limitation. It fails… | |||||
sbrunoUnsubmitted Not Done Inline ActionsIndeed. We should retest with this change instead. sbruno: Indeed. We should retest with this change instead. | |||||
krzysztof.galazka_intel.comAuthorUnsubmitted Not Done Inline ActionsFix proposed by Jeb solves the issue with m_collapse. I'm updating the review. krzysztof.galazka_intel.com: Fix proposed by Jeb solves the issue with m_collapse. I'm updating the review. | |||||
} | } | ||||
count++; | count++; | ||||
tmp = m; | tmp = m; | ||||
m = m->m_next; | m = m->m_next; | ||||
} while (m != NULL); | } while (m != NULL); | ||||
*nsegs = i; | *nsegs = i; | ||||
} | } | ||||
return (0); | return (0); | ||||
▲ Show 20 Lines • Show All 93 Lines • ▼ Show 20 Lines | |||||
retry: | retry: | ||||
err = iflib_busdma_load_mbuf_sg(txq, desc_tag, map, m_headp, segs, &nsegs, max_segs, BUS_DMA_NOWAIT); | err = iflib_busdma_load_mbuf_sg(txq, desc_tag, map, m_headp, segs, &nsegs, max_segs, BUS_DMA_NOWAIT); | ||||
defrag: | defrag: | ||||
if (__predict_false(err)) { | if (__predict_false(err)) { | ||||
switch (err) { | switch (err) { | ||||
case EFBIG: | case EFBIG: | ||||
/* try collapse once and defrag once */ | /* try collapse once and defrag once */ | ||||
if (remap == 0) | if (remap == 0) { | ||||
kmacyUnsubmitted Not Done Inline ActionsMost of this shuffling here is gratuitous. Please just put collapse failed stat increment under the remap 0 condition. kmacy: Most of this shuffling here is gratuitous. Please just put collapse failed stat increment under… | |||||
krzysztof.galazka_intel.comAuthorUnsubmitted Not Done Inline ActionsCould you elaborate? The point is to try m_defrag immediately after m_collapse returns NULL and not jump to defrag_failed where whole mbuf chain is dropped. It would be better to call m_defrag only if m_collapse failure wasn't caused by m_getcl but I don't see a way to do it without changing implementation of m_collapse to return reason why it failed to shorten the mbuf chain. krzysztof.galazka_intel.com: Could you elaborate? The point is to try m_defrag immediately after m_collapse returns NULL and… | |||||
kmacyUnsubmitted Not Done Inline ActionsI was confused by the fact that you're calling remap twice. There's no reason to support a second pass if you're just going to call m_defrag straight away. Also - I think what you're saying is we need to change the m_collapse interface to one that is more useful. Pass in a **mbuf for the result and have it return an error. The APIs are not immutable. We can just keep around a local copy in iflib for compat on older branches. kmacy: I was confused by the fact that you're calling remap twice. There's no reason to support a… | |||||
m_head = m_collapse(*m_headp, M_NOWAIT, max_segs); | m_head = m_collapse(*m_headp, M_NOWAIT, max_segs); | ||||
if (m_head == NULL) { | |||||
txq->ift_mbuf_collapse_failed++; | |||||
remap++; | |||||
} | |||||
} | |||||
if (remap == 1) | if (remap == 1) | ||||
m_head = m_defrag(*m_headp, M_NOWAIT); | m_head = m_defrag(*m_headp, M_NOWAIT); | ||||
remap++; | if (__predict_false(m_head == NULL || remap > 1)) | ||||
if (__predict_false(m_head == NULL)) | |||||
goto defrag_failed; | goto defrag_failed; | ||||
remap++; | |||||
txq->ift_mbuf_defrag++; | txq->ift_mbuf_defrag++; | ||||
*m_headp = m_head; | *m_headp = m_head; | ||||
goto retry; | goto retry; | ||||
break; | break; | ||||
case ENOMEM: | case ENOMEM: | ||||
txq->ift_no_tx_dma_setup++; | txq->ift_no_tx_dma_setup++; | ||||
break; | break; | ||||
default: | default: | ||||
▲ Show 20 Lines • Show All 1,034 Lines • ▼ Show 20 Lines | #endif | ||||
/* | /* | ||||
* Protect the stack against modern hardware | * Protect the stack against modern hardware | ||||
*/ | */ | ||||
if (scctx->isc_tx_tso_size_max > FREEBSD_TSO_SIZE_MAX) | if (scctx->isc_tx_tso_size_max > FREEBSD_TSO_SIZE_MAX) | ||||
scctx->isc_tx_tso_size_max = FREEBSD_TSO_SIZE_MAX; | scctx->isc_tx_tso_size_max = FREEBSD_TSO_SIZE_MAX; | ||||
/* TSO parameters - dig these out of the data sheet - simply correspond to tag setup */ | /* TSO parameters - dig these out of the data sheet - simply correspond to tag setup */ | ||||
ifp->if_hw_tsomaxsegcount = scctx->isc_tx_tso_segments_max; | |||||
/* use one segment less to avoid calling m_collapse/m_defrag */ | |||||
kmacyUnsubmitted Not Done Inline ActionsThis is the wrong place to fix this. The particular piece of hardware you're working on isn't the only device that iflib supports or can support. isc_tx_tso_segments_max is set in the driver. Please update the value there. kmacy: This is the wrong place to fix this. The particular piece of hardware you're working on isn't… | |||||
krzysztof.galazka_intel.comAuthorUnsubmitted Not Done Inline Actionsisc_tx_tso_segments_max is used in two places. Here to set if_hw_tsomaxsegcount and in iflib_encap to set max_segs for iflib_busdma_load_muf_sg. The problem is not with device supporting different value but with iflib_busdma_load_mbuf_sg failing to map mbuf chain to segments. My guess is that first mbuf in such chain has buffer not aligned to page boundary so it have to be mapped to more then one segment which causes both iflib_busdma_load_mbuf_sg and m_collapse to fail. My patch saves one segment for such situation and eliminates need to go through m_collapse and m_defrag. I see around 9% better TX performance for small send size and only around 0.1% less for large send size. Although I don't have access to platforms other then x86 so I think it might be good idea add an ifdef to limit the impact. krzysztof.galazka_intel.com: isc_tx_tso_segments_max is used in two places. Here to set if_hw_tsomaxsegcount and in… | |||||
kmacyUnsubmitted Not Done Inline ActionsYou're making a change for all drivers based on the value being, in effect, set wrong for your driver. kmacy: You're making a change for all drivers based on the value being, in effect, set wrong for your… | |||||
kmacyUnsubmitted Not Done Inline ActionsThis may make sense if you can establish that the same difference holds true for all drivers with segments / alignment. Otherwise it's best to admit that you can't reliably use that extra segment - at least on FreeBSD kmacy: This may make sense if you can establish that the same difference holds true for all drivers… | |||||
ifp->if_hw_tsomaxsegcount = scctx->isc_tx_tso_segments_max - 1; | |||||
ifp->if_hw_tsomax = scctx->isc_tx_tso_size_max; | ifp->if_hw_tsomax = scctx->isc_tx_tso_size_max; | ||||
ifp->if_hw_tsomaxsegsize = scctx->isc_tx_tso_segsize_max; | ifp->if_hw_tsomaxsegsize = scctx->isc_tx_tso_segsize_max; | ||||
if (scctx->isc_rss_table_size == 0) | if (scctx->isc_rss_table_size == 0) | ||||
scctx->isc_rss_table_size = 64; | scctx->isc_rss_table_size = 64; | ||||
scctx->isc_rss_table_mask = scctx->isc_rss_table_size-1; | scctx->isc_rss_table_mask = scctx->isc_rss_table_size-1; | ||||
GROUPTASK_INIT(&ctx->ifc_admin_task, 0, _task_fn_admin, ctx); | GROUPTASK_INIT(&ctx->ifc_admin_task, 0, _task_fn_admin, ctx); | ||||
/* XXX format name */ | /* XXX format name */ | ||||
▲ Show 20 Lines • Show All 1,354 Lines • ▼ Show 20 Lines | SYSCTL_ADD_QUAD(ctx_list, queue_list, OID_AUTO, "mbuf_defrag", | ||||
CTLFLAG_RD, | CTLFLAG_RD, | ||||
&txq->ift_mbuf_defrag, "# of times m_defrag was called"); | &txq->ift_mbuf_defrag, "# of times m_defrag was called"); | ||||
SYSCTL_ADD_QUAD(ctx_list, queue_list, OID_AUTO, "m_pullups", | SYSCTL_ADD_QUAD(ctx_list, queue_list, OID_AUTO, "m_pullups", | ||||
CTLFLAG_RD, | CTLFLAG_RD, | ||||
&txq->ift_pullups, "# of times m_pullup was called"); | &txq->ift_pullups, "# of times m_pullup was called"); | ||||
SYSCTL_ADD_QUAD(ctx_list, queue_list, OID_AUTO, "mbuf_defrag_failed", | SYSCTL_ADD_QUAD(ctx_list, queue_list, OID_AUTO, "mbuf_defrag_failed", | ||||
CTLFLAG_RD, | CTLFLAG_RD, | ||||
&txq->ift_mbuf_defrag_failed, "# of times m_defrag failed"); | &txq->ift_mbuf_defrag_failed, "# of times m_defrag failed"); | ||||
SYSCTL_ADD_QUAD(ctx_list, queue_list, OID_AUTO, "mbuf_collapse_failed", | |||||
CTLFLAG_RD, | |||||
&txq->ift_mbuf_collapse_failed, "# of times m_collapse failed"); | |||||
SYSCTL_ADD_QUAD(ctx_list, queue_list, OID_AUTO, "no_desc_avail", | SYSCTL_ADD_QUAD(ctx_list, queue_list, OID_AUTO, "no_desc_avail", | ||||
CTLFLAG_RD, | CTLFLAG_RD, | ||||
&txq->ift_no_desc_avail, "# of times no descriptors were available"); | &txq->ift_no_desc_avail, "# of times no descriptors were available"); | ||||
SYSCTL_ADD_QUAD(ctx_list, queue_list, OID_AUTO, "tx_map_failed", | SYSCTL_ADD_QUAD(ctx_list, queue_list, OID_AUTO, "tx_map_failed", | ||||
CTLFLAG_RD, | CTLFLAG_RD, | ||||
&txq->ift_map_failed, "# of times dma map failed"); | &txq->ift_map_failed, "# of times dma map failed"); | ||||
SYSCTL_ADD_QUAD(ctx_list, queue_list, OID_AUTO, "txd_encap_efbig", | SYSCTL_ADD_QUAD(ctx_list, queue_list, OID_AUTO, "txd_encap_efbig", | ||||
CTLFLAG_RD, | CTLFLAG_RD, | ||||
▲ Show 20 Lines • Show All 125 Lines • Show Last 20 Lines |
iflib_busdma_load_mbuf_sg() doesn't fail (EFBIG) because of a hardware limitation. It fails because the chain requires >= segments than the limit we specified (i >= max_segs). Should this conditional/goto (i >= max_segs) be moved to the beginning of this while-loop? If you imagine a case of setting the driver to only allow one segment, then this function will fail every time. But putting this conditional at the top will allow the segs struct to be filled out correctly for the one allowed segment.