From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 0FC28224B13 for ; Mon, 21 Apr 2025 22:28:37 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1745274517; cv=none; b=Vo0Bj+mF9++nrLbko0kEJ07jI61o36QK17gtzTyV4NfSU8tb1V10nazKkGguzhPZhKv5vefxmloTeGLicfqxg2YjflOQiOaRgV591qBzsyAWeet0+TSCRmRFsx+/ai6Xu02aNyiBb5JjHtbn2SaAMf70Sb/q9yGch56NgJkPQ1s= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1745274517; c=relaxed/simple; bh=LLmnmcDvnuCGomwuFUePu5WdsV9eqZgEgH0cxjoPcDQ=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=O34LTwpmVtaJLOd26soo5pIOAzdPGUb+jmePBSIOLO8OtYIzJtPbICsJ9TqAAa/xaeCUdUHF53oAYzhPQmyuCcZe8vMcr0Sf+JxlvaLLVsBtHxkQ8erxedcMT6oMj7ZvjG7uMEMqw6cp8Kqv8VQJy7/7Hck4li4EXVD/ZRW5ZTc= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=DDIHj82H; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="DDIHj82H" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 73F05C4AF09; Mon, 21 Apr 2025 22:28:36 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1745274516; bh=LLmnmcDvnuCGomwuFUePu5WdsV9eqZgEgH0cxjoPcDQ=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=DDIHj82HagbUqUO+loHgX9LA+Gqm3gjM2sAItwJFTuq0niz2YBqc75CGsi/AJunZq sfTOvj/NOwlbnsvUXtjxIZFBy4KYDrX1DTtv7b1fkvrClw0byLNZvYLIgX76CtJDog Ah66jrMFLLoomJQzv33141IKbWtzUyU8mHMta030/NHcphNhp6taQswVlAzxlmdlzv vtv9S4GylGj9cAP6MYH8u/vny2S5vE+QOxjmuGFjIi5adpjt8lOv+hTyLHw+KfPx/h AnT0qdGe0sCdoqZgXYXomR5g7vZDsl6IaYFgXxkn2d1w6ggleEIkB3qLvXNNzAOcwD kjIB82n1ZZ/zw== From: Jakub Kicinski To: davem@davemloft.net Cc: netdev@vger.kernel.org, edumazet@google.com, pabeni@redhat.com, andrew+netdev@lunn.ch, horms@kernel.org, donald.hunter@gmail.com, sdf@fomichev.me, almasrymina@google.com, dw@davidwei.uk, asml.silence@gmail.com, ap420073@gmail.com, jdamato@fastly.com, dtatulea@nvidia.com, michael.chan@broadcom.com, Jakub Kicinski Subject: [RFC net-next 04/22] net: clarify the meaning of netdev_config members Date: Mon, 21 Apr 2025 15:28:09 -0700 Message-ID: <20250421222827.283737-5-kuba@kernel.org> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250421222827.283737-1-kuba@kernel.org> References: <20250421222827.283737-1-kuba@kernel.org> Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit hds_thresh and hds_config are both inside struct netdev_config but have quite different semantics. hds_config is the user config with ternary semantics (on/off/unset). hds_thresh is a straight up value, populated by the driver at init and only modified by user space. We don't expect the drivers to have to pick a special hds_thresh value based on other configuration. The two approaches have different advantages and downsides. hds_thresh ("direct value") gives core easy access to current device settings, but there's no way to express whether the value comes from the user. It also requires the initialization by the driver. hds_config ("user config values") tells us what user wanted, but doesn't give us the current value in the core. Try to explain this a bit in the comments, so at we make a conscious choice for new values which semantics we expect. Move the init inside ethtool_ringparam_get_cfg() to reflect the semantics. Commit 216a61d33c07 ("net: ethtool: fix ethtool_ringparam_get_cfg() returns a hds_thresh value always as 0.") added the setting for the benefit of netdevsim which doesn't touch the value at all on get. Again, this is just to clarify the intention, shouldn't cause any functional change. Signed-off-by: Jakub Kicinski --- include/net/netdev_queues.h | 19 +++++++++++++++++-- net/ethtool/common.c | 3 ++- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/include/net/netdev_queues.h b/include/net/netdev_queues.h index ea709b59d827..f4eab6fc64f4 100644 --- a/include/net/netdev_queues.h +++ b/include/net/netdev_queues.h @@ -6,11 +6,26 @@ /** * struct netdev_config - queue-related configuration for a netdev - * @hds_thresh: HDS Threshold value. - * @hds_config: HDS value from userspace. */ struct netdev_config { + /* Direct value + * + * Driver default is expected to be fixed, and set in this struct + * at init. From that point on user may change the value. There is + * no explicit way to "unset" / restore driver default. + */ + /** @hds_thresh: HDS Threshold value (ETHTOOL_A_RINGS_HDS_THRESH). + */ u32 hds_thresh; + + /* User config values + * + * Contain user configuration. If "set" driver must obey. + * If "unset" driver is free to decide, and may change its choice + * as other parameters change. + */ + /** @hds_config: HDS enabled (ETHTOOL_A_RINGS_TCP_DATA_SPLIT). + */ u8 hds_config; }; diff --git a/net/ethtool/common.c b/net/ethtool/common.c index 49bea6b45bd5..502392395d45 100644 --- a/net/ethtool/common.c +++ b/net/ethtool/common.c @@ -825,12 +825,13 @@ void ethtool_ringparam_get_cfg(struct net_device *dev, memset(param, 0, sizeof(*param)); memset(kparam, 0, sizeof(*kparam)); + kparam->hds_thresh = dev->cfg->hds_thresh; + param->cmd = ETHTOOL_GRINGPARAM; dev->ethtool_ops->get_ringparam(dev, param, kparam, extack); /* Driver gives us current state, we want to return current config */ kparam->tcp_data_split = dev->cfg->hds_config; - kparam->hds_thresh = dev->cfg->hds_thresh; } static void ethtool_init_tsinfo(struct kernel_ethtool_ts_info *info) -- 2.49.0