From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jiri Pirko Subject: Re: [RFC PATCH net-next v2 06/17] ethtool: support for netlink notifications Date: Tue, 31 Jul 2018 08:46:00 +0200 Message-ID: <20180731064600.GC2154@nanopsycho> References: <20180730131655.GA10626@nanopsycho> <20180730170121.e6k5slbhdamclnt6@unicorn.suse.cz> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org, David Miller , Florian Fainelli , Roopa Prabhu , Jakub Kicinski , "John W. Linville" To: Michal Kubecek Return-path: Content-Disposition: inline In-Reply-To: <20180730170121.e6k5slbhdamclnt6@unicorn.suse.cz> Sender: linux-kernel-owner@vger.kernel.org List-Id: netdev.vger.kernel.org Mon, Jul 30, 2018 at 07:01:21PM CEST, mkubecek@suse.cz wrote: >On Mon, Jul 30, 2018 at 03:16:55PM +0200, Jiri Pirko wrote: >> Mon, Jul 30, 2018 at 02:53:12PM CEST, mkubecek@suse.cz wrote: >> >> [...] >> >> >diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h >> >index c1295c7a452e..c4b0c575d57e 100644 >> >--- a/include/linux/netdevice.h >> >+++ b/include/linux/netdevice.h >> >@@ -2444,6 +2444,7 @@ enum netdev_cmd { >> > NETDEV_CVLAN_FILTER_DROP_INFO, >> > NETDEV_SVLAN_FILTER_PUSH_INFO, >> > NETDEV_SVLAN_FILTER_DROP_INFO, >> >+ NETDEV_ETHTOOL, >> >> I don't understand why this goes through netdev notifier. What's the >> reason? > >To allow triggering a notification from other code (ethtool ioctl or >e.g. netdev_features_change()) when netlink interface is built as a >module. If it's a (performance?) problem, an alternative could be having >a global pointer which would be either null or point to ethtool_notify() >depending on whether the module is loaded (and ready). > >(Which made me realize I forgot to handle a race between module >unloading and processing a notification.) > >Another question is if we really need the option to build the netlink >interface as a module. I must admit my main motivation to have it as >a module is that it makes testing and debugging easier. Yeah. It is very core thing. I think it does not have to be a module.