From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.1 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 183BEC3F2D1 for ; Wed, 4 Mar 2020 18:27:13 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id E74822465D for ; Wed, 4 Mar 2020 18:27:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1583346433; bh=Q2id61DMuYuvh7JXreQCqLBb0f43InezHiFkD8TMoAY=; h=Date:From:To:Cc:Subject:In-Reply-To:References:List-ID:From; b=YRU3MdTSr0ZHWkrGBxWxhxa+nzbTrWU/JGma1p4SY00Nmv+1poX5Nr2YlulNtrL8c 9ms3rHNdwFZ61MSrOHXbihEWBhFZ5wvT2IiYzWNUSYhm5rA10cvnNTstxhjEblX2L3 X+Nef2C0gqGFpGjgqRR0p3P3gb++y2sre+awFOis= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730097AbgCDS1J (ORCPT ); Wed, 4 Mar 2020 13:27:09 -0500 Received: from mail.kernel.org ([198.145.29.99]:55640 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726748AbgCDS1J (ORCPT ); Wed, 4 Mar 2020 13:27:09 -0500 Received: from kicinski-fedora-PC1C0HJN (unknown [163.114.132.128]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 38E2E21775; Wed, 4 Mar 2020 18:27:07 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1583346428; bh=Q2id61DMuYuvh7JXreQCqLBb0f43InezHiFkD8TMoAY=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=dbBqxkucS5Dk0e0sG8GdYhDSnpW2e5jvb9zkWwuOXEepLHUOMPJBw8Eu3A27SSMXG FSLa8Wv0OEoqf7Sb4+7XROSlmnVPbGYxYhkPoYSwM/u8mmm1b8oWp/ci3XlG06nzpR EvUsdDGxOaXxXh7TQQEgCWTUBtZdJm3ObOXQBWN0= Date: Wed, 4 Mar 2020 10:27:05 -0800 From: Jakub Kicinski To: Alexander Duyck Cc: Michal Kubecek , davem@davemloft.net, thomas.lendacky@amd.com, benve@cisco.com, _govind@gmx.com, pkaustub@cisco.com, peppe.cavallaro@st.com, alexandre.torgue@st.com, joabreu@synopsys.com, snelson@pensando.io, yisen.zhuang@huawei.com, salil.mehta@huawei.com, jeffrey.t.kirsher@intel.com, jacob.e.keller@intel.com, michael.chan@broadcom.com, saeedm@mellanox.com, leon@kernel.org, netdev@vger.kernel.org Subject: Re: [PATCH net-next v2 01/12] ethtool: add infrastructure for centralized checking of coalescing parameters Message-ID: <20200304102705.192d3b0a@kicinski-fedora-PC1C0HJN> In-Reply-To: <45b3c493c3ce4aa79f882a8170f3420d348bb61e.camel@linux.intel.com> References: <20200304043354.716290-1-kuba@kernel.org> <20200304043354.716290-2-kuba@kernel.org> <20200304075926.GH4264@unicorn.suse.cz> <20200304100050.14a95c36@kicinski-fedora-PC1C0HJN> <45b3c493c3ce4aa79f882a8170f3420d348bb61e.camel@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Wed, 04 Mar 2020 10:12:30 -0800 Alexander Duyck wrote: > On Wed, 2020-03-04 at 10:00 -0800, Jakub Kicinski wrote: > > On Wed, 4 Mar 2020 08:59:26 +0100 Michal Kubecek wrote: =20 > > > Just an idea: perhaps we could use the fact that struct ethtool_coale= sce > > > is de facto an array so that this block could be replaced by a loop l= ike > > >=20 > > > u32 supported_types =3D dev->ethtool_ops->coalesce_types; > > > const u32 *values =3D &coalesce->rx_coalesce_usecs; > > >=20 > > > for (i =3D 0; i < __ETHTOOL_COALESCE_COUNT; i++) > > > if (values[i] && !(supported_types & BIT(i))) > > > return false; > > >=20 > > > and to be sure, BUILD_BUG_ON() or static_assert() check that the offs= et > > > of ->rate_sample_interval matches ETHTOOL_COALESCE_RATE_SAMPLE_INTERV= AL. =20 > >=20 > > I kind of prefer the greppability over the saved 40 lines :( > > But I'm happy to change if we get more votes for the more concise > > version. Or perhaps the Intel version with the warnings printed. =20 >=20 > I agree that it would make more sense to replace the types with an enum > definition, and then use the enum to define bits to be used by the > drivers. The only use for the enum would then be to automate the bit assignment? Sounds like we would save some lines for the code and added some for the definition. Maybe I'm missing the advantage the enum brings =F0=9F=A4=94 > > > > + return !dev->ethtool_ops->coalesce_types || > > > > + (dev->ethtool_ops->coalesce_types & used_types) =3D=3D used_type= s; > > > > +} =20 > > >=20 > > > I suggest to move the check for !dev->ethtool_ops->coalesce_types to = the > > > beginning of the function so that we avoid calculating the bitmap if = we > > > are not going to check it anyway. =20 > >=20 > > Good point! =20 >=20 > So one thing I just wanted to point out. The used_types won't necessarily > be correct because it is only actually checking for non-zero types. There > are some of these values where a zero is a valid input and the driver will > accept it, such as rx_coalesce_usecs for ixgbe. As such we might want to > rename the value to nonzero_types instead of used_types. Okay, I'll rename. I was also wondering if it should be "params" not "types". Initially I was hoping there are categories of coalescing that drivers implement, each with set of params. But it seems each vendor is just picking fields they like. I think I'll do s/types/params/ as well.