From nobody Thu Apr 18 14:54:49 2024 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4VL15m2Mjmz5H1wF; Thu, 18 Apr 2024 14:54:52 +0000 (UTC) (envelope-from glebius@freebsd.org) Received: from smtp.freebsd.org (smtp.freebsd.org [IPv6:2610:1c1:1:606c::24b:4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4VL15m1pCsz4f4S; Thu, 18 Apr 2024 14:54:52 +0000 (UTC) (envelope-from glebius@freebsd.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1713452092; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=Ng309tOzMlmuIJZK52IAgF4h4b3Zzc476eaTbMztQMI=; b=tqG5PzfhUT+cljHKVIREYcI8Ojd0fsDbEPaC+/OMuwdqZAG6DgEyqX3vr6isR6i+WltUDb B0UgIiXmsmwJRQaSXeJcFqi3JtQR3HWqKR7hqUdIgXWtjMCYtjU5Oz60mEW0BF6HMlJxEc YNGzB1f+B3ysa/pXIQw/bkLtMlYWZM8RiI9T2EchLibUHrSf1zT8SRkwVXJDmRQvOI1Sia yKOi4XYFMoV6ItuM74uqdP4q9LNQumaf6Q+4nIu60XIj8uZKjk/yTGy3p/TH3rQGnUsxIs LP0fiCyOaBTJdzG/eaL4GHfbIgZCPqL9LhQzDlmqleGY8Zdhx/Sb4M+SMIoDXg== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1713452092; a=rsa-sha256; cv=none; b=upo0jJ4TELZDn9bmNULoa88EVNk3Ig9sPUjddlGsZgXnVE0R7EzOqHm6ht2JQ5ed+f1liF j9tmn3mEIaz5L3HV+SlsVSxskyNJaDh9ekOsxfCf4o1JRESoAY9q69aLE8rYGpOH6TBLHA IBu1JD4hY78a/p/SmTjEPlWYcA93gBbNEXY5f/4bprCMCFhTpk/gQaGnXAGRXt+0tMVvAL Y0xHUPGGqfMt1lGqUGZ7U5V1sgK5WtU8gJt32IwhF4CGmOVAGODi9TxXtFRXgMP8bUlcWL tPzAV14opUnAwvvtlXN/XzrKcWO8Verc4Os0jsw7BE7OzjnpPPiCw1evux1dvg== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1713452092; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=Ng309tOzMlmuIJZK52IAgF4h4b3Zzc476eaTbMztQMI=; b=tFWOSFoFA+U2E9xgvPzcViAsq/dE9jXLZRW148JmTS4kFlhTqbn5gZS98ER6sbSWpg+2Ph GHmS2lEVVCKQELrkG4sxwOWqoRpWdkPgW4qZvVKtps109xihaf+Bt63bnCvi0Bo30rzvY9 5uHAJL1OYeu0rlrzQQs1Lmp8TXGYrvn9mW4H4aCgZ0lR2Y+vUsGUX8Dmr7nv40V92b912T kkR74rHL2Z8D/qqdidgn3Ok/CF1q7UZQP/G9durIpcXvpvufvetCCafux+scI1SPdjXm+b pleVdXp0aM5urTTWuBKOeOqe7115+l4Yp0TbAkgOrk1PC42MS1Y+KT0ZfOf0ow== Received: from cell.glebi.us (glebi.us [162.251.186.162]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) (Authenticated sender: glebius) by smtp.freebsd.org (Postfix) with ESMTPSA id 4VL15l4wHpz1Mfy; Thu, 18 Apr 2024 14:54:51 +0000 (UTC) (envelope-from glebius@freebsd.org) Date: Thu, 18 Apr 2024 07:54:49 -0700 From: Gleb Smirnoff To: Marko Zec Cc: src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-branches@freebsd.org Subject: Re: git: 8062f37d1c99 - stable/14 - vtnet: set VNET context in RX handler Message-ID: References: <202404161558.43GFwoEt095208@gitrepo.freebsd.org> <20240417102252.27e1038d@x25> List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240417102252.27e1038d@x25> Marko, On Wed, Apr 17, 2024 at 10:22:52AM +0200, Marko Zec wrote: M> > https://cgit.FreeBSD.org/src/commit/?id=8062f37d1c99ded250b36ad90fb32bb5f77ee600 M> ... M> > vtnet: set VNET context in RX handler M> M> Two questions: M> M> 1) why is vnet not resolved directly via ifp->if_vnet, i.e. why do we M> need to go through if_getvnet() here? Not only is if_getvnet() M> unnecessarily making a function call to return a value of a struct M> member already at hand, but is doing this on a hot path. if_getvnet() M> arrived in 0d2684e15e415cef652e0f75e88962a674bffe04 / D38202 as an M> "accessor" function with a declared intent to help a vendor maintaining M> their private out-of-tree drivers independent of changes in struct ifnet M> layout. While this indeed may make sense for certain vendor-specific M> use cases, why do we need to take a penalty of calling if_getvnet() in M> internal code, here and elsewhere? if_getvnet() use is starting to get M> widespread, and it would be nice if we could hear the reasoning behind M> this school of thought, and perhaps document when the call should be M> used, and when prefereably not? The intent to have struct ifnet opaque to drivers is already over 10 years old. Initially I had quite a radical branch in subversion, which was not finished. Later Juniper came with less radical changes, where they just hide every field under accessor function and we agreed it is all right as an intermediate step (which potentally can last very long). For vendor maintained drivers it is a clear win, for in house drivers not so clear. If the KPI fully hides ifnet, then resturcturings in sys/net can happen without touching a single driver in the tree. What you suggest is allowing two KPIs to exist in parallel officially - one for in house drivers and the other for vendor maintained. And allow that for an immesurable performance gain. I'd argue it is totally immesurable as we are talking about VirtIO driver - a driver that runs in a virtual machine, where no one can make a reproducible benchmark that would be able to detect such a micro optimization. I'd also argue that if somebody cares about performance at a level of optimizing out function calls, they'd probably be running without VIMAGE already and unlikely running in the virtual environment, as well. M> 2) why is CURVNET_SET_QUIET() used instead of CURVNET_SET()? Which VNET M> recursion are you trying to quiesce with the _QUIET() version? That's a good point, thanks! I will correct this. P.S. I'd really appreciate commits to main being reviewed before merge to stable/14. -- Gleb Smirnoff