Re: git: 8062f37d1c99 - stable/14 - vtnet: set VNET context in RX handler

On Tue, 16 Apr 2024 15:58:50 GMT
Gleb Smirnoff <glebius@FreeBSD.org> wrote:

> The branch stable/14 has been updated by glebius:
> 
> URL:
> https://cgit.FreeBSD.org/src/commit/?id=8062f37d1c99ded250b36ad90fb32bb5f77ee600
...
>     vtnet: set VNET context in RX handler

Two questions:

1) why is vnet not resolved directly via ifp->if_vnet, i.e. why do we
need to go through if_getvnet() here?  Not only is if_getvnet()
unnecessarily making a function call to return a value of a struct
member already at hand, but is doing this on a hot path.  if_getvnet()
arrived in 0d2684e15e415cef652e0f75e88962a674bffe04 / D38202 as an
"accessor" function with a declared intent to help a vendor maintaining
their private out-of-tree drivers independent of changes in struct ifnet
layout.  While this indeed may make sense for certain vendor-specific
use cases, why do we need to take a penalty of calling if_getvnet() in
internal code, here and elsewhere?  if_getvnet() use is starting to get
widespread, and it would be nice if we could hear the reasoning behind
this school of thought, and perhaps document when the call should be
used, and when prefereably not?

2) why is CURVNET_SET_QUIET() used instead of CURVNET_SET()?  Which VNET
recursion are you trying to quiesce with the _QUIET() version?

>     
>     The context is required for NIC-level pfil(9) filtering.
>     
>     (cherry picked from commit
> 3f2b9607756d0f92ca29c844db0718b313a06634) ---
>  sys/dev/virtio/network/if_vtnet.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/sys/dev/virtio/network/if_vtnet.c
> b/sys/dev/virtio/network/if_vtnet.c index 7d6411876b3d..245a6b6d7359
> 100644 --- a/sys/dev/virtio/network/if_vtnet.c
> +++ b/sys/dev/virtio/network/if_vtnet.c
> @@ -2095,6 +2095,7 @@ vtnet_rxq_eof(struct vtnet_rxq *rxq)
>  
>  	VTNET_RXQ_LOCK_ASSERT(rxq);
>  
> +	CURVNET_SET_QUIET(if_getvnet(ifp));
>  	while (count-- > 0) {
>  		struct mbuf *m;
>  		uint32_t len, nbufs, adjsz;
> @@ -2188,6 +2189,7 @@ vtnet_rxq_eof(struct vtnet_rxq *rxq)
>  #endif
>  		virtqueue_notify(vq);
>  	}
> +	CURVNET_RESTORE();
>  
>  	return (count > 0 ? 0 : EAGAIN);
>  }