Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Sat, 14 Nov 2020 10:54:23 -0800
From:   Jakub Kicinski <kuba@kernel.org>
To:     Huazhong Tan <tanhuazhong@huawei.com>,
        Michal Kubecek <mkubecek@suse.cz>,
        Andrew Lunn <andrew@lunn.ch>, Jiri Pirko <jiri@resnulli.us>
Cc:     <davem@davemloft.net>, <netdev@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <salil.mehta@huawei.com>,
        <yisen.zhuang@huawei.com>, <linuxarm@huawei.com>
Subject: Re: [PATCH V3 net-next 06/10] net: hns3: add ethtool priv-flag for
 DIM
Message-ID: <20201114105423.07c2ce67@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
In-Reply-To: <1605151998-12633-7-git-send-email-tanhuazhong@huawei.com>
References: <1605151998-12633-1-git-send-email-tanhuazhong@huawei.com>
        <1605151998-12633-7-git-send-email-tanhuazhong@huawei.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Thu, 12 Nov 2020 11:33:14 +0800 Huazhong Tan wrote:
> Add a control private flag in ethtool for enable/disable
> DIM feature.
> 
> Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com>

Please work on a common ethtool API for the configuration instead of
using private flags.

Private flags were overused because the old IOCTL-based ethtool was
hard to extend, but we have a netlink API now.

For example here you're making a choice between device and DIM
implementation of IRQ coalescing. You can add a new netlink attribute
to the ETHTOOL_MSG_COALESCE_GET/ETHTOOL_MSG_COALESCE_SET commands which
controls the type of adaptive coalescing (if enabled).


One question I don't think we have a strong answer for is how to handle
this extension from ethtool_ops point of view. Should we add a new
"extended" op which drivers may start implementing? Or separate the
structure passed in to the ops from the one used as uAPI?

Thoughts anyone?


Huazhong Tan, since the DIM and EQ/CQ patches may require more
infrastructure work feel free to repost the first 4 patches separately,
I can apply those as is.