From owner-dev-commits-src-all@freebsd.org Sun Jan 10 14:33:51 2021 Return-Path: Delivered-To: dev-commits-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id E64824CE7DB; Sun, 10 Jan 2021 14:33:51 +0000 (UTC) (envelope-from cy.schubert@cschubert.com) Received: from smtp-out-no.shaw.ca (smtp-out-no.shaw.ca [64.59.134.13]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "Client", Issuer "CA" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id 4DDK8b3VQDz4jt8; Sun, 10 Jan 2021 14:33:51 +0000 (UTC) (envelope-from cy.schubert@cschubert.com) Received: from spqr.komquats.com ([70.67.229.168]) by shaw.ca with ESMTPA id ybmtk7qPftdldybmukgvpD; Sun, 10 Jan 2021 07:33:49 -0700 X-Authority-Analysis: v=2.4 cv=INe8tijG c=1 sm=1 tr=0 ts=5ffb104d a=7AlCcx2GqMg+lh9P3BclKA==:117 a=7AlCcx2GqMg+lh9P3BclKA==:17 a=xqWC_Br6kY4A:10 a=kj9zAlcOel0A:10 a=EmqxpYm9HcoA:10 a=6I5d2MoRAAAA:8 a=YxBL1-UpAAAA:8 a=EkcXrb_YAAAA:8 a=y212I2TeAYNnVHBa6CIA:9 a=CjuIK1q_8ugA:10 a=IjZwj45LgO3ly-622nXo:22 a=Ia-lj3WSrqcvXOmTRaiG:22 a=LK5xJRSDVpKd5WXXoEvA:22 Received: from slippy.cwsent.com (slippy [IPv6:fc00:1:1:1::5b]) by spqr.komquats.com (Postfix) with ESMTPS id 92AF055D; Sun, 10 Jan 2021 06:33:46 -0800 (PST) Received: from slippy (localhost [127.0.0.1]) by slippy.cwsent.com (8.16.1/8.16.1) with ESMTP id 10AEXkaX075355; Sun, 10 Jan 2021 06:33:46 -0800 (PST) (envelope-from Cy.Schubert@cschubert.com) Message-Id: <202101101433.10AEXkaX075355@slippy.cwsent.com> X-Mailer: exmh version 2.9.0 11/07/2018 with nmh-1.7.1 Reply-to: Cy Schubert From: Cy Schubert X-os: FreeBSD X-Sender: cy@cwsent.com X-URL: http://www.cschubert.com/ To: Vincenzo Maffione cc: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: Re: git: 3d65fd97e85a - main - netmap: iflib: enable/disable krings on any interface reinit In-reply-to: <202101101205.10AC5ZrS084658@gitrepo.freebsd.org> References: <202101101205.10AC5ZrS084658@gitrepo.freebsd.org> Comments: In-reply-to Vincenzo Maffione message dated "Sun, 10 Jan 2021 12:05:35 +0000." Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Date: Sun, 10 Jan 2021 06:33:46 -0800 X-CMAE-Envelope: MS4xfATCwhsFg+5IyMOrac4RcE0ApAmsh5rqHROK47uFR0ANjzd3KpJpO3M5TAqvgADh38GGPXS/Pyi2VCGrDn/WBaSn0ERwimQL3jEkt1YmXB18Uq4YuSDD IP2yezStm9ruyyw6kbLnwntuyd5YDuvj2wlpmZiwYHZujBfMaPO3N8ijxhs9JbNLA8wXfRbyozM17gQdvl6XC9d08VCzdj29aTxIaJOwEopbkMPTPzER3dH7 VYbhilSRD9TcAz4g+VzPmnNk5hnrxIDOrpHddLtUtWCYZkWC1QGOKiiq1839y+0bEt0jJI4i33rwt1eqBtGTBm4k6uC5QrFFpjzcHkWdkDE= X-Rspamd-Queue-Id: 4DDK8b3VQDz4jt8 X-Spamd-Bar: ---- Authentication-Results: mx1.freebsd.org; none X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[] X-BeenThere: dev-commits-src-all@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for all branches of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 10 Jan 2021 14:33:52 -0000 In message <202101101205.10AC5ZrS084658@gitrepo.freebsd.org>, Vincenzo Maffione writes: > The branch main has been updated by vmaffione: > > URL: https://cgit.FreeBSD.org/src/commit/?id=3d65fd97e85ab807f3baa62aa979ae52 > 7914a3ea > > commit 3d65fd97e85ab807f3baa62aa979ae527914a3ea > Author: Vincenzo Maffione > AuthorDate: 2021-01-10 12:00:30 +0000 > Commit: Vincenzo Maffione > CommitDate: 2021-01-10 12:04:08 +0000 > > netmap: iflib: enable/disable krings on any interface reinit > > Since 1d238b07d5d4d9660ae0e0, krings are disabled before > a reinit cycle triggered by iflib_netmap_register. > However, this operation is actually necessary also for > any interface reinit triggered by other causes (i.e., > ifconfig commands). > We achieve this goal by moving the krings enable/disable > operation inside iflib_stop() and iflib_init_locked(). > > Once here, this change also removes some redundant operations > from iflib_netmap_register(), that are already performed by > iflib_stop(). > > PR: 252453 > MFC after: 1 week > --- > sys/net/iflib.c | 35 ++++++++++++++++++++--------------- > 1 file changed, 20 insertions(+), 15 deletions(-) > > diff --git a/sys/net/iflib.c b/sys/net/iflib.c > index cf02b1574c10..3de80ecaeb0c 100644 > --- a/sys/net/iflib.c > +++ b/sys/net/iflib.c > @@ -801,27 +801,18 @@ iflib_netmap_register(struct netmap_adapter *na, int on > off) > int status; > > CTX_LOCK(ctx); > - IFDI_INTR_DISABLE(ctx); > - > - /* Tell the stack that the interface is no longer active */ > - ifp->if_drv_flags &= ~(IFF_DRV_RUNNING | IFF_DRV_OACTIVE); > - > if (!CTX_IS_VF(ctx)) > IFDI_CRCSTRIP_SET(ctx, onoff, iflib_crcstrip); > > - /* > - * Stop any pending txsync/rxsync and prevent new ones > - * form starting. Processes blocked in poll() will get > - * POLLERR. > - */ > - netmap_disable_all_rings(ifp); > - > iflib_stop(ctx); > > /* > * Enable (or disable) netmap flags, and intercept (or restore) > * ifp->if_transmit. This is done once the device has been stopped > - * to prevent race conditions. > + * to prevent race conditions. Also, this must be done after > + * calling netmap_disable_all_rings() and before calling > + * netmap_enable_all_rings(), so that these two functions see the > + * updated state of the NAF_NETMAP_ON bit. > */ > if (onoff) { > nm_set_native_flags(na); > @@ -835,8 +826,6 @@ iflib_netmap_register(struct netmap_adapter *na, int onof > f) > if (status) > nm_clear_native_flags(na); > CTX_UNLOCK(ctx); > - /* Re-enable txsync/rxsync. */ > - netmap_enable_all_rings(ifp); > return (status); > } > > @@ -2388,6 +2377,12 @@ iflib_init_locked(if_ctx_t ctx) > if_setdrvflagbits(ifp, IFF_DRV_OACTIVE, IFF_DRV_RUNNING); > IFDI_INTR_DISABLE(ctx); > > + /* > + * See iflib_stop(). Useful in case iflib_init_locked() is > + * called without first calling iflib_stop(). > + */ > + netmap_disable_all_rings(ifp); > + > tx_ip_csum_flags = scctx->isc_tx_csum_flags & (CSUM_IP | CSUM_TCP | CSU > M_UDP | CSUM_SCTP); > tx_ip6_csum_flags = scctx->isc_tx_csum_flags & (CSUM_IP6_TCP | CSUM_IP6 > _UDP | CSUM_IP6_SCTP); > /* Set hardware offload abilities */ > @@ -2444,6 +2439,9 @@ done: > for (i = 0; i < sctx->isc_ntxqsets; i++, txq++) > callout_reset_on(&txq->ift_timer, iflib_timer_default, iflib_ti > mer, txq, > txq->ift_timer.c_cpu); > + > + /* Re-enable txsync/rxsync. */ > + netmap_enable_all_rings(ifp); > } > > static int > @@ -2489,6 +2487,13 @@ iflib_stop(if_ctx_t ctx) > IFDI_STOP(ctx); > DELAY(1000); > > + /* > + * Stop any pending txsync/rxsync and prevent new ones > + * form starting. Processes blocked in poll() will get > + * POLLERR. > + */ > + netmap_disable_all_rings(ctx->ifc_ifp); > + > iflib_debug_reset(); > /* Wait for current tx queue users to exit to disarm watchdog timer. */ > for (i = 0; i < scctx->isc_ntxqsets; i++, txq++) { > This commit makes a couple of assumptions 1. It's missing definition in a header for netmap_disable_all_rings() and netmap_enable_all_rings(). 2. It assumes that all kernels include device, that it is not loaded as a kld. The first assumption causes a build failure. The second assumption might require at the very least an UPDATING entry to warn people not to remove netmap from their kernels or rely on the module. And probably needs a __FreeBSD_version bump. -- Cheers, Cy Schubert FreeBSD UNIX: Web: https://FreeBSD.org NTP: Web: https://nwtime.org The need of the many outweighs the greed of the few.