Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751867AbbLBGBW (ORCPT ); Wed, 2 Dec 2015 01:01:22 -0500 Received: from mail-wm0-f41.google.com ([74.125.82.41]:37910 "EHLO mail-wm0-f41.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751177AbbLBGBT (ORCPT ); Wed, 2 Dec 2015 01:01:19 -0500 MIME-Version: 1.0 In-Reply-To: <20151201.221356.2176806670215219133.davem@davemloft.net> References: <1448921155-24764-1-git-send-email-ddecotig@gmail.com> <1448921155-24764-4-git-send-email-ddecotig@gmail.com> <20151201.221356.2176806670215219133.davem@davemloft.net> From: David Decotigny Date: Tue, 1 Dec 2015 22:00:58 -0800 X-Google-Sender-Auth: zjLG18HoZ7GeyNPQrX5_Ds8cRXI Message-ID: Subject: Re: [PATCH net-next v3 03/17] net: ethtool: add new ETHTOOL_GSETTINGS/SSETTINGS API To: David Miller Cc: Ben Hutchings , Amir Vadai , "linux-kernel@vger.kernel.org" , "netdev@vger.kernel.org" , "linux-api@vger.kernel.org" , linux-mips@linux-mips.org, fcoe-devel@open-fcoe.org, Eric Dumazet , Eugenia Emantayev , Or Gerlitz , Ido Shamay , Joe Perches , Saeed Mahameed , Govindarajulu Varadarajan <_govind@gmx.com>, Venkata Duvvuru , Jeff Kirsher , Eyal Perry , Pravin B Shelar , Ed Swierk , robert.w.love@intel.com, "James E.J. Bottomley" , Yuval.Mintz@qlogic.com Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2456 Lines: 50 Hello, There is a set of conversion routines ulong[]<->u32[] to address this 32/64-bit compat issue. Using a u32-based bitmap would require drivers to handle the u32 bitmaps themselves, this might be confusing, considering there is a standard bitmap api; and might be error-prone as well. Plus there is %*pb[l] format that's very helpful for debugging. That's why I preferred to handle the relative complexity of u32 bitmaps with the CPP conditionals in the non-driver code that handles the user/kernel interactions, and drivers can use the standard bitmap api transparently. I was currently moving/rewriting the u32/ulong conversion code to bitmap.{c,h} as Ben Hutchings was suggesting, which should hopefully make the code more digestible; and could possibly be used for other user/kernel interfaces. How about I send an updated version with this solution, and if it's still not right, I'll revisit with either u32[] everywhere or fixed-size bitmap instead of variable-size as here? Or maybe another option would be to implement a new u32[] bitmap_u32.{c,h} api, possibly using a set of macro tricks to share code with bitmap.{c,h}? On Tue, Dec 1, 2015 at 7:13 PM, David Miller wrote: > From: David Decotigny > Date: Mon, 30 Nov 2015 14:05:41 -0800 > >> This patch defines a new ETHTOOL_GSETTINGS/SSETTINGS API, handled by >> the new get_ksettings/set_ksettings callbacks. This API provides >> support for most legacy ethtool_cmd fields, adds support for larger >> link mode masks (up to 4064 bits, variable length), and removes >> ethtool_cmd deprecated fields (transceiver/maxrxpkt/maxtxpkt). > > Please do not define the mask using a non-fixed type. I know it makes > it easier to use the various bitmap helper routines if you use 'long', > but here it is clearly superior to use "u32" for the bitmap type and > do the bit operations by hand if necessary. > > Otherwise you have to have all of this ulong size CPP conditional code > which is incredibly ugly. > > Furthermore you have to use fixed sized types anyways so that we don't > need compat code to deal with 32-bit userspace applications making > these ethtool calls into a 64-bit kernel. > > THanks. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/