From mboxrd@z Thu Jan 1 00:00:00 1970 From: Saeed Mahameed Subject: Re: [PATCH net-next 0/4] mlx5: Create build configuration options Date: Sat, 28 Jan 2017 13:38:21 +0200 Message-ID: References: <20170126233241.2268449-1-tom@herbertland.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Cc: Saeed Mahameed , "David S. Miller" , Linux Netdev List , Kernel Team To: Tom Herbert Return-path: Received: from mail-qt0-f194.google.com ([209.85.216.194]:33359 "EHLO mail-qt0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750891AbdA1Lim (ORCPT ); Sat, 28 Jan 2017 06:38:42 -0500 Received: by mail-qt0-f194.google.com with SMTP id n13so50191985qtc.0 for ; Sat, 28 Jan 2017 03:38:42 -0800 (PST) In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: On Fri, Jan 27, 2017 at 8:13 PM, Tom Herbert wrote: > On Fri, Jan 27, 2017 at 9:58 AM, Saeed Mahameed > wrote: >> On Fri, Jan 27, 2017 at 1:32 AM, Tom Herbert wrote: >>> This patchset creates configuration options for sriov, vxlan, eswitch, >>> and tc features in the mlx5 driver. The purpose of this is to allow not >>> building these features. These features are optional advanced features >>> that are not required for a core Ethernet driver. A user can disable >>> these features which resuces the amount of code in the driver. Disabling >>> these features (and DCB) reduces the size of mlx5_core.o by about 16%. >>> This is also can reduce the complexity of backport and rebases since >>> user would no longer need to worry about dependencies with the rest of >>> the kernel that features which might not be of any interest to a user >>> may bring in. >>> >>> Tested: Build and ran the driver with all features enabled (the default) >>> and with none enabled (including DCB). Did not see any issues. I did >>> not explicity test operation of ayy of features in the list. >>> >> >> Basically I am not against this kind of change, infact i am with it, >> although I would have done some restructuring in the driver before i >> did such change ;), filling the code with ifdefs is not a neat thing. >> > If you wish, please take this as an RFC and feel free to structure the > code the right way. I think the intent is clear enough and looks like > davem isn't going to allow the directory restructuring so something > like this seems to be the best course of action now. > Right. >> I agree this will simplify backporting and provide some kind of >> feature separation inside the driver. >> But this will also increase the testing matrix we need to cover and >> increase the likelihood of kbuild breaks by an order of magnitude. >> > The testing matrix already exploded with the proliferation of > supported features. If anything this reduces the test matrix problem. > For instance, if we make a change to the core driver and functionality > properly isolated there is a much better chance that this won't affect > peripheral functionality and vice versa. It is just not feasible for > us to test every combination of NIC features for every change being > made. > Yes for isolated features, but for base functionality, we need to test it with all new device specific kconfig combinations on every patch! since a misplaced code inside or outside the correct ifdef can easily go unnoticed and break functionality. >> One more thing, do we really need a device specific flag per feature >> per vendor per device? can't we just use the same kconfig flag for >> all drivers and if there is a more generic system wide flag that >> covers the same feature >> can't we just use it, for instance instead of >> CONFIG__SRIOV why not use already existing CONFIG_PCI_IOV >> for all drivers ? >> > That sounds good to me. We already have CONFIG_RFS_ACCEL and others > that do that. > > Tom > >> Saeed. >> >>> >>> >>> Tom Herbert (4): >>> mlx5: Make building eswitch configurable >>> mlx5: Make building SR-IOV configurable >>> mlx5: Make building tc hardware offload configurable >>> mlx5: Make building vxlan hardware offload configurable >>> >>> drivers/net/ethernet/mellanox/mlx5/core/Kconfig | 35 ++++++ >>> drivers/net/ethernet/mellanox/mlx5/core/Makefile | 16 ++- >>> drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 129 ++++++++++++++++------ >>> drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 39 +++++-- >>> drivers/net/ethernet/mellanox/mlx5/core/eq.c | 4 +- >>> drivers/net/ethernet/mellanox/mlx5/core/lag.c | 2 + >>> drivers/net/ethernet/mellanox/mlx5/core/main.c | 32 ++++-- >>> drivers/net/ethernet/mellanox/mlx5/core/sriov.c | 6 +- >>> 8 files changed, 205 insertions(+), 58 deletions(-) >>> >>> -- >>> 2.9.3 >>>