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=-2.5 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_PASS,URIBL_BLOCKED,USER_AGENT_MUTT autolearn=ham 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 7B837C43381 for ; Thu, 21 Feb 2019 07:17:27 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 4E47E20838 for ; Thu, 21 Feb 2019 07:17:27 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726639AbfBUHR0 (ORCPT ); Thu, 21 Feb 2019 02:17:26 -0500 Received: from mx2.suse.de ([195.135.220.15]:33810 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1725932AbfBUHR0 (ORCPT ); Thu, 21 Feb 2019 02:17:26 -0500 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id D7518AF15; Thu, 21 Feb 2019 07:17:24 +0000 (UTC) Received: by unicorn.suse.cz (Postfix, from userid 1000) id 4B30EE00A5; Thu, 21 Feb 2019 08:17:23 +0100 (CET) Date: Thu, 21 Feb 2019 08:17:23 +0100 From: Michal Kubecek To: Florian Fainelli Cc: Jakub Kicinski , Jiri Pirko , davem@davemloft.net, netdev@vger.kernel.org, oss-drivers@netronome.com, andrew@lunn.ch Subject: Re: [PATCH net-next 3/3] nfp: devlink: allow flashing the device via devlink Message-ID: <20190221071723.GL23151@unicorn.suse.cz> References: <20190214214046.19182-1-jakub.kicinski@netronome.com> <20190214214046.19182-4-jakub.kicinski@netronome.com> <20190215101514.GD2343@nanopsycho> <20190215074429.6890e47e@cakuba.netronome.com> <20190219091942.GA3080@nanopsycho> <20190219164926.23981359@cakuba.netronome.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Wed, Feb 20, 2019 at 06:59:05PM -0800, Florian Fainelli wrote: > On 2/19/2019 4:49 PM, Jakub Kicinski wrote: > > On Tue, 19 Feb 2019 10:19:42 +0100, Jiri Pirko wrote: > >> Fri, Feb 15, 2019 at 04:44:29PM CET, jakub.kicinski@netronome.com wrote: > >>> On Fri, 15 Feb 2019 11:15:14 +0100, Jiri Pirko wrote: > >>>>> static const struct ethtool_ops nfp_net_ethtool_ops = { > >>>> > >>>> Why don't you use the compat fallback? I think you should. > >>> > >>> You and Michal both asked the same so let me answer the first to ask :) > >>> - if devlink is built as a module the fallback is not reachable. > >> > >> So the fallback is not really good as you can't use it for real drivers > >> anyway. Odd. Maybe we should compile devlink in without possibility to > >> have it as module. > > > > Ack, I'll make devlink a bool. > > Meh how about those poor and memory constrained embedded systems? > Ideally ethtool should/could have been modular as well, but that ship > has now sailed. I would certainly like to make the ioctl interface optional once we reach the end of "phase one", i.e. make ioctl-less ethtool possible. Looking at the code, I don't see an obvious reason why it couldn't be modular. There seem to be only few functions in net/core/ethtool.c which are called from outside and all seem to be simple helpers not really tied to the rest of the code, except for netdev_rss_key variable (needed for /proc/sys/net/core/netdev_rss_key). Some of them could even be inline. We could always put these into some net/ethtool/stub.c (ethtool-stub.c) and build unconditionally. So if keeping the option to have devlink (and ethtool) as a module is really desirable, I believe it can be done even now (unless I missed something important). > We have had similar issues with PHYLIB before where we wanted > net/core/ethtool.c to be able to call into generic PHYLIB functions to > obtain PHY statistics, an inline helper that de-references the PHY > device's driver function pointers solved that (look for > phy_ethtool_get_{strings,sset,stats}) while letting PHYLIB remain modular. There is also something similar in netfilter - nf_ct_hook, nfnl_ct_hook or nf_ipv6_ops. > devlink_compat_flash_update() is a bit big to be inlined, but why not? Most of it seems to be the lookup which I'm planning to factor out as a separate helper. But that would also need to be available to external code, of course. Michal