git: 3d65fd97e85a - main - netmap: iflib: enable/disable krings on any interface reinit

[Cy Schubert]

In message <202101101433.10AEXkaX075355 at slippy.cwsent.com>, Cy Schubert 
writes:
> In message <202101101205.10AC5ZrS084658 at gitrepo.freebsd.org>, Vincenzo 
> Maffione
>  writes:
> > The branch main has been updated by vmaffione:
> >
> > URL: https://cgit.FreeBSD.org/src/commit/?id=3d65fd97e85ab807f3baa62aa979ae
> 52
> > 7914a3ea
> >
> > commit 3d65fd97e85ab807f3baa62aa979ae527914a3ea
> > Author:     Vincenzo Maffione <vmaffione at FreeBSD.org>
> > AuthorDate: 2021-01-10 12:00:30 +0000
> > Commit:     Vincenzo Maffione <vmaffione at FreeBSD.org>
> > 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 on
> of
> > 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.

Nevermnd. I see what's going on.


-- 
Cheers,
Cy Schubert <Cy.Schubert at cschubert.com>
FreeBSD UNIX:  <cy at FreeBSD.org>   Web:  https://FreeBSD.org
NTP:           <cy at nwtime.org>    Web:  https://nwtime.org

	The need of the many outweighs the greed of the few.