Date: Fri, 23 Apr 2021 09:58:04 GMT From: Andrew Gallatin <gallatin@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: 3183d0b68072 - main - iflib: initialize LRO unconditionally Message-ID: <202104230958.13N9w4Jf005047@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by gallatin: URL: https://cgit.FreeBSD.org/src/commit/?id=3183d0b68072dda0e80bb6e03c970625f2823e97 commit 3183d0b68072dda0e80bb6e03c970625f2823e97 Author: Andrew Gallatin <gallatin@FreeBSD.org> AuthorDate: 2021-04-23 09:51:22 +0000 Commit: Andrew Gallatin <gallatin@FreeBSD.org> CommitDate: 2021-04-23 09:55:20 +0000 iflib: initialize LRO unconditionally Changes to the LRO code have exposed a bug in iflib where devices which are not capable of doing LRO are still calling tcp_lro_flush_all(), even when they have not initialized the LRO context. This used to be mostly harmless, but the LRO code now sets the VNET based on the ifp in the lro context and will try to access it through a NULL ifp resulting in a panic at boot. To fix this, we unconditionally initializes LRO so that we have a valid LRO context when calling tcp_lro_flush_all(). One alternative is to check the device capabilities before calling tcp_lro_flush_all() or adding a new state flag in the ctx. However, it seems unwise to add an extra, mostly useless test for higher performance devices when we can just initialize LRO for all devices. Reviewed by: erj, hselasky, markj, olivier Sponsored by: Netflix Differential Revision: https://reviews.freebsd.org/D29928 --- sys/net/iflib.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/sys/net/iflib.c b/sys/net/iflib.c index 6dbaff556a15..fc0814d0fc19 100644 --- a/sys/net/iflib.c +++ b/sys/net/iflib.c @@ -5891,15 +5891,13 @@ iflib_rx_structures_setup(if_ctx_t ctx) for (q = 0; q < ctx->ifc_softc_ctx.isc_nrxqsets; q++, rxq++) { #if defined(INET6) || defined(INET) - if (if_getcapabilities(ctx->ifc_ifp) & IFCAP_LRO) { - err = tcp_lro_init_args(&rxq->ifr_lc, ctx->ifc_ifp, - TCP_LRO_ENTRIES, min(1024, - ctx->ifc_softc_ctx.isc_nrxd[rxq->ifr_fl_offset])); - if (err != 0) { - device_printf(ctx->ifc_dev, - "LRO Initialization failed!\n"); - goto fail; - } + err = tcp_lro_init_args(&rxq->ifr_lc, ctx->ifc_ifp, + TCP_LRO_ENTRIES, min(1024, + ctx->ifc_softc_ctx.isc_nrxd[rxq->ifr_fl_offset])); + if (err != 0) { + device_printf(ctx->ifc_dev, + "LRO Initialization failed!\n"); + goto fail; } #endif IFDI_RXQ_SETUP(ctx, rxq->ifr_id); @@ -5914,8 +5912,7 @@ fail: */ rxq = ctx->ifc_rxqs; for (i = 0; i < q; ++i, rxq++) { - if (if_getcapabilities(ctx->ifc_ifp) & IFCAP_LRO) - tcp_lro_free(&rxq->ifr_lc); + tcp_lro_free(&rxq->ifr_lc); } return (err); #endif @@ -5938,8 +5935,7 @@ iflib_rx_structures_free(if_ctx_t ctx) iflib_dma_free(&rxq->ifr_ifdi[j]); iflib_rx_sds_free(rxq); #if defined(INET6) || defined(INET) - if (if_getcapabilities(ctx->ifc_ifp) & IFCAP_LRO) - tcp_lro_free(&rxq->ifr_lc); + tcp_lro_free(&rxq->ifr_lc); #endif } free(ctx->ifc_rxqs, M_IFLIB);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202104230958.13N9w4Jf005047>